On 15-08-20, 00:55, Stephen Boyd wrote:
> Quoting Viresh Kumar (2020-08-12 21:29:00)
> > diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> > index e8882e7fd8a5..5f5da257f58a 100644
> > --- a/drivers/opp/core.c
> > +++ b/drivers/opp/core.c
> > @@ -703,12 +703,10 @@ static int _generic_set_opp_regulator(struct
> > opp_table *opp_table,
> > * Enable the regulator after setting its voltages, otherwise it
> > breaks
> > * some boot-enabled regulators.
> > */
> > - if (unlikely(!opp_table->regulator_enabled)) {
> > + if (unlikely(!opp_table->enabled)) {
> > ret = regulator_enable(reg);
> > if (ret < 0)
> > dev_warn(dev, "Failed to enable regulator: %d",
> > ret);
> > - else
> > - opp_table->regulator_enabled = true;
>
> A quick glance makes this look unsafe now because we're only checking
> 'enabled' and not actually setting it when this function is called. I
> have to go back to the previous patch to understand where enabled is now
> set to confirm that it is OK. If it was all one patch all the context
> would be here.
The only case where things can go crazy are the cases where (for
example) clk_set_rate() fails, or something like that which would be a
bug and it shouldn't bother in the normal working of this code.
--
viresh
