On Tue, May 20, 2025 at 10:50:02PM +0000, Lothar Rubusch wrote:
> Prepare the interrupt handler. Add register entries to evaluate the
> incoming interrupt. Add functions to clear status registers and reset the
> FIFO.
...
> +static int adxl313_fifo_transfer(struct adxl313_data *data, int samples)
> +{
> + size_t count;
> + int i;
Does it need to be signed?
> + int ret;
> +
> + count = sizeof(data->fifo_buf[0]) * ADXL313_NUM_AXIS;
> + for (i = 0; i < samples; i++) {
> + ret = regmap_bulk_read(data->regmap, ADXL313_REG_XYZ_BASE,
> + data->fifo_buf + (i * count / 2), count);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
...
> +static void adxl313_fifo_reset(struct adxl313_data *data)
> +{
> + int regval;
Incorrect type. Please, go through your whole code and check that for regmap
returned value you use the same type. Using signed might lead to subtle and
hard-to-hunt mistakes due to integer promotion rules in C language.
> + int samples;
> +
> + adxl313_set_measure_en(data, false);
> +
> + /* clear samples */
> + samples = adxl313_get_samples(data);
> + if (samples)
> + adxl313_fifo_transfer(data, samples);
> +
> + /* clear interrupt register */
> + regmap_read(data->regmap, ADXL313_REG_INT_SOURCE, ®val);
No error check? At least comment why it's okay to go like this.
> + adxl313_set_measure_en(data, true);
> +}
...
> +static irqreturn_t adxl313_irq_handler(int irq, void *p)
> +{
> + struct iio_dev *indio_dev = p;
> + struct adxl313_data *data = iio_priv(indio_dev);
> + int int_stat;
> +
> + if (regmap_read(data->regmap, ADXL313_REG_INT_SOURCE, &int_stat))
> + return IRQ_NONE;
> +
> + if (FIELD_GET(ADXL313_INT_OVERRUN, int_stat))
> + goto err;
> +
> + return IRQ_HANDLED;
> +err:
> + adxl313_fifo_reset(data);
> +
> + return IRQ_HANDLED;
Seems you ignored my comment in previous review round. Please, re-read that
thread and discuss all the stuff you disagree with (I want to know why).
> +}
...
> + ret = devm_iio_kfifo_buffer_setup(dev, indio_dev,
One space too many.
> + &adxl313_buffer_ops);
> + if (ret)
> + return ret;
--
With Best Regards,
Andy Shevchenko
