[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251207125213.1f413e6b@jic23-huawei>
Date: Sun, 7 Dec 2025 12:52:13 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Antoniu Miclaus <antoniu.miclaus@...log.com>, Lars-Peter Clausen
<lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, David
Lechner <dlechner@...libre.com>, Nuno Sá
<nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 2/2] iio: amplifiers: adl8113: add driver support
On Sat, 6 Dec 2025 22:48:19 +0200
Andy Shevchenko <andriy.shevchenko@...el.com> wrote:
> On Sat, Dec 06, 2025 at 07:03:32PM +0000, Jonathan Cameron wrote:
> > On Fri, 5 Dec 2025 16:40:41 +0200
> > Antoniu Miclaus <antoniu.miclaus@...log.com> wrote:
>
> ...
>
> > > + /* Always include internal amplifier (14dB) */
> > > + st->gain_configs[i].path = ADL8113_INTERNAL_AMP;
> > > + st->gain_configs[i].gain_db = 14;
> >
> > Could do this as something like:
>
> I remember a discussion where it was against this approach due to compiler
> warnings or so. However, IIRC, there was slightly different pattern, i.e.
>
> foo[i].bar = ...
> foo[i++].baz = ...
>
> That said, I have no objection to your proposal, but we need to use it with
> a good compile test coverage (clang with `make W=1` for a starter.
That's my standard 1st build choice now because it seems to be fussiest ;)
I haven't tried this one however.
Googling came up with a link saying GCC gives warnings on cases where
not all structure elements are set. I think they always should be here
though which may make this fine (and act as a correct warning if they aren't).
That only works if the two uninitialized structure elements are dropped
obviously!
>
> OTOH, the original code is robust enough...
>
> > st->gain_configs[i++] = (struct adl8113_gain_config) {
> > .path = ADL8113_INTERNAL_AMP,
> > .gain_db = 14,
> > };
>
> ...and doesn't require a good understanding of differences between designated
> initialisers and compound literals.
Fair enough. Original is better than the i++ in only the second in my view.
Jonathan
>
> > st->gain_configs[i++] = (struct adl8113_gain_config) {
> > .path = ADL8113_INTERNAL_BYPASS,
> > .gain_db = -2,
> > };
>
>
Powered by blists - more mailing lists