| Message ID | 1457090634-14785-2-git-send-email-jonathanh@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/03/16 11:23, Jon Hunter wrote: > 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 <jonathanh@nvidia.com> > --- > drivers/base/power/domain.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/pm_domain.h | 16 ++++++++++++++++ > 2 files changed, 60 insertions(+) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 5bdb42bf40f4..b24893499454 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -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 > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index 49cd8890b873..2b6ee670b231 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -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 */ > @@ -229,6 +233,18 @@ static inline struct generic_pm_domain *of_genpd_get_from_provider( > #define __of_genpd_xlate_simple NULL > #define __of_genpd_xlate_onecell NULL > > +static inline int of_pm_genpd_add_device(struct of_phandle_args *args, > + struct device *dev) This should be of_genpd_add_device. Will fix. Jon
Hi Ulf, On 04/03/16 11:23, Jon Hunter wrote: > 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. 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. Cheers Jon
On 22 June 2016 at 16:58, Jon Hunter <jonathanh@nvidia.com> wrote: > Hi Ulf, > > On 04/03/16 11:23, Jon Hunter wrote: >> 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. > > 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. Kind regards Uffe
On 22/06/16 16:08, Ulf Hansson wrote: > On 22 June 2016 at 16:58, Jon Hunter <jonathanh@nvidia.com> wrote: >> Hi Ulf, >> >> On 04/03/16 11:23, Jon Hunter wrote: >>> 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. >> >> 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. 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. Cheers Jon
On 22 June 2016 at 17:22, Jon Hunter <jonathanh@nvidia.com> wrote: > > On 22/06/16 16:08, Ulf Hansson wrote: >> On 22 June 2016 at 16:58, Jon Hunter <jonathanh@nvidia.com> wrote: >>> Hi Ulf, >>> >>> On 04/03/16 11:23, Jon Hunter wrote: >>>> 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. >>> >>> 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. > > 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. You may very well be right that these will be needed. How about posting those patches as separate change and on top of the series. Then we can decide if we want to pick them now or later. > > Let me know what you think about my response to patch 6/8. I am on it. Kind regards Uffe
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 5bdb42bf40f4..b24893499454 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -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 diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 49cd8890b873..2b6ee670b231 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -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 */ @@ -229,6 +233,18 @@ static inline struct generic_pm_domain *of_genpd_get_from_provider( #define __of_genpd_xlate_simple NULL #define __of_genpd_xlate_onecell NULL +static inline int of_pm_genpd_add_device(struct of_phandle_args *args, + struct device *dev) +{ + return -ENODEV; +} + +static inline int of_genpd_add_subdomain(struct of_phandle_args *parent, + struct of_phandle_args *new_subdomain) +{ + return -ENODEV; +} + static inline int genpd_dev_pm_attach(struct device *dev) { return -ENODEV;
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 <jonathanh@nvidia.com> --- drivers/base/power/domain.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/pm_domain.h | 16 ++++++++++++++++ 2 files changed, 60 insertions(+)