linux-kernel - Re: [PATCH] regulator: rt5033: Use linear ranges to map all voltage selection

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:   Fri, 2 Jul 2021 23:47:50 +0800
From:   ChiYuan Huang <u0084500@...il.com>
To:     Axel Lin <axel.lin@...ics.com>
Cc:     Mark Brown <broonie@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        LKML <linux-kernel@...r.kernel.org>,
        ChiYuan Huang <cy_huang@...htek.com>
Subject: Re: [PATCH] regulator: rt5033: Use linear ranges to map all voltage selection

ChiYuan Huang <u0084500@...il.com> 於 2021年7月1日 週四 下午1:53寫道：
>
> Axel Lin <axel.lin@...ics.com> 於 2021年7月1日 週四 下午12:41寫道：
> >
> >
> >
> > cy_huang <u0084500@...il.com> 於 2021年7月1日 週四 上午11:54寫道：
> >>
> >> From: ChiYuan Huang <cy_huang@...htek.com>
> >>
> >> Instead of linear mapping, Use linear range to map all voltage selection.
> >>
> >> Signed-off-by: ChiYuan Huang <cy_huang@...htek.com>
> >> ---
> >> Even though commit 6549c46af855 ("regulator: rt5033: Fix n_voltages settings for BUCK and LDO")
> >> can fix the linear mapping to the correct min/max voltage
> >> But there're still non-step ranges for the reserved value.
> >>
> >> To use the linear range can fix it for mapping all voltage selection.
> >> ---
> >>  drivers/regulator/rt5033-regulator.c | 23 +++++++++++++++--------
> >>  1 file changed, 15 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/regulator/rt5033-regulator.c b/drivers/regulator/rt5033-regulator.c
> >> index 0e73116..2ff607c 100644
> >> --- a/drivers/regulator/rt5033-regulator.c
> >> +++ b/drivers/regulator/rt5033-regulator.c
> >> @@ -13,6 +13,16 @@
> >>  #include <linux/mfd/rt5033-private.h>
> >>  #include <linux/regulator/of_regulator.h>
> >>
> >> +static const struct linear_range rt5033_buck_ranges[] = {
> >> +       REGULATOR_LINEAR_RANGE(1000000, 0, 20, 100000),
> >> +       REGULATOR_LINEAR_RANGE(3000000, 21, 31, 0),
> >> +};
> >> +
> >> +static const struct linear_range rt5033_ldo_ranges[] = {
> >> +       REGULATOR_LINEAR_RANGE(1200000, 0, 18, 100000),
> >> +       REGULATOR_LINEAR_RANGE(3000000, 19, 31, 0),
> >> +};
> >> +
> >>  static const struct regulator_ops rt5033_safe_ldo_ops = {
> >>         .is_enabled             = regulator_is_enabled_regmap,
> >>         .enable                 = regulator_enable_regmap,
> >> @@ -24,8 +34,7 @@ static const struct regulator_ops rt5033_buck_ops = {
> >>         .is_enabled             = regulator_is_enabled_regmap,
> >>         .enable                 = regulator_enable_regmap,
> >>         .disable                = regulator_disable_regmap,
> >> -       .list_voltage           = regulator_list_voltage_linear,
> >> -       .map_voltage            = regulator_map_voltage_linear,
> >> +       .list_voltage           = regulator_list_voltage_linear_range,
> >>         .get_voltage_sel        = regulator_get_voltage_sel_regmap,
> >>         .set_voltage_sel        = regulator_set_voltage_sel_regmap,
> >>  };
> >> @@ -39,9 +48,8 @@ static const struct regulator_desc rt5033_supported_regulators[] = {
> >>                 .ops            = &rt5033_buck_ops,
> >>                 .type           = REGULATOR_VOLTAGE,
> >>                 .owner          = THIS_MODULE,
> >> -               .n_voltages     = RT5033_REGULATOR_BUCK_VOLTAGE_STEP_NUM,
> >> -               .min_uV         = RT5033_REGULATOR_BUCK_VOLTAGE_MIN,
> >> -               .uV_step        = RT5033_REGULATOR_BUCK_VOLTAGE_STEP,
> >> +               .linear_ranges  = rt5033_buck_ranges,
> >> +               .n_linear_ranges = ARRAY_SIZE(rt5033_buck_ranges),
> >>
> >
> > If you want to use linear range here, you need to change RT5033_REGULATOR_BUCK_VOLTAGE_STEP_NUM back to 32
> > rather than delete the .n_voltages setting.
>
> Sorry, I really forget the N_VOLTAGES.
> >
> > I'm fifty-fifty about the change because I don't see any benefit with converting to linear range (even though in theory it's correct).
> > The voltage of all entries in the second linear range is the *same* as the latest selector of the first linear range.
> > When the regulator core to choose the best selector, it will always select the latest selector of the first linear range if it meets the requested range anyway.
> > (Because the entries in the second linear range are not *better*, it's just the same.)
> >
> > If the initial version is this driver is using linear range then it's fine.
> > But given the initial version is using linear so when I fix the n_voltages setting I decide to not change it to linear range.
> > This makes it easier to fix older versions if necessary.
> > (I'm not sure if linear range is available in some old kernel versions, the initial version of this driver was committed in 2014).
> >
> >
> From the regulator register in probe, it will get the current voltage
> from the IC.
> If the vout sel is not is over N_VOLTAGES, it will return the error number.
>
> But as I think it's the side effect to change the vout step num.
> To use the linear range is just to guarantee all vout sel range are included.
>
> That's my initial thoughts.

Hi, Axel
   Like as you said,  the first revision is from 2014.
It is almost EOL for this product.
To fix this seems redundant.
Thx.
> > Regards,
> > Axel
> >