This is version 4 of the mma8452 driver improvements. This version removes
one patch that changed the iio event type for existing users. It can be
dealt with seperately and this series applies without it.
Also, changes according to Jonathan's review are applied and a few
devicetree maintainers are added as recipients.
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,
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, nothing should have
changed.
revision history
----------------
v4 cleanup; one bugfix patch removed from series; DT people added
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 | 188 ++++++++++++++++++++++++++++++++------------
1 file changed, 138 insertions(+), 50 deletions(-)
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 13ea1ea..8919bfb 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,52 @@ struct mma8452_data {
struct mutex lock;
u8 ctrl_reg1;
u8 data_cfg;
+ const struct mma_chip_info *chip_info;
+};
+
+/**
+ * struct mma_chip_info - chip specific data for Freescale's accelerometers
+ * @chip_id: WHO_AM_I register's value
+ * @channels: struct iio_chan_spec matching the device's
+ * capabilities
+ * @num_channels: number of channels
+ * @mma_scales: scale factors for converting register values
+ * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
+ * per mode: m/s^2 and micro m/s^2
+ * @ev_cfg: event config register address
+ * @ev_cfg_ele: latch bit in event config register
+ * @ev_cfg_chan_shift: number of the bit to enable events in X
+ * direction; in event config register
+ * @ev_src: event source register address
+ * @ev_src_xe: bit in event source register that indicates
+ * an event in X direction
+ * @ev_src_ye: bit in event source register that indicates
+ * an event in Y direction
+ * @ev_src_ze: bit in event source register that indicates
+ * an event in Z direction
+ * @ev_ths: event threshold register address
+ * @ev_ths_mask: mask for the threshold value
+ * @ev_count: event count (period) register address
+ *
+ * Since not all chips supported by the driver support comparing high pass
+ * filtered data for events (interrupts), different interrupt sources are
+ * used for different chips and the relevant registers are included here.
+ */
+struct mma_chip_info {
+ u8 chip_id;
+ const struct iio_chan_spec *channels;
+ int num_channels;
+ 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 +191,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 +225,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 +260,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 +270,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 +307,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 +462,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 +514,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 +528,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 +557,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));
}
static int mma8452_write_event_config(struct iio_dev *indio_dev,
@@ -528,20 +575,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 +598,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 +717,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 +729,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 +739,41 @@ 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,
+};
+
+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),
+ /*
+ * 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 for 12 bit here is given by:
+ * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665
+ */
+ .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 +890,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,6 +909,12 @@ 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;
@@ -833,14 +922,18 @@ static int mma8452_probe(struct i2c_client *client,
data = iio_priv(indio_dev);
data->client = client;
mutex_init(&data->lock);
+ data->chip_info = match->data;
+
+ 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 = id->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 +1052,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 | 38 +++++++++++++++++++++++++++++++++++---
2 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 00e7bcb..dc80dc9 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 MMA8452Q and similar Accelerometers 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 8919bfb..71b40f7 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;
@@ -745,8 +749,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,
};
static const struct mma_chip_info mma_chip_info_table[] = {
@@ -773,7 +785,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,
@@ -892,6 +921,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] },
{ }
};
@@ -906,7 +936,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);
@@ -1053,6 +1084,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 used in this series' next patch, for chips that don't support the
transient interrupt source.
The iio event type is IIO_EV_TYPE_MAG since the threshold for comparison
is no signed value. An interrupt occurs on positive and negative values
when the magnitude crosses the threshold value.
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 71b40f7..1337707 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
@@ -591,7 +601,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);
}
@@ -632,6 +643,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;
@@ -644,7 +656,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;
}
@@ -705,6 +720,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
},
};
+static const struct iio_event_spec mma8452_motion_event[] = {
+ {
+ .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) |
+ BIT(IIO_EV_INFO_PERIOD)
+ },
+};
+
/*
* Threshold is configured in fixed 8G/127 steps regardless of
* currently selected scale for measurement.
@@ -988,13 +1013,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 | 98 +++++++++++++++++++++++++++++++++++++++++----
2 files changed, 92 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index dc80dc9..fdb59e0 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 1337707..cc19a5d8 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;
@@ -767,6 +771,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),
@@ -781,9 +805,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,
};
static const struct mma_chip_info mma_chip_info_table[] = {
@@ -826,6 +866,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[] = {
@@ -947,6 +1019,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] },
{ }
};
@@ -958,13 +1032,6 @@ static int mma8452_probe(struct i2c_client *client,
int ret;
const struct of_device_id *match;
- ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
- if (ret < 0)
- return ret;
-
- if (ret != MMA8452_DEVICE_ID && ret != MMA8453_DEVICE_ID)
- return -ENODEV;
-
match = of_match_device(mma8452_dt_ids, &client->dev);
if (!match) {
dev_err(&client->dev, "unknown device model\n");
@@ -980,6 +1047,21 @@ static int mma8452_probe(struct i2c_client *client,
mutex_init(&data->lock);
data->chip_info = match->data;
+ ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
+ if (ret < 0)
+ return ret;
+
+ switch (ret) {
+ case MMA8452_DEVICE_ID:
+ case MMA8453_DEVICE_ID:
+ case MMA8652_DEVICE_ID:
+ case MMA8653_DEVICE_ID:
+ if (ret == data->chip_info->chip_id)
+ break;
+ default:
+ return -ENODEV;
+ }
+
dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n",
match->compatible, data->chip_info->chip_id);
@@ -1112,6 +1194,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
As we support more chips now, add a binding document and remove it from
i2c trivial-devices.txt list.
The binding document is further extended in a later patch of this series.
Signed-off-by: Martin Kepplinger <[email protected]>
Signed-off-by: Christoph Muellner <[email protected]>
---
.../devicetree/bindings/i2c/trivial-devices.txt | 1 -
.../devicetree/bindings/iio/accel/mma8452.txt | 21 +++++++++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/iio/accel/mma8452.txt
diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 00f8652..094238a 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -54,7 +54,6 @@ epson,rx8581 I2C-BUS INTERFACE REAL TIME CLOCK MODULE
fsl,mag3110 MAG3110: Xtrinsic High Accuracy, 3D Magnetometer
fsl,mc13892 MC13892: Power Management Integrated Circuit (PMIC) for i.MX35/51
fsl,mma8450 MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer
-fsl,mma8452 MMA8452Q: 3-axis 12-bit / 8-bit Digital Accelerometer
fsl,mpr121 MPR121: Proximity Capacitive Touch Sensor Controller
fsl,sgtl5000 SGTL5000: Ultra Low-Power Audio Codec
gmt,g751 G751: Digital Temperature Sensor and Thermal Watchdog with Two-Wire Interface
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 cc19a5d8..bc4215d 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
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 bc4215d..2b8ed67 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -748,7 +748,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..9bad1fc 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
+ - use_int2: assume interrupt pin wired to INT2 instead of INT1
Example:
@@ -18,4 +19,5 @@ Example:
reg = <0x1d>;
interrupt-parent = <&gpio1>;
interrupts = <5 0>;
+ use_int2;
};
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 2b8ed67..f8ba146 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1105,12 +1105,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 "use_int2" is found in DT */
+ if (!of_property_read_bool(client->dev.of_node, "use_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
Am 2015-07-06 um 14:34 schrieb Martin Kepplinger:
> This adds the freefall / motion interrupt source definitions to the driver.
> It is used in this series' next patch, for chips that don't support the
> transient interrupt source.
>
> The iio event type is IIO_EV_TYPE_MAG since the threshold for comparison
> is no signed value. An interrupt occurs on positive and negative values
> when the magnitude crosses the threshold value.
>
> 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 71b40f7..1337707 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
> @@ -591,7 +601,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);
> }
> @@ -632,6 +643,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;
>
> @@ -644,7 +656,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;
> }
> @@ -705,6 +720,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
> },
> };
>
> +static const struct iio_event_spec mma8452_motion_event[] = {
> + {
> + .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) |
> + BIT(IIO_EV_INFO_PERIOD)
> + },
> +};
> +
Quick thing: This series applies with all events _pushed_ being still
THRESH.
For the newly added motion events, I show MAG events to the user, but
sill, THRESH events are being pushed when they occur.
In case this inconsistency is not allowed, but you want to take the
series, please either change "IIO_EV_TYPE_MAG" to "IIO_EV_TYPE_THRESH"
or tell me to reply PATCH 3/8 with this half word changed here.
thanks!
> /*
> * Threshold is configured in fixed 8G/127 steps regardless of
> * currently selected scale for measurement.
> @@ -988,13 +1013,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,
>
Am 2015-07-06 um 14:34 schrieb Martin Kepplinger:
> This is version 4 of the mma8452 driver improvements. This version removes
> one patch that changed the iio event type for existing users. It can be
> dealt with seperately and this series applies without it.
>
> Also, changes according to Jonathan's review are applied and a few
> devicetree maintainers are added as recipients.
>
>
> 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,
> 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, nothing should have
> changed.
Any thoughts or reviewed-bys for these changes?
Since there are many more possible users of this driver after these
changes, see the devicetree bindings documentation file, maybe you are
among them and can quickly run it? ;)
thanks a lot,
martin
>
> revision history
> ----------------
> v4 cleanup; one bugfix patch removed from series; DT people added
> 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
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Am 2015-07-09 um 10:51 schrieb Martin Kepplinger:
> Am 2015-07-06 um 14:34 schrieb Martin Kepplinger:
>> This is version 4 of the mma8452 driver improvements. This version removes
>> one patch that changed the iio event type for existing users. It can be
>> dealt with seperately and this series applies without it.
>>
>> Also, changes according to Jonathan's review are applied and a few
>> devicetree maintainers are added as recipients.
>>
>>
>> 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,
>> 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, nothing should have
>> changed.
>
> Any thoughts or reviewed-bys for these changes?
>
> Since there are many more possible users of this driver after these
> changes, see the devicetree bindings documentation file, maybe you are
> among them and can quickly run it? ;)
>
> thanks a lot,
>
> martin
Yes, no, maybe? Anyways. This series applies to the current -next.
thanks
martin
>
>>
>> revision history
>> ----------------
>> v4 cleanup; one bugfix patch removed from series; DT people added
>> 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
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On 13 July 2015 09:07:55 BST, Martin Kepplinger <[email protected]> wrote:
>Am 2015-07-09 um 10:51 schrieb Martin Kepplinger:
>> Am 2015-07-06 um 14:34 schrieb Martin Kepplinger:
>>> This is version 4 of the mma8452 driver improvements. This version
>removes
>>> one patch that changed the iio event type for existing users. It can
>be
>>> dealt with seperately and this series applies without it.
>>>
>>> Also, changes according to Jonathan's review are applied and a few
>>> devicetree maintainers are added as recipients.
>>>
>>>
>>> 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,
>>> 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, nothing should have
>>> changed.
>>
>> Any thoughts or reviewed-bys for these changes?
>>
>> Since there are many more possible users of this driver after these
>> changes, see the devicetree bindings documentation file, maybe you
>are
>> among them and can quickly run it? ;)
>>
>> thanks a lot,
>>
>> martin
>
>Yes, no, maybe? Anyways. This series applies to the current -next.
>
>thanks
> martin
Sorry Martin
Combination of stupid day job week last week. (Of the jump on a plane and 18
hour days variety) also a rather large IKEA build (more fun!) have given me a
fair backlog. Will hopefully catch up during this week.
Initial work though will be getting pull requests out for stuff that has been in
tree for ages so may be later in the week.
Jonathan
>
>>
>>>
>>> revision history
>>> ----------------
>>> v4 cleanup; one bugfix patch removed from series; DT people added
>>> 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
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>linux-kernel" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On 06/07/15 13:34, 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]>
Looks good to me. One small comment inline.
I would however like some feedback (acks) from those more familiar with the particular
driver. Peter and Martin are the obvious candidates for that. So guys,
are you happy with this set?
> ---
> drivers/iio/accel/mma8452.c | 188 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 138 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 13ea1ea..8919bfb 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,52 @@ struct mma8452_data {
> struct mutex lock;
> u8 ctrl_reg1;
> u8 data_cfg;
> + const struct mma_chip_info *chip_info;
> +};
> +
> +/**
> + * struct mma_chip_info - chip specific data for Freescale's accelerometers
> + * @chip_id: WHO_AM_I register's value
> + * @channels: struct iio_chan_spec matching the device's
> + * capabilities
> + * @num_channels: number of channels
> + * @mma_scales: scale factors for converting register values
> + * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
> + * per mode: m/s^2 and micro m/s^2
> + * @ev_cfg: event config register address
> + * @ev_cfg_ele: latch bit in event config register
> + * @ev_cfg_chan_shift: number of the bit to enable events in X
> + * direction; in event config register
> + * @ev_src: event source register address
> + * @ev_src_xe: bit in event source register that indicates
> + * an event in X direction
> + * @ev_src_ye: bit in event source register that indicates
> + * an event in Y direction
> + * @ev_src_ze: bit in event source register that indicates
> + * an event in Z direction
> + * @ev_ths: event threshold register address
> + * @ev_ths_mask: mask for the threshold value
> + * @ev_count: event count (period) register address
> + *
> + * Since not all chips supported by the driver support comparing high pass
> + * filtered data for events (interrupts), different interrupt sources are
> + * used for different chips and the relevant registers are included here.
> + */
> +struct mma_chip_info {
> + u8 chip_id;
> + const struct iio_chan_spec *channels;
> + int num_channels;
> + 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 +191,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 +225,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 +260,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 +270,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 +307,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 +462,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 +514,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 +528,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 +557,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));
> }
>
> static int mma8452_write_event_config(struct iio_dev *indio_dev,
> @@ -528,20 +575,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 +598,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 +717,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 +729,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 +739,41 @@ 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,
> +};
> +
> +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),
> + /*
> + * 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 for 12 bit here is given by:
> + * g * N * 1000000 / 2048 for N = 2, 4, 8 and g=9.80665
> + */
> + .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 +890,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,6 +909,12 @@ 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;
> @@ -833,14 +922,18 @@ static int mma8452_probe(struct i2c_client *client,
> data = iio_priv(indio_dev);
> data->client = client;
> mutex_init(&data->lock);
> + data->chip_info = match->data;
> +
> + 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 = id->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 +1052,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,
This change is unconnected to the rest of patch and generally not
a great move. Why introduced a string define used in only one place?
> .of_match_table = of_match_ptr(mma8452_dt_ids),
> .pm = MMA8452_PM_OPS,
> },
>
On 06/07/15 13:34, 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]>
Again, looks good to me, but I'd like Peter / Martin's Acks ideally.
Jonathan
> ---
> drivers/iio/accel/Kconfig | 6 +++---
> drivers/iio/accel/mma8452.c | 38 +++++++++++++++++++++++++++++++++++---
> 2 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 00e7bcb..dc80dc9 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 MMA8452Q and similar Accelerometers 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 8919bfb..71b40f7 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;
> @@ -745,8 +749,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,
> };
>
> static const struct mma_chip_info mma_chip_info_table[] = {
> @@ -773,7 +785,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,
> @@ -892,6 +921,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] },
> { }
> };
>
> @@ -906,7 +936,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)
Perhaps switch this over to a switch statement to prepare for yet more
devices being supported in future?
> return -ENODEV;
>
> match = of_match_device(mma8452_dt_ids, &client->dev);
> @@ -1053,6 +1084,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 06/07/15 13:34, Martin Kepplinger wrote:
> This adds the freefall / motion interrupt source definitions to the driver.
> It is used in this series' next patch, for chips that don't support the
> transient interrupt source.
>
> The iio event type is IIO_EV_TYPE_MAG since the threshold for comparison
> is no signed value. An interrupt occurs on positive and negative values
> when the magnitude crosses the threshold value.
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> Signed-off-by: Christoph Muellner <[email protected]>
Again, looks fine to me.
> ---
> 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 71b40f7..1337707 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
> @@ -591,7 +601,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);
> }
> @@ -632,6 +643,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;
>
> @@ -644,7 +656,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;
> }
> @@ -705,6 +720,16 @@ static const struct iio_event_spec mma8452_transient_event[] = {
> },
> };
>
> +static const struct iio_event_spec mma8452_motion_event[] = {
> + {
> + .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) |
> + BIT(IIO_EV_INFO_PERIOD)
> + },
> +};
> +
> /*
> * Threshold is configured in fixed 8G/127 steps regardless of
> * currently selected scale for measurement.
> @@ -988,13 +1013,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 06/07/15 13:34, 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]>
The whole series looks good to me. Just want those acks from Peter and / or
Martin before I apply it.
Thanks,
Jonathan
> ---
> 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..9bad1fc 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
> + - use_int2: assume interrupt pin wired to INT2 instead of INT1
>
> Example:
>
> @@ -18,4 +19,5 @@ Example:
> reg = <0x1d>;
> interrupt-parent = <&gpio1>;
> interrupts = <5 0>;
> + use_int2;
> };
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 2b8ed67..f8ba146 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1105,12 +1105,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 "use_int2" is found in DT */
> + if (!of_property_read_bool(client->dev.of_node, "use_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,
>
Am 2015-07-19 um 15:47 schrieb Jonathan Cameron:
> On 06/07/15 13:34, 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]>
> The whole series looks good to me. Just want those acks from Peter and / or
> Martin before I apply it.
>
> Thanks,
>
> Jonathan
Peter at least replied to the first version of these patches, so he
should be around. I suspect that holidays get in the way now.
While I'm at it: I'll be on holidays pretty much all of august. In
general I could maintain the driver and have more improvements planned
on top of this patchset. Maybe I'll include a maintainers file entry
with those (later this year).
And: It shouldn't apply cleanly at least on your fixes-togreg branch of
iio.git. I don't know if this is in -next but in case you don't want to
resolve the diff, please feel free to ask for a clean version of the
whole patchset against -next at any time.
thanks,
martin
>> ---
>> 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..9bad1fc 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
>> + - use_int2: assume interrupt pin wired to INT2 instead of INT1
>>
>> Example:
>>
>> @@ -18,4 +19,5 @@ Example:
>> reg = <0x1d>;
>> interrupt-parent = <&gpio1>;
>> interrupts = <5 0>;
>> + use_int2;
>> };
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 2b8ed67..f8ba146 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -1105,12 +1105,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 "use_int2" is found in DT */
>> + if (!of_property_read_bool(client->dev.of_node, "use_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 19/07/15 15:47, Jonathan Cameron wrote:
> On 06/07/15 13:34, 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.
A remark about this.
The hardware allows each of the interrupt sources (threshold, freefall,
data available, wakeup, ...) to be individiually mapped to one of the
two available interrupt pins.
This patch only allows one pin or the other one to be selected for all
the sources.
Granted this will certainly cover 99% of use cases (even if both are
wired that means we can use either).
However there is one possible case not covered which is when one of the
interrupt signals is connected to some other circuitry (alarm,
power,...) in addition to being used as an interrupt to the CPU.
Normally for code I'd just say let's cross that bridge when we come to
it given that it's pretty unlikely.
However since this affects the DT binding and they can be awkward to
change while keeping compatibility maybe it would be better to use a
bitmask DT property rather than a boolean to actually describe the full
hardware capabilities and keep that door open?
Of course, actually supporting arbitary mappings would be a bit more
work in the driver (both irqs would need to requested...).
However it would be possible to use a bitmask in the DT but, for the
moment, only accept all zeros or all ones, leaving the rest for a future
driver update if and when it is needed.
I don't have a strong objection to doing it the current way, it's
certainly the simplest way and it's likely to be all we ever need just
pointing out a possible shortcoming.
I think It would be good to have the DT maintainers view on this.
>> Signed-off-by: Martin Kepplinger <[email protected]>
>> Signed-off-by: Christoph Muellner <[email protected]>
> The whole series looks good to me. Just want those acks from Peter and / or
> Martin before I apply it.
I'll try to give it a spin this week. Can't promise though
Regards,
Martin
On 2015-07-20 10:38, Martin Fuzzey wrote:
> On 19/07/15 15:47, Jonathan Cameron wrote:
>> On 06/07/15 13:34, 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.
> A remark about this.
>
> The hardware allows each of the interrupt sources (threshold, freefall,
> data available, wakeup, ...) to be individiually mapped to one of the
> two available interrupt pins.
> This patch only allows one pin or the other one to be selected for all
> the sources.
>
> Granted this will certainly cover 99% of use cases (even if both are
> wired that means we can use either).
> However there is one possible case not covered which is when one of the
> interrupt signals is connected to some other circuitry (alarm,
> power,...) in addition to being used as an interrupt to the CPU.
>
> Normally for code I'd just say let's cross that bridge when we come to
> it given that it's pretty unlikely.
> However since this affects the DT binding and they can be awkward to
> change while keeping compatibility maybe it would be better to use a
> bitmask DT property rather than a boolean to actually describe the full
> hardware capabilities and keep that door open?
>
> Of course, actually supporting arbitary mappings would be a bit more
> work in the driver (both irqs would need to requested...).
> However it would be possible to use a bitmask in the DT but, for the
> moment, only accept all zeros or all ones, leaving the rest for a future
> driver update if and when it is needed.
>
> I don't have a strong objection to doing it the current way, it's
> certainly the simplest way and it's likely to be all we ever need just
> pointing out a possible shortcoming.
>
> I think It would be good to have the DT maintainers view on this.
Your're right. I thought about it, and maybe the gain in simplicity is
not worth losing the theoretical option of supporting "weird" users.
I'll probably do another version after your or Peter's review and will
change this.
>
>>> Signed-off-by: Martin Kepplinger
>>> <[email protected]>
>>> Signed-off-by: Christoph Muellner
>>> <[email protected]>
>> The whole series looks good to me. Just want those acks from Peter
>> and / or
>> Martin before I apply it.
> I'll try to give it a spin this week. Can't promise though
>
Thanks so far. It *really* won't take you long to review ;)
>
> Regards,
>
> Martin
>