Re: [PATCH] iio: accel: adxl380: clamp FIFO sample count

Jonathan Cameron <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel
Message-ID <20260812060029.7dc3d25b@jic23-huawei>
On Mon, 10 Aug 2026 13:06:01 +0800
"Shengzhuo Wei" <[email protected]> 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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.