Re: [PATCH v4 02/14] iio: accel: adxl345: move INT enable to regmap cache

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 13 Mar 2025 16:50:37 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Replace the interrupt enable member variable to the regmap cache. This
> makes the function set_interrupts() obsolete. The interrupt enable
> register is written when the driver is probed. Thus it is perfectly
> cacheable.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Applied with a small tweak. I don't think you touch set_watermark
again later in the series so this shouldn't cause too much impact.

> ---
>  drivers/iio/accel/adxl345_core.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 6f337b26999a..10e2da7de17e 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -36,7 +36,6 @@ struct adxl345_state {
>  	struct regmap *regmap;
>  	bool fifo_delay; /* delay: delay is needed for SPI */
>  	int irq;
> -	u8 int_map;
>  	u8 watermark;
>  	u8 fifo_mode;
>  	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
> @@ -114,11 +113,6 @@ static int adxl345_set_measure_en(struct adxl345_state *st, bool en)
>  	return regmap_write(st->regmap, ADXL345_REG_POWER_CTL, val);
>  }
>  
> -static int adxl345_set_interrupts(struct adxl345_state *st)
> -{
> -	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, st->int_map);
> -}
> -
>  static int adxl345_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
>  			    int *val, int *val2, long mask)
> @@ -217,7 +211,7 @@ static int adxl345_reg_access(struct iio_dev *indio_dev, unsigned int reg,
>  static int adxl345_set_watermark(struct iio_dev *indio_dev, unsigned int value)
>  {
>  	struct adxl345_state *st = iio_priv(indio_dev);
> -	unsigned int fifo_mask = 0x1F;
> +	const unsigned int fifo_mask = 0x1F, watermark_mask = 0x02;
>  	int ret;
>  
>  	value = min(value, ADXL345_FIFO_SIZE - 1);
> @@ -227,7 +221,10 @@ static int adxl345_set_watermark(struct iio_dev *indio_dev, unsigned int value)
>  		return ret;
>  
>  	st->watermark = value;
> -	st->int_map |= ADXL345_INT_WATERMARK;
> +	ret = regmap_update_bits(st->regmap, ADXL345_REG_INT_ENABLE, watermark_mask,
> +				 ADXL345_INT_WATERMARK);
> +	if (ret)
> +		return ret;
tweaked to
	return regmap.
>  
>  	return 0;
>  }
> @@ -381,11 +378,6 @@ static void adxl345_fifo_reset(struct adxl345_state *st)
>  static int adxl345_buffer_postenable(struct iio_dev *indio_dev)
>  {
>  	struct adxl345_state *st = iio_priv(indio_dev);
> -	int ret;
> -
> -	ret = adxl345_set_interrupts(st);
> -	if (ret < 0)
> -		return ret;
>  
>  	st->fifo_mode = ADXL345_FIFO_STREAM;
>  	return adxl345_set_fifo(st);
> @@ -401,8 +393,7 @@ static int adxl345_buffer_predisable(struct iio_dev *indio_dev)
>  	if (ret < 0)
>  		return ret;
>  
> -	st->int_map = 0x00;
> -	return adxl345_set_interrupts(st);
> +	return regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, 0x00);
>  }
>  
>  static const struct iio_buffer_setup_ops adxl345_buffer_ops = {
> @@ -524,6 +515,11 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
>  	indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
>  	indio_dev->available_scan_masks = adxl345_scan_masks;
>  
> +	/* Reset interrupts at start up */
> +	ret = regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, 0x00);
> +	if (ret)
> +		return ret;
> +
>  	if (setup) {
>  		/* Perform optional initial bus specific configuration */
>  		ret = setup(dev, st->regmap);