| Message ID | E1YZ0b3-0000so-KG@rmk-PC.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 20 March 2015 at 18:20, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > The PM domain code contains two methods to get the generic PM domain > for a struct device. One is dev_to_genpd() which is only safe when > we know for certain that the device has a generic PM domain attached. > The other is coded into genpd_dev_pm_detach() which ensures that the > PM domain in the struct device is a generic PM domain (and so is safer). > > This commit factors out the safer version, documents it, and hides the > unsafe dev_to_genpd(). > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > drivers/base/power/domain.c | 46 +++++++++++++++++++++++++++++++-------------- > include/linux/pm_domain.h | 6 +++--- > 2 files changed, 35 insertions(+), 17 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index b3fbc21da2dc..89d2eea7f561 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -68,7 +68,36 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name) > return genpd; > } > > -struct generic_pm_domain *dev_to_genpd(struct device *dev) > +/* > + * Get the generic PM domain for a particular struct device. > + * This validates the struct device pointer, the PM domain pointer, > + * and checks that the PM domain pointer is a real generic PM domain. > + * Any failure results in NULL being returned. > + */ > +struct generic_pm_domain *pm_genpd_lookup_dev(struct device *dev) I wouldn't mind keeping also this function static, unless you foresee users outside genpd? If converting to static please rename it to genpd_lookup_dev(). > +{ > + struct generic_pm_domain *genpd = NULL, *gpd; > + > + if (IS_ERR_OR_NULL(dev) || IS_ERR_OR_NULL(dev->pm_domain)) > + return NULL; > + > + mutex_lock(&gpd_list_lock); > + list_for_each_entry(gpd, &gpd_list, gpd_list_node) { > + if (&gpd->domain == dev->pm_domain) { > + genpd = gpd; > + break; > + } > + } > + mutex_unlock(&gpd_list_lock); > + > + return genpd; > +} > + > +/* > + * This should only be used where we are certain that the pm_domain > + * attached to the device is a genpd domain. > + */ > +static struct generic_pm_domain *dev_to_genpd(struct device *dev) > { > if (IS_ERR_OR_NULL(dev->pm_domain)) > return ERR_PTR(-EINVAL); > @@ -2120,21 +2149,10 @@ EXPORT_SYMBOL_GPL(of_genpd_get_from_provider); > */ > static void genpd_dev_pm_detach(struct device *dev, bool power_off) > { > - struct generic_pm_domain *pd = NULL, *gpd; > + struct generic_pm_domain *pd; > int ret = 0; > > - if (!dev->pm_domain) > - return; > - > - mutex_lock(&gpd_list_lock); > - list_for_each_entry(gpd, &gpd_list, gpd_list_node) { > - if (&gpd->domain == dev->pm_domain) { > - pd = gpd; > - break; > - } > - } > - mutex_unlock(&gpd_list_lock); > - > + pd = pm_genpd_lookup_dev(dev); > if (!pd) > return; > > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index a9edab2c787a..441f17bdcb0a 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -129,7 +129,7 @@ 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 *dev_to_genpd(struct device *dev); > +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); > @@ -166,9 +166,9 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev) > { > return ERR_PTR(-ENOSYS); > } > -static inline struct generic_pm_domain *dev_to_genpd(struct device *dev) > +static inline struct generic_pm_domain *pm_genpd_lookup_dev(struct device *dev) > { > - return ERR_PTR(-ENOSYS); > + return NULL; > } > static inline int __pm_genpd_add_device(struct generic_pm_domain *genpd, > struct device *dev, > -- > 1.8.3.1 > Kind regards Uffe
On Mon, Mar 23, 2015 at 02:28:14PM +0100, Ulf Hansson wrote: > On 20 March 2015 at 18:20, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > The PM domain code contains two methods to get the generic PM domain > > for a struct device. One is dev_to_genpd() which is only safe when > > we know for certain that the device has a generic PM domain attached. > > The other is coded into genpd_dev_pm_detach() which ensures that the > > PM domain in the struct device is a generic PM domain (and so is safer). > > > > This commit factors out the safer version, documents it, and hides the > > unsafe dev_to_genpd(). > > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > --- > > drivers/base/power/domain.c | 46 +++++++++++++++++++++++++++++++-------------- > > include/linux/pm_domain.h | 6 +++--- > > 2 files changed, 35 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index b3fbc21da2dc..89d2eea7f561 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -68,7 +68,36 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name) > > return genpd; > > } > > > > -struct generic_pm_domain *dev_to_genpd(struct device *dev) > > +/* > > + * Get the generic PM domain for a particular struct device. > > + * This validates the struct device pointer, the PM domain pointer, > > + * and checks that the PM domain pointer is a real generic PM domain. > > + * Any failure results in NULL being returned. > > + */ > > +struct generic_pm_domain *pm_genpd_lookup_dev(struct device *dev) > > I wouldn't mind keeping also this function static, unless you foresee > users outside genpd? _I_ have users of it outside at the moment. Once the major churn in PM domains is over and I have no use for this externally, I'll consider making it static in my patch set - but all the time that doing so results in breakage for me... Plus, you're asking me to do yet another 20+ minute re-spin of this patch set. I'm going to refuse; I'm almost at the point of just not giving a damn on this stuff, it's wasting too much of my time. Especially as, yet again, you should have replied to the emails in the previous round suggesting this change. Rafael, please merge these three as we previously agreed. Thanks.
On Monday, March 23, 2015 03:17:40 PM Russell King - ARM Linux wrote: > On Mon, Mar 23, 2015 at 02:28:14PM +0100, Ulf Hansson wrote: > > On 20 March 2015 at 18:20, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > > The PM domain code contains two methods to get the generic PM domain > > > for a struct device. One is dev_to_genpd() which is only safe when > > > we know for certain that the device has a generic PM domain attached. > > > The other is coded into genpd_dev_pm_detach() which ensures that the > > > PM domain in the struct device is a generic PM domain (and so is safer). > > > > > > This commit factors out the safer version, documents it, and hides the > > > unsafe dev_to_genpd(). > > > > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > > --- > > > drivers/base/power/domain.c | 46 +++++++++++++++++++++++++++++++-------------- > > > include/linux/pm_domain.h | 6 +++--- > > > 2 files changed, 35 insertions(+), 17 deletions(-) > > > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > > index b3fbc21da2dc..89d2eea7f561 100644 > > > --- a/drivers/base/power/domain.c > > > +++ b/drivers/base/power/domain.c > > > @@ -68,7 +68,36 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name) > > > return genpd; > > > } > > > > > > -struct generic_pm_domain *dev_to_genpd(struct device *dev) > > > +/* > > > + * Get the generic PM domain for a particular struct device. > > > + * This validates the struct device pointer, the PM domain pointer, > > > + * and checks that the PM domain pointer is a real generic PM domain. > > > + * Any failure results in NULL being returned. > > > + */ > > > +struct generic_pm_domain *pm_genpd_lookup_dev(struct device *dev) > > > > I wouldn't mind keeping also this function static, unless you foresee > > users outside genpd? > > _I_ have users of it outside at the moment. > > Once the major churn in PM domains is over and I have no use for this > externally, I'll consider making it static in my patch set - but all > the time that doing so results in breakage for me... > > Plus, you're asking me to do yet another 20+ minute re-spin of this > patch set. I'm going to refuse; I'm almost at the point of just not > giving a damn on this stuff, it's wasting too much of my time. > Especially as, yet again, you should have replied to the emails in the > previous round suggesting this change. > > Rafael, please merge these three as we previously agreed. Thanks. I said I would do that, didn't I? I've applied this series plus https://patchwork.kernel.org/patch/6056201/ and https://patchwork.kernel.org/patch/6057441/ on top of that. Please let me know if you need anything more from the core.
On Tue, Mar 24, 2015 at 01:29:05AM +0100, Rafael J. Wysocki wrote: > On Monday, March 23, 2015 03:17:40 PM Russell King - ARM Linux wrote: > > Rafael, please merge these three as we previously agreed. Thanks. > > I said I would do that, didn't I? I wasn't sure if you were going to hold back on it as a result of Uwe's comments. > I've applied this series plus https://patchwork.kernel.org/patch/6056201/ > and https://patchwork.kernel.org/patch/6057441/ on top of that. > Please let me know if you need anything more from the core. I'll replicate what you've applied and test it here (build in progress...) Thanks!
On Thu, Mar 26, 2015 at 03:20:43PM +0000, Russell King - ARM Linux wrote:
> I'll replicate what you've applied and test it here (build in progress...)
It looks like it works as desired.
Thanks.
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index b3fbc21da2dc..89d2eea7f561 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -68,7 +68,36 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name) return genpd; } -struct generic_pm_domain *dev_to_genpd(struct device *dev) +/* + * Get the generic PM domain for a particular struct device. + * This validates the struct device pointer, the PM domain pointer, + * and checks that the PM domain pointer is a real generic PM domain. + * Any failure results in NULL being returned. + */ +struct generic_pm_domain *pm_genpd_lookup_dev(struct device *dev) +{ + struct generic_pm_domain *genpd = NULL, *gpd; + + if (IS_ERR_OR_NULL(dev) || IS_ERR_OR_NULL(dev->pm_domain)) + return NULL; + + mutex_lock(&gpd_list_lock); + list_for_each_entry(gpd, &gpd_list, gpd_list_node) { + if (&gpd->domain == dev->pm_domain) { + genpd = gpd; + break; + } + } + mutex_unlock(&gpd_list_lock); + + return genpd; +} + +/* + * This should only be used where we are certain that the pm_domain + * attached to the device is a genpd domain. + */ +static struct generic_pm_domain *dev_to_genpd(struct device *dev) { if (IS_ERR_OR_NULL(dev->pm_domain)) return ERR_PTR(-EINVAL); @@ -2120,21 +2149,10 @@ EXPORT_SYMBOL_GPL(of_genpd_get_from_provider); */ static void genpd_dev_pm_detach(struct device *dev, bool power_off) { - struct generic_pm_domain *pd = NULL, *gpd; + struct generic_pm_domain *pd; int ret = 0; - if (!dev->pm_domain) - return; - - mutex_lock(&gpd_list_lock); - list_for_each_entry(gpd, &gpd_list, gpd_list_node) { - if (&gpd->domain == dev->pm_domain) { - pd = gpd; - break; - } - } - mutex_unlock(&gpd_list_lock); - + pd = pm_genpd_lookup_dev(dev); if (!pd) return; diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index a9edab2c787a..441f17bdcb0a 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -129,7 +129,7 @@ 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 *dev_to_genpd(struct device *dev); +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); @@ -166,9 +166,9 @@ static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev) { return ERR_PTR(-ENOSYS); } -static inline struct generic_pm_domain *dev_to_genpd(struct device *dev) +static inline struct generic_pm_domain *pm_genpd_lookup_dev(struct device *dev) { - return ERR_PTR(-ENOSYS); + return NULL; } static inline int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
The PM domain code contains two methods to get the generic PM domain for a struct device. One is dev_to_genpd() which is only safe when we know for certain that the device has a generic PM domain attached. The other is coded into genpd_dev_pm_detach() which ensures that the PM domain in the struct device is a generic PM domain (and so is safer). This commit factors out the safer version, documents it, and hides the unsafe dev_to_genpd(). Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/base/power/domain.c | 46 +++++++++++++++++++++++++++++++-------------- include/linux/pm_domain.h | 6 +++--- 2 files changed, 35 insertions(+), 17 deletions(-)