LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Olliver Schinagl <oliver@schinagl.nl>
To: Mark Brown <broonie@kernel.org>
Cc: Axel Lin <axel.lin@ingics.com>, 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
Date: Wed, 27 Feb 2019 21:26:32 +0100 [thread overview]
Message-ID: <a724fa00-14a2-865b-ed8d-60fa6bda88ba@schinagl.nl> (raw)
In-Reply-To: <20190227200537.GB11065@sirena.org.uk>
On 27-02-2019 21:05, Mark Brown wrote:
> On Wed, Feb 27, 2019 at 08:41:46PM +0100, Olliver Schinagl wrote:
>> On 25-02-2019 18:25, Mark Brown wrote:
>>> If you find you need to describe what the fields are it would be much
>>> more constructive to add a comment at the top of the table saying what
>>> they are. As things are this isn't helping anyone - as a big pile of
>>> defines it's hard to read the values without context for how they're
>>> used and if you're looking at the table you can't tell what the
>>> regulator actually supports without going and decoding the defines.
>> Then the name of the define should be more constructive, which imo they
>> are reasonably? But as everything with programming, naming things is the
>> he hardest part, right?
> I really don't think that's it - I think that sometimes a data table is
> just a data table. There are some coding styles that work to avoid
> having raw numbers anywhere in code outside of defines at all costs but
> I do think that goes too far in cases like this where the name of the
> define is at some level just going to summarize what should go in a
> given slot in a table which adds little.
I'm not sure if we're still talking about the same thing or same table;
In any case, this is something up to personal taste in the end, and I am
in the camp that favors (readable, which of course could always be
improved) defines over magic values; especially when it comes to these
bit-selectors. And even if a little less here, to keep things consistent
with all defines is why at least I prefer the one approach. You guys
prefer the raw values. Two flavors of two opinions I suppose :)
next prev parent reply other threads:[~2019-02-27 20:26 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-20 16:50 Axel Lin
2019-02-20 21:38 ` Olliver Schinagl
2019-02-21 0:22 ` Axel Lin
2019-02-21 9:42 ` Mark Brown
2019-02-23 7:55 ` Olliver Schinagl
2019-02-23 12:54 ` Axel Lin
2019-02-23 20:37 ` Olliver Schinagl
2019-02-25 17:25 ` Mark Brown
2019-02-27 19:41 ` Olliver Schinagl
2019-02-27 20:05 ` Mark Brown
2019-02-27 20:26 ` Olliver Schinagl [this message]
2019-03-23 13:41 ` Axel Lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a724fa00-14a2-865b-ed8d-60fa6bda88ba@schinagl.nl \
--to=oliver@schinagl.nl \
--cc=axel.lin@ingics.com \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=plaes@plaes.org \
--cc=wens@csie.org \
--subject='Re: [PATCH] regulator: axp20x: Get rid of AXP20X_xxx_START/END/STEPS defines' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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).