Commit 37e14e4f authored by Adam Vodopjan's avatar Adam Vodopjan Committed by Damien Le Moal

ata: ahci: Fix PCS quirk application for suspend

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 c312ef17 ("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 commit
09d6ac8d ("libata/ahci: Fix PCS quirk application") landed in 5.3.8.
So testing my patch anywhere between commits c312ef17 and
09d6ac8d 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

Commit c312ef17 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: c312ef17 ("libata/ahci: Drop PCS quirk for Denverton and beyond")
Signed-off-by: default avatarAdam Vodopjan <grozzly@protonmail.com>
Signed-off-by: default avatarDamien Le Moal <damien.lemoal@opensource.wdc.com>
parent 1b929c02
...@@ -83,6 +83,7 @@ enum board_ids { ...@@ -83,6 +83,7 @@ enum board_ids {
static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent); static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
static void ahci_remove_one(struct pci_dev *dev); static void ahci_remove_one(struct pci_dev *dev);
static void ahci_shutdown_one(struct pci_dev *dev); static void ahci_shutdown_one(struct pci_dev *dev);
static void ahci_intel_pcs_quirk(struct pci_dev *pdev, struct ahci_host_priv *hpriv);
static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class, static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline); unsigned long deadline);
static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class, static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class,
...@@ -676,6 +677,25 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev, ...@@ -676,6 +677,25 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev,
ahci_save_initial_config(&pdev->dev, hpriv); ahci_save_initial_config(&pdev->dev, hpriv);
} }
static int ahci_pci_reset_controller(struct ata_host *host)
{
struct pci_dev *pdev = to_pci_dev(host->dev);
struct ahci_host_priv *hpriv = host->private_data;
int rc;
rc = ahci_reset_controller(host);
if (rc)
return rc;
/*
* If platform firmware failed to enable ports, try to enable
* them here.
*/
ahci_intel_pcs_quirk(pdev, hpriv);
return 0;
}
static void ahci_pci_init_controller(struct ata_host *host) static void ahci_pci_init_controller(struct ata_host *host)
{ {
struct ahci_host_priv *hpriv = host->private_data; struct ahci_host_priv *hpriv = host->private_data;
...@@ -870,7 +890,7 @@ static int ahci_pci_device_runtime_resume(struct device *dev) ...@@ -870,7 +890,7 @@ static int ahci_pci_device_runtime_resume(struct device *dev)
struct ata_host *host = pci_get_drvdata(pdev); struct ata_host *host = pci_get_drvdata(pdev);
int rc; int rc;
rc = ahci_reset_controller(host); rc = ahci_pci_reset_controller(host);
if (rc) if (rc)
return rc; return rc;
ahci_pci_init_controller(host); ahci_pci_init_controller(host);
...@@ -906,7 +926,7 @@ static int ahci_pci_device_resume(struct device *dev) ...@@ -906,7 +926,7 @@ static int ahci_pci_device_resume(struct device *dev)
ahci_mcp89_apple_enable(pdev); ahci_mcp89_apple_enable(pdev);
if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) { if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
rc = ahci_reset_controller(host); rc = ahci_pci_reset_controller(host);
if (rc) if (rc)
return rc; return rc;
...@@ -1784,12 +1804,6 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) ...@@ -1784,12 +1804,6 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
/* save initial config */ /* save initial config */
ahci_pci_save_initial_config(pdev, hpriv); ahci_pci_save_initial_config(pdev, hpriv);
/*
* If platform firmware failed to enable ports, try to enable
* them here.
*/
ahci_intel_pcs_quirk(pdev, hpriv);
/* prepare host */ /* prepare host */
if (hpriv->cap & HOST_CAP_NCQ) { if (hpriv->cap & HOST_CAP_NCQ) {
pi.flags |= ATA_FLAG_NCQ; pi.flags |= ATA_FLAG_NCQ;
...@@ -1899,7 +1913,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) ...@@ -1899,7 +1913,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (rc) if (rc)
return rc; return rc;
rc = ahci_reset_controller(host); rc = ahci_pci_reset_controller(host);
if (rc) if (rc)
return rc; return rc;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment