Re: [PATCH v2 06/10] iio: adc: ad7476: Drop convstart chan_spec

[Matti Vaittinen]

On 08/08/2025 00:16, Andy Shevchenko wrote:
> On Thu, Aug 7, 2025 at 11:35 AM Matti Vaittinen
> <mazziesaccount@xxxxxxxxx> wrote:
> > The ad7476 driver defines separate chan_spec structures for operation
> > with and without convstart GPIO. At quick glance this may seem as if the
> > driver did provide more than 1 data-channel to users - one for the
> > regular data, other for the data obtained with the convstart GPIO.
> >
> > The only difference between the 'convstart' and 'non convstart'
> > -channels is presence / absence of the BIT(IIO_CHAN_INFO_RAW) in
> > channel's flags.
> >
> > We can drop the convstart channel spec, and related convstart macro, by
> > allocating a mutable per driver instance channel spec an adding the flag
>
> and adding
>
> > in probe if needed. This will simplify the driver with the cost of added
> > memory consumption.
> >
> > Assuming there aren't systems with very many ADCs and very few
> > resources, this tradeoff seems worth making.
> >
> > Simplify the driver by dropping the 'convstart' channel spec and
> > allocating the chan spec for each driver instance.
>
> channel
>
> (you already used 'channel spec' above, be consistent)
>
> ...
>
> > -       int ret;
> > +       int ret, i;
>
> Why? Is 'i' going to be used to hold a signed value?
>
> ...
>
> > +       /*
> > +        * This will never realize. Unless someone changes the channel specs
>
> realize --> happen
>
> > +        * in this driver. And if someone does, without changing the loop
> > +        * below, then we'd better immediately produce a big fat error, before
> > +        * the change proceeds from that developer's table.
> > +        */
> > +       BUILD_BUG_ON(ARRAY_SIZE(st->channel) != ARRAY_SIZE(chip_info->channel));
>
> We have static_assert(). Why can't it be used?

Don't know. Can you please enlighten me why one is preferred over the other?

Thanks for the review Andy!

Yours,
	-- Matti