Re: [PATCH 03/10] iio: magnetometer: rm3100: Stop abusing the ->currentmode

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 15 Dec 2021 16:13:37 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> This is an internal variable for the core, here it is set to a "default"
> value by the driver in order to later be able to perform checks against
> it. None of this is needed because this check actually cares about the
> buffers being enabled or not. So it is an unproper side-channel access
> to the information "are the buffers enabled?", returned officially by
> the iio_buffer_enabled() helper. Use this helper instead.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Hi Miquel,

Make sure to +cc driver authors on v2.

Whilst I think this is safe we should definitely give them visibility.

Obviously some IIO drivers probably have authors who have long moved on
but this one is only 2018 vintage so Song Qiang might still have
access to hardware or be willing to do a review!

Jonathan


> ---
>  drivers/iio/magnetometer/rm3100-core.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magnetometer/rm3100-core.c
> index 13914273c999..be0057f82218 100644
> --- a/drivers/iio/magnetometer/rm3100-core.c
> +++ b/drivers/iio/magnetometer/rm3100-core.c
> @@ -141,18 +141,10 @@ static irqreturn_t rm3100_irq_handler(int irq, void *d)
>  	struct iio_dev *indio_dev = d;
>  	struct rm3100_data *data = iio_priv(indio_dev);
>  
> -	switch (indio_dev->currentmode) {
> -	case INDIO_DIRECT_MODE:
> +	if (!iio_buffer_enabled(indio_dev))
>  		complete(&data->measuring_done);
> -		break;
> -	case INDIO_BUFFER_TRIGGERED:
> +	else
>  		iio_trigger_poll(data->drdy_trig);
> -		break;
> -	default:
> -		dev_err(indio_dev->dev.parent,
> -			"device mode out of control, current mode: %d",
> -			indio_dev->currentmode);
> -	}
>  
>  	return IRQ_WAKE_THREAD;
>  }
> @@ -377,7 +369,7 @@ static int rm3100_set_samp_freq(struct iio_dev *indio_dev, int val, int val2)
>  			goto unlock_return;
>  	}
>  
> -	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> +	if (iio_buffer_enabled(indio_dev)) {
>  		/* Writing TMRC registers requires CMM reset. */
>  		ret = regmap_write(regmap, RM3100_REG_CMM, 0);
>  		if (ret < 0)
> @@ -553,7 +545,6 @@ int rm3100_common_probe(struct device *dev, struct regmap *regmap, int irq)
>  	indio_dev->channels = rm3100_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(rm3100_channels);
>  	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
> -	indio_dev->currentmode = INDIO_DIRECT_MODE;
>  
>  	if (!irq)
>  		data->use_interrupt = false;