Re: [PATCH v2 1/3] iio: accel: adxl345: Update adxl345

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 22/03/2024 01:37, Lothar Rubusch wrote:
> Move driver wide constants and fields into the header.

Why?

> Let probe call a separate setup function. Provide

Why?

> possibility for an SPI/I2C specific setup to be passed
> as function pointer to core.

Why?

Your commit message *MUST* explain why you are doing things.

> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/iio/accel/adxl345.h      |  44 +++++++++++-
>  drivers/iio/accel/adxl345_core.c | 117 +++++++++++++++++--------------
>  drivers/iio/accel/adxl345_i2c.c  |  30 ++++----
>  drivers/iio/accel/adxl345_spi.c  |  28 ++++----
>  4 files changed, 134 insertions(+), 85 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
> index 284bd387c..01493c999 100644
> --- a/drivers/iio/accel/adxl345.h
> +++ b/drivers/iio/accel/adxl345.h
> @@ -8,6 +8,39 @@
>  #ifndef _ADXL345_H_
>  #define _ADXL345_H_
>  
> +#include <linux/iio/iio.h>
> +
> +/* ADXL345 register definitions */
> +#define ADXL345_REG_DEVID		0x00
> +#define ADXL345_REG_OFSX		0x1E
> +#define ADXL345_REG_OFSY		0x1F
> +#define ADXL345_REG_OFSZ		0x20
> +#define ADXL345_REG_OFS_AXIS(index)	(ADXL345_REG_OFSX + (index))
> +#define ADXL345_REG_BW_RATE		0x2C
> +#define ADXL345_REG_POWER_CTL		0x2D
> +#define ADXL345_REG_DATA_FORMAT		0x31
> +#define ADXL345_REG_DATAX0		0x32
> +#define ADXL345_REG_DATAY0		0x34
> +#define ADXL345_REG_DATAZ0		0x36
> +#define ADXL345_REG_DATA_AXIS(index)	\
> +	(ADXL345_REG_DATAX0 + (index) * sizeof(__le16))
> +
> +#define ADXL345_BW_RATE			GENMASK(3, 0)
> +#define ADXL345_BASE_RATE_NANO_HZ	97656250LL
> +
> +#define ADXL345_POWER_CTL_MEASURE	BIT(3)
> +#define ADXL345_POWER_CTL_STANDBY	0x00
> +
> +#define ADXL345_DATA_FORMAT_FULL_RES	BIT(3) /* Up to 13-bits resolution */
> +#define ADXL345_DATA_FORMAT_SPI         BIT(6) /* spi-3wire */
> +#define ADXL345_DATA_FORMAT_2G		0
> +#define ADXL345_DATA_FORMAT_4G		1
> +#define ADXL345_DATA_FORMAT_8G		2
> +#define ADXL345_DATA_FORMAT_16G		3
> +#define ADXL345_DATA_FORMAT_MSK		~((u8) BIT(6)) /* ignore spi-3wire */
> +
> +#define ADXL345_DEVID			0xE5
> +
>  /*
>   * In full-resolution mode, scale factor is maintained at ~4 mg/LSB
>   * in all g ranges.
> @@ -23,11 +56,20 @@
>   */
>  #define ADXL375_USCALE	480000
>  
> +enum adxl345_device_type {
> +	ADXL345,
> +	ADXL375,
> +};
> +
>  struct adxl345_chip_info {
>  	const char *name;
>  	int uscale;
>  };
>  
> -int adxl345_core_probe(struct device *dev, struct regmap *regmap);
> +extern const struct adxl345_chip_info adxl3x5_chip_info[];
> +
> +int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> +		       const struct adxl345_chip_info *chip_info,
> +		       int (*setup)(struct device*, struct regmap*));

Last setup argument is entirely unused. Drop this change, it's not
related to this patchset. Neither explained.

Best regards,
Krzysztof