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 >