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:   Thu, 21 Feb 2019 09:42:37 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Axel Lin <axel.lin@...ics.com>
Cc:     Olliver Schinagl <oliver@...inagl.nl>,
        Chen-Yu Tsai <wens@...e.org>, Priit Laes <plaes@...es.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] regulator: axp20x: Get rid of AXP20X_xxx_START/END/STEPS
 defines

On Thu, Feb 21, 2019 at 08:22:53AM +0800, Axel Lin wrote:
> Olliver Schinagl <oliver@...inagl.nl> 於 2019年2月21日 週四 上午6:57寫道:
> > On February 20, 2019 5:50:13 PM GMT+01:00, Axel Lin <axel.lin@...ics.com> 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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists