Messages in this thread | | | From | "Vaittinen, Matti" <> | Subject | Re: [PATCH 1/1] regulators: bd718x7: Add enable times | Date | Thu, 17 Dec 2020 14:21:46 +0000 |
| |
On Wed, 2020-12-16 at 13:41 +0100, Guido Günther wrote: > Hi Matti, > On Wed, Dec 16, 2020 at 12:29:20PM +0000, Vaittinen, Matti wrote: > > Hello Guido, > > > > Thanks for looking at this! > > > > On Wed, 2020-12-16 at 12:05 +0100, Guido Günther wrote: > > > Use the typical startup times from the data sheet so boards get a > > > reasonable default. Not setting any enable time can lead to board > > > hangs > > > when e.g. clocks are enabled too soon afterwards. > > > > > > This fixes gpu power domain resume on the Librem 5. > > > > > > Signed-off-by: Guido Günther <agx@sigxcpu.org> > > > --- > > > drivers/regulator/bd718x7-regulator.c | 27 > > > +++++++++++++++++++++++++++ > > > 1 file changed, 27 insertions(+) > > > > > > diff --git a/drivers/regulator/bd718x7-regulator.c > > > b/drivers/regulator/bd718x7-regulator.c > > > index e6d5d98c3cea..d6d34aa4ee2e 100644 > > > --- a/drivers/regulator/bd718x7-regulator.c > > > +++ b/drivers/regulator/bd718x7-regulator.c > > > @@ -613,6 +613,7 @@ static struct bd718xx_regulator_data > > > bd71847_regulators[] = { > > > .vsel_mask = DVS_BUCK_RUN_MASK, > > > .enable_reg = BD718XX_REG_BUCK1_CTRL, > > > .enable_mask = BD718XX_BUCK_EN, > > > + .enable_time = 144, > > > > Where are these values obtained from? I have a feeling they might > > be > > board / load specific. If this is the case - can the "regulator- > > enable- > > ramp-delay" from device-tree be used instead to avoid hard-coding > > board > > specific values in the driver? Although, sane defaults would > > probably > > not be a bad idea - if I read code correctly then the constrains > > from > > DT can be used to override these values. > > They're the 'typical values' from the data sheet and it's basically > all > about setting a default for "regulator-enable-ramp-delay" to avoid > having every board do the same. If that's not the right thing todo > let > me know and i add these to each of our boards (which is where i > basically started from but then figured that this would be busywork > and every board would hit that problem). > > > I'd prefer well named defines over raw numeric values though. > > So s.th. like > > BD71837_BUCK1_STARTUP_TIME 144 > > (using the terminology from the datasheet)? If that works I'll send a > v2.
Just noticed I never replied. Sorry. This looks good to me!
Best Regards --Matti
| |