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@xxxxxxxxx>
> ---
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á
