[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160915180223.GE62872@google.com>
Date: Thu, 15 Sep 2016 11:02:23 -0700
From: Matthias Kaehlcke <mka@...omium.org>
To: Mark Brown <broonie@...nel.org>
Cc: lgirdwood@...il.com, Douglas Anderson <dianders@...omium.org>,
briannorris@...omium.org, javier@...hile0.org, robh+dt@...nel.org,
mark.rutland@....com, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v4 4/4] regulator: Prevent falling too fast
Hi Mark,
El Thu, Sep 15, 2016 at 03:39:45PM +0100 Mark Brown ha dit:
> On Tue, Sep 13, 2016 at 10:21:40AM -0700, Matthias Kaehlcke wrote:
>
> > Optimizing the delay time depends on the SoC; we have not measured
> > this across a wide variety of devices and thus have very conservative
> > numbers. The percentage is based on the trigger for OVP on the
> > regulator. In this case, OVP kicks in when the FB node is 20% over
> > regulation, which is equivalent to a 16% drop in voltage (1/1.2). For
> > our device safe-fall-percent is set to 16 and slowest-decay-rate is
> > 225.
>
> The obvious question here is how the OVP hardware knows about the new
> voltage and why we're bodging this in the regulator core rather than in
> the OVP hardware.
The OVP hardware is part of the regulator and the regulator is not
notified directly about voltage changes. The regulator transforms the
PWM input into DC output and does the OVP internally with the limits
described above.
> > > I'd like to see some more thorough analysis than just an "apparently"
> > > here. It's making the code more fiddly for something very handwavy.
>
> > It's well-understood why it's a percentage. DVS is controlled by
> > offsetting the FB current, and as above, OVP is based on how far you
> > are from the FB target.
>
> You might think you understand this clearly but nobody reading the
> commit message or looking at the code later on is going to be able
> do so when your commit message only contains vague handwaving.
In case there is a next revision of this patch (i.e. if it is deemed
useful beyond our specific use case) I can include the details in the
commit message of the next revision. Sorry for omitting them
initially, to me it seemed to be very device specific information, but
I understand that these details can be helpful to understand the
context.
Matthias
Powered by blists - more mailing lists