linux-kernel - Re: [PATCH] regulator: axp20x: AXP717: set ramp

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]
Message-ID: <CAGb2v66rd8AVUsTKG6Eic99fDmFxG95EtRsYrCoXhGrxn9EyFQ@mail.gmail.com>
Date: Sat, 7 Dec 2024 12:37:36 +0800
From: Chen-Yu Tsai <wens@...e.org>
To: Philippe Simons <simons.philippe@...il.com>
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 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.

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?


Thanks
ChenYu


>                  AXP717_DCDC4_CONTROL, AXP717_DCDC_V_OUT_MASK,
>                  AXP717_DCDC_OUTPUT_CONTROL, BIT(3)),
> --
> 2.47.1
>