Re: [PATCH v4 04/14] iio: accel: adxl345: introduce adxl345_push_event function

[Andy Shevchenko <andy.shevchenko@gmail.com>]

Thu, Mar 13, 2025 at 04:50:39PM +0000, Lothar Rubusch kirjoitti:
> Move the fifo handling into a separate function. This is a preparation
> for a generic handling of the interrupt status register results.
> 
> The interrupt status register is read into a variable int_stat. It carries
> status for various sensor events, handling of which is added in follow up
> patches. Evaluation of the int_stat variable is common for sensor events,
> such as tap detection, freefall, activity,... and for fifo events, such as
> data ready, overrun, watermark,... Also, dealing with in case error
> returns shall be common to all events. Thus migrate fifo read-out and push
> fifo content to iio channels into this function to be built up with
> additional event handling.

...

> +static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat)
> +{
> +	struct adxl345_state *st = iio_priv(indio_dev);
> +	int samples;
> +	int ret = -ENOENT;
> +
> +	if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {
> +		samples = adxl345_get_samples(st);
> +		if (samples < 0)

> +			return -EINVAL;

In the original code it makes no difference, but if you are going to share
this, I would expect to see

			return samples;

here. Why the error code is shadowed? If it's trully needed, it has to be
explained in the comment.


> +		if (adxl345_fifo_push(indio_dev, samples) < 0)
> +			return -EINVAL;
> +	}
> +
> +	return ret;
> +}

...

Jonathan, I saw that you had applied it, but I guess the above needs
a clarification.

-- 
With Best Regards,
Andy Shevchenko