[PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Ulf Hansson]

Changes in v2:
	- Addressed comments from Geert around DT doc.
	- Addressed comments from Jon around clarification of how to use this
	and changes to returned error codes.
	- Fixed build error in case CONFIG_PM was unset.

There are devices that are partitioned across multiple PM domains. Currently
these can't be supported well by the available PM infrastructures we have in
the kernel. This series is an attempt to address this.

The interesting parts happens from patch 5 an onwards, including a minor DT
update to the existing power-domain bindings, the 4 earlier are just trivial
clean-ups of some related code in genpd, which I happened to stumble over.

Some additional background:

One existing case where devices are partitioned across multiple PM domains, is
the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
series, trying to address these issues, however this is a new approach, while
it re-uses the same concepts from DT point of view.

The Tegra 124/210 X-USB subsystem contains of a host controller and a device
controller. Each controller have its own independent PM domain, but are being
partitioned across another shared PM domain for the USB super-speed logic.

Currently to make the drivers work, either the related PM domains needs to stay
powered on always or the PM domain topology needs to be in-correctly modelled
through sub-domains. In both cases PM domains may be powered on while they
don't need to be, so in the end this means - wasting power -.

As stated above, this series intends to address these problem from a PM
infrastructure point of view. More details are available in each changelog.

It should be noted that this series has been tested on HW, however only by using
a home-cooked test PM domain driver for genpd and together with a test driver.
This allowed me to play with PM domain (genpd), runtime PM and device links.

Any further deployment for real use cases are greatly appreciated. I am happy to
to help, if needed!

Kind regards
Ulf Hansson


Ulf Hansson (9):
  PM / Domains: Drop extern declarations of functions in pm_domain.h
  PM / Domains: Drop __pm_genpd_add_device()
  PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()
  PM / Domains: Drop unused parameter in genpd_allocate_dev_data()
  PM / Domains: dt: Allow power-domain property to be a list of
    specifiers
  PM / Domains: Don't attach devices in genpd with multi PM domains
  PM / Domains: Split genpd_dev_pm_attach()
  PM / Domains: Add support for multi PM domains per device to genpd
  PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM
    domains

 .../bindings/power/power_domain.txt           |  19 ++-
 drivers/base/power/common.c                   |  39 ++++-
 drivers/base/power/domain.c                   | 155 ++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c       |   2 +-
 include/linux/pm_domain.h                     |  79 ++++-----
 5 files changed, 216 insertions(+), 78 deletions(-)

-- 
2.17.0

[PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Ulf Hansson]

Changes in v2:
	- Addressed comments from Geert around DT doc.
	- Addressed comments from Jon around clarification of how to use this
	and changes to returned error codes.
	- Fixed build error in case CONFIG_PM was unset.

There are devices that are partitioned across multiple PM domains. Currently
these can't be supported well by the available PM infrastructures we have in
the kernel. This series is an attempt to address this.

The interesting parts happens from patch 5 an onwards, including a minor DT
update to the existing power-domain bindings, the 4 earlier are just trivial
clean-ups of some related code in genpd, which I happened to stumble over.

Some additional background:

One existing case where devices are partitioned across multiple PM domains, is
the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
series, trying to address these issues, however this is a new approach, while
it re-uses the same concepts from DT point of view.

The Tegra 124/210 X-USB subsystem contains of a host controller and a device
controller. Each controller have its own independent PM domain, but are being
partitioned across another shared PM domain for the USB super-speed logic.

Currently to make the drivers work, either the related PM domains needs to stay
powered on always or the PM domain topology needs to be in-correctly modelled
through sub-domains. In both cases PM domains may be powered on while they
don't need to be, so in the end this means - wasting power -.

As stated above, this series intends to address these problem from a PM
infrastructure point of view. More details are available in each changelog.

It should be noted that this series has been tested on HW, however only by using
a home-cooked test PM domain driver for genpd and together with a test driver.
This allowed me to play with PM domain (genpd), runtime PM and device links.

Any further deployment for real use cases are greatly appreciated. I am happy to
to help, if needed!

Kind regards
Ulf Hansson


Ulf Hansson (9):
  PM / Domains: Drop extern declarations of functions in pm_domain.h
  PM / Domains: Drop __pm_genpd_add_device()
  PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()
  PM / Domains: Drop unused parameter in genpd_allocate_dev_data()
  PM / Domains: dt: Allow power-domain property to be a list of
    specifiers
  PM / Domains: Don't attach devices in genpd with multi PM domains
  PM / Domains: Split genpd_dev_pm_attach()
  PM / Domains: Add support for multi PM domains per device to genpd
  PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM
    domains

 .../bindings/power/power_domain.txt           |  19 ++-
 drivers/base/power/common.c                   |  39 ++++-
 drivers/base/power/domain.c                   | 155 ++++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c       |   2 +-
 include/linux/pm_domain.h                     |  79 ++++-----
 5 files changed, 216 insertions(+), 78 deletions(-)

-- 
2.17.0

[PATCH v2 1/9] PM / Domains: Drop extern declarations of functions in pm_domain.h

[Ulf Hansson]

Using "extern" to declare a function in a public header file is somewhat
pointless, but also doesn't hurt. However, to make all the function
declarations in pm_domain.h to be consistent, let's drop the use of
"extern".

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 include/linux/pm_domain.h | 51 ++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 4e5764083fd8..c847e9a3033d 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -143,21 +143,17 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 	return to_gpd_data(dev->power.subsys_data->domain_data);
 }
 
-extern int __pm_genpd_add_device(struct generic_pm_domain *genpd,
-				 struct device *dev,
-				 struct gpd_timing_data *td);
-
-extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
-				  struct device *dev);
-extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
-				  struct generic_pm_domain *new_subdomain);
-extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
-				     struct generic_pm_domain *target);
-extern int pm_genpd_init(struct generic_pm_domain *genpd,
-			 struct dev_power_governor *gov, bool is_off);
-extern int pm_genpd_remove(struct generic_pm_domain *genpd);
-extern int dev_pm_genpd_set_performance_state(struct device *dev,
-					      unsigned int state);
+int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
+			  struct gpd_timing_data *td);
+int pm_genpd_remove_device(struct generic_pm_domain *genpd, struct device *dev);
+int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
+			   struct generic_pm_domain *new_subdomain);
+int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
+			      struct generic_pm_domain *target);
+int pm_genpd_init(struct generic_pm_domain *genpd,
+		  struct dev_power_governor *gov, bool is_off);
+int pm_genpd_remove(struct generic_pm_domain *genpd);
+int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
@@ -215,8 +211,8 @@ static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
 }
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS_SLEEP
-extern void pm_genpd_syscore_poweroff(struct device *dev);
-extern void pm_genpd_syscore_poweron(struct device *dev);
+void pm_genpd_syscore_poweroff(struct device *dev);
+void pm_genpd_syscore_poweron(struct device *dev);
 #else
 static inline void pm_genpd_syscore_poweroff(struct device *dev) {}
 static inline void pm_genpd_syscore_poweron(struct device *dev) {}
@@ -240,14 +236,13 @@ int of_genpd_add_provider_simple(struct device_node *np,
 int of_genpd_add_provider_onecell(struct device_node *np,
 				  struct genpd_onecell_data *data);
 void of_genpd_del_provider(struct device_node *np);
-extern int of_genpd_add_device(struct of_phandle_args *args,
-			       struct device *dev);
-extern int of_genpd_add_subdomain(struct of_phandle_args *parent,
-				  struct of_phandle_args *new_subdomain);
-extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
-extern int of_genpd_parse_idle_states(struct device_node *dn,
-			struct genpd_power_state **states, int *n);
-extern unsigned int of_genpd_opp_to_performance_state(struct device *dev,
+int of_genpd_add_device(struct of_phandle_args *args, struct device *dev);
+int of_genpd_add_subdomain(struct of_phandle_args *parent,
+			   struct of_phandle_args *new_subdomain);
+struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
+int of_genpd_parse_idle_states(struct device_node *dn,
+			       struct genpd_power_state **states, int *n);
+unsigned int of_genpd_opp_to_performance_state(struct device *dev,
 				struct device_node *opp_node);
 
 int genpd_dev_pm_attach(struct device *dev);
@@ -304,9 +299,9 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 #ifdef CONFIG_PM
-extern int dev_pm_domain_attach(struct device *dev, bool power_on);
-extern void dev_pm_domain_detach(struct device *dev, bool power_off);
-extern void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
+int dev_pm_domain_attach(struct device *dev, bool power_on);
+void dev_pm_domain_detach(struct device *dev, bool power_off);
+void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
 #else
 static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
 {
-- 
2.17.0

[PATCH v2 1/9] PM / Domains: Drop extern declarations of functions in pm_domain.h

[Ulf Hansson]

Using "extern" to declare a function in a public header file is somewhat
pointless, but also doesn't hurt. However, to make all the function
declarations in pm_domain.h to be consistent, let's drop the use of
"extern".

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 include/linux/pm_domain.h | 51 ++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 4e5764083fd8..c847e9a3033d 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -143,21 +143,17 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 	return to_gpd_data(dev->power.subsys_data->domain_data);
 }
 
-extern int __pm_genpd_add_device(struct generic_pm_domain *genpd,
-				 struct device *dev,
-				 struct gpd_timing_data *td);
-
-extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
-				  struct device *dev);
-extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
-				  struct generic_pm_domain *new_subdomain);
-extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
-				     struct generic_pm_domain *target);
-extern int pm_genpd_init(struct generic_pm_domain *genpd,
-			 struct dev_power_governor *gov, bool is_off);
-extern int pm_genpd_remove(struct generic_pm_domain *genpd);
-extern int dev_pm_genpd_set_performance_state(struct device *dev,
-					      unsigned int state);
+int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
+			  struct gpd_timing_data *td);
+int pm_genpd_remove_device(struct generic_pm_domain *genpd, struct device *dev);
+int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
+			   struct generic_pm_domain *new_subdomain);
+int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
+			      struct generic_pm_domain *target);
+int pm_genpd_init(struct generic_pm_domain *genpd,
+		  struct dev_power_governor *gov, bool is_off);
+int pm_genpd_remove(struct generic_pm_domain *genpd);
+int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
@@ -215,8 +211,8 @@ static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
 }
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS_SLEEP
-extern void pm_genpd_syscore_poweroff(struct device *dev);
-extern void pm_genpd_syscore_poweron(struct device *dev);
+void pm_genpd_syscore_poweroff(struct device *dev);
+void pm_genpd_syscore_poweron(struct device *dev);
 #else
 static inline void pm_genpd_syscore_poweroff(struct device *dev) {}
 static inline void pm_genpd_syscore_poweron(struct device *dev) {}
@@ -240,14 +236,13 @@ int of_genpd_add_provider_simple(struct device_node *np,
 int of_genpd_add_provider_onecell(struct device_node *np,
 				  struct genpd_onecell_data *data);
 void of_genpd_del_provider(struct device_node *np);
-extern int of_genpd_add_device(struct of_phandle_args *args,
-			       struct device *dev);
-extern int of_genpd_add_subdomain(struct of_phandle_args *parent,
-				  struct of_phandle_args *new_subdomain);
-extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
-extern int of_genpd_parse_idle_states(struct device_node *dn,
-			struct genpd_power_state **states, int *n);
-extern unsigned int of_genpd_opp_to_performance_state(struct device *dev,
+int of_genpd_add_device(struct of_phandle_args *args, struct device *dev);
+int of_genpd_add_subdomain(struct of_phandle_args *parent,
+			   struct of_phandle_args *new_subdomain);
+struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
+int of_genpd_parse_idle_states(struct device_node *dn,
+			       struct genpd_power_state **states, int *n);
+unsigned int of_genpd_opp_to_performance_state(struct device *dev,
 				struct device_node *opp_node);
 
 int genpd_dev_pm_attach(struct device *dev);
@@ -304,9 +299,9 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 #ifdef CONFIG_PM
-extern int dev_pm_domain_attach(struct device *dev, bool power_on);
-extern void dev_pm_domain_detach(struct device *dev, bool power_off);
-extern void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
+int dev_pm_domain_attach(struct device *dev, bool power_on);
+void dev_pm_domain_detach(struct device *dev, bool power_off);
+void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
 #else
 static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
 {
-- 
2.17.0

[PATCH v2 1/9] PM / Domains: Drop extern declarations of functions in pm_domain.h

[Ulf Hansson]

Using "extern" to declare a function in a public header file is somewhat
pointless, but also doesn't hurt. However, to make all the function
declarations in pm_domain.h to be consistent, let's drop the use of
"extern".

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 include/linux/pm_domain.h | 51 ++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 4e5764083fd8..c847e9a3033d 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -143,21 +143,17 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 	return to_gpd_data(dev->power.subsys_data->domain_data);
 }
 
-extern int __pm_genpd_add_device(struct generic_pm_domain *genpd,
-				 struct device *dev,
-				 struct gpd_timing_data *td);
-
-extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
-				  struct device *dev);
-extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
-				  struct generic_pm_domain *new_subdomain);
-extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
-				     struct generic_pm_domain *target);
-extern int pm_genpd_init(struct generic_pm_domain *genpd,
-			 struct dev_power_governor *gov, bool is_off);
-extern int pm_genpd_remove(struct generic_pm_domain *genpd);
-extern int dev_pm_genpd_set_performance_state(struct device *dev,
-					      unsigned int state);
+int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
+			  struct gpd_timing_data *td);
+int pm_genpd_remove_device(struct generic_pm_domain *genpd, struct device *dev);
+int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
+			   struct generic_pm_domain *new_subdomain);
+int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
+			      struct generic_pm_domain *target);
+int pm_genpd_init(struct generic_pm_domain *genpd,
+		  struct dev_power_governor *gov, bool is_off);
+int pm_genpd_remove(struct generic_pm_domain *genpd);
+int dev_pm_genpd_set_performance_state(struct device *dev, unsigned int state);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
@@ -215,8 +211,8 @@ static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
 }
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS_SLEEP
-extern void pm_genpd_syscore_poweroff(struct device *dev);
-extern void pm_genpd_syscore_poweron(struct device *dev);
+void pm_genpd_syscore_poweroff(struct device *dev);
+void pm_genpd_syscore_poweron(struct device *dev);
 #else
 static inline void pm_genpd_syscore_poweroff(struct device *dev) {}
 static inline void pm_genpd_syscore_poweron(struct device *dev) {}
@@ -240,14 +236,13 @@ int of_genpd_add_provider_simple(struct device_node *np,
 int of_genpd_add_provider_onecell(struct device_node *np,
 				  struct genpd_onecell_data *data);
 void of_genpd_del_provider(struct device_node *np);
-extern int of_genpd_add_device(struct of_phandle_args *args,
-			       struct device *dev);
-extern int of_genpd_add_subdomain(struct of_phandle_args *parent,
-				  struct of_phandle_args *new_subdomain);
-extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
-extern int of_genpd_parse_idle_states(struct device_node *dn,
-			struct genpd_power_state **states, int *n);
-extern unsigned int of_genpd_opp_to_performance_state(struct device *dev,
+int of_genpd_add_device(struct of_phandle_args *args, struct device *dev);
+int of_genpd_add_subdomain(struct of_phandle_args *parent,
+			   struct of_phandle_args *new_subdomain);
+struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
+int of_genpd_parse_idle_states(struct device_node *dn,
+			       struct genpd_power_state **states, int *n);
+unsigned int of_genpd_opp_to_performance_state(struct device *dev,
 				struct device_node *opp_node);
 
 int genpd_dev_pm_attach(struct device *dev);
@@ -304,9 +299,9 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 #ifdef CONFIG_PM
-extern int dev_pm_domain_attach(struct device *dev, bool power_on);
-extern void dev_pm_domain_detach(struct device *dev, bool power_off);
-extern void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
+int dev_pm_domain_attach(struct device *dev, bool power_on);
+void dev_pm_domain_detach(struct device *dev, bool power_off);
+void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
 #else
 static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
 {
-- 
2.17.0

[PATCH v2 2/9] PM / Domains: Drop __pm_genpd_add_device()

[Ulf Hansson]

There are still a few non-DT existing users of genpd, however neither of
them uses __pm_genpd_add_device(), hence let's drop it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 10 ++++------
 include/linux/pm_domain.h   | 14 +++-----------
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 71a1cc79fbaa..469edb401811 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1414,23 +1414,21 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 }
 
 /**
- * __pm_genpd_add_device - Add a device to an I/O PM domain.
+ * pm_genpd_add_device - Add a device to an I/O PM domain.
  * @genpd: PM domain to add the device to.
  * @dev: Device to be added.
- * @td: Set of PM QoS timing parameters to attach to the device.
  */
-int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
-			  struct gpd_timing_data *td)
+int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 {
 	int ret;
 
 	mutex_lock(&gpd_list_lock);
-	ret = genpd_add_device(genpd, dev, td);
+	ret = genpd_add_device(genpd, dev, NULL);
 	mutex_unlock(&gpd_list_lock);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(__pm_genpd_add_device);
+EXPORT_SYMBOL_GPL(pm_genpd_add_device);
 
 static int genpd_remove_device(struct generic_pm_domain *genpd,
 			       struct device *dev)
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index c847e9a3033d..79888fb4a81f 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -143,8 +143,7 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 	return to_gpd_data(dev->power.subsys_data->domain_data);
 }
 
-int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
-			  struct gpd_timing_data *td);
+int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev);
 int pm_genpd_remove_device(struct generic_pm_domain *genpd, struct device *dev);
 int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 			   struct generic_pm_domain *new_subdomain);
@@ -163,9 +162,8 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 {
 	return ERR_PTR(-ENOSYS);
 }
-static inline int __pm_genpd_add_device(struct generic_pm_domain *genpd,
-					struct device *dev,
-					struct gpd_timing_data *td)
+static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
+				      struct device *dev)
 {
 	return -ENOSYS;
 }
@@ -204,12 +202,6 @@ static inline int dev_pm_genpd_set_performance_state(struct device *dev,
 #define pm_domain_always_on_gov		(*(struct dev_power_governor *)(NULL))
 #endif
 
-static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
-				      struct device *dev)
-{
-	return __pm_genpd_add_device(genpd, dev, NULL);
-}
-
 #ifdef CONFIG_PM_GENERIC_DOMAINS_SLEEP
 void pm_genpd_syscore_poweroff(struct device *dev);
 void pm_genpd_syscore_poweron(struct device *dev);
-- 
2.17.0

[PATCH v2 2/9] PM / Domains: Drop __pm_genpd_add_device()

[Ulf Hansson]

There are still a few non-DT existing users of genpd, however neither of
them uses __pm_genpd_add_device(), hence let's drop it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 10 ++++------
 include/linux/pm_domain.h   | 14 +++-----------
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 71a1cc79fbaa..469edb401811 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1414,23 +1414,21 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 }
 
 /**
- * __pm_genpd_add_device - Add a device to an I/O PM domain.
+ * pm_genpd_add_device - Add a device to an I/O PM domain.
  * @genpd: PM domain to add the device to.
  * @dev: Device to be added.
- * @td: Set of PM QoS timing parameters to attach to the device.
  */
-int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
-			  struct gpd_timing_data *td)
+int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
 {
 	int ret;
 
 	mutex_lock(&gpd_list_lock);
-	ret = genpd_add_device(genpd, dev, td);
+	ret = genpd_add_device(genpd, dev, NULL);
 	mutex_unlock(&gpd_list_lock);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(__pm_genpd_add_device);
+EXPORT_SYMBOL_GPL(pm_genpd_add_device);
 
 static int genpd_remove_device(struct generic_pm_domain *genpd,
 			       struct device *dev)
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index c847e9a3033d..79888fb4a81f 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -143,8 +143,7 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 	return to_gpd_data(dev->power.subsys_data->domain_data);
 }
 
-int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
-			  struct gpd_timing_data *td);
+int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev);
 int pm_genpd_remove_device(struct generic_pm_domain *genpd, struct device *dev);
 int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 			   struct generic_pm_domain *new_subdomain);
@@ -163,9 +162,8 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 {
 	return ERR_PTR(-ENOSYS);
 }
-static inline int __pm_genpd_add_device(struct generic_pm_domain *genpd,
-					struct device *dev,
-					struct gpd_timing_data *td)
+static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
+				      struct device *dev)
 {
 	return -ENOSYS;
 }
@@ -204,12 +202,6 @@ static inline int dev_pm_genpd_set_performance_state(struct device *dev,
 #define pm_domain_always_on_gov		(*(struct dev_power_governor *)(NULL))
 #endif
 
-static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
-				      struct device *dev)
-{
-	return __pm_genpd_add_device(genpd, dev, NULL);
-}
-
 #ifdef CONFIG_PM_GENERIC_DOMAINS_SLEEP
 void pm_genpd_syscore_poweroff(struct device *dev);
 void pm_genpd_syscore_poweron(struct device *dev);
-- 
2.17.0

[PATCH v2 3/9] PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()

[Ulf Hansson]

There is no need to pass a genpd struct to pm_genpd_remove_device(), as we
already have the information about the PM domain (genpd) through the device
structure.

Additionally, we don't allow to remove a PM domain from a device, other
than the one it may have assigned to it, so really it does not make sense
to have a separate in-param for it.

For these reason, drop it and update the current only call to
pm_genpd_remove_device() from amdgpu_acp.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c             | 8 ++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 2 +-
 include/linux/pm_domain.h               | 5 ++---
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 469edb401811..c89b0ad068b7 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1475,13 +1475,13 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 
 /**
  * pm_genpd_remove_device - Remove a device from an I/O PM domain.
- * @genpd: PM domain to remove the device from.
  * @dev: Device to be removed.
  */
-int pm_genpd_remove_device(struct generic_pm_domain *genpd,
-			   struct device *dev)
+int pm_genpd_remove_device(struct device *dev)
 {
-	if (!genpd || genpd != genpd_lookup_dev(dev))
+	struct generic_pm_domain *genpd = genpd_lookup_dev(dev);
+
+	if (!genpd)
 		return -EINVAL;
 
 	return genpd_remove_device(genpd, dev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index a29362f9ef41..12558044acd4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -513,7 +513,7 @@ static int acp_hw_fini(void *handle)
 	if (adev->acp.acp_genpd) {
 		for (i = 0; i < ACP_DEVS ; i++) {
 			dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i);
-			ret = pm_genpd_remove_device(&adev->acp.acp_genpd->gpd, dev);
+			ret = pm_genpd_remove_device(dev);
 			/* If removal fails, dont giveup and try rest */
 			if (ret)
 				dev_err(dev, "remove dev from genpd failed\n");
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 79888fb4a81f..42e0d649e653 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -144,7 +144,7 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 }
 
 int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev);
-int pm_genpd_remove_device(struct generic_pm_domain *genpd, struct device *dev);
+int pm_genpd_remove_device(struct device *dev);
 int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 			   struct generic_pm_domain *new_subdomain);
 int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
@@ -167,8 +167,7 @@ static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
 {
 	return -ENOSYS;
 }
-static inline int pm_genpd_remove_device(struct generic_pm_domain *genpd,
-					 struct device *dev)
+static inline int pm_genpd_remove_device(struct device *dev)
 {
 	return -ENOSYS;
 }
-- 
2.17.0

[PATCH v2 3/9] PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()

[Ulf Hansson]

There is no need to pass a genpd struct to pm_genpd_remove_device(), as we
already have the information about the PM domain (genpd) through the device
structure.

Additionally, we don't allow to remove a PM domain from a device, other
than the one it may have assigned to it, so really it does not make sense
to have a separate in-param for it.

For these reason, drop it and update the current only call to
pm_genpd_remove_device() from amdgpu_acp.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian K?nig <christian.koenig@amd.com>
Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c             | 8 ++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 2 +-
 include/linux/pm_domain.h               | 5 ++---
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 469edb401811..c89b0ad068b7 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1475,13 +1475,13 @@ static int genpd_remove_device(struct generic_pm_domain *genpd,
 
 /**
  * pm_genpd_remove_device - Remove a device from an I/O PM domain.
- * @genpd: PM domain to remove the device from.
  * @dev: Device to be removed.
  */
-int pm_genpd_remove_device(struct generic_pm_domain *genpd,
-			   struct device *dev)
+int pm_genpd_remove_device(struct device *dev)
 {
-	if (!genpd || genpd != genpd_lookup_dev(dev))
+	struct generic_pm_domain *genpd = genpd_lookup_dev(dev);
+
+	if (!genpd)
 		return -EINVAL;
 
 	return genpd_remove_device(genpd, dev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index a29362f9ef41..12558044acd4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -513,7 +513,7 @@ static int acp_hw_fini(void *handle)
 	if (adev->acp.acp_genpd) {
 		for (i = 0; i < ACP_DEVS ; i++) {
 			dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i);
-			ret = pm_genpd_remove_device(&adev->acp.acp_genpd->gpd, dev);
+			ret = pm_genpd_remove_device(dev);
 			/* If removal fails, dont giveup and try rest */
 			if (ret)
 				dev_err(dev, "remove dev from genpd failed\n");
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 79888fb4a81f..42e0d649e653 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -144,7 +144,7 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 }
 
 int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev);
-int pm_genpd_remove_device(struct generic_pm_domain *genpd, struct device *dev);
+int pm_genpd_remove_device(struct device *dev);
 int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 			   struct generic_pm_domain *new_subdomain);
 int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
@@ -167,8 +167,7 @@ static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
 {
 	return -ENOSYS;
 }
-static inline int pm_genpd_remove_device(struct generic_pm_domain *genpd,
-					 struct device *dev)
+static inline int pm_genpd_remove_device(struct device *dev)
 {
 	return -ENOSYS;
 }
-- 
2.17.0

[PATCH v2 4/9] PM / Domains: Drop unused parameter in genpd_allocate_dev_data()

[Ulf Hansson]

The in-parameter struct generic_pm_domain *genpd to
genpd_allocate_dev_data() is unused, so let's drop it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index c89b0ad068b7..7ebf7993273a 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1316,7 +1316,6 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
 #endif /* CONFIG_PM_SLEEP */
 
 static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
-					struct generic_pm_domain *genpd,
 					struct gpd_timing_data *td)
 {
 	struct generic_pm_domain_data *gpd_data;
@@ -1385,7 +1384,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	gpd_data = genpd_alloc_dev_data(dev, genpd, td);
+	gpd_data = genpd_alloc_dev_data(dev, td);
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
-- 
2.17.0

[PATCH v2 4/9] PM / Domains: Drop unused parameter in genpd_allocate_dev_data()

[Ulf Hansson]

The in-parameter struct generic_pm_domain *genpd to
genpd_allocate_dev_data() is unused, so let's drop it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index c89b0ad068b7..7ebf7993273a 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1316,7 +1316,6 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
 #endif /* CONFIG_PM_SLEEP */
 
 static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
-					struct generic_pm_domain *genpd,
 					struct gpd_timing_data *td)
 {
 	struct generic_pm_domain_data *gpd_data;
@@ -1385,7 +1384,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	gpd_data = genpd_alloc_dev_data(dev, genpd, td);
+	gpd_data = genpd_alloc_dev_data(dev, td);
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
-- 
2.17.0

[PATCH v2 5/9] PM / Domains: dt: Allow power-domain property to be a list of specifiers

[Ulf Hansson]

To be able to describe topologies where devices are partitioned across
multiple power domains, let's extend the power-domain property to allow
being a list of PM domain specifiers.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Suggested-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- Fixed comments from Geert. Re-worded to "PM domain specifiers" and
	clarified DT example.

---
 .../bindings/power/power_domain.txt           | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 4733f76cbe48..9b387f861aed 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -111,8 +111,8 @@ Example 3:
 ==PM domain consumers==
 
 Required properties:
- - power-domains : A phandle and PM domain specifier as defined by bindings of
-                   the power controller specified by phandle.
+ - power-domains : A list of PM domain specifiers, as defined by bindings of
+		the power controller that is the PM domain provider.
 
 Example:
 
@@ -122,9 +122,18 @@ Example:
 		power-domains = <&power 0>;
 	};
 
-The node above defines a typical PM domain consumer device, which is located
-inside a PM domain with index 0 of a power controller represented by a node
-with the label "power".
+	leaky-device@12351000 {
+		compatible = "foo,i-leak-current";
+		reg = <0x12351000 0x1000>;
+		power-domains = <&power 0>, <&power 1> ;
+	};
+
+The first example above defines a typical PM domain consumer device, which is
+located inside a PM domain with index 0 of a power controller represented by a
+node with the label "power".
+In the second example the consumer device are partitioned across two PM domains,
+the first with index 0 and the second with index 1, of a power controller that
+is represented by a node with the label "power.
 
 Optional properties:
 - required-opps: This contains phandle to an OPP node in another device's OPP
-- 
2.17.0

[PATCH v2 5/9] PM / Domains: dt: Allow power-domain property to be a list of specifiers

[Ulf Hansson]

To be able to describe topologies where devices are partitioned across
multiple power domains, let's extend the power-domain property to allow
being a list of PM domain specifiers.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree at vger.kernel.org
Suggested-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- Fixed comments from Geert. Re-worded to "PM domain specifiers" and
	clarified DT example.

---
 .../bindings/power/power_domain.txt           | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 4733f76cbe48..9b387f861aed 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -111,8 +111,8 @@ Example 3:
 ==PM domain consumers==
 
 Required properties:
- - power-domains : A phandle and PM domain specifier as defined by bindings of
-                   the power controller specified by phandle.
+ - power-domains : A list of PM domain specifiers, as defined by bindings of
+		the power controller that is the PM domain provider.
 
 Example:
 
@@ -122,9 +122,18 @@ Example:
 		power-domains = <&power 0>;
 	};
 
-The node above defines a typical PM domain consumer device, which is located
-inside a PM domain with index 0 of a power controller represented by a node
-with the label "power".
+	leaky-device at 12351000 {
+		compatible = "foo,i-leak-current";
+		reg = <0x12351000 0x1000>;
+		power-domains = <&power 0>, <&power 1> ;
+	};
+
+The first example above defines a typical PM domain consumer device, which is
+located inside a PM domain with index 0 of a power controller represented by a
+node with the label "power".
+In the second example the consumer device are partitioned across two PM domains,
+the first with index 0 and the second with index 1, of a power controller that
+is represented by a node with the label "power.
 
 Optional properties:
 - required-opps: This contains phandle to an OPP node in another device's OPP
-- 
2.17.0

[PATCH v2 6/9] PM / Domains: Don't attach devices in genpd with multi PM domains

[Ulf Hansson]

The power-domain DT property may now contain a list of PM domain
specifiers, which represents that a device are partitioned across multiple
PM domains. This leads to a new situation in genpd_dev_pm_attach(), as only
one PM domain can be attached per device.

To remain things simple for the most common configuration, when a single PM
domain is used, let's treat the multiple PM domain case as being specific.

In other words, let's change genpd_dev_pm_attach() to check for multiple PM
domains and prevent it from attach any PM domain for this case. Instead,
leave this to be managed separately, from following changes to genpd.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Suggested-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- Minor update to changelog to mention "PM domain specifiers" rather
	than a "list of phandles".

---
 drivers/base/power/domain.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 7ebf7993273a..12a20f21974d 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2229,10 +2229,10 @@ static void genpd_dev_pm_sync(struct device *dev)
  * attaches the device to retrieved pm_domain ops.
  *
  * Returns 1 on successfully attached PM domain, 0 when the device don't need a
- * PM domain or a negative error code in case of failures. Note that if a
- * power-domain exists for the device, but it cannot be found or turned on,
- * then return -EPROBE_DEFER to ensure that the device is not probed and to
- * re-try again later.
+ * PM domain or when multiple power-domains exists for it, else a negative error
+ * code. Note that if a power-domain exists for the device, but it cannot be
+ * found or turned on, then return -EPROBE_DEFER to ensure that the device is
+ * not probed and to re-try again later.
  */
 int genpd_dev_pm_attach(struct device *dev)
 {
@@ -2243,10 +2243,18 @@ int genpd_dev_pm_attach(struct device *dev)
 	if (!dev->of_node)
 		return 0;
 
+	/*
+	 * Devices with multiple PM domains must be attached separately, as we
+	 * can only attach one PM domain per device.
+	 */
+	if (of_count_phandle_with_args(dev->of_node, "power-domains",
+				       "#power-domain-cells") != 1)
+		return 0;
+
 	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
 					"#power-domain-cells", 0, &pd_args);
 	if (ret < 0)
-		return 0;
+		return ret;
 
 	mutex_lock(&gpd_list_lock);
 	pd = genpd_get_from_provider(&pd_args);
-- 
2.17.0

[PATCH v2 6/9] PM / Domains: Don't attach devices in genpd with multi PM domains

[Ulf Hansson]

The power-domain DT property may now contain a list of PM domain
specifiers, which represents that a device are partitioned across multiple
PM domains. This leads to a new situation in genpd_dev_pm_attach(), as only
one PM domain can be attached per device.

To remain things simple for the most common configuration, when a single PM
domain is used, let's treat the multiple PM domain case as being specific.

In other words, let's change genpd_dev_pm_attach() to check for multiple PM
domains and prevent it from attach any PM domain for this case. Instead,
leave this to be managed separately, from following changes to genpd.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree at vger.kernel.org
Suggested-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- Minor update to changelog to mention "PM domain specifiers" rather
	than a "list of phandles".

---
 drivers/base/power/domain.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 7ebf7993273a..12a20f21974d 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2229,10 +2229,10 @@ static void genpd_dev_pm_sync(struct device *dev)
  * attaches the device to retrieved pm_domain ops.
  *
  * Returns 1 on successfully attached PM domain, 0 when the device don't need a
- * PM domain or a negative error code in case of failures. Note that if a
- * power-domain exists for the device, but it cannot be found or turned on,
- * then return -EPROBE_DEFER to ensure that the device is not probed and to
- * re-try again later.
+ * PM domain or when multiple power-domains exists for it, else a negative error
+ * code. Note that if a power-domain exists for the device, but it cannot be
+ * found or turned on, then return -EPROBE_DEFER to ensure that the device is
+ * not probed and to re-try again later.
  */
 int genpd_dev_pm_attach(struct device *dev)
 {
@@ -2243,10 +2243,18 @@ int genpd_dev_pm_attach(struct device *dev)
 	if (!dev->of_node)
 		return 0;
 
+	/*
+	 * Devices with multiple PM domains must be attached separately, as we
+	 * can only attach one PM domain per device.
+	 */
+	if (of_count_phandle_with_args(dev->of_node, "power-domains",
+				       "#power-domain-cells") != 1)
+		return 0;
+
 	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
 					"#power-domain-cells", 0, &pd_args);
 	if (ret < 0)
-		return 0;
+		return ret;
 
 	mutex_lock(&gpd_list_lock);
 	pd = genpd_get_from_provider(&pd_args);
-- 
2.17.0

[PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()

[Ulf Hansson]

To extend genpd to deal with allowing multiple PM domains per device, some
of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
by moving some of the code into a sub-function.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 12a20f21974d..2af99bfcbe3c 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2221,38 +2221,15 @@ static void genpd_dev_pm_sync(struct device *dev)
 	genpd_queue_power_off_work(pd);
 }
 
-/**
- * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
- * @dev: Device to attach.
- *
- * Parse device's OF node to find a PM domain specifier. If such is found,
- * attaches the device to retrieved pm_domain ops.
- *
- * Returns 1 on successfully attached PM domain, 0 when the device don't need a
- * PM domain or when multiple power-domains exists for it, else a negative error
- * code. Note that if a power-domain exists for the device, but it cannot be
- * found or turned on, then return -EPROBE_DEFER to ensure that the device is
- * not probed and to re-try again later.
- */
-int genpd_dev_pm_attach(struct device *dev)
+static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
+				 unsigned int index)
 {
 	struct of_phandle_args pd_args;
 	struct generic_pm_domain *pd;
 	int ret;
 
-	if (!dev->of_node)
-		return 0;
-
-	/*
-	 * Devices with multiple PM domains must be attached separately, as we
-	 * can only attach one PM domain per device.
-	 */
-	if (of_count_phandle_with_args(dev->of_node, "power-domains",
-				       "#power-domain-cells") != 1)
-		return 0;
-
-	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
-					"#power-domain-cells", 0, &pd_args);
+	ret = of_parse_phandle_with_args(np, "power-domains",
+				"#power-domain-cells", index, &pd_args);
 	if (ret < 0)
 		return ret;
 
@@ -2290,6 +2267,35 @@ int genpd_dev_pm_attach(struct device *dev)
 
 	return ret ? -EPROBE_DEFER : 1;
 }
+
+/**
+ * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
+ * @dev: Device to attach.
+ *
+ * Parse device's OF node to find a PM domain specifier. If such is found,
+ * attaches the device to retrieved pm_domain ops.
+ *
+ * Returns 1 on successfully attached PM domain, 0 when the device don't need a
+ * PM domain or when multiple power-domains exists for it, else a negative error
+ * code. Note that if a power-domain exists for the device, but it cannot be
+ * found or turned on, then return -EPROBE_DEFER to ensure that the device is
+ * not probed and to re-try again later.
+ */
+int genpd_dev_pm_attach(struct device *dev)
+{
+	if (!dev->of_node)
+		return 0;
+
+	/*
+	 * Devices with multiple PM domains must be attached separately, as we
+	 * can only attach one PM domain per device.
+	 */
+	if (of_count_phandle_with_args(dev->of_node, "power-domains",
+				       "#power-domain-cells") != 1)
+		return 0;
+
+	return __genpd_dev_pm_attach(dev, dev->of_node, 0);
+}
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 
 static const struct of_device_id idle_state_match[] = {
-- 
2.17.0

[PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()

[Ulf Hansson]

To extend genpd to deal with allowing multiple PM domains per device, some
of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
by moving some of the code into a sub-function.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 12a20f21974d..2af99bfcbe3c 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2221,38 +2221,15 @@ static void genpd_dev_pm_sync(struct device *dev)
 	genpd_queue_power_off_work(pd);
 }
 
-/**
- * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
- * @dev: Device to attach.
- *
- * Parse device's OF node to find a PM domain specifier. If such is found,
- * attaches the device to retrieved pm_domain ops.
- *
- * Returns 1 on successfully attached PM domain, 0 when the device don't need a
- * PM domain or when multiple power-domains exists for it, else a negative error
- * code. Note that if a power-domain exists for the device, but it cannot be
- * found or turned on, then return -EPROBE_DEFER to ensure that the device is
- * not probed and to re-try again later.
- */
-int genpd_dev_pm_attach(struct device *dev)
+static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
+				 unsigned int index)
 {
 	struct of_phandle_args pd_args;
 	struct generic_pm_domain *pd;
 	int ret;
 
-	if (!dev->of_node)
-		return 0;
-
-	/*
-	 * Devices with multiple PM domains must be attached separately, as we
-	 * can only attach one PM domain per device.
-	 */
-	if (of_count_phandle_with_args(dev->of_node, "power-domains",
-				       "#power-domain-cells") != 1)
-		return 0;
-
-	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
-					"#power-domain-cells", 0, &pd_args);
+	ret = of_parse_phandle_with_args(np, "power-domains",
+				"#power-domain-cells", index, &pd_args);
 	if (ret < 0)
 		return ret;
 
@@ -2290,6 +2267,35 @@ int genpd_dev_pm_attach(struct device *dev)
 
 	return ret ? -EPROBE_DEFER : 1;
 }
+
+/**
+ * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
+ * @dev: Device to attach.
+ *
+ * Parse device's OF node to find a PM domain specifier. If such is found,
+ * attaches the device to retrieved pm_domain ops.
+ *
+ * Returns 1 on successfully attached PM domain, 0 when the device don't need a
+ * PM domain or when multiple power-domains exists for it, else a negative error
+ * code. Note that if a power-domain exists for the device, but it cannot be
+ * found or turned on, then return -EPROBE_DEFER to ensure that the device is
+ * not probed and to re-try again later.
+ */
+int genpd_dev_pm_attach(struct device *dev)
+{
+	if (!dev->of_node)
+		return 0;
+
+	/*
+	 * Devices with multiple PM domains must be attached separately, as we
+	 * can only attach one PM domain per device.
+	 */
+	if (of_count_phandle_with_args(dev->of_node, "power-domains",
+				       "#power-domain-cells") != 1)
+		return 0;
+
+	return __genpd_dev_pm_attach(dev, dev->of_node, 0);
+}
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 
 static const struct of_device_id idle_state_match[] = {
-- 
2.17.0

[PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Ulf Hansson]

To support devices being partitioned across multiple PM domains, let's
begin with extending genpd to cope with these kind of configurations.

Therefore, add a new exported function genpd_dev_pm_attach_by_id(), which
is similar to the existing genpd_dev_pm_attach(), but with the difference
that it allows its callers to provide an index to the PM domain that it
wants to attach.

Note that, genpd_dev_pm_attach_by_id() shall only be called by the driver
core / PM core, similar to how the existing dev_pm_domain_attach() makes
use of genpd_dev_pm_attach(). However, this is implemented by following
changes on top.

Because, only one PM domain can be attached per device, genpd needs to
create a virtual device that it can attach/detach instead. More precisely,
let the new function genpd_dev_pm_attach_by_id() register a virtual struct
device via calling device_register(). Then let it attach this device to the
corresponding PM domain, rather than the one that is provided by the
caller. The actual attaching is done via re-using the existing genpd OF
functions.

At successful attachment, genpd_dev_pm_attach_by_id() returns the created
virtual device, which allows the caller to operate on it to deal with power
management. Following changes on top, provides more details in this
regards.

To deal with detaching of a PM domain for the multiple PM domains case,
let's also extend the existing genpd_dev_pm_detach() function, to cover the
cleanup of the created virtual device, via make it call device_unregister()
on it. In this way, there is no need to introduce a new function to deal
with detach for the multiple PM domain case, but instead the existing one
is re-used.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- Fixed comments from Jon. Clarified function descriptions
	and changelog.

---
 drivers/base/power/domain.c | 80 +++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   |  8 ++++
 2 files changed, 88 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 2af99bfcbe3c..2b496d79159d 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_genpd_remove_last);
 
+static void genpd_release_dev(struct device *dev)
+{
+	kfree(dev);
+}
+
+static struct bus_type genpd_bus_type = {
+	.name		= "genpd",
+};
+
 /**
  * genpd_dev_pm_detach - Detach a device from its PM domain.
  * @dev: Device to detach.
@@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
 
 	/* Check if PM domain can be powered off after removing this device. */
 	genpd_queue_power_off_work(pd);
+
+	/* Unregister the device if it was created by genpd. */
+	if (dev->bus == &genpd_bus_type)
+		device_unregister(dev);
 }
 
 static void genpd_dev_pm_sync(struct device *dev)
@@ -2298,6 +2311,67 @@ int genpd_dev_pm_attach(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 
+/**
+ * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
+ * @dev: Device to attach.
+ * @index: The index of the PM domain.
+ *
+ * Parse device's OF node to find a PM domain specifier at the provided @index.
+ * If such is found, creates a virtual device and attaches it to the retrieved
+ * pm_domain ops. To deal with detaching of the virtual device, the ->detach()
+ * callback in the struct dev_pm_domain are assigned to genpd_dev_pm_detach().
+ *
+ * Returns the created virtual device if successfully attached PM domain, NULL
+ * when the device don't need a PM domain, else an ERR_PTR() in case of
+ * failures. If a power-domain exists for the device, but cannot be found or
+ * turned on, then ERR_PTR(-EPROBE_DEFER) is returned to ensure that the device
+ * is not probed and to re-try again later.
+ */
+struct device *genpd_dev_pm_attach_by_id(struct device *dev,
+					 unsigned int index)
+{
+	struct device *genpd_dev;
+	int num_domains;
+	int ret;
+
+	if (!dev->of_node)
+		return NULL;
+
+	/* Deal only with devices using multiple PM domains. */
+	num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
+						 "#power-domain-cells");
+	if (num_domains < 2 || index >= num_domains)
+		return NULL;
+
+	/* Allocate and register device on the genpd bus. */
+	genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
+	if (!genpd_dev)
+		return ERR_PTR(-ENOMEM);
+
+	dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
+	genpd_dev->bus = &genpd_bus_type;
+	genpd_dev->release = genpd_release_dev;
+
+	ret = device_register(genpd_dev);
+	if (ret) {
+		kfree(genpd_dev);
+		return ERR_PTR(ret);
+	}
+
+	/* Try to attach the device to the PM domain at the specified index. */
+	ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
+	if (ret < 1) {
+		device_unregister(genpd_dev);
+		return ret ? ERR_PTR(ret) : NULL;
+	}
+
+	pm_runtime_set_active(genpd_dev);
+	pm_runtime_enable(genpd_dev);
+
+	return genpd_dev;
+}
+EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);
+
 static const struct of_device_id idle_state_match[] = {
 	{ .compatible = "domain-idle-state", },
 	{ }
@@ -2457,6 +2531,12 @@ unsigned int of_genpd_opp_to_performance_state(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(of_genpd_opp_to_performance_state);
 
+static int __init genpd_bus_init(void)
+{
+	return bus_register(&genpd_bus_type);
+}
+core_initcall(genpd_bus_init);
+
 #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 42e0d649e653..82458e8e2e01 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -237,6 +237,8 @@ unsigned int of_genpd_opp_to_performance_state(struct device *dev,
 				struct device_node *opp_node);
 
 int genpd_dev_pm_attach(struct device *dev);
+struct device *genpd_dev_pm_attach_by_id(struct device *dev,
+					 unsigned int index);
 #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
 static inline int of_genpd_add_provider_simple(struct device_node *np,
 					struct generic_pm_domain *genpd)
@@ -282,6 +284,12 @@ static inline int genpd_dev_pm_attach(struct device *dev)
 	return 0;
 }
 
+static inline struct device *genpd_dev_pm_attach_by_id(struct device *dev,
+						       unsigned int index)
+{
+	return NULL;
+}
+
 static inline
 struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 {
-- 
2.17.0

[PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Ulf Hansson]

To support devices being partitioned across multiple PM domains, let's
begin with extending genpd to cope with these kind of configurations.

Therefore, add a new exported function genpd_dev_pm_attach_by_id(), which
is similar to the existing genpd_dev_pm_attach(), but with the difference
that it allows its callers to provide an index to the PM domain that it
wants to attach.

Note that, genpd_dev_pm_attach_by_id() shall only be called by the driver
core / PM core, similar to how the existing dev_pm_domain_attach() makes
use of genpd_dev_pm_attach(). However, this is implemented by following
changes on top.

Because, only one PM domain can be attached per device, genpd needs to
create a virtual device that it can attach/detach instead. More precisely,
let the new function genpd_dev_pm_attach_by_id() register a virtual struct
device via calling device_register(). Then let it attach this device to the
corresponding PM domain, rather than the one that is provided by the
caller. The actual attaching is done via re-using the existing genpd OF
functions.

At successful attachment, genpd_dev_pm_attach_by_id() returns the created
virtual device, which allows the caller to operate on it to deal with power
management. Following changes on top, provides more details in this
regards.

To deal with detaching of a PM domain for the multiple PM domains case,
let's also extend the existing genpd_dev_pm_detach() function, to cover the
cleanup of the created virtual device, via make it call device_unregister()
on it. In this way, there is no need to introduce a new function to deal
with detach for the multiple PM domain case, but instead the existing one
is re-used.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- Fixed comments from Jon. Clarified function descriptions
	and changelog.

---
 drivers/base/power/domain.c | 80 +++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   |  8 ++++
 2 files changed, 88 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 2af99bfcbe3c..2b496d79159d 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_genpd_remove_last);
 
+static void genpd_release_dev(struct device *dev)
+{
+	kfree(dev);
+}
+
+static struct bus_type genpd_bus_type = {
+	.name		= "genpd",
+};
+
 /**
  * genpd_dev_pm_detach - Detach a device from its PM domain.
  * @dev: Device to detach.
@@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
 
 	/* Check if PM domain can be powered off after removing this device. */
 	genpd_queue_power_off_work(pd);
+
+	/* Unregister the device if it was created by genpd. */
+	if (dev->bus == &genpd_bus_type)
+		device_unregister(dev);
 }
 
 static void genpd_dev_pm_sync(struct device *dev)
@@ -2298,6 +2311,67 @@ int genpd_dev_pm_attach(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 
+/**
+ * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
+ * @dev: Device to attach.
+ * @index: The index of the PM domain.
+ *
+ * Parse device's OF node to find a PM domain specifier at the provided @index.
+ * If such is found, creates a virtual device and attaches it to the retrieved
+ * pm_domain ops. To deal with detaching of the virtual device, the ->detach()
+ * callback in the struct dev_pm_domain are assigned to genpd_dev_pm_detach().
+ *
+ * Returns the created virtual device if successfully attached PM domain, NULL
+ * when the device don't need a PM domain, else an ERR_PTR() in case of
+ * failures. If a power-domain exists for the device, but cannot be found or
+ * turned on, then ERR_PTR(-EPROBE_DEFER) is returned to ensure that the device
+ * is not probed and to re-try again later.
+ */
+struct device *genpd_dev_pm_attach_by_id(struct device *dev,
+					 unsigned int index)
+{
+	struct device *genpd_dev;
+	int num_domains;
+	int ret;
+
+	if (!dev->of_node)
+		return NULL;
+
+	/* Deal only with devices using multiple PM domains. */
+	num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
+						 "#power-domain-cells");
+	if (num_domains < 2 || index >= num_domains)
+		return NULL;
+
+	/* Allocate and register device on the genpd bus. */
+	genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
+	if (!genpd_dev)
+		return ERR_PTR(-ENOMEM);
+
+	dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
+	genpd_dev->bus = &genpd_bus_type;
+	genpd_dev->release = genpd_release_dev;
+
+	ret = device_register(genpd_dev);
+	if (ret) {
+		kfree(genpd_dev);
+		return ERR_PTR(ret);
+	}
+
+	/* Try to attach the device to the PM domain at the specified index. */
+	ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
+	if (ret < 1) {
+		device_unregister(genpd_dev);
+		return ret ? ERR_PTR(ret) : NULL;
+	}
+
+	pm_runtime_set_active(genpd_dev);
+	pm_runtime_enable(genpd_dev);
+
+	return genpd_dev;
+}
+EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);
+
 static const struct of_device_id idle_state_match[] = {
 	{ .compatible = "domain-idle-state", },
 	{ }
@@ -2457,6 +2531,12 @@ unsigned int of_genpd_opp_to_performance_state(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(of_genpd_opp_to_performance_state);
 
+static int __init genpd_bus_init(void)
+{
+	return bus_register(&genpd_bus_type);
+}
+core_initcall(genpd_bus_init);
+
 #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 42e0d649e653..82458e8e2e01 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -237,6 +237,8 @@ unsigned int of_genpd_opp_to_performance_state(struct device *dev,
 				struct device_node *opp_node);
 
 int genpd_dev_pm_attach(struct device *dev);
+struct device *genpd_dev_pm_attach_by_id(struct device *dev,
+					 unsigned int index);
 #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */
 static inline int of_genpd_add_provider_simple(struct device_node *np,
 					struct generic_pm_domain *genpd)
@@ -282,6 +284,12 @@ static inline int genpd_dev_pm_attach(struct device *dev)
 	return 0;
 }
 
+static inline struct device *genpd_dev_pm_attach_by_id(struct device *dev,
+						       unsigned int index)
+{
+	return NULL;
+}
+
 static inline
 struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 {
-- 
2.17.0

[PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Ulf Hansson]

The existing dev_pm_domain_attach() function, allows a single PM domain to
be attached per device. To be able to support devices that are partitioned
across multiple PM domains, let's introduce a new interface,
dev_pm_domain_attach_by_id().

The dev_pm_domain_attach_by_id() returns a new allocated struct device with
the corresponding attached PM domain. This enables for example a driver to
operate on the new device from a power management point of view. The driver
may then also benefit from using the received device, to set up so called
device-links towards its original device. Depending on the situation, these
links may then be dynamically changed.

The new interface is typically called by drivers during their probe phase,
in case they manages devices which uses multiple PM domains. If that is the
case, the driver also becomes responsible of managing the detaching of the
PM domains, which typically should be done at the remove phase. Detaching
is done by calling the existing dev_pm_domain_detach() function and for
each of the received devices from dev_pm_domain_attach_by_id().

Note, currently its only genpd that supports multiple PM domains per
device, but dev_pm_domain_attach_by_id() can easily by extended to cover
other PM domain types, if/when needed.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- Fixed comments from Jon. Clarified function descriptions/changelog and
	return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
	- Fix build error when CONFIG_PM is unset.

---
 drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
 include/linux/pm_domain.h   |  7 ++++++
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index 7ae62b6355b8..5e5ea0c239de 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c
@@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
 }
 EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
 
+/**
+ * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
+ * @dev: Device to attach.
+ * @index: The index of the PM domain.
+ *
+ * As @dev may only be attached to a single PM domain, the backend PM domain
+ * provider creates a virtual device to attach instead. If attachment succeeds,
+ * the ->detach() callback in the struct dev_pm_domain are assigned by the
+ * corresponding backend attach function, as to deal with detaching of the
+ * created virtual device.
+ *
+ * This function should typically be invoked by a driver during the probe phase,
+ * in case its device requires power management through multiple PM domains. The
+ * driver may benefit from using the received device, to configure device-links
+ * towards its original device. Depending on the use-case and if needed, the
+ * links may be dynamically changed by the driver, which allows it to control
+ * the power to the PM domains independently from each other.
+ *
+ * Callers must ensure proper synchronization of this function with power
+ * management callbacks.
+ *
+ * Returns the virtual created device when successfully attached to its PM
+ * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
+ * Note that, to detach the returned virtual device, the driver shall call
+ * dev_pm_domain_detach() on it, typically during the remove phase.
+ */
+struct device *dev_pm_domain_attach_by_id(struct device *dev,
+					  unsigned int index)
+{
+	if (dev->pm_domain)
+		return ERR_PTR(-EEXIST);
+
+	return genpd_dev_pm_attach_by_id(dev, index);
+}
+EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
+
 /**
  * dev_pm_domain_detach - Detach a device from its PM domain.
  * @dev: Device to detach.
  * @power_off: Used to indicate whether we should power off the device.
  *
- * This functions will reverse the actions from dev_pm_domain_attach() and thus
- * try to detach the @dev from its PM domain. Typically it should be invoked
- * from subsystem level code during the remove phase.
+ * This functions will reverse the actions from dev_pm_domain_attach() and
+ * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
+ * Typically it should be invoked during the remove phase, either from
+ * subsystem level code or from drivers.
  *
  * Callers must ensure proper synchronization of this function with power
  * management callbacks.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 82458e8e2e01..9206a4fef9ac 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 
 #ifdef CONFIG_PM
 int dev_pm_domain_attach(struct device *dev, bool power_on);
+struct device *dev_pm_domain_attach_by_id(struct device *dev,
+					  unsigned int index);
 void dev_pm_domain_detach(struct device *dev, bool power_off);
 void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
 #else
@@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
 {
 	return 0;
 }
+static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
+							unsigned int index)
+{
+	return NULL;
+}
 static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
 static inline void dev_pm_domain_set(struct device *dev,
 				     struct dev_pm_domain *pd) {}
-- 
2.17.0

[PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Ulf Hansson]

The existing dev_pm_domain_attach() function, allows a single PM domain to
be attached per device. To be able to support devices that are partitioned
across multiple PM domains, let's introduce a new interface,
dev_pm_domain_attach_by_id().

The dev_pm_domain_attach_by_id() returns a new allocated struct device with
the corresponding attached PM domain. This enables for example a driver to
operate on the new device from a power management point of view. The driver
may then also benefit from using the received device, to set up so called
device-links towards its original device. Depending on the situation, these
links may then be dynamically changed.

The new interface is typically called by drivers during their probe phase,
in case they manages devices which uses multiple PM domains. If that is the
case, the driver also becomes responsible of managing the detaching of the
PM domains, which typically should be done at the remove phase. Detaching
is done by calling the existing dev_pm_domain_detach() function and for
each of the received devices from dev_pm_domain_attach_by_id().

Note, currently its only genpd that supports multiple PM domains per
device, but dev_pm_domain_attach_by_id() can easily by extended to cover
other PM domain types, if/when needed.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	- Fixed comments from Jon. Clarified function descriptions/changelog and
	return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
	- Fix build error when CONFIG_PM is unset.

---
 drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
 include/linux/pm_domain.h   |  7 ++++++
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index 7ae62b6355b8..5e5ea0c239de 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c
@@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
 }
 EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
 
+/**
+ * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
+ * @dev: Device to attach.
+ * @index: The index of the PM domain.
+ *
+ * As @dev may only be attached to a single PM domain, the backend PM domain
+ * provider creates a virtual device to attach instead. If attachment succeeds,
+ * the ->detach() callback in the struct dev_pm_domain are assigned by the
+ * corresponding backend attach function, as to deal with detaching of the
+ * created virtual device.
+ *
+ * This function should typically be invoked by a driver during the probe phase,
+ * in case its device requires power management through multiple PM domains. The
+ * driver may benefit from using the received device, to configure device-links
+ * towards its original device. Depending on the use-case and if needed, the
+ * links may be dynamically changed by the driver, which allows it to control
+ * the power to the PM domains independently from each other.
+ *
+ * Callers must ensure proper synchronization of this function with power
+ * management callbacks.
+ *
+ * Returns the virtual created device when successfully attached to its PM
+ * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
+ * Note that, to detach the returned virtual device, the driver shall call
+ * dev_pm_domain_detach() on it, typically during the remove phase.
+ */
+struct device *dev_pm_domain_attach_by_id(struct device *dev,
+					  unsigned int index)
+{
+	if (dev->pm_domain)
+		return ERR_PTR(-EEXIST);
+
+	return genpd_dev_pm_attach_by_id(dev, index);
+}
+EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
+
 /**
  * dev_pm_domain_detach - Detach a device from its PM domain.
  * @dev: Device to detach.
  * @power_off: Used to indicate whether we should power off the device.
  *
- * This functions will reverse the actions from dev_pm_domain_attach() and thus
- * try to detach the @dev from its PM domain. Typically it should be invoked
- * from subsystem level code during the remove phase.
+ * This functions will reverse the actions from dev_pm_domain_attach() and
+ * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
+ * Typically it should be invoked during the remove phase, either from
+ * subsystem level code or from drivers.
  *
  * Callers must ensure proper synchronization of this function with power
  * management callbacks.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 82458e8e2e01..9206a4fef9ac 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
 
 #ifdef CONFIG_PM
 int dev_pm_domain_attach(struct device *dev, bool power_on);
+struct device *dev_pm_domain_attach_by_id(struct device *dev,
+					  unsigned int index);
 void dev_pm_domain_detach(struct device *dev, bool power_off);
 void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
 #else
@@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
 {
 	return 0;
 }
+static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
+							unsigned int index)
+{
+	return NULL;
+}
 static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
 static inline void dev_pm_domain_set(struct device *dev,
 				     struct dev_pm_domain *pd) {}
-- 
2.17.0

Re: [PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Jon Hunter]

Hi Ulf,

On 29/05/18 11:04, Ulf Hansson wrote:
> The existing dev_pm_domain_attach() function, allows a single PM domain to
> be attached per device. To be able to support devices that are partitioned
> across multiple PM domains, let's introduce a new interface,
> dev_pm_domain_attach_by_id().
> 
> The dev_pm_domain_attach_by_id() returns a new allocated struct device with
> the corresponding attached PM domain. This enables for example a driver to
> operate on the new device from a power management point of view. The driver
> may then also benefit from using the received device, to set up so called
> device-links towards its original device. Depending on the situation, these
> links may then be dynamically changed.

I have given this series a go with Tegra updating the XHCI driver to make
use of these new APIs. Good news it does appear to work fine for Tegra,
however, initially when looking at the device_link_add() API ...

/**
 * device_link_add - Create a link between two devices.
 * @consumer: Consumer end of the link.
 * @supplier: Supplier end of the link.
 * @flags: Link flags.

 ... I had assumed that the 'consumer' device would be the actual XHCI 
device (in the case of Tegra) and the 'supplier' device would be the new
genpd device. However, this did not work and I got the following WARN on
boot ...

[    2.050929] ---[ end trace eff0b5265e530c92 ]---
[    2.055567] WARNING: CPU: 2 PID: 1 at drivers/base/core.c:446 device_links_driver_bound+0xc0/0xd0
[    2.064422] Modules linked in:
[    2.067471] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G        W         4.17.0-rc7-next-20180529-00011-g4faf0dc0ebf3-dirty #32
[    2.078667] Hardware name: Google Pixel C (DT)
[    2.083101] pstate: 80000005 (Nzcv daif -PAN -UAO)
[    2.087881] pc : device_links_driver_bound+0xc0/0xd0
[    2.092832] lr : device_links_driver_bound+0x20/0xd0

Switching the Tegra XHCI device to be the 'supplier' and genpd device to
be the 'consumer' does work, but is this correct? Seems to be opposite to
what I expected. Maybe I am missing something?

> The new interface is typically called by drivers during their probe phase,
> in case they manages devices which uses multiple PM domains. If that is the
> case, the driver also becomes responsible of managing the detaching of the
> PM domains, which typically should be done at the remove phase. Detaching
> is done by calling the existing dev_pm_domain_detach() function and for
> each of the received devices from dev_pm_domain_attach_by_id().
> 
> Note, currently its only genpd that supports multiple PM domains per
> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
> other PM domain types, if/when needed.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Fixed comments from Jon. Clarified function descriptions/changelog and
> 	return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
> 	- Fix build error when CONFIG_PM is unset.
> 
> ---
>  drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
>  include/linux/pm_domain.h   |  7 ++++++
>  2 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
> index 7ae62b6355b8..5e5ea0c239de 100644
> --- a/drivers/base/power/common.c
> +++ b/drivers/base/power/common.c
> @@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>  
> +/**
> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
> + * @dev: Device to attach.

Nit ... I still don't think this is the device we are attaching to, but the
device the PM domains are associated with. IOW we are using this device to
lookup the PM domains.

> + * @index: The index of the PM domain.
> + *
> + * As @dev may only be attached to a single PM domain, the backend PM domain
> + * provider creates a virtual device to attach instead. If attachment succeeds,
> + * the ->detach() callback in the struct dev_pm_domain are assigned by the
> + * corresponding backend attach function, as to deal with detaching of the
> + * created virtual device.
> + *
> + * This function should typically be invoked by a driver during the probe phase,
> + * in case its device requires power management through multiple PM domains. The
> + * driver may benefit from using the received device, to configure device-links
> + * towards its original device. Depending on the use-case and if needed, the
> + * links may be dynamically changed by the driver, which allows it to control
> + * the power to the PM domains independently from each other.
> + *
> + * Callers must ensure proper synchronization of this function with power
> + * management callbacks.
> + *
> + * Returns the virtual created device when successfully attached to its PM
> + * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
> + * Note that, to detach the returned virtual device, the driver shall call
> + * dev_pm_domain_detach() on it, typically during the remove phase.
> + */
> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +					  unsigned int index)
> +{
> +	if (dev->pm_domain)
> +		return ERR_PTR(-EEXIST);
> +
> +	return genpd_dev_pm_attach_by_id(dev, index);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
> +
>  /**
>   * dev_pm_domain_detach - Detach a device from its PM domain.
>   * @dev: Device to detach.
>   * @power_off: Used to indicate whether we should power off the device.
>   *
> - * This functions will reverse the actions from dev_pm_domain_attach() and thus
> - * try to detach the @dev from its PM domain. Typically it should be invoked
> - * from subsystem level code during the remove phase.
> + * This functions will reverse the actions from dev_pm_domain_attach() and
> + * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
> + * Typically it should be invoked during the remove phase, either from
> + * subsystem level code or from drivers.
>   *
>   * Callers must ensure proper synchronization of this function with power
>   * management callbacks.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 82458e8e2e01..9206a4fef9ac 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>  
>  #ifdef CONFIG_PM
>  int dev_pm_domain_attach(struct device *dev, bool power_on);
> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +					  unsigned int index);
>  void dev_pm_domain_detach(struct device *dev, bool power_off);
>  void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
>  #else
> @@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
>  {
>  	return 0;
>  }
> +static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +							unsigned int index)
> +{
> +	return NULL;
> +}
>  static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
>  static inline void dev_pm_domain_set(struct device *dev,
>  				     struct dev_pm_domain *pd) {}
> 

My only other comments on this series are ...

1. I think it would be nice to have an dev_pm_domain_attach_by_name() and 
   that the DT binding has a 'power-domain-names' property.
2. I am wondering if there could be value in having a
   dev_pm_domain_attach_link_all() helper which would attach and link all
   PM domains at once. 

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Jon Hunter]

Hi Ulf,

On 29/05/18 11:04, Ulf Hansson wrote:
> The existing dev_pm_domain_attach() function, allows a single PM domain to
> be attached per device. To be able to support devices that are partitioned
> across multiple PM domains, let's introduce a new interface,
> dev_pm_domain_attach_by_id().
> 
> The dev_pm_domain_attach_by_id() returns a new allocated struct device with
> the corresponding attached PM domain. This enables for example a driver to
> operate on the new device from a power management point of view. The driver
> may then also benefit from using the received device, to set up so called
> device-links towards its original device. Depending on the situation, these
> links may then be dynamically changed.

I have given this series a go with Tegra updating the XHCI driver to make
use of these new APIs. Good news it does appear to work fine for Tegra,
however, initially when looking at the device_link_add() API ...

/**
 * device_link_add - Create a link between two devices.
 * @consumer: Consumer end of the link.
 * @supplier: Supplier end of the link.
 * @flags: Link flags.

 ... I had assumed that the 'consumer' device would be the actual XHCI 
device (in the case of Tegra) and the 'supplier' device would be the new
genpd device. However, this did not work and I got the following WARN on
boot ...

[    2.050929] ---[ end trace eff0b5265e530c92 ]---
[    2.055567] WARNING: CPU: 2 PID: 1 at drivers/base/core.c:446 device_links_driver_bound+0xc0/0xd0
[    2.064422] Modules linked in:
[    2.067471] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G        W         4.17.0-rc7-next-20180529-00011-g4faf0dc0ebf3-dirty #32
[    2.078667] Hardware name: Google Pixel C (DT)
[    2.083101] pstate: 80000005 (Nzcv daif -PAN -UAO)
[    2.087881] pc : device_links_driver_bound+0xc0/0xd0
[    2.092832] lr : device_links_driver_bound+0x20/0xd0

Switching the Tegra XHCI device to be the 'supplier' and genpd device to
be the 'consumer' does work, but is this correct? Seems to be opposite to
what I expected. Maybe I am missing something?

> The new interface is typically called by drivers during their probe phase,
> in case they manages devices which uses multiple PM domains. If that is the
> case, the driver also becomes responsible of managing the detaching of the
> PM domains, which typically should be done at the remove phase. Detaching
> is done by calling the existing dev_pm_domain_detach() function and for
> each of the received devices from dev_pm_domain_attach_by_id().
> 
> Note, currently its only genpd that supports multiple PM domains per
> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
> other PM domain types, if/when needed.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Fixed comments from Jon. Clarified function descriptions/changelog and
> 	return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
> 	- Fix build error when CONFIG_PM is unset.
> 
> ---
>  drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
>  include/linux/pm_domain.h   |  7 ++++++
>  2 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
> index 7ae62b6355b8..5e5ea0c239de 100644
> --- a/drivers/base/power/common.c
> +++ b/drivers/base/power/common.c
> @@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>  
> +/**
> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
> + * @dev: Device to attach.

Nit ... I still don't think this is the device we are attaching to, but the
device the PM domains are associated with. IOW we are using this device to
lookup the PM domains.

> + * @index: The index of the PM domain.
> + *
> + * As @dev may only be attached to a single PM domain, the backend PM domain
> + * provider creates a virtual device to attach instead. If attachment succeeds,
> + * the ->detach() callback in the struct dev_pm_domain are assigned by the
> + * corresponding backend attach function, as to deal with detaching of the
> + * created virtual device.
> + *
> + * This function should typically be invoked by a driver during the probe phase,
> + * in case its device requires power management through multiple PM domains. The
> + * driver may benefit from using the received device, to configure device-links
> + * towards its original device. Depending on the use-case and if needed, the
> + * links may be dynamically changed by the driver, which allows it to control
> + * the power to the PM domains independently from each other.
> + *
> + * Callers must ensure proper synchronization of this function with power
> + * management callbacks.
> + *
> + * Returns the virtual created device when successfully attached to its PM
> + * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
> + * Note that, to detach the returned virtual device, the driver shall call
> + * dev_pm_domain_detach() on it, typically during the remove phase.
> + */
> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +					  unsigned int index)
> +{
> +	if (dev->pm_domain)
> +		return ERR_PTR(-EEXIST);
> +
> +	return genpd_dev_pm_attach_by_id(dev, index);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
> +
>  /**
>   * dev_pm_domain_detach - Detach a device from its PM domain.
>   * @dev: Device to detach.
>   * @power_off: Used to indicate whether we should power off the device.
>   *
> - * This functions will reverse the actions from dev_pm_domain_attach() and thus
> - * try to detach the @dev from its PM domain. Typically it should be invoked
> - * from subsystem level code during the remove phase.
> + * This functions will reverse the actions from dev_pm_domain_attach() and
> + * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
> + * Typically it should be invoked during the remove phase, either from
> + * subsystem level code or from drivers.
>   *
>   * Callers must ensure proper synchronization of this function with power
>   * management callbacks.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 82458e8e2e01..9206a4fef9ac 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>  
>  #ifdef CONFIG_PM
>  int dev_pm_domain_attach(struct device *dev, bool power_on);
> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +					  unsigned int index);
>  void dev_pm_domain_detach(struct device *dev, bool power_off);
>  void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
>  #else
> @@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
>  {
>  	return 0;
>  }
> +static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +							unsigned int index)
> +{
> +	return NULL;
> +}
>  static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
>  static inline void dev_pm_domain_set(struct device *dev,
>  				     struct dev_pm_domain *pd) {}
> 

My only other comments on this series are ...

1. I think it would be nice to have an dev_pm_domain_attach_by_name() and 
   that the DT binding has a 'power-domain-names' property.
2. I am wondering if there could be value in having a
   dev_pm_domain_attach_link_all() helper which would attach and link all
   PM domains at once. 

Cheers
Jon

-- 
nvpublic

[PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Jon Hunter]

Hi Ulf,

On 29/05/18 11:04, Ulf Hansson wrote:
> The existing dev_pm_domain_attach() function, allows a single PM domain to
> be attached per device. To be able to support devices that are partitioned
> across multiple PM domains, let's introduce a new interface,
> dev_pm_domain_attach_by_id().
> 
> The dev_pm_domain_attach_by_id() returns a new allocated struct device with
> the corresponding attached PM domain. This enables for example a driver to
> operate on the new device from a power management point of view. The driver
> may then also benefit from using the received device, to set up so called
> device-links towards its original device. Depending on the situation, these
> links may then be dynamically changed.

I have given this series a go with Tegra updating the XHCI driver to make
use of these new APIs. Good news it does appear to work fine for Tegra,
however, initially when looking at the device_link_add() API ...

/**
 * device_link_add - Create a link between two devices.
 * @consumer: Consumer end of the link.
 * @supplier: Supplier end of the link.
 * @flags: Link flags.

 ... I had assumed that the 'consumer' device would be the actual XHCI 
device (in the case of Tegra) and the 'supplier' device would be the new
genpd device. However, this did not work and I got the following WARN on
boot ...

[    2.050929] ---[ end trace eff0b5265e530c92 ]---
[    2.055567] WARNING: CPU: 2 PID: 1 at drivers/base/core.c:446 device_links_driver_bound+0xc0/0xd0
[    2.064422] Modules linked in:
[    2.067471] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G        W         4.17.0-rc7-next-20180529-00011-g4faf0dc0ebf3-dirty #32
[    2.078667] Hardware name: Google Pixel C (DT)
[    2.083101] pstate: 80000005 (Nzcv daif -PAN -UAO)
[    2.087881] pc : device_links_driver_bound+0xc0/0xd0
[    2.092832] lr : device_links_driver_bound+0x20/0xd0

Switching the Tegra XHCI device to be the 'supplier' and genpd device to
be the 'consumer' does work, but is this correct? Seems to be opposite to
what I expected. Maybe I am missing something?

> The new interface is typically called by drivers during their probe phase,
> in case they manages devices which uses multiple PM domains. If that is the
> case, the driver also becomes responsible of managing the detaching of the
> PM domains, which typically should be done at the remove phase. Detaching
> is done by calling the existing dev_pm_domain_detach() function and for
> each of the received devices from dev_pm_domain_attach_by_id().
> 
> Note, currently its only genpd that supports multiple PM domains per
> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
> other PM domain types, if/when needed.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Fixed comments from Jon. Clarified function descriptions/changelog and
> 	return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
> 	- Fix build error when CONFIG_PM is unset.
> 
> ---
>  drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
>  include/linux/pm_domain.h   |  7 ++++++
>  2 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
> index 7ae62b6355b8..5e5ea0c239de 100644
> --- a/drivers/base/power/common.c
> +++ b/drivers/base/power/common.c
> @@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>  
> +/**
> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
> + * @dev: Device to attach.

Nit ... I still don't think this is the device we are attaching to, but the
device the PM domains are associated with. IOW we are using this device to
lookup the PM domains.

> + * @index: The index of the PM domain.
> + *
> + * As @dev may only be attached to a single PM domain, the backend PM domain
> + * provider creates a virtual device to attach instead. If attachment succeeds,
> + * the ->detach() callback in the struct dev_pm_domain are assigned by the
> + * corresponding backend attach function, as to deal with detaching of the
> + * created virtual device.
> + *
> + * This function should typically be invoked by a driver during the probe phase,
> + * in case its device requires power management through multiple PM domains. The
> + * driver may benefit from using the received device, to configure device-links
> + * towards its original device. Depending on the use-case and if needed, the
> + * links may be dynamically changed by the driver, which allows it to control
> + * the power to the PM domains independently from each other.
> + *
> + * Callers must ensure proper synchronization of this function with power
> + * management callbacks.
> + *
> + * Returns the virtual created device when successfully attached to its PM
> + * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
> + * Note that, to detach the returned virtual device, the driver shall call
> + * dev_pm_domain_detach() on it, typically during the remove phase.
> + */
> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +					  unsigned int index)
> +{
> +	if (dev->pm_domain)
> +		return ERR_PTR(-EEXIST);
> +
> +	return genpd_dev_pm_attach_by_id(dev, index);
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
> +
>  /**
>   * dev_pm_domain_detach - Detach a device from its PM domain.
>   * @dev: Device to detach.
>   * @power_off: Used to indicate whether we should power off the device.
>   *
> - * This functions will reverse the actions from dev_pm_domain_attach() and thus
> - * try to detach the @dev from its PM domain. Typically it should be invoked
> - * from subsystem level code during the remove phase.
> + * This functions will reverse the actions from dev_pm_domain_attach() and
> + * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
> + * Typically it should be invoked during the remove phase, either from
> + * subsystem level code or from drivers.
>   *
>   * Callers must ensure proper synchronization of this function with power
>   * management callbacks.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 82458e8e2e01..9206a4fef9ac 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>  
>  #ifdef CONFIG_PM
>  int dev_pm_domain_attach(struct device *dev, bool power_on);
> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +					  unsigned int index);
>  void dev_pm_domain_detach(struct device *dev, bool power_off);
>  void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
>  #else
> @@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
>  {
>  	return 0;
>  }
> +static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
> +							unsigned int index)
> +{
> +	return NULL;
> +}
>  static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
>  static inline void dev_pm_domain_set(struct device *dev,
>  				     struct dev_pm_domain *pd) {}
> 

My only other comments on this series are ...

1. I think it would be nice to have an dev_pm_domain_attach_by_name() and 
   that the DT binding has a 'power-domain-names' property.
2. I am wondering if there could be value in having a
   dev_pm_domain_attach_link_all() helper which would attach and link all
   PM domains at once. 

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Rafael J. Wysocki]

On Tue, May 29, 2018 at 12:04 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Changes in v2:
>         - Addressed comments from Geert around DT doc.
>         - Addressed comments from Jon around clarification of how to use this
>         and changes to returned error codes.
>         - Fixed build error in case CONFIG_PM was unset.
>
> There are devices that are partitioned across multiple PM domains. Currently
> these can't be supported well by the available PM infrastructures we have in
> the kernel. This series is an attempt to address this.
>
> The interesting parts happens from patch 5 an onwards, including a minor DT
> update to the existing power-domain bindings, the 4 earlier are just trivial
> clean-ups of some related code in genpd, which I happened to stumble over.
>
> Some additional background:
>
> One existing case where devices are partitioned across multiple PM domains, is
> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
> series, trying to address these issues, however this is a new approach, while
> it re-uses the same concepts from DT point of view.
>
> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
> controller. Each controller have its own independent PM domain, but are being
> partitioned across another shared PM domain for the USB super-speed logic.
>
> Currently to make the drivers work, either the related PM domains needs to stay
> powered on always or the PM domain topology needs to be in-correctly modelled
> through sub-domains. In both cases PM domains may be powered on while they
> don't need to be, so in the end this means - wasting power -.
>
> As stated above, this series intends to address these problem from a PM
> infrastructure point of view. More details are available in each changelog.
>
> It should be noted that this series has been tested on HW, however only by using
> a home-cooked test PM domain driver for genpd and together with a test driver.
> This allowed me to play with PM domain (genpd), runtime PM and device links.
>
> Any further deployment for real use cases are greatly appreciated. I am happy to
> to help, if needed!
>
> Kind regards
> Ulf Hansson
>
>
> Ulf Hansson (9):
>   PM / Domains: Drop extern declarations of functions in pm_domain.h
>   PM / Domains: Drop __pm_genpd_add_device()
>   PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()
>   PM / Domains: Drop unused parameter in genpd_allocate_dev_data()
>   PM / Domains: dt: Allow power-domain property to be a list of
>     specifiers
>   PM / Domains: Don't attach devices in genpd with multi PM domains
>   PM / Domains: Split genpd_dev_pm_attach()
>   PM / Domains: Add support for multi PM domains per device to genpd
>   PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM
>     domains
>
>  .../bindings/power/power_domain.txt           |  19 ++-
>  drivers/base/power/common.c                   |  39 ++++-
>  drivers/base/power/domain.c                   | 155 ++++++++++++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c       |   2 +-
>  include/linux/pm_domain.h                     |  79 ++++-----
>  5 files changed, 216 insertions(+), 78 deletions(-)

I can take patches [1-4/9] from this series for 4.18.

I need an ACK from Rob on the bindings change.

The rest of the series doesn't seem to be ready yet.

Thanks,
Rafael

[PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Rafael J. Wysocki]

On Tue, May 29, 2018 at 12:04 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Changes in v2:
>         - Addressed comments from Geert around DT doc.
>         - Addressed comments from Jon around clarification of how to use this
>         and changes to returned error codes.
>         - Fixed build error in case CONFIG_PM was unset.
>
> There are devices that are partitioned across multiple PM domains. Currently
> these can't be supported well by the available PM infrastructures we have in
> the kernel. This series is an attempt to address this.
>
> The interesting parts happens from patch 5 an onwards, including a minor DT
> update to the existing power-domain bindings, the 4 earlier are just trivial
> clean-ups of some related code in genpd, which I happened to stumble over.
>
> Some additional background:
>
> One existing case where devices are partitioned across multiple PM domains, is
> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
> series, trying to address these issues, however this is a new approach, while
> it re-uses the same concepts from DT point of view.
>
> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
> controller. Each controller have its own independent PM domain, but are being
> partitioned across another shared PM domain for the USB super-speed logic.
>
> Currently to make the drivers work, either the related PM domains needs to stay
> powered on always or the PM domain topology needs to be in-correctly modelled
> through sub-domains. In both cases PM domains may be powered on while they
> don't need to be, so in the end this means - wasting power -.
>
> As stated above, this series intends to address these problem from a PM
> infrastructure point of view. More details are available in each changelog.
>
> It should be noted that this series has been tested on HW, however only by using
> a home-cooked test PM domain driver for genpd and together with a test driver.
> This allowed me to play with PM domain (genpd), runtime PM and device links.
>
> Any further deployment for real use cases are greatly appreciated. I am happy to
> to help, if needed!
>
> Kind regards
> Ulf Hansson
>
>
> Ulf Hansson (9):
>   PM / Domains: Drop extern declarations of functions in pm_domain.h
>   PM / Domains: Drop __pm_genpd_add_device()
>   PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()
>   PM / Domains: Drop unused parameter in genpd_allocate_dev_data()
>   PM / Domains: dt: Allow power-domain property to be a list of
>     specifiers
>   PM / Domains: Don't attach devices in genpd with multi PM domains
>   PM / Domains: Split genpd_dev_pm_attach()
>   PM / Domains: Add support for multi PM domains per device to genpd
>   PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM
>     domains
>
>  .../bindings/power/power_domain.txt           |  19 ++-
>  drivers/base/power/common.c                   |  39 ++++-
>  drivers/base/power/domain.c                   | 155 ++++++++++++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c       |   2 +-
>  include/linux/pm_domain.h                     |  79 ++++-----
>  5 files changed, 216 insertions(+), 78 deletions(-)

I can take patches [1-4/9] from this series for 4.18.

I need an ACK from Rob on the bindings change.

The rest of the series doesn't seem to be ready yet.

Thanks,
Rafael

Re: [PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Ulf Hansson]

On 30 May 2018 at 11:19, Jon Hunter <jonathanh@nvidia.com> wrote:
> Hi Ulf,
>
> On 29/05/18 11:04, Ulf Hansson wrote:
>> The existing dev_pm_domain_attach() function, allows a single PM domain to
>> be attached per device. To be able to support devices that are partitioned
>> across multiple PM domains, let's introduce a new interface,
>> dev_pm_domain_attach_by_id().
>>
>> The dev_pm_domain_attach_by_id() returns a new allocated struct device with
>> the corresponding attached PM domain. This enables for example a driver to
>> operate on the new device from a power management point of view. The driver
>> may then also benefit from using the received device, to set up so called
>> device-links towards its original device. Depending on the situation, these
>> links may then be dynamically changed.
>
> I have given this series a go with Tegra updating the XHCI driver to make
> use of these new APIs. Good news it does appear to work fine for Tegra,
> however, initially when looking at the device_link_add() API ...
>
> /**
>  * device_link_add - Create a link between two devices.
>  * @consumer: Consumer end of the link.
>  * @supplier: Supplier end of the link.
>  * @flags: Link flags.
>
>  ... I had assumed that the 'consumer' device would be the actual XHCI
> device (in the case of Tegra) and the 'supplier' device would be the new
> genpd device. However, this did not work and I got the following WARN on
> boot ...
>
> [    2.050929] ---[ end trace eff0b5265e530c92 ]---
> [    2.055567] WARNING: CPU: 2 PID: 1 at drivers/base/core.c:446 device_links_driver_bound+0xc0/0xd0
> [    2.064422] Modules linked in:
> [    2.067471] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G        W         4.17.0-rc7-next-20180529-00011-g4faf0dc0ebf3-dirty #32
> [    2.078667] Hardware name: Google Pixel C (DT)
> [    2.083101] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [    2.087881] pc : device_links_driver_bound+0xc0/0xd0
> [    2.092832] lr : device_links_driver_bound+0x20/0xd0
>
> Switching the Tegra XHCI device to be the 'supplier' and genpd device to
> be the 'consumer' does work, but is this correct? Seems to be opposite to

It shall be the opposite. The Tegra XHCI device shall be the consumer.

> what I expected. Maybe I am missing something?

The problem you get is because the device returned from
dev_pm_domain_attach_by_id(), let's call it genpd_dev, doesn't have
the a driver.

You need to use a couple of device links flag, something like this:

link = device_link_add(dev, genpd_dev, DL_FLAG_STATELESS |
DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);

Moreover, you also need these commits, depending if you are running on
something else than Rafael's tree.

a0504aecba76 PM / runtime: Drop usage count for suppliers at device link removal
1e8378619841 PM / runtime: Fixup reference counting of device link
suppliers at probe

>
>> The new interface is typically called by drivers during their probe phase,
>> in case they manages devices which uses multiple PM domains. If that is the
>> case, the driver also becomes responsible of managing the detaching of the
>> PM domains, which typically should be done at the remove phase. Detaching
>> is done by calling the existing dev_pm_domain_detach() function and for
>> each of the received devices from dev_pm_domain_attach_by_id().
>>
>> Note, currently its only genpd that supports multiple PM domains per
>> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
>> other PM domain types, if/when needed.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>
>> Changes in v2:
>>       - Fixed comments from Jon. Clarified function descriptions/changelog and
>>       return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
>>       - Fix build error when CONFIG_PM is unset.
>>
>> ---
>>  drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
>>  include/linux/pm_domain.h   |  7 ++++++
>>  2 files changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
>> index 7ae62b6355b8..5e5ea0c239de 100644
>> --- a/drivers/base/power/common.c
>> +++ b/drivers/base/power/common.c
>> @@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
>>  }
>>  EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>>
>> +/**
>> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
>> + * @dev: Device to attach.
>
> Nit ... I still don't think this is the device we are attaching to, but the
> device the PM domains are associated with. IOW we are using this device to
> lookup the PM domains.

Right. I forgot to update that part of the description.

What about:

dev_pm_domain_attach_by_id - Associate a device with one of its PM domains.
@dev: The device used to lookup the PM domain.

>
>> + * @index: The index of the PM domain.
>> + *
>> + * As @dev may only be attached to a single PM domain, the backend PM domain
>> + * provider creates a virtual device to attach instead. If attachment succeeds,
>> + * the ->detach() callback in the struct dev_pm_domain are assigned by the
>> + * corresponding backend attach function, as to deal with detaching of the
>> + * created virtual device.
>> + *
>> + * This function should typically be invoked by a driver during the probe phase,
>> + * in case its device requires power management through multiple PM domains. The
>> + * driver may benefit from using the received device, to configure device-links
>> + * towards its original device. Depending on the use-case and if needed, the
>> + * links may be dynamically changed by the driver, which allows it to control
>> + * the power to the PM domains independently from each other.
>> + *
>> + * Callers must ensure proper synchronization of this function with power
>> + * management callbacks.
>> + *
>> + * Returns the virtual created device when successfully attached to its PM
>> + * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
>> + * Note that, to detach the returned virtual device, the driver shall call
>> + * dev_pm_domain_detach() on it, typically during the remove phase.
>> + */
>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>> +                                       unsigned int index)
>> +{
>> +     if (dev->pm_domain)
>> +             return ERR_PTR(-EEXIST);
>> +
>> +     return genpd_dev_pm_attach_by_id(dev, index);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
>> +
>>  /**
>>   * dev_pm_domain_detach - Detach a device from its PM domain.
>>   * @dev: Device to detach.
>>   * @power_off: Used to indicate whether we should power off the device.
>>   *
>> - * This functions will reverse the actions from dev_pm_domain_attach() and thus
>> - * try to detach the @dev from its PM domain. Typically it should be invoked
>> - * from subsystem level code during the remove phase.
>> + * This functions will reverse the actions from dev_pm_domain_attach() and
>> + * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
>> + * Typically it should be invoked during the remove phase, either from
>> + * subsystem level code or from drivers.
>>   *
>>   * Callers must ensure proper synchronization of this function with power
>>   * management callbacks.
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index 82458e8e2e01..9206a4fef9ac 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>>
>>  #ifdef CONFIG_PM
>>  int dev_pm_domain_attach(struct device *dev, bool power_on);
>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>> +                                       unsigned int index);
>>  void dev_pm_domain_detach(struct device *dev, bool power_off);
>>  void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
>>  #else
>> @@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
>>  {
>>       return 0;
>>  }
>> +static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
>> +                                                     unsigned int index)
>> +{
>> +     return NULL;
>> +}
>>  static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
>>  static inline void dev_pm_domain_set(struct device *dev,
>>                                    struct dev_pm_domain *pd) {}
>>
>
> My only other comments on this series are ...
>
> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>    that the DT binding has a 'power-domain-names' property.

I think it makes sense, but my plan was to do that as second step on
top. Are you okay with that as well?

> 2. I am wondering if there could be value in having a
>    dev_pm_domain_attach_link_all() helper which would attach and link all
>    PM domains at once.

Perhaps it can be useful, yes! However, maybe we can postpone that to
after this series. I want to keep the series as simple as possible,
then we can build upon it.

Kind regards
Uffe

[PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Ulf Hansson]

On 30 May 2018 at 11:19, Jon Hunter <jonathanh@nvidia.com> wrote:
> Hi Ulf,
>
> On 29/05/18 11:04, Ulf Hansson wrote:
>> The existing dev_pm_domain_attach() function, allows a single PM domain to
>> be attached per device. To be able to support devices that are partitioned
>> across multiple PM domains, let's introduce a new interface,
>> dev_pm_domain_attach_by_id().
>>
>> The dev_pm_domain_attach_by_id() returns a new allocated struct device with
>> the corresponding attached PM domain. This enables for example a driver to
>> operate on the new device from a power management point of view. The driver
>> may then also benefit from using the received device, to set up so called
>> device-links towards its original device. Depending on the situation, these
>> links may then be dynamically changed.
>
> I have given this series a go with Tegra updating the XHCI driver to make
> use of these new APIs. Good news it does appear to work fine for Tegra,
> however, initially when looking at the device_link_add() API ...
>
> /**
>  * device_link_add - Create a link between two devices.
>  * @consumer: Consumer end of the link.
>  * @supplier: Supplier end of the link.
>  * @flags: Link flags.
>
>  ... I had assumed that the 'consumer' device would be the actual XHCI
> device (in the case of Tegra) and the 'supplier' device would be the new
> genpd device. However, this did not work and I got the following WARN on
> boot ...
>
> [    2.050929] ---[ end trace eff0b5265e530c92 ]---
> [    2.055567] WARNING: CPU: 2 PID: 1 at drivers/base/core.c:446 device_links_driver_bound+0xc0/0xd0
> [    2.064422] Modules linked in:
> [    2.067471] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G        W         4.17.0-rc7-next-20180529-00011-g4faf0dc0ebf3-dirty #32
> [    2.078667] Hardware name: Google Pixel C (DT)
> [    2.083101] pstate: 80000005 (Nzcv daif -PAN -UAO)
> [    2.087881] pc : device_links_driver_bound+0xc0/0xd0
> [    2.092832] lr : device_links_driver_bound+0x20/0xd0
>
> Switching the Tegra XHCI device to be the 'supplier' and genpd device to
> be the 'consumer' does work, but is this correct? Seems to be opposite to

It shall be the opposite. The Tegra XHCI device shall be the consumer.

> what I expected. Maybe I am missing something?

The problem you get is because the device returned from
dev_pm_domain_attach_by_id(), let's call it genpd_dev, doesn't have
the a driver.

You need to use a couple of device links flag, something like this:

link = device_link_add(dev, genpd_dev, DL_FLAG_STATELESS |
DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);

Moreover, you also need these commits, depending if you are running on
something else than Rafael's tree.

a0504aecba76 PM / runtime: Drop usage count for suppliers at device link removal
1e8378619841 PM / runtime: Fixup reference counting of device link
suppliers at probe

>
>> The new interface is typically called by drivers during their probe phase,
>> in case they manages devices which uses multiple PM domains. If that is the
>> case, the driver also becomes responsible of managing the detaching of the
>> PM domains, which typically should be done at the remove phase. Detaching
>> is done by calling the existing dev_pm_domain_detach() function and for
>> each of the received devices from dev_pm_domain_attach_by_id().
>>
>> Note, currently its only genpd that supports multiple PM domains per
>> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
>> other PM domain types, if/when needed.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>
>> Changes in v2:
>>       - Fixed comments from Jon. Clarified function descriptions/changelog and
>>       return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
>>       - Fix build error when CONFIG_PM is unset.
>>
>> ---
>>  drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
>>  include/linux/pm_domain.h   |  7 ++++++
>>  2 files changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
>> index 7ae62b6355b8..5e5ea0c239de 100644
>> --- a/drivers/base/power/common.c
>> +++ b/drivers/base/power/common.c
>> @@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
>>  }
>>  EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>>
>> +/**
>> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
>> + * @dev: Device to attach.
>
> Nit ... I still don't think this is the device we are attaching to, but the
> device the PM domains are associated with. IOW we are using this device to
> lookup the PM domains.

Right. I forgot to update that part of the description.

What about:

dev_pm_domain_attach_by_id - Associate a device with one of its PM domains.
@dev: The device used to lookup the PM domain.

>
>> + * @index: The index of the PM domain.
>> + *
>> + * As @dev may only be attached to a single PM domain, the backend PM domain
>> + * provider creates a virtual device to attach instead. If attachment succeeds,
>> + * the ->detach() callback in the struct dev_pm_domain are assigned by the
>> + * corresponding backend attach function, as to deal with detaching of the
>> + * created virtual device.
>> + *
>> + * This function should typically be invoked by a driver during the probe phase,
>> + * in case its device requires power management through multiple PM domains. The
>> + * driver may benefit from using the received device, to configure device-links
>> + * towards its original device. Depending on the use-case and if needed, the
>> + * links may be dynamically changed by the driver, which allows it to control
>> + * the power to the PM domains independently from each other.
>> + *
>> + * Callers must ensure proper synchronization of this function with power
>> + * management callbacks.
>> + *
>> + * Returns the virtual created device when successfully attached to its PM
>> + * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
>> + * Note that, to detach the returned virtual device, the driver shall call
>> + * dev_pm_domain_detach() on it, typically during the remove phase.
>> + */
>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>> +                                       unsigned int index)
>> +{
>> +     if (dev->pm_domain)
>> +             return ERR_PTR(-EEXIST);
>> +
>> +     return genpd_dev_pm_attach_by_id(dev, index);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
>> +
>>  /**
>>   * dev_pm_domain_detach - Detach a device from its PM domain.
>>   * @dev: Device to detach.
>>   * @power_off: Used to indicate whether we should power off the device.
>>   *
>> - * This functions will reverse the actions from dev_pm_domain_attach() and thus
>> - * try to detach the @dev from its PM domain. Typically it should be invoked
>> - * from subsystem level code during the remove phase.
>> + * This functions will reverse the actions from dev_pm_domain_attach() and
>> + * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
>> + * Typically it should be invoked during the remove phase, either from
>> + * subsystem level code or from drivers.
>>   *
>>   * Callers must ensure proper synchronization of this function with power
>>   * management callbacks.
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index 82458e8e2e01..9206a4fef9ac 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>>
>>  #ifdef CONFIG_PM
>>  int dev_pm_domain_attach(struct device *dev, bool power_on);
>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>> +                                       unsigned int index);
>>  void dev_pm_domain_detach(struct device *dev, bool power_off);
>>  void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
>>  #else
>> @@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
>>  {
>>       return 0;
>>  }
>> +static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
>> +                                                     unsigned int index)
>> +{
>> +     return NULL;
>> +}
>>  static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
>>  static inline void dev_pm_domain_set(struct device *dev,
>>                                    struct dev_pm_domain *pd) {}
>>
>
> My only other comments on this series are ...
>
> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>    that the DT binding has a 'power-domain-names' property.

I think it makes sense, but my plan was to do that as second step on
top. Are you okay with that as well?

> 2. I am wondering if there could be value in having a
>    dev_pm_domain_attach_link_all() helper which would attach and link all
>    PM domains at once.

Perhaps it can be useful, yes! However, maybe we can postpone that to
after this series. I want to keep the series as simple as possible,
then we can build upon it.

Kind regards
Uffe

Re: [PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Ulf Hansson]

On 30 May 2018 at 11:30, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Tue, May 29, 2018 at 12:04 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> Changes in v2:
>>         - Addressed comments from Geert around DT doc.
>>         - Addressed comments from Jon around clarification of how to use this
>>         and changes to returned error codes.
>>         - Fixed build error in case CONFIG_PM was unset.
>>
>> There are devices that are partitioned across multiple PM domains. Currently
>> these can't be supported well by the available PM infrastructures we have in
>> the kernel. This series is an attempt to address this.
>>
>> The interesting parts happens from patch 5 an onwards, including a minor DT
>> update to the existing power-domain bindings, the 4 earlier are just trivial
>> clean-ups of some related code in genpd, which I happened to stumble over.
>>
>> Some additional background:
>>
>> One existing case where devices are partitioned across multiple PM domains, is
>> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
>> series, trying to address these issues, however this is a new approach, while
>> it re-uses the same concepts from DT point of view.
>>
>> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
>> controller. Each controller have its own independent PM domain, but are being
>> partitioned across another shared PM domain for the USB super-speed logic.
>>
>> Currently to make the drivers work, either the related PM domains needs to stay
>> powered on always or the PM domain topology needs to be in-correctly modelled
>> through sub-domains. In both cases PM domains may be powered on while they
>> don't need to be, so in the end this means - wasting power -.
>>
>> As stated above, this series intends to address these problem from a PM
>> infrastructure point of view. More details are available in each changelog.
>>
>> It should be noted that this series has been tested on HW, however only by using
>> a home-cooked test PM domain driver for genpd and together with a test driver.
>> This allowed me to play with PM domain (genpd), runtime PM and device links.
>>
>> Any further deployment for real use cases are greatly appreciated. I am happy to
>> to help, if needed!
>>
>> Kind regards
>> Ulf Hansson
>>
>>
>> Ulf Hansson (9):
>>   PM / Domains: Drop extern declarations of functions in pm_domain.h
>>   PM / Domains: Drop __pm_genpd_add_device()
>>   PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()
>>   PM / Domains: Drop unused parameter in genpd_allocate_dev_data()
>>   PM / Domains: dt: Allow power-domain property to be a list of
>>     specifiers
>>   PM / Domains: Don't attach devices in genpd with multi PM domains
>>   PM / Domains: Split genpd_dev_pm_attach()
>>   PM / Domains: Add support for multi PM domains per device to genpd
>>   PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM
>>     domains
>>
>>  .../bindings/power/power_domain.txt           |  19 ++-
>>  drivers/base/power/common.c                   |  39 ++++-
>>  drivers/base/power/domain.c                   | 155 ++++++++++++++----
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c       |   2 +-
>>  include/linux/pm_domain.h                     |  79 ++++-----
>>  5 files changed, 216 insertions(+), 78 deletions(-)
>
> I can take patches [1-4/9] from this series for 4.18.

Thanks!

>
> I need an ACK from Rob on the bindings change.

Yes, of course!

>
> The rest of the series doesn't seem to be ready yet.

It's getting reviewed and tested, so let's give it some more time.

BTW, from an overall design point of view, you seems okay with it?

Kind regards
Uffe

[PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Ulf Hansson]

On 30 May 2018 at 11:30, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Tue, May 29, 2018 at 12:04 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> Changes in v2:
>>         - Addressed comments from Geert around DT doc.
>>         - Addressed comments from Jon around clarification of how to use this
>>         and changes to returned error codes.
>>         - Fixed build error in case CONFIG_PM was unset.
>>
>> There are devices that are partitioned across multiple PM domains. Currently
>> these can't be supported well by the available PM infrastructures we have in
>> the kernel. This series is an attempt to address this.
>>
>> The interesting parts happens from patch 5 an onwards, including a minor DT
>> update to the existing power-domain bindings, the 4 earlier are just trivial
>> clean-ups of some related code in genpd, which I happened to stumble over.
>>
>> Some additional background:
>>
>> One existing case where devices are partitioned across multiple PM domains, is
>> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
>> series, trying to address these issues, however this is a new approach, while
>> it re-uses the same concepts from DT point of view.
>>
>> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
>> controller. Each controller have its own independent PM domain, but are being
>> partitioned across another shared PM domain for the USB super-speed logic.
>>
>> Currently to make the drivers work, either the related PM domains needs to stay
>> powered on always or the PM domain topology needs to be in-correctly modelled
>> through sub-domains. In both cases PM domains may be powered on while they
>> don't need to be, so in the end this means - wasting power -.
>>
>> As stated above, this series intends to address these problem from a PM
>> infrastructure point of view. More details are available in each changelog.
>>
>> It should be noted that this series has been tested on HW, however only by using
>> a home-cooked test PM domain driver for genpd and together with a test driver.
>> This allowed me to play with PM domain (genpd), runtime PM and device links.
>>
>> Any further deployment for real use cases are greatly appreciated. I am happy to
>> to help, if needed!
>>
>> Kind regards
>> Ulf Hansson
>>
>>
>> Ulf Hansson (9):
>>   PM / Domains: Drop extern declarations of functions in pm_domain.h
>>   PM / Domains: Drop __pm_genpd_add_device()
>>   PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()
>>   PM / Domains: Drop unused parameter in genpd_allocate_dev_data()
>>   PM / Domains: dt: Allow power-domain property to be a list of
>>     specifiers
>>   PM / Domains: Don't attach devices in genpd with multi PM domains
>>   PM / Domains: Split genpd_dev_pm_attach()
>>   PM / Domains: Add support for multi PM domains per device to genpd
>>   PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM
>>     domains
>>
>>  .../bindings/power/power_domain.txt           |  19 ++-
>>  drivers/base/power/common.c                   |  39 ++++-
>>  drivers/base/power/domain.c                   | 155 ++++++++++++++----
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c       |   2 +-
>>  include/linux/pm_domain.h                     |  79 ++++-----
>>  5 files changed, 216 insertions(+), 78 deletions(-)
>
> I can take patches [1-4/9] from this series for 4.18.

Thanks!

>
> I need an ACK from Rob on the bindings change.

Yes, of course!

>
> The rest of the series doesn't seem to be ready yet.

It's getting reviewed and tested, so let's give it some more time.

BTW, from an overall design point of view, you seems okay with it?

Kind regards
Uffe

Re: [PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Jon Hunter]

On 30/05/18 12:31, Ulf Hansson wrote:
> On 30 May 2018 at 11:19, Jon Hunter <jonathanh@nvidia.com> wrote:
>> Hi Ulf,
>>
>> On 29/05/18 11:04, Ulf Hansson wrote:
>>> The existing dev_pm_domain_attach() function, allows a single PM domain to
>>> be attached per device. To be able to support devices that are partitioned
>>> across multiple PM domains, let's introduce a new interface,
>>> dev_pm_domain_attach_by_id().
>>>
>>> The dev_pm_domain_attach_by_id() returns a new allocated struct device with
>>> the corresponding attached PM domain. This enables for example a driver to
>>> operate on the new device from a power management point of view. The driver
>>> may then also benefit from using the received device, to set up so called
>>> device-links towards its original device. Depending on the situation, these
>>> links may then be dynamically changed.
>>
>> I have given this series a go with Tegra updating the XHCI driver to make
>> use of these new APIs. Good news it does appear to work fine for Tegra,
>> however, initially when looking at the device_link_add() API ...
>>
>> /**
>>  * device_link_add - Create a link between two devices.
>>  * @consumer: Consumer end of the link.
>>  * @supplier: Supplier end of the link.
>>  * @flags: Link flags.
>>
>>  ... I had assumed that the 'consumer' device would be the actual XHCI
>> device (in the case of Tegra) and the 'supplier' device would be the new
>> genpd device. However, this did not work and I got the following WARN on
>> boot ...
>>
>> [    2.050929] ---[ end trace eff0b5265e530c92 ]---
>> [    2.055567] WARNING: CPU: 2 PID: 1 at drivers/base/core.c:446 device_links_driver_bound+0xc0/0xd0
>> [    2.064422] Modules linked in:
>> [    2.067471] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G        W         4.17.0-rc7-next-20180529-00011-g4faf0dc0ebf3-dirty #32
>> [    2.078667] Hardware name: Google Pixel C (DT)
>> [    2.083101] pstate: 80000005 (Nzcv daif -PAN -UAO)
>> [    2.087881] pc : device_links_driver_bound+0xc0/0xd0
>> [    2.092832] lr : device_links_driver_bound+0x20/0xd0
>>
>> Switching the Tegra XHCI device to be the 'supplier' and genpd device to
>> be the 'consumer' does work, but is this correct? Seems to be opposite to
> 
> It shall be the opposite. The Tegra XHCI device shall be the consumer.
> 
>> what I expected. Maybe I am missing something?
> 
> The problem you get is because the device returned from
> dev_pm_domain_attach_by_id(), let's call it genpd_dev, doesn't have
> the a driver.
> 
> You need to use a couple of device links flag, something like this:
> 
> link = device_link_add(dev, genpd_dev, DL_FLAG_STATELESS |
> DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);

Thanks, adding the DL_FLAG_STATELESS flag resolved the issue. I already added
the DL_FLAG_PM_RUNTIME but I did not bother with the DL_FLAG_RPM_ACTIVE as
from the description it appears that this will always keep it on? Seems to
work fine without it.

> Moreover, you also need these commits, depending if you are running on
> something else than Rafael's tree.
> 
> a0504aecba76 PM / runtime: Drop usage count for suppliers at device link removal
> 1e8378619841 PM / runtime: Fixup reference counting of device link
> suppliers at probe

Yes these are currently in -next and so I have these.
 
>>
>>> The new interface is typically called by drivers during their probe phase,
>>> in case they manages devices which uses multiple PM domains. If that is the
>>> case, the driver also becomes responsible of managing the detaching of the
>>> PM domains, which typically should be done at the remove phase. Detaching
>>> is done by calling the existing dev_pm_domain_detach() function and for
>>> each of the received devices from dev_pm_domain_attach_by_id().
>>>
>>> Note, currently its only genpd that supports multiple PM domains per
>>> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
>>> other PM domain types, if/when needed.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>
>>> Changes in v2:
>>>       - Fixed comments from Jon. Clarified function descriptions/changelog and
>>>       return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
>>>       - Fix build error when CONFIG_PM is unset.
>>>
>>> ---
>>>  drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
>>>  include/linux/pm_domain.h   |  7 ++++++
>>>  2 files changed, 47 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
>>> index 7ae62b6355b8..5e5ea0c239de 100644
>>> --- a/drivers/base/power/common.c
>>> +++ b/drivers/base/power/common.c
>>> @@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
>>>  }
>>>  EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>>>
>>> +/**
>>> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
>>> + * @dev: Device to attach.
>>
>> Nit ... I still don't think this is the device we are attaching to, but the
>> device the PM domains are associated with. IOW we are using this device to
>> lookup the PM domains.
> 
> Right. I forgot to update that part of the description.
> 
> What about:
> 
> dev_pm_domain_attach_by_id - Associate a device with one of its PM domains.
> @dev: The device used to lookup the PM domain.

Perfect.
 
>>
>>> + * @index: The index of the PM domain.
>>> + *
>>> + * As @dev may only be attached to a single PM domain, the backend PM domain
>>> + * provider creates a virtual device to attach instead. If attachment succeeds,
>>> + * the ->detach() callback in the struct dev_pm_domain are assigned by the
>>> + * corresponding backend attach function, as to deal with detaching of the
>>> + * created virtual device.
>>> + *
>>> + * This function should typically be invoked by a driver during the probe phase,
>>> + * in case its device requires power management through multiple PM domains. The
>>> + * driver may benefit from using the received device, to configure device-links
>>> + * towards its original device. Depending on the use-case and if needed, the
>>> + * links may be dynamically changed by the driver, which allows it to control
>>> + * the power to the PM domains independently from each other.
>>> + *
>>> + * Callers must ensure proper synchronization of this function with power
>>> + * management callbacks.
>>> + *
>>> + * Returns the virtual created device when successfully attached to its PM
>>> + * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
>>> + * Note that, to detach the returned virtual device, the driver shall call
>>> + * dev_pm_domain_detach() on it, typically during the remove phase.
>>> + */
>>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                       unsigned int index)
>>> +{
>>> +     if (dev->pm_domain)
>>> +             return ERR_PTR(-EEXIST);
>>> +
>>> +     return genpd_dev_pm_attach_by_id(dev, index);
>>> +}
>>> +EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
>>> +
>>>  /**
>>>   * dev_pm_domain_detach - Detach a device from its PM domain.
>>>   * @dev: Device to detach.
>>>   * @power_off: Used to indicate whether we should power off the device.
>>>   *
>>> - * This functions will reverse the actions from dev_pm_domain_attach() and thus
>>> - * try to detach the @dev from its PM domain. Typically it should be invoked
>>> - * from subsystem level code during the remove phase.
>>> + * This functions will reverse the actions from dev_pm_domain_attach() and
>>> + * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
>>> + * Typically it should be invoked during the remove phase, either from
>>> + * subsystem level code or from drivers.
>>>   *
>>>   * Callers must ensure proper synchronization of this function with power
>>>   * management callbacks.
>>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>>> index 82458e8e2e01..9206a4fef9ac 100644
>>> --- a/include/linux/pm_domain.h
>>> +++ b/include/linux/pm_domain.h
>>> @@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>>>
>>>  #ifdef CONFIG_PM
>>>  int dev_pm_domain_attach(struct device *dev, bool power_on);
>>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                       unsigned int index);
>>>  void dev_pm_domain_detach(struct device *dev, bool power_off);
>>>  void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
>>>  #else
>>> @@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
>>>  {
>>>       return 0;
>>>  }
>>> +static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                                     unsigned int index)
>>> +{
>>> +     return NULL;
>>> +}
>>>  static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
>>>  static inline void dev_pm_domain_set(struct device *dev,
>>>                                    struct dev_pm_domain *pd) {}
>>>
>>
>> My only other comments on this series are ...
>>
>> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>>    that the DT binding has a 'power-domain-names' property.
> 
> I think it makes sense, but my plan was to do that as second step on
> top. Are you okay with that as well?

Yes that is fine with me.

>> 2. I am wondering if there could be value in having a
>>    dev_pm_domain_attach_link_all() helper which would attach and link all
>>    PM domains at once.
> 
> Perhaps it can be useful, yes! However, maybe we can postpone that to
> after this series. I want to keep the series as simple as possible,
> then we can build upon it.

That's fine too and I can always add these if you prefer.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Jon Hunter]

On 30/05/18 12:31, Ulf Hansson wrote:
> On 30 May 2018 at 11:19, Jon Hunter <jonathanh@nvidia.com> wrote:
>> Hi Ulf,
>>
>> On 29/05/18 11:04, Ulf Hansson wrote:
>>> The existing dev_pm_domain_attach() function, allows a single PM domain to
>>> be attached per device. To be able to support devices that are partitioned
>>> across multiple PM domains, let's introduce a new interface,
>>> dev_pm_domain_attach_by_id().
>>>
>>> The dev_pm_domain_attach_by_id() returns a new allocated struct device with
>>> the corresponding attached PM domain. This enables for example a driver to
>>> operate on the new device from a power management point of view. The driver
>>> may then also benefit from using the received device, to set up so called
>>> device-links towards its original device. Depending on the situation, these
>>> links may then be dynamically changed.
>>
>> I have given this series a go with Tegra updating the XHCI driver to make
>> use of these new APIs. Good news it does appear to work fine for Tegra,
>> however, initially when looking at the device_link_add() API ...
>>
>> /**
>>  * device_link_add - Create a link between two devices.
>>  * @consumer: Consumer end of the link.
>>  * @supplier: Supplier end of the link.
>>  * @flags: Link flags.
>>
>>  ... I had assumed that the 'consumer' device would be the actual XHCI
>> device (in the case of Tegra) and the 'supplier' device would be the new
>> genpd device. However, this did not work and I got the following WARN on
>> boot ...
>>
>> [    2.050929] ---[ end trace eff0b5265e530c92 ]---
>> [    2.055567] WARNING: CPU: 2 PID: 1 at drivers/base/core.c:446 device_links_driver_bound+0xc0/0xd0
>> [    2.064422] Modules linked in:
>> [    2.067471] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G        W         4.17.0-rc7-next-20180529-00011-g4faf0dc0ebf3-dirty #32
>> [    2.078667] Hardware name: Google Pixel C (DT)
>> [    2.083101] pstate: 80000005 (Nzcv daif -PAN -UAO)
>> [    2.087881] pc : device_links_driver_bound+0xc0/0xd0
>> [    2.092832] lr : device_links_driver_bound+0x20/0xd0
>>
>> Switching the Tegra XHCI device to be the 'supplier' and genpd device to
>> be the 'consumer' does work, but is this correct? Seems to be opposite to
> 
> It shall be the opposite. The Tegra XHCI device shall be the consumer.
> 
>> what I expected. Maybe I am missing something?
> 
> The problem you get is because the device returned from
> dev_pm_domain_attach_by_id(), let's call it genpd_dev, doesn't have
> the a driver.
> 
> You need to use a couple of device links flag, something like this:
> 
> link = device_link_add(dev, genpd_dev, DL_FLAG_STATELESS |
> DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);

Thanks, adding the DL_FLAG_STATELESS flag resolved the issue. I already added
the DL_FLAG_PM_RUNTIME but I did not bother with the DL_FLAG_RPM_ACTIVE as
from the description it appears that this will always keep it on? Seems to
work fine without it.

> Moreover, you also need these commits, depending if you are running on
> something else than Rafael's tree.
> 
> a0504aecba76 PM / runtime: Drop usage count for suppliers at device link removal
> 1e8378619841 PM / runtime: Fixup reference counting of device link
> suppliers at probe

Yes these are currently in -next and so I have these.
 
>>
>>> The new interface is typically called by drivers during their probe phase,
>>> in case they manages devices which uses multiple PM domains. If that is the
>>> case, the driver also becomes responsible of managing the detaching of the
>>> PM domains, which typically should be done at the remove phase. Detaching
>>> is done by calling the existing dev_pm_domain_detach() function and for
>>> each of the received devices from dev_pm_domain_attach_by_id().
>>>
>>> Note, currently its only genpd that supports multiple PM domains per
>>> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
>>> other PM domain types, if/when needed.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>
>>> Changes in v2:
>>>       - Fixed comments from Jon. Clarified function descriptions/changelog and
>>>       return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
>>>       - Fix build error when CONFIG_PM is unset.
>>>
>>> ---
>>>  drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
>>>  include/linux/pm_domain.h   |  7 ++++++
>>>  2 files changed, 47 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
>>> index 7ae62b6355b8..5e5ea0c239de 100644
>>> --- a/drivers/base/power/common.c
>>> +++ b/drivers/base/power/common.c
>>> @@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
>>>  }
>>>  EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>>>
>>> +/**
>>> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
>>> + * @dev: Device to attach.
>>
>> Nit ... I still don't think this is the device we are attaching to, but the
>> device the PM domains are associated with. IOW we are using this device to
>> lookup the PM domains.
> 
> Right. I forgot to update that part of the description.
> 
> What about:
> 
> dev_pm_domain_attach_by_id - Associate a device with one of its PM domains.
> @dev: The device used to lookup the PM domain.

Perfect.
 
>>
>>> + * @index: The index of the PM domain.
>>> + *
>>> + * As @dev may only be attached to a single PM domain, the backend PM domain
>>> + * provider creates a virtual device to attach instead. If attachment succeeds,
>>> + * the ->detach() callback in the struct dev_pm_domain are assigned by the
>>> + * corresponding backend attach function, as to deal with detaching of the
>>> + * created virtual device.
>>> + *
>>> + * This function should typically be invoked by a driver during the probe phase,
>>> + * in case its device requires power management through multiple PM domains. The
>>> + * driver may benefit from using the received device, to configure device-links
>>> + * towards its original device. Depending on the use-case and if needed, the
>>> + * links may be dynamically changed by the driver, which allows it to control
>>> + * the power to the PM domains independently from each other.
>>> + *
>>> + * Callers must ensure proper synchronization of this function with power
>>> + * management callbacks.
>>> + *
>>> + * Returns the virtual created device when successfully attached to its PM
>>> + * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
>>> + * Note that, to detach the returned virtual device, the driver shall call
>>> + * dev_pm_domain_detach() on it, typically during the remove phase.
>>> + */
>>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                       unsigned int index)
>>> +{
>>> +     if (dev->pm_domain)
>>> +             return ERR_PTR(-EEXIST);
>>> +
>>> +     return genpd_dev_pm_attach_by_id(dev, index);
>>> +}
>>> +EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
>>> +
>>>  /**
>>>   * dev_pm_domain_detach - Detach a device from its PM domain.
>>>   * @dev: Device to detach.
>>>   * @power_off: Used to indicate whether we should power off the device.
>>>   *
>>> - * This functions will reverse the actions from dev_pm_domain_attach() and thus
>>> - * try to detach the @dev from its PM domain. Typically it should be invoked
>>> - * from subsystem level code during the remove phase.
>>> + * This functions will reverse the actions from dev_pm_domain_attach() and
>>> + * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
>>> + * Typically it should be invoked during the remove phase, either from
>>> + * subsystem level code or from drivers.
>>>   *
>>>   * Callers must ensure proper synchronization of this function with power
>>>   * management callbacks.
>>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>>> index 82458e8e2e01..9206a4fef9ac 100644
>>> --- a/include/linux/pm_domain.h
>>> +++ b/include/linux/pm_domain.h
>>> @@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>>>
>>>  #ifdef CONFIG_PM
>>>  int dev_pm_domain_attach(struct device *dev, bool power_on);
>>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                       unsigned int index);
>>>  void dev_pm_domain_detach(struct device *dev, bool power_off);
>>>  void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
>>>  #else
>>> @@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
>>>  {
>>>       return 0;
>>>  }
>>> +static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                                     unsigned int index)
>>> +{
>>> +     return NULL;
>>> +}
>>>  static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
>>>  static inline void dev_pm_domain_set(struct device *dev,
>>>                                    struct dev_pm_domain *pd) {}
>>>
>>
>> My only other comments on this series are ...
>>
>> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>>    that the DT binding has a 'power-domain-names' property.
> 
> I think it makes sense, but my plan was to do that as second step on
> top. Are you okay with that as well?

Yes that is fine with me.

>> 2. I am wondering if there could be value in having a
>>    dev_pm_domain_attach_link_all() helper which would attach and link all
>>    PM domains at once.
> 
> Perhaps it can be useful, yes! However, maybe we can postpone that to
> after this series. I want to keep the series as simple as possible,
> then we can build upon it.

That's fine too and I can always add these if you prefer.

Cheers
Jon

-- 
nvpublic

[PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Jon Hunter]

On 30/05/18 12:31, Ulf Hansson wrote:
> On 30 May 2018 at 11:19, Jon Hunter <jonathanh@nvidia.com> wrote:
>> Hi Ulf,
>>
>> On 29/05/18 11:04, Ulf Hansson wrote:
>>> The existing dev_pm_domain_attach() function, allows a single PM domain to
>>> be attached per device. To be able to support devices that are partitioned
>>> across multiple PM domains, let's introduce a new interface,
>>> dev_pm_domain_attach_by_id().
>>>
>>> The dev_pm_domain_attach_by_id() returns a new allocated struct device with
>>> the corresponding attached PM domain. This enables for example a driver to
>>> operate on the new device from a power management point of view. The driver
>>> may then also benefit from using the received device, to set up so called
>>> device-links towards its original device. Depending on the situation, these
>>> links may then be dynamically changed.
>>
>> I have given this series a go with Tegra updating the XHCI driver to make
>> use of these new APIs. Good news it does appear to work fine for Tegra,
>> however, initially when looking at the device_link_add() API ...
>>
>> /**
>>  * device_link_add - Create a link between two devices.
>>  * @consumer: Consumer end of the link.
>>  * @supplier: Supplier end of the link.
>>  * @flags: Link flags.
>>
>>  ... I had assumed that the 'consumer' device would be the actual XHCI
>> device (in the case of Tegra) and the 'supplier' device would be the new
>> genpd device. However, this did not work and I got the following WARN on
>> boot ...
>>
>> [    2.050929] ---[ end trace eff0b5265e530c92 ]---
>> [    2.055567] WARNING: CPU: 2 PID: 1 at drivers/base/core.c:446 device_links_driver_bound+0xc0/0xd0
>> [    2.064422] Modules linked in:
>> [    2.067471] CPU: 2 PID: 1 Comm: swapper/0 Tainted: G        W         4.17.0-rc7-next-20180529-00011-g4faf0dc0ebf3-dirty #32
>> [    2.078667] Hardware name: Google Pixel C (DT)
>> [    2.083101] pstate: 80000005 (Nzcv daif -PAN -UAO)
>> [    2.087881] pc : device_links_driver_bound+0xc0/0xd0
>> [    2.092832] lr : device_links_driver_bound+0x20/0xd0
>>
>> Switching the Tegra XHCI device to be the 'supplier' and genpd device to
>> be the 'consumer' does work, but is this correct? Seems to be opposite to
> 
> It shall be the opposite. The Tegra XHCI device shall be the consumer.
> 
>> what I expected. Maybe I am missing something?
> 
> The problem you get is because the device returned from
> dev_pm_domain_attach_by_id(), let's call it genpd_dev, doesn't have
> the a driver.
> 
> You need to use a couple of device links flag, something like this:
> 
> link = device_link_add(dev, genpd_dev, DL_FLAG_STATELESS |
> DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);

Thanks, adding the DL_FLAG_STATELESS flag resolved the issue. I already added
the DL_FLAG_PM_RUNTIME but I did not bother with the DL_FLAG_RPM_ACTIVE as
from the description it appears that this will always keep it on? Seems to
work fine without it.

> Moreover, you also need these commits, depending if you are running on
> something else than Rafael's tree.
> 
> a0504aecba76 PM / runtime: Drop usage count for suppliers at device link removal
> 1e8378619841 PM / runtime: Fixup reference counting of device link
> suppliers at probe

Yes these are currently in -next and so I have these.
 
>>
>>> The new interface is typically called by drivers during their probe phase,
>>> in case they manages devices which uses multiple PM domains. If that is the
>>> case, the driver also becomes responsible of managing the detaching of the
>>> PM domains, which typically should be done at the remove phase. Detaching
>>> is done by calling the existing dev_pm_domain_detach() function and for
>>> each of the received devices from dev_pm_domain_attach_by_id().
>>>
>>> Note, currently its only genpd that supports multiple PM domains per
>>> device, but dev_pm_domain_attach_by_id() can easily by extended to cover
>>> other PM domain types, if/when needed.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>>
>>> Changes in v2:
>>>       - Fixed comments from Jon. Clarified function descriptions/changelog and
>>>       return ERR_PTR(-EEXIST) in case a PM domain is already assigned.
>>>       - Fix build error when CONFIG_PM is unset.
>>>
>>> ---
>>>  drivers/base/power/common.c | 43 ++++++++++++++++++++++++++++++++++---
>>>  include/linux/pm_domain.h   |  7 ++++++
>>>  2 files changed, 47 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
>>> index 7ae62b6355b8..5e5ea0c239de 100644
>>> --- a/drivers/base/power/common.c
>>> +++ b/drivers/base/power/common.c
>>> @@ -116,14 +116,51 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
>>>  }
>>>  EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>>>
>>> +/**
>>> + * dev_pm_domain_attach_by_id - Attach a device to one of its PM domains.
>>> + * @dev: Device to attach.
>>
>> Nit ... I still don't think this is the device we are attaching to, but the
>> device the PM domains are associated with. IOW we are using this device to
>> lookup the PM domains.
> 
> Right. I forgot to update that part of the description.
> 
> What about:
> 
> dev_pm_domain_attach_by_id - Associate a device with one of its PM domains.
> @dev: The device used to lookup the PM domain.

Perfect.
 
>>
>>> + * @index: The index of the PM domain.
>>> + *
>>> + * As @dev may only be attached to a single PM domain, the backend PM domain
>>> + * provider creates a virtual device to attach instead. If attachment succeeds,
>>> + * the ->detach() callback in the struct dev_pm_domain are assigned by the
>>> + * corresponding backend attach function, as to deal with detaching of the
>>> + * created virtual device.
>>> + *
>>> + * This function should typically be invoked by a driver during the probe phase,
>>> + * in case its device requires power management through multiple PM domains. The
>>> + * driver may benefit from using the received device, to configure device-links
>>> + * towards its original device. Depending on the use-case and if needed, the
>>> + * links may be dynamically changed by the driver, which allows it to control
>>> + * the power to the PM domains independently from each other.
>>> + *
>>> + * Callers must ensure proper synchronization of this function with power
>>> + * management callbacks.
>>> + *
>>> + * Returns the virtual created device when successfully attached to its PM
>>> + * domain, NULL in case @dev don't need a PM domain, else an ERR_PTR().
>>> + * Note that, to detach the returned virtual device, the driver shall call
>>> + * dev_pm_domain_detach() on it, typically during the remove phase.
>>> + */
>>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                       unsigned int index)
>>> +{
>>> +     if (dev->pm_domain)
>>> +             return ERR_PTR(-EEXIST);
>>> +
>>> +     return genpd_dev_pm_attach_by_id(dev, index);
>>> +}
>>> +EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_id);
>>> +
>>>  /**
>>>   * dev_pm_domain_detach - Detach a device from its PM domain.
>>>   * @dev: Device to detach.
>>>   * @power_off: Used to indicate whether we should power off the device.
>>>   *
>>> - * This functions will reverse the actions from dev_pm_domain_attach() and thus
>>> - * try to detach the @dev from its PM domain. Typically it should be invoked
>>> - * from subsystem level code during the remove phase.
>>> + * This functions will reverse the actions from dev_pm_domain_attach() and
>>> + * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain.
>>> + * Typically it should be invoked during the remove phase, either from
>>> + * subsystem level code or from drivers.
>>>   *
>>>   * Callers must ensure proper synchronization of this function with power
>>>   * management callbacks.
>>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>>> index 82458e8e2e01..9206a4fef9ac 100644
>>> --- a/include/linux/pm_domain.h
>>> +++ b/include/linux/pm_domain.h
>>> @@ -299,6 +299,8 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>>>
>>>  #ifdef CONFIG_PM
>>>  int dev_pm_domain_attach(struct device *dev, bool power_on);
>>> +struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                       unsigned int index);
>>>  void dev_pm_domain_detach(struct device *dev, bool power_off);
>>>  void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
>>>  #else
>>> @@ -306,6 +308,11 @@ static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
>>>  {
>>>       return 0;
>>>  }
>>> +static inline struct device *dev_pm_domain_attach_by_id(struct device *dev,
>>> +                                                     unsigned int index)
>>> +{
>>> +     return NULL;
>>> +}
>>>  static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
>>>  static inline void dev_pm_domain_set(struct device *dev,
>>>                                    struct dev_pm_domain *pd) {}
>>>
>>
>> My only other comments on this series are ...
>>
>> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>>    that the DT binding has a 'power-domain-names' property.
> 
> I think it makes sense, but my plan was to do that as second step on
> top. Are you okay with that as well?

Yes that is fine with me.

>> 2. I am wondering if there could be value in having a
>>    dev_pm_domain_attach_link_all() helper which would attach and link all
>>    PM domains at once.
> 
> Perhaps it can be useful, yes! However, maybe we can postpone that to
> after this series. I want to keep the series as simple as possible,
> then we can build upon it.

That's fine too and I can always add these if you prefer.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Jon Hunter]

On 29/05/18 11:04, Ulf Hansson wrote:
> To support devices being partitioned across multiple PM domains, let's
> begin with extending genpd to cope with these kind of configurations.
> 
> Therefore, add a new exported function genpd_dev_pm_attach_by_id(), which
> is similar to the existing genpd_dev_pm_attach(), but with the difference
> that it allows its callers to provide an index to the PM domain that it
> wants to attach.
> 
> Note that, genpd_dev_pm_attach_by_id() shall only be called by the driver
> core / PM core, similar to how the existing dev_pm_domain_attach() makes
> use of genpd_dev_pm_attach(). However, this is implemented by following
> changes on top.
> 
> Because, only one PM domain can be attached per device, genpd needs to
> create a virtual device that it can attach/detach instead. More precisely,
> let the new function genpd_dev_pm_attach_by_id() register a virtual struct
> device via calling device_register(). Then let it attach this device to the
> corresponding PM domain, rather than the one that is provided by the
> caller. The actual attaching is done via re-using the existing genpd OF
> functions.
> 
> At successful attachment, genpd_dev_pm_attach_by_id() returns the created
> virtual device, which allows the caller to operate on it to deal with power
> management. Following changes on top, provides more details in this
> regards.
> 
> To deal with detaching of a PM domain for the multiple PM domains case,
> let's also extend the existing genpd_dev_pm_detach() function, to cover the
> cleanup of the created virtual device, via make it call device_unregister()
> on it. In this way, there is no need to introduce a new function to deal
> with detach for the multiple PM domain case, but instead the existing one
> is re-used.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Fixed comments from Jon. Clarified function descriptions
> 	and changelog.
> 
> ---
>  drivers/base/power/domain.c | 80 +++++++++++++++++++++++++++++++++++++
>  include/linux/pm_domain.h   |  8 ++++
>  2 files changed, 88 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 2af99bfcbe3c..2b496d79159d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>  }
>  EXPORT_SYMBOL_GPL(of_genpd_remove_last);
>  
> +static void genpd_release_dev(struct device *dev)
> +{
> +	kfree(dev);
> +}
> +
> +static struct bus_type genpd_bus_type = {
> +	.name		= "genpd",
> +};
> +
>  /**
>   * genpd_dev_pm_detach - Detach a device from its PM domain.
>   * @dev: Device to detach.
> @@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
>  
>  	/* Check if PM domain can be powered off after removing this device. */
>  	genpd_queue_power_off_work(pd);
> +
> +	/* Unregister the device if it was created by genpd. */
> +	if (dev->bus == &genpd_bus_type)
> +		device_unregister(dev);
>  }
>  
>  static void genpd_dev_pm_sync(struct device *dev)
> @@ -2298,6 +2311,67 @@ int genpd_dev_pm_attach(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>  
> +/**
> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
> + * @dev: Device to attach.

Can you update the description of the above as well?

Thanks
Jon

-- 
nvpublic

Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Jon Hunter]

On 29/05/18 11:04, Ulf Hansson wrote:
> To support devices being partitioned across multiple PM domains, let's
> begin with extending genpd to cope with these kind of configurations.
> 
> Therefore, add a new exported function genpd_dev_pm_attach_by_id(), which
> is similar to the existing genpd_dev_pm_attach(), but with the difference
> that it allows its callers to provide an index to the PM domain that it
> wants to attach.
> 
> Note that, genpd_dev_pm_attach_by_id() shall only be called by the driver
> core / PM core, similar to how the existing dev_pm_domain_attach() makes
> use of genpd_dev_pm_attach(). However, this is implemented by following
> changes on top.
> 
> Because, only one PM domain can be attached per device, genpd needs to
> create a virtual device that it can attach/detach instead. More precisely,
> let the new function genpd_dev_pm_attach_by_id() register a virtual struct
> device via calling device_register(). Then let it attach this device to the
> corresponding PM domain, rather than the one that is provided by the
> caller. The actual attaching is done via re-using the existing genpd OF
> functions.
> 
> At successful attachment, genpd_dev_pm_attach_by_id() returns the created
> virtual device, which allows the caller to operate on it to deal with power
> management. Following changes on top, provides more details in this
> regards.
> 
> To deal with detaching of a PM domain for the multiple PM domains case,
> let's also extend the existing genpd_dev_pm_detach() function, to cover the
> cleanup of the created virtual device, via make it call device_unregister()
> on it. In this way, there is no need to introduce a new function to deal
> with detach for the multiple PM domain case, but instead the existing one
> is re-used.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Fixed comments from Jon. Clarified function descriptions
> 	and changelog.
> 
> ---
>  drivers/base/power/domain.c | 80 +++++++++++++++++++++++++++++++++++++
>  include/linux/pm_domain.h   |  8 ++++
>  2 files changed, 88 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 2af99bfcbe3c..2b496d79159d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>  }
>  EXPORT_SYMBOL_GPL(of_genpd_remove_last);
>  
> +static void genpd_release_dev(struct device *dev)
> +{
> +	kfree(dev);
> +}
> +
> +static struct bus_type genpd_bus_type = {
> +	.name		= "genpd",
> +};
> +
>  /**
>   * genpd_dev_pm_detach - Detach a device from its PM domain.
>   * @dev: Device to detach.
> @@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
>  
>  	/* Check if PM domain can be powered off after removing this device. */
>  	genpd_queue_power_off_work(pd);
> +
> +	/* Unregister the device if it was created by genpd. */
> +	if (dev->bus == &genpd_bus_type)
> +		device_unregister(dev);
>  }
>  
>  static void genpd_dev_pm_sync(struct device *dev)
> @@ -2298,6 +2311,67 @@ int genpd_dev_pm_attach(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>  
> +/**
> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
> + * @dev: Device to attach.

Can you update the description of the above as well?

Thanks
Jon

-- 
nvpublic

[PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Jon Hunter]

On 29/05/18 11:04, Ulf Hansson wrote:
> To support devices being partitioned across multiple PM domains, let's
> begin with extending genpd to cope with these kind of configurations.
> 
> Therefore, add a new exported function genpd_dev_pm_attach_by_id(), which
> is similar to the existing genpd_dev_pm_attach(), but with the difference
> that it allows its callers to provide an index to the PM domain that it
> wants to attach.
> 
> Note that, genpd_dev_pm_attach_by_id() shall only be called by the driver
> core / PM core, similar to how the existing dev_pm_domain_attach() makes
> use of genpd_dev_pm_attach(). However, this is implemented by following
> changes on top.
> 
> Because, only one PM domain can be attached per device, genpd needs to
> create a virtual device that it can attach/detach instead. More precisely,
> let the new function genpd_dev_pm_attach_by_id() register a virtual struct
> device via calling device_register(). Then let it attach this device to the
> corresponding PM domain, rather than the one that is provided by the
> caller. The actual attaching is done via re-using the existing genpd OF
> functions.
> 
> At successful attachment, genpd_dev_pm_attach_by_id() returns the created
> virtual device, which allows the caller to operate on it to deal with power
> management. Following changes on top, provides more details in this
> regards.
> 
> To deal with detaching of a PM domain for the multiple PM domains case,
> let's also extend the existing genpd_dev_pm_detach() function, to cover the
> cleanup of the created virtual device, via make it call device_unregister()
> on it. In this way, there is no need to introduce a new function to deal
> with detach for the multiple PM domain case, but instead the existing one
> is re-used.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Fixed comments from Jon. Clarified function descriptions
> 	and changelog.
> 
> ---
>  drivers/base/power/domain.c | 80 +++++++++++++++++++++++++++++++++++++
>  include/linux/pm_domain.h   |  8 ++++
>  2 files changed, 88 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 2af99bfcbe3c..2b496d79159d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>  }
>  EXPORT_SYMBOL_GPL(of_genpd_remove_last);
>  
> +static void genpd_release_dev(struct device *dev)
> +{
> +	kfree(dev);
> +}
> +
> +static struct bus_type genpd_bus_type = {
> +	.name		= "genpd",
> +};
> +
>  /**
>   * genpd_dev_pm_detach - Detach a device from its PM domain.
>   * @dev: Device to detach.
> @@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
>  
>  	/* Check if PM domain can be powered off after removing this device. */
>  	genpd_queue_power_off_work(pd);
> +
> +	/* Unregister the device if it was created by genpd. */
> +	if (dev->bus == &genpd_bus_type)
> +		device_unregister(dev);
>  }
>  
>  static void genpd_dev_pm_sync(struct device *dev)
> @@ -2298,6 +2311,67 @@ int genpd_dev_pm_attach(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>  
> +/**
> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
> + * @dev: Device to attach.

Can you update the description of the above as well?

Thanks
Jon

-- 
nvpublic

Re: [PATCH v2 6/9] PM / Domains: Don't attach devices in genpd with multi PM domains

[Jon Hunter]

On 29/05/18 11:04, Ulf Hansson wrote:
> The power-domain DT property may now contain a list of PM domain
> specifiers, which represents that a device are partitioned across multiple
> PM domains. This leads to a new situation in genpd_dev_pm_attach(), as only
> one PM domain can be attached per device.
> 
> To remain things simple for the most common configuration, when a single PM
> domain is used, let's treat the multiple PM domain case as being specific.
> 
> In other words, let's change genpd_dev_pm_attach() to check for multiple PM
> domains and prevent it from attach any PM domain for this case. Instead,
> leave this to be managed separately, from following changes to genpd.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Minor update to changelog to mention "PM domain specifiers" rather
> 	than a "list of phandles".
> 
> ---
>  drivers/base/power/domain.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 7ebf7993273a..12a20f21974d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2229,10 +2229,10 @@ static void genpd_dev_pm_sync(struct device *dev)
>   * attaches the device to retrieved pm_domain ops.
>   *
>   * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> - * PM domain or a negative error code in case of failures. Note that if a
> - * power-domain exists for the device, but it cannot be found or turned on,
> - * then return -EPROBE_DEFER to ensure that the device is not probed and to
> - * re-try again later.
> + * PM domain or when multiple power-domains exists for it, else a negative error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
>   */
>  int genpd_dev_pm_attach(struct device *dev)
>  {
> @@ -2243,10 +2243,18 @@ int genpd_dev_pm_attach(struct device *dev)
>  	if (!dev->of_node)
>  		return 0;
>  
> +	/*
> +	 * Devices with multiple PM domains must be attached separately, as we
> +	 * can only attach one PM domain per device.
> +	 */
> +	if (of_count_phandle_with_args(dev->of_node, "power-domains",
> +				       "#power-domain-cells") != 1)
> +		return 0;
> +
>  	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
>  					"#power-domain-cells", 0, &pd_args);
>  	if (ret < 0)
> -		return 0;
> +		return ret;
>  
>  	mutex_lock(&gpd_list_lock);
>  	pd = genpd_get_from_provider(&pd_args);
> 

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 6/9] PM / Domains: Don't attach devices in genpd with multi PM domains

[Jon Hunter]

On 29/05/18 11:04, Ulf Hansson wrote:
> The power-domain DT property may now contain a list of PM domain
> specifiers, which represents that a device are partitioned across multiple
> PM domains. This leads to a new situation in genpd_dev_pm_attach(), as only
> one PM domain can be attached per device.
> 
> To remain things simple for the most common configuration, when a single PM
> domain is used, let's treat the multiple PM domain case as being specific.
> 
> In other words, let's change genpd_dev_pm_attach() to check for multiple PM
> domains and prevent it from attach any PM domain for this case. Instead,
> leave this to be managed separately, from following changes to genpd.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Minor update to changelog to mention "PM domain specifiers" rather
> 	than a "list of phandles".
> 
> ---
>  drivers/base/power/domain.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 7ebf7993273a..12a20f21974d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2229,10 +2229,10 @@ static void genpd_dev_pm_sync(struct device *dev)
>   * attaches the device to retrieved pm_domain ops.
>   *
>   * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> - * PM domain or a negative error code in case of failures. Note that if a
> - * power-domain exists for the device, but it cannot be found or turned on,
> - * then return -EPROBE_DEFER to ensure that the device is not probed and to
> - * re-try again later.
> + * PM domain or when multiple power-domains exists for it, else a negative error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
>   */
>  int genpd_dev_pm_attach(struct device *dev)
>  {
> @@ -2243,10 +2243,18 @@ int genpd_dev_pm_attach(struct device *dev)
>  	if (!dev->of_node)
>  		return 0;
>  
> +	/*
> +	 * Devices with multiple PM domains must be attached separately, as we
> +	 * can only attach one PM domain per device.
> +	 */
> +	if (of_count_phandle_with_args(dev->of_node, "power-domains",
> +				       "#power-domain-cells") != 1)
> +		return 0;
> +
>  	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
>  					"#power-domain-cells", 0, &pd_args);
>  	if (ret < 0)
> -		return 0;
> +		return ret;
>  
>  	mutex_lock(&gpd_list_lock);
>  	pd = genpd_get_from_provider(&pd_args);
> 

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

[PATCH v2 6/9] PM / Domains: Don't attach devices in genpd with multi PM domains

[Jon Hunter]

On 29/05/18 11:04, Ulf Hansson wrote:
> The power-domain DT property may now contain a list of PM domain
> specifiers, which represents that a device are partitioned across multiple
> PM domains. This leads to a new situation in genpd_dev_pm_attach(), as only
> one PM domain can be attached per device.
> 
> To remain things simple for the most common configuration, when a single PM
> domain is used, let's treat the multiple PM domain case as being specific.
> 
> In other words, let's change genpd_dev_pm_attach() to check for multiple PM
> domains and prevent it from attach any PM domain for this case. Instead,
> leave this to be managed separately, from following changes to genpd.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree at vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Minor update to changelog to mention "PM domain specifiers" rather
> 	than a "list of phandles".
> 
> ---
>  drivers/base/power/domain.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 7ebf7993273a..12a20f21974d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2229,10 +2229,10 @@ static void genpd_dev_pm_sync(struct device *dev)
>   * attaches the device to retrieved pm_domain ops.
>   *
>   * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> - * PM domain or a negative error code in case of failures. Note that if a
> - * power-domain exists for the device, but it cannot be found or turned on,
> - * then return -EPROBE_DEFER to ensure that the device is not probed and to
> - * re-try again later.
> + * PM domain or when multiple power-domains exists for it, else a negative error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
>   */
>  int genpd_dev_pm_attach(struct device *dev)
>  {
> @@ -2243,10 +2243,18 @@ int genpd_dev_pm_attach(struct device *dev)
>  	if (!dev->of_node)
>  		return 0;
>  
> +	/*
> +	 * Devices with multiple PM domains must be attached separately, as we
> +	 * can only attach one PM domain per device.
> +	 */
> +	if (of_count_phandle_with_args(dev->of_node, "power-domains",
> +				       "#power-domain-cells") != 1)
> +		return 0;
> +
>  	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
>  					"#power-domain-cells", 0, &pd_args);
>  	if (ret < 0)
> -		return 0;
> +		return ret;
>  
>  	mutex_lock(&gpd_list_lock);
>  	pd = genpd_get_from_provider(&pd_args);
> 

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()

[Jon Hunter]

On 29/05/18 11:04, Ulf Hansson wrote:
> To extend genpd to deal with allowing multiple PM domains per device, some
> of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
> by moving some of the code into a sub-function.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 12a20f21974d..2af99bfcbe3c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2221,38 +2221,15 @@ static void genpd_dev_pm_sync(struct device *dev)
>  	genpd_queue_power_off_work(pd);
>  }
>  
> -/**
> - * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> - * @dev: Device to attach.
> - *
> - * Parse device's OF node to find a PM domain specifier. If such is found,
> - * attaches the device to retrieved pm_domain ops.
> - *
> - * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> - * PM domain or when multiple power-domains exists for it, else a negative error
> - * code. Note that if a power-domain exists for the device, but it cannot be
> - * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> - * not probed and to re-try again later.
> - */
> -int genpd_dev_pm_attach(struct device *dev)
> +static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
> +				 unsigned int index)
>  {
>  	struct of_phandle_args pd_args;
>  	struct generic_pm_domain *pd;
>  	int ret;
>  
> -	if (!dev->of_node)
> -		return 0;
> -
> -	/*
> -	 * Devices with multiple PM domains must be attached separately, as we
> -	 * can only attach one PM domain per device.
> -	 */
> -	if (of_count_phandle_with_args(dev->of_node, "power-domains",
> -				       "#power-domain-cells") != 1)
> -		return 0;
> -
> -	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
> -					"#power-domain-cells", 0, &pd_args);
> +	ret = of_parse_phandle_with_args(np, "power-domains",
> +				"#power-domain-cells", index, &pd_args);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -2290,6 +2267,35 @@ int genpd_dev_pm_attach(struct device *dev)
>  
>  	return ret ? -EPROBE_DEFER : 1;
>  }
> +
> +/**
> + * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> + * @dev: Device to attach.
> + *
> + * Parse device's OF node to find a PM domain specifier. If such is found,
> + * attaches the device to retrieved pm_domain ops.
> + *
> + * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> + * PM domain or when multiple power-domains exists for it, else a negative error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
> + */
> +int genpd_dev_pm_attach(struct device *dev)
> +{
> +	if (!dev->of_node)
> +		return 0;
> +
> +	/*
> +	 * Devices with multiple PM domains must be attached separately, as we
> +	 * can only attach one PM domain per device.
> +	 */
> +	if (of_count_phandle_with_args(dev->of_node, "power-domains",
> +				       "#power-domain-cells") != 1)
> +		return 0;
> +
> +	return __genpd_dev_pm_attach(dev, dev->of_node, 0);
> +}
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>  
>  static const struct of_device_id idle_state_match[] = {

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()

[Jon Hunter]

On 29/05/18 11:04, Ulf Hansson wrote:
> To extend genpd to deal with allowing multiple PM domains per device, some
> of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
> by moving some of the code into a sub-function.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 12a20f21974d..2af99bfcbe3c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2221,38 +2221,15 @@ static void genpd_dev_pm_sync(struct device *dev)
>  	genpd_queue_power_off_work(pd);
>  }
>  
> -/**
> - * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> - * @dev: Device to attach.
> - *
> - * Parse device's OF node to find a PM domain specifier. If such is found,
> - * attaches the device to retrieved pm_domain ops.
> - *
> - * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> - * PM domain or when multiple power-domains exists for it, else a negative error
> - * code. Note that if a power-domain exists for the device, but it cannot be
> - * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> - * not probed and to re-try again later.
> - */
> -int genpd_dev_pm_attach(struct device *dev)
> +static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
> +				 unsigned int index)
>  {
>  	struct of_phandle_args pd_args;
>  	struct generic_pm_domain *pd;
>  	int ret;
>  
> -	if (!dev->of_node)
> -		return 0;
> -
> -	/*
> -	 * Devices with multiple PM domains must be attached separately, as we
> -	 * can only attach one PM domain per device.
> -	 */
> -	if (of_count_phandle_with_args(dev->of_node, "power-domains",
> -				       "#power-domain-cells") != 1)
> -		return 0;
> -
> -	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
> -					"#power-domain-cells", 0, &pd_args);
> +	ret = of_parse_phandle_with_args(np, "power-domains",
> +				"#power-domain-cells", index, &pd_args);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -2290,6 +2267,35 @@ int genpd_dev_pm_attach(struct device *dev)
>  
>  	return ret ? -EPROBE_DEFER : 1;
>  }
> +
> +/**
> + * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> + * @dev: Device to attach.
> + *
> + * Parse device's OF node to find a PM domain specifier. If such is found,
> + * attaches the device to retrieved pm_domain ops.
> + *
> + * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> + * PM domain or when multiple power-domains exists for it, else a negative error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
> + */
> +int genpd_dev_pm_attach(struct device *dev)
> +{
> +	if (!dev->of_node)
> +		return 0;
> +
> +	/*
> +	 * Devices with multiple PM domains must be attached separately, as we
> +	 * can only attach one PM domain per device.
> +	 */
> +	if (of_count_phandle_with_args(dev->of_node, "power-domains",
> +				       "#power-domain-cells") != 1)
> +		return 0;
> +
> +	return __genpd_dev_pm_attach(dev, dev->of_node, 0);
> +}
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>  
>  static const struct of_device_id idle_state_match[] = {

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

[PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()

[Jon Hunter]

On 29/05/18 11:04, Ulf Hansson wrote:
> To extend genpd to deal with allowing multiple PM domains per device, some
> of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
> by moving some of the code into a sub-function.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 12a20f21974d..2af99bfcbe3c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2221,38 +2221,15 @@ static void genpd_dev_pm_sync(struct device *dev)
>  	genpd_queue_power_off_work(pd);
>  }
>  
> -/**
> - * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> - * @dev: Device to attach.
> - *
> - * Parse device's OF node to find a PM domain specifier. If such is found,
> - * attaches the device to retrieved pm_domain ops.
> - *
> - * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> - * PM domain or when multiple power-domains exists for it, else a negative error
> - * code. Note that if a power-domain exists for the device, but it cannot be
> - * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> - * not probed and to re-try again later.
> - */
> -int genpd_dev_pm_attach(struct device *dev)
> +static int __genpd_dev_pm_attach(struct device *dev, struct device_node *np,
> +				 unsigned int index)
>  {
>  	struct of_phandle_args pd_args;
>  	struct generic_pm_domain *pd;
>  	int ret;
>  
> -	if (!dev->of_node)
> -		return 0;
> -
> -	/*
> -	 * Devices with multiple PM domains must be attached separately, as we
> -	 * can only attach one PM domain per device.
> -	 */
> -	if (of_count_phandle_with_args(dev->of_node, "power-domains",
> -				       "#power-domain-cells") != 1)
> -		return 0;
> -
> -	ret = of_parse_phandle_with_args(dev->of_node, "power-domains",
> -					"#power-domain-cells", 0, &pd_args);
> +	ret = of_parse_phandle_with_args(np, "power-domains",
> +				"#power-domain-cells", index, &pd_args);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -2290,6 +2267,35 @@ int genpd_dev_pm_attach(struct device *dev)
>  
>  	return ret ? -EPROBE_DEFER : 1;
>  }
> +
> +/**
> + * genpd_dev_pm_attach - Attach a device to its PM domain using DT.
> + * @dev: Device to attach.
> + *
> + * Parse device's OF node to find a PM domain specifier. If such is found,
> + * attaches the device to retrieved pm_domain ops.
> + *
> + * Returns 1 on successfully attached PM domain, 0 when the device don't need a
> + * PM domain or when multiple power-domains exists for it, else a negative error
> + * code. Note that if a power-domain exists for the device, but it cannot be
> + * found or turned on, then return -EPROBE_DEFER to ensure that the device is
> + * not probed and to re-try again later.
> + */
> +int genpd_dev_pm_attach(struct device *dev)
> +{
> +	if (!dev->of_node)
> +		return 0;
> +
> +	/*
> +	 * Devices with multiple PM domains must be attached separately, as we
> +	 * can only attach one PM domain per device.
> +	 */
> +	if (of_count_phandle_with_args(dev->of_node, "power-domains",
> +				       "#power-domain-cells") != 1)
> +		return 0;
> +
> +	return __genpd_dev_pm_attach(dev, dev->of_node, 0);
> +}
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>  
>  static const struct of_device_id idle_state_match[] = {

Acked-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 5/9] PM / Domains: dt: Allow power-domain property to be a list of specifiers

[Rob Herring]

On Tue, May 29, 2018 at 12:04:17PM +0200, Ulf Hansson wrote:
> To be able to describe topologies where devices are partitioned across
> multiple power domains, let's extend the power-domain property to allow
> being a list of PM domain specifiers.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Fixed comments from Geert. Re-worded to "PM domain specifiers" and
> 	clarified DT example.
> 
> ---
>  .../bindings/power/power_domain.txt           | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v2 5/9] PM / Domains: dt: Allow power-domain property to be a list of specifiers

[Rob Herring]

On Tue, May 29, 2018 at 12:04:17PM +0200, Ulf Hansson wrote:
> To be able to describe topologies where devices are partitioned across
> multiple power domains, let's extend the power-domain property to allow
> being a list of PM domain specifiers.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree at vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	- Fixed comments from Geert. Re-worded to "PM domain specifiers" and
> 	clarified DT example.
> 
> ---
>  .../bindings/power/power_domain.txt           | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 1/9] PM / Domains: Drop extern declarations of functions in pm_domain.h

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> Using "extern" to declare a function in a public header file is somewhat
> pointless, but also doesn't hurt. However, to make all the function
> declarations in pm_domain.h to be consistent, let's drop the use of
> "extern".
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  include/linux/pm_domain.h | 51 ++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 28 deletions(-)

Finally.

I wanted to do that earlier as checkpatch complained non stop about
such declarations :)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH v2 1/9] PM / Domains: Drop extern declarations of functions in pm_domain.h

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> Using "extern" to declare a function in a public header file is somewhat
> pointless, but also doesn't hurt. However, to make all the function
> declarations in pm_domain.h to be consistent, let's drop the use of
> "extern".
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  include/linux/pm_domain.h | 51 ++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 28 deletions(-)

Finally.

I wanted to do that earlier as checkpatch complained non stop about
such declarations :)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 2/9] PM / Domains: Drop __pm_genpd_add_device()

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> There are still a few non-DT existing users of genpd, however neither of
> them uses __pm_genpd_add_device(), hence let's drop it.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 10 ++++------
>  include/linux/pm_domain.h   | 14 +++-----------
>  2 files changed, 7 insertions(+), 17 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH v2 2/9] PM / Domains: Drop __pm_genpd_add_device()

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> There are still a few non-DT existing users of genpd, however neither of
> them uses __pm_genpd_add_device(), hence let's drop it.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 10 ++++------
>  include/linux/pm_domain.h   | 14 +++-----------
>  2 files changed, 7 insertions(+), 17 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 3/9] PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> There is no need to pass a genpd struct to pm_genpd_remove_device(), as we
> already have the information about the PM domain (genpd) through the device
> structure.
> 
> Additionally, we don't allow to remove a PM domain from a device, other
> than the one it may have assigned to it, so really it does not make sense
> to have a separate in-param for it.
> 
> For these reason, drop it and update the current only call to
> pm_genpd_remove_device() from amdgpu_acp.
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c             | 8 ++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 2 +-
>  include/linux/pm_domain.h               | 5 ++---
>  3 files changed, 7 insertions(+), 8 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH v2 3/9] PM / Domains: Drop genpd as in-param for pm_genpd_remove_device()

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> There is no need to pass a genpd struct to pm_genpd_remove_device(), as we
> already have the information about the PM domain (genpd) through the device
> structure.
> 
> Additionally, we don't allow to remove a PM domain from a device, other
> than the one it may have assigned to it, so really it does not make sense
> to have a separate in-param for it.
> 
> For these reason, drop it and update the current only call to
> pm_genpd_remove_device() from amdgpu_acp.
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian K?nig <christian.koenig@amd.com>
> Cc: David (ChunMing) Zhou <David1.Zhou@amd.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c             | 8 ++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 2 +-
>  include/linux/pm_domain.h               | 5 ++---
>  3 files changed, 7 insertions(+), 8 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 4/9] PM / Domains: Drop unused parameter in genpd_allocate_dev_data()

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> The in-parameter struct generic_pm_domain *genpd to
> genpd_allocate_dev_data() is unused, so let's drop it.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH v2 4/9] PM / Domains: Drop unused parameter in genpd_allocate_dev_data()

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> The in-parameter struct generic_pm_domain *genpd to
> genpd_allocate_dev_data() is unused, so let's drop it.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Ulf Hansson]

[...]

>> +/**
>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>> + * @dev: Device to attach.
>
> Can you update the description of the above as well?

Yes, like below?

genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
@dev: The device used to lookup the PM domain.

Kind regards
Uffe

[PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Ulf Hansson]

[...]

>> +/**
>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>> + * @dev: Device to attach.
>
> Can you update the description of the above as well?

Yes, like below?

genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
@dev: The device used to lookup the PM domain.

Kind regards
Uffe

Re: [PATCH v2 5/9] PM / Domains: dt: Allow power-domain property to be a list of specifiers

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> To be able to describe topologies where devices are partitioned across
> multiple power domains, let's extend the power-domain property to allow
> being a list of PM domain specifiers.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH v2 5/9] PM / Domains: dt: Allow power-domain property to be a list of specifiers

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> To be able to describe topologies where devices are partitioned across
> multiple power domains, let's extend the power-domain property to allow
> being a list of PM domain specifiers.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree at vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> To extend genpd to deal with allowing multiple PM domains per device, some
> of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
> by moving some of the code into a sub-function.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 27 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH v2 7/9] PM / Domains: Split genpd_dev_pm_attach()

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> To extend genpd to deal with allowing multiple PM domains per device, some
> of the code in genpd_dev_pm_attach() can be re-used. Let's prepare for this
> by moving some of the code into a sub-function.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 60 ++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 27 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Jon Hunter]

On 31/05/18 07:17, Ulf Hansson wrote:
> [...]
> 
>>> +/**
>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>> + * @dev: Device to attach.
>>
>> Can you update the description of the above as well?
> 
> Yes, like below?
> 
> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
> @dev: The device used to lookup the PM domain.

Yes perfect.

Thanks
Jon

-- 
nvpublic

Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Jon Hunter]

On 31/05/18 07:17, Ulf Hansson wrote:
> [...]
> 
>>> +/**
>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>> + * @dev: Device to attach.
>>
>> Can you update the description of the above as well?
> 
> Yes, like below?
> 
> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
> @dev: The device used to lookup the PM domain.

Yes perfect.

Thanks
Jon

-- 
nvpublic

[PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Jon Hunter]

On 31/05/18 07:17, Ulf Hansson wrote:
> [...]
> 
>>> +/**
>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>> + * @dev: Device to attach.
>>
>> Can you update the description of the above as well?
> 
> Yes, like below?
> 
> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
> @dev: The device used to lookup the PM domain.

Yes perfect.

Thanks
Jon

-- 
nvpublic

Re: [PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> Changes in v2:
> 	- Addressed comments from Geert around DT doc.
> 	- Addressed comments from Jon around clarification of how to use this
> 	and changes to returned error codes.
> 	- Fixed build error in case CONFIG_PM was unset.
> 
> There are devices that are partitioned across multiple PM domains. Currently
> these can't be supported well by the available PM infrastructures we have in
> the kernel. This series is an attempt to address this.
> 
> The interesting parts happens from patch 5 an onwards, including a minor DT
> update to the existing power-domain bindings, the 4 earlier are just trivial
> clean-ups of some related code in genpd, which I happened to stumble over.
> 
> Some additional background:
> 
> One existing case where devices are partitioned across multiple PM domains, is
> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
> series, trying to address these issues, however this is a new approach, while
> it re-uses the same concepts from DT point of view.
> 
> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
> controller. Each controller have its own independent PM domain, but are being
> partitioned across another shared PM domain for the USB super-speed logic.
> 
> Currently to make the drivers work, either the related PM domains needs to stay
> powered on always or the PM domain topology needs to be in-correctly modelled
> through sub-domains. In both cases PM domains may be powered on while they
> don't need to be, so in the end this means - wasting power -.
> 
> As stated above, this series intends to address these problem from a PM
> infrastructure point of view. More details are available in each changelog.
> 
> It should be noted that this series has been tested on HW, however only by using
> a home-cooked test PM domain driver for genpd and together with a test driver.
> This allowed me to play with PM domain (genpd), runtime PM and device links.
> 
> Any further deployment for real use cases are greatly appreciated. I am happy to
> to help, if needed!

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

[PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Viresh Kumar]

On 29-05-18, 12:04, Ulf Hansson wrote:
> Changes in v2:
> 	- Addressed comments from Geert around DT doc.
> 	- Addressed comments from Jon around clarification of how to use this
> 	and changes to returned error codes.
> 	- Fixed build error in case CONFIG_PM was unset.
> 
> There are devices that are partitioned across multiple PM domains. Currently
> these can't be supported well by the available PM infrastructures we have in
> the kernel. This series is an attempt to address this.
> 
> The interesting parts happens from patch 5 an onwards, including a minor DT
> update to the existing power-domain bindings, the 4 earlier are just trivial
> clean-ups of some related code in genpd, which I happened to stumble over.
> 
> Some additional background:
> 
> One existing case where devices are partitioned across multiple PM domains, is
> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
> series, trying to address these issues, however this is a new approach, while
> it re-uses the same concepts from DT point of view.
> 
> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
> controller. Each controller have its own independent PM domain, but are being
> partitioned across another shared PM domain for the USB super-speed logic.
> 
> Currently to make the drivers work, either the related PM domains needs to stay
> powered on always or the PM domain topology needs to be in-correctly modelled
> through sub-domains. In both cases PM domains may be powered on while they
> don't need to be, so in the end this means - wasting power -.
> 
> As stated above, this series intends to address these problem from a PM
> infrastructure point of view. More details are available in each changelog.
> 
> It should be noted that this series has been tested on HW, however only by using
> a home-cooked test PM domain driver for genpd and together with a test driver.
> This allowed me to play with PM domain (genpd), runtime PM and device links.
> 
> Any further deployment for real use cases are greatly appreciated. I am happy to
> to help, if needed!

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Ulf Hansson]

On 31 May 2018 at 10:03, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 31/05/18 07:17, Ulf Hansson wrote:
>> [...]
>>
>>>> +/**
>>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>>> + * @dev: Device to attach.
>>>
>>> Can you update the description of the above as well?
>>
>> Yes, like below?
>>
>> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
>> @dev: The device used to lookup the PM domain.
>
> Yes perfect.

I assume I can consider this as an ack and tested by tag, both for
patch 8 and 9?

Kind regards
Uffe

[PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Ulf Hansson]

On 31 May 2018 at 10:03, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 31/05/18 07:17, Ulf Hansson wrote:
>> [...]
>>
>>>> +/**
>>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>>> + * @dev: Device to attach.
>>>
>>> Can you update the description of the above as well?
>>
>> Yes, like below?
>>
>> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
>> @dev: The device used to lookup the PM domain.
>
> Yes perfect.

I assume I can consider this as an ack and tested by tag, both for
patch 8 and 9?

Kind regards
Uffe

Re: [PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Ulf Hansson]

On 31 May 2018 at 11:14, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29-05-18, 12:04, Ulf Hansson wrote:
>> Changes in v2:
>>       - Addressed comments from Geert around DT doc.
>>       - Addressed comments from Jon around clarification of how to use this
>>       and changes to returned error codes.
>>       - Fixed build error in case CONFIG_PM was unset.
>>
>> There are devices that are partitioned across multiple PM domains. Currently
>> these can't be supported well by the available PM infrastructures we have in
>> the kernel. This series is an attempt to address this.
>>
>> The interesting parts happens from patch 5 an onwards, including a minor DT
>> update to the existing power-domain bindings, the 4 earlier are just trivial
>> clean-ups of some related code in genpd, which I happened to stumble over.
>>
>> Some additional background:
>>
>> One existing case where devices are partitioned across multiple PM domains, is
>> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
>> series, trying to address these issues, however this is a new approach, while
>> it re-uses the same concepts from DT point of view.
>>
>> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
>> controller. Each controller have its own independent PM domain, but are being
>> partitioned across another shared PM domain for the USB super-speed logic.
>>
>> Currently to make the drivers work, either the related PM domains needs to stay
>> powered on always or the PM domain topology needs to be in-correctly modelled
>> through sub-domains. In both cases PM domains may be powered on while they
>> don't need to be, so in the end this means - wasting power -.
>>
>> As stated above, this series intends to address these problem from a PM
>> infrastructure point of view. More details are available in each changelog.
>>
>> It should be noted that this series has been tested on HW, however only by using
>> a home-cooked test PM domain driver for genpd and together with a test driver.
>> This allowed me to play with PM domain (genpd), runtime PM and device links.
>>
>> Any further deployment for real use cases are greatly appreciated. I am happy to
>> to help, if needed!
>
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

Thanks!

Kind regards
Uffe

[PATCH v2 0/9] PM / Domains: Add support for multi PM domains per device

[Ulf Hansson]

On 31 May 2018 at 11:14, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29-05-18, 12:04, Ulf Hansson wrote:
>> Changes in v2:
>>       - Addressed comments from Geert around DT doc.
>>       - Addressed comments from Jon around clarification of how to use this
>>       and changes to returned error codes.
>>       - Fixed build error in case CONFIG_PM was unset.
>>
>> There are devices that are partitioned across multiple PM domains. Currently
>> these can't be supported well by the available PM infrastructures we have in
>> the kernel. This series is an attempt to address this.
>>
>> The interesting parts happens from patch 5 an onwards, including a minor DT
>> update to the existing power-domain bindings, the 4 earlier are just trivial
>> clean-ups of some related code in genpd, which I happened to stumble over.
>>
>> Some additional background:
>>
>> One existing case where devices are partitioned across multiple PM domains, is
>> the Nvida Tegra 124/210 X-USB subsystem. A while ago Jon Hunter (Nvidia) sent a
>> series, trying to address these issues, however this is a new approach, while
>> it re-uses the same concepts from DT point of view.
>>
>> The Tegra 124/210 X-USB subsystem contains of a host controller and a device
>> controller. Each controller have its own independent PM domain, but are being
>> partitioned across another shared PM domain for the USB super-speed logic.
>>
>> Currently to make the drivers work, either the related PM domains needs to stay
>> powered on always or the PM domain topology needs to be in-correctly modelled
>> through sub-domains. In both cases PM domains may be powered on while they
>> don't need to be, so in the end this means - wasting power -.
>>
>> As stated above, this series intends to address these problem from a PM
>> infrastructure point of view. More details are available in each changelog.
>>
>> It should be noted that this series has been tested on HW, however only by using
>> a home-cooked test PM domain driver for genpd and together with a test driver.
>> This allowed me to play with PM domain (genpd), runtime PM and device links.
>>
>> Any further deployment for real use cases are greatly appreciated. I am happy to
>> to help, if needed!
>
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

Thanks!

Kind regards
Uffe

Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Jon Hunter]

On 31/05/18 11:44, Ulf Hansson wrote:
> On 31 May 2018 at 10:03, Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>> On 31/05/18 07:17, Ulf Hansson wrote:
>>> [...]
>>>
>>>>> +/**
>>>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>>>> + * @dev: Device to attach.
>>>>
>>>> Can you update the description of the above as well?
>>>
>>> Yes, like below?
>>>
>>> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
>>> @dev: The device used to lookup the PM domain.
>>
>> Yes perfect.
> 
> I assume I can consider this as an ack and tested by tag, both for
> patch 8 and 9?

Yes you can.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Jon Hunter]

On 31/05/18 11:44, Ulf Hansson wrote:
> On 31 May 2018 at 10:03, Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>> On 31/05/18 07:17, Ulf Hansson wrote:
>>> [...]
>>>
>>>>> +/**
>>>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>>>> + * @dev: Device to attach.
>>>>
>>>> Can you update the description of the above as well?
>>>
>>> Yes, like below?
>>>
>>> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
>>> @dev: The device used to lookup the PM domain.
>>
>> Yes perfect.
> 
> I assume I can consider this as an ack and tested by tag, both for
> patch 8 and 9?

Yes you can.

Cheers
Jon

-- 
nvpublic

[PATCH v2 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Jon Hunter]

On 31/05/18 11:44, Ulf Hansson wrote:
> On 31 May 2018 at 10:03, Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>> On 31/05/18 07:17, Ulf Hansson wrote:
>>> [...]
>>>
>>>>> +/**
>>>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>>>> + * @dev: Device to attach.
>>>>
>>>> Can you update the description of the above as well?
>>>
>>> Yes, like below?
>>>
>>> genpd_dev_pm_attach_by_id() - Associate a device with one of its PM domains.
>>> @dev: The device used to lookup the PM domain.
>>
>> Yes perfect.
> 
> I assume I can consider this as an ack and tested by tag, both for
> patch 8 and 9?

Yes you can.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Rajendra Nayak]

On 05/30/2018 08:00 PM, Jon Hunter wrote:
>>> My only other comments on this series are ...
>>>
>>> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>>>    that the DT binding has a 'power-domain-names' property.
>> I think it makes sense, but my plan was to do that as second step on
>> top. Are you okay with that as well?
> Yes that is fine with me.

Ulf, do you have a patch for this? I am working on moving some of qcom
remoteproc drivers to use this series and it will help to have a
dev_pm_domain_attach_by_name() api.

Thanks,
Rajendra
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Rajendra Nayak]

On 05/30/2018 08:00 PM, Jon Hunter wrote:
>>> My only other comments on this series are ...
>>>
>>> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>>>    that the DT binding has a 'power-domain-names' property.
>> I think it makes sense, but my plan was to do that as second step on
>> top. Are you okay with that as well?
> Yes that is fine with me.

Ulf, do you have a patch for this? I am working on moving some of qcom
remoteproc drivers to use this series and it will help to have a
dev_pm_domain_attach_by_name() api.

Thanks,
Rajendra
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Ulf Hansson]

On 27 June 2018 at 10:55, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>
>
> On 05/30/2018 08:00 PM, Jon Hunter wrote:
>>>> My only other comments on this series are ...
>>>>
>>>> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>>>>    that the DT binding has a 'power-domain-names' property.
>>> I think it makes sense, but my plan was to do that as second step on
>>> top. Are you okay with that as well?
>> Yes that is fine with me.
>
> Ulf, do you have a patch for this? I am working on moving some of qcom
> remoteproc drivers to use this series and it will help to have a
> dev_pm_domain_attach_by_name() api.

Right. Besides this I have been working on a little improvement,
avoiding to power the PM domain while attaching.

I post a series in a day or two.

Kind regards
Uffe

[PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Ulf Hansson]

On 27 June 2018 at 10:55, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>
>
> On 05/30/2018 08:00 PM, Jon Hunter wrote:
>>>> My only other comments on this series are ...
>>>>
>>>> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>>>>    that the DT binding has a 'power-domain-names' property.
>>> I think it makes sense, but my plan was to do that as second step on
>>> top. Are you okay with that as well?
>> Yes that is fine with me.
>
> Ulf, do you have a patch for this? I am working on moving some of qcom
> remoteproc drivers to use this series and it will help to have a
> dev_pm_domain_attach_by_name() api.

Right. Besides this I have been working on a little improvement,
avoiding to power the PM domain while attaching.

I post a series in a day or two.

Kind regards
Uffe

Re: [PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Rajendra Nayak]

On 06/27/2018 02:35 PM, Ulf Hansson wrote:
> On 27 June 2018 at 10:55, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>
>>
>> On 05/30/2018 08:00 PM, Jon Hunter wrote:
>>>>> My only other comments on this series are ...
>>>>>
>>>>> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>>>>>    that the DT binding has a 'power-domain-names' property.
>>>> I think it makes sense, but my plan was to do that as second step on
>>>> top. Are you okay with that as well?
>>> Yes that is fine with me.
>>
>> Ulf, do you have a patch for this? I am working on moving some of qcom
>> remoteproc drivers to use this series and it will help to have a
>> dev_pm_domain_attach_by_name() api.
> 
> Right. Besides this I have been working on a little improvement,
> avoiding to power the PM domain while attaching.
> 
> I post a series in a day or two.

great, thank you.

> 
> Kind regards
> Uffe
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v2 9/9] PM / Domains: Add dev_pm_domain_attach_by_id() to manage multi PM domains

[Rajendra Nayak]

On 06/27/2018 02:35 PM, Ulf Hansson wrote:
> On 27 June 2018 at 10:55, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>>
>>
>> On 05/30/2018 08:00 PM, Jon Hunter wrote:
>>>>> My only other comments on this series are ...
>>>>>
>>>>> 1. I think it would be nice to have an dev_pm_domain_attach_by_name() and
>>>>>    that the DT binding has a 'power-domain-names' property.
>>>> I think it makes sense, but my plan was to do that as second step on
>>>> top. Are you okay with that as well?
>>> Yes that is fine with me.
>>
>> Ulf, do you have a patch for this? I am working on moving some of qcom
>> remoteproc drivers to use this series and it will help to have a
>> dev_pm_domain_attach_by_name() api.
> 
> Right. Besides this I have been working on a little improvement,
> avoiding to power the PM domain while attaching.
> 
> I post a series in a day or two.

great, thank you.

> 
> Kind regards
> Uffe
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation