Message ID | 20210311192105.14998-1-digetx@gmail.com |
---|---|
Headers | show |
Series | Introduce devm_pm_opp_* API | expand |
On Thu, Mar 11, 2021 at 10:20:58PM +0300, Dmitry Osipenko wrote:
> Acked-by: Mark brown <broonie@kernel.org>
Typo there.
On 11-03-21, 22:20, Dmitry Osipenko wrote: > +struct opp_table *devm_pm_opp_set_clkname(struct device *dev, const char *name) > +{ > + struct opp_table *opp_table; > + int err; > + > + opp_table = dev_pm_opp_set_clkname(dev, name); > + if (IS_ERR(opp_table)) > + return opp_table; > + > + err = devm_add_action_or_reset(dev, devm_pm_opp_clkname_release, opp_table); > + if (err) > + opp_table = ERR_PTR(err); > + > + return opp_table; > +} I wonder if we still need to return opp_table from here, or a simple integer is fine.. The callers shouldn't be required to use the OPP table directly anymore I believe and so better simplify the return part of this and all other routines you are adding here.. If there is a user which needs the opp_table, let it use the regular non-devm variant.
On Fri, 12 Mar 2021 at 06:33, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 11-03-21, 22:20, Dmitry Osipenko wrote: > > +struct opp_table *devm_pm_opp_set_clkname(struct device *dev, const char *name) > > +{ > > + struct opp_table *opp_table; > > + int err; > > + > > + opp_table = dev_pm_opp_set_clkname(dev, name); > > + if (IS_ERR(opp_table)) > > + return opp_table; > > + > > + err = devm_add_action_or_reset(dev, devm_pm_opp_clkname_release, opp_table); > > + if (err) > > + opp_table = ERR_PTR(err); > > + > > + return opp_table; > > +} > > I wonder if we still need to return opp_table from here, or a simple > integer is fine.. The callers shouldn't be required to use the OPP > table directly anymore I believe and so better simplify the return > part of this and all other routines you are adding here.. Yes, please. I was thinking along the same lines, when I reviewed the mmc patch (patch9). > > If there is a user which needs the opp_table, let it use the regular > non-devm variant. Kind regards Uffe
12.03.2021 08:26, Viresh Kumar пишет: > On 11-03-21, 22:20, Dmitry Osipenko wrote: >> From: Yangtao Li <tiny.windzz@gmail.com> >> >> Add devres wrapper for dev_pm_opp_register_notifier() to simplify driver >> code. >> >> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/opp/core.c | 38 ++++++++++++++++++++++++++++++++++++++ >> include/linux/pm_opp.h | 6 ++++++ >> 2 files changed, 44 insertions(+) > > As I said in the previous version, I am not sure if we need this patch > at all. This has only one user. > I'll drop this patch in v3, thanks.
12.03.2021 13:36, Ulf Hansson пишет: > On Fri, 12 Mar 2021 at 06:33, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> >> On 11-03-21, 22:20, Dmitry Osipenko wrote: >>> +struct opp_table *devm_pm_opp_set_clkname(struct device *dev, const char *name) >>> +{ >>> + struct opp_table *opp_table; >>> + int err; >>> + >>> + opp_table = dev_pm_opp_set_clkname(dev, name); >>> + if (IS_ERR(opp_table)) >>> + return opp_table; >>> + >>> + err = devm_add_action_or_reset(dev, devm_pm_opp_clkname_release, opp_table); >>> + if (err) >>> + opp_table = ERR_PTR(err); >>> + >>> + return opp_table; >>> +} >> >> I wonder if we still need to return opp_table from here, or a simple >> integer is fine.. The callers shouldn't be required to use the OPP >> table directly anymore I believe and so better simplify the return >> part of this and all other routines you are adding here.. > > Yes, please. I was thinking along the same lines, when I reviewed the > mmc patch (patch9). > >> >> If there is a user which needs the opp_table, let it use the regular >> non-devm variant. Indeed, that's a very good suggestion! The opp_table isn't needed by the devm users, I'll change it in v3, thanks!