Re: [PATCH] iio: accel: adxl380: clamp FIFO sample count
From: Jonathan Cameron
Date: Wed Aug 12 2026 - 01:01:20 EST
On Mon, 10 Aug 2026 13:06:01 +0800
"Shengzhuo Wei" <me@xxxxxxxx> wrote:
> 在 2026-08-10 00:28,Jonathan Cameron 写道:
> > > Fixes: df36de13677a ("iio: accel: add ADXL380 driver")
> >
> > In my opinion at least, these are not fixes. In general we don't expect
> > drivers to be hardened against broken hardware returning out of spec
> > values. I don't mind taking simple cases though that don't complicate
> > the code much and if anything make it a little easier to follow,
> > but I don't currently see any reason to mark them as a fix.
> >
> > So drop that tag for v2.
> >
>
> Hi Jonathan,
>
> Thanks. Understood — I'll drop the Fixes tag and stop clamping.
>
> > This is papering over what we think is a hardware failure. Unless I am
> > missing something the device is returning garbage, otherwise we are in
> > range and this has no affect. We have no idea how much data there is
> > if we get a value outside the expected range.
> >
> > As such I'd expect an error print and probably no attempt to carry
> > on reading as we have no idea what happened.
>
> For v2 I'll treat an out-of-range count as a hardware error,
> log it, and skip the read rather than carrying on:
>
> ret = adxl380_get_fifo_entries(st, &fifo_entries);
> if (ret)
> return IRQ_HANDLED;
>
> if (fifo_entries > ADXL380_FIFO_SAMPLES) {
> dev_err_ratelimited(st->dev,
> "invalid FIFO entry count %u (max %lu)\n",
> fifo_entries, ADXL380_FIFO_SAMPLES);
> return IRQ_HANDLED;
> }
>
> fifo_entries = rounddown(fifo_entries, st->fifo_set_size);
> ret = regmap_noinc_read(st->regmap, ADXL380_FIFO_DATA, &st->fifo_buf,
> sizeof(*st->fifo_buf) * fifo_entries);
>
> Same for adxl367 (push_fifo_data: dev_err_ratelimited and return true
> without reading the FIFO).
>
> I'll send the two as a single series with a cover letter, no Fixes tags.
>
> Let me know if this looks OK to you, or if you'd change anything, and
> I'll send the v2 series.
>
Looks good to me. The rate limit is interesting but perhaps does make
sense here given we are kind of assuming the hardware is stuck in
a bad condition. On the other hand, it's complexity for a path we
never expect to take in practice. Definitely make sure to add a
brief description of why that is used in the patch description.
Thanks
Jonathan
> Best regards,
> Shengzhuo Wei
>