2022-04-16 00:21:31

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 4/4] regulator: sm5703-regulator: Add regulators support for SM5703 MFD

On Thu, Apr 14, 2022 at 08:06:15PM +0300, Markuss Broks wrote:

> +++ b/drivers/regulator/Makefile
> @@ -173,5 +173,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
> obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
> obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
> obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
> +obj-$(CONFIG_REGULATOR_SM5703) += sm5703-regulator.o
>

Please keep the Kconfig and Makefile sorted.

> +static const int sm5703_buck_voltagemap[] = {
> + 1000000, 1000000, 1000000, 1000000,
> + 1000000, 1000000, 1000000, 1000000,
> + 1000000, 1000000, 1000000, 1100000,
> + 1200000, 1300000, 1400000, 1500000,
> + 1600000, 1700000, 1800000, 1900000,
> + 2000000, 2100000, 2200000, 2300000,
> + 2400000, 2500000, 2600000, 2700000,
> + 2800000, 2900000, 3000000, 3000000,
> +};

Are the repeated values at the ends of the array actually valid or do
they indicate the part being driven out of spec? If it's out of spec
then it's better to exclude them since otherwise we might choose one of
those values when configuring a voltage which may not actually work as
desired.


Attachments:
(No filename) (1.11 kB)
signature.asc (499.00 B)
Download all attachments

2022-04-16 00:43:02

by Markuss Broks

[permalink] [raw]
Subject: Re: [PATCH 4/4] regulator: sm5703-regulator: Add regulators support for SM5703 MFD

Hi Mark,

On 4/14/22 20:51, Mark Brown wrote:
> On Thu, Apr 14, 2022 at 08:06:15PM +0300, Markuss Broks wrote:
>
>> +++ b/drivers/regulator/Makefile
>> @@ -173,5 +173,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
>> obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
>> obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>> obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
>> +obj-$(CONFIG_REGULATOR_SM5703) += sm5703-regulator.o
>>
> Please keep the Kconfig and Makefile sorted.
>
>> +static const int sm5703_buck_voltagemap[] = {
>> + 1000000, 1000000, 1000000, 1000000,
>> + 1000000, 1000000, 1000000, 1000000,
>> + 1000000, 1000000, 1000000, 1100000,
>> + 1200000, 1300000, 1400000, 1500000,
>> + 1600000, 1700000, 1800000, 1900000,
>> + 2000000, 2100000, 2200000, 2300000,
>> + 2400000, 2500000, 2600000, 2700000,
>> + 2800000, 2900000, 3000000, 3000000,
>> +};
> Are the repeated values at the ends of the array actually valid or do
> they indicate the part being driven out of spec? If it's out of spec
> then it's better to exclude them since otherwise we might choose one of
> those values when configuring a voltage which may not actually work as
> desired.

I am pretty sure those values are valid. This voltage map is taken
directly from the downstream driver (I wish I had actual
documentation...) , which also backs it by saying 32 voltage levels are
supported. However, I find it rather improper to have separate values
which result in same voltage level. Still, I'm not sure something like
selector offset exists in struct regulator_desc or elsewhere.

- Markuss