On Sat, 23 Mar 2024 13:16:56 +0100 Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote: > (...) > > > Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx> > > > --- > > > 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 > > > + > (...) > > I think I see your point. My patch has more noise and lacks a logic > structure in proceding. > I will resubmit, but may I ask one question in particular. I moved the > entire list of register > defines from the adxl345_core.c to the common adxl345.h. > For setting spi-3wire with my approach, only two of those defines are > needed. I think it is > nicer for readability to keep the defines together, though, in a > commonly shared header. > Nevertheless most of the defines are just used locally in the .._core.c > Should I move them for refactory? Move them as a block (which you did). It's confusing to have only a subset of defines in one place. > I feel there is no reason to move them. On the other hand I see many > drivers keep them in a common header. Hence, is there a best practice > which justifies moving them to a header?
