Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 21 Feb 2019 09:42:37 +0000
From:   Mark Brown <broonie@kernel.org>
To:     Axel Lin <axel.lin@ingics.com>
Cc:     Olliver Schinagl <oliver@schinagl.nl>,
        Chen-Yu Tsai <wens@csie.org>, Priit Laes <plaes@plaes.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] regulator: axp20x: Get rid of AXP20X_xxx_START/END/STEPS
 defines
Message-ID: <20190221094237.GA5970@sirena.org.uk>
References: <20190220165013.12774-1-axel.lin@ingics.com>
 <24E35288-677D-4223-B94A-52A4F37792A8@schinagl.nl>
 <CAFRkauAPGEBxj7k2GvmFwhyc7ZuofhXou17b75F3G-mUWVaf9A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="ikeVEW9yuYc//A+q"
Content-Disposition: inline
In-Reply-To: <CAFRkauAPGEBxj7k2GvmFwhyc7ZuofhXou17b75F3G-mUWVaf9A@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--ikeVEW9yuYc//A+q
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 21, 2019 at 08:22:53AM +0800, Axel Lin wrote:
> Olliver Schinagl <oliver@schinagl.nl> =E6=96=BC 2019=E5=B9=B42=E6=9C=8821=
=E6=97=A5 =E9=80=B1=E5=9B=9B =E4=B8=8A=E5=8D=886:57=E5=AF=AB=E9=81=93=EF=BC=
=9A
> > On February 20, 2019 5:50:13 PM GMT+01:00, Axel Lin <axel.lin@ingics.co=
m> 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[] =3D {
>        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 i=
s 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.

--ikeVEW9yuYc//A+q
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAlxucowACgkQJNaLcl1U
h9BGHwf/Q+ifyezTt87Xs3xlYSMNusmi3EIumBSCRu65XFMH1MM7qA/xGCQCcWF0
4k/0iSwQbo7wbHwdsDaptxPk0sOflhgKNwlO3Lm+R33MbO6JH2Rn/ARamzeipI8T
guPQ+QrlMy4GFlAfufnWZOf7hqZh6JEpWw5N5q23swZ4ELlLFkLydRI4jk2SuCkT
KMfjYE/ylYcECaHCtbClI8ixZVz8HaCjI0UyoAkGUy04XA63IxOkNRRYxEtzhNNr
iZrZvoRwC+RWtkE71Vags0O4lARbwn2Y8NZAZHFUj7Qk0D9N5uxD/kYxQIKPpcOo
7aVK9oPOKbFMmeR2B/9iu93d7f5IMg==
=Ia8M
-----END PGP SIGNATURE-----

--ikeVEW9yuYc//A+q--