(...) > > 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? 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?
