Re: [PATCH v2] ata: libata-acpi: Do not assume 40 wire cable if no devices are enabled

[Damien Le Moal <dlemoal@kernel.org>]

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