| Message ID | 61854037cd4d3ac367cfda3f02fd1557b1b3bb8b.1612153322.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] opp: Don't ignore clk_get() errors other than -ENOENT | expand |
01.02.2021 07:22, Viresh Kumar пишет: > Not all devices that need to use OPP core need to have clocks, a missing > clock is fine in which case -ENOENT shall be returned by clk_get(). > > Anything else is an error and must be handled properly. > > Reported-by: Dmitry Osipenko <digetx@gmail.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > V2: > - s/ENODEV/ENOENT > - Use dev_err_probe() > > Stephen, is the understanding correct that -ENOENT is the only error > returned for missing clocks ? > > drivers/opp/core.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index a518173fd64a..0beb3ee79523 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -1252,6 +1252,8 @@ static struct opp_table *_update_opp_table_clk(struct device *dev, > struct opp_table *opp_table, > bool getclk) > { > + int ret; > + > /* > * Return early if we don't need to get clk or we have already tried it > * earlier. > @@ -1261,18 +1263,19 @@ static struct opp_table *_update_opp_table_clk(struct device *dev, > > /* Find clk for the device */ > opp_table->clk = clk_get(dev, NULL); > - if (IS_ERR(opp_table->clk)) { > - int ret = PTR_ERR(opp_table->clk); > > - if (ret == -EPROBE_DEFER) { > - dev_pm_opp_put_opp_table(opp_table); > - return ERR_PTR(ret); > - } > + ret = PTR_ERR_OR_ZERO(opp_table->clk); > + if (!ret) > + return opp_table; > > + if (ret == -ENOENT) { > dev_dbg(dev, "%s: Couldn't find clock: %d\n", __func__, ret); > + return opp_table; > } > > - return opp_table; > + dev_pm_opp_put_opp_table(opp_table); > + > + return ERR_PTR(dev_err_probe(dev, ret, "Couldn't find clock\n")); > } > > /* > Thanks, looks okay. Although, I think the previous variant of coding style was a bit more appropriate, i.e. like this: dev_err_probe(dev, "%s: Couldn't find clock: %d\n", __func__, ret); return ERR_PTR(ret);
On 01-02-21, 22:54, Dmitry Osipenko wrote: > Thanks, looks okay. > > Although, I think the previous variant of coding style was a bit more appropriate, i.e. like this: > > dev_err_probe(dev, "%s: Couldn't find clock: %d\n", __func__, ret); > > return ERR_PTR(ret); Okay will move to that. -- viresh
Quoting Viresh Kumar (2021-01-31 20:22:58) > Not all devices that need to use OPP core need to have clocks, a missing > clock is fine in which case -ENOENT shall be returned by clk_get(). > > Anything else is an error and must be handled properly. > > Reported-by: Dmitry Osipenko <digetx@gmail.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > V2: > - s/ENODEV/ENOENT > - Use dev_err_probe() > > Stephen, is the understanding correct that -ENOENT is the only error > returned for missing clocks ? Yeah pretty much. See clk_get_optional().
diff --git a/drivers/opp/core.c b/drivers/opp/core.c index a518173fd64a..0beb3ee79523 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1252,6 +1252,8 @@ static struct opp_table *_update_opp_table_clk(struct device *dev, struct opp_table *opp_table, bool getclk) { + int ret; + /* * Return early if we don't need to get clk or we have already tried it * earlier. @@ -1261,18 +1263,19 @@ static struct opp_table *_update_opp_table_clk(struct device *dev, /* Find clk for the device */ opp_table->clk = clk_get(dev, NULL); - if (IS_ERR(opp_table->clk)) { - int ret = PTR_ERR(opp_table->clk); - if (ret == -EPROBE_DEFER) { - dev_pm_opp_put_opp_table(opp_table); - return ERR_PTR(ret); - } + ret = PTR_ERR_OR_ZERO(opp_table->clk); + if (!ret) + return opp_table; + if (ret == -ENOENT) { dev_dbg(dev, "%s: Couldn't find clock: %d\n", __func__, ret); + return opp_table; } - return opp_table; + dev_pm_opp_put_opp_table(opp_table); + + return ERR_PTR(dev_err_probe(dev, ret, "Couldn't find clock\n")); } /*
Not all devices that need to use OPP core need to have clocks, a missing clock is fine in which case -ENOENT shall be returned by clk_get(). Anything else is an error and must be handled properly. Reported-by: Dmitry Osipenko <digetx@gmail.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- V2: - s/ENODEV/ENOENT - Use dev_err_probe() Stephen, is the understanding correct that -ENOENT is the only error returned for missing clocks ? drivers/opp/core.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) -- 2.25.0.rc1.19.g042ed3e048af