Re: [PATCH v5 2/2] iio: light: add AS7343 multi-spectral sensor driver

[Chang Yu]

On Thu, Sep 24, 2026 at 09:22:05PM +0300, Andy Shevchenko wrote:
> On Sat, Sep 19, 2026 at 04:51:45PM -0700, Chang Yu wrote:
> > Add a driver for the AMS AS7343 14-channel multi-spectral sensor.
> >
> > The AS7343 is a 14-channel spectral sensor featuring 11 visible
> > channels, 1 near-infrared channel, 1 clear channel (VIS), and 1
> > flicker detection channel.
> >
> > The driver exposes 12 spectral channels (11 visible light and 1
> > near-infrared) via sysfs. Runtime PM is implemented to stop
> > measurements when the device is suspended or torn down. Power is
> > never cut (PON=1 always) to preserve register values.
> >
> > Future patches will add configurable gain and integration time,
> > interrupt support, buffered reads, VIS channel, and flicker
> > detection.
>
> ...
>
> > +#include <linux/build_bug.h>
>
> This is unusual to meet...
>
> ...
>
> > +static inline u8 as7343_atime_steps(unsigned int step)
> > +{
> > +   BUILD_BUG_ON(step < 1 || step > 256);
>
> How is this any useful? The step == 1 gives the result 0, the step 256,
> 255 respectively. This covers the whole range of the returned variable
> (u8).
>
> > +   return step - 1;
> > +}
> > +
> > +static inline u16 as7343_astep_x2780ns(unsigned int n)
> > +{
> > +   BUILD_BUG_ON(n < 1 || n > 65535);
> > +   return n - 1;
> > +}
>
> Ditto.
>
> ...
>
> TL;DR: I do not see anything meaningful from these BUILD_BUG_ON():s.
> It's not a problem if the result gives just wrapped-up value. Or is it?
>
For now just letting them wrap back up is fine. The values are hard coded
so it's unlikely to happen anyways. But since these are going to be
configurable by the user in the future, I'll make these into runtime checks
as per Jonathan's advice so that we have a way to tell the user that they
are doing something wrong.