On Wed, Nov 18, 2020 at 12:24 AM Nishanth Menon <nm@xxxxxx> wrote: > On 16:25-20201117, Arnd Bergmann wrote: > > Yes, this was indeed a bug that has been around for some time now :( > > I tested with a variant of the above (did'nt like that > oinfo was being assigned an invalid address) > Boot log: https://pastebin.ubuntu.com/p/nZfz3HF8N6/ (with the same > config as in the report): Would you prefer to me to send the following > as a formal patch? Awesome, thanks for the new patch and testing it! Yes, please send this as a proper patch to have it picked up into the regulator tree as a bugfix. Reported-by: Naresh Kamboju <naresh.kamboju@xxxxxxxxxx> Reviewed-by: Arnd Bergmann <arnd@xxxxxxxx> > diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c > index 3e60bff76194..9f0a4d50cead 100644 > --- a/drivers/regulator/ti-abb-regulator.c > +++ b/drivers/regulator/ti-abb-regulator.c > @@ -342,8 +342,17 @@ static int ti_abb_set_voltage_sel(struct regulator_dev *rdev, unsigned sel) > return ret; > } > > - /* If data is exactly the same, then just update index, no change */ > info = &abb->info[sel]; > + /* > + * When Linux kernel is starting up, we are'nt sure of the > + * Bias configuration that bootloader has configured. > + * So, we get to know the actual setting the first time > + * we are asked to transition. > + */ > + if (abb->current_info_idx == -EINVAL) > + goto just_set_abb; > + > + /* If data is exactly the same, then just update index, no change */ > oinfo = &abb->info[abb->current_info_idx]; > if (!memcmp(info, oinfo, sizeof(*info))) { > dev_dbg(dev, "%s: Same data new idx=%d, old idx=%d\n", __func__, > @@ -351,6 +360,7 @@ static int ti_abb_set_voltage_sel(struct regulator_dev *rdev, unsigned sel) > goto out; > } > > +just_set_abb: > ret = ti_abb_set_opp(rdev, abb, info); > > out: