* [PATCH v3 0/2] iio: accel: Add support for ADXL313 accelerometer @ 2021-08-11 21:16 Lucas Stankus 2021-08-11 21:17 ` [PATCH v3 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313 Lucas Stankus 2021-08-11 21:18 ` [PATCH v3 2/2] iio: accel: Add driver support " Lucas Stankus 0 siblings, 2 replies; 10+ messages in thread From: Lucas Stankus @ 2021-08-11 21:16 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, robh+dt, Dragos.Bogdan, Darius.Berghe Cc: linux-iio, devicetree, linux-kernel Add driver support and dt-bindings documentation for ADXL313 digital accelerometer. Changelog v2 -> v3: - Add minItems to interrupts and interrupt-names in the dt doc - Move spi cpol and cpha configuration from the dt doc down to the driver - Use wild card for adxl313 files in MAINTAINERS - Use acceleration scale directly instead of defining a macro for it - Use clamp_val to check if calibbias input value is in range - Add comma to the last fields of the regmap configuration structs Lucas Stankus (2): dt-bindings: iio: accel: Add binding documentation for ADXL313 iio: accel: Add driver support for ADXL313 .../bindings/iio/accel/adi,adxl313.yaml | 86 +++++ MAINTAINERS | 6 + drivers/iio/accel/Kconfig | 29 ++ drivers/iio/accel/Makefile | 3 + drivers/iio/accel/adxl313.h | 57 +++ drivers/iio/accel/adxl313_core.c | 331 ++++++++++++++++++ drivers/iio/accel/adxl313_i2c.c | 65 ++++ drivers/iio/accel/adxl313_spi.c | 91 +++++ 8 files changed, 668 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml create mode 100644 drivers/iio/accel/adxl313.h create mode 100644 drivers/iio/accel/adxl313_core.c create mode 100644 drivers/iio/accel/adxl313_i2c.c create mode 100644 drivers/iio/accel/adxl313_spi.c -- 2.32.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313 2021-08-11 21:16 [PATCH v3 0/2] iio: accel: Add support for ADXL313 accelerometer Lucas Stankus @ 2021-08-11 21:17 ` Lucas Stankus 2021-08-17 21:22 ` Rob Herring 2021-08-11 21:18 ` [PATCH v3 2/2] iio: accel: Add driver support " Lucas Stankus 1 sibling, 1 reply; 10+ messages in thread From: Lucas Stankus @ 2021-08-11 21:17 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, robh+dt, Dragos.Bogdan, Darius.Berghe Cc: linux-iio, devicetree, linux-kernel Add device tree binding documentation for ADXL313 3-axis accelerometer. Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com> --- .../bindings/iio/accel/adi,adxl313.yaml | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml new file mode 100644 index 000000000000..d6afc1b8c272 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml @@ -0,0 +1,86 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/accel/adi,adxl313.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices ADXL313 3-Axis Digital Accelerometer + +maintainers: + - Lucas Stankus <lucas.p.stankus@gmail.com> + +description: | + Analog Devices ADXL313 3-Axis Digital Accelerometer that supports + both I2C & SPI interfaces. + https://www.analog.com/en/products/adxl313.html + +properties: + compatible: + enum: + - adi,adxl313 + + reg: + maxItems: 1 + + spi-3wire: true + + spi-max-frequency: true + + vs-supply: + description: Regulator that supplies power to the accelerometer + + vdd-supply: + description: Regulator that supplies the digital interface supply voltage + + interrupts: + minItems: 1 + maxItems: 2 + + interrupt-names: + minItems: 1 + maxItems: 2 + items: + enum: + - INT1 + - INT2 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + /* Example for a I2C device node */ + accelerometer@53 { + compatible = "adi,adxl313"; + reg = <0x53>; + interrupt-parent = <&gpio0>; + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "INT1"; + }; + }; + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + spi { + #address-cells = <1>; + #size-cells = <0>; + + /* Example for a SPI device node */ + accelerometer@0 { + compatible = "adi,adxl313"; + reg = <0>; + spi-max-frequency = <5000000>; + interrupt-parent = <&gpio0>; + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "INT1"; + }; + }; -- 2.32.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313 2021-08-11 21:17 ` [PATCH v3 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313 Lucas Stankus @ 2021-08-17 21:22 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2021-08-17 21:22 UTC (permalink / raw) To: Lucas Stankus Cc: Michael.Hennerich, linux-iio, Dragos.Bogdan, Darius.Berghe, devicetree, lars, jic23, linux-kernel, robh+dt On Wed, 11 Aug 2021 18:17:07 -0300, Lucas Stankus wrote: > Add device tree binding documentation for ADXL313 3-axis accelerometer. > > Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com> > --- > .../bindings/iio/accel/adi,adxl313.yaml | 86 +++++++++++++++++++ > 1 file changed, 86 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml > Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] iio: accel: Add driver support for ADXL313 2021-08-11 21:16 [PATCH v3 0/2] iio: accel: Add support for ADXL313 accelerometer Lucas Stankus 2021-08-11 21:17 ` [PATCH v3 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313 Lucas Stankus @ 2021-08-11 21:18 ` Lucas Stankus 2021-08-12 11:05 ` Andy Shevchenko 2021-08-15 15:26 ` Jonathan Cameron 1 sibling, 2 replies; 10+ messages in thread From: Lucas Stankus @ 2021-08-11 21:18 UTC (permalink / raw) To: lars, Michael.Hennerich, jic23, robh+dt, Dragos.Bogdan, Darius.Berghe Cc: linux-iio, devicetree, linux-kernel ADXL313 is a small, thin, low power, 3-axis accelerometer with high resolution measurement up to +/-4g. It includes an integrated 32-level FIFO and has activity and inactivity sensing capabilities. Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com> Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com> --- MAINTAINERS | 6 + drivers/iio/accel/Kconfig | 29 +++ drivers/iio/accel/Makefile | 3 + drivers/iio/accel/adxl313.h | 57 ++++++ drivers/iio/accel/adxl313_core.c | 331 +++++++++++++++++++++++++++++++ drivers/iio/accel/adxl313_i2c.c | 65 ++++++ drivers/iio/accel/adxl313_spi.c | 91 +++++++++ 7 files changed, 582 insertions(+) create mode 100644 drivers/iio/accel/adxl313.h create mode 100644 drivers/iio/accel/adxl313_core.c create mode 100644 drivers/iio/accel/adxl313_i2c.c create mode 100644 drivers/iio/accel/adxl313_spi.c diff --git a/MAINTAINERS b/MAINTAINERS index d0464b83b5b5..b2ada8ca1453 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -585,6 +585,12 @@ L: platform-driver-x86@vger.kernel.org S: Maintained F: drivers/platform/x86/adv_swbutton.c +ADXL313 THREE-AXIS DIGITAL ACCELEROMETER DRIVER +M: Lucas Stankus <lucas.p.stankus@gmail.com> +S: Supported +F: Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml +F: drivers/iio/accel/adxl313* + ADXL34X THREE-AXIS DIGITAL ACCELEROMETER DRIVER (ADXL345/ADXL346) M: Michael Hennerich <michael.hennerich@analog.com> S: Supported diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig index 2f0c0d512ae7..fb8349a88cd3 100644 --- a/drivers/iio/accel/Kconfig +++ b/drivers/iio/accel/Kconfig @@ -30,6 +30,35 @@ config ADIS16209 To compile this driver as a module, say M here: the module will be called adis16209. +config ADXL313 + tristate + +config ADXL313_I2C + tristate "Analog Devices ADXL313 3-Axis Digital Accelerometer I2C Driver" + depends on I2C + select ADXL313 + select REGMAP_I2C + help + Say Y here if you want to build i2c support for the Analog Devices + ADXL313 3-axis digital accelerometer. + + To compile this driver as a module, choose M here: the module + will be called adxl313_i2c and you will also get adxl313_core + for the core module. + +config ADXL313_SPI + tristate "Analog Devices ADXL313 3-Axis Digital Accelerometer SPI Driver" + depends on SPI + select ADXL313 + select REGMAP_SPI + help + Say Y here if you want to build spi support for the Analog Devices + ADXL313 3-axis digital accelerometer. + + To compile this driver as a module, choose M here: the module + will be called adxl313_spi and you will also get adxl313_core + for the core module. + config ADXL345 tristate diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile index 89280e823bcd..fadc92816e24 100644 --- a/drivers/iio/accel/Makefile +++ b/drivers/iio/accel/Makefile @@ -6,6 +6,9 @@ # When adding new entries keep the list in alphabetical order obj-$(CONFIG_ADIS16201) += adis16201.o obj-$(CONFIG_ADIS16209) += adis16209.o +obj-$(CONFIG_ADXL313) += adxl313_core.o +obj-$(CONFIG_ADXL313_I2C) += adxl313_i2c.o +obj-$(CONFIG_ADXL313_SPI) += adxl313_spi.o obj-$(CONFIG_ADXL345) += adxl345_core.o obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h new file mode 100644 index 000000000000..c170b5236988 --- /dev/null +++ b/drivers/iio/accel/adxl313.h @@ -0,0 +1,57 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * ADXL313 3-Axis Digital Accelerometer + * + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> + */ + +#ifndef _ADXL313_H_ +#define _ADXL313_H_ + +/* ADXL313 register definitions */ +#define ADXL313_REG_DEVID0 0x00 +#define ADXL313_REG_DEVID1 0x01 +#define ADXL313_REG_PARTID 0x02 +#define ADXL313_REG_XID 0x04 +#define ADXL313_REG_SOFT_RESET 0x18 +#define ADXL313_REG_OFS_AXIS(index) (0x1E + (index)) +#define ADXL313_REG_THRESH_ACT 0x24 +#define ADXL313_REG_ACT_INACT_CTL 0x27 +#define ADXL313_REG_BW_RATE 0x2C +#define ADXL313_REG_POWER_CTL 0x2D +#define ADXL313_REG_INT_MAP 0x2F +#define ADXL313_REG_DATA_FORMAT 0x31 +#define ADXL313_REG_DATAX 0x32 +#define ADXL313_REG_DATAY 0x34 +#define ADXL313_REG_DATAZ 0x36 +#define ADXL313_REG_FIFO_CTL 0x38 +#define ADXL313_REG_FIFO_STATUS 0x39 + +#define ADXL313_DEVID0 0xAD +#define ADXL313_DEVID1 0x1D +#define ADXL313_PARTID 0xCB +#define ADXL313_SOFT_RESET 0x52 + +#define ADXL313_RATE_MSK GENMASK(3, 0) +#define ADXL313_RATE_BASE 6 + +#define ADXL313_POWER_CTL_MSK GENMASK(3, 2) +#define ADXL313_MEASUREMENT_MODE BIT(3) + +#define ADXL313_RANGE_MSK GENMASK(1, 0) +#define ADXL313_RANGE_4G 3 + +#define ADXL313_FULL_RES BIT(3) +#define ADXL313_SPI_3WIRE BIT(6) +#define ADXL313_I2C_DISABLE BIT(6) + +extern const struct regmap_access_table adxl313_readable_regs_table; + +extern const struct regmap_access_table adxl313_writable_regs_table; + +int adxl313_core_probe(struct device *dev, + struct regmap *regmap, + const char *name, + int (*interface_specific_setup)(struct device *, + struct regmap *)); +#endif /* _ADXL313_H_ */ diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c new file mode 100644 index 000000000000..8f3d4f5281c4 --- /dev/null +++ b/drivers/iio/accel/adxl313_core.c @@ -0,0 +1,331 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ADXL313 3-Axis Digital Accelerometer + * + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> + * + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf + */ + +#include <linux/bitfield.h> +#include <linux/iio/iio.h> +#include <linux/module.h> +#include <linux/regmap.h> + +#include "adxl313.h" + +const struct regmap_range adxl313_readable_reg_range[] = { + regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_XID), + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_FIFO_STATUS) +}; + +const struct regmap_access_table adxl313_readable_regs_table = { + .yes_ranges = adxl313_readable_reg_range, + .n_yes_ranges = ARRAY_SIZE(adxl313_readable_reg_range) +}; +EXPORT_SYMBOL_GPL(adxl313_readable_regs_table); + +const struct regmap_range adxl313_writable_reg_range[] = { + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_INT_MAP), + regmap_reg_range(ADXL313_REG_DATA_FORMAT, ADXL313_REG_DATA_FORMAT), + regmap_reg_range(ADXL313_REG_FIFO_CTL, ADXL313_REG_FIFO_CTL) +}; + +const struct regmap_access_table adxl313_writable_regs_table = { + .yes_ranges = adxl313_writable_reg_range, + .n_yes_ranges = ARRAY_SIZE(adxl313_writable_reg_range) +}; +EXPORT_SYMBOL_GPL(adxl313_writable_regs_table); + +struct adxl313_data { + struct regmap *regmap; + struct mutex lock; /* lock to protect transf_buf */ + __le16 transf_buf ____cacheline_aligned; +}; + +static const int adxl313_odr_freqs[][2] = { + [0] = { 6, 250000 }, + [1] = { 12, 500000 }, + [2] = { 25, 0 }, + [3] = { 50, 0 }, + [4] = { 100, 0 }, + [5] = { 200, 0 }, + [6] = { 400, 0 }, + [7] = { 800, 0 }, + [8] = { 1600, 0 }, + [9] = { 3200, 0 }, +}; + +#define ADXL313_ACCEL_CHANNEL(index, addr, axis) { \ + .type = IIO_ACCEL, \ + .address = addr, \ + .modified = 1, \ + .channel2 = IIO_MOD_##axis, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_CALIBBIAS), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ + .info_mask_shared_by_type_available = \ + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ + .scan_index = index, \ + .scan_type = { \ + .sign = 's', \ + .realbits = 13, \ + .storagebits = 16, \ + .endianness = IIO_LE, \ + }, \ +} + +static const struct iio_chan_spec adxl313_channels[] = { + ADXL313_ACCEL_CHANNEL(0, ADXL313_REG_DATAX, X), + ADXL313_ACCEL_CHANNEL(1, ADXL313_REG_DATAY, Y), + ADXL313_ACCEL_CHANNEL(2, ADXL313_REG_DATAZ, Z), +}; + +static int adxl313_set_odr(struct adxl313_data *data, + unsigned int freq1, unsigned int freq2) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(adxl313_odr_freqs); i++) { + if (adxl313_odr_freqs[i][0] == freq1 && + adxl313_odr_freqs[i][1] == freq2) + break; + } + + if (i == ARRAY_SIZE(adxl313_odr_freqs)) + return -EINVAL; + + return regmap_update_bits(data->regmap, ADXL313_REG_BW_RATE, + ADXL313_RATE_MSK, + FIELD_PREP(ADXL313_RATE_MSK, + ADXL313_RATE_BASE + i)); +} + +static int adxl313_read_axis(struct adxl313_data *data, + struct iio_chan_spec const *chan) +{ + int ret; + + mutex_lock(&data->lock); + + ret = regmap_bulk_read(data->regmap, + chan->address, + &data->transf_buf, 2); + if (ret) + goto unlock_ret; + + ret = le16_to_cpu(data->transf_buf); + +unlock_ret: + mutex_unlock(&data->lock); + return ret; +} + +static int adxl313_read_freq_avail(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + const int **vals, int *type, int *length, + long mask) +{ + switch (mask) { + case IIO_CHAN_INFO_SAMP_FREQ: + *vals = (const int *)adxl313_odr_freqs; + *length = ARRAY_SIZE(adxl313_odr_freqs) * 2; + *type = IIO_VAL_INT_PLUS_MICRO; + return IIO_AVAIL_LIST; + default: + return -EINVAL; + } +} + +static int adxl313_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct adxl313_data *data = iio_priv(indio_dev); + unsigned int regval; + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + ret = adxl313_read_axis(data, chan); + if (ret < 0) + return ret; + + *val = sign_extend32(ret, chan->scan_type.realbits - 1); + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + /* + * Scale for any g range is given in datasheet as + * 1024 LSB/g = 0.0009765625 * 9.80665 = 0.009576806640625 m/s^2 + */ + *val = 0; + *val2 = 9576806; + return IIO_VAL_INT_PLUS_NANO; + case IIO_CHAN_INFO_CALIBBIAS: + ret = regmap_read(data->regmap, + ADXL313_REG_OFS_AXIS(chan->scan_index), + ®val); + if (ret) + return ret; + + /* + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale + * factor at full resolution + */ + *val = sign_extend32(regval, 7) * 4; + return IIO_VAL_INT; + case IIO_CHAN_INFO_SAMP_FREQ: + ret = regmap_read(data->regmap, ADXL313_REG_BW_RATE, ®val); + if (ret) + return ret; + + ret = FIELD_GET(ADXL313_RATE_MSK, regval) - ADXL313_RATE_BASE; + *val = adxl313_odr_freqs[ret][0]; + *val2 = adxl313_odr_freqs[ret][1]; + return IIO_VAL_INT_PLUS_MICRO; + default: + return -EINVAL; + } +} + +static int adxl313_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) +{ + struct adxl313_data *data = iio_priv(indio_dev); + + switch (mask) { + case IIO_CHAN_INFO_CALIBBIAS: + /* + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale + * factor at full resolution + */ + if (clamp_val(val, -128 * 4, 127 * 4) != val) + return -EINVAL; + + return regmap_write(data->regmap, + ADXL313_REG_OFS_AXIS(chan->scan_index), + val / 4); + case IIO_CHAN_INFO_SAMP_FREQ: + return adxl313_set_odr(data, val, val2); + default: + return -EINVAL; + } +} + +static const struct iio_info adxl313_info = { + .read_raw = adxl313_read_raw, + .write_raw = adxl313_write_raw, + .read_avail = adxl313_read_freq_avail +}; + +static int adxl313_setup(struct device *dev, struct adxl313_data *data, + int (*interface_specific_setup)(struct device *, + struct regmap *)) +{ + unsigned int regval; + int ret; + + /* Ensures the device is in a consistent state after start up */ + ret = regmap_write(data->regmap, ADXL313_REG_SOFT_RESET, + ADXL313_SOFT_RESET); + if (ret) + return ret; + + if (interface_specific_setup) { + ret = interface_specific_setup(dev, data->regmap); + if (ret) + return ret; + } + + ret = regmap_read(data->regmap, ADXL313_REG_DEVID0, ®val); + if (ret) + return ret; + + if (regval != ADXL313_DEVID0) { + dev_err(dev, "Invalid manufacturer ID: 0x%02x\n", regval); + return -ENODEV; + } + + ret = regmap_read(data->regmap, ADXL313_REG_DEVID1, ®val); + if (ret) + return ret; + + if (regval != ADXL313_DEVID1) { + dev_err(dev, "Invalid mems ID: 0x%02x\n", regval); + return -ENODEV; + } + + ret = regmap_read(data->regmap, ADXL313_REG_PARTID, ®val); + if (ret) + return ret; + + if (regval != ADXL313_PARTID) { + dev_err(dev, "Invalid device ID: 0x%02x\n", regval); + return -ENODEV; + } + + /* Sets the range to +/- 4g */ + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, + ADXL313_RANGE_MSK, + FIELD_PREP(ADXL313_RANGE_MSK, + ADXL313_RANGE_4G)); + if (ret) + return ret; + + /* Enables full resolution */ + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, + ADXL313_FULL_RES, ADXL313_FULL_RES); + if (ret) + return ret; + + /* Enables measurement mode */ + return regmap_update_bits(data->regmap, ADXL313_REG_POWER_CTL, + ADXL313_POWER_CTL_MSK, + ADXL313_MEASUREMENT_MODE); +} + +int adxl313_core_probe(struct device *dev, + struct regmap *regmap, + const char *name, + int (*interface_specific_setup)(struct device *, + struct regmap *)) +{ + struct adxl313_data *data; + struct iio_dev *indio_dev; + int ret; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + data->regmap = regmap; + mutex_init(&data->lock); + + indio_dev->name = name; + indio_dev->info = &adxl313_info; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->channels = adxl313_channels; + indio_dev->num_channels = ARRAY_SIZE(adxl313_channels); + + ret = adxl313_setup(dev, data, interface_specific_setup); + if (ret) { + dev_err(dev, "ADXL313 setup failed\n"); + return ret; + } + + return devm_iio_device_register(dev, indio_dev); +} +EXPORT_SYMBOL_GPL(adxl313_core_probe); + +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer core driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/iio/accel/adxl313_i2c.c b/drivers/iio/accel/adxl313_i2c.c new file mode 100644 index 000000000000..517c6f1c43ad --- /dev/null +++ b/drivers/iio/accel/adxl313_i2c.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ADXL313 3-Axis Digital Accelerometer + * + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> + * + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf + */ + +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/regmap.h> + +#include "adxl313.h" + +static const struct regmap_config adxl313_i2c_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .rd_table = &adxl313_readable_regs_table, + .wr_table = &adxl313_writable_regs_table, + .max_register = 0x39, +}; + +static int adxl313_i2c_probe(struct i2c_client *client) +{ + struct regmap *regmap; + + regmap = devm_regmap_init_i2c(client, &adxl313_i2c_regmap_config); + if (IS_ERR(regmap)) { + dev_err(&client->dev, "Error initializing i2c regmap: %ld\n", + PTR_ERR(regmap)); + return PTR_ERR(regmap); + } + + return adxl313_core_probe(&client->dev, regmap, client->name, NULL); +} + +static const struct i2c_device_id adxl313_i2c_id[] = { + { "adxl313" }, + { } +}; + +MODULE_DEVICE_TABLE(i2c, adxl313_i2c_id); + +static const struct of_device_id adxl313_of_match[] = { + { .compatible = "adi,adxl313" }, + { } +}; + +MODULE_DEVICE_TABLE(of, adxl313_of_match); + +static struct i2c_driver adxl313_i2c_driver = { + .driver = { + .name = "adxl313_i2c", + .of_match_table = adxl313_of_match, + }, + .probe_new = adxl313_i2c_probe, + .id_table = adxl313_i2c_id +}; + +module_i2c_driver(adxl313_i2c_driver); + +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer I2C driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c new file mode 100644 index 000000000000..4d625b1753a5 --- /dev/null +++ b/drivers/iio/accel/adxl313_spi.c @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ADXL313 3-Axis Digital Accelerometer + * + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> + * + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf + */ + +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/spi/spi.h> + +#include "adxl313.h" + +static const struct regmap_config adxl313_spi_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .rd_table = &adxl313_readable_regs_table, + .wr_table = &adxl313_writable_regs_table, + .max_register = 0x39, + /* Setting bits 7 and 6 enables multiple-byte read */ + .read_flag_mask = BIT(7) | BIT(6), +}; + +static int adxl313_spi_setup(struct device *dev, struct regmap *regmap) +{ + struct spi_device *spi = container_of(dev, struct spi_device, dev); + int ret; + + if (spi->mode & SPI_3WIRE) { + ret = regmap_write(regmap, ADXL313_REG_DATA_FORMAT, + ADXL313_SPI_3WIRE); + if (ret) + return ret; + } + + return regmap_update_bits(regmap, ADXL313_REG_POWER_CTL, + ADXL313_I2C_DISABLE, ADXL313_I2C_DISABLE); +} + +static int adxl313_spi_probe(struct spi_device *spi) +{ + const struct spi_device_id *id = spi_get_device_id(spi); + struct regmap *regmap; + int ret; + + spi->mode |= SPI_MODE_3; + ret = spi_setup(spi); + if (ret) + return ret; + + regmap = devm_regmap_init_spi(spi, &adxl313_spi_regmap_config); + if (IS_ERR(regmap)) { + dev_err(&spi->dev, "Error initializing spi regmap: %ld\n", + PTR_ERR(regmap)); + return PTR_ERR(regmap); + } + + return adxl313_core_probe(&spi->dev, regmap, id->name, + &adxl313_spi_setup); +} + +static const struct spi_device_id adxl313_spi_id[] = { + { "adxl313" }, + { } +}; + +MODULE_DEVICE_TABLE(spi, adxl313_spi_id); + +static const struct of_device_id adxl313_of_match[] = { + { .compatible = "adi,adxl313" }, + { } +}; + +MODULE_DEVICE_TABLE(of, adxl313_of_match); + +static struct spi_driver adxl313_spi_driver = { + .driver = { + .name = "adxl313_spi", + .of_match_table = adxl313_of_match, + }, + .probe = adxl313_spi_probe, + .id_table = adxl313_spi_id +}; + +module_spi_driver(adxl313_spi_driver); + +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer SPI driver"); +MODULE_LICENSE("GPL v2"); -- 2.32.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] iio: accel: Add driver support for ADXL313 2021-08-11 21:18 ` [PATCH v3 2/2] iio: accel: Add driver support " Lucas Stankus @ 2021-08-12 11:05 ` Andy Shevchenko 2021-08-18 20:01 ` Lucas Stankus 2021-08-15 15:26 ` Jonathan Cameron 1 sibling, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2021-08-12 11:05 UTC (permalink / raw) To: Lucas Stankus Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, Rob Herring, Bogdan, Dragos, Darius, linux-iio, devicetree, Linux Kernel Mailing List On Thu, Aug 12, 2021 at 12:19 AM Lucas Stankus <lucas.p.stankus@gmail.com> wrote: > > ADXL313 is a small, thin, low power, 3-axis accelerometer with high > resolution measurement up to +/-4g. It includes an integrated 32-level > FIFO and has activity and inactivity sensing capabilities. Thanks for an update, my comments below. After addressing them, feel free to add Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com> > Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com> > --- > MAINTAINERS | 6 + > drivers/iio/accel/Kconfig | 29 +++ > drivers/iio/accel/Makefile | 3 + > drivers/iio/accel/adxl313.h | 57 ++++++ > drivers/iio/accel/adxl313_core.c | 331 +++++++++++++++++++++++++++++++ > drivers/iio/accel/adxl313_i2c.c | 65 ++++++ > drivers/iio/accel/adxl313_spi.c | 91 +++++++++ > 7 files changed, 582 insertions(+) > create mode 100644 drivers/iio/accel/adxl313.h > create mode 100644 drivers/iio/accel/adxl313_core.c > create mode 100644 drivers/iio/accel/adxl313_i2c.c > create mode 100644 drivers/iio/accel/adxl313_spi.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d0464b83b5b5..b2ada8ca1453 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -585,6 +585,12 @@ L: platform-driver-x86@vger.kernel.org > S: Maintained > F: drivers/platform/x86/adv_swbutton.c > > +ADXL313 THREE-AXIS DIGITAL ACCELEROMETER DRIVER > +M: Lucas Stankus <lucas.p.stankus@gmail.com> > +S: Supported > +F: Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml > +F: drivers/iio/accel/adxl313* > + > ADXL34X THREE-AXIS DIGITAL ACCELEROMETER DRIVER (ADXL345/ADXL346) > M: Michael Hennerich <michael.hennerich@analog.com> > S: Supported > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > index 2f0c0d512ae7..fb8349a88cd3 100644 > --- a/drivers/iio/accel/Kconfig > +++ b/drivers/iio/accel/Kconfig > @@ -30,6 +30,35 @@ config ADIS16209 > To compile this driver as a module, say M here: the module will be > called adis16209. > > +config ADXL313 > + tristate > + > +config ADXL313_I2C > + tristate "Analog Devices ADXL313 3-Axis Digital Accelerometer I2C Driver" > + depends on I2C > + select ADXL313 > + select REGMAP_I2C > + help > + Say Y here if you want to build i2c support for the Analog Devices > + ADXL313 3-axis digital accelerometer. > + > + To compile this driver as a module, choose M here: the module > + will be called adxl313_i2c and you will also get adxl313_core > + for the core module. > + > +config ADXL313_SPI > + tristate "Analog Devices ADXL313 3-Axis Digital Accelerometer SPI Driver" > + depends on SPI > + select ADXL313 > + select REGMAP_SPI > + help > + Say Y here if you want to build spi support for the Analog Devices > + ADXL313 3-axis digital accelerometer. > + > + To compile this driver as a module, choose M here: the module > + will be called adxl313_spi and you will also get adxl313_core > + for the core module. > + > config ADXL345 > tristate > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > index 89280e823bcd..fadc92816e24 100644 > --- a/drivers/iio/accel/Makefile > +++ b/drivers/iio/accel/Makefile > @@ -6,6 +6,9 @@ > # When adding new entries keep the list in alphabetical order > obj-$(CONFIG_ADIS16201) += adis16201.o > obj-$(CONFIG_ADIS16209) += adis16209.o > +obj-$(CONFIG_ADXL313) += adxl313_core.o > +obj-$(CONFIG_ADXL313_I2C) += adxl313_i2c.o > +obj-$(CONFIG_ADXL313_SPI) += adxl313_spi.o > obj-$(CONFIG_ADXL345) += adxl345_core.o > obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o > diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h > new file mode 100644 > index 000000000000..c170b5236988 > --- /dev/null > +++ b/drivers/iio/accel/adxl313.h > @@ -0,0 +1,57 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * ADXL313 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > + */ > + > +#ifndef _ADXL313_H_ > +#define _ADXL313_H_ > + > +/* ADXL313 register definitions */ > +#define ADXL313_REG_DEVID0 0x00 > +#define ADXL313_REG_DEVID1 0x01 > +#define ADXL313_REG_PARTID 0x02 > +#define ADXL313_REG_XID 0x04 > +#define ADXL313_REG_SOFT_RESET 0x18 > +#define ADXL313_REG_OFS_AXIS(index) (0x1E + (index)) > +#define ADXL313_REG_THRESH_ACT 0x24 > +#define ADXL313_REG_ACT_INACT_CTL 0x27 > +#define ADXL313_REG_BW_RATE 0x2C > +#define ADXL313_REG_POWER_CTL 0x2D > +#define ADXL313_REG_INT_MAP 0x2F > +#define ADXL313_REG_DATA_FORMAT 0x31 > +#define ADXL313_REG_DATAX 0x32 > +#define ADXL313_REG_DATAY 0x34 > +#define ADXL313_REG_DATAZ 0x36 > +#define ADXL313_REG_FIFO_CTL 0x38 > +#define ADXL313_REG_FIFO_STATUS 0x39 > + > +#define ADXL313_DEVID0 0xAD > +#define ADXL313_DEVID1 0x1D > +#define ADXL313_PARTID 0xCB > +#define ADXL313_SOFT_RESET 0x52 > + > +#define ADXL313_RATE_MSK GENMASK(3, 0) > +#define ADXL313_RATE_BASE 6 > + > +#define ADXL313_POWER_CTL_MSK GENMASK(3, 2) > +#define ADXL313_MEASUREMENT_MODE BIT(3) > + > +#define ADXL313_RANGE_MSK GENMASK(1, 0) > +#define ADXL313_RANGE_4G 3 > + > +#define ADXL313_FULL_RES BIT(3) > +#define ADXL313_SPI_3WIRE BIT(6) > +#define ADXL313_I2C_DISABLE BIT(6) > + > +extern const struct regmap_access_table adxl313_readable_regs_table; > + > +extern const struct regmap_access_table adxl313_writable_regs_table; > + > +int adxl313_core_probe(struct device *dev, > + struct regmap *regmap, > + const char *name, > + int (*interface_specific_setup)(struct device *, > + struct regmap *)); I think it's quite a long name for an internal function. At least "specific" word may be dropped, but I will go even for "interface" drop and explaining this parameter in the kernel doc of the probe() function/ > +#endif /* _ADXL313_H_ */ > diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c > new file mode 100644 > index 000000000000..8f3d4f5281c4 > --- /dev/null > +++ b/drivers/iio/accel/adxl313_core.c > @@ -0,0 +1,331 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * ADXL313 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > + * > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > + */ > + > +#include <linux/bitfield.h> > +#include <linux/iio/iio.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > + > +#include "adxl313.h" > + > +const struct regmap_range adxl313_readable_reg_range[] = { > + regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_XID), > + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), > + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), > + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), > + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_FIFO_STATUS) + comma (the rule of thumb is to check all similar code pieces even if you got a comment only once) > +}; > + > +const struct regmap_access_table adxl313_readable_regs_table = { > + .yes_ranges = adxl313_readable_reg_range, > + .n_yes_ranges = ARRAY_SIZE(adxl313_readable_reg_range) + Comma > +}; > +EXPORT_SYMBOL_GPL(adxl313_readable_regs_table); > + > +const struct regmap_range adxl313_writable_reg_range[] = { > + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), > + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), > + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), > + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_INT_MAP), > + regmap_reg_range(ADXL313_REG_DATA_FORMAT, ADXL313_REG_DATA_FORMAT), > + regmap_reg_range(ADXL313_REG_FIFO_CTL, ADXL313_REG_FIFO_CTL) Ditto. > +}; > + > +const struct regmap_access_table adxl313_writable_regs_table = { > + .yes_ranges = adxl313_writable_reg_range, > + .n_yes_ranges = ARRAY_SIZE(adxl313_writable_reg_range) Ditto. > +}; > +EXPORT_SYMBOL_GPL(adxl313_writable_regs_table); > + > +struct adxl313_data { > + struct regmap *regmap; > + struct mutex lock; /* lock to protect transf_buf */ > + __le16 transf_buf ____cacheline_aligned; > +}; > + > +static const int adxl313_odr_freqs[][2] = { > + [0] = { 6, 250000 }, > + [1] = { 12, 500000 }, > + [2] = { 25, 0 }, > + [3] = { 50, 0 }, > + [4] = { 100, 0 }, > + [5] = { 200, 0 }, > + [6] = { 400, 0 }, > + [7] = { 800, 0 }, > + [8] = { 1600, 0 }, > + [9] = { 3200, 0 }, > +}; > + > +#define ADXL313_ACCEL_CHANNEL(index, addr, axis) { \ > + .type = IIO_ACCEL, \ > + .address = addr, \ > + .modified = 1, \ > + .channel2 = IIO_MOD_##axis, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_CALIBBIAS), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > + .info_mask_shared_by_type_available = \ > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > + .scan_index = index, \ > + .scan_type = { \ > + .sign = 's', \ > + .realbits = 13, \ > + .storagebits = 16, \ > + .endianness = IIO_LE, \ > + }, \ > +} > + > +static const struct iio_chan_spec adxl313_channels[] = { > + ADXL313_ACCEL_CHANNEL(0, ADXL313_REG_DATAX, X), > + ADXL313_ACCEL_CHANNEL(1, ADXL313_REG_DATAY, Y), > + ADXL313_ACCEL_CHANNEL(2, ADXL313_REG_DATAZ, Z), > +}; > + > +static int adxl313_set_odr(struct adxl313_data *data, > + unsigned int freq1, unsigned int freq2) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(adxl313_odr_freqs); i++) { > + if (adxl313_odr_freqs[i][0] == freq1 && > + adxl313_odr_freqs[i][1] == freq2) > + break; > + } > + > + if (i == ARRAY_SIZE(adxl313_odr_freqs)) > + return -EINVAL; > + > + return regmap_update_bits(data->regmap, ADXL313_REG_BW_RATE, > + ADXL313_RATE_MSK, > + FIELD_PREP(ADXL313_RATE_MSK, > + ADXL313_RATE_BASE + i)); One line? > +} > + > +static int adxl313_read_axis(struct adxl313_data *data, > + struct iio_chan_spec const *chan) > +{ > + int ret; > + > + mutex_lock(&data->lock); > + > + ret = regmap_bulk_read(data->regmap, > + chan->address, > + &data->transf_buf, 2); > + if (ret) > + goto unlock_ret; > + > + ret = le16_to_cpu(data->transf_buf); > + > +unlock_ret: > + mutex_unlock(&data->lock); > + return ret; > +} > + > +static int adxl313_read_freq_avail(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + const int **vals, int *type, int *length, > + long mask) > +{ > + switch (mask) { > + case IIO_CHAN_INFO_SAMP_FREQ: > + *vals = (const int *)adxl313_odr_freqs; > + *length = ARRAY_SIZE(adxl313_odr_freqs) * 2; > + *type = IIO_VAL_INT_PLUS_MICRO; > + return IIO_AVAIL_LIST; > + default: > + return -EINVAL; > + } > +} > + > +static int adxl313_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct adxl313_data *data = iio_priv(indio_dev); > + unsigned int regval; > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = adxl313_read_axis(data, chan); > + if (ret < 0) > + return ret; > + > + *val = sign_extend32(ret, chan->scan_type.realbits - 1); > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SCALE: > + /* > + * Scale for any g range is given in datasheet as > + * 1024 LSB/g = 0.0009765625 * 9.80665 = 0.009576806640625 m/s^2 > + */ > + *val = 0; > + *val2 = 9576806; > + return IIO_VAL_INT_PLUS_NANO; > + case IIO_CHAN_INFO_CALIBBIAS: > + ret = regmap_read(data->regmap, > + ADXL313_REG_OFS_AXIS(chan->scan_index), > + ®val); > + if (ret) > + return ret; > + > + /* > + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale > + * factor at full resolution > + */ > + *val = sign_extend32(regval, 7) * 4; > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SAMP_FREQ: > + ret = regmap_read(data->regmap, ADXL313_REG_BW_RATE, ®val); > + if (ret) > + return ret; > + > + ret = FIELD_GET(ADXL313_RATE_MSK, regval) - ADXL313_RATE_BASE; > + *val = adxl313_odr_freqs[ret][0]; > + *val2 = adxl313_odr_freqs[ret][1]; > + return IIO_VAL_INT_PLUS_MICRO; > + default: > + return -EINVAL; > + } > +} > + > +static int adxl313_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, int val2, long mask) > +{ > + struct adxl313_data *data = iio_priv(indio_dev); > + > + switch (mask) { > + case IIO_CHAN_INFO_CALIBBIAS: > + /* > + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale > + * factor at full resolution > + */ > + if (clamp_val(val, -128 * 4, 127 * 4) != val) > + return -EINVAL; > + > + return regmap_write(data->regmap, > + ADXL313_REG_OFS_AXIS(chan->scan_index), > + val / 4); > + case IIO_CHAN_INFO_SAMP_FREQ: > + return adxl313_set_odr(data, val, val2); > + default: > + return -EINVAL; > + } > +} > + > +static const struct iio_info adxl313_info = { > + .read_raw = adxl313_read_raw, > + .write_raw = adxl313_write_raw, > + .read_avail = adxl313_read_freq_avail + Comma > +}; > + > +static int adxl313_setup(struct device *dev, struct adxl313_data *data, > + int (*interface_specific_setup)(struct device *, > + struct regmap *)) Same comment against nameing, "setup" would be enough here. > +{ > + unsigned int regval; > + int ret; > + > + /* Ensures the device is in a consistent state after start up */ > + ret = regmap_write(data->regmap, ADXL313_REG_SOFT_RESET, > + ADXL313_SOFT_RESET); > + if (ret) > + return ret; > + > + if (interface_specific_setup) { > + ret = interface_specific_setup(dev, data->regmap); > + if (ret) > + return ret; > + } > + > + ret = regmap_read(data->regmap, ADXL313_REG_DEVID0, ®val); > + if (ret) > + return ret; > + > + if (regval != ADXL313_DEVID0) { > + dev_err(dev, "Invalid manufacturer ID: 0x%02x\n", regval); > + return -ENODEV; > + } > + > + ret = regmap_read(data->regmap, ADXL313_REG_DEVID1, ®val); > + if (ret) > + return ret; > + > + if (regval != ADXL313_DEVID1) { > + dev_err(dev, "Invalid mems ID: 0x%02x\n", regval); > + return -ENODEV; > + } > + > + ret = regmap_read(data->regmap, ADXL313_REG_PARTID, ®val); > + if (ret) > + return ret; > + > + if (regval != ADXL313_PARTID) { > + dev_err(dev, "Invalid device ID: 0x%02x\n", regval); > + return -ENODEV; > + } > + > + /* Sets the range to +/- 4g */ > + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, > + ADXL313_RANGE_MSK, > + FIELD_PREP(ADXL313_RANGE_MSK, > + ADXL313_RANGE_4G)); > + if (ret) > + return ret; > + > + /* Enables full resolution */ > + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, > + ADXL313_FULL_RES, ADXL313_FULL_RES); > + if (ret) > + return ret; > + > + /* Enables measurement mode */ > + return regmap_update_bits(data->regmap, ADXL313_REG_POWER_CTL, > + ADXL313_POWER_CTL_MSK, > + ADXL313_MEASUREMENT_MODE); > +} Provide a kernel doc for the probe() > +int adxl313_core_probe(struct device *dev, > + struct regmap *regmap, > + const char *name, > + int (*interface_specific_setup)(struct device *, > + struct regmap *)) Ditto for the "setup" name. > +{ > + struct adxl313_data *data; > + struct iio_dev *indio_dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + data->regmap = regmap; > + mutex_init(&data->lock); > + > + indio_dev->name = name; > + indio_dev->info = &adxl313_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = adxl313_channels; > + indio_dev->num_channels = ARRAY_SIZE(adxl313_channels); > + > + ret = adxl313_setup(dev, data, interface_specific_setup); > + if (ret) { > + dev_err(dev, "ADXL313 setup failed\n"); > + return ret; > + } > + > + return devm_iio_device_register(dev, indio_dev); > +} > +EXPORT_SYMBOL_GPL(adxl313_core_probe); > + > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer core driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/accel/adxl313_i2c.c b/drivers/iio/accel/adxl313_i2c.c > new file mode 100644 > index 000000000000..517c6f1c43ad > --- /dev/null > +++ b/drivers/iio/accel/adxl313_i2c.c > @@ -0,0 +1,65 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * ADXL313 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > + * > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > + */ > + > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > + > +#include "adxl313.h" > + > +static const struct regmap_config adxl313_i2c_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .rd_table = &adxl313_readable_regs_table, > + .wr_table = &adxl313_writable_regs_table, > + .max_register = 0x39, > +}; > + > +static int adxl313_i2c_probe(struct i2c_client *client) > +{ > + struct regmap *regmap; > + > + regmap = devm_regmap_init_i2c(client, &adxl313_i2c_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(&client->dev, "Error initializing i2c regmap: %ld\n", > + PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } > + > + return adxl313_core_probe(&client->dev, regmap, client->name, NULL); > +} > + > +static const struct i2c_device_id adxl313_i2c_id[] = { > + { "adxl313" }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(i2c, adxl313_i2c_id); > + > +static const struct of_device_id adxl313_of_match[] = { > + { .compatible = "adi,adxl313" }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(of, adxl313_of_match); > + > +static struct i2c_driver adxl313_i2c_driver = { > + .driver = { > + .name = "adxl313_i2c", > + .of_match_table = adxl313_of_match, > + }, > + .probe_new = adxl313_i2c_probe, > + .id_table = adxl313_i2c_id + Comma > +}; > + > +module_i2c_driver(adxl313_i2c_driver); > + > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer I2C driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c > new file mode 100644 > index 000000000000..4d625b1753a5 > --- /dev/null > +++ b/drivers/iio/accel/adxl313_spi.c > @@ -0,0 +1,91 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * ADXL313 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > + * > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > + */ > + > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/spi/spi.h> > + > +#include "adxl313.h" > + > +static const struct regmap_config adxl313_spi_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .rd_table = &adxl313_readable_regs_table, > + .wr_table = &adxl313_writable_regs_table, > + .max_register = 0x39, > + /* Setting bits 7 and 6 enables multiple-byte read */ > + .read_flag_mask = BIT(7) | BIT(6), > +}; > + > +static int adxl313_spi_setup(struct device *dev, struct regmap *regmap) > +{ > + struct spi_device *spi = container_of(dev, struct spi_device, dev); > + int ret; > + > + if (spi->mode & SPI_3WIRE) { > + ret = regmap_write(regmap, ADXL313_REG_DATA_FORMAT, > + ADXL313_SPI_3WIRE); > + if (ret) > + return ret; > + } > + > + return regmap_update_bits(regmap, ADXL313_REG_POWER_CTL, > + ADXL313_I2C_DISABLE, ADXL313_I2C_DISABLE); > +} > + > +static int adxl313_spi_probe(struct spi_device *spi) > +{ > + const struct spi_device_id *id = spi_get_device_id(spi); > + struct regmap *regmap; > + int ret; > + > + spi->mode |= SPI_MODE_3; > + ret = spi_setup(spi); > + if (ret) > + return ret; > + > + regmap = devm_regmap_init_spi(spi, &adxl313_spi_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(&spi->dev, "Error initializing spi regmap: %ld\n", > + PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } > + > + return adxl313_core_probe(&spi->dev, regmap, id->name, > + &adxl313_spi_setup); > +} > + > +static const struct spi_device_id adxl313_spi_id[] = { > + { "adxl313" }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(spi, adxl313_spi_id); > + > +static const struct of_device_id adxl313_of_match[] = { > + { .compatible = "adi,adxl313" }, > + { } > +}; > + > +MODULE_DEVICE_TABLE(of, adxl313_of_match); > + > +static struct spi_driver adxl313_spi_driver = { > + .driver = { > + .name = "adxl313_spi", > + .of_match_table = adxl313_of_match, > + }, > + .probe = adxl313_spi_probe, > + .id_table = adxl313_spi_id + Comma > +}; > + > +module_spi_driver(adxl313_spi_driver); > + > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer SPI driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.32.0 > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] iio: accel: Add driver support for ADXL313 2021-08-12 11:05 ` Andy Shevchenko @ 2021-08-18 20:01 ` Lucas Stankus 2021-08-29 14:12 ` Jonathan Cameron 2021-08-29 14:19 ` Andy Shevchenko 0 siblings, 2 replies; 10+ messages in thread From: Lucas Stankus @ 2021-08-18 20:01 UTC (permalink / raw) To: Andy Shevchenko Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, Rob Herring, Bogdan, Dragos, Darius, linux-iio, devicetree, Linux Kernel Mailing List On Thu, Aug 12, 2021 at 8:06 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Thu, Aug 12, 2021 at 12:19 AM Lucas Stankus > <lucas.p.stankus@gmail.com> wrote: > > > > ADXL313 is a small, thin, low power, 3-axis accelerometer with high > > resolution measurement up to +/-4g. It includes an integrated 32-level > > FIFO and has activity and inactivity sensing capabilities. > > Thanks for an update, my comments below. > After addressing them, feel free to add > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Sorry for the late reply. I've addressed most of your comments for the v4, but the two I've commented below. > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > > Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com> > > Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com> > > --- > > MAINTAINERS | 6 + > > drivers/iio/accel/Kconfig | 29 +++ > > drivers/iio/accel/Makefile | 3 + > > drivers/iio/accel/adxl313.h | 57 ++++++ > > drivers/iio/accel/adxl313_core.c | 331 +++++++++++++++++++++++++++++++ > > drivers/iio/accel/adxl313_i2c.c | 65 ++++++ > > drivers/iio/accel/adxl313_spi.c | 91 +++++++++ > > 7 files changed, 582 insertions(+) > > create mode 100644 drivers/iio/accel/adxl313.h > > create mode 100644 drivers/iio/accel/adxl313_core.c > > create mode 100644 drivers/iio/accel/adxl313_i2c.c > > create mode 100644 drivers/iio/accel/adxl313_spi.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index d0464b83b5b5..b2ada8ca1453 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -585,6 +585,12 @@ L: platform-driver-x86@vger.kernel.org > > S: Maintained > > F: drivers/platform/x86/adv_swbutton.c > > > > +ADXL313 THREE-AXIS DIGITAL ACCELEROMETER DRIVER > > +M: Lucas Stankus <lucas.p.stankus@gmail.com> > > +S: Supported > > +F: Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml > > +F: drivers/iio/accel/adxl313* > > + > > ADXL34X THREE-AXIS DIGITAL ACCELEROMETER DRIVER (ADXL345/ADXL346) > > M: Michael Hennerich <michael.hennerich@analog.com> > > S: Supported > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > index 2f0c0d512ae7..fb8349a88cd3 100644 > > --- a/drivers/iio/accel/Kconfig > > +++ b/drivers/iio/accel/Kconfig > > @@ -30,6 +30,35 @@ config ADIS16209 > > To compile this driver as a module, say M here: the module will be > > called adis16209. > > > > +config ADXL313 > > + tristate > > + > > +config ADXL313_I2C > > + tristate "Analog Devices ADXL313 3-Axis Digital Accelerometer I2C Driver" > > + depends on I2C > > + select ADXL313 > > + select REGMAP_I2C > > + help > > + Say Y here if you want to build i2c support for the Analog Devices > > + ADXL313 3-axis digital accelerometer. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called adxl313_i2c and you will also get adxl313_core > > + for the core module. > > + > > +config ADXL313_SPI > > + tristate "Analog Devices ADXL313 3-Axis Digital Accelerometer SPI Driver" > > + depends on SPI > > + select ADXL313 > > + select REGMAP_SPI > > + help > > + Say Y here if you want to build spi support for the Analog Devices > > + ADXL313 3-axis digital accelerometer. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called adxl313_spi and you will also get adxl313_core > > + for the core module. > > + > > config ADXL345 > > tristate > > > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > index 89280e823bcd..fadc92816e24 100644 > > --- a/drivers/iio/accel/Makefile > > +++ b/drivers/iio/accel/Makefile > > @@ -6,6 +6,9 @@ > > # When adding new entries keep the list in alphabetical order > > obj-$(CONFIG_ADIS16201) += adis16201.o > > obj-$(CONFIG_ADIS16209) += adis16209.o > > +obj-$(CONFIG_ADXL313) += adxl313_core.o > > +obj-$(CONFIG_ADXL313_I2C) += adxl313_i2c.o > > +obj-$(CONFIG_ADXL313_SPI) += adxl313_spi.o > > obj-$(CONFIG_ADXL345) += adxl345_core.o > > obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > > obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o > > diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h > > new file mode 100644 > > index 000000000000..c170b5236988 > > --- /dev/null > > +++ b/drivers/iio/accel/adxl313.h > > @@ -0,0 +1,57 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +/* > > + * ADXL313 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > > + */ > > + > > +#ifndef _ADXL313_H_ > > +#define _ADXL313_H_ > > + > > +/* ADXL313 register definitions */ > > +#define ADXL313_REG_DEVID0 0x00 > > +#define ADXL313_REG_DEVID1 0x01 > > +#define ADXL313_REG_PARTID 0x02 > > +#define ADXL313_REG_XID 0x04 > > +#define ADXL313_REG_SOFT_RESET 0x18 > > +#define ADXL313_REG_OFS_AXIS(index) (0x1E + (index)) > > +#define ADXL313_REG_THRESH_ACT 0x24 > > +#define ADXL313_REG_ACT_INACT_CTL 0x27 > > +#define ADXL313_REG_BW_RATE 0x2C > > +#define ADXL313_REG_POWER_CTL 0x2D > > +#define ADXL313_REG_INT_MAP 0x2F > > +#define ADXL313_REG_DATA_FORMAT 0x31 > > +#define ADXL313_REG_DATAX 0x32 > > +#define ADXL313_REG_DATAY 0x34 > > +#define ADXL313_REG_DATAZ 0x36 > > +#define ADXL313_REG_FIFO_CTL 0x38 > > +#define ADXL313_REG_FIFO_STATUS 0x39 > > + > > +#define ADXL313_DEVID0 0xAD > > +#define ADXL313_DEVID1 0x1D > > +#define ADXL313_PARTID 0xCB > > +#define ADXL313_SOFT_RESET 0x52 > > + > > +#define ADXL313_RATE_MSK GENMASK(3, 0) > > +#define ADXL313_RATE_BASE 6 > > + > > +#define ADXL313_POWER_CTL_MSK GENMASK(3, 2) > > +#define ADXL313_MEASUREMENT_MODE BIT(3) > > + > > +#define ADXL313_RANGE_MSK GENMASK(1, 0) > > +#define ADXL313_RANGE_4G 3 > > + > > +#define ADXL313_FULL_RES BIT(3) > > +#define ADXL313_SPI_3WIRE BIT(6) > > +#define ADXL313_I2C_DISABLE BIT(6) > > + > > +extern const struct regmap_access_table adxl313_readable_regs_table; > > + > > +extern const struct regmap_access_table adxl313_writable_regs_table; > > + > > +int adxl313_core_probe(struct device *dev, > > + struct regmap *regmap, > > + const char *name, > > > + int (*interface_specific_setup)(struct device *, > > + struct regmap *)); > > I think it's quite a long name for an internal function. At least > "specific" word may be dropped, but I will go even for "interface" > drop and explaining this parameter in the kernel doc of the probe() > function/ > > > +#endif /* _ADXL313_H_ */ > > diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c > > new file mode 100644 > > index 000000000000..8f3d4f5281c4 > > --- /dev/null > > +++ b/drivers/iio/accel/adxl313_core.c > > @@ -0,0 +1,331 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * ADXL313 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > > + * > > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/iio/iio.h> > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > + > > +#include "adxl313.h" > > + > > +const struct regmap_range adxl313_readable_reg_range[] = { > > + regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_XID), > > + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), > > + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), > > + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), > > + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_FIFO_STATUS) > > + comma (the rule of thumb is to check all similar code pieces even if > you got a comment only once) > > > +}; > > + > > +const struct regmap_access_table adxl313_readable_regs_table = { > > + .yes_ranges = adxl313_readable_reg_range, > > + .n_yes_ranges = ARRAY_SIZE(adxl313_readable_reg_range) > > + Comma > > > +}; > > +EXPORT_SYMBOL_GPL(adxl313_readable_regs_table); > > + > > +const struct regmap_range adxl313_writable_reg_range[] = { > > + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), > > + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), > > + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), > > + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_INT_MAP), > > + regmap_reg_range(ADXL313_REG_DATA_FORMAT, ADXL313_REG_DATA_FORMAT), > > + regmap_reg_range(ADXL313_REG_FIFO_CTL, ADXL313_REG_FIFO_CTL) > > Ditto. > > > +}; > > + > > +const struct regmap_access_table adxl313_writable_regs_table = { > > + .yes_ranges = adxl313_writable_reg_range, > > + .n_yes_ranges = ARRAY_SIZE(adxl313_writable_reg_range) > > Ditto. > > > > +}; > > +EXPORT_SYMBOL_GPL(adxl313_writable_regs_table); > > + > > +struct adxl313_data { > > + struct regmap *regmap; > > + struct mutex lock; /* lock to protect transf_buf */ > > + __le16 transf_buf ____cacheline_aligned; > > +}; > > + > > +static const int adxl313_odr_freqs[][2] = { > > + [0] = { 6, 250000 }, > > + [1] = { 12, 500000 }, > > + [2] = { 25, 0 }, > > + [3] = { 50, 0 }, > > + [4] = { 100, 0 }, > > + [5] = { 200, 0 }, > > + [6] = { 400, 0 }, > > + [7] = { 800, 0 }, > > + [8] = { 1600, 0 }, > > + [9] = { 3200, 0 }, > > +}; > > + > > +#define ADXL313_ACCEL_CHANNEL(index, addr, axis) { \ > > + .type = IIO_ACCEL, \ > > + .address = addr, \ > > + .modified = 1, \ > > + .channel2 = IIO_MOD_##axis, \ > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > > + BIT(IIO_CHAN_INFO_CALIBBIAS), \ > > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ > > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > > + .info_mask_shared_by_type_available = \ > > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > > + .scan_index = index, \ > > + .scan_type = { \ > > + .sign = 's', \ > > + .realbits = 13, \ > > + .storagebits = 16, \ > > + .endianness = IIO_LE, \ > > + }, \ > > +} > > + > > +static const struct iio_chan_spec adxl313_channels[] = { > > + ADXL313_ACCEL_CHANNEL(0, ADXL313_REG_DATAX, X), > > + ADXL313_ACCEL_CHANNEL(1, ADXL313_REG_DATAY, Y), > > + ADXL313_ACCEL_CHANNEL(2, ADXL313_REG_DATAZ, Z), > > +}; > > + > > +static int adxl313_set_odr(struct adxl313_data *data, > > + unsigned int freq1, unsigned int freq2) > > +{ > > + unsigned int i; > > + > > + for (i = 0; i < ARRAY_SIZE(adxl313_odr_freqs); i++) { > > + if (adxl313_odr_freqs[i][0] == freq1 && > > + adxl313_odr_freqs[i][1] == freq2) > > + break; > > + } > > + > > + if (i == ARRAY_SIZE(adxl313_odr_freqs)) > > + return -EINVAL; > > + > > + return regmap_update_bits(data->regmap, ADXL313_REG_BW_RATE, > > + ADXL313_RATE_MSK, > > > + FIELD_PREP(ADXL313_RATE_MSK, > > + ADXL313_RATE_BASE + i)); > > One line? > Line to long, 87 chars =/ > > +} > > + > > +static int adxl313_read_axis(struct adxl313_data *data, > > + struct iio_chan_spec const *chan) > > +{ > > + int ret; > > + > > + mutex_lock(&data->lock); > > + > > + ret = regmap_bulk_read(data->regmap, > > + chan->address, > > + &data->transf_buf, 2); > > + if (ret) > > + goto unlock_ret; > > + > > + ret = le16_to_cpu(data->transf_buf); > > + > > +unlock_ret: > > + mutex_unlock(&data->lock); > > + return ret; > > +} > > + > > +static int adxl313_read_freq_avail(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + const int **vals, int *type, int *length, > > + long mask) > > +{ > > + switch (mask) { > > + case IIO_CHAN_INFO_SAMP_FREQ: > > + *vals = (const int *)adxl313_odr_freqs; > > + *length = ARRAY_SIZE(adxl313_odr_freqs) * 2; > > + *type = IIO_VAL_INT_PLUS_MICRO; > > + return IIO_AVAIL_LIST; > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static int adxl313_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int *val, int *val2, long mask) > > +{ > > + struct adxl313_data *data = iio_priv(indio_dev); > > + unsigned int regval; > > + int ret; > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_RAW: > > + ret = adxl313_read_axis(data, chan); > > + if (ret < 0) > > + return ret; > > + > > + *val = sign_extend32(ret, chan->scan_type.realbits - 1); > > + return IIO_VAL_INT; > > + case IIO_CHAN_INFO_SCALE: > > + /* > > + * Scale for any g range is given in datasheet as > > + * 1024 LSB/g = 0.0009765625 * 9.80665 = 0.009576806640625 m/s^2 > > + */ > > + *val = 0; > > + *val2 = 9576806; > > + return IIO_VAL_INT_PLUS_NANO; > > + case IIO_CHAN_INFO_CALIBBIAS: > > + ret = regmap_read(data->regmap, > > + ADXL313_REG_OFS_AXIS(chan->scan_index), > > + ®val); > > + if (ret) > > + return ret; > > + > > + /* > > + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale > > + * factor at full resolution > > + */ > > + *val = sign_extend32(regval, 7) * 4; > > + return IIO_VAL_INT; > > + case IIO_CHAN_INFO_SAMP_FREQ: > > + ret = regmap_read(data->regmap, ADXL313_REG_BW_RATE, ®val); > > + if (ret) > > + return ret; > > + > > + ret = FIELD_GET(ADXL313_RATE_MSK, regval) - ADXL313_RATE_BASE; > > + *val = adxl313_odr_freqs[ret][0]; > > + *val2 = adxl313_odr_freqs[ret][1]; > > + return IIO_VAL_INT_PLUS_MICRO; > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static int adxl313_write_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int val, int val2, long mask) > > +{ > > + struct adxl313_data *data = iio_priv(indio_dev); > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_CALIBBIAS: > > + /* > > + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale > > + * factor at full resolution > > + */ > > + if (clamp_val(val, -128 * 4, 127 * 4) != val) > > + return -EINVAL; > > + > > + return regmap_write(data->regmap, > > + ADXL313_REG_OFS_AXIS(chan->scan_index), > > + val / 4); > > + case IIO_CHAN_INFO_SAMP_FREQ: > > + return adxl313_set_odr(data, val, val2); > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static const struct iio_info adxl313_info = { > > + .read_raw = adxl313_read_raw, > > + .write_raw = adxl313_write_raw, > > + .read_avail = adxl313_read_freq_avail > > + Comma > > > +}; > > + > > +static int adxl313_setup(struct device *dev, struct adxl313_data *data, > > + int (*interface_specific_setup)(struct device *, > > + struct regmap *)) > > Same comment against nameing, "setup" would be enough here. > > > +{ > > + unsigned int regval; > > + int ret; > > + > > + /* Ensures the device is in a consistent state after start up */ > > + ret = regmap_write(data->regmap, ADXL313_REG_SOFT_RESET, > > + ADXL313_SOFT_RESET); > > + if (ret) > > + return ret; > > + > > + if (interface_specific_setup) { > > + ret = interface_specific_setup(dev, data->regmap); > > + if (ret) > > + return ret; > > + } > > + > > + ret = regmap_read(data->regmap, ADXL313_REG_DEVID0, ®val); > > + if (ret) > > + return ret; > > + > > + if (regval != ADXL313_DEVID0) { > > + dev_err(dev, "Invalid manufacturer ID: 0x%02x\n", regval); > > + return -ENODEV; > > + } > > + > > + ret = regmap_read(data->regmap, ADXL313_REG_DEVID1, ®val); > > + if (ret) > > + return ret; > > + > > + if (regval != ADXL313_DEVID1) { > > + dev_err(dev, "Invalid mems ID: 0x%02x\n", regval); > > + return -ENODEV; > > + } > > + > > + ret = regmap_read(data->regmap, ADXL313_REG_PARTID, ®val); > > + if (ret) > > + return ret; > > + > > + if (regval != ADXL313_PARTID) { > > + dev_err(dev, "Invalid device ID: 0x%02x\n", regval); > > + return -ENODEV; > > + } > > + > > + /* Sets the range to +/- 4g */ > > + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, > > + ADXL313_RANGE_MSK, > > + FIELD_PREP(ADXL313_RANGE_MSK, > > + ADXL313_RANGE_4G)); > > + if (ret) > > + return ret; > > + > > + /* Enables full resolution */ > > + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, > > + ADXL313_FULL_RES, ADXL313_FULL_RES); > > + if (ret) > > + return ret; > > + > > + /* Enables measurement mode */ > > + return regmap_update_bits(data->regmap, ADXL313_REG_POWER_CTL, > > + ADXL313_POWER_CTL_MSK, > > + ADXL313_MEASUREMENT_MODE); > > +} > > Provide a kernel doc for the probe() > Is there some driver that did something similar that I can take inspiration from? I'm not sure where I should add this doc and how I should format it, so a north would be appreciated :) > > +int adxl313_core_probe(struct device *dev, > > + struct regmap *regmap, > > + const char *name, > > + int (*interface_specific_setup)(struct device *, > > + struct regmap *)) > > Ditto for the "setup" name. > > > +{ > > + struct adxl313_data *data; > > + struct iio_dev *indio_dev; > > + int ret; > > + > > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + data = iio_priv(indio_dev); > > + data->regmap = regmap; > > + mutex_init(&data->lock); > > + > > + indio_dev->name = name; > > + indio_dev->info = &adxl313_info; > > + indio_dev->modes = INDIO_DIRECT_MODE; > > + indio_dev->channels = adxl313_channels; > > + indio_dev->num_channels = ARRAY_SIZE(adxl313_channels); > > + > > + ret = adxl313_setup(dev, data, interface_specific_setup); > > + if (ret) { > > + dev_err(dev, "ADXL313 setup failed\n"); > > + return ret; > > + } > > + > > + return devm_iio_device_register(dev, indio_dev); > > +} > > +EXPORT_SYMBOL_GPL(adxl313_core_probe); > > + > > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer core driver"); > > +MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/iio/accel/adxl313_i2c.c b/drivers/iio/accel/adxl313_i2c.c > > new file mode 100644 > > index 000000000000..517c6f1c43ad > > --- /dev/null > > +++ b/drivers/iio/accel/adxl313_i2c.c > > @@ -0,0 +1,65 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * ADXL313 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > > + * > > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > > + */ > > + > > +#include <linux/i2c.h> > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > + > > +#include "adxl313.h" > > + > > +static const struct regmap_config adxl313_i2c_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .rd_table = &adxl313_readable_regs_table, > > + .wr_table = &adxl313_writable_regs_table, > > + .max_register = 0x39, > > +}; > > + > > +static int adxl313_i2c_probe(struct i2c_client *client) > > +{ > > + struct regmap *regmap; > > + > > + regmap = devm_regmap_init_i2c(client, &adxl313_i2c_regmap_config); > > + if (IS_ERR(regmap)) { > > + dev_err(&client->dev, "Error initializing i2c regmap: %ld\n", > > + PTR_ERR(regmap)); > > + return PTR_ERR(regmap); > > + } > > + > > + return adxl313_core_probe(&client->dev, regmap, client->name, NULL); > > +} > > + > > +static const struct i2c_device_id adxl313_i2c_id[] = { > > + { "adxl313" }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(i2c, adxl313_i2c_id); > > + > > +static const struct of_device_id adxl313_of_match[] = { > > + { .compatible = "adi,adxl313" }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(of, adxl313_of_match); > > + > > +static struct i2c_driver adxl313_i2c_driver = { > > + .driver = { > > + .name = "adxl313_i2c", > > + .of_match_table = adxl313_of_match, > > + }, > > + .probe_new = adxl313_i2c_probe, > > + .id_table = adxl313_i2c_id > > + Comma > > > +}; > > + > > +module_i2c_driver(adxl313_i2c_driver); > > + > > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer I2C driver"); > > +MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c > > new file mode 100644 > > index 000000000000..4d625b1753a5 > > --- /dev/null > > +++ b/drivers/iio/accel/adxl313_spi.c > > @@ -0,0 +1,91 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * ADXL313 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > > + * > > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > +#include <linux/spi/spi.h> > > + > > +#include "adxl313.h" > > + > > +static const struct regmap_config adxl313_spi_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .rd_table = &adxl313_readable_regs_table, > > + .wr_table = &adxl313_writable_regs_table, > > + .max_register = 0x39, > > + /* Setting bits 7 and 6 enables multiple-byte read */ > > + .read_flag_mask = BIT(7) | BIT(6), > > +}; > > + > > +static int adxl313_spi_setup(struct device *dev, struct regmap *regmap) > > +{ > > + struct spi_device *spi = container_of(dev, struct spi_device, dev); > > + int ret; > > + > > + if (spi->mode & SPI_3WIRE) { > > + ret = regmap_write(regmap, ADXL313_REG_DATA_FORMAT, > > + ADXL313_SPI_3WIRE); > > + if (ret) > > + return ret; > > + } > > + > > + return regmap_update_bits(regmap, ADXL313_REG_POWER_CTL, > > + ADXL313_I2C_DISABLE, ADXL313_I2C_DISABLE); > > +} > > + > > +static int adxl313_spi_probe(struct spi_device *spi) > > +{ > > + const struct spi_device_id *id = spi_get_device_id(spi); > > + struct regmap *regmap; > > + int ret; > > + > > + spi->mode |= SPI_MODE_3; > > + ret = spi_setup(spi); > > + if (ret) > > + return ret; > > + > > + regmap = devm_regmap_init_spi(spi, &adxl313_spi_regmap_config); > > + if (IS_ERR(regmap)) { > > + dev_err(&spi->dev, "Error initializing spi regmap: %ld\n", > > + PTR_ERR(regmap)); > > + return PTR_ERR(regmap); > > + } > > + > > + return adxl313_core_probe(&spi->dev, regmap, id->name, > > + &adxl313_spi_setup); > > +} > > + > > +static const struct spi_device_id adxl313_spi_id[] = { > > + { "adxl313" }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(spi, adxl313_spi_id); > > + > > +static const struct of_device_id adxl313_of_match[] = { > > + { .compatible = "adi,adxl313" }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(of, adxl313_of_match); > > + > > +static struct spi_driver adxl313_spi_driver = { > > + .driver = { > > + .name = "adxl313_spi", > > + .of_match_table = adxl313_of_match, > > + }, > > + .probe = adxl313_spi_probe, > > + .id_table = adxl313_spi_id > > + Comma > > > +}; > > + > > +module_spi_driver(adxl313_spi_driver); > > + > > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer SPI driver"); > > +MODULE_LICENSE("GPL v2"); > > -- > > 2.32.0 > > > > > -- > With Best Regards, > Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] iio: accel: Add driver support for ADXL313 2021-08-18 20:01 ` Lucas Stankus @ 2021-08-29 14:12 ` Jonathan Cameron 2021-08-29 14:19 ` Andy Shevchenko 1 sibling, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2021-08-29 14:12 UTC (permalink / raw) To: Lucas Stankus Cc: Andy Shevchenko, Lars-Peter Clausen, Michael Hennerich, Rob Herring, Bogdan, Dragos, Darius, linux-iio, devicetree, Linux Kernel Mailing List On Wed, 18 Aug 2021 17:01:43 -0300 Lucas Stankus <lucas.p.stankus@gmail.com> wrote: > On Thu, Aug 12, 2021 at 8:06 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > > > On Thu, Aug 12, 2021 at 12:19 AM Lucas Stankus > > <lucas.p.stankus@gmail.com> wrote: > > > > > > ADXL313 is a small, thin, low power, 3-axis accelerometer with high > > > resolution measurement up to +/-4g. It includes an integrated 32-level > > > FIFO and has activity and inactivity sensing capabilities. > > > > Thanks for an update, my comments below. > > After addressing them, feel free to add > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > > Sorry for the late reply. I've addressed most of your comments for the > v4, but the two I've commented below. > > > > > Provide a kernel doc for the probe() > > > > Is there some driver that did something similar that I can take > inspiration from? I'm not sure where I should add this doc and how I > should format it, so a north would be appreciated :) https://elixir.bootlin.com/linux/latest/source/Documentation/doc-guide/kernel-doc.rst In theory all exported functions should have kernel-doc, but in practice we sometimes skip it for a function only intended to have one or two users. However, in this case the "setup" parameter is sufficiently unusual that it would be good to have detailed documents. Make sure to run a test build of the docs to pick up on any formatting issues. That help file explains how to do it. > > > > +int adxl313_core_probe(struct device *dev, > > > + struct regmap *regmap, > > > + const char *name, > > > + int (*interface_specific_setup)(struct device *, > > > + struct regmap *)) > > > > Ditto for the "setup" name. > > > > > +{ > > > + struct adxl313_data *data; > > > + struct iio_dev *indio_dev; > > > + int ret; > > > + > > > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > > > + if (!indio_dev) > > > + return -ENOMEM; > > > + > > > + data = iio_priv(indio_dev); > > > + data->regmap = regmap; > > > + mutex_init(&data->lock); > > > + > > > + indio_dev->name = name; > > > + indio_dev->info = &adxl313_info; > > > + indio_dev->modes = INDIO_DIRECT_MODE; > > > + indio_dev->channels = adxl313_channels; > > > + indio_dev->num_channels = ARRAY_SIZE(adxl313_channels); > > > + > > > + ret = adxl313_setup(dev, data, interface_specific_setup); > > > + if (ret) { > > > + dev_err(dev, "ADXL313 setup failed\n"); > > > + return ret; > > > + } > > > + > > > + return devm_iio_device_register(dev, indio_dev); > > > +} > > > +EXPORT_SYMBOL_GPL(adxl313_core_probe); > > > + > > > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > > > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer core driver"); > > > +MODULE_LICENSE("GPL v2"); > > > diff --git a/drivers/iio/accel/adxl313_i2c.c b/drivers/iio/accel/adxl313_i2c.c > > > new file mode 100644 > > > index 000000000000..517c6f1c43ad > > > --- /dev/null > > > +++ b/drivers/iio/accel/adxl313_i2c.c > > > @@ -0,0 +1,65 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * ADXL313 3-Axis Digital Accelerometer > > > + * > > > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > > > + * > > > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > > > + */ > > > + > > > +#include <linux/i2c.h> > > > +#include <linux/module.h> > > > +#include <linux/regmap.h> > > > + > > > +#include "adxl313.h" > > > + > > > +static const struct regmap_config adxl313_i2c_regmap_config = { > > > + .reg_bits = 8, > > > + .val_bits = 8, > > > + .rd_table = &adxl313_readable_regs_table, > > > + .wr_table = &adxl313_writable_regs_table, > > > + .max_register = 0x39, > > > +}; > > > + > > > +static int adxl313_i2c_probe(struct i2c_client *client) > > > +{ > > > + struct regmap *regmap; > > > + > > > + regmap = devm_regmap_init_i2c(client, &adxl313_i2c_regmap_config); > > > + if (IS_ERR(regmap)) { > > > + dev_err(&client->dev, "Error initializing i2c regmap: %ld\n", > > > + PTR_ERR(regmap)); > > > + return PTR_ERR(regmap); > > > + } > > > + > > > + return adxl313_core_probe(&client->dev, regmap, client->name, NULL); > > > +} > > > + > > > +static const struct i2c_device_id adxl313_i2c_id[] = { > > > + { "adxl313" }, > > > + { } > > > +}; > > > + > > > +MODULE_DEVICE_TABLE(i2c, adxl313_i2c_id); > > > + > > > +static const struct of_device_id adxl313_of_match[] = { > > > + { .compatible = "adi,adxl313" }, > > > + { } > > > +}; > > > + > > > +MODULE_DEVICE_TABLE(of, adxl313_of_match); > > > + > > > +static struct i2c_driver adxl313_i2c_driver = { > > > + .driver = { > > > + .name = "adxl313_i2c", > > > + .of_match_table = adxl313_of_match, > > > + }, > > > + .probe_new = adxl313_i2c_probe, > > > + .id_table = adxl313_i2c_id > > > > + Comma > > > > > +}; > > > + > > > +module_i2c_driver(adxl313_i2c_driver); > > > + > > > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > > > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer I2C driver"); > > > +MODULE_LICENSE("GPL v2"); > > > diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c > > > new file mode 100644 > > > index 000000000000..4d625b1753a5 > > > --- /dev/null > > > +++ b/drivers/iio/accel/adxl313_spi.c > > > @@ -0,0 +1,91 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * ADXL313 3-Axis Digital Accelerometer > > > + * > > > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > > > + * > > > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > > > + */ > > > + > > > +#include <linux/module.h> > > > +#include <linux/regmap.h> > > > +#include <linux/spi/spi.h> > > > + > > > +#include "adxl313.h" > > > + > > > +static const struct regmap_config adxl313_spi_regmap_config = { > > > + .reg_bits = 8, > > > + .val_bits = 8, > > > + .rd_table = &adxl313_readable_regs_table, > > > + .wr_table = &adxl313_writable_regs_table, > > > + .max_register = 0x39, > > > + /* Setting bits 7 and 6 enables multiple-byte read */ > > > + .read_flag_mask = BIT(7) | BIT(6), > > > +}; > > > + > > > +static int adxl313_spi_setup(struct device *dev, struct regmap *regmap) > > > +{ > > > + struct spi_device *spi = container_of(dev, struct spi_device, dev); > > > + int ret; > > > + > > > + if (spi->mode & SPI_3WIRE) { > > > + ret = regmap_write(regmap, ADXL313_REG_DATA_FORMAT, > > > + ADXL313_SPI_3WIRE); > > > + if (ret) > > > + return ret; > > > + } > > > + > > > + return regmap_update_bits(regmap, ADXL313_REG_POWER_CTL, > > > + ADXL313_I2C_DISABLE, ADXL313_I2C_DISABLE); > > > +} > > > + > > > +static int adxl313_spi_probe(struct spi_device *spi) > > > +{ > > > + const struct spi_device_id *id = spi_get_device_id(spi); > > > + struct regmap *regmap; > > > + int ret; > > > + > > > + spi->mode |= SPI_MODE_3; > > > + ret = spi_setup(spi); > > > + if (ret) > > > + return ret; > > > + > > > + regmap = devm_regmap_init_spi(spi, &adxl313_spi_regmap_config); > > > + if (IS_ERR(regmap)) { > > > + dev_err(&spi->dev, "Error initializing spi regmap: %ld\n", > > > + PTR_ERR(regmap)); > > > + return PTR_ERR(regmap); > > > + } > > > + > > > + return adxl313_core_probe(&spi->dev, regmap, id->name, > > > + &adxl313_spi_setup); > > > +} > > > + > > > +static const struct spi_device_id adxl313_spi_id[] = { > > > + { "adxl313" }, > > > + { } > > > +}; > > > + > > > +MODULE_DEVICE_TABLE(spi, adxl313_spi_id); > > > + > > > +static const struct of_device_id adxl313_of_match[] = { > > > + { .compatible = "adi,adxl313" }, > > > + { } > > > +}; > > > + > > > +MODULE_DEVICE_TABLE(of, adxl313_of_match); > > > + > > > +static struct spi_driver adxl313_spi_driver = { > > > + .driver = { > > > + .name = "adxl313_spi", > > > + .of_match_table = adxl313_of_match, > > > + }, > > > + .probe = adxl313_spi_probe, > > > + .id_table = adxl313_spi_id > > > > + Comma > > > > > +}; > > > + > > > +module_spi_driver(adxl313_spi_driver); > > > + > > > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > > > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer SPI driver"); > > > +MODULE_LICENSE("GPL v2"); > > > -- > > > 2.32.0 > > > > > > > > > -- > > With Best Regards, > > Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] iio: accel: Add driver support for ADXL313 2021-08-18 20:01 ` Lucas Stankus 2021-08-29 14:12 ` Jonathan Cameron @ 2021-08-29 14:19 ` Andy Shevchenko 1 sibling, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2021-08-29 14:19 UTC (permalink / raw) To: Lucas Stankus Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, Rob Herring, Bogdan, Dragos, Darius, linux-iio, devicetree, Linux Kernel Mailing List On Wed, Aug 18, 2021 at 11:02 PM Lucas Stankus <lucas.p.stankus@gmail.com> wrote: > On Thu, Aug 12, 2021 at 8:06 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Thu, Aug 12, 2021 at 12:19 AM Lucas Stankus > > <lucas.p.stankus@gmail.com> wrote: ... > > > + return regmap_update_bits(data->regmap, ADXL313_REG_BW_RATE, > > > + ADXL313_RATE_MSK, > > > > > + FIELD_PREP(ADXL313_RATE_MSK, > > > + ADXL313_RATE_BASE + i)); > > > > One line? > > > > Line to long, 87 chars =/ I think it's fine in this case. It will keep the entire FIELD_PREP() on one line. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] iio: accel: Add driver support for ADXL313 2021-08-11 21:18 ` [PATCH v3 2/2] iio: accel: Add driver support " Lucas Stankus 2021-08-12 11:05 ` Andy Shevchenko @ 2021-08-15 15:26 ` Jonathan Cameron 2021-08-18 20:07 ` Lucas Stankus 1 sibling, 1 reply; 10+ messages in thread From: Jonathan Cameron @ 2021-08-15 15:26 UTC (permalink / raw) To: Lucas Stankus Cc: lars, Michael.Hennerich, robh+dt, Dragos.Bogdan, Darius.Berghe, linux-iio, devicetree, linux-kernel On Wed, 11 Aug 2021 18:18:02 -0300 Lucas Stankus <lucas.p.stankus@gmail.com> wrote: > ADXL313 is a small, thin, low power, 3-axis accelerometer with high > resolution measurement up to +/-4g. It includes an integrated 32-level > FIFO and has activity and inactivity sensing capabilities. > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com> > Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com> Hi Lucas, I took another close look at this and I would like you to not use scan_index as you are here. It has a meaning tightly coupled to 'scans' which only exist as a concept once you have buffered mode. The preferred way to get channel specific values is to use addr and add a level of indirection (so a lookup structure of some type). A few more trivial things to add to what Andy raised inline below. Jonathan ... > +#endif /* _ADXL313_H_ */ > diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c > new file mode 100644 > index 000000000000..8f3d4f5281c4 > --- /dev/null > +++ b/drivers/iio/accel/adxl313_core.c > @@ -0,0 +1,331 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * ADXL313 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > + * > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > + */ > + > +#include <linux/bitfield.h> > +#include <linux/iio/iio.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > + > +#include "adxl313.h" > + > +const struct regmap_range adxl313_readable_reg_range[] = { > + regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_XID), > + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), > + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), > + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), > + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_FIFO_STATUS) > +}; > + > +const struct regmap_access_table adxl313_readable_regs_table = { > + .yes_ranges = adxl313_readable_reg_range, > + .n_yes_ranges = ARRAY_SIZE(adxl313_readable_reg_range) > +}; > +EXPORT_SYMBOL_GPL(adxl313_readable_regs_table); > + > +const struct regmap_range adxl313_writable_reg_range[] = { > + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), > + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), > + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), > + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_INT_MAP), > + regmap_reg_range(ADXL313_REG_DATA_FORMAT, ADXL313_REG_DATA_FORMAT), > + regmap_reg_range(ADXL313_REG_FIFO_CTL, ADXL313_REG_FIFO_CTL) > +}; > + > +const struct regmap_access_table adxl313_writable_regs_table = { > + .yes_ranges = adxl313_writable_reg_range, > + .n_yes_ranges = ARRAY_SIZE(adxl313_writable_reg_range) > +}; > +EXPORT_SYMBOL_GPL(adxl313_writable_regs_table); > + > +struct adxl313_data { > + struct regmap *regmap; > + struct mutex lock; /* lock to protect transf_buf */ > + __le16 transf_buf ____cacheline_aligned; > +}; > + > +static const int adxl313_odr_freqs[][2] = { > + [0] = { 6, 250000 }, > + [1] = { 12, 500000 }, > + [2] = { 25, 0 }, > + [3] = { 50, 0 }, > + [4] = { 100, 0 }, > + [5] = { 200, 0 }, > + [6] = { 400, 0 }, > + [7] = { 800, 0 }, > + [8] = { 1600, 0 }, > + [9] = { 3200, 0 }, > +}; > + > +#define ADXL313_ACCEL_CHANNEL(index, addr, axis) { \ > + .type = IIO_ACCEL, \ > + .address = addr, \ > + .modified = 1, \ > + .channel2 = IIO_MOD_##axis, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_CALIBBIAS), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > + .info_mask_shared_by_type_available = \ > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > + .scan_index = index, \ I'm not in general keen on seeing scan_index being set in a driver that isn't providing a buffer. Whilst some of the scan_type can have valid meaning without buffering, scan_index should not. The use you are making of it here, should use addr. So put a channel enum value into addr and use that to look up both the _raw address to read and anything else channel specific. That will give more consistent code than you currently have with a mixture of addr and scan_index being used depending on the particular thing being read about the channel. > + .scan_type = { \ > + .sign = 's', \ > + .realbits = 13, \ > + .storagebits = 16, \ > + .endianness = IIO_LE, \ sign, storagebits and endianness are things that are only likely to matter once buffered supported is added (where they relect the datalayout). For now please drop setting them at all. > + }, \ > +} > + > +static const struct iio_chan_spec adxl313_channels[] = { > + ADXL313_ACCEL_CHANNEL(0, ADXL313_REG_DATAX, X), > + ADXL313_ACCEL_CHANNEL(1, ADXL313_REG_DATAY, Y), > + ADXL313_ACCEL_CHANNEL(2, ADXL313_REG_DATAZ, Z), > +}; > + > +static int adxl313_set_odr(struct adxl313_data *data, > + unsigned int freq1, unsigned int freq2) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(adxl313_odr_freqs); i++) { > + if (adxl313_odr_freqs[i][0] == freq1 && > + adxl313_odr_freqs[i][1] == freq2) > + break; > + } > + > + if (i == ARRAY_SIZE(adxl313_odr_freqs)) > + return -EINVAL; > + > + return regmap_update_bits(data->regmap, ADXL313_REG_BW_RATE, > + ADXL313_RATE_MSK, > + FIELD_PREP(ADXL313_RATE_MSK, > + ADXL313_RATE_BASE + i)); > +} > + > +static int adxl313_read_axis(struct adxl313_data *data, > + struct iio_chan_spec const *chan) > +{ > + int ret; > + > + mutex_lock(&data->lock); > + > + ret = regmap_bulk_read(data->regmap, > + chan->address, > + &data->transf_buf, 2); sizeof(data->trasf_buf) > + if (ret) > + goto unlock_ret; > + > + ret = le16_to_cpu(data->transf_buf); > + > +unlock_ret: > + mutex_unlock(&data->lock); > + return ret; > +} > + > +static int adxl313_read_freq_avail(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + const int **vals, int *type, int *length, > + long mask) > +{ > + switch (mask) { > + case IIO_CHAN_INFO_SAMP_FREQ: > + *vals = (const int *)adxl313_odr_freqs; > + *length = ARRAY_SIZE(adxl313_odr_freqs) * 2; > + *type = IIO_VAL_INT_PLUS_MICRO; > + return IIO_AVAIL_LIST; > + default: > + return -EINVAL; > + } > +} > + > +static int adxl313_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct adxl313_data *data = iio_priv(indio_dev); > + unsigned int regval; > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + ret = adxl313_read_axis(data, chan); > + if (ret < 0) > + return ret; > + > + *val = sign_extend32(ret, chan->scan_type.realbits - 1); > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SCALE: > + /* > + * Scale for any g range is given in datasheet as > + * 1024 LSB/g = 0.0009765625 * 9.80665 = 0.009576806640625 m/s^2 > + */ > + *val = 0; > + *val2 = 9576806; > + return IIO_VAL_INT_PLUS_NANO; > + case IIO_CHAN_INFO_CALIBBIAS: > + ret = regmap_read(data->regmap, > + ADXL313_REG_OFS_AXIS(chan->scan_index), As mentioned above, don't use scan_index for this purpose. There is no particular reason the scan_index for a given channel won't be changed in future as part of some layout optimization etc. addr is for this purpose and often contains an enum value to that gives a driver specific index of channels, then used to do things like you are doing here. > + ®val); > + if (ret) > + return ret; > + > + /* > + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale > + * factor at full resolution > + */ > + *val = sign_extend32(regval, 7) * 4; > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SAMP_FREQ: > + ret = regmap_read(data->regmap, ADXL313_REG_BW_RATE, ®val); > + if (ret) > + return ret; > + > + ret = FIELD_GET(ADXL313_RATE_MSK, regval) - ADXL313_RATE_BASE; > + *val = adxl313_odr_freqs[ret][0]; > + *val2 = adxl313_odr_freqs[ret][1]; > + return IIO_VAL_INT_PLUS_MICRO; > + default: > + return -EINVAL; > + } > +} > + > +static int adxl313_write_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, int val2, long mask) > +{ > + struct adxl313_data *data = iio_priv(indio_dev); > + > + switch (mask) { > + case IIO_CHAN_INFO_CALIBBIAS: > + /* > + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale > + * factor at full resolution > + */ > + if (clamp_val(val, -128 * 4, 127 * 4) != val) > + return -EINVAL; > + > + return regmap_write(data->regmap, > + ADXL313_REG_OFS_AXIS(chan->scan_index), > + val / 4); > + case IIO_CHAN_INFO_SAMP_FREQ: > + return adxl313_set_odr(data, val, val2); > + default: > + return -EINVAL; > + } > +} > + > +static const struct iio_info adxl313_info = { > + .read_raw = adxl313_read_raw, > + .write_raw = adxl313_write_raw, > + .read_avail = adxl313_read_freq_avail > +}; > + > +static int adxl313_setup(struct device *dev, struct adxl313_data *data, > + int (*interface_specific_setup)(struct device *, > + struct regmap *)) > +{ > + unsigned int regval; > + int ret; > + > + /* Ensures the device is in a consistent state after start up */ > + ret = regmap_write(data->regmap, ADXL313_REG_SOFT_RESET, > + ADXL313_SOFT_RESET); > + if (ret) > + return ret; > + > + if (interface_specific_setup) { > + ret = interface_specific_setup(dev, data->regmap); > + if (ret) > + return ret; > + } > + > + ret = regmap_read(data->regmap, ADXL313_REG_DEVID0, ®val); > + if (ret) > + return ret; > + > + if (regval != ADXL313_DEVID0) { > + dev_err(dev, "Invalid manufacturer ID: 0x%02x\n", regval); > + return -ENODEV; > + } > + > + ret = regmap_read(data->regmap, ADXL313_REG_DEVID1, ®val); > + if (ret) > + return ret; > + > + if (regval != ADXL313_DEVID1) { > + dev_err(dev, "Invalid mems ID: 0x%02x\n", regval); > + return -ENODEV; > + } > + > + ret = regmap_read(data->regmap, ADXL313_REG_PARTID, ®val); > + if (ret) > + return ret; > + > + if (regval != ADXL313_PARTID) { > + dev_err(dev, "Invalid device ID: 0x%02x\n", regval); > + return -ENODEV; > + } > + > + /* Sets the range to +/- 4g */ > + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, > + ADXL313_RANGE_MSK, > + FIELD_PREP(ADXL313_RANGE_MSK, > + ADXL313_RANGE_4G)); > + if (ret) > + return ret; > + > + /* Enables full resolution */ > + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, > + ADXL313_FULL_RES, ADXL313_FULL_RES); > + if (ret) > + return ret; > + > + /* Enables measurement mode */ > + return regmap_update_bits(data->regmap, ADXL313_REG_POWER_CTL, > + ADXL313_POWER_CTL_MSK, > + ADXL313_MEASUREMENT_MODE); > +} > + > +int adxl313_core_probe(struct device *dev, > + struct regmap *regmap, > + const char *name, > + int (*interface_specific_setup)(struct device *, > + struct regmap *)) > +{ > + struct adxl313_data *data; > + struct iio_dev *indio_dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + data->regmap = regmap; > + mutex_init(&data->lock); > + > + indio_dev->name = name; > + indio_dev->info = &adxl313_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = adxl313_channels; > + indio_dev->num_channels = ARRAY_SIZE(adxl313_channels); > + > + ret = adxl313_setup(dev, data, interface_specific_setup); > + if (ret) { > + dev_err(dev, "ADXL313 setup failed\n"); > + return ret; > + } > + > + return devm_iio_device_register(dev, indio_dev); > +} > +EXPORT_SYMBOL_GPL(adxl313_core_probe); > + > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer core driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c > new file mode 100644 > index 000000000000..4d625b1753a5 > --- /dev/null > +++ b/drivers/iio/accel/adxl313_spi.c > @@ -0,0 +1,91 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * ADXL313 3-Axis Digital Accelerometer > + * > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > + * > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > + */ > + > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/spi/spi.h> Seeing as you are going to be doing a v4, please add explicit includes for mod_devicetable.h which contains the of_device_id struct definition. Same for the i2c driver. > + > +#include "adxl313.h" > + ... ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] iio: accel: Add driver support for ADXL313 2021-08-15 15:26 ` Jonathan Cameron @ 2021-08-18 20:07 ` Lucas Stankus 0 siblings, 0 replies; 10+ messages in thread From: Lucas Stankus @ 2021-08-18 20:07 UTC (permalink / raw) To: Jonathan Cameron Cc: Lars-Peter Clausen, Hennerich, Michael, Rob Herring, Bogdan, Dragos, Berghe, Darius, linux-iio, devicetree, Linux Kernel Mailing List On Sun, Aug 15, 2021 at 12:23 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Wed, 11 Aug 2021 18:18:02 -0300 > Lucas Stankus <lucas.p.stankus@gmail.com> wrote: > > > ADXL313 is a small, thin, low power, 3-axis accelerometer with high > > resolution measurement up to +/-4g. It includes an integrated 32-level > > FIFO and has activity and inactivity sensing capabilities. > > > > Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > > Signed-off-by: Lucas Stankus <lucas.p.stankus@gmail.com> > > Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com> > > Hi Lucas, > > I took another close look at this and I would like you to not use scan_index > as you are here. It has a meaning tightly coupled to 'scans' which only exist > as a concept once you have buffered mode. The preferred way to get channel > specific values is to use addr and add a level of indirection (so a lookup > structure of some type). A few more trivial things to add to what Andy raised > inline below. > > Jonathan > Okay, sorry for the misuse from my side, I'll address it in the v4 as well as the other minor comments. > > ... > > > +#endif /* _ADXL313_H_ */ > > diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c > > new file mode 100644 > > index 000000000000..8f3d4f5281c4 > > --- /dev/null > > +++ b/drivers/iio/accel/adxl313_core.c > > @@ -0,0 +1,331 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * ADXL313 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > > + * > > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/iio/iio.h> > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > + > > +#include "adxl313.h" > > + > > +const struct regmap_range adxl313_readable_reg_range[] = { > > + regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_XID), > > + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), > > + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), > > + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), > > + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_FIFO_STATUS) > > +}; > > + > > +const struct regmap_access_table adxl313_readable_regs_table = { > > + .yes_ranges = adxl313_readable_reg_range, > > + .n_yes_ranges = ARRAY_SIZE(adxl313_readable_reg_range) > > +}; > > +EXPORT_SYMBOL_GPL(adxl313_readable_regs_table); > > + > > +const struct regmap_range adxl313_writable_reg_range[] = { > > + regmap_reg_range(ADXL313_REG_SOFT_RESET, ADXL313_REG_SOFT_RESET), > > + regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)), > > + regmap_reg_range(ADXL313_REG_THRESH_ACT, ADXL313_REG_ACT_INACT_CTL), > > + regmap_reg_range(ADXL313_REG_BW_RATE, ADXL313_REG_INT_MAP), > > + regmap_reg_range(ADXL313_REG_DATA_FORMAT, ADXL313_REG_DATA_FORMAT), > > + regmap_reg_range(ADXL313_REG_FIFO_CTL, ADXL313_REG_FIFO_CTL) > > +}; > > + > > +const struct regmap_access_table adxl313_writable_regs_table = { > > + .yes_ranges = adxl313_writable_reg_range, > > + .n_yes_ranges = ARRAY_SIZE(adxl313_writable_reg_range) > > +}; > > +EXPORT_SYMBOL_GPL(adxl313_writable_regs_table); > > + > > +struct adxl313_data { > > + struct regmap *regmap; > > + struct mutex lock; /* lock to protect transf_buf */ > > + __le16 transf_buf ____cacheline_aligned; > > +}; > > + > > +static const int adxl313_odr_freqs[][2] = { > > + [0] = { 6, 250000 }, > > + [1] = { 12, 500000 }, > > + [2] = { 25, 0 }, > > + [3] = { 50, 0 }, > > + [4] = { 100, 0 }, > > + [5] = { 200, 0 }, > > + [6] = { 400, 0 }, > > + [7] = { 800, 0 }, > > + [8] = { 1600, 0 }, > > + [9] = { 3200, 0 }, > > +}; > > + > > +#define ADXL313_ACCEL_CHANNEL(index, addr, axis) { \ > > + .type = IIO_ACCEL, \ > > + .address = addr, \ > > + .modified = 1, \ > > + .channel2 = IIO_MOD_##axis, \ > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > > + BIT(IIO_CHAN_INFO_CALIBBIAS), \ > > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \ > > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > > + .info_mask_shared_by_type_available = \ > > + BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > > + .scan_index = index, \ > > I'm not in general keen on seeing scan_index being set in a driver that isn't providing > a buffer. Whilst some of the scan_type can have valid meaning without buffering, > scan_index should not. > > The use you are making of it here, should use addr. So put a channel enum value > into addr and use that to look up both the _raw address to read and anything else > channel specific. That will give more consistent code than you currently have > with a mixture of addr and scan_index being used depending on the particular > thing being read about the channel. > > > + .scan_type = { \ > > + .sign = 's', \ > > + .realbits = 13, \ > > + .storagebits = 16, \ > > + .endianness = IIO_LE, \ > > sign, storagebits and endianness are things that are only likely to matter once buffered > supported is added (where they relect the datalayout). For now please drop setting > them at all. > > > + }, \ > > +} > > + > > +static const struct iio_chan_spec adxl313_channels[] = { > > + ADXL313_ACCEL_CHANNEL(0, ADXL313_REG_DATAX, X), > > + ADXL313_ACCEL_CHANNEL(1, ADXL313_REG_DATAY, Y), > > + ADXL313_ACCEL_CHANNEL(2, ADXL313_REG_DATAZ, Z), > > +}; > > + > > +static int adxl313_set_odr(struct adxl313_data *data, > > + unsigned int freq1, unsigned int freq2) > > +{ > > + unsigned int i; > > + > > + for (i = 0; i < ARRAY_SIZE(adxl313_odr_freqs); i++) { > > + if (adxl313_odr_freqs[i][0] == freq1 && > > + adxl313_odr_freqs[i][1] == freq2) > > + break; > > + } > > + > > + if (i == ARRAY_SIZE(adxl313_odr_freqs)) > > + return -EINVAL; > > + > > + return regmap_update_bits(data->regmap, ADXL313_REG_BW_RATE, > > + ADXL313_RATE_MSK, > > + FIELD_PREP(ADXL313_RATE_MSK, > > + ADXL313_RATE_BASE + i)); > > +} > > + > > +static int adxl313_read_axis(struct adxl313_data *data, > > + struct iio_chan_spec const *chan) > > +{ > > + int ret; > > + > > + mutex_lock(&data->lock); > > + > > + ret = regmap_bulk_read(data->regmap, > > + chan->address, > > + &data->transf_buf, 2); > sizeof(data->trasf_buf) > > + if (ret) > > + goto unlock_ret; > > + > > + ret = le16_to_cpu(data->transf_buf); > > + > > +unlock_ret: > > + mutex_unlock(&data->lock); > > + return ret; > > +} > > + > > +static int adxl313_read_freq_avail(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + const int **vals, int *type, int *length, > > + long mask) > > +{ > > + switch (mask) { > > + case IIO_CHAN_INFO_SAMP_FREQ: > > + *vals = (const int *)adxl313_odr_freqs; > > + *length = ARRAY_SIZE(adxl313_odr_freqs) * 2; > > + *type = IIO_VAL_INT_PLUS_MICRO; > > + return IIO_AVAIL_LIST; > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static int adxl313_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int *val, int *val2, long mask) > > +{ > > + struct adxl313_data *data = iio_priv(indio_dev); > > + unsigned int regval; > > + int ret; > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_RAW: > > + ret = adxl313_read_axis(data, chan); > > + if (ret < 0) > > + return ret; > > + > > + *val = sign_extend32(ret, chan->scan_type.realbits - 1); > > + return IIO_VAL_INT; > > + case IIO_CHAN_INFO_SCALE: > > + /* > > + * Scale for any g range is given in datasheet as > > + * 1024 LSB/g = 0.0009765625 * 9.80665 = 0.009576806640625 m/s^2 > > + */ > > + *val = 0; > > + *val2 = 9576806; > > + return IIO_VAL_INT_PLUS_NANO; > > + case IIO_CHAN_INFO_CALIBBIAS: > > + ret = regmap_read(data->regmap, > > + ADXL313_REG_OFS_AXIS(chan->scan_index), > As mentioned above, don't use scan_index for this purpose. There is no particular > reason the scan_index for a given channel won't be changed in future as part of some layout > optimization etc. addr is for this purpose and often contains an enum value to that > gives a driver specific index of channels, then used to do things like you are doing here. > > > + ®val); > > + if (ret) > > + return ret; > > + > > + /* > > + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale > > + * factor at full resolution > > + */ > > + *val = sign_extend32(regval, 7) * 4; > > + return IIO_VAL_INT; > > + case IIO_CHAN_INFO_SAMP_FREQ: > > + ret = regmap_read(data->regmap, ADXL313_REG_BW_RATE, ®val); > > + if (ret) > > + return ret; > > + > > + ret = FIELD_GET(ADXL313_RATE_MSK, regval) - ADXL313_RATE_BASE; > > + *val = adxl313_odr_freqs[ret][0]; > > + *val2 = adxl313_odr_freqs[ret][1]; > > + return IIO_VAL_INT_PLUS_MICRO; > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static int adxl313_write_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, > > + int val, int val2, long mask) > > +{ > > + struct adxl313_data *data = iio_priv(indio_dev); > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_CALIBBIAS: > > + /* > > + * 8-bit resolution at +/- 0.5g, that is 4x accel data scale > > + * factor at full resolution > > + */ > > + if (clamp_val(val, -128 * 4, 127 * 4) != val) > > + return -EINVAL; > > + > > + return regmap_write(data->regmap, > > + ADXL313_REG_OFS_AXIS(chan->scan_index), > > + val / 4); > > + case IIO_CHAN_INFO_SAMP_FREQ: > > + return adxl313_set_odr(data, val, val2); > > + default: > > + return -EINVAL; > > + } > > +} > > + > > +static const struct iio_info adxl313_info = { > > + .read_raw = adxl313_read_raw, > > + .write_raw = adxl313_write_raw, > > + .read_avail = adxl313_read_freq_avail > > +}; > > + > > +static int adxl313_setup(struct device *dev, struct adxl313_data *data, > > + int (*interface_specific_setup)(struct device *, > > + struct regmap *)) > > +{ > > + unsigned int regval; > > + int ret; > > + > > + /* Ensures the device is in a consistent state after start up */ > > + ret = regmap_write(data->regmap, ADXL313_REG_SOFT_RESET, > > + ADXL313_SOFT_RESET); > > + if (ret) > > + return ret; > > + > > + if (interface_specific_setup) { > > + ret = interface_specific_setup(dev, data->regmap); > > + if (ret) > > + return ret; > > + } > > + > > + ret = regmap_read(data->regmap, ADXL313_REG_DEVID0, ®val); > > + if (ret) > > + return ret; > > + > > + if (regval != ADXL313_DEVID0) { > > + dev_err(dev, "Invalid manufacturer ID: 0x%02x\n", regval); > > + return -ENODEV; > > + } > > + > > + ret = regmap_read(data->regmap, ADXL313_REG_DEVID1, ®val); > > + if (ret) > > + return ret; > > + > > + if (regval != ADXL313_DEVID1) { > > + dev_err(dev, "Invalid mems ID: 0x%02x\n", regval); > > + return -ENODEV; > > + } > > + > > + ret = regmap_read(data->regmap, ADXL313_REG_PARTID, ®val); > > + if (ret) > > + return ret; > > + > > + if (regval != ADXL313_PARTID) { > > + dev_err(dev, "Invalid device ID: 0x%02x\n", regval); > > + return -ENODEV; > > + } > > + > > + /* Sets the range to +/- 4g */ > > + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, > > + ADXL313_RANGE_MSK, > > + FIELD_PREP(ADXL313_RANGE_MSK, > > + ADXL313_RANGE_4G)); > > + if (ret) > > + return ret; > > + > > + /* Enables full resolution */ > > + ret = regmap_update_bits(data->regmap, ADXL313_REG_DATA_FORMAT, > > + ADXL313_FULL_RES, ADXL313_FULL_RES); > > + if (ret) > > + return ret; > > + > > + /* Enables measurement mode */ > > + return regmap_update_bits(data->regmap, ADXL313_REG_POWER_CTL, > > + ADXL313_POWER_CTL_MSK, > > + ADXL313_MEASUREMENT_MODE); > > +} > > + > > +int adxl313_core_probe(struct device *dev, > > + struct regmap *regmap, > > + const char *name, > > + int (*interface_specific_setup)(struct device *, > > + struct regmap *)) > > +{ > > + struct adxl313_data *data; > > + struct iio_dev *indio_dev; > > + int ret; > > + > > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + data = iio_priv(indio_dev); > > + data->regmap = regmap; > > + mutex_init(&data->lock); > > + > > + indio_dev->name = name; > > + indio_dev->info = &adxl313_info; > > + indio_dev->modes = INDIO_DIRECT_MODE; > > + indio_dev->channels = adxl313_channels; > > + indio_dev->num_channels = ARRAY_SIZE(adxl313_channels); > > + > > + ret = adxl313_setup(dev, data, interface_specific_setup); > > + if (ret) { > > + dev_err(dev, "ADXL313 setup failed\n"); > > + return ret; > > + } > > + > > + return devm_iio_device_register(dev, indio_dev); > > +} > > +EXPORT_SYMBOL_GPL(adxl313_core_probe); > > + > > +MODULE_AUTHOR("Lucas Stankus <lucas.p.stankus@gmail.com>"); > > +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer core driver"); > > +MODULE_LICENSE("GPL v2"); > > > > > diff --git a/drivers/iio/accel/adxl313_spi.c b/drivers/iio/accel/adxl313_spi.c > > new file mode 100644 > > index 000000000000..4d625b1753a5 > > --- /dev/null > > +++ b/drivers/iio/accel/adxl313_spi.c > > @@ -0,0 +1,91 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * ADXL313 3-Axis Digital Accelerometer > > + * > > + * Copyright (c) 2021 Lucas Stankus <lucas.p.stankus@gmail.com> > > + * > > + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > +#include <linux/spi/spi.h> > > Seeing as you are going to be doing a v4, please add explicit includes > for mod_devicetable.h which contains the of_device_id struct definition. > Same for the i2c driver. > > > + > > +#include "adxl313.h" > > + > ... ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-08-29 14:20 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-11 21:16 [PATCH v3 0/2] iio: accel: Add support for ADXL313 accelerometer Lucas Stankus 2021-08-11 21:17 ` [PATCH v3 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313 Lucas Stankus 2021-08-17 21:22 ` Rob Herring 2021-08-11 21:18 ` [PATCH v3 2/2] iio: accel: Add driver support " Lucas Stankus 2021-08-12 11:05 ` Andy Shevchenko 2021-08-18 20:01 ` Lucas Stankus 2021-08-29 14:12 ` Jonathan Cameron 2021-08-29 14:19 ` Andy Shevchenko 2021-08-15 15:26 ` Jonathan Cameron 2021-08-18 20:07 ` Lucas Stankus
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.