Patchwork libata: replace sata_settings with devslp_timing

login
register
mail settings
Submitter Shane Huang
Date Dec. 17, 2012, 3:18 p.m.
Message ID <1355757539-4529-1-git-send-email-shane.huang@amd.com>
Download mbox | patch
Permalink /patch/206897/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Shane Huang - Dec. 17, 2012, 3:18 p.m.
NCQ capability was used to check availability of SATA Settings page
from Identify Device Data Log, which contains DevSlp timing variables.
It does not work on some HDDs and leads to error messages.

IDENTIFY word 78 bit 5(Hardware Feature Control) can't work either
because it is only the sufficient condition of Identify Device data
log, not the necessary condition.

This patch replaced ata_device->sata_settings with ->devslp_timing
to only save DevSlp timing variables(8 bytes), instead of the whole
SATA Settings page(512 bytes).

Reported-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Shane Huang <shane.huang@amd.com>
Cc: stable@kernel.org
---
 drivers/ata/libahci.c     |    6 +++---
 drivers/ata/libata-core.c |   22 +++++++++++++---------
 include/linux/ata.h       |    8 +++++---
 include/linux/libata.h    |    4 ++--
 4 files changed, 23 insertions(+), 17 deletions(-)
Shane Huang - Dec. 26, 2012, 2:29 a.m.
Jeff,

> NCQ capability was used to check availability of SATA Settings page
> from Identify Device Data Log, which contains DevSlp timing variables.
> It does not work on some HDDs and leads to error messages.
> 
> IDENTIFY word 78 bit 5(Hardware Feature Control) can't work either
> because it is only the sufficient condition of Identify Device data
> log, not the necessary condition.
> 
> This patch replaced ata_device->sata_settings with ->devslp_timing
> to only save DevSlp timing variables(8 bytes), instead of the whole
> SATA Settings page(512 bytes).
> 
> Reported-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Shane Huang <shane.huang@amd.com>
> Cc: stable@kernel.org

Happy holidays!

Do you have comments to this patch? It also fixed bugzilla #51881
reported and verified by Kouichi.

Please also help to apply it into the stable tree.

Thanks,
Shane


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Dec. 26, 2012, 10:41 a.m.
Hello.

On 17-12-2012 19:18, Shane Huang wrote:

> NCQ capability was used to check availability of SATA Settings page
> from Identify Device Data Log, which contains DevSlp timing variables.
> It does not work on some HDDs and leads to error messages.

> IDENTIFY word 78 bit 5(Hardware Feature Control) can't work either
> because it is only the sufficient condition of Identify Device data
> log, not the necessary condition.

> This patch replaced ata_device->sata_settings with ->devslp_timing
> to only save DevSlp timing variables(8 bytes), instead of the whole
> SATA Settings page(512 bytes).

> Reported-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Shane Huang <shane.huang@amd.com>
> Cc: stable@kernel.org

    The new address is stable@vger.kernel.org. The old address doesn't work 
anymore.

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shane Huang - Dec. 27, 2012, 2:01 a.m.
Sergei,

> The new address is stable@vger.kernel.org. The old address doesn't
> work anymore.

Understood, thanks. I expect Jeff's help and won't resend the patch.

Regards,
Shane

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 320712a..6cd7805 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1951,13 +1951,13 @@  static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
 	/* Use the nominal value 10 ms if the read MDAT is zero,
 	 * the nominal value of DETO is 20 ms.
 	 */
-	if (dev->sata_settings[ATA_LOG_DEVSLP_VALID] &
+	if (dev->devslp_timing[ATA_LOG_DEVSLP_VALID] &
 	    ATA_LOG_DEVSLP_VALID_MASK) {
-		mdat = dev->sata_settings[ATA_LOG_DEVSLP_MDAT] &
+		mdat = dev->devslp_timing[ATA_LOG_DEVSLP_MDAT] &
 		       ATA_LOG_DEVSLP_MDAT_MASK;
 		if (!mdat)
 			mdat = 10;
-		deto = dev->sata_settings[ATA_LOG_DEVSLP_DETO];
+		deto = dev->devslp_timing[ATA_LOG_DEVSLP_DETO];
 		if (!deto)
 			deto = 20;
 	} else {
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 9e8b99a..46cd3f4 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2325,24 +2325,28 @@  int ata_dev_configure(struct ata_device *dev)
 			}
 		}
 
-		/* check and mark DevSlp capability */
-		if (ata_id_has_devslp(dev->id))
-			dev->flags |= ATA_DFLAG_DEVSLP;
-
-		/* Obtain SATA Settings page from Identify Device Data Log,
-		 * which contains DevSlp timing variables etc.
-		 * Exclude old devices with ata_id_has_ncq()
+		/* Check and mark DevSlp capability. Get DevSlp timing variables
+		 * from SATA Settings page of Identify Device Data Log.
 		 */
-		if (ata_id_has_ncq(dev->id)) {
+		if (ata_id_has_devslp(dev->id)) {
+			u8 sata_setting[ATA_SECT_SIZE];
+			int i, j;
+
+			dev->flags |= ATA_DFLAG_DEVSLP;
 			err_mask = ata_read_log_page(dev,
 						     ATA_LOG_SATA_ID_DEV_DATA,
 						     ATA_LOG_SATA_SETTINGS,
-						     dev->sata_settings,
+						     sata_setting,
 						     1);
 			if (err_mask)
 				ata_dev_dbg(dev,
 					    "failed to get Identify Device Data, Emask 0x%x\n",
 					    err_mask);
+			else
+				for (i = 0; i < ATA_LOG_DEVSLP_SIZE; i++) {
+					j = ATA_LOG_DEVSLP_OFFSET + i;
+					dev->devslp_timing[i] = sata_setting[j];
+				}
 		}
 
 		dev->cdb_len = 16;
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 408da95..8f7a3d6 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -297,10 +297,12 @@  enum {
 	ATA_LOG_SATA_NCQ	= 0x10,
 	ATA_LOG_SATA_ID_DEV_DATA  = 0x30,
 	ATA_LOG_SATA_SETTINGS	  = 0x08,
-	ATA_LOG_DEVSLP_MDAT	  = 0x30,
+	ATA_LOG_DEVSLP_OFFSET	  = 0x30,
+	ATA_LOG_DEVSLP_SIZE	  = 0x08,
+	ATA_LOG_DEVSLP_MDAT	  = 0x00,
 	ATA_LOG_DEVSLP_MDAT_MASK  = 0x1F,
-	ATA_LOG_DEVSLP_DETO	  = 0x31,
-	ATA_LOG_DEVSLP_VALID	  = 0x37,
+	ATA_LOG_DEVSLP_DETO	  = 0x01,
+	ATA_LOG_DEVSLP_VALID	  = 0x07,
 	ATA_LOG_DEVSLP_VALID_MASK = 0x80,
 
 	/* READ/WRITE LONG (obsolete) */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 83ba0ab..649e5f8 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -652,8 +652,8 @@  struct ata_device {
 		u32		gscr[SATA_PMP_GSCR_DWORDS]; /* PMP GSCR block */
 	};
 
-	/* Identify Device Data Log (30h), SATA Settings (page 08h) */
-	u8			sata_settings[ATA_SECT_SIZE];
+	/* DEVSLP Timing Variables from Identify Device Data Log */
+	u8			devslp_timing[ATA_LOG_DEVSLP_SIZE];
 
 	/* error history */
 	int			spdn_cnt;