Re: [PATCH 1/2] iio: adc: ti-ads1298: Add safe usage guarantee for device communication

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 23 Apr 2025 15:46:25 -0300
Arthur-Prince <r2.arthur.prince@gmail.com> wrote:

> Add safe usage guarantee for device communication in 
> ads1298_reg_access/ ads1298_write_raw (ti-ads1298) that 
> prevent simultaneous read/write operations.
> 
> Signed-off-by: Arthur-Prince <r2.arthur.prince@gmail.com>
> Co-developed-by: Mariana Valério <mariana.valerio2@hotmail.com>
> Signed-off-by: Mariana Valério <mariana.valerio2@hotmail.com>
Two different things seem to be going on in here...

> ---
>  drivers/iio/adc/ti-ads1298.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
> index 03f762415fa5..438bcc3c74da 100644
> --- a/drivers/iio/adc/ti-ads1298.c
> +++ b/drivers/iio/adc/ti-ads1298.c
> @@ -355,9 +355,19 @@ static int ads1298_write_raw(struct iio_dev *indio_dev,
>  {
>  	struct ads1298_private *priv = iio_priv(indio_dev);
>  
> +	int ret;
> +
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		return ads1298_set_samp_freq(priv, val);
> +		ret = iio_device_claim_direct_mode(indio_dev);
> +		if (ret)
> +			return ret;
> +
> +		ret = ads1298_set_samp_freq(priv, val);
> +
> +		iio_device_release_direct_mode(indio_dev);
This prevents changing the sampling frequency.  Some drivers prevent this
whilst in buffered mode, but why is that necessary here?

> +
> +		return ret;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -417,10 +427,17 @@ static int ads1298_reg_access(struct iio_dev *indio_dev, unsigned int reg,
>  {
>  	struct ads1298_private *priv = iio_priv(indio_dev);
>  
> +	int ret;
> +	ret = iio_device_claim_direct_mode(indio_dev);
For this one it's a debug interface. If the issue is people shooting themselves in the
foot then we don't normally prevent that for these.

If there is something fundamental about regmap implementation
that makes these accesses have side effects unconnected to what is being
written then that needs calling out.

> +	if (ret)
> +		return ret;
>  	if (readval)
> -		return regmap_read(priv->regmap, reg, readval);
> -
> -	return regmap_write(priv->regmap, reg, writeval);
> +		ret = regmap_read(priv->regmap, reg, readval);
> +	else
> +		ret = regmap_write(priv->regmap, reg, writeval);
> +	
> +	iio_device_release_direct_mode(indio_dev);
> +	return ret;
>  }
>  
>  static void ads1298_rdata_unmark_busy(struct ads1298_private *priv)