Re: [PATCH 1/2] iio: adxl345: add spi-3wire

["Nuno Sá" <noname.nuno@gmail.com>]

On Tue, 2024-03-19 at 21:27 +0000, Lothar Rubusch wrote:
> Adds the spi-3wire feature and adds general refactoring to the
> iio driver.
> 
> The patch moves driver wide constants and fields into the
> header. Thereby reduces redundant info struct definitions.
> Allows to pass a function pointer from SPI/I2C specific probe,
> and smaller refactorings. A regmap_update_bits() in the core
> file replaces the regmap_write() to format_data.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---

On top of what Krzysztof already said I would also like for you to split the 
spi-3wire (which is adding a new feature) from the refactor in two different
patches. One more comment inline...

>  drivers/iio/accel/adxl345.h      |  44 +++++++++++-
>  drivers/iio/accel/adxl345_core.c | 116 +++++++++++++++++--------------
>  drivers/iio/accel/adxl345_i2c.c  |  30 ++++----
>  drivers/iio/accel/adxl345_spi.c  |  50 ++++++++-----
>  4 files changed, 153 insertions(+), 87 deletions(-)
> 

...

> diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
> index 93ca349f1..e456b61c6 100644
> --- a/drivers/iio/accel/adxl345_spi.c
> +++ b/drivers/iio/accel/adxl345_spi.c
> @@ -20,48 +20,62 @@ static const struct regmap_config
> adxl345_spi_regmap_config = {
>  	.read_flag_mask = BIT(7) | BIT(6),
>  };
>  
> +static int adxl345_spi_setup(struct device *dev, struct regmap *regmap)
> +{
> +	struct spi_device *spi = container_of(dev, struct spi_device, dev);
> +	int ret;
> +
> +	if (spi->mode & SPI_3WIRE) {
> +		ret = regmap_write(regmap, ADXL345_REG_DATA_FORMAT,
> +				   ADXL345_DATA_FORMAT_SPI);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;

I think this would be neater:

if (!(spi->mode & SPI_3WIRE))
	return 0;

return regmap_write(regmap, ADXL345_REG_DATA_FORMAT,
		    ADXL345_DATA_FORMAT_SPI);

- Nuno Sá