Re: [PATCH v3] libata/ahci: Fix PCS quirk application for suspend

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 12/9/22 00:25, Adam Tukaj wrote:
> Since kernel 5.3.4 my laptop (ICH8M controller) does not see Kingston
> SV300S37A60G SSD disk connected into a SATA connector on wake from suspend.
> The problem was introduced in c312ef176399 "libata/ahci: Drop PCS quirk for
> Denverton and beyond": the quirk is not applied on wake from suspend as it
> originally was.
> 
> It is worth to mention the commit contained another bug: the quirk is not
> applied at all to controllers which require it. The fix 09d6ac8dc51a
> "libata/ahci: Fix PCS quirk application" landed in 5.3.8. So testing my
> patch anywhere between c312ef176399 and 09d6ac8dc51a is pointless.
> 
> Not all disks trigger the problem. For example nothing bad happens with
> Western Digital WD5000LPCX HDD.
> 
> Test hardware:
> - Acer 5920G with ICH8M SATA controller
> - sda: some SATA HDD connnected into the DVD drive IDE port with a SATA-IDE
> caddy. It is a boot disk
> - sdb: Kingston SV300S37A60G SSD connected into the only SATA port
> 
> Sample "dmesg --notime | grep -E '^(sd |ata)'" output on wake:
> 
> sd 0:0:0:0: [sda] Starting disk
> sd 2:0:0:0: [sdb] Starting disk
> ata4: SATA link down (SStatus 4 SControl 300)
> ata3: SATA link down (SStatus 4 SControl 300)
> ata1.00: ACPI cmd ef/03:0c:00:00:00:a0 (SET FEATURES) filtered out
> ata1.00: ACPI cmd ef/03:42:00:00:00:a0 (SET FEATURES) filtered out
> ata1: FORCE: cable set to 80c
> ata5: SATA link down (SStatus 0 SControl 300)
> ata3: SATA link down (SStatus 4 SControl 300)
> ata3: SATA link down (SStatus 4 SControl 300)
> ata3.00: disabled
> sd 2:0:0:0: rejecting I/O to offline device
> ata3.00: detaching (SCSI 2:0:0:0)
> sd 2:0:0:0: [sdb] Start/Stop Unit failed: Result: hostbyte=DID_NO_CONNECT
> 	driverbyte=DRIVER_OK
> sd 2:0:0:0: [sdb] Synchronizing SCSI cache
> sd 2:0:0:0: [sdb] Synchronize Cache(10) failed: Result:
> 	hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK
> sd 2:0:0:0: [sdb] Stopping disk
> sd 2:0:0:0: [sdb] Start/Stop Unit failed: Result: hostbyte=DID_BAD_TARGET
> 	driverbyte=DRIVER_OK
> 
> c312ef176399 dropped ahci_pci_reset_controller() which internally calls
> ahci_reset_controller() and applies the PCS quirk if needed after that. It
> was called each time a reset was required instead of just
> ahci_reset_controller(). This patch puts the function back in place.
> 
> Fixes: c312ef176399 ("libata/ahci: Drop PCS quirk for Denverton and beyond")
> Signed-off-by: Adam Vodopjan <grozzly@protonmail.com>

Looks good now. I will queue this up for 6.2 and add cc: stable.
Thanks.

-- 
Damien Le Moal
Western Digital Research