Messages in this thread | | | From | ChiYuan Huang <> | Date | Fri, 2 Jul 2021 23:47:50 +0800 | Subject | Re: [PATCH] regulator: rt5033: Use linear ranges to map all voltage selection |
| |
ChiYuan Huang <u0084500@gmail.com> 於 2021年7月1日 週四 下午1:53寫道: > > Axel Lin <axel.lin@ingics.com> 於 2021年7月1日 週四 下午12:41寫道: > > > > > > > > cy_huang <u0084500@gmail.com> 於 2021年7月1日 週四 上午11:54寫道: > >> > >> From: ChiYuan Huang <cy_huang@richtek.com> > >> > >> Instead of linear mapping, Use linear range to map all voltage selection. > >> > >> Signed-off-by: ChiYuan Huang <cy_huang@richtek.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 > >
| |