On Tue, 25 Jun 2024 18:55:27 -0300
Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx> wrote:
> Add support for AD4000 series of low noise, low power, high speed,
> successive approximation register (SAR) ADCs.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx>
Hi Marcelo,
You've clearly gotten some good review for this version so I only
had a quick scan through. One thing did jump out at me though.
> +
> +static int ad4000_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + return IIO_VAL_INT_PLUS_NANO;
> + default:
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> +}
> +
> +static int ad4000_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val, int val2,
> + long mask)
> +{
> + struct ad4000_state *st = iio_priv(indio_dev);
> + unsigned int reg_val;
> + bool span_comp_en;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + mutex_lock(&st->lock);
> + ret = ad4000_read_reg(st, ®_val);
> + if (ret < 0)
> + goto err_unlock;
> +
> + span_comp_en = val2 == st->scale_tbl[1][1];
> + reg_val &= ~AD4000_CFG_SPAN_COMP;
> + reg_val |= FIELD_PREP(AD4000_CFG_SPAN_COMP, span_comp_en);
> +
> + ret = ad4000_write_reg(st, reg_val);
> + if (ret < 0)
> + goto err_unlock;
> +
> + st->span_comp = span_comp_en;
> +err_unlock:
> + iio_device_release_direct_mode(indio_dev);
> + mutex_unlock(&st->lock);
Lock ordering needs another look. I'm not sure we an trigger
a deadlock but it definitely looks problematic.
J
