Re: [PATCH 3/4] opp: Reused enabled flag and remove regulator_enabled

[Viresh Kumar]

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