[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <51e3683f-be53-4bb7-a994-ffd05744a745@linaro.org>
Date: Fri, 22 Mar 2024 06:53:38 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Lothar Rubusch <l.rubusch@...il.com>, lars@...afoo.de,
Michael.Hennerich@...log.com, jic23@...nel.org, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, eraretuya@...il.com
Subject: Re: [PATCH v2 1/3] iio: accel: adxl345: Update adxl345
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@...il.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
Powered by blists - more mailing lists