Re: [PATCH v5 2/3] iio: accel: add ADXL380 driver

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 8 Jul 2024 13:40:12 +0300
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:

> The ADXL380/ADXL382 is a low noise density, low power, 3-axis
> accelerometer with selectable measurement ranges. The ADXL380 supports
> the +/-4 g, +/-8 g, and +/-16 g ranges, and the ADXL382 supports
> +/-15 g, +/-30 g and +/-60 g ranges.
> The ADXL380/ADXL382 offers industry leading noise, enabling precision
> applications with minimal calibration. The low noise, and low power
> ADXL380/ADXL382 enables accurate measurement in an environment with
> high vibration, heart sounds and audio.
> 
> In addition to its low power consumption, the ADXL380/ADXL382 has many
> features to enable true system level performance. These include a
> built-in micropower temperature sensor, single / double / triple tap
> detection and a state machine to prevent a false triggering. In
> addition, the ADXL380/ADXL382 has provisions for external control of
> the sampling time and/or an external clock.
> 
> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>

One locking (kind of) issue that I'll tidy up.

Applied to the testing branch of iio.git which will become togreg once
rebased on rc1 when that is available.

In the meantime 0-day will poke at it and see what we missed.

Thanks,

Jonathan

> diff --git a/drivers/iio/accel/adxl380.c b/drivers/iio/accel/adxl380.c
> new file mode 100644
> index 000000000000..684778ace345
> --- /dev/null
> +++ b/drivers/iio/accel/adxl380.c
> @@ -0,0 +1,1906 @@
;
> +
> +static int adxl380_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long info)
> +{
> +	struct adxl380_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_device_claim_direct_mode(indio_dev);
> +		if (ret)
> +			return ret;
> +
> +		ret = adxl380_read_chn(st, chan->address);
> +		if (ret)
> +			return ret;

This returns without release direct mode which means deadlock
if you enable the buffer (that will wait unlike here).

If nothing else comes up I'll make the minimal change of
moving this down to after the release.


> +
> +		iio_device_release_direct_mode(indio_dev);
> +
> +		*val = sign_extend32(ret >> chan->scan_type.shift,
> +				     chan->scan_type.realbits - 1);
> +		return IIO_VAL_INT;