On 2025/05/19 10:56, Tasos Sahanidis wrote:
> On at least an ASRock 990FX Extreme 4 with a VIA VT6330, the devices
> have not yet been enabled by the first time ata_acpi_cbl_80wire() is
> called. This means that the ata_for_each_dev loop is never entered,
> and a 40 wire cable is assumed.
>
> The VIA controller on this board does not report the cable in the PCI
> config space, thus having to fall back to ACPI even though no SATA
> bridge is present.
>
> The _GTM values are correctly reported by the firmware through ACPI,
> which has already set up faster transfer modes, but due to the above
> the controller is forced down to a maximum of UDMA/33.
>
> Resolve this by modifying ata_acpi_cbl_80wire() to directly return the
> cable type. First, an unknown cable is assumed which preserves the mode
> set by the firmware, and then on subsequent calls when the devices have
> been enabled, an 80 wire cable is correctly detected.
>
> Since the function now directly returns the cable type, it has been
> renamed to ata_acpi_cbl_pata_type().
Nit: "it has been renamed" -> "it is renamed"
> @@ -530,13 +534,17 @@ int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm)
> xfer_mask = ata_acpi_gtm_xfermask(dev, gtm);
> ata_unpack_xfermask(xfer_mask, NULL, NULL, &udma_mask);
>
> - if (udma_mask & ~ATA_UDMA_MASK_40C)
> - return 1;
> + ret = ATA_CBL_PATA40;
> +
> + if (udma_mask & ~ATA_UDMA_MASK_40C) {
> + ret = ATA_CBL_PATA80;
Please change this to "return ATA_CBL_PATA80;" and change the last return at the
end of the function to "return ATA_CBL_PATA40;". That will be cleaner.
Other than these, this looks good.
--
Damien Le Moal
Western Digital Research
