* [PATCH] regulator: rt5033: Use linear ranges to map all voltage selection
@ 2021-07-01 3:54 cy_huang
[not found] ` <CAFRkauCNf6fP8zAz+0gY_Vzb_wCtVyYqLjw8s1T+t2s=bR0RQw@mail.gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: cy_huang @ 2021-07-01 3:54 UTC (permalink / raw)
To: broonie; +Cc: lgirdwood, axel.lin, linux-kernel, ChiYuan Huang
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),
.enable_reg = RT5033_REG_CTRL,
.enable_mask = RT5033_CTRL_EN_BUCK_MASK,
.vsel_reg = RT5033_REG_BUCK_CTRL,
@@ -55,9 +63,8 @@ static const struct regulator_desc rt5033_supported_regulators[] = {
.ops = &rt5033_buck_ops,
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,
- .n_voltages = RT5033_REGULATOR_LDO_VOLTAGE_STEP_NUM,
- .min_uV = RT5033_REGULATOR_LDO_VOLTAGE_MIN,
- .uV_step = RT5033_REGULATOR_LDO_VOLTAGE_STEP,
+ .linear_ranges = rt5033_ldo_ranges,
+ .n_linear_ranges = ARRAY_SIZE(rt5033_ldo_ranges),
.enable_reg = RT5033_REG_CTRL,
.enable_mask = RT5033_CTRL_EN_LDO_MASK,
.vsel_reg = RT5033_REG_LDO_CTRL,
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] regulator: rt5033: Use linear ranges to map all voltage selection
[not found] ` <CAFRkauCNf6fP8zAz+0gY_Vzb_wCtVyYqLjw8s1T+t2s=bR0RQw@mail.gmail.com>
@ 2021-07-01 5:53 ` ChiYuan Huang
2021-07-02 15:47 ` ChiYuan Huang
0 siblings, 1 reply; 5+ messages in thread
From: ChiYuan Huang @ 2021-07-01 5:53 UTC (permalink / raw)
To: Axel Lin; +Cc: Mark Brown, Liam Girdwood, LKML, ChiYuan Huang
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.
> Regards,
> Axel
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] regulator: rt5033: Use linear ranges to map all voltage selection
2021-07-01 5:53 ` ChiYuan Huang
@ 2021-07-02 15:47 ` ChiYuan Huang
2021-07-05 16:52 ` Mark Brown
0 siblings, 1 reply; 5+ messages in thread
From: ChiYuan Huang @ 2021-07-02 15:47 UTC (permalink / raw)
To: Axel Lin; +Cc: Mark Brown, Liam Girdwood, LKML, ChiYuan Huang
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
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] regulator: rt5033: Use linear ranges to map all voltage selection
2021-07-02 15:47 ` ChiYuan Huang
@ 2021-07-05 16:52 ` Mark Brown
2021-07-06 6:32 ` ChiYuan Huang
0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2021-07-05 16:52 UTC (permalink / raw)
To: ChiYuan Huang; +Cc: Axel Lin, Liam Girdwood, LKML, ChiYuan Huang
[-- Attachment #1: Type: text/plain, Size: 856 bytes --]
On Fri, Jul 02, 2021 at 11:47:50PM +0800, ChiYuan Huang wrote:
> ChiYuan Huang <u0084500@gmail.com> 於 2021年7月1日 週四 下午1:53寫道:
> > Axel Lin <axel.lin@ingics.com> 於 2021年7月1日 週四 下午12:41寫道:
> > 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.
> Like as you said, the first revision is from 2014.
> It is almost EOL for this product.
> To fix this seems redundant.
Even if something is old it's still posible that people can use it, and
if someone's done the work to fix it then why not?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] regulator: rt5033: Use linear ranges to map all voltage selection
2021-07-05 16:52 ` Mark Brown
@ 2021-07-06 6:32 ` ChiYuan Huang
0 siblings, 0 replies; 5+ messages in thread
From: ChiYuan Huang @ 2021-07-06 6:32 UTC (permalink / raw)
To: Mark Brown; +Cc: Axel Lin, Liam Girdwood, LKML, ChiYuan Huang
Mark Brown <broonie@kernel.org> 於 2021年7月6日 週二 上午12:53寫道:
>
> On Fri, Jul 02, 2021 at 11:47:50PM +0800, ChiYuan Huang wrote:
> > ChiYuan Huang <u0084500@gmail.com> 於 2021年7月1日 週四 下午1:53寫道:
> > > Axel Lin <axel.lin@ingics.com> 於 2021年7月1日 週四 下午12:41寫道:
>
> > > 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.
>
> > Like as you said, the first revision is from 2014.
> > It is almost EOL for this product.
> > To fix this seems redundant.
>
> Even if something is old it's still posible that people can use it, and
> if someone's done the work to fix it then why not?
Got it, I'll send the v2 patch to fix it.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-06 6:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-01 3:54 [PATCH] regulator: rt5033: Use linear ranges to map all voltage selection cy_huang
[not found] ` <CAFRkauCNf6fP8zAz+0gY_Vzb_wCtVyYqLjw8s1T+t2s=bR0RQw@mail.gmail.com>
2021-07-01 5:53 ` ChiYuan Huang
2021-07-02 15:47 ` ChiYuan Huang
2021-07-05 16:52 ` Mark Brown
2021-07-06 6:32 ` ChiYuan Huang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).