| 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
| ||
|
Message-ID: <CADomA49gYKNGn9R-HaKFPQ77PR6vyHRHNYCJ=3iY_qyiTE8uYg@mail.gmail.com>
Date: Sat, 7 Dec 2024 10:18:48 +0100
From: Philippe Simons <simons.philippe@...il.com>
To: wens@...e.org
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
"open list:VOLTAGE AND CURRENT REGULATOR FRAMEWORK" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] regulator: axp20x: AXP717: set ramp_delay
On Sat, Dec 7, 2024 at 5:37 AM Chen-Yu Tsai <wens@...e.org> wrote:
>
> On Fri, Dec 6, 2024 at 8:38 PM Philippe Simons
> <simons.philippe@...il.com> wrote:
> >
> > AXP717 datasheet says that regulator ramp delay is 15.625 us/step,
> > which is 10mV in our case.
>
> Thanks! It looks like the reason the delay is needed is because the
> voltage ramp control feature is on by default on this chip.
>
> That is not entirely correct. 10mV is the smallest step forDCDC1,
> DCDC2, and DCDC3, so it makes sense to calculate the ramp delay
> using that.
>
> For DCDC4, the step is 100mV, so the delay would be different.
>
> > Add a AXP_DESC_RANGES_DELAY macro and update AXP_DESC_RANGES macro to
> > expand to AXP_DESC_RANGES_DELAY with ramp_delay = 0
> >
> > Signed-off-by: Philippe Simons <simons.philippe@...il.com>
>
> The ramp rate / delay is actually configurable between 15.625 us/step
> and 31.250 us/step. However the setting shared among all DCDCs that
> support this function and have it turned on. I'm not sure how that
> should be modeled if we want to make it configurable.
there is axp20x_set_ramp_delay() ops, but it seems to only handle
axp209 DCDC2 and LDO3
>
> Also looking at older AXP PMICs:
>
> - AXP803: ramp rate 2.5mV / us, on by default, has register bit that can
> polled to check for ramp completion
> - AXP813: same as AXP803
>
> - AXP805: Similar to AXP717, where rate is given in time / step, the
> options are the same two and step is different for different
> buck regulators; difference is not all buck regulators support
> this feature, and also the rate is individually configurable
> for each one. Off by default.
>
> - AXP223: 1.6mV/us or 0.8mV/us; off by default; only two buck regulators
> support it; individually configurable.
>
> > ---
> > drivers/regulator/axp20x-regulator.c | 23 +++++++++++++++--------
> > 1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> > index a8e91d9d0..8f035db13 100644
> > --- a/drivers/regulator/axp20x-regulator.c
> > +++ b/drivers/regulator/axp20x-regulator.c
> > @@ -419,8 +419,8 @@
> > .ops = &axp20x_ops_fixed \
> > }
> >
> > -#define AXP_DESC_RANGES(_family, _id, _match, _supply, _ranges, _n_voltages, \
> > - _vreg, _vmask, _ereg, _emask) \
> > +#define AXP_DESC_RANGES_DELAY(_family, _id, _match, _supply, _ranges, _n_voltages, \
> > + _vreg, _vmask, _ereg, _emask, _ramp_delay) \
> > [_family##_##_id] = { \
> > .name = (_match), \
> > .supply_name = (_supply), \
> > @@ -437,8 +437,15 @@
> > .linear_ranges = (_ranges), \
> > .n_linear_ranges = ARRAY_SIZE(_ranges), \
> > .ops = &axp20x_ops_range, \
> > + .ramp_delay = (_ramp_delay), \
> > }
> >
> > +#define AXP_DESC_RANGES(_family, _id, _match, _supply, _ranges, _n_voltages, \
> > + _vreg, _vmask, _ereg, _emask) \
> > + AXP_DESC_RANGES_DELAY(_family, _id, _match, _supply, _ranges, _n_voltages, \
> > + _vreg, _vmask, _ereg, _emask, 0)
> > +
> > +
> > static const int axp209_dcdc2_ldo3_slew_rates[] = {
> > 1600,
> > 800,
> > @@ -781,18 +788,18 @@ static const struct linear_range axp717_dcdc3_ranges[] = {
> > };
> >
> > static const struct regulator_desc axp717_regulators[] = {
> > - AXP_DESC_RANGES(AXP717, DCDC1, "dcdc1", "vin1",
> > + AXP_DESC_RANGES_DELAY(AXP717, DCDC1, "dcdc1", "vin1",
> > axp717_dcdc1_ranges, AXP717_DCDC1_NUM_VOLTAGES,
> > AXP717_DCDC1_CONTROL, AXP717_DCDC_V_OUT_MASK,
> > - AXP717_DCDC_OUTPUT_CONTROL, BIT(0)),
> > - AXP_DESC_RANGES(AXP717, DCDC2, "dcdc2", "vin2",
> > + AXP717_DCDC_OUTPUT_CONTROL, BIT(0), 640),
> > + AXP_DESC_RANGES_DELAY(AXP717, DCDC2, "dcdc2", "vin2",
> > axp717_dcdc2_ranges, AXP717_DCDC2_NUM_VOLTAGES,
> > AXP717_DCDC2_CONTROL, AXP717_DCDC_V_OUT_MASK,
> > - AXP717_DCDC_OUTPUT_CONTROL, BIT(1)),
> > - AXP_DESC_RANGES(AXP717, DCDC3, "dcdc3", "vin3",
> > + AXP717_DCDC_OUTPUT_CONTROL, BIT(1), 640),
> > + AXP_DESC_RANGES_DELAY(AXP717, DCDC3, "dcdc3", "vin3",
> > axp717_dcdc3_ranges, AXP717_DCDC3_NUM_VOLTAGES,
> > AXP717_DCDC3_CONTROL, AXP717_DCDC_V_OUT_MASK,
> > - AXP717_DCDC_OUTPUT_CONTROL, BIT(2)),
> > + AXP717_DCDC_OUTPUT_CONTROL, BIT(2), 640),
> > AXP_DESC(AXP717, DCDC4, "dcdc4", "vin4", 1000, 3700, 100,
>
> Can you also add it to DCDC4 for completeness?
I'll do in V2
>
>
> Thanks
> ChenYu
>
>
> > AXP717_DCDC4_CONTROL, AXP717_DCDC_V_OUT_MASK,
> > AXP717_DCDC_OUTPUT_CONTROL, BIT(3)),
> > --
> > 2.47.1
> >
Powered by blists - more mailing lists