[PATCH] libata: Set longer timeout for SETFEATURES_SPINUP

[PATCH] libata: Set longer timeout for SETFEATURES_SPINUP

[Damien Le Moal]

libata: Set longer timeout for SETFEATURES_SPINUP
    
For SATA drives with power-up in standby (PUIS) feature set,
SETFEATURES_SPINUP execution may be required to get
complete IDENTIFY data. However, the timeout used for its
execution is the same as for all other SETFEATURES commands,
that is, 5 seconds. This is too short for some disks to complete
timeout and allow IDENTIFY to return complete data (e.g.
disks with large indirection tables stored on media), resulting
in ata_dev_read_id to fail. For this feature, allow a larger
timeout of 30 seconds.

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e83fc3d..ecbbee2 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4537,7 +4537,8 @@ unsigned int ata_dev_set_feature(struct ata_device *dev, u8 enable, u8 feature)
 	tf.protocol = ATA_PROT_NODATA;
 	tf.nsect = feature;
 
-	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
+	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0,
+				     enable == SETFEATURES_SPINUP ? SETFEATURES_SPINUP_TIMEOUT : 0);
 
 	DPRINTK("EXIT, err_mask=%x\n", err_mask);
 	return err_mask;
diff --git a/include/linux/ata.h b/include/linux/ata.h
index fed3641..e3a9ab6 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -371,6 +371,7 @@ enum {
 	SETFEATURES_AAM_OFF	= 0xC2,
 
 	SETFEATURES_SPINUP	= 0x07, /* Spin-up drive */
+	SETFEATURES_SPINUP_TIMEOUT	= 30000,
 
 	SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
 	SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */


------------------------
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital company
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa, 
Kanagawa, 252-0888 Japan
www.hgst.com 
Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Re: [PATCH] libata: Set longer timeout for SETFEATURES_SPINUP

[Hannes Reinecke]

On 03/18/2016 05:49 AM, Damien Le Moal wrote:
> 
> libata: Set longer timeout for SETFEATURES_SPINUP
>     
> For SATA drives with power-up in standby (PUIS) feature set,
> SETFEATURES_SPINUP execution may be required to get
> complete IDENTIFY data. However, the timeout used for its
> execution is the same as for all other SETFEATURES commands,
> that is, 5 seconds. This is too short for some disks to complete
> timeout and allow IDENTIFY to return complete data (e.g.
> disks with large indirection tables stored on media), resulting
> in ata_dev_read_id to fail. For this feature, allow a larger
> timeout of 30 seconds.
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

(BTW, you might want to add your Signed-off-by:, too :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libata: Set longer timeout for SETFEATURES_SPINUP

[Damien Le Moal]

>On 03/18/2016 05:49 AM, Damien Le Moal wrote:
>> 
>> libata: Set longer timeout for SETFEATURES_SPINUP
>>     
>> For SATA drives with power-up in standby (PUIS) feature set,
>> SETFEATURES_SPINUP execution may be required to get
>> complete IDENTIFY data. However, the timeout used for its
>> execution is the same as for all other SETFEATURES commands,
>> that is, 5 seconds. This is too short for some disks to complete
>> timeout and allow IDENTIFY to return complete data (e.g.
>> disks with large indirection tables stored on media), resulting
>> in ata_dev_read_id to fail. For this feature, allow a larger
>> timeout of 30 seconds.
>> 
>Reviewed-by: Hannes Reinecke <hare@suse.com>
>
>(BTW, you might want to add your Signed-off-by:, too :-)

Hannes,

Sorry about this. Here it is...

libata: Set longer timeout for SETFEATURES_SPINUP
    
For SATA drives with power-up in standby (PUIS) feature set,
SETFEATURES_SPINUP execution may be required to get
complete IDENTIFY data. However, the timeout used for its
execution is the same as for all other SETFEATURES commands,
that is, 5 seconds. This is too short for some disks to complete
spinup and allow IDENTIFY to return complete data (e.g.
disks with large indirection tables stored on media), resulting
in ata_dev_read_id to fail. For this feature, allow a larger
timeout of 30 seconds.

Signed-off-by: Damien Le Moal <damien.lemoal@hgst.com>

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e83fc3d..ecbbee2 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4537,7 +4537,8 @@ unsigned int ata_dev_set_feature(struct ata_device *dev, u8 enable, u8 feature)
 	tf.protocol = ATA_PROT_NODATA;
 	tf.nsect = feature;
 
-	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
+	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0,
+				     enable == SETFEATURES_SPINUP ? SETFEATURES_SPINUP_TIMEOUT : 0);
 
 	DPRINTK("EXIT, err_mask=%x\n", err_mask);
 	return err_mask;
diff --git a/include/linux/ata.h b/include/linux/ata.h
index fed3641..e3a9ab6 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -371,6 +371,7 @@ enum {
 	SETFEATURES_AAM_OFF	= 0xC2,
 
 	SETFEATURES_SPINUP	= 0x07, /* Spin-up drive */
+	SETFEATURES_SPINUP_TIMEOUT	= 30000,
 
 	SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
 	SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */


------------------------
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital company
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa, 
Kanagawa, 252-0888 Japan
www.hgst.com

Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Re: [PATCH] libata: Set longer timeout for SETFEATURES_SPINUP

[Martin K. Petersen]

>>>>> "Damien" == Damien Le Moal <Damien.LeMoal@hgst.com> writes:

Damien,

Damien> libata: Set longer timeout for SETFEATURES_SPINUP
    
Please submit to linux-ide@vger.kernel.org.

-- 
Martin K. Petersen	Oracle Linux Engineering