Introduce the DATA_READY trigger and enable triggered buffering. Additional
changes include introduction of functions set_mode and data_ready, allow
either INT1/INT2 pin be used by specifying interrupt-names.
Triggered buffer was tested on both DATA_READY trigger and the hrtimer software
trigger.
~ # ls /sys/bus/iio/devices/
iio:device0 trigger0 trigger1
~ # ls /config/iio/triggers/hrtimer/
t1
~ # cat /sys/bus/iio/devices/trigger0/name
t1
~ # cat /sys/bus/iio/devices/trigger1/name
adxl345-dev0
~ # iio_generic_buffer -n adxl345 -t t1 -c 5 -l 20 -a
iio device number being used is 0
iio trigger number being used is 0
Enabling all channels
Enabling: in_accel_y_en
Enabling: in_accel_x_en
Enabling: in_timestamp_en
Enabling: in_accel_z_en
/sys/bus/iio/devices/iio:device0 t1
0.306400 0.880900 9.575000 1493432411145155964
0.306400 0.880900 9.575000 1493432411185154872
0.306400 0.919200 9.536700 1493432411225167667
0.306400 0.880900 9.536700 1493432411265157809
0.344700 0.919200 9.536700 1493432411295108767
Disabling: in_accel_y_en
Disabling: in_accel_x_en
Disabling: in_timestamp_en
Disabling: in_accel_z_en
~ # iio_generic_buffer -n adxl345 -t adxl345-dev0 -c 5 -l 20 -a
iio device number being used is 0
iio trigger number being used is 1
Enabling all channels
Enabling: in_accel_y_en
Enabling: in_accel_x_en
Enabling: in_timestamp_en
Enabling: in_accel_z_en
/sys/bus/iio/devices/iio:device0 adxl345-dev0
0.344700 0.919200 9.689900 1493432411336475189
0.344700 0.880900 9.575000 1493432411336475189
0.306400 0.957500 9.651600 1493432411336475189
0.306400 0.880900 9.575000 1493432411336475189
0.306400 0.919200 9.536700 1493432411336475189
Disabling: in_accel_y_en
Disabling: in_accel_x_en
Disabling: in_timestamp_en
Disabling: in_accel_z_en
~ #
Changes in v2:
* Provide a detailed commit message to those missing it
* Add Rob's Acked-by tag
* Make function naming more clear: drdy -> data_ready
* Switch from while to do..while
* Rename regval to val
* Switch to usleep_range() for shorter delay
* Add comment to justify delay
* Change error code -EIO to -EAGAIN
* Endianness issue: scrap the get_triple function entirely, make no
changes in terms of reading registers in read_raw
* Introduce mutex earlier rather than in succeeding patch
* Probe:
* Fix random whitespace omission
* Remove configuring to standby mode, the device powers on in standby
mode so this is not needed
* use variable 'regval' to hold value to be written to the register and call
regmap_write() unconditionally
* fix line splitting in devm_request_threaded_irq() and devm_iio_trigger_alloc()
* Switch to devm_iio_trigger_register()
* Switch to devm_iio_triggered_buffer_setup()
* Move the of_irq_get_byname() check in core file in order to avoid
introducing another parameter in probe()
* adxl345_irq():
* return values directly
* switch from iio_trigger_poll() to iio_trigger_poll_chained(), the former
should only be called at the top-half not at the bottom-half.
* adxl345_drdy_trigger_set_state():
* move regmap_get_device() to definition block
* regmap_update_bits(): line splitting - one parameter per line, remove extra
parenthesis
* adxl345_trigger_handler()
* if using external trigger, place a adxl345_data_ready() call before
performing a bulk read
* Since get_triple() is scrapped, place a direct bulk read here
* Move mutex unlocking below goto label
* Remove i2c_check_functionality() that could introduce regression
Eva Rachel Retuya (4):
dt-bindings: iio: accel: adxl345: Add optional interrupt-names support
iio: accel: adxl345_core: Introduce set_mode and data_ready functions
iio: accel: adxl345: Setup DATA_READY trigger
iio: accel: adxl345: Add support for triggered buffer
.../devicetree/bindings/iio/accel/adxl345.txt | 4 +
drivers/iio/accel/Kconfig | 2 +
drivers/iio/accel/adxl345.h | 2 +-
drivers/iio/accel/adxl345_core.c | 281 ++++++++++++++++++++-
drivers/iio/accel/adxl345_i2c.c | 3 +-
drivers/iio/accel/adxl345_spi.c | 2 +-
6 files changed, 278 insertions(+), 16 deletions(-)
--
2.7.4
Add interrupt-names property in order to specify interrupt pin in use.
Signed-off-by: Eva Rachel Retuya <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Change in v2:
* Add Rob's Acked-by tag
Documentation/devicetree/bindings/iio/accel/adxl345.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/accel/adxl345.txt b/Documentation/devicetree/bindings/iio/accel/adxl345.txt
index e7111b0..a7c9022 100644
--- a/Documentation/devicetree/bindings/iio/accel/adxl345.txt
+++ b/Documentation/devicetree/bindings/iio/accel/adxl345.txt
@@ -15,6 +15,8 @@ Optional properties:
in Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
- interrupts: interrupt mapping for IRQ as documented in
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+ - interrupt-names: specify INTx pin in use. Should be "INT1" for INT1 pin or
+ "INT2" for INT2 pin.
Example for a I2C device node:
@@ -23,6 +25,7 @@ Example for a I2C device node:
reg = <0x53>;
interrupt-parent = <&gpio1>;
interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "INT1";
};
Example for a SPI device node:
@@ -35,4 +38,5 @@ Example for a SPI device node:
spi-cpha;
interrupt-parent = <&gpio1>;
interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "INT1";
};
--
2.7.4
Move code that enables measurement/standby mode into its own function
and call that function when appropriate. Previously, we set the sensor
to measurement in probe and back to standby during remove. Change it
here to only enter measurement mode when request for data is initiated.
The DATA_READY bit is always set if the corresponding event occurs.
Introduce the data_ready function that monitors the INT_SOURCE register
of this bit. This is done to ensure consistent readings.
Signed-off-by: Eva Rachel Retuya <[email protected]>
---
Changes in v2:
* Make function naming more clear: drdy -> data_ready
* Switch from while to do..while
* Rename regval to val
* Switch to usleep_range() for shorter delay
* Add comment to justify delay
* Change error code -EIO to -EAGAIN
* Endianness issue: scrap the get_triple function entirely, make no
changes in terms of reading registers in read_raw
* Introduce mutex here rather than in succeeding patch
* Probe:
* Fix random whitespace omission
* Remove configuring to standby mode, the device powers on in standby
mode so this is not needed
drivers/iio/accel/adxl345_core.c | 84 +++++++++++++++++++++++++++++++++-------
1 file changed, 69 insertions(+), 15 deletions(-)
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index 9ccb582..b8a212c 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -8,6 +8,7 @@
* directory of this archive for more details.
*/
+#include <linux/delay.h>
#include <linux/module.h>
#include <linux/regmap.h>
@@ -17,6 +18,7 @@
#define ADXL345_REG_DEVID 0x00
#define ADXL345_REG_POWER_CTL 0x2D
+#define ADXL345_REG_INT_SOURCE 0x30
#define ADXL345_REG_DATA_FORMAT 0x31
#define ADXL345_REG_DATAX0 0x32
#define ADXL345_REG_DATAY0 0x34
@@ -25,6 +27,10 @@
#define ADXL345_POWER_CTL_MEASURE BIT(3)
#define ADXL345_POWER_CTL_STANDBY 0x00
+/* INT_ENABLE/INT_MAP/INT_SOURCE bits */
+#define ADXL345_INT_DATA_READY BIT(7)
+#define ADXL345_INT_OVERRUN 0
+
#define ADXL345_DATA_FORMAT_FULL_RES BIT(3) /* Up to 13-bits resolution */
#define ADXL345_DATA_FORMAT_2G 0
#define ADXL345_DATA_FORMAT_4G 1
@@ -44,9 +50,49 @@ static const int adxl345_uscale = 38300;
struct adxl345_data {
struct regmap *regmap;
+ struct mutex lock; /* protect this data structure */
u8 data_range;
};
+static int adxl345_set_mode(struct adxl345_data *data, u8 mode)
+{
+ struct device *dev = regmap_get_device(data->regmap);
+ int ret;
+
+ ret = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, mode);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set power mode, %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int adxl345_data_ready(struct adxl345_data *data)
+{
+ struct device *dev = regmap_get_device(data->regmap);
+ int tries = 5;
+ u32 val;
+ int ret;
+
+ do {
+ /*
+ * 1/ODR + 1.1ms; 11.1ms at ODR of 0.10 Hz
+ * Sensor currently operates at default ODR of 100 Hz
+ */
+ usleep_range(1100, 11100);
+
+ ret = regmap_read(data->regmap, ADXL345_REG_INT_SOURCE, &val);
+ if (ret < 0)
+ return ret;
+ if ((val & ADXL345_INT_DATA_READY) == ADXL345_INT_DATA_READY)
+ return 0;
+ } while (--tries);
+ dev_err(dev, "Data is not yet ready, try again.\n");
+
+ return -EAGAIN;
+}
+
#define ADXL345_CHANNEL(reg, axis) { \
.type = IIO_ACCEL, \
.modified = 1, \
@@ -72,6 +118,19 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
+ mutex_lock(&data->lock);
+ ret = adxl345_set_mode(data, ADXL345_POWER_CTL_MEASURE);
+ if (ret < 0) {
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ ret = adxl345_data_ready(data);
+ if (ret < 0) {
+ adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
/*
* Data is stored in adjacent registers:
* ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte
@@ -79,10 +138,15 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
*/
ret = regmap_bulk_read(data->regmap, chan->address, ®val,
sizeof(regval));
- if (ret < 0)
+ mutex_unlock(&data->lock);
+ if (ret < 0) {
+ adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
return ret;
+ }
*val = sign_extend32(le16_to_cpu(regval), 12);
+ adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
+
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
*val = 0;
@@ -136,6 +200,8 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
return ret;
}
+ mutex_init(&data->lock);
+
indio_dev->dev.parent = dev;
indio_dev->name = name;
indio_dev->info = &adxl345_info;
@@ -143,20 +209,9 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
indio_dev->channels = adxl345_channels;
indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
- /* Enable measurement mode */
- ret = regmap_write(data->regmap, ADXL345_REG_POWER_CTL,
- ADXL345_POWER_CTL_MEASURE);
- if (ret < 0) {
- dev_err(dev, "Failed to enable measurement mode: %d\n", ret);
- return ret;
- }
-
ret = iio_device_register(indio_dev);
- if (ret < 0) {
+ if (ret < 0)
dev_err(dev, "iio_device_register failed: %d\n", ret);
- regmap_write(data->regmap, ADXL345_REG_POWER_CTL,
- ADXL345_POWER_CTL_STANDBY);
- }
return ret;
}
@@ -169,8 +224,7 @@ int adxl345_core_remove(struct device *dev)
iio_device_unregister(indio_dev);
- return regmap_write(data->regmap, ADXL345_REG_POWER_CTL,
- ADXL345_POWER_CTL_STANDBY);
+ return adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
}
EXPORT_SYMBOL_GPL(adxl345_core_remove);
--
2.7.4
The ADXL345 provides a DATA_READY interrupt function to signal
availability of new data. This interrupt function is latched and can be
cleared by reading the data registers. The polarity is set to active
high by default.
Support this functionality by setting it up as an IIO trigger.
In addition, two output pins INT1 and INT2 are available for driving
interrupts. Allow mapping to either pins by specifying the
interrupt-names property in device tree.
Signed-off-by: Eva Rachel Retuya <[email protected]>
---
Changes in v2:
* Provide a detailed commit message
* Move the of_irq_get_byname() check in core file in order to avoid
introducing another parameter in probe()
* adxl345_irq():
* return values directly
* switch from iio_trigger_poll() to iio_trigger_poll_chained(), the former
should only be called at the top-half not at the bottom-half.
* adxl345_drdy_trigger_set_state():
* move regmap_get_device() to definition block
* regmap_update_bits(): line splitting - one parameter per line, remove extra
parenthesis
* probe()
* use variable 'regval' to hold value to be written to the register and call
regmap_write() unconditionally
* fix line splitting in devm_request_threaded_irq() and devm_iio_trigger_alloc()
* Switch to devm_iio_trigger_register()
drivers/iio/accel/adxl345.h | 2 +-
drivers/iio/accel/adxl345_core.c | 104 ++++++++++++++++++++++++++++++++++++++-
drivers/iio/accel/adxl345_i2c.c | 3 +-
drivers/iio/accel/adxl345_spi.c | 2 +-
4 files changed, 107 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
index c1ddf39..d2fa806 100644
--- a/drivers/iio/accel/adxl345.h
+++ b/drivers/iio/accel/adxl345.h
@@ -11,7 +11,7 @@
#ifndef _ADXL345_H_
#define _ADXL345_H_
-int adxl345_core_probe(struct device *dev, struct regmap *regmap,
+int adxl345_core_probe(struct device *dev, struct regmap *regmap, int irq,
const char *name);
int adxl345_core_remove(struct device *dev);
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index b8a212c..b8be0d7 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -9,15 +9,20 @@
*/
#include <linux/delay.h>
+#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/of_irq.h>
#include <linux/regmap.h>
#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
#include "adxl345.h"
#define ADXL345_REG_DEVID 0x00
#define ADXL345_REG_POWER_CTL 0x2D
+#define ADXL345_REG_INT_ENABLE 0x2E
+#define ADXL345_REG_INT_MAP 0x2F
#define ADXL345_REG_INT_SOURCE 0x30
#define ADXL345_REG_DATA_FORMAT 0x31
#define ADXL345_REG_DATAX0 0x32
@@ -39,6 +44,8 @@
#define ADXL345_DEVID 0xE5
+#define ADXL345_IRQ_NAME "adxl345_event"
+
/*
* In full-resolution mode, scale factor is maintained at ~4 mg/LSB
* in all g ranges.
@@ -49,6 +56,8 @@
static const int adxl345_uscale = 38300;
struct adxl345_data {
+ struct iio_trigger *data_ready_trig;
+ bool data_ready_trig_on;
struct regmap *regmap;
struct mutex lock; /* protect this data structure */
u8 data_range;
@@ -158,17 +167,62 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
return -EINVAL;
}
+static irqreturn_t adxl345_irq(int irq, void *p)
+{
+ struct iio_dev *indio_dev = p;
+ struct adxl345_data *data = iio_priv(indio_dev);
+ int ret;
+ u32 int_stat;
+
+ ret = regmap_read(data->regmap, ADXL345_REG_INT_SOURCE, &int_stat);
+ if (ret < 0)
+ return IRQ_HANDLED;
+
+ if (int_stat & ADXL345_INT_DATA_READY) {
+ iio_trigger_poll_chained(data->data_ready_trig);
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
+static int adxl345_drdy_trigger_set_state(struct iio_trigger *trig, bool state)
+{
+ struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+ struct adxl345_data *data = iio_priv(indio_dev);
+ struct device *dev = regmap_get_device(data->regmap);
+ int ret;
+
+ ret = regmap_update_bits(data->regmap,
+ ADXL345_REG_INT_ENABLE,
+ ADXL345_INT_DATA_READY,
+ state ? ADXL345_INT_DATA_READY : 0);
+ if (ret < 0) {
+ dev_err(dev, "Failed to update INT_ENABLE bits\n");
+ return ret;
+ }
+ data->data_ready_trig_on = state;
+
+ return ret;
+}
+
+static const struct iio_trigger_ops adxl345_trigger_ops = {
+ .owner = THIS_MODULE,
+ .set_trigger_state = adxl345_drdy_trigger_set_state,
+};
+
static const struct iio_info adxl345_info = {
.driver_module = THIS_MODULE,
.read_raw = adxl345_read_raw,
};
-int adxl345_core_probe(struct device *dev, struct regmap *regmap,
+int adxl345_core_probe(struct device *dev, struct regmap *regmap, int irq,
const char *name)
{
struct adxl345_data *data;
struct iio_dev *indio_dev;
u32 regval;
+ int of_irq;
int ret;
ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val);
@@ -199,6 +253,22 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
dev_err(dev, "Failed to set data range: %d\n", ret);
return ret;
}
+ /*
+ * Any bits set to 0 send their respective interrupts to the INT1 pin,
+ * whereas bits set to 1 send their respective interrupts to the INT2
+ * pin. Map all interrupts to the specified pin.
+ */
+ of_irq = of_irq_get_byname(dev->of_node, "INT2");
+ if (of_irq == irq)
+ regval = 0xFF;
+ else
+ regval = 0x00;
+
+ ret = regmap_write(data->regmap, ADXL345_REG_INT_MAP, regval);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set up interrupts: %d\n", ret);
+ return ret;
+ }
mutex_init(&data->lock);
@@ -209,6 +279,38 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
indio_dev->channels = adxl345_channels;
indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
+ if (irq > 0) {
+ ret = devm_request_threaded_irq(dev,
+ irq,
+ NULL,
+ adxl345_irq,
+ IRQF_TRIGGER_HIGH |
+ IRQF_ONESHOT,
+ ADXL345_IRQ_NAME,
+ indio_dev);
+ if (ret < 0) {
+ dev_err(dev, "Failed to request irq: %d\n", irq);
+ return ret;
+ }
+
+ data->data_ready_trig = devm_iio_trigger_alloc(dev,
+ "%s-dev%d",
+ indio_dev->name,
+ indio_dev->id);
+ if (!data->data_ready_trig)
+ return -ENOMEM;
+
+ data->data_ready_trig->dev.parent = dev;
+ data->data_ready_trig->ops = &adxl345_trigger_ops;
+ iio_trigger_set_drvdata(data->data_ready_trig, indio_dev);
+
+ ret = devm_iio_trigger_register(dev, data->data_ready_trig);
+ if (ret) {
+ dev_err(dev, "Failed to register trigger: %d\n", ret);
+ return ret;
+ }
+ }
+
ret = iio_device_register(indio_dev);
if (ret < 0)
dev_err(dev, "iio_device_register failed: %d\n", ret);
diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c
index 05e1ec4..31af702 100644
--- a/drivers/iio/accel/adxl345_i2c.c
+++ b/drivers/iio/accel/adxl345_i2c.c
@@ -34,7 +34,8 @@ static int adxl345_i2c_probe(struct i2c_client *client,
return PTR_ERR(regmap);
}
- return adxl345_core_probe(&client->dev, regmap, id ? id->name : NULL);
+ return adxl345_core_probe(&client->dev, regmap, client->irq,
+ id ? id->name : NULL);
}
static int adxl345_i2c_remove(struct i2c_client *client)
diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
index 6d65819..75a8c12 100644
--- a/drivers/iio/accel/adxl345_spi.c
+++ b/drivers/iio/accel/adxl345_spi.c
@@ -42,7 +42,7 @@ static int adxl345_spi_probe(struct spi_device *spi)
return PTR_ERR(regmap);
}
- return adxl345_core_probe(&spi->dev, regmap, id->name);
+ return adxl345_core_probe(&spi->dev, regmap, spi->irq, id->name);
}
static int adxl345_spi_remove(struct spi_device *spi)
--
2.7.4
Previously, the only way to capture data is to read the exposed sysfs
files in_accel_[x/y/z]_raw and applying the scale from in_accel_scale.
Provide a way for continuous data capture that allows multiple data
channels to be read at once by setting up buffer support.
Initialize scan_type fields that describe the buffer and make sure to
claim and release direct mode in read_raw. The latter is done to ensure
no raw reads are attempted when utilizing the buffer and vice versa.
Lastly, add triggered buffer support that allows utilization of any
given trigger such as DATA_READY, hrtimer, etc. to initiate capturing of
data and placing said data in the buffer. The trigger handler performs
an all-axes read with timestamp.
Signed-off-by: Eva Rachel Retuya <[email protected]>
---
Changes in v2:
* Provide a more detailed commit message
* adxl345_trigger_handler()
* if using external trigger, place a adxl345_data_ready() call before
performing a bulk read
* Since get_triple() is scrapped, place a direct bulk read here
* Move mutex unlocking below goto label
* Switch to devm_iio_triggered_buffer_setup()
* Remove i2c_check_functionality() that could introduce regression
drivers/iio/accel/Kconfig | 2 +
drivers/iio/accel/adxl345_core.c | 101 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 15de262..45092da 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -7,6 +7,8 @@ menu "Accelerometers"
config ADXL345
tristate
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
config ADXL345_I2C
tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver"
diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index b8be0d7..40dbdce 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -14,8 +14,11 @@
#include <linux/of_irq.h>
#include <linux/regmap.h>
+#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
#include "adxl345.h"
@@ -55,12 +58,20 @@
*/
static const int adxl345_uscale = 38300;
+enum adxl345_scan_index {
+ ADXL345_IDX_X,
+ ADXL345_IDX_Y,
+ ADXL345_IDX_Z,
+ ADXL345_IDX_TSTAMP,
+};
+
struct adxl345_data {
struct iio_trigger *data_ready_trig;
bool data_ready_trig_on;
struct regmap *regmap;
struct mutex lock; /* protect this data structure */
u8 data_range;
+ s16 buffer[8]; /* 3 x 16-bit channels + padding + 64-bit timestamp */
};
static int adxl345_set_mode(struct adxl345_data *data, u8 mode)
@@ -109,12 +120,25 @@ static int adxl345_data_ready(struct adxl345_data *data)
.address = reg, \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_index = ADXL345_IDX_##axis, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 13, \
+ .storagebits = 16, \
+ .endianness = IIO_LE, \
+ }, \
}
static const struct iio_chan_spec adxl345_channels[] = {
ADXL345_CHANNEL(ADXL345_REG_DATAX0, X),
ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y),
ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z),
+ IIO_CHAN_SOFT_TIMESTAMP(ADXL345_IDX_TSTAMP),
+};
+
+static const unsigned long adxl345_scan_masks[] = {
+ BIT(ADXL345_IDX_X) | BIT(ADXL345_IDX_Y) | BIT(ADXL345_IDX_Z),
+ 0
};
static int adxl345_read_raw(struct iio_dev *indio_dev,
@@ -127,6 +151,10 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
mutex_lock(&data->lock);
ret = adxl345_set_mode(data, ADXL345_POWER_CTL_MEASURE);
if (ret < 0) {
@@ -148,12 +176,14 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
ret = regmap_bulk_read(data->regmap, chan->address, ®val,
sizeof(regval));
mutex_unlock(&data->lock);
+ iio_device_release_direct_mode(indio_dev);
if (ret < 0) {
adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
return ret;
}
- *val = sign_extend32(le16_to_cpu(regval), 12);
+ *val = sign_extend32(le16_to_cpu(regval),
+ chan->scan_type.realbits - 1);
adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
return IIO_VAL_INT;
@@ -186,6 +216,64 @@ static irqreturn_t adxl345_irq(int irq, void *p)
return IRQ_NONE;
}
+static irqreturn_t adxl345_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct adxl345_data *data = iio_priv(indio_dev);
+ int ret;
+
+ mutex_lock(&data->lock);
+ /* Make sure data is ready when using external trigger */
+ if (!data->data_ready_trig_on) {
+ ret = adxl345_data_ready(data);
+ if (ret < 0)
+ goto error;
+ }
+
+ ret = regmap_bulk_read(data->regmap, ADXL345_REG_DATAX0, data->buffer,
+ sizeof(__le16) * 3);
+ if (ret < 0)
+ goto error;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
+ pf->timestamp);
+error:
+ mutex_unlock(&data->lock);
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static int adxl345_triggered_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct adxl345_data *data = iio_priv(indio_dev);
+ int ret;
+
+ ret = iio_triggered_buffer_postenable(indio_dev);
+ if (ret)
+ return ret;
+
+ return adxl345_set_mode(data, ADXL345_POWER_CTL_MEASURE);
+}
+
+static int adxl345_triggered_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct adxl345_data *data = iio_priv(indio_dev);
+ int ret;
+
+ ret = adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
+ if (ret)
+ return ret;
+
+ return iio_triggered_buffer_predisable(indio_dev);
+}
+
+static const struct iio_buffer_setup_ops adxl345_buffer_setup_ops = {
+ .postenable = adxl345_triggered_buffer_postenable,
+ .predisable = adxl345_triggered_buffer_predisable,
+};
+
static int adxl345_drdy_trigger_set_state(struct iio_trigger *trig, bool state)
{
struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
@@ -278,6 +366,7 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap, int irq,
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = adxl345_channels;
indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
+ indio_dev->available_scan_masks = adxl345_scan_masks;
if (irq > 0) {
ret = devm_request_threaded_irq(dev,
@@ -311,6 +400,16 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap, int irq,
}
}
+ ret = devm_iio_triggered_buffer_setup(dev,
+ indio_dev,
+ iio_pollfunc_store_time,
+ adxl345_trigger_handler,
+ &adxl345_buffer_setup_ops);
+ if (ret < 0) {
+ dev_err(dev, "iio_triggered_buffer_setup failed: %d\n", ret);
+ return ret;
+ }
+
ret = iio_device_register(indio_dev);
if (ret < 0)
dev_err(dev, "iio_device_register failed: %d\n", ret);
--
2.7.4
On 29/04/17 08:48, Eva Rachel Retuya wrote:
> Move code that enables measurement/standby mode into its own function
> and call that function when appropriate. Previously, we set the sensor
> to measurement in probe and back to standby during remove. Change it
> here to only enter measurement mode when request for data is initiated.
>
> The DATA_READY bit is always set if the corresponding event occurs.
> Introduce the data_ready function that monitors the INT_SOURCE register
> of this bit. This is done to ensure consistent readings.
>
> Signed-off-by: Eva Rachel Retuya <[email protected]>
> ---
> Changes in v2:
> * Make function naming more clear: drdy -> data_ready
> * Switch from while to do..while
> * Rename regval to val
> * Switch to usleep_range() for shorter delay
> * Add comment to justify delay
> * Change error code -EIO to -EAGAIN
> * Endianness issue: scrap the get_triple function entirely, make no
> changes in terms of reading registers in read_raw
> * Introduce mutex here rather than in succeeding patch
> * Probe:
> * Fix random whitespace omission
> * Remove configuring to standby mode, the device powers on in standby
> mode so this is not needed
>
> drivers/iio/accel/adxl345_core.c | 84 +++++++++++++++++++++++++++++++++-------
> 1 file changed, 69 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 9ccb582..b8a212c 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -8,6 +8,7 @@
> * directory of this archive for more details.
> */
>
> +#include <linux/delay.h>
> #include <linux/module.h>
> #include <linux/regmap.h>
>
> @@ -17,6 +18,7 @@
>
> #define ADXL345_REG_DEVID 0x00
> #define ADXL345_REG_POWER_CTL 0x2D
> +#define ADXL345_REG_INT_SOURCE 0x30
> #define ADXL345_REG_DATA_FORMAT 0x31
> #define ADXL345_REG_DATAX0 0x32
> #define ADXL345_REG_DATAY0 0x34
> @@ -25,6 +27,10 @@
> #define ADXL345_POWER_CTL_MEASURE BIT(3)
> #define ADXL345_POWER_CTL_STANDBY 0x00
>
> +/* INT_ENABLE/INT_MAP/INT_SOURCE bits */
> +#define ADXL345_INT_DATA_READY BIT(7)
> +#define ADXL345_INT_OVERRUN 0
> +
> #define ADXL345_DATA_FORMAT_FULL_RES BIT(3) /* Up to 13-bits resolution */
> #define ADXL345_DATA_FORMAT_2G 0
> #define ADXL345_DATA_FORMAT_4G 1
> @@ -44,9 +50,49 @@ static const int adxl345_uscale = 38300;
>
> struct adxl345_data {
> struct regmap *regmap;
> + struct mutex lock; /* protect this data structure */
> u8 data_range;
> };
>
> +static int adxl345_set_mode(struct adxl345_data *data, u8 mode)
> +{
> + struct device *dev = regmap_get_device(data->regmap);
> + int ret;
> +
> + ret = regmap_write(data->regmap, ADXL345_REG_POWER_CTL, mode);
> + if (ret < 0) {
> + dev_err(dev, "Failed to set power mode, %d\n", ret);
> + return ret;
drop the return ret here and just return ret at the end of the function.
One of the static checkers will probably moan about this otherwise.
> + }
> +
> + return 0;
> +}
> +
> +static int adxl345_data_ready(struct adxl345_data *data)
> +{
So this is a polling the dataready bit. Will ensure we always
get fresh data when a read occurs. Please add a comment to
that effect as that's not always how devices work.
> + struct device *dev = regmap_get_device(data->regmap);
> + int tries = 5;
> + u32 val;
> + int ret;
> +
> + do {
> + /*
> + * 1/ODR + 1.1ms; 11.1ms at ODR of 0.10 Hz
> + * Sensor currently operates at default ODR of 100 Hz
> + */
> + usleep_range(1100, 11100);
That's a huge range to allow... I'm not following the argument for why.
Or do we have a stray 0?
> +
> + ret = regmap_read(data->regmap, ADXL345_REG_INT_SOURCE, &val);
> + if (ret < 0)
> + return ret;
> + if ((val & ADXL345_INT_DATA_READY) == ADXL345_INT_DATA_READY)
> + return 0;
> + } while (--tries);
> + dev_err(dev, "Data is not yet ready, try again.\n");
> +
This is almost certainly a hardware fault. I'd be more brutal with
the error and return -EIO. If you get here your hardware is very unlikely
to be working correctly if you try again.
> + return -EAGAIN;
> +}
> +
> #define ADXL345_CHANNEL(reg, axis) { \
> .type = IIO_ACCEL, \
> .modified = 1, \
> @@ -72,6 +118,19 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> + mutex_lock(&data->lock);
> + ret = adxl345_set_mode(data, ADXL345_POWER_CTL_MEASURE);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + ret = adxl345_data_ready(data);
> + if (ret < 0) {
> + adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
> + mutex_unlock(&data->lock);
What is the logic that puts the mutex_unlock here in the error case
and before the set_mode in the normal path? Even if it doesn't
matter make them the same as it is less likely to raise questions
in the future!
> + return ret;
> + }
> /*
> * Data is stored in adjacent registers:
> * ADXL345_REG_DATA(X0/Y0/Z0) contain the least significant byte
> @@ -79,10 +138,15 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
> */
> ret = regmap_bulk_read(data->regmap, chan->address, ®val,
> sizeof(regval));
> - if (ret < 0)
> + mutex_unlock(&data->lock);
> + if (ret < 0) {
> + adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
> return ret;
> + }
>
> *val = sign_extend32(le16_to_cpu(regval), 12);
> + adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
> +
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> *val = 0;
> @@ -136,6 +200,8 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> return ret;
> }
>
> + mutex_init(&data->lock);
> +
> indio_dev->dev.parent = dev;
> indio_dev->name = name;
> indio_dev->info = &adxl345_info;
> @@ -143,20 +209,9 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> indio_dev->channels = adxl345_channels;
> indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
>
> - /* Enable measurement mode */
> - ret = regmap_write(data->regmap, ADXL345_REG_POWER_CTL,
> - ADXL345_POWER_CTL_MEASURE);
> - if (ret < 0) {
> - dev_err(dev, "Failed to enable measurement mode: %d\n", ret);
> - return ret;
> - }
> -
> ret = iio_device_register(indio_dev);
> - if (ret < 0) {
> + if (ret < 0)
> dev_err(dev, "iio_device_register failed: %d\n", ret);
> - regmap_write(data->regmap, ADXL345_REG_POWER_CTL,
> - ADXL345_POWER_CTL_STANDBY);
> - }
>
> return ret;
> }
> @@ -169,8 +224,7 @@ int adxl345_core_remove(struct device *dev)
>
> iio_device_unregister(indio_dev);
>
> - return regmap_write(data->regmap, ADXL345_REG_POWER_CTL,
> - ADXL345_POWER_CTL_STANDBY);
> + return adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
Under what circumstances would we not already be in the correct state?
A brief comment here would be good.
> }
> EXPORT_SYMBOL_GPL(adxl345_core_remove);
>
>
On 29/04/17 08:49, Eva Rachel Retuya wrote:
> The ADXL345 provides a DATA_READY interrupt function to signal
> availability of new data. This interrupt function is latched and can be
> cleared by reading the data registers. The polarity is set to active
> high by default.
>
> Support this functionality by setting it up as an IIO trigger.
>
> In addition, two output pins INT1 and INT2 are available for driving
> interrupts. Allow mapping to either pins by specifying the
> interrupt-names property in device tree.
>
> Signed-off-by: Eva Rachel Retuya <[email protected]>
Coming together nicely, but a few more bits and pieces inline...
One slight worry is that the irq names stuff is to restrictive
as we want to direct different interrupts to different pins if
both are supported!
Jonathan
> ---
> Changes in v2:
> * Provide a detailed commit message
> * Move the of_irq_get_byname() check in core file in order to avoid
> introducing another parameter in probe()
> * adxl345_irq():
> * return values directly
> * switch from iio_trigger_poll() to iio_trigger_poll_chained(), the former
> should only be called at the top-half not at the bottom-half.
> * adxl345_drdy_trigger_set_state():
> * move regmap_get_device() to definition block
> * regmap_update_bits(): line splitting - one parameter per line, remove extra
> parenthesis
> * probe()
> * use variable 'regval' to hold value to be written to the register and call
> regmap_write() unconditionally
> * fix line splitting in devm_request_threaded_irq() and devm_iio_trigger_alloc()
> * Switch to devm_iio_trigger_register()
>
> drivers/iio/accel/adxl345.h | 2 +-
> drivers/iio/accel/adxl345_core.c | 104 ++++++++++++++++++++++++++++++++++++++-
> drivers/iio/accel/adxl345_i2c.c | 3 +-
> drivers/iio/accel/adxl345_spi.c | 2 +-
> 4 files changed, 107 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h
> index c1ddf39..d2fa806 100644
> --- a/drivers/iio/accel/adxl345.h
> +++ b/drivers/iio/accel/adxl345.h
> @@ -11,7 +11,7 @@
> #ifndef _ADXL345_H_
> #define _ADXL345_H_
>
> -int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> +int adxl345_core_probe(struct device *dev, struct regmap *regmap, int irq,
> const char *name);
> int adxl345_core_remove(struct device *dev);
>
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index b8a212c..b8be0d7 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -9,15 +9,20 @@
> */
>
> #include <linux/delay.h>
> +#include <linux/interrupt.h>
> #include <linux/module.h>
> +#include <linux/of_irq.h>
> #include <linux/regmap.h>
>
> #include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
>
> #include "adxl345.h"
>
> #define ADXL345_REG_DEVID 0x00
> #define ADXL345_REG_POWER_CTL 0x2D
> +#define ADXL345_REG_INT_ENABLE 0x2E
> +#define ADXL345_REG_INT_MAP 0x2F
> #define ADXL345_REG_INT_SOURCE 0x30
> #define ADXL345_REG_DATA_FORMAT 0x31
> #define ADXL345_REG_DATAX0 0x32
> @@ -39,6 +44,8 @@
>
> #define ADXL345_DEVID 0xE5
>
> +#define ADXL345_IRQ_NAME "adxl345_event"
I'd just put this inline. It doesn't really give any benefit to
have this defined at the top.
> +
> /*
> * In full-resolution mode, scale factor is maintained at ~4 mg/LSB
> * in all g ranges.
> @@ -49,6 +56,8 @@
> static const int adxl345_uscale = 38300;
>
> struct adxl345_data {
> + struct iio_trigger *data_ready_trig;
> + bool data_ready_trig_on;
> struct regmap *regmap;
> struct mutex lock; /* protect this data structure */
> u8 data_range;
> @@ -158,17 +167,62 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
> return -EINVAL;
> }
>
> +static irqreturn_t adxl345_irq(int irq, void *p)
> +{
> + struct iio_dev *indio_dev = p;
> + struct adxl345_data *data = iio_priv(indio_dev);
> + int ret;
> + u32 int_stat;
> +
> + ret = regmap_read(data->regmap, ADXL345_REG_INT_SOURCE, &int_stat);
> + if (ret < 0)
> + return IRQ_HANDLED;
> +
> + if (int_stat & ADXL345_INT_DATA_READY) {
> + iio_trigger_poll_chained(data->data_ready_trig);
> + return IRQ_HANDLED;
> + }
> +
> + return IRQ_NONE;
> +}
> +
> +static int adxl345_drdy_trigger_set_state(struct iio_trigger *trig, bool state)
> +{
> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> + struct adxl345_data *data = iio_priv(indio_dev);
> + struct device *dev = regmap_get_device(data->regmap);
> + int ret;
> +
> + ret = regmap_update_bits(data->regmap,
> + ADXL345_REG_INT_ENABLE,
> + ADXL345_INT_DATA_READY,
> + state ? ADXL345_INT_DATA_READY : 0);
> + if (ret < 0) {
> + dev_err(dev, "Failed to update INT_ENABLE bits\n");
> + return ret;
> + }
> + data->data_ready_trig_on = state;
> +
> + return ret;
> +}
> +
> +static const struct iio_trigger_ops adxl345_trigger_ops = {
> + .owner = THIS_MODULE,
> + .set_trigger_state = adxl345_drdy_trigger_set_state,
> +};
> +
> static const struct iio_info adxl345_info = {
> .driver_module = THIS_MODULE,
> .read_raw = adxl345_read_raw,
> };
>
> -int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> +int adxl345_core_probe(struct device *dev, struct regmap *regmap, int irq,
> const char *name)
> {
> struct adxl345_data *data;
> struct iio_dev *indio_dev;
> u32 regval;
> + int of_irq;
> int ret;
>
> ret = regmap_read(regmap, ADXL345_REG_DEVID, ®val);
> @@ -199,6 +253,22 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> dev_err(dev, "Failed to set data range: %d\n", ret);
> return ret;
> }
> + /*
> + * Any bits set to 0 send their respective interrupts to the INT1 pin,
> + * whereas bits set to 1 send their respective interrupts to the INT2
> + * pin. Map all interrupts to the specified pin.
This is an interesting comment. The usual reason for dual interrupt
pins is precisely to not map all functions to the same one. That allows
for a saving in querying which interrupt it is by having just the data ready
on one pin and just the events on the other...
Perhaps the current approach won't support that mode of operation?
Clearly we can't merge a binding that enforces them all being the same
and then change it later as it'll be incompatible.
I'm not quite sure how one should do this sort of stuff in DT though.
Rob?
> + */
> + of_irq = of_irq_get_byname(dev->of_node, "INT2");
> + if (of_irq == irq)
> + regval = 0xFF;
> + else
> + regval = 0x00;
> +
> + ret = regmap_write(data->regmap, ADXL345_REG_INT_MAP, regval);
> + if (ret < 0) {
> + dev_err(dev, "Failed to set up interrupts: %d\n", ret);
> + return ret;
> + }
>
> mutex_init(&data->lock);
>
> @@ -209,6 +279,38 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
> indio_dev->channels = adxl345_channels;
> indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
>
> + if (irq > 0) {
> + ret = devm_request_threaded_irq(dev,
> + irq,
> + NULL,
> + adxl345_irq,
> + IRQF_TRIGGER_HIGH |
> + IRQF_ONESHOT,
> + ADXL345_IRQ_NAME,
> + indio_dev);
> + if (ret < 0) {
> + dev_err(dev, "Failed to request irq: %d\n", irq);
> + return ret;
> + }
> +
> + data->data_ready_trig = devm_iio_trigger_alloc(dev,
> + "%s-dev%d",
> + indio_dev->name,
> + indio_dev->id);
> + if (!data->data_ready_trig)
> + return -ENOMEM;
> +
> + data->data_ready_trig->dev.parent = dev;
> + data->data_ready_trig->ops = &adxl345_trigger_ops;
> + iio_trigger_set_drvdata(data->data_ready_trig, indio_dev);
> +
> + ret = devm_iio_trigger_register(dev, data->data_ready_trig);
> + if (ret) {
> + dev_err(dev, "Failed to register trigger: %d\n", ret);
> + return ret;
> + }
> + }
> +
> ret = iio_device_register(indio_dev);
> if (ret < 0)
> dev_err(dev, "iio_device_register failed: %d\n", ret);
> diff --git a/drivers/iio/accel/adxl345_i2c.c b/drivers/iio/accel/adxl345_i2c.c
> index 05e1ec4..31af702 100644
> --- a/drivers/iio/accel/adxl345_i2c.c
> +++ b/drivers/iio/accel/adxl345_i2c.c
> @@ -34,7 +34,8 @@ static int adxl345_i2c_probe(struct i2c_client *client,
> return PTR_ERR(regmap);
> }
>
> - return adxl345_core_probe(&client->dev, regmap, id ? id->name : NULL);
> + return adxl345_core_probe(&client->dev, regmap, client->irq,
> + id ? id->name : NULL);
> }
>
> static int adxl345_i2c_remove(struct i2c_client *client)
> diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
> index 6d65819..75a8c12 100644
> --- a/drivers/iio/accel/adxl345_spi.c
> +++ b/drivers/iio/accel/adxl345_spi.c
> @@ -42,7 +42,7 @@ static int adxl345_spi_probe(struct spi_device *spi)
> return PTR_ERR(regmap);
> }
>
> - return adxl345_core_probe(&spi->dev, regmap, id->name);
> + return adxl345_core_probe(&spi->dev, regmap, spi->irq, id->name);
> }
>
> static int adxl345_spi_remove(struct spi_device *spi)
>
On 29/04/17 08:49, Eva Rachel Retuya wrote:
> Previously, the only way to capture data is to read the exposed sysfs
> files in_accel_[x/y/z]_raw and applying the scale from in_accel_scale.
> Provide a way for continuous data capture that allows multiple data
> channels to be read at once by setting up buffer support.
>
> Initialize scan_type fields that describe the buffer and make sure to
> claim and release direct mode in read_raw. The latter is done to ensure
> no raw reads are attempted when utilizing the buffer and vice versa.
>
> Lastly, add triggered buffer support that allows utilization of any
> given trigger such as DATA_READY, hrtimer, etc. to initiate capturing of
> data and placing said data in the buffer. The trigger handler performs
> an all-axes read with timestamp.
>
> Signed-off-by: Eva Rachel Retuya <[email protected]>
Few minor bits inline... I'm a little bit in two minds about the
holding up waiting for new data when using another trigger...
Jonathan
> ---
> Changes in v2:
> * Provide a more detailed commit message
> * adxl345_trigger_handler()
> * if using external trigger, place a adxl345_data_ready() call before
> performing a bulk read
> * Since get_triple() is scrapped, place a direct bulk read here
> * Move mutex unlocking below goto label
> * Switch to devm_iio_triggered_buffer_setup()
> * Remove i2c_check_functionality() that could introduce regression
>
> drivers/iio/accel/Kconfig | 2 +
> drivers/iio/accel/adxl345_core.c | 101 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 15de262..45092da 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -7,6 +7,8 @@ menu "Accelerometers"
>
> config ADXL345
> tristate
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
>
> config ADXL345_I2C
> tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver"
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index b8be0d7..40dbdce 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -14,8 +14,11 @@
> #include <linux/of_irq.h>
> #include <linux/regmap.h>
>
> +#include <linux/iio/buffer.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
>
> #include "adxl345.h"
>
> @@ -55,12 +58,20 @@
> */
> static const int adxl345_uscale = 38300;
>
> +enum adxl345_scan_index {
> + ADXL345_IDX_X,
> + ADXL345_IDX_Y,
> + ADXL345_IDX_Z,
> + ADXL345_IDX_TSTAMP,
> +};
> +
> struct adxl345_data {
> struct iio_trigger *data_ready_trig;
> bool data_ready_trig_on;
> struct regmap *regmap;
> struct mutex lock; /* protect this data structure */
> u8 data_range;
> + s16 buffer[8]; /* 3 x 16-bit channels + padding + 64-bit timestamp */
> };
>
> static int adxl345_set_mode(struct adxl345_data *data, u8 mode)
> @@ -109,12 +120,25 @@ static int adxl345_data_ready(struct adxl345_data *data)
> .address = reg, \
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_index = ADXL345_IDX_##axis, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 13, \
> + .storagebits = 16, \
> + .endianness = IIO_LE, \
> + }, \
> }
>
> static const struct iio_chan_spec adxl345_channels[] = {
> ADXL345_CHANNEL(ADXL345_REG_DATAX0, X),
> ADXL345_CHANNEL(ADXL345_REG_DATAY0, Y),
> ADXL345_CHANNEL(ADXL345_REG_DATAZ0, Z),
> + IIO_CHAN_SOFT_TIMESTAMP(ADXL345_IDX_TSTAMP),
> +};
> +
> +static const unsigned long adxl345_scan_masks[] = {
> + BIT(ADXL345_IDX_X) | BIT(ADXL345_IDX_Y) | BIT(ADXL345_IDX_Z),
> + 0
> };
>
> static int adxl345_read_raw(struct iio_dev *indio_dev,
> @@ -127,6 +151,10 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> +
> mutex_lock(&data->lock);
> ret = adxl345_set_mode(data, ADXL345_POWER_CTL_MEASURE);
> if (ret < 0) {
> @@ -148,12 +176,14 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
> ret = regmap_bulk_read(data->regmap, chan->address, ®val,
> sizeof(regval));
> mutex_unlock(&data->lock);
> + iio_device_release_direct_mode(indio_dev);
> if (ret < 0) {
> adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
> return ret;
> }
>
> - *val = sign_extend32(le16_to_cpu(regval), 12);
> + *val = sign_extend32(le16_to_cpu(regval),
> + chan->scan_type.realbits - 1)
This change isn't really needed, but I suppose it does little harm...
> adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
>
> return IIO_VAL_INT;
> @@ -186,6 +216,64 @@ static irqreturn_t adxl345_irq(int irq, void *p)
> return IRQ_NONE;
> }
>
> +static irqreturn_t adxl345_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct adxl345_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + mutex_lock(&data->lock);
> + /* Make sure data is ready when using external trigger */
I 'think' this is only really relevant for the very first one.
After that general rule of thumb is that if an external trigger
is too quick - bad luck you'll get repeated data.
One of the reasons we would want to use another trigger is to
support capture in parallel from several sensors - if we 'hold'
like this we'll get out of sync.
As such I wonder if a better strategy would be to 'hold' for the
first reading in the buffer enable - thus guaranteeing valid
data before we start. After that we wouldn't need to check this
here.
What do others think?
> + if (!data->data_ready_trig_on) {
> + ret = adxl345_data_ready(data);
> + if (ret < 0)
> + goto error;
> + }
> +
> + ret = regmap_bulk_read(data->regmap, ADXL345_REG_DATAX0, data->buffer,
> + sizeof(__le16) * 3);
> + if (ret < 0)
> + goto error;
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> + pf->timestamp);
> +error:
> + mutex_unlock(&data->lock);
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int adxl345_triggered_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct adxl345_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = iio_triggered_buffer_postenable(indio_dev);
> + if (ret)
> + return ret;
> +
> + return adxl345_set_mode(data, ADXL345_POWER_CTL_MEASURE);
> +}
> +
> +static int adxl345_triggered_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct adxl345_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = adxl345_set_mode(data, ADXL345_POWER_CTL_STANDBY);
> + if (ret)
> + return ret;
> +
> + return iio_triggered_buffer_predisable(indio_dev);
> +}
> +
> +static const struct iio_buffer_setup_ops adxl345_buffer_setup_ops = {
> + .postenable = adxl345_triggered_buffer_postenable,
> + .predisable = adxl345_triggered_buffer_predisable,
> +};
> +
> static int adxl345_drdy_trigger_set_state(struct iio_trigger *trig, bool state)
> {
> struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> @@ -278,6 +366,7 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap, int irq,
> indio_dev->modes = INDIO_DIRECT_MODE;
> indio_dev->channels = adxl345_channels;
> indio_dev->num_channels = ARRAY_SIZE(adxl345_channels);
> + indio_dev->available_scan_masks = adxl345_scan_masks;
>
> if (irq > 0) {
> ret = devm_request_threaded_irq(dev,
> @@ -311,6 +400,16 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap, int irq,
> }
> }
>
> + ret = devm_iio_triggered_buffer_setup(dev,
> + indio_dev,
> + iio_pollfunc_store_time,
> + adxl345_trigger_handler,
> + &adxl345_buffer_setup_ops);
> + if (ret < 0) {
> + dev_err(dev, "iio_triggered_buffer_setup failed: %d\n", ret);
> + return ret;
> + }
> +
> ret = iio_device_register(indio_dev);
> if (ret < 0)
> dev_err(dev, "iio_device_register failed: %d\n", ret);
>