Patchwork [v2,2/2] libata: PM code cleanup for ata port

login
register
mail settings
Submitter Aaron Lu
Date Jan. 25, 2013, 6:32 a.m.
Message ID <510226F9.5000709@intel.com>
Download mbox | patch
Permalink /patch/215557/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Aaron Lu - Jan. 25, 2013, 6:32 a.m.
For system freeze, if the port is already runtime suspended, leave it
alone and just return. The port will be resumed on thaw before it will
be used.

And since we will call get_noresume for every device during prepare
phase, and the port is resumed during thaw phase, it can't be in runtime
suspended state during the poweroff phase. So remove the
runtime_suspended check in poweroff callback.

And for all suspend(freeze/suspend/poweroff/etc.), there is no need to
touch the device, so set no_autopsy and no_recovery for them all.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 drivers/ata/libata-core.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)
Sergei Shtylyov - Jan. 25, 2013, 11:33 a.m.
Hello.

On 25-01-2013 10:32, Aaron Lu wrote:

> For system freeze, if the port is already runtime suspended, leave it
> alone and just return. The port will be resumed on thaw before it will
> be used.

> And since we will call get_noresume for every device during prepare
> phase, and the port is resumed during thaw phase, it can't be in runtime
> suspended state during the poweroff phase. So remove the
> runtime_suspended check in poweroff callback.

> And for all suspend(freeze/suspend/poweroff/etc.), there is no need to
> touch the device, so set no_autopsy and no_recovery for them all.

> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
>   drivers/ata/libata-core.c | 16 ++++------------
>   1 file changed, 4 insertions(+), 12 deletions(-)

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 6a5ef86..439c608 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5334,9 +5334,6 @@ static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
>
>   static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg, int *async)
>   {
> -	unsigned int ehi_flags = ATA_EHI_QUIET;
> -	int rc;
> -
>   	/*
>   	 * On some hardware, device fails to respond after spun down
>   	 * for suspend.  As the device won't be used before being
> @@ -5345,11 +5342,9 @@ static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg, int
>   	 *
>   	 * http://thread.gmane.org/gmane.linux.ide/46764
>   	 */
> -	if (mesg.event & PM_EVENT_SUSPEND)
> -		ehi_flags |= ATA_EHI_NO_AUTOPSY | ATA_EHI_NO_RECOVERY;
> -
> -	rc = ata_port_request_pm(ap, mesg, 0, ehi_flags, async);
> -	return rc;
> +	unsigned int ehi_flags = ATA_EHI_QUIET | ATA_EHI_NO_AUTOPSY |
> +				 ATA_EHI_NO_RECOVERY;

    Please keep the existing coding style and insert empty line after 
declarations.
    Wait, you don't need this variable at all...

> +	return ata_port_request_pm(ap, mesg, 0, ehi_flags, async);
>   }

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
Rafael J. Wysocki - Jan. 25, 2013, 12:29 p.m.
On Friday, January 25, 2013 03:33:36 PM Sergei Shtylyov wrote:
> Hello.
> 
> On 25-01-2013 10:32, Aaron Lu wrote:
> 
> > For system freeze, if the port is already runtime suspended, leave it
> > alone and just return. The port will be resumed on thaw before it will
> > be used.
> 
> > And since we will call get_noresume for every device during prepare
> > phase, and the port is resumed during thaw phase, it can't be in runtime
> > suspended state during the poweroff phase. So remove the
> > runtime_suspended check in poweroff callback.
> 
> > And for all suspend(freeze/suspend/poweroff/etc.), there is no need to
> > touch the device, so set no_autopsy and no_recovery for them all.
> 
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > ---
> >   drivers/ata/libata-core.c | 16 ++++------------
> >   1 file changed, 4 insertions(+), 12 deletions(-)
> 
> > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> > index 6a5ef86..439c608 100644
> > --- a/drivers/ata/libata-core.c
> > +++ b/drivers/ata/libata-core.c
> > @@ -5334,9 +5334,6 @@ static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
> >
> >   static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg, int *async)
> >   {
> > -	unsigned int ehi_flags = ATA_EHI_QUIET;
> > -	int rc;
> > -
> >   	/*
> >   	 * On some hardware, device fails to respond after spun down
> >   	 * for suspend.  As the device won't be used before being
> > @@ -5345,11 +5342,9 @@ static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg, int
> >   	 *
> >   	 * http://thread.gmane.org/gmane.linux.ide/46764
> >   	 */
> > -	if (mesg.event & PM_EVENT_SUSPEND)
> > -		ehi_flags |= ATA_EHI_NO_AUTOPSY | ATA_EHI_NO_RECOVERY;
> > -
> > -	rc = ata_port_request_pm(ap, mesg, 0, ehi_flags, async);
> > -	return rc;
> > +	unsigned int ehi_flags = ATA_EHI_QUIET | ATA_EHI_NO_AUTOPSY |
> > +				 ATA_EHI_NO_RECOVERY;
> 
>     Please keep the existing coding style and insert empty line after 
> declarations.
>     Wait, you don't need this variable at all...

Well, I suppose Aaron just wanted to avoid ugly line breaks in the return
line below and whether or not to keep the empty line in this case is a matter
of taste IMHO.

In my personal opinion it would be cleaner with the empty line.

> > +	return ata_port_request_pm(ap, mesg, 0, ehi_flags, async);
> >   }

Thanks,
Rafael
Jeff Garzik - Jan. 25, 2013, 8:34 p.m.
On 01/25/2013 01:32 AM, Aaron Lu wrote:
> For system freeze, if the port is already runtime suspended, leave it
> alone and just return. The port will be resumed on thaw before it will
> be used.
>
> And since we will call get_noresume for every device during prepare
> phase, and the port is resumed during thaw phase, it can't be in runtime
> suspended state during the poweroff phase. So remove the
> runtime_suspended check in poweroff callback.
>
> And for all suspend(freeze/suspend/poweroff/etc.), there is no need to
> touch the device, so set no_autopsy and no_recovery for them all.
>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
>   drivers/ata/libata-core.c | 16 ++++------------
>   1 file changed, 4 insertions(+), 12 deletions(-)

applied 1-2



--
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/libata-core.c b/drivers/ata/libata-core.c
index 6a5ef86..439c608 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5334,9 +5334,6 @@  static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
 
 static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg, int *async)
 {
-	unsigned int ehi_flags = ATA_EHI_QUIET;
-	int rc;
-
 	/*
 	 * On some hardware, device fails to respond after spun down
 	 * for suspend.  As the device won't be used before being
@@ -5345,11 +5342,9 @@  static int __ata_port_suspend_common(struct ata_port *ap, pm_message_t mesg, int
 	 *
 	 * http://thread.gmane.org/gmane.linux.ide/46764
 	 */
-	if (mesg.event & PM_EVENT_SUSPEND)
-		ehi_flags |= ATA_EHI_NO_AUTOPSY | ATA_EHI_NO_RECOVERY;
-
-	rc = ata_port_request_pm(ap, mesg, 0, ehi_flags, async);
-	return rc;
+	unsigned int ehi_flags = ATA_EHI_QUIET | ATA_EHI_NO_AUTOPSY |
+				 ATA_EHI_NO_RECOVERY;
+	return ata_port_request_pm(ap, mesg, 0, ehi_flags, async);
 }
 
 static int ata_port_suspend_common(struct device *dev, pm_message_t mesg)
@@ -5370,16 +5365,13 @@  static int ata_port_suspend(struct device *dev)
 static int ata_port_do_freeze(struct device *dev)
 {
 	if (pm_runtime_suspended(dev))
-		pm_runtime_resume(dev);
+		return 0;
 
 	return ata_port_suspend_common(dev, PMSG_FREEZE);
 }
 
 static int ata_port_poweroff(struct device *dev)
 {
-	if (pm_runtime_suspended(dev))
-		return 0;
-
 	return ata_port_suspend_common(dev, PMSG_HIBERNATE);
 }