Re: [PATCH] iio: accel: adxl380: fix FIFO watermark bit 8 always written as 0

[Jonathan Cameron]

On Sat, 28 Feb 2026 10:50:00 -0600
David Lechner <dlechner@xxxxxxxxxxxx> wrote:

> On 2/27/26 6:43 AM, Antoniu Miclaus wrote:
> > FIELD_PREP(BIT(0), fifo_samples & BIT(8)) produces either 0 or 256,
> > and since FIELD_PREP masks to bit 0, 256 & 1 evaluates to 0. Use !!
> > to convert the result to a proper 0-or-1 value.
> > 
> > Fixes: df36de13677a ("iio: accel: add ADXL380 driver")
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
> > ---
> >  drivers/iio/accel/adxl380.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/accel/adxl380.c b/drivers/iio/accel/adxl380.c
> > index 8fab2fdbe147..a51d1d61c412 100644
> > --- a/drivers/iio/accel/adxl380.c
> > +++ b/drivers/iio/accel/adxl380.c
> > @@ -877,7 +877,7 @@ static int adxl380_set_fifo_samples(struct adxl380_state *st)  
> 
> Some relevant context...
> 
> 	u16 fifo_samples = st->watermark * st->fifo_set_size;
> 
> >  	ret = regmap_update_bits(st->regmap, ADXL380_FIFO_CONFIG_0_REG,
> >  				 ADXL380_FIFO_SAMPLES_8_MSK,
> >  				 FIELD_PREP(ADXL380_FIFO_SAMPLES_8_MSK,
> > -					    (fifo_samples & BIT(8))));
> > +					    !!(fifo_samples & BIT(8))));  
> 
> Technically, this works, but in terms of understanding the code I think
> fifo_samples >= BIT(8) would make more sense.
> 
> fifo_samples is a count, not bit flags.
It's an odd bit of code, but then it's unusual hardware too.
To me it is about bit 8 not the value being big enough to have a bit 8 because
if there were yet another register that was for BIT(9) then
 >= BIT(8) would give the wrong value if BIT(9) was set and BIT(8) was not.

We could express this what is going on in a bit more explicit detail maybe?

	FIELD_PREP(ADXL380_FIFO_SAMPLES_8_MSK,
		   FIELD_GET(BIT(8), fifo_samples));

So there we are extracting the 8th bit and writing to the location for the 8th
bit.
> 
> 
> >  	if (ret)
> >  		return ret;
> >    
>