| Message ID | 20240322003713.6918-2-l.rubusch@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adxl345: add spi-3wire | expand |
On 22/03/2024 01:37, Lothar Rubusch wrote: > Move driver wide constants and fields into the header. > Let probe call a separate setup function. Provide > possibility for an SPI/I2C specific setup to be passed > as function pointer to core. Subject: you received feedback already of not calling things "update". Everything is update. No, write descriptive text. If you cannot, means you are doing way too many things in one patch. Please read submitting-patches document. > > 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 How is all this related to the patch? I don't understand. Several parts of this patch are not explained / obvious. > + > /* > * 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*)); > > #endif /* _ADXL345_H_ */ > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c > index 8bd30a23e..040c3f05a 100644 > --- a/drivers/iio/accel/adxl345_core.c > +++ b/drivers/iio/accel/adxl345_core.c > @@ -17,38 +17,9 @@ > > #include "adxl345.h" > > -#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_2G 0 > -#define ADXL345_DATA_FORMAT_4G 1 > -#define ADXL345_DATA_FORMAT_8G 2 > -#define ADXL345_DATA_FORMAT_16G 3 Why? ... > > return devm_iio_device_register(dev, indio_dev); > } > diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c > index a3084b0a8..3f882e2e0 100644 > --- a/drivers/iio/accel/adxl345_i2c.c > +++ b/drivers/iio/accel/adxl345_i2c.c > @@ -9,6 +9,7 @@ > */ > > #include <linux/i2c.h> > +#include <linux/mod_devicetable.h> One more... how is this related? > #include <linux/module.h> > #include <linux/regmap.h> > > @@ -21,41 +22,36 @@ static const struct regmap_config adxl345_i2c_regmap_config = { > > static int adxl345_i2c_probe(struct i2c_client *client) > { > + const struct adxl345_chip_info *chip_data; > struct regmap *regmap; > > + /* Retrieve device data, i.e. the name, to pass it to the core */ > + chip_data = i2c_get_match_data(client); > + > regmap = devm_regmap_init_i2c(client, &adxl345_i2c_regmap_config); > if (IS_ERR(regmap)) > - return dev_err_probe(&client->dev, PTR_ERR(regmap), "Error initializing regmap\n"); > + return dev_err_probe(&client->dev, PTR_ERR(regmap), > + "Error initializing regmap\n"); How is this change related to your commit? Stop doing unrelated changes. > > - return adxl345_core_probe(&client->dev, regmap); > + return adxl345_core_probe(&client->dev, regmap, chip_data, NULL); > } > > -static const struct adxl345_chip_info adxl345_i2c_info = { > - .name = "adxl345", > - .uscale = ADXL345_USCALE, > -}; ... > MODULE_DEVICE_TABLE(acpi, adxl345_acpi_match); > diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c > index 93ca349f1..c26bac462 100644 > --- a/drivers/iio/accel/adxl345_spi.c > +++ b/drivers/iio/accel/adxl345_spi.c > @@ -22,8 +22,14 @@ static const struct regmap_config adxl345_spi_regmap_config = { > > static int adxl345_spi_probe(struct spi_device *spi) > { > + const struct adxl345_chip_info *chip_data; > struct regmap *regmap; > > + /* Retrieve device name to pass it as driver specific data */ > + chip_data = device_get_match_data(&spi->dev); > + if (!chip_data) > + chip_data = spi_get_device_match_data(spi); That's not how you use it spi_get_device_match_data(). Open the function and read it... it should be obvious that you now duplicate code. Best regards, Krzysztof
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
On Fri, 2024-03-22 at 00:37 +0000, Lothar Rubusch wrote: > Move driver wide constants and fields into the header. > Let probe call a separate setup function. Provide > possibility for an SPI/I2C specific setup to be passed > as function pointer to core. > > 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*)); > > #endif /* _ADXL345_H_ */ > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c > index 8bd30a23e..040c3f05a 100644 > --- a/drivers/iio/accel/adxl345_core.c > +++ b/drivers/iio/accel/adxl345_core.c > @@ -17,38 +17,9 @@ > > #include "adxl345.h" > > -#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_2G 0 > -#define ADXL345_DATA_FORMAT_4G 1 > -#define ADXL345_DATA_FORMAT_8G 2 > -#define ADXL345_DATA_FORMAT_16G 3 > - > -#define ADXL345_DEVID 0xE5 > - > struct adxl345_data { > const struct adxl345_chip_info *info; > struct regmap *regmap; > - u8 data_range; > }; > > #define ADXL345_CHANNEL(index, axis) { \ > @@ -62,6 +33,18 @@ struct adxl345_data { > BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > } > > +const struct adxl345_chip_info adxl3x5_chip_info[] = { > + [ADXL345] = { > + .name = "adxl345", > + .uscale = ADXL345_USCALE, > + }, > + [ADXL375] = { > + .name = "adxl375", > + .uscale = ADXL375_USCALE, > + }, > +}; > +EXPORT_SYMBOL_NS_GPL(adxl3x5_chip_info, IIO_ADXL345); > + > static const struct iio_chan_spec adxl345_channels[] = { > ADXL345_CHANNEL(0, X), > ADXL345_CHANNEL(1, Y), > @@ -197,14 +180,21 @@ static void adxl345_powerdown(void *regmap) > regmap_write(regmap, ADXL345_REG_POWER_CTL, ADXL345_POWER_CTL_STANDBY); > } > > -int adxl345_core_probe(struct device *dev, struct regmap *regmap) > +static int adxl345_setup(struct device *dev, struct adxl345_data *data, > + int (*setup)(struct device*, struct regmap*)) > { > - struct adxl345_data *data; > - struct iio_dev *indio_dev; > u32 regval; > int ret; > > - ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); > + /* Perform bus specific settings if available */ > + if (setup) { > + ret = setup(dev, data->regmap); > + if (ret) > + return ret; > + } nit: likely a better name would be bus_setup(). Then you could drop the comment as it becomes useless... > + > + /* Read out DEVID */ > + ret = regmap_read(data->regmap, ADXL345_REG_DEVID, ®val); > if (ret < 0) > return dev_err_probe(dev, ret, "Error reading device ID\n"); > > @@ -212,37 +202,62 @@ int adxl345_core_probe(struct device *dev, struct regmap > *regmap) > return dev_err_probe(dev, -ENODEV, "Invalid device ID: %x, > expected %x\n", > regval, ADXL345_DEVID); > > + /* Update data_format to full-resolution mode */ > + ret = regmap_update_bits(data->regmap, ADXL345_REG_DATA_FORMAT, > + ADXL345_DATA_FORMAT_MSK, > ADXL345_DATA_FORMAT_FULL_RES); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to update data_format > register\n"); > + > + /* Enable measurement mode */ > + ret = adxl345_powerup(data->regmap); > + if (ret < 0) > + return dev_err_probe(dev, ret, "Failed to enable measurement > mode\n"); > + > + ret = devm_add_action_or_reset(dev, adxl345_powerdown, data->regmap); > + if (ret < 0) > + return ret; > + > + return 0; > +} > + > +/** > + * adxl345_core_probe() - probe and setup for the adxl345 accelerometer, > + * also covers the adlx375 accelerometer > + * @dev: Driver model representation of the device > + * @regmap: Regmap instance for the device > + * @chip_info: Structure containing device specific data > + * @setup: Setup routine to be executed right before the standard device > + * setup, can also be set to NULL if not required > + * > + * Return: 0 on success, negative errno on error > + */ > +int adxl345_core_probe(struct device *dev, struct regmap *regmap, > + const struct adxl345_chip_info *chip_info, > + int (*setup)(struct device*, struct regmap*)) > +{ > + struct adxl345_data *data; > + struct iio_dev *indio_dev; > + int ret; > + > indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > if (!indio_dev) > return -ENOMEM; > > data = iio_priv(indio_dev); > data->regmap = regmap; > - /* Enable full-resolution mode */ > - data->data_range = ADXL345_DATA_FORMAT_FULL_RES; > - data->info = device_get_match_data(dev); > - if (!data->info) > - return -ENODEV; > - > - ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, > - data->data_range); > - if (ret < 0) > - return dev_err_probe(dev, ret, "Failed to set data range\n"); > + data->info = chip_info; > > - indio_dev->name = data->info->name; > + indio_dev->name = chip_info->name; > indio_dev->info = &adxl345_info; > indio_dev->modes = INDIO_DIRECT_MODE; > indio_dev->channels = adxl345_channels; > indio_dev->num_channels = ARRAY_SIZE(adxl345_channels); > > - /* Enable measurement mode */ > - ret = adxl345_powerup(data->regmap); > - if (ret < 0) > - return dev_err_probe(dev, ret, "Failed to enable measurement > mode\n"); > - > - ret = devm_add_action_or_reset(dev, adxl345_powerdown, data->regmap); > - if (ret < 0) > + ret = adxl345_setup(dev, data, setup); > + if (ret) { > + dev_err(dev, "ADXL345 setup failed\n"); > return ret; > + } > > return devm_iio_device_register(dev, indio_dev); > } > diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c > index a3084b0a8..3f882e2e0 100644 > --- a/drivers/iio/accel/adxl345_i2c.c > +++ b/drivers/iio/accel/adxl345_i2c.c > @@ -9,6 +9,7 @@ > */ > > #include <linux/i2c.h> > +#include <linux/mod_devicetable.h> > #include <linux/module.h> > #include <linux/regmap.h> > > @@ -21,41 +22,36 @@ static const struct regmap_config adxl345_i2c_regmap_config = { > > static int adxl345_i2c_probe(struct i2c_client *client) > { > + const struct adxl345_chip_info *chip_data; > struct regmap *regmap; > > + /* Retrieve device data, i.e. the name, to pass it to the core */ > + chip_data = i2c_get_match_data(client); > + While unlikely use a proper pattern. Meaning, check for NULL pointers and bail out in that case... Also, you need to justify why are you moving these calls to the bus in your commit message. It seems to me that you're trying to refactor too much in a single patch. Maybe step back and try to separate changes in different patches. Like this one (passing chip_info) from the bus file could be in it's own patch. Will also (or should at least :)) "force" you to have a more dedicated commit message explaining why you're introducing the change. - Nuno Sá
On Fri, 2024-03-22 at 06:53 +0100, Krzysztof Kozlowski wrote: > 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. > Yeah, you need to make it explicit in the message that this change is in preparation for a future change (adding the 3-wire spi mode). Otherwise it's natural for reviewers to make questions about it... Maybe another one that could live in it#s own patch. - Nuno Sá >
(...) > > 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 > > + (...) 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?
On Sat, 23 Mar 2024 13:16:56 +0100 Lothar Rubusch <l.rubusch@gmail.com> wrote: > (...) > > > 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 > > > + > (...) > > 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?
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*)); #endif /* _ADXL345_H_ */ diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c index 8bd30a23e..040c3f05a 100644 --- a/drivers/iio/accel/adxl345_core.c +++ b/drivers/iio/accel/adxl345_core.c @@ -17,38 +17,9 @@ #include "adxl345.h" -#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_2G 0 -#define ADXL345_DATA_FORMAT_4G 1 -#define ADXL345_DATA_FORMAT_8G 2 -#define ADXL345_DATA_FORMAT_16G 3 - -#define ADXL345_DEVID 0xE5 - struct adxl345_data { const struct adxl345_chip_info *info; struct regmap *regmap; - u8 data_range; }; #define ADXL345_CHANNEL(index, axis) { \ @@ -62,6 +33,18 @@ struct adxl345_data { BIT(IIO_CHAN_INFO_SAMP_FREQ), \ } +const struct adxl345_chip_info adxl3x5_chip_info[] = { + [ADXL345] = { + .name = "adxl345", + .uscale = ADXL345_USCALE, + }, + [ADXL375] = { + .name = "adxl375", + .uscale = ADXL375_USCALE, + }, +}; +EXPORT_SYMBOL_NS_GPL(adxl3x5_chip_info, IIO_ADXL345); + static const struct iio_chan_spec adxl345_channels[] = { ADXL345_CHANNEL(0, X), ADXL345_CHANNEL(1, Y), @@ -197,14 +180,21 @@ static void adxl345_powerdown(void *regmap) regmap_write(regmap, ADXL345_REG_POWER_CTL, ADXL345_POWER_CTL_STANDBY); } -int adxl345_core_probe(struct device *dev, struct regmap *regmap) +static int adxl345_setup(struct device *dev, struct adxl345_data *data, + int (*setup)(struct device*, struct regmap*)) { - struct adxl345_data *data; - struct iio_dev *indio_dev; u32 regval; int ret; - ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); + /* Perform bus specific settings if available */ + if (setup) { + ret = setup(dev, data->regmap); + if (ret) + return ret; + } + + /* Read out DEVID */ + ret = regmap_read(data->regmap, ADXL345_REG_DEVID, ®val); if (ret < 0) return dev_err_probe(dev, ret, "Error reading device ID\n"); @@ -212,37 +202,62 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap) return dev_err_probe(dev, -ENODEV, "Invalid device ID: %x, expected %x\n", regval, ADXL345_DEVID); + /* Update data_format to full-resolution mode */ + ret = regmap_update_bits(data->regmap, ADXL345_REG_DATA_FORMAT, + ADXL345_DATA_FORMAT_MSK, ADXL345_DATA_FORMAT_FULL_RES); + if (ret) + return dev_err_probe(dev, ret, "Failed to update data_format register\n"); + + /* Enable measurement mode */ + ret = adxl345_powerup(data->regmap); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to enable measurement mode\n"); + + ret = devm_add_action_or_reset(dev, adxl345_powerdown, data->regmap); + if (ret < 0) + return ret; + + return 0; +} + +/** + * adxl345_core_probe() - probe and setup for the adxl345 accelerometer, + * also covers the adlx375 accelerometer + * @dev: Driver model representation of the device + * @regmap: Regmap instance for the device + * @chip_info: Structure containing device specific data + * @setup: Setup routine to be executed right before the standard device + * setup, can also be set to NULL if not required + * + * Return: 0 on success, negative errno on error + */ +int adxl345_core_probe(struct device *dev, struct regmap *regmap, + const struct adxl345_chip_info *chip_info, + int (*setup)(struct device*, struct regmap*)) +{ + struct adxl345_data *data; + struct iio_dev *indio_dev; + int ret; + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); if (!indio_dev) return -ENOMEM; data = iio_priv(indio_dev); data->regmap = regmap; - /* Enable full-resolution mode */ - data->data_range = ADXL345_DATA_FORMAT_FULL_RES; - data->info = device_get_match_data(dev); - if (!data->info) - return -ENODEV; - - ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, - data->data_range); - if (ret < 0) - return dev_err_probe(dev, ret, "Failed to set data range\n"); + data->info = chip_info; - indio_dev->name = data->info->name; + indio_dev->name = chip_info->name; indio_dev->info = &adxl345_info; indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->channels = adxl345_channels; indio_dev->num_channels = ARRAY_SIZE(adxl345_channels); - /* Enable measurement mode */ - ret = adxl345_powerup(data->regmap); - if (ret < 0) - return dev_err_probe(dev, ret, "Failed to enable measurement mode\n"); - - ret = devm_add_action_or_reset(dev, adxl345_powerdown, data->regmap); - if (ret < 0) + ret = adxl345_setup(dev, data, setup); + if (ret) { + dev_err(dev, "ADXL345 setup failed\n"); return ret; + } return devm_iio_device_register(dev, indio_dev); } diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c index a3084b0a8..3f882e2e0 100644 --- a/drivers/iio/accel/adxl345_i2c.c +++ b/drivers/iio/accel/adxl345_i2c.c @@ -9,6 +9,7 @@ */ #include <linux/i2c.h> +#include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/regmap.h> @@ -21,41 +22,36 @@ static const struct regmap_config adxl345_i2c_regmap_config = { static int adxl345_i2c_probe(struct i2c_client *client) { + const struct adxl345_chip_info *chip_data; struct regmap *regmap; + /* Retrieve device data, i.e. the name, to pass it to the core */ + chip_data = i2c_get_match_data(client); + regmap = devm_regmap_init_i2c(client, &adxl345_i2c_regmap_config); if (IS_ERR(regmap)) - return dev_err_probe(&client->dev, PTR_ERR(regmap), "Error initializing regmap\n"); + return dev_err_probe(&client->dev, PTR_ERR(regmap), + "Error initializing regmap\n"); - return adxl345_core_probe(&client->dev, regmap); + return adxl345_core_probe(&client->dev, regmap, chip_data, NULL); } -static const struct adxl345_chip_info adxl345_i2c_info = { - .name = "adxl345", - .uscale = ADXL345_USCALE, -}; - -static const struct adxl345_chip_info adxl375_i2c_info = { - .name = "adxl375", - .uscale = ADXL375_USCALE, -}; - static const struct i2c_device_id adxl345_i2c_id[] = { - { "adxl345", (kernel_ulong_t)&adxl345_i2c_info }, - { "adxl375", (kernel_ulong_t)&adxl375_i2c_info }, + { "adxl345", (kernel_ulong_t)&adxl3x5_chip_info[ADXL345] }, + { "adxl375", (kernel_ulong_t)&adxl3x5_chip_info[ADXL375] }, { } }; MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); static const struct of_device_id adxl345_of_match[] = { - { .compatible = "adi,adxl345", .data = &adxl345_i2c_info }, - { .compatible = "adi,adxl375", .data = &adxl375_i2c_info }, + { .compatible = "adi,adxl345", .data = &adxl3x5_chip_info[ADXL345] }, + { .compatible = "adi,adxl375", .data = &adxl3x5_chip_info[ADXL375] }, { } }; MODULE_DEVICE_TABLE(of, adxl345_of_match); static const struct acpi_device_id adxl345_acpi_match[] = { - { "ADS0345", (kernel_ulong_t)&adxl345_i2c_info }, + { "ADS0345", (kernel_ulong_t)&adxl3x5_chip_info[ADXL345] }, { } }; MODULE_DEVICE_TABLE(acpi, adxl345_acpi_match); diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c index 93ca349f1..c26bac462 100644 --- a/drivers/iio/accel/adxl345_spi.c +++ b/drivers/iio/accel/adxl345_spi.c @@ -22,8 +22,14 @@ static const struct regmap_config adxl345_spi_regmap_config = { static int adxl345_spi_probe(struct spi_device *spi) { + const struct adxl345_chip_info *chip_data; struct regmap *regmap; + /* Retrieve device name to pass it as driver specific data */ + chip_data = device_get_match_data(&spi->dev); + if (!chip_data) + chip_data = spi_get_device_match_data(spi); + /* Bail out if max_speed_hz exceeds 5 MHz */ if (spi->max_speed_hz > ADXL345_MAX_SPI_FREQ_HZ) return dev_err_probe(&spi->dev, -EINVAL, "SPI CLK, %d Hz exceeds 5 MHz\n", @@ -33,35 +39,25 @@ static int adxl345_spi_probe(struct spi_device *spi) if (IS_ERR(regmap)) return dev_err_probe(&spi->dev, PTR_ERR(regmap), "Error initializing regmap\n"); - return adxl345_core_probe(&spi->dev, regmap); + return adxl345_core_probe(&spi->dev, regmap, chip_data, NULL); } -static const struct adxl345_chip_info adxl345_spi_info = { - .name = "adxl345", - .uscale = ADXL345_USCALE, -}; - -static const struct adxl345_chip_info adxl375_spi_info = { - .name = "adxl375", - .uscale = ADXL375_USCALE, -}; - static const struct spi_device_id adxl345_spi_id[] = { - { "adxl345", (kernel_ulong_t)&adxl345_spi_info }, - { "adxl375", (kernel_ulong_t)&adxl375_spi_info }, + { "adxl345", (kernel_ulong_t)&adxl3x5_chip_info[ADXL345] }, + { "adxl375", (kernel_ulong_t)&adxl3x5_chip_info[ADXL375] }, { } }; MODULE_DEVICE_TABLE(spi, adxl345_spi_id); static const struct of_device_id adxl345_of_match[] = { - { .compatible = "adi,adxl345", .data = &adxl345_spi_info }, - { .compatible = "adi,adxl375", .data = &adxl375_spi_info }, + { .compatible = "adi,adxl345", .data = &adxl3x5_chip_info[ADXL345] }, + { .compatible = "adi,adxl375", .data = &adxl3x5_chip_info[ADXL375] }, { } }; MODULE_DEVICE_TABLE(of, adxl345_of_match); static const struct acpi_device_id adxl345_acpi_match[] = { - { "ADS0345", (kernel_ulong_t)&adxl345_spi_info }, + { "ADS0345", (kernel_ulong_t)&adxl3x5_chip_info[ADXL345] }, { } }; MODULE_DEVICE_TABLE(acpi, adxl345_acpi_match);
Move driver wide constants and fields into the header. Let probe call a separate setup function. Provide possibility for an SPI/I2C specific setup to be passed as function pointer to core. 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(-)