From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: "guido.gunther@puri.sm" <guido.gunther@puri.sm>
Cc: "lgirdwood@gmail.com" <lgirdwood@gmail.com>,
linux-power <linux-power@fi.rohmeurope.com>,
"broonie@kernel.org" <broonie@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"angus.ainslie@puri.sm" <angus.ainslie@puri.sm>
Subject: Re: [PATCH 1/1] regulators: bd718x7: Add enable times
Date: Thu, 17 Dec 2020 14:21:46 +0000 [thread overview]
Message-ID: <beba25e85db20649aa040fc0ae549895c9265f6f.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <20201216124133.GA31109@bogon.m.sigxcpu.org>
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
next prev parent reply other threads:[~2020-12-17 14:23 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-16 11:05 [PATCH 0/1] regulators: bd718x7: Add enable times Guido Günther
2020-12-16 11:05 ` [PATCH 1/1] " Guido Günther
2020-12-16 12:29 ` Vaittinen, Matti
2020-12-16 12:41 ` Guido Günther
2020-12-16 12:48 ` Mark Brown
2020-12-16 17:47 ` Vaittinen, Matti
2020-12-17 14:21 ` Vaittinen, Matti [this message]
2020-12-16 13:06 ` [PATCH 0/1] " Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=beba25e85db20649aa040fc0ae549895c9265f6f.camel@fi.rohmeurope.com \
--to=matti.vaittinen@fi.rohmeurope.com \
--cc=angus.ainslie@puri.sm \
--cc=broonie@kernel.org \
--cc=guido.gunther@puri.sm \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-power@fi.rohmeurope.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).