Message ID | 20210311192105.14998-1-digetx@gmail.com |
---|---|
Headers | show |
Series | Introduce devm_pm_opp_* API | expand |
11.03.2021 22:44, Mark Brown пишет: > On Thu, Mar 11, 2021 at 10:20:58PM +0300, Dmitry Osipenko wrote: > >> Acked-by: Mark brown <broonie@kernel.org> > > Typo there. > Good catch! Although, that should be a patchwork fault since it auto-added acks when I downloaded v1 patches and I haven't changed them. I'll fix it in v3 or, if there won't be anything else to improve, then maybe Viresh could fix it up while applying patches.
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 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!