Linux Power Management development
 help / color / mirror / Atom feed
* Re: ehci_hcd related S3 lockup on ASUS laptops, again
From: Steven Rostedt @ 2012-04-12 16:52 UTC (permalink / raw)
  To: Andrey Rahmatullin; +Cc: jrnieder, Greg KH, linux-pm, USB list
In-Reply-To: <20120412164903.GB12852@belkar.wrar.name>

On Thu, 2012-04-12 at 22:49 +0600, Andrey Rahmatullin wrote:
> On Thu, Apr 12, 2012 at 12:09:00PM -0400, Alan Stern wrote:
> > > > Here's a diagnostic patch that will give us a little more information.  
> > > > Keep the previous change (so that pci_prepare_to_sleep gets called 
> > > > whether ehci-hcd is bound or not) and let's see what it says.  Try 
> > > > doing it both with and without ehci-hcd bound.  Running this with "echo 
> > > > devices >/sys/power/pm_test" ought to be good enough.
> > > I don't see that line with ehci_hcd both bound and unbound and both in
> > > 'devices' test and in a real S3.
> > 
> > You mean the new dev_info message did not appear at all?  Is
> > pci_prepare_to_sleep getting called properly?  Or does the routine exit
> > early because target_state is equal to PCI_POWER_ERROR?
> Either dev_info is silenced on my system or pci_pm_suspend_noirq is not
> called during the 'devices' pm test.
> 

Did you undo the original change that removed the call to
pci_prepare_to_sleep()?

-- Steve

^ permalink raw reply

* Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again
From: Andrey Rahmatullin @ 2012-04-12 16:49 UTC (permalink / raw)
  To: Alan Stern
  Cc: Steven Rostedt, jrnieder-Re5JQEeQqe8AvxtiuMwx3w,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, USB list,
	Greg KH
In-Reply-To: <Pine.LNX.4.44L0.1204121203530.1496-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

On Thu, Apr 12, 2012 at 12:09:00PM -0400, Alan Stern wrote:
> > > Here's a diagnostic patch that will give us a little more information.  
> > > Keep the previous change (so that pci_prepare_to_sleep gets called 
> > > whether ehci-hcd is bound or not) and let's see what it says.  Try 
> > > doing it both with and without ehci-hcd bound.  Running this with "echo 
> > > devices >/sys/power/pm_test" ought to be good enough.
> > I don't see that line with ehci_hcd both bound and unbound and both in
> > 'devices' test and in a real S3.
> 
> You mean the new dev_info message did not appear at all?  Is
> pci_prepare_to_sleep getting called properly?  Or does the routine exit
> early because target_state is equal to PCI_POWER_ERROR?
Either dev_info is silenced on my system or pci_pm_suspend_noirq is not
called during the 'devices' pm test.

-- 
WBR, wRAR

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: ehci_hcd related S3 lockup on ASUS laptops, again
From: Steven Rostedt @ 2012-04-12 16:33 UTC (permalink / raw)
  To: Alan Stern; +Cc: jrnieder, Andrey Rahmatullin, linux-pm, USB list, Greg KH
In-Reply-To: <Pine.LNX.4.44L0.1204121017460.1496-100000@iolanthe.rowland.org>

On Thu, 2012-04-12 at 10:28 -0400, Alan Stern wrote:

> Hmmm.  This is a situation where the wakeup setting might matter.  Did 
> the /sys/bus/pci/devices/0000:00:1[ad].0/power/wakeup files both 
> contain "enabled" when you ran the test?

They're both enabled for me too.

> 
> Here's a diagnostic patch that will give us a little more information.  
> Keep the previous change (so that pci_prepare_to_sleep gets called 
> whether ehci-hcd is bound or not) and let's see what it says.  Try 
> doing it both with and without ehci-hcd bound.  Running this with "echo 
> devices >/sys/power/pm_test" ought to be good enough.

Where do I read out the dev info?

-- Steve

> 
> Alan Stern
> 
> 
> 
> Index: usb-3.4/drivers/pci/pci.c
> ===================================================================
> --- usb-3.4.orig/drivers/pci/pci.c
> +++ usb-3.4/drivers/pci/pci.c
> @@ -1720,6 +1720,9 @@ int pci_prepare_to_sleep(struct pci_dev
>  
>  	error = pci_set_power_state(dev, target_state);
>  
> +	dev_info(&dev->dev, "target %d wakeup %d error %d\n",
> +			target_state, device_may_wakeup(&dev->dev), error);
> +
>  	if (error)
>  		pci_enable_wake(dev, target_state, false);
>  

^ permalink raw reply

* Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again
From: Alan Stern @ 2012-04-12 16:09 UTC (permalink / raw)
  To: Andrey Rahmatullin
  Cc: Steven Rostedt, jrnieder-Re5JQEeQqe8AvxtiuMwx3w,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, USB list,
	Greg KH
In-Reply-To: <20120412153750.GA12852-hAV9HEAGFNe6YibBOCjzsw@public.gmane.org>

On Thu, 12 Apr 2012, Andrey Rahmatullin wrote:

> > Here's a diagnostic patch that will give us a little more information.  
> > Keep the previous change (so that pci_prepare_to_sleep gets called 
> > whether ehci-hcd is bound or not) and let's see what it says.  Try 
> > doing it both with and without ehci-hcd bound.  Running this with "echo 
> > devices >/sys/power/pm_test" ought to be good enough.
> I don't see that line with ehci_hcd both bound and unbound and both in
> 'devices' test and in a real S3.

You mean the new dev_info message did not appear at all?  Is
pci_prepare_to_sleep getting called properly?  Or does the routine exit
early because target_state is equal to PCI_POWER_ERROR?

Alan Stern

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

^ permalink raw reply

* Re: ehci_hcd related S3 lockup on ASUS laptops, again
From: Andrey Rahmatullin @ 2012-04-12 15:37 UTC (permalink / raw)
  To: Alan Stern; +Cc: jrnieder, Greg KH, linux-pm, USB list, Steven Rostedt
In-Reply-To: <Pine.LNX.4.44L0.1204121017460.1496-100000@iolanthe.rowland.org>


[-- Attachment #1.1: Type: text/plain, Size: 2194 bytes --]

On Thu, Apr 12, 2012 at 10:28:31AM -0400, Alan Stern wrote:
> > > > > I replaced the hcd_pci_suspend_noirq call with retval=0 and suspend/resume
> > > > > was successful.
> > > >  
> > > > This indicates that your computer doesn't like to suspend while the
> > > > EHCI controllers are in D3hot.  I have no idea why not.
> > > > 
> > > > If you want a really thorough test, try changing 
> > > > drivers/pci/pci-driver.c:pci_pm_suspend_noirq.  In the "if (!pm)" 
> > > > clause, add a call to
> > > > 
> > > > 	pci_prepare_to_sleep(pci_dev);
> > > > 
> > > > just after the pci_save_state line.  Then try suspending with the 
> > > > script enabled (ehci-hcd unbound from the controllers).  If this fails 
> > > > then we'll know it is the source of the trouble.
> > > Looks like it works even with this line added.
> > > 
> > 
> > I reverted the retval change (that worked) and added this line. Put the
> > script back and did a suspend. The suspend and resume worked without
> > issue.
> 
> Hmmm.  This is a situation where the wakeup setting might matter.  Did 
> the /sys/bus/pci/devices/0000:00:1[ad].0/power/wakeup files both 
> contain "enabled" when you ran the test?
Yes.

> Here's a diagnostic patch that will give us a little more information.  
> Keep the previous change (so that pci_prepare_to_sleep gets called 
> whether ehci-hcd is bound or not) and let's see what it says.  Try 
> doing it both with and without ehci-hcd bound.  Running this with "echo 
> devices >/sys/power/pm_test" ought to be good enough.
I don't see that line with ehci_hcd both bound and unbound and both in
'devices' test and in a real S3.

> Index: usb-3.4/drivers/pci/pci.c
> ===================================================================
> --- usb-3.4.orig/drivers/pci/pci.c
> +++ usb-3.4/drivers/pci/pci.c
> @@ -1720,6 +1720,9 @@ int pci_prepare_to_sleep(struct pci_dev
>  
>  	error = pci_set_power_state(dev, target_state);
>  
> +	dev_info(&dev->dev, "target %d wakeup %d error %d\n",
> +			target_state, device_may_wakeup(&dev->dev), error);
> +
>  	if (error)
>  		pci_enable_wake(dev, target_state, false);
>  
> 
> 

-- 
WBR, wRAR

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again
From: Alan Stern @ 2012-04-12 14:28 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andrey Rahmatullin, jrnieder-Re5JQEeQqe8AvxtiuMwx3w,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, USB list,
	Greg KH
In-Reply-To: <1334193773.23924.316.camel-f9ZlEuEWxVcI6MkJdU+c8EEOCMrvLtNR@public.gmane.org>

On Wed, 11 Apr 2012, Steven Rostedt wrote:

> On Thu, 2012-04-12 at 04:09 +0600, Andrey Rahmatullin wrote:
> > On Wed, Apr 11, 2012 at 05:13:51PM -0400, Alan Stern wrote:
> > > > I replaced the hcd_pci_suspend_noirq call with retval=0 and suspend/resume
> > > > was successful.
> > >  
> > > This indicates that your computer doesn't like to suspend while the
> > > EHCI controllers are in D3hot.  I have no idea why not.
> > > 
> > > If you want a really thorough test, try changing 
> > > drivers/pci/pci-driver.c:pci_pm_suspend_noirq.  In the "if (!pm)" 
> > > clause, add a call to
> > > 
> > > 	pci_prepare_to_sleep(pci_dev);
> > > 
> > > just after the pci_save_state line.  Then try suspending with the 
> > > script enabled (ehci-hcd unbound from the controllers).  If this fails 
> > > then we'll know it is the source of the trouble.
> > Looks like it works even with this line added.
> > 
> 
> I reverted the retval change (that worked) and added this line. Put the
> script back and did a suspend. The suspend and resume worked without
> issue.

Hmmm.  This is a situation where the wakeup setting might matter.  Did 
the /sys/bus/pci/devices/0000:00:1[ad].0/power/wakeup files both 
contain "enabled" when you ran the test?

Here's a diagnostic patch that will give us a little more information.  
Keep the previous change (so that pci_prepare_to_sleep gets called 
whether ehci-hcd is bound or not) and let's see what it says.  Try 
doing it both with and without ehci-hcd bound.  Running this with "echo 
devices >/sys/power/pm_test" ought to be good enough.

Alan Stern



Index: usb-3.4/drivers/pci/pci.c
===================================================================
--- usb-3.4.orig/drivers/pci/pci.c
+++ usb-3.4/drivers/pci/pci.c
@@ -1720,6 +1720,9 @@ int pci_prepare_to_sleep(struct pci_dev
 
 	error = pci_set_power_state(dev, target_state);
 
+	dev_info(&dev->dev, "target %d wakeup %d error %d\n",
+			target_state, device_may_wakeup(&dev->dev), error);
+
 	if (error)
 		pci_enable_wake(dev, target_state, false);
 

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

^ permalink raw reply

* Re: ehci_hcd related S3 lockup on ASUS laptops, again
From: Steven Rostedt @ 2012-04-12  1:22 UTC (permalink / raw)
  To: Andrey Rahmatullin; +Cc: jrnieder, Greg KH, linux-pm, USB list
In-Reply-To: <20120411220944.GC3677@belkar.wrar.name>

On Thu, 2012-04-12 at 04:09 +0600, Andrey Rahmatullin wrote:
> On Wed, Apr 11, 2012 at 05:13:51PM -0400, Alan Stern wrote:
> > > I replaced the hcd_pci_suspend_noirq call with retval=0 and suspend/resume
> > > was successful.
> >  
> > This indicates that your computer doesn't like to suspend while the
> > EHCI controllers are in D3hot.  I have no idea why not.
> > 
> > If you want a really thorough test, try changing 
> > drivers/pci/pci-driver.c:pci_pm_suspend_noirq.  In the "if (!pm)" 
> > clause, add a call to
> > 
> > 	pci_prepare_to_sleep(pci_dev);
> > 
> > just after the pci_save_state line.  Then try suspending with the 
> > script enabled (ehci-hcd unbound from the controllers).  If this fails 
> > then we'll know it is the source of the trouble.
> Looks like it works even with this line added.
> 

I reverted the retval change (that worked) and added this line. Put the
script back and did a suspend. The suspend and resume worked without
issue.

Here's my change just in case I did it incorrectly:

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 12d1e81..e026390 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -713,6 +713,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
 
        if (!pm) {
                pci_save_state(pci_dev);
+               pci_prepare_to_sleep(pci_dev);
                return 0;
        }
 
-- Steve

^ permalink raw reply related

* Re: ehci_hcd related S3 lockup on ASUS laptops, again
From: Andrey Rahmatullin @ 2012-04-11 22:09 UTC (permalink / raw)
  To: Alan Stern; +Cc: jrnieder, Greg KH, linux-pm, USB list, Steven Rostedt
In-Reply-To: <Pine.LNX.4.44L0.1204111703180.1351-100000@iolanthe.rowland.org>


[-- Attachment #1.1: Type: text/plain, Size: 755 bytes --]

On Wed, Apr 11, 2012 at 05:13:51PM -0400, Alan Stern wrote:
> > I replaced the hcd_pci_suspend_noirq call with retval=0 and suspend/resume
> > was successful.
>  
> This indicates that your computer doesn't like to suspend while the
> EHCI controllers are in D3hot.  I have no idea why not.
> 
> If you want a really thorough test, try changing 
> drivers/pci/pci-driver.c:pci_pm_suspend_noirq.  In the "if (!pm)" 
> clause, add a call to
> 
> 	pci_prepare_to_sleep(pci_dev);
> 
> just after the pci_save_state line.  Then try suspending with the 
> script enabled (ehci-hcd unbound from the controllers).  If this fails 
> then we'll know it is the source of the trouble.
Looks like it works even with this line added.

-- 
WBR, wRAR

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again
From: Steven Rostedt @ 2012-04-11 21:19 UTC (permalink / raw)
  To: Alan Stern
  Cc: Andrey Rahmatullin, jrnieder-Re5JQEeQqe8AvxtiuMwx3w,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, USB list,
	Greg KH
In-Reply-To: <Pine.LNX.4.44L0.1204111703180.1351-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>

On Wed, 2012-04-11 at 17:13 -0400, Alan Stern wrote:
> On Wed, 11 Apr 2012, Steven Rostedt wrote:
> 
> > On Wed, 2012-04-11 at 15:12 -0400, Alan Stern wrote:
> > 
> > > If so, setting the value back to 0 before suspending (or never setting 
> > > it to 1 in the first place) might be important.  You can test this 
> > > easily enough.  In drivers/usb/host/ehci-pci.c:ehci_pci_suspend(), add 
> > > a line saying
> > > 
> > > 	ehci_writel(ehci, 0, &ehci->regs->configured_flag);
> > > 
> > > just before the spin_lock_irqrestore.  This will invalidate the
> > > driver's criterion for determining whether or not the controller's
> > > state got messed up during the suspend; we can worry about that later.
> > 
> > I just tried the above, and it made no difference. Note, I don't even
> > get to suspend. It locks up in suspend, so I haven't even tried a resume
> > yet.
> 
> What do you mean, you don't get to suspend?  Is that a typo?

I mean, as I hit suspend, the screen goes blank, but the backlight stays
on, and the system just hangs there. It never gets to the point where
the machine is in "suspend" mode. I have to press and hold the power
button to force a shutdown.

> 
> If the system locks up during the suspend procedure, it must do so
> after this code runs.  I know that because it runs during the "devices"  
> phase of suspending, and you said that "echo devices
> >/sys/power/pm_test" works.
> 
> > > You can try getting rid of the call to pci_prepare_to_sleep in 
> > > drivers/usb/core/hcd-pci.c:hcd_pci_suspend_noirq.  This will prevent 
> > > the controller from being put into D3hot and might interfere with 
> > > wakeup detection.
> > > 
> > 
> > What do I do with the retval? -EIO, 0, or other?
> 
> 0 or -EIO, either one.

I set it to 0 and it worked! (was able to suspend and resume)

> 
> 
> On Thu, 12 Apr 2012, Andrey Rahmatullin wrote:
> 
> > I replaced the hcd_pci_suspend_noirq call with retval=0 and suspend/resume
> > was successful.
>  
> This indicates that your computer doesn't like to suspend while the
> EHCI controllers are in D3hot.  I have no idea why not.
> 
> If you want a really thorough test, try changing 
> drivers/pci/pci-driver.c:pci_pm_suspend_noirq.  In the "if (!pm)" 
> clause, add a call to
> 
> 	pci_prepare_to_sleep(pci_dev);
> 
> just after the pci_save_state line.  Then try suspending with the 
> script enabled (ehci-hcd unbound from the controllers).  If this fails 
> then we'll know it is the source of the trouble.

I have to go for a few hours, but I can try this when I get back. Even
if Andrey does it first. I'll verify it it too.

Thanks Alan and Andrey!

-- Steve


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

^ permalink raw reply

* Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again
From: Steven Rostedt @ 2012-04-11 21:15 UTC (permalink / raw)
  To: Andrey Rahmatullin
  Cc: Alan Stern, jrnieder-Re5JQEeQqe8AvxtiuMwx3w,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, greg-U8xfFu+wG4EAvxtiuMwx3w
In-Reply-To: <20120411205204.GB3677-hAV9HEAGFNe6YibBOCjzsw@public.gmane.org>

On Thu, 2012-04-12 at 02:52 +0600, Andrey Rahmatullin wrote:
> On Wed, Apr 11, 2012 at 03:12:13PM -0400, Alan Stern wrote:
> > You can try getting rid of the call to pci_prepare_to_sleep in 
> > drivers/usb/core/hcd-pci.c:hcd_pci_suspend_noirq.  This will prevent 
> > the controller from being put into D3hot and might interfere with 
> > wakeup detection.
> > 
> > I don't know how sigificant the difference is between
> > pci_disable_device and pci_disable_enabled_device.  Probably not very, 
> > since all it involves is whether or not to disable bus-mastering on the 
> > controller.
> I replaced the hcd_pci_suspend_noirq call with retval=0 and suspend/resume
> was successful.
> 

I just did the update and it was successful for me as well.

Seems there's some kind of issue with the hcd_pci_suspend_noirq() call.

Thanks!


-- Steve


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

^ permalink raw reply

* Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again
From: Alan Stern @ 2012-04-11 21:13 UTC (permalink / raw)
  To: Andrey Rahmatullin, Steven Rostedt
  Cc: jrnieder-Re5JQEeQqe8AvxtiuMwx3w,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, USB list,
	Greg KH
In-Reply-To: <1334177035.23924.299.camel-f9ZlEuEWxVcI6MkJdU+c8EEOCMrvLtNR@public.gmane.org>

On Wed, 11 Apr 2012, Steven Rostedt wrote:

> On Wed, 2012-04-11 at 15:12 -0400, Alan Stern wrote:
> 
> > If so, setting the value back to 0 before suspending (or never setting 
> > it to 1 in the first place) might be important.  You can test this 
> > easily enough.  In drivers/usb/host/ehci-pci.c:ehci_pci_suspend(), add 
> > a line saying
> > 
> > 	ehci_writel(ehci, 0, &ehci->regs->configured_flag);
> > 
> > just before the spin_lock_irqrestore.  This will invalidate the
> > driver's criterion for determining whether or not the controller's
> > state got messed up during the suspend; we can worry about that later.
> 
> I just tried the above, and it made no difference. Note, I don't even
> get to suspend. It locks up in suspend, so I haven't even tried a resume
> yet.

What do you mean, you don't get to suspend?  Is that a typo?

If the system locks up during the suspend procedure, it must do so
after this code runs.  I know that because it runs during the "devices"  
phase of suspending, and you said that "echo devices
>/sys/power/pm_test" works.

> > You can try getting rid of the call to pci_prepare_to_sleep in 
> > drivers/usb/core/hcd-pci.c:hcd_pci_suspend_noirq.  This will prevent 
> > the controller from being put into D3hot and might interfere with 
> > wakeup detection.
> > 
> 
> What do I do with the retval? -EIO, 0, or other?

0 or -EIO, either one.


On Thu, 12 Apr 2012, Andrey Rahmatullin wrote:

> I replaced the hcd_pci_suspend_noirq call with retval=0 and suspend/resume
> was successful.
 
This indicates that your computer doesn't like to suspend while the
EHCI controllers are in D3hot.  I have no idea why not.

If you want a really thorough test, try changing 
drivers/pci/pci-driver.c:pci_pm_suspend_noirq.  In the "if (!pm)" 
clause, add a call to

	pci_prepare_to_sleep(pci_dev);

just after the pci_save_state line.  Then try suspending with the 
script enabled (ehci-hcd unbound from the controllers).  If this fails 
then we'll know it is the source of the trouble.

Alan Stern

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

^ permalink raw reply

* Re: ehci_hcd related S3 lockup on ASUS laptops, again
From: Andrey Rahmatullin @ 2012-04-11 20:52 UTC (permalink / raw)
  To: Alan Stern; +Cc: jrnieder, greg, linux-pm, linux-usb, Steven Rostedt
In-Reply-To: <Pine.LNX.4.44L0.1204111429510.1351-100000@iolanthe.rowland.org>


[-- Attachment #1.1: Type: text/plain, Size: 630 bytes --]

On Wed, Apr 11, 2012 at 03:12:13PM -0400, Alan Stern wrote:
> You can try getting rid of the call to pci_prepare_to_sleep in 
> drivers/usb/core/hcd-pci.c:hcd_pci_suspend_noirq.  This will prevent 
> the controller from being put into D3hot and might interfere with 
> wakeup detection.
> 
> I don't know how sigificant the difference is between
> pci_disable_device and pci_disable_enabled_device.  Probably not very, 
> since all it involves is whether or not to disable bus-mastering on the 
> controller.
I replaced the hcd_pci_suspend_noirq call with retval=0 and suspend/resume
was successful.

-- 
WBR, wRAR

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again
From: Steven Rostedt @ 2012-04-11 20:43 UTC (permalink / raw)
  To: Alan Stern
  Cc: jrnieder-Re5JQEeQqe8AvxtiuMwx3w, Andrey Rahmatullin,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, greg-U8xfFu+wG4EAvxtiuMwx3w
In-Reply-To: <Pine.LNX.4.44L0.1204111429510.1351-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>

On Wed, 2012-04-11 at 15:12 -0400, Alan Stern wrote:

> If so, setting the value back to 0 before suspending (or never setting 
> it to 1 in the first place) might be important.  You can test this 
> easily enough.  In drivers/usb/host/ehci-pci.c:ehci_pci_suspend(), add 
> a line saying
> 
> 	ehci_writel(ehci, 0, &ehci->regs->configured_flag);
> 
> just before the spin_lock_irqrestore.  This will invalidate the
> driver's criterion for determining whether or not the controller's
> state got messed up during the suspend; we can worry about that later.

I just tried the above, and it made no difference. Note, I don't even
get to suspend. It locks up in suspend, so I haven't even tried a resume
yet.

> 
> 
> There are other differences, at the PCI level, that might also be 
> significant.  When the driver is not present, the PCI core calls 
> pci_disable_enabled_device.  But when the driver is loaded, instead 
> it calls pci_disable_device and pci_prepare_to_sleep.
> 
> You can try getting rid of the call to pci_prepare_to_sleep in 
> drivers/usb/core/hcd-pci.c:hcd_pci_suspend_noirq.  This will prevent 
> the controller from being put into D3hot and might interfere with 
> wakeup detection.
> 

What do I do with the retval? -EIO, 0, or other?

-- Steve

> I don't know how sigificant the difference is between
> pci_disable_device and pci_disable_enabled_device.  Probably not very, 
> since all it involves is whether or not to disable bus-mastering on the 
> controller.
> 
> Alan Stern


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

^ permalink raw reply

* Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again
From: Alan Stern @ 2012-04-11 19:12 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: jrnieder-Re5JQEeQqe8AvxtiuMwx3w, Andrey Rahmatullin,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, greg-U8xfFu+wG4EAvxtiuMwx3w
In-Reply-To: <Pine.LNX.4.44L0.1204111324100.1351-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>

On Wed, 11 Apr 2012, Alan Stern wrote:

> On Wed, 11 Apr 2012, Steven Rostedt wrote:

> > I'm willing to test fixes, as the issue still exists for my laptop if I
> > remove the script.
> 
> It would be great if we had something to try out.  So far I'm not aware
> of any suggestions from anybody as to the underlying cause of the
> problem or how to fix it.
> 
> Ideas welcome.

All right, here are some ideas.  As far as I can tell, there's only a
handful of differences in the EHCI hardware state between a normal
suspend and a suspend in which ehci-hcd has been removed or unbound.

There are several differences in the controller registers that I
seriously doubt will have any effect.  These are things like the
ASYNCLISTADDR register, which contains the address of the start of the
async schedule when the driver is present and 0 when the driver is not 
present.  These registers are ignored when the controller isn't 
actively running.

There's also the port status & control register for port 1.  When the
driver is present, this register indicate that the port is enabled
(it's connected to the built-in "rate-matching" hub), whereas without
the driver the port is disabled.  We could test whether disabling the
port makes any difference, but I hope it doesn't -- disabling the port
has the effect of logically disconnecting everything that was connected
to the controller.

Finally there's one more thing: the CONFIGFLAG register.  Without the
driver this register contains 0, and the driver sets it to 1.  For the
Intel chipset you're using this shouldn't do anything, because this bit
controls port routing between the EHCI controller and the companion
controller, and your chipset has no companion controller.  I haven't
looked at the datasheet for Intel's Cougar Point chipset, and it's
possible they have repurposed this register.

If so, setting the value back to 0 before suspending (or never setting 
it to 1 in the first place) might be important.  You can test this 
easily enough.  In drivers/usb/host/ehci-pci.c:ehci_pci_suspend(), add 
a line saying

	ehci_writel(ehci, 0, &ehci->regs->configured_flag);

just before the spin_lock_irqrestore.  This will invalidate the
driver's criterion for determining whether or not the controller's
state got messed up during the suspend; we can worry about that later.


There are other differences, at the PCI level, that might also be 
significant.  When the driver is not present, the PCI core calls 
pci_disable_enabled_device.  But when the driver is loaded, instead 
it calls pci_disable_device and pci_prepare_to_sleep.

You can try getting rid of the call to pci_prepare_to_sleep in 
drivers/usb/core/hcd-pci.c:hcd_pci_suspend_noirq.  This will prevent 
the controller from being put into D3hot and might interfere with 
wakeup detection.

I don't know how sigificant the difference is between
pci_disable_device and pci_disable_enabled_device.  Probably not very, 
since all it involves is whether or not to disable bus-mastering on the 
controller.

Alan Stern

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

^ permalink raw reply

* Re: ehci_hcd related S3 lockup on ASUS laptops, again
From: Alan Stern @ 2012-04-11 17:25 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andrey Rahmatullin, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	jrnieder-Re5JQEeQqe8AvxtiuMwx3w, greg-U8xfFu+wG4EAvxtiuMwx3w
In-Reply-To: <1334164013.23924.271.camel-f9ZlEuEWxVcI6MkJdU+c8EEOCMrvLtNR@public.gmane.org>

On Wed, 11 Apr 2012, Steven Rostedt wrote:

> On Wed, 2012-04-11 at 22:55 +0600, Andrey Rahmatullin wrote:
> > On many ASUS laptops and probably on some non-ASUS ones you need to unload
> > ehci_hcd or unbind both USB controllers from it before entering S3, else
> > the system will lockup. Here are some links:
> > 
> > http://thread.gmane.org/gmane.linux.kernel/1222803
> > https://bugzilla.kernel.org/show_bug.cgi?id=37632
> > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=658778
> > http://ubuntuforums.org/showthread.php?t=1444822
> > 
> > This still happens on my K53E on v3.4-rc2-16-ga9e1e53.
> > 
> > Unbinding just one of two controllers doesn't help.
> > 
> > Disabling /sys/bus/pci/devices/0000:00:1[ad].0/power/wakeup doesn't help.
> > echo mem>/sys/power/state doesn't hang with 'core' in /sys/power/pm_test,
> > only with 'none'.
> > 
> > Windows 7 enters S3 perfectly well.
> > 
> > I didn't see any other suggestions how to debug this. I can provide any
> > debug data if that will help. This seems to be a very important problem
> > for owners of certain devices.
> > 
> 
> Yeah, I never got a real fix. I'm still using the script that removes
> the driver and adds it back during the suspend/resume sequence. That
> seems to be a working work-around for me.
> 
> That said, I would love to have this fixed for real. Not just for me,
> but for anyone else that is suffering from the same issue. I'm a kernel
> developer and can easily include work arounds like this. But for anyone
> else, this is a total fail for Linux in general.
> 
> I'm willing to test fixes, as the issue still exists for my laptop if I
> remove the script.

It would be great if we had something to try out.  So far I'm not aware
of any suggestions from anybody as to the underlying cause of the
problem or how to fix it.

Ideas welcome.

Alan Stern

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

^ permalink raw reply

* Re: ehci_hcd related S3 lockup on ASUS laptops, again
From: Steven Rostedt @ 2012-04-11 17:06 UTC (permalink / raw)
  To: Andrey Rahmatullin
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	jrnieder-Re5JQEeQqe8AvxtiuMwx3w, greg-U8xfFu+wG4EAvxtiuMwx3w
In-Reply-To: <20120411165531.GA3717-hAV9HEAGFNe6YibBOCjzsw@public.gmane.org>

On Wed, 2012-04-11 at 22:55 +0600, Andrey Rahmatullin wrote:
> On many ASUS laptops and probably on some non-ASUS ones you need to unload
> ehci_hcd or unbind both USB controllers from it before entering S3, else
> the system will lockup. Here are some links:
> 
> http://thread.gmane.org/gmane.linux.kernel/1222803
> https://bugzilla.kernel.org/show_bug.cgi?id=37632
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=658778
> http://ubuntuforums.org/showthread.php?t=1444822
> 
> This still happens on my K53E on v3.4-rc2-16-ga9e1e53.
> 
> Unbinding just one of two controllers doesn't help.
> 
> Disabling /sys/bus/pci/devices/0000:00:1[ad].0/power/wakeup doesn't help.
> echo mem>/sys/power/state doesn't hang with 'core' in /sys/power/pm_test,
> only with 'none'.
> 
> Windows 7 enters S3 perfectly well.
> 
> I didn't see any other suggestions how to debug this. I can provide any
> debug data if that will help. This seems to be a very important problem
> for owners of certain devices.
> 

Yeah, I never got a real fix. I'm still using the script that removes
the driver and adds it back during the suspend/resume sequence. That
seems to be a working work-around for me.

That said, I would love to have this fixed for real. Not just for me,
but for anyone else that is suffering from the same issue. I'm a kernel
developer and can easily include work arounds like this. But for anyone
else, this is a total fail for Linux in general.

I'm willing to test fixes, as the issue still exists for my laptop if I
remove the script.

-- Steve


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

^ permalink raw reply

* ehci_hcd related S3 lockup on ASUS laptops, again
From: Andrey Rahmatullin @ 2012-04-11 16:55 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: jrnieder-Re5JQEeQqe8AvxtiuMwx3w, greg-U8xfFu+wG4EAvxtiuMwx3w,
	rostedt-nx8X9YLhiw1AfugRpC6u6w

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

On many ASUS laptops and probably on some non-ASUS ones you need to unload
ehci_hcd or unbind both USB controllers from it before entering S3, else
the system will lockup. Here are some links:

http://thread.gmane.org/gmane.linux.kernel/1222803
https://bugzilla.kernel.org/show_bug.cgi?id=37632
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=658778
http://ubuntuforums.org/showthread.php?t=1444822

This still happens on my K53E on v3.4-rc2-16-ga9e1e53.

Unbinding just one of two controllers doesn't help.

Disabling /sys/bus/pci/devices/0000:00:1[ad].0/power/wakeup doesn't help.
echo mem>/sys/power/state doesn't hang with 'core' in /sys/power/pm_test,
only with 'none'.

Windows 7 enters S3 perfectly well.

I didn't see any other suggestions how to debug this. I can provide any
debug data if that will help. This seems to be a very important problem
for owners of certain devices.

-- 
WBR, wRAR

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH V4]mmc: remove MMC bus legacy suspend/resume method
From: Chris Ball @ 2012-04-11 14:44 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Chuanxiao Dong, linux-mmc@vger.kernel.org,
	linux-pm@lists.linux-foundation.org
In-Reply-To: <4F858A25.20302@stericsson.com>

Hi,

On Wed, Apr 11 2012, Ulf Hansson wrote:
> On 04/11/2012 01:54 PM, Chuanxiao Dong wrote:
>> MMC bus is using legacy suspend/resume method, which is not compatible if
>> runtime pm callbacks are used. In this scenario, MMC bus suspend/resume
>> callbacks cannot be called when system entering S3. So change to use the new
>> defined dev_pm_ops for system sleeping mode
>>
>> Signed-off-by: Chuanxiao Dong<chuanxiao.dong@intel.com>
>> ---
>> Changes in v2:
>> 	use SET_SYSTEM_SLEEP_PM_OPS to define sleep callbacks as Rafael
>> 	suggested
>>
>> Changes in v3:
>> 	remove NULL pointer define for runtime callbacks when PM_RUNTIME is not
>> 	selected, as Ulf Hansson suggested
>>
>> Changes in v4:
>> 	fix the warning when disable CONFIG_PM_RUNTIME
>>
>
> Looks good to me! Acked-by: Ulf Hansson <ulf.hansson@stericsson.com>

Thanks!  Pushed to mmc-next for 3.4.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

^ permalink raw reply

* Re: [RFC] Runtime PM for host controllers
From: Alan Stern @ 2012-04-11 14:30 UTC (permalink / raw)
  To: Kevin Cernekee; +Cc: linux-pm
In-Reply-To: <CAJiQ=7A=744EZ1pW0vmpW=+CgdoDBYT0vc-=182Y8eU-3pZqvg@mail.gmail.com>

On Tue, 10 Apr 2012, Kevin Cernekee wrote:

> > PCI devices have two different power wells, one for normal use and one
> > for use during suspend.  As far as I know, there is no way to turn off
> > power to the auxiliary well short of turning off the entire computer.
> > Rafael may have a more complete picture.
> 
> That may be the case on PCs,

It should be true on any system -- PC, SoC, or anything else -- that 
uses PCI.  But again, my knowledge is limited.

>  but on many embedded systems (mine
> included), entire hardware blocks can be clock-gated or power-gated
> without shutting off the rest of the system.

Then why isn't there a way to clock-gate or power-gate the parts of a 
hardware block used for data communication while leaving the circuits 
involved in wakeup detection active?  Such a capability is part of the 
EHCI specification.

> Going back to the NAS example I mentioned earlier - I would imagine
> that merely resuming the USB/SATA controller + HDD when a new request
> arrives is going to provide a much faster recovery time than rebinding
> + redetecting + remounting.

Certainly.  And leaving the USB controller suspended but powered up
would be even faster.

Furthermore, what happens when the user plugs in a new USB device, or
unplugs one that is already attached?  The USB specification is very
clear about what device suspend entails; completely turning off the
power violates the spec.

> (Of course that raises issues like: what if somebody unplugs the
> current HDD, then plugs in a different HDD, while the USB/SATA core is
> asleep?)

Right.  Although we do try to detect that sort of thing, since it can
also happen during hibernation.  But our detection methods are not 100%
accurate.

Alan Stern

^ permalink raw reply

* Re: [PATCH V4]mmc: remove MMC bus legacy suspend/resume method
From: Ulf Hansson @ 2012-04-11 13:41 UTC (permalink / raw)
  To: Chuanxiao Dong
  Cc: linux-mmc@vger.kernel.org, linux-pm@lists.linux-foundation.org,
	cjb@laptop.org
In-Reply-To: <20120411115438.GA303@intel.com>

On 04/11/2012 01:54 PM, Chuanxiao Dong wrote:
> MMC bus is using legacy suspend/resume method, which is not compatible if
> runtime pm callbacks are used. In this scenario, MMC bus suspend/resume
> callbacks cannot be called when system entering S3. So change to use the new
> defined dev_pm_ops for system sleeping mode
>
> Signed-off-by: Chuanxiao Dong<chuanxiao.dong@intel.com>
> ---
> Changes in v2:
> 	use SET_SYSTEM_SLEEP_PM_OPS to define sleep callbacks as Rafael
> 	suggested
>
> Changes in v3:
> 	remove NULL pointer define for runtime callbacks when PM_RUNTIME is not
> 	selected, as Ulf Hansson suggested
>
> Changes in v4:
> 	fix the warning when disable CONFIG_PM_RUNTIME
>

Looks good to me! Acked-by: Ulf Hansson <ulf.hansson@stericsson.com>

Kind regards
Ulf Hansson

^ permalink raw reply

* Re: [PATCH V2 0/6] thermal: exynos: Add kernel thermal support for exynos platform
From: Amit Kachhap @ 2012-04-11 12:47 UTC (permalink / raw)
  To: Zhang Rui
  Cc: linux-samsung-soc, linaro-dev, patches, linux-kernel, lm-sensors,
	linux-acpi, linux-pm
In-Reply-To: <1334019502.2387.211.camel@rui.sh.intel.com>

Hi Rui,

Thanks for looking into the patches.

On 10 April 2012 06:28, Zhang Rui <rui.zhang@intel.com> wrote:
> Hi, Amit,
>
> On 三, 2012-04-04 at 10:02 +0530, Amit Kachhap wrote:
>> Hi Len/Rui,
>>
>> Any comment or feedback from your side about the status of this patch?
>> Is it merge-able or major re-work is needed? I have fixed most of the
>> comments in this patchset and currently working on some of the minor
>> comments received and will submit them shortly.
>>
> Sorry for the late response.
>
> First of all, it makes sense to me to introduce the generic cpufrq
> cooling implementation.
ok thanks
> But I still have some questions.
> I think the key reason why THERMAL_TRIP_STATE_INSTANCE is introduced is
> that the MONIROR_ZONE and WARN_ZONE on exynos4 can not fit into the
> current passive handling in the generic thermal layer well, right?
> e.g. there is no tc1/tc2 on exynos4.
>
> If yes, is it possible that we can enhance the passive cooling to
> support the generic processor cooling?
> say, introduce another way to throttle the processor in
> thermal_zone_device_passive when tc1 and tc2 are not available?

I agree that this new trip type code can be moved into passive trip
type when tc1 and tc2 are 0. but this is special type of cooling
devices behaviour where only instances of the same cooling device is
binded to a trip point. The order of mapping is the only
differentiating criteria and there are some checks used to implement
this like
1) The trip points should be in ascending order.(This is missing in my
original patch, so I added below)
2) The set_cur_state has to be called for the exact temp range so
get_cur_state(&state) and set_cur_state(state ++/state--) logic is not
possible.
3) set_cur_state is called as set_cur_state(cdev_instance).
There is a chance that people might confuse that these checks are
applicable for passive trip types also which is not the case here.

@@ -1187,6 +1228,21 @@ struct thermal_zone_device
*thermal_zone_device_register(char *type,
                tz->ops->get_trip_type(tz, count, &trip_type);
                if (trip_type == THERMAL_TRIP_PASSIVE)
                        passive = 1;
+               /*
+                * For THERMAL_TRIP_STATE_INSTANCE trips, thermal zone should
+                * be in ascending order.
+               */
+               if (trip_type == THERMAL_TRIP_STATE_INSTANCE) {
+                       tz->ops->get_trip_temp(tz, count, &trip_temp);
+                       if (first_trip_temp == 0)
+                               first_trip_temp = trip_temp;
+                       else if (first_trip_temp < trip_temp)
+                               first_trip_temp = trip_temp;
+                       else if (first_trip_temp > trip_temp) {
+                               pr_warn("Zone trip points should be in
ascending order\n");
+                               goto unregister;
+                       }
+               }
        }

        if (!passive)

Anyway there is other alternative where this new trip type is not
needed and I can just use the existing trip type THERMAL_TRIP_ACTIVE
and create 2 separate cooling devices for MONITOR_ZONE and WARN_ZONE.
I had thought to make this generic and just to manage with 1 cooling
device.
What is your view?

Thanks,
Amit Daniel


>
> thanks,
> rui
>
>> Regards,
>> Amit Daniel
>>
>> On 19 March 2012 11:47, Amit Daniel Kachhap <amit.kachhap@linaro.org> wrote:
>> > Changes since V1:
>> > *Moved the sensor driver to driver/thermal folder from driver/hwmon folder
>> >  as suggested by Mark Brown and Guenter Roeck
>> > *Added notifier support to notify the registered drivers of any cpu cooling
>> >  action. The driver can modify the default cooling behaviour(eg set different
>> >  max clip frequency).
>> > *The percentage based frequency replaced with absolute clipped frequency.
>> > *Some more conditional checks when setting max frequency.
>> > *Renamed the new trip type THERMAL_TRIP_STATE_ACTIVE to
>> >  THERMAL_TRIP_STATE_INSTANCE
>> > *Many review comments from R, Durgadoss <durgadoss.r@intel.com> and
>> >  eduardo.valentin@ti.com implemented.
>> > *Removed cooling stats through debugfs patch
>> > *The V1 based can be found here,
>> >  https://lkml.org/lkml/2012/2/22/123
>> >  http://lkml.org/lkml/2012/3/3/32
>> >
>> > Changes since RFC:
>> > *Changed the cpu cooling registration/unregistration API's to instance based
>> > *Changed the STATE_ACTIVE trip type to pass correct instance id
>> > *Adding support to restore back the policy->max_freq after doing frequency
>> >  clipping.
>> > *Moved the trip cooling stats from sysfs node to debugfs node as suggested
>> >  by Greg KH greg@kroah.com
>> > *Incorporated several review comments from eduardo.valentin@ti.com
>> > *Moved the Temperature sensor driver from driver/hwmon/ to driver/mfd
>> >  as discussed with Guenter Roeck <guenter.roeck@ericsson.com> and
>> >  Donggeun Kim <dg77.kim@samsung.com> (https://lkml.org/lkml/2012/1/5/7)
>> > *Some changes according to the changes in common cpu cooling APIs
>> > *The RFC based patches can be found here,
>> >  https://lkml.org/lkml/2011/12/13/186
>> >  https://lkml.org/lkml/2011/12/21/169
>> >
>> >
>> > Brief Description:
>> >
>> > 1) The generic cooling devices code is placed inside driver/thermal/* as
>> > placing inside acpi folder will need un-necessary enabling of acpi code. This
>> > codes is architecture independent.
>> >
>> > 2) This patchset adds a new trip type THERMAL_TRIP_STATE_INSTANCE which passes
>> > cooling device instance number and may be helpful for cpufreq cooling devices
>> > to take the correct cooling action. This trip type avoids the temperature
>> > comparision check again inside the cooling handler.
>> >
>> > 3) This patchset adds generic cpu cooling low level implementation through
>> > frequency clipping and cpu hotplug. In future, other cpu related cooling
>> > devices may be added here. An ACPI version of this already exists
>> > (drivers/acpi/processor_thermal.c). But this will be useful for platforms
>> > like ARM using the generic thermal interface along with the generic cpu
>> > cooling devices. The cooling device registration API's return cooling device
>> > pointers which can be easily binded with the thermal zone trip points.
>> > The important APIs exposed are,
>> >   a)struct thermal_cooling_device *cpufreq_cooling_register(
>> >        struct freq_clip_table *tab_ptr, unsigned int tab_size,
>> >        const struct cpumask *mask_val)
>> >   b)void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>> >
>> > 4) Samsung exynos platform thermal implementation is done using the generic
>> > cpu cooling APIs and the new trip type. The temperature sensor driver present
>> > in the hwmon folder(registered as hwmon driver) is moved to thermal folder
>> > and registered as a thermal driver.
>> >
>> > All this patchset is based on Kernel version 3.3-rc7
>> >
>> > A simple data/control flow diagrams is shown below,
>> >
>> > Core Linux thermal <----->  Exynos thermal interface <----- Temperature Sensor
>> >          |                             |
>> >         \|/                            |
>> >  Cpufreq cooling device <---------------
>> >
>> >
>> > Amit Daniel Kachhap (6):
>> >  thermal: Add a new trip type to use cooling device instance number
>> >  thermal: Add generic cpufreq cooling implementation
>> >  thermal: Add generic cpuhotplug cooling implementation
>> >  hwmon: exynos4: Move thermal sensor driver to driver/thermal
>> >    directory
>> >  thermal: exynos4: Register the tmu sensor with the kernel thermal
>> >    layer
>> >  ARM: exynos4: Add thermal sensor driver platform device support
>> >
>> >  Documentation/hwmon/exynos4_tmu           |   81 ---
>> >  Documentation/thermal/cpu-cooling-api.txt |   76 +++
>> >  Documentation/thermal/exynos4_tmu         |   52 ++
>> >  Documentation/thermal/sysfs-api.txt       |    4 +-
>> >  arch/arm/mach-exynos/Kconfig              |   11 +
>> >  arch/arm/mach-exynos/Makefile             |    1 +
>> >  arch/arm/mach-exynos/clock.c              |    4 +
>> >  arch/arm/mach-exynos/dev-tmu.c            |   39 ++
>> >  arch/arm/mach-exynos/include/mach/irqs.h  |    2 +
>> >  arch/arm/mach-exynos/include/mach/map.h   |    1 +
>> >  arch/arm/mach-exynos/mach-origen.c        |    1 +
>> >  arch/arm/plat-samsung/include/plat/devs.h |    1 +
>> >  drivers/hwmon/Kconfig                     |   10 -
>> >  drivers/hwmon/Makefile                    |    1 -
>> >  drivers/hwmon/exynos4_tmu.c               |  514 -------------------
>> >  drivers/thermal/Kconfig                   |   21 +
>> >  drivers/thermal/Makefile                  |    2 +
>> >  drivers/thermal/cpu_cooling.c             |  529 +++++++++++++++++++
>> >  drivers/thermal/exynos4_thermal.c         |  790 +++++++++++++++++++++++++++++
>> >  drivers/thermal/thermal_sys.c             |   45 ++-
>> >  include/linux/cpu_cooling.h               |   78 +++
>> >  include/linux/platform_data/exynos4_tmu.h |    7 +
>> >  include/linux/thermal.h                   |    1 +
>> >  23 files changed, 1660 insertions(+), 611 deletions(-)
>> >  delete mode 100644 Documentation/hwmon/exynos4_tmu
>> >  create mode 100644 Documentation/thermal/cpu-cooling-api.txt
>> >  create mode 100644 Documentation/thermal/exynos4_tmu
>> >  create mode 100644 arch/arm/mach-exynos/dev-tmu.c
>> >  delete mode 100644 drivers/hwmon/exynos4_tmu.c
>> >  create mode 100644 drivers/thermal/cpu_cooling.c
>> >  create mode 100644 drivers/thermal/exynos4_thermal.c
>> >  create mode 100644 include/linux/cpu_cooling.h
>> >
>
>
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

^ permalink raw reply

* [PATCH V4]mmc: remove MMC bus legacy suspend/resume method
From: Chuanxiao Dong @ 2012-04-11 11:54 UTC (permalink / raw)
  To: linux-mmc, linux-pm; +Cc: cjb, ulf.hansson

MMC bus is using legacy suspend/resume method, which is not compatible if
runtime pm callbacks are used. In this scenario, MMC bus suspend/resume
callbacks cannot be called when system entering S3. So change to use the new
defined dev_pm_ops for system sleeping mode

Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
Changes in v2:
	use SET_SYSTEM_SLEEP_PM_OPS to define sleep callbacks as Rafael
	suggested

Changes in v3:
	remove NULL pointer define for runtime callbacks when PM_RUNTIME is not
	selected, as Ulf Hansson suggested

Changes in v4:
	fix the warning when disable CONFIG_PM_RUNTIME

 drivers/mmc/card/block.c |    2 +-
 drivers/mmc/core/bus.c   |   24 ++++++++----------------
 include/linux/mmc/card.h |    2 +-
 3 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index f2020d3..3582c03 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1826,7 +1826,7 @@ static void mmc_blk_remove(struct mmc_card *card)
 }
 
 #ifdef CONFIG_PM
-static int mmc_blk_suspend(struct mmc_card *card, pm_message_t state)
+static int mmc_blk_suspend(struct mmc_card *card)
 {
 	struct mmc_blk_data *part_md;
 	struct mmc_blk_data *md = mmc_get_drvdata(card);
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 5d011a3..aad8516 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -122,14 +122,14 @@ static int mmc_bus_remove(struct device *dev)
 	return 0;
 }
 
-static int mmc_bus_suspend(struct device *dev, pm_message_t state)
+static int mmc_bus_suspend(struct device *dev)
 {
 	struct mmc_driver *drv = to_mmc_driver(dev->driver);
 	struct mmc_card *card = mmc_dev_to_card(dev);
 	int ret = 0;
 
 	if (dev->driver && drv->suspend)
-		ret = drv->suspend(card, state);
+		ret = drv->suspend(card);
 	return ret;
 }
 
@@ -165,20 +165,14 @@ static int mmc_runtime_idle(struct device *dev)
 	return pm_runtime_suspend(dev);
 }
 
+#endif /* !CONFIG_PM_RUNTIME */
+
 static const struct dev_pm_ops mmc_bus_pm_ops = {
-	.runtime_suspend	= mmc_runtime_suspend,
-	.runtime_resume		= mmc_runtime_resume,
-	.runtime_idle		= mmc_runtime_idle,
+	SET_RUNTIME_PM_OPS(mmc_runtime_suspend, mmc_runtime_resume,
+			mmc_runtime_idle)
+	SET_SYSTEM_SLEEP_PM_OPS(mmc_bus_suspend, mmc_bus_resume)
 };
 
-#define MMC_PM_OPS_PTR	(&mmc_bus_pm_ops)
-
-#else /* !CONFIG_PM_RUNTIME */
-
-#define MMC_PM_OPS_PTR	NULL
-
-#endif /* !CONFIG_PM_RUNTIME */
-
 static struct bus_type mmc_bus_type = {
 	.name		= "mmc",
 	.dev_attrs	= mmc_dev_attrs,
@@ -186,9 +180,7 @@ static struct bus_type mmc_bus_type = {
 	.uevent		= mmc_bus_uevent,
 	.probe		= mmc_bus_probe,
 	.remove		= mmc_bus_remove,
-	.suspend	= mmc_bus_suspend,
-	.resume		= mmc_bus_resume,
-	.pm		= MMC_PM_OPS_PTR,
+	.pm		= &mmc_bus_pm_ops,
 };
 
 int mmc_register_bus(void)
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 1a1ca71..c984c9a 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -480,7 +480,7 @@ struct mmc_driver {
 	struct device_driver drv;
 	int (*probe)(struct mmc_card *);
 	void (*remove)(struct mmc_card *);
-	int (*suspend)(struct mmc_card *, pm_message_t);
+	int (*suspend)(struct mmc_card *);
 	int (*resume)(struct mmc_card *);
 };
 
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH V3]mmc: remove MMC bus legacy suspend/resume method
From: Dong, Chuanxiao @ 2012-04-11 11:53 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: linux-pm@lists.linux-foundation.org, cjb@laptop.org,
	linux-mmc@vger.kernel.org
In-Reply-To: <4F844494.9070109@stericsson.com>



> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@stericsson.com]
> Sent: Tuesday, April 10, 2012 10:33 PM
> To: Dong, Chuanxiao
> Cc: linux-mmc@vger.kernel.org; linux-pm@lists.linux-foundation.org;
> cjb@laptop.org; linus.walleij@linaro.org; rjw@sisk.pl
> Subject: Re: [PATCH V3]mmc: remove MMC bus legacy suspend/resume method
> 
> On 04/10/2012 04:07 PM, Chuanxiao Dong wrote:
> > MMC bus is using legacy suspend/resume method, which is not compatible
> > if runtime pm callbacks are used. In this scenario, MMC bus
> > suspend/resume callbacks cannot be called when system entering S3. So
> > change to use the new defined dev_pm_ops for system sleeping mode
> >
> > Signed-off-by: Chuanxiao Dong<chuanxiao.dong@intel.com>
> > ---
> > Changes in v2:
> > 	use SET_SYSTEM_SLEEP_PM_OPS to define sleep callbacks as Rafael
> > 	suggested
> >
> > Changes in v3:
> > 	remove NULL pointer define for runtime callbacks when PM_RUNTIME is not
> > 	selected, as Ulf Hansson suggested
> >
> >   drivers/mmc/card/block.c |    2 +-
> >   drivers/mmc/core/bus.c   |   24 ++++++------------------
> >   include/linux/mmc/card.h |    2 +-
> >   3 files changed, 8 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index
> > f2020d3..3582c03 100644
> > --- a/drivers/mmc/card/block.c
> > +++ b/drivers/mmc/card/block.c
> > @@ -1826,7 +1826,7 @@ static void mmc_blk_remove(struct mmc_card *card)
> >   }
> >
> >   #ifdef CONFIG_PM
> > -static int mmc_blk_suspend(struct mmc_card *card, pm_message_t state)
> > +static int mmc_blk_suspend(struct mmc_card *card)
> >   {
> >   	struct mmc_blk_data *part_md;
> >   	struct mmc_blk_data *md = mmc_get_drvdata(card); diff --git
> > a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c index
> > 5d011a3..1996c9b 100644
> > --- a/drivers/mmc/core/bus.c
> > +++ b/drivers/mmc/core/bus.c
> > @@ -122,14 +122,14 @@ static int mmc_bus_remove(struct device *dev)
> >   	return 0;
> >   }
> >
> > -static int mmc_bus_suspend(struct device *dev, pm_message_t state)
> > +static int mmc_bus_suspend(struct device *dev)
> >   {
> >   	struct mmc_driver *drv = to_mmc_driver(dev->driver);
> >   	struct mmc_card *card = mmc_dev_to_card(dev);
> >   	int ret = 0;
> >
> >   	if (dev->driver&&  drv->suspend)
> > -		ret = drv->suspend(card, state);
> > +		ret = drv->suspend(card);
> >   	return ret;
> >   }
> >
> > @@ -144,8 +144,6 @@ static int mmc_bus_resume(struct device *dev)
> >   	return ret;
> >   }
> >
> > -#ifdef CONFIG_PM_RUNTIME
> 
> I don't think this "idfef" shall removed. Won't that trigger compiler warnings
> when !CONFIG_PM_RUNTIME?
Yes, missed this. Will fix it. Thanks a lot!

Thanks
Chuanxiao

> 
> Just remove these three lines from patch v2...
> 
> +#define mmc_runtime_suspend	NULL
> +#define mmc_runtime_resume	NULL
> +#define mmc_runtime_idle	NULL
> 
> ...and thus the "else" to the CONFIG_PM_RUNTIME, as done below.
> 
> > -
> >   static int mmc_runtime_suspend(struct device *dev)
> >   {
> >   	struct mmc_card *card = mmc_dev_to_card(dev); @@ -166,19 +164,11
> @@
> > static int mmc_runtime_idle(struct device *dev)
> >   }
> >
> >   static const struct dev_pm_ops mmc_bus_pm_ops = {
> > -	.runtime_suspend	= mmc_runtime_suspend,
> > -	.runtime_resume		= mmc_runtime_resume,
> > -	.runtime_idle		= mmc_runtime_idle,
> > +	SET_RUNTIME_PM_OPS(mmc_runtime_suspend, mmc_runtime_resume,
> > +			mmc_runtime_idle)
> > +	SET_SYSTEM_SLEEP_PM_OPS(mmc_bus_suspend, mmc_bus_resume)
> >   };
> >
> > -#define MMC_PM_OPS_PTR	(&mmc_bus_pm_ops)
> > -
> > -#else /* !CONFIG_PM_RUNTIME */
> > -
> > -#define MMC_PM_OPS_PTR	NULL
> > -
> > -#endif /* !CONFIG_PM_RUNTIME */
> > -
> >   static struct bus_type mmc_bus_type = {
> >   	.name		= "mmc",
> >   	.dev_attrs	= mmc_dev_attrs,
> > @@ -186,9 +176,7 @@ static struct bus_type mmc_bus_type = {
> >   	.uevent		= mmc_bus_uevent,
> >   	.probe		= mmc_bus_probe,
> >   	.remove		= mmc_bus_remove,
> > -	.suspend	= mmc_bus_suspend,
> > -	.resume		= mmc_bus_resume,
> > -	.pm		= MMC_PM_OPS_PTR,
> > +	.pm		=&mmc_bus_pm_ops,
> >   };
> >
> >   int mmc_register_bus(void)
> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index
> > 1a1ca71..c984c9a 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -480,7 +480,7 @@ struct mmc_driver {
> >   	struct device_driver drv;
> >   	int (*probe)(struct mmc_card *);
> >   	void (*remove)(struct mmc_card *);
> > -	int (*suspend)(struct mmc_card *, pm_message_t);
> > +	int (*suspend)(struct mmc_card *);
> >   	int (*resume)(struct mmc_card *);
> >   };
> >
> 
> Kind regards
> Ulf Hansson

^ permalink raw reply

* Re: [RFC] Runtime PM for host controllers
From: Mark Brown @ 2012-04-11  9:07 UTC (permalink / raw)
  To: Kevin Cernekee; +Cc: linux-pm
In-Reply-To: <CAJiQ=7A=744EZ1pW0vmpW=+CgdoDBYT0vc-=182Y8eU-3pZqvg@mail.gmail.com>

On Tue, Apr 10, 2012 at 03:42:33PM -0700, Kevin Cernekee wrote:
> On Tue, Apr 10, 2012 at 12:48 PM, Alan Stern <stern@rowland.harvard.edu> wrote:

> > PCI devices have two different power wells, one for normal use and one
> > for use during suspend.  As far as I know, there is no way to turn off
> > power to the auxiliary well short of turning off the entire computer.
> > Rafael may have a more complete picture.

> That may be the case on PCs, but on many embedded systems (mine
> included), entire hardware blocks can be clock-gated or power-gated
> without shutting off the rest of the system.

Do you have any numbers here?  With systems that care about power
consumption on this level you tend to find there's good enough power
management within the IP block down to the level where you need to start
suspending the system itself to make a noticable impact.

Sometimes this needs to be accomplished in conjunction with things like
pin muxing - for example, configuring the pins to wake on an edge and
then having the driver detect this edge and wake up the IP itself in
order to actually handle the event.

> Probably, but it seems a little clunky when we have a "runtime
> suspend" framework already in place...

The goal of the runtime suspend framework is to be transparent to the
end user.

^ permalink raw reply

* Re: [RFC] Runtime PM for host controllers
From: Kevin Cernekee @ 2012-04-10 22:42 UTC (permalink / raw)
  To: Alan Stern; +Cc: linux-pm
In-Reply-To: <Pine.LNX.4.44L0.1204101535090.1605-100000@iolanthe.rowland.org>

On Tue, Apr 10, 2012 at 12:48 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> On my systems, it would take a lot of extra power to maintain the
>> ability to detect device hotplug.  I am wondering if the PCI based
>> controller has a better way of handling it in the hardware than we do,
>> or if the power savings from D3hot are just very small.
>
> PCI devices have two different power wells, one for normal use and one
> for use during suspend.  As far as I know, there is no way to turn off
> power to the auxiliary well short of turning off the entire computer.
> Rafael may have a more complete picture.

That may be the case on PCs, but on many embedded systems (mine
included), entire hardware blocks can be clock-gated or power-gated
without shutting off the rest of the system.

>> > If it does not
>> > involve any such loss then no "user-initiated suspend" should be
>> > needed, because the existing runtime suspend methods will do the job.
>>
>> Right.  Unfortunately I don't really have any devices that work this
>> way.  Most are "all or nothing."
>
> Is the loss of functionality you're talking about essentially the same
> as removing the driver?

>From a hardware perspective, yes.

> I don't think anybody would object to a
> driverless device being put into a non-functional low-power mode, since
> without a driver, it's pretty non-functional anyway.  It's easy for
> userspace to unbind a device from its driver; would that then do what
> you want?

Probably, but it seems a little clunky when we have a "runtime
suspend" framework already in place...

Unbinding/binding the device is slower than suspending it, and might
be a problem if there are mounted filesystems involved.

Going back to the NAS example I mentioned earlier - I would imagine
that merely resuming the USB/SATA controller + HDD when a new request
arrives is going to provide a much faster recovery time than rebinding
+ redetecting + remounting.

(Of course that raises issues like: what if somebody unplugs the
current HDD, then plugs in a different HDD, while the USB/SATA core is
asleep?)

>> >> This is not a very clean way of doing things, so I would prefer to use
>> >> runtime PM if at all possible.  But I don't think my hardware is
>> >> designed in a way that is compatible with "opportunistic suspend."
>> >
>> > In what way is it incompatible?
>>
>> My devices lose functionality when they are put into low-power modes.
>
> What functionality do they lose?

Everything, including register access, PHY links, and hotplug detection.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox