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

[Jonathan Cameron <jic23@kernel.org>]

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