On 5/14/25 20:29, 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 returning EAGAIN in ata_acpi_cbl_80wire() if no devices
> have been detected and modify pata_via to handle this scenario.
>
> 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.
>
> Signed-off-by: Tasos Sahanidis <tasos@xxxxxxxxxxxx>
> ---
> drivers/ata/libata-acpi.c | 11 ++++++++---
> drivers/ata/pata_via.c | 13 ++++++++++---
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> index b7f0bf795521..c508a19c2495 100644
> --- a/drivers/ata/libata-acpi.c
> +++ b/drivers/ata/libata-acpi.c
> @@ -523,6 +523,7 @@ EXPORT_SYMBOL_GPL(ata_acpi_gtm_xfermask);
> int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm)
> {
> struct ata_device *dev;
> + int ret = -EAGAIN;
See below, but adding:
gtm = ata_acpi_init_gtm(ap);
if (!gtm)
return ATA_CBL_PATA40;
would be an additional nice cleanup.
>
> ata_for_each_dev(dev, &ap->link, ENABLED) {
> unsigned int xfer_mask, udma_mask;
> @@ -530,11 +531,15 @@ 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 = 0;
> +
> + if (udma_mask & ~ATA_UDMA_MASK_40C) {
> + ret = 1;
> + break;
> + }
> }
>
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
>
> diff --git a/drivers/ata/pata_via.c b/drivers/ata/pata_via.c
> index 696b99720dcb..4d03b4a1ea4d 100644
> --- a/drivers/ata/pata_via.c
> +++ b/drivers/ata/pata_via.c
> @@ -182,6 +182,7 @@ static int via_cable_detect(struct ata_port *ap) {
> const struct via_isa_bridge *config = ap->host->private_data;
> struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> u32 ata66;
> + const struct ata_acpi_gtm *gtm;
>
> if (via_cable_override(pdev))
> return ATA_CBL_PATA40_SHORT;
> @@ -202,9 +203,15 @@ static int via_cable_detect(struct ata_port *ap) {
> if (ata66 & (0x10100000 >> (16 * ap->port_no)))
> return ATA_CBL_PATA80;
> /* Check with ACPI so we can spot BIOS reported SATA bridges */
> - if (ata_acpi_init_gtm(ap) &&
> - ata_acpi_cbl_80wire(ap, ata_acpi_init_gtm(ap)))
> - return ATA_CBL_PATA80;
> + gtm = ata_acpi_init_gtm(ap);
> + if (gtm) {
> + int cbl = ata_acpi_cbl_80wire(ap, gtm);
While at it, maybe change ata_acpi_cbl_80wire() returns ATA_CBL_PATA_UNK,
ATA_UDMA_MASK_40C or ATA_CBL_PATA80, and rename that function to something like:
ata_acpi_cbl_pata_type()
The call to ata_acpi_init_gtm() can also go into that function, thus removing
the gtm argument.
With that, this big ugly hunk would become:
return ata_acpi_cbl_pata_type(ap);
> +
> + if (cbl < 0)
> + return ATA_CBL_PATA_UNK;
> + else if (cbl == 1)
> + return ATA_CBL_PATA80;
> + }
> return ATA_CBL_PATA40;
> }
>
--
Damien Le Moal
Western Digital Research