PM / Domains: Add support for removing PM domains

Ideally, if we are returning a reference to a PM domain via a call to of_genpd_get_from_provider(), then we should keep track of such references via a reference count. The reference count could then be used to determine if a PM domain can be safely removed. Alternatively, it is possible to avoid such external references by providing APIs to access the PM domain and hence, eliminate any calls to of_genpd_get_from_provider().

Add new helper functions for adding a device and a subdomain to a PM domain when using device-tree, so that external calls to of_genpd_get_from_provider() can be removed.

Signed-off-by Jon Hunter <[email protected]>

---
 drivers/base/power/domain.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   | 16 ++++++++++++++++
 2 files changed, 60 insertions(+)

drivers/base/power/domain.c +44 -0

index 5bdb42bf40f4..b24893499454 100644
@@ -1706,6 +1706,50 @@ struct generic_pm_domain *of_genpd_get_from_provider(
 EXPORT_SYMBOL_GPL(of_genpd_get_from_provider);
 
 /**
+ * of_genpd_add_device() - Add a device to an I/O PM domain
+ * @genpdspec: OF phandle args to use for look-up PM domain
+ * @dev: Device to be added.
+ *
+ * Looks-up an I/O PM domain based upon phandle args provided and adds
+ * the device to the PM domain. Returns a negative error code on failure.
+ */
+int of_genpd_add_device(struct of_phandle_args *genpdspec, struct device *dev)
+{
+	struct generic_pm_domain *genpd;
+
+	genpd = of_genpd_get_from_provider(genpdspec);
+	if (IS_ERR(genpd))
+		return PTR_ERR(genpd);
+
+	return pm_genpd_add_device(genpd, dev);
+}
+
+/**
+ * of_genpd_add_subdomain - Add a subdomain to an I/O PM domain.
+ * @parent_spec: OF phandle args to use for parent PM domain look-up
+ * @subdomain_spec: OF phandle args to use for subdomain look-up
+ *
+ * Looks-up a parent PM domain and subdomain based upon phandle args
+ * provided and adds the subdomain to the parent PM domain. Returns a
+ * negative error code on failure.
+ */
+int of_genpd_add_subdomain(struct of_phandle_args *parent_spec,
+			   struct of_phandle_args *subdomain_spec)
+{
+	struct generic_pm_domain *parent, *subdomain;
+
+	parent = of_genpd_get_from_provider(parent_spec);
+	if (IS_ERR(parent))
+		return PTR_ERR(parent);
+
+	subdomain = of_genpd_get_from_provider(subdomain_spec);
+	if (IS_ERR(subdomain))
+		return PTR_ERR(subdomain);
+
+	return pm_genpd_add_subdomain(parent, subdomain);
+}
+
+/**
  * genpd_dev_pm_detach - Detach a device from its PM domain.
  * @dev: Device to detach.
  * @power_off: Currently not used

include/linux/pm_domain.h +4 -0

index 49cd8890b873..2b6ee670b231 100644
@@ -210,6 +210,10 @@ struct generic_pm_domain *__of_genpd_xlate_simple(
 struct generic_pm_domain *__of_genpd_xlate_onecell(
 					struct of_phandle_args *genpdspec,
 					void *data);
+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);
 
 int genpd_dev_pm_attach(struct device *dev);
 #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */

Ideally, if we are returning a reference to a PM domain via a call to of_genpd_get_from_provider(), then we should keep track of such references via a reference count. The reference count could then be used to determine if a PM domain can be safely removed. Alternatively, it is possible to avoid such external references by providing APIs to access the PM domain and hence, eliminate any calls to of_genpd_get_from_provider().

Add new helper functions for adding a device and a subdomain to a PM domain when using device-tree, so that external calls to of_genpd_get_from_provider() can be removed.

Hi Ulf,

Quoted 8 lines · Jon Hunter

Ideally, if we are returning a reference to a PM domain via a call to of_genpd_get_from_provider(), then we should keep track of such references via a reference count. The reference count could then be used to determine if a PM domain can be safely removed. Alternatively, it is possible to avoid such external references by providing APIs to access the PM domain and hence, eliminate any calls to of_genpd_get_from_provider().

Quoted 3 lines

Add new helper functions for adding a device and a subdomain to a PM domain when using device-tree, so that external calls to of_genpd_get_from_provider() can be removed.

While we are at it, does it make sense to add helpers for of_genpd_remove_device/subdomain() as well? Seems that these could be useful for doing the inverse when cleaning up.

Quoted 11 lines · Jon Hunter

Hi Ulf,

Quoted 8 lines · Jon Hunter

Ideally, if we are returning a reference to a PM domain via a call to of_genpd_get_from_provider(), then we should keep track of such references via a reference count. The reference count could then be used to determine if a PM domain can be safely removed. Alternatively, it is possible to avoid such external references by providing APIs to access the PM domain and hence, eliminate any calls to of_genpd_get_from_provider().

Quoted 7 lines

Quoted 3 lines

Add new helper functions for adding a device and a subdomain to a PM domain when using device-tree, so that external calls to of_genpd_get_from_provider() can be removed.

While we are at it, does it make sense to add helpers for of_genpd_remove_device/subdomain() as well? Seems that these could be useful for doing the inverse when cleaning up.

I would prefer if we could avoid adding new APIs until we really see the need for it.

Moreover, I would like to avoid us adding OF specific APIs, unless those can be really justified.
Hope that made sense.

Quoted 20 lines · Ulf Hansson

Quoted 11 lines · Jon Hunter

Hi Ulf,

Quoted 8 lines · Jon Hunter

Ideally, if we are returning a reference to a PM domain via a call to of_genpd_get_from_provider(), then we should keep track of such references via a reference count. The reference count could then be used to determine if a PM domain can be safely removed. Alternatively, it is possible to avoid such external references by providing APIs to access the PM domain and hence, eliminate any calls to of_genpd_get_from_provider().

Quoted 7 lines

Quoted 3 lines

Add new helper functions for adding a device and a subdomain to a PM domain when using device-tree, so that external calls to of_genpd_get_from_provider() can be removed.

While we are at it, does it make sense to add helpers for of_genpd_remove_device/subdomain() as well? Seems that these could be useful for doing the inverse when cleaning up.

Quoted 6 lines

I would prefer if we could avoid adding new APIs until we really see the need for it.

Moreover, I would like to avoid us adding OF specific APIs, unless those can be really justified.
Hope that made sense.

Yes makes sense. However, after this series, the pm_genpd_remove_device/subdomain really become provider only APIs because clients can no longer get access to the genpd struct. Although today none of the users of the new of_genpd_add_device/subdomain do any clean-up on failure, it is possible that someone may. Therefore, I was thinking that we should have a way for a client to remove a subdomain or device it has added.

Does that make sense? I guess we could always add those as needed.

I am looking at a use-case for usb where we are populating the pm-domains at runtime and we may need to clean-up on failure. However, I can always wait to add more APIs until we really need them.

Let me know what you think about my response to patch 6/8.

There should be no need to expose the generic_pm_domain structure and this eliminates the need to implement reference counting for any external reference to a PM domain. Therefore, make the functions pm_genpd_lookup_dev() and of_genpd_get_from_provider() private to the PM domain core.

Signed-off-by Jon Hunter <[email protected]>

---
 drivers/base/power/domain.c |  5 ++---
 include/linux/pm_domain.h   | 14 --------------
 2 files changed, 2 insertions(+), 17 deletions(-)

drivers/base/power/domain.c +0 -0

index b24893499454..c2ba1d6dbad3 100644

drivers/base/power/domain.c +0 -1

        struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);

@@ -1703,7 +1703,6 @@ struct generic_pm_domain *of_genpd_get_from_provider(
 
        return genpd;
 }
-EXPORT_SYMBOL_GPL(of_genpd_get_from_provider);
 
 /**
  * of_genpd_add_device() - Add a device to an I/O PM domain

include/linux/pm_domain.h +0 -15

index 2b6ee670b231..510512d5390e 100644
@@ -119,7 +119,6 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
        return to_gpd_data(dev->power.subsys_data->domain_data);
 }
 
-extern struct generic_pm_domain *pm_genpd_lookup_dev(struct device *dev);
 extern int __pm_genpd_add_device(struct generic_pm_domain *genpd,
                                 struct device *dev,
                                 struct gpd_timing_data *td);

@@ -141,10 +140,6 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
 {
        return ERR_PTR(-ENOSYS);
 }
-static inline struct generic_pm_domain *pm_genpd_lookup_dev(struct device *dev)
-{
-       return NULL;
-}
 static inline int __pm_genpd_add_device(struct generic_pm_domain *genpd,
                                        struct device *dev,
                                        struct gpd_timing_data *td)

@@ -201,9 +196,6 @@ typedef struct generic_pm_domain *(*genpd_xlate_t)(struct of_phandle_args *args,
 int __of_genpd_add_provider(struct device_node *np, genpd_xlate_t xlate,
                        void *data);
 void of_genpd_del_provider(struct device_node *np);
-struct generic_pm_domain *of_genpd_get_from_provider(
-                       struct of_phandle_args *genpdspec);
-
 struct generic_pm_domain *__of_genpd_xlate_simple(
                                        struct of_phandle_args *genpdspec,
                                        void *data);

@@ -224,12 +216,6 @@ static inline int __of_genpd_add_provider(struct device_node *np,
 }
 static inline void of_genpd_del_provider(struct device_node *np) {}
 
-static inline struct generic_pm_domain *of_genpd_get_from_provider(
-                       struct of_phandle_args *genpdspec)
-{
-       return NULL;
-}
-
 #define __of_genpd_xlate_simple                NULL
 #define __of_genpd_xlate_onecell       NULL
 
--

2.1.4

When a PM domain provider is added, there is currently no way to tell if the PM domain is actually present in the system. Naturally, the PM domain provider should not be registered if the PM domain has not been added. Nonetheless, to verify that the PM domain associated with a provider is present, store the 'provider_data' in the PM domain structure when adding the provider and make sure that the PM domain is found the list of PM domains registered.

The of_genpd_add_provider_simple() and of_genpd_add_provider_onecell() functions have been updated to store the 'provider_data' by default to avoid having to modify all the current PM domain provider implementations.

By doing this, we can also verify that the provider has been removed from the list of providers before removing a PM domain.

Signed-off-by Jon Hunter <[email protected]>

---
 drivers/base/power/domain.c | 74 +++++++++++++++++++++++++++++++++++++++++++--
 include/linux/pm_domain.h   | 13 +++++---
 2 files changed, 79 insertions(+), 8 deletions(-)

drivers/base/power/domain.c +0 -0

index c2ba1d6dbad3..72055fef6256 100644

drivers/base/power/domain.c +15 -0

+{
+       struct generic_pm_domain *gpd;
+
+       if (!data)
+               return false;
+
+       list_for_each_entry(gpd, &gpd_list, gpd_list_node)
+               if (gpd->provider_data == data)
+                       return true;
+
+       return false;
+}
+
+/**
  * __of_genpd_xlate_simple() - Xlate function for direct node-domain mapping
  * @genpdspec: OF phandle args to map into a PM domain
  * @data: xlate function private data - pointer to struct generic_pm_domain

@@ -1625,9 +1643,15 @@ EXPORT_SYMBOL_GPL(__of_genpd_xlate_onecell);
  * @np: Device node pointer associated with the PM domain provider.
  * @xlate: Callback for decoding PM domain from phandle arguments.
  * @data: Context pointer for @xlate callback.
+ *

drivers/base/power/domain.c +7 -0

 {
        struct of_genpd_provider *cp;
 

@@ -1635,6 +1659,13 @@ int __of_genpd_add_provider(struct device_node *np, genpd_xlate_t xlate,
        if (!cp)
                return -ENOMEM;
 
+       mutex_lock(&gpd_list_lock);
+
+       if (!pm_genpd_provider_present(data)) {
+               mutex_unlock(&gpd_list_lock);
+               return -EINVAL;
+       }
+
        cp->node = of_node_get(np);
        cp->data = data;
        cp->xlate = xlate;

drivers/base/power/domain.c +37 -0

+
+/**
+ * of_genpd_add_provider_simple() - Register a simple PM domain provider
+ * @np: Device node pointer associated with the PM domain provider.
+ * @genpd: Pointer to PM domain associated with the PM domain provider.
+ */
+int of_genpd_add_provider_simple(struct device_node *np,
+                                struct generic_pm_domain *genpd)
+{
+       if (!np || !genpd)
+               return -EINVAL;
+
+       genpd->provider_data = genpd;
+
+       return __of_genpd_add_provider(np, __of_genpd_xlate_simple, genpd);
+}
+EXPORT_SYMBOL_GPL(of_genpd_add_provider_simple);
+
+/**
+ * of_genpd_add_provider_onecell() - Register a onecell PM domain provider
+ * @np: Device node pointer associated with the PM domain provider.
+ * @data: Pointer to the data associated with the PM domain provider.
+ */
+int of_genpd_add_provider_onecell(struct device_node *np,
+                                 struct genpd_onecell_data *data)
+{
+       unsigned int i;
+
+       if (!np || !data)
+               return -EINVAL;
+
+       for (i = 0; i < data->num_domains; i++)
+               data->domains[i]->provider_data = data;
+
+       return __of_genpd_add_provider(np, __of_genpd_xlate_onecell, data);
+}
+EXPORT_SYMBOL_GPL(of_genpd_add_provider_onecell);
 
 /**
  * of_genpd_del_provider() - Remove a previously registered PM domain provider

include/linux/pm_domain.h +0 -0

index 510512d5390e..bed84413546f 100644

include/linux/pm_domain.h +7 -6

        const char *name;
        atomic_t sd_count;      /* Number of subdomains with power "on" */
        enum gpd_status status; /* Current state of the domain */

@@ -193,8 +194,10 @@ typedef struct generic_pm_domain *(*genpd_xlate_t)(struct of_phandle_args *args,
                                                void *data);
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
-int __of_genpd_add_provider(struct device_node *np, genpd_xlate_t xlate,
-                       void *data);
+int of_genpd_add_provider_simple(struct device_node *np,
+                                struct generic_pm_domain *genpd);
+int of_genpd_add_provider_onecell(struct device_node *np,
+                                 struct genpd_onecell_data *data);
 void of_genpd_del_provider(struct device_node *np);
 struct generic_pm_domain *__of_genpd_xlate_simple(
                                        struct of_phandle_args *genpdspec,

@@ -235,18 +238,18 @@ static inline int genpd_dev_pm_attach(struct device *dev)
 {
        return -ENODEV;
 }
-#endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 static inline int of_genpd_add_provider_simple(struct device_node *np,
                                        struct generic_pm_domain *genpd)
 {
-       return __of_genpd_add_provider(np, __of_genpd_xlate_simple, genpd);
+       return -ENOTSUPP;
 }
 static inline int of_genpd_add_provider_onecell(struct device_node *np,
                                        struct genpd_onecell_data *data)
 {
-       return __of_genpd_add_provider(np, __of_genpd_xlate_onecell, data);
+       return -ENOTSUPP;
 }
+#endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
 
 #ifdef CONFIG_PM
 extern int dev_pm_domain_attach(struct device *dev, bool power_on);
--

2.1.4

To remove a PM domain from the system, it is necessary to ensure that any PM domain providers associated with the PM domain have been removed. Otherwise it could be possible to obtain a pointer to a PM domain structure that has been removed.

PM domains now have a reference to the pointer for the PM domain provider's data variable. Add a function so that a PM domain can remove a PM domain provider by referencing the data pointer.

Signed-off-by Jon Hunter <[email protected]>

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

drivers/base/power/domain.c +24 -0

index 72055fef6256..438885f2455f 100644
@@ -1738,6 +1738,30 @@ void of_genpd_del_provider(struct device_node *np)
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
 /**
+ * of_genpd_del_provider_by_data() - Remove a registered PM domain provider
+ * @data: Pointer to the data associated with the PM domain provider
+ *
+ * Look up a PM domain provider based upon a pointer to it's data and
+ * remove the PM domain provider from the list of providers.
+ */
+void of_genpd_del_provider_by_data(void *data)
+{
+       struct of_genpd_provider *c, *cp;
+
+       mutex_lock(&of_genpd_mutex);
+       list_for_each_entry_safe(cp, c, &of_genpd_providers, link) {
+               if (cp->data == data) {
+                       list_del(&cp->link);
+                       of_node_put(cp->node);
+                       kfree(cp);
+                       break;
+               }
+       }
+       mutex_unlock(&of_genpd_mutex);
+}
+EXPORT_SYMBOL_GPL(of_genpd_del_provider_by_data);
+
+/**
  * of_genpd_get_from_provider() - Look-up PM domain
  * @genpdspec: OF phandle args to use for look-up
  *

include/linux/pm_domain.h +0 -0

index bed84413546f..7b7921a65cb0 100644

include/linux/pm_domain.h +2 -1

+void of_genpd_del_provider_by_data(void *data);
 struct generic_pm_domain *__of_genpd_xlate_simple(
                                        struct of_phandle_args *genpdspec,
                                        void *data);

@@ -218,6 +219,7 @@ static inline int __of_genpd_add_provider(struct device_node *np,
        return 0;
 }
 static inline void of_genpd_del_provider(struct device_node *np) {}
+static inline void of_genpd_del_provider_by_data(void *data) {}
 
 #define __of_genpd_xlate_simple                NULL
 #define __of_genpd_xlate_onecell       NULL
--

2.1.4

Quoted 55 lines · Jon Hunter

To remove a PM domain from the system, it is necessary to ensure that any PM domain providers associated with the PM domain have been removed. Otherwise it could be possible to obtain a pointer to a PM domain structure that has been removed.

PM domains now have a reference to the pointer for the PM domain provider's data variable. Add a function so that a PM domain can remove a PM domain provider by referencing the data pointer.

Signed-off-by Jon Hunter <[email protected]>

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

drivers/base/power/domain.c +24 -0

index 72055fef6256..438885f2455f 100644
@@ -1738,6 +1738,30 @@ void of_genpd_del_provider(struct device_node *np)
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
 /**
+ * of_genpd_del_provider_by_data() - Remove a registered PM domain provider
+ * @data: Pointer to the data associated with the PM domain provider
+ *
+ * Look up a PM domain provider based upon a pointer to it's data and
+ * remove the PM domain provider from the list of providers.
+ */
+void of_genpd_del_provider_by_data(void *data)
+{
+       struct of_genpd_provider *c, *cp;
+
+       mutex_lock(&of_genpd_mutex);
+       list_for_each_entry_safe(cp, c, &of_genpd_providers, link) {
+               if (cp->data == data) {
+                       list_del(&cp->link);
+                       of_node_put(cp->node);
+                       kfree(cp);
+                       break;
+               }
+       }
+       mutex_unlock(&of_genpd_mutex);
+}
+EXPORT_SYMBOL_GPL(of_genpd_del_provider_by_data);
+
+/**
  * of_genpd_get_from_provider() - Look-up PM domain
  * @genpdspec: OF phandle args to use for look-up
  *

include/linux/pm_domain.h +0 -0

index bed84413546f..7b7921a65cb0 100644

include/linux/pm_domain.h int of_genpd_add_provider_simple(struct device_node *np, · line 201

Quoted 3 lines

include/linux/pm_domain.h +0 -0

@@ -199,6 +199,7 @@ 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);

There's currently only one user of of_genpd_del_provider().

Could this patch just convert that user to the new API, so we don't need to keep both the legacy and new one?

I guess we could then just stick to the name "of_genpd_del_provider()".

Quoted 62 lines · Ulf Hansson

Quoted 55 lines · Jon Hunter

To remove a PM domain from the system, it is necessary to ensure that any PM domain providers associated with the PM domain have been removed. Otherwise it could be possible to obtain a pointer to a PM domain structure that has been removed.

PM domains now have a reference to the pointer for the PM domain provider's data variable. Add a function so that a PM domain can remove a PM domain provider by referencing the data pointer.

Signed-off-by Jon Hunter <[email protected]>

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

drivers/base/power/domain.c +24 -0

index 72055fef6256..438885f2455f 100644
@@ -1738,6 +1738,30 @@ void of_genpd_del_provider(struct device_node *np)
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
 /**
+ * of_genpd_del_provider_by_data() - Remove a registered PM domain provider
+ * @data: Pointer to the data associated with the PM domain provider
+ *
+ * Look up a PM domain provider based upon a pointer to it's data and
+ * remove the PM domain provider from the list of providers.
+ */
+void of_genpd_del_provider_by_data(void *data)
+{
+       struct of_genpd_provider *c, *cp;
+
+       mutex_lock(&of_genpd_mutex);
+       list_for_each_entry_safe(cp, c, &of_genpd_providers, link) {
+               if (cp->data == data) {
+                       list_del(&cp->link);
+                       of_node_put(cp->node);
+                       kfree(cp);
+                       break;
+               }
+       }
+       mutex_unlock(&of_genpd_mutex);
+}
+EXPORT_SYMBOL_GPL(of_genpd_del_provider_by_data);
+
+/**
  * of_genpd_get_from_provider() - Look-up PM domain
  * @genpdspec: OF phandle args to use for look-up
  *

include/linux/pm_domain.h +0 -0

index bed84413546f..7b7921a65cb0 100644

include/linux/pm_domain.h int of_genpd_add_provider_simple(struct device_node *np, · line 201

Quoted 3 lines

include/linux/pm_domain.h +0 -0

@@ -199,6 +199,7 @@ 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);

There's currently only one user of of_genpd_del_provider().

Quoted 4 lines

Could this patch just convert that user to the new API, so we don't need to keep both the legacy and new one?

I guess we could then just stick to the name "of_genpd_del_provider()".

I had a look at this and to do that we would end up with of_genpd_del_provider(struct device_node *np, void *data) where the user should only pass one of the arguments. It seems a bit odd. However, unless I have forgotten something, I wonder if we should just make of_genpd_del_provider_by_name() a local function and not export this at all? It seems more natural for users to delete a provider by the device_node than by name rather than the data argument.

The only problem I see with making of_genpd_del_provider_by_name() local is that I need to add a prototype for the function at the top of the domain.c source file so that it builds because __pm_genpd_remove() is defined above it. Yes I could move __pm_genpd_remove() to the bottom of the file but then it is not located next to pm_genpd_init() which seems odd.

Let me know what you think.

Quoted 64 lines · Ulf Hansson

Quoted 57 lines · Jon Hunter

To remove a PM domain from the system, it is necessary to ensure that any PM domain providers associated with the PM domain have been removed. Otherwise it could be possible to obtain a pointer to a PM domain structure that has been removed.

PM domains now have a reference to the pointer for the PM domain provider's data variable. Add a function so that a PM domain can remove a PM domain provider by referencing the data pointer.

Signed-off-by Jon Hunter <[email protected]>

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

drivers/base/power/domain.c +24 -0

index 72055fef6256..438885f2455f 100644
@@ -1738,6 +1738,30 @@ void of_genpd_del_provider(struct device_node *np)
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
 /**
+ * of_genpd_del_provider_by_data() - Remove a registered PM domain provider
+ * @data: Pointer to the data associated with the PM domain provider
+ *
+ * Look up a PM domain provider based upon a pointer to it's data and
+ * remove the PM domain provider from the list of providers.
+ */
+void of_genpd_del_provider_by_data(void *data)
+{
+       struct of_genpd_provider *c, *cp;
+
+       mutex_lock(&of_genpd_mutex);
+       list_for_each_entry_safe(cp, c, &of_genpd_providers, link) {
+               if (cp->data == data) {
+                       list_del(&cp->link);
+                       of_node_put(cp->node);
+                       kfree(cp);
+                       break;
+               }
+       }
+       mutex_unlock(&of_genpd_mutex);
+}
+EXPORT_SYMBOL_GPL(of_genpd_del_provider_by_data);
+
+/**
  * of_genpd_get_from_provider() - Look-up PM domain
  * @genpdspec: OF phandle args to use for look-up
  *

include/linux/pm_domain.h +0 -0

index bed84413546f..7b7921a65cb0 100644

include/linux/pm_domain.h int of_genpd_add_provider_simple(struct device_node *np, · line 201

Quoted 3 lines

include/linux/pm_domain.h +0 -0

@@ -199,6 +199,7 @@ 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);

There's currently only one user of of_genpd_del_provider().

Quoted 4 lines

Could this patch just convert that user to the new API, so we don't need to keep both the legacy and new one?

I guess we could then just stick to the name "of_genpd_del_provider()".

I had a look at this and to do that we would end up with of_genpd_del_provider(struct device_node *np, void *data) where the user should only pass one of the arguments. It seems a bit odd. However, unless I have forgotten something, I wonder if we should just make of_genpd_del_provider_by_name() a local function and not export this at all? It seems more natural for users to delete a provider by the device_node than by name rather than the data argument.

The only problem I see with making of_genpd_del_provider_by_name() local is that I need to add a prototype for the function at the top of the domain.c source file so that it builds because __pm_genpd_remove() is defined above it. Yes I could move __pm_genpd_remove() to the bottom of the file but then it is not located next to pm_genpd_init() which seems odd.

Let me know what you think.

Quoted 85 lines · Jon Hunter

Quoted 64 lines · Ulf Hansson

Quoted 57 lines · Jon Hunter

To remove a PM domain from the system, it is necessary to ensure that any PM domain providers associated with the PM domain have been removed. Otherwise it could be possible to obtain a pointer to a PM domain structure that has been removed.

PM domains now have a reference to the pointer for the PM domain provider's data variable. Add a function so that a PM domain can remove a PM domain provider by referencing the data pointer.

Signed-off-by Jon Hunter <[email protected]>

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

drivers/base/power/domain.c +24 -0

index 72055fef6256..438885f2455f 100644
@@ -1738,6 +1738,30 @@ void of_genpd_del_provider(struct device_node *np)
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
 /**
+ * of_genpd_del_provider_by_data() - Remove a registered PM domain provider
+ * @data: Pointer to the data associated with the PM domain provider
+ *
+ * Look up a PM domain provider based upon a pointer to it's data and
+ * remove the PM domain provider from the list of providers.
+ */
+void of_genpd_del_provider_by_data(void *data)
+{
+       struct of_genpd_provider *c, *cp;
+
+       mutex_lock(&of_genpd_mutex);
+       list_for_each_entry_safe(cp, c, &of_genpd_providers, link) {
+               if (cp->data == data) {
+                       list_del(&cp->link);
+                       of_node_put(cp->node);
+                       kfree(cp);
+                       break;
+               }
+       }
+       mutex_unlock(&of_genpd_mutex);
+}
+EXPORT_SYMBOL_GPL(of_genpd_del_provider_by_data);
+
+/**
  * of_genpd_get_from_provider() - Look-up PM domain
  * @genpdspec: OF phandle args to use for look-up
  *

include/linux/pm_domain.h +0 -0

index bed84413546f..7b7921a65cb0 100644

include/linux/pm_domain.h int of_genpd_add_provider_simple(struct device_node *np, · line 201

Quoted 3 lines

include/linux/pm_domain.h +0 -0

@@ -199,6 +199,7 @@ 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);

There's currently only one user of of_genpd_del_provider().

Quoted 4 lines

Could this patch just convert that user to the new API, so we don't need to keep both the legacy and new one?

I guess we could then just stick to the name "of_genpd_del_provider()".

I had a look at this and to do that we would end up with of_genpd_del_provider(struct device_node *np, void *data) where the user should only pass one of the arguments. It seems a bit odd. However, unless I have forgotten something, I wonder if we should just make of_genpd_del_provider_by_name() a local function and not export this at all? It seems more natural for users to delete a provider by the device_node than by name rather than the data argument.

The only problem I see with making of_genpd_del_provider_by_name() local is that I need to add a prototype for the function at the top of the domain.c source file so that it builds because __pm_genpd_remove() is defined above it. Yes I could move __pm_genpd_remove() to the bottom of the file but then it is not located next to pm_genpd_init() which seems odd.

Quoted 1 line

Let me know what you think.

Sorry for delay! I have now looked into this in more detail.

When an genpd provider is added today, it's supposed to get a corresponding *unique* OF device node associated with it, right!?

If we store this OF device node from the provider in the struct generic_pm_domain, instead of the "provider_data pointer", we wouldn't need to the add of_genpd_del_provider_by_data() at all. Because we can use the currently available of_genpd_del_provider(), right!?

Or what am I missing? :-)

Quoted 86 lines · Ulf Hansson

Quoted 85 lines · Jon Hunter

Quoted 64 lines · Ulf Hansson

Quoted 57 lines · Jon Hunter

To remove a PM domain from the system, it is necessary to ensure that any PM domain providers associated with the PM domain have been removed. Otherwise it could be possible to obtain a pointer to a PM domain structure that has been removed.

PM domains now have a reference to the pointer for the PM domain provider's data variable. Add a function so that a PM domain can remove a PM domain provider by referencing the data pointer.

Signed-off-by Jon Hunter <[email protected]>

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

drivers/base/power/domain.c +24 -0

index 72055fef6256..438885f2455f 100644
@@ -1738,6 +1738,30 @@ void of_genpd_del_provider(struct device_node *np)
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
 /**
+ * of_genpd_del_provider_by_data() - Remove a registered PM domain provider
+ * @data: Pointer to the data associated with the PM domain provider
+ *
+ * Look up a PM domain provider based upon a pointer to it's data and
+ * remove the PM domain provider from the list of providers.
+ */
+void of_genpd_del_provider_by_data(void *data)
+{
+       struct of_genpd_provider *c, *cp;
+
+       mutex_lock(&of_genpd_mutex);
+       list_for_each_entry_safe(cp, c, &of_genpd_providers, link) {
+               if (cp->data == data) {
+                       list_del(&cp->link);
+                       of_node_put(cp->node);
+                       kfree(cp);
+                       break;
+               }
+       }
+       mutex_unlock(&of_genpd_mutex);
+}
+EXPORT_SYMBOL_GPL(of_genpd_del_provider_by_data);
+
+/**
  * of_genpd_get_from_provider() - Look-up PM domain
  * @genpdspec: OF phandle args to use for look-up
  *

include/linux/pm_domain.h +0 -0

index bed84413546f..7b7921a65cb0 100644

include/linux/pm_domain.h int of_genpd_add_provider_simple(struct device_node *np, · line 201

Quoted 3 lines

include/linux/pm_domain.h +0 -0

@@ -199,6 +199,7 @@ 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);

There's currently only one user of of_genpd_del_provider().

Quoted 4 lines

Could this patch just convert that user to the new API, so we don't need to keep both the legacy and new one?

I guess we could then just stick to the name "of_genpd_del_provider()".

I had a look at this and to do that we would end up with of_genpd_del_provider(struct device_node *np, void *data) where the user should only pass one of the arguments. It seems a bit odd. However, unless I have forgotten something, I wonder if we should just make of_genpd_del_provider_by_name() a local function and not export this at all? It seems more natural for users to delete a provider by the device_node than by name rather than the data argument.

The only problem I see with making of_genpd_del_provider_by_name() local is that I need to add a prototype for the function at the top of the domain.c source file so that it builds because __pm_genpd_remove() is defined above it. Yes I could move __pm_genpd_remove() to the bottom of the file but then it is not located next to pm_genpd_init() which seems odd.

Quoted 3 lines

Quoted 1 line

Let me know what you think.

Sorry for delay! I have now looked into this in more detail.

No problem. Thanks!

Quoted 9 lines

When an genpd provider is added today, it's supposed to get a corresponding *unique* OF device node associated with it, right!?

If we store this OF device node from the provider in the struct generic_pm_domain, instead of the "provider_data pointer", we wouldn't need to the add of_genpd_del_provider_by_data() at all. Because we can use the currently available of_genpd_del_provider(), right!?

Or what am I missing? :-)

To remove a PM domain from the system, it is necessary to ensure that any PM domain providers associated with the PM domain have been removed. Otherwise it could be possible to obtain a pointer to a PM domain structure that has been removed.

PM domains now have a reference to the pointer for the PM domain provider's data variable. Add a function so that a PM domain can remove a PM domain provider by referencing the data pointer.

Signed-off-by Jon Hunter <[email protected]>

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

drivers/base/power/domain.c +10 -0

index 72055fef6256..438885f2455f 100644
@@ -1738,6 +1738,30 @@ void of_genpd_del_provider(struct device_node *np)
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
 /**
+ * of_genpd_del_provider_by_data() - Remove a registered PM domain provider
+ * @data: Pointer to the data associated with the PM domain provider
+ *
+ * Look up a PM domain provider based upon a pointer to it's data and
+ * remove the PM domain provider from the list of providers.
+ */
+void of_genpd_del_provider_by_data(void *data)
+{
+	struct of_genpd_provider *c, *cp;
+

The genpd framework allows users to add PM domains via the pm_genpd_init() function, however, there is no corresponding function to remove a PM domain. For most devices this may be fine as the PM domains are never removed, however, for devices that wish to populate the PM domains from within a driver, having the ability to remove a PM domain if the probing of the device fails or the driver is unloaded is necessary.

Add the function pm_genpd_remove() to remove a PM domain by referencing it's generic_pm_domain structure.

If a device supports nested or subdomains, then the PM domains should be removed in reverse order to ensure that the subdomains are removed first. Hence, add the function pm_genpd_remove_tail() to remove the last PM domain added by a given provider and return the generic_pm_domain structure for the PM domain that was removed.

PM domains can only be removed if they are not a parent domain to another PM domain and have no devices associated with them.

When removing PM domains, the PM domain will also be removed from the list of providers, if it was registered.

Signed-off-by Jon Hunter <[email protected]>

---
 drivers/base/power/domain.c | 96 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   | 12 ++++++
 2 files changed, 108 insertions(+)

drivers/base/power/domain.c +5 -0

index 9b33377bf01b..17090e1c91d6 100644
@@ -1556,6 +1556,102 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 }
 EXPORT_SYMBOL_GPL(pm_genpd_init);
 
+/**
+ * __pm_genpd_remove - Remove a generic I/O PM domain
+ * @genpd: Pointer to PM domain that is to be removed.
+ *
+ * To remove the PM domain, this function:

drivers/base/power/domain.c +46 -0

+{
+       struct gpd_link *l, *link;
+       int ret = 0;
+
+       if (IS_ERR_OR_NULL(genpd))
+               return -EINVAL;
+
+       if (genpd->provider_data)
+               of_genpd_del_provider_by_data(genpd->provider_data);
+
+       mutex_lock(&genpd->lock);
+
+       if (!list_empty(&genpd->master_links) || genpd->device_count) {
+               mutex_unlock(&genpd->lock);
+               pr_err("%s: unable to remove %s\n", __func__, genpd->name);
+               return -EBUSY;
+       }
+
+       list_for_each_entry_safe(link, l, &genpd->slave_links, slave_node) {
+               list_del(&link->master_node);
+               list_del(&link->slave_node);
+               kfree(link);
+       }
+
+       list_del(&genpd->gpd_list_node);
+       mutex_unlock(&genpd->lock);
+       cancel_work_sync(&genpd->power_off_work);
+       pr_debug("%s: removed %s\n", __func__, genpd->name);
+
+       return ret;
+}
+
+/**
+ * pm_genpd_remove - Remove a generic I/O PM domain
+ * @genpd: Pointer to PM domain that is to be removed.
+ */
+int pm_genpd_remove(struct generic_pm_domain *genpd)
+{
+       int ret;
+
+       mutex_lock(&gpd_list_lock);
+       ret = __pm_genpd_remove(genpd);
+       mutex_unlock(&gpd_list_lock);
+
+       return ret;
+}

drivers/base/power/domain.c +25 -0

+ *
+ * Returns a valid pointer to struct generic_pm_domain on success or
+ * ERR_PTR() on failure.
+ */
+struct generic_pm_domain *pm_genpd_remove_tail(struct device *provider)
+{
+       struct generic_pm_domain *g, *gpd, *genpd = ERR_PTR(-ENOENT);
+       int ret;
+
+       if (IS_ERR_OR_NULL(provider))
+               return ERR_PTR(-EINVAL);
+
+       mutex_lock(&gpd_list_lock);
+       list_for_each_entry_safe(gpd, g, &gpd_list, gpd_list_node) {
+               if (gpd->provider == provider) {
+                       ret = __pm_genpd_remove(gpd);
+                       genpd = ret ? ERR_PTR(ret) : gpd;
+                       break;
+               }
+       }
+       mutex_unlock(&gpd_list_lock);
+
+       return genpd;
+}
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.

include/linux/pm_domain.h +12 -1

index 7b7921a65cb0..78b23718392f 100644
@@ -53,6 +53,7 @@ struct generic_pm_domain {
        struct mutex lock;
        struct dev_power_governor *gov;
        struct work_struct power_off_work;
+       struct device *provider;        /* Identity of the domain provider */
        void *provider_data;
        const char *name;
        atomic_t sd_count;      /* Number of subdomains with power "on" */

@@ -132,6 +133,8 @@ extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
                                     struct generic_pm_domain *target);
 extern void 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 struct generic_pm_domain *pm_genpd_remove_tail(struct device *provider);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;

@@ -166,6 +169,15 @@ static inline void pm_genpd_init(struct generic_pm_domain *genpd,
                                 struct dev_power_governor *gov, bool is_off)
 {
 }
+static inline int pm_genpd_remove(struct generic_pm_domain *genpd)
+{
+       return -ENOTSUPP;
+}
+static inline
+struct generic_pm_domain *pm_genpd_remove_tail(struct device *provider)
+{
+       return ERR_PTR(-ENOTSUPP);
+}
 #endif
 
 static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
--

2.1.4

Quoted 10 lines · Jon Hunter

The genpd framework allows users to add PM domains via the pm_genpd_init() function, however, there is no corresponding function to remove a PM domain. For most devices this may be fine as the PM domains are never removed, however, for devices that wish to populate the PM domains from within a driver, having the ability to remove a PM domain if the probing of the device fails or the driver is unloaded is necessary.

Add the function pm_genpd_remove() to remove a PM domain by referencing it's generic_pm_domain structure.

Quoted 5 lines

If a device supports nested or subdomains, then the PM domains should be removed in reverse order to ensure that the subdomains are removed first. Hence, add the function pm_genpd_remove_tail() to remove the last PM domain added by a given provider and return the generic_pm_domain structure for the PM domain that was removed.

Perhaps split this up, so the pm_genpd_remove_tail() gets added in a separate patch on top.

Quoted 30 lines

PM domains can only be removed if they are not a parent domain to another PM domain and have no devices associated with them.

When removing PM domains, the PM domain will also be removed from the list of providers, if it was registered.

Signed-off-by Jon Hunter <[email protected]>

---
 drivers/base/power/domain.c | 96 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h   | 12 ++++++
 2 files changed, 108 insertions(+)

drivers/base/power/domain.c +9 -0

index 9b33377bf01b..17090e1c91d6 100644
@@ -1556,6 +1556,102 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 }
 EXPORT_SYMBOL_GPL(pm_genpd_init);
 
+/**
+ * __pm_genpd_remove - Remove a generic I/O PM domain
+ * @genpd: Pointer to PM domain that is to be removed.
+ *
+ * To remove the PM domain, this function:
+ *  - Removes the PM domain from the list of providers, if registered.
+ *  - Removes the PM domain as a subdomain to any parent domains,
+ *    if it was added.
+ *  - Removes the PM domain from the list of registered PM domains.

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 1573

Quoted 6 lines

drivers/base/power/domain.c +6 -0

+ *
+ * The PM domain will only be removed, if it is not a parent to any
+ * other PM domain and has no devices associated with it. Must be called
+ * with the gpd_list_lock held.
+ */
+static int __pm_genpd_remove(struct generic_pm_domain *genpd)

Please rename to genpd_remove() as it's a static function.

Quoted 51 lines

drivers/base/power/domain.c +51 -0

+{
+       struct gpd_link *l, *link;
+       int ret = 0;
+
+       if (IS_ERR_OR_NULL(genpd))
+               return -EINVAL;
+
+       if (genpd->provider_data)
+               of_genpd_del_provider_by_data(genpd->provider_data);
+
+       mutex_lock(&genpd->lock);
+
+       if (!list_empty(&genpd->master_links) || genpd->device_count) {
+               mutex_unlock(&genpd->lock);
+               pr_err("%s: unable to remove %s\n", __func__, genpd->name);
+               return -EBUSY;
+       }
+
+       list_for_each_entry_safe(link, l, &genpd->slave_links, slave_node) {
+               list_del(&link->master_node);
+               list_del(&link->slave_node);
+               kfree(link);
+       }
+
+       list_del(&genpd->gpd_list_node);
+       mutex_unlock(&genpd->lock);
+       cancel_work_sync(&genpd->power_off_work);
+       pr_debug("%s: removed %s\n", __func__, genpd->name);
+
+       return ret;
+}
+
+/**
+ * pm_genpd_remove - Remove a generic I/O PM domain
+ * @genpd: Pointer to PM domain that is to be removed.
+ */
+int pm_genpd_remove(struct generic_pm_domain *genpd)
+{
+       int ret;
+
+       mutex_lock(&gpd_list_lock);
+       ret = __pm_genpd_remove(genpd);
+       mutex_unlock(&gpd_list_lock);
+
+       return ret;
+}
+
+/**
+ * pm_genpd_remove_tail - Remove the last PM domain registered for a provider
+ * @provider: Pointer to device structure associated with provider
+ *

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 1629

Quoted 5 lines

drivers/base/power/domain.c +5 -0

+ * Find the last PM domain that was added by the provider whose 'provider'
+ * device structure matches the device structure given. The 'provider'
+ * device structure for a given PM domain should be initialised by the
+ * device that is creating the PM domains and hence, calling
+ * pm_genpd_init().

Maybe make this a bigger "Important note" as what is needed to actually benefit from using this function.

In order to safely remove PM domains there are a few changes that need to be made to ensure that no one is holding an external reference to a PM domain after it has been removed. One solution, implemented here, solves this by eliminating external references to PM domain. Another alternative would be to employ some reference counting for the PM domains, however, I did not see a good reason for allowing external references in the first place (as always there could be something that I have over-looked!).

Jon Hunter (8):
PM / Domains: Add new helper functions for device-tree
ARM: EXYNOS: Remove calls to of_genpd_get_from_provider()
staging: board: Remove calls to of_genpd_get_from_provider() PM / Domains: Don't expose generic_pm_domain structure
PM / Domains: Verify the PM domain is present when adding a provider PM / Domains: Remove a provider by referencing the data pointer PM / Domains: Prepare for adding support to remove PM domains PM / Domains: Add support for removing PM domains

 arch/arm/mach-exynos/pm_domains.c |  23 +--
 drivers/base/power/domain.c       | 324 +++++++++++++++++++++++++++++++++++---
 drivers/staging/board/board.c     |   9 +-
 include/linux/pm_domain.h         |  55 ++++---
 4 files changed, 348 insertions(+), 63 deletions(-)

--
2.1.4

Quoted 24 lines · Jon Hunter

In order to safely remove PM domains there are a few changes that need to be made to ensure that no one is holding an external reference to a PM domain after it has been removed. One solution, implemented here, solves this by eliminating external references to PM domain. Another alternative would be to employ some reference counting for the PM domains, however, I did not see a good reason for allowing external references in the first place (as always there could be something that I have over-looked!).

Jon Hunter (8):
PM / Domains: Add new helper functions for device-tree
ARM: EXYNOS: Remove calls to of_genpd_get_from_provider()
staging: board: Remove calls to of_genpd_get_from_provider() PM / Domains: Don't expose generic_pm_domain structure
PM / Domains: Verify the PM domain is present when adding a provider PM / Domains: Remove a provider by referencing the data pointer PM / Domains: Prepare for adding support to remove PM domains PM / Domains: Add support for removing PM domains

 arch/arm/mach-exynos/pm_domains.c |  23 +--
 drivers/base/power/domain.c       | 324 +++++++++++++++++++++++++++++++++++---
 drivers/staging/board/board.c     |   9 +-
 include/linux/pm_domain.h         |  55 ++++---
 4 files changed, 348 insertions(+), 63 deletions(-)

Quoted 3 lines

--
2.1.4

Jon, thanks for posting this!
My initial impressions is the I like the ideas for where you moves this.

I will review it in more details as soon as I can (travelling next week).

Hi Uffe,

Quoted 29 lines · Ulf Hansson

Quoted 27 lines · Jon Hunter

In order to safely remove PM domains there are a few changes that need to be made to ensure that no one is holding an external reference to a PM domain after it has been removed. One solution, implemented here, solves this by eliminating external references to PM domain. Another alternative would be to employ some reference counting for the PM domains, however, I did not see a good reason for allowing external references in the first place (as always there could be something that I have over-looked!).

Jon Hunter (8):
PM / Domains: Add new helper functions for device-tree
ARM: EXYNOS: Remove calls to of_genpd_get_from_provider()
staging: board: Remove calls to of_genpd_get_from_provider() PM / Domains: Don't expose generic_pm_domain structure
PM / Domains: Verify the PM domain is present when adding a provider PM / Domains: Remove a provider by referencing the data pointer PM / Domains: Prepare for adding support to remove PM domains PM / Domains: Add support for removing PM domains

 arch/arm/mach-exynos/pm_domains.c |  23 +--
 drivers/base/power/domain.c       | 324 +++++++++++++++++++++++++++++++++++---
 drivers/staging/board/board.c     |   9 +-
 include/linux/pm_domain.h         |  55 ++++---
 4 files changed, 348 insertions(+), 63 deletions(-)

--
2.1.4

Quoted 4 lines

Jon, thanks for posting this!
My initial impressions is the I like the ideas for where you moves this.

I will review it in more details as soon as I can (travelling next week).

Just wanted to see if you have had chance to look at this yet? Not urgent.

In order to safely remove PM domains there are a few changes that need to be made to ensure that no one is holding an external reference to a PM domain after it has been removed. One solution, implemented here, solves this by eliminating external references to PM domain. Another alternative would be to employ some reference counting for the PM domains, however, I did not see a good reason for allowing external references in the first place (as always there could be something that I have over-looked!).

Jon Hunter (8):
PM / Domains: Add new helper functions for device-tree
ARM: EXYNOS: Remove calls to of_genpd_get_from_provider()
staging: board: Remove calls to of_genpd_get_from_provider() PM / Domains: Don't expose generic_pm_domain structure
PM / Domains: Verify the PM domain is present when adding a provider PM / Domains: Remove a provider by referencing the data pointer PM / Domains: Prepare for adding support to remove PM domains PM / Domains: Add support for removing PM domains

 arch/arm/mach-exynos/pm_domains.c |  23 +--
 drivers/base/power/domain.c       | 324 +++++++++++++++++++++++++++++++++++---
 drivers/staging/board/board.c     |   9 +-
 include/linux/pm_domain.h         |  55 ++++---
 4 files changed, 348 insertions(+), 63 deletions(-)

--
2.1.4