Re: [PATCH 4/6] iio: adc: ti-ads1015: Deduplicate channel macros

Jonathan Cameron
2022-03-20T13:15:02Z

On Fri, 11 Mar 2022 19:51:56 +0100
Marek Vasut <marex@denx.de> wrote:

> On 3/11/22 18:03, Andy Shevchenko wrote:
> > On Fri, Mar 11, 2022 at 12:33:49PM +0100, Marek Vasut wrote:
> >> On 3/11/22 12:29, Andy Shevchenko wrote:
> >>> On Fri, Mar 11, 2022 at 1:55 AM Marek Vasut <marex@denx.de> wrote:
> >>>> On 3/10/22 15:25, Andy Shevchenko wrote:
> >>>>> On Thu, Mar 10, 2022 at 01:34:00AM +0100, Marek Vasut wrote:
> >
> > ...
> >
> >>>>>> - .realbits = 12, \
> >>>>>> + .realbits = (_realbits), \
> >>>>>> .storagebits = 16, \
> >>>>>
> >>>>> This seems inconsistent a bit. What if the next chip wants to have more than
> >>>>> 16 bits in realbits?
> >>>>
> >>>> When such a chip exists, this can be parametrized as well.
> >>>
> >>> Yes, My point is that it's error prone.
> >>
> >> Won't IIO core warn if realbits > storagebits ?
> >
> > If it's the case, then it's very good!
>
> No, apparently it won't .

Easy to add I think and a good idea. Though can only be a runtime
check obviously.

Put a verification check in iio_buffer_add_channel_sys() which
is registering the _type attr used to get access to this info from
user space.

Jonathan

>
> > ...
> >
> >>>>> I see two options:
> >>>>> 1) add static assert to make sure realbits <= storagebits;
> >>>>
> >>>> Does static_assert work in array of structures (I don't think it does) ?
> >>>
> >>> You can check, but IIRC some of the macros have it. Don't remember the
> >>> details, though.
> >>
> >> I already checked before replying, hence my question, as I didn't find a way
> >> to make it work.
> >
> > It seems that current use cases have it either in functions or in
> > the expressions as ({...}). I dunno if the result of ({...}) can be
> > a data structure or compound literal.
>
> I added a patch to v2, but ugh, it isn't nice:
>
> [PATCH v2 7/7] iio: adc: ti-ads1015: Add static assert to test if
> shifted realbits fit into storagebits