LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Olliver Schinagl <oliver@schinagl.nl>
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 20:05:37 +0000	[thread overview]
Message-ID: <20190227200537.GB11065@sirena.org.uk> (raw)
In-Reply-To: <c277f7e1-09d9-b6e1-e5d6-8b2f88042e9c@schinagl.nl>

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2019-02-27 20:05 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 [this message]
2019-02-27 20:26                   ` Olliver Schinagl
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=20190227200537.GB11065@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=axel.lin@ingics.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oliver@schinagl.nl \
    --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).