Re: [PATCH v2 08/22] iio: accel: adxl345: initialize FIFO delay value for SPI

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 17 Nov 2024 18:26:37 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add the possibility to delay FIFO access when SPI is used. According to
> the datasheet this is needed for the adxl345. When initialization
> happens over SPI the need for delay is to be signalized, and the delay
> will be used.
A specific reference to a section of the specification might be useful
here.

One trivial comment inline, but otherwise looks fine to me.

> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/iio/accel/adxl345.h      | 2 +-
>  drivers/iio/accel/adxl345_core.c | 6 +++++-
>  drivers/iio/accel/adxl345_i2c.c  | 2 +-
>  drivers/iio/accel/adxl345_spi.c  | 3 +++
>  4 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
> index cf4132715c..4ba493f636 100644
> --- a/drivers/iio/accel/adxl345.h
> +++ b/drivers/iio/accel/adxl345.h
> @@ -62,7 +62,7 @@ struct adxl345_chip_info {
>  };
>  
>  int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> -		       int irq,
> +		       int irq, bool fifo_delay_default,
>  		       int (*setup)(struct device*, struct regmap*));
>  
>  #endif /* _ADXL345_H_ */
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 902bd3568b..51b229cc44 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -22,6 +22,7 @@ struct adxl34x_state {
>  	int irq;
>  	const struct adxl345_chip_info *info;
>  	struct regmap *regmap;
> +	bool fifo_delay; /* delay: delay is needed for SPI */
>  };
>  
>  #define ADXL345_CHANNEL(index, axis) {					\
> @@ -199,13 +200,14 @@ static const struct iio_info adxl345_info = {
>   * @dev:	Driver model representation of the device
>   * @regmap:	Regmap instance for the device
>   * @irq:	Interrupt handling for async usage
> + * @fifo_delay_default: Using FIFO with SPI needs delay
>   * @setup:	Setup routine to be executed right before the standard device
>   *		setup
>   *
>   * Return: 0 on success, negative errno on error
>   */
>  int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> -		       int irq,
> +		       int irq, bool fifo_delay_default,
>  		       int (*setup)(struct device*, struct regmap*))
>  {
>  	struct adxl34x_state *st;
> @@ -234,6 +236,8 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
>  	if (!st->info)
>  		return -ENODEV;
>  
> +	st->fifo_delay = fifo_delay_default;
> +
>  	indio_dev->name = st->info->name;
>  	indio_dev->info = &adxl345_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c
> index 604b706c29..fa1b7e7026 100644
> --- a/drivers/iio/accel/adxl345_i2c.c
> +++ b/drivers/iio/accel/adxl345_i2c.c
> @@ -27,7 +27,7 @@ static int adxl345_i2c_probe(struct i2c_client *client)
>  	if (IS_ERR(regmap))
>  		return dev_err_probe(&client->dev, PTR_ERR(regmap), "Error initializing regmap\n");
>  
> -	return adxl345_core_probe(&client->dev, regmap, client->irq, NULL);
> +	return adxl345_core_probe(&client->dev, regmap, client->irq, false, NULL);
>  }
>  
>  static const struct adxl345_chip_info adxl345_i2c_info = {
> diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
> index 39e7d71e1d..75940d9c1c 100644
> --- a/drivers/iio/accel/adxl345_spi.c
> +++ b/drivers/iio/accel/adxl345_spi.c
> @@ -12,6 +12,7 @@
>  #include "adxl345.h"
>  
>  #define ADXL345_MAX_SPI_FREQ_HZ		5000000
> +#define ADXL345_MAX_FREQ_NO_FIFO_DELAY	1500000
>  
>  static const struct regmap_config adxl345_spi_regmap_config = {
>  	.reg_bits = 8,
> @@ -41,10 +42,12 @@ static int adxl345_spi_probe(struct spi_device *spi)
>  	if (spi->mode & SPI_3WIRE)
>  		return adxl345_core_probe(&spi->dev, regmap,
>  					  spi->irq,
> +					  spi->max_speed_hz > ADXL345_MAX_FREQ_NO_FIFO_DELAY,
>  					  adxl345_spi_setup);
>  	else
>  		return adxl345_core_probe(&spi->dev, regmap,
>  					  spi->irq,
> +					  spi->max_speed_hz > ADXL345_MAX_FREQ_NO_FIFO_DELAY,

use a local variable to establish this without the very long line.

>  					  NULL);
>  }
>