| Message ID | 20210912184458.17995-3-digetx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Make probe() of Tegra devfreq driver completely resource-managed | expand |
On 21. 9. 13. 오전 3:44, Dmitry Osipenko wrote: > Add resource-managed variant of devfreq_add_governor(). > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/devfreq/devfreq.c | 26 ++++++++++++++++++++++++++ > drivers/devfreq/governor.h | 3 +++ > 2 files changed, 29 insertions(+) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 85faa7a5c7d1..d3af000ec290 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -1301,6 +1301,32 @@ int devfreq_add_governor(struct devfreq_governor *governor) > } > EXPORT_SYMBOL(devfreq_add_governor); > > +static void devm_devfreq_remove_governor(void *governor) > +{ > + devfreq_remove_governor(governor); Because devfreq_remove_governor has the return value, you need to check the return value and then print error at least. WARN_ON(devfreq_remove_governor(governor)); > +} > + > +/** > + * devm_devfreq_add_governor() - Add devfreq governor > + * @dev: device which adds devfreq governor > + * @governor: the devfreq governor to be added > + * > + * This is a resource-managed variant of devfreq_add_governor(). > + */ > +int devm_devfreq_add_governor(struct device *dev, > + struct devfreq_governor *governor) > +{ > + int err; > + > + err = devfreq_add_governor(governor); > + if (err) > + return err; > + > + return devm_add_action_or_reset(dev, devm_devfreq_remove_governor, > + governor); > +} > +EXPORT_SYMBOL(devm_devfreq_add_governor); > + > /** > * devfreq_remove_governor() - Remove devfreq feature from a device. > * @governor: the devfreq governor to be removed > diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h > index 2d69a0ce6291..0d70a9ad951e 100644 > --- a/drivers/devfreq/governor.h > +++ b/drivers/devfreq/governor.h > @@ -94,4 +94,7 @@ static inline int devfreq_update_stats(struct devfreq *df) > > return df->profile->get_dev_status(df->dev.parent, &df->last_status); > } > + > +int devm_devfreq_add_governor(struct device *dev, > + struct devfreq_governor *governor); Better to add under devfreq_remove_governor definition in order to gather the similar functions. > #endif /* _GOVERNOR_H */ > -- Best Regards, Samsung Electronics Chanwoo Choi
Hi, There is ordering dependency between devfreq device and devfreq governor. Theoretically, devfreq governor must be released after released of devfreq device. devm_* support the release ordering by the sequence of registration in probe()? On 21. 9. 13. 오전 3:44, Dmitry Osipenko wrote: > Add resource-managed variant of devfreq_add_governor(). > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/devfreq/devfreq.c | 26 ++++++++++++++++++++++++++ > drivers/devfreq/governor.h | 3 +++ > 2 files changed, 29 insertions(+) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 85faa7a5c7d1..d3af000ec290 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -1301,6 +1301,32 @@ int devfreq_add_governor(struct devfreq_governor *governor) > } > EXPORT_SYMBOL(devfreq_add_governor); > > +static void devm_devfreq_remove_governor(void *governor) > +{ > + devfreq_remove_governor(governor); > +} > + > +/** > + * devm_devfreq_add_governor() - Add devfreq governor > + * @dev: device which adds devfreq governor > + * @governor: the devfreq governor to be added > + * > + * This is a resource-managed variant of devfreq_add_governor(). > + */ > +int devm_devfreq_add_governor(struct device *dev, > + struct devfreq_governor *governor) > +{ > + int err; > + > + err = devfreq_add_governor(governor); > + if (err) > + return err; > + > + return devm_add_action_or_reset(dev, devm_devfreq_remove_governor, > + governor); > +} > +EXPORT_SYMBOL(devm_devfreq_add_governor); > + > /** > * devfreq_remove_governor() - Remove devfreq feature from a device. > * @governor: the devfreq governor to be removed > diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h > index 2d69a0ce6291..0d70a9ad951e 100644 > --- a/drivers/devfreq/governor.h > +++ b/drivers/devfreq/governor.h > @@ -94,4 +94,7 @@ static inline int devfreq_update_stats(struct devfreq *df) > > return df->profile->get_dev_status(df->dev.parent, &df->last_status); > } > + > +int devm_devfreq_add_governor(struct device *dev, > + struct devfreq_governor *governor); > #endif /* _GOVERNOR_H */ > -- Best Regards, Samsung Electronics Chanwoo Choi
15.09.2021 21:37, Chanwoo Choi пишет: > Hi, > > There is ordering dependency between devfreq device and devfreq > governor. Theoretically, devfreq governor must be released after > released of devfreq device. > > devm_* support the release ordering by the sequence of registration > in probe()? Yes, the release order is always opposite to the registration order.
15.09.2021 21:23, Chanwoo Choi пишет: > On 21. 9. 13. 오전 3:44, Dmitry Osipenko wrote: >> Add resource-managed variant of devfreq_add_governor(). >> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/devfreq/devfreq.c | 26 ++++++++++++++++++++++++++ >> drivers/devfreq/governor.h | 3 +++ >> 2 files changed, 29 insertions(+) >> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >> index 85faa7a5c7d1..d3af000ec290 100644 >> --- a/drivers/devfreq/devfreq.c >> +++ b/drivers/devfreq/devfreq.c >> @@ -1301,6 +1301,32 @@ int devfreq_add_governor(struct >> devfreq_governor *governor) >> } >> EXPORT_SYMBOL(devfreq_add_governor); >> +static void devm_devfreq_remove_governor(void *governor) >> +{ >> + devfreq_remove_governor(governor); > > Because devfreq_remove_governor has the return value, > you need to check the return value and then print error at least. > > WARN_ON(devfreq_remove_governor(governor)); ... >> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h >> index 2d69a0ce6291..0d70a9ad951e 100644 >> --- a/drivers/devfreq/governor.h >> +++ b/drivers/devfreq/governor.h >> @@ -94,4 +94,7 @@ static inline int devfreq_update_stats(struct >> devfreq *df) >> return df->profile->get_dev_status(df->dev.parent, >> &df->last_status); >> } >> + >> +int devm_devfreq_add_governor(struct device *dev, >> + struct devfreq_governor *governor); > > Better to add under devfreq_remove_governor definition in order to > gather the similar functions. I'll change it in v2, thanks.
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 85faa7a5c7d1..d3af000ec290 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -1301,6 +1301,32 @@ int devfreq_add_governor(struct devfreq_governor *governor) } EXPORT_SYMBOL(devfreq_add_governor); +static void devm_devfreq_remove_governor(void *governor) +{ + devfreq_remove_governor(governor); +} + +/** + * devm_devfreq_add_governor() - Add devfreq governor + * @dev: device which adds devfreq governor + * @governor: the devfreq governor to be added + * + * This is a resource-managed variant of devfreq_add_governor(). + */ +int devm_devfreq_add_governor(struct device *dev, + struct devfreq_governor *governor) +{ + int err; + + err = devfreq_add_governor(governor); + if (err) + return err; + + return devm_add_action_or_reset(dev, devm_devfreq_remove_governor, + governor); +} +EXPORT_SYMBOL(devm_devfreq_add_governor); + /** * devfreq_remove_governor() - Remove devfreq feature from a device. * @governor: the devfreq governor to be removed diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h index 2d69a0ce6291..0d70a9ad951e 100644 --- a/drivers/devfreq/governor.h +++ b/drivers/devfreq/governor.h @@ -94,4 +94,7 @@ static inline int devfreq_update_stats(struct devfreq *df) return df->profile->get_dev_status(df->dev.parent, &df->last_status); } + +int devm_devfreq_add_governor(struct device *dev, + struct devfreq_governor *governor); #endif /* _GOVERNOR_H */
Add resource-managed variant of devfreq_add_governor(). Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- drivers/devfreq/devfreq.c | 26 ++++++++++++++++++++++++++ drivers/devfreq/governor.h | 3 +++ 2 files changed, 29 insertions(+)