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

[Tomeu Vizoso]

On 25 October 2015 at 16:18, Rafael J. Wysocki  wrote:
> 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, 

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
> +++ 

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

[Tomeu Vizoso]

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
+++ b/drivers/base/power/clock_ops.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #ifdef CONFIG_PM
@@ -346,7 +347,7 @@ static int pm_clk_notify(struct notifier_block *nb,
if (error)
break;
 
-