| Message ID | 20121006161429.GD12462@game.jcrosoft.org |
|---|---|
| State | New |
| Headers | show |
On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > Hi, > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > are available in the git repository at: > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > ---------------------------------------------------------------- > pm: add suspend_mem and suspend_standby support > > Today when we go to suspend we can not knwon at drivers level if we go in > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > suspend_standby. No way. Device drivers shouldn't be concerned about that. > If a bus or drivers does not need to care about this distinction we fallback to > suspend. This will allow to do not update the current bus or drivers. > > This is needed as example by at91 OHCI, UDC and atmel serial I wonder why. Thanks, Rafael
On Sun, Oct 07, 2012 at 12:18:21AM +0200, Rafael J. Wysocki wrote: > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > Hi, > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > are available in the git repository at: > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > ---------------------------------------------------------------- > > pm: add suspend_mem and suspend_standby support > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > suspend_standby. > > No way. Device drivers shouldn't be concerned about that. > > > If a bus or drivers does not need to care about this distinction we fallback to > > suspend. This will allow to do not update the current bus or drivers. > > > > This is needed as example by at91 OHCI, UDC and atmel serial > > I wonder why. Yes, that's odd, please post the patches, I don't take random git pull requests, sorry. greg k-h
On 21:01 Sat 06 Oct , Greg Kroah-Hartman wrote: > On Sun, Oct 07, 2012 at 12:18:21AM +0200, Rafael J. Wysocki wrote: > > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > Hi, > > > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > > > are available in the git repository at: > > > > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > > > ---------------------------------------------------------------- > > > pm: add suspend_mem and suspend_standby support > > > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > > suspend_standby. > > > > No way. Device drivers shouldn't be concerned about that. > > > > > If a bus or drivers does not need to care about this distinction we fallback to > > > suspend. This will allow to do not update the current bus or drivers. > > > > > > This is needed as example by at91 OHCI, UDC and atmel serial > > > > I wonder why. > > Yes, that's odd, please post the patches, I don't take random git pull > requests, sorry. the patch did not follow the first e-mail do not why I've already resend them this afternoon sorry Best Regards J.
On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > Hi, > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > are available in the git repository at: > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > ---------------------------------------------------------------- > > pm: add suspend_mem and suspend_standby support > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > suspend_standby. > > No way. Device drivers shouldn't be concerned about that. I do need it on at91 as we swith to slow_clock in MEM suspend and some ip need special handling when switching to slow_clock Best Regards, J.
On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > Hi, > > > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > > > are available in the git repository at: > > > > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > > > ---------------------------------------------------------------- > > > pm: add suspend_mem and suspend_standby support > > > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > > suspend_standby. > > > > No way. Device drivers shouldn't be concerned about that. > I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > need special handling when switching to slow_clock Well, my answer to that is: please fix your platform code instead of hacking the PM core to work around its problems. Thanks, Rafael
On Saturday 06 of October 2012 21:01:28 Greg Kroah-Hartman wrote: > On Sun, Oct 07, 2012 at 12:18:21AM +0200, Rafael J. Wysocki wrote: > > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > Hi, > > > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > > > are available in the git repository at: > > > > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > > > ---------------------------------------------------------------- > > > pm: add suspend_mem and suspend_standby support > > > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > > suspend_standby. > > > > No way. Device drivers shouldn't be concerned about that. > > > > > If a bus or drivers does not need to care about this distinction we fallback to > > > suspend. This will allow to do not update the current bus or drivers. > > > > > > This is needed as example by at91 OHCI, UDC and atmel serial > > > > I wonder why. > > Yes, that's odd, please post the patches, I don't take random git pull > requests, sorry. Well, this particular one would be for me, but I'm not going to take it too. :-) I'll have a look at the patches tomorrow, but the rule is we're not adding more device PM callbacks on a whim. Thanks, Rafael
On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: > On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > > On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > > > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > Hi, > > > > > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > > > > > are available in the git repository at: > > > > > > > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > > > > > ---------------------------------------------------------------- > > > > pm: add suspend_mem and suspend_standby support > > > > > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > > > suspend_standby. > > > > > > No way. Device drivers shouldn't be concerned about that. > > I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > > need special handling when switching to slow_clock > > Well, my answer to that is: please fix your platform code instead of > hacking the PM core to work around its problems. how can I fix drivers pm issue when I no way to known at driver level the real suspend, the PM core is supposed to proivde the right information to the drivers so the driver can put it's in the right pm mode. If the pm core can not provide such inforation the PM core is broken as we will have to do dirty hack. Any generic framework is supposed to evolve for real user need here I've one so I udpate the core to mach a need Best Regards, J.
On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: > > On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > > > > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > > Hi, > > > > > > > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > > > > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > > > > > > > are available in the git repository at: > > > > > > > > > > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > > > > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > > > > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > > > > > > > ---------------------------------------------------------------- > > > > > pm: add suspend_mem and suspend_standby support > > > > > > > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > > > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > > > > suspend_standby. > > > > > > > > No way. Device drivers shouldn't be concerned about that. > > > I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > > > need special handling when switching to slow_clock > > > > Well, my answer to that is: please fix your platform code instead of > > hacking the PM core to work around its problems. > how can I fix drivers pm issue when I no way to known at driver level the > real suspend, the PM core is supposed to proivde the right information to the > drivers so the driver can put it's in the right pm mode. If the pm core can not > provide such inforation the PM core is broken as we will have to do dirty > hack. Why do you need to know the difference in your driver? We used to provide this information a long time ago, but it turned out to not be needed at all and just caused problems. > Any generic framework is supposed to evolve for real user need here I've one > so I udpate the core to mach a need I'll push back and ask again why your driver cares about this? It shouldn't. greg k-h
On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: > On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > > On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: > > > On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > > > > > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > > > Hi, > > > > > > > > > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > > > > > > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > > > > > > > > > are available in the git repository at: > > > > > > > > > > > > > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > > > > > > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > > > > > > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > > > > > > > > > ---------------------------------------------------------------- > > > > > > pm: add suspend_mem and suspend_standby support > > > > > > > > > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > > > > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > > > > > suspend_standby. > > > > > > > > > > No way. Device drivers shouldn't be concerned about that. > > > > I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > > > > need special handling when switching to slow_clock > > > > > > Well, my answer to that is: please fix your platform code instead of > > > hacking the PM core to work around its problems. > > how can I fix drivers pm issue when I no way to known at driver level the > > real suspend, the PM core is supposed to proivde the right information to the > > drivers so the driver can put it's in the right pm mode. If the pm core can not > > provide such inforation the PM core is broken as we will have to do dirty > > hack. > > Why do you need to know the difference in your driver? We used to > provide this information a long time ago, but it turned out to not be > needed at all and just caused problems. because on at91 I need to handle the mem standby at drviers level. We do it today already by a hack in different drivers at91_udc (usb device), atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching to mem pm. On at91 when switch to mem we shutdown everything and run form a slow clock - this is done at soc level - but those IP have issue and need specific care before doing so. Ohterwise when the SoC will wakeup but those IP will not in this patch series I send the update of those 3 drivers too and kill the hack > > > Any generic framework is supposed to evolve for real user need here I've one > > so I udpate the core to mach a need > > I'll push back and ask again why your driver cares about this? It > shouldn't. I'm doing it right now in the kernel but in a dirty way and want to fix it. And honestly If I could not do it I will Best Regards, J.
On Tue, 9 Oct 2012, Jean-Christophe PLAGNIOL-VILLARD wrote: > > Why do you need to know the difference in your driver? We used to > > provide this information a long time ago, but it turned out to not be > > needed at all and just caused problems. > because on at91 I need to handle the mem standby at drviers level. > > We do it today already by a hack in different drivers at91_udc (usb device), > atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching > to mem pm. On at91 when switch to mem we shutdown everything and run form a slow > clock - this is done at soc level - but those IP have issue and need specific > care before doing so. Ohterwise when the SoC will wakeup but those IP will not > > in this patch series I send the update of those 3 drivers too > and kill the hack How about adding a platform-specific callback routine to tell drivers whether or not they will run from the slow clock? Something like: bool at91_suspend_will_use_slow_clock(void); That would not involve making any changes to the driver core, and your three drivers could still get the information they need. Alan Stern
On 11:44 Tue 09 Oct , Alan Stern wrote: > On Tue, 9 Oct 2012, Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > Why do you need to know the difference in your driver? We used to > > > provide this information a long time ago, but it turned out to not be > > > needed at all and just caused problems. > > because on at91 I need to handle the mem standby at drviers level. > > > > We do it today already by a hack in different drivers at91_udc (usb device), > > atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching > > to mem pm. On at91 when switch to mem we shutdown everything and run form a slow > > clock - this is done at soc level - but those IP have issue and need specific > > care before doing so. Ohterwise when the SoC will wakeup but those IP will not > > > > in this patch series I send the update of those 3 drivers too > > and kill the hack > > How about adding a platform-specific callback routine to tell drivers > whether or not they will run from the slow clock? Something like: > > bool at91_suspend_will_use_slow_clock(void); > > That would not involve making any changes to the driver core, and your > three drivers could still get the information they need. I already do so and I want to drop it as the IP is shared with multiple ARCH and I want to have a clean code no hack and no platform data I do DT and drop board code so far, I've a dt code without any platform data provide via c code Best Regards, J.
On Tuesday 09 of October 2012 07:58:54 Greg Kroah-Hartman wrote: > On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > > On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: > > > On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > > > > > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > > > Hi, > > > > > > > > > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > > > > > > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > > > > > > > > > are available in the git repository at: > > > > > > > > > > > > > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > > > > > > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > > > > > > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > > > > > > > > > ---------------------------------------------------------------- > > > > > > pm: add suspend_mem and suspend_standby support > > > > > > > > > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > > > > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > > > > > suspend_standby. > > > > > > > > > > No way. Device drivers shouldn't be concerned about that. > > > > I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > > > > need special handling when switching to slow_clock > > > > > > Well, my answer to that is: please fix your platform code instead of > > > hacking the PM core to work around its problems. > > how can I fix drivers pm issue when I no way to known at driver level the > > real suspend, the PM core is supposed to proivde the right information to the > > drivers so the driver can put it's in the right pm mode. If the pm core can not > > provide such inforation the PM core is broken as we will have to do dirty > > hack. > > Why do you need to know the difference in your driver? We used to > provide this information a long time ago, but it turned out to not be > needed at all and just caused problems. We actualy have never made a difference between "standby" and "suspend" as far as drivers are concerned. What we used to have was one "suspend" callback for suspend to RAM and all phases of hibernation, so we needed to pass an argument indicating what operation was to be carried out to it. Thanks, Rafael
On Tuesday 09 of October 2012 17:17:04 Jean-Christophe PLAGNIOL-VILLARD wrote: > On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: > > On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > > > On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: > > > > On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > > On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > > > > > > On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > > > > Hi, > > > > > > > > > > > > > > The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > > > > > > > > > > > > > Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > > > > > > > > > > > > > are available in the git repository at: > > > > > > > > > > > > > > > > > > > > > git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > > > > > > > > > > > > > for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > > > > > > > > > > > > > arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > > > > > > > > > > > > > ---------------------------------------------------------------- > > > > > > > pm: add suspend_mem and suspend_standby support > > > > > > > > > > > > > > Today when we go to suspend we can not knwon at drivers level if we go in > > > > > > > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > > > > > > suspend_standby. > > > > > > > > > > > > No way. Device drivers shouldn't be concerned about that. > > > > > I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > > > > > need special handling when switching to slow_clock > > > > > > > > Well, my answer to that is: please fix your platform code instead of > > > > hacking the PM core to work around its problems. > > > how can I fix drivers pm issue when I no way to known at driver level the > > > real suspend, the PM core is supposed to proivde the right information to the > > > drivers so the driver can put it's in the right pm mode. If the pm core can not > > > provide such inforation the PM core is broken as we will have to do dirty > > > hack. > > > > Why do you need to know the difference in your driver? We used to > > provide this information a long time ago, but it turned out to not be > > needed at all and just caused problems. > because on at91 I need to handle the mem standby at drviers level. This is not an answer. You're basically saying "it has to be done this way, because it has to be done this way". > We do it today already by a hack in different drivers at91_udc (usb device), > atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching > to mem pm. On at91 when switch to mem we shutdown everything and run form a slow > clock - this is done at soc level - but those IP have issue and need specific > care before doing so. Ohterwise when the SoC will wakeup but those IP will not > > in this patch series I send the update of those 3 drivers too > and kill the hack Well, let's start over. What's the difference between "suspend to RAM" (mem) and "standby" on your platform? > > > Any generic framework is supposed to evolve for real user need here I've one > > > so I udpate the core to mach a need Yes, if there are more users needing a change. If there's only one, I think not really. Thanks, Rafael
On Tuesday 09 of October 2012 17:52:41 Jean-Christophe PLAGNIOL-VILLARD wrote: > On 11:44 Tue 09 Oct , Alan Stern wrote: > > On Tue, 9 Oct 2012, Jean-Christophe PLAGNIOL-VILLARD wrote: > > > > > > Why do you need to know the difference in your driver? We used to > > > > provide this information a long time ago, but it turned out to not be > > > > needed at all and just caused problems. > > > because on at91 I need to handle the mem standby at drviers level. > > > > > > We do it today already by a hack in different drivers at91_udc (usb device), > > > atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching > > > to mem pm. On at91 when switch to mem we shutdown everything and run form a slow > > > clock - this is done at soc level - but those IP have issue and need specific > > > care before doing so. Ohterwise when the SoC will wakeup but those IP will not > > > > > > in this patch series I send the update of those 3 drivers too > > > and kill the hack > > > > How about adding a platform-specific callback routine to tell drivers > > whether or not they will run from the slow clock? Something like: > > > > bool at91_suspend_will_use_slow_clock(void); > > > > That would not involve making any changes to the driver core, and your > > three drivers could still get the information they need. > I already do so and I want to drop it as the IP is shared with multiple ARCH > and I want to have a clean code no hack So, what about actually adding a platform layer to your PM instead of doing all PM in device drivers? ACPI has exactly the same problem that you have, but it's never has to tell device drivers what system sleep state is going to be entered. Drivers just call acpi_pm_device_sleep_state() and that tells them what power state to put the device into (which depends on the sleep state being entered, but drivers really don't need to know that state). Thanks, Rafael
On Sunday 07 of October 2012 09:27:15 Jean-Christophe PLAGNIOL-VILLARD wrote: > Today when we go to suspend we can not knwon at drivers level if we go in > STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > suspend_standby. > > If a bus does not need to care about this distinction we fallback to > suspend. This will allow to do not update the current bus or drivers. > > This is needed as example by at91 OHCI and atmel serial > > Cc: Nicolas Ferre <nicolas.ferre@atmel.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> > Cc: linux-pm@vger.kernel.org So for me, this patch is a total no-go. Apart from the fact that I don't see any difference between .suspend() and .suspend_mem(), it doesn't cover the late/early and noirq phases. > --- > drivers/base/power/main.c | 12 ++++++++++++ > include/linux/pm.h | 9 +++++++++ > kernel/power/suspend.c | 16 ++++++++++++++-- > 3 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c > index a3c1404..581e135 100644 > --- a/drivers/base/power/main.c > +++ b/drivers/base/power/main.c > @@ -218,9 +218,21 @@ static void dpm_wait_for_children(struct device *dev, bool async) > */ > static pm_callback_t pm_op(const struct dev_pm_ops *ops, pm_message_t state) > { > + pm_callback_t callback = NULL; > + > switch (state.event) { > #ifdef CONFIG_SUSPEND > case PM_EVENT_SUSPEND: > + switch (state.param) { > + case PM_SUSPEND_MEM: > + callback = ops->suspend_mem; > + break; > + case PM_SUSPEND_STANDBY: > + callback = ops->suspend_standby; > + break; > + } > + if (callback) > + return callback; > return ops->suspend; > case PM_EVENT_RESUME: > return ops->resume; > diff --git a/include/linux/pm.h b/include/linux/pm.h > index 007e687..aff344b 100644 > --- a/include/linux/pm.h > +++ b/include/linux/pm.h > @@ -49,6 +49,7 @@ extern const char power_group_name[]; /* = "power" */ > > typedef struct pm_message { > int event; > + int param; > } pm_message_t; Please don't do that. We have the struct platform_suspend_ops with callbacks that take the state argument and we pass the information about the system sleep state being entered in there. The way this information is interpreted depends on the platform and the actions of device drivers resulting from that must be platform-dependent too. Your design makes a very strong assumption that the interpretation of "standby" by the platform and device drivers will always be the same, which generally need not be the case. [And what if the given platform actually has more than two sleep states? Are you going to add a separate callback for each of them?] Make your drivers ask the platform what power state to put the devices into instead of doing this. > /** > @@ -265,6 +266,8 @@ struct dev_pm_ops { > int (*prepare)(struct device *dev); > void (*complete)(struct device *dev); > int (*suspend)(struct device *dev); > + int (*suspend_mem)(struct device *dev); > + int (*suspend_standby)(struct device *dev); > int (*resume)(struct device *dev); > int (*freeze)(struct device *dev); > int (*thaw)(struct device *dev); > @@ -295,8 +298,12 @@ struct dev_pm_ops { > .thaw = resume_fn, \ > .poweroff = suspend_fn, \ > .restore = resume_fn, > +#define SET_SYSTEM_SLEEP_STANDBY_MEM_PM_OPS(suspend_standby_fn, suspend_mem_fn) \ > + .suspend_standby = suspend_standby_fn, \ > + .suspend_mem = suspend_mem_fn, > #else > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) > +#define SET_SYSTEM_SLEEP_STANDBY_MEM_PM_OPS(suspend_standby_fn, suspend_mem_fn) > #endif > > #ifdef CONFIG_PM_RUNTIME > @@ -414,6 +421,8 @@ const struct dev_pm_ops name = { \ > #define PMSG_FREEZE ((struct pm_message){ .event = PM_EVENT_FREEZE, }) > #define PMSG_QUIESCE ((struct pm_message){ .event = PM_EVENT_QUIESCE, }) > #define PMSG_SUSPEND ((struct pm_message){ .event = PM_EVENT_SUSPEND, }) > +#define PMSG_SUSPEND_MEM ((struct pm_message){ .event = PM_EVENT_SUSPEND, .param = PM_SUSPEND_MEM, }) > +#define PMSG_SUSPEND_STANDBY ((struct pm_message){ .event = PM_EVENT_SUSPEND, .param = PM_SUSPEND_STANDBY, }) > #define PMSG_HIBERNATE ((struct pm_message){ .event = PM_EVENT_HIBERNATE, }) > #define PMSG_RESUME ((struct pm_message){ .event = PM_EVENT_RESUME, }) > #define PMSG_THAW ((struct pm_message){ .event = PM_EVENT_THAW, }) > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c > index c8b7446..9505b55 100644 > --- a/kernel/power/suspend.c > +++ b/kernel/power/suspend.c > @@ -126,6 +126,18 @@ void __attribute__ ((weak)) arch_suspend_enable_irqs(void) > local_irq_enable(); > } > > +static pm_message_t suspend_state_to_pm_state(suspend_state_t state) > +{ > + switch (state) { > + case PM_SUSPEND_STANDBY: > + return PMSG_SUSPEND_STANDBY; > + case PM_SUSPEND_MEM: > + return PMSG_SUSPEND_MEM; > + default: > + return PMSG_SUSPEND; > + } > +} Well, this is more than ugly. > + > /** > * suspend_enter - Make the system enter the given sleep state. > * @state: System sleep state to enter. > @@ -143,7 +155,7 @@ static int suspend_enter(suspend_state_t state, bool *wakeup) > goto Platform_finish; > } > > - error = dpm_suspend_end(PMSG_SUSPEND); > + error = dpm_suspend_end(suspend_state_to_pm_state(state)); > if (error) { > printk(KERN_ERR "PM: Some devices failed to power down\n"); > goto Platform_finish; > @@ -215,7 +227,7 @@ int suspend_devices_and_enter(suspend_state_t state) > suspend_console(); > ftrace_stop(); > suspend_test_start(); > - error = dpm_suspend_start(PMSG_SUSPEND); > + error = dpm_suspend_start(suspend_state_to_pm_state(state)); > if (error) { > printk(KERN_ERR "PM: Some devices failed to suspend\n"); > goto Recover_platform; Thanks, Rafael
On 10/10/2012 3:42 AM, Rafael J. Wysocki wrote: > On Tuesday 09 of October 2012 17:17:04 Jean-Christophe PLAGNIOL-VILLARD wrote: >> On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: >>> On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: >>>> On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: >>>>> On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>> On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: >>>>>>> On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: >>>>>>>> >>>>>>>> Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) >>>>>>>> >>>>>>>> are available in the git repository at: >>>>>>>> >>>>>>>> >>>>>>>> git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem >>>>>>>> >>>>>>>> for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: >>>>>>>> >>>>>>>> arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) >>>>>>>> >>>>>>>> ---------------------------------------------------------------- >>>>>>>> pm: add suspend_mem and suspend_standby support >>>>>>>> >>>>>>>> Today when we go to suspend we can not knwon at drivers level if we go in >>>>>>>> STANDBY or MEM. Fix this by introducing two new callback suspend_mem and >>>>>>>> suspend_standby. >>>>>>> >>>>>>> No way. Device drivers shouldn't be concerned about that. >>>>>> I do need it on at91 as we swith to slow_clock in MEM suspend and some ip >>>>>> need special handling when switching to slow_clock >>>>> >>>>> Well, my answer to that is: please fix your platform code instead of >>>>> hacking the PM core to work around its problems. >>>> how can I fix drivers pm issue when I no way to known at driver level the >>>> real suspend, the PM core is supposed to proivde the right information to the >>>> drivers so the driver can put it's in the right pm mode. If the pm core can not >>>> provide such inforation the PM core is broken as we will have to do dirty >>>> hack. >>> >>> Why do you need to know the difference in your driver? We used to >>> provide this information a long time ago, but it turned out to not be >>> needed at all and just caused problems. >> because on at91 I need to handle the mem standby at drviers level. > > This is not an answer. You're basically saying "it has to be done this way, > because it has to be done this way". > >> We do it today already by a hack in different drivers at91_udc (usb device), >> atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching >> to mem pm. On at91 when switch to mem we shutdown everything and run form a slow >> clock - this is done at soc level - but those IP have issue and need specific >> care before doing so. Ohterwise when the SoC will wakeup but those IP will not >> >> in this patch series I send the update of those 3 drivers too >> and kill the hack > > Well, let's start over. What's the difference between "suspend to RAM" (mem) > and "standby" on your platform? > >>>> Any generic framework is supposed to evolve for real user need here I've one >>>> so I udpate the core to mach a need > > Yes, if there are more users needing a change. If there's only one, I think not > really. > We came across such a need while supporting various low-power modes in AM335x SoC. I also wanted to put similar proposal, its good that Jean submitted patches and we are having this discussion early. Let me try to describe the need and issue here, In case of AM33xx device, we have different low-power modes supported by HW, for this discussion I will just talk about DeepSleep-0 and StandBy mode supported by HW (few other modes also described in spec)- Below is the Spec definition of these two low-power modes, PowerMode Application state States ======================================================================== DeepSleep0 Everything is preserved Master Oscillator = OFF including SDRAM. VDD_MPU = 0.95v VDD_CORE = 0.95v PD_WKUP = ON PD_MPU = OFF PD_PER = OFF PD_GFX = OFF All IOs & RTC supplies are ON Standby Everything is preserved Master Oscillator = ON including SDRAM. One PLL is ON Only required module clocks VDD_MPU = 0.95v are enabled rest are clock VDD_CORE = OPP50 gated PD_WKUP = ON PD_MPU = ON PD_PER = ON PD_GFX = OFF All IOs & RTC supplies are ON If more PLLs /power domains/ modules are required, the power will increase accordingly Now we have two major parameters here, Power Consumption and Latency (Sleep + Wakeup). Since DeepSleep-0 is as good as off state, it gives very less power consumption Vs huge Latency number, whereas, in case of standby the power consumption is more but the Latency is reduced. In addition to that, there are certain HW related parameters and issues contribute to these parameters. For example, In case of LCD driver the suspend/resume latency is in the range of 10msec, which can be reduced if driver knows that he is not entering or entered in DeepSleep-0 and choose not to do certain things. Another example would be, USB driver, we have seen Latency numbers in the range of 200msec, and of which 100 msec can simply be taken away if driver knows about the sleep state. I understand that, there is trade-off between power and latency numbers, but for certain use-cases latency is more important and it is not possible without telling driver about the low-power state. Thanks, Vaibhav > Thanks, > Rafael > >
On 10.10.2012 09:17, Vaibhav Hiremath wrote: > On 10/10/2012 3:42 AM, Rafael J. Wysocki wrote: >> On Tuesday 09 of October 2012 17:17:04 Jean-Christophe PLAGNIOL-VILLARD wrote: >>> On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: >>>> On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>> On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: >>>>>> On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>>> On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: >>>>>>>> On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: >>>>>>>>> >>>>>>>>> Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) >>>>>>>>> >>>>>>>>> are available in the git repository at: >>>>>>>>> >>>>>>>>> >>>>>>>>> git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem >>>>>>>>> >>>>>>>>> for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: >>>>>>>>> >>>>>>>>> arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) >>>>>>>>> >>>>>>>>> ---------------------------------------------------------------- >>>>>>>>> pm: add suspend_mem and suspend_standby support >>>>>>>>> >>>>>>>>> Today when we go to suspend we can not knwon at drivers level if we go in >>>>>>>>> STANDBY or MEM. Fix this by introducing two new callback suspend_mem and >>>>>>>>> suspend_standby. >>>>>>>> >>>>>>>> No way. Device drivers shouldn't be concerned about that. >>>>>>> I do need it on at91 as we swith to slow_clock in MEM suspend and some ip >>>>>>> need special handling when switching to slow_clock >>>>>> >>>>>> Well, my answer to that is: please fix your platform code instead of >>>>>> hacking the PM core to work around its problems. >>>>> how can I fix drivers pm issue when I no way to known at driver level the >>>>> real suspend, the PM core is supposed to proivde the right information to the >>>>> drivers so the driver can put it's in the right pm mode. If the pm core can not >>>>> provide such inforation the PM core is broken as we will have to do dirty >>>>> hack. >>>> >>>> Why do you need to know the difference in your driver? We used to >>>> provide this information a long time ago, but it turned out to not be >>>> needed at all and just caused problems. >>> because on at91 I need to handle the mem standby at drviers level. >> >> This is not an answer. You're basically saying "it has to be done this way, >> because it has to be done this way". >> >>> We do it today already by a hack in different drivers at91_udc (usb device), >>> atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching >>> to mem pm. On at91 when switch to mem we shutdown everything and run form a slow >>> clock - this is done at soc level - but those IP have issue and need specific >>> care before doing so. Ohterwise when the SoC will wakeup but those IP will not >>> >>> in this patch series I send the update of those 3 drivers too >>> and kill the hack >> >> Well, let's start over. What's the difference between "suspend to RAM" (mem) >> and "standby" on your platform? >> >>>>> Any generic framework is supposed to evolve for real user need here I've one >>>>> so I udpate the core to mach a need >> >> Yes, if there are more users needing a change. If there's only one, I think not >> really. >> > > We came across such a need while supporting various low-power modes in > AM335x SoC. I also wanted to put similar proposal, its good that Jean > submitted patches and we are having this discussion early. > > Let me try to describe the need and issue here, > > In case of AM33xx device, we have different low-power modes supported > by HW, for this discussion I will just talk about DeepSleep-0 and > StandBy mode supported by HW (few other modes also described in spec)- > > Below is the Spec definition of these two low-power modes, Thanks for this summary, which raises a question for me that might be slightly unreleated to the general discussion, but still: I was just looking at the code that ships with the BSP kernel, and the approach there is to load a binary firmare blob to the M3 UMEM and then communicates with mailbox commands in order to put the system to suspend. Is this how it should be done in mainline as well or are there any plans to implement that behaviour natively? Thanks, Daniel
On Wed, Oct 10, 2012 at 14:50:25, Daniel Mack wrote: > On 10.10.2012 09:17, Vaibhav Hiremath wrote: > > On 10/10/2012 3:42 AM, Rafael J. Wysocki wrote: > >> On Tuesday 09 of October 2012 17:17:04 Jean-Christophe PLAGNIOL-VILLARD wrote: > >>> On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: > >>>> On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>>> On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: > >>>>>> On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>>>>> On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > >>>>>>>> On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>>>>>>> Hi, > >>>>>>>>> > >>>>>>>>> The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > >>>>>>>>> > >>>>>>>>> Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > >>>>>>>>> > >>>>>>>>> are available in the git repository at: > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > >>>>>>>>> > >>>>>>>>> for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > >>>>>>>>> > >>>>>>>>> arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > >>>>>>>>> > >>>>>>>>> ---------------------------------------------------------------- > >>>>>>>>> pm: add suspend_mem and suspend_standby support > >>>>>>>>> > >>>>>>>>> Today when we go to suspend we can not knwon at drivers level if we go in > >>>>>>>>> STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > >>>>>>>>> suspend_standby. > >>>>>>>> > >>>>>>>> No way. Device drivers shouldn't be concerned about that. > >>>>>>> I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > >>>>>>> need special handling when switching to slow_clock > >>>>>> > >>>>>> Well, my answer to that is: please fix your platform code instead of > >>>>>> hacking the PM core to work around its problems. > >>>>> how can I fix drivers pm issue when I no way to known at driver level the > >>>>> real suspend, the PM core is supposed to proivde the right information to the > >>>>> drivers so the driver can put it's in the right pm mode. If the pm core can not > >>>>> provide such inforation the PM core is broken as we will have to do dirty > >>>>> hack. > >>>> > >>>> Why do you need to know the difference in your driver? We used to > >>>> provide this information a long time ago, but it turned out to not be > >>>> needed at all and just caused problems. > >>> because on at91 I need to handle the mem standby at drviers level. > >> > >> This is not an answer. You're basically saying "it has to be done this way, > >> because it has to be done this way". > >> > >>> We do it today already by a hack in different drivers at91_udc (usb device), > >>> atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching > >>> to mem pm. On at91 when switch to mem we shutdown everything and run form a slow > >>> clock - this is done at soc level - but those IP have issue and need specific > >>> care before doing so. Ohterwise when the SoC will wakeup but those IP will not > >>> > >>> in this patch series I send the update of those 3 drivers too > >>> and kill the hack > >> > >> Well, let's start over. What's the difference between "suspend to RAM" (mem) > >> and "standby" on your platform? > >> > >>>>> Any generic framework is supposed to evolve for real user need here I've one > >>>>> so I udpate the core to mach a need > >> > >> Yes, if there are more users needing a change. If there's only one, I think not > >> really. > >> > > > > We came across such a need while supporting various low-power modes in > > AM335x SoC. I also wanted to put similar proposal, its good that Jean > > submitted patches and we are having this discussion early. > > > > Let me try to describe the need and issue here, > > > > In case of AM33xx device, we have different low-power modes supported > > by HW, for this discussion I will just talk about DeepSleep-0 and > > StandBy mode supported by HW (few other modes also described in spec)- > > > > Below is the Spec definition of these two low-power modes, > > Thanks for this summary, which raises a question for me that might be > slightly unreleated to the general discussion, but still: > > I was just looking at the code that ships with the BSP kernel, and the > approach there is to load a binary firmare blob to the M3 UMEM and then > communicates with mailbox commands in order to put the system to suspend. > Ohh, Great, then you are aware of AM33xx power management support. As you are aware, M3 here works as a soft-PRCM block, so certain things has to be done on M3 while entering into low-power state. > Is this how it should be done in mainline as well or are there any plans > to implement that behaviour natively? > Yes, we will follow similar approach for Mainline, its under development and soon you will see patches getting submitted to the list for review. The first step is to get deepsleep-0 support in Mainline and then other will follow. Thanks, Vaibhav > > Thanks, > Daniel > >
On 10.10.2012 11:29, Hiremath, Vaibhav wrote: > On Wed, Oct 10, 2012 at 14:50:25, Daniel Mack wrote: >> On 10.10.2012 09:17, Vaibhav Hiremath wrote: >>> On 10/10/2012 3:42 AM, Rafael J. Wysocki wrote: >>>> On Tuesday 09 of October 2012 17:17:04 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>> On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: >>>>>> On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>>> On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: >>>>>>>> On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>>>>> On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: >>>>>>>>>> On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: >>>>>>>>>>> >>>>>>>>>>> Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) >>>>>>>>>>> >>>>>>>>>>> are available in the git repository at: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem >>>>>>>>>>> >>>>>>>>>>> for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: >>>>>>>>>>> >>>>>>>>>>> arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) >>>>>>>>>>> >>>>>>>>>>> ---------------------------------------------------------------- >>>>>>>>>>> pm: add suspend_mem and suspend_standby support >>>>>>>>>>> >>>>>>>>>>> Today when we go to suspend we can not knwon at drivers level if we go in >>>>>>>>>>> STANDBY or MEM. Fix this by introducing two new callback suspend_mem and >>>>>>>>>>> suspend_standby. >>>>>>>>>> >>>>>>>>>> No way. Device drivers shouldn't be concerned about that. >>>>>>>>> I do need it on at91 as we swith to slow_clock in MEM suspend and some ip >>>>>>>>> need special handling when switching to slow_clock >>>>>>>> >>>>>>>> Well, my answer to that is: please fix your platform code instead of >>>>>>>> hacking the PM core to work around its problems. >>>>>>> how can I fix drivers pm issue when I no way to known at driver level the >>>>>>> real suspend, the PM core is supposed to proivde the right information to the >>>>>>> drivers so the driver can put it's in the right pm mode. If the pm core can not >>>>>>> provide such inforation the PM core is broken as we will have to do dirty >>>>>>> hack. >>>>>> >>>>>> Why do you need to know the difference in your driver? We used to >>>>>> provide this information a long time ago, but it turned out to not be >>>>>> needed at all and just caused problems. >>>>> because on at91 I need to handle the mem standby at drviers level. >>>> >>>> This is not an answer. You're basically saying "it has to be done this way, >>>> because it has to be done this way". >>>> >>>>> We do it today already by a hack in different drivers at91_udc (usb device), >>>>> atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching >>>>> to mem pm. On at91 when switch to mem we shutdown everything and run form a slow >>>>> clock - this is done at soc level - but those IP have issue and need specific >>>>> care before doing so. Ohterwise when the SoC will wakeup but those IP will not >>>>> >>>>> in this patch series I send the update of those 3 drivers too >>>>> and kill the hack >>>> >>>> Well, let's start over. What's the difference between "suspend to RAM" (mem) >>>> and "standby" on your platform? >>>> >>>>>>> Any generic framework is supposed to evolve for real user need here I've one >>>>>>> so I udpate the core to mach a need >>>> >>>> Yes, if there are more users needing a change. If there's only one, I think not >>>> really. >>>> >>> >>> We came across such a need while supporting various low-power modes in >>> AM335x SoC. I also wanted to put similar proposal, its good that Jean >>> submitted patches and we are having this discussion early. >>> >>> Let me try to describe the need and issue here, >>> >>> In case of AM33xx device, we have different low-power modes supported >>> by HW, for this discussion I will just talk about DeepSleep-0 and >>> StandBy mode supported by HW (few other modes also described in spec)- >>> >>> Below is the Spec definition of these two low-power modes, >> >> Thanks for this summary, which raises a question for me that might be >> slightly unreleated to the general discussion, but still: >> >> I was just looking at the code that ships with the BSP kernel, and the >> approach there is to load a binary firmare blob to the M3 UMEM and then >> communicates with mailbox commands in order to put the system to suspend. >> > > Ohh, Great, then you are aware of AM33xx power management support. > As you are aware, M3 here works as a soft-PRCM block, so certain things has > to be done on M3 while entering into low-power state. > >> Is this how it should be done in mainline as well or are there any plans >> to implement that behaviour natively? >> > > Yes, we will follow similar approach for Mainline, its under development and > soon you will see patches getting submitted to the list for review. The > first step is to get deepsleep-0 support in Mainline and then other will > follow. Ok, great. I'll test them as soon as they hit my mailbox :) Thanks, Daniel
On 12:47 Wed 10 Oct , Vaibhav Hiremath wrote: > > > On 10/10/2012 3:42 AM, Rafael J. Wysocki wrote: > > On Tuesday 09 of October 2012 17:17:04 Jean-Christophe PLAGNIOL-VILLARD wrote: > >> On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: > >>> On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>> On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: > >>>>> On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>>>> On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > >>>>>>> On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>>>>>> Hi, > >>>>>>>> > >>>>>>>> The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > >>>>>>>> > >>>>>>>> Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > >>>>>>>> > >>>>>>>> are available in the git repository at: > >>>>>>>> > >>>>>>>> > >>>>>>>> git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > >>>>>>>> > >>>>>>>> for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > >>>>>>>> > >>>>>>>> arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > >>>>>>>> > >>>>>>>> ---------------------------------------------------------------- > >>>>>>>> pm: add suspend_mem and suspend_standby support > >>>>>>>> > >>>>>>>> Today when we go to suspend we can not knwon at drivers level if we go in > >>>>>>>> STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > >>>>>>>> suspend_standby. > >>>>>>> > >>>>>>> No way. Device drivers shouldn't be concerned about that. > >>>>>> I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > >>>>>> need special handling when switching to slow_clock > >>>>> > >>>>> Well, my answer to that is: please fix your platform code instead of > >>>>> hacking the PM core to work around its problems. > >>>> how can I fix drivers pm issue when I no way to known at driver level the > >>>> real suspend, the PM core is supposed to proivde the right information to the > >>>> drivers so the driver can put it's in the right pm mode. If the pm core can not > >>>> provide such inforation the PM core is broken as we will have to do dirty > >>>> hack. > >>> > >>> Why do you need to know the difference in your driver? We used to > >>> provide this information a long time ago, but it turned out to not be > >>> needed at all and just caused problems. > >> because on at91 I need to handle the mem standby at drviers level. > > > > This is not an answer. You're basically saying "it has to be done this way, > > because it has to be done this way". > > > >> We do it today already by a hack in different drivers at91_udc (usb device), > >> atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching > >> to mem pm. On at91 when switch to mem we shutdown everything and run form a slow > >> clock - this is done at soc level - but those IP have issue and need specific > >> care before doing so. Ohterwise when the SoC will wakeup but those IP will not > >> > >> in this patch series I send the update of those 3 drivers too > >> and kill the hack > > > > Well, let's start over. What's the difference between "suspend to RAM" (mem) > > and "standby" on your platform? > > > >>>> Any generic framework is supposed to evolve for real user need here I've one > >>>> so I udpate the core to mach a need > > > > Yes, if there are more users needing a change. If there's only one, I think not > > really. > > > > We came across such a need while supporting various low-power modes in > AM335x SoC. I also wanted to put similar proposal, its good that Jean > submitted patches and we are having this discussion early. > > Let me try to describe the need and issue here, > > In case of AM33xx device, we have different low-power modes supported > by HW, for this discussion I will just talk about DeepSleep-0 and > StandBy mode supported by HW (few other modes also described in spec)- > > Below is the Spec definition of these two low-power modes, > > PowerMode Application state States > ======================================================================== > DeepSleep0 Everything is preserved Master Oscillator = OFF > including SDRAM. VDD_MPU = 0.95v > VDD_CORE = 0.95v > PD_WKUP = ON > PD_MPU = OFF > PD_PER = OFF > PD_GFX = OFF > All IOs & RTC supplies are ON > > > Standby Everything is preserved Master Oscillator = ON > including SDRAM. One PLL is ON > Only required module clocks VDD_MPU = 0.95v > are enabled rest are clock VDD_CORE = OPP50 > gated PD_WKUP = ON > PD_MPU = ON > PD_PER = ON > PD_GFX = OFF > All IOs & RTC supplies are ON > If more PLLs /power domains/ > modules are required, the > power will increase > accordingly > > > > Now we have two major parameters here, Power Consumption and Latency > (Sleep + Wakeup). Since DeepSleep-0 is as good as off state, it gives > very less power consumption Vs huge Latency number, whereas, in case of > standby the power consumption is more but the Latency is reduced. > > In addition to that, there are certain HW related parameters and issues > contribute to these parameters. For example, > > In case of LCD driver the suspend/resume latency is in the range of > 10msec, which can be reduced if driver knows that he is not entering or > entered in DeepSleep-0 and choose not to do certain things. > Another example would be, USB driver, we have seen Latency numbers in > the range of 200msec, and of which 100 msec can simply be taken away if > driver knows about the sleep state. > > > I understand that, there is trade-off between power and latency numbers, > but for certain use-cases latency is more important and it is not > possible without telling driver about the low-power state. I work on ST SoC where I have also such standby mode too and similar issue Best Regards, J.
On Wednesday 10 of October 2012 12:47:31 Vaibhav Hiremath wrote: > > On 10/10/2012 3:42 AM, Rafael J. Wysocki wrote: > > On Tuesday 09 of October 2012 17:17:04 Jean-Christophe PLAGNIOL-VILLARD wrote: > >> On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: > >>> On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>> On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: > >>>>> On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>>>> On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > >>>>>>> On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>>>>>> Hi, > >>>>>>>> > >>>>>>>> The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > >>>>>>>> > >>>>>>>> Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > >>>>>>>> > >>>>>>>> are available in the git repository at: > >>>>>>>> > >>>>>>>> > >>>>>>>> git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > >>>>>>>> > >>>>>>>> for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > >>>>>>>> > >>>>>>>> arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > >>>>>>>> > >>>>>>>> ---------------------------------------------------------------- > >>>>>>>> pm: add suspend_mem and suspend_standby support > >>>>>>>> > >>>>>>>> Today when we go to suspend we can not knwon at drivers level if we go in > >>>>>>>> STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > >>>>>>>> suspend_standby. > >>>>>>> > >>>>>>> No way. Device drivers shouldn't be concerned about that. > >>>>>> I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > >>>>>> need special handling when switching to slow_clock > >>>>> > >>>>> Well, my answer to that is: please fix your platform code instead of > >>>>> hacking the PM core to work around its problems. > >>>> how can I fix drivers pm issue when I no way to known at driver level the > >>>> real suspend, the PM core is supposed to proivde the right information to the > >>>> drivers so the driver can put it's in the right pm mode. If the pm core can not > >>>> provide such inforation the PM core is broken as we will have to do dirty > >>>> hack. > >>> > >>> Why do you need to know the difference in your driver? We used to > >>> provide this information a long time ago, but it turned out to not be > >>> needed at all and just caused problems. > >> because on at91 I need to handle the mem standby at drviers level. > > > > This is not an answer. You're basically saying "it has to be done this way, > > because it has to be done this way". > > > >> We do it today already by a hack in different drivers at91_udc (usb device), > >> atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching > >> to mem pm. On at91 when switch to mem we shutdown everything and run form a slow > >> clock - this is done at soc level - but those IP have issue and need specific > >> care before doing so. Ohterwise when the SoC will wakeup but those IP will not > >> > >> in this patch series I send the update of those 3 drivers too > >> and kill the hack > > > > Well, let's start over. What's the difference between "suspend to RAM" (mem) > > and "standby" on your platform? > > > >>>> Any generic framework is supposed to evolve for real user need here I've one > >>>> so I udpate the core to mach a need > > > > Yes, if there are more users needing a change. If there's only one, I think not > > really. > > > > We came across such a need while supporting various low-power modes in > AM335x SoC. I also wanted to put similar proposal, its good that Jean > submitted patches and we are having this discussion early. > > Let me try to describe the need and issue here, > > In case of AM33xx device, we have different low-power modes supported > by HW, for this discussion I will just talk about DeepSleep-0 and > StandBy mode supported by HW (few other modes also described in spec)- > > Below is the Spec definition of these two low-power modes, > > PowerMode Application state States > ======================================================================== > DeepSleep0 Everything is preserved Master Oscillator = OFF > including SDRAM. VDD_MPU = 0.95v > VDD_CORE = 0.95v > PD_WKUP = ON > PD_MPU = OFF > PD_PER = OFF > PD_GFX = OFF > All IOs & RTC supplies are ON > > > Standby Everything is preserved Master Oscillator = ON > including SDRAM. One PLL is ON > Only required module clocks VDD_MPU = 0.95v > are enabled rest are clock VDD_CORE = OPP50 > gated PD_WKUP = ON > PD_MPU = ON > PD_PER = ON > PD_GFX = OFF > All IOs & RTC supplies are ON > If more PLLs /power domains/ > modules are required, the > power will increase > accordingly > > > > Now we have two major parameters here, Power Consumption and Latency > (Sleep + Wakeup). Since DeepSleep-0 is as good as off state, it gives > very less power consumption Vs huge Latency number, whereas, in case of > standby the power consumption is more but the Latency is reduced. > > In addition to that, there are certain HW related parameters and issues > contribute to these parameters. For example, > > In case of LCD driver the suspend/resume latency is in the range of > 10msec, which can be reduced if driver knows that he is not entering or > entered in DeepSleep-0 and choose not to do certain things. > Another example would be, USB driver, we have seen Latency numbers in > the range of 200msec, and of which 100 msec can simply be taken away if > driver knows about the sleep state. > > > I understand that, there is trade-off between power and latency numbers, > but for certain use-cases latency is more important and it is not > possible without telling driver about the low-power state. I understand the problem at hand, but the question is who's going to tell the driver about that. Arguably, the PM core is not the right source of that information, because if a driver is told "we're going into standby", it will have to ask the platform about what "standby" actually means. Thanks, Rafael
On 01:05 Thu 11 Oct , Rafael J. Wysocki wrote: > On Wednesday 10 of October 2012 12:47:31 Vaibhav Hiremath wrote: > > > > On 10/10/2012 3:42 AM, Rafael J. Wysocki wrote: > > > On Tuesday 09 of October 2012 17:17:04 Jean-Christophe PLAGNIOL-VILLARD wrote: > > >> On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: > > >>> On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: > > >>>> On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: > > >>>>> On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: > > >>>>>> On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: > > >>>>>>> On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: > > >>>>>>>> Hi, > > >>>>>>>> > > >>>>>>>> The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: > > >>>>>>>> > > >>>>>>>> Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) > > >>>>>>>> > > >>>>>>>> are available in the git repository at: > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem > > >>>>>>>> > > >>>>>>>> for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: > > >>>>>>>> > > >>>>>>>> arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) > > >>>>>>>> > > >>>>>>>> ---------------------------------------------------------------- > > >>>>>>>> pm: add suspend_mem and suspend_standby support > > >>>>>>>> > > >>>>>>>> Today when we go to suspend we can not knwon at drivers level if we go in > > >>>>>>>> STANDBY or MEM. Fix this by introducing two new callback suspend_mem and > > >>>>>>>> suspend_standby. > > >>>>>>> > > >>>>>>> No way. Device drivers shouldn't be concerned about that. > > >>>>>> I do need it on at91 as we swith to slow_clock in MEM suspend and some ip > > >>>>>> need special handling when switching to slow_clock > > >>>>> > > >>>>> Well, my answer to that is: please fix your platform code instead of > > >>>>> hacking the PM core to work around its problems. > > >>>> how can I fix drivers pm issue when I no way to known at driver level the > > >>>> real suspend, the PM core is supposed to proivde the right information to the > > >>>> drivers so the driver can put it's in the right pm mode. If the pm core can not > > >>>> provide such inforation the PM core is broken as we will have to do dirty > > >>>> hack. > > >>> > > >>> Why do you need to know the difference in your driver? We used to > > >>> provide this information a long time ago, but it turned out to not be > > >>> needed at all and just caused problems. > > >> because on at91 I need to handle the mem standby at drviers level. > > > > > > This is not an answer. You're basically saying "it has to be done this way, > > > because it has to be done this way". > > > > > >> We do it today already by a hack in different drivers at91_udc (usb device), > > >> atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching > > >> to mem pm. On at91 when switch to mem we shutdown everything and run form a slow > > >> clock - this is done at soc level - but those IP have issue and need specific > > >> care before doing so. Ohterwise when the SoC will wakeup but those IP will not > > >> > > >> in this patch series I send the update of those 3 drivers too > > >> and kill the hack > > > > > > Well, let's start over. What's the difference between "suspend to RAM" (mem) > > > and "standby" on your platform? > > > > > >>>> Any generic framework is supposed to evolve for real user need here I've one > > >>>> so I udpate the core to mach a need > > > > > > Yes, if there are more users needing a change. If there's only one, I think not > > > really. > > > > > > > We came across such a need while supporting various low-power modes in > > AM335x SoC. I also wanted to put similar proposal, its good that Jean > > submitted patches and we are having this discussion early. > > > > Let me try to describe the need and issue here, > > > > In case of AM33xx device, we have different low-power modes supported > > by HW, for this discussion I will just talk about DeepSleep-0 and > > StandBy mode supported by HW (few other modes also described in spec)- > > > > Below is the Spec definition of these two low-power modes, > > > > PowerMode Application state States > > ======================================================================== > > DeepSleep0 Everything is preserved Master Oscillator = OFF > > including SDRAM. VDD_MPU = 0.95v > > VDD_CORE = 0.95v > > PD_WKUP = ON > > PD_MPU = OFF > > PD_PER = OFF > > PD_GFX = OFF > > All IOs & RTC supplies are ON > > > > > > Standby Everything is preserved Master Oscillator = ON > > including SDRAM. One PLL is ON > > Only required module clocks VDD_MPU = 0.95v > > are enabled rest are clock VDD_CORE = OPP50 > > gated PD_WKUP = ON > > PD_MPU = ON > > PD_PER = ON > > PD_GFX = OFF > > All IOs & RTC supplies are ON > > If more PLLs /power domains/ > > modules are required, the > > power will increase > > accordingly > > > > > > > > Now we have two major parameters here, Power Consumption and Latency > > (Sleep + Wakeup). Since DeepSleep-0 is as good as off state, it gives > > very less power consumption Vs huge Latency number, whereas, in case of > > standby the power consumption is more but the Latency is reduced. > > > > In addition to that, there are certain HW related parameters and issues > > contribute to these parameters. For example, > > > > In case of LCD driver the suspend/resume latency is in the range of > > 10msec, which can be reduced if driver knows that he is not entering or > > entered in DeepSleep-0 and choose not to do certain things. > > Another example would be, USB driver, we have seen Latency numbers in > > the range of 200msec, and of which 100 msec can simply be taken away if > > driver knows about the sleep state. > > > > > > I understand that, there is trade-off between power and latency numbers, > > but for certain use-cases latency is more important and it is not > > possible without telling driver about the low-power state. > > I understand the problem at hand, but the question is who's going to tell > the driver about that. > > Arguably, the PM core is not the right source of that information, because > if a driver is told "we're going into standby", it will have to ask the > platform about what "standby" actually means. who is going to tell the kernel to go to standby and which the sysfs handle via PM core so here the one who alwasy have the information is the pm core Best Regards, J.
[...] > > > I understand that, there is trade-off between power and latency numbers, > > > but for certain use-cases latency is more important and it is not > > > possible without telling driver about the low-power state. > > > > I understand the problem at hand, but the question is who's going to tell > > the driver about that. > > > > Arguably, the PM core is not the right source of that information, because > > if a driver is told "we're going into standby", it will have to ask the > > platform about what "standby" actually means. > > who is going to tell the kernel to go to standby and which > > the sysfs handle via PM core > > so here the one who alwasy have the information is the pm core The core tells the _platform_ "the user wants us to go into standby" or "the user wants us to go into suspend", but it doesn't really know what "standby" or "suspend" actually mean. It can't, because these things are not universally well defined and there are differences between platforms in that area. Different platforms may require different things to be done to devices for "standby" and so drivers should not assume that "standby" will always mean the same thing. For example, if your driver is written for and ARM SoC and then the same IP block happens to be used on an x86-based thing, whatever the driver did and was specific to ARM is no longer valid. So if your ARM SoC required drivers to do something special during a "suspend" transition which wasn't necessary to do during a "standby" transition, it doesn't necessarily mean that the new platform will do so as well. The "mem" and "standby" things may have different meanings for the new platform and for this reason drivers _have_ _to_ ask the _platform_ what states to put the devices into _or_, alternatively, leave the platform-specific handling of the devices to the platform (that's what device drivers on ACPI-based x86 do). They should not expect the PM core to tell them that, simply because the PM core has no idea what that may be. And by the way, ACPI-based ARM platforms are reportedly in the works and they _will_ use the same IP blocks that are already in use on ARM. On those platforms drivers will be supposed to use ACPI for power management and the whole design you're proposing will not make sense any more for them. Thanks, Rafael
On 10.10.2012 11:29, Hiremath, Vaibhav wrote: > On Wed, Oct 10, 2012 at 14:50:25, Daniel Mack wrote: >> On 10.10.2012 09:17, Vaibhav Hiremath wrote: >>> On 10/10/2012 3:42 AM, Rafael J. Wysocki wrote: >>>> On Tuesday 09 of October 2012 17:17:04 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>> On 07:58 Tue 09 Oct , Greg Kroah-Hartman wrote: >>>>>> On Tue, Oct 09, 2012 at 01:46:33PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>>> On 22:02 Sun 07 Oct , Rafael J. Wysocki wrote: >>>>>>>> On Sunday 07 of October 2012 15:12:01 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>>>>> On 00:18 Sun 07 Oct , Rafael J. Wysocki wrote: >>>>>>>>>> On Saturday 06 of October 2012 18:14:29 Jean-Christophe PLAGNIOL-VILLARD wrote: >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: >>>>>>>>>>> >>>>>>>>>>> Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) >>>>>>>>>>> >>>>>>>>>>> are available in the git repository at: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem >>>>>>>>>>> >>>>>>>>>>> for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: >>>>>>>>>>> >>>>>>>>>>> arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) >>>>>>>>>>> >>>>>>>>>>> ---------------------------------------------------------------- >>>>>>>>>>> pm: add suspend_mem and suspend_standby support >>>>>>>>>>> >>>>>>>>>>> Today when we go to suspend we can not knwon at drivers level if we go in >>>>>>>>>>> STANDBY or MEM. Fix this by introducing two new callback suspend_mem and >>>>>>>>>>> suspend_standby. >>>>>>>>>> >>>>>>>>>> No way. Device drivers shouldn't be concerned about that. >>>>>>>>> I do need it on at91 as we swith to slow_clock in MEM suspend and some ip >>>>>>>>> need special handling when switching to slow_clock >>>>>>>> >>>>>>>> Well, my answer to that is: please fix your platform code instead of >>>>>>>> hacking the PM core to work around its problems. >>>>>>> how can I fix drivers pm issue when I no way to known at driver level the >>>>>>> real suspend, the PM core is supposed to proivde the right information to the >>>>>>> drivers so the driver can put it's in the right pm mode. If the pm core can not >>>>>>> provide such inforation the PM core is broken as we will have to do dirty >>>>>>> hack. >>>>>> >>>>>> Why do you need to know the difference in your driver? We used to >>>>>> provide this information a long time ago, but it turned out to not be >>>>>> needed at all and just caused problems. >>>>> because on at91 I need to handle the mem standby at drviers level. >>>> >>>> This is not an answer. You're basically saying "it has to be done this way, >>>> because it has to be done this way". >>>> >>>>> We do it today already by a hack in different drivers at91_udc (usb device), >>>>> atmel_serail and at91_ohci. Those 3 IP have specifci handling when switching >>>>> to mem pm. On at91 when switch to mem we shutdown everything and run form a slow >>>>> clock - this is done at soc level - but those IP have issue and need specific >>>>> care before doing so. Ohterwise when the SoC will wakeup but those IP will not >>>>> >>>>> in this patch series I send the update of those 3 drivers too >>>>> and kill the hack >>>> >>>> Well, let's start over. What's the difference between "suspend to RAM" (mem) >>>> and "standby" on your platform? >>>> >>>>>>> Any generic framework is supposed to evolve for real user need here I've one >>>>>>> so I udpate the core to mach a need >>>> >>>> Yes, if there are more users needing a change. If there's only one, I think not >>>> really. >>>> >>> >>> We came across such a need while supporting various low-power modes in >>> AM335x SoC. I also wanted to put similar proposal, its good that Jean >>> submitted patches and we are having this discussion early. >>> >>> Let me try to describe the need and issue here, >>> >>> In case of AM33xx device, we have different low-power modes supported >>> by HW, for this discussion I will just talk about DeepSleep-0 and >>> StandBy mode supported by HW (few other modes also described in spec)- >>> >>> Below is the Spec definition of these two low-power modes, >> >> Thanks for this summary, which raises a question for me that might be >> slightly unreleated to the general discussion, but still: >> >> I was just looking at the code that ships with the BSP kernel, and the >> approach there is to load a binary firmare blob to the M3 UMEM and then >> communicates with mailbox commands in order to put the system to suspend. >> > > Ohh, Great, then you are aware of AM33xx power management support. > As you are aware, M3 here works as a soft-PRCM block, so certain things has > to be done on M3 while entering into low-power state. > >> Is this how it should be done in mainline as well or are there any plans >> to implement that behaviour natively? >> > > Yes, we will follow similar approach for Mainline, its under development and > soon you will see patches getting submitted to the list for review. The > first step is to get deepsleep-0 support in Mainline and then other will > follow. May I ask whether there is anything yet I can test? Thanks, Daniel
(Changed $SUBJECT to reflect the topic being discussed) On Wed, Oct 31, 2012 at 15:57:13, Daniel Mack wrote: [...] > > May I ask whether there is anything yet I can test? I am working on basic suspend-resume patches for AM33XX which builds on top of the current linux-omap/master. Will be posting it later this week. Regards, Vaibhav
Hi, The following changes since commit 5f3d2f2e1a63679cf1c4a4210f2f1cc2f335bef6: Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc (2012-10-06 03:16:12 +0900) are available in the git repository at: git://git.jcrosoft.org/linux-2.6.git tags/pm_suspend_standby_mem for you to fetch changes up to b73c8f97aa8e720bd3b921159687d00626c99d63: arm: at91: drop at91_suspend_entering_slow_clock (2012-10-06 18:06:25 +0800) ---------------------------------------------------------------- pm: add suspend_mem and suspend_standby support Today when we go to suspend we can not knwon at drivers level if we go in STANDBY or MEM. Fix this by introducing two new callback suspend_mem and suspend_standby. If a bus or drivers does not need to care about this distinction we fallback to suspend. This will allow to do not update the current bus or drivers. This is needed as example by at91 OHCI, UDC and atmel serial Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> ---------------------------------------------------------------- Jean-Christophe PLAGNIOL-VILLARD (6): pm: add suspend_mem and suspend_standby support platform: pm: add suspend_mem and suspend_standby support tty: atmel_serial: switch pm ops usb: ohci-at91: switch pm ops usb: at91_ude: switch pm ops arm: at91: drop at91_suspend_entering_slow_clock arch/arm/mach-at91/include/mach/board.h | 3 --- arch/arm/mach-at91/pm.c | 24 ------------------------ drivers/base/platform.c | 40 ++++++++++++++++++++++++++++++++++++++++ drivers/base/power/main.c | 12 ++++++++++++ drivers/tty/serial/atmel_serial.c | 52 +++++++++++++++++++++++++++------------------------- drivers/usb/gadget/at91_udc.c | 34 +++++++++++++++++++++++----------- drivers/usb/host/ohci-at91.c | 38 ++++++++++++++++++++++++-------------- include/linux/platform_device.h | 6 ++++++ include/linux/pm.h | 9 +++++++++ kernel/power/suspend.c | 16 ++++++++++++++-- 10 files changed, 155 insertions(+), 79 deletions(-) Best Regards, J.