* [PATCH v4 1/6] iio: imu: Add support for Kionix KMX61 sensor
2014-12-03 13:31 [PATCH v4 0/6] Add support for KMX61 sensor Daniel Baluta
@ 2014-12-03 13:31 ` Daniel Baluta
2014-12-12 13:32 ` Jonathan Cameron
2014-12-14 22:43 ` Hartmut Knaack
2014-12-03 13:31 ` [PATCH v4 2/6] iio: imu: kmx61: Add acpi support Daniel Baluta
` (4 subsequent siblings)
5 siblings, 2 replies; 20+ messages in thread
From: Daniel Baluta @ 2014-12-03 13:31 UTC (permalink / raw)
To: jic23, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel, daniel.baluta
Minimal implementation for KMX61 6-axis accelerometer/magnetometer. It exports
raw accel/magn readings together with scale and sampling frequency.
This driver uses two IIO devices one for accelerometer and one for magnetometer.
Datasheet will be available at:
http://www.kionix.com/6-axis-accelerometer-magnetometer/kmx61
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/Kconfig | 9 +
drivers/iio/imu/Makefile | 2 +
drivers/iio/imu/kmx61.c | 691 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 702 insertions(+)
create mode 100644 drivers/iio/imu/kmx61.c
diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index 2b0e451..db4221d 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -25,6 +25,15 @@ config ADIS16480
Say yes here to build support for Analog Devices ADIS16375, ADIS16480,
ADIS16485, ADIS16488 inertial sensors.
+config KMX61
+ tristate "Kionix KMX61 6-axis accelerometer and magnetometer"
+ depends on I2C
+ help
+ Say Y here if you want to build a driver for Kionix KMX61 6-axis
+ accelerometer and magnetometer.
+ To compile this driver as module, choose M here: the module will
+ be called kmx61.
+
source "drivers/iio/imu/inv_mpu6050/Kconfig"
endmenu
diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
index 114d2c1..e1e6e3d 100644
--- a/drivers/iio/imu/Makefile
+++ b/drivers/iio/imu/Makefile
@@ -14,3 +14,5 @@ adis_lib-$(CONFIG_IIO_ADIS_LIB_BUFFER) += adis_buffer.o
obj-$(CONFIG_IIO_ADIS_LIB) += adis_lib.o
obj-y += inv_mpu6050/
+
+obj-$(CONFIG_KMX61) += kmx61.o
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
new file mode 100644
index 0000000..5b44371
--- /dev/null
+++ b/drivers/iio/imu/kmx61.c
@@ -0,0 +1,691 @@
+/*
+ * KMX61 - Kionix 6-axis Accelerometer/Magnetometer
+ *
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * IIO driver for KMX61 (7-bit I2C slave address 0x0E or 0x0F).
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define KMX61_DRV_NAME "kmx61"
+
+#define KMX61_REG_WHO_AM_I 0x00
+
+/*
+ * three 16-bit accelerometer output registers for X/Y/Z axis
+ * we use only XOUT_L as a base register, all other addresses
+ * can be obtained by applying an offset and are provided here
+ * only for clarity.
+ */
+#define KMX61_ACC_XOUT_L 0x0A
+#define KMX61_ACC_XOUT_H 0x0B
+#define KMX61_ACC_YOUT_L 0x0C
+#define KMX61_ACC_YOUT_H 0x0D
+#define KMX61_ACC_ZOUT_L 0x0E
+#define KMX61_ACC_ZOUT_H 0x0F
+
+/*
+ * one 16-bit temperature output register
+ */
+#define KMX61_TEMP_L 0x10
+#define KMX61_TEMP_H 0x11
+
+/*
+ * three 16-bit magnetometer output registers for X/Y/Z axis
+ */
+#define KMX61_MAG_XOUT_L 0x12
+#define KMX61_MAG_XOUT_H 0x13
+#define KMX61_MAG_YOUT_L 0x14
+#define KMX61_MAG_YOUT_H 0x15
+#define KMX61_MAG_ZOUT_L 0x16
+#define KMX61_MAG_ZOUT_H 0x17
+
+#define KMX61_REG_STBY 0x29
+#define KMX61_REG_CTRL1 0x2A
+#define KMX61_REG_ODCNTL 0x2C
+
+#define KMX61_ACC_STBY_BIT BIT(0)
+#define KMX61_MAG_STBY_BIT BIT(1)
+#define KMX61_ACT_STBY_BIT BIT(7)
+
+#define KMX61_ALL_STBY (KMX61_ACC_STBY_BIT | KMX61_MAG_STBY_BIT)
+
+#define KMX61_REG_CTRL1_GSEL_MASK 0x03
+
+#define KMX61_ACC_ODR_SHIFT 0
+#define KMX61_MAG_ODR_SHIFT 4
+#define KMX61_ACC_ODR_MASK 0x0F
+#define KMX61_MAG_ODR_MASK 0xF0
+
+#define KMX61_CHIP_ID 0x12
+
+/* KMX61 devices */
+#define KMX61_ACC 0x01
+#define KMX61_MAG 0x02
+
+struct kmx61_data {
+ struct i2c_client *client;
+
+ /* serialize access to non-atomic ops, e.g set_mode */
+ struct mutex lock;
+
+ /* standby state */
+ bool acc_stby;
+ bool mag_stby;
+
+ /* config bits */
+ u8 range;
+ u8 odr_bits;
+
+ /* accelerometer specific data */
+ struct iio_dev *acc_indio_dev;
+
+ /* magnetometer specific data */
+ struct iio_dev *mag_indio_dev;
+};
+
+enum kmx61_range {
+ KMX61_RANGE_2G,
+ KMX61_RANGE_4G,
+ KMX61_RANGE_8G,
+};
+
+enum kmx61_axis {
+ KMX61_AXIS_X,
+ KMX61_AXIS_Y,
+ KMX61_AXIS_Z,
+};
+
+u16 kmx61_uscale_table[] = {9582, 19163, 38326};
+
+static const struct {
+ int val;
+ int val2;
+ u8 odr_bits;
+} kmx61_samp_freq_table[] = { {12, 500000, 0x00},
+ {25, 0, 0x01},
+ {50, 0, 0x02},
+ {100, 0, 0x03},
+ {200, 0, 0x04},
+ {400, 0, 0x05},
+ {800, 0, 0x06},
+ {1600, 0, 0x07},
+ {0, 781000, 0x08},
+ {1, 563000, 0x09},
+ {3, 125000, 0x0A},
+ {6, 250000, 0x0B} };
+
+static IIO_CONST_ATTR(accel_scale_available, "0.009582 0.019163 0.038326");
+static IIO_CONST_ATTR(magn_scale_available, "0.001465");
+static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
+ "0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800");
+
+static struct attribute *kmx61_acc_attributes[] = {
+ &iio_const_attr_accel_scale_available.dev_attr.attr,
+ &iio_const_attr_sampling_frequency_available.dev_attr.attr,
+ NULL,
+};
+
+static struct attribute *kmx61_mag_attributes[] = {
+ &iio_const_attr_magn_scale_available.dev_attr.attr,
+ &iio_const_attr_sampling_frequency_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group kmx61_acc_attribute_group = {
+ .attrs = kmx61_acc_attributes,
+};
+
+static const struct attribute_group kmx61_mag_attribute_group = {
+ .attrs = kmx61_mag_attributes,
+};
+
+#define KMX61_ACC_CHAN(_axis) { \
+ .type = IIO_ACCEL, \
+ .modified = 1, \
+ .channel2 = IIO_MOD_ ## _axis, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .address = KMX61_ACC, \
+ .scan_index = KMX61_AXIS_ ## _axis, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 12, \
+ .storagebits = 16, \
+ .shift = 4, \
+ .endianness = IIO_LE, \
+ }, \
+}
+
+#define KMX61_MAG_CHAN(_axis) { \
+ .type = IIO_MAGN, \
+ .modified = 1, \
+ .channel2 = IIO_MOD_ ## _axis, \
+ .address = KMX61_MAG, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .scan_index = KMX61_AXIS_ ## _axis, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 14, \
+ .storagebits = 16, \
+ .shift = 2, \
+ .endianness = IIO_LE, \
+ }, \
+}
+
+static const struct iio_chan_spec kmx61_acc_channels[] = {
+ KMX61_ACC_CHAN(X),
+ KMX61_ACC_CHAN(Y),
+ KMX61_ACC_CHAN(Z),
+};
+
+static const struct iio_chan_spec kmx61_mag_channels[] = {
+ KMX61_MAG_CHAN(X),
+ KMX61_MAG_CHAN(Y),
+ KMX61_MAG_CHAN(Z),
+};
+
+static void kmx61_set_data(struct iio_dev *indio_dev, struct kmx61_data *data)
+{
+ struct kmx61_data **priv = iio_priv(indio_dev);
+
+ *priv = data;
+}
+
+static struct kmx61_data *kmx61_get_data(struct iio_dev *indio_dev)
+{
+ return *(struct kmx61_data **)iio_priv(indio_dev);
+}
+
+static int kmx61_convert_freq_to_bit(int val, int val2)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
+ if (val == kmx61_samp_freq_table[i].val &&
+ val2 == kmx61_samp_freq_table[i].val2)
+ return kmx61_samp_freq_table[i].odr_bits;
+ return -EINVAL;
+}
+
+/**
+ * kmx61_set_mode() - set KMX61 device operating mode
+ * @data - kmx61 device private data pointer
+ * @mode - bitmask, indicating operating mode for @device
+ * @device - bitmask, indicating device for which @mode needs to be set
+ * @update - update stby bits stored in device's private @data
+ *
+ * For each sensor (accelerometer/magnetometer) there are two operating modes
+ * STANDBY and OPERATION. Neither accel nor magn can be disabled independently
+ * if they are both enabled. Internal sensors state is saved in acc_stby and
+ * mag_stby members of driver's private @data.
+ */
+static int kmx61_set_mode(struct kmx61_data *data, u8 mode, u8 device,
+ bool update)
+{
+ int ret;
+ int acc_stby = -1, mag_stby = -1;
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_STBY);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_stby\n");
+ return ret;
+ }
+ if (device & KMX61_ACC) {
+ if (mode & KMX61_ACC_STBY_BIT) {
+ ret |= KMX61_ACC_STBY_BIT;
+ acc_stby = 1;
+ } else {
+ ret &= ~KMX61_ACC_STBY_BIT;
+ acc_stby = 0;
+ }
+ }
+
+ if (device & KMX61_MAG) {
+ if (mode & KMX61_MAG_STBY_BIT) {
+ ret |= KMX61_MAG_STBY_BIT;
+ mag_stby = 1;
+ } else {
+ ret &= ~KMX61_MAG_STBY_BIT;
+ mag_stby = 0;
+ }
+ }
+
+ if (mode & KMX61_ACT_STBY_BIT)
+ ret |= KMX61_ACT_STBY_BIT;
+
+ ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_STBY, ret);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error writing reg_stby\n");
+ return ret;
+ }
+
+ if (acc_stby != -1 && update)
+ data->acc_stby = acc_stby;
+ if (mag_stby != -1 && update)
+ data->mag_stby = mag_stby;
+
+ return 0;
+}
+
+static int kmx61_get_mode(struct kmx61_data *data, u8 *mode, u8 device)
+{
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_STBY);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_stby\n");
+ return ret;
+ }
+ *mode = 0;
+
+ if (device & KMX61_ACC) {
+ if (ret & KMX61_ACC_STBY_BIT)
+ *mode |= KMX61_ACC_STBY_BIT;
+ else
+ *mode &= ~KMX61_ACC_STBY_BIT;
+ }
+
+ if (device & KMX61_MAG) {
+ if (ret & KMX61_MAG_STBY_BIT)
+ *mode |= KMX61_MAG_STBY_BIT;
+ else
+ *mode &= ~KMX61_MAG_STBY_BIT;
+ }
+
+ return 0;
+}
+
+static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
+{
+ int ret;
+ u8 mode;
+ int lodr_bits, odr_bits;
+
+ ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
+ if (ret < 0)
+ return ret;
+
+ lodr_bits = kmx61_convert_freq_to_bit(val, val2);
+ if (lodr_bits < 0)
+ return lodr_bits;
+
+ /* To change ODR, accel and magn must be in STDBY */
+ ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG,
+ true);
+ if (ret < 0)
+ return ret;
+
+ odr_bits = 0;
+ if (device & KMX61_ACC)
+ odr_bits |= lodr_bits << KMX61_ACC_ODR_SHIFT;
+ if (device & KMX61_MAG)
+ odr_bits |= lodr_bits << KMX61_MAG_ODR_SHIFT;
+
+ ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_ODCNTL,
+ odr_bits);
+ if (ret < 0)
+ return ret;
+
+ return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
+}
+
+static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
+ u8 device)
+{ int i;
+ u8 lodr_bits;
+
+ if (device & KMX61_ACC)
+ lodr_bits = (data->odr_bits >> KMX61_ACC_ODR_SHIFT) &
+ KMX61_ACC_ODR_MASK;
+ else if (device & KMX61_MAG)
+ lodr_bits = (data->odr_bits >> KMX61_MAG_ODR_SHIFT) &
+ KMX61_MAG_ODR_MASK;
+ else
+ return -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
+ if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
+ *val = kmx61_samp_freq_table[i].val;
+ *val2 = kmx61_samp_freq_table[i].val2;
+ return 0;
+ }
+ return -EINVAL;
+}
+
+static int kmx61_set_range(struct kmx61_data *data, u8 range)
+{
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
+ return ret;
+ }
+
+ ret &= ~KMX61_REG_CTRL1_GSEL_MASK;
+ ret |= range & KMX61_REG_CTRL1_GSEL_MASK;
+
+ ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
+ return ret;
+ }
+
+ data->range = range;
+
+ return 0;
+}
+
+static int kmx61_set_scale(struct kmx61_data *data, u16 uscale)
+{
+ int ret, i;
+ u8 mode;
+
+ for (i = 0; i < ARRAY_SIZE(kmx61_uscale_table); i++) {
+ if (kmx61_uscale_table[i] == uscale) {
+ ret = kmx61_get_mode(data, &mode,
+ KMX61_ACC | KMX61_MAG);
+ if (ret < 0)
+ return ret;
+
+ ret = kmx61_set_mode(data, KMX61_ALL_STBY,
+ KMX61_ACC | KMX61_MAG, true);
+ if (ret < 0)
+ return ret;
+
+ ret = kmx61_set_range(data, i);
+ if (ret < 0)
+ return ret;
+
+ return kmx61_set_mode(data, mode,
+ KMX61_ACC | KMX61_MAG, true);
+ }
+ }
+ return -EINVAL;
+}
+
+static int kmx61_chip_init(struct kmx61_data *data)
+{
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_WHO_AM_I);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading who_am_i\n");
+ return ret;
+ }
+
+ if (ret != KMX61_CHIP_ID) {
+ dev_err(&data->client->dev,
+ "Wrong chip id, got %x expected %x\n",
+ ret, KMX61_CHIP_ID);
+ return -EINVAL;
+ }
+
+ /* set accel 12bit, 4g range */
+ ret = kmx61_set_range(data, KMX61_RANGE_4G);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_ODCNTL);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_odcntl\n");
+ return ret;
+ }
+ data->odr_bits = ret;
+
+ /* set acc/magn to OPERATION mode */
+ ret = kmx61_set_mode(data, 0, KMX61_ACC | KMX61_MAG, true);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int kmx61_read_measurement(struct kmx61_data *data, u8 base, u8 offset)
+{
+ int ret;
+ u8 reg = base + offset * 2;
+
+ ret = i2c_smbus_read_word_data(data->client, reg);
+ if (ret < 0)
+ dev_err(&data->client->dev, "failed to read reg at %x\n", reg);
+
+ return ret;
+}
+
+static int kmx61_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ int ret;
+ u8 base_reg;
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ switch (chan->type) {
+ case IIO_ACCEL:
+ base_reg = KMX61_ACC_XOUT_L;
+ break;
+ case IIO_MAGN:
+ base_reg = KMX61_MAG_XOUT_L;
+ break;
+ default:
+ return -EINVAL;
+ }
+ mutex_lock(&data->lock);
+
+ ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
+ if (ret < 0) {
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+ *val = sign_extend32(ret >> chan->scan_type.shift,
+ chan->scan_type.realbits - 1);
+
+ mutex_unlock(&data->lock);
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_ACCEL:
+ *val = 0;
+ *val2 = kmx61_uscale_table[data->range];
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_MAGN:
+ /* 14 bits res, 1465 microGauss per magn count */
+ *val = 0;
+ *val2 = 1465;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+ ret = kmx61_get_odr(data, val, val2, chan->address);
+ mutex_unlock(&data->lock);
+ if (ret)
+ return -EINVAL;
+ return IIO_VAL_INT_PLUS_MICRO;
+ }
+ return -EINVAL;
+}
+
+static int kmx61_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val,
+ int val2, long mask)
+{
+ int ret;
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+ ret = kmx61_set_odr(data, val, val2, chan->address);
+ mutex_unlock(&data->lock);
+ return ret;
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_ACCEL:
+ if (val != 0)
+ return -EINVAL;
+ mutex_lock(&data->lock);
+ ret = kmx61_set_scale(data, val2);
+ mutex_unlock(&data->lock);
+ return ret;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info kmx61_acc_info = {
+ .driver_module = THIS_MODULE,
+ .read_raw = kmx61_read_raw,
+ .write_raw = kmx61_write_raw,
+ .attrs = &kmx61_acc_attribute_group,
+};
+
+static const struct iio_info kmx61_mag_info = {
+ .driver_module = THIS_MODULE,
+ .read_raw = kmx61_read_raw,
+ .write_raw = kmx61_write_raw,
+ .attrs = &kmx61_mag_attribute_group,
+};
+
+static struct iio_dev *kmx61_indiodev_setup(struct kmx61_data *data,
+ const struct iio_info *info,
+ const struct iio_chan_spec *chan,
+ int num_channels,
+ const char *name)
+{
+ struct iio_dev *indio_dev;
+
+ indio_dev = devm_iio_device_alloc(&data->client->dev, sizeof(data));
+ if (!indio_dev)
+ return ERR_PTR(-ENOMEM);
+
+ kmx61_set_data(indio_dev, data);
+
+ indio_dev->dev.parent = &data->client->dev;
+ indio_dev->channels = chan;
+ indio_dev->num_channels = num_channels;
+ indio_dev->name = name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = info;
+
+ return indio_dev;
+}
+
+static int kmx61_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int ret;
+ struct kmx61_data *data;
+ const char *name = NULL;
+
+ data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, data);
+ data->client = client;
+
+ mutex_init(&data->lock);
+
+ data->acc_indio_dev =
+ kmx61_indiodev_setup(data, &kmx61_acc_info,
+ kmx61_acc_channels,
+ ARRAY_SIZE(kmx61_acc_channels),
+ name);
+ if (IS_ERR(data->acc_indio_dev))
+ return PTR_ERR(data->acc_indio_dev);
+
+ data->mag_indio_dev =
+ kmx61_indiodev_setup(data, &kmx61_mag_info,
+ kmx61_mag_channels,
+ ARRAY_SIZE(kmx61_mag_channels),
+ name);
+ if (IS_ERR(data->mag_indio_dev))
+ return PTR_ERR(data->mag_indio_dev);
+
+ ret = kmx61_chip_init(data);
+ if (ret < 0)
+ return ret;
+
+ ret = iio_device_register(data->acc_indio_dev);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to register acc iio device\n");
+ goto err_chip_uninit;
+ }
+
+ ret = iio_device_register(data->mag_indio_dev);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to register mag iio device\n");
+ goto err_iio_unregister;
+ }
+
+ return 0;
+
+err_iio_unregister:
+ iio_device_unregister(data->acc_indio_dev);
+err_chip_uninit:
+ kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
+ return ret;
+}
+
+static int kmx61_remove(struct i2c_client *client)
+{
+ struct kmx61_data *data = i2c_get_clientdata(client);
+
+ iio_device_unregister(data->acc_indio_dev);
+ iio_device_unregister(data->mag_indio_dev);
+
+ mutex_lock(&data->lock);
+ kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
+ mutex_unlock(&data->lock);
+
+ return 0;
+}
+
+static const struct i2c_device_id kmx61_id[] = {
+ {"kmx611021", 0},
+ {}
+};
+
+MODULE_DEVICE_TABLE(i2c, kmx61_id);
+
+static struct i2c_driver kmx61_driver = {
+ .driver = {
+ .name = KMX61_DRV_NAME,
+ },
+ .probe = kmx61_probe,
+ .remove = kmx61_remove,
+ .id_table = kmx61_id,
+};
+
+module_i2c_driver(kmx61_driver);
+
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
+MODULE_DESCRIPTION("KMX61 accelerometer/magnetometer driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 1/6] iio: imu: Add support for Kionix KMX61 sensor
2014-12-03 13:31 ` [PATCH v4 1/6] iio: imu: Add support for Kionix " Daniel Baluta
@ 2014-12-12 13:32 ` Jonathan Cameron
2014-12-14 22:43 ` Hartmut Knaack
1 sibling, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2014-12-12 13:32 UTC (permalink / raw)
To: Daniel Baluta, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel
On 03/12/14 13:31, Daniel Baluta wrote:
> Minimal implementation for KMX61 6-axis accelerometer/magnetometer. It exports
> raw accel/magn readings together with scale and sampling frequency.
>
> This driver uses two IIO devices one for accelerometer and one for magnetometer.
>
> Datasheet will be available at:
> http://www.kionix.com/6-axis-accelerometer-magnetometer/kmx61
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Very nice.
Given how long ago I took the v3 patch, I've reverted it rather than dropping it
so as not to make too much of a mess of the tree history.
Applied this version on top of the togreg branch of iio.git
Thanks,
Jonathan
> ---
> drivers/iio/imu/Kconfig | 9 +
> drivers/iio/imu/Makefile | 2 +
> drivers/iio/imu/kmx61.c | 691 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 702 insertions(+)
> create mode 100644 drivers/iio/imu/kmx61.c
>
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index 2b0e451..db4221d 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -25,6 +25,15 @@ config ADIS16480
> Say yes here to build support for Analog Devices ADIS16375, ADIS16480,
> ADIS16485, ADIS16488 inertial sensors.
>
> +config KMX61
> + tristate "Kionix KMX61 6-axis accelerometer and magnetometer"
> + depends on I2C
> + help
> + Say Y here if you want to build a driver for Kionix KMX61 6-axis
> + accelerometer and magnetometer.
> + To compile this driver as module, choose M here: the module will
> + be called kmx61.
> +
> source "drivers/iio/imu/inv_mpu6050/Kconfig"
>
> endmenu
> diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
> index 114d2c1..e1e6e3d 100644
> --- a/drivers/iio/imu/Makefile
> +++ b/drivers/iio/imu/Makefile
> @@ -14,3 +14,5 @@ adis_lib-$(CONFIG_IIO_ADIS_LIB_BUFFER) += adis_buffer.o
> obj-$(CONFIG_IIO_ADIS_LIB) += adis_lib.o
>
> obj-y += inv_mpu6050/
> +
> +obj-$(CONFIG_KMX61) += kmx61.o
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> new file mode 100644
> index 0000000..5b44371
> --- /dev/null
> +++ b/drivers/iio/imu/kmx61.c
> @@ -0,0 +1,691 @@
> +/*
> + * KMX61 - Kionix 6-axis Accelerometer/Magnetometer
> + *
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * IIO driver for KMX61 (7-bit I2C slave address 0x0E or 0x0F).
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define KMX61_DRV_NAME "kmx61"
> +
> +#define KMX61_REG_WHO_AM_I 0x00
> +
> +/*
> + * three 16-bit accelerometer output registers for X/Y/Z axis
> + * we use only XOUT_L as a base register, all other addresses
> + * can be obtained by applying an offset and are provided here
> + * only for clarity.
> + */
> +#define KMX61_ACC_XOUT_L 0x0A
> +#define KMX61_ACC_XOUT_H 0x0B
> +#define KMX61_ACC_YOUT_L 0x0C
> +#define KMX61_ACC_YOUT_H 0x0D
> +#define KMX61_ACC_ZOUT_L 0x0E
> +#define KMX61_ACC_ZOUT_H 0x0F
> +
> +/*
> + * one 16-bit temperature output register
> + */
> +#define KMX61_TEMP_L 0x10
> +#define KMX61_TEMP_H 0x11
> +
> +/*
> + * three 16-bit magnetometer output registers for X/Y/Z axis
> + */
> +#define KMX61_MAG_XOUT_L 0x12
> +#define KMX61_MAG_XOUT_H 0x13
> +#define KMX61_MAG_YOUT_L 0x14
> +#define KMX61_MAG_YOUT_H 0x15
> +#define KMX61_MAG_ZOUT_L 0x16
> +#define KMX61_MAG_ZOUT_H 0x17
> +
> +#define KMX61_REG_STBY 0x29
> +#define KMX61_REG_CTRL1 0x2A
> +#define KMX61_REG_ODCNTL 0x2C
> +
> +#define KMX61_ACC_STBY_BIT BIT(0)
> +#define KMX61_MAG_STBY_BIT BIT(1)
> +#define KMX61_ACT_STBY_BIT BIT(7)
> +
> +#define KMX61_ALL_STBY (KMX61_ACC_STBY_BIT | KMX61_MAG_STBY_BIT)
> +
> +#define KMX61_REG_CTRL1_GSEL_MASK 0x03
> +
> +#define KMX61_ACC_ODR_SHIFT 0
> +#define KMX61_MAG_ODR_SHIFT 4
> +#define KMX61_ACC_ODR_MASK 0x0F
> +#define KMX61_MAG_ODR_MASK 0xF0
> +
> +#define KMX61_CHIP_ID 0x12
> +
> +/* KMX61 devices */
> +#define KMX61_ACC 0x01
> +#define KMX61_MAG 0x02
> +
> +struct kmx61_data {
> + struct i2c_client *client;
> +
> + /* serialize access to non-atomic ops, e.g set_mode */
> + struct mutex lock;
> +
> + /* standby state */
> + bool acc_stby;
> + bool mag_stby;
> +
> + /* config bits */
> + u8 range;
> + u8 odr_bits;
> +
> + /* accelerometer specific data */
> + struct iio_dev *acc_indio_dev;
> +
> + /* magnetometer specific data */
> + struct iio_dev *mag_indio_dev;
> +};
> +
> +enum kmx61_range {
> + KMX61_RANGE_2G,
> + KMX61_RANGE_4G,
> + KMX61_RANGE_8G,
> +};
> +
> +enum kmx61_axis {
> + KMX61_AXIS_X,
> + KMX61_AXIS_Y,
> + KMX61_AXIS_Z,
> +};
> +
> +u16 kmx61_uscale_table[] = {9582, 19163, 38326};
> +
> +static const struct {
> + int val;
> + int val2;
> + u8 odr_bits;
> +} kmx61_samp_freq_table[] = { {12, 500000, 0x00},
> + {25, 0, 0x01},
> + {50, 0, 0x02},
> + {100, 0, 0x03},
> + {200, 0, 0x04},
> + {400, 0, 0x05},
> + {800, 0, 0x06},
> + {1600, 0, 0x07},
> + {0, 781000, 0x08},
> + {1, 563000, 0x09},
> + {3, 125000, 0x0A},
> + {6, 250000, 0x0B} };
> +
> +static IIO_CONST_ATTR(accel_scale_available, "0.009582 0.019163 0.038326");
> +static IIO_CONST_ATTR(magn_scale_available, "0.001465");
> +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> + "0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800");
> +
> +static struct attribute *kmx61_acc_attributes[] = {
> + &iio_const_attr_accel_scale_available.dev_attr.attr,
> + &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static struct attribute *kmx61_mag_attributes[] = {
> + &iio_const_attr_magn_scale_available.dev_attr.attr,
> + &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group kmx61_acc_attribute_group = {
> + .attrs = kmx61_acc_attributes,
> +};
> +
> +static const struct attribute_group kmx61_mag_attribute_group = {
> + .attrs = kmx61_mag_attributes,
> +};
> +
> +#define KMX61_ACC_CHAN(_axis) { \
> + .type = IIO_ACCEL, \
> + .modified = 1, \
> + .channel2 = IIO_MOD_ ## _axis, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .address = KMX61_ACC, \
> + .scan_index = KMX61_AXIS_ ## _axis, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + .shift = 4, \
> + .endianness = IIO_LE, \
> + }, \
> +}
> +
> +#define KMX61_MAG_CHAN(_axis) { \
> + .type = IIO_MAGN, \
> + .modified = 1, \
> + .channel2 = IIO_MOD_ ## _axis, \
> + .address = KMX61_MAG, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .scan_index = KMX61_AXIS_ ## _axis, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 14, \
> + .storagebits = 16, \
> + .shift = 2, \
> + .endianness = IIO_LE, \
> + }, \
> +}
> +
> +static const struct iio_chan_spec kmx61_acc_channels[] = {
> + KMX61_ACC_CHAN(X),
> + KMX61_ACC_CHAN(Y),
> + KMX61_ACC_CHAN(Z),
> +};
> +
> +static const struct iio_chan_spec kmx61_mag_channels[] = {
> + KMX61_MAG_CHAN(X),
> + KMX61_MAG_CHAN(Y),
> + KMX61_MAG_CHAN(Z),
> +};
> +
> +static void kmx61_set_data(struct iio_dev *indio_dev, struct kmx61_data *data)
> +{
> + struct kmx61_data **priv = iio_priv(indio_dev);
> +
> + *priv = data;
> +}
> +
> +static struct kmx61_data *kmx61_get_data(struct iio_dev *indio_dev)
> +{
> + return *(struct kmx61_data **)iio_priv(indio_dev);
> +}
> +
> +static int kmx61_convert_freq_to_bit(int val, int val2)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> + if (val == kmx61_samp_freq_table[i].val &&
> + val2 == kmx61_samp_freq_table[i].val2)
> + return kmx61_samp_freq_table[i].odr_bits;
> + return -EINVAL;
> +}
> +
> +/**
> + * kmx61_set_mode() - set KMX61 device operating mode
> + * @data - kmx61 device private data pointer
> + * @mode - bitmask, indicating operating mode for @device
> + * @device - bitmask, indicating device for which @mode needs to be set
> + * @update - update stby bits stored in device's private @data
> + *
> + * For each sensor (accelerometer/magnetometer) there are two operating modes
> + * STANDBY and OPERATION. Neither accel nor magn can be disabled independently
> + * if they are both enabled. Internal sensors state is saved in acc_stby and
> + * mag_stby members of driver's private @data.
> + */
> +static int kmx61_set_mode(struct kmx61_data *data, u8 mode, u8 device,
> + bool update)
> +{
> + int ret;
> + int acc_stby = -1, mag_stby = -1;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_STBY);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_stby\n");
> + return ret;
> + }
> + if (device & KMX61_ACC) {
> + if (mode & KMX61_ACC_STBY_BIT) {
> + ret |= KMX61_ACC_STBY_BIT;
> + acc_stby = 1;
> + } else {
> + ret &= ~KMX61_ACC_STBY_BIT;
> + acc_stby = 0;
> + }
> + }
> +
> + if (device & KMX61_MAG) {
> + if (mode & KMX61_MAG_STBY_BIT) {
> + ret |= KMX61_MAG_STBY_BIT;
> + mag_stby = 1;
> + } else {
> + ret &= ~KMX61_MAG_STBY_BIT;
> + mag_stby = 0;
> + }
> + }
> +
> + if (mode & KMX61_ACT_STBY_BIT)
> + ret |= KMX61_ACT_STBY_BIT;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_STBY, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_stby\n");
> + return ret;
> + }
> +
> + if (acc_stby != -1 && update)
> + data->acc_stby = acc_stby;
> + if (mag_stby != -1 && update)
> + data->mag_stby = mag_stby;
> +
> + return 0;
> +}
> +
> +static int kmx61_get_mode(struct kmx61_data *data, u8 *mode, u8 device)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_STBY);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_stby\n");
> + return ret;
> + }
> + *mode = 0;
> +
> + if (device & KMX61_ACC) {
> + if (ret & KMX61_ACC_STBY_BIT)
> + *mode |= KMX61_ACC_STBY_BIT;
> + else
> + *mode &= ~KMX61_ACC_STBY_BIT;
> + }
> +
> + if (device & KMX61_MAG) {
> + if (ret & KMX61_MAG_STBY_BIT)
> + *mode |= KMX61_MAG_STBY_BIT;
> + else
> + *mode &= ~KMX61_MAG_STBY_BIT;
> + }
> +
> + return 0;
> +}
> +
> +static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
> +{
> + int ret;
> + u8 mode;
> + int lodr_bits, odr_bits;
> +
> + ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
> + if (ret < 0)
> + return ret;
> +
> + lodr_bits = kmx61_convert_freq_to_bit(val, val2);
> + if (lodr_bits < 0)
> + return lodr_bits;
> +
> + /* To change ODR, accel and magn must be in STDBY */
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG,
> + true);
> + if (ret < 0)
> + return ret;
> +
> + odr_bits = 0;
> + if (device & KMX61_ACC)
> + odr_bits |= lodr_bits << KMX61_ACC_ODR_SHIFT;
> + if (device & KMX61_MAG)
> + odr_bits |= lodr_bits << KMX61_MAG_ODR_SHIFT;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_ODCNTL,
> + odr_bits);
> + if (ret < 0)
> + return ret;
> +
> + return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> +}
> +
> +static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
> + u8 device)
> +{ int i;
> + u8 lodr_bits;
> +
> + if (device & KMX61_ACC)
> + lodr_bits = (data->odr_bits >> KMX61_ACC_ODR_SHIFT) &
> + KMX61_ACC_ODR_MASK;
> + else if (device & KMX61_MAG)
> + lodr_bits = (data->odr_bits >> KMX61_MAG_ODR_SHIFT) &
> + KMX61_MAG_ODR_MASK;
> + else
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> + if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
> + *val = kmx61_samp_freq_table[i].val;
> + *val2 = kmx61_samp_freq_table[i].val2;
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +static int kmx61_set_range(struct kmx61_data *data, u8 range)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> + return ret;
> + }
> +
> + ret &= ~KMX61_REG_CTRL1_GSEL_MASK;
> + ret |= range & KMX61_REG_CTRL1_GSEL_MASK;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> + return ret;
> + }
> +
> + data->range = range;
> +
> + return 0;
> +}
> +
> +static int kmx61_set_scale(struct kmx61_data *data, u16 uscale)
> +{
> + int ret, i;
> + u8 mode;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_uscale_table); i++) {
> + if (kmx61_uscale_table[i] == uscale) {
> + ret = kmx61_get_mode(data, &mode,
> + KMX61_ACC | KMX61_MAG);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY,
> + KMX61_ACC | KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_range(data, i);
> + if (ret < 0)
> + return ret;
> +
> + return kmx61_set_mode(data, mode,
> + KMX61_ACC | KMX61_MAG, true);
> + }
> + }
> + return -EINVAL;
> +}
> +
> +static int kmx61_chip_init(struct kmx61_data *data)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_WHO_AM_I);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading who_am_i\n");
> + return ret;
> + }
> +
> + if (ret != KMX61_CHIP_ID) {
> + dev_err(&data->client->dev,
> + "Wrong chip id, got %x expected %x\n",
> + ret, KMX61_CHIP_ID);
> + return -EINVAL;
> + }
> +
> + /* set accel 12bit, 4g range */
> + ret = kmx61_set_range(data, KMX61_RANGE_4G);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_ODCNTL);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_odcntl\n");
> + return ret;
> + }
> + data->odr_bits = ret;
> +
> + /* set acc/magn to OPERATION mode */
> + ret = kmx61_set_mode(data, 0, KMX61_ACC | KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int kmx61_read_measurement(struct kmx61_data *data, u8 base, u8 offset)
> +{
> + int ret;
> + u8 reg = base + offset * 2;
> +
> + ret = i2c_smbus_read_word_data(data->client, reg);
> + if (ret < 0)
> + dev_err(&data->client->dev, "failed to read reg at %x\n", reg);
> +
> + return ret;
> +}
> +
> +static int kmx61_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + int ret;
> + u8 base_reg;
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + base_reg = KMX61_ACC_XOUT_L;
> + break;
> + case IIO_MAGN:
> + base_reg = KMX61_MAG_XOUT_L;
> + break;
> + default:
> + return -EINVAL;
> + }
> + mutex_lock(&data->lock);
> +
> + ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> + *val = sign_extend32(ret >> chan->scan_type.shift,
> + chan->scan_type.realbits - 1);
> +
> + mutex_unlock(&data->lock);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + *val = 0;
> + *val2 = kmx61_uscale_table[data->range];
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_MAGN:
> + /* 14 bits res, 1465 microGauss per magn count */
> + *val = 0;
> + *val2 = 1465;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
> + return -EINVAL;
> +
> + mutex_lock(&data->lock);
> + ret = kmx61_get_odr(data, val, val2, chan->address);
> + mutex_unlock(&data->lock);
> + if (ret)
> + return -EINVAL;
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> + return -EINVAL;
> +}
> +
> +static int kmx61_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
> +{
> + int ret;
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
> + return -EINVAL;
> +
> + mutex_lock(&data->lock);
> + ret = kmx61_set_odr(data, val, val2, chan->address);
> + mutex_unlock(&data->lock);
> + return ret;
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + if (val != 0)
> + return -EINVAL;
> + mutex_lock(&data->lock);
> + ret = kmx61_set_scale(data, val2);
> + mutex_unlock(&data->lock);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info kmx61_acc_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = kmx61_read_raw,
> + .write_raw = kmx61_write_raw,
> + .attrs = &kmx61_acc_attribute_group,
> +};
> +
> +static const struct iio_info kmx61_mag_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = kmx61_read_raw,
> + .write_raw = kmx61_write_raw,
> + .attrs = &kmx61_mag_attribute_group,
> +};
> +
> +static struct iio_dev *kmx61_indiodev_setup(struct kmx61_data *data,
> + const struct iio_info *info,
> + const struct iio_chan_spec *chan,
> + int num_channels,
> + const char *name)
> +{
> + struct iio_dev *indio_dev;
> +
> + indio_dev = devm_iio_device_alloc(&data->client->dev, sizeof(data));
> + if (!indio_dev)
> + return ERR_PTR(-ENOMEM);
> +
> + kmx61_set_data(indio_dev, data);
> +
> + indio_dev->dev.parent = &data->client->dev;
> + indio_dev->channels = chan;
> + indio_dev->num_channels = num_channels;
> + indio_dev->name = name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = info;
> +
> + return indio_dev;
> +}
> +
> +static int kmx61_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int ret;
> + struct kmx61_data *data;
> + const char *name = NULL;
> +
> + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, data);
> + data->client = client;
> +
> + mutex_init(&data->lock);
> +
> + data->acc_indio_dev =
> + kmx61_indiodev_setup(data, &kmx61_acc_info,
> + kmx61_acc_channels,
> + ARRAY_SIZE(kmx61_acc_channels),
> + name);
> + if (IS_ERR(data->acc_indio_dev))
> + return PTR_ERR(data->acc_indio_dev);
> +
> + data->mag_indio_dev =
> + kmx61_indiodev_setup(data, &kmx61_mag_info,
> + kmx61_mag_channels,
> + ARRAY_SIZE(kmx61_mag_channels),
> + name);
> + if (IS_ERR(data->mag_indio_dev))
> + return PTR_ERR(data->mag_indio_dev);
> +
> + ret = kmx61_chip_init(data);
> + if (ret < 0)
> + return ret;
> +
> + ret = iio_device_register(data->acc_indio_dev);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to register acc iio device\n");
> + goto err_chip_uninit;
> + }
> +
> + ret = iio_device_register(data->mag_indio_dev);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to register mag iio device\n");
> + goto err_iio_unregister;
> + }
> +
> + return 0;
> +
> +err_iio_unregister:
> + iio_device_unregister(data->acc_indio_dev);
> +err_chip_uninit:
> + kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + return ret;
> +}
> +
> +static int kmx61_remove(struct i2c_client *client)
> +{
> + struct kmx61_data *data = i2c_get_clientdata(client);
> +
> + iio_device_unregister(data->acc_indio_dev);
> + iio_device_unregister(data->mag_indio_dev);
> +
> + mutex_lock(&data->lock);
> + kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + mutex_unlock(&data->lock);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id kmx61_id[] = {
> + {"kmx611021", 0},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, kmx61_id);
> +
> +static struct i2c_driver kmx61_driver = {
> + .driver = {
> + .name = KMX61_DRV_NAME,
> + },
> + .probe = kmx61_probe,
> + .remove = kmx61_remove,
> + .id_table = kmx61_id,
> +};
> +
> +module_i2c_driver(kmx61_driver);
> +
> +MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
> +MODULE_DESCRIPTION("KMX61 accelerometer/magnetometer driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 1/6] iio: imu: Add support for Kionix KMX61 sensor
2014-12-03 13:31 ` [PATCH v4 1/6] iio: imu: Add support for Kionix " Daniel Baluta
2014-12-12 13:32 ` Jonathan Cameron
@ 2014-12-14 22:43 ` Hartmut Knaack
2014-12-15 10:50 ` Daniel Baluta
1 sibling, 1 reply; 20+ messages in thread
From: Hartmut Knaack @ 2014-12-14 22:43 UTC (permalink / raw)
To: Daniel Baluta, jic23, pmeerw, srinivas.pandruvada; +Cc: linux-iio, linux-kernel
Daniel Baluta schrieb am 03.12.2014 um 14:31:
> Minimal implementation for KMX61 6-axis accelerometer/magnetometer. It exports
> raw accel/magn readings together with scale and sampling frequency.
>
> This driver uses two IIO devices one for accelerometer and one for magnetometer.
>
Hi Daniel,
I think you dropped slightly too much code. In kmx61_set_odr() you stored the set
odr bits in data->odr_bits previously, but now it's gone. But kmx61_get_odr()
accesses data->odr_bits, which now remains in the state from driver initialization.
Other than that, it's looking good.
Thanks,
Hartmut
> Datasheet will be available at:
> http://www.kionix.com/6-axis-accelerometer-magnetometer/kmx61
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
> drivers/iio/imu/Kconfig | 9 +
> drivers/iio/imu/Makefile | 2 +
> drivers/iio/imu/kmx61.c | 691 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 702 insertions(+)
> create mode 100644 drivers/iio/imu/kmx61.c
>
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index 2b0e451..db4221d 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -25,6 +25,15 @@ config ADIS16480
> Say yes here to build support for Analog Devices ADIS16375, ADIS16480,
> ADIS16485, ADIS16488 inertial sensors.
>
> +config KMX61
> + tristate "Kionix KMX61 6-axis accelerometer and magnetometer"
> + depends on I2C
> + help
> + Say Y here if you want to build a driver for Kionix KMX61 6-axis
> + accelerometer and magnetometer.
> + To compile this driver as module, choose M here: the module will
> + be called kmx61.
> +
> source "drivers/iio/imu/inv_mpu6050/Kconfig"
>
> endmenu
> diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
> index 114d2c1..e1e6e3d 100644
> --- a/drivers/iio/imu/Makefile
> +++ b/drivers/iio/imu/Makefile
> @@ -14,3 +14,5 @@ adis_lib-$(CONFIG_IIO_ADIS_LIB_BUFFER) += adis_buffer.o
> obj-$(CONFIG_IIO_ADIS_LIB) += adis_lib.o
>
> obj-y += inv_mpu6050/
> +
> +obj-$(CONFIG_KMX61) += kmx61.o
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> new file mode 100644
> index 0000000..5b44371
> --- /dev/null
> +++ b/drivers/iio/imu/kmx61.c
> @@ -0,0 +1,691 @@
> +/*
> + * KMX61 - Kionix 6-axis Accelerometer/Magnetometer
> + *
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * IIO driver for KMX61 (7-bit I2C slave address 0x0E or 0x0F).
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define KMX61_DRV_NAME "kmx61"
> +
> +#define KMX61_REG_WHO_AM_I 0x00
> +
> +/*
> + * three 16-bit accelerometer output registers for X/Y/Z axis
> + * we use only XOUT_L as a base register, all other addresses
> + * can be obtained by applying an offset and are provided here
> + * only for clarity.
> + */
> +#define KMX61_ACC_XOUT_L 0x0A
> +#define KMX61_ACC_XOUT_H 0x0B
> +#define KMX61_ACC_YOUT_L 0x0C
> +#define KMX61_ACC_YOUT_H 0x0D
> +#define KMX61_ACC_ZOUT_L 0x0E
> +#define KMX61_ACC_ZOUT_H 0x0F
> +
> +/*
> + * one 16-bit temperature output register
> + */
> +#define KMX61_TEMP_L 0x10
> +#define KMX61_TEMP_H 0x11
> +
> +/*
> + * three 16-bit magnetometer output registers for X/Y/Z axis
> + */
> +#define KMX61_MAG_XOUT_L 0x12
> +#define KMX61_MAG_XOUT_H 0x13
> +#define KMX61_MAG_YOUT_L 0x14
> +#define KMX61_MAG_YOUT_H 0x15
> +#define KMX61_MAG_ZOUT_L 0x16
> +#define KMX61_MAG_ZOUT_H 0x17
> +
> +#define KMX61_REG_STBY 0x29
> +#define KMX61_REG_CTRL1 0x2A
> +#define KMX61_REG_ODCNTL 0x2C
> +
> +#define KMX61_ACC_STBY_BIT BIT(0)
> +#define KMX61_MAG_STBY_BIT BIT(1)
> +#define KMX61_ACT_STBY_BIT BIT(7)
> +
> +#define KMX61_ALL_STBY (KMX61_ACC_STBY_BIT | KMX61_MAG_STBY_BIT)
> +
> +#define KMX61_REG_CTRL1_GSEL_MASK 0x03
> +
> +#define KMX61_ACC_ODR_SHIFT 0
> +#define KMX61_MAG_ODR_SHIFT 4
> +#define KMX61_ACC_ODR_MASK 0x0F
> +#define KMX61_MAG_ODR_MASK 0xF0
> +
> +#define KMX61_CHIP_ID 0x12
> +
> +/* KMX61 devices */
> +#define KMX61_ACC 0x01
> +#define KMX61_MAG 0x02
> +
> +struct kmx61_data {
> + struct i2c_client *client;
> +
> + /* serialize access to non-atomic ops, e.g set_mode */
> + struct mutex lock;
> +
> + /* standby state */
> + bool acc_stby;
> + bool mag_stby;
> +
> + /* config bits */
> + u8 range;
> + u8 odr_bits;
> +
> + /* accelerometer specific data */
> + struct iio_dev *acc_indio_dev;
> +
> + /* magnetometer specific data */
> + struct iio_dev *mag_indio_dev;
> +};
> +
> +enum kmx61_range {
> + KMX61_RANGE_2G,
> + KMX61_RANGE_4G,
> + KMX61_RANGE_8G,
> +};
> +
> +enum kmx61_axis {
> + KMX61_AXIS_X,
> + KMX61_AXIS_Y,
> + KMX61_AXIS_Z,
> +};
> +
> +u16 kmx61_uscale_table[] = {9582, 19163, 38326};
> +
> +static const struct {
> + int val;
> + int val2;
> + u8 odr_bits;
> +} kmx61_samp_freq_table[] = { {12, 500000, 0x00},
> + {25, 0, 0x01},
> + {50, 0, 0x02},
> + {100, 0, 0x03},
> + {200, 0, 0x04},
> + {400, 0, 0x05},
> + {800, 0, 0x06},
> + {1600, 0, 0x07},
> + {0, 781000, 0x08},
> + {1, 563000, 0x09},
> + {3, 125000, 0x0A},
> + {6, 250000, 0x0B} };
> +
> +static IIO_CONST_ATTR(accel_scale_available, "0.009582 0.019163 0.038326");
> +static IIO_CONST_ATTR(magn_scale_available, "0.001465");
> +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> + "0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800");
> +
> +static struct attribute *kmx61_acc_attributes[] = {
> + &iio_const_attr_accel_scale_available.dev_attr.attr,
> + &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static struct attribute *kmx61_mag_attributes[] = {
> + &iio_const_attr_magn_scale_available.dev_attr.attr,
> + &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group kmx61_acc_attribute_group = {
> + .attrs = kmx61_acc_attributes,
> +};
> +
> +static const struct attribute_group kmx61_mag_attribute_group = {
> + .attrs = kmx61_mag_attributes,
> +};
> +
> +#define KMX61_ACC_CHAN(_axis) { \
> + .type = IIO_ACCEL, \
> + .modified = 1, \
> + .channel2 = IIO_MOD_ ## _axis, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .address = KMX61_ACC, \
> + .scan_index = KMX61_AXIS_ ## _axis, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 12, \
> + .storagebits = 16, \
> + .shift = 4, \
> + .endianness = IIO_LE, \
> + }, \
> +}
> +
> +#define KMX61_MAG_CHAN(_axis) { \
> + .type = IIO_MAGN, \
> + .modified = 1, \
> + .channel2 = IIO_MOD_ ## _axis, \
> + .address = KMX61_MAG, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> + .scan_index = KMX61_AXIS_ ## _axis, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 14, \
> + .storagebits = 16, \
> + .shift = 2, \
> + .endianness = IIO_LE, \
> + }, \
> +}
> +
> +static const struct iio_chan_spec kmx61_acc_channels[] = {
> + KMX61_ACC_CHAN(X),
> + KMX61_ACC_CHAN(Y),
> + KMX61_ACC_CHAN(Z),
> +};
> +
> +static const struct iio_chan_spec kmx61_mag_channels[] = {
> + KMX61_MAG_CHAN(X),
> + KMX61_MAG_CHAN(Y),
> + KMX61_MAG_CHAN(Z),
> +};
> +
> +static void kmx61_set_data(struct iio_dev *indio_dev, struct kmx61_data *data)
> +{
> + struct kmx61_data **priv = iio_priv(indio_dev);
> +
> + *priv = data;
> +}
> +
> +static struct kmx61_data *kmx61_get_data(struct iio_dev *indio_dev)
> +{
> + return *(struct kmx61_data **)iio_priv(indio_dev);
> +}
> +
> +static int kmx61_convert_freq_to_bit(int val, int val2)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> + if (val == kmx61_samp_freq_table[i].val &&
> + val2 == kmx61_samp_freq_table[i].val2)
> + return kmx61_samp_freq_table[i].odr_bits;
> + return -EINVAL;
> +}
> +
> +/**
> + * kmx61_set_mode() - set KMX61 device operating mode
> + * @data - kmx61 device private data pointer
> + * @mode - bitmask, indicating operating mode for @device
> + * @device - bitmask, indicating device for which @mode needs to be set
> + * @update - update stby bits stored in device's private @data
> + *
> + * For each sensor (accelerometer/magnetometer) there are two operating modes
> + * STANDBY and OPERATION. Neither accel nor magn can be disabled independently
> + * if they are both enabled. Internal sensors state is saved in acc_stby and
> + * mag_stby members of driver's private @data.
> + */
> +static int kmx61_set_mode(struct kmx61_data *data, u8 mode, u8 device,
> + bool update)
> +{
> + int ret;
> + int acc_stby = -1, mag_stby = -1;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_STBY);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_stby\n");
> + return ret;
> + }
> + if (device & KMX61_ACC) {
> + if (mode & KMX61_ACC_STBY_BIT) {
> + ret |= KMX61_ACC_STBY_BIT;
> + acc_stby = 1;
> + } else {
> + ret &= ~KMX61_ACC_STBY_BIT;
> + acc_stby = 0;
> + }
> + }
> +
> + if (device & KMX61_MAG) {
> + if (mode & KMX61_MAG_STBY_BIT) {
> + ret |= KMX61_MAG_STBY_BIT;
> + mag_stby = 1;
> + } else {
> + ret &= ~KMX61_MAG_STBY_BIT;
> + mag_stby = 0;
> + }
> + }
> +
> + if (mode & KMX61_ACT_STBY_BIT)
> + ret |= KMX61_ACT_STBY_BIT;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_STBY, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_stby\n");
> + return ret;
> + }
> +
> + if (acc_stby != -1 && update)
> + data->acc_stby = acc_stby;
> + if (mag_stby != -1 && update)
> + data->mag_stby = mag_stby;
> +
> + return 0;
> +}
> +
> +static int kmx61_get_mode(struct kmx61_data *data, u8 *mode, u8 device)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_STBY);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_stby\n");
> + return ret;
> + }
> + *mode = 0;
> +
> + if (device & KMX61_ACC) {
> + if (ret & KMX61_ACC_STBY_BIT)
> + *mode |= KMX61_ACC_STBY_BIT;
> + else
> + *mode &= ~KMX61_ACC_STBY_BIT;
> + }
> +
> + if (device & KMX61_MAG) {
> + if (ret & KMX61_MAG_STBY_BIT)
> + *mode |= KMX61_MAG_STBY_BIT;
> + else
> + *mode &= ~KMX61_MAG_STBY_BIT;
> + }
> +
> + return 0;
> +}
> +
> +static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
> +{
> + int ret;
> + u8 mode;
> + int lodr_bits, odr_bits;
> +
> + ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
> + if (ret < 0)
> + return ret;
> +
> + lodr_bits = kmx61_convert_freq_to_bit(val, val2);
> + if (lodr_bits < 0)
> + return lodr_bits;
> +
> + /* To change ODR, accel and magn must be in STDBY */
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG,
> + true);
> + if (ret < 0)
> + return ret;
> +
> + odr_bits = 0;
> + if (device & KMX61_ACC)
> + odr_bits |= lodr_bits << KMX61_ACC_ODR_SHIFT;
> + if (device & KMX61_MAG)
> + odr_bits |= lodr_bits << KMX61_MAG_ODR_SHIFT;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_ODCNTL,
> + odr_bits);
> + if (ret < 0)
> + return ret;
> +
> + return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> +}
> +
> +static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
> + u8 device)
> +{ int i;
> + u8 lodr_bits;
> +
> + if (device & KMX61_ACC)
> + lodr_bits = (data->odr_bits >> KMX61_ACC_ODR_SHIFT) &
> + KMX61_ACC_ODR_MASK;
> + else if (device & KMX61_MAG)
> + lodr_bits = (data->odr_bits >> KMX61_MAG_ODR_SHIFT) &
> + KMX61_MAG_ODR_MASK;
> + else
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> + if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
> + *val = kmx61_samp_freq_table[i].val;
> + *val2 = kmx61_samp_freq_table[i].val2;
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +static int kmx61_set_range(struct kmx61_data *data, u8 range)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> + return ret;
> + }
> +
> + ret &= ~KMX61_REG_CTRL1_GSEL_MASK;
> + ret |= range & KMX61_REG_CTRL1_GSEL_MASK;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> + return ret;
> + }
> +
> + data->range = range;
> +
> + return 0;
> +}
> +
> +static int kmx61_set_scale(struct kmx61_data *data, u16 uscale)
> +{
> + int ret, i;
> + u8 mode;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_uscale_table); i++) {
> + if (kmx61_uscale_table[i] == uscale) {
> + ret = kmx61_get_mode(data, &mode,
> + KMX61_ACC | KMX61_MAG);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY,
> + KMX61_ACC | KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_range(data, i);
> + if (ret < 0)
> + return ret;
> +
> + return kmx61_set_mode(data, mode,
> + KMX61_ACC | KMX61_MAG, true);
> + }
> + }
> + return -EINVAL;
> +}
> +
> +static int kmx61_chip_init(struct kmx61_data *data)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_WHO_AM_I);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading who_am_i\n");
> + return ret;
> + }
> +
> + if (ret != KMX61_CHIP_ID) {
> + dev_err(&data->client->dev,
> + "Wrong chip id, got %x expected %x\n",
> + ret, KMX61_CHIP_ID);
> + return -EINVAL;
> + }
> +
> + /* set accel 12bit, 4g range */
> + ret = kmx61_set_range(data, KMX61_RANGE_4G);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_ODCNTL);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_odcntl\n");
> + return ret;
> + }
> + data->odr_bits = ret;
> +
> + /* set acc/magn to OPERATION mode */
> + ret = kmx61_set_mode(data, 0, KMX61_ACC | KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int kmx61_read_measurement(struct kmx61_data *data, u8 base, u8 offset)
> +{
> + int ret;
> + u8 reg = base + offset * 2;
> +
> + ret = i2c_smbus_read_word_data(data->client, reg);
> + if (ret < 0)
> + dev_err(&data->client->dev, "failed to read reg at %x\n", reg);
> +
> + return ret;
> +}
> +
> +static int kmx61_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + int ret;
> + u8 base_reg;
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + base_reg = KMX61_ACC_XOUT_L;
> + break;
> + case IIO_MAGN:
> + base_reg = KMX61_MAG_XOUT_L;
> + break;
> + default:
> + return -EINVAL;
> + }
> + mutex_lock(&data->lock);
> +
> + ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> + *val = sign_extend32(ret >> chan->scan_type.shift,
> + chan->scan_type.realbits - 1);
> +
> + mutex_unlock(&data->lock);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + *val = 0;
> + *val2 = kmx61_uscale_table[data->range];
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_MAGN:
> + /* 14 bits res, 1465 microGauss per magn count */
> + *val = 0;
> + *val2 = 1465;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
> + return -EINVAL;
> +
> + mutex_lock(&data->lock);
> + ret = kmx61_get_odr(data, val, val2, chan->address);
> + mutex_unlock(&data->lock);
> + if (ret)
> + return -EINVAL;
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> + return -EINVAL;
> +}
> +
> +static int kmx61_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val,
> + int val2, long mask)
> +{
> + int ret;
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
> + return -EINVAL;
> +
> + mutex_lock(&data->lock);
> + ret = kmx61_set_odr(data, val, val2, chan->address);
> + mutex_unlock(&data->lock);
> + return ret;
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + if (val != 0)
> + return -EINVAL;
> + mutex_lock(&data->lock);
> + ret = kmx61_set_scale(data, val2);
> + mutex_unlock(&data->lock);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info kmx61_acc_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = kmx61_read_raw,
> + .write_raw = kmx61_write_raw,
> + .attrs = &kmx61_acc_attribute_group,
> +};
> +
> +static const struct iio_info kmx61_mag_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = kmx61_read_raw,
> + .write_raw = kmx61_write_raw,
> + .attrs = &kmx61_mag_attribute_group,
> +};
> +
> +static struct iio_dev *kmx61_indiodev_setup(struct kmx61_data *data,
> + const struct iio_info *info,
> + const struct iio_chan_spec *chan,
> + int num_channels,
> + const char *name)
> +{
> + struct iio_dev *indio_dev;
> +
> + indio_dev = devm_iio_device_alloc(&data->client->dev, sizeof(data));
> + if (!indio_dev)
> + return ERR_PTR(-ENOMEM);
> +
> + kmx61_set_data(indio_dev, data);
> +
> + indio_dev->dev.parent = &data->client->dev;
> + indio_dev->channels = chan;
> + indio_dev->num_channels = num_channels;
> + indio_dev->name = name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = info;
> +
> + return indio_dev;
> +}
> +
> +static int kmx61_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int ret;
> + struct kmx61_data *data;
> + const char *name = NULL;
> +
> + data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, data);
> + data->client = client;
> +
> + mutex_init(&data->lock);
> +
> + data->acc_indio_dev =
> + kmx61_indiodev_setup(data, &kmx61_acc_info,
> + kmx61_acc_channels,
> + ARRAY_SIZE(kmx61_acc_channels),
> + name);
> + if (IS_ERR(data->acc_indio_dev))
> + return PTR_ERR(data->acc_indio_dev);
> +
> + data->mag_indio_dev =
> + kmx61_indiodev_setup(data, &kmx61_mag_info,
> + kmx61_mag_channels,
> + ARRAY_SIZE(kmx61_mag_channels),
> + name);
> + if (IS_ERR(data->mag_indio_dev))
> + return PTR_ERR(data->mag_indio_dev);
> +
> + ret = kmx61_chip_init(data);
> + if (ret < 0)
> + return ret;
> +
> + ret = iio_device_register(data->acc_indio_dev);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to register acc iio device\n");
> + goto err_chip_uninit;
> + }
> +
> + ret = iio_device_register(data->mag_indio_dev);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to register mag iio device\n");
> + goto err_iio_unregister;
> + }
> +
> + return 0;
> +
> +err_iio_unregister:
> + iio_device_unregister(data->acc_indio_dev);
> +err_chip_uninit:
> + kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + return ret;
> +}
> +
> +static int kmx61_remove(struct i2c_client *client)
> +{
> + struct kmx61_data *data = i2c_get_clientdata(client);
> +
> + iio_device_unregister(data->acc_indio_dev);
> + iio_device_unregister(data->mag_indio_dev);
> +
> + mutex_lock(&data->lock);
> + kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + mutex_unlock(&data->lock);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id kmx61_id[] = {
> + {"kmx611021", 0},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, kmx61_id);
> +
> +static struct i2c_driver kmx61_driver = {
> + .driver = {
> + .name = KMX61_DRV_NAME,
> + },
> + .probe = kmx61_probe,
> + .remove = kmx61_remove,
> + .id_table = kmx61_id,
> +};
> +
> +module_i2c_driver(kmx61_driver);
> +
> +MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
> +MODULE_DESCRIPTION("KMX61 accelerometer/magnetometer driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 1/6] iio: imu: Add support for Kionix KMX61 sensor
2014-12-14 22:43 ` Hartmut Knaack
@ 2014-12-15 10:50 ` Daniel Baluta
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Baluta @ 2014-12-15 10:50 UTC (permalink / raw)
To: Hartmut Knaack
Cc: Daniel Baluta, Jonathan Cameron, Peter Meerwald,
Srinivas Pandruvada, linux-iio@vger.kernel.org,
Linux Kernel Mailing List
On Mon, Dec 15, 2014 at 12:43 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
> Daniel Baluta schrieb am 03.12.2014 um 14:31:
>> Minimal implementation for KMX61 6-axis accelerometer/magnetometer. It exports
>> raw accel/magn readings together with scale and sampling frequency.
>>
>> This driver uses two IIO devices one for accelerometer and one for magnetometer.
>>
> Hi Daniel,
> I think you dropped slightly too much code. In kmx61_set_odr() you stored the set
> odr bits in data->odr_bits previously, but now it's gone. But kmx61_get_odr()
> accesses data->odr_bits, which now remains in the state from driver initialization.
> Other than that, it's looking good.
Hi Hartmut,
Good catch. I will send a patch asap.
Thanks for feedback!
Daniel.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 2/6] iio: imu: kmx61: Add acpi support
2014-12-03 13:31 [PATCH v4 0/6] Add support for KMX61 sensor Daniel Baluta
2014-12-03 13:31 ` [PATCH v4 1/6] iio: imu: Add support for Kionix " Daniel Baluta
@ 2014-12-03 13:31 ` Daniel Baluta
2014-12-12 13:35 ` Jonathan Cameron
2014-12-03 13:31 ` [PATCH v4 3/6] iio: imu: kmx61: Add PM runtime support Daniel Baluta
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Daniel Baluta @ 2014-12-03 13:31 UTC (permalink / raw)
To: jic23, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel, daniel.baluta
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 5b44371..893f7c8 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -13,10 +13,13 @@
#include <linux/module.h>
#include <linux/i2c.h>
+#include <linux/acpi.h>
+#include <linux/gpio/consumer.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
#define KMX61_DRV_NAME "kmx61"
+#define KMX61_GPIO_NAME "kmx61_int"
#define KMX61_REG_WHO_AM_I 0x00
@@ -573,6 +576,44 @@ static const struct iio_info kmx61_mag_info = {
.attrs = &kmx61_mag_attribute_group,
};
+static const char *kmx61_match_acpi_device(struct device *dev)
+{
+ const struct acpi_device_id *id;
+
+ id = acpi_match_device(dev->driver->acpi_match_table, dev);
+ if (!id)
+ return NULL;
+ return dev_name(dev);
+}
+
+static int kmx61_gpio_probe(struct i2c_client *client, struct kmx61_data *data)
+{
+ struct device *dev;
+ struct gpio_desc *gpio;
+ int ret;
+
+ if (!client)
+ return -EINVAL;
+
+ dev = &client->dev;
+
+ /* data ready gpio interrupt pin */
+ gpio = devm_gpiod_get_index(dev, KMX61_GPIO_NAME, 0);
+ if (IS_ERR(gpio)) {
+ dev_err(dev, "acpi gpio get index failed\n");
+ return PTR_ERR(gpio);
+ }
+
+ ret = gpiod_direction_input(gpio);
+ if (ret)
+ return ret;
+
+ ret = gpiod_to_irq(gpio);
+
+ dev_dbg(dev, "GPIO resource, no:%d irq:%d\n", desc_to_gpio(gpio), ret);
+ return ret;
+}
+
static struct iio_dev *kmx61_indiodev_setup(struct kmx61_data *data,
const struct iio_info *info,
const struct iio_chan_spec *chan,
@@ -613,6 +654,13 @@ static int kmx61_probe(struct i2c_client *client,
mutex_init(&data->lock);
+ if (id)
+ name = id->name;
+ else if (ACPI_HANDLE(&client->dev))
+ name = kmx61_match_acpi_device(&client->dev);
+ else
+ return -ENODEV;
+
data->acc_indio_dev =
kmx61_indiodev_setup(data, &kmx61_acc_info,
kmx61_acc_channels,
@@ -633,6 +681,9 @@ static int kmx61_probe(struct i2c_client *client,
if (ret < 0)
return ret;
+ if (client->irq < 0)
+ client->irq = kmx61_gpio_probe(client, data);
+
ret = iio_device_register(data->acc_indio_dev);
if (ret < 0) {
dev_err(&client->dev, "Failed to register acc iio device\n");
@@ -668,6 +719,13 @@ static int kmx61_remove(struct i2c_client *client)
return 0;
}
+static const struct acpi_device_id kmx61_acpi_match[] = {
+ {"KMX61021", 0},
+ {}
+};
+
+MODULE_DEVICE_TABLE(acpi, kmx61_acpi_match);
+
static const struct i2c_device_id kmx61_id[] = {
{"kmx611021", 0},
{}
@@ -678,6 +736,7 @@ MODULE_DEVICE_TABLE(i2c, kmx61_id);
static struct i2c_driver kmx61_driver = {
.driver = {
.name = KMX61_DRV_NAME,
+ .acpi_match_table = ACPI_PTR(kmx61_acpi_match),
},
.probe = kmx61_probe,
.remove = kmx61_remove,
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 2/6] iio: imu: kmx61: Add acpi support
2014-12-03 13:31 ` [PATCH v4 2/6] iio: imu: kmx61: Add acpi support Daniel Baluta
@ 2014-12-12 13:35 ` Jonathan Cameron
0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2014-12-12 13:35 UTC (permalink / raw)
To: Daniel Baluta, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel
On 03/12/14 13:31, Daniel Baluta wrote:
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Applied to the togreg branch of iio.git
Thanks,
Jonathan
> ---
> drivers/iio/imu/kmx61.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 5b44371..893f7c8 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -13,10 +13,13 @@
>
> #include <linux/module.h>
> #include <linux/i2c.h>
> +#include <linux/acpi.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
>
> #define KMX61_DRV_NAME "kmx61"
> +#define KMX61_GPIO_NAME "kmx61_int"
>
> #define KMX61_REG_WHO_AM_I 0x00
>
> @@ -573,6 +576,44 @@ static const struct iio_info kmx61_mag_info = {
> .attrs = &kmx61_mag_attribute_group,
> };
>
> +static const char *kmx61_match_acpi_device(struct device *dev)
> +{
> + const struct acpi_device_id *id;
> +
> + id = acpi_match_device(dev->driver->acpi_match_table, dev);
> + if (!id)
> + return NULL;
> + return dev_name(dev);
> +}
> +
> +static int kmx61_gpio_probe(struct i2c_client *client, struct kmx61_data *data)
> +{
> + struct device *dev;
> + struct gpio_desc *gpio;
> + int ret;
> +
> + if (!client)
> + return -EINVAL;
> +
> + dev = &client->dev;
> +
> + /* data ready gpio interrupt pin */
> + gpio = devm_gpiod_get_index(dev, KMX61_GPIO_NAME, 0);
> + if (IS_ERR(gpio)) {
> + dev_err(dev, "acpi gpio get index failed\n");
> + return PTR_ERR(gpio);
> + }
> +
> + ret = gpiod_direction_input(gpio);
> + if (ret)
> + return ret;
> +
> + ret = gpiod_to_irq(gpio);
> +
> + dev_dbg(dev, "GPIO resource, no:%d irq:%d\n", desc_to_gpio(gpio), ret);
> + return ret;
> +}
> +
> static struct iio_dev *kmx61_indiodev_setup(struct kmx61_data *data,
> const struct iio_info *info,
> const struct iio_chan_spec *chan,
> @@ -613,6 +654,13 @@ static int kmx61_probe(struct i2c_client *client,
>
> mutex_init(&data->lock);
>
> + if (id)
> + name = id->name;
> + else if (ACPI_HANDLE(&client->dev))
> + name = kmx61_match_acpi_device(&client->dev);
> + else
> + return -ENODEV;
> +
> data->acc_indio_dev =
> kmx61_indiodev_setup(data, &kmx61_acc_info,
> kmx61_acc_channels,
> @@ -633,6 +681,9 @@ static int kmx61_probe(struct i2c_client *client,
> if (ret < 0)
> return ret;
>
> + if (client->irq < 0)
> + client->irq = kmx61_gpio_probe(client, data);
> +
> ret = iio_device_register(data->acc_indio_dev);
> if (ret < 0) {
> dev_err(&client->dev, "Failed to register acc iio device\n");
> @@ -668,6 +719,13 @@ static int kmx61_remove(struct i2c_client *client)
> return 0;
> }
>
> +static const struct acpi_device_id kmx61_acpi_match[] = {
> + {"KMX61021", 0},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, kmx61_acpi_match);
> +
> static const struct i2c_device_id kmx61_id[] = {
> {"kmx611021", 0},
> {}
> @@ -678,6 +736,7 @@ MODULE_DEVICE_TABLE(i2c, kmx61_id);
> static struct i2c_driver kmx61_driver = {
> .driver = {
> .name = KMX61_DRV_NAME,
> + .acpi_match_table = ACPI_PTR(kmx61_acpi_match),
> },
> .probe = kmx61_probe,
> .remove = kmx61_remove,
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 3/6] iio: imu: kmx61: Add PM runtime support
2014-12-03 13:31 [PATCH v4 0/6] Add support for KMX61 sensor Daniel Baluta
2014-12-03 13:31 ` [PATCH v4 1/6] iio: imu: Add support for Kionix " Daniel Baluta
2014-12-03 13:31 ` [PATCH v4 2/6] iio: imu: kmx61: Add acpi support Daniel Baluta
@ 2014-12-03 13:31 ` Daniel Baluta
2014-12-12 13:37 ` Jonathan Cameron
2014-12-14 22:55 ` Hartmut Knaack
2014-12-03 13:31 ` [PATCH v4 4/6] iio: imu: kmx61: Add PM sleep support Daniel Baluta
` (2 subsequent siblings)
5 siblings, 2 replies; 20+ messages in thread
From: Daniel Baluta @ 2014-12-03 13:31 UTC (permalink / raw)
To: jic23, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel, daniel.baluta
By default both sensors are ACTIVE, in this way the driver
will work even if CONFIG_PM_RUNTIME is not selected.
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 112 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 893f7c8..7536043 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -15,6 +15,8 @@
#include <linux/i2c.h>
#include <linux/acpi.h>
#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
@@ -69,6 +71,8 @@
#define KMX61_ACC_ODR_MASK 0x0F
#define KMX61_MAG_ODR_MASK 0xF0
+#define KMX61_SLEEP_DELAY_MS 2000
+
#define KMX61_CHIP_ID 0x12
/* KMX61 devices */
@@ -85,6 +89,10 @@ struct kmx61_data {
bool acc_stby;
bool mag_stby;
+ /* power state */
+ bool acc_ps;
+ bool mag_ps;
+
/* config bits */
u8 range;
u8 odr_bits;
@@ -457,6 +465,58 @@ static int kmx61_chip_init(struct kmx61_data *data)
return 0;
}
+/**
+ * kmx61_set_power_state() - set power state for kmx61 @device
+ * @data - kmx61 device private pointer
+ * @on - power state to be set for @device
+ * @device - bitmask indicating device for which @on state needs to be set
+ *
+ * Notice that when ACC power state needs to be set to ON and MAG is in
+ * OPERATION then we know that kmx61_runtime_resume was already called
+ * so we must set ACC OPERATION mode here. The same happens when MAG power
+ * state needs to be set to ON and ACC is in OPERATION.
+ */
+static int kmx61_set_power_state(struct kmx61_data *data, bool on, u8 device)
+{
+#ifdef CONFIG_PM_RUNTIME
+ int ret;
+
+ if (device & KMX61_ACC) {
+ if (on && !data->acc_ps && !data->mag_stby) {
+ ret = kmx61_set_mode(data, 0, KMX61_ACC, true);
+ if (ret < 0)
+ return ret;
+ }
+ data->acc_ps = on;
+ }
+ if (device & KMX61_MAG) {
+ if (on && !data->mag_ps && !data->acc_stby) {
+ ret = kmx61_set_mode(data, 0, KMX61_MAG, true);
+ if (ret < 0)
+ return ret;
+ }
+ data->mag_ps = on;
+ }
+
+ if (on) {
+ ret = pm_runtime_get_sync(&data->client->dev);
+ } else {
+ pm_runtime_mark_last_busy(&data->client->dev);
+ ret = pm_runtime_put_autosuspend(&data->client->dev);
+ }
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "Failed: kmx61_set_power_state for %d, ret %d\n",
+ on, ret);
+ if (on)
+ pm_runtime_put_noidle(&data->client->dev);
+
+ return ret;
+ }
+#endif
+ return 0;
+}
+
static int kmx61_read_measurement(struct kmx61_data *data, u8 base, u8 offset)
{
int ret;
@@ -491,13 +551,16 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
}
mutex_lock(&data->lock);
+ kmx61_set_power_state(data, true, chan->address);
ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
if (ret < 0) {
+ kmx61_set_power_state(data, false, chan->address);
mutex_unlock(&data->lock);
return ret;
}
*val = sign_extend32(ret >> chan->scan_type.shift,
chan->scan_type.realbits - 1);
+ kmx61_set_power_state(data, false, chan->address);
mutex_unlock(&data->lock);
return IIO_VAL_INT;
@@ -693,12 +756,22 @@ static int kmx61_probe(struct i2c_client *client,
ret = iio_device_register(data->mag_indio_dev);
if (ret < 0) {
dev_err(&client->dev, "Failed to register mag iio device\n");
- goto err_iio_unregister;
+ goto err_iio_unregister_acc;
}
+ ret = pm_runtime_set_active(&client->dev);
+ if (ret < 0)
+ goto err_iio_unregister_mag;
+
+ pm_runtime_enable(&client->dev);
+ pm_runtime_set_autosuspend_delay(&client->dev, KMX61_SLEEP_DELAY_MS);
+ pm_runtime_use_autosuspend(&client->dev);
+
return 0;
-err_iio_unregister:
+err_iio_unregister_mag:
+ iio_device_unregister(data->mag_indio_dev);
+err_iio_unregister_acc:
iio_device_unregister(data->acc_indio_dev);
err_chip_uninit:
kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
@@ -709,6 +782,10 @@ static int kmx61_remove(struct i2c_client *client)
{
struct kmx61_data *data = i2c_get_clientdata(client);
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+ pm_runtime_put_noidle(&client->dev);
+
iio_device_unregister(data->acc_indio_dev);
iio_device_unregister(data->mag_indio_dev);
@@ -719,6 +796,38 @@ static int kmx61_remove(struct i2c_client *client)
return 0;
}
+
+#ifdef CONFIG_PM_RUNTIME
+static int kmx61_runtime_suspend(struct device *dev)
+{
+ struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
+ int ret;
+
+ mutex_lock(&data->lock);
+ ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
+ mutex_unlock(&data->lock);
+
+ return ret;
+}
+
+static int kmx61_runtime_resume(struct device *dev)
+{
+ struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
+ u8 stby = 0;
+
+ if (!data->acc_ps)
+ stby |= KMX61_ACC_STBY_BIT;
+ if (!data->mag_ps)
+ stby |= KMX61_MAG_STBY_BIT;
+
+ return kmx61_set_mode(data, stby, KMX61_ACC | KMX61_MAG, true);
+}
+#endif
+
+static const struct dev_pm_ops kmx61_pm_ops = {
+ SET_RUNTIME_PM_OPS(kmx61_runtime_suspend, kmx61_runtime_resume, NULL)
+};
+
static const struct acpi_device_id kmx61_acpi_match[] = {
{"KMX61021", 0},
{}
@@ -737,6 +846,7 @@ static struct i2c_driver kmx61_driver = {
.driver = {
.name = KMX61_DRV_NAME,
.acpi_match_table = ACPI_PTR(kmx61_acpi_match),
+ .pm = &kmx61_pm_ops,
},
.probe = kmx61_probe,
.remove = kmx61_remove,
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 3/6] iio: imu: kmx61: Add PM runtime support
2014-12-03 13:31 ` [PATCH v4 3/6] iio: imu: kmx61: Add PM runtime support Daniel Baluta
@ 2014-12-12 13:37 ` Jonathan Cameron
2014-12-14 22:55 ` Hartmut Knaack
1 sibling, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2014-12-12 13:37 UTC (permalink / raw)
To: Daniel Baluta, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel
On 03/12/14 13:31, Daniel Baluta wrote:
> By default both sensors are ACTIVE, in this way the driver
> will work even if CONFIG_PM_RUNTIME is not selected.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Applied to the togreg branch of iio.git
Thanks,
Jonathan
> ---
> drivers/iio/imu/kmx61.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 112 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 893f7c8..7536043 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -15,6 +15,8 @@
> #include <linux/i2c.h>
> #include <linux/acpi.h>
> #include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/pm_runtime.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
>
> @@ -69,6 +71,8 @@
> #define KMX61_ACC_ODR_MASK 0x0F
> #define KMX61_MAG_ODR_MASK 0xF0
>
> +#define KMX61_SLEEP_DELAY_MS 2000
> +
> #define KMX61_CHIP_ID 0x12
>
> /* KMX61 devices */
> @@ -85,6 +89,10 @@ struct kmx61_data {
> bool acc_stby;
> bool mag_stby;
>
> + /* power state */
> + bool acc_ps;
> + bool mag_ps;
> +
> /* config bits */
> u8 range;
> u8 odr_bits;
> @@ -457,6 +465,58 @@ static int kmx61_chip_init(struct kmx61_data *data)
> return 0;
> }
>
> +/**
> + * kmx61_set_power_state() - set power state for kmx61 @device
> + * @data - kmx61 device private pointer
> + * @on - power state to be set for @device
> + * @device - bitmask indicating device for which @on state needs to be set
> + *
> + * Notice that when ACC power state needs to be set to ON and MAG is in
> + * OPERATION then we know that kmx61_runtime_resume was already called
> + * so we must set ACC OPERATION mode here. The same happens when MAG power
> + * state needs to be set to ON and ACC is in OPERATION.
> + */
> +static int kmx61_set_power_state(struct kmx61_data *data, bool on, u8 device)
> +{
> +#ifdef CONFIG_PM_RUNTIME
> + int ret;
> +
> + if (device & KMX61_ACC) {
> + if (on && !data->acc_ps && !data->mag_stby) {
> + ret = kmx61_set_mode(data, 0, KMX61_ACC, true);
> + if (ret < 0)
> + return ret;
> + }
> + data->acc_ps = on;
> + }
> + if (device & KMX61_MAG) {
> + if (on && !data->mag_ps && !data->acc_stby) {
> + ret = kmx61_set_mode(data, 0, KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> + }
> + data->mag_ps = on;
> + }
> +
> + if (on) {
> + ret = pm_runtime_get_sync(&data->client->dev);
> + } else {
> + pm_runtime_mark_last_busy(&data->client->dev);
> + ret = pm_runtime_put_autosuspend(&data->client->dev);
> + }
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed: kmx61_set_power_state for %d, ret %d\n",
> + on, ret);
> + if (on)
> + pm_runtime_put_noidle(&data->client->dev);
> +
> + return ret;
> + }
> +#endif
> + return 0;
> +}
> +
> static int kmx61_read_measurement(struct kmx61_data *data, u8 base, u8 offset)
> {
> int ret;
> @@ -491,13 +551,16 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
> }
> mutex_lock(&data->lock);
>
> + kmx61_set_power_state(data, true, chan->address);
> ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
> if (ret < 0) {
> + kmx61_set_power_state(data, false, chan->address);
> mutex_unlock(&data->lock);
> return ret;
> }
> *val = sign_extend32(ret >> chan->scan_type.shift,
> chan->scan_type.realbits - 1);
> + kmx61_set_power_state(data, false, chan->address);
>
> mutex_unlock(&data->lock);
> return IIO_VAL_INT;
> @@ -693,12 +756,22 @@ static int kmx61_probe(struct i2c_client *client,
> ret = iio_device_register(data->mag_indio_dev);
> if (ret < 0) {
> dev_err(&client->dev, "Failed to register mag iio device\n");
> - goto err_iio_unregister;
> + goto err_iio_unregister_acc;
> }
>
> + ret = pm_runtime_set_active(&client->dev);
> + if (ret < 0)
> + goto err_iio_unregister_mag;
> +
> + pm_runtime_enable(&client->dev);
> + pm_runtime_set_autosuspend_delay(&client->dev, KMX61_SLEEP_DELAY_MS);
> + pm_runtime_use_autosuspend(&client->dev);
> +
> return 0;
>
> -err_iio_unregister:
> +err_iio_unregister_mag:
> + iio_device_unregister(data->mag_indio_dev);
> +err_iio_unregister_acc:
> iio_device_unregister(data->acc_indio_dev);
> err_chip_uninit:
> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> @@ -709,6 +782,10 @@ static int kmx61_remove(struct i2c_client *client)
> {
> struct kmx61_data *data = i2c_get_clientdata(client);
>
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + pm_runtime_put_noidle(&client->dev);
> +
> iio_device_unregister(data->acc_indio_dev);
> iio_device_unregister(data->mag_indio_dev);
>
> @@ -719,6 +796,38 @@ static int kmx61_remove(struct i2c_client *client)
> return 0;
> }
>
> +
> +#ifdef CONFIG_PM_RUNTIME
> +static int kmx61_runtime_suspend(struct device *dev)
> +{
> + struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
> + int ret;
> +
> + mutex_lock(&data->lock);
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static int kmx61_runtime_resume(struct device *dev)
> +{
> + struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
> + u8 stby = 0;
> +
> + if (!data->acc_ps)
> + stby |= KMX61_ACC_STBY_BIT;
> + if (!data->mag_ps)
> + stby |= KMX61_MAG_STBY_BIT;
> +
> + return kmx61_set_mode(data, stby, KMX61_ACC | KMX61_MAG, true);
> +}
> +#endif
> +
> +static const struct dev_pm_ops kmx61_pm_ops = {
> + SET_RUNTIME_PM_OPS(kmx61_runtime_suspend, kmx61_runtime_resume, NULL)
> +};
> +
> static const struct acpi_device_id kmx61_acpi_match[] = {
> {"KMX61021", 0},
> {}
> @@ -737,6 +846,7 @@ static struct i2c_driver kmx61_driver = {
> .driver = {
> .name = KMX61_DRV_NAME,
> .acpi_match_table = ACPI_PTR(kmx61_acpi_match),
> + .pm = &kmx61_pm_ops,
> },
> .probe = kmx61_probe,
> .remove = kmx61_remove,
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 3/6] iio: imu: kmx61: Add PM runtime support
2014-12-03 13:31 ` [PATCH v4 3/6] iio: imu: kmx61: Add PM runtime support Daniel Baluta
2014-12-12 13:37 ` Jonathan Cameron
@ 2014-12-14 22:55 ` Hartmut Knaack
2014-12-15 16:11 ` Daniel Baluta
1 sibling, 1 reply; 20+ messages in thread
From: Hartmut Knaack @ 2014-12-14 22:55 UTC (permalink / raw)
To: Daniel Baluta, jic23, pmeerw, srinivas.pandruvada; +Cc: linux-iio, linux-kernel
Daniel Baluta schrieb am 03.12.2014 um 14:31:
> By default both sensors are ACTIVE, in this way the driver
> will work even if CONFIG_PM_RUNTIME is not selected.
>
Since kmx61_set_power_state() can return error codes, wouldn't it be better to
handle them (as long as you are not using it in an error handler)?
Thanks,
Hartmut
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
> drivers/iio/imu/kmx61.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 112 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 893f7c8..7536043 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -15,6 +15,8 @@
> #include <linux/i2c.h>
> #include <linux/acpi.h>
> #include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/pm_runtime.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
>
> @@ -69,6 +71,8 @@
> #define KMX61_ACC_ODR_MASK 0x0F
> #define KMX61_MAG_ODR_MASK 0xF0
>
> +#define KMX61_SLEEP_DELAY_MS 2000
> +
> #define KMX61_CHIP_ID 0x12
>
> /* KMX61 devices */
> @@ -85,6 +89,10 @@ struct kmx61_data {
> bool acc_stby;
> bool mag_stby;
>
> + /* power state */
> + bool acc_ps;
> + bool mag_ps;
> +
> /* config bits */
> u8 range;
> u8 odr_bits;
> @@ -457,6 +465,58 @@ static int kmx61_chip_init(struct kmx61_data *data)
> return 0;
> }
>
> +/**
> + * kmx61_set_power_state() - set power state for kmx61 @device
> + * @data - kmx61 device private pointer
> + * @on - power state to be set for @device
> + * @device - bitmask indicating device for which @on state needs to be set
> + *
> + * Notice that when ACC power state needs to be set to ON and MAG is in
> + * OPERATION then we know that kmx61_runtime_resume was already called
> + * so we must set ACC OPERATION mode here. The same happens when MAG power
> + * state needs to be set to ON and ACC is in OPERATION.
> + */
> +static int kmx61_set_power_state(struct kmx61_data *data, bool on, u8 device)
> +{
> +#ifdef CONFIG_PM_RUNTIME
> + int ret;
> +
> + if (device & KMX61_ACC) {
> + if (on && !data->acc_ps && !data->mag_stby) {
> + ret = kmx61_set_mode(data, 0, KMX61_ACC, true);
> + if (ret < 0)
> + return ret;
> + }
> + data->acc_ps = on;
> + }
> + if (device & KMX61_MAG) {
> + if (on && !data->mag_ps && !data->acc_stby) {
> + ret = kmx61_set_mode(data, 0, KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> + }
> + data->mag_ps = on;
> + }
> +
> + if (on) {
> + ret = pm_runtime_get_sync(&data->client->dev);
> + } else {
> + pm_runtime_mark_last_busy(&data->client->dev);
> + ret = pm_runtime_put_autosuspend(&data->client->dev);
> + }
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed: kmx61_set_power_state for %d, ret %d\n",
> + on, ret);
> + if (on)
> + pm_runtime_put_noidle(&data->client->dev);
> +
> + return ret;
> + }
> +#endif
> + return 0;
> +}
> +
> static int kmx61_read_measurement(struct kmx61_data *data, u8 base, u8 offset)
> {
> int ret;
> @@ -491,13 +551,16 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
> }
> mutex_lock(&data->lock);
>
> + kmx61_set_power_state(data, true, chan->address);
> ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
> if (ret < 0) {
> + kmx61_set_power_state(data, false, chan->address);
> mutex_unlock(&data->lock);
> return ret;
> }
> *val = sign_extend32(ret >> chan->scan_type.shift,
> chan->scan_type.realbits - 1);
> + kmx61_set_power_state(data, false, chan->address);
>
> mutex_unlock(&data->lock);
> return IIO_VAL_INT;
> @@ -693,12 +756,22 @@ static int kmx61_probe(struct i2c_client *client,
> ret = iio_device_register(data->mag_indio_dev);
> if (ret < 0) {
> dev_err(&client->dev, "Failed to register mag iio device\n");
> - goto err_iio_unregister;
> + goto err_iio_unregister_acc;
> }
>
> + ret = pm_runtime_set_active(&client->dev);
> + if (ret < 0)
> + goto err_iio_unregister_mag;
> +
> + pm_runtime_enable(&client->dev);
> + pm_runtime_set_autosuspend_delay(&client->dev, KMX61_SLEEP_DELAY_MS);
> + pm_runtime_use_autosuspend(&client->dev);
> +
> return 0;
>
> -err_iio_unregister:
> +err_iio_unregister_mag:
> + iio_device_unregister(data->mag_indio_dev);
> +err_iio_unregister_acc:
> iio_device_unregister(data->acc_indio_dev);
> err_chip_uninit:
> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> @@ -709,6 +782,10 @@ static int kmx61_remove(struct i2c_client *client)
> {
> struct kmx61_data *data = i2c_get_clientdata(client);
>
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + pm_runtime_put_noidle(&client->dev);
> +
> iio_device_unregister(data->acc_indio_dev);
> iio_device_unregister(data->mag_indio_dev);
>
> @@ -719,6 +796,38 @@ static int kmx61_remove(struct i2c_client *client)
> return 0;
> }
>
> +
> +#ifdef CONFIG_PM_RUNTIME
> +static int kmx61_runtime_suspend(struct device *dev)
> +{
> + struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
> + int ret;
> +
> + mutex_lock(&data->lock);
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static int kmx61_runtime_resume(struct device *dev)
> +{
> + struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
> + u8 stby = 0;
> +
> + if (!data->acc_ps)
> + stby |= KMX61_ACC_STBY_BIT;
> + if (!data->mag_ps)
> + stby |= KMX61_MAG_STBY_BIT;
> +
> + return kmx61_set_mode(data, stby, KMX61_ACC | KMX61_MAG, true);
> +}
> +#endif
> +
> +static const struct dev_pm_ops kmx61_pm_ops = {
> + SET_RUNTIME_PM_OPS(kmx61_runtime_suspend, kmx61_runtime_resume, NULL)
> +};
> +
> static const struct acpi_device_id kmx61_acpi_match[] = {
> {"KMX61021", 0},
> {}
> @@ -737,6 +846,7 @@ static struct i2c_driver kmx61_driver = {
> .driver = {
> .name = KMX61_DRV_NAME,
> .acpi_match_table = ACPI_PTR(kmx61_acpi_match),
> + .pm = &kmx61_pm_ops,
> },
> .probe = kmx61_probe,
> .remove = kmx61_remove,
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 3/6] iio: imu: kmx61: Add PM runtime support
2014-12-14 22:55 ` Hartmut Knaack
@ 2014-12-15 16:11 ` Daniel Baluta
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Baluta @ 2014-12-15 16:11 UTC (permalink / raw)
To: Hartmut Knaack
Cc: Daniel Baluta, Jonathan Cameron, Peter Meerwald,
Srinivas Pandruvada, linux-iio@vger.kernel.org,
Linux Kernel Mailing List
On Mon, Dec 15, 2014 at 12:55 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
> Daniel Baluta schrieb am 03.12.2014 um 14:31:
>> By default both sensors are ACTIVE, in this way the driver
>> will work even if CONFIG_PM_RUNTIME is not selected.
>>
> Since kmx61_set_power_state() can return error codes, wouldn't it be better to
> handle them (as long as you are not using it in an error handler)?
> Thanks,
That's correct I will send a patch asap.
Daniel.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 4/6] iio: imu: kmx61: Add PM sleep support
2014-12-03 13:31 [PATCH v4 0/6] Add support for KMX61 sensor Daniel Baluta
` (2 preceding siblings ...)
2014-12-03 13:31 ` [PATCH v4 3/6] iio: imu: kmx61: Add PM runtime support Daniel Baluta
@ 2014-12-03 13:31 ` Daniel Baluta
2014-12-12 13:45 ` Jonathan Cameron
2014-12-03 13:31 ` [PATCH v4 5/6] iio: imu: kmx61: Add support for data ready triggers Daniel Baluta
2014-12-03 13:31 ` [PATCH v4 6/6] iio: imu: kmx61: Add support for any motion trigger Daniel Baluta
5 siblings, 1 reply; 20+ messages in thread
From: Daniel Baluta @ 2014-12-03 13:31 UTC (permalink / raw)
To: jic23, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel, daniel.baluta
Per sensor state (ACTIVE/STANDBY) is saved in driver's
private data (acc_stby/mag_stby) and restored when
resume is called.
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 7536043..f69ae5a7 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -16,6 +16,7 @@
#include <linux/acpi.h>
#include <linux/gpio/consumer.h>
#include <linux/interrupt.h>
+#include <linux/pm.h>
#include <linux/pm_runtime.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
@@ -796,6 +797,33 @@ static int kmx61_remove(struct i2c_client *client)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
+static int kmx61_suspend(struct device *dev)
+{
+ int ret;
+ struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
+
+ mutex_lock(&data->lock);
+ ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG,
+ false);
+ mutex_unlock(&data->lock);
+
+ return ret;
+}
+
+static int kmx61_resume(struct device *dev)
+{
+ u8 stby = 0;
+ struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
+
+ if (data->acc_stby)
+ stby |= KMX61_ACC_STBY_BIT;
+ if (data->mag_stby)
+ stby |= KMX61_MAG_STBY_BIT;
+
+ return kmx61_set_mode(data, stby, KMX61_ACC | KMX61_MAG, true);
+}
+#endif
#ifdef CONFIG_PM_RUNTIME
static int kmx61_runtime_suspend(struct device *dev)
@@ -825,6 +853,7 @@ static int kmx61_runtime_resume(struct device *dev)
#endif
static const struct dev_pm_ops kmx61_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(kmx61_suspend, kmx61_resume)
SET_RUNTIME_PM_OPS(kmx61_runtime_suspend, kmx61_runtime_resume, NULL)
};
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 4/6] iio: imu: kmx61: Add PM sleep support
2014-12-03 13:31 ` [PATCH v4 4/6] iio: imu: kmx61: Add PM sleep support Daniel Baluta
@ 2014-12-12 13:45 ` Jonathan Cameron
0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2014-12-12 13:45 UTC (permalink / raw)
To: Daniel Baluta, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel
On 03/12/14 13:31, Daniel Baluta wrote:
> Per sensor state (ACTIVE/STANDBY) is saved in driver's
> private data (acc_stby/mag_stby) and restored when
> resume is called.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
applied.
> ---
> drivers/iio/imu/kmx61.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 7536043..f69ae5a7 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -16,6 +16,7 @@
> #include <linux/acpi.h>
> #include <linux/gpio/consumer.h>
> #include <linux/interrupt.h>
> +#include <linux/pm.h>
> #include <linux/pm_runtime.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> @@ -796,6 +797,33 @@ static int kmx61_remove(struct i2c_client *client)
> return 0;
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int kmx61_suspend(struct device *dev)
> +{
> + int ret;
> + struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
> +
> + mutex_lock(&data->lock);
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG,
> + false);
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static int kmx61_resume(struct device *dev)
> +{
> + u8 stby = 0;
> + struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
> +
> + if (data->acc_stby)
> + stby |= KMX61_ACC_STBY_BIT;
> + if (data->mag_stby)
> + stby |= KMX61_MAG_STBY_BIT;
> +
> + return kmx61_set_mode(data, stby, KMX61_ACC | KMX61_MAG, true);
> +}
> +#endif
>
> #ifdef CONFIG_PM_RUNTIME
> static int kmx61_runtime_suspend(struct device *dev)
> @@ -825,6 +853,7 @@ static int kmx61_runtime_resume(struct device *dev)
> #endif
>
> static const struct dev_pm_ops kmx61_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(kmx61_suspend, kmx61_resume)
> SET_RUNTIME_PM_OPS(kmx61_runtime_suspend, kmx61_runtime_resume, NULL)
> };
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 5/6] iio: imu: kmx61: Add support for data ready triggers
2014-12-03 13:31 [PATCH v4 0/6] Add support for KMX61 sensor Daniel Baluta
` (3 preceding siblings ...)
2014-12-03 13:31 ` [PATCH v4 4/6] iio: imu: kmx61: Add PM sleep support Daniel Baluta
@ 2014-12-03 13:31 ` Daniel Baluta
2014-12-12 13:49 ` Jonathan Cameron
2014-12-14 23:04 ` Hartmut Knaack
2014-12-03 13:31 ` [PATCH v4 6/6] iio: imu: kmx61: Add support for any motion trigger Daniel Baluta
5 siblings, 2 replies; 20+ messages in thread
From: Daniel Baluta @ 2014-12-03 13:31 UTC (permalink / raw)
To: jic23, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel, daniel.baluta
This creates a data ready trigger per IIO device.
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/Kconfig | 2 +
drivers/iio/imu/kmx61.c | 295 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 296 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index db4221d..5e610f7 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -28,6 +28,8 @@ config ADIS16480
config KMX61
tristate "Kionix KMX61 6-axis accelerometer and magnetometer"
depends on I2C
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
help
Say Y here if you want to build a driver for Kionix KMX61 6-axis
accelerometer and magnetometer.
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index f69ae5a7..0f6816a 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -20,9 +20,14 @@
#include <linux/pm_runtime.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
#define KMX61_DRV_NAME "kmx61"
#define KMX61_GPIO_NAME "kmx61_int"
+#define KMX61_IRQ_NAME "kmx61_event"
#define KMX61_REG_WHO_AM_I 0x00
@@ -55,9 +60,11 @@
#define KMX61_MAG_ZOUT_L 0x16
#define KMX61_MAG_ZOUT_H 0x17
+#define KMX61_REG_INL 0x28
#define KMX61_REG_STBY 0x29
#define KMX61_REG_CTRL1 0x2A
#define KMX61_REG_ODCNTL 0x2C
+#define KMX61_REG_INC1 0x2D
#define KMX61_ACC_STBY_BIT BIT(0)
#define KMX61_MAG_STBY_BIT BIT(1)
@@ -67,6 +74,13 @@
#define KMX61_REG_CTRL1_GSEL_MASK 0x03
+#define KMX61_REG_CTRL1_BIT_RES BIT(4)
+#define KMX61_REG_CTRL1_BIT_DRDYE BIT(5)
+
+#define KMX61_REG_INC1_BIT_DRDYM BIT(1)
+#define KMX61_REG_INC1_BIT_DRDYA BIT(2)
+#define KMX61_REG_INC1_BIT_IEN BIT(5)
+
#define KMX61_ACC_ODR_SHIFT 0
#define KMX61_MAG_ODR_SHIFT 4
#define KMX61_ACC_ODR_MASK 0x0F
@@ -100,9 +114,13 @@ struct kmx61_data {
/* accelerometer specific data */
struct iio_dev *acc_indio_dev;
+ struct iio_trigger *acc_dready_trig;
+ bool acc_dready_trig_on;
/* magnetometer specific data */
struct iio_dev *mag_indio_dev;
+ struct iio_trigger *mag_dready_trig;
+ bool mag_dready_trig_on;
};
enum kmx61_range {
@@ -466,6 +484,69 @@ static int kmx61_chip_init(struct kmx61_data *data)
return 0;
}
+static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
+ bool status, u8 device)
+{
+ u8 mode;
+ int ret;
+
+ ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
+ if (ret < 0)
+ return ret;
+
+ ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INC1);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
+ return ret;
+ }
+
+ if (status) {
+ ret |= KMX61_REG_INC1_BIT_IEN;
+ if (device & KMX61_ACC)
+ ret |= KMX61_REG_INC1_BIT_DRDYA;
+ if (device & KMX61_MAG)
+ ret |= KMX61_REG_INC1_BIT_DRDYM;
+ } else {
+ ret &= ~KMX61_REG_INC1_BIT_IEN;
+ if (device & KMX61_ACC)
+ ret &= ~KMX61_REG_INC1_BIT_DRDYA;
+ if (device & KMX61_MAG)
+ ret &= ~KMX61_REG_INC1_BIT_DRDYM;
+ }
+ ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_INC1, ret);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
+ return ret;
+ }
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
+ return ret;
+ }
+
+ if (status)
+ ret |= KMX61_REG_CTRL1_BIT_DRDYE;
+ else
+ ret &= ~KMX61_REG_CTRL1_BIT_DRDYE;
+
+ ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
+ return ret;
+ }
+
+ ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
/**
* kmx61_set_power_state() - set power state for kmx61 @device
* @data - kmx61 device private pointer
@@ -626,11 +707,34 @@ static int kmx61_write_raw(struct iio_dev *indio_dev,
}
}
+static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
+ struct iio_trigger *trig)
+{
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+
+ if (data->acc_dready_trig != trig)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int kmx61_mag_validate_trigger(struct iio_dev *indio_dev,
+ struct iio_trigger *trig)
+{
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+
+ if (data->mag_dready_trig != trig)
+ return -EINVAL;
+
+ return 0;
+}
+
static const struct iio_info kmx61_acc_info = {
.driver_module = THIS_MODULE,
.read_raw = kmx61_read_raw,
.write_raw = kmx61_write_raw,
.attrs = &kmx61_acc_attribute_group,
+ .validate_trigger = kmx61_acc_validate_trigger,
};
static const struct iio_info kmx61_mag_info = {
@@ -638,8 +742,109 @@ static const struct iio_info kmx61_mag_info = {
.read_raw = kmx61_read_raw,
.write_raw = kmx61_write_raw,
.attrs = &kmx61_mag_attribute_group,
+ .validate_trigger = kmx61_mag_validate_trigger,
+};
+
+
+static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
+ bool state)
+{
+ int ret = 0;
+ u8 device;
+
+ struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+ struct kmx61_data *data = iio_priv(indio_dev);
+
+ mutex_lock(&data->lock);
+
+ if (data->acc_dready_trig == trig)
+ device = KMX61_ACC;
+ else
+ device = KMX61_MAG;
+
+ ret = kmx61_set_power_state(data, state, device);
+ if (ret < 0) {
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ ret = kmx61_setup_new_data_interrupt(data, state, device);
+ if (ret < 0) {
+ kmx61_set_power_state(data, false, device);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ if (data->acc_dready_trig == trig)
+ data->acc_dready_trig_on = state;
+ else
+ data->mag_dready_trig_on = state;
+
+ mutex_unlock(&data->lock);
+
+ return 0;
+}
+
+static int kmx61_trig_try_reenable(struct iio_trigger *trig)
+{
+ struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_inl\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct iio_trigger_ops kmx61_trigger_ops = {
+ .set_trigger_state = kmx61_data_rdy_trigger_set_state,
+ .try_reenable = kmx61_trig_try_reenable,
+ .owner = THIS_MODULE,
};
+static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
+{
+ struct kmx61_data *data = private;
+
+ if (data->acc_dready_trig_on)
+ iio_trigger_poll(data->acc_dready_trig);
+ if (data->mag_dready_trig_on)
+ iio_trigger_poll(data->mag_dready_trig);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t kmx61_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+ int bit, ret, i = 0;
+ s16 buffer[8];
+
+ mutex_lock(&data->lock);
+ for_each_set_bit(bit, indio_dev->buffer->scan_mask,
+ indio_dev->masklength) {
+ ret = kmx61_read_measurement(data, KMX61_ACC_XOUT_L, bit);
+ if (ret < 0) {
+ mutex_unlock(&data->lock);
+ goto err;
+ }
+ buffer[i++] = ret;
+ }
+ mutex_unlock(&data->lock);
+
+ iio_push_to_buffers(indio_dev, buffer);
+err:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
static const char *kmx61_match_acpi_device(struct device *dev)
{
const struct acpi_device_id *id;
@@ -702,6 +907,32 @@ static struct iio_dev *kmx61_indiodev_setup(struct kmx61_data *data,
return indio_dev;
}
+static struct iio_trigger *kmx61_trigger_setup(struct kmx61_data *data,
+ struct iio_dev *indio_dev,
+ const char *tag)
+{
+ struct iio_trigger *trig;
+ int ret;
+
+ trig = devm_iio_trigger_alloc(&data->client->dev,
+ "%s-%s-dev%d",
+ indio_dev->name,
+ tag,
+ indio_dev->id);
+ if (!trig)
+ return ERR_PTR(-ENOMEM);
+
+ trig->dev.parent = &data->client->dev;
+ trig->ops = &kmx61_trigger_ops;
+ iio_trigger_set_drvdata(trig, indio_dev);
+
+ ret = iio_trigger_register(trig);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return trig;
+}
+
static int kmx61_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -748,10 +979,55 @@ static int kmx61_probe(struct i2c_client *client,
if (client->irq < 0)
client->irq = kmx61_gpio_probe(client, data);
+ if (client->irq >= 0) {
+ ret = devm_request_threaded_irq(&client->dev, client->irq,
+ kmx61_data_rdy_trig_poll,
+ NULL,
+ IRQF_TRIGGER_RISING,
+ KMX61_IRQ_NAME,
+ data);
+ if (ret)
+ goto err_chip_uninit;
+
+ data->acc_dready_trig =
+ kmx61_trigger_setup(data, data->acc_indio_dev,
+ "dready");
+ if (IS_ERR(data->acc_dready_trig))
+ return PTR_ERR(data->acc_dready_trig);
+
+ data->mag_dready_trig =
+ kmx61_trigger_setup(data, data->mag_indio_dev,
+ "dready");
+ if (IS_ERR(data->mag_dready_trig)) {
+ ret = PTR_ERR(data->mag_dready_trig);
+ goto err_trigger_unregister;
+ }
+
+ ret = iio_triggered_buffer_setup(data->acc_indio_dev,
+ &iio_pollfunc_store_time,
+ kmx61_trigger_handler,
+ NULL);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "Failed to setup acc triggered buffer\n");
+ goto err_trigger_unregister;
+ }
+
+ ret = iio_triggered_buffer_setup(data->mag_indio_dev,
+ &iio_pollfunc_store_time,
+ kmx61_trigger_handler,
+ NULL);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "Failed to setup mag triggered buffer\n");
+ goto err_trigger_unregister;
+ }
+ }
+
ret = iio_device_register(data->acc_indio_dev);
if (ret < 0) {
dev_err(&client->dev, "Failed to register acc iio device\n");
- goto err_chip_uninit;
+ goto err_buffer_cleanup;
}
ret = iio_device_register(data->mag_indio_dev);
@@ -774,6 +1050,16 @@ err_iio_unregister_mag:
iio_device_unregister(data->mag_indio_dev);
err_iio_unregister_acc:
iio_device_unregister(data->acc_indio_dev);
+err_buffer_cleanup:
+ if (client->irq >= 0) {
+ iio_triggered_buffer_cleanup(data->acc_indio_dev);
+ iio_triggered_buffer_cleanup(data->mag_indio_dev);
+ }
+err_trigger_unregister:
+ if (data->acc_dready_trig)
+ iio_trigger_unregister(data->acc_dready_trig);
+ if (data->mag_dready_trig)
+ iio_trigger_unregister(data->mag_dready_trig);
err_chip_uninit:
kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
return ret;
@@ -790,6 +1076,13 @@ static int kmx61_remove(struct i2c_client *client)
iio_device_unregister(data->acc_indio_dev);
iio_device_unregister(data->mag_indio_dev);
+ if (client->irq >= 0) {
+ iio_triggered_buffer_cleanup(data->acc_indio_dev);
+ iio_triggered_buffer_cleanup(data->mag_indio_dev);
+ iio_trigger_unregister(data->acc_dready_trig);
+ iio_trigger_unregister(data->mag_dready_trig);
+ }
+
mutex_lock(&data->lock);
kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
mutex_unlock(&data->lock);
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/6] iio: imu: kmx61: Add support for data ready triggers
2014-12-03 13:31 ` [PATCH v4 5/6] iio: imu: kmx61: Add support for data ready triggers Daniel Baluta
@ 2014-12-12 13:49 ` Jonathan Cameron
2014-12-14 23:04 ` Hartmut Knaack
1 sibling, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2014-12-12 13:49 UTC (permalink / raw)
To: Daniel Baluta, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel
On 03/12/14 13:31, Daniel Baluta wrote:
> This creates a data ready trigger per IIO device.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Looks good. Applied to the togreg branch of iio.git.
Thanks,
Jonathan
> ---
> drivers/iio/imu/Kconfig | 2 +
> drivers/iio/imu/kmx61.c | 295 +++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 296 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index db4221d..5e610f7 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -28,6 +28,8 @@ config ADIS16480
> config KMX61
> tristate "Kionix KMX61 6-axis accelerometer and magnetometer"
> depends on I2C
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> help
> Say Y here if you want to build a driver for Kionix KMX61 6-axis
> accelerometer and magnetometer.
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index f69ae5a7..0f6816a 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -20,9 +20,14 @@
> #include <linux/pm_runtime.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
>
> #define KMX61_DRV_NAME "kmx61"
> #define KMX61_GPIO_NAME "kmx61_int"
> +#define KMX61_IRQ_NAME "kmx61_event"
>
> #define KMX61_REG_WHO_AM_I 0x00
>
> @@ -55,9 +60,11 @@
> #define KMX61_MAG_ZOUT_L 0x16
> #define KMX61_MAG_ZOUT_H 0x17
>
> +#define KMX61_REG_INL 0x28
> #define KMX61_REG_STBY 0x29
> #define KMX61_REG_CTRL1 0x2A
> #define KMX61_REG_ODCNTL 0x2C
> +#define KMX61_REG_INC1 0x2D
>
> #define KMX61_ACC_STBY_BIT BIT(0)
> #define KMX61_MAG_STBY_BIT BIT(1)
> @@ -67,6 +74,13 @@
>
> #define KMX61_REG_CTRL1_GSEL_MASK 0x03
>
> +#define KMX61_REG_CTRL1_BIT_RES BIT(4)
> +#define KMX61_REG_CTRL1_BIT_DRDYE BIT(5)
> +
> +#define KMX61_REG_INC1_BIT_DRDYM BIT(1)
> +#define KMX61_REG_INC1_BIT_DRDYA BIT(2)
> +#define KMX61_REG_INC1_BIT_IEN BIT(5)
> +
> #define KMX61_ACC_ODR_SHIFT 0
> #define KMX61_MAG_ODR_SHIFT 4
> #define KMX61_ACC_ODR_MASK 0x0F
> @@ -100,9 +114,13 @@ struct kmx61_data {
>
> /* accelerometer specific data */
> struct iio_dev *acc_indio_dev;
> + struct iio_trigger *acc_dready_trig;
> + bool acc_dready_trig_on;
>
> /* magnetometer specific data */
> struct iio_dev *mag_indio_dev;
> + struct iio_trigger *mag_dready_trig;
> + bool mag_dready_trig_on;
> };
>
> enum kmx61_range {
> @@ -466,6 +484,69 @@ static int kmx61_chip_init(struct kmx61_data *data)
> return 0;
> }
>
> +static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
> + bool status, u8 device)
> +{
> + u8 mode;
> + int ret;
> +
> + ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INC1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> + return ret;
> + }
> +
> + if (status) {
> + ret |= KMX61_REG_INC1_BIT_IEN;
> + if (device & KMX61_ACC)
> + ret |= KMX61_REG_INC1_BIT_DRDYA;
> + if (device & KMX61_MAG)
> + ret |= KMX61_REG_INC1_BIT_DRDYM;
> + } else {
> + ret &= ~KMX61_REG_INC1_BIT_IEN;
> + if (device & KMX61_ACC)
> + ret &= ~KMX61_REG_INC1_BIT_DRDYA;
> + if (device & KMX61_MAG)
> + ret &= ~KMX61_REG_INC1_BIT_DRDYM;
> + }
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_INC1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
> + return ret;
> + }
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> + return ret;
> + }
> +
> + if (status)
> + ret |= KMX61_REG_CTRL1_BIT_DRDYE;
> + else
> + ret &= ~KMX61_REG_CTRL1_BIT_DRDYE;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> + return ret;
> + }
> +
> + ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> /**
> * kmx61_set_power_state() - set power state for kmx61 @device
> * @data - kmx61 device private pointer
> @@ -626,11 +707,34 @@ static int kmx61_write_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + if (data->acc_dready_trig != trig)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int kmx61_mag_validate_trigger(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + if (data->mag_dready_trig != trig)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static const struct iio_info kmx61_acc_info = {
> .driver_module = THIS_MODULE,
> .read_raw = kmx61_read_raw,
> .write_raw = kmx61_write_raw,
> .attrs = &kmx61_acc_attribute_group,
> + .validate_trigger = kmx61_acc_validate_trigger,
> };
>
> static const struct iio_info kmx61_mag_info = {
> @@ -638,8 +742,109 @@ static const struct iio_info kmx61_mag_info = {
> .read_raw = kmx61_read_raw,
> .write_raw = kmx61_write_raw,
> .attrs = &kmx61_mag_attribute_group,
> + .validate_trigger = kmx61_mag_validate_trigger,
> +};
> +
> +
> +static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> + bool state)
> +{
> + int ret = 0;
> + u8 device;
> +
> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> + struct kmx61_data *data = iio_priv(indio_dev);
> +
> + mutex_lock(&data->lock);
> +
> + if (data->acc_dready_trig == trig)
> + device = KMX61_ACC;
> + else
> + device = KMX61_MAG;
> +
> + ret = kmx61_set_power_state(data, state, device);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + ret = kmx61_setup_new_data_interrupt(data, state, device);
> + if (ret < 0) {
> + kmx61_set_power_state(data, false, device);
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + if (data->acc_dready_trig == trig)
> + data->acc_dready_trig_on = state;
> + else
> + data->mag_dready_trig_on = state;
> +
> + mutex_unlock(&data->lock);
> +
> + return 0;
> +}
> +
> +static int kmx61_trig_try_reenable(struct iio_trigger *trig)
> +{
> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_inl\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct iio_trigger_ops kmx61_trigger_ops = {
> + .set_trigger_state = kmx61_data_rdy_trigger_set_state,
> + .try_reenable = kmx61_trig_try_reenable,
> + .owner = THIS_MODULE,
> };
>
> +static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
> +{
> + struct kmx61_data *data = private;
> +
> + if (data->acc_dready_trig_on)
> + iio_trigger_poll(data->acc_dready_trig);
> + if (data->mag_dready_trig_on)
> + iio_trigger_poll(data->mag_dready_trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t kmx61_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> + int bit, ret, i = 0;
> + s16 buffer[8];
> +
> + mutex_lock(&data->lock);
> + for_each_set_bit(bit, indio_dev->buffer->scan_mask,
> + indio_dev->masklength) {
> + ret = kmx61_read_measurement(data, KMX61_ACC_XOUT_L, bit);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + goto err;
> + }
> + buffer[i++] = ret;
> + }
> + mutex_unlock(&data->lock);
> +
> + iio_push_to_buffers(indio_dev, buffer);
> +err:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> static const char *kmx61_match_acpi_device(struct device *dev)
> {
> const struct acpi_device_id *id;
> @@ -702,6 +907,32 @@ static struct iio_dev *kmx61_indiodev_setup(struct kmx61_data *data,
> return indio_dev;
> }
>
> +static struct iio_trigger *kmx61_trigger_setup(struct kmx61_data *data,
> + struct iio_dev *indio_dev,
> + const char *tag)
> +{
> + struct iio_trigger *trig;
> + int ret;
> +
> + trig = devm_iio_trigger_alloc(&data->client->dev,
> + "%s-%s-dev%d",
> + indio_dev->name,
> + tag,
> + indio_dev->id);
> + if (!trig)
> + return ERR_PTR(-ENOMEM);
> +
> + trig->dev.parent = &data->client->dev;
> + trig->ops = &kmx61_trigger_ops;
> + iio_trigger_set_drvdata(trig, indio_dev);
> +
> + ret = iio_trigger_register(trig);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return trig;
> +}
> +
> static int kmx61_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -748,10 +979,55 @@ static int kmx61_probe(struct i2c_client *client,
> if (client->irq < 0)
> client->irq = kmx61_gpio_probe(client, data);
>
> + if (client->irq >= 0) {
> + ret = devm_request_threaded_irq(&client->dev, client->irq,
> + kmx61_data_rdy_trig_poll,
> + NULL,
> + IRQF_TRIGGER_RISING,
> + KMX61_IRQ_NAME,
> + data);
> + if (ret)
> + goto err_chip_uninit;
> +
> + data->acc_dready_trig =
> + kmx61_trigger_setup(data, data->acc_indio_dev,
> + "dready");
> + if (IS_ERR(data->acc_dready_trig))
> + return PTR_ERR(data->acc_dready_trig);
> +
> + data->mag_dready_trig =
> + kmx61_trigger_setup(data, data->mag_indio_dev,
> + "dready");
> + if (IS_ERR(data->mag_dready_trig)) {
> + ret = PTR_ERR(data->mag_dready_trig);
> + goto err_trigger_unregister;
> + }
> +
> + ret = iio_triggered_buffer_setup(data->acc_indio_dev,
> + &iio_pollfunc_store_time,
> + kmx61_trigger_handler,
> + NULL);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed to setup acc triggered buffer\n");
> + goto err_trigger_unregister;
> + }
> +
> + ret = iio_triggered_buffer_setup(data->mag_indio_dev,
> + &iio_pollfunc_store_time,
> + kmx61_trigger_handler,
> + NULL);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed to setup mag triggered buffer\n");
> + goto err_trigger_unregister;
> + }
> + }
> +
> ret = iio_device_register(data->acc_indio_dev);
> if (ret < 0) {
> dev_err(&client->dev, "Failed to register acc iio device\n");
> - goto err_chip_uninit;
> + goto err_buffer_cleanup;
> }
>
> ret = iio_device_register(data->mag_indio_dev);
> @@ -774,6 +1050,16 @@ err_iio_unregister_mag:
> iio_device_unregister(data->mag_indio_dev);
> err_iio_unregister_acc:
> iio_device_unregister(data->acc_indio_dev);
> +err_buffer_cleanup:
> + if (client->irq >= 0) {
> + iio_triggered_buffer_cleanup(data->acc_indio_dev);
> + iio_triggered_buffer_cleanup(data->mag_indio_dev);
> + }
> +err_trigger_unregister:
> + if (data->acc_dready_trig)
> + iio_trigger_unregister(data->acc_dready_trig);
> + if (data->mag_dready_trig)
> + iio_trigger_unregister(data->mag_dready_trig);
> err_chip_uninit:
> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> return ret;
> @@ -790,6 +1076,13 @@ static int kmx61_remove(struct i2c_client *client)
> iio_device_unregister(data->acc_indio_dev);
> iio_device_unregister(data->mag_indio_dev);
>
> + if (client->irq >= 0) {
> + iio_triggered_buffer_cleanup(data->acc_indio_dev);
> + iio_triggered_buffer_cleanup(data->mag_indio_dev);
> + iio_trigger_unregister(data->acc_dready_trig);
> + iio_trigger_unregister(data->mag_dready_trig);
> + }
> +
> mutex_lock(&data->lock);
> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> mutex_unlock(&data->lock);
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 5/6] iio: imu: kmx61: Add support for data ready triggers
2014-12-03 13:31 ` [PATCH v4 5/6] iio: imu: kmx61: Add support for data ready triggers Daniel Baluta
2014-12-12 13:49 ` Jonathan Cameron
@ 2014-12-14 23:04 ` Hartmut Knaack
1 sibling, 0 replies; 20+ messages in thread
From: Hartmut Knaack @ 2014-12-14 23:04 UTC (permalink / raw)
To: Daniel Baluta, jic23, pmeerw, srinivas.pandruvada; +Cc: linux-iio, linux-kernel
Daniel Baluta schrieb am 03.12.2014 um 14:31:
> This creates a data ready trigger per IIO device.
>
I found some issues here, and some recommendations. See inline.
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
> drivers/iio/imu/Kconfig | 2 +
> drivers/iio/imu/kmx61.c | 295 +++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 296 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index db4221d..5e610f7 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -28,6 +28,8 @@ config ADIS16480
> config KMX61
> tristate "Kionix KMX61 6-axis accelerometer and magnetometer"
> depends on I2C
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> help
> Say Y here if you want to build a driver for Kionix KMX61 6-axis
> accelerometer and magnetometer.
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index f69ae5a7..0f6816a 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -20,9 +20,14 @@
> #include <linux/pm_runtime.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
>
> #define KMX61_DRV_NAME "kmx61"
> #define KMX61_GPIO_NAME "kmx61_int"
> +#define KMX61_IRQ_NAME "kmx61_event"
>
> #define KMX61_REG_WHO_AM_I 0x00
>
> @@ -55,9 +60,11 @@
> #define KMX61_MAG_ZOUT_L 0x16
> #define KMX61_MAG_ZOUT_H 0x17
>
> +#define KMX61_REG_INL 0x28
> #define KMX61_REG_STBY 0x29
> #define KMX61_REG_CTRL1 0x2A
> #define KMX61_REG_ODCNTL 0x2C
> +#define KMX61_REG_INC1 0x2D
>
> #define KMX61_ACC_STBY_BIT BIT(0)
> #define KMX61_MAG_STBY_BIT BIT(1)
> @@ -67,6 +74,13 @@
>
> #define KMX61_REG_CTRL1_GSEL_MASK 0x03
>
> +#define KMX61_REG_CTRL1_BIT_RES BIT(4)
> +#define KMX61_REG_CTRL1_BIT_DRDYE BIT(5)
> +
> +#define KMX61_REG_INC1_BIT_DRDYM BIT(1)
> +#define KMX61_REG_INC1_BIT_DRDYA BIT(2)
> +#define KMX61_REG_INC1_BIT_IEN BIT(5)
> +
> #define KMX61_ACC_ODR_SHIFT 0
> #define KMX61_MAG_ODR_SHIFT 4
> #define KMX61_ACC_ODR_MASK 0x0F
> @@ -100,9 +114,13 @@ struct kmx61_data {
>
> /* accelerometer specific data */
> struct iio_dev *acc_indio_dev;
> + struct iio_trigger *acc_dready_trig;
> + bool acc_dready_trig_on;
>
> /* magnetometer specific data */
> struct iio_dev *mag_indio_dev;
> + struct iio_trigger *mag_dready_trig;
> + bool mag_dready_trig_on;
> };
>
> enum kmx61_range {
> @@ -466,6 +484,69 @@ static int kmx61_chip_init(struct kmx61_data *data)
> return 0;
> }
>
> +static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
> + bool status, u8 device)
> +{
> + u8 mode;
> + int ret;
> +
> + ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INC1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> + return ret;
> + }
> +
> + if (status) {
> + ret |= KMX61_REG_INC1_BIT_IEN;
> + if (device & KMX61_ACC)
> + ret |= KMX61_REG_INC1_BIT_DRDYA;
> + if (device & KMX61_MAG)
> + ret |= KMX61_REG_INC1_BIT_DRDYM;
> + } else {
> + ret &= ~KMX61_REG_INC1_BIT_IEN;
> + if (device & KMX61_ACC)
> + ret &= ~KMX61_REG_INC1_BIT_DRDYA;
> + if (device & KMX61_MAG)
> + ret &= ~KMX61_REG_INC1_BIT_DRDYM;
> + }
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_INC1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
> + return ret;
> + }
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> + return ret;
> + }
> +
> + if (status)
> + ret |= KMX61_REG_CTRL1_BIT_DRDYE;
> + else
> + ret &= ~KMX61_REG_CTRL1_BIT_DRDYE;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> + return ret;
> + }
> +
> + ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> + if (ret)
> + return ret;
> +
> + return 0;
No need to check ret here, could be simply: return kmx61_set_mode(...);
> +}
> +
> /**
> * kmx61_set_power_state() - set power state for kmx61 @device
> * @data - kmx61 device private pointer
> @@ -626,11 +707,34 @@ static int kmx61_write_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + if (data->acc_dready_trig != trig)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int kmx61_mag_validate_trigger(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + if (data->mag_dready_trig != trig)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static const struct iio_info kmx61_acc_info = {
> .driver_module = THIS_MODULE,
> .read_raw = kmx61_read_raw,
> .write_raw = kmx61_write_raw,
> .attrs = &kmx61_acc_attribute_group,
> + .validate_trigger = kmx61_acc_validate_trigger,
> };
>
> static const struct iio_info kmx61_mag_info = {
> @@ -638,8 +742,109 @@ static const struct iio_info kmx61_mag_info = {
> .read_raw = kmx61_read_raw,
> .write_raw = kmx61_write_raw,
> .attrs = &kmx61_mag_attribute_group,
> + .validate_trigger = kmx61_mag_validate_trigger,
> +};
> +
> +
> +static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> + bool state)
> +{
> + int ret = 0;
No need to initialize ret.
> + u8 device;
> +
> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> + struct kmx61_data *data = iio_priv(indio_dev);
Shouldn't that be using kmx61_get_data()?
> +
> + mutex_lock(&data->lock);
> +
> + if (data->acc_dready_trig == trig)
> + device = KMX61_ACC;
> + else
> + device = KMX61_MAG;
> +
> + ret = kmx61_set_power_state(data, state, device);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + ret = kmx61_setup_new_data_interrupt(data, state, device);
> + if (ret < 0) {
> + kmx61_set_power_state(data, false, device);
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + if (data->acc_dready_trig == trig)
> + data->acc_dready_trig_on = state;
> + else
> + data->mag_dready_trig_on = state;
> +
> + mutex_unlock(&data->lock);
> +
> + return 0;
Could reduce code repetition by placing an error-out label above the
mutex_unlock and then just do: return (ret < 0) ? ret : 0;
(Or even: return ret;)
> +}
> +
> +static int kmx61_trig_try_reenable(struct iio_trigger *trig)
> +{
> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_inl\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct iio_trigger_ops kmx61_trigger_ops = {
> + .set_trigger_state = kmx61_data_rdy_trigger_set_state,
> + .try_reenable = kmx61_trig_try_reenable,
> + .owner = THIS_MODULE,
> };
>
> +static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
> +{
> + struct kmx61_data *data = private;
> +
> + if (data->acc_dready_trig_on)
> + iio_trigger_poll(data->acc_dready_trig);
> + if (data->mag_dready_trig_on)
> + iio_trigger_poll(data->mag_dready_trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t kmx61_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> + int bit, ret, i = 0;
> + s16 buffer[8];
> +
> + mutex_lock(&data->lock);
> + for_each_set_bit(bit, indio_dev->buffer->scan_mask,
> + indio_dev->masklength) {
> + ret = kmx61_read_measurement(data, KMX61_ACC_XOUT_L, bit);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + goto err;
> + }
> + buffer[i++] = ret;
> + }
> + mutex_unlock(&data->lock);
> +
> + iio_push_to_buffers(indio_dev, buffer);
> +err:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> static const char *kmx61_match_acpi_device(struct device *dev)
> {
> const struct acpi_device_id *id;
> @@ -702,6 +907,32 @@ static struct iio_dev *kmx61_indiodev_setup(struct kmx61_data *data,
> return indio_dev;
> }
>
> +static struct iio_trigger *kmx61_trigger_setup(struct kmx61_data *data,
> + struct iio_dev *indio_dev,
> + const char *tag)
> +{
> + struct iio_trigger *trig;
> + int ret;
> +
> + trig = devm_iio_trigger_alloc(&data->client->dev,
> + "%s-%s-dev%d",
> + indio_dev->name,
> + tag,
> + indio_dev->id);
> + if (!trig)
> + return ERR_PTR(-ENOMEM);
> +
> + trig->dev.parent = &data->client->dev;
> + trig->ops = &kmx61_trigger_ops;
> + iio_trigger_set_drvdata(trig, indio_dev);
> +
> + ret = iio_trigger_register(trig);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return trig;
> +}
> +
> static int kmx61_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> @@ -748,10 +979,55 @@ static int kmx61_probe(struct i2c_client *client,
> if (client->irq < 0)
> client->irq = kmx61_gpio_probe(client, data);
>
> + if (client->irq >= 0) {
> + ret = devm_request_threaded_irq(&client->dev, client->irq,
> + kmx61_data_rdy_trig_poll,
> + NULL,
> + IRQF_TRIGGER_RISING,
> + KMX61_IRQ_NAME,
> + data);
> + if (ret)
> + goto err_chip_uninit;
> +
> + data->acc_dready_trig =
> + kmx61_trigger_setup(data, data->acc_indio_dev,
> + "dready");
> + if (IS_ERR(data->acc_dready_trig))
> + return PTR_ERR(data->acc_dready_trig);
Better store error value in ret and goto err_chip_uninit.
> +
> + data->mag_dready_trig =
> + kmx61_trigger_setup(data, data->mag_indio_dev,
> + "dready");
> + if (IS_ERR(data->mag_dready_trig)) {
> + ret = PTR_ERR(data->mag_dready_trig);
> + goto err_trigger_unregister;
> + }
> +
> + ret = iio_triggered_buffer_setup(data->acc_indio_dev,
> + &iio_pollfunc_store_time,
> + kmx61_trigger_handler,
> + NULL);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed to setup acc triggered buffer\n");
> + goto err_trigger_unregister;
> + }
> +
> + ret = iio_triggered_buffer_setup(data->mag_indio_dev,
> + &iio_pollfunc_store_time,
> + kmx61_trigger_handler,
> + NULL);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "Failed to setup mag triggered buffer\n");
> + goto err_trigger_unregister;
Better make sure to call iio_triggered_buffer_cleanup for data->acc_indio_dev?
> + }
> + }
> +
> ret = iio_device_register(data->acc_indio_dev);
> if (ret < 0) {
> dev_err(&client->dev, "Failed to register acc iio device\n");
> - goto err_chip_uninit;
> + goto err_buffer_cleanup;
> }
>
> ret = iio_device_register(data->mag_indio_dev);
> @@ -774,6 +1050,16 @@ err_iio_unregister_mag:
> iio_device_unregister(data->mag_indio_dev);
> err_iio_unregister_acc:
> iio_device_unregister(data->acc_indio_dev);
> +err_buffer_cleanup:
> + if (client->irq >= 0) {
> + iio_triggered_buffer_cleanup(data->acc_indio_dev);
> + iio_triggered_buffer_cleanup(data->mag_indio_dev);
> + }
> +err_trigger_unregister:
> + if (data->acc_dready_trig)
> + iio_trigger_unregister(data->acc_dready_trig);
> + if (data->mag_dready_trig)
> + iio_trigger_unregister(data->mag_dready_trig);
> err_chip_uninit:
> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> return ret;
> @@ -790,6 +1076,13 @@ static int kmx61_remove(struct i2c_client *client)
> iio_device_unregister(data->acc_indio_dev);
> iio_device_unregister(data->mag_indio_dev);
>
> + if (client->irq >= 0) {
> + iio_triggered_buffer_cleanup(data->acc_indio_dev);
> + iio_triggered_buffer_cleanup(data->mag_indio_dev);
> + iio_trigger_unregister(data->acc_dready_trig);
> + iio_trigger_unregister(data->mag_dready_trig);
Minor nitpick: could be in reverse order of initialization (mag before acc).
Although we missed it for iio_device_unregister() :-(
> + }
> +
> mutex_lock(&data->lock);
> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> mutex_unlock(&data->lock);
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 6/6] iio: imu: kmx61: Add support for any motion trigger
2014-12-03 13:31 [PATCH v4 0/6] Add support for KMX61 sensor Daniel Baluta
` (4 preceding siblings ...)
2014-12-03 13:31 ` [PATCH v4 5/6] iio: imu: kmx61: Add support for data ready triggers Daniel Baluta
@ 2014-12-03 13:31 ` Daniel Baluta
2014-12-12 13:54 ` Jonathan Cameron
2014-12-15 23:19 ` Hartmut Knaack
5 siblings, 2 replies; 20+ messages in thread
From: Daniel Baluta @ 2014-12-03 13:31 UTC (permalink / raw)
To: jic23, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel, daniel.baluta
We use WUFE (Wake Up from Sleep Engine) and BTSE (Back to Sleep Engine)
to detect general motion input.
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 433 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 426 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 0f6816a..972424b 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -20,6 +20,7 @@
#include <linux/pm_runtime.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
#include <linux/iio/trigger.h>
#include <linux/iio/buffer.h>
#include <linux/iio/triggered_buffer.h>
@@ -30,6 +31,8 @@
#define KMX61_IRQ_NAME "kmx61_event"
#define KMX61_REG_WHO_AM_I 0x00
+#define KMX61_REG_INS1 0x01
+#define KMX61_REG_INS2 0x02
/*
* three 16-bit accelerometer output registers for X/Y/Z axis
@@ -63,20 +66,36 @@
#define KMX61_REG_INL 0x28
#define KMX61_REG_STBY 0x29
#define KMX61_REG_CTRL1 0x2A
+#define KMX61_REG_CTRL2 0x2B
#define KMX61_REG_ODCNTL 0x2C
#define KMX61_REG_INC1 0x2D
+#define KMX61_REG_WUF_THRESH 0x3D
+#define KMX61_REG_WUF_TIMER 0x3E
+
#define KMX61_ACC_STBY_BIT BIT(0)
#define KMX61_MAG_STBY_BIT BIT(1)
#define KMX61_ACT_STBY_BIT BIT(7)
#define KMX61_ALL_STBY (KMX61_ACC_STBY_BIT | KMX61_MAG_STBY_BIT)
+#define KMX61_REG_INS1_BIT_WUFS BIT(1)
+
+#define KMX61_REG_INS2_BIT_ZP BIT(0)
+#define KMX61_REG_INS2_BIT_ZN BIT(1)
+#define KMX61_REG_INS2_BIT_YP BIT(2)
+#define KMX61_REG_INS2_BIT_YN BIT(3)
+#define KMX61_REG_INS2_BIT_XP BIT(4)
+#define KMX61_REG_INS2_BIT_XN BIT(5)
+
#define KMX61_REG_CTRL1_GSEL_MASK 0x03
#define KMX61_REG_CTRL1_BIT_RES BIT(4)
#define KMX61_REG_CTRL1_BIT_DRDYE BIT(5)
+#define KMX61_REG_CTRL1_BIT_WUFE BIT(6)
+#define KMX61_REG_CTRL1_BIT_BTSE BIT(7)
+#define KMX61_REG_INC1_BIT_WUFS BIT(0)
#define KMX61_REG_INC1_BIT_DRDYM BIT(1)
#define KMX61_REG_INC1_BIT_DRDYA BIT(2)
#define KMX61_REG_INC1_BIT_IEN BIT(5)
@@ -86,6 +105,11 @@
#define KMX61_ACC_ODR_MASK 0x0F
#define KMX61_MAG_ODR_MASK 0xF0
+#define KMX61_OWUF_MASK 0x7
+
+#define KMX61_DEFAULT_WAKE_THRESH 1
+#define KMX61_DEFAULT_WAKE_DURATION 1
+
#define KMX61_SLEEP_DELAY_MS 2000
#define KMX61_CHIP_ID 0x12
@@ -111,11 +135,16 @@ struct kmx61_data {
/* config bits */
u8 range;
u8 odr_bits;
+ u8 wake_thresh;
+ u8 wake_duration;
/* accelerometer specific data */
struct iio_dev *acc_indio_dev;
struct iio_trigger *acc_dready_trig;
+ struct iio_trigger *motion_trig;
bool acc_dready_trig_on;
+ bool motion_trig_on;
+ bool ev_enable_state;
/* magnetometer specific data */
struct iio_dev *mag_indio_dev;
@@ -154,6 +183,23 @@ static const struct {
{3, 125000, 0x0A},
{6, 250000, 0x0B} };
+static const struct {
+ int val;
+ int val2;
+ int odr_bits;
+} kmx61_wake_up_odr_table[] = { {0, 781000, 0x00},
+ {1, 563000, 0x01},
+ {3, 125000, 0x02},
+ {6, 250000, 0x03},
+ {12, 500000, 0x04},
+ {25, 0, 0x05},
+ {50, 0, 0x06},
+ {100, 0, 0x06},
+ {200, 0, 0x06},
+ {400, 0, 0x06},
+ {800, 0, 0x06},
+ {1600, 0, 0x06} };
+
static IIO_CONST_ATTR(accel_scale_available, "0.009582 0.019163 0.038326");
static IIO_CONST_ATTR(magn_scale_available, "0.001465");
static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
@@ -179,6 +225,14 @@ static const struct attribute_group kmx61_mag_attribute_group = {
.attrs = kmx61_mag_attributes,
};
+static const struct iio_event_spec kmx61_event = {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_EITHER,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_ENABLE) |
+ BIT(IIO_EV_INFO_PERIOD),
+};
+
#define KMX61_ACC_CHAN(_axis) { \
.type = IIO_ACCEL, \
.modified = 1, \
@@ -195,6 +249,8 @@ static const struct attribute_group kmx61_mag_attribute_group = {
.shift = 4, \
.endianness = IIO_LE, \
}, \
+ .event_spec = &kmx61_event, \
+ .num_event_specs = 1 \
}
#define KMX61_MAG_CHAN(_axis) { \
@@ -250,6 +306,31 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
return -EINVAL;
}
+static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
+ if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
+ *val = kmx61_samp_freq_table[i].val;
+ *val2 = kmx61_samp_freq_table[i].val2;
+ return 0;
+ }
+ return -EINVAL;
+}
+
+
+static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(kmx61_wake_up_odr_table); ++i)
+ if (kmx61_wake_up_odr_table[i].val == val &&
+ kmx61_wake_up_odr_table[i].val2 == val2)
+ return kmx61_wake_up_odr_table[i].odr_bits;
+ return -EINVAL;
+}
+
/**
* kmx61_set_mode() - set KMX61 device operating mode
* @data - kmx61 device private data pointer
@@ -338,6 +419,21 @@ static int kmx61_get_mode(struct kmx61_data *data, u8 *mode, u8 device)
return 0;
}
+int kmx61_set_wake_up_odr(struct kmx61_data *data, int val, int val2)
+{
+ int ret, odr_bits;
+
+ odr_bits = kmx61_convert_wake_up_odr_to_bit(val, val2);
+ if (odr_bits < 0)
+ return odr_bits;
+
+ ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL2,
+ odr_bits);
+ if (ret < 0)
+ dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
+ return ret;
+}
+
static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
{
int ret;
@@ -369,6 +465,12 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
if (ret < 0)
return ret;
+ if (device & KMX61_ACC) {
+ ret = kmx61_set_wake_up_odr(data, val, val2);
+ if (ret)
+ return ret;
+ }
+
return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
}
@@ -449,7 +551,7 @@ static int kmx61_set_scale(struct kmx61_data *data, u16 uscale)
static int kmx61_chip_init(struct kmx61_data *data)
{
- int ret;
+ int ret, val, val2;
ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_WHO_AM_I);
if (ret < 0) {
@@ -476,11 +578,23 @@ static int kmx61_chip_init(struct kmx61_data *data)
}
data->odr_bits = ret;
+ /* set output data rate for wake up (motion detection) function */
+ ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
+ if (ret < 0)
+ return ret;
+
+ ret = kmx61_set_wake_up_odr(data, val, val2);
+ if (ret < 0)
+ return ret;
+
/* set acc/magn to OPERATION mode */
ret = kmx61_set_mode(data, 0, KMX61_ACC | KMX61_MAG, true);
if (ret < 0)
return ret;
+ data->wake_thresh = KMX61_DEFAULT_WAKE_THRESH;
+ data->wake_duration = KMX61_DEFAULT_WAKE_DURATION;
+
return 0;
}
@@ -547,6 +661,87 @@ static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
return 0;
}
+static int kmx61_chip_update_thresholds(struct kmx61_data *data)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ KMX61_REG_WUF_TIMER,
+ data->wake_duration);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Errow writing reg_wuf_timer\n");
+ return ret;
+ }
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ KMX61_REG_WUF_THRESH,
+ data->wake_thresh);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error writing reg_wuf_thresh\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
+ bool status, u8 device)
+{
+ u8 mode;
+ int ret;
+
+ ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
+ if (ret < 0)
+ return ret;
+
+ ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
+ if (ret < 0)
+ return ret;
+
+ ret = kmx61_chip_update_thresholds(data);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INC1);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_inc1\n");
+ return ret;
+ }
+ if (status)
+ ret |= (KMX61_REG_INC1_BIT_IEN | KMX61_REG_INC1_BIT_WUFS);
+ else
+ ret &= ~(KMX61_REG_INC1_BIT_IEN | KMX61_REG_INC1_BIT_WUFS);
+
+ ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_INC1, ret);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error writing reg_inc1\n");
+ return ret;
+ }
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
+ return ret;
+ }
+
+ if (status)
+ ret |= KMX61_REG_CTRL1_BIT_WUFE | KMX61_REG_CTRL1_BIT_BTSE;
+ else
+ ret &= ~(KMX61_REG_CTRL1_BIT_WUFE | KMX61_REG_CTRL1_BIT_BTSE);
+
+ ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
+ return ret;
+ }
+ mode |= KMX61_ACT_STBY_BIT;
+ ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
/**
* kmx61_set_power_state() - set power state for kmx61 @device
* @data - kmx61 device private pointer
@@ -707,12 +902,111 @@ static int kmx61_write_raw(struct iio_dev *indio_dev,
}
}
+static int kmx61_read_event(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int *val, int *val2)
+{
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+
+ *val2 = 0;
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ *val = data->wake_thresh;
+ break;
+ case IIO_EV_INFO_PERIOD:
+ *val = data->wake_duration;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return IIO_VAL_INT;
+}
+
+static int kmx61_write_event(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int val, int val2)
+{
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+
+ if (data->ev_enable_state)
+ return -EBUSY;
+
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ data->wake_thresh = val;
+ break;
+ case IIO_EV_INFO_PERIOD:
+ data->wake_duration = val;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return IIO_VAL_INT;
+}
+
+static int kmx61_read_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir)
+{
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+
+ return data->ev_enable_state;
+}
+
+static int kmx61_write_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ int state)
+{
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
+ int ret;
+
+ if (state && data->ev_enable_state)
+ return 0;
+
+ mutex_lock(&data->lock);
+
+ if (!state && data->motion_trig_on) {
+ data->ev_enable_state = 0;
+ mutex_unlock(&data->lock);
+ return 0;
+ }
+
+ ret = kmx61_set_power_state(data, state, KMX61_ACC);
+ if (ret < 0) {
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
+ if (ret < 0) {
+ kmx61_set_power_state(data, false, KMX61_ACC);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ data->ev_enable_state = state;
+ mutex_unlock(&data->lock);
+
+ return 0;
+}
+
static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
struct iio_trigger *trig)
{
struct kmx61_data *data = kmx61_get_data(indio_dev);
- if (data->acc_dready_trig != trig)
+ if (data->acc_dready_trig != trig && data->motion_trig != trig)
return -EINVAL;
return 0;
@@ -734,6 +1028,10 @@ static const struct iio_info kmx61_acc_info = {
.read_raw = kmx61_read_raw,
.write_raw = kmx61_write_raw,
.attrs = &kmx61_acc_attribute_group,
+ .read_event_value = kmx61_read_event,
+ .write_event_value = kmx61_write_event,
+ .read_event_config = kmx61_read_event_config,
+ .write_event_config = kmx61_write_event_config,
.validate_trigger = kmx61_acc_validate_trigger,
};
@@ -753,11 +1051,18 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
u8 device;
struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
- struct kmx61_data *data = iio_priv(indio_dev);
+ struct kmx61_data *data = kmx61_get_data(indio_dev);
mutex_lock(&data->lock);
- if (data->acc_dready_trig == trig)
+ if (!state && data->ev_enable_state && data->motion_trig_on) {
+ data->motion_trig_on = false;
+ mutex_unlock(&data->lock);
+ return 0;
+ }
+
+
+ if (data->acc_dready_trig == trig || data->motion_trig)
device = KMX61_ACC;
else
device = KMX61_MAG;
@@ -768,7 +1073,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
return ret;
}
- ret = kmx61_setup_new_data_interrupt(data, state, device);
+ if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
+ ret = kmx61_setup_new_data_interrupt(data, state, device);
+ else
+ ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
if (ret < 0) {
kmx61_set_power_state(data, false, device);
mutex_unlock(&data->lock);
@@ -777,8 +1085,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
if (data->acc_dready_trig == trig)
data->acc_dready_trig_on = state;
- else
+ else if (data->mag_dready_trig == trig)
data->mag_dready_trig_on = state;
+ else
+ data->motion_trig_on = state;
mutex_unlock(&data->lock);
@@ -806,6 +1116,99 @@ static const struct iio_trigger_ops kmx61_trigger_ops = {
.owner = THIS_MODULE,
};
+static irqreturn_t kmx61_event_handler(int irq, void *private)
+{
+ struct kmx61_data *data = private;
+ struct iio_dev *indio_dev = data->acc_indio_dev;
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_ins1\n");
+ goto ack_intr;
+ }
+
+ if (ret & KMX61_REG_INS1_BIT_WUFS) {
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS2);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "Error reading reg_ins2\n");
+ goto ack_intr;
+ }
+
+ if (ret & KMX61_REG_INS2_BIT_XN)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL,
+ 0,
+ IIO_MOD_X,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_FALLING),
+ 0);
+
+ if (ret & KMX61_REG_INS2_BIT_XP)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL,
+ 0,
+ IIO_MOD_X,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING),
+ 0);
+
+ if (ret & KMX61_REG_INS2_BIT_YN)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL,
+ 0,
+ IIO_MOD_Y,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_FALLING),
+ 0);
+
+ if (ret & KMX61_REG_INS2_BIT_YP)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL,
+ 0,
+ IIO_MOD_Y,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING),
+ 0);
+
+ if (ret & KMX61_REG_INS2_BIT_ZN)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL,
+ 0,
+ IIO_MOD_Z,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_FALLING),
+ 0);
+
+ if (ret & KMX61_REG_INS2_BIT_ZP)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL,
+ 0,
+ IIO_MOD_Z,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING),
+ 0);
+ }
+
+ack_intr:
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
+ if (ret < 0)
+ dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
+
+ ret |= KMX61_REG_CTRL1_BIT_RES;
+ ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
+ if (ret < 0)
+ dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
+ if (ret < 0)
+ dev_err(&data->client->dev, "Error reading reg_inl\n");
+
+ ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
+
+ return IRQ_HANDLED;
+}
+
static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
{
struct kmx61_data *data = private;
@@ -815,6 +1218,11 @@ static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
if (data->mag_dready_trig_on)
iio_trigger_poll(data->mag_dready_trig);
+ if (data->motion_trig_on)
+ iio_trigger_poll(data->motion_trig);
+
+ if (data->ev_enable_state)
+ return IRQ_WAKE_THREAD;
return IRQ_HANDLED;
}
@@ -982,7 +1390,7 @@ static int kmx61_probe(struct i2c_client *client,
if (client->irq >= 0) {
ret = devm_request_threaded_irq(&client->dev, client->irq,
kmx61_data_rdy_trig_poll,
- NULL,
+ kmx61_event_handler,
IRQF_TRIGGER_RISING,
KMX61_IRQ_NAME,
data);
@@ -1003,6 +1411,14 @@ static int kmx61_probe(struct i2c_client *client,
goto err_trigger_unregister;
}
+ data->motion_trig =
+ kmx61_trigger_setup(data, data->acc_indio_dev,
+ "any-motion");
+ if (IS_ERR(data->motion_trig)) {
+ ret = PTR_ERR(data->motion_trig);
+ goto err_trigger_unregister;
+ }
+
ret = iio_triggered_buffer_setup(data->acc_indio_dev,
&iio_pollfunc_store_time,
kmx61_trigger_handler,
@@ -1060,6 +1476,8 @@ err_trigger_unregister:
iio_trigger_unregister(data->acc_dready_trig);
if (data->mag_dready_trig)
iio_trigger_unregister(data->mag_dready_trig);
+ if (data->motion_trig)
+ iio_trigger_unregister(data->motion_trig);
err_chip_uninit:
kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
return ret;
@@ -1081,6 +1499,7 @@ static int kmx61_remove(struct i2c_client *client)
iio_triggered_buffer_cleanup(data->mag_indio_dev);
iio_trigger_unregister(data->acc_dready_trig);
iio_trigger_unregister(data->mag_dready_trig);
+ iio_trigger_unregister(data->motion_trig);
}
mutex_lock(&data->lock);
--
1.9.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 6/6] iio: imu: kmx61: Add support for any motion trigger
2014-12-03 13:31 ` [PATCH v4 6/6] iio: imu: kmx61: Add support for any motion trigger Daniel Baluta
@ 2014-12-12 13:54 ` Jonathan Cameron
2014-12-15 23:19 ` Hartmut Knaack
1 sibling, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2014-12-12 13:54 UTC (permalink / raw)
To: Daniel Baluta, pmeerw, srinivas.pandruvada, knaack.h
Cc: linux-iio, linux-kernel
On 03/12/14 13:31, Daniel Baluta wrote:
> We use WUFE (Wake Up from Sleep Engine) and BTSE (Back to Sleep Engine)
> to detect general motion input.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Interesting patch. Looks good. Applied to the togreg branch of iio.git
Thanks,
A nice looking set.
Jonathan
> ---
> drivers/iio/imu/kmx61.c | 433 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 426 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 0f6816a..972424b 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -20,6 +20,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
> #include <linux/iio/trigger.h>
> #include <linux/iio/buffer.h>
> #include <linux/iio/triggered_buffer.h>
> @@ -30,6 +31,8 @@
> #define KMX61_IRQ_NAME "kmx61_event"
>
> #define KMX61_REG_WHO_AM_I 0x00
> +#define KMX61_REG_INS1 0x01
> +#define KMX61_REG_INS2 0x02
>
> /*
> * three 16-bit accelerometer output registers for X/Y/Z axis
> @@ -63,20 +66,36 @@
> #define KMX61_REG_INL 0x28
> #define KMX61_REG_STBY 0x29
> #define KMX61_REG_CTRL1 0x2A
> +#define KMX61_REG_CTRL2 0x2B
> #define KMX61_REG_ODCNTL 0x2C
> #define KMX61_REG_INC1 0x2D
>
> +#define KMX61_REG_WUF_THRESH 0x3D
> +#define KMX61_REG_WUF_TIMER 0x3E
> +
> #define KMX61_ACC_STBY_BIT BIT(0)
> #define KMX61_MAG_STBY_BIT BIT(1)
> #define KMX61_ACT_STBY_BIT BIT(7)
>
> #define KMX61_ALL_STBY (KMX61_ACC_STBY_BIT | KMX61_MAG_STBY_BIT)
>
> +#define KMX61_REG_INS1_BIT_WUFS BIT(1)
> +
> +#define KMX61_REG_INS2_BIT_ZP BIT(0)
> +#define KMX61_REG_INS2_BIT_ZN BIT(1)
> +#define KMX61_REG_INS2_BIT_YP BIT(2)
> +#define KMX61_REG_INS2_BIT_YN BIT(3)
> +#define KMX61_REG_INS2_BIT_XP BIT(4)
> +#define KMX61_REG_INS2_BIT_XN BIT(5)
> +
> #define KMX61_REG_CTRL1_GSEL_MASK 0x03
>
> #define KMX61_REG_CTRL1_BIT_RES BIT(4)
> #define KMX61_REG_CTRL1_BIT_DRDYE BIT(5)
> +#define KMX61_REG_CTRL1_BIT_WUFE BIT(6)
> +#define KMX61_REG_CTRL1_BIT_BTSE BIT(7)
>
> +#define KMX61_REG_INC1_BIT_WUFS BIT(0)
> #define KMX61_REG_INC1_BIT_DRDYM BIT(1)
> #define KMX61_REG_INC1_BIT_DRDYA BIT(2)
> #define KMX61_REG_INC1_BIT_IEN BIT(5)
> @@ -86,6 +105,11 @@
> #define KMX61_ACC_ODR_MASK 0x0F
> #define KMX61_MAG_ODR_MASK 0xF0
>
> +#define KMX61_OWUF_MASK 0x7
> +
> +#define KMX61_DEFAULT_WAKE_THRESH 1
> +#define KMX61_DEFAULT_WAKE_DURATION 1
> +
> #define KMX61_SLEEP_DELAY_MS 2000
>
> #define KMX61_CHIP_ID 0x12
> @@ -111,11 +135,16 @@ struct kmx61_data {
> /* config bits */
> u8 range;
> u8 odr_bits;
> + u8 wake_thresh;
> + u8 wake_duration;
>
> /* accelerometer specific data */
> struct iio_dev *acc_indio_dev;
> struct iio_trigger *acc_dready_trig;
> + struct iio_trigger *motion_trig;
> bool acc_dready_trig_on;
> + bool motion_trig_on;
> + bool ev_enable_state;
>
> /* magnetometer specific data */
> struct iio_dev *mag_indio_dev;
> @@ -154,6 +183,23 @@ static const struct {
> {3, 125000, 0x0A},
> {6, 250000, 0x0B} };
>
> +static const struct {
> + int val;
> + int val2;
> + int odr_bits;
> +} kmx61_wake_up_odr_table[] = { {0, 781000, 0x00},
> + {1, 563000, 0x01},
> + {3, 125000, 0x02},
> + {6, 250000, 0x03},
> + {12, 500000, 0x04},
> + {25, 0, 0x05},
> + {50, 0, 0x06},
> + {100, 0, 0x06},
> + {200, 0, 0x06},
> + {400, 0, 0x06},
> + {800, 0, 0x06},
> + {1600, 0, 0x06} };
> +
> static IIO_CONST_ATTR(accel_scale_available, "0.009582 0.019163 0.038326");
> static IIO_CONST_ATTR(magn_scale_available, "0.001465");
> static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> @@ -179,6 +225,14 @@ static const struct attribute_group kmx61_mag_attribute_group = {
> .attrs = kmx61_mag_attributes,
> };
>
> +static const struct iio_event_spec kmx61_event = {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_EITHER,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE) |
> + BIT(IIO_EV_INFO_PERIOD),
> +};
> +
> #define KMX61_ACC_CHAN(_axis) { \
> .type = IIO_ACCEL, \
> .modified = 1, \
> @@ -195,6 +249,8 @@ static const struct attribute_group kmx61_mag_attribute_group = {
> .shift = 4, \
> .endianness = IIO_LE, \
> }, \
> + .event_spec = &kmx61_event, \
> + .num_event_specs = 1 \
> }
>
> #define KMX61_MAG_CHAN(_axis) { \
> @@ -250,6 +306,31 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
> return -EINVAL;
> }
>
> +static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> + if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
> + *val = kmx61_samp_freq_table[i].val;
> + *val2 = kmx61_samp_freq_table[i].val2;
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> +
> +static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_wake_up_odr_table); ++i)
> + if (kmx61_wake_up_odr_table[i].val == val &&
> + kmx61_wake_up_odr_table[i].val2 == val2)
> + return kmx61_wake_up_odr_table[i].odr_bits;
> + return -EINVAL;
> +}
> +
> /**
> * kmx61_set_mode() - set KMX61 device operating mode
> * @data - kmx61 device private data pointer
> @@ -338,6 +419,21 @@ static int kmx61_get_mode(struct kmx61_data *data, u8 *mode, u8 device)
> return 0;
> }
>
> +int kmx61_set_wake_up_odr(struct kmx61_data *data, int val, int val2)
> +{
> + int ret, odr_bits;
> +
> + odr_bits = kmx61_convert_wake_up_odr_to_bit(val, val2);
> + if (odr_bits < 0)
> + return odr_bits;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL2,
> + odr_bits);
> + if (ret < 0)
> + dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
> + return ret;
> +}
> +
> static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
> {
> int ret;
> @@ -369,6 +465,12 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
> if (ret < 0)
> return ret;
>
> + if (device & KMX61_ACC) {
> + ret = kmx61_set_wake_up_odr(data, val, val2);
> + if (ret)
> + return ret;
> + }
> +
> return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> }
>
> @@ -449,7 +551,7 @@ static int kmx61_set_scale(struct kmx61_data *data, u16 uscale)
>
> static int kmx61_chip_init(struct kmx61_data *data)
> {
> - int ret;
> + int ret, val, val2;
>
> ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_WHO_AM_I);
> if (ret < 0) {
> @@ -476,11 +578,23 @@ static int kmx61_chip_init(struct kmx61_data *data)
> }
> data->odr_bits = ret;
>
> + /* set output data rate for wake up (motion detection) function */
> + ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_wake_up_odr(data, val, val2);
> + if (ret < 0)
> + return ret;
> +
> /* set acc/magn to OPERATION mode */
> ret = kmx61_set_mode(data, 0, KMX61_ACC | KMX61_MAG, true);
> if (ret < 0)
> return ret;
>
> + data->wake_thresh = KMX61_DEFAULT_WAKE_THRESH;
> + data->wake_duration = KMX61_DEFAULT_WAKE_DURATION;
> +
> return 0;
> }
>
> @@ -547,6 +661,87 @@ static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
> return 0;
> }
>
> +static int kmx61_chip_update_thresholds(struct kmx61_data *data)
> +{
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(data->client,
> + KMX61_REG_WUF_TIMER,
> + data->wake_duration);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Errow writing reg_wuf_timer\n");
> + return ret;
> + }
> +
> + ret = i2c_smbus_write_byte_data(data->client,
> + KMX61_REG_WUF_THRESH,
> + data->wake_thresh);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_wuf_thresh\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
> + bool status, u8 device)
> +{
> + u8 mode;
> + int ret;
> +
> + ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_chip_update_thresholds(data);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INC1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_inc1\n");
> + return ret;
> + }
> + if (status)
> + ret |= (KMX61_REG_INC1_BIT_IEN | KMX61_REG_INC1_BIT_WUFS);
> + else
> + ret &= ~(KMX61_REG_INC1_BIT_IEN | KMX61_REG_INC1_BIT_WUFS);
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_INC1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_inc1\n");
> + return ret;
> + }
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> + return ret;
> + }
> +
> + if (status)
> + ret |= KMX61_REG_CTRL1_BIT_WUFE | KMX61_REG_CTRL1_BIT_BTSE;
> + else
> + ret &= ~(KMX61_REG_CTRL1_BIT_WUFE | KMX61_REG_CTRL1_BIT_BTSE);
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> + return ret;
> + }
> + mode |= KMX61_ACT_STBY_BIT;
> + ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> /**
> * kmx61_set_power_state() - set power state for kmx61 @device
> * @data - kmx61 device private pointer
> @@ -707,12 +902,111 @@ static int kmx61_write_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int kmx61_read_event(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int *val, int *val2)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + *val2 = 0;
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + *val = data->wake_thresh;
> + break;
> + case IIO_EV_INFO_PERIOD:
> + *val = data->wake_duration;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int kmx61_write_event(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int val, int val2)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + if (data->ev_enable_state)
> + return -EBUSY;
> +
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + data->wake_thresh = val;
> + break;
> + case IIO_EV_INFO_PERIOD:
> + data->wake_duration = val;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int kmx61_read_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + return data->ev_enable_state;
> +}
> +
> +static int kmx61_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + int state)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> + int ret;
> +
> + if (state && data->ev_enable_state)
> + return 0;
> +
> + mutex_lock(&data->lock);
> +
> + if (!state && data->motion_trig_on) {
> + data->ev_enable_state = 0;
> + mutex_unlock(&data->lock);
> + return 0;
> + }
> +
> + ret = kmx61_set_power_state(data, state, KMX61_ACC);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
> + if (ret < 0) {
> + kmx61_set_power_state(data, false, KMX61_ACC);
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + data->ev_enable_state = state;
> + mutex_unlock(&data->lock);
> +
> + return 0;
> +}
> +
> static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
> struct iio_trigger *trig)
> {
> struct kmx61_data *data = kmx61_get_data(indio_dev);
>
> - if (data->acc_dready_trig != trig)
> + if (data->acc_dready_trig != trig && data->motion_trig != trig)
> return -EINVAL;
>
> return 0;
> @@ -734,6 +1028,10 @@ static const struct iio_info kmx61_acc_info = {
> .read_raw = kmx61_read_raw,
> .write_raw = kmx61_write_raw,
> .attrs = &kmx61_acc_attribute_group,
> + .read_event_value = kmx61_read_event,
> + .write_event_value = kmx61_write_event,
> + .read_event_config = kmx61_read_event_config,
> + .write_event_config = kmx61_write_event_config,
> .validate_trigger = kmx61_acc_validate_trigger,
> };
>
> @@ -753,11 +1051,18 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> u8 device;
>
> struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> - struct kmx61_data *data = iio_priv(indio_dev);
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
>
> mutex_lock(&data->lock);
>
> - if (data->acc_dready_trig == trig)
> + if (!state && data->ev_enable_state && data->motion_trig_on) {
> + data->motion_trig_on = false;
> + mutex_unlock(&data->lock);
> + return 0;
> + }
> +
> +
> + if (data->acc_dready_trig == trig || data->motion_trig)
> device = KMX61_ACC;
> else
> device = KMX61_MAG;
> @@ -768,7 +1073,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> return ret;
> }
>
> - ret = kmx61_setup_new_data_interrupt(data, state, device);
> + if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
> + ret = kmx61_setup_new_data_interrupt(data, state, device);
> + else
> + ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
> if (ret < 0) {
> kmx61_set_power_state(data, false, device);
> mutex_unlock(&data->lock);
> @@ -777,8 +1085,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>
> if (data->acc_dready_trig == trig)
> data->acc_dready_trig_on = state;
> - else
> + else if (data->mag_dready_trig == trig)
> data->mag_dready_trig_on = state;
> + else
> + data->motion_trig_on = state;
>
> mutex_unlock(&data->lock);
>
> @@ -806,6 +1116,99 @@ static const struct iio_trigger_ops kmx61_trigger_ops = {
> .owner = THIS_MODULE,
> };
>
> +static irqreturn_t kmx61_event_handler(int irq, void *private)
> +{
> + struct kmx61_data *data = private;
> + struct iio_dev *indio_dev = data->acc_indio_dev;
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ins1\n");
> + goto ack_intr;
> + }
> +
> + if (ret & KMX61_REG_INS1_BIT_WUFS) {
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS2);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ins2\n");
> + goto ack_intr;
> + }
> +
> + if (ret & KMX61_REG_INS2_BIT_XN)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_X,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_FALLING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_XP)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_X,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_RISING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_YN)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_Y,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_FALLING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_YP)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_Y,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_RISING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_ZN)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_Z,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_FALLING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_ZP)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_Z,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_RISING),
> + 0);
> + }
> +
> +ack_intr:
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
> + if (ret < 0)
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> +
> + ret |= KMX61_REG_CTRL1_BIT_RES;
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
> + if (ret < 0)
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
> + if (ret < 0)
> + dev_err(&data->client->dev, "Error reading reg_inl\n");
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
> +
> + return IRQ_HANDLED;
> +}
> +
> static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
> {
> struct kmx61_data *data = private;
> @@ -815,6 +1218,11 @@ static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
> if (data->mag_dready_trig_on)
> iio_trigger_poll(data->mag_dready_trig);
>
> + if (data->motion_trig_on)
> + iio_trigger_poll(data->motion_trig);
> +
> + if (data->ev_enable_state)
> + return IRQ_WAKE_THREAD;
> return IRQ_HANDLED;
> }
>
> @@ -982,7 +1390,7 @@ static int kmx61_probe(struct i2c_client *client,
> if (client->irq >= 0) {
> ret = devm_request_threaded_irq(&client->dev, client->irq,
> kmx61_data_rdy_trig_poll,
> - NULL,
> + kmx61_event_handler,
> IRQF_TRIGGER_RISING,
> KMX61_IRQ_NAME,
> data);
> @@ -1003,6 +1411,14 @@ static int kmx61_probe(struct i2c_client *client,
> goto err_trigger_unregister;
> }
>
> + data->motion_trig =
> + kmx61_trigger_setup(data, data->acc_indio_dev,
> + "any-motion");
> + if (IS_ERR(data->motion_trig)) {
> + ret = PTR_ERR(data->motion_trig);
> + goto err_trigger_unregister;
> + }
> +
> ret = iio_triggered_buffer_setup(data->acc_indio_dev,
> &iio_pollfunc_store_time,
> kmx61_trigger_handler,
> @@ -1060,6 +1476,8 @@ err_trigger_unregister:
> iio_trigger_unregister(data->acc_dready_trig);
> if (data->mag_dready_trig)
> iio_trigger_unregister(data->mag_dready_trig);
> + if (data->motion_trig)
> + iio_trigger_unregister(data->motion_trig);
> err_chip_uninit:
> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> return ret;
> @@ -1081,6 +1499,7 @@ static int kmx61_remove(struct i2c_client *client)
> iio_triggered_buffer_cleanup(data->mag_indio_dev);
> iio_trigger_unregister(data->acc_dready_trig);
> iio_trigger_unregister(data->mag_dready_trig);
> + iio_trigger_unregister(data->motion_trig);
> }
>
> mutex_lock(&data->lock);
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 6/6] iio: imu: kmx61: Add support for any motion trigger
2014-12-03 13:31 ` [PATCH v4 6/6] iio: imu: kmx61: Add support for any motion trigger Daniel Baluta
2014-12-12 13:54 ` Jonathan Cameron
@ 2014-12-15 23:19 ` Hartmut Knaack
2014-12-16 11:08 ` Daniel Baluta
1 sibling, 1 reply; 20+ messages in thread
From: Hartmut Knaack @ 2014-12-15 23:19 UTC (permalink / raw)
To: Daniel Baluta, jic23, pmeerw, srinivas.pandruvada; +Cc: linux-iio, linux-kernel
Daniel Baluta schrieb am 03.12.2014 um 14:31:
> We use WUFE (Wake Up from Sleep Engine) and BTSE (Back to Sleep Engine)
> to detect general motion input.
>
A few recommendations, issues and questions inline.
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
> drivers/iio/imu/kmx61.c | 433 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 426 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 0f6816a..972424b 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -20,6 +20,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
> #include <linux/iio/trigger.h>
> #include <linux/iio/buffer.h>
> #include <linux/iio/triggered_buffer.h>
> @@ -30,6 +31,8 @@
> #define KMX61_IRQ_NAME "kmx61_event"
>
> #define KMX61_REG_WHO_AM_I 0x00
> +#define KMX61_REG_INS1 0x01
> +#define KMX61_REG_INS2 0x02
>
> /*
> * three 16-bit accelerometer output registers for X/Y/Z axis
> @@ -63,20 +66,36 @@
> #define KMX61_REG_INL 0x28
> #define KMX61_REG_STBY 0x29
> #define KMX61_REG_CTRL1 0x2A
> +#define KMX61_REG_CTRL2 0x2B
> #define KMX61_REG_ODCNTL 0x2C
> #define KMX61_REG_INC1 0x2D
>
> +#define KMX61_REG_WUF_THRESH 0x3D
> +#define KMX61_REG_WUF_TIMER 0x3E
> +
> #define KMX61_ACC_STBY_BIT BIT(0)
> #define KMX61_MAG_STBY_BIT BIT(1)
> #define KMX61_ACT_STBY_BIT BIT(7)
>
> #define KMX61_ALL_STBY (KMX61_ACC_STBY_BIT | KMX61_MAG_STBY_BIT)
>
> +#define KMX61_REG_INS1_BIT_WUFS BIT(1)
> +
> +#define KMX61_REG_INS2_BIT_ZP BIT(0)
> +#define KMX61_REG_INS2_BIT_ZN BIT(1)
> +#define KMX61_REG_INS2_BIT_YP BIT(2)
> +#define KMX61_REG_INS2_BIT_YN BIT(3)
> +#define KMX61_REG_INS2_BIT_XP BIT(4)
> +#define KMX61_REG_INS2_BIT_XN BIT(5)
> +
> #define KMX61_REG_CTRL1_GSEL_MASK 0x03
>
> #define KMX61_REG_CTRL1_BIT_RES BIT(4)
> #define KMX61_REG_CTRL1_BIT_DRDYE BIT(5)
> +#define KMX61_REG_CTRL1_BIT_WUFE BIT(6)
> +#define KMX61_REG_CTRL1_BIT_BTSE BIT(7)
>
> +#define KMX61_REG_INC1_BIT_WUFS BIT(0)
> #define KMX61_REG_INC1_BIT_DRDYM BIT(1)
> #define KMX61_REG_INC1_BIT_DRDYA BIT(2)
> #define KMX61_REG_INC1_BIT_IEN BIT(5)
> @@ -86,6 +105,11 @@
> #define KMX61_ACC_ODR_MASK 0x0F
> #define KMX61_MAG_ODR_MASK 0xF0
>
> +#define KMX61_OWUF_MASK 0x7
> +
> +#define KMX61_DEFAULT_WAKE_THRESH 1
> +#define KMX61_DEFAULT_WAKE_DURATION 1
> +
> #define KMX61_SLEEP_DELAY_MS 2000
>
> #define KMX61_CHIP_ID 0x12
> @@ -111,11 +135,16 @@ struct kmx61_data {
> /* config bits */
> u8 range;
> u8 odr_bits;
> + u8 wake_thresh;
> + u8 wake_duration;
>
> /* accelerometer specific data */
> struct iio_dev *acc_indio_dev;
> struct iio_trigger *acc_dready_trig;
> + struct iio_trigger *motion_trig;
> bool acc_dready_trig_on;
> + bool motion_trig_on;
> + bool ev_enable_state;
>
> /* magnetometer specific data */
> struct iio_dev *mag_indio_dev;
> @@ -154,6 +183,23 @@ static const struct {
> {3, 125000, 0x0A},
> {6, 250000, 0x0B} };
>
> +static const struct {
> + int val;
> + int val2;
> + int odr_bits;
> +} kmx61_wake_up_odr_table[] = { {0, 781000, 0x00},
> + {1, 563000, 0x01},
> + {3, 125000, 0x02},
> + {6, 250000, 0x03},
> + {12, 500000, 0x04},
> + {25, 0, 0x05},
> + {50, 0, 0x06},
> + {100, 0, 0x06},
> + {200, 0, 0x06},
> + {400, 0, 0x06},
> + {800, 0, 0x06},
> + {1600, 0, 0x06} };
Is it intentional, that values above 50 share the same odr_bits?
> +
> static IIO_CONST_ATTR(accel_scale_available, "0.009582 0.019163 0.038326");
> static IIO_CONST_ATTR(magn_scale_available, "0.001465");
> static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> @@ -179,6 +225,14 @@ static const struct attribute_group kmx61_mag_attribute_group = {
> .attrs = kmx61_mag_attributes,
> };
>
> +static const struct iio_event_spec kmx61_event = {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_EITHER,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE) |
> + BIT(IIO_EV_INFO_PERIOD),
> +};
> +
> #define KMX61_ACC_CHAN(_axis) { \
> .type = IIO_ACCEL, \
> .modified = 1, \
> @@ -195,6 +249,8 @@ static const struct attribute_group kmx61_mag_attribute_group = {
> .shift = 4, \
> .endianness = IIO_LE, \
> }, \
> + .event_spec = &kmx61_event, \
> + .num_event_specs = 1 \
> }
>
> #define KMX61_MAG_CHAN(_axis) { \
> @@ -250,6 +306,31 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
> return -EINVAL;
> }
>
> +static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> + if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
> + *val = kmx61_samp_freq_table[i].val;
> + *val2 = kmx61_samp_freq_table[i].val2;
> + return 0;
> + }
> + return -EINVAL;
Given that kmx61_samp_freq_table[].odr_bits is practically representing the
index, it could be handled in the way that we did when accessing
kmx61_uscale_table. Just check odr_bits to be in range of valid indexes and
then access the tables' elements with odr_bits used as index.
Not sure, why I didn't mention it on my first review - maybe I thought you
would pick it up by yourself if you liked the idea with the _scale_table.
> +}
> +
> +
> +static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(kmx61_wake_up_odr_table); ++i)
> + if (kmx61_wake_up_odr_table[i].val == val &&
> + kmx61_wake_up_odr_table[i].val2 == val2)
> + return kmx61_wake_up_odr_table[i].odr_bits;
> + return -EINVAL;
> +}
> +
> /**
> * kmx61_set_mode() - set KMX61 device operating mode
> * @data - kmx61 device private data pointer
> @@ -338,6 +419,21 @@ static int kmx61_get_mode(struct kmx61_data *data, u8 *mode, u8 device)
> return 0;
> }
>
> +int kmx61_set_wake_up_odr(struct kmx61_data *data, int val, int val2)
> +{
> + int ret, odr_bits;
> +
> + odr_bits = kmx61_convert_wake_up_odr_to_bit(val, val2);
> + if (odr_bits < 0)
> + return odr_bits;
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL2,
> + odr_bits);
> + if (ret < 0)
> + dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
> + return ret;
> +}
> +
> static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
> {
> int ret;
> @@ -369,6 +465,12 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
> if (ret < 0)
> return ret;
>
> + if (device & KMX61_ACC) {
> + ret = kmx61_set_wake_up_odr(data, val, val2);
> + if (ret)
> + return ret;
> + }
> +
> return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> }
>
> @@ -449,7 +551,7 @@ static int kmx61_set_scale(struct kmx61_data *data, u16 uscale)
>
> static int kmx61_chip_init(struct kmx61_data *data)
> {
> - int ret;
> + int ret, val, val2;
>
> ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_WHO_AM_I);
> if (ret < 0) {
> @@ -476,11 +578,23 @@ static int kmx61_chip_init(struct kmx61_data *data)
> }
> data->odr_bits = ret;
>
> + /* set output data rate for wake up (motion detection) function */
> + ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_wake_up_odr(data, val, val2);
> + if (ret < 0)
> + return ret;
> +
> /* set acc/magn to OPERATION mode */
> ret = kmx61_set_mode(data, 0, KMX61_ACC | KMX61_MAG, true);
> if (ret < 0)
> return ret;
>
> + data->wake_thresh = KMX61_DEFAULT_WAKE_THRESH;
> + data->wake_duration = KMX61_DEFAULT_WAKE_DURATION;
> +
> return 0;
> }
>
> @@ -547,6 +661,87 @@ static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
> return 0;
> }
>
> +static int kmx61_chip_update_thresholds(struct kmx61_data *data)
> +{
> + int ret;
> +
> + ret = i2c_smbus_write_byte_data(data->client,
> + KMX61_REG_WUF_TIMER,
> + data->wake_duration);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Errow writing reg_wuf_timer\n");
> + return ret;
> + }
> +
> + ret = i2c_smbus_write_byte_data(data->client,
> + KMX61_REG_WUF_THRESH,
> + data->wake_thresh);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_wuf_thresh\n");
> + return ret;
> + }
> +
> + return 0;
Like in kmx61_set_wake_up_odr, you could return ret here and drop it in the
last error handler.
> +}
> +
> +static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
> + bool status, u8 device)
device is unused in this function.
> +{
> + u8 mode;
> + int ret;
> +
> + ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> + if (ret < 0)
> + return ret;
> +
> + ret = kmx61_chip_update_thresholds(data);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INC1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_inc1\n");
> + return ret;
> + }
> + if (status)
> + ret |= (KMX61_REG_INC1_BIT_IEN | KMX61_REG_INC1_BIT_WUFS);
> + else
> + ret &= ~(KMX61_REG_INC1_BIT_IEN | KMX61_REG_INC1_BIT_WUFS);
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_INC1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_inc1\n");
> + return ret;
> + }
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> + return ret;
> + }
> +
> + if (status)
> + ret |= KMX61_REG_CTRL1_BIT_WUFE | KMX61_REG_CTRL1_BIT_BTSE;
> + else
> + ret &= ~(KMX61_REG_CTRL1_BIT_WUFE | KMX61_REG_CTRL1_BIT_BTSE);
> +
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> + return ret;
> + }
> + mode |= KMX61_ACT_STBY_BIT;
> + ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> + if (ret)
> + return ret;
> +
> + return 0;
Just like in the end of kmx61_set_odr(): return kmx61_set_mode(...);
> +}
> +
> /**
> * kmx61_set_power_state() - set power state for kmx61 @device
> * @data - kmx61 device private pointer
> @@ -707,12 +902,111 @@ static int kmx61_write_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int kmx61_read_event(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int *val, int *val2)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + *val2 = 0;
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + *val = data->wake_thresh;
> + break;
> + case IIO_EV_INFO_PERIOD:
> + *val = data->wake_duration;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return IIO_VAL_INT;
Stylewise, we usually put the return IIO_VAL_INT (or similar) inside each case
if there is nothing more to be done (like unlocking mutexes, ...). Makes it
slightly more obvious, what type the data is supposed to be.
> +}
> +
> +static int kmx61_write_event(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int val, int val2)
Parameters should be indented one more space.
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + if (data->ev_enable_state)
> + return -EBUSY;
> +
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + data->wake_thresh = val;
> + break;
> + case IIO_EV_INFO_PERIOD:
> + data->wake_duration = val;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return IIO_VAL_INT;
Same here.
> +}
> +
> +static int kmx61_read_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir)
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> +
> + return data->ev_enable_state;
> +}
> +
> +static int kmx61_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + int state)
Parameters should be indented one more space.
> +{
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> + int ret;
> +
> + if (state && data->ev_enable_state)
> + return 0;
> +
> + mutex_lock(&data->lock);
> +
> + if (!state && data->motion_trig_on) {
> + data->ev_enable_state = 0;
Assign true/false to bools?
> + mutex_unlock(&data->lock);
> + return 0;
> + }
> +
> + ret = kmx61_set_power_state(data, state, KMX61_ACC);
> + if (ret < 0) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
> + if (ret < 0) {
> + kmx61_set_power_state(data, false, KMX61_ACC);
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> + data->ev_enable_state = state;
You could place an error label here and return ret below to reduce some code
repetition in the error handlers above.
> + mutex_unlock(&data->lock);
> +
> + return 0;
> +}
> +
> static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
> struct iio_trigger *trig)
> {
> struct kmx61_data *data = kmx61_get_data(indio_dev);
>
> - if (data->acc_dready_trig != trig)
> + if (data->acc_dready_trig != trig && data->motion_trig != trig)
> return -EINVAL;
>
> return 0;
> @@ -734,6 +1028,10 @@ static const struct iio_info kmx61_acc_info = {
> .read_raw = kmx61_read_raw,
> .write_raw = kmx61_write_raw,
> .attrs = &kmx61_acc_attribute_group,
> + .read_event_value = kmx61_read_event,
> + .write_event_value = kmx61_write_event,
> + .read_event_config = kmx61_read_event_config,
> + .write_event_config = kmx61_write_event_config,
> .validate_trigger = kmx61_acc_validate_trigger,
> };
>
> @@ -753,11 +1051,18 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> u8 device;
>
> struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> - struct kmx61_data *data = iio_priv(indio_dev);
> + struct kmx61_data *data = kmx61_get_data(indio_dev);
Oh well, fixing the issue I pointed out in your previous patch. LOL and ouch!
>
> mutex_lock(&data->lock);
>
> - if (data->acc_dready_trig == trig)
> + if (!state && data->ev_enable_state && data->motion_trig_on) {
> + data->motion_trig_on = false;
> + mutex_unlock(&data->lock);
> + return 0;
> + }
> +
> +
> + if (data->acc_dready_trig == trig || data->motion_trig)
Wouldn't you want to compare data->motion_trig with trig?
> device = KMX61_ACC;
> else
> device = KMX61_MAG;
> @@ -768,7 +1073,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> return ret;
> }
>
> - ret = kmx61_setup_new_data_interrupt(data, state, device);
> + if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
> + ret = kmx61_setup_new_data_interrupt(data, state, device);
> + else
> + ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
> if (ret < 0) {
> kmx61_set_power_state(data, false, device);
> mutex_unlock(&data->lock);
> @@ -777,8 +1085,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>
> if (data->acc_dready_trig == trig)
> data->acc_dready_trig_on = state;
> - else
> + else if (data->mag_dready_trig == trig)
> data->mag_dready_trig_on = state;
> + else
> + data->motion_trig_on = state;
>
> mutex_unlock(&data->lock);
>
> @@ -806,6 +1116,99 @@ static const struct iio_trigger_ops kmx61_trigger_ops = {
> .owner = THIS_MODULE,
> };
>
> +static irqreturn_t kmx61_event_handler(int irq, void *private)
> +{
> + struct kmx61_data *data = private;
> + struct iio_dev *indio_dev = data->acc_indio_dev;
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ins1\n");
> + goto ack_intr;
> + }
> +
> + if (ret & KMX61_REG_INS1_BIT_WUFS) {
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS2);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "Error reading reg_ins2\n");
> + goto ack_intr;
> + }
> +
> + if (ret & KMX61_REG_INS2_BIT_XN)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_X,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_FALLING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_XP)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_X,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_RISING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_YN)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_Y,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_FALLING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_YP)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_Y,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_RISING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_ZN)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_Z,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_FALLING),
> + 0);
> +
> + if (ret & KMX61_REG_INS2_BIT_ZP)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
> + 0,
> + IIO_MOD_Z,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_RISING),
> + 0);
> + }
> +
> +ack_intr:
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
> + if (ret < 0)
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> +
> + ret |= KMX61_REG_CTRL1_BIT_RES;
> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
> + if (ret < 0)
> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
Typo: Error writing ...
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
> + if (ret < 0)
> + dev_err(&data->client->dev, "Error reading reg_inl\n");
> +
> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
Missing error handling/message here.
> +
> + return IRQ_HANDLED;
> +}
> +
> static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
> {
> struct kmx61_data *data = private;
> @@ -815,6 +1218,11 @@ static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
> if (data->mag_dready_trig_on)
> iio_trigger_poll(data->mag_dready_trig);
>
> + if (data->motion_trig_on)
> + iio_trigger_poll(data->motion_trig);
> +
> + if (data->ev_enable_state)
> + return IRQ_WAKE_THREAD;
> return IRQ_HANDLED;
> }
>
> @@ -982,7 +1390,7 @@ static int kmx61_probe(struct i2c_client *client,
> if (client->irq >= 0) {
> ret = devm_request_threaded_irq(&client->dev, client->irq,
> kmx61_data_rdy_trig_poll,
> - NULL,
> + kmx61_event_handler,
> IRQF_TRIGGER_RISING,
> KMX61_IRQ_NAME,
> data);
> @@ -1003,6 +1411,14 @@ static int kmx61_probe(struct i2c_client *client,
> goto err_trigger_unregister;
> }
>
> + data->motion_trig =
> + kmx61_trigger_setup(data, data->acc_indio_dev,
> + "any-motion");
> + if (IS_ERR(data->motion_trig)) {
> + ret = PTR_ERR(data->motion_trig);
> + goto err_trigger_unregister;
> + }
> +
> ret = iio_triggered_buffer_setup(data->acc_indio_dev,
> &iio_pollfunc_store_time,
> kmx61_trigger_handler,
> @@ -1060,6 +1476,8 @@ err_trigger_unregister:
> iio_trigger_unregister(data->acc_dready_trig);
> if (data->mag_dready_trig)
> iio_trigger_unregister(data->mag_dready_trig);
> + if (data->motion_trig)
> + iio_trigger_unregister(data->motion_trig);
> err_chip_uninit:
> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> return ret;
> @@ -1081,6 +1499,7 @@ static int kmx61_remove(struct i2c_client *client)
> iio_triggered_buffer_cleanup(data->mag_indio_dev);
> iio_trigger_unregister(data->acc_dready_trig);
> iio_trigger_unregister(data->mag_dready_trig);
> + iio_trigger_unregister(data->motion_trig);
> }
>
> mutex_lock(&data->lock);
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 6/6] iio: imu: kmx61: Add support for any motion trigger
2014-12-15 23:19 ` Hartmut Knaack
@ 2014-12-16 11:08 ` Daniel Baluta
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Baluta @ 2014-12-16 11:08 UTC (permalink / raw)
To: Hartmut Knaack
Cc: Daniel Baluta, Jonathan Cameron, Peter Meerwald,
Srinivas Pandruvada, linux-iio@vger.kernel.org,
Linux Kernel Mailing List
On Tue, Dec 16, 2014 at 1:19 AM, Hartmut Knaack <knaack.h@gmx.de> wrote:
> Daniel Baluta schrieb am 03.12.2014 um 14:31:
>> We use WUFE (Wake Up from Sleep Engine) and BTSE (Back to Sleep Engine)
>> to detect general motion input.
>>
> A few recommendations, issues and questions inline.
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> ---
>> drivers/iio/imu/kmx61.c | 433 +++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 426 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index 0f6816a..972424b 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -20,6 +20,7 @@
>> #include <linux/pm_runtime.h>
>> #include <linux/iio/iio.h>
>> #include <linux/iio/sysfs.h>
>> +#include <linux/iio/events.h>
>> #include <linux/iio/trigger.h>
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/triggered_buffer.h>
>> @@ -30,6 +31,8 @@
>> #define KMX61_IRQ_NAME "kmx61_event"
>>
>> #define KMX61_REG_WHO_AM_I 0x00
>> +#define KMX61_REG_INS1 0x01
>> +#define KMX61_REG_INS2 0x02
>>
>> /*
>> * three 16-bit accelerometer output registers for X/Y/Z axis
>> @@ -63,20 +66,36 @@
>> #define KMX61_REG_INL 0x28
>> #define KMX61_REG_STBY 0x29
>> #define KMX61_REG_CTRL1 0x2A
>> +#define KMX61_REG_CTRL2 0x2B
>> #define KMX61_REG_ODCNTL 0x2C
>> #define KMX61_REG_INC1 0x2D
>>
>> +#define KMX61_REG_WUF_THRESH 0x3D
>> +#define KMX61_REG_WUF_TIMER 0x3E
>> +
>> #define KMX61_ACC_STBY_BIT BIT(0)
>> #define KMX61_MAG_STBY_BIT BIT(1)
>> #define KMX61_ACT_STBY_BIT BIT(7)
>>
>> #define KMX61_ALL_STBY (KMX61_ACC_STBY_BIT | KMX61_MAG_STBY_BIT)
>>
>> +#define KMX61_REG_INS1_BIT_WUFS BIT(1)
>> +
>> +#define KMX61_REG_INS2_BIT_ZP BIT(0)
>> +#define KMX61_REG_INS2_BIT_ZN BIT(1)
>> +#define KMX61_REG_INS2_BIT_YP BIT(2)
>> +#define KMX61_REG_INS2_BIT_YN BIT(3)
>> +#define KMX61_REG_INS2_BIT_XP BIT(4)
>> +#define KMX61_REG_INS2_BIT_XN BIT(5)
>> +
>> #define KMX61_REG_CTRL1_GSEL_MASK 0x03
>>
>> #define KMX61_REG_CTRL1_BIT_RES BIT(4)
>> #define KMX61_REG_CTRL1_BIT_DRDYE BIT(5)
>> +#define KMX61_REG_CTRL1_BIT_WUFE BIT(6)
>> +#define KMX61_REG_CTRL1_BIT_BTSE BIT(7)
>>
>> +#define KMX61_REG_INC1_BIT_WUFS BIT(0)
>> #define KMX61_REG_INC1_BIT_DRDYM BIT(1)
>> #define KMX61_REG_INC1_BIT_DRDYA BIT(2)
>> #define KMX61_REG_INC1_BIT_IEN BIT(5)
>> @@ -86,6 +105,11 @@
>> #define KMX61_ACC_ODR_MASK 0x0F
>> #define KMX61_MAG_ODR_MASK 0xF0
>>
>> +#define KMX61_OWUF_MASK 0x7
>> +
>> +#define KMX61_DEFAULT_WAKE_THRESH 1
>> +#define KMX61_DEFAULT_WAKE_DURATION 1
>> +
>> #define KMX61_SLEEP_DELAY_MS 2000
>>
>> #define KMX61_CHIP_ID 0x12
>> @@ -111,11 +135,16 @@ struct kmx61_data {
>> /* config bits */
>> u8 range;
>> u8 odr_bits;
>> + u8 wake_thresh;
>> + u8 wake_duration;
>>
>> /* accelerometer specific data */
>> struct iio_dev *acc_indio_dev;
>> struct iio_trigger *acc_dready_trig;
>> + struct iio_trigger *motion_trig;
>> bool acc_dready_trig_on;
>> + bool motion_trig_on;
>> + bool ev_enable_state;
>>
>> /* magnetometer specific data */
>> struct iio_dev *mag_indio_dev;
>> @@ -154,6 +183,23 @@ static const struct {
>> {3, 125000, 0x0A},
>> {6, 250000, 0x0B} };
>>
>> +static const struct {
>> + int val;
>> + int val2;
>> + int odr_bits;
>> +} kmx61_wake_up_odr_table[] = { {0, 781000, 0x00},
>> + {1, 563000, 0x01},
>> + {3, 125000, 0x02},
>> + {6, 250000, 0x03},
>> + {12, 500000, 0x04},
>> + {25, 0, 0x05},
>> + {50, 0, 0x06},
>> + {100, 0, 0x06},
>> + {200, 0, 0x06},
>> + {400, 0, 0x06},
>> + {800, 0, 0x06},
>> + {1600, 0, 0x06} };
> Is it intentional, that values above 50 share the same odr_bits?
Yes, this is as intended. I followed the idea from kxcjk-1013 driver.
Experimenting with ODR of > 50HZ makes no benefit to system c states.
>> +
>> static IIO_CONST_ATTR(accel_scale_available, "0.009582 0.019163 0.038326");
>> static IIO_CONST_ATTR(magn_scale_available, "0.001465");
>> static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
>> @@ -179,6 +225,14 @@ static const struct attribute_group kmx61_mag_attribute_group = {
>> .attrs = kmx61_mag_attributes,
>> };
>>
>> +static const struct iio_event_spec kmx61_event = {
>> + .type = IIO_EV_TYPE_THRESH,
>> + .dir = IIO_EV_DIR_EITHER,
>> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
>> + BIT(IIO_EV_INFO_ENABLE) |
>> + BIT(IIO_EV_INFO_PERIOD),
>> +};
>> +
>> #define KMX61_ACC_CHAN(_axis) { \
>> .type = IIO_ACCEL, \
>> .modified = 1, \
>> @@ -195,6 +249,8 @@ static const struct attribute_group kmx61_mag_attribute_group = {
>> .shift = 4, \
>> .endianness = IIO_LE, \
>> }, \
>> + .event_spec = &kmx61_event, \
>> + .num_event_specs = 1 \
>> }
>>
>> #define KMX61_MAG_CHAN(_axis) { \
>> @@ -250,6 +306,31 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
>> return -EINVAL;
>> }
>>
>> +static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>> + if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
>> + *val = kmx61_samp_freq_table[i].val;
>> + *val2 = kmx61_samp_freq_table[i].val2;
>> + return 0;
>> + }
>> + return -EINVAL;
> Given that kmx61_samp_freq_table[].odr_bits is practically representing the
> index, it could be handled in the way that we did when accessing
> kmx61_uscale_table. Just check odr_bits to be in range of valid indexes and
> then access the tables' elements with odr_bits used as index.
> Not sure, why I didn't mention it on my first review - maybe I thought you
> would pick it up by yourself if you liked the idea with the _scale_table.
Oh, ok.
>> +}
>> +
>> +
>> +static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(kmx61_wake_up_odr_table); ++i)
>> + if (kmx61_wake_up_odr_table[i].val == val &&
>> + kmx61_wake_up_odr_table[i].val2 == val2)
>> + return kmx61_wake_up_odr_table[i].odr_bits;
>> + return -EINVAL;
>> +}
>> +
>> /**
>> * kmx61_set_mode() - set KMX61 device operating mode
>> * @data - kmx61 device private data pointer
>> @@ -338,6 +419,21 @@ static int kmx61_get_mode(struct kmx61_data *data, u8 *mode, u8 device)
>> return 0;
>> }
>>
>> +int kmx61_set_wake_up_odr(struct kmx61_data *data, int val, int val2)
>> +{
>> + int ret, odr_bits;
>> +
>> + odr_bits = kmx61_convert_wake_up_odr_to_bit(val, val2);
>> + if (odr_bits < 0)
>> + return odr_bits;
>> +
>> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL2,
>> + odr_bits);
>> + if (ret < 0)
>> + dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
>> + return ret;
>> +}
>> +
>> static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
>> {
>> int ret;
>> @@ -369,6 +465,12 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
>> if (ret < 0)
>> return ret;
>>
>> + if (device & KMX61_ACC) {
>> + ret = kmx61_set_wake_up_odr(data, val, val2);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
>> }
>>
>> @@ -449,7 +551,7 @@ static int kmx61_set_scale(struct kmx61_data *data, u16 uscale)
>>
>> static int kmx61_chip_init(struct kmx61_data *data)
>> {
>> - int ret;
>> + int ret, val, val2;
>>
>> ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_WHO_AM_I);
>> if (ret < 0) {
>> @@ -476,11 +578,23 @@ static int kmx61_chip_init(struct kmx61_data *data)
>> }
>> data->odr_bits = ret;
>>
>> + /* set output data rate for wake up (motion detection) function */
>> + ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = kmx61_set_wake_up_odr(data, val, val2);
>> + if (ret < 0)
>> + return ret;
>> +
>> /* set acc/magn to OPERATION mode */
>> ret = kmx61_set_mode(data, 0, KMX61_ACC | KMX61_MAG, true);
>> if (ret < 0)
>> return ret;
>>
>> + data->wake_thresh = KMX61_DEFAULT_WAKE_THRESH;
>> + data->wake_duration = KMX61_DEFAULT_WAKE_DURATION;
>> +
>> return 0;
>> }
>>
>> @@ -547,6 +661,87 @@ static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
>> return 0;
>> }
>>
>> +static int kmx61_chip_update_thresholds(struct kmx61_data *data)
>> +{
>> + int ret;
>> +
>> + ret = i2c_smbus_write_byte_data(data->client,
>> + KMX61_REG_WUF_TIMER,
>> + data->wake_duration);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Errow writing reg_wuf_timer\n");
>> + return ret;
>> + }
>> +
>> + ret = i2c_smbus_write_byte_data(data->client,
>> + KMX61_REG_WUF_THRESH,
>> + data->wake_thresh);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error writing reg_wuf_thresh\n");
>> + return ret;
>> + }
>> +
>> + return 0;
> Like in kmx61_set_wake_up_odr, you could return ret here and drop it in the
> last error handler.
ok.
>> +}
>> +
>> +static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
>> + bool status, u8 device)
> device is unused in this function.
>> +{
>> + u8 mode;
>> + int ret;
>> +
>> + ret = kmx61_get_mode(data, &mode, KMX61_ACC | KMX61_MAG);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = kmx61_chip_update_thresholds(data);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INC1);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error reading reg_inc1\n");
>> + return ret;
>> + }
>> + if (status)
>> + ret |= (KMX61_REG_INC1_BIT_IEN | KMX61_REG_INC1_BIT_WUFS);
>> + else
>> + ret &= ~(KMX61_REG_INC1_BIT_IEN | KMX61_REG_INC1_BIT_WUFS);
>> +
>> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_INC1, ret);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error writing reg_inc1\n");
>> + return ret;
>> + }
>> +
>> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
>> + return ret;
>> + }
>> +
>> + if (status)
>> + ret |= KMX61_REG_CTRL1_BIT_WUFE | KMX61_REG_CTRL1_BIT_BTSE;
>> + else
>> + ret &= ~(KMX61_REG_CTRL1_BIT_WUFE | KMX61_REG_CTRL1_BIT_BTSE);
>> +
>> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
>> + return ret;
>> + }
>> + mode |= KMX61_ACT_STBY_BIT;
>> + ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
>> + if (ret)
>> + return ret;
>> +
>> + return 0;
> Just like in the end of kmx61_set_odr(): return kmx61_set_mode(...);
ok.
>> +}
>> +
>> /**
>> * kmx61_set_power_state() - set power state for kmx61 @device
>> * @data - kmx61 device private pointer
>> @@ -707,12 +902,111 @@ static int kmx61_write_raw(struct iio_dev *indio_dev,
>> }
>> }
>>
>> +static int kmx61_read_event(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + enum iio_event_type type,
>> + enum iio_event_direction dir,
>> + enum iio_event_info info,
>> + int *val, int *val2)
>> +{
>> + struct kmx61_data *data = kmx61_get_data(indio_dev);
>> +
>> + *val2 = 0;
>> + switch (info) {
>> + case IIO_EV_INFO_VALUE:
>> + *val = data->wake_thresh;
>> + break;
>> + case IIO_EV_INFO_PERIOD:
>> + *val = data->wake_duration;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return IIO_VAL_INT;
> Stylewise, we usually put the return IIO_VAL_INT (or similar) inside each case
> if there is nothing more to be done (like unlocking mutexes, ...). Makes it
> slightly more obvious, what type the data is supposed to be.
>> +}
ok.
>> +
>> +static int kmx61_write_event(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + enum iio_event_type type,
>> + enum iio_event_direction dir,
>> + enum iio_event_info info,
>> + int val, int val2)
> Parameters should be indented one more space.
ok.
>> +{
>> + struct kmx61_data *data = kmx61_get_data(indio_dev);
>> +
>> + if (data->ev_enable_state)
>> + return -EBUSY;
>> +
>> + switch (info) {
>> + case IIO_EV_INFO_VALUE:
>> + data->wake_thresh = val;
>> + break;
>> + case IIO_EV_INFO_PERIOD:
>> + data->wake_duration = val;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return IIO_VAL_INT;
> Same here.
ok.
>> +}
>> +
>> +static int kmx61_read_event_config(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + enum iio_event_type type,
>> + enum iio_event_direction dir)
>> +{
>> + struct kmx61_data *data = kmx61_get_data(indio_dev);
>> +
>> + return data->ev_enable_state;
>> +}
>> +
>> +static int kmx61_write_event_config(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + enum iio_event_type type,
>> + enum iio_event_direction dir,
>> + int state)
> Parameters should be indented one more space.
ok.
>> +{
>> + struct kmx61_data *data = kmx61_get_data(indio_dev);
>> + int ret;
>> +
>> + if (state && data->ev_enable_state)
>> + return 0;
>> +
>> + mutex_lock(&data->lock);
>> +
>> + if (!state && data->motion_trig_on) {
>> + data->ev_enable_state = 0;
> Assign true/false to bools?
ok.
>> + mutex_unlock(&data->lock);
>> + return 0;
>> + }
>> +
>> + ret = kmx61_set_power_state(data, state, KMX61_ACC);
>> + if (ret < 0) {
>> + mutex_unlock(&data->lock);
>> + return ret;
>> + }
>> +
>> + ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
>> + if (ret < 0) {
>> + kmx61_set_power_state(data, false, KMX61_ACC);
>> + mutex_unlock(&data->lock);
>> + return ret;
>> + }
>> +
>> + data->ev_enable_state = state;
> You could place an error label here and return ret below to reduce some code
> repetition in the error handlers above.
>> + mutex_unlock(&data->lock);
>> +
>> + return 0;
>> +}
>> +
>> static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
>> struct iio_trigger *trig)
>> {
>> struct kmx61_data *data = kmx61_get_data(indio_dev);
>>
>> - if (data->acc_dready_trig != trig)
>> + if (data->acc_dready_trig != trig && data->motion_trig != trig)
>> return -EINVAL;
>>
>> return 0;
>> @@ -734,6 +1028,10 @@ static const struct iio_info kmx61_acc_info = {
>> .read_raw = kmx61_read_raw,
>> .write_raw = kmx61_write_raw,
>> .attrs = &kmx61_acc_attribute_group,
>> + .read_event_value = kmx61_read_event,
>> + .write_event_value = kmx61_write_event,
>> + .read_event_config = kmx61_read_event_config,
>> + .write_event_config = kmx61_write_event_config,
>> .validate_trigger = kmx61_acc_validate_trigger,
>> };
>>
>> @@ -753,11 +1051,18 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>> u8 device;
>>
>> struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>> - struct kmx61_data *data = iio_priv(indio_dev);
>> + struct kmx61_data *data = kmx61_get_data(indio_dev);
> Oh well, fixing the issue I pointed out in your previous patch. LOL and ouch!
:)). That explains why the driver didn't crash when I tested it.
>>
>> mutex_lock(&data->lock);
>>
>> - if (data->acc_dready_trig == trig)
>> + if (!state && data->ev_enable_state && data->motion_trig_on) {
>> + data->motion_trig_on = false;
>> + mutex_unlock(&data->lock);
>> + return 0;
>> + }
>> +
>> +
>> + if (data->acc_dready_trig == trig || data->motion_trig)
> Wouldn't you want to compare data->motion_trig with trig?
This is a nice catch.
>> device = KMX61_ACC;
>> else
>> device = KMX61_MAG;
>> @@ -768,7 +1073,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>> return ret;
>> }
>>
>> - ret = kmx61_setup_new_data_interrupt(data, state, device);
>> + if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
>> + ret = kmx61_setup_new_data_interrupt(data, state, device);
>> + else
>> + ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
>> if (ret < 0) {
>> kmx61_set_power_state(data, false, device);
>> mutex_unlock(&data->lock);
>> @@ -777,8 +1085,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>>
>> if (data->acc_dready_trig == trig)
>> data->acc_dready_trig_on = state;
>> - else
>> + else if (data->mag_dready_trig == trig)
>> data->mag_dready_trig_on = state;
>> + else
>> + data->motion_trig_on = state;
>>
>> mutex_unlock(&data->lock);
>>
>> @@ -806,6 +1116,99 @@ static const struct iio_trigger_ops kmx61_trigger_ops = {
>> .owner = THIS_MODULE,
>> };
>>
>> +static irqreturn_t kmx61_event_handler(int irq, void *private)
>> +{
>> + struct kmx61_data *data = private;
>> + struct iio_dev *indio_dev = data->acc_indio_dev;
>> + int ret;
>> +
>> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error reading reg_ins1\n");
>> + goto ack_intr;
>> + }
>> +
>> + if (ret & KMX61_REG_INS1_BIT_WUFS) {
>> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS2);
>> + if (ret < 0) {
>> + dev_err(&data->client->dev, "Error reading reg_ins2\n");
>> + goto ack_intr;
>> + }
>> +
>> + if (ret & KMX61_REG_INS2_BIT_XN)
>> + iio_push_event(indio_dev,
>> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
>> + 0,
>> + IIO_MOD_X,
>> + IIO_EV_TYPE_THRESH,
>> + IIO_EV_DIR_FALLING),
>> + 0);
>> +
>> + if (ret & KMX61_REG_INS2_BIT_XP)
>> + iio_push_event(indio_dev,
>> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
>> + 0,
>> + IIO_MOD_X,
>> + IIO_EV_TYPE_THRESH,
>> + IIO_EV_DIR_RISING),
>> + 0);
>> +
>> + if (ret & KMX61_REG_INS2_BIT_YN)
>> + iio_push_event(indio_dev,
>> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
>> + 0,
>> + IIO_MOD_Y,
>> + IIO_EV_TYPE_THRESH,
>> + IIO_EV_DIR_FALLING),
>> + 0);
>> +
>> + if (ret & KMX61_REG_INS2_BIT_YP)
>> + iio_push_event(indio_dev,
>> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
>> + 0,
>> + IIO_MOD_Y,
>> + IIO_EV_TYPE_THRESH,
>> + IIO_EV_DIR_RISING),
>> + 0);
>> +
>> + if (ret & KMX61_REG_INS2_BIT_ZN)
>> + iio_push_event(indio_dev,
>> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
>> + 0,
>> + IIO_MOD_Z,
>> + IIO_EV_TYPE_THRESH,
>> + IIO_EV_DIR_FALLING),
>> + 0);
>> +
>> + if (ret & KMX61_REG_INS2_BIT_ZP)
>> + iio_push_event(indio_dev,
>> + IIO_MOD_EVENT_CODE(IIO_ACCEL,
>> + 0,
>> + IIO_MOD_Z,
>> + IIO_EV_TYPE_THRESH,
>> + IIO_EV_DIR_RISING),
>> + 0);
>> + }
>> +
>> +ack_intr:
>> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_CTRL1);
>> + if (ret < 0)
>> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
>> +
>> + ret |= KMX61_REG_CTRL1_BIT_RES;
>> + ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
>> + if (ret < 0)
>> + dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> Typo: Error writing ...
ok.
>> +
>> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
>> + if (ret < 0)
>> + dev_err(&data->client->dev, "Error reading reg_inl\n");
>> +
>> + ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
> Missing error handling/message here.
ok.
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
>> {
>> struct kmx61_data *data = private;
>> @@ -815,6 +1218,11 @@ static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
>> if (data->mag_dready_trig_on)
>> iio_trigger_poll(data->mag_dready_trig);
>>
>> + if (data->motion_trig_on)
>> + iio_trigger_poll(data->motion_trig);
>> +
>> + if (data->ev_enable_state)
>> + return IRQ_WAKE_THREAD;
>> return IRQ_HANDLED;
>> }
>>
>> @@ -982,7 +1390,7 @@ static int kmx61_probe(struct i2c_client *client,
>> if (client->irq >= 0) {
>> ret = devm_request_threaded_irq(&client->dev, client->irq,
>> kmx61_data_rdy_trig_poll,
>> - NULL,
>> + kmx61_event_handler,
>> IRQF_TRIGGER_RISING,
>> KMX61_IRQ_NAME,
>> data);
>> @@ -1003,6 +1411,14 @@ static int kmx61_probe(struct i2c_client *client,
>> goto err_trigger_unregister;
>> }
>>
>> + data->motion_trig =
>> + kmx61_trigger_setup(data, data->acc_indio_dev,
>> + "any-motion");
>> + if (IS_ERR(data->motion_trig)) {
>> + ret = PTR_ERR(data->motion_trig);
>> + goto err_trigger_unregister;
>> + }
>> +
>> ret = iio_triggered_buffer_setup(data->acc_indio_dev,
>> &iio_pollfunc_store_time,
>> kmx61_trigger_handler,
>> @@ -1060,6 +1476,8 @@ err_trigger_unregister:
>> iio_trigger_unregister(data->acc_dready_trig);
>> if (data->mag_dready_trig)
>> iio_trigger_unregister(data->mag_dready_trig);
>> + if (data->motion_trig)
>> + iio_trigger_unregister(data->motion_trig);
>> err_chip_uninit:
>> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
>> return ret;
>> @@ -1081,6 +1499,7 @@ static int kmx61_remove(struct i2c_client *client)
>> iio_triggered_buffer_cleanup(data->mag_indio_dev);
>> iio_trigger_unregister(data->acc_dready_trig);
>> iio_trigger_unregister(data->mag_dready_trig);
>> + iio_trigger_unregister(data->motion_trig);
>> }
>>
>> mutex_lock(&data->lock);
Thanks a lot for the feedback, Hartmut!
Daniel.
^ permalink raw reply [flat|nested] 20+ messages in thread