Re: [PATCH] regulator: axp20x: Get rid of AXP20X_xxx_START/END/STEPS defines

[Mark Brown]

On Thu, Feb 21, 2019 at 08:22:53AM +0800, Axel Lin wrote:
> Olliver Schinagl <oliver@xxxxxxxxxxx> æ 2019å2æ21æ éå äå6:57åéï
> > On February 20, 2019 5:50:13 PM GMT+01:00, Axel Lin <axel.lin@xxxxxxxxxx> wrote:

> > >The AXP20X_xxx_START/END/STEPS defines make the code hard to read and
> > >very hard to check the linear range settings because it needs to check
> > >the defines one-by-one.
> > >The original code without the defines is very good in readability
> > >as the meaning of each field of REGULATOR_LINEAR_RANGE is clear.
> > >So I suggest to get rid of AXP20X_xxx_START/END/STEPS defines.

> > Are you suggesting that magic values and hex numbers are more readable?

> For example:
> static const struct regulator_linear_range axp803_dcdc234_ranges[] = {
>        REGULATOR_LINEAR_RANGE(500000, 0x0, 0x46, 10000),
>        REGULATOR_LINEAR_RANGE(1220000, 0x47, 0x4b, 20000),
> };
> Above looks very clear to me as it describes the linear ranges:
> 1st linear range: min_uV is 500000, from selector 0 ~ 0x46, the uV_step is 10000
> 2nd linear range: min_uV is 1220000, from selector 0x47 ~ 0x4b, the
> uV_step is 20000
> And it's easy to check the min_sel and max_sel in each linear range.

Frankly I tend to agree with Axel here - the defines are used in exactly
one place so their main impact is that you can't read the ranges
directly in the array but have to jump around to read through the
defines.

Attachment: signature.asc
Description: PGP signature