linux-kernel - Re: [PATCH 1/1] regulators: bd718x7: Add enable times

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 16 Dec 2020 13:41:33 +0100
From:   Guido Günther <guido.gunther@...i.sm>
To:     "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
Cc:     "lgirdwood@...il.com" <lgirdwood@...il.com>,
        linux-power <linux-power@...rohmeurope.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "angus.ainslie@...i.sm" <angus.ainslie@...i.sm>
Subject: Re: [PATCH 1/1] regulators: bd718x7: Add enable times

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@...xcpu.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.

Cheers,
 --Guido

> 
> Best Regards
>     Matti Vaittinen
> 
>