2021-08-05 07:05:15

by Lucas Stankus

[permalink] [raw]
Subject: [PATCH v2 0/2] iio: accel: Add support for ADXL313 accelerometer

Add driver support and dt-bindings documentation for ADXL313 digital
accelerometer.

Changelog v1 -> v2:
- Add vs-supply, vdd-supply and interrupt-names fields in the dt doc
- Add default case for switch statements
- Use function pointer argument in core_probe for interface specific setup
- Check `spi->mode & SPI_3WIRE` to enable the device's 3wire mode
- Remove unnecessary 0s from id structs and commas after null terminators

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 | 90 +++++
MAINTAINERS | 9 +
drivers/iio/accel/Kconfig | 29 ++
drivers/iio/accel/Makefile | 3 +
drivers/iio/accel/adxl313.h | 63 ++++
drivers/iio/accel/adxl313_core.c | 321 ++++++++++++++++++
drivers/iio/accel/adxl313_i2c.c | 65 ++++
drivers/iio/accel/adxl313_spi.c | 85 +++++
8 files changed, 665 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


2021-08-05 07:05:52

by Lucas Stankus

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313

Add device tree binding documentation for ADXL313 3-axis accelerometer.

Signed-off-by: Lucas Stankus <[email protected]>
---
.../bindings/iio/accel/adi,adxl313.yaml | 90 +++++++++++++++++++
1 file changed, 90 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..fea03b6790f3
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
@@ -0,0 +1,90 @@
+# 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 <[email protected]>
+
+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-cpha: true
+
+ spi-cpol: 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:
+ maxItems: 2
+
+ interrupt-names:
+ 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>;
+ spi-cpol;
+ spi-cpha;
+ interrupt-parent = <&gpio0>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "INT1";
+ };
+ };
--
2.32.0

2021-08-05 08:22:43

by Lucas Stankus

[permalink] [raw]
Subject: [PATCH v2 2/2] iio: accel: Add driver support for ADXL313

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 <[email protected]>
---
MAINTAINERS | 9 +
drivers/iio/accel/Kconfig | 29 +++
drivers/iio/accel/Makefile | 3 +
drivers/iio/accel/adxl313.h | 63 ++++++
drivers/iio/accel/adxl313_core.c | 321 +++++++++++++++++++++++++++++++
drivers/iio/accel/adxl313_i2c.c | 65 +++++++
drivers/iio/accel/adxl313_spi.c | 85 ++++++++
7 files changed, 575 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 a61f4f3b78a9..566055450b6b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -585,6 +585,15 @@ L: [email protected]
S: Maintained
F: drivers/platform/x86/adv_swbutton.c

+ADXL313 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
+M: Lucas Stankus <[email protected]>
+S: Supported
+F: Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
+F: drivers/iio/accel/adxl313.h
+F: drivers/iio/accel/adxl313_core.c
+F: drivers/iio/accel/adxl313_i2c.c
+F: drivers/iio/accel/adxl313_spi.c
+
ADXL34X THREE-AXIS DIGITAL ACCELEROMETER DRIVER (ADXL345/ADXL346)
M: Michael Hennerich <[email protected]>
S: Supported
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 0e56ace61103..ae621532e716 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..ea5994227a2e
--- /dev/null
+++ b/drivers/iio/accel/adxl313.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * ADXL313 3-Axis Digital Accelerometer
+ *
+ * Copyright (c) 2021 Lucas Stankus <[email protected]>
+ */
+
+#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)
+
+/*
+ * Scale for any g range is given in datasheet as
+ * 1024 LSB/g = 0.0009765625 * 9.80665 = 0.009576806640625 m/s^2
+ */
+#define ADXL313_NSCALE 9576806
+
+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..ed4484607556
--- /dev/null
+++ b/drivers/iio/accel/adxl313_core.c
@@ -0,0 +1,321 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADXL313 3-Axis Digital Accelerometer
+ *
+ * Copyright (c) 2021 Lucas Stankus <[email protected]>
+ *
+ * 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:
+ *val = 0;
+ *val2 = ADXL313_NSCALE;
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ ret = regmap_read(data->regmap,
+ ADXL313_REG_OFS_AXIS(chan->scan_index),
+ &regval);
+ 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, &regval);
+ 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 (val > 127 * 4 || val < -128 * 4)
+ 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;
+
+ ret = regmap_read(data->regmap, ADXL313_REG_DEVID0, &regval);
+ if (ret)
+ return ret;
+
+ if (interface_specific_setup) {
+ ret = interface_specific_setup(dev, data->regmap);
+ 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, &regval);
+ 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, &regval);
+ 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 <[email protected]>");
+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..d60e757cae07
--- /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 <[email protected]>
+ *
+ * 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 <[email protected]>");
+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..b5804522d9ff
--- /dev/null
+++ b/drivers/iio/accel/adxl313_spi.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ADXL313 3-Axis Digital Accelerometer
+ *
+ * Copyright (c) 2021 Lucas Stankus <[email protected]>
+ *
+ * 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;
+
+ 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 <[email protected]>");
+MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer SPI driver");
+MODULE_LICENSE("GPL v2");
--
2.32.0

2021-08-05 12:01:51

by Alexandru Ardelean

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] iio: accel: Add driver support for ADXL313

On Thu, Aug 5, 2021 at 10:05 AM Lucas Stankus <[email protected]> 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.
>

Reviewed-by: Alexandru Ardelean <[email protected]>

> Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADXL313.pdf
> Signed-off-by: Lucas Stankus <[email protected]>
> ---
> MAINTAINERS | 9 +
> drivers/iio/accel/Kconfig | 29 +++
> drivers/iio/accel/Makefile | 3 +
> drivers/iio/accel/adxl313.h | 63 ++++++
> drivers/iio/accel/adxl313_core.c | 321 +++++++++++++++++++++++++++++++
> drivers/iio/accel/adxl313_i2c.c | 65 +++++++
> drivers/iio/accel/adxl313_spi.c | 85 ++++++++
> 7 files changed, 575 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 a61f4f3b78a9..566055450b6b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -585,6 +585,15 @@ L: [email protected]
> S: Maintained
> F: drivers/platform/x86/adv_swbutton.c
>
> +ADXL313 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
> +M: Lucas Stankus <[email protected]>
> +S: Supported
> +F: Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
> +F: drivers/iio/accel/adxl313.h
> +F: drivers/iio/accel/adxl313_core.c
> +F: drivers/iio/accel/adxl313_i2c.c
> +F: drivers/iio/accel/adxl313_spi.c
> +
> ADXL34X THREE-AXIS DIGITAL ACCELEROMETER DRIVER (ADXL345/ADXL346)
> M: Michael Hennerich <[email protected]>
> S: Supported
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 0e56ace61103..ae621532e716 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..ea5994227a2e
> --- /dev/null
> +++ b/drivers/iio/accel/adxl313.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * ADXL313 3-Axis Digital Accelerometer
> + *
> + * Copyright (c) 2021 Lucas Stankus <[email protected]>
> + */
> +
> +#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)
> +
> +/*
> + * Scale for any g range is given in datasheet as
> + * 1024 LSB/g = 0.0009765625 * 9.80665 = 0.009576806640625 m/s^2
> + */
> +#define ADXL313_NSCALE 9576806
> +
> +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..ed4484607556
> --- /dev/null
> +++ b/drivers/iio/accel/adxl313_core.c
> @@ -0,0 +1,321 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ADXL313 3-Axis Digital Accelerometer
> + *
> + * Copyright (c) 2021 Lucas Stankus <[email protected]>
> + *
> + * 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:
> + *val = 0;
> + *val2 = ADXL313_NSCALE;
> + return IIO_VAL_INT_PLUS_NANO;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + ret = regmap_read(data->regmap,
> + ADXL313_REG_OFS_AXIS(chan->scan_index),
> + &regval);
> + 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, &regval);
> + 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 (val > 127 * 4 || val < -128 * 4)
> + 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;
> +
> + ret = regmap_read(data->regmap, ADXL313_REG_DEVID0, &regval);
> + if (ret)
> + return ret;
> +
> + if (interface_specific_setup) {
> + ret = interface_specific_setup(dev, data->regmap);
> + 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, &regval);
> + 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, &regval);
> + 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 <[email protected]>");
> +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..d60e757cae07
> --- /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 <[email protected]>
> + *
> + * 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 <[email protected]>");
> +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..b5804522d9ff
> --- /dev/null
> +++ b/drivers/iio/accel/adxl313_spi.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ADXL313 3-Axis Digital Accelerometer
> + *
> + * Copyright (c) 2021 Lucas Stankus <[email protected]>
> + *
> + * 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;
> +
> + 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 <[email protected]>");
> +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer SPI driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.32.0
>

2021-08-06 23:56:36

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313

On Thu, 05 Aug 2021 03:29:37 -0300, Lucas Stankus wrote:
> Add device tree binding documentation for ADXL313 3-axis accelerometer.
>
> Signed-off-by: Lucas Stankus <[email protected]>
> ---
> .../bindings/iio/accel/adi,adxl313.yaml | 90 +++++++++++++++++++
> 1 file changed, 90 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml: properties:interrupt-names:items: {'enum': ['INT1', 'INT2']} is not of type 'array'
from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml: ignoring, error in schema: properties: interrupt-names: items
warning: no schema found in file: ./Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
Documentation/devicetree/bindings/iio/accel/adi,adxl313.example.dt.yaml:0:0: /example-0/i2c0/accelerometer@53: failed to match any schema with compatible: ['adi,adxl313']
Documentation/devicetree/bindings/iio/accel/adi,adxl313.example.dt.yaml:0:0: /example-1/spi/accelerometer@0: failed to match any schema with compatible: ['adi,adxl313']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1513753

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2021-08-07 00:00:50

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313

On Thu, Aug 05, 2021 at 03:29:37AM -0300, Lucas Stankus wrote:
> Add device tree binding documentation for ADXL313 3-axis accelerometer.
>
> Signed-off-by: Lucas Stankus <[email protected]>
> ---
> .../bindings/iio/accel/adi,adxl313.yaml | 90 +++++++++++++++++++
> 1 file changed, 90 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..fea03b6790f3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
> @@ -0,0 +1,90 @@
> +# 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 <[email protected]>
> +
> +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-cpha: true
> +
> + spi-cpol: true

These 3 generally shouldn't be needed, but can be set from the driver.
If they are valid, is any combination of them really valid?

> +
> + 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:
> + maxItems: 2

This means there must be 2 entries. If 1 is valid, you need 'minItems'.

> +
> + interrupt-names:
> + maxItems: 2

You need 'minItems' too to fix the error.

> + 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>;
> + spi-cpol;
> + spi-cpha;
> + interrupt-parent = <&gpio0>;
> + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "INT1";
> + };
> + };
> --
> 2.32.0
>
>

2021-08-07 00:26:16

by Lucas Stankus

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] iio: accel: Add driver support for ADXL313

On Fri, Aug 6, 2021 at 9:35 AM Andy Shevchenko
<[email protected]> wrote:
>
>
>
> On Thursday, August 5, 2021, Lucas Stankus <[email protected]> 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 <[email protected]>
>> ---
>> MAINTAINERS | 9 +
>> drivers/iio/accel/Kconfig | 29 +++
>> drivers/iio/accel/Makefile | 3 +
>> drivers/iio/accel/adxl313.h | 63 ++++++
>> drivers/iio/accel/adxl313_core.c | 321 +++++++++++++++++++++++++++++++
>> drivers/iio/accel/adxl313_i2c.c | 65 +++++++
>> drivers/iio/accel/adxl313_spi.c | 85 ++++++++
>> 7 files changed, 575 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 a61f4f3b78a9..566055450b6b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -585,6 +585,15 @@ L: [email protected]
>> S: Maintained
>> F: drivers/platform/x86/adv_swbutton.c
>>
>> +ADXL313 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
>> +M: Lucas Stankus <[email protected]>
>> +S: Supported
>> +F: Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
>> +F: drivers/iio/accel/adxl313.h
>> +F: drivers/iio/accel/adxl313_core.c
>> +F: drivers/iio/accel/adxl313_i2c.c
>> +F: drivers/iio/accel/adxl313_spi.c
>
>
>
> adxl313*?
>

Didn't know this would work, but I think I prefer the way it is now.
Are you proposing this as a suggestion or more of a change request?

>>
>> +
>> ADXL34X THREE-AXIS DIGITAL ACCELEROMETER DRIVER (ADXL345/ADXL346)
>> M: Michael Hennerich <[email protected]>
>> S: Supported
>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>> index 0e56ace61103..ae621532e716 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..ea5994227a2e
>> --- /dev/null
>> +++ b/drivers/iio/accel/adxl313.h
>> @@ -0,0 +1,63 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * ADXL313 3-Axis Digital Accelerometer
>> + *
>> + * Copyright (c) 2021 Lucas Stankus <[email protected]>
>> + */
>> +
>> +#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)
>> +
>> +/*
>> + * Scale for any g range is given in datasheet as
>> + * 1024 LSB/g = 0.0009765625 * 9.80665 = 0.009576806640625 m/s^2
>> + */
>> +#define ADXL313_NSCALE 9576806
>
>
>
> Is it in nanonewtons? Perhaps add a suffix _nN?
>

It's actually in meters per second squared, so I couldn't come up with
a good suffix. Do you have any suggestions?

>>
>> +
>> +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..ed4484607556
>> --- /dev/null
>> +++ b/drivers/iio/accel/adxl313_core.c
>> @@ -0,0 +1,321 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * ADXL313 3-Axis Digital Accelerometer
>> + *
>> + * Copyright (c) 2021 Lucas Stankus <[email protected]>
>> + *
>> + * 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:
>> + *val = 0;
>> + *val2 = ADXL313_NSCALE;
>> + return IIO_VAL_INT_PLUS_NANO;
>> + case IIO_CHAN_INFO_CALIBBIAS:
>> + ret = regmap_read(data->regmap,
>> + ADXL313_REG_OFS_AXIS(chan->scan_index),
>> + &regval);
>> + 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, &regval);
>> + 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 (val > 127 * 4 || val < -128 * 4)
>
>
> if (clamp_val(val, ...) != val) ?
>
> But actually i would like rather to see predefined macro is_in_range() in minmax.h.
>

Yeah, a `is_in_range()` macro would be nice for this case, but anyway,
I think the clamp_val is a cleaner solution.

>
>>
>> + 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;
>> +
>> + ret = regmap_read(data->regmap, ADXL313_REG_DEVID0, &regval);
>> + if (ret)
>> + return ret;
>> +
>> + if (interface_specific_setup) {
>> + ret = interface_specific_setup(dev, data->regmap);
>> + 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, &regval);
>> + 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, &regval);
>> + 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 <[email protected]>");
>> +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..d60e757cae07
>> --- /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 <[email protected]>
>> + *
>> + * 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
>
>
>
> Leave comma here
>

I'll add them for the v3.

>>
>> +};
>> +
>> +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 <[email protected]>");
>> +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..b5804522d9ff
>> --- /dev/null
>> +++ b/drivers/iio/accel/adxl313_spi.c
>> @@ -0,0 +1,85 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * ADXL313 3-Axis Digital Accelerometer
>> + *
>> + * Copyright (c) 2021 Lucas Stankus <[email protected]>
>> + *
>> + * 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)
>
>
>
> Ditto
>
>> +};
>> +
>> +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;
>> +
>> + 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 <[email protected]>");
>> +MODULE_DESCRIPTION("ADXL313 3-Axis Digital Accelerometer SPI driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.32.0
>>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

2021-08-07 00:35:35

by Lucas Stankus

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313

On Fri, Aug 6, 2021 at 3:10 PM Rob Herring <[email protected]> wrote:
>
> On Thu, Aug 05, 2021 at 03:29:37AM -0300, Lucas Stankus wrote:
> > Add device tree binding documentation for ADXL313 3-axis accelerometer.
> >
> > Signed-off-by: Lucas Stankus <[email protected]>
> > ---
> > .../bindings/iio/accel/adi,adxl313.yaml | 90 +++++++++++++++++++
> > 1 file changed, 90 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..fea03b6790f3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
> > @@ -0,0 +1,90 @@
> > +# 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 <[email protected]>
> > +
> > +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-cpha: true
> > +
> > + spi-cpol: true
>
> These 3 generally shouldn't be needed, but can be set from the driver.
> If they are valid, is any combination of them really valid?
>

Only the 3wire is optional, both cpha and cpol are required for proper
spi connection.

> > +
> > + 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:
> > + maxItems: 2
>
> This means there must be 2 entries. If 1 is valid, you need 'minItems'.
>

I'll add 'minItems' for the v3 then, thanks!

> > +
> > + interrupt-names:
> > + maxItems: 2
>
> You need 'minItems' too to fix the error.
>

Thank you again and sorry for not catching that error before submitting.

> > + 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>;
> > + spi-cpol;
> > + spi-cpha;
> > + interrupt-parent = <&gpio0>;
> > + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > + interrupt-names = "INT1";
> > + };
> > + };
> > --
> > 2.32.0
> >
> >

2021-08-08 14:55:36

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] iio: accel: Add driver support for ADXL313

On Fri, 6 Aug 2021 21:22:46 -0300
Lucas Stankus <[email protected]> wrote:

> On Fri, Aug 6, 2021 at 9:35 AM Andy Shevchenko
> <[email protected]> wrote:
> >
> >
> >
> > On Thursday, August 5, 2021, Lucas Stankus <[email protected]> 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 <[email protected]>
> >> ---
> >> MAINTAINERS | 9 +
> >> drivers/iio/accel/Kconfig | 29 +++
> >> drivers/iio/accel/Makefile | 3 +
> >> drivers/iio/accel/adxl313.h | 63 ++++++
> >> drivers/iio/accel/adxl313_core.c | 321 +++++++++++++++++++++++++++++++
> >> drivers/iio/accel/adxl313_i2c.c | 65 +++++++
> >> drivers/iio/accel/adxl313_spi.c | 85 ++++++++
> >> 7 files changed, 575 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 a61f4f3b78a9..566055450b6b 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -585,6 +585,15 @@ L: [email protected]
> >> S: Maintained
> >> F: drivers/platform/x86/adv_swbutton.c
> >>
> >> +ADXL313 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
> >> +M: Lucas Stankus <[email protected]>
> >> +S: Supported
> >> +F: Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
> >> +F: drivers/iio/accel/adxl313.h
> >> +F: drivers/iio/accel/adxl313_core.c
> >> +F: drivers/iio/accel/adxl313_i2c.c
> >> +F: drivers/iio/accel/adxl313_spi.c
> >
> >
> >
> > adxl313*?
> >
>
> Didn't know this would work, but I think I prefer the way it is now.
> Are you proposing this as a suggestion or more of a change request?

It's a bit neater and very unlikely we'll get a clash on that wild card
in the long run, so I'd 'slightly' prefer this as well.


...

>> +/*
> >> + * Scale for any g range is given in datasheet as
> >> + * 1024 LSB/g = 0.0009765625 * 9.80665 = 0.009576806640625 m/s^2
> >> + */
> >> +#define ADXL313_NSCALE 9576806
> >
> >
> >
> > Is it in nanonewtons? Perhaps add a suffix _nN?
> >
>
> It's actually in meters per second squared, so I couldn't come up with
> a good suffix. Do you have any suggestions?

Easy. Don't have a #define :)

In all seriousness it isn't a 'magic' number, it's an actual real world
value, so move the comment down to where it's used and put the number
directly were it is needed.


Jonathan

2021-08-08 15:08:43

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: accel: Add binding documentation for ADXL313

On Fri, 6 Aug 2021 21:33:44 -0300
Lucas Stankus <[email protected]> wrote:

> On Fri, Aug 6, 2021 at 3:10 PM Rob Herring <[email protected]> wrote:
> >
> > On Thu, Aug 05, 2021 at 03:29:37AM -0300, Lucas Stankus wrote:
> > > Add device tree binding documentation for ADXL313 3-axis accelerometer.
> > >
> > > Signed-off-by: Lucas Stankus <[email protected]>
> > > ---
> > > .../bindings/iio/accel/adi,adxl313.yaml | 90 +++++++++++++++++++
> > > 1 file changed, 90 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..fea03b6790f3
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml
> > > @@ -0,0 +1,90 @@
> > > +# 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 <[email protected]>
> > > +
> > > +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-cpha: true
> > > +
> > > + spi-cpol: true
> >
> > These 3 generally shouldn't be needed, but can be set from the driver.
> > If they are valid, is any combination of them really valid?
> >
>
> Only the 3wire is optional, both cpha and cpol are required for proper
> spi connection.

We've been round this one a few time, and last time we discussed the
cases where you'd need these in DT (because of inverters on the bus)
https://lore.kernel.org/linux-iio/[email protected]/
conclusion was, that we don't want to put the burden on the dt files
for those odd cases. The equivalent for interrupt lines is interestingly
different because in those cases the two-cell version includes the
type of interrupt, so it makes little sense to push that down into the
drivers as well.

Mind you I'm not 100% sure how we would retrofit a binding if necessary
for the inverted cases. Hope we don't hit one here :)

As you note, 3wire is needed in the binding because it's optional.

Jonathan


>
> > > +
> > > + 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:
> > > + maxItems: 2
> >
> > This means there must be 2 entries. If 1 is valid, you need 'minItems'.
> >
>
> I'll add 'minItems' for the v3 then, thanks!
>
> > > +
> > > + interrupt-names:
> > > + maxItems: 2
> >
> > You need 'minItems' too to fix the error.
> >
>
> Thank you again and sorry for not catching that error before submitting.
>
> > > + 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>;
> > > + spi-cpol;
> > > + spi-cpha;
> > > + interrupt-parent = <&gpio0>;
> > > + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > > + interrupt-names = "INT1";
> > > + };
> > > + };
> > > --
> > > 2.32.0
> > >
> > >