Re: [PATCH v11 0/4] Allow USB devices to remain runtime-suspended when sleeping

[Rafael J. Wysocki]

On Monday, January 04, 2016 06:27:18 PM Derek Basehore wrote:
> On Mon, Nov 02, 2015 at 02:50:40AM +0100, Rafael J. Wysocki wrote:
> > 
> > I've queued up this series for the second half of the v4.4 merge window.
> > 
> > Thanks,
> > Rafael
> > 
> > 
> > ___
> > linux-arm-kernel mailing list
> > linux-arm-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> What's the status of this patch series? I haven't seen the patches
> posted for v4.4, plus there's the issue that Dan found that needs to be
> addressed.
> 
> Is there a new revision of the patch series being worked on?

Tomeu is not working on one AFAICS, but I may just revive his series.

Do you have a pointer to the Dan's report?

Thanks,
Rafael

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

Re: [PATCH v11 0/4] Allow USB devices to remain runtime-suspended when sleeping

[Rafael J. Wysocki]

On Tuesday, October 27, 2015 03:38:47 PM Tomeu Vizoso wrote:
> Hi,
> 
> this is v11 of an attempt to make it easier for devices to remain in
> runtime PM when the system goes to sleep, mainly to reduce the time
> spent resuming devices.
> 
> For this, we interpret the absence of all PM callback implementations as
> it being safe to do direct_complete, so their ancestors aren't prevented
> from remaining runtime-suspended.
> 
> Additionally, the prepare() callback of USB devices will return 1 if
> runtime PM is enabled and the current wakeup settings are correct.
> 
> With these changes, a uvcvideo device (for example) stays in runtime
> suspend when the system goes to sleep and is left in that state when the
> system resumes, not delaying it unnecessarily.
> 
> Thanks,
> 
> Tomeu
> 
> Changes in v11:
> - Move calls to dev_pm_domain_set() out from >power.lock as that
>   isn't needed for dev->pm_domain.
> 
> Changes in v10:
> - Remove superfluous call to pm_runtime_enabled() as suggested by Alan
> 
> Changes in v9:
> - Add docs noting the need for the device lock to be held before calling
>   device_is_bound()
> - Add docs noting the need for the device lock to be held before calling
>   dev_pm_domain_set()
> - Move to CONFIG_PM_SLEEP as suggested by Rafael and Ulf.
> - Rename from device_check_pm_callbacks to device_pm_check_callbacks to
>   follow with the naming convention of existing API.
> - Re-add calling to device_pm_check_callbacks from device registration
>   and when updating the PM domain of a device.
> 
> Changes in v8:
> - Add device_is_bound()
> - Add dev_pm_domain_set() and update code to use it
> - Move no_pm_callbacks field into CONFIG_PM_SLEEP
> - Call device_check_pm_callbacks only after a device is bound or unbound
> 
> Changes in v7:
> - Reduce indentation by adding a label in device_prepare()
> 
> Changes in v6:
> - Add stub for !CONFIG_PM.
> - Move implementation of device_check_pm_callbacks to power/main.c as it
>   doesn't belong to CONFIG_PM_SLEEP.
> - Take dev->power.lock before modifying flag.
> 
> Changes in v5:
> - Check for all dev_pm_ops instances associated to a device, updating a
>   no_pm_callbacks flag at the times when that could change.
> 
> Tomeu Vizoso (4):
>   device core: add device_is_bound()
>   PM / Domains: add setter for dev.pm_domain
>   PM / sleep: Go direct_complete if driver has no callbacks
>   USB / PM: Allow USB devices to remain runtime-suspended when sleeping
> 
>  arch/arm/mach-omap2/omap_device.c |  7 ---
>  drivers/acpi/acpi_lpss.c  |  5 +++--
>  drivers/acpi/device_pm.c  |  5 +++--
>  drivers/base/dd.c | 21 +++--
>  drivers/base/power/clock_ops.c|  5 +++--
>  drivers/base/power/common.c   | 24 
>  drivers/base/power/domain.c   |  8 ++--
>  drivers/base/power/main.c | 35 +++
>  drivers/base/power/power.h|  3 +++
>  drivers/gpu/vga/vga_switcheroo.c  | 10 +-
>  drivers/misc/mei/pci-me.c |  5 +++--
>  drivers/misc/mei/pci-txe.c|  5 +++--
>  drivers/usb/core/port.c   |  6 ++
>  drivers/usb/core/usb.c|  8 +++-
>  include/linux/device.h|  2 ++
>  include/linux/pm.h|  1 +
>  include/linux/pm_domain.h |  3 +++
>  17 files changed, 130 insertions(+), 23 deletions(-)

I've queued up this series for the second half of the v4.4 merge window.

Thanks,
Rafael

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

Re: [PATCH v10 2/4] PM / Domains: add setter for dev.pm_domain

[Rafael J. Wysocki]

On Wednesday, October 21, 2015 05:34:12 PM Tomeu Vizoso wrote:
> Adds a function that sets the pointer to dev_pm_domain in struct device
> and that warns if the device has already finished probing. The reason
> why we want to enforce that is because in the general case that can
> cause problems and also that we can simplify code quite a bit if we can
> always assume that.
> 
> This patch also changes all current code that directly sets the
> dev.pm_domain pointer.
> 
> Signed-off-by: Tomeu Vizoso 
> Reviewed-by: Ulf Hansson 
> ---
> 
> Changes in v9:
> - Add docs noting the need for the device lock to be held before calling
>   dev_pm_domain_set()
> 
> Changes in v8:
> - Add dev_pm_domain_set() and update code to use it
> 
>  arch/arm/mach-omap2/omap_device.c |  7 ---
>  drivers/acpi/acpi_lpss.c  |  5 +++--
>  drivers/acpi/device_pm.c  |  5 +++--
>  drivers/base/power/clock_ops.c|  5 +++--
>  drivers/base/power/common.c   | 21 +
>  drivers/base/power/domain.c   |  4 ++--
>  drivers/gpu/vga/vga_switcheroo.c  | 10 +-
>  drivers/misc/mei/pci-me.c |  5 +++--
>  drivers/misc/mei/pci-txe.c|  5 +++--
>  include/linux/pm_domain.h |  3 +++
>  10 files changed, 50 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_device.c 
> b/arch/arm/mach-omap2/omap_device.c
> index 4cb8fd9f741f..204101d11632 100644
> --- a/arch/arm/mach-omap2/omap_device.c
> +++ b/arch/arm/mach-omap2/omap_device.c
> @@ -32,6 +32,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -168,7 +169,7 @@ static int omap_device_build_from_dt(struct 
> platform_device *pdev)
>   r->name = dev_name(>dev);
>   }
>  
> - pdev->dev.pm_domain = _device_pm_domain;
> + dev_pm_domain_set(>dev, _device_pm_domain);
>  
>   if (device_active) {
>   omap_device_enable(pdev);
> @@ -180,7 +181,7 @@ odbfd_exit1:
>  odbfd_exit:
>   /* if data/we are at fault.. load up a fail handler */
>   if (ret)
> - pdev->dev.pm_domain = _device_fail_pm_domain;
> + dev_pm_domain_set(>dev, _device_fail_pm_domain);
>  
>   return ret;
>  }
> @@ -701,7 +702,7 @@ int omap_device_register(struct platform_device *pdev)
>  {
>   pr_debug("omap_device: %s: registering\n", pdev->name);
>  
> - pdev->dev.pm_domain = _device_pm_domain;
> + dev_pm_domain_set(>dev, _device_pm_domain);
>   return platform_device_add(pdev);
>  }
>  
> diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> index f51bd0d0bc17..cc6e1abc69b3 100644
> --- a/drivers/acpi/acpi_lpss.c
> +++ b/drivers/acpi/acpi_lpss.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -706,7 +707,7 @@ static int acpi_lpss_platform_notify(struct 
> notifier_block *nb,
>  
>   switch (action) {
>   case BUS_NOTIFY_ADD_DEVICE:
> - pdev->dev.pm_domain = _lpss_pm_domain;
> + dev_pm_domain_set(>dev, _lpss_pm_domain);
>   if (pdata->dev_desc->flags & LPSS_LTR)
>   return sysfs_create_group(>dev.kobj,
> _attr_group);
> @@ -714,7 +715,7 @@ static int acpi_lpss_platform_notify(struct 
> notifier_block *nb,
>   case BUS_NOTIFY_DEL_DEVICE:
>   if (pdata->dev_desc->flags & LPSS_LTR)
>   sysfs_remove_group(>dev.kobj, _attr_group);
> - pdev->dev.pm_domain = NULL;
> + dev_pm_domain_set(>dev, NULL);
>   break;
>   default:
>   break;
> diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
> index 4806b7f856c4..8c5955bf9bbf 100644
> --- a/drivers/acpi/device_pm.c
> +++ b/drivers/acpi/device_pm.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "internal.h"
> @@ -1076,7 +1077,7 @@ static void acpi_dev_pm_detach(struct device *dev, bool 
> power_off)
>   struct acpi_device *adev = ACPI_COMPANION(dev);
>  
>   if (adev && dev->pm_domain == _general_pm_domain) {
> - dev->pm_domain = NULL;
> + dev_pm_domain_set(dev, NULL);
>   acpi_remove_pm_notifier(adev);
>   if (power_off) {
>   /*
> @@ -1128,7 +1129,7 @@ int acpi_dev_pm_attach(struct device *dev, bool 
> power_on)
>   return -EBUSY;
>  
>   acpi_add_pm_notifier(adev, dev, acpi_pm_notify_work_func);
> - dev->pm_domain = _general_pm_domain;
> + dev_pm_domain_set(dev, _general_pm_domain);
>   if (power_on) {
>   acpi_dev_pm_full_power(adev);
>   acpi_device_wakeup(adev, ACPI_STATE_S0, false);
> diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
> index 652b5a367c1f..e80fda6c03a9 100644
> --- a/drivers/base/power/clock_ops.c
> +++ 

Re: [PATCH v3 1/2] Trace: PM: promote event 'power_domain_target' to generic power domains.

[Rafael J. Wysocki]

On Wednesday, October 14, 2015 10:18:27 AM Marc Titinger wrote:
> On 14/10/2015 02:55, Rafael J. Wysocki wrote:
> > On Monday, September 28, 2015 03:20:44 PM Marc Titinger wrote:
> >> - change arg3 to a state name string: we got the current CPU rom the trace
> >> backend already. This also prepares for multiple/named states in the power
> >> domain, consistent with idle-states. states in the domain may match given
> >> CPU states in this case, we will trace them by their name, and potentially
> >> use arg2 as a link to their index for the cpuidle driver.
> >>
> >> - tested with Juno DP
> >>
> >> -0 [000]42.395510: power_domain_target:  a53_pd index=0 
> >> 'cluster-sleep-0'
> >> -0 [000]42.395528: cpu_idle: state=4294967295 
> >> cpu_id=0
> >> -0 [000]42.395668: cpu_idle: state=2 cpu_id=0
> >>
> >> - compiled for Omap2+
> >>
> >> Signed-off-by: Marc Titinger <mtitin...@baylibre.com>
> >
> > Hi,
> >
> > What's your intent regarding this series?  Do you want it to be applied
> > separately, or is it going to be part of a larger series?
> 
> Hi Rafael,
> 
> v3 is a missed attempt to rebase this on the current head, but I did not 
> fix v3 because thinking twice it's not that useful until Axel Haslam's 
> multiple genpd states stuff is merged in. I'm finding v2 useful, shall I 
> re-post v2 as v4 for clarity then ?

That's up to you mostly.

If I get ACKs from all of the appropriate people on those, I can easily apply 
them.

Thanks,
Rafael

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

Re: [PATCH v3 1/2] Trace: PM: promote event 'power_domain_target' to generic power domains.

[Rafael J. Wysocki]

On Monday, September 28, 2015 03:20:44 PM Marc Titinger wrote:
> - change arg3 to a state name string: we got the current CPU rom the trace
> backend already. This also prepares for multiple/named states in the power
> domain, consistent with idle-states. states in the domain may match given
> CPU states in this case, we will trace them by their name, and potentially
> use arg2 as a link to their index for the cpuidle driver.
> 
> - tested with Juno DP
> 
> -0 [000]42.395510: power_domain_target:  a53_pd index=0 
> 'cluster-sleep-0'
> -0 [000]42.395528: cpu_idle: state=4294967295 
> cpu_id=0
> -0 [000]42.395668: cpu_idle: state=2 cpu_id=0
> 
> - compiled for Omap2+
> 
> Signed-off-by: Marc Titinger 

Hi,

What's your intent regarding this series?  Do you want it to be applied
separately, or is it going to be part of a larger series?

Thanks,
Rafael

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

Re: [PATCH v9 0/4] Allow USB devices to remain runtime-suspended when sleeping

[Rafael J. Wysocki]

On Monday, October 05, 2015 04:45:28 PM Tomeu Vizoso wrote:
> Hi,
> 
> this is v9 of an attempt to make it easier for devices to remain in
> runtime PM when the system goes to sleep, mainly to reduce the time
> spent resuming devices.
> 
> For this, we interpret the absence of all PM callback implementations as
> it being safe to do direct_complete, so their ancestors aren't prevented
> from remaining runtime-suspended.
> 
> Additionally, the prepare() callback of USB devices will return 1 if
> runtime PM is enabled and the current wakeup settings are correct.
> 
> With these changes, a uvcvideo device (for example) stays in runtime
> suspend when the system goes to sleep and is left in that state when the
> system resumes, not delaying it unnecessarily.
> 
> Thanks,
> 
> Tomeu
> 
> Changes in v9:
> - Add docs noting the need for the device lock to be held before calling
>   device_is_bound()
> - Add docs noting the need for the device lock to be held before calling
>   dev_pm_domain_set()
> - Move to CONFIG_PM_SLEEP as suggested by Rafael and Ulf.
> - Rename from device_check_pm_callbacks to device_pm_check_callbacks to
>   follow with the naming convention of existing API.
> - Re-add calling to device_pm_check_callbacks from device registration
>   and when updating the PM domain of a device.
> 
> Changes in v8:
> - Add device_is_bound()
> - Add dev_pm_domain_set() and update code to use it
> - Move no_pm_callbacks field into CONFIG_PM_SLEEP
> - Call device_check_pm_callbacks only after a device is bound or unbound
> 
> Changes in v7:
> - Reduce indentation by adding a label in device_prepare()
> 
> Changes in v6:
> - Add stub for !CONFIG_PM.
> - Move implementation of device_check_pm_callbacks to power/main.c as it
>   doesn't belong to CONFIG_PM_SLEEP.
> - Take dev->power.lock before modifying flag.
> 
> Changes in v5:
> - Check for all dev_pm_ops instances associated to a device, updating a
>   no_pm_callbacks flag at the times when that could change.
> 
> Tomeu Vizoso (4):
>   device core: add device_is_bound()
>   PM / Domains: add setter for dev.pm_domain
>   PM / sleep: Go direct_complete if driver has no callbacks
>   USB / PM: Allow USB devices to remain runtime-suspended when sleeping
> 
>  arch/arm/mach-omap2/omap_device.c |  7 ---
>  drivers/acpi/acpi_lpss.c  |  5 +++--
>  drivers/acpi/device_pm.c  |  5 +++--
>  drivers/base/dd.c | 21 +++--
>  drivers/base/power/clock_ops.c|  5 +++--
>  drivers/base/power/common.c   | 24 
>  drivers/base/power/domain.c   |  6 --
>  drivers/base/power/main.c | 35 +++
>  drivers/base/power/power.h|  3 +++
>  drivers/gpu/vga/vga_switcheroo.c  | 10 +-
>  drivers/misc/mei/pci-me.c |  5 +++--
>  drivers/misc/mei/pci-txe.c|  5 +++--
>  drivers/usb/core/port.c   |  6 ++
>  drivers/usb/core/usb.c| 11 ++-
>  include/linux/device.h|  2 ++
>  include/linux/pm.h|  1 +
>  include/linux/pm_domain.h |  3 +++
>  17 files changed, 131 insertions(+), 23 deletions(-)

The series looks good to me now, but patch [4/4] needs an ACK from the USB
maintainers and patch [1/4] needs an ACK from Greg.

Thanks,
Rafael

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

Re: [PATCH] base: power: wakeirq: don't leak dev-power.wakeirq

[Rafael J. Wysocki]

On Tue, Jul 7, 2015 at 10:11 AM, Felipe Balbi ba...@ti.com wrote:
 On Tue, Jul 07, 2015 at 12:40:53AM -0700, Tony Lindgren wrote:
 * Rafael J. Wysocki r...@rjwysocki.net [150706 15:49]:
  On Monday, July 06, 2015 01:01:18 PM Felipe Balbi wrote:
   on a first call to dev_pm_attach_wake_irq(), if it
   fails, it will leave dev-power.wakeirq set to a
   dangling pointer. Instead, let's clear it to make
   sure a subsequent call to dev_pm_attach_wake_irq()
   has chance to succeed.
  
   Cc: Tony Lindgren tml...@atomide.com
   Signed-off-by: Felipe Balbi ba...@ti.com
   ---
drivers/base/power/wakeirq.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
  
   diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
   index 7470004ca810..394d250a1ad8 100644
   --- a/drivers/base/power/wakeirq.c
   +++ b/drivers/base/power/wakeirq.c
   @@ -50,9 +50,16 @@ static int dev_pm_attach_wake_irq(struct device *dev, 
   int irq,
  
 err = device_wakeup_attach_irq(dev, wirq);
 if (err)
   - return err;
   + goto err_cleanup;
  
 return 0;
   +
   +err_cleanup:
   + spin_lock_irqsave(dev-power.lock, flags);
   + dev-power.wakeirq = NULL;
   + spin_unlock_irqrestore(dev-power.lock, flags);
   +
   + return err;
}
 
  Too many labels for me and the fact that acquiring of the lock again in 
  the error
  patch doesn't look good.
 
  However, we can do the entire device_wakeup_attach_irq() under the lock 
  (after
  removing the locking from it), because we're its only caller.
 
  So what about the below instead (build-tested only)?

 Nice, still works for me and simplifies things:

 Tested-by: Tony Lindgren t...@atomide.com

 Cool, thanks for testing Tony. Rafael, I'm fine with your version too.
 FWIW:

 Reported-by: Felipe Balbi ba...@ti.com

OK, applied.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] base: power: wakeirq: don't leak dev-power.wakeirq

[Rafael J. Wysocki]

On Monday, July 06, 2015 01:01:18 PM Felipe Balbi wrote:
 on a first call to dev_pm_attach_wake_irq(), if it
 fails, it will leave dev-power.wakeirq set to a
 dangling pointer. Instead, let's clear it to make
 sure a subsequent call to dev_pm_attach_wake_irq()
 has chance to succeed.
 
 Cc: Tony Lindgren tml...@atomide.com
 Signed-off-by: Felipe Balbi ba...@ti.com
 ---
  drivers/base/power/wakeirq.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
 index 7470004ca810..394d250a1ad8 100644
 --- a/drivers/base/power/wakeirq.c
 +++ b/drivers/base/power/wakeirq.c
 @@ -50,9 +50,16 @@ static int dev_pm_attach_wake_irq(struct device *dev, int 
 irq,
  
   err = device_wakeup_attach_irq(dev, wirq);
   if (err)
 - return err;
 + goto err_cleanup;
  
   return 0;
 +
 +err_cleanup:
 + spin_lock_irqsave(dev-power.lock, flags);
 + dev-power.wakeirq = NULL;
 + spin_unlock_irqrestore(dev-power.lock, flags);
 +
 + return err;
  }

Too many labels for me and the fact that acquiring of the lock again in the 
error
patch doesn't look good.

However, we can do the entire device_wakeup_attach_irq() under the lock (after
removing the locking from it), because we're its only caller.

So what about the below instead (build-tested only)?

Rafael


---
 drivers/base/power/wakeirq.c |   12 +---
 drivers/base/power/wakeup.c  |   31 ++-
 2 files changed, 15 insertions(+), 28 deletions(-)

Index: linux-pm/drivers/base/power/wakeirq.c
===
--- linux-pm.orig/drivers/base/power/wakeirq.c
+++ linux-pm/drivers/base/power/wakeirq.c
@@ -45,14 +45,12 @@ static int dev_pm_attach_wake_irq(struct
return -EEXIST;
}
 
-   dev-power.wakeirq = wirq;
-   spin_unlock_irqrestore(dev-power.lock, flags);
-
err = device_wakeup_attach_irq(dev, wirq);
-   if (err)
-   return err;
+   if (!err)
+   dev-power.wakeirq = wirq;
 
-   return 0;
+   spin_unlock_irqrestore(dev-power.lock, flags);
+   return err;
 }
 
 /**
@@ -105,10 +103,10 @@ void dev_pm_clear_wake_irq(struct device
return;
 
spin_lock_irqsave(dev-power.lock, flags);
+   device_wakeup_detach_irq(dev);
dev-power.wakeirq = NULL;
spin_unlock_irqrestore(dev-power.lock, flags);
 
-   device_wakeup_detach_irq(dev);
if (wirq-dedicated_irq)
free_irq(wirq-irq, wirq);
kfree(wirq);
Index: linux-pm/drivers/base/power/wakeup.c
===
--- linux-pm.orig/drivers/base/power/wakeup.c
+++ linux-pm/drivers/base/power/wakeup.c
@@ -281,32 +281,25 @@ EXPORT_SYMBOL_GPL(device_wakeup_enable);
  * Attach a device wakeirq to the wakeup source so the device
  * wake IRQ can be configured automatically for suspend and
  * resume.
+ *
+ * Call under the device's power.lock lock.
  */
 int device_wakeup_attach_irq(struct device *dev,
 struct wake_irq *wakeirq)
 {
struct wakeup_source *ws;
-   int ret = 0;
 
-   spin_lock_irq(dev-power.lock);
ws = dev-power.wakeup;
if (!ws) {
dev_err(dev, forgot to call call device_init_wakeup?\n);
-   ret = -EINVAL;
-   goto unlock;
+   return -EINVAL;
}
 
-   if (ws-wakeirq) {
-   ret = -EEXIST;
-   goto unlock;
-   }
+   if (ws-wakeirq)
+   return -EEXIST;
 
ws-wakeirq = wakeirq;
-
-unlock:
-   spin_unlock_irq(dev-power.lock);
-
-   return ret;
+   return 0;
 }
 
 /**
@@ -314,20 +307,16 @@ unlock:
  * @dev: Device to handle
  *
  * Removes a device wakeirq from the wakeup source.
+ *
+ * Call under the device's power.lock lock.
  */
 void device_wakeup_detach_irq(struct device *dev)
 {
struct wakeup_source *ws;
 
-   spin_lock_irq(dev-power.lock);
ws = dev-power.wakeup;
-   if (!ws)
-   goto unlock;
-
-   ws-wakeirq = NULL;
-
-unlock:
-   spin_unlock_irq(dev-power.lock);
+   if (ws)
+   ws-wakeirq = NULL;
 }
 
 /**

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

Re: [PATCH 2/2] cpufreq: dt: allow driver to boot automatically

[Rafael J. Wysocki]

On Tue, Jun 16, 2015 at 11:40 PM, Felipe Balbi ba...@ti.com wrote:
 On Fri, May 08, 2015 at 02:57:30PM -0500, Felipe Balbi wrote:
 by adding the missing MODULE_ALIAS(), cpufreq-dt
 can be autoloaded by udev/systemd.

 Signed-off-by: Felipe Balbi ba...@ti.com

 looks like this wasn't applied anywhere. Viresh, can you apply this
 patch please ?

I've overlooked it, sorry about that.  Applied now, thanks!


 ---
  drivers/cpufreq/cpufreq-dt.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
 index bab67db54b7e..528a82bf5038 100644
 --- a/drivers/cpufreq/cpufreq-dt.c
 +++ b/drivers/cpufreq/cpufreq-dt.c
 @@ -416,6 +416,7 @@ static struct platform_driver dt_cpufreq_platdrv = {
  };
  module_platform_driver(dt_cpufreq_platdrv);

 +MODULE_ALIAS(platform:cpufreq-dt);
  MODULE_AUTHOR(Viresh Kumar viresh.ku...@linaro.org);
  MODULE_AUTHOR(Shawn Guo shawn@linaro.org);
  MODULE_DESCRIPTION(Generic cpufreq driver);
 --
 2.4.0.rc3


 --
 balbi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] PM / Wakeirq: Fix typo in prototype for dev_pm_set_dedicated_wake_irq

[Rafael J. Wysocki]

On Friday, May 29, 2015 12:18:37 PM Felipe Balbi wrote:
 
 --2fjX3cMESU3XgGmZ
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 Content-Transfer-Encoding: quoted-printable
 
 On Fri, May 29, 2015 at 09:54:02AM -0700, Tony Lindgren wrote:
  Looks like I only built test the dev_pm_set_wake_irq and not the
  dev_pm_set_dedicated_wake_irq case on x86.
 =20
  Turns out there's a typo for the dev_pm_set_dedicated_wake_irq
  prototype that causes a build error if CONFIG_COMPILE_TEST
  and CONFIG_MMC_OMAP_HS are selected.
 =20
  Cc: Stephen Rothwell s...@canb.auug.org.au
  Cc: Ulf Hansson ulf.hans...@linaro.org
  Reported-by: Jim Davis jim.ep...@gmail.com
  Signed-off-by: Tony Lindgren t...@atomide.com
 
 cool:
 
 Reviewed-by: Felipe Balbi ba...@ti.com

Applied, thanks!


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] mmc: omap_hsmmc: Change wake-up interrupt to use generic wakeirq

[Rafael J. Wysocki]

On 5/25/2015 10:38 AM, Ulf Hansson wrote:

On 14 May 2015 at 01:36, Tony Lindgren t...@atomide.com wrote:

We can now use generic wakeirq handling and remove the custom handling
for the wake-up interrupts.

Signed-off-by: Tony Lindgren t...@atomide.com

Rafael, if you are fine with it, please take this one through your
linux-pm tree.

Acked-by: Ulf Hansson ulf.hans...@linaro.org


I thought that Tony would resubmit this one.  Tony?

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

Re: [PATCH 2/5] PM / Wakeirq: Add automated device wake IRQ handling

[Rafael J. Wysocki]

On Tuesday, May 19, 2015 07:10:57 PM Tony Lindgren wrote:
 * Rafael J. Wysocki r...@rjwysocki.net [150519 17:01]:
  On Tuesday, May 19, 2015 04:27:56 PM Tony Lindgren wrote:
   * Rafael J. Wysocki r...@rjwysocki.net [150519 16:07]:
On Wednesday, May 20, 2015 12:41:06 AM Thomas Gleixner wrote:
 On Wed, 20 May 2015, Rafael J. Wysocki wrote:
  This one looks really good. :-)
  
  If it doesn't depend on anything, I can apply it right away, so 
  please let me
  know.
   
   Sure works for me, it just has a dependency to patch #1 in this
   series ([PATCH 1/5] PM / Runtime: Update last_busy in rpm_resume).
  
  OK, I'll queue them both up for 4.2, then.
  
  Please let me know if you need a special branch with them.
 
 A separate branch would be nice so I can merge that in too.

OK

I added a new pm-wakeirq branch to linux-pm.git with these two patches and
one other commit related to runtime PM (all based on 4.1-rc4).  This branch
will not be rebased going forward.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] PM / Wakeirq: Add automated device wake IRQ handling

[Rafael J. Wysocki]

On Wednesday, May 20, 2015 12:41:06 AM Thomas Gleixner wrote:
 On Wed, 20 May 2015, Rafael J. Wysocki wrote:
  This one looks really good. :-)
  
  If it doesn't depend on anything, I can apply it right away, so please let 
  me
  know.
 
 Can you pretty please trim your replies? It's a PITA to scroll through
 several pages of quoted patch just to find your signature as the extra
 content.

Sorry about that.

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

Re: [PATCH 0/5] PM / clock_ops: provide default runtime ops and cleanup users

[Rafael J. Wysocki]

On Thursday, April 23, 2015 02:03:08 PM Rajendra Nayak wrote:
 Most users of PM clocks do the exact same thing in runtime callbacks.
 Provide default callbacks and cleanup the existing users (keystone/davinci
 /omap1/sh)
 
 Rajendra Nayak (5):
   PM / clock_ops: Provide default runtime ops to users
   arm: keystone: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS
   arm: omap1: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS
   arm: davinci: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS
   drivers: sh: remove boilerplate code and use USE_PM_CLK_RUNTIME_OPS
 
  arch/arm/mach-davinci/pm_domain.c  | 32 +-
  arch/arm/mach-keystone/pm_domain.c | 33 +-
  arch/arm/mach-omap1/pm_bus.c   | 37 ++
  drivers/base/power/clock_ops.c | 38 ++
  drivers/sh/pm_runtime.c| 47 
 ++
  include/linux/pm_clock.h   | 10 
  6 files changed, 54 insertions(+), 143 deletions(-)

It is not particularly clear to me who is supposed to apply this series, but
I can do that if people don't have problems with that.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] PM / Wakeirq: Add minimal device wakeirq helper functions

[Rafael J. Wysocki]

On Sunday, March 08, 2015 11:43:34 AM Alan Stern wrote:
 On Sat, 7 Mar 2015, Rafael J. Wysocki wrote:
 
  But this is part of a bigger picture.  Namely, if a separete wakeup 
  interrupt
  is required for a device, the device's power.can_wakeup flag cannot be set
  until that interrupt has been successfully requested.  Also for devices that
  can signal wakeup via their own IO interrupts, it would make sense to allow
  those interrupts to be registered somehow as wakeup interrupts.
  
  So I wonder if we can define a new struct along the lines of your
  struct wakeirq_source, but call it struct wake_irq and make it look
  something like this:
  
  struct wake_irq {
 struct device *dev;
 int irq;
 irq_handler_t handler;
  };
  
  Then, add a struct wake_irq pointer to struct dev_pm_info *and* to
  struct wakeup_source.  Next, make dev_pm_request_wake_irq() allocate the
  structure and request the interrupt and only set the pointer to it from
  struct dev_pm_info *along* *with* power.can_wakeup if all that was
  successful.
  
  For devices that use their own IO IRQ for wakeup, we can add something
  like dev_pm_set_wake_irq() that will work analogously, but without 
  requesting
  the interrupt.  It will just set the dev and irq members of struct wake_irq
  and point struct dev_pm_info to it and set its power.can_wakeup flag.
  
  Then, device_wakeup_enable() will be able to see that the device has a
  wakeup IRQ and it may then point its own struct wake_irq pointer to that.
  The core may then use that pointer to trigger enable_irq_wake() for the
  IRQ in question and it will cover the devices that don't need separate
  wakeup interrupts too.
  
  Does that make sense to you?
 
 Can we back up a little?  What is the basic problem the two of you are 
 trying to solve?

Essentially, code duplication between drivers that all need to do the same
thing which can be moved to the core quite easily.

Rafael

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

Re: [PATCH 1/4] PM / Wakeirq: Add minimal device wakeirq helper functions

[Rafael J. Wysocki]

On Friday, March 06, 2015 03:05:40 PM Tony Lindgren wrote:
 * Alan Stern st...@rowland.harvard.edu [150306 11:05]:
  On Fri, 6 Mar 2015, Tony Lindgren wrote:
  
 + struct wakeirq_source *wirq = _wirq;
 + irqreturn_t ret = IRQ_NONE;
 +
 + /* We don't want RPM_ASYNC or RPM_NOWAIT here */
 + if (pm_runtime_suspended(wirq-dev)) {

What if the device is resumed on a different CPU right here?
   
   Good point, sounds like we need to do this in some pm_runtime
   function directly for the locking.

 + pm_runtime_mark_last_busy(wirq-dev);
 + pm_runtime_resume(wirq-dev);

Calling this with disabled interrupts is a bad idea in general.
Is the device guaranteed to have power.irq_safe set?
   
   Well right now it's using threaded irq, and I'd like to get rid of
   the pm_runtime calls in the regular driver interrupts completely.
   We need to ensure the device runtime_resume is completed before
   returning IRQ_HANDLED here.
  
  In general, runtime_resume methods are allowed to sleep.  They can't be
  used in an interrupt handler top half unless the driver has
  specifically promised they are IRQ-safe.  That's what Rafael was
  getting at.
 
 Yes I understand, otherwise things certainly would not work :)
 
  Of course, if this routine is a threaded-irq bottom half then there's 
  no problem.
 
 Right this is threaded-irq bottom half because the devices may
 need to restore state and start regulators.
  
I guess what you want to call here is pm_request_resume() and
I wouldn't say that calling pm_runtime_mark_last_busy() on a
suspended device was valid.
   
   I'll verify again, but I believe the issue was that without doing
   mark_last_busy here the device falls back asleep right away.
   That probably should be fixed in pm_runtime in general if that's
   the case.
  
  It's up to the subsystem to handle this.  For example, the USB 
  subsystem's runtime-resume routine calls pm_runtime_mark_last_busy.
 
 Hmm.. OK thanks this probably explains why pm_request_resume() did
 not work.
 
 For omaps, I could call this from the interconnect related code,
 but then how dow we deal with the subsystems that don't call it?

Good question. :-)

   Considering the above, should we add a new function something like
   pm_resume_complete() that does not need irq_safe set but does
   not return until the device has completed resume?
  
  That doesn't make sense.  You're asking for a routine that is allowed
  to sleep but can safely be called in interrupt context.
 
 Oh it naturally would not work in irq context, it's for the bottom
 half again. But I'll take a look if we can just call
 pm_request_resume() and disable_irq() on the wakeirq in without
 waiting for the device driver runtime_suspend to disable the wakeirq.
 That would minimize the interface to just dev_pm_request_wakeirq()
 and dev_pm_free_wakeirq().

But this is part of a bigger picture.  Namely, if a separete wakeup interrupt
is required for a device, the device's power.can_wakeup flag cannot be set
until that interrupt has been successfully requested.  Also for devices that
can signal wakeup via their own IO interrupts, it would make sense to allow
those interrupts to be registered somehow as wakeup interrupts.

So I wonder if we can define a new struct along the lines of your
struct wakeirq_source, but call it struct wake_irq and make it look
something like this:

struct wake_irq {
   struct device *dev;
   int irq;
   irq_handler_t handler;
};

Then, add a struct wake_irq pointer to struct dev_pm_info *and* to
struct wakeup_source.  Next, make dev_pm_request_wake_irq() allocate the
structure and request the interrupt and only set the pointer to it from
struct dev_pm_info *along* *with* power.can_wakeup if all that was
successful.

For devices that use their own IO IRQ for wakeup, we can add something
like dev_pm_set_wake_irq() that will work analogously, but without requesting
the interrupt.  It will just set the dev and irq members of struct wake_irq
and point struct dev_pm_info to it and set its power.can_wakeup flag.

Then, device_wakeup_enable() will be able to see that the device has a
wakeup IRQ and it may then point its own struct wake_irq pointer to that.
The core may then use that pointer to trigger enable_irq_wake() for the
IRQ in question and it will cover the devices that don't need separate
wakeup interrupts too.

Does that make sense to you?

Rafael

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

Re: [PATCH 1/4] PM / Wakeirq: Add minimal device wakeirq helper functions

[Rafael J. Wysocki]

On Friday, March 06, 2015 02:05:43 PM Alan Stern wrote:
 On Fri, 6 Mar 2015, Tony Lindgren wrote:
 
+   struct wakeirq_source *wirq = _wirq;
+   irqreturn_t ret = IRQ_NONE;
+
+   /* We don't want RPM_ASYNC or RPM_NOWAIT here */
+   if (pm_runtime_suspended(wirq-dev)) {
   
   What if the device is resumed on a different CPU right here?
  
  Good point, sounds like we need to do this in some pm_runtime
  function directly for the locking.
   
+   pm_runtime_mark_last_busy(wirq-dev);
+   pm_runtime_resume(wirq-dev);
   
   Calling this with disabled interrupts is a bad idea in general.
   Is the device guaranteed to have power.irq_safe set?
  
  Well right now it's using threaded irq, and I'd like to get rid of
  the pm_runtime calls in the regular driver interrupts completely.
  We need to ensure the device runtime_resume is completed before
  returning IRQ_HANDLED here.
 
 In general, runtime_resume methods are allowed to sleep.  They can't be
 used in an interrupt handler top half unless the driver has
 specifically promised they are IRQ-safe.  That's what Rafael was
 getting at.
 
 Of course, if this routine is a threaded-irq bottom half then there's 
 no problem.

Yup.  I overlooked the threaded part.

   I guess what you want to call here is pm_request_resume() and
   I wouldn't say that calling pm_runtime_mark_last_busy() on a
   suspended device was valid.
  
  I'll verify again, but I believe the issue was that without doing
  mark_last_busy here the device falls back asleep right away.
  That probably should be fixed in pm_runtime in general if that's
  the case.
 
 It's up to the subsystem to handle this.  For example, the USB 
 subsystem's runtime-resume routine calls pm_runtime_mark_last_busy.

I'm wondering, though, if there's any reason for us to avoid updating
power.last_busy in rpm_resume().

If I was a driver writer, I'd expect the core to do that for me quite frankly.

Rafael

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

Re: [PATCH 1/4] PM / Wakeirq: Add minimal device wakeirq helper functions

[Rafael J. Wysocki]

On Fri, Mar 6, 2015 at 3:02 AM, Rafael J. Wysocki r...@rjwysocki.net wrote:
 Please always CC linux-pm on CC patches.

Doh.  That was supposed to say Please always CC linux-pm on PM patches.

I really should not reply to email when I'm too tired ...
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] PM / Wakeirq: Add minimal device wakeirq helper functions

[Rafael J. Wysocki]

 mode 100644
 index 000..3ecbc1a
 --- /dev/null
 +++ b/include/linux/pm_wakeirq.h
 @@ -0,0 +1,69 @@
 +/*
 + * pm_wakeirq.h - Device wakeirq helper functions
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed as is WITHOUT ANY WARRANTY of any
 + * kind, whether express or implied; without even the implied warranty
 + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#ifndef _LINUX_PM_WAKEIRQ_H
 +#define _LINUX_PM_WAKEIRQ_H
 +
 +struct wakeirq_source {
 + struct device *dev;
 + int wakeirq;
 + bool initialized;
 + bool enabled;
 + irq_handler_t handler;
 + void *data;
 +};

Well, so now we have struct wakeup_source already and here we get struct 
wakeirq_source
and they mean different things ...

 +
 +#ifdef CONFIG_PM_WAKEIRQ
 +
 +extern int dev_pm_wakeirq_request(struct device *dev,
 +   int wakeirq,
 +   irq_handler_t handler,
 +   unsigned long irqflags,
 +   void *data,
 +   struct wakeirq_source *wirq);
 +extern void dev_pm_wakeirq_free(struct wakeirq_source *wirq);
 +extern void dev_pm_wakeirq_enable(struct wakeirq_source *wirq);
 +extern void dev_pm_wakeirq_disable(struct wakeirq_source *wirq);
 +extern void dev_pm_wakeirq_arm_for_suspend(struct wakeirq_source *wirq);
 +
 +#else/* !CONFIG_PM_WAKEIRQ */
 +
 +static inline int dev_pm_wakeirq_request(struct device *dev,
 +  int wakeirq,
 +  irq_handler_t handler,
 +  unsigned long irqflags,
 +  void *data,
 +  struct wakeirq_source *wirq)
 +{
 + return 0;
 +}
 +
 +static inline void dev_pm_wakeirq_free(struct wakeirq_source *wirq)
 +{
 +}
 +
 +static inline void dev_pm_wakeirq_enable(struct wakeirq_source *wirq)
 +{
 +}
 +
 +static inline void dev_pm_wakeirq_disable(struct wakeirq_source *wirq)
 +{
 +}
 +
 +static inline void
 +dev_pm_wakeirq_arm_for_suspend(struct wakeirq_source *wirq)
 +{
 +}
 +
 +#endif   /* CONFIG_PM_WAKEIRQ */
 +#endif   /* _LINUX_PM_WAKEIRQ_H */
 diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
 index 7e01f78..c249845 100644
 --- a/kernel/power/Kconfig
 +++ b/kernel/power/Kconfig
 @@ -267,6 +267,10 @@ config PM_CLK
   def_bool y
   depends on PM  HAVE_CLK
  
 +config PM_WAKEIRQ
 + bool
 + depends on PM
 +
  config PM_GENERIC_DOMAINS
   bool
   depends on PM
 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] treewide: Convert clockevents_notify to use int cpu

[Rafael J. Wysocki]

On Wednesday, December 10, 2014 03:28:53 PM Joe Perches wrote:
 As far as I can tell, there's no value indirecting
 the cpu passed to this function via a void *.
 
 Update all the callers and called functions from within
 clockevents_notify.
 
 Miscellanea:
 
 Add pr_fmt and convert one printk(KERN_ERR to pr_err

That is fine by me, so

Acked-by: Rafael J. Wysocki rafael.j.wyso...@intel.com

for the cpuidle and ACPI changes.

 Signed-off-by: Joe Perches j...@perches.com
 ---
  arch/arm/mach-omap2/cpuidle44xx.c  |  7 +++
  arch/arm/mach-tegra/cpuidle-tegra114.c |  4 ++--
  arch/arm/mach-tegra/cpuidle-tegra20.c  |  8 
  arch/arm/mach-tegra/cpuidle-tegra30.c  |  8 
  arch/x86/kernel/process.c  |  6 +++---
  arch/x86/xen/suspend.c |  2 +-
  drivers/acpi/acpi_pad.c|  9 +
  drivers/acpi/processor_idle.c  |  4 ++--
  drivers/cpuidle/driver.c   |  3 +--
  drivers/idle/intel_idle.c  |  7 +++
  include/linux/clockchips.h |  6 +++---
  kernel/sched/idle.c|  4 ++--
  kernel/time/clockevents.c  | 15 +++
  kernel/time/hrtimer.c  |  6 ++
  kernel/time/tick-broadcast.c   | 16 
  kernel/time/tick-common.c  | 16 
  kernel/time/tick-internal.h| 18 +-
  kernel/time/timekeeping.c  |  4 ++--
  18 files changed, 69 insertions(+), 74 deletions(-)
 
 diff --git a/arch/arm/mach-omap2/cpuidle44xx.c 
 b/arch/arm/mach-omap2/cpuidle44xx.c
 index 01e398a..5d50aa1 100644
 --- a/arch/arm/mach-omap2/cpuidle44xx.c
 +++ b/arch/arm/mach-omap2/cpuidle44xx.c
 @@ -112,7 +112,7 @@ static int omap_enter_idle_coupled(struct cpuidle_device 
 *dev,
   mpuss_can_lose_context = (cx-mpu_state == PWRDM_POWER_RET) 
(cx-mpu_logic_state == PWRDM_POWER_OFF);
  
 - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, cpu_id);
 + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, cpu_id);
  
   /*
* Call idle CPU PM enter notifier chain so that
 @@ -169,7 +169,7 @@ static int omap_enter_idle_coupled(struct cpuidle_device 
 *dev,
   if (dev-cpu == 0  mpuss_can_lose_context)
   cpu_cluster_pm_exit();
  
 - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, cpu_id);
 + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, cpu_id);
  
  fail:
   cpuidle_coupled_parallel_barrier(dev, abort_barrier);
 @@ -184,8 +184,7 @@ fail:
   */
  static void omap_setup_broadcast_timer(void *arg)
  {
 - int cpu = smp_processor_id();
 - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, cpu);
 + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, smp_processor_id());
  }
  
  static struct cpuidle_driver omap4_idle_driver = {
 diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c 
 b/arch/arm/mach-tegra/cpuidle-tegra114.c
 index f2b586d..3b2fc3f 100644
 --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
 +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
 @@ -44,7 +44,7 @@ static int tegra114_idle_power_down(struct cpuidle_device 
 *dev,
   tegra_set_cpu_in_lp2();
   cpu_pm_enter();
  
 - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev-cpu);
 + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev-cpu);
  
   call_firmware_op(prepare_idle);
  
 @@ -52,7 +52,7 @@ static int tegra114_idle_power_down(struct cpuidle_device 
 *dev,
   if (call_firmware_op(do_idle, 0) == -ENOSYS)
   cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
  
 - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, dev-cpu);
 + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, dev-cpu);
  
   cpu_pm_exit();
   tegra_clear_cpu_in_lp2();
 diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c 
 b/arch/arm/mach-tegra/cpuidle-tegra20.c
 index 4f25a7c..ab30758 100644
 --- a/arch/arm/mach-tegra/cpuidle-tegra20.c
 +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
 @@ -136,11 +136,11 @@ static bool tegra20_cpu_cluster_power_down(struct 
 cpuidle_device *dev,
   if (tegra20_reset_cpu_1() || !tegra_cpu_rail_off_ready())
   return false;
  
 - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev-cpu);
 + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev-cpu);
  
   tegra_idle_lp2_last();
  
 - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, dev-cpu);
 + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, dev-cpu);
  
   if (cpu_online(1))
   tegra20_wake_cpu1_from_reset();
 @@ -153,13 +153,13 @@ static bool tegra20_idle_enter_lp2_cpu_1(struct 
 cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
  {
 - clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev-cpu);
 + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev-cpu);
  
   cpu_suspend(0

[PATCH] ARM / PM: Replace CONFIG_PM_RUNTIME with CONFIG_PM

[Rafael J. Wysocki]

From: Rafael J. Wysocki rafael.j.wyso...@intel.com

After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is
selected) PM_RUNTIME is always set if PM is set, so #ifdef blocks
depending on CONFIG_PM_RUNTIME may now be changed to depend on
CONFIG_PM.

Replace CONFIG_PM_RUNTIME with CONFIG_PM everywhere in the code under
arch/arm/ (the defconfig files will be modified later).

Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---

Note: This depends on commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if
PM_SLEEP is selected) which is only in linux-next at the moment (via the
linux-pm tree).

Please let me know if it is OK to take this one into linux-pm.

---
 arch/arm/kernel/perf_event.c   |2 +-
 arch/arm/mach-davinci/pm_domain.c  |2 +-
 arch/arm/mach-keystone/pm_domain.c |2 +-
 arch/arm/mach-omap1/pm_bus.c   |4 ++--
 arch/arm/mach-omap2/io.c   |2 +-
 arch/arm/mach-omap2/omap_device.c  |2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

Index: linux-pm/arch/arm/kernel/perf_event.c
===
--- linux-pm.orig/arch/arm/kernel/perf_event.c
+++ linux-pm/arch/arm/kernel/perf_event.c
@@ -481,7 +481,7 @@ static void armpmu_disable(struct pmu *p
armpmu-stop(armpmu);
 }
 
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
 static int armpmu_runtime_resume(struct device *dev)
 {
struct arm_pmu_platdata *plat = dev_get_platdata(dev);
Index: linux-pm/arch/arm/mach-davinci/pm_domain.c
===
--- linux-pm.orig/arch/arm/mach-davinci/pm_domain.c
+++ linux-pm/arch/arm/mach-davinci/pm_domain.c
@@ -14,7 +14,7 @@
 #include linux/pm_clock.h
 #include linux/platform_device.h
 
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
 static int davinci_pm_runtime_suspend(struct device *dev)
 {
int ret;
Index: linux-pm/arch/arm/mach-keystone/pm_domain.c
===
--- linux-pm.orig/arch/arm/mach-keystone/pm_domain.c
+++ linux-pm/arch/arm/mach-keystone/pm_domain.c
@@ -19,7 +19,7 @@
 #include linux/clk-provider.h
 #include linux/of.h
 
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
 static int keystone_pm_runtime_suspend(struct device *dev)
 {
int ret;
Index: linux-pm/arch/arm/mach-omap1/pm_bus.c
===
--- linux-pm.orig/arch/arm/mach-omap1/pm_bus.c
+++ linux-pm/arch/arm/mach-omap1/pm_bus.c
@@ -21,7 +21,7 @@
 
 #include soc.h
 
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
 static int omap1_pm_runtime_suspend(struct device *dev)
 {
int ret;
@@ -59,7 +59,7 @@ static struct dev_pm_domain default_pm_d
 #define OMAP1_PM_DOMAIN (default_pm_domain)
 #else
 #define OMAP1_PM_DOMAIN NULL
-#endif /* CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM */
 
 static struct pm_clk_notifier_block platform_bus_notifier = {
.pm_domain = OMAP1_PM_DOMAIN,
Index: linux-pm/arch/arm/mach-omap2/io.c
===
--- linux-pm.orig/arch/arm/mach-omap2/io.c
+++ linux-pm/arch/arm/mach-omap2/io.c
@@ -359,7 +359,7 @@ static void __init omap_hwmod_init_posts
u8 postsetup_state;
 
/* Set the default postsetup state for all hwmods */
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
postsetup_state = _HWMOD_STATE_IDLE;
 #else
postsetup_state = _HWMOD_STATE_ENABLED;
Index: linux-pm/arch/arm/mach-omap2/omap_device.c
===
--- linux-pm.orig/arch/arm/mach-omap2/omap_device.c
+++ linux-pm/arch/arm/mach-omap2/omap_device.c
@@ -588,7 +588,7 @@ odbs_exit:
return ERR_PTR(ret);
 }
 
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
 static int _od_runtime_suspend(struct device *dev)
 {
struct platform_device *pdev = to_platform_device(dev);

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

Re: [PATCH] hsi / OMAP / PM: Replace CONFIG_PM_RUNTIME with CONFIG_PM

[Rafael J. Wysocki]

On Wednesday, December 03, 2014 08:49:12 PM Arnd Bergmann wrote:
 On Wednesday 03 December 2014 03:02:24 Rafael J. Wysocki wrote:
  From: Rafael J. Wysocki rafael.j.wyso...@intel.com
  
  After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is
  selected) PM_RUNTIME is always set if PM is set, so #ifdef blocks
  depending on CONFIG_PM_RUNTIME may now be changed to depend on
  CONFIG_PM.
  
  Do that for the omap_ssi driver.
  
 
 Just for clarification: does that mean that PM, PM_RUNTIME, and PM_SLEEP
 are all synonyms now, or is there still some other combination that
 allows a subset to be set?

PM_RUNTIME is always set when PM is, but PM_SLEEP still may not be set then.

The plan is to drop PM_RUNTIME and use PM as a user-visible option instead
of it (which can be selected automatically by PM_SLEEP too).

As stated here:
http://marc.info/?l=linux-kernelm=141710589711818w=4
http://marc.info/?l=linux-kernelm=141712284917133w=4

 Before we do lots of s/CONFIG_PM_SLEEP/CONFIG_PM/ changes in lots of
 other drivers, it would be nice to come up with a new set of macros
 to replace all the SET_RUNTIME_PM_OPS/SIMPLE_DEV_PM_OPS/..._OPS
 with a version that avoided the #ifdefs altogether.

I don't think we can avoid all of the #ifdefs and the macros still work
(except for one which is redundant, but I have a patch to clean that up).

Rafael

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

[PATCH] gpio / PM: Replace CONFIG_PM_RUNTIME with CONFIG_PM

[Rafael J. Wysocki]

From: Rafael J. Wysocki rafael.j.wyso...@intel.com

After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is
selected) PM_RUNTIME is always set if PM is set, so #ifdef blocks
depending on CONFIG_PM_RUNTIME may now be changed to depend on
CONFIG_PM.

Replace CONFIG_PM_RUNTIME with CONFIG_PM in drivers/gpio/gpio-omap.c.

Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---

Note: This depends on commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if
PM_SLEEP is selected) which is only in linux-next at the moment (via the
linux-pm tree).

Please let me know if it is OK to take this one into linux-pm.

---
 drivers/gpio/gpio-omap.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/gpio/gpio-omap.c
===
--- linux-pm.orig/drivers/gpio/gpio-omap.c
+++ linux-pm/drivers/gpio/gpio-omap.c
@@ -1259,7 +1259,7 @@ static int omap_gpio_probe(struct platfo
 
 #ifdef CONFIG_ARCH_OMAP2PLUS
 
-#if defined(CONFIG_PM_RUNTIME)
+#if defined(CONFIG_PM)
 static void omap_gpio_restore_context(struct gpio_bank *bank);
 
 static int omap_gpio_runtime_suspend(struct device *dev)
@@ -1440,7 +1440,7 @@ static int omap_gpio_runtime_resume(stru
 
return 0;
 }
-#endif /* CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM */
 
 void omap2_gpio_prepare_for_idle(int pwr_mode)
 {
@@ -1468,7 +1468,7 @@ void omap2_gpio_resume_after_idle(void)
}
 }
 
-#if defined(CONFIG_PM_RUNTIME)
+#if defined(CONFIG_PM)
 static void omap_gpio_init_context(struct gpio_bank *p)
 {
struct omap_gpio_reg_offs *regs = p-regs;
@@ -1525,7 +1525,7 @@ static void omap_gpio_restore_context(st
writel_relaxed(bank-context.irqenable2,
bank-base + bank-regs-irqenable2);
 }
-#endif /* CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM */
 #else
 #define omap_gpio_runtime_suspend NULL
 #define omap_gpio_runtime_resume NULL

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

[PATCH] i2c-omap / PM: Drop CONFIG_PM_RUNTIME from i2c-omap.c

[Rafael J. Wysocki]

From: Rafael J. Wysocki rafael.j.wyso...@intel.com

After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is
selected) PM_RUNTIME is always set if PM is set, so some #ifdef blocks
depending on CONFIG_PM_RUNTIME may be dropped now.

Do that in drivers/i2c/busses/i2c-omap.c.

Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---

Note: This depends on commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if
PM_SLEEP is selected) which is only in linux-next at the moment (via the
linux-pm tree).

Please let me know if it is OK to take this one into linux-pm.

---
 drivers/i2c/busses/i2c-omap.c |2 --
 1 file changed, 2 deletions(-)

Index: linux-pm/drivers/i2c/busses/i2c-omap.c
===
--- linux-pm.orig/drivers/i2c/busses/i2c-omap.c
+++ linux-pm/drivers/i2c/busses/i2c-omap.c
@@ -1280,7 +1280,6 @@ static int omap_i2c_remove(struct platfo
 }
 
 #ifdef CONFIG_PM
-#ifdef CONFIG_PM_RUNTIME
 static int omap_i2c_runtime_suspend(struct device *dev)
 {
struct platform_device *pdev = to_platform_device(dev);
@@ -1318,7 +1317,6 @@ static int omap_i2c_runtime_resume(struc
 
return 0;
 }
-#endif /* CONFIG_PM_RUNTIME */
 
 static struct dev_pm_ops omap_i2c_pm_ops = {
SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend,

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

[PATCH] hsi / OMAP / PM: Replace CONFIG_PM_RUNTIME with CONFIG_PM

[Rafael J. Wysocki]

From: Rafael J. Wysocki rafael.j.wyso...@intel.com

After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is
selected) PM_RUNTIME is always set if PM is set, so #ifdef blocks
depending on CONFIG_PM_RUNTIME may now be changed to depend on
CONFIG_PM.

Do that for the omap_ssi driver.

Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---

Note: This depends on commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if
PM_SLEEP is selected) which is only in linux-next at the moment (via the
linux-pm tree).

Please let me know if it is OK to take this one into linux-pm.

---
 drivers/hsi/controllers/omap_ssi.c  |2 +-
 drivers/hsi/controllers/omap_ssi_port.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/hsi/controllers/omap_ssi.c
===
--- linux-pm.orig/drivers/hsi/controllers/omap_ssi.c
+++ linux-pm/drivers/hsi/controllers/omap_ssi.c
@@ -555,7 +555,7 @@ static int __exit ssi_remove(struct plat
return 0;
 }
 
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
 static int omap_ssi_runtime_suspend(struct device *dev)
 {
struct hsi_controller *ssi = dev_get_drvdata(dev);
Index: linux-pm/drivers/hsi/controllers/omap_ssi_port.c
===
--- linux-pm.orig/drivers/hsi/controllers/omap_ssi_port.c
+++ linux-pm/drivers/hsi/controllers/omap_ssi_port.c
@@ -1260,7 +1260,7 @@ static int __exit ssi_port_remove(struct
return 0;
 }
 
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
 static int ssi_save_port_ctx(struct omap_ssi_port *omap_port)
 {
struct hsi_port *port = to_hsi_port(omap_port-dev);

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

Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper

[Rafael J. Wysocki]

On Thursday, September 25, 2014 04:27:58 PM Wolfram Sang wrote:
 
 --Bn2rw/3z4jIqBvZU
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 Content-Transfer-Encoding: quoted-printable
 
 On Thu, Sep 25, 2014 at 09:22:01AM -0500, Felipe Balbi wrote:
  On Thu, Sep 25, 2014 at 01:27:18PM +0530, Vinod Koul wrote:
   On Wed, Sep 24, 2014 at 03:32:19PM -0500, Felipe Balbi wrote:
   OK, I guess this is as good as it gets.
  =20
   What tree would you like it go through?
 =20
  Do we really need this new helper ? I mean, the very moment when =
 we
  decide to implement -runtime_idle() we will need to get rid of t=
 his
  change. I wonder if it's really valid...
=20
 I'm not sure I'm following?  This seems to simply implement what dr=
 ivers
 have been doing already as one function.  Why would it be invalid t=
 o reduce
 code duplication?
   =20
For two reasons:
   =20
1) the helper has no inteligence whatsoever. It just calls the same
functions.
   =20
2) the duplication will vanish whenever someone implements
-runtime_idle() and have that call pm_runtime_autosuspend() (like PCI
and USB buses are doing today). This will just be yet another line th=
 at
needs to change.
   =20
Frankly though, no strong feelings, I just think it's a commit that
doesn't bring that any benefits other than looking like one line was
removed.
   and yes that is what it tries to do nothing more nothing less. If in fu=
 ture
   there are no users (today we have quite a few), then we can remove the =
 dead
   macro, no harm. But that is not the situation today.
 =20
  as I said, a commit that's bound to be useless. It's not like you're
  saving 10 lines of code, it's only one. Replacing two simple lines with
  a function which takes joke almost as many characters to type /joke.
 =20
  IMO, this is pretty useless and I'd rather not see them in the drivers I
  maintain, sorry.
 
 It is not a NACK from me; yet from a high-level perspective I agree with
 Felipe.

OK

I'd rather not merge something that driver people don't want to use.

Vinod?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper

[Rafael J. Wysocki]

On Wednesday, September 24, 2014 09:44:50 PM Vinod Koul wrote:
 This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
 which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
 sequentially. Then we do a tree wide update of current patterns which are
 present. As evident from log below this pattern is frequent in the
 kernel.
 
 This series can be found at
 git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
 topic/pm_runtime_last_busy_and_autosuspend
 
 Fengguang's kbuild has tested it so it shouldn't break things for anyone.
 Barring one patch (explictyly mentioned in its changelog) rest are simple
 replacements.
 
 If all are okay, this should be merged thru PM tree as it depends on macro
 addition.
 
 Subhransu S. Prusty (1):
   PM: Add helper pm_runtime_last_busy_and_autosuspend()
 
 Vinod Koul (26):
   dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
   extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
   drm/i915: use pm_runtime_last_busy_and_autosuspend helper
   drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
   drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
   vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
   i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
   i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
   i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
   mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
   mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
   mei: use pm_runtime_last_busy_and_autosuspend helper
   mmc: use pm_runtime_last_busy_and_autosuspend helper
   mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
   mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
   mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
   mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
   NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
   pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
   spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
   spi: orion: use pm_runtime_last_busy_and_autosuspend helper
   spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
   spi: core: use pm_runtime_last_busy_and_autosuspend helper
   tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
   usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
   video: fbdev: use pm_runtime_last_busy_and_autosuspend helper
 
  Documentation/power/runtime_pm.txt  |4 ++
  drivers/dma/ste_dma40.c |   30 -
  drivers/extcon/extcon-arizona.c |6 +--
  drivers/gpu/drm/i915/intel_pm.c |3 +-
  drivers/gpu/drm/nouveau/nouveau_connector.c |3 +-
  drivers/gpu/drm/nouveau/nouveau_drm.c   |9 +---
  drivers/gpu/drm/radeon/radeon_connectors.c  |   15 ++
  drivers/gpu/drm/radeon/radeon_drv.c |5 +-
  drivers/gpu/drm/radeon/radeon_kms.c |6 +--
  drivers/gpu/vga/vga_switcheroo.c|7 +--
  drivers/i2c/busses/i2c-designware-core.c|3 +-
  drivers/i2c/busses/i2c-omap.c   |6 +--
  drivers/i2c/busses/i2c-qup.c|3 +-
  drivers/mfd/ab8500-gpadc.c  |6 +--
  drivers/mfd/arizona-irq.c   |3 +-
  drivers/misc/mei/client.c   |   12 ++
  drivers/mmc/core/core.c |3 +-
  drivers/mmc/host/mmci.c |   12 ++
  drivers/mmc/host/omap_hsmmc.c   |   19 ++---
  drivers/mmc/host/sdhci-pxav3.c  |6 +--
  drivers/mmc/host/sdhci.c|3 +-
  drivers/nfc/trf7970a.c  |3 +-
  drivers/power/pm2301_charger.c  |3 +-
  drivers/spi/spi-omap2-mcspi.c   |9 +---
  drivers/spi/spi-orion.c |3 +-
  drivers/spi/spi-ti-qspi.c   |5 +-
  drivers/spi/spi.c   |6 +--
  drivers/tty/serial/omap-serial.c|   60 
 +--
  drivers/usb/musb/omap2430.c |6 +--
  drivers/video/fbdev/auo_k190x.c |9 +---
  include/linux/pm_runtime.h  |6 +++
  31 files changed, 97 insertions(+), 177 deletions(-)

OK, I guess this is as good as it gets.

What tree would you like it go through?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] PM / OPP: Remove ARCH_HAS_OPP

[Rafael J. Wysocki]

On Monday, June 09, 2014 08:49:17 PM Mark Brown wrote:
 On Fri, Jun 06, 2014 at 11:15:06PM +0200, Rafael J. Wysocki wrote:
  On Friday, June 06, 2014 02:08:50 PM Mark Brown wrote:
 
   Yes, the conversion to make ARCH_HAS_OPP unused is in Raphael's tree for
   the merge window.
 
  Perhaps already merged?
 
 Yes, looks like it is.

But it doesn't apply for me on top of the current Linus' tree.

Can you please rebase?

Rafael


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] PM / OPP: Remove ARCH_HAS_OPP

[Rafael J. Wysocki]

On Monday, June 09, 2014 11:51:42 PM Mark Brown wrote:
 
 --fYgRtaZIy+F1uhp1
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 On Tue, Jun 10, 2014 at 12:22:07AM +0200, Rafael J. Wysocki wrote:
  On Monday, June 09, 2014 08:49:17 PM Mark Brown wrote:
 
   Yes, looks like it is.
 
  But it doesn't apply for me on top of the current Linus' tree.
 
  Can you please rebase?
 
 It probably depends on other people's trees too, I just killed all the
 usages in -next - I gather the SH trees added some new ones for example.
 I'd expect it'll apply after the merge window, if not that's the time to
 look at it.

OK

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

Re: [PATCH] PM / OPP: Remove ARCH_HAS_OPP

[Rafael J. Wysocki]

On Friday, June 06, 2014 02:08:50 PM Mark Brown wrote:
 
 --cU9XODsizZBnwgll
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 On Fri, Jun 06, 2014 at 09:50:06PM +0900, Simon Horman wrote:
  On Fri, Jun 06, 2014 at 09:14:01PM +0900, Magnus Damm wrote:
 
   I'm not sure about the expected merge order for this kind of change vs
   queued up stuff in the renesas git tree, but I believe the following
   patch selects ARCH_HAS_OPP:
 
   [PATCH v3] ARM: shmobile: Mark all SoCs in shmobile as CPUFreq, capable
 
  I propose that we fix that up by adding an incremental patch to
  mach-shmobile via my renesas tree once the dependency (assuming there is
  one) is in Linus's tree.
 
 Yes, the conversion to make ARCH_HAS_OPP unused is in Raphael's tree for
 the merge window.

Perhaps already merged?

Rafael

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

Re: Ethernet controller not starting

[Rafael J. Wysocki]

On Monday, March 03, 2014 02:41:01 PM Jon Ringle wrote:
 I'm working on porting an ARM board from linux-3.10 to linux-3.12 (now
 the latest LTS kernel).
 I found that Ethernet controller on the board no longer comes up on 
 linux-3.12.
 I was able to bisect the issue I'm having to the following commit:
 
45f0a85c8258741d11bda25c0a5669c06267204a is the first bad commit
commit 45f0a85c8258741d11bda25c0a5669c06267204a
Author: Rafael J. Wysocki rafael.j.wyso...@intel.com
Date:   Mon Jun 3 21:49:52 2013 +0200
 
PM / Runtime: Rework the runtime idle helper routine
 
The runtime idle helper routine, rpm_idle(), currently ignores
return values from .runtime_idle() callbacks executed by it.
However, it turns out that many subsystems use
pm_generic_runtime_idle() which checks the return value of the
driver's callback and executes pm_runtime_suspend() for the device
unless that value is not 0.  If that logic is moved to rpm_idle()
instead, pm_generic_runtime_idle() can be dropped and its users
will not need any .runtime_idle() callbacks any more.
 
Moreover, the PCI, SCSI, and SATA subsystems' .runtime_idle()
routines, pci_pm_runtime_idle(), scsi_runtime_idle(), and
ata_port_runtime_idle(), respectively, as well as a few drivers'
ones may be simplified if rpm_idle() calls rpm_suspend() after 0 has
been returned by the .runtime_idle() callback executed by it.
 
To reduce overall code bloat, make the changes described above.
 
Tested-by: Mika Westerberg mika.westerb...@linux.intel.com
Tested-by: Kevin Hilman khil...@linaro.org
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Acked-by: Kevin Hilman khil...@linaro.org
Reviewed-by: Ulf Hansson ulf.hans...@linaro.org
Acked-by: Alan Stern st...@rowland.harvard.edu
 
 Can anyone offer any suggestions on what I should be looking for to
 fix this on my board?

Any pointers to the driver in question?

Rafael

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

Re: pm_runtime functions and IS_ERR_VALUE (was Re: [PATCH v6] ARM: omap: edma: add suspend suspend/resume hooks)

[Rafael J. Wysocki]

. Unecessary IS_ERR_VALUE at line %s % 
 (pm_runtime_api, p2[0].line)
 +coccilib.report.print_report(p1[0],msg)
 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] PM / AVS: SmartReflex: misc. cleanups for v3.11

[Rafael J. Wysocki]

On Thursday, June 13, 2013 03:16:30 PM Kevin Hilman wrote:
 Hi Rafael,
 
 Please pull the following cleanups to  AVS/SmartReflex for v3.11.

Pulled, thanks Kevin!

Rafael


 The following changes since commit 317ddd256b9c24b0d78fa8018f80f1e495481a10:
 
   Linux 3.10-rc5 (2013-06-08 17:41:04 -0700)
 
 are available in the git repository at:
 
   git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git 
 tags/omap-pm-v3.11/avs
 
 for you to fetch changes up to efca406b940e93e6af38c597eecd5fb79b39b7ea:
 
   PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex (2013-06-10 
 10:50:48 -0700)
 
 
 PM / AVS: SmartReflex: misc. cleanups for v3.11
 
 
 Andrii Tseglytskyi (6):
   PM / AVS: SmartReflex: disable runtime PM on driver remove
   PM / AVS: SmartReflex: fix driver name
   PM / AVS: SmartReflex: use omap_sr * for errgen interfaces
   PM / AVS: SmartReflex: use omap_sr * for minmax interfaces
   PM / AVS: SmartReflex: use omap_sr * for enable/disable interface
   PM / AVS: SmartReflex: use devm_* API to initialize SmartReflex
 
 Nishanth Menon (1):
   PM / AVS: SmartReflex: disable errgen before vpbound disable
 
  arch/arm/mach-omap2/smartreflex-class3.c |   8 +-
  drivers/power/avs/smartreflex.c  | 154 
 +--
  include/linux/power/smartreflex.h|  10 +-
  3 files changed, 72 insertions(+), 100 deletions(-)
 --
 To unsubscribe from this list: send the line unsubscribe linux-pm in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] CPUidle: OMAP cleanups for v3.10

[Rafael J. Wysocki]

On Tuesday, April 09, 2013 04:40:11 PM Kevin Hilman wrote:
 [resend with correct address for linux-pm]
 
 Rafael,
 
 Please pull the following OMAP CPUidle changes for v3.10.
 
 Due to dependencies on other CPUidle changes that are already in your
 linux-next branch, this branch is based on the commit where you merged
 your pm-cpuidle-next branch into linux-next.
 
 Kevin
 
 The following changes since commit f69e44b2059f2238ac558b4a115ebcdefe20b9be:
 
   Merge branch 'pm-cpuidle-next' into linux-next (2013-04-08 12:32:07 +0200)
 
 are available in the git repository at:
 
 
   git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git 
 tags/omap-pm-v3.10/cleanup/cpuidle-v2

Pulled into my bleeding-edge branch.  I'll move it to linux-next tomorrow.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] cpufreq: OMAP: instantiate omap-cpufreq as a platform_driver

[Rafael J. Wysocki]

,
 + },
 + .probe  = omap_cpufreq_probe,
 + .remove = omap_cpufreq_remove,
 +};
 +module_platform_driver(omap_cpufreq_platdrv);
 +
  MODULE_DESCRIPTION(cpufreq driver for OMAP SoCs);
  MODULE_LICENSE(GPL);
 -module_init(omap_cpufreq_init);
 -module_exit(omap_cpufreq_exit);
 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] OPP usage fixes for RCU locking

[Rafael J. Wysocki]

On Monday, January 21, 2013 02:45:32 PM MyungJoo Ham wrote:
 On Sat, Jan 19, 2013 at 7:28 AM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Friday, January 18, 2013 01:52:31 PM Nishanth Menon wrote:
  Hi,
  Despite being documented in function documentation and in
  Documentation/power/opp.txt, many of the users of OPP APIs
  dont honor RCU lock usage appropriately.
 
  This recently appeared in IRC discussion earlier today [1].
  I did an audit of current usage and the following series
  is a result of this.
 
  NOTE:
  1. The patch PM / devfreq: exynos4_bus: honor RCU lock usage has only
 been build tested as I dont have an exynos platform to try it on. I have
 tried to make it as least intrusive as possible and at least reviewed
 to ensure I haven't screwed anything up.
 
  Other than this, I have added appropriate tested by information in 
  requisite
  patches.
 
  Thanks for the fixes.
 
  MyungJoo, do you want me to take the devfreq ones too?
 
  Rafael
 
 Yes, please take RCU-OPP patches. Having those patches splitted
 doesn't seem beneficial.

OK, I will take them.

 I'll let other devfreq patches be based on this after you get them applied.

OK

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] OPP usage fixes for RCU locking

[Rafael J. Wysocki]

On Friday, January 18, 2013 01:52:31 PM Nishanth Menon wrote:
 Hi,
 Despite being documented in function documentation and in
 Documentation/power/opp.txt, many of the users of OPP APIs
 dont honor RCU lock usage appropriately.
 
 This recently appeared in IRC discussion earlier today [1]. 
 I did an audit of current usage and the following series
 is a result of this.
 
 NOTE:
 1. The patch PM / devfreq: exynos4_bus: honor RCU lock usage has only
been build tested as I dont have an exynos platform to try it on. I have
tried to make it as least intrusive as possible and at least reviewed
to ensure I haven't screwed anything up.
 
 Other than this, I have added appropriate tested by information in requisite
 patches.

Thanks for the fixes.

MyungJoo, do you want me to take the devfreq ones too?

Rafael


 Series is based off: v3.8-rc4 tag
 Also available in the following location[2]:
 https://github.com/nmenon/linux-2.6-playground branch: post/pm/opp-fixes-v1
 
 Nishanth Menon (4):
   cpufreq: OMAP: use RCU locks around usage of OPP
   cpufreq: cpufreq-cpu0: use RCU locks around usage of OPP
   PM / devfreq: add locking documentation for recommend_opp
   PM / devfreq: exynos4_bus: honor RCU lock usage
 
  drivers/cpufreq/cpufreq-cpu0.c |5 +++
  drivers/cpufreq/omap-cpufreq.c |3 ++
  drivers/devfreq/devfreq.c  |5 +++
  drivers/devfreq/exynos4_bus.c  |   94 
 
  4 files changed, 80 insertions(+), 27 deletions(-)
 
 [1] http://www.beagleboard.org/irclogs/index.php?date=2013-01-18#T14:14:07
 [2] 
 https://github.com/nmenon/linux-2.6-playground/commits/post/pm/opp-fixes-v1
 
 Regards,
 Nishanth Menon
 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices

[Rafael J. Wysocki]

Hi,

Well, I'm not any less busy than yesterday, as it turns out, but I'm expecting
that to continue tomorrow, so I may as well have a look at it now. :-)

On Tuesday, December 04, 2012 12:10:32 PM Alan Stern wrote:
 [CC: list trimmed; the people who were on it should be subscribed to at 
 least one of the lists anyway.]
 

[...]

 
  Not only regulators involved, clock or other things might be involved too.
  Also the same power domain might be shared with more than one port,
  so it is better to introduce power domain to the problem. Looks
  generic_pm_domain is overkill, so I introduced power controller which
  only focuses on power on/off and being shared by multiple devices.   
 
 Even though it is overkill, it may be better than introducing a new 
 abstraction.  After all, this is exactly the sort of thing that PM 
 domains were originally created to handle.
 
 Rafael, do you have any advice on this?  The generic_pm_domain 
 structure is fairly complicated, there's a lot of code in 
 drivers/base/power/domain.c (it's the biggest source file in its 
 directory), and I'm not aware of any documentation.

Yeah, documentation is missing, which obviously is my fault.

That code is designed to cover the case in which multiple devices share
a power switch that can be used to remove power from all of them at
once (eg. through a register write).  It also assumes that there will
be a stop operation, such as disable clock, allowing each device in
the domain to be put into a shallow low-power state (individually) without
the necessity to save its state.  Device states only have to be saved
when the power switch is about to be used, which generally happens
when they all have been stopped (their runtime PM status is RPM_SUSPENDED).

The stop operation may be defined in a different way for each device in the
domain (actually, that applies to the save state, restore state, and
start - opposite to stop - operations too) and PM QoS latency constraints
are used to decide if and when to turn the whole domain off.  Moreover, it
supports hierarchies of power domains that may be pretty much arbitarily
complicated.

A big part of this code is for the handling of system suspend/resume
in such a way that it is consistent with runtime PM.

For USB ports I'd recommend to use something simpler. :-)

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: OMAP baseline test results for v3.7-rc1

[Rafael J. Wysocki]

On Wednesday, October 31, 2012 04:34:21 AM Jean Pihet wrote:
 Paul,
 
 - Added Rafael for the PM QoS discussion -
 
 On Tue, Oct 30, 2012 at 10:04 AM, Paul Walmsley p...@pwsan.com wrote:
  On Tue, 30 Oct 2012, Paul Walmsley wrote:
 
  Based on a very quick look, I'd say the original patch 3db11fe is broken.
  I don't see how it can ensure that its PM_QOS_CPU_DMA_LATENCY request is
  honored when CONFIG_CPU_IDLE=n. CONFIG_CPU_IDLE=n is the default for
  omap2plus_defconfig.
 
  So in fact to follow up on this, looks like one of two changes are needed:
 
  1. Revert 3db11fe
 
  or
 
  2. If CONFIG_CPU_IDLE=n, the driver needs to call disable_hlt() in place
  of the pm_qos constraint add, and call enable_hlt() in place of the pm_qos
  constraint remove.
 I do not think this is correct. Using disable_hlt is a too radical
 solution and will prevent the idle completely, this is not what we
 want.
 
 Rafael, what do you think?

Well, I agree.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2] Input: omap4-keypad: Add pinctrl support

[Rafael J. Wysocki]

On Tuesday, October 30, 2012 10:51:11 PM Linus Walleij wrote:
 On Tue, Oct 30, 2012 at 7:37 PM, Mark Brown
 broo...@opensource.wolfsonmicro.com wrote:
 
  More seriously the amount of time we seem to have been spending recently
  on changes which end up requiring us to go through essentially every
  driver and add code to them (often several times) doesn't seem like
  we're doing a good job here.
 
 If this is your main concern you should be made aware that there
 are people out there planning to supplant the existing DT probe paths
 that are now being added to each and every ARM-related driver
 with an ACPI probe path as ARM servers come into the picture.

That's correct.

  pinctrl is really noticable because it's
  new but it's not the only thing.  As a subsystem maintainer this code
  just makes me want to add new subsystem features to pull the code out of
  drivers but obviously that's not something that should be being done at
  the subsystem level.
 
 We did manage to drag the power/voltage domain per se out
 of the AMBA bus, and recommend that people (like us) do that
 business using the power domains.
 
 I think most people (including OMAP) have bought
 into the concept of using the runtime PM framework and power
 domains to control the power domain switches.
 
 It's this wider concept of using the loose concept PM resource
 domains to control also clocks and pins that is at stake, and so
 far the runtime PM core people (Rafael and Magnus) has not said
 much so I think we need some kind of indication from them as to
 what is to happen, long-term, with drivers handling their own clocks
 and pins. Should it be centralized or not? If it's to be centralized it
 needs to become a large piece of infrastructure refactoring and
 needs the attention of Linaro and the like to happen.

Well, I personally think it should be centralized somehow.  I'm not quite
sure how to achieve that, though.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] cpufreq: OMAP: fixes for v3.7-rc2

[Rafael J. Wysocki]

On Wednesday 03 of October 2012 16:00:25 Kevin Hilman wrote:
 From: Kevin Hilman khil...@ti.com
 
 Here's a series with a couple bug fixes and a couple fixes that
 make this driver support newer OMAP-based SoCs.
 
 The 'get_cpu_device' patch is needed due to a change in the OMAP
 OMAP PM core code which enforces use of get_cpu_device() instead of
 a deprecated OMAP-specific API.
 
 The usage of plat/*.h headers breaks single zImage, so platforms are
 cleaning up and/or removing plat/*.h so the driver needs to be fixed
 accordingly.
 
 This series is based on the merge of Rafael's pm-for-3.7-rc1 tag into
 Linus' master branch: commit 16642a2e7be23bbda013fc32d8f6c68982eab603.
 
 Tested CPUfreq on OMAP platforms: 3430/n900, 3530/Overo,
 3730/OveroSTORM, 3730/Beagle-XM, 4430/Panda.
 
 Rafael, if you're OK with this series, I'll get a pull request
 ASAP so it can be included for v3.7-rc2.

The patches are fine by me, but there may be a bit of a timing issue with
them, because I'll be travelling between October 12 and October 21 inclusive
and I won't be pushing stuff to kernel.org during that time.

So I think it would be better to merge this material through the arm-soc tree,
if that's not a problem.  If you decide to do so, please feel free to add my
ACK to the patches.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE, even when disabled

[Rafael J. Wysocki]

On Saturday, September 22, 2012, Kevin Hilman wrote:
 From: Kevin Hilman khil...@ti.com
 
 There are several drivers where the return value of
 pm_runtime_get_sync() is used to decide whether or not it is safe to
 access hardware and that don't provide .suspend() callbacks for system
 suspend (but may use late/noirq callbacks.)  If such a driver happens
 to call pm_runtime_get_sync() during system suspend, after the core
 has disabled runtime PM, it will get the error code and will decide
 that the hardware should not be accessed, although this may be a wrong
 conclusion, depending on the state of the device when runtime PM was
 disabled.
 
 Drivers might work around this problem by using a test like:
 
ret = pm_runtime_get_sync(dev);
if (!ret || (ret == -EACCES  driver_private_data(dev)-suspended)) {
   /* access hardware */
}
 
 where driver_private_data(dev)-suspended is a flag set by the
 driver's .suspend() method (that would have to be added for this
 purpose).  However, that potentially would need to be done by multiple
 drivers which means quite a lot of duplicated code and bloat.
 
 To avoid that we can use the observation that the core sets
 dev-power.is_suspended before disabling runtime PM and use that
 instead of the driver's private flag.  Still, potentially many drivers
 would need to repeat that same check in quite a few places, so it's
 better to let the core do it.
 
 Then we can be a bit smarter and check whether or not runtime PM was
 disabled by the core only (disable_depth == 1) or by someone else in
 addition to the core (disable_depth  1).  In the former case
 rpm_resume() can return 1 if the runtime PM status is RPM_ACTIVE,
 because it means the device was active when the core disabled runtime
 PM.  In the latter case it should still return -EACCES, because it
 isn't clear why runtime PM has been disabled.
 
 Tested on AM3730/Beagle-xM where a wakeup IRQ firing during the late
 suspend phase triggers runtime PM activity in the I2C driver since the
 wakeup IRQ is on an I2C-connected PMIC.
 
 Cc: Rafael J. Wysocki r...@sisk.pl
 Cc: Alan Stern st...@rowland.harvard.edu
 Signed-off-by: Kevin Hilman khil...@ti.com
 ---
 v2: 
 - major changelog rewrite, based largely on input from Rafael 
 - add check for disable_depth == 1 and move to separate if statement,
   both suggested by Alan Stern

OK, this looks good to me, thanks!

Alan, what do you think?

Rafael


  drivers/base/power/runtime.c |3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
 index 7d9c1cb..d43856b 100644
 --- a/drivers/base/power/runtime.c
 +++ b/drivers/base/power/runtime.c
 @@ -509,6 +509,9 @@ static int rpm_resume(struct device *dev, int rpmflags)
   repeat:
   if (dev-power.runtime_error)
   retval = -EINVAL;
 + else if (dev-power.disable_depth == 1  dev-power.is_suspended
 +   dev-power.runtime_status == RPM_ACTIVE)
 + retval = 1;
   else if (dev-power.disable_depth  0)
   retval = -EACCES;
   if (retval)
 

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

Re: [PATCH v2] PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE, even when disabled

[Rafael J. Wysocki]

On Saturday, September 22, 2012, Alan Stern wrote:
 On Sat, 22 Sep 2012, Rafael J. Wysocki wrote:
 
  On Saturday, September 22, 2012, Kevin Hilman wrote:
 
  OK, this looks good to me, thanks!
  
  Alan, what do you think?
  
  Rafael
 
   --- a/drivers/base/power/runtime.c
   +++ b/drivers/base/power/runtime.c
   @@ -509,6 +509,9 @@ static int rpm_resume(struct device *dev, int 
   rpmflags)
 repeat:
 if (dev-power.runtime_error)
 retval = -EINVAL;
   + else if (dev-power.disable_depth == 1  dev-power.is_suspended
   +   dev-power.runtime_status == RPM_ACTIVE)
   + retval = 1;
 else if (dev-power.disable_depth  0)
 retval = -EACCES;
 if (retval)
 
 Well, I'd prefer the indentation on the continuation line to be 
 different from the indentation of the following line, and I'd prefer 
 to have a comment explaining the reason for the exception.
 
 But these are only matters of taste; the implementation itself looks 
 good.

Thanks!

I've applied the patch as v3.7 material (and fixed up the white space).

Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND] PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE, even when disabled

[Rafael J. Wysocki]

On Friday, September 21, 2012, Alan Stern wrote:
 On Thu, 20 Sep 2012, Rafael J. Wysocki wrote:
 
  On Thursday, September 20, 2012, Kevin Hilman wrote:
   From: Kevin Hilman khil...@ti.com
   
   When runtime PM is disabled, what we want is for callbacks not to be
   called from then on.  However, currently, when runtime PM is disabled,
   operations such as 'get' will also fail even if the device is
   currently active.
   
   Since calling 'get' on a device that is already RPM_ACTIVE does not
   involve calling the callbacks, it should be allowed to succeed, even
   if runtime PM is disabled.
   
   This is particularily useful in runtime PM enabled drivers that are
   used during system suspend.  Because runtime PM is disabled during
   system suspend, currently any driver's use of pm_runtime_get* will
   fail with -EACCES.  This is expected if the device was already runtime
   suspended, but if the device is actually active (due to recent usage,
   autosuspend timeout not expired, or pm_runtime_resume() called in
   -suspend() method), the pm_runtime_get*() call should actually
   succeed.
  
  I'd say the problem is when the drive in question uses the return value of
  pm_runtime_get_sync(), for example, to decide whether or not it is safe
  to access the hardware.  In that case it may decide that accessing the
  hardware is unsafe during system suspend, although that's not really the
  case.  So the change you're proposing allows drivers of this kind  (and 
  there
  may be a substantial number of them) to be simplified slightly.
 
 Kevin makes a good case that pm_runtime_resume() and related functions 
 should succeed even when runtime PM is disabled, if the device is 
 already in the desired state.
 
 The same may be true for pm_runtime_suspend().  What do you think?

I've discussed that with Kevin.  The problem is that the runtime PM
status may be changed at will when runtime PM is disabled by using
__pm_runtime_set_status(), so the status generally cannod be trusted
if power.disable_depth  0.

During system suspend, however, runtime PM is disabled by the core and
if neither the driver nor the subsystem has disabled it in the meantime,
the status should be actually valid.

   To permit the usage described above, add a check to rpm_resume() so
   that success is returned in the case where a driver is suspended (it's
   -suspend callback has been called) but is still RPM_ACTIVE.
   
   This patch was developed in close collaboration with Rafael J. Wysocki
   r...@sisk.pl
   
   Tested on AM3730/Beagle-xM where wakeup IRQ firing during the late
   suspend phase triggers runtime PM activity in the I2C driver since the
   wakeup IRQ is on an I2C-connected PMIC.
   
   Cc: Rafael J. Wysocki r...@sisk.pl
   Signed-off-by: Kevin Hilman khil...@ti.com
  
  OK, I wonder if anyone has any objections against this.  Alan?
 
 The way the patch is written contradicts the documentation:
 
   * A request to execute -runtime_resume() will cancel any pending or
 scheduled requests to execute the other callbacks for the same device,
 except for scheduled autosuspends.

I'm not sure where the contradiction is.  The patch simply changes the
behavior for disabled runtime PM, which is to return a nonzero value immediately
anyway.

   @@ -510,7 +510,8 @@ static int rpm_resume(struct device *dev, int 
   rpmflags)
 if (dev-power.runtime_error)
 retval = -EINVAL;
 else if (dev-power.disable_depth  0)
   - retval = -EACCES;
   + retval = dev-power.is_suspended  
   + dev-power.runtime_status == RPM_ACTIVE ? 1 : -EACCES;
 if (retval)
 goto out;
 
 Also, the is_suspended test seems irrelevant in general -- it makes 
 sense in terms of the scenario Kevin described but that's not the 
 stated purpose of the patch.

Well, it is, although the changelog doesn't state it sufficiently clearly. :-)

It fact, it is an optimization, because the driver can have a suspended
flag that will be set in its .suspend() routine and then do something like:

ret = pm_runtime_get_sync(dev);
if (!ret || (private_driver_data(dev)-suspended  ret == -EACCES) {
  /* access the hardware */
}

but if the above patch is applied, the driver won't need to do that (and
that's something multiple drivers may need to do, so it seems to be worth
optimizing).

 Both of these problems can be addressed by writing the code as follows:
 
   if (dev-power.runtime_error)
   retval = -EINVAL;
   else if (dev-power.disable_depth  0) {
 
   /* Special case: allow this if the device is already active */
   if (dev-power.runtime_status != RPM_ACTIVE)
   retval = -EACCES;
   }
   if (retval)
   goto out;

We could do that too, but I'm a bit concerned about the situations where
runtime PM is disabled by the driver itself or by the subsystem, because
in those cases whoever disables runtime PM would have to make sure

Re: [PATCH] PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE, even when disabled

[Rafael J. Wysocki]

Hi,

On Thursday, September 20, 2012, Kevin Hilman wrote:
 From: Kevin Hilman khil...@ti.com
 
 When runtime PM is disabled, what we want is for callbacks not to be
 called from then on.  However, currently, when runtime PM is disabled,
 operations such as 'get' will also fail even if the device is
 currently active.
 
 Since calling 'get' on a device that is already RPM_ACTIVE does not
 involve calling the callbacks, it should be allowed to succeed, even
 if runtime PM is disabled.
 
 This is particularily useful in runtime PM enabled drivers that are
 used during system suspend.  Because runtime PM is disabled during
 system suspend, currently any driver's use of pm_runtime_get* will
 fail with -EACCES.  This is expected if the device was already runtime
 suspended, but if the device is actually active (due to recent usage,
 autosuspend timeout not expired, or pm_runtime_resume() called in
 -suspend() method), the pm_runtime_get*() call should actually
 succeed.
 
 To permit the usage described above, add a check to rpm_resume() so
 that success is returned in the case where a driver is suspended (it's
 -suspend callback has been called) but is still RPM_ACTIVE.
 
 This patch was developed in close collaboration with Rafael J. Wysocki
 r...@sisk.pl
 
 Tested on AM3730/Beagle-xM where wakeup IRQ firing during the late
 suspend phase triggers runtime PM activity in the I2C driver since the
 wakeup IRQ is on an I2C-connected PMIC.

Please resend it with a CC to linux...@vger.kernel.org.  Nobody reads
linux...@lists.linux-foundation.org today, I suppose ...

Thanks,
Rafael


 Cc: Rafael J. Wysocki r...@sisk.pl
 Signed-off-by: Kevin Hilman khil...@ti.com
 ---
  drivers/base/power/runtime.c |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
 index 7d9c1cb..dafa5ec 100644
 --- a/drivers/base/power/runtime.c
 +++ b/drivers/base/power/runtime.c
 @@ -510,7 +510,8 @@ static int rpm_resume(struct device *dev, int rpmflags)
   if (dev-power.runtime_error)
   retval = -EINVAL;
   else if (dev-power.disable_depth  0)
 - retval = -EACCES;
 + retval = dev-power.is_suspended  
 + dev-power.runtime_status == RPM_ACTIVE ? 1 : -EACCES;
   if (retval)
   goto out;
  
 

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

Re: [PATCH RESEND] PM / Runtime: let rpm_resume() succeed if RPM_ACTIVE, even when disabled

[Rafael J. Wysocki]

On Thursday, September 20, 2012, Kevin Hilman wrote:
 From: Kevin Hilman khil...@ti.com
 
 When runtime PM is disabled, what we want is for callbacks not to be
 called from then on.  However, currently, when runtime PM is disabled,
 operations such as 'get' will also fail even if the device is
 currently active.
 
 Since calling 'get' on a device that is already RPM_ACTIVE does not
 involve calling the callbacks, it should be allowed to succeed, even
 if runtime PM is disabled.
 
 This is particularily useful in runtime PM enabled drivers that are
 used during system suspend.  Because runtime PM is disabled during
 system suspend, currently any driver's use of pm_runtime_get* will
 fail with -EACCES.  This is expected if the device was already runtime
 suspended, but if the device is actually active (due to recent usage,
 autosuspend timeout not expired, or pm_runtime_resume() called in
 -suspend() method), the pm_runtime_get*() call should actually
 succeed.

I'd say the problem is when the drive in question uses the return value of
pm_runtime_get_sync(), for example, to decide whether or not it is safe
to access the hardware.  In that case it may decide that accessing the
hardware is unsafe during system suspend, although that's not really the
case.  So the change you're proposing allows drivers of this kind  (and there
may be a substantial number of them) to be simplified slightly.

 To permit the usage described above, add a check to rpm_resume() so
 that success is returned in the case where a driver is suspended (it's
 -suspend callback has been called) but is still RPM_ACTIVE.
 
 This patch was developed in close collaboration with Rafael J. Wysocki
 r...@sisk.pl
 
 Tested on AM3730/Beagle-xM where wakeup IRQ firing during the late
 suspend phase triggers runtime PM activity in the I2C driver since the
 wakeup IRQ is on an I2C-connected PMIC.
 
 Cc: Rafael J. Wysocki r...@sisk.pl
 Signed-off-by: Kevin Hilman khil...@ti.com

OK, I wonder if anyone has any objections against this.  Alan?

Rafael


 ---
 This version is just a resend with the vger address for linux-pm.
 
  drivers/base/power/runtime.c |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
 index 7d9c1cb..dafa5ec 100644
 --- a/drivers/base/power/runtime.c
 +++ b/drivers/base/power/runtime.c
 @@ -510,7 +510,8 @@ static int rpm_resume(struct device *dev, int rpmflags)
   if (dev-power.runtime_error)
   retval = -EINVAL;
   else if (dev-power.disable_depth  0)
 - retval = -EACCES;
 + retval = dev-power.is_suspended  
 + dev-power.runtime_status == RPM_ACTIVE ? 1 : -EACCES;
   if (retval)
   goto out;
  
 

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

Re: [PATCH] cpufreq: OMAP: Check IS_ERR() instead of NULL for omap_device_get_by_hwmod_name

[Rafael J. Wysocki]

On Wednesday, September 19, 2012, Kevin Hilman wrote:
 Rafael J. Wysocki r...@sisk.pl writes:
 
  On Tuesday, September 18, 2012, Axel Lin wrote:
  omap_device_get_by_hwmod_name() returns ERR_PTR on error.
  
  Signed-off-by: Axel Lin axel@gmail.com
 
  Kevin, should I take this?
 
 Yes, please.
 
 Acked-by: Kevin Hilman khil...@ti.com

Done.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] CPUFreq: OMAP: remove unnecessary plat/ includes

[Rafael J. Wysocki]

On Thursday, September 20, 2012, Paul Walmsley wrote:
 Hi
 
 On Fri, 14 Sep 2012, Rafael J. Wysocki wrote:
 
  On Wednesday, September 12, 2012, Paul Walmsley wrote:
   
   Remove some unnecessary plat/ includes that are interfering with 
   multi-subarch
   ARM kernels.
   
   Signed-off-by: Paul Walmsley p...@pwsan.com
   Cc: Kevin Hilman khil...@ti.com
   Cc: Rafael J. Wysocki r...@sisk.pl
   Acked-by: Kevin Hilman khil...@ti.com
  
  Acked-by: Rafael J. Wysocki r...@sisk.pl
  
   ---
   
   Still awaiting some final testing here.  Rafael, was wondering if you 
   would be willing to ack this so we can merge it via the OMAP tree?   
   Otherwise the patch can be split into an OMAP part and a CPUFreq part 
   that 
   can go in across two merge windows.
   
arch/arm/mach-omap2/clock2420_data.c |1 +
arch/arm/mach-omap2/clock2430_data.c |1 +
arch/arm/mach-omap2/clock3xxx_data.c |1 +
arch/arm/mach-omap2/clock44xx_data.c |1 +
drivers/cpufreq/omap-cpufreq.c   |   19 +--
5 files changed, 5 insertions(+), 18 deletions(-)
 
 Thanks Rafael.  Looks like you've got cpufreq: OMAP: Check IS_ERR() 
 instead of NULL for omap_device_get_by_hwmod_name queued which conflicts 
 with this patch, so will just queue the arch/arm/mach-omap2/ changes for 
 now.  Then the drivers/cpufreq/omap-cpufreq.c part can go in during 3.8.

OK

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] cpufreq: OMAP: Check IS_ERR() instead of NULL for omap_device_get_by_hwmod_name

[Rafael J. Wysocki]

On Tuesday, September 18, 2012, Axel Lin wrote:
 omap_device_get_by_hwmod_name() returns ERR_PTR on error.
 
 Signed-off-by: Axel Lin axel@gmail.com

Kevin, should I take this?

Rafael


 ---
  drivers/cpufreq/omap-cpufreq.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
 index 6e22f44..65f8e9a 100644
 --- a/drivers/cpufreq/omap-cpufreq.c
 +++ b/drivers/cpufreq/omap-cpufreq.c
 @@ -266,9 +266,9 @@ static int __init omap_cpufreq_init(void)
   }
  
   mpu_dev = omap_device_get_by_hwmod_name(mpu);
 - if (!mpu_dev) {
 + if (IS_ERR(mpu_dev)) {
   pr_warning(%s: unable to get the mpu device\n, __func__);
 - return -EINVAL;
 + return PTR_ERR(mpu_dev);
   }
  
   mpu_reg = regulator_get(mpu_dev, vcc);
 

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

Re: [PATCH] CPUFreq: OMAP: remove unnecessary plat/ includes

[Rafael J. Wysocki]

On Wednesday, September 12, 2012, Paul Walmsley wrote:
 
 Remove some unnecessary plat/ includes that are interfering with multi-subarch
 ARM kernels.
 
 Signed-off-by: Paul Walmsley p...@pwsan.com
 Cc: Kevin Hilman khil...@ti.com
 Cc: Rafael J. Wysocki r...@sisk.pl
 Acked-by: Kevin Hilman khil...@ti.com

Acked-by: Rafael J. Wysocki r...@sisk.pl

 ---
 
 Still awaiting some final testing here.  Rafael, was wondering if you 
 would be willing to ack this so we can merge it via the OMAP tree?   
 Otherwise the patch can be split into an OMAP part and a CPUFreq part that 
 can go in across two merge windows.
 
  arch/arm/mach-omap2/clock2420_data.c |1 +
  arch/arm/mach-omap2/clock2430_data.c |1 +
  arch/arm/mach-omap2/clock3xxx_data.c |1 +
  arch/arm/mach-omap2/clock44xx_data.c |1 +
  drivers/cpufreq/omap-cpufreq.c   |   19 +--
  5 files changed, 5 insertions(+), 18 deletions(-)
 
 diff --git a/arch/arm/mach-omap2/clock2420_data.c 
 b/arch/arm/mach-omap2/clock2420_data.c
 index ea8e883..de1b9a4 100644
 --- a/arch/arm/mach-omap2/clock2420_data.c
 +++ b/arch/arm/mach-omap2/clock2420_data.c
 @@ -1897,6 +1897,7 @@ static struct omap_clk omap2420_clks[] = {
   CLK(NULL,   pka_ick,  pka_ick,   CK_242X),
   CLK(NULL,   usb_fck,  usb_fck,   CK_242X),
   CLK(musb-hdrc,fck,  osc_ck,CK_242X),
 + CLK(NULL,   cpufreq_ck,   virt_prcm_set, CK_242X),
  };
  
  /*
 diff --git a/arch/arm/mach-omap2/clock2430_data.c 
 b/arch/arm/mach-omap2/clock2430_data.c
 index cacabb0..d3ecdf7 100644
 --- a/arch/arm/mach-omap2/clock2430_data.c
 +++ b/arch/arm/mach-omap2/clock2430_data.c
 @@ -1993,6 +1993,7 @@ static struct omap_clk omap2430_clks[] = {
   CLK(NULL,   timer_32k_ck,  func_32k_ck,   CK_243X),
   CLK(NULL,   timer_sys_ck, sys_ck,CK_243X),
   CLK(NULL,   timer_ext_ck, alt_ck,CK_243X),
 + CLK(NULL,   cpufreq_ck,   virt_prcm_set, CK_243X),
  };
  
  /*
 diff --git a/arch/arm/mach-omap2/clock3xxx_data.c 
 b/arch/arm/mach-omap2/clock3xxx_data.c
 index 83bed9a..ea4690c 100644
 --- a/arch/arm/mach-omap2/clock3xxx_data.c
 +++ b/arch/arm/mach-omap2/clock3xxx_data.c
 @@ -3466,6 +3466,7 @@ static struct omap_clk omap3xxx_clks[] = {
   CLK(NULL,   uart4_ick,uart4_ick_am35xx,  CK_AM35XX),
   CLK(NULL,   timer_32k_ck, omap_32k_fck,  CK_3XXX),
   CLK(NULL,   timer_sys_ck, sys_ck,CK_3XXX),
 + CLK(NULL,   cpufreq_ck,   dpll1_ck,  CK_3XXX),
  };
  
  
 diff --git a/arch/arm/mach-omap2/clock44xx_data.c 
 b/arch/arm/mach-omap2/clock44xx_data.c
 index d7f55e4..9b31767 100644
 --- a/arch/arm/mach-omap2/clock44xx_data.c
 +++ b/arch/arm/mach-omap2/clock44xx_data.c
 @@ -3325,6 +3325,7 @@ static struct omap_clk omap44xx_clks[] = {
   CLK(omap_timer.6, timer_sys_ck, syc_clk_div_ck,
 CK_443X),
   CLK(omap_timer.7, timer_sys_ck, syc_clk_div_ck,
 CK_443X),
   CLK(omap_timer.8, timer_sys_ck, syc_clk_div_ck,
 CK_443X),
 + CLK(NULL,   cpufreq_ck,   dpll_mpu_ck,   CK_443X),
  };
  
  int __init omap4xxx_clk_init(void)
 diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
 index b47034e..2737d08 100644
 --- a/drivers/cpufreq/omap-cpufreq.c
 +++ b/drivers/cpufreq/omap-cpufreq.c
 @@ -30,13 +30,9 @@
  #include asm/smp_plat.h
  #include asm/cpu.h
  
 -#include plat/clock.h
 -#include plat/omap-pm.h
  #include plat/common.h
  #include plat/omap_device.h
  
 -#include mach/hardware.h
 -
  /* OPP tolerance in percentage */
  #define  OPP_TOLERANCE   4
  
 @@ -53,7 +49,6 @@ static struct lpj_info global_lpj_ref;
  static struct cpufreq_frequency_table *freq_table;
  static atomic_t freq_table_users = ATOMIC_INIT(0);
  static struct clk *mpu_clk;
 -static char *mpu_clk_name;
  static struct device *mpu_dev;
  static struct regulator *mpu_reg;
  
 @@ -207,7 +202,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy 
 *policy)
  {
   int result = 0;
  
 - mpu_clk = clk_get(NULL, mpu_clk_name);
 + mpu_clk = clk_get(NULL, cpufreq_ck);
   if (IS_ERR(mpu_clk))
   return PTR_ERR(mpu_clk);
  
 @@ -288,18 +283,6 @@ static struct cpufreq_driver omap_driver = {
  
  static int __init omap_cpufreq_init(void)
  {
 - if (cpu_is_omap24xx())
 - mpu_clk_name = virt_prcm_set;
 - else if (cpu_is_omap34xx())
 - mpu_clk_name = dpll1_ck;
 - else if (cpu_is_omap44xx())
 - mpu_clk_name = dpll_mpu_ck;
 -
 - if (!mpu_clk_name) {
 - pr_err(%s: unsupported Silicon?\n, __func__);
 - return -EINVAL;
 - }
 -
   mpu_dev = omap_device_get_by_hwmod_name(mpu);
   if (!mpu_dev) {
   pr_warning(%s: unable to get the mpu device\n, __func__);
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http

Re: [PATCH v2 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems

[Rafael J. Wysocki]

On Thursday, August 09, 2012, Rajendra Nayak wrote:
 On OMAP4, if the first CPU fails to get a valid frequency table (this
 could happen if the platform does not register any OPP table), the
 subsequent CPU instances end up dealing with a NULL freq_table and
 crash.
 
 Check for an already existing freq_table, before trying to create one,
 and increment the freq_table_users only if the table is sucessfully
 created.
 
 Signed-off-by: Rajendra Nayak rna...@ti.com
 Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: linux...@vger.kernel.org

Kevin, are you going to merge this?

Rafael


 ---
  drivers/cpufreq/omap-cpufreq.c |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
 index 17fa04d..b47034e 100644
 --- a/drivers/cpufreq/omap-cpufreq.c
 +++ b/drivers/cpufreq/omap-cpufreq.c
 @@ -218,7 +218,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy 
 *policy)
  
   policy-cur = policy-min = policy-max = omap_getspeed(policy-cpu);
  
 - if (atomic_inc_return(freq_table_users) == 1)
 + if (!freq_table)
   result = opp_init_cpufreq_table(mpu_dev, freq_table);
  
   if (result) {
 @@ -227,6 +227,8 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy 
 *policy)
   goto fail_ck;
   }
  
 + atomic_inc_return(freq_table_users);
 +
   result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
   if (result)
   goto fail_table;
 

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

Re: [PATCH v2 1/2] cpufreq: OMAP: Handle missing frequency table on SMP systems

[Rafael J. Wysocki]

On Thursday, August 09, 2012, Kevin Hilman wrote:
 Rafael J. Wysocki r...@sisk.pl writes:
 
  On Thursday, August 09, 2012, Rajendra Nayak wrote:
  On OMAP4, if the first CPU fails to get a valid frequency table (this
  could happen if the platform does not register any OPP table), the
  subsequent CPU instances end up dealing with a NULL freq_table and
  crash.
  
  Check for an already existing freq_table, before trying to create one,
  and increment the freq_table_users only if the table is sucessfully
  created.
  
  Signed-off-by: Rajendra Nayak rna...@ti.com
  Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
  Cc: linux...@vger.kernel.org
 
  Kevin, are you going to merge this?
 
 
 Yes, I plan to queue this with 2/2 for v3.6-rc.

If you have any cpufreq patches for v3.7, can you please tell me where to
pull them from (and when)?

Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] MFD: twl-core: move device_init_wakeup to after platform_device_add.

[Rafael J. Wysocki]

On Saturday, July 07, 2012, NeilBrown wrote:
 
 device_init_wakeup uses the dev_name() of the device to set the
 name of the wakeup_source which appears in
 /sys/kernel/debug/wakeup_sources.
 
 For a platform device, that name is not set until platform_device_add
 calls dev_set_name.
 
 So the call to device_init_wakeup() must be after the call to
 platform_device_add().
 Making this change causes correct names to appear in the
 wakeup_sources file.
 
 
 Signed-off-by: NeilBrown ne...@suse.de

Acked-by: Rafael J. Wysocki r...@sisk.pl

 
 diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
 index 6fc90be..b012efd 100644
 --- a/drivers/mfd/twl-core.c
 +++ b/drivers/mfd/twl-core.c
 @@ -568,7 +568,6 @@ add_numbered_child(unsigned chip, const char *name, int 
 num,
   goto err;
   }
  
 - device_init_wakeup(pdev-dev, can_wakeup);
   pdev-dev.parent = twl-client-dev;
  
   if (pdata) {
 @@ -593,6 +592,8 @@ add_numbered_child(unsigned chip, const char *name, int 
 num,
   }
  
   status = platform_device_add(pdev);
 + if (status == 0)
 + device_init_wakeup(pdev-dev, can_wakeup);
  
  err:
   if (status  0) {
 

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

[PATCH] omap-rng: Use struct dev_pm_ops for power management

[Rafael J. Wysocki]

From: Rafael J. Wysocki r...@sisk.pl

Make the omap-rng driver define its PM callbacks through
a struct dev_pm_ops object rather than by using legacy PM hooks
in struct platform_driver.

Signed-off-by: Rafael J. Wysocki r...@sisk.pl
---
 drivers/char/hw_random/omap-rng.c |   13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

Index: linux/drivers/char/hw_random/omap-rng.c
===
--- linux.orig/drivers/char/hw_random/omap-rng.c
+++ linux/drivers/char/hw_random/omap-rng.c
@@ -162,22 +162,24 @@ static int __exit omap_rng_remove(struct
 
 #ifdef CONFIG_PM
 
-static int omap_rng_suspend(struct platform_device *pdev, pm_message_t message)
+static int omap_rng_suspend(struct device *dev)
 {
omap_rng_write_reg(RNG_MASK_REG, 0x0);
return 0;
 }
 
-static int omap_rng_resume(struct platform_device *pdev)
+static int omap_rng_resume(struct device *dev)
 {
omap_rng_write_reg(RNG_MASK_REG, 0x1);
return 0;
 }
 
+static SIMPLE_DEV_PM_OPS(omap_rng_pm, omap_rng_suspend, omap_rng_resume);
+#defineOMAP_RNG_PM (omap_rng_pm)
+
 #else
 
-#defineomap_rng_suspendNULL
-#defineomap_rng_resume NULL
+#defineOMAP_RNG_PM NULL
 
 #endif
 
@@ -188,11 +190,10 @@ static struct platform_driver omap_rng_d
.driver = {
.name   = omap_rng,
.owner  = THIS_MODULE,
+   .pm = OMAP_RNG_PM,
},
.probe  = omap_rng_probe,
.remove = __exit_p(omap_rng_remove),
-   .suspend= omap_rng_suspend,
-   .resume = omap_rng_resume
 };
 
 static int __init omap_rng_init(void)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/9] PM: Create the AVS class of drivers

[Rafael J. Wysocki]

On Wednesday, April 18, 2012, Kevin Hilman wrote:
 Rafael,
 
 jean.pi...@newoldbits.com writes:
 
  From: Jean Pihet j-pi...@ti.com
 
  AVS is a power management technique which controls the operating
  voltage of a device in order to optimize (i.e. reduce) its power
  consumption. The voltage is adapted depending on static factors
  (chip manufacturing process) and dynamic factors (temperature
  depending performance).
  AVS is also called SmartReflex on OMAP devices.
 
  To that end, create the AVS framework in drivers/power/avs and
  move the OMAP SmartReflex code to the new directory.
 
 I've done a review of this series and had a few very minor comments.
 
 Once those are addressed, are you OK with us moving this AVS driver
 under drivers/power?

Yes, I am.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Rafael J. Wysocki]

On Wednesday, March 21, 2012, Amit Kucheria wrote:
 On Wed, Mar 21, 2012 at 12:48 AM, Kevin Hilman khil...@ti.com wrote:
  Arnd Bergmann arnd.bergm...@linaro.org writes:
 
  On Tuesday 20 March 2012, Robert Lee wrote:
  This patch series moves various functionality duplicated in platform
  cpuidle drivers to the core cpuidle driver. Also, the platform irq
  disabling was removed as it appears that all calls into
  cpuidle_call_idle will have already called local_irq_disable().
 
  These changes have been pulled into linux-next.
 
  Len, Andrew, can a request be made for Linus to pull these changes?
 
  FWIW, Len seems to be rather inactive on the kernel mailing list right
  now and generally not very interested in anything outside of x86 and
  acpi. If he doesn't reply in the next few days and Andrew also isn't
  interested in handling these patches, I'd suggest you just send the pull
  request to Linus, with Len on Cc and explain that you tried to send
  them through him but gave up in the end.
 
  FWIW, I have not had good luck getting response for proposed core
  CPUidle changes either:
 
 http://lkml.org/lkml/2011/9/19/374
 
  Maybe it's time that drivers/cpuidle gets a maintainer.  With lots of
  discussions of scheduler changes that affect load estimation, I suspect
  we're all going to have a bit of CPUidle work to do in the
  not-so-distant future.
 
 
 I don't mean to be piling on to Len here, but Daniel Lezcano too has a
 bunch of clean ups that didn't get any maintainer review for over two
 months. He has now refreshed them for 3.3 and is getting ready to send
 them out again. We (Linaro) expect to be spending a lot of time on
 cpuidle in the future and would be glad to help review, test and
 collect patches into a tree for Linus/Andrew to pull while we wait for
 Len to respond or another maintainer to emerge.

Well, I discussed that before with Arjan and he said he would maintain
CPUidle if Len didn't have the time, but it seems he didn't expect that
there would be a lot of work on it in the near future.

So, I suggest that if neither Len nor Arjan reappear shortly, people can
send CPUidle patches to me.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/10] ARM: OMAP: SmartReflex: Move smartreflex driver to drivers/

[Rafael J. Wysocki]

On Thursday, March 15, 2012, jean.pi...@newoldbits.com wrote:
 From: Jean Pihet j-pi...@ti.com
 
 After a clean-up of the interfaces the OMAP IP driver and class
 support code is now a generic driver.
 Move it to drivers/power/avs/.
 
 The build is controlled by the following Kconfig options:
  . CONFIG_POWER_AVS: general knob for Adaptive Voltage Scaling support,
  . CONFIG_POWER_AVS_OMAP: AVS support on OMAP containing the version 1
 or version 2 of the SmartReflex IP,
  . CONFIG_POWER_AVS_OMAP_CLASS3: Class 3 implementation of Smartreflex.
 
 Signed-off-by: Jean Pihet j-pi...@ti.com
 ---
[...]
 diff --git a/drivers/power/Makefile b/drivers/power/Makefile
 index e429008..e4a8fd2 100644
 --- a/drivers/power/Makefile
 +++ b/drivers/power/Makefile
 @@ -41,3 +41,5 @@ obj-$(CONFIG_CHARGER_GPIO)  += gpio-charger.o
  obj-$(CONFIG_CHARGER_MANAGER)+= charger-manager.o
  obj-$(CONFIG_CHARGER_MAX8997)+= max8997_charger.o
  obj-$(CONFIG_CHARGER_MAX8998)+= max8998_charger.o
 +
 +obj-$(CONFIG_POWER_AVS)  += avs/
 diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
 new file mode 100644
 index 000..35b21c4
 --- /dev/null
 +++ b/drivers/power/avs/Kconfig
 @@ -0,0 +1,45 @@
 +menuconfig POWER_AVS
 + tristate Adaptive Voltage Scaling class support
 + help
 +   AVS is a power management technique which finely controls the
 +   operating voltage of a device in order to optimize (i.e. reduce)
 +   its power consumption.
 +   At a given operating point the voltage is adapted depending on
 +   static factors (chip manufacturing process) and dynamic factors
 +   (temperature depending performance).
 +   AVS is also called SmartReflex on OMAP devices.
 +
 +   Say Y here to enable Adaptive Voltage Scaling class support.
 +
 +if POWER_AVS
 +
 +config POWER_AVS_OMAP

I think it would be better to keep this and the next one in the OMAP
arch Kconfig.

 + bool AVS support for the OMAP IP versions 12
 + depends on (ARCH_OMAP3 || ARCH_OMAP4)  PM
 + help
 +   Say Y to enable AVS support on OMAP containing the version 1 or
 +   version 2 of the SmartReflex IP.
 +   V1 is the 65nm version used in OMAP3430.
 +   V2 is the update for the 45nm version of the IP used in OMAP3630
 +   and OMAP4430
 +
 +   Please note, that by default SmartReflex is only
 +   initialized and not enabled. To enable the automatic voltage
 +   compensation for vdd mpu and vdd core from user space,
 +   user must write 1 to
 + /debug/smartreflex/sr_X/autocomp,
 +   where X is mpu_iva or core for OMAP3.
 +   Optionally autocompensation can be enabled in the kernel
 +   by default during system init via the enable_on_init flag
 +   which an be passed as platform data to the smartreflex driver.
 +
 +config POWER_AVS_OMAP_CLASS3
 + bool Class 3 mode of Smartreflex Implementation
 + depends on POWER_AVS_OMAP  TWL4030_CORE
 + help
 +   Say Y to enable Class 3 implementation of Smartreflex
 +
 +   Class 3 implementation of Smartreflex employs continuous hardware
 +   voltage calibration.
 +
 +endif # POWER_AVS

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 2/2] mtd : Make the mtd_suspend return 0 if the suspend is not implemented

[Rafael J. Wysocki]

On Thursday, February 02, 2012, Artem Bityutskiy wrote:
 On Mon, 2012-01-30 at 23:58 +0100, Rafael J. Wysocki wrote:
  Thanks, but is anyone actually going to push it to Linus any time soon?
 
 I agree, but I am not the maintainer so cannot answer. David Woodhouse
 is aware of this, but I do not know when he gonna send the pull request.

Well, the problem is being reported by more and more people, so I'd
like to push the fix quickly.

David, please let me know if you don't want me to include the $subject patch
into the next PM fixes pull request.  If I don't hear from you, I'll push it
next week for -rc4.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 2/2] mtd : Make the mtd_suspend return 0 if the suspend is not implemented

[Rafael J. Wysocki]

On Thursday, February 02, 2012, Russell King - ARM Linux wrote:
 On Thu, Feb 02, 2012 at 07:40:26PM +0100, Rafael J. Wysocki wrote:
  On Thursday, February 02, 2012, Artem Bityutskiy wrote:
   On Mon, 2012-01-30 at 23:58 +0100, Rafael J. Wysocki wrote:
Thanks, but is anyone actually going to push it to Linus any time soon?
   
   I agree, but I am not the maintainer so cannot answer. David Woodhouse
   is aware of this, but I do not know when he gonna send the pull request.
  
  Well, the problem is being reported by more and more people, so I'd
  like to push the fix quickly.
  
  David, please let me know if you don't want me to include the $subject patch
  into the next PM fixes pull request.  If I don't hear from you, I'll push it
  next week for -rc4.
 
 I too have it in my tree as a patch which Artem sent me, as it's rather
 fundamental to not oopsing the kernel when you suspend/resume on any ARM
 platform.

It actually affects everyone using MTD and suspend (not only ARM) and that's
why I thought I'd take it, but I'm fine with any other resolution as long as
the patch goes to Linus before 3.3-final.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 2/2] mtd : Make the mtd_suspend return 0 if the suspend is not implemented

[Rafael J. Wysocki]

On Monday, January 30, 2012, Artem Bityutskiy wrote:
 On Tue, 2012-01-24 at 09:06 +, Russell King - ARM Linux wrote:
   However, the bug made it into the 3.3 merge window, so shouldn't this
   bugfix be sent upstream immediately?
  
  David is the MTD maintainer, and Artem just helps out.  I believe Artem
  is waiting for David to finish travelling before asking David (last seen
  at Hong Kong airport) to pull these fixes.  David in turn will pass them
  onto Linus.  Plus, Linus only started adding to -rc1 yesterday, so its a
  little early to expect this to be fixed.
 
 Hi,
 
 here is the latest version of the fix.
 
 http://git.infradead.org/users/dedekind/l2-mtd-2.6.git/commit/283d43b9ce2952535aa89c0195085e2a1b7e5fce
 
 Also attached.
 
 From 283d43b9ce2952535aa89c0195085e2a1b7e5fce Mon Sep 17 00:00:00 2001
 From: Artem Bityutskiy artem.bityuts...@linux.intel.com
 Date: Mon, 16 Jan 2012 11:07:16 +0200
 Subject: [PATCH] mtd: fix MTD suspend
 
 Commits 3fe4bae88460869a8e553397cd9057a4ee7ca341 and
 079c985e7a6f4ce60f931cebfdd5ee3c3 broke MTD suspend in 2 ways:
 
 1. When the '-suspend' method is not present, we return -EOPNOTSUPP,
 but
the callers of 'mtd_suspend()' expects 0 instead.
 2. Checking of the 'mtd' parameter against NULL has been incorrectly
 removed
in 'mtd_cls_suspend()'.
 
 This patch fixes the breakages. This has been found, analyzed, reported
 and tested by Rafael J. Wysocki r...@sisk.pl.
 
 Note, this patch is not needed in the stable tree because it causes a
 regression introduced during the v3.3 merge window.
 
 Reported-by: Rafael J. Wysocki r...@sisk.pl
 Tested-by: Rafael J. Wysocki r...@sisk.pl
 Tested-by: Russell King rmk+ker...@arm.linux.org.uk
 Signed-off-by: Artem Bityutskiy artem.bityuts...@linux.intel.com

Thanks, but is anyone actually going to push it to Linus any time soon?

If not, then I can do that through the linux-pm tree.  I'll be sending
at least one more pull request with fixes for v3.3.

Thanks,
Rafael


 ---
  drivers/mtd/mtdcore.c   |2 +-
  include/linux/mtd/mtd.h |4 +---
  2 files changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
 index b265188..de96865 100644
 --- a/drivers/mtd/mtdcore.c
 +++ b/drivers/mtd/mtdcore.c
 @@ -119,7 +119,7 @@ static int mtd_cls_suspend(struct device *dev,
 pm_message_t state)
  {
   struct mtd_info *mtd = dev_get_drvdata(dev);
  
 - return mtd_suspend(mtd);
 + return mtd ? mtd_suspend(mtd) : 0;
  }
  
  static int mtd_cls_resume(struct device *dev)
 diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
 index 1a81fde..d8c7aad 100644
 --- a/include/linux/mtd/mtd.h
 +++ b/include/linux/mtd/mtd.h
 @@ -427,9 +427,7 @@ static inline int mtd_is_locked(struct mtd_info
 *mtd, loff_t ofs, uint64_t len)
  
  static inline int mtd_suspend(struct mtd_info *mtd)
  {
 - if (!mtd-suspend)
 - return -EOPNOTSUPP;
 - return mtd-suspend(mtd);
 + return mtd-suspend ? mtd-suspend(mtd) : 0;
  }
  
  static inline void mtd_resume(struct mtd_info *mtd)
 

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

Re: PM / OPP: Fix build when CONFIG_PM_OPP is not set

[Rafael J. Wysocki]

On Wednesday, November 02, 2011, Tony Lindgren wrote:
 Commit 03ca370fbf7b76d6d002380dbdc2cdc2319f9c80 (PM / OPP: Add
 OPP availability change notifier) does not compile if CONFIG_PM_OPP
 is not set:
 
 arch/arm/plat-omap/omap-pm-noop.o: In function `opp_get_notifier':
 include/linux/opp.h:103: multiple definition of `opp_get_notifier'
 include/linux/opp.h:103: first defined here
 
 Also fix incorrect comment.
 
 Cc: MyungJoo Ham myungjoo@samsung.com
 Cc: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Mike Turquette mturque...@ti.com
 Cc: Kevin Hilman khil...@ti.com
 Cc: Rafael J. Wysocki r...@sisk.pl
 Signed-off-by: Tony Lindgren t...@atomide.com

Applied to linux-pm/linux-next.

Thanks,
Rafael


 ---
 
 I'm seeing this with omap1_defconfig at least.
 
 --- a/include/linux/opp.h
 +++ b/include/linux/opp.h
 @@ -97,11 +97,11 @@ static inline int opp_disable(struct device *dev, 
 unsigned long freq)
   return 0;
  }
  
 -struct srcu_notifier_head *opp_get_notifier(struct device *dev)
 +static inline struct srcu_notifier_head *opp_get_notifier(struct device *dev)
  {
   return ERR_PTR(-EINVAL);
  }
 -#endif   /* CONFIG_PM */
 +#endif   /* CONFIG_PM_OPP */
  
  #if defined(CONFIG_CPU_FREQ)  defined(CONFIG_PM_OPP)
  int opp_init_cpufreq_table(struct device *dev,
 
 

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

Re: [PATCH] PM QoS: update Documentation for the pm_qos and dev_pm_qos frameworks

[Rafael J. Wysocki]

On Monday, October 03, 2011, jean.pi...@newoldbits.com wrote:
 From: Jean Pihet j-pi...@ti.com
 
 Update the documentation for the recently updated pm_qos API, kernel
 and user space.
 Add the documentation for the per-device PM QoS (dev_pm_qos) framework
 API.
 
 Signed-off-by: Jean Pihet j-pi...@ti.com

Applied to linux-pm/pm-qos (and merged into linux-pm/linux-next).

Thanks,
Rafael


 ---
  Documentation/power/pm_qos_interface.txt |   92 
 --
  1 files changed, 87 insertions(+), 5 deletions(-)
 
 diff --git a/Documentation/power/pm_qos_interface.txt 
 b/Documentation/power/pm_qos_interface.txt
 index bfed898..17e130a 100644
 --- a/Documentation/power/pm_qos_interface.txt
 +++ b/Documentation/power/pm_qos_interface.txt
 @@ -4,14 +4,19 @@ This interface provides a kernel and user mode interface 
 for registering
  performance expectations by drivers, subsystems and user space applications 
 on
  one of the parameters.
  
 -Currently we have {cpu_dma_latency, network_latency, network_throughput} as 
 the
 -initial set of pm_qos parameters.
 +Two different PM QoS frameworks are available:
 +1. PM QoS classes for cpu_dma_latency, network_latency, network_throughput.
 +2. the per-device PM QoS framework provides the API to manage the per-device 
 latency
 +constraints.
  
  Each parameters have defined units:
   * latency: usec
   * timeout: usec
   * throughput: kbs (kilo bit / sec)
  
 +
 +1. PM QoS framework
 +
  The infrastructure exposes multiple misc device nodes one per implemented
  parameter.  The set of parameters implement is defined by pm_qos_power_init()
  and pm_qos_params.h.  This is done because having the available parameters
 @@ -23,14 +28,18 @@ an aggregated target value.  The aggregated target value 
 is updated with
  changes to the request list or elements of the list.  Typically the
  aggregated target value is simply the max or min of the request values held
  in the parameter list elements.
 +Note: the aggregated target value is implemented as an atomic variable so 
 that
 +reading the aggregated value does not require any locking mechanism.
 +
  
  From kernel mode the use of this interface is simple:
  
 -handle = pm_qos_add_request(param_class, target_value):
 -Will insert an element into the list for that identified PM_QOS class with 
 the
 +void pm_qos_add_request(handle, param_class, target_value):
 +Will insert an element into the list for that identified PM QoS class with 
 the
  target value.  Upon change to this list the new target is recomputed and any
  registered notifiers are called only if the target value is now different.
 -Clients of pm_qos need to save the returned handle.
 +Clients of pm_qos need to save the returned handle for future use in other
 +pm_qos API functions.
  
  void pm_qos_update_request(handle, new_target_value):
  Will update the list element pointed to by the handle with the new target 
 value
 @@ -42,6 +51,20 @@ Will remove the element.  After removal it will update the 
 aggregate target and
  call the notification tree if the target was changed as a result of removing
  the request.
  
 +int pm_qos_request(param_class):
 +Returns the aggregated value for a given PM QoS class.
 +
 +int pm_qos_request_active(handle):
 +Returns if the request is still active, i.e. it has not been removed from a
 +PM QoS class constraints list.
 +
 +int pm_qos_add_notifier(param_class, notifier):
 +Adds a notification callback function to the PM QoS class. The callback is
 +called when the aggregated value for the PM QoS class is changed.
 +
 +int pm_qos_remove_notifier(int param_class, notifier):
 +Removes the notification callback function for the PM QoS class.
 +
  
  From user mode:
  Only processes can register a pm_qos request.  To provide for automatic
 @@ -63,4 +86,63 @@ To remove the user mode request for a target value simply 
 close the device
  node.
  
  
 +2. PM QoS per-device latency framework
 +
 +For each device a list of performance requests is maintained along with
 +an aggregated target value.  The aggregated target value is updated with
 +changes to the request list or elements of the list.  Typically the
 +aggregated target value is simply the max or min of the request values held
 +in the parameter list elements.
 +Note: the aggregated target value is implemented as an atomic variable so 
 that
 +reading the aggregated value does not require any locking mechanism.
 +
 +
 +From kernel mode the use of this interface is the following:
 +
 +int dev_pm_qos_add_request(device, handle, value):
 +Will insert an element into the list for that identified device with the
 +target value.  Upon change to this list the new target is recomputed and any
 +registered notifiers are called only if the target value is now different.
 +Clients of dev_pm_qos need to save the handle for future use in other
 +dev_pm_qos API functions.
 +
 +int dev_pm_qos_update_request(handle, new_value):
 +Will update the list element pointed to by the 

Re: [RFC/PATCH] PM QoS: main memory throughput constraints

[Rafael J. Wysocki]

Hi,

On Friday, September 30, 2011, Jean Pihet wrote:
 Hi,
 
 Here is a patch which adds a PM_QOS_MEMORY_THROUGHPUT class to the PM
 QoS framework. The idea is to provide a memory or SDMA throughput
 constraints class, which can be applied to the low level platform code
 using the callback notification mechanism and also a MISC /dev entry
 for the constraints from user space.
 
 The first user of this class of constraints is the OMAP platform. The
 aggregated constraint is fed to the DVFS layer in order to control the
 main interconnect (L3) frequency and so the memory bandwidth. This
 comes as a subsequent patch.
 
 This patch is RFC since more throughput constraints could be added in
 the future, in order to control more clocks in the system. Is it worth
 adding a class for every new subsystem to control?
 
 What do you think?
 
 The patch follows asap as a reply to this message. It applies on
 Rafael' latest linux-pm tree, cf.
 https://github.com/rjwysocki/linux-pm/commits/pm-qos.

Quite frankly, I'm not very fond of creating more special device files for
PM QoS at this point.  It would be nice if you could give an example of
how you intend to use it, especially from the user space's viewpoint.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP: omap_device: fix !CONFIG_SUSPEND case in _noirq handlers

[Rafael J. Wysocki]

On Thursday, September 01, 2011, Arnd Bergmann wrote:
 On Thursday 01 September 2011 11:12:02 Kevin Hilman wrote:
  The suspend/resume _noirq handlers were #ifdef'd out in the
  !CONFIG_SUSPEND case, but were still assigned to the dev_pm_ops
  struct.  Fix by defining them to NULL in the !CONFIG_SUSPEND case.
  
  Reported-by: Arnd Bergmann a...@arndb.de
  Signed-off-by: Kevin Hilman khil...@ti.com
 
 Acked-by: Arnd Bergmann a...@arndb.de
 
 Thansk for the fast response!

I'll apply the patch when kernel.org is back in order.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] Power management fixes for 3.1

[Rafael J. Wysocki]

Hi Linus,

Please pull power management fixes for 3.1 from:

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-fixes

They include:

* Fixes related to the sh-sci driver which uses runtime PM helpers in
  such a way that they may be run with interrupts off, but it doesn't
  tell the core about that (so the core happily enables interrupts when
  they should be off), and which also causes pm_clk_suspend() and
  pm_clk_resume() to be run with interrupts off (so they shouldn't use
  a mutex).

* shi7372 LCDC suspend fix from Magnus Damm.

* OMAP suspend callbacks fix from Kevin Hilman.

* Runtime PM documentation correction.

Thanks!


 Documentation/power/runtime_pm.txt  |3 +-
 arch/arm/mach-shmobile/board-ap4evb.c   |1 +
 arch/arm/mach-shmobile/board-mackerel.c |1 +
 arch/arm/mach-shmobile/clock-sh7372.c   |2 +
 arch/arm/plat-omap/omap_device.c|3 +-
 drivers/base/power/clock_ops.c  |   40 +--
 drivers/tty/serial/sh-sci.c |1 +
 7 files changed, 30 insertions(+), 21 deletions(-)

---

Kevin Hilman (1):
  OMAP: omap_device: only override _noirq methods, not normal suspend/resume

Magnus Damm (2):
  ARM: mach-shmobile: sh7372 LCDC1 suspend fix
  ARM: mach-shmobile: sh7372 LCDC1 suspend fix V2 (incremental)

Rafael J. Wysocki (3):
  PM: Use spinlock instead of mutex in clock management functions
  sh-sci / PM: Use power.irq_safe
  PM / Runtime: Correct documentation of pm_runtime_irq_safe()

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

Re: Issue: Runtime API usage in wake-up device irq_handler during wakeup from system-wide-suspend.

[Rafael J. Wysocki]

On Friday, August 26, 2011, Govindraj.R wrote:
 Hello,
 
 During system_wide_suspend pm runtime is disabled.
 I.e. __pm_runtime_disable is called from __device_suspend.
 Now, if a wakeup interrupt is triggered and the wakeup device irq handler
 is called even before device_resume and pm_runtime_enable happens,
 the device irq_handler proceeds to enable clock with runtime API to
 handle wakeup event.
  
 Wouldn't this result in system wide abort since the pm_runtime is not enabled
 yet from dpm_resume?
 As we end up accessing regs after doing runtime get_sync.
  
 Looks like this scenario is not handled currently.
 Or Am I missing something here?

To be precise, what do you mean by wakeup interrupt?

Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Friday, August 26, 2011, mark gross wrote:
 On Fri, Aug 26, 2011 at 11:25:13AM +0900, MyungJoo Ham wrote:
  On Fri, Jul 1, 2011 at 12:11 AM,  jean.pi...@newoldbits.com wrote:
   @@ -129,19 +146,19 @@ static const struct file_operations 
   pm_qos_power_fops = {
/* unlocked internal variant */
static inline int pm_qos_get_value(struct pm_qos_object *o)
{
   -   if (plist_head_empty(o-requests))
   +   if (plist_head_empty(o-requests))
  return o-default_value;
  
  switch (o-type) {
   -   case PM_QOS_MIN:
   -   return plist_first(o-requests)-prio;
   +   case PM_QOS_MIN:
   +   return plist_first(o-requests)-prio;
  
   -   case PM_QOS_MAX:
   -   return plist_last(o-requests)-prio;
   +   case PM_QOS_MAX:
   +   return plist_last(o-requests)-prio;
  
   -   default:
   -   /* runtime check for not using enum */
   -   BUG();
   +   default:
   +   /* runtime check for not using enum */
   +   BUG();
  }
}
  
  Hello,
  
  
  Sorry to jump in this late, but, I've got a question in choosing QoS
  value from the list with pm_qos_get_value function and pm_qos_type.
  
  For QoS objects such as network throughput, wouldn't PM_QOS_ADD be
  more appropriate than PM_QOS_MAX?
  If A is requesting 10MB/s on NIC-X and B is requesting 5MB/s on NIC-X,
  I guess PM QOS should say NIC-X that it needs to provide 15MB/s, not
  10MB/s. Or, are we assuming that A and B will never put streams at the
  same time?
 
 This was brought up a few years back as well.  The reason we kept the
 aggregate QoS for network Throughput as a max instead of a sum was that
 there where already a number of interfaces for network shaping and we
 didn't have a good answer to the problem of what to do when the
 throughput qos requested exceeds hardware capabilities.  
 
 Now that I have more experience with handsets I'm not sure it makes any
 practical difference to sum or max the throughput qos requests.  So if
 someone would like it to aggregate throughputs by summation of requests
 that could be done.  
 
 It turns out its the latency requests that are really used.

Moreover, the throughput QoS is really difficult to handle, because
quite often there's no good mapping between energy-saving measures one
can use and the resulting throughput.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] OMAP: omap_device: only override _noirq methods, not normal suspend/resume

[Rafael J. Wysocki]

On Thursday, August 25, 2011, Kevin Hilman wrote:
 commit c03f007a8bf0e092caeb6856a5c8a850df10b974 (OMAP: PM:
 omap_device: add system PM methods for PM domain handling) mistakenly
 used SET_SYSTEM_SLEEP_PM_OPS() when trying to configure custom methods
 for the PM domains noirq methods.  Fix that by setting only the
 suspend_noirq and resume_noirq methods with custom versions.
 
 Note that all other PM domain methods (including the normal
 suspend/resume methods) are populated using USE_PLATFORM_PM_SLEEP_OPS,
 which configures them all to the default subsystem (platform_bus)
 methods.
 
 Reported-by: Santosh Shilimkar santosh.shilim...@ti.com
 Tested-by: Santosh Shilimkar santosh.shilim...@ti.com
 Signed-off-by: Kevin Hilman khil...@ti.com

Applied to linux-pm/pm-fixes, will be pushed for 3.1, thanks!

Rafael


 ---
  arch/arm/plat-omap/omap_device.c |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
 
 diff --git a/arch/arm/plat-omap/omap_device.c 
 b/arch/arm/plat-omap/omap_device.c
 index b6b4097..9a6a538 100644
 --- a/arch/arm/plat-omap/omap_device.c
 +++ b/arch/arm/plat-omap/omap_device.c
 @@ -622,7 +622,8 @@ static struct dev_pm_domain omap_device_pm_domain = {
   SET_RUNTIME_PM_OPS(_od_runtime_suspend, _od_runtime_resume,
  _od_runtime_idle)
   USE_PLATFORM_PM_SLEEP_OPS
 - SET_SYSTEM_SLEEP_PM_OPS(_od_suspend_noirq, _od_resume_noirq)
 + .suspend_noirq = _od_suspend_noirq,
 + .resume_noirq = _od_resume_noirq,
   }
  };
  
 

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

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Thursday, August 25, 2011, Mark Brown wrote:
 On Tue, Aug 23, 2011 at 11:31:54PM +0200, Rafael J. Wysocki wrote:
 
  Perhaps.  Still, that requires some policy to be put into drivers,
  which I don't think is entirely correct.
 
 So, I don't really have the bandwidth to discuss this properly for at
 least the next week or so - is it possible to find some time in the
 power track at LPC for this?

I think so, I'm going to add PM QoS to the agenda anyway.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Thursday, August 25, 2011, Jean Pihet wrote:
 On Thu, Aug 25, 2011 at 4:17 PM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Thursday, August 25, 2011, Mark Brown wrote:
  On Tue, Aug 23, 2011 at 11:31:54PM +0200, Rafael J. Wysocki wrote:
 
   Perhaps.  Still, that requires some policy to be put into drivers,
   which I don't think is entirely correct.
 
  So, I don't really have the bandwidth to discuss this properly for at
  least the next week or so - is it possible to find some time in the
  power track at LPC for this?
 
  I think so, I'm going to add PM QoS to the agenda anyway.
 Unfortunately I cannot attend the conference this time.

Sorry to hear that.

 Can you please keep me in the loop of the discussions?

Sure, we will.

 I am glad to help implementing the code for the user space API.

Great, thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Tuesday, August 23, 2011, Mark Brown wrote:
 On Sun, Aug 21, 2011 at 08:05:53PM +0200, Rafael J. Wysocki wrote:
  On Sunday, August 21, 2011, Mark Brown wrote:
 
   I don't understand why the driver would need to know what situation it's
   in.  I'd been working on the basis that the idea was that the driver
   said what the constraints it has are and then some code with a more
   system wide view would make the actual decisions for things outside the
   driver domian.
 
  That's correct, but in order to figure out a sensible default
  the driver generally would need to know what the expectations with
  respect to it are.  Otherwise it can very well generate a random
  number and use that.
 
 Right, but I'd expect that should be something that the driver can
 generally do based on knowledge of what it needs to deliver to its
 users.  It doesn't need to be tunable to that - for example, input
 devices will have a reasonable idea of the response time needed to
 deliver interactive performance.  If it's really got no idea then
 hopefully not providng any constraints will do something sensible.

Perhaps.  Still, that requires some policy to be put into drivers,
which I don't think is entirely correct.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/7] PM QoS: add a per-device latency constraints framework

[Rafael J. Wysocki]

On Monday, August 22, 2011, Kevin Hilman wrote:
 jean.pi...@newoldbits.com writes:
 
  From: Jean Pihet j-pi...@ti.com
 
  High level implementation:
 
  1. Preparation of the PM QoS for the addition of a device PM QoS constraints
 framework:
. rename and move of the PM QoS implementation files to kernel/power/qos.c
  and include/linux/pm_qos.h
. rename of API parameters and internal fields names
. Move around the PM QoS misc devices management code for better 
  readability
. re-organize the internal data structs
. generalize and export the constraints management core code
 
  2. Implementation of the per-device PM QoS constraints:
. create drivers/base/power/qos.c for the implementation
. create a device PM QoS API, which calls the PM QoS constraints 
  management
  core code
. the per-device latency constraints data strctures are stored in the 
  device
  dev_pm_info struct
. the device PM code calls the init and destroy of the per-device 
  constraints
  data struct in order to support the dynamic insertion and removal of the
  devices in the system.
. to minimize the data usage by the per-device constraints, the data 
  struct
  is only allocated at the first call to dev_pm_qos_add_request. The data
  is later free'd when the device is removed from the system
. per-device notification callbacks can be registered and called upon a
  change to the aggregated constraint value
. a global mutex protects the constraints users from the data being
  allocated and free'd.
 
  3. add a global notification mechanism for the device constraints
. add a global notification chain that gets called upon changes to the
  aggregated constraint value for any device.
. the notification callbacks are passing the full constraint request data
  in order for the callees to have access to it. The current use is for 
  the
  platform low-level code to access the target device of the constraint
 
 Reviewed-by: Kevin Hilman khil...@ti.com

I guess that applies to the entire patchset?

Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Sunday, August 21, 2011, Mark Brown wrote:
 On Sat, Aug 20, 2011 at 09:14:37PM +0200, Rafael J. Wysocki wrote:
 
  I guess you mean the driver here and I'm not really sure it can.
  For instance, the driver may not know what configuration it works in,
  e.g. is there a power domain or a hierarchy of those and how much time
  it takes to power them all down and up and what the power break even is.
 
 I don't understand why the driver would need to know what situation it's
 in.  I'd been working on the basis that the idea was that the driver
 said what the constraints it has are and then some code with a more
 system wide view would make the actual decisions for things outside the
 driver domian.

That's correct, but in order to figure out a sensible default
the driver generally would need to know what the expectations with
respect to it are.  Otherwise it can very well generate a random
number and use that.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Saturday, August 20, 2011, Mark Brown wrote:
 On Fri, Aug 19, 2011 at 10:42:20PM +0200, Rafael J. Wysocki wrote:
 
  I really wouldn't like the discussion to go in circles.
 
  First, please tell me what in particular you are objecting to,
  because I don't think that's any of the patches that have been
  sent to the linux-pm list to date.
 
 The original issue that Kevin raised and CCed me in on was the idea of
 exposing raw per-device wakeup latency constraints to userspace; it
 seems much better to expose user level requirements via subsystem
 interfaces and let the subsystem and driver translate these into actual
 wakeup latency constraints:
 
   
 https://lists.linux-foundation.org/pipermail/linux-pm/2011-August/032422.html
   
 https://lists.linux-foundation.org/pipermail/linux-pm/2011-August/032428.html

OK, so let's clarify things.

I'm not sure what you mean by exposing per-device wakeup latency constraints
to user space.  I simply thought it might be useful to allow user space to
add and remove those, in analogy to what it can do with the currently available
PM QoS constraints.

My point is that this won't prevent kernel subsystems from adding their
own PM QoS constraints to devices and the constraints added by user space
will only have effect if they make the devices stay active for longer
times.  In consequence, they may only increase the energy usage of the system
and shouldn't affect functionality.

The reason why I think this may be useful is that I can readily imagine
scenarios in which user space (think a distribution or system vendor) will
want to do something that hasn't been anticipated by kernel developers
and the ability to add per-device PM QoS constraints will allow it to do that
without modifying the kernel.  Also, it will help to test and debug new drivers
and subsystems.

I don't want to prevent kernel subsystems from using their own interfaces to
acquire user-level input that translates into PM QoS constraints.  I simply
think it won't _hurt_ to provide user space with an additional level of
control over per-device PM QoS constraints.

 This is much easier for users as it translates into something they're
 actually doing (and in most cases the driver can make it Just Work) and
 it means that off the shelf applications will end up tuning the system
 appropriately by themselves.

The _end_ users really don't care.  They'll use whatever settings the user
interface exposes to them.  Moreover, even if there is an interface for
user space to add per-device PM QoS constraints, that doesn't mean the
user space is _required_ to use it.  If it doesn't, everything will work
as you describe.

 I'm additionally concerned that if we expose this stuff directly to userspace
 that's an open invitation to driver authors to not even bother trying to make
 the kernel figure this stuff out by itself and to instead tie the system
 together with magic userspace.

As I said, I don't think we can prevent that from happening anyway.
Worst case people will add strange interfaces to drivers making them
incompatible with anything except for their crazy user space.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Saturday, August 20, 2011, Mark Brown wrote:
 On Sat, Aug 20, 2011 at 09:48:25AM -0400, Alan Stern wrote:
 
  No, I as wasking about driver- and subsystem-specific interfaces to
  userspace that translate into things users are already doing.  Kevin's
  example was a touchscreen (although that was really an example of
  setting a power usage level, not a latency constraint).  Do you have
  any others?
 
 Any sort of media streaming would be an obvious example - the
 application picks the amount of data it buffers and how often it's
 notified of progress depending on the usage which then controls how
 quickly the system needs to handle things.

Well, what about other types of devices?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Saturday, August 20, 2011, Mark Brown wrote:
 On Sat, Aug 20, 2011 at 11:35:58AM +0200, Rafael J. Wysocki wrote:
 
  I'm not sure what you mean by exposing per-device wakeup latency 
  constraints
  to user space.  I simply thought it might be useful to allow user space to
  add and remove those, in analogy to what it can do with the currently 
  available
  PM QoS constraints.
 
 I linked to the mails from Kevin...  To be honest I'm rather surprised
 this stuff is getting exposed directly to userspace with write support.
 
  My point is that this won't prevent kernel subsystems from adding their
  own PM QoS constraints to devices and the constraints added by user space
  will only have effect if they make the devices stay active for longer
  times.  In consequence, they may only increase the energy usage of the 
  system
  and shouldn't affect functionality.
 
 Well, clearly that's not the only use - demand for increased energy
 consumption by itself from users is generally quite low :)
 
  The reason why I think this may be useful is that I can readily imagine
  scenarios in which user space (think a distribution or system vendor) will
  want to do something that hasn't been anticipated by kernel developers
  and the ability to add per-device PM QoS constraints will allow it to do 
  that
 
 Coming at this from the embedded perspective modifying the kernel just
 isn't an issue.  It's quite depressing in other cases too but some of
 the circumstances you mentioned in previous messages are sensible do
 make sense to me.  It does seem like this is a system specific problem
 which we should be able to enable as part of the support for those
 systems.

I don't think that's platform-specific in general.  For example, there are
devices that don't really belong to any media-streaming-alike framework
and we may (and probably will) want to use PM QoS with them, because they
may be included in PM domains and influence power management of other
devices.  Think of a serial console.

  without modifying the kernel.  Also, it will help to test and debug new 
  drivers
  and subsystems.
 
 If it were a debugfs facility I'd not be concerned.

If that's going to be per-device, it really is much easier to put it into
sysfs (we already have per-device PM debug interfaces in there).  It may
depend on CONFIG_PM_ADVANCED_DEBUG or something like this, though, at least
to start with.

  I don't want to prevent kernel subsystems from using their own interfaces to
  acquire user-level input that translates into PM QoS constraints.  I simply
  think it won't _hurt_ to provide user space with an additional level of
  control over per-device PM QoS constraints.
 
 My experience has been that once you start providing an interface people
 will find it, start using it and it takes a lot of work to convince them
 that they ought to do something better.  This is totally reasonable from
 when you look at things from their point of view, especially people who
 aren't used to being able to improve anything except their own driver.

There are ways to make people do right things. :-)

Anyway, if an interface is provided, people are free to use it and it
kind of is their problem if they do that for unreasonable things.  That
might be an issue for us if we were to remove that interface going forward,
but I'm not sure if that's going to ever happen.

   I'm additionally concerned that if we expose this stuff directly to 
   userspace
   that's an open invitation to driver authors to not even bother trying to 
   make
   the kernel figure this stuff out by itself and to instead tie the system
   together with magic userspace.
 
  As I said, I don't think we can prevent that from happening anyway.
  Worst case people will add strange interfaces to drivers making them
  incompatible with anything except for their crazy user space.
 
 That's not really a problem - if people are adding their own crazy
 interfaces it's clear that they've done that.  It'll show up as a red
 flag to anyone looking at their stuff and this will create pressure on
 them to fix their code or at least do a better job for the next thing.
 
 The goal isn't to tie people's hands to stop them doing silly things,
 it's to make it clear that that is what they're doing.

The same applies to using kernel interfaces.  If someone uses a sane
interface for doing crazy stuff, that's their problem and should show
up as a red flag just as well.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Saturday, August 20, 2011, Mark Brown wrote:
 On Sat, Aug 20, 2011 at 06:34:34PM +0200, Rafael J. Wysocki wrote:
  On Saturday, August 20, 2011, Mark Brown wrote:
 
   Any sort of media streaming would be an obvious example - the
   application picks the amount of data it buffers and how often it's
   notified of progress depending on the usage which then controls how
   quickly the system needs to handle things.
 
  Well, what about other types of devices?
 
 Other than the input case (which is a latency issue - there's two
 components, one is how much data is delivered for things like
 touchscreens which stream and the other is how quickly the first data is
 delivered) nothing immediately springs to mind but this may just be a
 product of what I'm most familiar with.  I don't really see this as a
 problem, for a lot of devices it's probably the case that the device can
 figure out something sensible to do without any help.

I guess you mean the driver here and I'm not really sure it can.
For instance, the driver may not know what configuration it works in,
e.g. is there a power domain or a hierarchy of those and how much time
it takes to power them all down and up and what the power break even is.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Saturday, August 20, 2011, Mark Brown wrote:
 On Sat, Aug 20, 2011 at 06:51:42PM +0200, Rafael J. Wysocki wrote:
  On Saturday, August 20, 2011, Mark Brown wrote:
 
   Coming at this from the embedded perspective modifying the kernel just
   isn't an issue.  It's quite depressing in other cases too but some of
   the circumstances you mentioned in previous messages are sensible do
   make sense to me.  It does seem like this is a system specific problem
   which we should be able to enable as part of the support for those
   systems.
 
  I don't think that's platform-specific in general.  For example, there are
  devices that don't really belong to any media-streaming-alike framework
  and we may (and probably will) want to use PM QoS with them, because they
  may be included in PM domains and influence power management of other
  devices.  Think of a serial console.
 
 Using PM QoS doesn't seem platform specific of course.  Having userspace
 need to go in and do per-device overrides in order to get things working
 does.
 
without modifying the kernel.  Also, it will help to test and debug new 
drivers
and subsystems.
 
   If it were a debugfs facility I'd not be concerned.
 
  If that's going to be per-device, it really is much easier to put it into
  sysfs (we already have per-device PM debug interfaces in there).  It may
  depend on CONFIG_PM_ADVANCED_DEBUG or something like this, though, at least
  to start with.
 
 Yeah, the debugfs device attachment stuff is slightly annoying but it's
 fairly straightforward to create an appropriate heirachy - there's
 several subsystems doing that sort of stuff by using dev_name().

I'm not a big fan of that, sorry.  Besides, as I said, we already have
debug PM interfaces in sysfs, so I don't see the reason not to add
another one.

   That's not really a problem - if people are adding their own crazy
   interfaces it's clear that they've done that.  It'll show up as a red
   flag to anyone looking at their stuff and this will create pressure on
   them to fix their code or at least do a better job for the next thing.
 
   The goal isn't to tie people's hands to stop them doing silly things,
   it's to make it clear that that is what they're doing.
 
  The same applies to using kernel interfaces.  If someone uses a sane
  interface for doing crazy stuff, that's their problem and should show
  up as a red flag just as well.
 
 The big problem I'm seeing here is that there's nothing about having
 userspace do all the knob twiddling that looks crazy just from looking
 at the system.  The controls are there and in the standard kernel
 interface, it's not like there's any ABIs or large piles of in kernel
 code that need to be added (if anything the in kernel code should look
 simpler as there's less going on).

Well, I'm kind of not seeing that as a big problem.  At least, this is not
a technical issue, but a social one.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

Hi,

I really wouldn't like the discussion to go in circles.

First, please tell me what in particular you are objecting to,
because I don't think that's any of the patches that have been
sent to the linux-pm list to date.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 0/7] PM QoS: add a per-device latency constraints framework

[Rafael J. Wysocki]

Hi,

On Wednesday, August 17, 2011, jean.pi...@newoldbits.com wrote:
 From: Jean Pihet j-pi...@ti.com
 
 High level implementation:
 
 1. Preparation of the PM QoS for the addition of a device PM QoS constraints
framework:
   . rename and move of the PM QoS implementation files to kernel/power/qos.c
 and include/linux/pm_qos.h
   . rename of API parameters and internal fields names
   . Move around the PM QoS misc devices management code for better readability
   . re-organize the internal data structs
   . generalize and export the constraints management core code
 
 2. Implementation of the per-device PM QoS constraints:
   . create drivers/base/power/qos.c for the implementation
   . create a device PM QoS API, which calls the PM QoS constraints management
 core code
   . the per-device latency constraints data strctures are stored in the device
 dev_pm_info struct
   . the device PM code calls the init and destroy of the per-device 
 constraints
 data struct in order to support the dynamic insertion and removal of the
 devices in the system.
   . to minimize the data usage by the per-device constraints, the data struct
 is only allocated at the first call to dev_pm_qos_add_request. The data
 is later free'd when the device is removed from the system
   . per-device notification callbacks can be registered and called upon a
 change to the aggregated constraint value
   . a global mutex protects the constraints users from the data being
 allocated and free'd.
 
 3. add a global notification mechanism for the device constraints
   . add a global notification chain that gets called upon changes to the
 aggregated constraint value for any device.
   . the notification callbacks are passing the full constraint request data
 in order for the callees to have access to it. The current use is for the
 platform low-level code to access the target device of the constraint
 
 
 Questions:
 1. the user space API is still under discussions on linux-omap and linux-pm 
 MLs,
cf. [1]. The idea is to add a user-space API for the devices constratins
PM QoS, using a sysfs entry per device
 
 [1] http://marc.info/?l=linux-omapm=131232344503327w=2
 
 ToDo:
 1. write Documentation for the new PM QoS class, once the code is agreed on
 2. submit patches for the OMAP low level code to control the power domains
states from the device constraints, using a global notification callback
 

 Based on the master branch of the linux-omap git tree (3.1.0-rc1). Compile
 tested using OMAP and x86 generic defconfigs.
 
 Tested on OMAP3 Beagleboard (ES2.x).
 Need testing on platforms other than OMAP, because of the impact on the
 device insertion/removal in device_pm_add/remove

OK

If that were my code, I'd probably change a couple of things more, but
it looks good enough to me to take as is.  I'm going to push this patchset
for 3.2.

Thanks a lot for your hard work on it,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/15] PM QoS: generalize and export the constraints management code

[Rafael J. Wysocki]

On Tuesday, August 16, 2011, mark gross wrote:
 On Tue, Aug 16, 2011 at 08:44:19AM +0200, Jean Pihet wrote:
  On Tue, Aug 16, 2011 at 6:08 AM, mark gross markgr...@thegnar.org wrote:
   On Sun, Aug 14, 2011 at 03:37:43PM +0200, Rafael J. Wysocki wrote:
   On Sunday, August 14, 2011, Jean Pihet wrote:
Hi Rafael, Mark,
   
On Sat, Aug 13, 2011 at 10:34 PM, Rafael J. Wysocki r...@sisk.pl 
wrote:
 On Saturday, August 13, 2011, mark gross wrote:
 On Thu, Aug 11, 2011 at 05:06:42PM +0200, jean.pi...@newoldbits.com 
 wrote:
  From: Jean Pihet j-pi...@ti.com
 
  In preparation for the per-device constratins support:
  - rename update_target to pm_qos_update_target
  - generalize and export pm_qos_update_target for usage by the 
  upcoming
  per-device latency constraints framework:
 . operate on struct pm_qos_constraints for constraints 
  management,
 . introduce an 'action' parameter for constraints 
  add/update/remove,
 . the return value indicates if the aggregated constraint 
  value has
   changed,
  - update the internal code to operate on struct pm_qos_constraints
  - add a NULL pointer check in the API functions
 
  Signed-off-by: Jean Pihet j-pi...@ti.com
...
  +/* Action requested to pm_qos_update_target */
  +enum pm_qos_req_action {
  +   PM_QOS_ADD_REQ, /* Add a new request */
  +   PM_QOS_UPDATE_REQ,  /* Update an existing request */
  +   PM_QOS_REMOVE_REQ   /* Remove an existing request */
  +};
  +

 What do you need this enum for?  The function names *_update_*, 
 *_add_*,
 and  *_remove_* seem to be pretty redundant if you have to pass an 
 enum
 that could possibly conflict with the function name.

   #ifdef CONFIG_PM
  +int pm_qos_update_target(struct pm_qos_constraints *c, struct 
  plist_node *node,
  +enum pm_qos_req_action action, int value);
 The action for update_target better damn well be 
 PM_QOS_UPDATE_REQ or
 there is something strange going on  BTW what shold this 
 function do
 if the pm_qos_req_action was *not* the UPDATE one?
   
The meaning of pm_qos_update_target is 'update the PM QoS target
constraints lists'. As described in the changelog the intention of
this patch is to implement the constraints lists management logic in
update_target and simplify the API functions (add/update/remove). It
is also exported for the upcoming (patch 06/15]) to use it as well.
  
   The enums are fine by me and they allow us to simplify the code
   quite a bit.
  
   Ok, but they look a bit sloppy to me as we now have an API that says
   add we can actually pass in an enum that says remove.
  We have an API that says 'update target' that we pass in a parameter
  that says 'add request', 'update request' or 'remove request'.
  If it is required I could just rename the internal function
  update_target, in a later patch.
 
 You are right.  I thought I saw the enum added to the other API's for
 some reason.  Still, with this update we have an API overloaded through
 that enum parameter providing 2 add, 2 delete and 2 update API's  Each
 pair doing about the same thing.
 
 To me it feels like a baby step toward an ioctl type of API that I don't
 like.  I'm not going to fight about it any more but I don't like such
 API's as they tend to get hard to read and get out of control.
 
 please rethink this a little.

For real _users_, the API is still add, update and remove,
but _internally_ those functions use the same worker routine with an
argument specifying the operation to carry out.  This reduces code
duplication quite a bit and it quite common in the kernel.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/15] PM QoS: implement the per-device PM QoS constraints

[Rafael J. Wysocki]

Hi,

On Tuesday, August 16, 2011, jean.pi...@newoldbits.com wrote:
 From: Jean Pihet j-pi...@ti.com
 
 Implement the per-device PM QoS constraints by creating a device
 PM QoS API, which calls the PM QoS constraints management core code.
 
 The per-device latency constraints data strctures are stored
 in the device dev_pm_info struct.
 
 The device PM code calls the init and destroy of the per-device constraints
 data struct in order to support the dynamic insertion and removal of the
 devices in the system.
 
 To minimize the data usage by the per-device constraints, the data struct
 is only allocated at the first call to dev_pm_qos_add_request.
 The data is later free'd when the device is removed from the system.
 A global mutex protects the constraints users from the data being
 allocated and free'd.
 
 Signed-off-by: Jean Pihet j-pi...@ti.com
 ---
  drivers/base/power/Makefile |4 +-
  drivers/base/power/main.c   |3 +
  drivers/base/power/qos.c|  291 
 +++
  include/linux/pm.h  |9 ++
  include/linux/pm_qos.h  |   42 ++
  5 files changed, 347 insertions(+), 2 deletions(-)
  create mode 100644 drivers/base/power/qos.c
 
 diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
 index 2639ae7..b707447 100644
 --- a/drivers/base/power/Makefile
 +++ b/drivers/base/power/Makefile
 @@ -1,4 +1,4 @@
 -obj-$(CONFIG_PM) += sysfs.o generic_ops.o
 +obj-$(CONFIG_PM) += sysfs.o generic_ops.o qos.o
  obj-$(CONFIG_PM_SLEEP)   += main.o wakeup.o
  obj-$(CONFIG_PM_RUNTIME) += runtime.o
  obj-$(CONFIG_PM_TRACE_RTC)   += trace.o
 @@ -6,4 +6,4 @@ obj-$(CONFIG_PM_OPP)  += opp.o
  obj-$(CONFIG_PM_GENERIC_DOMAINS) +=  domain.o
  obj-$(CONFIG_HAVE_CLK)   += clock_ops.o
  
 -ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
 \ No newline at end of file
 +ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
 diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
 index a854591..956443f 100644
 --- a/drivers/base/power/main.c
 +++ b/drivers/base/power/main.c
 @@ -22,6 +22,7 @@
  #include linux/mutex.h
  #include linux/pm.h
  #include linux/pm_runtime.h
 +#include linux/pm_qos.h
  #include linux/resume-trace.h
  #include linux/interrupt.h
  #include linux/sched.h
 @@ -97,6 +98,7 @@ void device_pm_add(struct device *dev)
   dev_name(dev-parent));
   list_add_tail(dev-power.entry, dpm_list);
   mutex_unlock(dpm_list_mtx);
 + dev_pm_qos_constraints_init(dev);
  }
  
  /**
 @@ -107,6 +109,7 @@ void device_pm_remove(struct device *dev)
  {
   pr_debug(PM: Removing info for %s:%s\n,
dev-bus ? dev-bus-name : No Bus, dev_name(dev));
 + dev_pm_qos_constraints_destroy(dev);
   complete_all(dev-power.completion);
   mutex_lock(dpm_list_mtx);
   list_del_init(dev-power.entry);
 diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
 new file mode 100644
 index 000..304d68d
 --- /dev/null
 +++ b/drivers/base/power/qos.c
 @@ -0,0 +1,291 @@
 +/*

Please add a copyright notice.

 + * This module exposes the interface to kernel space for specifying
 + * per-device PM QoS dependencies. It provides infrastructure for 
 registration
 + * of:
 + *
 + * Dependents on a QoS value : register requests
 + * Watchers of QoS value : get notified when target QoS value changes
 + *
 + * This QoS design is best effort based. Dependents register their QoS needs.
 + * Watchers register to keep track of the current QoS needs of the system.
 + *
 + * Note about the per-device constraint data struct allocation:
 + * . The per-device constraints data struct ptr is tored into the device
 + *dev_pm_info.
 + * . To minimize the data usage by the per-device constraints, the data 
 struct
 + *   is only allocated at the first call to dev_pm_qos_add_request.
 + * . The data is later free'd when the device is removed from the system.
 + * . The constraints_state variable from dev_pm_info tracks the data struct
 + *allocation state:
 + *DEV_PM_QOS_NO_DEVICE: No device present or device removed, no data
 + * allocated,
 + *DEV_PM_QOS_DEVICE_PRESENT: Device present, data not allocated and will 
 be
 + * allocated at the first call to dev_pm_qos_add_request,
 + *DEV_PM_QOS_ALLOCATED: Device present, data allocated. The per-device
 + * PM QoS constraints framework is operational and constraints can be
 + * added, updated or removed using the dev_pm_qos_* API.
 + *  . A global mutex protects the constraints users from the data being
 + * allocated and free'd.
 + */
 +
 +#include linux/pm_qos.h
 +#include linux/spinlock.h
 +#include linux/slab.h
 +#include linux/device.h
 +#include linux/mutex.h
 +
 +
 +static DEFINE_MUTEX(dev_pm_qos_mtx);
 +static void dev_pm_qos_constraints_allocate(struct device *dev);

This header wouldn't be necessary if you put the definition of
dev_pm_qos_constraints_allocate() before dev_pm_qos_add_request().

 +
 +/**
 + * 

Re: [PATCH 07/15] PM QoS: add a global notification mechanism for the device constraints

[Rafael J. Wysocki]

Hi,

On Tuesday, August 16, 2011, jean.pi...@newoldbits.com wrote:
 From: Jean Pihet j-pi...@ti.com
 
 Add a global notification chain that gets called upon changes to the
 aggregated constraint value for any device.
 The notification callbacks are passing the full constraint request data
 in order for the callees to have access to it. The current use is for the
 platform low-level code to access the target device of the constraint.
 
 Signed-off-by: Jean Pihet j-pi...@ti.com
 ---
  drivers/base/power/qos.c |   84 
 --
  include/linux/pm_qos.h   |   11 ++
  kernel/power/qos.c   |2 +-
  3 files changed, 78 insertions(+), 19 deletions(-)
 
 diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
 index 304d68d..b52b3e8 100644
 --- a/drivers/base/power/qos.c
 +++ b/drivers/base/power/qos.c
 @@ -8,6 +8,12 @@
   *
   * This QoS design is best effort based. Dependents register their QoS needs.
   * Watchers register to keep track of the current QoS needs of the system.
 + * Watchers can register different types of notification callbacks:
 + *  . a per-device notification callback using the dev_pm_qos_*_notifier API.
 + *The notification chain data is stored in the per-device constraint
 + *data struct.
 + *  . a system-wide notification callback using the 
 dev_pm_qos_*_global_notifier
 + *API. The notification chain data is stored in a static variable.
   *
   * Note about the per-device constraint data struct allocation:
   * . The per-device constraints data struct ptr is tored into the device
 @@ -36,8 +42,32 @@
  
  
  static DEFINE_MUTEX(dev_pm_qos_mtx);
 +static BLOCKING_NOTIFIER_HEAD(dev_pm_notifiers);
  static void dev_pm_qos_constraints_allocate(struct device *dev);
  
 +/*
 + * Update the constraints list using the PM QoS core code and
 + * if needed call the per-device and the global notification callbacks
 + */

Well, if you add kerneldoc comments, please make them follow the standard,
even if that's a static function.

 +static int _apply_constraint(struct dev_pm_qos_request *req,

The initial underscore looks odd and is not really necessary.  Please drop it.

The rest of the patch looks fine.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/15] PM QoS: re-organize data structs

[Rafael J. Wysocki]

On Sunday, August 14, 2011, Jean Pihet wrote:
 Rafael, Mark,
 
 On Sat, Aug 13, 2011 at 10:58 PM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Saturday, August 13, 2011, mark gross wrote:
  On Thu, Aug 11, 2011 at 05:06:41PM +0200, jean.pi...@newoldbits.com wrote:
   From: Jean Pihet j-pi...@ti.com
  
   In preparation for the per-device constratins support, re-organize
   the data strctures:
   - add a struct pm_qos_constraints which contains the constraints
   related data
   - update struct pm_qos_object contents to the PM QoS internal object
   data. Add a pointer to struct pm_qos_constraints
   - update the internal code to use the new data structs.
  
   Signed-off-by: Jean Pihet j-pi...@ti.com
   ---
include/linux/pm_qos.h |   19 ++
kernel/power/qos.c |   90 
   ++-
2 files changed, 61 insertions(+), 48 deletions(-)
  
   diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
   index 6b0968f..9772311 100644
   --- a/include/linux/pm_qos.h
   +++ b/include/linux/pm_qos.h
   @@ -25,6 +25,25 @@ struct pm_qos_request {
   int pm_qos_class;
};
  
   +enum pm_qos_type {
   +   PM_QOS_UNITIALIZED,
  what is this for?
 
  I seem to remember discussing that previously, but I can't recall what
  it's for.  Jean?
 
 
 Sorry it is a left over from the previous version, it has been used to
 detect non initialized data structs.
 It is still used to detect an error in pm_qos_get_value and so by its
 callers pm_qos_update_target and pm_qos_power_read. I have to admit
 the usefulness is quite limited.
 
 Is the removal of PM_QOS_UNITIALIZED needed? I would say no.

I agree.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/15] PM QoS: generalize and export the constraints management code

[Rafael J. Wysocki]

On Sunday, August 14, 2011, Jean Pihet wrote:
 Hi Rafael, Mark,
 
 On Sat, Aug 13, 2011 at 10:34 PM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Saturday, August 13, 2011, mark gross wrote:
  On Thu, Aug 11, 2011 at 05:06:42PM +0200, jean.pi...@newoldbits.com wrote:
   From: Jean Pihet j-pi...@ti.com
  
   In preparation for the per-device constratins support:
   - rename update_target to pm_qos_update_target
   - generalize and export pm_qos_update_target for usage by the upcoming
   per-device latency constraints framework:
  . operate on struct pm_qos_constraints for constraints management,
  . introduce an 'action' parameter for constraints add/update/remove,
  . the return value indicates if the aggregated constraint value has
changed,
   - update the internal code to operate on struct pm_qos_constraints
   - add a NULL pointer check in the API functions
  
   Signed-off-by: Jean Pihet j-pi...@ti.com
 ...
   +/* Action requested to pm_qos_update_target */
   +enum pm_qos_req_action {
   +   PM_QOS_ADD_REQ, /* Add a new request */
   +   PM_QOS_UPDATE_REQ,  /* Update an existing request */
   +   PM_QOS_REMOVE_REQ   /* Remove an existing request */
   +};
   +
 
  What do you need this enum for?  The function names *_update_*, *_add_*,
  and  *_remove_* seem to be pretty redundant if you have to pass an enum
  that could possibly conflict with the function name.
 
#ifdef CONFIG_PM
   +int pm_qos_update_target(struct pm_qos_constraints *c, struct 
   plist_node *node,
   +enum pm_qos_req_action action, int value);
  The action for update_target better damn well be PM_QOS_UPDATE_REQ or
  there is something strange going on  BTW what shold this function do
  if the pm_qos_req_action was *not* the UPDATE one?
 
 The meaning of pm_qos_update_target is 'update the PM QoS target
 constraints lists'. As described in the changelog the intention of
 this patch is to implement the constraints lists management logic in
 update_target and simplify the API functions (add/update/remove). It
 is also exported for the upcoming (patch 06/15]) to use it as well.

The enums are fine by me and they allow us to simplify the code
quite a bit.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/15] PM QoS: implement the per-device PM QoS constraints

[Rafael J. Wysocki]

Hi,

On Sunday, August 14, 2011, Jean Pihet wrote:
...
  +
  + if (dev_pm_qos_request_active(req)) {
  + WARN(1, KERN_ERR dev_pm_qos_add_request() called for 
  already 
  + added request\n);
  + return;
  + }
  + req-dev = dev;
  +
  + /* Allocate the constraints struct on the first call to add_request 
  */
  + if (req-dev-power.constraints_state == DEV_PM_QOS_DEVICE_PRESENT)
  + dev_pm_qos_constraints_allocate(dev);
 
  Why not to do
 
  +   if (!req-dev-power.constraints)
  +   dev_pm_qos_constraints_allocate(dev);
 
 Cf. my comments at the end of this message for the data structs
 alloc/free and the locking.
 
 
  +
  + /* Silently return if the device has been removed */
  + if (req-dev-power.constraints_state != DEV_PM_QOS_ALLOCATED)
  + return;
  +
 
  Hmm.  What will happen if two callers run dev_pm_qos_add_request()
  concurrently for the same device?
 update_target is using the power.lock to protect the constraints lists 
 changes.

I was referring to the scenario in particular:

Suppose that there are no constraits for dev initially.
A - calls dev_pm_qos_add_request(dev)
B - calls dev_pm_qos_add_request(dev)
A - sees power.constraints_state == DEV_PM_QOS_DEVICE_PRESENT
 and calls dev_pm_qos_constraints_allocate(dev)
B - sees power.constraints_state == DEV_PM_QOS_DEVICE_PRESENT
 and calls dev_pm_qos_constraints_allocate(dev)

As a result, the structure allocated by A is leaked.

...
 
 Your remarks are inline with the concerns I had about the adata
 structs alloc/free and the locking.
 
 1) data structs alloc/free
 As described in the changelog:
  To minimize the data usage by the per-device constraints, the data struct
  is only allocated at the first call to dev_pm_qos_add_request.
  The data is later free'd when the device is removed from the system.
 A basic state machine is needed in order to allocate the data at the
 first call to add_request and free it when the device is removed.
 Another option is to allocate the data when the device is added to the
 system and free it when the device is removed. That would make the
 code simpler but it is using a lot of memory for unneeded data.

That's fine.  You simply need to be more careful about the possible
race conditions when the constraints objects are created and destroyed.

 2) Race conditions
 I will add a lock to isolate the API callers from
 dev_pm_qos_constraints_destroy().
 The power.lock is already used by update_target so another lock is
 needed to protect the data allocation/free.

OK

 I will rework this tomorrow and re-submit asap when it is ready.
 Is that OK?

Yes, it is.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 00/15] PM QoS: add a per-device latency constraints class

[Rafael J. Wysocki]

On Sunday, August 14, 2011, Jean Pihet wrote:
 Rafael,
 
 On Fri, Aug 12, 2011 at 11:56 PM, Rafael J. Wysocki r...@sisk.pl wrote:
  Hi,
 
  On Friday, August 12, 2011, Jean Pihet wrote:
  Hi Rafael,
 
  2011/8/12 Rafael J. Wysocki r...@sisk.pl:
   On Thursday, August 11, 2011, jean.pi...@newoldbits.com wrote:
   From: Jean Pihet j-pi...@ti.com
  
   This patch set is in an RFC state, for review and comments.
  
  ...
  
  
   Questions:
   1. the user space API is still under discussions on linux-omap and 
   linux-pm MLs,
  cf. [1]. The idea is to add a user-space API for the devices 
   constratins
  PM QoS, using a sysfs entry per device
  
   [1] http://marc.info/?l=linux-omapm=131232344503327w=2
  
   ToDo:
   1. write Documentation for the new PM QoS class, once the RFC is agreed 
   on
   2. validate the constraints framework on OMAP4 HW (done on OMAP3)
   3. Need testing on platforms other than OMAP
   4. refine the power domains wake-up latency and the cpuidle figures
   5. re-visit the OMAP power domains states initialization procedure. 
   Currently
  the power states that have been changed from the constraints API 
   which were
  applied before the initialization of the power domains are lost
  
  
   Based on the master branch of the linux-omap git tree (3.0.0-rc7). 
   Compile
   tested using OMAP and x86 generic defconfigs.
  
   Lightly tested on OMAP3 Beagleboard (ES2.x).
   Need testing on platforms other than OMAP, because of the impact on the
   device insertion/removal in device_pm_add/remove
  
   The patchset looks really good to me, I don't think I have any major
   complaints about this version.
  Ok good to hear it! I tried to address all comments and concerns in
  this release.
 
  
   The only thing I'd like to ask at the moment is whether or not the
   compilation of drivers/base/power/qos.c should depend on
   CONFIG_PM_RUNTIME.  Do you think it will be used by system suspend code 
   on any
   platforms?
  I would say it should only depend on CONFIG_PM because the dev PM QoS
  API can be used from any kernel code, being runtime PM code or not.
  I leave the decision to the PM framework experts.
 
  
   Also, I'd like to take the final patchset for 3.2,
  Ok good!
 
   but I don't feel
   confident enough about the OMAP patches.
  The OMAP patches have been reviewed a few times already and the
  comments have been taken into account. Also i has been tested
  correctly on OMAP3.
 
   If you want me to take them too,
   please make sure they are ACKed by the OMAP maintainers.
  For sure I need the Acks. I guess I now need to annoy OMAP folks about it 
  ;p
  In the case the Acks are not gathered on time the generic patches
  could be merged in, then the OMAP generic code. Do you think it is a
  viable option?
 
  Yes, it is.  I can take patches [1-7/15] alone.
 
  The only concern I have is about the on-going OMAP PM initialization
  clean-up task, cf. ToDo list:
 5. re-visit the OMAP power domains states initialization
  procedure. Currently
the power states that have been changed from the constraints
  API which were
applied before the initialization of the power domains are lost
 
  On the other hand some testing is needed on platforms other than OMAP,
  because of the impact on the device insertion/removal in
  device_pm_add/remove functions. I tested the SD card insertion/removal
  on OMAP3.
 
  OK, so are you going to make any more changes to patches [1-7/15]?
 I am now reworking [06/15] after your comments.
 Is that OK timewise?

It should be fine.  I still need to have a closer look at [7/15] later today,
I'll let you know if there's anything I'd like to change in there.

Thanks,
Rafael


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

Re: [PATCH 07/15] PM QoS: add a global notification mechanism for the device constraints

[Rafael J. Wysocki]

Hi,

There is some code duplication in this patch that should better be
avoided (details below).

On Thursday, August 11, 2011, jean.pi...@newoldbits.com wrote:
 From: Jean Pihet j-pi...@ti.com
 
 Add a global notification chain that gets called upon changes to the
 aggregated constraint value for any device.
 The notification callbacks are passing the full constraint request data
 in order for the callees to have access to it. The current use is for the
 platform low-level code to access the target device of the constraint.
 
 Signed-off-by: Jean Pihet j-pi...@ti.com
 ---
  drivers/base/power/qos.c |  114 -
  include/linux/pm_qos.h   |   11 
  kernel/power/qos.c   |2 +-
  3 files changed, 113 insertions(+), 14 deletions(-)
 
 diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
 index 465e419..4b0b316 100644
 --- a/drivers/base/power/qos.c
 +++ b/drivers/base/power/qos.c
 @@ -8,6 +8,12 @@
   *
   * This QoS design is best effort based. Dependents register their QoS needs.
   * Watchers register to keep track of the current QoS needs of the system.
 + * Watchers can register different types of notification callbacks:
 + *  . a per-device notification callback using the dev_pm_qos_*_notifier API.
 + *The notification chain data is stored in the per-device constraint
 + *data struct.
 + *  . a system-wide notification callback using the 
 dev_pm_qos_*_global_notifier
 + *API. The notification chain data is stored in a static variable.
   *
   * Note about the per-device constraint data struct allocation:
   * . The per-device constraints data struct ptr is tored into the device
 @@ -41,6 +47,7 @@
  #include linux/kernel.h
  
  
 +static BLOCKING_NOTIFIER_HEAD(dev_pm_notifiers);
  static void dev_pm_qos_constraints_allocate(struct device *dev);
  
  int dev_pm_qos_request_active(struct dev_pm_qos_request *req)
 @@ -64,6 +71,8 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_request_active);
  void dev_pm_qos_add_request(struct dev_pm_qos_request *req, struct device 
 *dev,
   s32 value)
  {
 + int ret, curr_value;
 +
   if (!req) /*guard against callers passing in null */
   return;
  
 @@ -82,8 +91,19 @@ void dev_pm_qos_add_request(struct dev_pm_qos_request 
 *req, struct device *dev,
   if (req-dev-power.constraints_state != DEV_PM_QOS_ALLOCATED)
   return;
  
 - pm_qos_update_target(dev-power.constraints,
 -  req-node, PM_QOS_ADD_REQ, value);

The following code:

 + /*
 +  * Update constraints list and call the per-device callbacks if needed
 +  */
 + ret = pm_qos_update_target(dev-power.constraints,
 +req-node, PM_QOS_ADD_REQ, value);
 +
 + if (ret) {
 + /* Call the global callbacks if needed */
 + curr_value = pm_qos_read_value(req-dev-power.constraints);
 + blocking_notifier_call_chain(dev_pm_notifiers,
 +  (unsigned long)curr_value,
 +  req);
 + }

is used in dev_pm_qos_update_request() and dev_pm_qos_remove_request()
with the only difference being the command given to pm_qos_update_target().
This asks for a common function, eg. dev_pm_qos_update_target(), containing
that code that will be called by all of them (and, apparently, by
dev_pm_qos_constraints_destroy()).

...
 @@ -250,9 +329,18 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
* Update constraints list and call the per-device
* callbacks if needed
*/
 - pm_qos_update_target(req-dev-power.constraints,
 -  req-node, PM_QOS_REMOVE_REQ,
 -  PM_QOS_DEFAULT_VALUE);
 + ret |= pm_qos_update_target(req-dev-power.constraints,
 + req-node,
 + PM_QOS_REMOVE_REQ,
 + PM_QOS_DEFAULT_VALUE);

I'm not sure why you're using the binary or operator here.  Shouldn't that be
a simple assignment?

 +
 + if (ret) {
 + /* Call the global callbacks if needed */
 + curr_value = dev-power.constraints-default_value;
 + blocking_notifier_call_chain(dev_pm_notifiers,
 +  (unsigned long)curr_value,
 +  req);
 + }
  
   kfree(dev-power.constraints-notifiers);
   kfree(dev-power.constraints);

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/15] PM QoS: generalize and export the constraints management code

[Rafael J. Wysocki]

On Saturday, August 13, 2011, mark gross wrote:
 On Thu, Aug 11, 2011 at 05:06:42PM +0200, jean.pi...@newoldbits.com wrote:
  From: Jean Pihet j-pi...@ti.com
  
  In preparation for the per-device constratins support:
  - rename update_target to pm_qos_update_target
  - generalize and export pm_qos_update_target for usage by the upcoming
  per-device latency constraints framework:
 . operate on struct pm_qos_constraints for constraints management,
 . introduce an 'action' parameter for constraints add/update/remove,
 . the return value indicates if the aggregated constraint value has
   changed,
  - update the internal code to operate on struct pm_qos_constraints
  - add a NULL pointer check in the API functions
  
  Signed-off-by: Jean Pihet j-pi...@ti.com
  
  ---
   include/linux/pm_qos.h |   14 ++
   kernel/power/qos.c |  123 
  ++--
   2 files changed, 81 insertions(+), 56 deletions(-)
  
  diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
  index 9772311..84aa150 100644
  --- a/include/linux/pm_qos.h
  +++ b/include/linux/pm_qos.h
  @@ -44,7 +44,16 @@ struct pm_qos_constraints {
  struct blocking_notifier_head *notifiers;
   };
   
  +/* Action requested to pm_qos_update_target */
  +enum pm_qos_req_action {
  +   PM_QOS_ADD_REQ, /* Add a new request */
  +   PM_QOS_UPDATE_REQ,  /* Update an existing request */
  +   PM_QOS_REMOVE_REQ   /* Remove an existing request */
  +};
  +
 
 What do you need this enum for?  The function names *_update_*, *_add_*,
 and  *_remove_* seem to be pretty redundant if you have to pass an enum
 that could possibly conflict with the function name.
 
   #ifdef CONFIG_PM
  +int pm_qos_update_target(struct pm_qos_constraints *c, struct plist_node 
  *node,
  +enum pm_qos_req_action action, int value);
 The action for update_target better damn well be PM_QOS_UPDATE_REQ or
 there is something strange going on  BTW what shold this function do
 if the pm_qos_req_action was *not* the UPDATE one?
 
 
 pm_qos_update_target should be a static to the C- file along with the
 enum pm_qos_req_action.
 
 
   void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
  s32 value);
   void pm_qos_update_request(struct pm_qos_request *req,
  @@ -56,6 +65,11 @@ int pm_qos_add_notifier(int pm_qos_class, struct 
  notifier_block *notifier);
   int pm_qos_remove_notifier(int pm_qos_class, struct notifier_block 
  *notifier);
   int pm_qos_request_active(struct pm_qos_request *req);
   #else
  +static inline int pm_qos_update_target(struct pm_qos_constraints *c,
  +  struct plist_node *node,
  +  enum pm_qos_req_action action,
  +  int value)
  +   { return 0; }
   static inline void pm_qos_add_request(struct pm_qos_request *req,
int pm_qos_class, s32 value)
  { return; }
  diff --git a/kernel/power/qos.c b/kernel/power/qos.c
  index 66e8d6f..fc60f96 100644
  --- a/kernel/power/qos.c
  +++ b/kernel/power/qos.c
  @@ -121,17 +121,17 @@ static const struct file_operations pm_qos_power_fops 
  = {
   };
   
   /* unlocked internal variant */
  -static inline int pm_qos_get_value(struct pm_qos_object *o)
  +static inline int pm_qos_get_value(struct pm_qos_constraints *c)
   {
  -   if (plist_head_empty(o-constraints-list))
  -   return o-constraints-default_value;
  +   if (plist_head_empty(c-list))
  +   return c-default_value;
   
  -   switch (o-constraints-type) {
  +   switch (c-type) {
  case PM_QOS_MIN:
  -   return plist_first(o-constraints-list)-prio;
  +   return plist_first(c-list)-prio;
   
  case PM_QOS_MAX:
  -   return plist_last(o-constraints-list)-prio;
  +   return plist_last(c-list)-prio;
   
  default:
  /* runtime check for not using enum */
  @@ -139,47 +139,73 @@ static inline int pm_qos_get_value(struct 
  pm_qos_object *o)
  }
   }
   
  -static inline s32 pm_qos_read_value(struct pm_qos_object *o)
  +static inline s32 pm_qos_read_value(struct pm_qos_constraints *c)
   {
  -   return o-constraints-target_value;
  +   return c-target_value;
   }
   
  -static inline void pm_qos_set_value(struct pm_qos_object *o, s32 value)
  +static inline void pm_qos_set_value(struct pm_qos_constraints *c, s32 
  value)
   {
  -   o-constraints-target_value = value;
  +   c-target_value = value;
   }
   
  -static void update_target(struct pm_qos_object *o, struct plist_node *node,
  - int del, int value)
  +/**
  + * pm_qos_update_target - manages the constraints list and calls the 
  notifiers
  + *  if needed
  + * @c: constraints data struct
  + * @node: request to add to the list, to update or to remove
  + * @action: action to take on the constraints list
  + * @value: 

Re: [PATCH 04/15] PM QoS: re-organize data structs

[Rafael J. Wysocki]

On Saturday, August 13, 2011, mark gross wrote:
 On Thu, Aug 11, 2011 at 05:06:41PM +0200, jean.pi...@newoldbits.com wrote:
  From: Jean Pihet j-pi...@ti.com
  
  In preparation for the per-device constratins support, re-organize
  the data strctures:
  - add a struct pm_qos_constraints which contains the constraints
  related data
  - update struct pm_qos_object contents to the PM QoS internal object
  data. Add a pointer to struct pm_qos_constraints
  - update the internal code to use the new data structs.
  
  Signed-off-by: Jean Pihet j-pi...@ti.com
  ---
   include/linux/pm_qos.h |   19 ++
   kernel/power/qos.c |   90 
  ++-
   2 files changed, 61 insertions(+), 48 deletions(-)
  
  diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
  index 6b0968f..9772311 100644
  --- a/include/linux/pm_qos.h
  +++ b/include/linux/pm_qos.h
  @@ -25,6 +25,25 @@ struct pm_qos_request {
  int pm_qos_class;
   };
   
  +enum pm_qos_type {
  +   PM_QOS_UNITIALIZED,
 what is this for?

I seem to remember discussing that previously, but I can't recall what
it's for.  Jean?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/15] PM QoS: implement the per-device PM QoS constraints

[Rafael J. Wysocki]

Hi,

Well, it looks like I should have reviewed this one more carefully.

On Thursday, August 11, 2011, jean.pi...@newoldbits.com wrote:
 From: Jean Pihet j-pi...@ti.com
 
 Implement the per-device PM QoS constraints by creating a device
 PM QoS API, which calls the PM QoS constraints management core code.
 
 The per-device latency constraints data strctures are stored
 in the device dev_pm_info struct.
 
 The device PM code calls the init and destroy of the per-device constraints
 data struct in order to support the dynamic insertion and removal of the
 devices in the system.
 
 To minimize the data usage by the per-device constraints, the data struct
 is only allocated at the first call to dev_pm_qos_add_request.
 The data is later free'd when the device is removed from the system.
 
 Signed-off-by: Jean Pihet j-pi...@ti.com
 ---
  drivers/base/power/Makefile |4 +-
  drivers/base/power/main.c   |3 +
  drivers/base/power/qos.c|  262 
 +++
  include/linux/pm.h  |9 ++
  include/linux/pm_qos.h  |   39 +++
  5 files changed, 315 insertions(+), 2 deletions(-)
  create mode 100644 drivers/base/power/qos.c
 
 diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
 index 3647e11..1a61f89 100644
 --- a/drivers/base/power/Makefile
 +++ b/drivers/base/power/Makefile
 @@ -1,8 +1,8 @@
 -obj-$(CONFIG_PM) += sysfs.o generic_ops.o
 +obj-$(CONFIG_PM) += sysfs.o generic_ops.o qos.o
  obj-$(CONFIG_PM_SLEEP)   += main.o wakeup.o
  obj-$(CONFIG_PM_RUNTIME) += runtime.o
  obj-$(CONFIG_PM_TRACE_RTC)   += trace.o
  obj-$(CONFIG_PM_OPP) += opp.o
  obj-$(CONFIG_HAVE_CLK)   += clock_ops.o
  
 -ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
 \ No newline at end of file
 +ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
 diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
 index 06f09bf..f5c0e0e 100644
 --- a/drivers/base/power/main.c
 +++ b/drivers/base/power/main.c
 @@ -22,6 +22,7 @@
  #include linux/mutex.h
  #include linux/pm.h
  #include linux/pm_runtime.h
 +#include linux/pm_qos.h
  #include linux/resume-trace.h
  #include linux/interrupt.h
  #include linux/sched.h
 @@ -97,6 +98,7 @@ void device_pm_add(struct device *dev)
   dev_name(dev-parent));
   list_add_tail(dev-power.entry, dpm_list);
   mutex_unlock(dpm_list_mtx);
 + dev_pm_qos_constraints_init(dev);
  }
  
  /**
 @@ -107,6 +109,7 @@ void device_pm_remove(struct device *dev)
  {
   pr_debug(PM: Removing info for %s:%s\n,
dev-bus ? dev-bus-name : No Bus, dev_name(dev));
 + dev_pm_qos_constraints_destroy(dev);
   complete_all(dev-power.completion);
   mutex_lock(dpm_list_mtx);
   list_del_init(dev-power.entry);
 diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
 new file mode 100644
 index 000..465e419
 --- /dev/null
 +++ b/drivers/base/power/qos.c
 @@ -0,0 +1,262 @@
 +/*
 + * This module exposes the interface to kernel space for specifying
 + * per-device PM QoS dependencies. It provides infrastructure for 
 registration
 + * of:
 + *
 + * Dependents on a QoS value : register requests
 + * Watchers of QoS value : get notified when target QoS value changes
 + *
 + * This QoS design is best effort based. Dependents register their QoS needs.
 + * Watchers register to keep track of the current QoS needs of the system.
 + *
 + * Note about the per-device constraint data struct allocation:
 + * . The per-device constraints data struct ptr is tored into the device
 + *dev_pm_info.
 + * . To minimize the data usage by the per-device constraints, the data 
 struct
 + *   is only allocated at the first call to dev_pm_qos_add_request.
 + * . The data is later free'd when the device is removed from the system.
 + * . The constraints_state variable from dev_pm_info tracks the data struct
 + *allocation state:
 + *DEV_PM_QOS_NO_DEVICE: No device present or device removed, no data
 + * allocated,
 + *DEV_PM_QOS_DEVICE_PRESENT: Device present, data not allocated and will 
 be
 + * allocated at the first call to dev_pm_qos_add_request,
 + *DEV_PM_QOS_ALLOCATED: Device present, data allocated. The per-device
 + * PM QoS constraints framework is operational and constraints can be
 + * added, updated or removed using the dev_pm_qos_* API.
 + */
 +
 +/*#define DEBUG*/

Please remove this.

 +
 +#include linux/pm_qos.h
 +#include linux/sched.h
 +#include linux/spinlock.h
 +#include linux/slab.h
 +#include linux/time.h
 +#include linux/fs.h
 +#include linux/device.h
 +#include linux/string.h
 +#include linux/platform_device.h
 +#include linux/init.h
 +#include linux/kernel.h

Are you sure all of the headers are necessary?

 +
 +
 +static void dev_pm_qos_constraints_allocate(struct device *dev);
 +
 +int dev_pm_qos_request_active(struct dev_pm_qos_request *req)
 +{
 + return req-dev != 0;
 +}
 +EXPORT_SYMBOL_GPL(dev_pm_qos_request_active);

That should be a 

Re: [PATCH v4 00/15] PM QoS: add a per-device latency constraints class

[Rafael J. Wysocki]

On Thursday, August 11, 2011, jean.pi...@newoldbits.com wrote:
 From: Jean Pihet j-pi...@ti.com
 
 This patch set is in an RFC state, for review and comments.
 
 High level implementation:
 
 1. Preparation of the PM QoS for the addition of a device PM QoS constraints
framework:
   . rename and move of the PM QoS implementation files to kernel/power/qos.c
 and include/linux/pm_qos.h
   . rename of API parameters and internal fields names
   . Move around the PM QoS misc devices management code for better readability
   . re-organize the internal data structs
   . generalize and export the constraints management core code
 
 2. Implementation of the per-device PM QoS constraints:
   . create drivers/base/power/qos.c for the implementation
   . create a device PM QoS API, which calls the PM QoS constraints management
 core code
   . the per-device latency constraints data strctures are stored in the device
 dev_pm_info struct
   . the device PM code calls the init and destroy of the per-device 
 constraints
 data struct in order to support the dynamic insertion and removal of the
 devices in the system.
   . to minimize the data usage by the per-device constraints, the data struct
 is only allocated at the first call to dev_pm_qos_add_request. The data
 is later free'd when the device is removed from the system
   . per-device notification callbacks can be registered and called upon a
 change to the aggregated constraint value
 
 3. add a global notification mechanism for the device constraints
   . add a global notification chain that gets called upon changes to the
 aggregated constraint value for any device.
   . the notification callbacks are passing the full constraint request data
 in order for the callees to have access to it. The current use is for the
 platform low-level code to access the target device of the constraint
 
 4. Implement the OMAP low level constraints management code
   . create a PM layer plugin for per-device constraints, compiled under
 CONFIG_OMAP_PM_CONSTRAINTS=y
   . implement the devices wake-up latency constraints using the global
 device PM QoS notification handler which applies the constraints to the
 underlying layer
   . implement the low level code which controls the power domains next power
 states, through the hwmod and pwrdm layers
   . add (preliminary) power domains wake-up latency figures for OMAP34
   . cpuidle is a CPU centric framework so it decides the MPU next power state
 based on the MPU exit_latency and target_residency figures. The rest of
 the power domains get their next power state programmed from the devices
 PM QoS framework, via the devices wake-up latency constraints.
   . convert the OMAP I2C driver to the PM QoS API for MPU latency constraints
 
 
 Questions:
 1. the user space API is still under discussions on linux-omap and linux-pm 
 MLs,
cf. [1]. The idea is to add a user-space API for the devices constratins
PM QoS, using a sysfs entry per device
 
 [1] http://marc.info/?l=linux-omapm=131232344503327w=2
 
 ToDo:
 1. write Documentation for the new PM QoS class, once the RFC is agreed on
 2. validate the constraints framework on OMAP4 HW (done on OMAP3)
 3. Need testing on platforms other than OMAP
 4. refine the power domains wake-up latency and the cpuidle figures
 5. re-visit the OMAP power domains states initialization procedure. Currently
the power states that have been changed from the constraints API which were
applied before the initialization of the power domains are lost
 

 Based on the master branch of the linux-omap git tree (3.0.0-rc7). Compile
 tested using OMAP and x86 generic defconfigs.
 
 Lightly tested on OMAP3 Beagleboard (ES2.x).
 Need testing on platforms other than OMAP, because of the impact on the
 device insertion/removal in device_pm_add/remove

The patchset looks really good to me, I don't think I have any major
complaints about this version.

The only thing I'd like to ask at the moment is whether or not the
compilation of drivers/base/power/qos.c should depend on
CONFIG_PM_RUNTIME.  Do you think it will be used by system suspend code on any
platforms?

Also, I'd like to take the final patchset for 3.2, but I don't feel
confident enough about the OMAP patches.  If you want me to take them too,
please make sure they are ACKed by the OMAP maintainers.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 00/15] PM QoS: add a per-device latency constraints class

[Rafael J. Wysocki]

Hi,

On Friday, August 12, 2011, Jean Pihet wrote:
 Hi Rafael,
 
 2011/8/12 Rafael J. Wysocki r...@sisk.pl:
  On Thursday, August 11, 2011, jean.pi...@newoldbits.com wrote:
  From: Jean Pihet j-pi...@ti.com
 
  This patch set is in an RFC state, for review and comments.
 
 ...
 
 
  Questions:
  1. the user space API is still under discussions on linux-omap and 
  linux-pm MLs,
 cf. [1]. The idea is to add a user-space API for the devices constratins
 PM QoS, using a sysfs entry per device
 
  [1] http://marc.info/?l=linux-omapm=131232344503327w=2
 
  ToDo:
  1. write Documentation for the new PM QoS class, once the RFC is agreed on
  2. validate the constraints framework on OMAP4 HW (done on OMAP3)
  3. Need testing on platforms other than OMAP
  4. refine the power domains wake-up latency and the cpuidle figures
  5. re-visit the OMAP power domains states initialization procedure. 
  Currently
 the power states that have been changed from the constraints API which 
  were
 applied before the initialization of the power domains are lost
 
 
  Based on the master branch of the linux-omap git tree (3.0.0-rc7). Compile
  tested using OMAP and x86 generic defconfigs.
 
  Lightly tested on OMAP3 Beagleboard (ES2.x).
  Need testing on platforms other than OMAP, because of the impact on the
  device insertion/removal in device_pm_add/remove
 
  The patchset looks really good to me, I don't think I have any major
  complaints about this version.
 Ok good to hear it! I tried to address all comments and concerns in
 this release.
 
 
  The only thing I'd like to ask at the moment is whether or not the
  compilation of drivers/base/power/qos.c should depend on
  CONFIG_PM_RUNTIME.  Do you think it will be used by system suspend code on 
  any
  platforms?
 I would say it should only depend on CONFIG_PM because the dev PM QoS
 API can be used from any kernel code, being runtime PM code or not.
 I leave the decision to the PM framework experts.
 
 
  Also, I'd like to take the final patchset for 3.2,
 Ok good!
 
  but I don't feel
  confident enough about the OMAP patches.
 The OMAP patches have been reviewed a few times already and the
 comments have been taken into account. Also i has been tested
 correctly on OMAP3.
 
  If you want me to take them too,
  please make sure they are ACKed by the OMAP maintainers.
 For sure I need the Acks. I guess I now need to annoy OMAP folks about it ;p
 In the case the Acks are not gathered on time the generic patches
 could be merged in, then the OMAP generic code. Do you think it is a
 viable option?

Yes, it is.  I can take patches [1-7/15] alone.

 The only concern I have is about the on-going OMAP PM initialization
 clean-up task, cf. ToDo list:
5. re-visit the OMAP power domains states initialization
 procedure. Currently
   the power states that have been changed from the constraints
 API which were
   applied before the initialization of the power domains are lost
 
 On the other hand some testing is needed on platforms other than OMAP,
 because of the impact on the device insertion/removal in
 device_pm_add/remove functions. I tested the SD card insertion/removal
 on OMAP3.

OK, so are you going to make any more changes to patches [1-7/15]?

Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Sunday, August 07, 2011, Mark Brown wrote:
 On Sat, Aug 06, 2011 at 09:46:20PM +0200, Rafael J. Wysocki wrote:
  On Saturday, August 06, 2011, Mark Brown wrote:
 
   I wouldn't say we've got to rely on userspace here - it seems like we
   ought to be able to use DMI or other system data to identify the
   affected systems and activate the workarounds.
 
  That's only practical on systems where the kernel can be rebuilt.
  Moreover, if that affects individual devices, using DMI-based blacklists
  is not really practical at all.
 
 OK, so this does sound like there's probably a genuine issue on PCs due
 to limitations in the environment.  Is it possible to expose these
 things to userspace in a way that's limited to the affected platforms?

Well, in principle we could make it depend in CONFIG_ACPI or something like
this, but I'm not sure that will be well-received. :-)

   You're right that it doesn't stop the kernel doing anything, the concern
   is that people just won't bother making the kernel work properly and
   will just do their power management in userspace and not bother fixing
   the kernel.
 
  I wouldn't call it fixing the kernel.  I'd rather say putting workarounds
  into the kernel, which I'm not sure is the right thing to at least in some
  cases.
 
 That does sound like a fair characterization for PCs.  For embedded
 systems where we have a *much* better knowledge of the hardware we're
 running on you're just working with the basics of what the hardware
 needs to run - the hardware needs whatever it needs and no matter what
 you think of the quality of the hardware there's an expectation that the
 kernel is ging to be able to work.

In the particular case in question, though, there's some freedom.  Namely,
the hardware will work for many different QoS constraints and it's not
precisely known in principle and upfront which one would be optimal for
a given workload.

   Punting to userspace seems like it is creating the
   expectation that we can't make the kernel work and isn't great from a
   usability perspective since users shouldn't really be worrying about bus
   performance or so on, it's not something that's visible at the level
   applications work at.
 
  However, platform builders may want to fine tuned things and I'm not sure
  we should require them to patch the kernel for this purpose.
 
 As I've said it's not the fine tuning that I'm worried about, it's the
 specific mechanism that's being suggested.  Being able to tune things in
 a way that's relevant to the device being tuned seems entirely sensible.

Do you know any better mechanism consistent accross all devices?
Please be specific. :-)

   I'm not sure I can see a lot of cases where you'd have root access and
   not be able to do kernel updates if required?  Having stuff in debugfs
   for diagnostics doesn't strike me as a problem if that's the issue.
 
  Root access doesn't necessarily mean you have all of the requisite
  tools (like compilers etc.) and installing them isn't always trivial
  (think of systems like phones etc.), let alone building the kernel from
  sources (where you don't necessarily know the original .config used for
  building your device's kernel).
 
 Phones are exactly the sort of case I'm primarily concerned with here.
 
 Realistically if you're in a position where you need to work at this
 very low level on an embedded device you can replace the entire firmware
 on the device.  We don't want to end up in the situation where we've got
 kernel support for a device and the only way to get it to actually run
 sensibly is to install the silicon vendor's proprietary userspace, and
 we especially don't want to end up in the situation where that userspace
 is using standard and supported kernel interfaces to do its thing.

Well, the wakelocks example shows clearly that preventing certain interfaces
from being merged into mainline doesn't actually prevent people from using
them in actual products.  I claim it's way better if a vendor uses its
proprietary user space with the mainline kernel than if that vendor patches
the kernel and _then_ uses its proprietary user space with it.  Your
argumentation seems to suggest that we encourage the latter.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Saturday, August 06, 2011, Mark Brown wrote:
 On Fri, Aug 05, 2011 at 09:37:36PM +0200, Rafael J. Wysocki wrote:
  On Friday, August 05, 2011, Mark Brown wrote:
 
   Do you have any examples of this that aren't better expressed in device
   specific terms?
 
  I'm not sure what you mean exactly, but if you take two PC-like systems
  with similar hardware configurations, but different BIOS-es and motherboard
  layouts, it's very likely that on one of them PCI PME won't be routed
  correctly, for example.  In that case the kernel has no way to figure out
  that that system is broken, the problem can only be worked around from user
  space by diabling runtime PM on the affected PCI devices.  I expect similar
  problems to appear for the PM QoS settings.
 
 I wouldn't say we've got to rely on userspace here - it seems like we
 ought to be able to use DMI or other system data to identify the
 affected systems and activate the workarounds.

That's only practical on systems where the kernel can be rebuilt.
Moreover, if that affects individual devices, using DMI-based blacklists
is not really practical at all.

   It's not that users don't know what they're doing, it's
   that working around system integration and stability issues in userspace
   isn't really progressing things well or helping with maintainability.
 
  No, it's not, but sometimes we simply don't have the choice.  Besides,
  in the particular case of PM QoS, the constraints set by user space will
  simply be added to the constraints set by kernel subsystems.  Thus they
  won't prevent any kernel subsystem from specifying its own constraints,
  but they will give user space the option to override the constraints
  originating from the kernel.
 
 You're right that it doesn't stop the kernel doing anything, the concern
 is that people just won't bother making the kernel work properly and
 will just do their power management in userspace and not bother fixing
 the kernel.

I wouldn't call it fixing the kernel.  I'd rather say putting workarounds
into the kernel, which I'm not sure is the right thing to at least in some
cases.

 Punting to userspace seems like it is creating the
 expectation that we can't make the kernel work and isn't great from a
 usability perspective since users shouldn't really be worrying about bus
 performance or so on, it's not something that's visible at the level
 applications work at.

However, platform builders may want to fine tuned things and I'm not sure
we should require them to patch the kernel for this purpose.

   Generally if the user has sufficient access to be able to do anything
   with this stuff they've got just as much access to the kernel as to
   userspace.
 
  Do you mean they may rebuild the kernel?  That isn't always possible.
 
 I'm not sure I can see a lot of cases where you'd have root access and
 not be able to do kernel updates if required?  Having stuff in debugfs
 for diagnostics doesn't strike me as a problem if that's the issue.

Root access doesn't necessarily mean you have all of the requisite
tools (like compilers etc.) and installing them isn't always trivial
(think of systems like phones etc.), let alone building the kernel from
sources (where you don't necessarily know the original .config used for
building your device's kernel).

IOW, I don't buy the you can always rebuild the kernel if necessary
argument.  It simply is not true in general.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [RFC/PATCH v2] PM / Runtime: allow _put_sync() from interrupts-disabled context

[Rafael J. Wysocki]

On Friday, August 05, 2011, Kevin Hilman wrote:
 Rafael J. Wysocki r...@sisk.pl writes:
 
  On Friday, July 22, 2011, Kevin Hilman wrote:
  Currently the use of pm_runtime_put_sync() is not safe from
  interrupts-disabled context because rpm_idle() will release the
  spinlock and enable interrupts for the idle callbacks.  This enables
  interrupts during a time where interrupts were expected to be
  disabled, and can have strange side effects on drivers that expected
  interrupts to be disabled.
  
  This is not a bug since the documentation clearly states that only
  _put_sync_suspend() is safe in IRQ-safe mode.
  
  However, pm_runtime_put_sync() could be made safe when in IRQ-safe
  mode by releasing the spinlock but not re-enabling interrupts, which
  is what this patch aims to do.
  
  Problem was found when using some buggy drivers that set
  pm_runtime_irq_safe() and used _put_sync() in interrupts-disabled
  context.
  
  The offending drivers have been fixed to use _put_sync_suspend(),
  But this patch is an RFC to see if it might make sense to allow
  using _put_sync() from interrupts-disabled context.
 
  OK, I'm going to take this for 3.2.
 
 Rafael, 
 
 Since you're planning to merge this, maybe we should consider merging
 this as a fix for v3.1, and possibly even for v3.0 stable.  That way,
 any current drivers using irq_safe and the normal _put_sync() will not
 have this problem.

I think I can push it for 3.1, but I don't think it's stable material.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Friday, August 05, 2011, Mark Brown wrote:
 On Thu, Aug 04, 2011 at 09:15:30PM +0200, Rafael J. Wysocki wrote:
  On Thursday, August 04, 2011, Mark Brown wrote:
 
   On the one hand that's true.  On the other hand that just seems like
   going down a bad road where we have drivers that only work when run with
   a magic userspace that may or may not be published which is just going
 
  First off, we're doing this already (user space can block runtime PM, for
  one example, because there are systems where runtime PM doesn't work
  although it works on other systems with analogous hardware and pretty
  much the same set of drivers).
 
 Yeah, I've never been terribly convinced about that and for the things
 that drivers need to manualy implement (like wake configuration) it's
 widely ignored.
 
  Second, I think there are valid use cases in which user space _really_ knows
  better than the kernel.  I'm opposed to the idea that users shouldn't be 
  given
  control of their systems, because they may not know what they're doing.
 
 Do you have any examples of this that aren't better expressed in device
 specific terms?

I'm not sure what you mean exactly, but if you take two PC-like systems
with similar hardware configurations, but different BIOS-es and motherboard
layouts, it's very likely that on one of them PCI PME won't be routed
correctly, for example.  In that case the kernel has no way to figure out
that that system is broken, the problem can only be worked around from user
space by diabling runtime PM on the affected PCI devices.  I expect similar
problems to appear for the PM QoS settings.

 It's not that users don't know what they're doing, it's
 that working around system integration and stability issues in userspace
 isn't really progressing things well or helping with maintainability.

No, it's not, but sometimes we simply don't have the choice.  Besides,
in the particular case of PM QoS, the constraints set by user space will
simply be added to the constraints set by kernel subsystems.  Thus they
won't prevent any kernel subsystem from specifying its own constraints,
but they will give user space the option to override the constraints
originating from the kernel.

 Generally if the user has sufficient access to be able to do anything
 with this stuff they've got just as much access to the kernel as to
 userspace.

Do you mean they may rebuild the kernel?  That isn't always possible.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Thursday, August 04, 2011, Mark Brown wrote:
 On Wed, Aug 03, 2011 at 12:16:17AM +0200, Rafael J. Wysocki wrote:
  On Tuesday, August 02, 2011, Kevin Hilman wrote:
 
   I disagree and think that both are quite realistic (mainly because they
   exist today, albiet mostly out of tree because no generic QoS framework
   exist.  e.g. on OMAP, we have OMAP-specific *kernel* APIs for requesting
   per-device wakeup latencies, and drivers and frameworks are using them.)
  
  I'm sure there are frameworks using such things.  I'm also sure there
  are frameworks that don't.  BTW, the we have it out of the tree argument 
  is
  not very useful, so I'd appreciate it if you didn't use it.
 
 It's useful to know if people have tried things; it doesn't mean it's
 going to be OK for mainline but it is a data point.
 
   In this case, the video framework (V4L2) might not want any knobs
   exposed to userspace because userspace simply doesn't have the knowledge
   to set appropriate constraints.  I'm less familiar with audio, but I
   believe audio would be similar (sample rate, number of channels, mixing
   with other concurrent audio streams, etc. etc. are all known by the
   kernel-side code.)
 
 Yeah, that sort of stuff and also data like wakeup latencies required to
 service interrupts.
 
  I still don't understand what's wrong with allowing user space to _add_
  requirements.  The will only override the drivers' or frameworks' 
  requirements
  if they are stronger, so the functionality shouldn't be hurt.  They may 
  cause
  some more energy to be used, but if user space wants that, it's pretty much
  fine by me.
 
 On the one hand that's true.  On the other hand that just seems like
 going down a bad road where we have drivers that only work when run with
 a magic userspace that may or may not be published which is just going
 to make people miserable.  I'm not sure there are many people who would
 choose to use more power without wanting some functional change so
 presumably any users would be seeking to work around some kernel problem
 and adding the user interface seems to be saying that this is OK,
 expected and a natural part of power optimization.

First off, we're doing this already (user space can block runtime PM, for
one example, because there are systems where runtime PM doesn't work
although it works on other systems with analogous hardware and pretty
much the same set of drivers).

Second, I think there are valid use cases in which user space _really_ knows
better than the kernel.  I'm opposed to the idea that users shouldn't be given
control of their systems, because they may not know what they're doing.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/13] PM: QoS: rename pm_qos_params files to pm_qos

[Rafael J. Wysocki]

On Tuesday, August 02, 2011, Jean Pihet wrote:
 Rafael,
 
 2011/7/29 Rafael J. Wysocki r...@sisk.pl:
  On Thursday, July 28, 2011, jean.pi...@newoldbits.com wrote:
  From: Jean Pihet j-pi...@ti.com
 
  The PM QoS implementation files are better named
  kernel/pm_qos.c and include/linux/pm_qos.h.
 
 ...
 
   create mode 100644 include/linux/pm_qos.h
   delete mode 100644 include/linux/pm_qos_params.h
 
  That I agree with.
 
   create mode 100644 kernel/pm_qos.c
   delete mode 100644 kernel/pm_qos_params.c
 
  As I said, please move that file to kernel/power and call it qos.c.
 Ok
 
 
  That said the device interface should be located in drivers/base/power
  to follow our current conventions.
 By device interface I understand the following:
 - the user space API (per-device sysfs entry),
 - the in-kernel device specific PM QoS API. If needed, cf. comments on
 [04/13] about that.
 
 Is that correct?

Yes, it is.

I think that the definitions of the dev_pm_qos_*() funtions mentioned
in there may go into drivers/base/power/qos.c and they can call functions
from kernel/power/qos.c.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

[Rafael J. Wysocki]

On Tuesday, August 02, 2011, Kevin Hilman wrote:
 Rafael J. Wysocki r...@sisk.pl writes:
 
  Hi,
 
  On Thursday, June 30, 2011, jean.pi...@newoldbits.com wrote:
  From: Jean Pihet j-pi...@ti.com
  
  - add a new PM QoS class PM_QOS_DEV_WAKEUP_LATENCY for device wake-up
  constraints. Due to the per-device nature of the new class the constraints
  list is stored inside the device dev_pm_info struct instead of the internal
  per-class constraints lists.
 
  I think PM_QOS_DEV_LATENCY might be a better name.
 
  The new class is only available from kernel drivers and so is not exported
  to user space.
 
  It should be available to user space, however, because in many cases drivers
  simply have no idea what values to use (after all, the use decides if he
  wants to trade worse video playback quality for better battery life, for
  example).
 
 
 FWIW, I think it's wrong to expose the raw per-device constraints
 directly to userspace.
 
 I think it's the responsibility of the subsystems (video, audio, input,
 etc.) to expose QoS knobs to userspace as they see fit and now allow
 userspace to tinker directly with QoS constraints.

This assumes that those subsystems or rather frameworks (a bus type or
a device class is a subsystem in the terminology used throughout the PM
documentation) will (a) know about PM QoS and (b) will care to handle it.
Both (a) and (b) seem to be unrealistic IMHO.

We already export wakeup and runtime PM knobs per device via sysfs and
I'm not so sure why PM QoS is different in that respect.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html