From: Matti Vaittinen <mazziesaccount@gmail.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: "Matti Vaittinen" <matti.vaittinen@fi.rohmeurope.com>,
"Lars-Peter Clausen" <lars@metafoo.de>,
"Michael Hennerich" <Michael.Hennerich@analog.com>,
"Jonathan Cameron" <jic23@kernel.org>,
"David Lechner" <dlechner@baylibre.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
"Liam Girdwood" <lgirdwood@gmail.com>,
"Mark Brown" <broonie@kernel.org>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 1/2] iio: adc: ad7476: Simplify chip type detection
Date: Mon, 4 Aug 2025 08:56:20 +0300 [thread overview]
Message-ID: <6aa66380-8109-472e-8869-bcdc4b0114aa@gmail.com> (raw)
In-Reply-To: <CAHp75VerC+m1XMpZFO6dLp+0HdB1+Xn47on38Mg5AXfDYwWd4A@mail.gmail.com>
On 02/08/2025 01:01, Andy Shevchenko wrote:
> On Fri, Aug 1, 2025 at 12:07 PM Matti Vaittinen
> <mazziesaccount@gmail.com> wrote:
>>
>> The ad7476 driver uses a table of structures for defining the IC variant
>> specific data. Table is indexed using enum values, which are picked by
>> SPI ID.
>>
>> Having the table and an enum adds extra complexity. It is potentially
>> unsafe if someone alters the enumeration values, or size of the IC data
>> table.
>
> I don't see the problem here. I like the part about converting ID
> tables to use chip_info instead of plain integers, but other than that
> I do not see how enum is worse than the split version.
The potential culprit with using the enum for array indexing is, that it
requires the array size and enum values (used for indexing) to stay in
sync. Eg, used enum values must be smaller than the size of the array.
Also, the chip-info items in the array must be kept in locations which
match the enum values.
Yes, we have ways to do this, often using the last enum value as the
size of the array, and/or using designated array initializers. It still
requires programmer to do this correctly. Changing enum at the top of
the file may break the array indexing (in seemingly unrelated place, at
the bottom of the file). I agree this is pretty trivial issue, but it's
still a thing to keep in mind.
Splitting the chip-info in own structs and using direct pointer to the
struct makes it harder to get it wrong.
Finally, dropping the enum makes adding code which does decisions based
on the chip-ID less appealing. It hopefully encourages adding _all_ IC
specific quirks in the chip-info instead, which will keep the code path
(IMHO) cleaner when all chip-specifics are in the chip-info.
Anyways, Thanks for the feedback!
Yours,
-- Matti
next prev parent reply other threads:[~2025-08-04 5:56 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-01 10:06 [RFC PATCH 0/2] iio: adc: ad7476: Simplifications Matti Vaittinen
2025-08-01 10:07 ` [RFC PATCH 1/2] iio: adc: ad7476: Simplify chip type detection Matti Vaittinen
2025-08-01 11:09 ` Jonathan Cameron
2025-08-04 5:57 ` Matti Vaittinen
2025-08-04 8:33 ` Andy Shevchenko
2025-08-01 22:01 ` Andy Shevchenko
2025-08-04 5:56 ` Matti Vaittinen [this message]
2025-08-04 8:31 ` Andy Shevchenko
2025-08-01 10:07 ` [RFC PATCH 2/2] iio: adc: ad7476: Simplify scale handling Matti Vaittinen
2025-08-01 11:12 ` Jonathan Cameron
2025-08-05 16:09 ` David Lechner
2025-08-06 5:08 ` Matti Vaittinen
2025-08-01 12:23 ` [RFC PATCH 0/2] iio: adc: ad7476: Simplifications Nuno Sá
2025-08-02 10:59 ` Jonathan Cameron
2025-08-04 5:29 ` Matti Vaittinen
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=6aa66380-8109-472e-8869-bcdc4b0114aa@gmail.com \
--to=mazziesaccount@gmail.com \
--cc=Michael.Hennerich@analog.com \
--cc=andy.shevchenko@gmail.com \
--cc=andy@kernel.org \
--cc=broonie@kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=lgirdwood@gmail.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matti.vaittinen@fi.rohmeurope.com \
--cc=nuno.sa@analog.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®