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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



There is a bug 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. Because of that my laptop (ICH8M controller) doesnt see KINGSTON SV300S37A60G SSD disk connected into a SATA connector on wake since kernel 5.3.4 or better to say 5.3.8 because there was another error in c312ef176399 until a fix arrived in 09d6ac8dc51a "libata/ahci: Fix PCS quirk application". Btw 4.19.y lts branch is affected as well.

The problem somewhy doesnt trigger on another disk though (WD5000LPCX HDD). I discovered it upgrading the laptop with the SSD in place of a HDD with some 5.4 kernel.

Here is my 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 to test kernels
- sdb: KINGSTON SV300S37A60G SSD connected into the only SATA port

Booting into vanilla 5.3.8 and beyond (built from upstream sources with configs extracted from https://kernel.ubuntu.com/~kernel-ppa/mainline/) I see both disks in lsblk. After wake from suspend the SSD is gone from lsblk output.

Here is 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

The patch is tested on 6.0.10, it solves the problem for my hardware. Compared to c312ef176399, I miraculously revived ahci_pci_reset_controller() and intergrated internals of ahci_intel_pcs_quirk() into it.

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index c1eca72b4575..2f2c8176808c 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -677,6 +677,43 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev,
 	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);
+	const struct pci_device_id *id = pci_match_id(ahci_pci_tbl, pdev);
+	int rc;
+
+	rc = ahci_reset_controller(host);
+	if (rc)
+		return rc;
+
+	/*
+	 * Only apply the 6-port PCS quirk for known legacy platforms.
+	 * Skip applying the quirk on Denverton and beyond
+	 */
+	if (id && id->vendor == PCI_VENDOR_ID_INTEL &&
+			((enum board_ids) id->driver_data) < board_ahci_pcs7) {
+		struct ahci_host_priv *hpriv = host->private_data;
+		u16 tmp16;
+
+		/*
+		 * port_map is determined from PORTS_IMPL PCI register which is
+		 * implemented as write or write-once register.  If the register
+		 * isn't programmed, ahci automatically generates it from number
+		 * of ports, which is good enough for PCS programming. It is
+		 * otherwise expected that platform firmware enables the ports
+		 * before the OS boots.
+		 */
+		pci_read_config_word(pdev, PCS_6, &tmp16);
+		if ((tmp16 & hpriv->port_map) != hpriv->port_map) {
+			tmp16 |= hpriv->port_map;
+			pci_write_config_word(pdev, PCS_6, tmp16);
+		}
+	}
+
+	return 0;
+}
+
 static void ahci_pci_init_controller(struct ata_host *host)
 {
 	struct ahci_host_priv *hpriv = host->private_data;
@@ -871,7 +908,7 @@ static int ahci_pci_device_runtime_resume(struct device *dev)
 	struct ata_host *host = pci_get_drvdata(pdev);
 	int rc;
 
-	rc = ahci_reset_controller(host);
+	rc = ahci_pci_reset_controller(host);
 	if (rc)
 		return rc;
 	ahci_pci_init_controller(host);
@@ -907,7 +944,7 @@ static int ahci_pci_device_resume(struct device *dev)
 		ahci_mcp89_apple_enable(pdev);
 
 	if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
-		rc = ahci_reset_controller(host);
+		rc = ahci_pci_reset_controller(host);
 		if (rc)
 			return rc;
 
@@ -1624,36 +1661,6 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap,
 		ap->target_lpm_policy = policy;
 }
 
-static void ahci_intel_pcs_quirk(struct pci_dev *pdev, struct ahci_host_priv *hpriv)
-{
-	const struct pci_device_id *id = pci_match_id(ahci_pci_tbl, pdev);
-	u16 tmp16;
-
-	/*
-	 * Only apply the 6-port PCS quirk for known legacy platforms.
-	 */
-	if (!id || id->vendor != PCI_VENDOR_ID_INTEL)
-		return;
-
-	/* Skip applying the quirk on Denverton and beyond */
-	if (((enum board_ids) id->driver_data) >= board_ahci_pcs7)
-		return;
-
-	/*
-	 * port_map is determined from PORTS_IMPL PCI register which is
-	 * implemented as write or write-once register.  If the register
-	 * isn't programmed, ahci automatically generates it from number
-	 * of ports, which is good enough for PCS programming. It is
-	 * otherwise expected that platform firmware enables the ports
-	 * before the OS boots.
-	 */
-	pci_read_config_word(pdev, PCS_6, &tmp16);
-	if ((tmp16 & hpriv->port_map) != hpriv->port_map) {
-		tmp16 |= hpriv->port_map;
-		pci_write_config_word(pdev, PCS_6, tmp16);
-	}
-}
-
 static ssize_t remapped_nvme_show(struct device *dev,
 				  struct device_attribute *attr,
 				  char *buf)
@@ -1788,12 +1795,6 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* save initial config */
 	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 */
 	if (hpriv->cap & HOST_CAP_NCQ) {
 		pi.flags |= ATA_FLAG_NCQ;
@@ -1903,7 +1904,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		return rc;
 
-	rc = ahci_reset_controller(host);
+	rc = ahci_pci_reset_controller(host);
 	if (rc)
 		return rc;





[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux