This is version 3 of the mma8452 driver improvements. Version 3 adds one
patch to allow all possible pin wirings for users and adds more relevant
people to the discussion, I forgot about before. I'm sorry.
These changes add support for motion interrupts and 3 more accelerometer
chips, two of which use them because they don't support the until now
included transient interrupt sources:
MMA8453Q, MMA8652FC and MMA8653FC; datasheets are in the commit messages.
The driver and module name remains the same, seperating it from the device
names it now supports.
On top of this, there are minor documentation additions, and more notably,
the change of the existing iio event type from IIO_EV_TYPE_THRESH to
IIO_EV_TYPE_MAG since it's the magnitude of the threshold that is relevant
for these devices, not a signed value. Finally it allows to use the driver,
no matter how the interrupt pins are wired on your board.
Please review and test if you can. For MMA8452Q, other than the event type,
nothing should have changed.
revision history
----------------
v3 adds one patch to allow all possible pin wirings; adds more email
recipients
v2 splits the work into a series of smaller pieces
v1 initial post
This adds a struct mma_chip_info to hold data that will remain specific to
the chip in use. It is provided during probe() and linked in
struct of_device_id.
Also this suggests that the driver is called "mma8452" and now handles the
MMA8452Q device, but is not limited to it.
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
drivers/iio/accel/mma8452.c | 161 ++++++++++++++++++++++++++++++--------------
1 file changed, 110 insertions(+), 51 deletions(-)
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index e8e2077..b2ebfcb 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -23,7 +23,9 @@
#include <linux/iio/triggered_buffer.h>
#include <linux/iio/events.h>
#include <linux/delay.h>
+#include <linux/of_device.h>
+#define DRIVER_NAME "mma8452"
#define MMA8452_STATUS 0x00
#define MMA8452_OUT_X 0x01 /* MSB first, 12-bit */
#define MMA8452_OUT_Y 0x03
@@ -78,6 +80,25 @@ struct mma8452_data {
struct mutex lock;
u8 ctrl_reg1;
u8 data_cfg;
+ const struct mma_chip_info *chip_info;
+};
+
+struct mma_chip_info {
+ u8 chip_id;
+ const struct iio_chan_spec *channels;
+ int num_channels;
+ /* 3 modes: 2g, 4g, 8g; 2 ints: m/s^2 and micro m/s^2 */
+ const int mma_scales[3][2];
+ u8 ev_cfg;
+ u8 ev_cfg_ele;
+ u8 ev_cfg_chan_shift;
+ u8 ev_src;
+ u8 ev_src_xe;
+ u8 ev_src_ye;
+ u8 ev_src_ze;
+ u8 ev_ths;
+ u8 ev_ths_mask;
+ u8 ev_count;
};
static int mma8452_drdy(struct mma8452_data *data)
@@ -143,16 +164,6 @@ static const int mma8452_samp_freq[8][2] = {
{6, 250000}, {1, 560000}
};
-/*
- * Hardware has fullscale of -2G, -4G, -8G corresponding to raw value -2048
- * The userspace interface uses m/s^2 and we declare micro units
- * So scale factor is given by:
- * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665
- */
-static const int mma8452_scales[3][2] = {
- {0, 9577}, {0, 19154}, {0, 38307}
-};
-
/* Datasheet table 35 (step time vs sample frequency) */
static const int mma8452_transient_time_step_us[8] = {
1250,
@@ -187,8 +198,11 @@ static ssize_t mma8452_show_samp_freq_avail(struct device *dev,
static ssize_t mma8452_show_scale_avail(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return mma8452_show_int_plus_micros(buf, mma8452_scales,
- ARRAY_SIZE(mma8452_scales));
+ struct mma8452_data *data = iio_priv(i2c_get_clientdata(
+ to_i2c_client(dev)));
+
+ return mma8452_show_int_plus_micros(buf, data->chip_info->mma_scales,
+ ARRAY_SIZE(data->chip_info->mma_scales));
}
static ssize_t mma8452_show_hp_cutoff_avail(struct device *dev,
@@ -219,8 +233,8 @@ static int mma8452_get_samp_freq_index(struct mma8452_data *data,
static int mma8452_get_scale_index(struct mma8452_data *data,
int val, int val2)
{
- return mma8452_get_int_plus_micros_index(mma8452_scales,
- ARRAY_SIZE(mma8452_scales), val, val2);
+ return mma8452_get_int_plus_micros_index(data->chip_info->mma_scales,
+ ARRAY_SIZE(data->chip_info->mma_scales), val, val2);
}
static int mma8452_get_hp_filter_index(struct mma8452_data *data,
@@ -229,7 +243,7 @@ static int mma8452_get_hp_filter_index(struct mma8452_data *data,
int i = mma8452_get_odr_index(data);
return mma8452_get_int_plus_micros_index(mma8452_hp_filter_cutoff[i],
- ARRAY_SIZE(mma8452_scales[0]), val, val2);
+ ARRAY_SIZE(data->chip_info->mma_scales[0]), val, val2);
}
static int mma8452_read_hp_filter(struct mma8452_data *data, int *hz, int *uHz)
@@ -266,13 +280,14 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
mutex_unlock(&data->lock);
if (ret < 0)
return ret;
- *val = sign_extend32(
- be16_to_cpu(buffer[chan->scan_index]) >> 4, 11);
+ *val = sign_extend32(be16_to_cpu(
+ buffer[chan->scan_index]) >> chan->scan_type.shift,
+ chan->scan_type.realbits - 1);
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
i = data->data_cfg & MMA8452_DATA_CFG_FS_MASK;
- *val = mma8452_scales[i][0];
- *val2 = mma8452_scales[i][1];
+ *val = data->chip_info->mma_scales[i][0];
+ *val2 = data->chip_info->mma_scales[i][1];
return IIO_VAL_INT_PLUS_MICRO;
case IIO_CHAN_INFO_SAMP_FREQ:
i = mma8452_get_odr_index(data);
@@ -420,16 +435,16 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
switch (info) {
case IIO_EV_INFO_VALUE:
ret = i2c_smbus_read_byte_data(data->client,
- MMA8452_TRANSIENT_THS);
+ data->chip_info->ev_ths);
if (ret < 0)
return ret;
- *val = ret & MMA8452_TRANSIENT_THS_MASK;
+ *val = ret & data->chip_info->ev_ths_mask;
return IIO_VAL_INT;
case IIO_EV_INFO_PERIOD:
ret = i2c_smbus_read_byte_data(data->client,
- MMA8452_TRANSIENT_COUNT);
+ data->chip_info->ev_count);
if (ret < 0)
return ret;
@@ -472,8 +487,11 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
switch (info) {
case IIO_EV_INFO_VALUE:
- return mma8452_change_config(data, MMA8452_TRANSIENT_THS,
- val & MMA8452_TRANSIENT_THS_MASK);
+ if (val < 0 || val > 127) /* LSB 0.6178 m/s^2 */
+ return -EINVAL;
+
+ return mma8452_change_config(data, data->chip_info->ev_ths,
+ val);
case IIO_EV_INFO_PERIOD:
steps = (val * USEC_PER_SEC + val2) /
@@ -483,7 +501,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
if (steps > 0xff)
return -EINVAL;
- return mma8452_change_config(data, MMA8452_TRANSIENT_COUNT,
+ return mma8452_change_config(data, data->chip_info->ev_count,
steps);
case IIO_EV_INFO_HIGH_PASS_FILTER_3DB:
reg = i2c_smbus_read_byte_data(data->client,
@@ -512,13 +530,15 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev,
enum iio_event_direction dir)
{
struct mma8452_data *data = iio_priv(indio_dev);
+ const struct mma_chip_info *chip = data->chip_info;
int ret;
- ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
+ ret = i2c_smbus_read_byte_data(data->client,
+ data->chip_info->ev_cfg);
if (ret < 0)
return ret;
- return ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index) ? 1 : 0;
+ return ret & BIT(chan->scan_index + chip->ev_cfg_chan_shift) ? 1 : 0;
}
static int mma8452_write_event_config(struct iio_dev *indio_dev,
@@ -528,20 +548,21 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
int state)
{
struct mma8452_data *data = iio_priv(indio_dev);
+ const struct mma_chip_info *chip = data->chip_info;
int val;
- val = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
+ val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
if (val < 0)
return val;
if (state)
- val |= MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index);
+ val |= BIT(chan->scan_index + chip->ev_cfg_chan_shift);
else
- val &= ~MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index);
+ val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
val |= MMA8452_TRANSIENT_CFG_ELE;
- return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val);
+ return mma8452_change_config(data, chip->ev_cfg, val);
}
static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
@@ -550,25 +571,25 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
s64 ts = iio_get_time_ns();
int src;
- src = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_SRC);
+ src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src);
if (src < 0)
return;
- if (src & MMA8452_TRANSIENT_SRC_XTRANSE)
+ if (src & data->chip_info->ev_src_xe)
iio_push_event(indio_dev,
IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
IIO_EV_TYPE_THRESH,
IIO_EV_DIR_RISING),
ts);
- if (src & MMA8452_TRANSIENT_SRC_YTRANSE)
+ if (src & data->chip_info->ev_src_ye)
iio_push_event(indio_dev,
IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
IIO_EV_TYPE_THRESH,
IIO_EV_DIR_RISING),
ts);
- if (src & MMA8452_TRANSIENT_SRC_ZTRANSE)
+ if (src & data->chip_info->ev_src_ze)
iio_push_event(indio_dev,
IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
IIO_EV_TYPE_THRESH,
@@ -669,7 +690,7 @@ static struct attribute_group mma8452_event_attribute_group = {
.name = "events",
};
-#define MMA8452_CHANNEL(axis, idx) { \
+#define MMA8452_CHANNEL(axis, idx, bits) { \
.type = IIO_ACCEL, \
.modified = 1, \
.channel2 = IIO_MOD_##axis, \
@@ -681,9 +702,9 @@ static struct attribute_group mma8452_event_attribute_group = {
.scan_index = idx, \
.scan_type = { \
.sign = 's', \
- .realbits = 12, \
+ .realbits = (bits), \
.storagebits = 16, \
- .shift = 4, \
+ .shift = 16 - (bits), \
.endianness = IIO_BE, \
}, \
.event_spec = mma8452_transient_event, \
@@ -691,12 +712,42 @@ static struct attribute_group mma8452_event_attribute_group = {
}
static const struct iio_chan_spec mma8452_channels[] = {
- MMA8452_CHANNEL(X, 0),
- MMA8452_CHANNEL(Y, 1),
- MMA8452_CHANNEL(Z, 2),
+ MMA8452_CHANNEL(X, 0, 12),
+ MMA8452_CHANNEL(Y, 1, 12),
+ MMA8452_CHANNEL(Z, 2, 12),
IIO_CHAN_SOFT_TIMESTAMP(3),
};
+enum {
+ mma8452,
+};
+
+/*
+ * Hardware has fullscale of -2G, -4G, -8G corresponding to raw value -2048
+ * for 12 bit or -512 for 10 bit.
+ * The userspace interface uses m/s^2 and we declare micro units
+ * So scale factor is given by:
+ * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665
+ */
+
+static const struct mma_chip_info mma_chip_info_table[] = {
+ [mma8452] = {
+ .chip_id = MMA8452_DEVICE_ID,
+ .channels = mma8452_channels,
+ .num_channels = ARRAY_SIZE(mma8452_channels),
+ .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} },
+ .ev_cfg = MMA8452_TRANSIENT_CFG,
+ .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
+ .ev_cfg_chan_shift = 1,
+ .ev_src = MMA8452_TRANSIENT_SRC,
+ .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE,
+ .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE,
+ .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE,
+ .ev_ths = MMA8452_TRANSIENT_THS,
+ .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
+ .ev_count = MMA8452_TRANSIENT_COUNT,
+ },
+};
static struct attribute *mma8452_attributes[] = {
&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
@@ -813,12 +864,18 @@ static int mma8452_reset(struct i2c_client *client)
return -ETIMEDOUT;
}
+static const struct of_device_id mma8452_dt_ids[] = {
+ { .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
+ { }
+};
+
static int mma8452_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct mma8452_data *data;
struct iio_dev *indio_dev;
int ret;
+ const struct of_device_id *match;
ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
if (ret < 0)
@@ -826,21 +883,28 @@ static int mma8452_probe(struct i2c_client *client,
if (ret != MMA8452_DEVICE_ID)
return -ENODEV;
+ match = of_match_device(mma8452_dt_ids, &client->dev);
+ if (!match) {
+ dev_err(&client->dev, "unknown device model\n");
+ return -ENODEV;
+ }
+
indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
if (!indio_dev)
return -ENOMEM;
data = iio_priv(indio_dev);
data->client = client;
+ data->chip_info = match->data;
mutex_init(&data->lock);
i2c_set_clientdata(client, indio_dev);
indio_dev->info = &mma8452_info;
- indio_dev->name = id->name;
+ indio_dev->name = DRIVER_NAME;
indio_dev->dev.parent = &client->dev;
indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->channels = mma8452_channels;
- indio_dev->num_channels = ARRAY_SIZE(mma8452_channels);
+ indio_dev->channels = data->chip_info->channels;
+ indio_dev->num_channels = data->chip_info->num_channels;
indio_dev->available_scan_masks = mma8452_scan_masks;
ret = mma8452_reset(client);
@@ -959,19 +1023,14 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
#endif
static const struct i2c_device_id mma8452_id[] = {
- { "mma8452", 0 },
+ { "mma8452", mma8452 },
{ }
};
MODULE_DEVICE_TABLE(i2c, mma8452_id);
-static const struct of_device_id mma8452_dt_ids[] = {
- { .compatible = "fsl,mma8452" },
- { }
-};
-
static struct i2c_driver mma8452_driver = {
.driver = {
- .name = "mma8452",
+ .name = DRIVER_NAME,
.of_match_table = of_match_ptr(mma8452_dt_ids),
.pm = MMA8452_PM_OPS,
},
--
2.1.4
This adds support for the 10 bit version if Freescale's accelerometers
of this series. The datasheet is available at Freescale's website:
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8453Q.pdf
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
drivers/iio/accel/Kconfig | 6 +++---
drivers/iio/accel/mma8452.c | 41 ++++++++++++++++++++++++++++++++++++++---
2 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 00e7bcb..91fab16 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -87,13 +87,13 @@ config KXSD9
will be called kxsd9.
config MMA8452
- tristate "Freescale MMA8452Q Accelerometer Driver"
+ tristate "Freescale MMA8452 Accelerometer Driver"
depends on I2C
select IIO_BUFFER
select IIO_TRIGGERED_BUFFER
help
- Say yes here to build support for the Freescale MMA8452Q 3-axis
- accelerometer.
+ Say yes here to build support for the following Freescale 3-axis
+ accelerometers: MMA8452Q, MMA8453Q.
To compile this driver as a module, choose M here: the module
will be called mma8452.
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index b2ebfcb..209b0d7 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1,5 +1,8 @@
/*
- * mma8452.c - Support for Freescale MMA8452Q 3-axis 12-bit accelerometer
+ * mma8452.c - Support for following Freescale 3-axis accelerometers:
+ *
+ * MMA8452Q
+ * MMA8453Q
*
* Copyright 2014 Peter Meerwald <[email protected]>
*
@@ -27,7 +30,7 @@
#define DRIVER_NAME "mma8452"
#define MMA8452_STATUS 0x00
-#define MMA8452_OUT_X 0x01 /* MSB first, 12-bit */
+#define MMA8452_OUT_X 0x01 /* MSB first, 10 or 12-bit */
#define MMA8452_OUT_Y 0x03
#define MMA8452_OUT_Z 0x05
#define MMA8452_INT_SRC 0x0c
@@ -74,6 +77,7 @@
#define MMA8452_INT_TRANS BIT(5)
#define MMA8452_DEVICE_ID 0x2a
+#define MMA8453_DEVICE_ID 0x3a
struct mma8452_data {
struct i2c_client *client;
@@ -718,8 +722,16 @@ static const struct iio_chan_spec mma8452_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(3),
};
+static const struct iio_chan_spec mma8453_channels[] = {
+ MMA8452_CHANNEL(X, 0, 10),
+ MMA8452_CHANNEL(Y, 1, 10),
+ MMA8452_CHANNEL(Z, 2, 10),
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
enum {
mma8452,
+ mma8453,
};
/*
@@ -747,7 +759,24 @@ static const struct mma_chip_info mma_chip_info_table[] = {
.ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
.ev_count = MMA8452_TRANSIENT_COUNT,
},
+ [mma8453] = {
+ .chip_id = MMA8453_DEVICE_ID,
+ .channels = mma8453_channels,
+ .num_channels = ARRAY_SIZE(mma8453_channels),
+ .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} },
+ .ev_cfg = MMA8452_TRANSIENT_CFG,
+ .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
+ .ev_cfg_chan_shift = 1,
+ .ev_src = MMA8452_TRANSIENT_SRC,
+ .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE,
+ .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE,
+ .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE,
+ .ev_ths = MMA8452_TRANSIENT_THS,
+ .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
+ .ev_count = MMA8452_TRANSIENT_COUNT,
+ },
};
+
static struct attribute *mma8452_attributes[] = {
&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
@@ -866,6 +895,7 @@ static int mma8452_reset(struct i2c_client *client)
static const struct of_device_id mma8452_dt_ids[] = {
{ .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
+ { .compatible = "fsl,mma8453", .data = &mma_chip_info_table[mma8453] },
{ }
};
@@ -880,7 +910,8 @@ static int mma8452_probe(struct i2c_client *client,
ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
if (ret < 0)
return ret;
- if (ret != MMA8452_DEVICE_ID)
+
+ if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID)
return -ENODEV;
match = of_match_device(mma8452_dt_ids, &client->dev);
@@ -898,6 +929,9 @@ static int mma8452_probe(struct i2c_client *client,
data->chip_info = match->data;
mutex_init(&data->lock);
+ dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n",
+ match->compatible, data->chip_info->chip_id);
+
i2c_set_clientdata(client, indio_dev);
indio_dev->info = &mma8452_info;
indio_dev->name = DRIVER_NAME;
@@ -1024,6 +1058,7 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
static const struct i2c_device_id mma8452_id[] = {
{ "mma8452", mma8452 },
+ { "mma8453", mma8453 },
{ }
};
MODULE_DEVICE_TABLE(i2c, mma8452_id);
--
2.1.4
This adds the freefall / motion interrupt source definitions to the driver.
It is not in use now, but mma_chip_info and iio_chan_spec can easily be
adapted to use it instead of the transient interrupt source.
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
drivers/iio/accel/mma8452.c | 45 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 209b0d7..0498b6e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -38,9 +38,18 @@
#define MMA8452_DATA_CFG 0x0e
#define MMA8452_HP_FILTER_CUTOFF 0x0f
#define MMA8452_HP_FILTER_CUTOFF_SEL_MASK (BIT(0) | BIT(1))
+#define MMA8452_FF_MT_CFG 0x15
+#define MMA8452_FF_MT_CFG_OAE BIT(6)
+#define MMA8452_FF_MT_CFG_ELE BIT(7)
+#define MMA8452_FF_MT_SRC 0x16
+#define MMA8452_FF_MT_SRC_XHE BIT(1)
+#define MMA8452_FF_MT_SRC_YHE BIT(3)
+#define MMA8452_FF_MT_SRC_ZHE BIT(5)
+#define MMA8452_FF_MT_THS 0x17
+#define MMA8452_FF_MT_THS_MASK 0x7f
+#define MMA8452_FF_MT_COUNT 0x18
#define MMA8452_TRANSIENT_CFG 0x1d
#define MMA8452_TRANSIENT_CFG_ELE BIT(4)
-#define MMA8452_TRANSIENT_CFG_CHAN(chan) BIT(chan + 1)
#define MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0)
#define MMA8452_TRANSIENT_SRC 0x1e
#define MMA8452_TRANSIENT_SRC_XTRANSE BIT(1)
@@ -74,6 +83,7 @@
#define MMA8452_DATA_CFG_HPF_MASK BIT(4)
#define MMA8452_INT_DRDY BIT(0)
+#define MMA8452_INT_FF_MT BIT(2)
#define MMA8452_INT_TRANS BIT(5)
#define MMA8452_DEVICE_ID 0x2a
@@ -564,7 +574,8 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
else
val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
- val |= MMA8452_TRANSIENT_CFG_ELE;
+ val |= chip->ev_cfg_ele;
+ val |= MMA8452_FF_MT_CFG_OAE;
return mma8452_change_config(data, chip->ev_cfg, val);
}
@@ -605,6 +616,7 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
{
struct iio_dev *indio_dev = p;
struct mma8452_data *data = iio_priv(indio_dev);
+ const struct mma_chip_info *chip = data->chip_info;
int ret = IRQ_NONE;
int src;
@@ -617,7 +629,10 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
ret = IRQ_HANDLED;
}
- if (src & MMA8452_INT_TRANS) {
+ if ((src & MMA8452_INT_TRANS &&
+ chip->ev_src == MMA8452_TRANSIENT_SRC) ||
+ (src & MMA8452_INT_FF_MT &&
+ chip->ev_src == MMA8452_FF_MT_SRC)) {
mma8452_transient_interrupt(indio_dev);
ret = IRQ_HANDLED;
}
@@ -678,6 +693,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
},
};
+static const struct iio_event_spec mma8452_motion_event[] = {
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_separate = BIT(IIO_EV_INFO_ENABLE),
+ .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_PERIOD)
+ },
+};
+
/*
* Threshold is configured in fixed 8G/127 steps regardless of
* currently selected scale for measurement.
@@ -962,13 +987,15 @@ static int mma8452_probe(struct i2c_client *client,
if (client->irq) {
/*
- * Although we enable the transient interrupt source once and
- * for all here the transient event detection itself is not
- * enabled until userspace asks for it by
- * mma8452_write_event_config()
+ * Although we enable the interrupt sources once and for
+ * all here the event detection itself is not enabled until
+ * userspace asks for it by mma8452_write_event_config()
*/
- int supported_interrupts = MMA8452_INT_DRDY | MMA8452_INT_TRANS;
- int enabled_interrupts = MMA8452_INT_TRANS;
+ int supported_interrupts = MMA8452_INT_DRDY |
+ MMA8452_INT_TRANS |
+ MMA8452_INT_FF_MT;
+ int enabled_interrupts = MMA8452_INT_TRANS |
+ MMA8452_INT_FF_MT;
/* Assume wired to INT1 pin */
ret = i2c_smbus_write_byte_data(client,
--
2.1.4
MMA8652FC and MMA8653FC don't provide the transient interrupt source, so
the motion interrupt source is used by providing a new iio_chan_spec
definition, so that other supported devices are not affected by this.
Datasheets for the newly supported devices are available at Freescale's
website:
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8652FC.pdf
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8653FC.pdf
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
drivers/iio/accel/Kconfig | 2 +-
drivers/iio/accel/mma8452.c | 79 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 79 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 91fab16..684c8b5 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -93,7 +93,7 @@ config MMA8452
select IIO_TRIGGERED_BUFFER
help
Say yes here to build support for the following Freescale 3-axis
- accelerometers: MMA8452Q, MMA8453Q.
+ accelerometers: MMA8452Q, MMA8453Q, MMA8652FC, MMA8653FC.
To compile this driver as a module, choose M here: the module
will be called mma8452.
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 0498b6e..ccce925 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -3,6 +3,8 @@
*
* MMA8452Q
* MMA8453Q
+ * MMA8652FC
+ * MMA8653FC
*
* Copyright 2014 Peter Meerwald <[email protected]>
*
@@ -88,6 +90,8 @@
#define MMA8452_DEVICE_ID 0x2a
#define MMA8453_DEVICE_ID 0x3a
+#define MMA8652_DEVICE_ID 0x4a
+#define MMA8653_DEVICE_ID 0x5a
struct mma8452_data {
struct i2c_client *client;
@@ -740,6 +744,26 @@ static struct attribute_group mma8452_event_attribute_group = {
.num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
}
+#define MMA8652_CHANNEL(axis, idx, bits) { \
+ .type = IIO_ACCEL, \
+ .modified = 1, \
+ .channel2 = IIO_MOD_##axis, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_CALIBBIAS), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_index = idx, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = (bits), \
+ .storagebits = 16, \
+ .shift = 16 - (bits), \
+ .endianness = IIO_BE, \
+ }, \
+ .event_spec = mma8452_motion_event, \
+ .num_event_specs = ARRAY_SIZE(mma8452_motion_event), \
+}
+
static const struct iio_chan_spec mma8452_channels[] = {
MMA8452_CHANNEL(X, 0, 12),
MMA8452_CHANNEL(Y, 1, 12),
@@ -754,9 +778,25 @@ static const struct iio_chan_spec mma8453_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(3),
};
+static const struct iio_chan_spec mma8652_channels[] = {
+ MMA8652_CHANNEL(X, 0, 12),
+ MMA8652_CHANNEL(Y, 1, 12),
+ MMA8652_CHANNEL(Z, 2, 12),
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const struct iio_chan_spec mma8653_channels[] = {
+ MMA8652_CHANNEL(X, 0, 10),
+ MMA8652_CHANNEL(Y, 1, 10),
+ MMA8652_CHANNEL(Z, 2, 10),
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
enum {
mma8452,
mma8453,
+ mma8652,
+ mma8653,
};
/*
@@ -800,6 +840,38 @@ static const struct mma_chip_info mma_chip_info_table[] = {
.ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
.ev_count = MMA8452_TRANSIENT_COUNT,
},
+ [mma8652] = {
+ .chip_id = MMA8652_DEVICE_ID,
+ .channels = mma8652_channels,
+ .num_channels = ARRAY_SIZE(mma8652_channels),
+ .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} },
+ .ev_cfg = MMA8452_FF_MT_CFG,
+ .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
+ .ev_cfg_chan_shift = 3,
+ .ev_src = MMA8452_FF_MT_SRC,
+ .ev_src_xe = MMA8452_FF_MT_SRC_XHE,
+ .ev_src_ye = MMA8452_FF_MT_SRC_YHE,
+ .ev_src_ze = MMA8452_FF_MT_SRC_ZHE,
+ .ev_ths = MMA8452_FF_MT_THS,
+ .ev_ths_mask = MMA8452_FF_MT_THS_MASK,
+ .ev_count = MMA8452_FF_MT_COUNT,
+ },
+ [mma8653] = {
+ .chip_id = MMA8653_DEVICE_ID,
+ .channels = mma8653_channels,
+ .num_channels = ARRAY_SIZE(mma8653_channels),
+ .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} },
+ .ev_cfg = MMA8452_FF_MT_CFG,
+ .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
+ .ev_cfg_chan_shift = 3,
+ .ev_src = MMA8452_FF_MT_SRC,
+ .ev_src_xe = MMA8452_FF_MT_SRC_XHE,
+ .ev_src_ye = MMA8452_FF_MT_SRC_YHE,
+ .ev_src_ze = MMA8452_FF_MT_SRC_ZHE,
+ .ev_ths = MMA8452_FF_MT_THS,
+ .ev_ths_mask = MMA8452_FF_MT_THS_MASK,
+ .ev_count = MMA8452_FF_MT_COUNT,
+ },
};
static struct attribute *mma8452_attributes[] = {
@@ -921,6 +993,8 @@ static int mma8452_reset(struct i2c_client *client)
static const struct of_device_id mma8452_dt_ids[] = {
{ .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
{ .compatible = "fsl,mma8453", .data = &mma_chip_info_table[mma8453] },
+ { .compatible = "fsl,mma8652", .data = &mma_chip_info_table[mma8652] },
+ { .compatible = "fsl,mma8653", .data = &mma_chip_info_table[mma8653] },
{ }
};
@@ -936,7 +1010,8 @@ static int mma8452_probe(struct i2c_client *client,
if (ret < 0)
return ret;
- if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID)
+ if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID &&
+ ret != MMA8652_DEVICE_ID && ret != MMA8653_DEVICE_ID)
return -ENODEV;
match = of_match_device(mma8452_dt_ids, &client->dev);
@@ -1086,6 +1161,8 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
static const struct i2c_device_id mma8452_id[] = {
{ "mma8452", mma8452 },
{ "mma8453", mma8453 },
+ { "mma8652", mma8652 },
+ { "mma8653", mma8653 },
{ }
};
MODULE_DEVICE_TABLE(i2c, mma8452_id);
--
2.1.4
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
.../devicetree/bindings/iio/accel/mma8452.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt
diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
new file mode 100644
index 0000000..8d98e05
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -0,0 +1,21 @@
+Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
+
+Required properties:
+
+ - compatible: should be "fsl,mma8653", "fsl,mma8652", "fsl,mma8453" or
+ "fsl,mma8452" respectively.
+ - reg: the I2C address of the chip
+
+Optional properties:
+
+ - interrupt-parent: should be the phandle for the interrupt controller
+ - interrupts: interrupt mapping for GPIO IRQ
+
+Example:
+
+ mma8653fc@1d {
+ compatible = "fsl,mma8653";
+ reg = <0x1d>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <5 0>;
+ };
--
2.1.4
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
drivers/iio/accel/mma8452.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index ccce925..7f6e3b4 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -6,6 +6,7 @@
* MMA8652FC
* MMA8653FC
*
+ * Copyright 2015 Martin Kepplinger <[email protected]>
* Copyright 2014 Peter Meerwald <[email protected]>
*
* This file is subject to the terms and conditions of version 2 of
--
2.1.4
IIO_EV_TYPE_THRESH in rising direction describes an event where the
threshold is crossed in rising direction, positive or negative values
being possible. This is not the case here.
Since the threshold is no signed value and only the magnitude is compared,
IIO_EV_TYPE_MAG is what describes the behaviour of these devices, see the
sysfs-bus-iio ABI Documentation.
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
drivers/iio/accel/mma8452.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 7f6e3b4..e23ebd0 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -598,21 +598,21 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
if (src & data->chip_info->ev_src_xe)
iio_push_event(indio_dev,
IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
- IIO_EV_TYPE_THRESH,
+ IIO_EV_TYPE_MAG,
IIO_EV_DIR_RISING),
ts);
if (src & data->chip_info->ev_src_ye)
iio_push_event(indio_dev,
IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
- IIO_EV_TYPE_THRESH,
+ IIO_EV_TYPE_MAG,
IIO_EV_DIR_RISING),
ts);
if (src & data->chip_info->ev_src_ze)
iio_push_event(indio_dev,
IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
- IIO_EV_TYPE_THRESH,
+ IIO_EV_TYPE_MAG,
IIO_EV_DIR_RISING),
ts);
}
@@ -689,7 +689,7 @@ static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
static const struct iio_event_spec mma8452_transient_event[] = {
{
- .type = IIO_EV_TYPE_THRESH,
+ .type = IIO_EV_TYPE_MAG,
.dir = IIO_EV_DIR_RISING,
.mask_separate = BIT(IIO_EV_INFO_ENABLE),
.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
@@ -700,7 +700,7 @@ static const struct iio_event_spec mma8452_transient_event[] = {
static const struct iio_event_spec mma8452_motion_event[] = {
{
- .type = IIO_EV_TYPE_THRESH,
+ .type = IIO_EV_TYPE_MAG,
.dir = IIO_EV_DIR_RISING,
.mask_separate = BIT(IIO_EV_INFO_ENABLE),
.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
--
2.1.4
This doesn't actually change anything since the core names the sysfs folder
for the iio event attributes "events" anyways. It only leaves the job to the
core.
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
drivers/iio/accel/mma8452.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index e23ebd0..2491ed0 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -721,7 +721,6 @@ static struct attribute *mma8452_event_attributes[] = {
static struct attribute_group mma8452_event_attribute_group = {
.attrs = mma8452_event_attributes,
- .name = "events",
};
#define MMA8452_CHANNEL(axis, idx, bits) { \
--
2.1.4
For the devices supported by the mma8452 driver, two interrupt pins are
available to route the interrupt signals to. By default INT1 is assumed.
This adds a simple boolean DT property, for users to configure it for
INT2, if that is the wired interrupt pin for them.
This is important for everyone to be able to use this driver, no matter
how their chip is wired.
Since this doesn't change the default behaviour, it doesn't break anything
for existing users.
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
Documentation/devicetree/bindings/iio/accel/mma8452.txt | 2 ++
drivers/iio/accel/mma8452.c | 14 ++++++++------
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
index 8d98e05..0048415 100644
--- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
+++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
@@ -10,6 +10,7 @@ Optional properties:
- interrupt-parent: should be the phandle for the interrupt controller
- interrupts: interrupt mapping for GPIO IRQ
+ - int2: assume interrupt pin wired to INT2 instead of INT1
Example:
@@ -18,4 +19,5 @@ Example:
reg = <0x1d>;
interrupt-parent = <&gpio1>;
interrupts = <5 0>;
+ int2;
};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 2491ed0..0768e66 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1072,12 +1072,14 @@ static int mma8452_probe(struct i2c_client *client,
int enabled_interrupts = MMA8452_INT_TRANS |
MMA8452_INT_FF_MT;
- /* Assume wired to INT1 pin */
- ret = i2c_smbus_write_byte_data(client,
- MMA8452_CTRL_REG5,
- supported_interrupts);
- if (ret < 0)
- return ret;
+ /* Assume wired to INT1 pin, except "int2" is found in DT */
+ if (!of_property_read_bool(client->dev.of_node, "int2")) {
+ ret = i2c_smbus_write_byte_data(client,
+ MMA8452_CTRL_REG5,
+ supported_interrupts);
+ if (ret < 0)
+ return ret;
+ }
ret = i2c_smbus_write_byte_data(client,
MMA8452_CTRL_REG4,
--
2.1.4
On 04/07/15 14:55, Martin Kepplinger wrote:
> This adds a struct mma_chip_info to hold data that will remain specific to
> the chip in use. It is provided during probe() and linked in
> struct of_device_id.
>
> Also this suggests that the driver is called "mma8452" and now handles the
> MMA8452Q device, but is not limited to it.
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> Signed-off-by: Christoph Muellner <[email protected]>
Hi Martin,
Sorry for my lack of reply on this. Stupid week of insane hours at work.
Just about awake again now. Anyhow, only major in here is the change
to what goes in indio_dev->name which should be the part name, not the
driver one.
Otherwise, few minor bits and bobs inline that you can pretty much ignore
if you like.
Jonathan
> ---
> drivers/iio/accel/mma8452.c | 161 ++++++++++++++++++++++++++++++--------------
> 1 file changed, 110 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index e8e2077..b2ebfcb 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -23,7 +23,9 @@
> #include <linux/iio/triggered_buffer.h>
> #include <linux/iio/events.h>
> #include <linux/delay.h>
> +#include <linux/of_device.h>
>
> +#define DRIVER_NAME "mma8452"
> #define MMA8452_STATUS 0x00
> #define MMA8452_OUT_X 0x01 /* MSB first, 12-bit */
> #define MMA8452_OUT_Y 0x03
> @@ -78,6 +80,25 @@ struct mma8452_data {
> struct mutex lock;
> u8 ctrl_reg1;
> u8 data_cfg;
> + const struct mma_chip_info *chip_info;
> +};
> +
This is big an uggly enough that I'd be tempted to add some formal
(e.g. kerneldoc) documentation to the structure. Not vital, but
nice to have!
> +struct mma_chip_info {
> + u8 chip_id;
> + const struct iio_chan_spec *channels;
> + int num_channels;
> + /* 3 modes: 2g, 4g, 8g; 2 ints: m/s^2 and micro m/s^2 */
> + const int mma_scales[3][2];
> + u8 ev_cfg;
> + u8 ev_cfg_ele;
> + u8 ev_cfg_chan_shift;
> + u8 ev_src;
> + u8 ev_src_xe;
> + u8 ev_src_ye;
> + u8 ev_src_ze;
> + u8 ev_ths;
> + u8 ev_ths_mask;
> + u8 ev_count;
> };
>
> static int mma8452_drdy(struct mma8452_data *data)
> @@ -143,16 +164,6 @@ static const int mma8452_samp_freq[8][2] = {
> {6, 250000}, {1, 560000}
> };
>
> -/*
> - * Hardware has fullscale of -2G, -4G, -8G corresponding to raw value -2048
> - * The userspace interface uses m/s^2 and we declare micro units
> - * So scale factor is given by:
> - * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665
> - */
> -static const int mma8452_scales[3][2] = {
> - {0, 9577}, {0, 19154}, {0, 38307}
> -};
> -
> /* Datasheet table 35 (step time vs sample frequency) */
> static const int mma8452_transient_time_step_us[8] = {
> 1250,
> @@ -187,8 +198,11 @@ static ssize_t mma8452_show_samp_freq_avail(struct device *dev,
> static ssize_t mma8452_show_scale_avail(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return mma8452_show_int_plus_micros(buf, mma8452_scales,
> - ARRAY_SIZE(mma8452_scales));
> + struct mma8452_data *data = iio_priv(i2c_get_clientdata(
> + to_i2c_client(dev)));
> +
> + return mma8452_show_int_plus_micros(buf, data->chip_info->mma_scales,
> + ARRAY_SIZE(data->chip_info->mma_scales));
> }
>
> static ssize_t mma8452_show_hp_cutoff_avail(struct device *dev,
> @@ -219,8 +233,8 @@ static int mma8452_get_samp_freq_index(struct mma8452_data *data,
> static int mma8452_get_scale_index(struct mma8452_data *data,
> int val, int val2)
> {
> - return mma8452_get_int_plus_micros_index(mma8452_scales,
> - ARRAY_SIZE(mma8452_scales), val, val2);
> + return mma8452_get_int_plus_micros_index(data->chip_info->mma_scales,
> + ARRAY_SIZE(data->chip_info->mma_scales), val, val2);
> }
>
> static int mma8452_get_hp_filter_index(struct mma8452_data *data,
> @@ -229,7 +243,7 @@ static int mma8452_get_hp_filter_index(struct mma8452_data *data,
> int i = mma8452_get_odr_index(data);
>
> return mma8452_get_int_plus_micros_index(mma8452_hp_filter_cutoff[i],
> - ARRAY_SIZE(mma8452_scales[0]), val, val2);
> + ARRAY_SIZE(data->chip_info->mma_scales[0]), val, val2);
> }
>
> static int mma8452_read_hp_filter(struct mma8452_data *data, int *hz, int *uHz)
> @@ -266,13 +280,14 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
> mutex_unlock(&data->lock);
> if (ret < 0)
> return ret;
> - *val = sign_extend32(
> - be16_to_cpu(buffer[chan->scan_index]) >> 4, 11);
> + *val = sign_extend32(be16_to_cpu(
> + buffer[chan->scan_index]) >> chan->scan_type.shift,
> + chan->scan_type.realbits - 1);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> i = data->data_cfg & MMA8452_DATA_CFG_FS_MASK;
> - *val = mma8452_scales[i][0];
> - *val2 = mma8452_scales[i][1];
> + *val = data->chip_info->mma_scales[i][0];
> + *val2 = data->chip_info->mma_scales[i][1];
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_CHAN_INFO_SAMP_FREQ:
> i = mma8452_get_odr_index(data);
> @@ -420,16 +435,16 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
> switch (info) {
> case IIO_EV_INFO_VALUE:
> ret = i2c_smbus_read_byte_data(data->client,
> - MMA8452_TRANSIENT_THS);
> + data->chip_info->ev_ths);
> if (ret < 0)
> return ret;
>
> - *val = ret & MMA8452_TRANSIENT_THS_MASK;
> + *val = ret & data->chip_info->ev_ths_mask;
> return IIO_VAL_INT;
>
> case IIO_EV_INFO_PERIOD:
> ret = i2c_smbus_read_byte_data(data->client,
> - MMA8452_TRANSIENT_COUNT);
> + data->chip_info->ev_count);
> if (ret < 0)
> return ret;
>
> @@ -472,8 +487,11 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
>
> switch (info) {
> case IIO_EV_INFO_VALUE:
> - return mma8452_change_config(data, MMA8452_TRANSIENT_THS,
> - val & MMA8452_TRANSIENT_THS_MASK);
> + if (val < 0 || val > 127) /* LSB 0.6178 m/s^2 */
> + return -EINVAL;
> +
> + return mma8452_change_config(data, data->chip_info->ev_ths,
> + val);
>
> case IIO_EV_INFO_PERIOD:
> steps = (val * USEC_PER_SEC + val2) /
> @@ -483,7 +501,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
> if (steps > 0xff)
> return -EINVAL;
>
> - return mma8452_change_config(data, MMA8452_TRANSIENT_COUNT,
> + return mma8452_change_config(data, data->chip_info->ev_count,
> steps);
> case IIO_EV_INFO_HIGH_PASS_FILTER_3DB:
> reg = i2c_smbus_read_byte_data(data->client,
> @@ -512,13 +530,15 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev,
> enum iio_event_direction dir)
> {
> struct mma8452_data *data = iio_priv(indio_dev);
> + const struct mma_chip_info *chip = data->chip_info;
> int ret;
>
> - ret = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
> + ret = i2c_smbus_read_byte_data(data->client,
> + data->chip_info->ev_cfg);
> if (ret < 0)
> return ret;
>
> - return ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index) ? 1 : 0;
> + return ret & BIT(chan->scan_index + chip->ev_cfg_chan_shift) ? 1 : 0;
Could tidy this up a bit whilst here if you like (I don't mind if you do
or not ;)
return !!(ret & BIT(chan->scan_index + chip->ev_cfg_chan_shift)) is
my personal slight preferene.
> }
>
> static int mma8452_write_event_config(struct iio_dev *indio_dev,
> @@ -528,20 +548,21 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
> int state)
> {
> struct mma8452_data *data = iio_priv(indio_dev);
> + const struct mma_chip_info *chip = data->chip_info;
> int val;
>
> - val = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_CFG);
> + val = i2c_smbus_read_byte_data(data->client, chip->ev_cfg);
> if (val < 0)
> return val;
>
> if (state)
> - val |= MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index);
> + val |= BIT(chan->scan_index + chip->ev_cfg_chan_shift);
> else
> - val &= ~MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index);
> + val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
>
> val |= MMA8452_TRANSIENT_CFG_ELE;
>
> - return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, val);
> + return mma8452_change_config(data, chip->ev_cfg, val);
> }
>
> static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
> @@ -550,25 +571,25 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
> s64 ts = iio_get_time_ns();
> int src;
>
> - src = i2c_smbus_read_byte_data(data->client, MMA8452_TRANSIENT_SRC);
> + src = i2c_smbus_read_byte_data(data->client, data->chip_info->ev_src);
> if (src < 0)
> return;
>
> - if (src & MMA8452_TRANSIENT_SRC_XTRANSE)
> + if (src & data->chip_info->ev_src_xe)
> iio_push_event(indio_dev,
> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
> IIO_EV_TYPE_THRESH,
> IIO_EV_DIR_RISING),
> ts);
>
> - if (src & MMA8452_TRANSIENT_SRC_YTRANSE)
> + if (src & data->chip_info->ev_src_ye)
> iio_push_event(indio_dev,
> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
> IIO_EV_TYPE_THRESH,
> IIO_EV_DIR_RISING),
> ts);
>
> - if (src & MMA8452_TRANSIENT_SRC_ZTRANSE)
> + if (src & data->chip_info->ev_src_ze)
> iio_push_event(indio_dev,
> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
> IIO_EV_TYPE_THRESH,
> @@ -669,7 +690,7 @@ static struct attribute_group mma8452_event_attribute_group = {
> .name = "events",
> };
>
> -#define MMA8452_CHANNEL(axis, idx) { \
> +#define MMA8452_CHANNEL(axis, idx, bits) { \
> .type = IIO_ACCEL, \
> .modified = 1, \
> .channel2 = IIO_MOD_##axis, \
> @@ -681,9 +702,9 @@ static struct attribute_group mma8452_event_attribute_group = {
> .scan_index = idx, \
> .scan_type = { \
> .sign = 's', \
> - .realbits = 12, \
> + .realbits = (bits), \
> .storagebits = 16, \
> - .shift = 4, \
> + .shift = 16 - (bits), \
> .endianness = IIO_BE, \
> }, \
> .event_spec = mma8452_transient_event, \
> @@ -691,12 +712,42 @@ static struct attribute_group mma8452_event_attribute_group = {
> }
>
> static const struct iio_chan_spec mma8452_channels[] = {
> - MMA8452_CHANNEL(X, 0),
> - MMA8452_CHANNEL(Y, 1),
> - MMA8452_CHANNEL(Z, 2),
> + MMA8452_CHANNEL(X, 0, 12),
> + MMA8452_CHANNEL(Y, 1, 12),
> + MMA8452_CHANNEL(Z, 2, 12),
> IIO_CHAN_SOFT_TIMESTAMP(3),
> };
>
> +enum {
> + mma8452,
> +};
> +
> +/*
> + * Hardware has fullscale of -2G, -4G, -8G corresponding to raw value -2048
> + * for 12 bit or -512 for 10 bit.
> + * The userspace interface uses m/s^2 and we declare micro units
> + * So scale factor is given by:
> + * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665
> + */
No particularly problem with this comment, other than 'Why is it here?'
I'd move it down to just above the actual structure element below, then
it is obvious what it is talking about.
> +
> +static const struct mma_chip_info mma_chip_info_table[] = {
> + [mma8452] = {
> + .chip_id = MMA8452_DEVICE_ID,
Not used in this patch. Introduce it when it is :)
> + .channels = mma8452_channels,
> + .num_channels = ARRAY_SIZE(mma8452_channels),
> + .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} },
> + .ev_cfg = MMA8452_TRANSIENT_CFG,
> + .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
> + .ev_cfg_chan_shift = 1,
> + .ev_src = MMA8452_TRANSIENT_SRC,
> + .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE,
> + .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE,
> + .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE,
> + .ev_ths = MMA8452_TRANSIENT_THS,
> + .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
> + .ev_count = MMA8452_TRANSIENT_COUNT,
> + },
> +};
> static struct attribute *mma8452_attributes[] = {
> &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> &iio_dev_attr_in_accel_scale_available.dev_attr.attr,
> @@ -813,12 +864,18 @@ static int mma8452_reset(struct i2c_client *client)
> return -ETIMEDOUT;
> }
>
> +static const struct of_device_id mma8452_dt_ids[] = {
> + { .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
> + { }
> +};
> +
> static int mma8452_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> struct mma8452_data *data;
> struct iio_dev *indio_dev;
> int ret;
> + const struct of_device_id *match;
>
> ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
> if (ret < 0)
> @@ -826,21 +883,28 @@ static int mma8452_probe(struct i2c_client *client,
> if (ret != MMA8452_DEVICE_ID)
> return -ENODEV;
>
> + match = of_match_device(mma8452_dt_ids, &client->dev);
> + if (!match) {
> + dev_err(&client->dev, "unknown device model\n");
> + return -ENODEV;
> + }
> +
> indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> if (!indio_dev)
> return -ENOMEM;
>
> data = iio_priv(indio_dev);
> data->client = client;
> + data->chip_info = match->data;
> mutex_init(&data->lock);
>
> i2c_set_clientdata(client, indio_dev);
> indio_dev->info = &mma8452_info;
> - indio_dev->name = id->name;
> + indio_dev->name = DRIVER_NAME;
The name provided here should be the part name (as per id->name)
not the driver name.
> indio_dev->dev.parent = &client->dev;
> indio_dev->modes = INDIO_DIRECT_MODE;
> - indio_dev->channels = mma8452_channels;
> - indio_dev->num_channels = ARRAY_SIZE(mma8452_channels);
> + indio_dev->channels = data->chip_info->channels;
> + indio_dev->num_channels = data->chip_info->num_channels;
> indio_dev->available_scan_masks = mma8452_scan_masks;
>
> ret = mma8452_reset(client);
> @@ -959,19 +1023,14 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
> #endif
>
> static const struct i2c_device_id mma8452_id[] = {
> - { "mma8452", 0 },
> + { "mma8452", mma8452 },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, mma8452_id);
>
> -static const struct of_device_id mma8452_dt_ids[] = {
> - { .compatible = "fsl,mma8452" },
> - { }
> -};
> -
> static struct i2c_driver mma8452_driver = {
> .driver = {
> - .name = "mma8452",
> + .name = DRIVER_NAME,
> .of_match_table = of_match_ptr(mma8452_dt_ids),
> .pm = MMA8452_PM_OPS,
> },
>
On 04/07/15 14:55, Martin Kepplinger wrote:
> This adds support for the 10 bit version if Freescale's accelerometers
> of this series. The datasheet is available at Freescale's website:
>
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8453Q.pdf
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> Signed-off-by: Christoph Muellner <[email protected]>
Couple of minor bits inline.
> ---
> drivers/iio/accel/Kconfig | 6 +++---
> drivers/iio/accel/mma8452.c | 41 ++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 00e7bcb..91fab16 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -87,13 +87,13 @@ config KXSD9
> will be called kxsd9.
>
> config MMA8452
> - tristate "Freescale MMA8452Q Accelerometer Driver"
> + tristate "Freescale MMA8452 Accelerometer Driver"
Why the rename in the above menu item? Usual trick here
if the message is getting too long is to add 'and similar.' then
ensure all supported parts are as you have, clearly mentioned in the help.
> depends on I2C
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
> help
> - Say yes here to build support for the Freescale MMA8452Q 3-axis
> - accelerometer.
> + Say yes here to build support for the following Freescale 3-axis
> + accelerometers: MMA8452Q, MMA8453Q.
>
> To compile this driver as a module, choose M here: the module
> will be called mma8452.
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index b2ebfcb..209b0d7 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1,5 +1,8 @@
> /*
> - * mma8452.c - Support for Freescale MMA8452Q 3-axis 12-bit accelerometer
> + * mma8452.c - Support for following Freescale 3-axis accelerometers:
> + *
> + * MMA8452Q
> + * MMA8453Q
> *
> * Copyright 2014 Peter Meerwald <[email protected]>
> *
> @@ -27,7 +30,7 @@
>
> #define DRIVER_NAME "mma8452"
> #define MMA8452_STATUS 0x00
> -#define MMA8452_OUT_X 0x01 /* MSB first, 12-bit */
> +#define MMA8452_OUT_X 0x01 /* MSB first, 10 or 12-bit */
> #define MMA8452_OUT_Y 0x03
> #define MMA8452_OUT_Z 0x05
> #define MMA8452_INT_SRC 0x0c
> @@ -74,6 +77,7 @@
> #define MMA8452_INT_TRANS BIT(5)
>
> #define MMA8452_DEVICE_ID 0x2a
> +#define MMA8453_DEVICE_ID 0x3a
>
> struct mma8452_data {
> struct i2c_client *client;
> @@ -718,8 +722,16 @@ static const struct iio_chan_spec mma8452_channels[] = {
> IIO_CHAN_SOFT_TIMESTAMP(3),
> };
>
> +static const struct iio_chan_spec mma8453_channels[] = {
> + MMA8452_CHANNEL(X, 0, 10),
> + MMA8452_CHANNEL(Y, 1, 10),
> + MMA8452_CHANNEL(Z, 2, 10),
> + IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> enum {
> mma8452,
> + mma8453,
> };
>
> /*
> @@ -747,7 +759,24 @@ static const struct mma_chip_info mma_chip_info_table[] = {
> .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
> .ev_count = MMA8452_TRANSIENT_COUNT,
> },
> + [mma8453] = {
> + .chip_id = MMA8453_DEVICE_ID,
> + .channels = mma8453_channels,
> + .num_channels = ARRAY_SIZE(mma8453_channels),
> + .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} },
> + .ev_cfg = MMA8452_TRANSIENT_CFG,
> + .ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
> + .ev_cfg_chan_shift = 1,
> + .ev_src = MMA8452_TRANSIENT_SRC,
> + .ev_src_xe = MMA8452_TRANSIENT_SRC_XTRANSE,
> + .ev_src_ye = MMA8452_TRANSIENT_SRC_YTRANSE,
> + .ev_src_ze = MMA8452_TRANSIENT_SRC_ZTRANSE,
> + .ev_ths = MMA8452_TRANSIENT_THS,
> + .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
> + .ev_count = MMA8452_TRANSIENT_COUNT,
> + },
> };
> +
> static struct attribute *mma8452_attributes[] = {
> &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> &iio_dev_attr_in_accel_scale_available.dev_attr.attr,
> @@ -866,6 +895,7 @@ static int mma8452_reset(struct i2c_client *client)
>
> static const struct of_device_id mma8452_dt_ids[] = {
> { .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
> + { .compatible = "fsl,mma8453", .data = &mma_chip_info_table[mma8453] },
> { }
> };
>
> @@ -880,7 +910,8 @@ static int mma8452_probe(struct i2c_client *client,
> ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
> if (ret < 0)
> return ret;
> - if (ret != MMA8452_DEVICE_ID)
> +
> + if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID)
> return -ENODEV;
Change this to a switch now as it'll only get uglier as more parts are
added.
>
> match = of_match_device(mma8452_dt_ids, &client->dev);
> @@ -898,6 +929,9 @@ static int mma8452_probe(struct i2c_client *client,
> data->chip_info = match->data;
> mutex_init(&data->lock);
>
> + dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n",
> + match->compatible, data->chip_info->chip_id);
> +
This bit should probably have been in the previous patch, though as
it's trivial, who really cares ;) Does explain the chip_id
unused element in the previous patch. Best bet may be to move it back
to that patch now and solve both issues.
> i2c_set_clientdata(client, indio_dev);
> indio_dev->info = &mma8452_info;
> indio_dev->name = DRIVER_NAME;
> @@ -1024,6 +1058,7 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
>
> static const struct i2c_device_id mma8452_id[] = {
> { "mma8452", mma8452 },
> + { "mma8453", mma8453 },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, mma8452_id);
>
On 04/07/15 14:55, Martin Kepplinger wrote:
> This adds the freefall / motion interrupt source definitions to the driver.
> It is not in use now, but mma_chip_info and iio_chan_spec can easily be
> adapted to use it instead of the transient interrupt source.
So some support is added, but nothing done with it? Not sure we want
to do this as it adds unused code paths.
Why not support this fully?
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> Signed-off-by: Christoph Muellner <[email protected]>
> ---
> drivers/iio/accel/mma8452.c | 45 ++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 209b0d7..0498b6e 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -38,9 +38,18 @@
> #define MMA8452_DATA_CFG 0x0e
> #define MMA8452_HP_FILTER_CUTOFF 0x0f
> #define MMA8452_HP_FILTER_CUTOFF_SEL_MASK (BIT(0) | BIT(1))
> +#define MMA8452_FF_MT_CFG 0x15
> +#define MMA8452_FF_MT_CFG_OAE BIT(6)
> +#define MMA8452_FF_MT_CFG_ELE BIT(7)
> +#define MMA8452_FF_MT_SRC 0x16
> +#define MMA8452_FF_MT_SRC_XHE BIT(1)
> +#define MMA8452_FF_MT_SRC_YHE BIT(3)
> +#define MMA8452_FF_MT_SRC_ZHE BIT(5)
> +#define MMA8452_FF_MT_THS 0x17
> +#define MMA8452_FF_MT_THS_MASK 0x7f
> +#define MMA8452_FF_MT_COUNT 0x18
> #define MMA8452_TRANSIENT_CFG 0x1d
> #define MMA8452_TRANSIENT_CFG_ELE BIT(4)
> -#define MMA8452_TRANSIENT_CFG_CHAN(chan) BIT(chan + 1)
> #define MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0)
> #define MMA8452_TRANSIENT_SRC 0x1e
> #define MMA8452_TRANSIENT_SRC_XTRANSE BIT(1)
> @@ -74,6 +83,7 @@
> #define MMA8452_DATA_CFG_HPF_MASK BIT(4)
>
> #define MMA8452_INT_DRDY BIT(0)
> +#define MMA8452_INT_FF_MT BIT(2)
> #define MMA8452_INT_TRANS BIT(5)
>
> #define MMA8452_DEVICE_ID 0x2a
> @@ -564,7 +574,8 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
> else
> val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
>
> - val |= MMA8452_TRANSIENT_CFG_ELE;
> + val |= chip->ev_cfg_ele;
> + val |= MMA8452_FF_MT_CFG_OAE;
>
> return mma8452_change_config(data, chip->ev_cfg, val);
> }
> @@ -605,6 +616,7 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
> {
> struct iio_dev *indio_dev = p;
> struct mma8452_data *data = iio_priv(indio_dev);
> + const struct mma_chip_info *chip = data->chip_info;
> int ret = IRQ_NONE;
> int src;
>
> @@ -617,7 +629,10 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
> ret = IRQ_HANDLED;
> }
>
> - if (src & MMA8452_INT_TRANS) {
> + if ((src & MMA8452_INT_TRANS &&
> + chip->ev_src == MMA8452_TRANSIENT_SRC) ||
> + (src & MMA8452_INT_FF_MT &&
> + chip->ev_src == MMA8452_FF_MT_SRC)) {
> mma8452_transient_interrupt(indio_dev);
> ret = IRQ_HANDLED;
> }
> @@ -678,6 +693,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
> },
> };
>
> +static const struct iio_event_spec mma8452_motion_event[] = {
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_RISING,
> + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> + .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_PERIOD)
> + },
> +};
> +
> /*
> * Threshold is configured in fixed 8G/127 steps regardless of
> * currently selected scale for measurement.
> @@ -962,13 +987,15 @@ static int mma8452_probe(struct i2c_client *client,
>
> if (client->irq) {
> /*
> - * Although we enable the transient interrupt source once and
> - * for all here the transient event detection itself is not
> - * enabled until userspace asks for it by
> - * mma8452_write_event_config()
> + * Although we enable the interrupt sources once and for
> + * all here the event detection itself is not enabled until
> + * userspace asks for it by mma8452_write_event_config()
> */
> - int supported_interrupts = MMA8452_INT_DRDY | MMA8452_INT_TRANS;
> - int enabled_interrupts = MMA8452_INT_TRANS;
> + int supported_interrupts = MMA8452_INT_DRDY |
> + MMA8452_INT_TRANS |
> + MMA8452_INT_FF_MT;
> + int enabled_interrupts = MMA8452_INT_TRANS |
> + MMA8452_INT_FF_MT;
>
> /* Assume wired to INT1 pin */
> ret = i2c_smbus_write_byte_data(client,
>
On 05/07/15 12:44, Jonathan Cameron wrote:
> On 04/07/15 14:55, Martin Kepplinger wrote:
>> This adds the freefall / motion interrupt source definitions to the driver.
>> It is not in use now, but mma_chip_info and iio_chan_spec can easily be
>> adapted to use it instead of the transient interrupt source.
> So some support is added, but nothing done with it? Not sure we want
> to do this as it adds unused code paths.
>
> Why not support this fully?
Ah, I get it now. Used in next patch. Please add a comment here to indicate
that!
>>
>> Signed-off-by: Martin Kepplinger <[email protected]>
>> Signed-off-by: Christoph Muellner <[email protected]>
>> ---
>> drivers/iio/accel/mma8452.c | 45 ++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 36 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 209b0d7..0498b6e 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -38,9 +38,18 @@
>> #define MMA8452_DATA_CFG 0x0e
>> #define MMA8452_HP_FILTER_CUTOFF 0x0f
>> #define MMA8452_HP_FILTER_CUTOFF_SEL_MASK (BIT(0) | BIT(1))
>> +#define MMA8452_FF_MT_CFG 0x15
>> +#define MMA8452_FF_MT_CFG_OAE BIT(6)
>> +#define MMA8452_FF_MT_CFG_ELE BIT(7)
>> +#define MMA8452_FF_MT_SRC 0x16
>> +#define MMA8452_FF_MT_SRC_XHE BIT(1)
>> +#define MMA8452_FF_MT_SRC_YHE BIT(3)
>> +#define MMA8452_FF_MT_SRC_ZHE BIT(5)
>> +#define MMA8452_FF_MT_THS 0x17
>> +#define MMA8452_FF_MT_THS_MASK 0x7f
>> +#define MMA8452_FF_MT_COUNT 0x18
>> #define MMA8452_TRANSIENT_CFG 0x1d
>> #define MMA8452_TRANSIENT_CFG_ELE BIT(4)
>> -#define MMA8452_TRANSIENT_CFG_CHAN(chan) BIT(chan + 1)
>> #define MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0)
>> #define MMA8452_TRANSIENT_SRC 0x1e
>> #define MMA8452_TRANSIENT_SRC_XTRANSE BIT(1)
>> @@ -74,6 +83,7 @@
>> #define MMA8452_DATA_CFG_HPF_MASK BIT(4)
>>
>> #define MMA8452_INT_DRDY BIT(0)
>> +#define MMA8452_INT_FF_MT BIT(2)
>> #define MMA8452_INT_TRANS BIT(5)
>>
>> #define MMA8452_DEVICE_ID 0x2a
>> @@ -564,7 +574,8 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
>> else
>> val &= ~BIT(chan->scan_index + chip->ev_cfg_chan_shift);
>>
>> - val |= MMA8452_TRANSIENT_CFG_ELE;
>> + val |= chip->ev_cfg_ele;
>> + val |= MMA8452_FF_MT_CFG_OAE;
>>
>> return mma8452_change_config(data, chip->ev_cfg, val);
>> }
>> @@ -605,6 +616,7 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
>> {
>> struct iio_dev *indio_dev = p;
>> struct mma8452_data *data = iio_priv(indio_dev);
>> + const struct mma_chip_info *chip = data->chip_info;
>> int ret = IRQ_NONE;
>> int src;
>>
>> @@ -617,7 +629,10 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
>> ret = IRQ_HANDLED;
>> }
>>
>> - if (src & MMA8452_INT_TRANS) {
>> + if ((src & MMA8452_INT_TRANS &&
>> + chip->ev_src == MMA8452_TRANSIENT_SRC) ||
>> + (src & MMA8452_INT_FF_MT &&
>> + chip->ev_src == MMA8452_FF_MT_SRC)) {
>> mma8452_transient_interrupt(indio_dev);
>> ret = IRQ_HANDLED;
>> }
>> @@ -678,6 +693,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
>> },
>> };
>>
>> +static const struct iio_event_spec mma8452_motion_event[] = {
>> + {
>> + .type = IIO_EV_TYPE_THRESH,
>> + .dir = IIO_EV_DIR_RISING,
>> + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
>> + .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>> + BIT(IIO_EV_INFO_PERIOD)
>> + },
>> +};
>> +
>> /*
>> * Threshold is configured in fixed 8G/127 steps regardless of
>> * currently selected scale for measurement.
>> @@ -962,13 +987,15 @@ static int mma8452_probe(struct i2c_client *client,
>>
>> if (client->irq) {
>> /*
>> - * Although we enable the transient interrupt source once and
>> - * for all here the transient event detection itself is not
>> - * enabled until userspace asks for it by
>> - * mma8452_write_event_config()
>> + * Although we enable the interrupt sources once and for
>> + * all here the event detection itself is not enabled until
>> + * userspace asks for it by mma8452_write_event_config()
>> */
>> - int supported_interrupts = MMA8452_INT_DRDY | MMA8452_INT_TRANS;
>> - int enabled_interrupts = MMA8452_INT_TRANS;
>> + int supported_interrupts = MMA8452_INT_DRDY |
>> + MMA8452_INT_TRANS |
>> + MMA8452_INT_FF_MT;
>> + int enabled_interrupts = MMA8452_INT_TRANS |
>> + MMA8452_INT_FF_MT;
>>
>> /* Assume wired to INT1 pin */
>> ret = i2c_smbus_write_byte_data(client,
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 04/07/15 14:55, Martin Kepplinger wrote:
> MMA8652FC and MMA8653FC don't provide the transient interrupt source, so
> the motion interrupt source is used by providing a new iio_chan_spec
> definition, so that other supported devices are not affected by this.
>
> Datasheets for the newly supported devices are available at Freescale's
> website:
>
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8652FC.pdf
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA8653FC.pdf
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> Signed-off-by: Christoph Muellner <[email protected]>
Only really one comment which is a follow through from an earlier suggestion.
Looking good.
> ---
> drivers/iio/accel/Kconfig | 2 +-
> drivers/iio/accel/mma8452.c | 79 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 79 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 91fab16..684c8b5 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -93,7 +93,7 @@ config MMA8452
> select IIO_TRIGGERED_BUFFER
> help
> Say yes here to build support for the following Freescale 3-axis
> - accelerometers: MMA8452Q, MMA8453Q.
> + accelerometers: MMA8452Q, MMA8453Q, MMA8652FC, MMA8653FC.
>
> To compile this driver as a module, choose M here: the module
> will be called mma8452.
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 0498b6e..ccce925 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -3,6 +3,8 @@
> *
> * MMA8452Q
> * MMA8453Q
> + * MMA8652FC
> + * MMA8653FC
> *
> * Copyright 2014 Peter Meerwald <[email protected]>
> *
> @@ -88,6 +90,8 @@
>
> #define MMA8452_DEVICE_ID 0x2a
> #define MMA8453_DEVICE_ID 0x3a
> +#define MMA8652_DEVICE_ID 0x4a
> +#define MMA8653_DEVICE_ID 0x5a
>
> struct mma8452_data {
> struct i2c_client *client;
> @@ -740,6 +744,26 @@ static struct attribute_group mma8452_event_attribute_group = {
> .num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
> }
>
> +#define MMA8652_CHANNEL(axis, idx, bits) { \
> + .type = IIO_ACCEL, \
> + .modified = 1, \
> + .channel2 = IIO_MOD_##axis, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_CALIBBIAS), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .scan_index = idx, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = (bits), \
> + .storagebits = 16, \
> + .shift = 16 - (bits), \
> + .endianness = IIO_BE, \
> + }, \
> + .event_spec = mma8452_motion_event, \
> + .num_event_specs = ARRAY_SIZE(mma8452_motion_event), \
> +}
> +
> static const struct iio_chan_spec mma8452_channels[] = {
> MMA8452_CHANNEL(X, 0, 12),
> MMA8452_CHANNEL(Y, 1, 12),
> @@ -754,9 +778,25 @@ static const struct iio_chan_spec mma8453_channels[] = {
> IIO_CHAN_SOFT_TIMESTAMP(3),
> };
>
> +static const struct iio_chan_spec mma8652_channels[] = {
> + MMA8652_CHANNEL(X, 0, 12),
> + MMA8652_CHANNEL(Y, 1, 12),
> + MMA8652_CHANNEL(Z, 2, 12),
> + IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +static const struct iio_chan_spec mma8653_channels[] = {
> + MMA8652_CHANNEL(X, 0, 10),
> + MMA8652_CHANNEL(Y, 1, 10),
> + MMA8652_CHANNEL(Z, 2, 10),
> + IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> enum {
> mma8452,
> mma8453,
> + mma8652,
> + mma8653,
> };
>
> /*
> @@ -800,6 +840,38 @@ static const struct mma_chip_info mma_chip_info_table[] = {
> .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
> .ev_count = MMA8452_TRANSIENT_COUNT,
> },
> + [mma8652] = {
> + .chip_id = MMA8652_DEVICE_ID,
> + .channels = mma8652_channels,
> + .num_channels = ARRAY_SIZE(mma8652_channels),
> + .mma_scales = { {0, 9577}, {0, 19154}, {0, 38307} },
> + .ev_cfg = MMA8452_FF_MT_CFG,
> + .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
> + .ev_cfg_chan_shift = 3,
> + .ev_src = MMA8452_FF_MT_SRC,
> + .ev_src_xe = MMA8452_FF_MT_SRC_XHE,
> + .ev_src_ye = MMA8452_FF_MT_SRC_YHE,
> + .ev_src_ze = MMA8452_FF_MT_SRC_ZHE,
> + .ev_ths = MMA8452_FF_MT_THS,
> + .ev_ths_mask = MMA8452_FF_MT_THS_MASK,
> + .ev_count = MMA8452_FF_MT_COUNT,
> + },
> + [mma8653] = {
> + .chip_id = MMA8653_DEVICE_ID,
> + .channels = mma8653_channels,
> + .num_channels = ARRAY_SIZE(mma8653_channels),
> + .mma_scales = { {0, 38307}, {0, 76614}, {0, 153228} },
> + .ev_cfg = MMA8452_FF_MT_CFG,
> + .ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
> + .ev_cfg_chan_shift = 3,
> + .ev_src = MMA8452_FF_MT_SRC,
> + .ev_src_xe = MMA8452_FF_MT_SRC_XHE,
> + .ev_src_ye = MMA8452_FF_MT_SRC_YHE,
> + .ev_src_ze = MMA8452_FF_MT_SRC_ZHE,
> + .ev_ths = MMA8452_FF_MT_THS,
> + .ev_ths_mask = MMA8452_FF_MT_THS_MASK,
> + .ev_count = MMA8452_FF_MT_COUNT,
> + },
> };
>
> static struct attribute *mma8452_attributes[] = {
> @@ -921,6 +993,8 @@ static int mma8452_reset(struct i2c_client *client)
> static const struct of_device_id mma8452_dt_ids[] = {
> { .compatible = "fsl,mma8452", .data = &mma_chip_info_table[mma8452] },
> { .compatible = "fsl,mma8453", .data = &mma_chip_info_table[mma8453] },
> + { .compatible = "fsl,mma8652", .data = &mma_chip_info_table[mma8652] },
> + { .compatible = "fsl,mma8653", .data = &mma_chip_info_table[mma8653] },
> { }
> };
>
> @@ -936,7 +1010,8 @@ static int mma8452_probe(struct i2c_client *client,
> if (ret < 0)
> return ret;
>
> - if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID)
> + if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID &&
> + ret != MMA8652_DEVICE_ID && ret != MMA8653_DEVICE_ID)
And now the advantages of a switch become apparent ;)
switch (ret) {
case MMA8452_DEVICE_ID:
case MMA8453_DEVICE_ID:
..
break;
default:
return -ENODEV;
Also, you should really be verifying that not only is it a supported
part, but that it is the one that our board configuration is telling us
is there.
> return -ENODEV;
>
> match = of_match_device(mma8452_dt_ids, &client->dev);
> @@ -1086,6 +1161,8 @@ static SIMPLE_DEV_PM_OPS(mma8452_pm_ops, mma8452_suspend, mma8452_resume);
> static const struct i2c_device_id mma8452_id[] = {
> { "mma8452", mma8452 },
> { "mma8453", mma8453 },
> + { "mma8652", mma8652 },
> + { "mma8653", mma8653 },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, mma8452_id);
>
On 04/07/15 14:55, Martin Kepplinger wrote:
> Signed-off-by: Martin Kepplinger <[email protected]>
> Signed-off-by: Christoph Muellner <[email protected]>
This is trivial enough I'll take it without a device tree ack (after a few days)
but convention is to CC all device tree patches to the maintainers of devicetree
bindings as well as the devicetree list.
Jonathan
> ---
> .../devicetree/bindings/iio/accel/mma8452.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> new file mode 100644
> index 0000000..8d98e05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> @@ -0,0 +1,21 @@
> +Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
> +
> +Required properties:
> +
> + - compatible: should be "fsl,mma8653", "fsl,mma8652", "fsl,mma8453" or
> + "fsl,mma8452" respectively.
> + - reg: the I2C address of the chip
> +
> +Optional properties:
> +
> + - interrupt-parent: should be the phandle for the interrupt controller
> + - interrupts: interrupt mapping for GPIO IRQ
> +
> +Example:
> +
> + mma8653fc@1d {
> + compatible = "fsl,mma8653";
> + reg = <0x1d>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <5 0>;
> + };
>
On 04/07/15 14:55, Martin Kepplinger wrote:
> IIO_EV_TYPE_THRESH in rising direction describes an event where the
> threshold is crossed in rising direction, positive or negative values
> being possible. This is not the case here.
>
> Since the threshold is no signed value and only the magnitude is compared,
> IIO_EV_TYPE_MAG is what describes the behaviour of these devices, see the
> sysfs-bus-iio ABI Documentation.
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> Signed-off-by: Christoph Muellner <[email protected]>
This is a fix and so should have been the first patch in the series. It will
want to go via a different tree (iio-fixes) and probably be marked for stable.
I would however like Peter's ack on this as well before taking it.
> ---
> drivers/iio/accel/mma8452.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 7f6e3b4..e23ebd0 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -598,21 +598,21 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
> if (src & data->chip_info->ev_src_xe)
> iio_push_event(indio_dev,
> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
> - IIO_EV_TYPE_THRESH,
> + IIO_EV_TYPE_MAG,
> IIO_EV_DIR_RISING),
> ts);
>
> if (src & data->chip_info->ev_src_ye)
> iio_push_event(indio_dev,
> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
> - IIO_EV_TYPE_THRESH,
> + IIO_EV_TYPE_MAG,
> IIO_EV_DIR_RISING),
> ts);
>
> if (src & data->chip_info->ev_src_ze)
> iio_push_event(indio_dev,
> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
> - IIO_EV_TYPE_THRESH,
> + IIO_EV_TYPE_MAG,
> IIO_EV_DIR_RISING),
> ts);
> }
> @@ -689,7 +689,7 @@ static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
>
> static const struct iio_event_spec mma8452_transient_event[] = {
> {
> - .type = IIO_EV_TYPE_THRESH,
> + .type = IIO_EV_TYPE_MAG,
> .dir = IIO_EV_DIR_RISING,
> .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
> @@ -700,7 +700,7 @@ static const struct iio_event_spec mma8452_transient_event[] = {
>
> static const struct iio_event_spec mma8452_motion_event[] = {
> {
> - .type = IIO_EV_TYPE_THRESH,
> + .type = IIO_EV_TYPE_MAG,
> .dir = IIO_EV_DIR_RISING,
> .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>
Am 2015-07-05 um 13:48 schrieb Jonathan Cameron:
> On 04/07/15 14:55, Martin Kepplinger wrote:
>> Signed-off-by: Martin Kepplinger <[email protected]>
>> Signed-off-by: Christoph Muellner <[email protected]>
> This is trivial enough I'll take it without a device tree ack (after a few days)
> but convention is to CC all device tree patches to the maintainers of devicetree
> bindings as well as the devicetree list.
>
> Jonathan
Thanks a lot so far. I'll send a next version based on your review and
will add devicetree people. Maybe you can take the series cleanly in one
go after that. Someone that quickly runs it, other than me, would be
awesome of course ;)
martin
>> ---
>> .../devicetree/bindings/iio/accel/mma8452.txt | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> new file mode 100644
>> index 0000000..8d98e05
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> @@ -0,0 +1,21 @@
>> +Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
>> +
>> +Required properties:
>> +
>> + - compatible: should be "fsl,mma8653", "fsl,mma8652", "fsl,mma8453" or
>> + "fsl,mma8452" respectively.
>> + - reg: the I2C address of the chip
>> +
>> +Optional properties:
>> +
>> + - interrupt-parent: should be the phandle for the interrupt controller
>> + - interrupts: interrupt mapping for GPIO IRQ
>> +
>> +Example:
>> +
>> + mma8653fc@1d {
>> + compatible = "fsl,mma8653";
>> + reg = <0x1d>;
>> + interrupt-parent = <&gpio1>;
>> + interrupts = <5 0>;
>> + };
>>
>
On 04/07/15 14:55, Martin Kepplinger wrote:
> For the devices supported by the mma8452 driver, two interrupt pins are
> available to route the interrupt signals to. By default INT1 is assumed.
>
> This adds a simple boolean DT property, for users to configure it for
> INT2, if that is the wired interrupt pin for them.
>
> This is important for everyone to be able to use this driver, no matter
> how their chip is wired.
>
> Since this doesn't change the default behaviour, it doesn't break anything
> for existing users.
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> Signed-off-by: Christoph Muellner <[email protected]>
I just wonder if we can make the naming more obvious.
> ---
> Documentation/devicetree/bindings/iio/accel/mma8452.txt | 2 ++
> drivers/iio/accel/mma8452.c | 14 ++++++++------
> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> index 8d98e05..0048415 100644
> --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
> @@ -10,6 +10,7 @@ Optional properties:
>
> - interrupt-parent: should be the phandle for the interrupt controller
> - interrupts: interrupt mapping for GPIO IRQ
> + - int2: assume interrupt pin wired to INT2 instead of INT1
use_int2 perhaps?
>
> Example:
>
> @@ -18,4 +19,5 @@ Example:
> reg = <0x1d>;
> interrupt-parent = <&gpio1>;
> interrupts = <5 0>;
> + int2;
> };
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 2491ed0..0768e66 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1072,12 +1072,14 @@ static int mma8452_probe(struct i2c_client *client,
> int enabled_interrupts = MMA8452_INT_TRANS |
> MMA8452_INT_FF_MT;
>
> - /* Assume wired to INT1 pin */
> - ret = i2c_smbus_write_byte_data(client,
> - MMA8452_CTRL_REG5,
> - supported_interrupts);
> - if (ret < 0)
> - return ret;
> + /* Assume wired to INT1 pin, except "int2" is found in DT */
> + if (!of_property_read_bool(client->dev.of_node, "int2")) {
> + ret = i2c_smbus_write_byte_data(client,
> + MMA8452_CTRL_REG5,
> + supported_interrupts);
> + if (ret < 0)
> + return ret;
> + }
>
> ret = i2c_smbus_write_byte_data(client,
> MMA8452_CTRL_REG4,
>
On 05/07/15 12:50, Martin Kepplinger wrote:
> Am 2015-07-05 um 13:48 schrieb Jonathan Cameron:
>> On 04/07/15 14:55, Martin Kepplinger wrote:
>>> Signed-off-by: Martin Kepplinger <[email protected]>
>>> Signed-off-by: Christoph Muellner <[email protected]>
>> This is trivial enough I'll take it without a device tree ack (after a few days)
>> but convention is to CC all device tree patches to the maintainers of devicetree
>> bindings as well as the devicetree list.
>>
>> Jonathan
>
> Thanks a lot so far. I'll send a next version based on your review and
> will add devicetree people. Maybe you can take the series cleanly in one
> go after that. Someone that quickly runs it, other than me, would be
> awesome of course ;)
Wuss ;) You'd be amazed how many invasive changes go in without anyone
every finding anyone to test them on particular obscure parts.
Still it is indeed always nice.
>
> martin
>>> ---
>>> .../devicetree/bindings/iio/accel/mma8452.txt | 21 +++++++++++++++++++++
>>> 1 file changed, 21 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>> new file mode 100644
>>> index 0000000..8d98e05
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>> @@ -0,0 +1,21 @@
>>> +Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
>>> +
>>> +Required properties:
>>> +
>>> + - compatible: should be "fsl,mma8653", "fsl,mma8652", "fsl,mma8453" or
>>> + "fsl,mma8452" respectively.
>>> + - reg: the I2C address of the chip
>>> +
>>> +Optional properties:
>>> +
>>> + - interrupt-parent: should be the phandle for the interrupt controller
>>> + - interrupts: interrupt mapping for GPIO IRQ
>>> +
>>> +Example:
>>> +
>>> + mma8653fc@1d {
>>> + compatible = "fsl,mma8653";
>>> + reg = <0x1d>;
>>> + interrupt-parent = <&gpio1>;
>>> + interrupts = <5 0>;
>>> + };
>>>
>>
>
On 05/07/15 13:48, Jonathan Cameron wrote:
> On 04/07/15 14:55, Martin Kepplinger wrote:
>> Signed-off-by: Martin Kepplinger <[email protected]>
>> Signed-off-by: Christoph Muellner <[email protected]>
> This is trivial enough I'll take it without a device tree ack (after a few days)
> but convention is to CC all device tree patches to the maintainers of devicetree
> bindings as well as the devicetree list.
Currently the binding is covered by
Documentation/devicetree/bindings/i2c/trivial-devices.txt
Shouldn't that one be removed now that a full binding document is being
created?
Regards,
Martin
> Jonathan
>> ---
>> .../devicetree/bindings/iio/accel/mma8452.txt | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> new file mode 100644
>> index 0000000..8d98e05
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>> @@ -0,0 +1,21 @@
>> +Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial accelerometer
>> +
>> +Required properties:
>> +
>> + - compatible: should be "fsl,mma8653", "fsl,mma8652", "fsl,mma8453" or
>> + "fsl,mma8452" respectively.
>> + - reg: the I2C address of the chip
>> +
>> +Optional properties:
>> +
>> + - interrupt-parent: should be the phandle for the interrupt controller
>> + - interrupts: interrupt mapping for GPIO IRQ
>> +
>> +Example:
>> +
>> + mma8653fc@1d {
>> + compatible = "fsl,mma8653";
>> + reg = <0x1d>;
>> + interrupt-parent = <&gpio1>;
>> + interrupts = <5 0>;
>> + };
>>
Am 2015-07-06 um 10:06 schrieb Martin Fuzzey:
> On 05/07/15 13:48, Jonathan Cameron wrote:
>> On 04/07/15 14:55, Martin Kepplinger wrote:
>>> Signed-off-by: Martin Kepplinger
>>> <[email protected]>
>>> Signed-off-by: Christoph Muellner
>>> <[email protected]>
>> This is trivial enough I'll take it without a device tree ack (after a
>> few days)
>> but convention is to CC all device tree patches to the maintainers of
>> devicetree
>> bindings as well as the devicetree list.
>
> Currently the binding is covered by
> Documentation/devicetree/bindings/i2c/trivial-devices.txt
> Shouldn't that one be removed now that a full binding document is being
> created?
>
True, thanks very much! Included for the new version of the patches.
> Regards,
>
> Martin
>
>> Jonathan
>>> ---
>>> .../devicetree/bindings/iio/accel/mma8452.txt | 21
>>> +++++++++++++++++++++
>>> 1 file changed, 21 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>> b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>> new file mode 100644
>>> index 0000000..8d98e05
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt
>>> @@ -0,0 +1,21 @@
>>> +Freescale MMA8452Q, MMA8453Q, MMA8652FC or MMA8653FC triaxial
>>> accelerometer
>>> +
>>> +Required properties:
>>> +
>>> + - compatible: should be "fsl,mma8653", "fsl,mma8652",
>>> "fsl,mma8453" or
>>> + "fsl,mma8452" respectively.
>>> + - reg: the I2C address of the chip
>>> +
>>> +Optional properties:
>>> +
>>> + - interrupt-parent: should be the phandle for the interrupt
>>> controller
>>> + - interrupts: interrupt mapping for GPIO IRQ
>>> +
>>> +Example:
>>> +
>>> + mma8653fc@1d {
>>> + compatible = "fsl,mma8653";
>>> + reg = <0x1d>;
>>> + interrupt-parent = <&gpio1>;
>>> + interrupts = <5 0>;
>>> + };
>>>
>
On 05/07/15 13:50, Jonathan Cameron wrote:
> On 04/07/15 14:55, Martin Kepplinger wrote:
>> IIO_EV_TYPE_THRESH in rising direction describes an event where the
>> threshold is crossed in rising direction, positive or negative values
>> being possible. This is not the case here.
>>
>> Since the threshold is no signed value and only the magnitude is compared,
>> IIO_EV_TYPE_MAG is what describes the behaviour of these devices, see the
>> sysfs-bus-iio ABI Documentation.
Fwiw there was some discussion of this before the initial submission:
http://www.spinics.net/lists/linux-iio/msg14039.html
Initially I used a magnitude too but Jonathan convinced me it should be
a threshold.
"
The moment you know the sign of the magnitude it stops being a magnitude
and becomes a generic threshold. Report it as such and control it as such.
"
Thing is that the hardware indeed only compares the absolute value for
the threshold *but* indicates with the event the sign.
However it is true that the driver doesn't currently do anything with
the sign information.
>> Signed-off-by: Martin Kepplinger <[email protected]>
>> Signed-off-by: Christoph Muellner <[email protected]>
> This is a fix and so should have been the first patch in the series. It will
> want to go via a different tree (iio-fixes) and probably be marked for stable.
>
> I would however like Peter's ack on this as well before taking it.
>> ---
>> drivers/iio/accel/mma8452.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 7f6e3b4..e23ebd0 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -598,21 +598,21 @@ static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
>> if (src & data->chip_info->ev_src_xe)
>> iio_push_event(indio_dev,
>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
>> - IIO_EV_TYPE_THRESH,
>> + IIO_EV_TYPE_MAG,
>> IIO_EV_DIR_RISING),
>> ts);
>>
>> if (src & data->chip_info->ev_src_ye)
>> iio_push_event(indio_dev,
>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
>> - IIO_EV_TYPE_THRESH,
>> + IIO_EV_TYPE_MAG,
>> IIO_EV_DIR_RISING),
>> ts);
>>
>> if (src & data->chip_info->ev_src_ze)
>> iio_push_event(indio_dev,
>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
>> - IIO_EV_TYPE_THRESH,
>> + IIO_EV_TYPE_MAG,
>> IIO_EV_DIR_RISING),
>> ts);
>> }
>> @@ -689,7 +689,7 @@ static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
>>
>> static const struct iio_event_spec mma8452_transient_event[] = {
>> {
>> - .type = IIO_EV_TYPE_THRESH,
>> + .type = IIO_EV_TYPE_MAG,
>> .dir = IIO_EV_DIR_RISING,
>> .mask_separate = BIT(IIO_EV_INFO_ENABLE),
>> .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>> @@ -700,7 +700,7 @@ static const struct iio_event_spec mma8452_transient_event[] = {
>>
>> static const struct iio_event_spec mma8452_motion_event[] = {
>> {
>> - .type = IIO_EV_TYPE_THRESH,
>> + .type = IIO_EV_TYPE_MAG,
>> .dir = IIO_EV_DIR_RISING,
>> .mask_separate = BIT(IIO_EV_INFO_ENABLE),
>> .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>>
On 6 July 2015 09:39:12 BST, Martin Fuzzey <[email protected]> wrote:
>On 05/07/15 13:50, Jonathan Cameron wrote:
>> On 04/07/15 14:55, Martin Kepplinger wrote:
>>> IIO_EV_TYPE_THRESH in rising direction describes an event where the
>>> threshold is crossed in rising direction, positive or negative
>values
>>> being possible. This is not the case here.
>>>
>>> Since the threshold is no signed value and only the magnitude is
>compared,
>>> IIO_EV_TYPE_MAG is what describes the behaviour of these devices,
>see the
>>> sysfs-bus-iio ABI Documentation.
>
>Fwiw there was some discussion of this before the initial submission:
>
>http://www.spinics.net/lists/linux-iio/msg14039.html
>
>Initially I used a magnitude too but Jonathan convinced me it should be
>
>a threshold.
>
>"
>
>The moment you know the sign of the magnitude it stops being a
>magnitude
>and becomes a generic threshold. Report it as such and control it as
>such.
>
>"
>
>Thing is that the hardware indeed only compares the absolute value for
>the threshold *but* indicates with the event the sign.
>However it is true that the driver doesn't currently do anything with
>the sign information.
Ah. That explains the confusion.
This is a common enough case.
Usual approach is the slightly hacky option of having two threshold events. Setting limit on either effects both.
Enabling either effects both (or you can eat the wrong one in driver if you prefer
when only one is enabled)
>
>
>>> Signed-off-by: Martin Kepplinger
><[email protected]>
>>> Signed-off-by: Christoph Muellner
><[email protected]>
>> This is a fix and so should have been the first patch in the series.
>It will
>> want to go via a different tree (iio-fixes) and probably be marked
>for stable.
>>
>> I would however like Peter's ack on this as well before taking it.
>>> ---
>>> drivers/iio/accel/mma8452.c | 10 +++++-----
>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/iio/accel/mma8452.c
>b/drivers/iio/accel/mma8452.c
>>> index 7f6e3b4..e23ebd0 100644
>>> --- a/drivers/iio/accel/mma8452.c
>>> +++ b/drivers/iio/accel/mma8452.c
>>> @@ -598,21 +598,21 @@ static void mma8452_transient_interrupt(struct
>iio_dev *indio_dev)
>>> if (src & data->chip_info->ev_src_xe)
>>> iio_push_event(indio_dev,
>>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
>>> - IIO_EV_TYPE_THRESH,
>>> + IIO_EV_TYPE_MAG,
>>> IIO_EV_DIR_RISING),
>>> ts);
>>>
>>> if (src & data->chip_info->ev_src_ye)
>>> iio_push_event(indio_dev,
>>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
>>> - IIO_EV_TYPE_THRESH,
>>> + IIO_EV_TYPE_MAG,
>>> IIO_EV_DIR_RISING),
>>> ts);
>>>
>>> if (src & data->chip_info->ev_src_ze)
>>> iio_push_event(indio_dev,
>>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
>>> - IIO_EV_TYPE_THRESH,
>>> + IIO_EV_TYPE_MAG,
>>> IIO_EV_DIR_RISING),
>>> ts);
>>> }
>>> @@ -689,7 +689,7 @@ static int mma8452_reg_access_dbg(struct iio_dev
>*indio_dev,
>>>
>>> static const struct iio_event_spec mma8452_transient_event[] = {
>>> {
>>> - .type = IIO_EV_TYPE_THRESH,
>>> + .type = IIO_EV_TYPE_MAG,
>>> .dir = IIO_EV_DIR_RISING,
>>> .mask_separate = BIT(IIO_EV_INFO_ENABLE),
>>> .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>>> @@ -700,7 +700,7 @@ static const struct iio_event_spec
>mma8452_transient_event[] = {
>>>
>>> static const struct iio_event_spec mma8452_motion_event[] = {
>>> {
>>> - .type = IIO_EV_TYPE_THRESH,
>>> + .type = IIO_EV_TYPE_MAG,
>>> .dir = IIO_EV_DIR_RISING,
>>> .mask_separate = BIT(IIO_EV_INFO_ENABLE),
>>> .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Am 2015-07-07 um 09:05 schrieb Jonathan Cameron:
>
>
> On 6 July 2015 09:39:12 BST, Martin Fuzzey <[email protected]> wrote:
>> On 05/07/15 13:50, Jonathan Cameron wrote:
>>> On 04/07/15 14:55, Martin Kepplinger wrote:
>>>> IIO_EV_TYPE_THRESH in rising direction describes an event where the
>>>> threshold is crossed in rising direction, positive or negative
>> values
>>>> being possible. This is not the case here.
>>>>
>>>> Since the threshold is no signed value and only the magnitude is
>> compared,
>>>> IIO_EV_TYPE_MAG is what describes the behaviour of these devices,
>> see the
>>>> sysfs-bus-iio ABI Documentation.
>>
>> Fwiw there was some discussion of this before the initial submission:
>>
>> http://www.spinics.net/lists/linux-iio/msg14039.html
>>
>> Initially I used a magnitude too but Jonathan convinced me it should be
>>
>> a threshold.
>>
>> "
>>
>> The moment you know the sign of the magnitude it stops being a
>> magnitude
>> and becomes a generic threshold. Report it as such and control it as
>> such.
>>
>> "
>>
>> Thing is that the hardware indeed only compares the absolute value for
>> the threshold *but* indicates with the event the sign.
>> However it is true that the driver doesn't currently do anything with
>> the sign information.
> Ah. That explains the confusion.
> This is a common enough case.
> Usual approach is the slightly hacky option of having two threshold events. Setting limit on either effects both.
> Enabling either effects both (or you can eat the wrong one in driver if you prefer
> when only one is enabled)
I tried to implement THRESH somehow but it gets too messy. Yes, what you
*can* read for an event is positive or negative acceleration value (g,
whatever). But, say, positive g doesn't at all indicate THRESH RISING .
It could be negative g in freefall mode on one axis, and so on...
So the sign of the current value doesn't help you turning MAG into
TRESH. Really I think, MAG is what describes these devices.
>>
>>
>>>> Signed-off-by: Martin Kepplinger
>> <[email protected]>
>>>> Signed-off-by: Christoph Muellner
>> <[email protected]>
>>> This is a fix and so should have been the first patch in the series.
>> It will
>>> want to go via a different tree (iio-fixes) and probably be marked
>> for stable.
>>>
>>> I would however like Peter's ack on this as well before taking it.
>>>> ---
>>>> drivers/iio/accel/mma8452.c | 10 +++++-----
>>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/accel/mma8452.c
>> b/drivers/iio/accel/mma8452.c
>>>> index 7f6e3b4..e23ebd0 100644
>>>> --- a/drivers/iio/accel/mma8452.c
>>>> +++ b/drivers/iio/accel/mma8452.c
>>>> @@ -598,21 +598,21 @@ static void mma8452_transient_interrupt(struct
>> iio_dev *indio_dev)
>>>> if (src & data->chip_info->ev_src_xe)
>>>> iio_push_event(indio_dev,
>>>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X,
>>>> - IIO_EV_TYPE_THRESH,
>>>> + IIO_EV_TYPE_MAG,
>>>> IIO_EV_DIR_RISING),
>>>> ts);
>>>>
>>>> if (src & data->chip_info->ev_src_ye)
>>>> iio_push_event(indio_dev,
>>>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Y,
>>>> - IIO_EV_TYPE_THRESH,
>>>> + IIO_EV_TYPE_MAG,
>>>> IIO_EV_DIR_RISING),
>>>> ts);
>>>>
>>>> if (src & data->chip_info->ev_src_ze)
>>>> iio_push_event(indio_dev,
>>>> IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_Z,
>>>> - IIO_EV_TYPE_THRESH,
>>>> + IIO_EV_TYPE_MAG,
>>>> IIO_EV_DIR_RISING),
>>>> ts);
>>>> }
>>>> @@ -689,7 +689,7 @@ static int mma8452_reg_access_dbg(struct iio_dev
>> *indio_dev,
>>>>
>>>> static const struct iio_event_spec mma8452_transient_event[] = {
>>>> {
>>>> - .type = IIO_EV_TYPE_THRESH,
>>>> + .type = IIO_EV_TYPE_MAG,
>>>> .dir = IIO_EV_DIR_RISING,
>>>> .mask_separate = BIT(IIO_EV_INFO_ENABLE),
>>>> .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>>>> @@ -700,7 +700,7 @@ static const struct iio_event_spec
>> mma8452_transient_event[] = {
>>>>
>>>> static const struct iio_event_spec mma8452_motion_event[] = {
>>>> {
>>>> - .type = IIO_EV_TYPE_THRESH,
>>>> + .type = IIO_EV_TYPE_MAG,
>>>> .dir = IIO_EV_DIR_RISING,
>>>> .mask_separate = BIT(IIO_EV_INFO_ENABLE),
>>>> .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
>>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>