* [PATCH] libata-core: Allow longer timeout for drive spinup from PUIS @ 2016-03-22 8:22 Damien Le Moal 2016-03-30 18:38 ` Tejun Heo 0 siblings, 1 reply; 7+ messages in thread From: Damien Le Moal @ 2016-03-22 8:22 UTC (permalink / raw) To: linux-ide When spinning a drive from powered on standby mode (PUIS, SETFEATURES_SPINUP is executed with the default timeout used for any SETFEATURES subcommand, that is 5+10 seconds. The total 15s is too short for some drives to complete spinup (e.g. drives with a large indirection table stored on media), resulting in ata_dev_read_id to fail twice on the execution of SETFEATURES_SPINUP. For this feature, allow a larger default timeout of 30 seconds. However, in the same spirit as with the timeout of other feature subcommands, do not ignore ata_probe_timeout if it is set). 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 55e257c..c94a2a1 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4528,6 +4528,7 @@ unsigned int ata_dev_set_feature(struct ata_device *dev, u8 enable, u8 feature) { struct ata_taskfile tf; unsigned int err_mask; + unsigned long timeout = 0; /* set up set-features taskfile */ DPRINTK("set features - SATA features\n"); @@ -4539,7 +4540,10 @@ 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); + if (enable == SETFEATURES_SPINUP) + timeout = (ata_probe_timeout ? + ata_probe_timeout : SETFEATURES_SPINUP_TIMEOUT) * 1000; + err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, timeout); DPRINTK("EXIT, err_mask=%x\n", err_mask); return err_mask; diff --git a/include/linux/ata.h b/include/linux/ata.h index c1a2f34..f460e7f 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -371,7 +371,8 @@ enum { SETFEATURES_AAM_ON = 0x42, SETFEATURES_AAM_OFF = 0xC2, - SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ + SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ + SETFEATURES_SPINUP_TIMEOUT = 30, /* 30s timeout for drive spin-up from PUIS */ 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. ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] libata-core: Allow longer timeout for drive spinup from PUIS 2016-03-22 8:22 [PATCH] libata-core: Allow longer timeout for drive spinup from PUIS Damien Le Moal @ 2016-03-30 18:38 ` Tejun Heo 2016-04-02 12:23 ` Damien Le Moal 0 siblings, 1 reply; 7+ messages in thread From: Tejun Heo @ 2016-03-30 18:38 UTC (permalink / raw) To: Damien Le Moal; +Cc: linux-ide Hello, Damien. On Tue, Mar 22, 2016 at 08:22:22AM +0000, Damien Le Moal wrote: > diff --git a/include/linux/ata.h b/include/linux/ata.h > index c1a2f34..f460e7f 100644 > --- a/include/linux/ata.h > +++ b/include/linux/ata.h > @@ -371,7 +371,8 @@ enum { > SETFEATURES_AAM_ON = 0x42, > SETFEATURES_AAM_OFF = 0xC2, > > - SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ > + SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ > + SETFEATURES_SPINUP_TIMEOUT = 30, /* 30s timeout for drive spin-up from PUIS */ Other timeout constants are defined in msecs. Let's do the same here. Also, can you please send the email as plain-text without the base64 encoding? If you can't, please send the patch as an attachment instead. Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libata-core: Allow longer timeout for drive spinup from PUIS 2016-03-30 18:38 ` Tejun Heo @ 2016-04-02 12:23 ` Damien Le Moal 2016-04-04 16:18 ` Tejun Heo 0 siblings, 1 reply; 7+ messages in thread From: Damien Le Moal @ 2016-04-02 12:23 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-ide [-- Attachment #1: Type: text/plain, Size: 1928 bytes --] Tejun, >On Tue, Mar 22, 2016 at 08:22:22AM +0000, Damien Le Moal wrote: >> diff --git a/include/linux/ata.h b/include/linux/ata.h >> index c1a2f34..f460e7f 100644 >> --- a/include/linux/ata.h >> +++ b/include/linux/ata.h >> @@ -371,7 +371,8 @@ enum { >> SETFEATURES_AAM_ON = 0x42, >> SETFEATURES_AAM_OFF = 0xC2, >> >> - SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ >> + SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ >> + SETFEATURES_SPINUP_TIMEOUT = 30, /* 30s timeout for drive spin-up from PUIS */ > >Other timeout constants are defined in msecs. Let's do the same here. >Also, can you please send the email as plain-text without the base64 >encoding? If you can't, please send the patch as an attachment >instead. Sorry about the email format (I am sending emails in plain text, but my Outlook exchange client/exchange server does not seem to be respecting that format). Here is the patch as attachment, reworked to define the timeout in milliseconds as you suggested. Thank you. Best regards. -- Damien Le Moal, Ph.D. Sr Manager, System Software Group, WW Research, HGST, a Western Digital company Damien.LeMoal@hgst.com Tel: (+81) 0466-98-3593 (Ext. 51-3593) 1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan www.hgst.com <http://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. [-- Attachment #2: set-features-spinup-timeout.patch --] [-- Type: application/octet-stream, Size: 2361 bytes --] commit c75ab67dcbf5ac1e88abd967b07a0279ab488559 Author: Damien Le Moal <damien.lemoal@hgst.com> Date: Tue Mar 22 10:36:48 2016 +0900 libata-core: Allow longer timeout for drive spinup from PUIS When spinning up a drive from powered on standby mode (PUIS), SETFEATURES_SPINUP is executed with the default timeout used for any SETFEATURES subcommand, that is 5+10 seconds. The total 15s is too short for some drives to complete spinup (e.g. drives with a large indirection table stored on media), resulting in ata_dev_read_id to fail twice on the execution of SETFEATURES_SPINUP. For this feature, allow a larger default timeout of 30 seconds. However, in the same spirit as with the timeout of other feature subcommands, do not ignore ata_probe_timeout if it is set). 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 55e257c..c94a2a1 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4528,6 +4528,7 @@ unsigned int ata_dev_set_feature(struct ata_device *dev, u8 enable, u8 feature) { struct ata_taskfile tf; unsigned int err_mask; + unsigned long timeout = 0; /* set up set-features taskfile */ DPRINTK("set features - SATA features\n"); @@ -4539,7 +4540,10 @@ 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); + if (enable == SETFEATURES_SPINUP) + timeout = ata_probe_timeout ? + ata_probe_timeout * 1000 : SETFEATURES_SPINUP_TIMEOUT; + err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, timeout); DPRINTK("EXIT, err_mask=%x\n", err_mask); return err_mask; diff --git a/include/linux/ata.h b/include/linux/ata.h index c1a2f34..f460e7f 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -371,7 +371,8 @@ enum { SETFEATURES_AAM_ON = 0x42, SETFEATURES_AAM_OFF = 0xC2, - SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ + SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ + SETFEATURES_SPINUP_TIMEOUT = 30000, /* 30s timeout for drive spin-up from PUIS */ SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */ SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */ ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] libata-core: Allow longer timeout for drive spinup from PUIS 2016-04-02 12:23 ` Damien Le Moal @ 2016-04-04 16:18 ` Tejun Heo 2016-04-07 1:22 ` Damien Le Moal 0 siblings, 1 reply; 7+ messages in thread From: Tejun Heo @ 2016-04-04 16:18 UTC (permalink / raw) To: Damien Le Moal; +Cc: linux-ide On Sat, Apr 02, 2016 at 12:23:02PM +0000, Damien Le Moal wrote: > Sorry about the email format (I am sending emails in plain text, but my > Outlook exchange client/exchange server does not seem to be respecting > that format). > > Here is the patch as attachment, reworked to define the timeout in > milliseconds as you suggested. Applied to libata/for-4.7. Can you please use git format-patch to generate the patch file from next time? Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libata-core: Allow longer timeout for drive spinup from PUIS 2016-04-04 16:18 ` Tejun Heo @ 2016-04-07 1:22 ` Damien Le Moal 2016-04-07 11:00 ` James Bottomley 2016-04-07 17:46 ` Tejun Heo 0 siblings, 2 replies; 7+ messages in thread From: Damien Le Moal @ 2016-04-07 1:22 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-ide Tejun, >Applied to libata/for-4.7. Can you please use git format-patch to >generate the patch file from next time? Thanks ! For the patch, I used “git show xxx > file”... Is this not correct ? Should I be using a “—pretty=xxx” formatting ? Best regards. -- Damien Le Moal, Ph.D. Sr Manager, System Software Group, WW Research, HGST, a Western Digital company Damien.LeMoal@hgst.com Tel: (+81) 0466-98-3593 (Ext. 51-3593) 1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan www.hgst.com <http://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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libata-core: Allow longer timeout for drive spinup from PUIS 2016-04-07 1:22 ` Damien Le Moal @ 2016-04-07 11:00 ` James Bottomley 2016-04-07 17:46 ` Tejun Heo 1 sibling, 0 replies; 7+ messages in thread From: James Bottomley @ 2016-04-07 11:00 UTC (permalink / raw) To: Damien Le Moal, Tejun Heo; +Cc: linux-ide On Thu, 2016-04-07 at 01:22 +0000, Damien Le Moal wrote: > Tejun, > > > Applied to libata/for-4.7. Can you please use git format-patch to > > generate the patch file from next time? > > Thanks ! > For the patch, I used “git show xxx > file”... > Is this not correct ? Should I be using a “—pretty=xxx” formatting ? If you want to use git show, then you need --pretty=email (depending on whether you want to add the commit statistics, you can also add --stat --patch). If you have a patch series, it's often better to use git-format-patch -n --cover-letter Which generates a set of numbered files for the entire patch series. James ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libata-core: Allow longer timeout for drive spinup from PUIS 2016-04-07 1:22 ` Damien Le Moal 2016-04-07 11:00 ` James Bottomley @ 2016-04-07 17:46 ` Tejun Heo 1 sibling, 0 replies; 7+ messages in thread From: Tejun Heo @ 2016-04-07 17:46 UTC (permalink / raw) To: Damien Le Moal; +Cc: linux-ide Hello, On Thu, Apr 07, 2016 at 01:22:19AM +0000, Damien Le Moal wrote: > >Applied to libata/for-4.7. Can you please use git format-patch to > >generate the patch file from next time? > > Thanks ! > For the patch, I used “git show xxx > file”... > Is this not correct ? Should I be using a “—pretty=xxx” formatting ? I always use git format-patch. For a single patch, you can simply do "git format-patch HEAD^". Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-04-07 17:46 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-22 8:22 [PATCH] libata-core: Allow longer timeout for drive spinup from PUIS Damien Le Moal 2016-03-30 18:38 ` Tejun Heo 2016-04-02 12:23 ` Damien Le Moal 2016-04-04 16:18 ` Tejun Heo 2016-04-07 1:22 ` Damien Le Moal 2016-04-07 11:00 ` James Bottomley 2016-04-07 17:46 ` Tejun Heo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.