* [PATCH 0/4] iio: accel: adxl345: Split driver into core and I2C then add SPI support
@ 2017-02-16 10:02 Eva Rachel Retuya
2017-02-16 10:02 ` [PATCH 1/4] iio: accel: adxl345: Use I2C regmap instead of direct I2C access Eva Rachel Retuya
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Eva Rachel Retuya @ 2017-02-16 10:02 UTC (permalink / raw)
To: jic23, linux-iio
Cc: knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich,
daniel.baluta, amsfield22, florian.vaussard, linux-kernel,
Eva Rachel Retuya
This patchset modifies the adxl345 to use regmap. In doing so, we can
easily introduce SPI support and let regmap handle the rest.
Recap of basic features: read_raw for x, y and z axes, scale. After
applying this series, driver now supports the SPI protocol and enumeration
of device via ACPI.
Eva Rachel Retuya (4):
iio: accel: adxl345: Use I2C regmap instead of direct I2C access
iio: accel: adxl345: Split driver into core and I2C
iio: accel: adxl345: Add SPI support
iio: accel: adxl345: Add ACPI support
drivers/iio/accel/Kconfig | 22 ++++-
drivers/iio/accel/Makefile | 4 +-
drivers/iio/accel/adxl345.c | 194 ---------------------------------------
drivers/iio/accel/adxl345.h | 18 ++++
drivers/iio/accel/adxl345_core.c | 182 ++++++++++++++++++++++++++++++++++++
drivers/iio/accel/adxl345_i2c.c | 79 ++++++++++++++++
drivers/iio/accel/adxl345_spi.c | 84 +++++++++++++++++
7 files changed, 386 insertions(+), 197 deletions(-)
delete mode 100644 drivers/iio/accel/adxl345.c
create mode 100644 drivers/iio/accel/adxl345.h
create mode 100644 drivers/iio/accel/adxl345_core.c
create mode 100644 drivers/iio/accel/adxl345_i2c.c
create mode 100644 drivers/iio/accel/adxl345_spi.c
--
2.7.4
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/4] iio: accel: adxl345: Use I2C regmap instead of direct I2C access 2017-02-16 10:02 [PATCH 0/4] iio: accel: adxl345: Split driver into core and I2C then add SPI support Eva Rachel Retuya @ 2017-02-16 10:02 ` Eva Rachel Retuya 2017-02-19 13:04 ` Jonathan Cameron 2017-02-16 10:02 ` [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C Eva Rachel Retuya ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Eva Rachel Retuya @ 2017-02-16 10:02 UTC (permalink / raw) To: jic23, linux-iio Cc: knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel, Eva Rachel Retuya Convert the driver to use regmap instead of I2C-specific functions. This is done in preparation for splitting this driver into core and I2C-specific code as well as introduction of SPI driver. Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> --- drivers/iio/accel/Kconfig | 1 + drivers/iio/accel/adxl345.c | 43 +++++++++++++++++++++++++++++++------------ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig index 2308bac..26b8614 100644 --- a/drivers/iio/accel/Kconfig +++ b/drivers/iio/accel/Kconfig @@ -9,6 +9,7 @@ config ADXL345 tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver" depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) depends on I2C + select REGMAP_I2C help Say Y here if you want to build support for the Analog Devices ADXL345 3-axis digital accelerometer. diff --git a/drivers/iio/accel/adxl345.c b/drivers/iio/accel/adxl345.c index c34991f..a1fdf60 100644 --- a/drivers/iio/accel/adxl345.c +++ b/drivers/iio/accel/adxl345.c @@ -14,6 +14,7 @@ #include <linux/i2c.h> #include <linux/module.h> +#include <linux/regmap.h> #include <linux/iio/iio.h> @@ -46,9 +47,15 @@ static const int adxl345_uscale = 38300; struct adxl345_data { struct i2c_client *client; + struct regmap *regmap; u8 data_range; }; +static const struct regmap_config adxl345_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + #define ADXL345_CHANNEL(reg, axis) { \ .type = IIO_ACCEL, \ .modified = 1, \ @@ -70,6 +77,7 @@ static int adxl345_read_raw(struct iio_dev *indio_dev, { struct adxl345_data *data = iio_priv(indio_dev); int ret; + __le16 regval; switch (mask) { case IIO_CHAN_INFO_RAW: @@ -78,11 +86,12 @@ static int adxl345_read_raw(struct iio_dev *indio_dev, * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte */ - ret = i2c_smbus_read_word_data(data->client, chan->address); + ret = regmap_bulk_read(data->regmap, chan->address, ®val, + sizeof(regval)); if (ret < 0) return ret; - *val = sign_extend32(ret, 12); + *val = sign_extend32(le16_to_cpu(regval), 12); return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: *val = 0; @@ -104,15 +113,24 @@ static int adxl345_probe(struct i2c_client *client, { struct adxl345_data *data; struct iio_dev *indio_dev; + struct regmap *regmap; int ret; + u32 regval; + + regmap = devm_regmap_init_i2c(client, &adxl345_regmap_config); + if (IS_ERR(regmap)) { + dev_err(&client->dev, "Error initializing regmap: %d\n", + (int)PTR_ERR(regmap)); + return PTR_ERR(regmap); + } - ret = i2c_smbus_read_byte_data(client, ADXL345_REG_DEVID); + ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); if (ret < 0) { dev_err(&client->dev, "Error reading device ID: %d\n", ret); return ret; } - if (ret != ADXL345_DEVID) { + if (regval != ADXL345_DEVID) { dev_err(&client->dev, "Invalid device ID: %d\n", ret); return -ENODEV; } @@ -124,11 +142,12 @@ static int adxl345_probe(struct i2c_client *client, data = iio_priv(indio_dev); i2c_set_clientdata(client, indio_dev); data->client = client; + data->regmap = regmap; /* Enable full-resolution mode */ data->data_range = ADXL345_DATA_FORMAT_FULL_RES; - ret = i2c_smbus_write_byte_data(data->client, ADXL345_REG_DATA_FORMAT, - data->data_range); + ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, + data->data_range); if (ret < 0) { dev_err(&client->dev, "Failed to set data range: %d\n", ret); return ret; @@ -142,8 +161,8 @@ static int adxl345_probe(struct i2c_client *client, indio_dev->num_channels = ARRAY_SIZE(adxl345_channels); /* Enable measurement mode */ - ret = i2c_smbus_write_byte_data(data->client, ADXL345_REG_POWER_CTL, - ADXL345_POWER_CTL_MEASURE); + ret = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, + ADXL345_POWER_CTL_MEASURE); if (ret < 0) { dev_err(&client->dev, "Failed to enable measurement mode: %d\n", ret); @@ -153,8 +172,8 @@ static int adxl345_probe(struct i2c_client *client, ret = iio_device_register(indio_dev); if (ret < 0) { dev_err(&client->dev, "iio_device_register failed: %d\n", ret); - i2c_smbus_write_byte_data(data->client, ADXL345_REG_POWER_CTL, - ADXL345_POWER_CTL_STANDBY); + regmap_write(data->regmap, ADXL345_REG_POWER_CTL, + ADXL345_POWER_CTL_STANDBY); } return ret; @@ -167,8 +186,8 @@ static int adxl345_remove(struct i2c_client *client) iio_device_unregister(indio_dev); - return i2c_smbus_write_byte_data(data->client, ADXL345_REG_POWER_CTL, - ADXL345_POWER_CTL_STANDBY); + return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, + ADXL345_POWER_CTL_STANDBY); } static const struct i2c_device_id adxl345_i2c_id[] = { -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] iio: accel: adxl345: Use I2C regmap instead of direct I2C access 2017-02-16 10:02 ` [PATCH 1/4] iio: accel: adxl345: Use I2C regmap instead of direct I2C access Eva Rachel Retuya @ 2017-02-19 13:04 ` Jonathan Cameron 0 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2017-02-19 13:04 UTC (permalink / raw) To: Eva Rachel Retuya, linux-iio Cc: knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On 16/02/17 10:02, Eva Rachel Retuya wrote: > Convert the driver to use regmap instead of I2C-specific functions. This > is done in preparation for splitting this driver into core and > I2C-specific code as well as introduction of SPI driver. > > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> One minor point inline. Jonathan > --- > drivers/iio/accel/Kconfig | 1 + > drivers/iio/accel/adxl345.c | 43 +++++++++++++++++++++++++++++++------------ > 2 files changed, 32 insertions(+), 12 deletions(-) > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > index 2308bac..26b8614 100644 > --- a/drivers/iio/accel/Kconfig > +++ b/drivers/iio/accel/Kconfig > @@ -9,6 +9,7 @@ config ADXL345 > tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver" > depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > depends on I2C > + select REGMAP_I2C > help > Say Y here if you want to build support for the Analog Devices > ADXL345 3-axis digital accelerometer. > diff --git a/drivers/iio/accel/adxl345.c b/drivers/iio/accel/adxl345.c > index c34991f..a1fdf60 100644 > --- a/drivers/iio/accel/adxl345.c > +++ b/drivers/iio/accel/adxl345.c > @@ -14,6 +14,7 @@ > > #include <linux/i2c.h> > #include <linux/module.h> > +#include <linux/regmap.h> > > #include <linux/iio/iio.h> > > @@ -46,9 +47,15 @@ static const int adxl345_uscale = 38300; > > struct adxl345_data { > struct i2c_client *client; > + struct regmap *regmap; > u8 data_range; > }; > > +static const struct regmap_config adxl345_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > +}; > + > #define ADXL345_CHANNEL(reg, axis) { \ > .type = IIO_ACCEL, \ > .modified = 1, \ > @@ -70,6 +77,7 @@ static int adxl345_read_raw(struct iio_dev *indio_dev, > { > struct adxl345_data *data = iio_priv(indio_dev); > int ret; > + __le16 regval; > > switch (mask) { > case IIO_CHAN_INFO_RAW: > @@ -78,11 +86,12 @@ static int adxl345_read_raw(struct iio_dev *indio_dev, > * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte > * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte > */ > - ret = i2c_smbus_read_word_data(data->client, chan->address); > + ret = regmap_bulk_read(data->regmap, chan->address, ®val, > + sizeof(regval)); > if (ret < 0) > return ret; > > - *val = sign_extend32(ret, 12); > + *val = sign_extend32(le16_to_cpu(regval), 12); > return IIO_VAL_INT; > case IIO_CHAN_INFO_SCALE: > *val = 0; > @@ -104,15 +113,24 @@ static int adxl345_probe(struct i2c_client *client, > { > struct adxl345_data *data; > struct iio_dev *indio_dev; > + struct regmap *regmap; > int ret; > + u32 regval; > + > + regmap = devm_regmap_init_i2c(client, &adxl345_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(&client->dev, "Error initializing regmap: %d\n", > + (int)PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } > > - ret = i2c_smbus_read_byte_data(client, ADXL345_REG_DEVID); > + ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); > if (ret < 0) { > dev_err(&client->dev, "Error reading device ID: %d\n", ret); > return ret; > } > > - if (ret != ADXL345_DEVID) { > + if (regval != ADXL345_DEVID) { > dev_err(&client->dev, "Invalid device ID: %d\n", ret); > return -ENODEV; > } > @@ -124,11 +142,12 @@ static int adxl345_probe(struct i2c_client *client, > data = iio_priv(indio_dev); > i2c_set_clientdata(client, indio_dev); > data->client = client; Leaving client behind means that you have a driver that is still implicitly associated with i2c in more places than it should be. I'd normally expect to either see all the access to the underlying dev done through a local copy of the client's device pointer, or through the device pointer that can be retrieved form the regmap. > + data->regmap = regmap; > /* Enable full-resolution mode */ > data->data_range = ADXL345_DATA_FORMAT_FULL_RES; > > - ret = i2c_smbus_write_byte_data(data->client, ADXL345_REG_DATA_FORMAT, > - data->data_range); > + ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, > + data->data_range); > if (ret < 0) { > dev_err(&client->dev, "Failed to set data range: %d\n", ret); > return ret; > @@ -142,8 +161,8 @@ static int adxl345_probe(struct i2c_client *client, > indio_dev->num_channels = ARRAY_SIZE(adxl345_channels); > > /* Enable measurement mode */ > - ret = i2c_smbus_write_byte_data(data->client, ADXL345_REG_POWER_CTL, > - ADXL345_POWER_CTL_MEASURE); > + ret = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > + ADXL345_POWER_CTL_MEASURE); > if (ret < 0) { > dev_err(&client->dev, "Failed to enable measurement mode: %d\n", > ret); > @@ -153,8 +172,8 @@ static int adxl345_probe(struct i2c_client *client, > ret = iio_device_register(indio_dev); > if (ret < 0) { > dev_err(&client->dev, "iio_device_register failed: %d\n", ret); > - i2c_smbus_write_byte_data(data->client, ADXL345_REG_POWER_CTL, > - ADXL345_POWER_CTL_STANDBY); > + regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > + ADXL345_POWER_CTL_STANDBY); > } > > return ret; > @@ -167,8 +186,8 @@ static int adxl345_remove(struct i2c_client *client) > > iio_device_unregister(indio_dev); > > - return i2c_smbus_write_byte_data(data->client, ADXL345_REG_POWER_CTL, > - ADXL345_POWER_CTL_STANDBY); > + return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > + ADXL345_POWER_CTL_STANDBY); > } > > static const struct i2c_device_id adxl345_i2c_id[] = { > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C 2017-02-16 10:02 [PATCH 0/4] iio: accel: adxl345: Split driver into core and I2C then add SPI support Eva Rachel Retuya 2017-02-16 10:02 ` [PATCH 1/4] iio: accel: adxl345: Use I2C regmap instead of direct I2C access Eva Rachel Retuya @ 2017-02-16 10:02 ` Eva Rachel Retuya 2017-02-19 13:11 ` Jonathan Cameron 2017-02-16 10:02 ` [PATCH 3/4] iio: accel: adxl345: Add SPI support Eva Rachel Retuya 2017-02-16 10:02 ` [PATCH 4/4] iio: accel: adxl345: Add ACPI support Eva Rachel Retuya 3 siblings, 1 reply; 16+ messages in thread From: Eva Rachel Retuya @ 2017-02-16 10:02 UTC (permalink / raw) To: jic23, linux-iio Cc: knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel, Eva Rachel Retuya Move I2C-specific code into its own file and rely on regmap to access registers. The core code provides access to x, y, z and scale readings. Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> --- drivers/iio/accel/Kconfig | 8 +- drivers/iio/accel/Makefile | 3 +- drivers/iio/accel/adxl345.c | 213 --------------------------------------- drivers/iio/accel/adxl345.h | 18 ++++ drivers/iio/accel/adxl345_core.c | 182 +++++++++++++++++++++++++++++++++ drivers/iio/accel/adxl345_i2c.c | 70 +++++++++++++ 6 files changed, 278 insertions(+), 216 deletions(-) delete mode 100644 drivers/iio/accel/adxl345.c create mode 100644 drivers/iio/accel/adxl345.h create mode 100644 drivers/iio/accel/adxl345_core.c create mode 100644 drivers/iio/accel/adxl345_i2c.c diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig index 26b8614..5bdd87f 100644 --- a/drivers/iio/accel/Kconfig +++ b/drivers/iio/accel/Kconfig @@ -6,16 +6,20 @@ menu "Accelerometers" config ADXL345 - tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver" + tristate + +config ADXL345_I2C + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver" depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) depends on I2C + select ADXL345 select REGMAP_I2C help Say Y here if you want to build support for the Analog Devices ADXL345 3-axis digital accelerometer. To compile this driver as a module, choose M here: the - module will be called adxl345. + module will be called adxl345_i2c. config BMA180 tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile index 618488d..3f4a6d6 100644 --- a/drivers/iio/accel/Makefile +++ b/drivers/iio/accel/Makefile @@ -3,7 +3,8 @@ # # When adding new entries keep the list in alphabetical order -obj-$(CONFIG_ADXL345) += adxl345.o +obj-$(CONFIG_ADXL345) += adxl345_core.o +obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o obj-$(CONFIG_BMA180) += bma180.o obj-$(CONFIG_BMA220) += bma220_spi.o obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o diff --git a/drivers/iio/accel/adxl345.c b/drivers/iio/accel/adxl345.c deleted file mode 100644 index a1fdf60..0000000 --- a/drivers/iio/accel/adxl345.c +++ /dev/null @@ -1,213 +0,0 @@ -/* - * ADXL345 3-Axis Digital Accelerometer - * - * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> - * - * This file is subject to the terms and conditions of version 2 of - * the GNU General Public License. See the file COPYING in the main - * directory of this archive for more details. - * - * IIO driver for ADXL345 - * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or - * 0x53 (ALT ADDRESS pin grounded) - */ - -#include <linux/i2c.h> -#include <linux/module.h> -#include <linux/regmap.h> - -#include <linux/iio/iio.h> - -#define ADXL345_REG_DEVID 0x00 -#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_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 - -/* - * In full-resolution mode, scale factor is maintained at ~4 mg/LSB - * in all g ranges. - * - * At +/- 16g with 13-bit resolution, scale is computed as: - * (16 + 16) * 9.81 / (2^13 - 1) = 0.0383 - */ -static const int adxl345_uscale = 38300; - -struct adxl345_data { - struct i2c_client *client; - struct regmap *regmap; - u8 data_range; -}; - -static const struct regmap_config adxl345_regmap_config = { - .reg_bits = 8, - .val_bits = 8, -}; - -#define ADXL345_CHANNEL(reg, axis) { \ - .type = IIO_ACCEL, \ - .modified = 1, \ - .channel2 = IIO_MOD_##axis, \ - .address = reg, \ - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ -} - -static const struct iio_chan_spec adxl345_channels[] = { - ADXL345_CHANNEL(ADXL345_REG_DATAX0, X), - ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y), - ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z), -}; - -static int adxl345_read_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int *val, int *val2, long mask) -{ - struct adxl345_data *data = iio_priv(indio_dev); - int ret; - __le16 regval; - - switch (mask) { - case IIO_CHAN_INFO_RAW: - /* - * Data is stored in adjacent registers: - * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte - * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte - */ - ret = regmap_bulk_read(data->regmap, chan->address, ®val, - sizeof(regval)); - if (ret < 0) - return ret; - - *val = sign_extend32(le16_to_cpu(regval), 12); - return IIO_VAL_INT; - case IIO_CHAN_INFO_SCALE: - *val = 0; - *val2 = adxl345_uscale; - - return IIO_VAL_INT_PLUS_MICRO; - } - - return -EINVAL; -} - -static const struct iio_info adxl345_info = { - .driver_module = THIS_MODULE, - .read_raw = adxl345_read_raw, -}; - -static int adxl345_probe(struct i2c_client *client, - const struct i2c_device_id *id) -{ - struct adxl345_data *data; - struct iio_dev *indio_dev; - struct regmap *regmap; - int ret; - u32 regval; - - regmap = devm_regmap_init_i2c(client, &adxl345_regmap_config); - if (IS_ERR(regmap)) { - dev_err(&client->dev, "Error initializing regmap: %d\n", - (int)PTR_ERR(regmap)); - return PTR_ERR(regmap); - } - - ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); - if (ret < 0) { - dev_err(&client->dev, "Error reading device ID: %d\n", ret); - return ret; - } - - if (regval != ADXL345_DEVID) { - dev_err(&client->dev, "Invalid device ID: %d\n", ret); - return -ENODEV; - } - - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); - if (!indio_dev) - return -ENOMEM; - - data = iio_priv(indio_dev); - i2c_set_clientdata(client, indio_dev); - data->client = client; - data->regmap = regmap; - /* Enable full-resolution mode */ - data->data_range = ADXL345_DATA_FORMAT_FULL_RES; - - ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, - data->data_range); - if (ret < 0) { - dev_err(&client->dev, "Failed to set data range: %d\n", ret); - return ret; - } - - indio_dev->dev.parent = &client->dev; - indio_dev->name = id->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 = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, - ADXL345_POWER_CTL_MEASURE); - if (ret < 0) { - dev_err(&client->dev, "Failed to enable measurement mode: %d\n", - ret); - return ret; - } - - ret = iio_device_register(indio_dev); - if (ret < 0) { - dev_err(&client->dev, "iio_device_register failed: %d\n", ret); - regmap_write(data->regmap, ADXL345_REG_POWER_CTL, - ADXL345_POWER_CTL_STANDBY); - } - - return ret; -} - -static int adxl345_remove(struct i2c_client *client) -{ - struct iio_dev *indio_dev = i2c_get_clientdata(client); - struct adxl345_data *data = iio_priv(indio_dev); - - iio_device_unregister(indio_dev); - - return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, - ADXL345_POWER_CTL_STANDBY); -} - -static const struct i2c_device_id adxl345_i2c_id[] = { - { "adxl345", 0 }, - { } -}; - -MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); - -static struct i2c_driver adxl345_driver = { - .driver = { - .name = "adxl345", - }, - .probe = adxl345_probe, - .remove = adxl345_remove, - .id_table = adxl345_i2c_id, -}; - -module_i2c_driver(adxl345_driver); - -MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); -MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer driver"); -MODULE_LICENSE("GPL v2"); diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h new file mode 100644 index 0000000..fca3e25 --- /dev/null +++ b/drivers/iio/accel/adxl345.h @@ -0,0 +1,18 @@ +/* + * ADXL345 3-Axis Digital Accelerometer + * + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> + * + * This file is subject to the terms and conditions of version 2 of + * the GNU General Public License. See the file COPYING in the main + * directory of this archive for more details. + */ + +#ifndef _ADXL345_H_ +#define _ADXL345_H_ + +int adxl345_common_probe(struct device *dev, struct regmap *regmap, + const char *name); +int adxl345_common_remove(struct device *dev); + +#endif /* _ADXL345_H_ */ diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c new file mode 100644 index 0000000..ce09804 --- /dev/null +++ b/drivers/iio/accel/adxl345_core.c @@ -0,0 +1,182 @@ +/* + * ADXL345 3-Axis Digital Accelerometer + * + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> + * + * This file is subject to the terms and conditions of version 2 of + * the GNU General Public License. See the file COPYING in the main + * directory of this archive for more details. + * + * IIO core driver for ADXL345 + */ + +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/regmap.h> + +#include <linux/iio/iio.h> + +#include "adxl345.h" + +#define ADXL345_REG_DEVID 0x00 +#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_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 + +/* + * In full-resolution mode, scale factor is maintained at ~4 mg/LSB + * in all g ranges. + * + * At +/- 16g with 13-bit resolution, scale is computed as: + * (16 + 16) * 9.81 / (2^13 - 1) = 0.0383 + */ +static const int adxl345_uscale = 38300; + +struct adxl345_data { + struct regmap *regmap; + u8 data_range; +}; + +#define ADXL345_CHANNEL(reg, axis) { \ + .type = IIO_ACCEL, \ + .modified = 1, \ + .channel2 = IIO_MOD_##axis, \ + .address = reg, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ +} + +static const struct iio_chan_spec adxl345_channels[] = { + ADXL345_CHANNEL(ADXL345_REG_DATAX0, X), + ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y), + ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z), +}; + +static int adxl345_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct adxl345_data *data = iio_priv(indio_dev); + int ret; + __le16 regval; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + /* + * Data is stored in adjacent registers: + * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte + * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte + */ + ret = regmap_bulk_read(data->regmap, chan->address, ®val, + sizeof(regval)); + if (ret < 0) + return ret; + + *val = sign_extend32(le16_to_cpu(regval), 12); + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + *val = 0; + *val2 = adxl345_uscale; + + return IIO_VAL_INT_PLUS_MICRO; + } + + return -EINVAL; +} + +static const struct iio_info adxl345_info = { + .driver_module = THIS_MODULE, + .read_raw = adxl345_read_raw, +}; + +int adxl345_common_probe(struct device *dev, struct regmap *regmap, + const char *name) +{ + struct adxl345_data *data; + struct iio_dev *indio_dev; + int ret; + u32 regval; + + ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); + if (ret < 0) { + dev_err(dev, "Error reading device ID: %d\n", ret); + return ret; + } + + if (regval != ADXL345_DEVID) { + dev_err(dev, "Invalid device ID: %d\n", ret); + return -ENODEV; + } + + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + dev_set_drvdata(dev, indio_dev); + data->regmap = regmap; + /* Enable full-resolution mode */ + data->data_range = ADXL345_DATA_FORMAT_FULL_RES; + + ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, + data->data_range); + if (ret < 0) { + dev_err(dev, "Failed to set data range: %d\n", ret); + return ret; + } + + indio_dev->dev.parent = dev; + indio_dev->name = 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 = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, + ADXL345_POWER_CTL_MEASURE); + if (ret < 0) { + dev_err(dev, "Failed to enable measurement mode: %d\n", + ret); + return ret; + } + + ret = iio_device_register(indio_dev); + if (ret < 0) { + dev_err(dev, "iio_device_register failed: %d\n", ret); + regmap_write(data->regmap, ADXL345_REG_POWER_CTL, + ADXL345_POWER_CTL_STANDBY); + } + + return ret; +} +EXPORT_SYMBOL_GPL(adxl345_common_probe); + +int adxl345_common_remove(struct device *dev) +{ + struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct adxl345_data *data = iio_priv(indio_dev); + + iio_device_unregister(indio_dev); + + return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, + ADXL345_POWER_CTL_STANDBY); +} +EXPORT_SYMBOL_GPL(adxl345_common_remove); + +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer core driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c new file mode 100644 index 0000000..b114eb0 --- /dev/null +++ b/drivers/iio/accel/adxl345_i2c.c @@ -0,0 +1,70 @@ +/* + * ADXL345 3-Axis Digital Accelerometer + * + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> + * + * This file is subject to the terms and conditions of version 2 of + * the GNU General Public License. See the file COPYING in the main + * directory of this archive for more details. + * + * I2C driver for ADXL345 + * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or + * 0x53 (ALT ADDRESS pin grounded) + */ + +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/regmap.h> + +#include "adxl345.h" + +static const struct regmap_config adxl345_i2c_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + +static int adxl345_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct regmap *regmap; + const char *name = NULL; + + regmap = devm_regmap_init_i2c(client, &adxl345_i2c_regmap_config); + if (IS_ERR(regmap)) { + dev_err(&client->dev, "Error initializing i2c regmap: %d\n", + (int)PTR_ERR(regmap)); + return PTR_ERR(regmap); + } + + if (id) + name = id->name; + + return adxl345_common_probe(&client->dev, regmap, name); +} + +static int adxl345_i2c_remove(struct i2c_client *client) +{ + return adxl345_common_remove(&client->dev); +} + +static const struct i2c_device_id adxl345_i2c_id[] = { + { "adxl345", 0 }, + { } +}; + +MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); + +static struct i2c_driver adxl345_i2c_driver = { + .driver = { + .name = "adxl345_i2c", + }, + .probe = adxl345_i2c_probe, + .remove = adxl345_i2c_remove, + .id_table = adxl345_i2c_id, +}; + +module_i2c_driver(adxl345_i2c_driver); + +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer I2C driver"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C 2017-02-16 10:02 ` [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C Eva Rachel Retuya @ 2017-02-19 13:11 ` Jonathan Cameron 2017-02-20 6:41 ` Eva Rachel Retuya 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2017-02-19 13:11 UTC (permalink / raw) To: Eva Rachel Retuya, linux-iio Cc: knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On 16/02/17 10:02, Eva Rachel Retuya wrote: > Move I2C-specific code into its own file and rely on regmap to access > registers. The core code provides access to x, y, z and scale readings. > > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> A few minor comment inline. This unwound the comment about client->dev in the previous patch, but I'd prefer to see it done there as then the changes in here will be easier to see (with move detection hopefully working!) Jonathan > --- > drivers/iio/accel/Kconfig | 8 +- > drivers/iio/accel/Makefile | 3 +- > drivers/iio/accel/adxl345.c | 213 --------------------------------------- > drivers/iio/accel/adxl345.h | 18 ++++ > drivers/iio/accel/adxl345_core.c | 182 +++++++++++++++++++++++++++++++++ > drivers/iio/accel/adxl345_i2c.c | 70 +++++++++++++ This is a case where allowing git to notice the moves would have lead to easier to read patches (unless this was sufficiently complex that git couldn't follow it ;) > 6 files changed, 278 insertions(+), 216 deletions(-) > delete mode 100644 drivers/iio/accel/adxl345.c > create mode 100644 drivers/iio/accel/adxl345.h > create mode 100644 drivers/iio/accel/adxl345_core.c > create mode 100644 drivers/iio/accel/adxl345_i2c.c > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > index 26b8614..5bdd87f 100644 > --- a/drivers/iio/accel/Kconfig > +++ b/drivers/iio/accel/Kconfig > @@ -6,16 +6,20 @@ > menu "Accelerometers" > > config ADXL345 > - tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver" > + tristate > + > +config ADXL345_I2C > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver" > depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > depends on I2C > + select ADXL345 > select REGMAP_I2C > help > Say Y here if you want to build support for the Analog Devices > ADXL345 3-axis digital accelerometer. > > To compile this driver as a module, choose M here: the > - module will be called adxl345. > + module will be called adxl345_i2c. There are a couple of ways to do this. I personally kind of prefer the way the bmc150 does it as it gives a cleaner set of options in Kconfig. Look at the various ways it is done and if you still prefer this one then fair enough (it's how we always used to do it ;) > > config BMA180 > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > index 618488d..3f4a6d6 100644 > --- a/drivers/iio/accel/Makefile > +++ b/drivers/iio/accel/Makefile > @@ -3,7 +3,8 @@ > # > > # When adding new entries keep the list in alphabetical order > -obj-$(CONFIG_ADXL345) += adxl345.o > +obj-$(CONFIG_ADXL345) += adxl345_core.o > +obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > obj-$(CONFIG_BMA180) += bma180.o > obj-$(CONFIG_BMA220) += bma220_spi.o > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > diff --git a/drivers/iio/accel/adxl345.c b/drivers/iio/accel/adxl345.c > deleted file mode 100644 > index a1fdf60..0000000 > --- a/drivers/iio/accel/adxl345.c > +++ /dev/null > @@ -1,213 +0,0 @@ > -/* > - * ADXL345 3-Axis Digital Accelerometer > - * > - * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > - * > - * This file is subject to the terms and conditions of version 2 of > - * the GNU General Public License. See the file COPYING in the main > - * directory of this archive for more details. > - * > - * IIO driver for ADXL345 > - * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or > - * 0x53 (ALT ADDRESS pin grounded) > - */ > - > -#include <linux/i2c.h> > -#include <linux/module.h> > -#include <linux/regmap.h> > - > -#include <linux/iio/iio.h> > - > -#define ADXL345_REG_DEVID 0x00 > -#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_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 > - > -/* > - * In full-resolution mode, scale factor is maintained at ~4 mg/LSB > - * in all g ranges. > - * > - * At +/- 16g with 13-bit resolution, scale is computed as: > - * (16 + 16) * 9.81 / (2^13 - 1) = 0.0383 > - */ > -static const int adxl345_uscale = 38300; > - > -struct adxl345_data { > - struct i2c_client *client; > - struct regmap *regmap; > - u8 data_range; > -}; > - > -static const struct regmap_config adxl345_regmap_config = { > - .reg_bits = 8, > - .val_bits = 8, > -}; > - > -#define ADXL345_CHANNEL(reg, axis) { \ > - .type = IIO_ACCEL, \ > - .modified = 1, \ > - .channel2 = IIO_MOD_##axis, \ > - .address = reg, \ > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > -} > - > -static const struct iio_chan_spec adxl345_channels[] = { > - ADXL345_CHANNEL(ADXL345_REG_DATAX0, X), > - ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y), > - ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z), > -}; > - > -static int adxl345_read_raw(struct iio_dev *indio_dev, > - struct iio_chan_spec const *chan, > - int *val, int *val2, long mask) > -{ > - struct adxl345_data *data = iio_priv(indio_dev); > - int ret; > - __le16 regval; > - > - switch (mask) { > - case IIO_CHAN_INFO_RAW: > - /* > - * Data is stored in adjacent registers: > - * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte > - * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte > - */ > - ret = regmap_bulk_read(data->regmap, chan->address, ®val, > - sizeof(regval)); > - if (ret < 0) > - return ret; > - > - *val = sign_extend32(le16_to_cpu(regval), 12); > - return IIO_VAL_INT; > - case IIO_CHAN_INFO_SCALE: > - *val = 0; > - *val2 = adxl345_uscale; > - > - return IIO_VAL_INT_PLUS_MICRO; > - } > - > - return -EINVAL; > -} > - > -static const struct iio_info adxl345_info = { > - .driver_module = THIS_MODULE, > - .read_raw = adxl345_read_raw, > -}; > - > -static int adxl345_probe(struct i2c_client *client, > - const struct i2c_device_id *id) > -{ > - struct adxl345_data *data; > - struct iio_dev *indio_dev; > - struct regmap *regmap; > - int ret; > - u32 regval; > - > - regmap = devm_regmap_init_i2c(client, &adxl345_regmap_config); > - if (IS_ERR(regmap)) { > - dev_err(&client->dev, "Error initializing regmap: %d\n", > - (int)PTR_ERR(regmap)); > - return PTR_ERR(regmap); > - } > - > - ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); > - if (ret < 0) { > - dev_err(&client->dev, "Error reading device ID: %d\n", ret); > - return ret; > - } > - > - if (regval != ADXL345_DEVID) { > - dev_err(&client->dev, "Invalid device ID: %d\n", ret); > - return -ENODEV; > - } > - > - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > - if (!indio_dev) > - return -ENOMEM; > - > - data = iio_priv(indio_dev); > - i2c_set_clientdata(client, indio_dev); > - data->client = client; > - data->regmap = regmap; > - /* Enable full-resolution mode */ > - data->data_range = ADXL345_DATA_FORMAT_FULL_RES; > - > - ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, > - data->data_range); > - if (ret < 0) { > - dev_err(&client->dev, "Failed to set data range: %d\n", ret); > - return ret; > - } > - > - indio_dev->dev.parent = &client->dev; > - indio_dev->name = id->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 = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > - ADXL345_POWER_CTL_MEASURE); > - if (ret < 0) { > - dev_err(&client->dev, "Failed to enable measurement mode: %d\n", > - ret); > - return ret; > - } > - > - ret = iio_device_register(indio_dev); > - if (ret < 0) { > - dev_err(&client->dev, "iio_device_register failed: %d\n", ret); > - regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > - ADXL345_POWER_CTL_STANDBY); > - } > - > - return ret; > -} > - > -static int adxl345_remove(struct i2c_client *client) > -{ > - struct iio_dev *indio_dev = i2c_get_clientdata(client); > - struct adxl345_data *data = iio_priv(indio_dev); > - > - iio_device_unregister(indio_dev); > - > - return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > - ADXL345_POWER_CTL_STANDBY); > -} > - > -static const struct i2c_device_id adxl345_i2c_id[] = { > - { "adxl345", 0 }, > - { } > -}; > - > -MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); > - > -static struct i2c_driver adxl345_driver = { > - .driver = { > - .name = "adxl345", > - }, > - .probe = adxl345_probe, > - .remove = adxl345_remove, > - .id_table = adxl345_i2c_id, > -}; > - > -module_i2c_driver(adxl345_driver); > - > -MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > -MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer driver"); > -MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h > new file mode 100644 > index 0000000..fca3e25 > --- /dev/null > +++ b/drivers/iio/accel/adxl345.h > @@ -0,0 +1,18 @@ > +/* > + * ADXL345 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > + * > + * This file is subject to the terms and conditions of version 2 of > + * the GNU General Public License. See the file COPYING in the main > + * directory of this archive for more details. > + */ > + > +#ifndef _ADXL345_H_ > +#define _ADXL345_H_ > + > +int adxl345_common_probe(struct device *dev, struct regmap *regmap, > + const char *name); > +int adxl345_common_remove(struct device *dev); > + > +#endif /* _ADXL345_H_ */ > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c > new file mode 100644 > index 0000000..ce09804 > --- /dev/null > +++ b/drivers/iio/accel/adxl345_core.c > @@ -0,0 +1,182 @@ > +/* > + * ADXL345 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > + * > + * This file is subject to the terms and conditions of version 2 of > + * the GNU General Public License. See the file COPYING in the main > + * directory of this archive for more details. > + * > + * IIO core driver for ADXL345 > + */ > + > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > + > +#include <linux/iio/iio.h> > + > +#include "adxl345.h" > + > +#define ADXL345_REG_DEVID 0x00 > +#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_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 > + > +/* > + * In full-resolution mode, scale factor is maintained at ~4 mg/LSB > + * in all g ranges. > + * > + * At +/- 16g with 13-bit resolution, scale is computed as: > + * (16 + 16) * 9.81 / (2^13 - 1) = 0.0383 > + */ > +static const int adxl345_uscale = 38300; > + > +struct adxl345_data { > + struct regmap *regmap; > + u8 data_range; > +}; > + > +#define ADXL345_CHANNEL(reg, axis) { \ > + .type = IIO_ACCEL, \ > + .modified = 1, \ > + .channel2 = IIO_MOD_##axis, \ > + .address = reg, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > +} > + > +static const struct iio_chan_spec adxl345_channels[] = { > + ADXL345_CHANNEL(ADXL345_REG_DATAX0, X), > + ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y), > + ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z), > +}; > + > +static int adxl345_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct adxl345_data *data = iio_priv(indio_dev); > + int ret; > + __le16 regval; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + /* > + * Data is stored in adjacent registers: > + * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte > + * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte > + */ > + ret = regmap_bulk_read(data->regmap, chan->address, ®val, > + sizeof(regval)); > + if (ret < 0) > + return ret; > + > + *val = sign_extend32(le16_to_cpu(regval), 12); > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SCALE: > + *val = 0; > + *val2 = adxl345_uscale; > + > + return IIO_VAL_INT_PLUS_MICRO; > + } > + > + return -EINVAL; > +} > + > +static const struct iio_info adxl345_info = { > + .driver_module = THIS_MODULE, > + .read_raw = adxl345_read_raw, > +}; > + > +int adxl345_common_probe(struct device *dev, struct regmap *regmap, > + const char *name) > +{ > + struct adxl345_data *data; > + struct iio_dev *indio_dev; > + int ret; > + u32 regval; > + > + ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); > + if (ret < 0) { > + dev_err(dev, "Error reading device ID: %d\n", ret); > + return ret; > + } > + > + if (regval != ADXL345_DEVID) { > + dev_err(dev, "Invalid device ID: %d\n", ret); > + return -ENODEV; > + } > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + dev_set_drvdata(dev, indio_dev); > + data->regmap = regmap; > + /* Enable full-resolution mode */ > + data->data_range = ADXL345_DATA_FORMAT_FULL_RES; > + > + ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, > + data->data_range); > + if (ret < 0) { > + dev_err(dev, "Failed to set data range: %d\n", ret); > + return ret; > + } > + > + indio_dev->dev.parent = dev; > + indio_dev->name = 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 = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > + ADXL345_POWER_CTL_MEASURE); > + if (ret < 0) { > + dev_err(dev, "Failed to enable measurement mode: %d\n", > + ret); > + return ret; > + } > + > + ret = iio_device_register(indio_dev); > + if (ret < 0) { > + dev_err(dev, "iio_device_register failed: %d\n", ret); > + regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > + ADXL345_POWER_CTL_STANDBY); > + } > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(adxl345_common_probe); > + > +int adxl345_common_remove(struct device *dev) > +{ > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > + struct adxl345_data *data = iio_priv(indio_dev); > + > + iio_device_unregister(indio_dev); > + > + return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > + ADXL345_POWER_CTL_STANDBY); > +} > +EXPORT_SYMBOL_GPL(adxl345_common_remove); > + > +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer core driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c > new file mode 100644 > index 0000000..b114eb0 > --- /dev/null > +++ b/drivers/iio/accel/adxl345_i2c.c > @@ -0,0 +1,70 @@ > +/* > + * ADXL345 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > + * > + * This file is subject to the terms and conditions of version 2 of > + * the GNU General Public License. See the file COPYING in the main > + * directory of this archive for more details. > + * > + * I2C driver for ADXL345 > + * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or > + * 0x53 (ALT ADDRESS pin grounded) > + */ > + > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > + > +#include "adxl345.h" > + > +static const struct regmap_config adxl345_i2c_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > +}; > + > +static int adxl345_i2c_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct regmap *regmap; > + const char *name = NULL; > + > + regmap = devm_regmap_init_i2c(client, &adxl345_i2c_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(&client->dev, "Error initializing i2c regmap: %d\n", > + (int)PTR_ERR(regmap)); Ok, so the client->dev stuff effectively gets unwound here anyway. End result is good, but nice to do it cleanly in the first patch as then the apparent changes (if move detection works) in here will be more minor. > + return PTR_ERR(regmap); > + } > + > + if (id) > + name = id->name; > + > + return adxl345_common_probe(&client->dev, regmap, name); > +} > + > +static int adxl345_i2c_remove(struct i2c_client *client) > +{ > + return adxl345_common_remove(&client->dev); > +} > + > +static const struct i2c_device_id adxl345_i2c_id[] = { > + { "adxl345", 0 }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); > + > +static struct i2c_driver adxl345_i2c_driver = { > + .driver = { > + .name = "adxl345_i2c", > + }, > + .probe = adxl345_i2c_probe, > + .remove = adxl345_i2c_remove, > + .id_table = adxl345_i2c_id, > +}; > + > +module_i2c_driver(adxl345_i2c_driver); > + > +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer I2C driver"); > +MODULE_LICENSE("GPL v2"); > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C 2017-02-19 13:11 ` Jonathan Cameron @ 2017-02-20 6:41 ` Eva Rachel Retuya 2017-02-20 9:32 ` Eva Rachel Retuya 0 siblings, 1 reply; 16+ messages in thread From: Eva Rachel Retuya @ 2017-02-20 6:41 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-iio, knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On Sun, Feb 19, 2017 at 01:11:29PM +0000, Jonathan Cameron wrote: > On 16/02/17 10:02, Eva Rachel Retuya wrote: > > Move I2C-specific code into its own file and rely on regmap to access > > registers. The core code provides access to x, y, z and scale readings. > > > > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> > A few minor comment inline. This unwound the comment about client->dev > in the previous patch, but I'd prefer to see it done there as then the > changes in here will be easier to see (with move detection hopefully working!) > > Jonathan > > --- > > drivers/iio/accel/Kconfig | 8 +- > > drivers/iio/accel/Makefile | 3 +- > > drivers/iio/accel/adxl345.c | 213 --------------------------------------- > > drivers/iio/accel/adxl345.h | 18 ++++ > > drivers/iio/accel/adxl345_core.c | 182 +++++++++++++++++++++++++++++++++ > > drivers/iio/accel/adxl345_i2c.c | 70 +++++++++++++ > This is a case where allowing git to notice the moves would have lead > to easier to read patches (unless this was sufficiently complex that git > couldn't follow it ;) Hello Jonathan, Thanks for the feedback. I amended the relevant commits as you say and git detects the file adxl345.c -> adxl345_core.c as 78% renamed. The reason for that percentage is that I did some probe/remove renames and some other deletions here and there. Should I commit them separately in order to get the move detection to work? > > 6 files changed, 278 insertions(+), 216 deletions(-) > > delete mode 100644 drivers/iio/accel/adxl345.c > > create mode 100644 drivers/iio/accel/adxl345.h > > create mode 100644 drivers/iio/accel/adxl345_core.c > > create mode 100644 drivers/iio/accel/adxl345_i2c.c > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > index 26b8614..5bdd87f 100644 > > --- a/drivers/iio/accel/Kconfig > > +++ b/drivers/iio/accel/Kconfig > > @@ -6,16 +6,20 @@ > > menu "Accelerometers" > > > > config ADXL345 > > - tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver" > > + tristate > > + > > +config ADXL345_I2C > > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver" > > depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > > depends on I2C > > + select ADXL345 > > select REGMAP_I2C > > help > > Say Y here if you want to build support for the Analog Devices > > ADXL345 3-axis digital accelerometer. > > > > To compile this driver as a module, choose M here: the > > - module will be called adxl345. > > + module will be called adxl345_i2c. > There are a couple of ways to do this. I personally kind of prefer the > way the bmc150 does it as it gives a cleaner set of options in Kconfig. > > Look at the various ways it is done and if you still prefer this one then > fair enough (it's how we always used to do it ;) Ack. I've seen it but went this way just to be explicit. I agree with doing it cleaner and will reflect it on v2. Eva > > > > config BMA180 > > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > index 618488d..3f4a6d6 100644 > > --- a/drivers/iio/accel/Makefile > > +++ b/drivers/iio/accel/Makefile > > @@ -3,7 +3,8 @@ > > # > > > > # When adding new entries keep the list in alphabetical order > > -obj-$(CONFIG_ADXL345) += adxl345.o > > +obj-$(CONFIG_ADXL345) += adxl345_core.o > > +obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > > obj-$(CONFIG_BMA180) += bma180.o > > obj-$(CONFIG_BMA220) += bma220_spi.o > > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > > diff --git a/drivers/iio/accel/adxl345.c b/drivers/iio/accel/adxl345.c > > deleted file mode 100644 > > index a1fdf60..0000000 > > --- a/drivers/iio/accel/adxl345.c > > +++ /dev/null > > @@ -1,213 +0,0 @@ > > -/* > > - * ADXL345 3-Axis Digital Accelerometer > > - * > > - * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > > - * > > - * This file is subject to the terms and conditions of version 2 of > > - * the GNU General Public License. See the file COPYING in the main > > - * directory of this archive for more details. > > - * > > - * IIO driver for ADXL345 > > - * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or > > - * 0x53 (ALT ADDRESS pin grounded) > > - */ > > - > > -#include <linux/i2c.h> > > -#include <linux/module.h> > > -#include <linux/regmap.h> > > - > > -#include <linux/iio/iio.h> > > - > > -#define ADXL345_REG_DEVID 0x00 > > -#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_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 > > - > > -/* > > - * In full-resolution mode, scale factor is maintained at ~4 mg/LSB > > - * in all g ranges. > > - * > > - * At +/- 16g with 13-bit resolution, scale is computed as: > > - * (16 + 16) * 9.81 / (2^13 - 1) = 0.0383 > > - */ > > -static const int adxl345_uscale = 38300; > > - > > -struct adxl345_data { > > - struct i2c_client *client; > > - struct regmap *regmap; > > - u8 data_range; > > -}; > > - > > -static const struct regmap_config adxl345_regmap_config = { > > - .reg_bits = 8, > > - .val_bits = 8, > > -}; > > - > > -#define ADXL345_CHANNEL(reg, axis) { \ > > - .type = IIO_ACCEL, \ > > - .modified = 1, \ > > - .channel2 = IIO_MOD_##axis, \ > > - .address = reg, \ > > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > > -} > > - > > -static const struct iio_chan_spec adxl345_channels[] = { > > - ADXL345_CHANNEL(ADXL345_REG_DATAX0, X), > > - ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y), > > - ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z), > > -}; > > - > > -static int adxl345_read_raw(struct iio_dev *indio_dev, > > - struct iio_chan_spec const *chan, > > - int *val, int *val2, long mask) > > -{ > > - struct adxl345_data *data = iio_priv(indio_dev); > > - int ret; > > - __le16 regval; > > - > > - switch (mask) { > > - case IIO_CHAN_INFO_RAW: > > - /* > > - * Data is stored in adjacent registers: > > - * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte > > - * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte > > - */ > > - ret = regmap_bulk_read(data->regmap, chan->address, ®val, > > - sizeof(regval)); > > - if (ret < 0) > > - return ret; > > - > > - *val = sign_extend32(le16_to_cpu(regval), 12); > > - return IIO_VAL_INT; > > - case IIO_CHAN_INFO_SCALE: > > - *val = 0; > > - *val2 = adxl345_uscale; > > - > > - return IIO_VAL_INT_PLUS_MICRO; > > - } > > - > > - return -EINVAL; > > -} > > - > > -static const struct iio_info adxl345_info = { > > - .driver_module = THIS_MODULE, > > - .read_raw = adxl345_read_raw, > > -}; > > - > > -static int adxl345_probe(struct i2c_client *client, > > - const struct i2c_device_id *id) > > -{ > > - struct adxl345_data *data; > > - struct iio_dev *indio_dev; > > - struct regmap *regmap; > > - int ret; > > - u32 regval; > > - > > - regmap = devm_regmap_init_i2c(client, &adxl345_regmap_config); > > - if (IS_ERR(regmap)) { > > - dev_err(&client->dev, "Error initializing regmap: %d\n", > > - (int)PTR_ERR(regmap)); > > - return PTR_ERR(regmap); > > - } > > - > > - ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); > > - if (ret < 0) { > > - dev_err(&client->dev, "Error reading device ID: %d\n", ret); > > - return ret; > > - } > > - > > - if (regval != ADXL345_DEVID) { > > - dev_err(&client->dev, "Invalid device ID: %d\n", ret); > > - return -ENODEV; > > - } > > - > > - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > > - if (!indio_dev) > > - return -ENOMEM; > > - > > - data = iio_priv(indio_dev); > > - i2c_set_clientdata(client, indio_dev); > > - data->client = client; > > - data->regmap = regmap; > > - /* Enable full-resolution mode */ > > - data->data_range = ADXL345_DATA_FORMAT_FULL_RES; > > - > > - ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, > > - data->data_range); > > - if (ret < 0) { > > - dev_err(&client->dev, "Failed to set data range: %d\n", ret); > > - return ret; > > - } > > - > > - indio_dev->dev.parent = &client->dev; > > - indio_dev->name = id->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 = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > - ADXL345_POWER_CTL_MEASURE); > > - if (ret < 0) { > > - dev_err(&client->dev, "Failed to enable measurement mode: %d\n", > > - ret); > > - return ret; > > - } > > - > > - ret = iio_device_register(indio_dev); > > - if (ret < 0) { > > - dev_err(&client->dev, "iio_device_register failed: %d\n", ret); > > - regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > - ADXL345_POWER_CTL_STANDBY); > > - } > > - > > - return ret; > > -} > > - > > -static int adxl345_remove(struct i2c_client *client) > > -{ > > - struct iio_dev *indio_dev = i2c_get_clientdata(client); > > - struct adxl345_data *data = iio_priv(indio_dev); > > - > > - iio_device_unregister(indio_dev); > > - > > - return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > - ADXL345_POWER_CTL_STANDBY); > > -} > > - > > -static const struct i2c_device_id adxl345_i2c_id[] = { > > - { "adxl345", 0 }, > > - { } > > -}; > > - > > -MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); > > - > > -static struct i2c_driver adxl345_driver = { > > - .driver = { > > - .name = "adxl345", > > - }, > > - .probe = adxl345_probe, > > - .remove = adxl345_remove, > > - .id_table = adxl345_i2c_id, > > -}; > > - > > -module_i2c_driver(adxl345_driver); > > - > > -MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > > -MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer driver"); > > -MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h > > new file mode 100644 > > index 0000000..fca3e25 > > --- /dev/null > > +++ b/drivers/iio/accel/adxl345.h > > @@ -0,0 +1,18 @@ > > +/* > > + * ADXL345 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > > + * > > + * This file is subject to the terms and conditions of version 2 of > > + * the GNU General Public License. See the file COPYING in the main > > + * directory of this archive for more details. > > + */ > > + > > +#ifndef _ADXL345_H_ > > +#define _ADXL345_H_ > > + > > +int adxl345_common_probe(struct device *dev, struct regmap *regmap, > > + const char *name); > > +int adxl345_common_remove(struct device *dev); > > + > > +#endif /* _ADXL345_H_ */ > > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c > > new file mode 100644 > > index 0000000..ce09804 > > --- /dev/null > > +++ b/drivers/iio/accel/adxl345_core.c > > @@ -0,0 +1,182 @@ > > +/* > > + * ADXL345 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > > + * > > + * This file is subject to the terms and conditions of version 2 of > > + * the GNU General Public License. See the file COPYING in the main > > + * directory of this archive for more details. > > + * > > + * IIO core driver for ADXL345 > > + */ > > + > > +#include <linux/i2c.h> > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > + > > +#include <linux/iio/iio.h> > > + > > +#include "adxl345.h" > > + > > +#define ADXL345_REG_DEVID 0x00 > > +#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_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 > > + > > +/* > > + * In full-resolution mode, scale factor is maintained at ~4 mg/LSB > > + * in all g ranges. > > + * > > + * At +/- 16g with 13-bit resolution, scale is computed as: > > + * (16 + 16) * 9.81 / (2^13 - 1) = 0.0383 > > + */ > > +static const int adxl345_uscale = 38300; > > + > > +struct adxl345_data { > > + struct regmap *regmap; > > + u8 data_range; > > +}; > > + > > +#define ADXL345_CHANNEL(reg, axis) { \ > > + .type = IIO_ACCEL, \ > > + .modified = 1, \ > > + .channel2 = IIO_MOD_##axis, \ > > + .address = reg, \ > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > > +} > > + > > +static const struct iio_chan_spec adxl345_channels[] = { > > + ADXL345_CHANNEL(ADXL345_REG_DATAX0, X), > > + ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y), > > + ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z), > > +}; > > + > > +static int adxl345_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int *val, int *val2, long mask) > > +{ > > + struct adxl345_data *data = iio_priv(indio_dev); > > + int ret; > > + __le16 regval; > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_RAW: > > + /* > > + * Data is stored in adjacent registers: > > + * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte > > + * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte > > + */ > > + ret = regmap_bulk_read(data->regmap, chan->address, ®val, > > + sizeof(regval)); > > + if (ret < 0) > > + return ret; > > + > > + *val = sign_extend32(le16_to_cpu(regval), 12); > > + return IIO_VAL_INT; > > + case IIO_CHAN_INFO_SCALE: > > + *val = 0; > > + *val2 = adxl345_uscale; > > + > > + return IIO_VAL_INT_PLUS_MICRO; > > + } > > + > > + return -EINVAL; > > +} > > + > > +static const struct iio_info adxl345_info = { > > + .driver_module = THIS_MODULE, > > + .read_raw = adxl345_read_raw, > > +}; > > + > > +int adxl345_common_probe(struct device *dev, struct regmap *regmap, > > + const char *name) > > +{ > > + struct adxl345_data *data; > > + struct iio_dev *indio_dev; > > + int ret; > > + u32 regval; > > + > > + ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); > > + if (ret < 0) { > > + dev_err(dev, "Error reading device ID: %d\n", ret); > > + return ret; > > + } > > + > > + if (regval != ADXL345_DEVID) { > > + dev_err(dev, "Invalid device ID: %d\n", ret); > > + return -ENODEV; > > + } > > + > > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + data = iio_priv(indio_dev); > > + dev_set_drvdata(dev, indio_dev); > > + data->regmap = regmap; > > + /* Enable full-resolution mode */ > > + data->data_range = ADXL345_DATA_FORMAT_FULL_RES; > > + > > + ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, > > + data->data_range); > > + if (ret < 0) { > > + dev_err(dev, "Failed to set data range: %d\n", ret); > > + return ret; > > + } > > + > > + indio_dev->dev.parent = dev; > > + indio_dev->name = 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 = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > + ADXL345_POWER_CTL_MEASURE); > > + if (ret < 0) { > > + dev_err(dev, "Failed to enable measurement mode: %d\n", > > + ret); > > + return ret; > > + } > > + > > + ret = iio_device_register(indio_dev); > > + if (ret < 0) { > > + dev_err(dev, "iio_device_register failed: %d\n", ret); > > + regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > + ADXL345_POWER_CTL_STANDBY); > > + } > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(adxl345_common_probe); > > + > > +int adxl345_common_remove(struct device *dev) > > +{ > > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > > + struct adxl345_data *data = iio_priv(indio_dev); > > + > > + iio_device_unregister(indio_dev); > > + > > + return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > + ADXL345_POWER_CTL_STANDBY); > > +} > > +EXPORT_SYMBOL_GPL(adxl345_common_remove); > > + > > +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer core driver"); > > +MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c > > new file mode 100644 > > index 0000000..b114eb0 > > --- /dev/null > > +++ b/drivers/iio/accel/adxl345_i2c.c > > @@ -0,0 +1,70 @@ > > +/* > > + * ADXL345 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > > + * > > + * This file is subject to the terms and conditions of version 2 of > > + * the GNU General Public License. See the file COPYING in the main > > + * directory of this archive for more details. > > + * > > + * I2C driver for ADXL345 > > + * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or > > + * 0x53 (ALT ADDRESS pin grounded) > > + */ > > + > > +#include <linux/i2c.h> > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > + > > +#include "adxl345.h" > > + > > +static const struct regmap_config adxl345_i2c_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > +}; > > + > > +static int adxl345_i2c_probe(struct i2c_client *client, > > + const struct i2c_device_id *id) > > +{ > > + struct regmap *regmap; > > + const char *name = NULL; > > + > > + regmap = devm_regmap_init_i2c(client, &adxl345_i2c_regmap_config); > > + if (IS_ERR(regmap)) { > > + dev_err(&client->dev, "Error initializing i2c regmap: %d\n", > > + (int)PTR_ERR(regmap)); > Ok, so the client->dev stuff effectively gets unwound here anyway. > End result is good, but nice to do it cleanly in the first patch as > then the apparent changes (if move detection works) in here will be more > minor. > > + return PTR_ERR(regmap); > > + } > > + > > + if (id) > > + name = id->name; > > + > > + return adxl345_common_probe(&client->dev, regmap, name); > > +} > > + > > +static int adxl345_i2c_remove(struct i2c_client *client) > > +{ > > + return adxl345_common_remove(&client->dev); > > +} > > + > > +static const struct i2c_device_id adxl345_i2c_id[] = { > > + { "adxl345", 0 }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); > > + > > +static struct i2c_driver adxl345_i2c_driver = { > > + .driver = { > > + .name = "adxl345_i2c", > > + }, > > + .probe = adxl345_i2c_probe, > > + .remove = adxl345_i2c_remove, > > + .id_table = adxl345_i2c_id, > > +}; > > + > > +module_i2c_driver(adxl345_i2c_driver); > > + > > +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer I2C driver"); > > +MODULE_LICENSE("GPL v2"); > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C 2017-02-20 6:41 ` Eva Rachel Retuya @ 2017-02-20 9:32 ` Eva Rachel Retuya 2017-02-20 11:47 ` Andy Shevchenko 0 siblings, 1 reply; 16+ messages in thread From: Eva Rachel Retuya @ 2017-02-20 9:32 UTC (permalink / raw) To: Jonathan Cameron, linux-iio, knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On Mon, Feb 20, 2017 at 02:41:12PM +0800, Eva Rachel Retuya wrote: > On Sun, Feb 19, 2017 at 01:11:29PM +0000, Jonathan Cameron wrote: > > On 16/02/17 10:02, Eva Rachel Retuya wrote: > > > Move I2C-specific code into its own file and rely on regmap to access > > > registers. The core code provides access to x, y, z and scale readings. > > > > > > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> > > A few minor comment inline. This unwound the comment about client->dev > > in the previous patch, but I'd prefer to see it done there as then the > > changes in here will be easier to see (with move detection hopefully working!) > > > > Jonathan > > > --- > > > drivers/iio/accel/Kconfig | 8 +- > > > drivers/iio/accel/Makefile | 3 +- > > > drivers/iio/accel/adxl345.c | 213 --------------------------------------- > > > drivers/iio/accel/adxl345.h | 18 ++++ > > > drivers/iio/accel/adxl345_core.c | 182 +++++++++++++++++++++++++++++++++ > > > drivers/iio/accel/adxl345_i2c.c | 70 +++++++++++++ > > This is a case where allowing git to notice the moves would have lead > > to easier to read patches (unless this was sufficiently complex that git > > couldn't follow it ;) > > Hello Jonathan, > > Thanks for the feedback. I amended the relevant commits as you say and > git detects the file adxl345.c -> adxl345_core.c as 78% renamed. The > reason for that percentage is that I did some probe/remove renames and > some other deletions here and there. Should I commit them separately in > order to get the move detection to work? > Moved the deletions/modifications into patch 1. I hope I got it right this time? Thanks, Eva > > > 6 files changed, 278 insertions(+), 216 deletions(-) > > > delete mode 100644 drivers/iio/accel/adxl345.c > > > create mode 100644 drivers/iio/accel/adxl345.h > > > create mode 100644 drivers/iio/accel/adxl345_core.c > > > create mode 100644 drivers/iio/accel/adxl345_i2c.c > > > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > > index 26b8614..5bdd87f 100644 > > > --- a/drivers/iio/accel/Kconfig > > > +++ b/drivers/iio/accel/Kconfig > > > @@ -6,16 +6,20 @@ > > > menu "Accelerometers" > > > > > > config ADXL345 > > > - tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver" > > > + tristate > > > + > > > +config ADXL345_I2C > > > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver" > > > depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > > > depends on I2C > > > + select ADXL345 > > > select REGMAP_I2C > > > help > > > Say Y here if you want to build support for the Analog Devices > > > ADXL345 3-axis digital accelerometer. > > > > > > To compile this driver as a module, choose M here: the > > > - module will be called adxl345. > > > + module will be called adxl345_i2c. > > There are a couple of ways to do this. I personally kind of prefer the > > way the bmc150 does it as it gives a cleaner set of options in Kconfig. > > > > Look at the various ways it is done and if you still prefer this one then > > fair enough (it's how we always used to do it ;) > > Ack. I've seen it but went this way just to be explicit. I agree with doing > it cleaner and will reflect it on v2. > > Eva > > > > > > > config BMA180 > > > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > > index 618488d..3f4a6d6 100644 > > > --- a/drivers/iio/accel/Makefile > > > +++ b/drivers/iio/accel/Makefile > > > @@ -3,7 +3,8 @@ > > > # > > > > > > # When adding new entries keep the list in alphabetical order > > > -obj-$(CONFIG_ADXL345) += adxl345.o > > > +obj-$(CONFIG_ADXL345) += adxl345_core.o > > > +obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > > > obj-$(CONFIG_BMA180) += bma180.o > > > obj-$(CONFIG_BMA220) += bma220_spi.o > > > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > > > diff --git a/drivers/iio/accel/adxl345.c b/drivers/iio/accel/adxl345.c > > > deleted file mode 100644 > > > index a1fdf60..0000000 > > > --- a/drivers/iio/accel/adxl345.c > > > +++ /dev/null > > > @@ -1,213 +0,0 @@ > > > -/* > > > - * ADXL345 3-Axis Digital Accelerometer > > > - * > > > - * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > > > - * > > > - * This file is subject to the terms and conditions of version 2 of > > > - * the GNU General Public License. See the file COPYING in the main > > > - * directory of this archive for more details. > > > - * > > > - * IIO driver for ADXL345 > > > - * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or > > > - * 0x53 (ALT ADDRESS pin grounded) > > > - */ > > > - > > > -#include <linux/i2c.h> > > > -#include <linux/module.h> > > > -#include <linux/regmap.h> > > > - > > > -#include <linux/iio/iio.h> > > > - > > > -#define ADXL345_REG_DEVID 0x00 > > > -#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_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 > > > - > > > -/* > > > - * In full-resolution mode, scale factor is maintained at ~4 mg/LSB > > > - * in all g ranges. > > > - * > > > - * At +/- 16g with 13-bit resolution, scale is computed as: > > > - * (16 + 16) * 9.81 / (2^13 - 1) = 0.0383 > > > - */ > > > -static const int adxl345_uscale = 38300; > > > - > > > -struct adxl345_data { > > > - struct i2c_client *client; > > > - struct regmap *regmap; > > > - u8 data_range; > > > -}; > > > - > > > -static const struct regmap_config adxl345_regmap_config = { > > > - .reg_bits = 8, > > > - .val_bits = 8, > > > -}; > > > - > > > -#define ADXL345_CHANNEL(reg, axis) { \ > > > - .type = IIO_ACCEL, \ > > > - .modified = 1, \ > > > - .channel2 = IIO_MOD_##axis, \ > > > - .address = reg, \ > > > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > > > -} > > > - > > > -static const struct iio_chan_spec adxl345_channels[] = { > > > - ADXL345_CHANNEL(ADXL345_REG_DATAX0, X), > > > - ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y), > > > - ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z), > > > -}; > > > - > > > -static int adxl345_read_raw(struct iio_dev *indio_dev, > > > - struct iio_chan_spec const *chan, > > > - int *val, int *val2, long mask) > > > -{ > > > - struct adxl345_data *data = iio_priv(indio_dev); > > > - int ret; > > > - __le16 regval; > > > - > > > - switch (mask) { > > > - case IIO_CHAN_INFO_RAW: > > > - /* > > > - * Data is stored in adjacent registers: > > > - * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte > > > - * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte > > > - */ > > > - ret = regmap_bulk_read(data->regmap, chan->address, ®val, > > > - sizeof(regval)); > > > - if (ret < 0) > > > - return ret; > > > - > > > - *val = sign_extend32(le16_to_cpu(regval), 12); > > > - return IIO_VAL_INT; > > > - case IIO_CHAN_INFO_SCALE: > > > - *val = 0; > > > - *val2 = adxl345_uscale; > > > - > > > - return IIO_VAL_INT_PLUS_MICRO; > > > - } > > > - > > > - return -EINVAL; > > > -} > > > - > > > -static const struct iio_info adxl345_info = { > > > - .driver_module = THIS_MODULE, > > > - .read_raw = adxl345_read_raw, > > > -}; > > > - > > > -static int adxl345_probe(struct i2c_client *client, > > > - const struct i2c_device_id *id) > > > -{ > > > - struct adxl345_data *data; > > > - struct iio_dev *indio_dev; > > > - struct regmap *regmap; > > > - int ret; > > > - u32 regval; > > > - > > > - regmap = devm_regmap_init_i2c(client, &adxl345_regmap_config); > > > - if (IS_ERR(regmap)) { > > > - dev_err(&client->dev, "Error initializing regmap: %d\n", > > > - (int)PTR_ERR(regmap)); > > > - return PTR_ERR(regmap); > > > - } > > > - > > > - ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); > > > - if (ret < 0) { > > > - dev_err(&client->dev, "Error reading device ID: %d\n", ret); > > > - return ret; > > > - } > > > - > > > - if (regval != ADXL345_DEVID) { > > > - dev_err(&client->dev, "Invalid device ID: %d\n", ret); > > > - return -ENODEV; > > > - } > > > - > > > - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > > > - if (!indio_dev) > > > - return -ENOMEM; > > > - > > > - data = iio_priv(indio_dev); > > > - i2c_set_clientdata(client, indio_dev); > > > - data->client = client; > > > - data->regmap = regmap; > > > - /* Enable full-resolution mode */ > > > - data->data_range = ADXL345_DATA_FORMAT_FULL_RES; > > > - > > > - ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, > > > - data->data_range); > > > - if (ret < 0) { > > > - dev_err(&client->dev, "Failed to set data range: %d\n", ret); > > > - return ret; > > > - } > > > - > > > - indio_dev->dev.parent = &client->dev; > > > - indio_dev->name = id->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 = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > > - ADXL345_POWER_CTL_MEASURE); > > > - if (ret < 0) { > > > - dev_err(&client->dev, "Failed to enable measurement mode: %d\n", > > > - ret); > > > - return ret; > > > - } > > > - > > > - ret = iio_device_register(indio_dev); > > > - if (ret < 0) { > > > - dev_err(&client->dev, "iio_device_register failed: %d\n", ret); > > > - regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > > - ADXL345_POWER_CTL_STANDBY); > > > - } > > > - > > > - return ret; > > > -} > > > - > > > -static int adxl345_remove(struct i2c_client *client) > > > -{ > > > - struct iio_dev *indio_dev = i2c_get_clientdata(client); > > > - struct adxl345_data *data = iio_priv(indio_dev); > > > - > > > - iio_device_unregister(indio_dev); > > > - > > > - return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > > - ADXL345_POWER_CTL_STANDBY); > > > -} > > > - > > > -static const struct i2c_device_id adxl345_i2c_id[] = { > > > - { "adxl345", 0 }, > > > - { } > > > -}; > > > - > > > -MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); > > > - > > > -static struct i2c_driver adxl345_driver = { > > > - .driver = { > > > - .name = "adxl345", > > > - }, > > > - .probe = adxl345_probe, > > > - .remove = adxl345_remove, > > > - .id_table = adxl345_i2c_id, > > > -}; > > > - > > > -module_i2c_driver(adxl345_driver); > > > - > > > -MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > > > -MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer driver"); > > > -MODULE_LICENSE("GPL v2"); > > > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h > > > new file mode 100644 > > > index 0000000..fca3e25 > > > --- /dev/null > > > +++ b/drivers/iio/accel/adxl345.h > > > @@ -0,0 +1,18 @@ > > > +/* > > > + * ADXL345 3-Axis Digital Accelerometer > > > + * > > > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > > > + * > > > + * This file is subject to the terms and conditions of version 2 of > > > + * the GNU General Public License. See the file COPYING in the main > > > + * directory of this archive for more details. > > > + */ > > > + > > > +#ifndef _ADXL345_H_ > > > +#define _ADXL345_H_ > > > + > > > +int adxl345_common_probe(struct device *dev, struct regmap *regmap, > > > + const char *name); > > > +int adxl345_common_remove(struct device *dev); > > > + > > > +#endif /* _ADXL345_H_ */ > > > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c > > > new file mode 100644 > > > index 0000000..ce09804 > > > --- /dev/null > > > +++ b/drivers/iio/accel/adxl345_core.c > > > @@ -0,0 +1,182 @@ > > > +/* > > > + * ADXL345 3-Axis Digital Accelerometer > > > + * > > > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > > > + * > > > + * This file is subject to the terms and conditions of version 2 of > > > + * the GNU General Public License. See the file COPYING in the main > > > + * directory of this archive for more details. > > > + * > > > + * IIO core driver for ADXL345 > > > + */ > > > + > > > +#include <linux/i2c.h> > > > +#include <linux/module.h> > > > +#include <linux/regmap.h> > > > + > > > +#include <linux/iio/iio.h> > > > + > > > +#include "adxl345.h" > > > + > > > +#define ADXL345_REG_DEVID 0x00 > > > +#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_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 > > > + > > > +/* > > > + * In full-resolution mode, scale factor is maintained at ~4 mg/LSB > > > + * in all g ranges. > > > + * > > > + * At +/- 16g with 13-bit resolution, scale is computed as: > > > + * (16 + 16) * 9.81 / (2^13 - 1) = 0.0383 > > > + */ > > > +static const int adxl345_uscale = 38300; > > > + > > > +struct adxl345_data { > > > + struct regmap *regmap; > > > + u8 data_range; > > > +}; > > > + > > > +#define ADXL345_CHANNEL(reg, axis) { \ > > > + .type = IIO_ACCEL, \ > > > + .modified = 1, \ > > > + .channel2 = IIO_MOD_##axis, \ > > > + .address = reg, \ > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > > > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > > > +} > > > + > > > +static const struct iio_chan_spec adxl345_channels[] = { > > > + ADXL345_CHANNEL(ADXL345_REG_DATAX0, X), > > > + ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y), > > > + ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z), > > > +}; > > > + > > > +static int adxl345_read_raw(struct iio_dev *indio_dev, > > > + struct iio_chan_spec const *chan, > > > + int *val, int *val2, long mask) > > > +{ > > > + struct adxl345_data *data = iio_priv(indio_dev); > > > + int ret; > > > + __le16 regval; > > > + > > > + switch (mask) { > > > + case IIO_CHAN_INFO_RAW: > > > + /* > > > + * Data is stored in adjacent registers: > > > + * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte > > > + * and ADXL345_REG_DATA(X0/Y0/Z0) + 1 the most significant byte > > > + */ > > > + ret = regmap_bulk_read(data->regmap, chan->address, ®val, > > > + sizeof(regval)); > > > + if (ret < 0) > > > + return ret; > > > + > > > + *val = sign_extend32(le16_to_cpu(regval), 12); > > > + return IIO_VAL_INT; > > > + case IIO_CHAN_INFO_SCALE: > > > + *val = 0; > > > + *val2 = adxl345_uscale; > > > + > > > + return IIO_VAL_INT_PLUS_MICRO; > > > + } > > > + > > > + return -EINVAL; > > > +} > > > + > > > +static const struct iio_info adxl345_info = { > > > + .driver_module = THIS_MODULE, > > > + .read_raw = adxl345_read_raw, > > > +}; > > > + > > > +int adxl345_common_probe(struct device *dev, struct regmap *regmap, > > > + const char *name) > > > +{ > > > + struct adxl345_data *data; > > > + struct iio_dev *indio_dev; > > > + int ret; > > > + u32 regval; > > > + > > > + ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val); > > > + if (ret < 0) { > > > + dev_err(dev, "Error reading device ID: %d\n", ret); > > > + return ret; > > > + } > > > + > > > + if (regval != ADXL345_DEVID) { > > > + dev_err(dev, "Invalid device ID: %d\n", ret); > > > + return -ENODEV; > > > + } > > > + > > > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > > > + if (!indio_dev) > > > + return -ENOMEM; > > > + > > > + data = iio_priv(indio_dev); > > > + dev_set_drvdata(dev, indio_dev); > > > + data->regmap = regmap; > > > + /* Enable full-resolution mode */ > > > + data->data_range = ADXL345_DATA_FORMAT_FULL_RES; > > > + > > > + ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, > > > + data->data_range); > > > + if (ret < 0) { > > > + dev_err(dev, "Failed to set data range: %d\n", ret); > > > + return ret; > > > + } > > > + > > > + indio_dev->dev.parent = dev; > > > + indio_dev->name = 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 = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > > + ADXL345_POWER_CTL_MEASURE); > > > + if (ret < 0) { > > > + dev_err(dev, "Failed to enable measurement mode: %d\n", > > > + ret); > > > + return ret; > > > + } > > > + > > > + ret = iio_device_register(indio_dev); > > > + if (ret < 0) { > > > + dev_err(dev, "iio_device_register failed: %d\n", ret); > > > + regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > > + ADXL345_POWER_CTL_STANDBY); > > > + } > > > + > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(adxl345_common_probe); > > > + > > > +int adxl345_common_remove(struct device *dev) > > > +{ > > > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > > > + struct adxl345_data *data = iio_priv(indio_dev); > > > + > > > + iio_device_unregister(indio_dev); > > > + > > > + return regmap_write(data->regmap, ADXL345_REG_POWER_CTL, > > > + ADXL345_POWER_CTL_STANDBY); > > > +} > > > +EXPORT_SYMBOL_GPL(adxl345_common_remove); > > > + > > > +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > > > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer core driver"); > > > +MODULE_LICENSE("GPL v2"); > > > diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c > > > new file mode 100644 > > > index 0000000..b114eb0 > > > --- /dev/null > > > +++ b/drivers/iio/accel/adxl345_i2c.c > > > @@ -0,0 +1,70 @@ > > > +/* > > > + * ADXL345 3-Axis Digital Accelerometer > > > + * > > > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > > > + * > > > + * This file is subject to the terms and conditions of version 2 of > > > + * the GNU General Public License. See the file COPYING in the main > > > + * directory of this archive for more details. > > > + * > > > + * I2C driver for ADXL345 > > > + * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or > > > + * 0x53 (ALT ADDRESS pin grounded) > > > + */ > > > + > > > +#include <linux/i2c.h> > > > +#include <linux/module.h> > > > +#include <linux/regmap.h> > > > + > > > +#include "adxl345.h" > > > + > > > +static const struct regmap_config adxl345_i2c_regmap_config = { > > > + .reg_bits = 8, > > > + .val_bits = 8, > > > +}; > > > + > > > +static int adxl345_i2c_probe(struct i2c_client *client, > > > + const struct i2c_device_id *id) > > > +{ > > > + struct regmap *regmap; > > > + const char *name = NULL; > > > + > > > + regmap = devm_regmap_init_i2c(client, &adxl345_i2c_regmap_config); > > > + if (IS_ERR(regmap)) { > > > + dev_err(&client->dev, "Error initializing i2c regmap: %d\n", > > > + (int)PTR_ERR(regmap)); > > Ok, so the client->dev stuff effectively gets unwound here anyway. > > End result is good, but nice to do it cleanly in the first patch as > > then the apparent changes (if move detection works) in here will be more > > minor. > > > + return PTR_ERR(regmap); > > > + } > > > + > > > + if (id) > > > + name = id->name; > > > + > > > + return adxl345_common_probe(&client->dev, regmap, name); > > > +} > > > + > > > +static int adxl345_i2c_remove(struct i2c_client *client) > > > +{ > > > + return adxl345_common_remove(&client->dev); > > > +} > > > + > > > +static const struct i2c_device_id adxl345_i2c_id[] = { > > > + { "adxl345", 0 }, > > > + { } > > > +}; > > > + > > > +MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); > > > + > > > +static struct i2c_driver adxl345_i2c_driver = { > > > + .driver = { > > > + .name = "adxl345_i2c", > > > + }, > > > + .probe = adxl345_i2c_probe, > > > + .remove = adxl345_i2c_remove, > > > + .id_table = adxl345_i2c_id, > > > +}; > > > + > > > +module_i2c_driver(adxl345_i2c_driver); > > > + > > > +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > > > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer I2C driver"); > > > +MODULE_LICENSE("GPL v2"); > > > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C 2017-02-20 9:32 ` Eva Rachel Retuya @ 2017-02-20 11:47 ` Andy Shevchenko 0 siblings, 0 replies; 16+ messages in thread From: Andy Shevchenko @ 2017-02-20 11:47 UTC (permalink / raw) To: Jonathan Cameron, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, Dmitry Torokhov, Michael Hennerich, daniel.baluta, amsfield22, Florian Vaussard, linux-kernel On Mon, Feb 20, 2017 at 11:32 AM, Eva Rachel Retuya <eraretuya@gmail.com> wrote: > On Mon, Feb 20, 2017 at 02:41:12PM +0800, Eva Rachel Retuya wrote: >> On Sun, Feb 19, 2017 at 01:11:29PM +0000, Jonathan Cameron wrote: >> > On 16/02/17 10:02, Eva Rachel Retuya wrote: >> Hello Jonathan, >> >> Thanks for the feedback. I amended the relevant commits as you say and >> git detects the file adxl345.c -> adxl345_core.c as 78% renamed. The >> reason for that percentage is that I did some probe/remove renames and >> some other deletions here and there. Should I commit them separately in >> order to get the move detection to work? >> > > Moved the deletions/modifications into patch 1. I hope I got it right > this time? I'm not a Jonathan, but there my remarks. 1. Yes, this version looks better. 2. Just a hint: check -M (for move) and -C (for copy) parameters to git format-patch. 3. And please avoid over quoting when answering mails. Thanks. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/4] iio: accel: adxl345: Add SPI support 2017-02-16 10:02 [PATCH 0/4] iio: accel: adxl345: Split driver into core and I2C then add SPI support Eva Rachel Retuya 2017-02-16 10:02 ` [PATCH 1/4] iio: accel: adxl345: Use I2C regmap instead of direct I2C access Eva Rachel Retuya 2017-02-16 10:02 ` [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C Eva Rachel Retuya @ 2017-02-16 10:02 ` Eva Rachel Retuya 2017-02-19 13:12 ` Jonathan Cameron 2017-02-16 10:02 ` [PATCH 4/4] iio: accel: adxl345: Add ACPI support Eva Rachel Retuya 3 siblings, 1 reply; 16+ messages in thread From: Eva Rachel Retuya @ 2017-02-16 10:02 UTC (permalink / raw) To: jic23, linux-iio Cc: knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel, Eva Rachel Retuya Add SPI driver for initializing spi regmap for the adxl345 core driver. The driver supports the same functionality as I2C namely the x, y, z and scale readings. Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> --- drivers/iio/accel/Kconfig | 13 +++++++ drivers/iio/accel/Makefile | 1 + drivers/iio/accel/adxl345_spi.c | 75 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 drivers/iio/accel/adxl345_spi.c diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig index 5bdd87f..d474fed 100644 --- a/drivers/iio/accel/Kconfig +++ b/drivers/iio/accel/Kconfig @@ -21,6 +21,19 @@ config ADXL345_I2C To compile this driver as a module, choose M here: the module will be called adxl345_i2c. +config ADXL345_SPI + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer SPI Driver" + depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) + depends on SPI + select ADXL345 + select REGMAP_SPI + help + Say Y here if you want to build support for the Analog Devices + ADXL345 3-axis digital accelerometer. + + To compile this driver as a module, choose M here: the + module will be called adxl345_spi. + config BMA180 tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" depends on I2C diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile index 3f4a6d6..31fba19 100644 --- a/drivers/iio/accel/Makefile +++ b/drivers/iio/accel/Makefile @@ -5,6 +5,7 @@ # When adding new entries keep the list in alphabetical order obj-$(CONFIG_ADXL345) += adxl345_core.o obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o +obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o obj-$(CONFIG_BMA180) += bma180.o obj-$(CONFIG_BMA220) += bma220_spi.o obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c new file mode 100644 index 0000000..5fcd1fa --- /dev/null +++ b/drivers/iio/accel/adxl345_spi.c @@ -0,0 +1,75 @@ +/* + * ADXL345 3-Axis Digital Accelerometer + * + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> + * + * This file is subject to the terms and conditions of version 2 of + * the GNU General Public License. See the file COPYING in the main + * directory of this archive for more details. + * + * SPI driver for ADXL345 + */ + +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/spi/spi.h> + +#include "adxl345.h" + +#define ADXL345_MAX_SPI_FREQ_HZ 5000000 + +static const struct regmap_config adxl345_spi_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + /* Setting bits 7 and 6 enables multiple-byte read */ + .read_flag_mask = BIT(7) | BIT(6), +}; + +static int adxl345_spi_probe(struct spi_device *spi) +{ + struct regmap *regmap; + const struct spi_device_id *id = spi_get_device_id(spi); + + /* Bail out if max_speed_hz exceeds 5 MHz */ + if (spi->max_speed_hz > ADXL345_MAX_SPI_FREQ_HZ) { + dev_err(&spi->dev, "SPI CLK, %d Hz exceeds 5 MHz\n", + spi->max_speed_hz); + return -EINVAL; + } + + regmap = devm_regmap_init_spi(spi, &adxl345_spi_regmap_config); + if (IS_ERR(regmap)) { + dev_err(&spi->dev, "Error initializing spi regmap: %d\n", + (int)PTR_ERR(regmap)); + return PTR_ERR(regmap); + } + + return adxl345_common_probe(&spi->dev, regmap, id->name); +} + +static int adxl345_spi_remove(struct spi_device *spi) +{ + return adxl345_common_remove(&spi->dev); +} + +static const struct spi_device_id adxl345_spi_id[] = { + { "adxl345", 0 }, + { } +}; + +MODULE_DEVICE_TABLE(spi, adxl345_spi_id); + +static struct spi_driver adxl345_spi_driver = { + .driver = { + .name = "adxl345_spi", + }, + .probe = adxl345_spi_probe, + .remove = adxl345_spi_remove, + .id_table = adxl345_spi_id, +}; + +module_spi_driver(adxl345_spi_driver); + +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer SPI driver"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] iio: accel: adxl345: Add SPI support 2017-02-16 10:02 ` [PATCH 3/4] iio: accel: adxl345: Add SPI support Eva Rachel Retuya @ 2017-02-19 13:12 ` Jonathan Cameron 2017-02-20 6:46 ` Eva Rachel Retuya 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2017-02-19 13:12 UTC (permalink / raw) To: Eva Rachel Retuya, linux-iio Cc: knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On 16/02/17 10:02, Eva Rachel Retuya wrote: > Add SPI driver for initializing spi regmap for the adxl345 core driver. > The driver supports the same functionality as I2C namely the x, y, z and > scale readings. > > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> Looks nice. I didn't know about the readmask stuff in regmap so always good to see something new. Jonathan > --- > drivers/iio/accel/Kconfig | 13 +++++++ > drivers/iio/accel/Makefile | 1 + > drivers/iio/accel/adxl345_spi.c | 75 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 89 insertions(+) > create mode 100644 drivers/iio/accel/adxl345_spi.c > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > index 5bdd87f..d474fed 100644 > --- a/drivers/iio/accel/Kconfig > +++ b/drivers/iio/accel/Kconfig > @@ -21,6 +21,19 @@ config ADXL345_I2C > To compile this driver as a module, choose M here: the > module will be called adxl345_i2c. > > +config ADXL345_SPI > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer SPI Driver" > + depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > + depends on SPI > + select ADXL345 > + select REGMAP_SPI > + help > + Say Y here if you want to build support for the Analog Devices > + ADXL345 3-axis digital accelerometer. > + > + To compile this driver as a module, choose M here: the > + module will be called adxl345_spi. > + > config BMA180 > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > depends on I2C > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > index 3f4a6d6..31fba19 100644 > --- a/drivers/iio/accel/Makefile > +++ b/drivers/iio/accel/Makefile > @@ -5,6 +5,7 @@ > # When adding new entries keep the list in alphabetical order > obj-$(CONFIG_ADXL345) += adxl345_core.o > obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > +obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o > obj-$(CONFIG_BMA180) += bma180.o > obj-$(CONFIG_BMA220) += bma220_spi.o > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c > new file mode 100644 > index 0000000..5fcd1fa > --- /dev/null > +++ b/drivers/iio/accel/adxl345_spi.c > @@ -0,0 +1,75 @@ > +/* > + * ADXL345 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > + * > + * This file is subject to the terms and conditions of version 2 of > + * the GNU General Public License. See the file COPYING in the main > + * directory of this archive for more details. > + * > + * SPI driver for ADXL345 > + */ > + > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/spi/spi.h> > + > +#include "adxl345.h" > + > +#define ADXL345_MAX_SPI_FREQ_HZ 5000000 > + > +static const struct regmap_config adxl345_spi_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + /* Setting bits 7 and 6 enables multiple-byte read */ > + .read_flag_mask = BIT(7) | BIT(6), Nice. I didn't know about that one ;) > +}; > + > +static int adxl345_spi_probe(struct spi_device *spi) > +{ > + struct regmap *regmap; > + const struct spi_device_id *id = spi_get_device_id(spi); > + > + /* Bail out if max_speed_hz exceeds 5 MHz */ > + if (spi->max_speed_hz > ADXL345_MAX_SPI_FREQ_HZ) { > + dev_err(&spi->dev, "SPI CLK, %d Hz exceeds 5 MHz\n", > + spi->max_speed_hz); > + return -EINVAL; > + } > + > + regmap = devm_regmap_init_spi(spi, &adxl345_spi_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(&spi->dev, "Error initializing spi regmap: %d\n", > + (int)PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } > + > + return adxl345_common_probe(&spi->dev, regmap, id->name); > +} > + > +static int adxl345_spi_remove(struct spi_device *spi) > +{ > + return adxl345_common_remove(&spi->dev); > +} > + > +static const struct spi_device_id adxl345_spi_id[] = { > + { "adxl345", 0 }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(spi, adxl345_spi_id); > + > +static struct spi_driver adxl345_spi_driver = { > + .driver = { > + .name = "adxl345_spi", > + }, > + .probe = adxl345_spi_probe, > + .remove = adxl345_spi_remove, > + .id_table = adxl345_spi_id, > +}; > + > +module_spi_driver(adxl345_spi_driver); > + > +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer SPI driver"); > +MODULE_LICENSE("GPL v2"); > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/4] iio: accel: adxl345: Add SPI support 2017-02-19 13:12 ` Jonathan Cameron @ 2017-02-20 6:46 ` Eva Rachel Retuya 0 siblings, 0 replies; 16+ messages in thread From: Eva Rachel Retuya @ 2017-02-20 6:46 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-iio, knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On Sun, Feb 19, 2017 at 01:12:50PM +0000, Jonathan Cameron wrote: > On 16/02/17 10:02, Eva Rachel Retuya wrote: > > Add SPI driver for initializing spi regmap for the adxl345 core driver. > > The driver supports the same functionality as I2C namely the x, y, z and > > scale readings. > > > > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> > Looks nice. I didn't know about the readmask stuff in regmap so > always good to see something new. > Me too. The SPI reads were unusual, and the datasheet mentions some sort of mask to enable proper consecutive register reads. Grepped the mask and discovered this. Since then, the reads are no longer incorrect :) Eva > Jonathan > > --- > > drivers/iio/accel/Kconfig | 13 +++++++ > > drivers/iio/accel/Makefile | 1 + > > drivers/iio/accel/adxl345_spi.c | 75 +++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 89 insertions(+) > > create mode 100644 drivers/iio/accel/adxl345_spi.c > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > index 5bdd87f..d474fed 100644 > > --- a/drivers/iio/accel/Kconfig > > +++ b/drivers/iio/accel/Kconfig > > @@ -21,6 +21,19 @@ config ADXL345_I2C > > To compile this driver as a module, choose M here: the > > module will be called adxl345_i2c. > > > > +config ADXL345_SPI > > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer SPI Driver" > > + depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) > > + depends on SPI > > + select ADXL345 > > + select REGMAP_SPI > > + help > > + Say Y here if you want to build support for the Analog Devices > > + ADXL345 3-axis digital accelerometer. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called adxl345_spi. > > + > > config BMA180 > > tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver" > > depends on I2C > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > index 3f4a6d6..31fba19 100644 > > --- a/drivers/iio/accel/Makefile > > +++ b/drivers/iio/accel/Makefile > > @@ -5,6 +5,7 @@ > > # When adding new entries keep the list in alphabetical order > > obj-$(CONFIG_ADXL345) += adxl345_core.o > > obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > > +obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o > > obj-$(CONFIG_BMA180) += bma180.o > > obj-$(CONFIG_BMA220) += bma220_spi.o > > obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o > > diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c > > new file mode 100644 > > index 0000000..5fcd1fa > > --- /dev/null > > +++ b/drivers/iio/accel/adxl345_spi.c > > @@ -0,0 +1,75 @@ > > +/* > > + * ADXL345 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com> > > + * > > + * This file is subject to the terms and conditions of version 2 of > > + * the GNU General Public License. See the file COPYING in the main > > + * directory of this archive for more details. > > + * > > + * SPI driver for ADXL345 > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > +#include <linux/spi/spi.h> > > + > > +#include "adxl345.h" > > + > > +#define ADXL345_MAX_SPI_FREQ_HZ 5000000 > > + > > +static const struct regmap_config adxl345_spi_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + /* Setting bits 7 and 6 enables multiple-byte read */ > > + .read_flag_mask = BIT(7) | BIT(6), > Nice. I didn't know about that one ;) > > +}; > > + > > +static int adxl345_spi_probe(struct spi_device *spi) > > +{ > > + struct regmap *regmap; > > + const struct spi_device_id *id = spi_get_device_id(spi); > > + > > + /* Bail out if max_speed_hz exceeds 5 MHz */ > > + if (spi->max_speed_hz > ADXL345_MAX_SPI_FREQ_HZ) { > > + dev_err(&spi->dev, "SPI CLK, %d Hz exceeds 5 MHz\n", > > + spi->max_speed_hz); > > + return -EINVAL; > > + } > > + > > + regmap = devm_regmap_init_spi(spi, &adxl345_spi_regmap_config); > > + if (IS_ERR(regmap)) { > > + dev_err(&spi->dev, "Error initializing spi regmap: %d\n", > > + (int)PTR_ERR(regmap)); > > + return PTR_ERR(regmap); > > + } > > + > > + return adxl345_common_probe(&spi->dev, regmap, id->name); > > +} > > + > > +static int adxl345_spi_remove(struct spi_device *spi) > > +{ > > + return adxl345_common_remove(&spi->dev); > > +} > > + > > +static const struct spi_device_id adxl345_spi_id[] = { > > + { "adxl345", 0 }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(spi, adxl345_spi_id); > > + > > +static struct spi_driver adxl345_spi_driver = { > > + .driver = { > > + .name = "adxl345_spi", > > + }, > > + .probe = adxl345_spi_probe, > > + .remove = adxl345_spi_remove, > > + .id_table = adxl345_spi_id, > > +}; > > + > > +module_spi_driver(adxl345_spi_driver); > > + > > +MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>"); > > +MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer SPI driver"); > > +MODULE_LICENSE("GPL v2"); > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/4] iio: accel: adxl345: Add ACPI support 2017-02-16 10:02 [PATCH 0/4] iio: accel: adxl345: Split driver into core and I2C then add SPI support Eva Rachel Retuya ` (2 preceding siblings ...) 2017-02-16 10:02 ` [PATCH 3/4] iio: accel: adxl345: Add SPI support Eva Rachel Retuya @ 2017-02-16 10:02 ` Eva Rachel Retuya 2017-02-19 10:01 ` Lars-Peter Clausen 3 siblings, 1 reply; 16+ messages in thread From: Eva Rachel Retuya @ 2017-02-16 10:02 UTC (permalink / raw) To: jic23, linux-iio Cc: knaack.h, lars, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel, Eva Rachel Retuya Allow probing the adxl345 on both I2C and SPI protocols using ACPI. Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com> --- drivers/iio/accel/adxl345_i2c.c | 9 +++++++++ drivers/iio/accel/adxl345_spi.c | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c index b114eb0..13080ff 100644 --- a/drivers/iio/accel/adxl345_i2c.c +++ b/drivers/iio/accel/adxl345_i2c.c @@ -12,6 +12,7 @@ * 0x53 (ALT ADDRESS pin grounded) */ +#include <linux/acpi.h> #include <linux/i2c.h> #include <linux/module.h> #include <linux/regmap.h> @@ -54,9 +55,17 @@ static const struct i2c_device_id adxl345_i2c_id[] = { MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); +static const struct acpi_device_id adxl345_acpi_id[] = { + { "ADX0345", 0 }, + { } +}; + +MODULE_DEVICE_TABLE(acpi, adxl345_acpi_id); + static struct i2c_driver adxl345_i2c_driver = { .driver = { .name = "adxl345_i2c", + .acpi_match_table = ACPI_PTR(adxl345_acpi_id), }, .probe = adxl345_i2c_probe, .remove = adxl345_i2c_remove, diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c index 5fcd1fa..3f2a1db 100644 --- a/drivers/iio/accel/adxl345_spi.c +++ b/drivers/iio/accel/adxl345_spi.c @@ -10,6 +10,7 @@ * SPI driver for ADXL345 */ +#include <linux/acpi.h> #include <linux/module.h> #include <linux/regmap.h> #include <linux/spi/spi.h> @@ -59,9 +60,17 @@ static const struct spi_device_id adxl345_spi_id[] = { MODULE_DEVICE_TABLE(spi, adxl345_spi_id); +static const struct acpi_device_id adxl345_acpi_id[] = { + { "ADX0345", 0 }, + { } +}; + +MODULE_DEVICE_TABLE(acpi, adxl345_acpi_id); + static struct spi_driver adxl345_spi_driver = { .driver = { .name = "adxl345_spi", + .acpi_match_table = ACPI_PTR(adxl345_acpi_id), }, .probe = adxl345_spi_probe, .remove = adxl345_spi_remove, -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] iio: accel: adxl345: Add ACPI support 2017-02-16 10:02 ` [PATCH 4/4] iio: accel: adxl345: Add ACPI support Eva Rachel Retuya @ 2017-02-19 10:01 ` Lars-Peter Clausen 2017-02-19 12:15 ` Eva Rachel Retuya 0 siblings, 1 reply; 16+ messages in thread From: Lars-Peter Clausen @ 2017-02-19 10:01 UTC (permalink / raw) To: Eva Rachel Retuya, jic23, linux-iio Cc: knaack.h, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On 02/16/2017 11:02 AM, Eva Rachel Retuya wrote: [...] > @@ -54,9 +55,17 @@ static const struct i2c_device_id adxl345_i2c_id[] = { > > MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); > > +static const struct acpi_device_id adxl345_acpi_id[] = { > + { "ADX0345", 0 }, Who allocated this ID? ADX does not seem to be a registered vendor ID (http://www.uefi.org/PNP_ACPI_Registry). > + { } > +}; ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] iio: accel: adxl345: Add ACPI support 2017-02-19 10:01 ` Lars-Peter Clausen @ 2017-02-19 12:15 ` Eva Rachel Retuya 2017-02-20 12:07 ` Lars-Peter Clausen 0 siblings, 1 reply; 16+ messages in thread From: Eva Rachel Retuya @ 2017-02-19 12:15 UTC (permalink / raw) To: Lars-Peter Clausen Cc: jic23, linux-iio, knaack.h, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On Sun, Feb 19, 2017 at 11:01:23AM +0100, Lars-Peter Clausen wrote: > On 02/16/2017 11:02 AM, Eva Rachel Retuya wrote: > [...] > > @@ -54,9 +55,17 @@ static const struct i2c_device_id adxl345_i2c_id[] = { > > > > MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); > > > > +static const struct acpi_device_id adxl345_acpi_id[] = { > > + { "ADX0345", 0 }, > > Who allocated this ID? ADX does not seem to be a registered vendor ID > (http://www.uefi.org/PNP_ACPI_Registry). > Hello Lars, Pardon the ignorance. I was not aware of this when I set it like that. Is "ADS0345" OK? Will submit a new version with that ID. Thanks, Eva > > + { } > > +}; > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] iio: accel: adxl345: Add ACPI support 2017-02-19 12:15 ` Eva Rachel Retuya @ 2017-02-20 12:07 ` Lars-Peter Clausen 2017-02-21 15:18 ` Eva Rachel Retuya 0 siblings, 1 reply; 16+ messages in thread From: Lars-Peter Clausen @ 2017-02-20 12:07 UTC (permalink / raw) To: jic23, linux-iio, knaack.h, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On 02/19/2017 01:15 PM, Eva Rachel Retuya wrote: > On Sun, Feb 19, 2017 at 11:01:23AM +0100, Lars-Peter Clausen wrote: >> On 02/16/2017 11:02 AM, Eva Rachel Retuya wrote: >> [...] >>> @@ -54,9 +55,17 @@ static const struct i2c_device_id adxl345_i2c_id[] = { >>> >>> MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); >>> >>> +static const struct acpi_device_id adxl345_acpi_id[] = { >>> + { "ADX0345", 0 }, >> >> Who allocated this ID? ADX does not seem to be a registered vendor ID >> (http://www.uefi.org/PNP_ACPI_Registry). >> > > Hello Lars, > > Pardon the ignorance. I was not aware of this when I set it like that. > Is "ADS0345" OK? Will submit a new version with that ID. Excellent question. ACPI is not like devicetree where we can just randomly choose IDs. The namespaces are more controlled. The vendor IDs are managed and allocated by the UEFI forum, each vendor then allocates device IDs in its vendor namespace for specific purposes. Unless you own a vendor ID or the device ID has been allocated by the vendor for you you shouldn't use the ID. If you have a ACPI based system which features the adxl345 maybe using PRP0001[1] might be the better approach. - Lars [1] https://www.kernel.org/doc/Documentation/acpi/enumeration.txt ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/4] iio: accel: adxl345: Add ACPI support 2017-02-20 12:07 ` Lars-Peter Clausen @ 2017-02-21 15:18 ` Eva Rachel Retuya 0 siblings, 0 replies; 16+ messages in thread From: Eva Rachel Retuya @ 2017-02-21 15:18 UTC (permalink / raw) To: Lars-Peter Clausen Cc: jic23, linux-iio, knaack.h, pmeerw, dmitry.torokhov, michael.hennerich, daniel.baluta, amsfield22, florian.vaussard, linux-kernel On Mon, Feb 20, 2017 at 01:07:38PM +0100, Lars-Peter Clausen wrote: > On 02/19/2017 01:15 PM, Eva Rachel Retuya wrote: > > On Sun, Feb 19, 2017 at 11:01:23AM +0100, Lars-Peter Clausen wrote: > >> On 02/16/2017 11:02 AM, Eva Rachel Retuya wrote: > >> [...] > >>> @@ -54,9 +55,17 @@ static const struct i2c_device_id adxl345_i2c_id[] = { > >>> > >>> MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id); > >>> > >>> +static const struct acpi_device_id adxl345_acpi_id[] = { > >>> + { "ADX0345", 0 }, > >> > >> Who allocated this ID? ADX does not seem to be a registered vendor ID > >> (http://www.uefi.org/PNP_ACPI_Registry). > >> > > > > Hello Lars, > > > > Pardon the ignorance. I was not aware of this when I set it like that. > > Is "ADS0345" OK? Will submit a new version with that ID. > > Excellent question. ACPI is not like devicetree where we can just randomly > choose IDs. The namespaces are more controlled. The vendor IDs are managed > and allocated by the UEFI forum, each vendor then allocates device IDs in > its vendor namespace for specific purposes. Unless you own a vendor ID or > the device ID has been allocated by the vendor for you you shouldn't use the ID. > > If you have a ACPI based system which features the adxl345 maybe using > PRP0001[1] might be the better approach. > > - Lars > > [1] https://www.kernel.org/doc/Documentation/acpi/enumeration.txt Since the ID has not been allocated yet, will drop this patch at the moment. Thanks for the explanation. Eva > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-02-21 15:18 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-16 10:02 [PATCH 0/4] iio: accel: adxl345: Split driver into core and I2C then add SPI support Eva Rachel Retuya 2017-02-16 10:02 ` [PATCH 1/4] iio: accel: adxl345: Use I2C regmap instead of direct I2C access Eva Rachel Retuya 2017-02-19 13:04 ` Jonathan Cameron 2017-02-16 10:02 ` [PATCH 2/4] iio: accel: adxl345: Split driver into core and I2C Eva Rachel Retuya 2017-02-19 13:11 ` Jonathan Cameron 2017-02-20 6:41 ` Eva Rachel Retuya 2017-02-20 9:32 ` Eva Rachel Retuya 2017-02-20 11:47 ` Andy Shevchenko 2017-02-16 10:02 ` [PATCH 3/4] iio: accel: adxl345: Add SPI support Eva Rachel Retuya 2017-02-19 13:12 ` Jonathan Cameron 2017-02-20 6:46 ` Eva Rachel Retuya 2017-02-16 10:02 ` [PATCH 4/4] iio: accel: adxl345: Add ACPI support Eva Rachel Retuya 2017-02-19 10:01 ` Lars-Peter Clausen 2017-02-19 12:15 ` Eva Rachel Retuya 2017-02-20 12:07 ` Lars-Peter Clausen 2017-02-21 15:18 ` Eva Rachel Retuya
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.