linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
@ 2015-10-17 22:25 Joachim Eastwood
  2015-10-19 10:45 ` Lars-Peter Clausen
                   ` (5 more replies)
  0 siblings, 6 replies; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-17 22:25 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw; +Cc: Joachim Eastwood, linux-iio

Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
I2C and SPI bus support. This is a rather simple driver that
currently doesn't support all the hardware features of MMA7455L.

Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---

 drivers/iio/accel/Kconfig   |  14 ++
 drivers/iio/accel/Makefile  |   1 +
 drivers/iio/accel/mma7455.c | 424 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 439 insertions(+)
 create mode 100644 drivers/iio/accel/mma7455.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index a59047d7657e..8ccb3de85484 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -99,6 +99,20 @@ config KXCJK1013
 	  To compile this driver as a module, choose M here: the module will
 	  be called kxcjk-1013.
 
+config MMA7455
+	tristate "Freescale MMA7455L Accelerometer Driver"
+	depends on I2C || SPI_MASTER
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	select REGMAP_I2C if I2C
+	select REGMAP_SPI if SPI_MASTER
+	help
+	  Say yes here to build support for the Freescale MMA7455L 3-axis
+	  accelerometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mma7455.
+
 config MMA8452
 	tristate "Freescale MMA8452Q Accelerometer Driver"
 	depends on I2C
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index ebd2675b2a02..6ac9382761f8 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel.o
 obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
 obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
 obj-$(CONFIG_KXSD9)	+= kxsd9.o
+obj-$(CONFIG_MMA7455)	+= mma7455.o
 obj-$(CONFIG_MMA8452)	+= mma8452.o
 
 obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
diff --git a/drivers/iio/accel/mma7455.c b/drivers/iio/accel/mma7455.c
new file mode 100644
index 000000000000..fa62f2bcb54a
--- /dev/null
+++ b/drivers/iio/accel/mma7455.c
@@ -0,0 +1,424 @@
+/*
+ * mma7455.c - Support for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * Based on MMA8452Q IIO driver
+ * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * UNSUPPORTED hardware features:
+ *  - 8-bit mode with different scales
+ *  - INT1/INT2 interrupts
+ *  - Offset calibration
+ *  - Events
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#define MMA7455_REG_XOUTL		0x00
+#define MMA7455_REG_XOUTH		0x01
+#define MMA7455_REG_YOUTL		0x02
+#define MMA7455_REG_YOUTH		0x03
+#define MMA7455_REG_ZOUTL		0x04
+#define MMA7455_REG_ZOUTH		0x05
+#define MMA7455_REG_STATUS		0x09
+#define  MMA7455_STATUS_DRDY		BIT(0)
+#define MMA7455_REG_WHOAMI		0x0f
+#define  MMA7455_WHOAMI_ID		0x55
+#define MMA7455_REG_MCTL		0x16
+#define  MMA7455_MCTL_MODE_STANDBY	0x00
+#define  MMA7455_MCTL_MODE_MEASURE	0x01
+#define MMA7455_REG_CTL1		0x18
+#define  MMA7455_CTL1_DFBW_MASK		BIT(7)
+#define  MMA7455_CTL1_DFBW_125HZ	BIT(7)
+#define  MMA7455_CTL1_DFBW_62_5HZ	0
+#define MMA7455_REG_TW			0x1e
+
+/*
+ * When MMA7455 is used in 10-bit it has a fullscale of -8g
+ * corresponding to raw value -512. The userspace interface
+ * uses m/s^2 and we declare micro units.
+ * So scale factor is given by:
+ *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
+ */
+#define MMA7455_10BIT_SCALE	153229
+
+struct mma7455_data {
+	struct regmap *regmap;
+	struct device *dev;
+};
+
+static int mma7455_drdy(struct mma7455_data *mma7455)
+{
+	unsigned int reg;
+	int tries = 10;
+	int ret;
+
+	while (tries-- > 0) {
+		ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
+		if (ret)
+			return ret;
+
+		if (reg & MMA7455_STATUS_DRDY)
+			return 0;
+
+		msleep(20);
+	}
+
+	dev_warn(mma7455->dev, "data not ready\n");
+
+	return -EIO;
+}
+
+static irqreturn_t mma7455_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
+	int ret;
+
+	ret = mma7455_drdy(mma7455);
+	if (ret)
+		goto done;
+
+	ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
+			       sizeof(s16) * 3);
+	if (ret)
+		goto done;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int mma7455_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	unsigned int reg = 0;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (iio_buffer_enabled(indio_dev))
+			return -EBUSY;
+
+		ret = mma7455_drdy(mma7455);
+		if (ret)
+			return ret;
+
+		ret = regmap_bulk_read(mma7455->regmap, chan->scan_index * 2,
+				       &reg, sizeof(s16));
+		if (ret)
+			return ret;
+
+		*val = sign_extend32(reg, 9);
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = MMA7455_10BIT_SCALE;
+
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
+		if (ret)
+			return ret;
+
+		if (reg & MMA7455_CTL1_DFBW_MASK)
+			*val = 250;
+		else
+			*val = 125;
+
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int mma7455_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	int i;
+
+	if (iio_buffer_enabled(indio_dev))
+		return -EBUSY;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (val == 250)
+			i = MMA7455_CTL1_DFBW_125HZ;
+		else if (val == 125)
+			i = MMA7455_CTL1_DFBW_62_5HZ;
+		else
+			return -EINVAL;
+
+		return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
+					  MMA7455_CTL1_DFBW_MASK, i);
+
+	case IIO_CHAN_INFO_SCALE:
+		/* In 10-bit mode there is only one scale available */
+		if (val == 0 && val2 == MMA7455_10BIT_SCALE)
+			return 0;
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t mma7455_show_samp_freq_avail(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "125 250\n");
+}
+
+static ssize_t mma7455_show_scale_avail(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "0.%u\n", MMA7455_10BIT_SCALE);
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma7455_show_samp_freq_avail);
+static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
+		       mma7455_show_scale_avail, NULL, 0);
+
+static struct attribute *mma7455_attributes[] = {
+	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
+	&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group mma7455_group = {
+	.attrs = mma7455_attributes,
+};
+
+static const struct iio_info mma7455_info = {
+	.attrs = &mma7455_group,
+	.read_raw = mma7455_read_raw,
+	.write_raw = mma7455_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+#define MMA7455_CHANNEL(axis, idx) { \
+	.type = IIO_ACCEL, \
+	.modified = 1, \
+	.channel2 = IIO_MOD_##axis, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+				    BIT(IIO_CHAN_INFO_SCALE), \
+	.scan_index = idx, \
+	.scan_type = { \
+		.sign = 's', \
+		.realbits = 10, \
+		.storagebits = 16, \
+		.shift = 0, \
+		.endianness = IIO_LE, \
+	}, \
+}
+
+static const struct iio_chan_spec mma7455_channels[] = {
+	MMA7455_CHANNEL(X, 0),
+	MMA7455_CHANNEL(Y, 1),
+	MMA7455_CHANNEL(Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const unsigned long mma7455_scan_masks[] = {0x7, 0};
+
+static int mma7455_probe(struct device *dev, struct regmap *regmap)
+{
+	struct mma7455_data *mma7455;
+	struct iio_dev *indio_dev;
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(regmap, MMA7455_REG_WHOAMI, &reg);
+	if (ret) {
+		dev_err(dev, "unable to read reg\n");
+		return ret;
+	}
+
+	if (reg != MMA7455_WHOAMI_ID) {
+		dev_err(dev, "device id mismatch\n");
+		return -ENODEV;
+	}
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*mma7455));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, indio_dev);
+	mma7455 = iio_priv(indio_dev);
+	mma7455->regmap = regmap;
+	mma7455->dev = dev;
+
+	indio_dev->info = &mma7455_info;
+	indio_dev->name = "mma7455l";
+	indio_dev->dev.parent = dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = mma7455_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mma7455_channels);
+	indio_dev->available_scan_masks = mma7455_scan_masks;
+
+	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
+		     MMA7455_MCTL_MODE_MEASURE);
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					 mma7455_trigger_handler, NULL);
+	if (ret) {
+		dev_err(dev, "unable to setup triggered buffer\n");
+		return ret;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "unable to register device\n");
+		iio_triggered_buffer_cleanup(indio_dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int mma7455_remove(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
+		     MMA7455_MCTL_MODE_STANDBY);
+
+	return 0;
+}
+
+static const struct regmap_config mma7455_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MMA7455_REG_TW,
+};
+
+#if IS_ENABLED(CONFIG_SPI_MASTER)
+static int mma7455_spi_probe(struct spi_device *spi)
+{
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_spi(spi, &mma7455_regmap);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return mma7455_probe(&spi->dev, regmap);
+}
+
+static int mma7455_spi_remove(struct spi_device *spi)
+{
+	return mma7455_remove(&spi->dev);
+}
+
+static const struct spi_device_id mma7455_spi_id[] = {
+	{ "mma7455", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, mma7455_spi_id);
+
+static struct spi_driver mma7455_spi_driver = {
+	.probe = mma7455_spi_probe,
+	.remove = mma7455_spi_remove,
+	.id_table = mma7455_spi_id,
+	.driver = {
+		.name = "mma7455-spi",
+	},
+};
+#endif
+
+#if IS_ENABLED(CONFIG_I2C)
+static int mma7455_i2c_probe(struct i2c_client *i2c,
+			     const struct i2c_device_id *id)
+{
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_i2c(i2c, &mma7455_regmap);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return mma7455_probe(&i2c->dev, regmap);
+}
+
+static int mma7455_i2c_remove(struct i2c_client *i2c)
+{
+	return mma7455_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id mma7455_i2c_id[] = {
+	{ "mma7455", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mma7455_i2c_id);
+
+static struct i2c_driver mma7455_i2c_driver = {
+	.probe = mma7455_i2c_probe,
+	.remove = mma7455_i2c_remove,
+	.id_table = mma7455_i2c_id,
+	.driver = {
+		.name	= "mma7455-i2c",
+	},
+};
+#endif
+
+static int __init mma7455_modinit(void)
+{
+	int ret;
+#if IS_ENABLED(CONFIG_I2C)
+	ret = i2c_add_driver(&mma7455_i2c_driver);
+	if (ret)
+		pr_err("failed to register MMA7455L I2C driver: %d\n", ret);
+#endif
+#if IS_ENABLED(CONFIG_SPI_MASTER)
+	ret = spi_register_driver(&mma7455_spi_driver);
+	if (ret)
+		pr_err("failed to register MMA7455L SPI driver: %d\n", ret);
+#endif
+	return ret;
+}
+module_init(mma7455_modinit);
+
+static void __exit mma7455_exit(void)
+{
+#if IS_ENABLED(CONFIG_I2C)
+	i2c_del_driver(&mma7455_i2c_driver);
+#endif
+#if IS_ENABLED(CONFIG_SPI_MASTER)
+	spi_unregister_driver(&mma7455_spi_driver);
+#endif
+}
+module_exit(mma7455_exit);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L I2C/SPI accelerometer driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-17 22:25 [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver Joachim Eastwood
@ 2015-10-19 10:45 ` Lars-Peter Clausen
  2015-10-19 12:19   ` Joachim Eastwood
  2015-10-19 11:00 ` Peter Meerwald
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 36+ messages in thread
From: Lars-Peter Clausen @ 2015-10-19 10:45 UTC (permalink / raw)
  To: Joachim Eastwood, jic23, knaack.h, pmeerw; +Cc: linux-iio

On 10/18/2015 12:25 AM, Joachim Eastwood wrote:
> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
> I2C and SPI bus support. This is a rather simple driver that
> currently doesn't support all the hardware features of MMA7455L.
> 
> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>

Looks pretty good in general.

[...];
> +
> +static int mma7455_drdy(struct mma7455_data *mma7455)
> +{
> +	unsigned int reg;
> +	int tries = 10;
> +	int ret;
> +
> +	while (tries-- > 0) {
> +		ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (reg & MMA7455_STATUS_DRDY)
> +			return 0;
> +
> +		msleep(20);

I'm not to sure about this, when the IRQ fires the data should either be
ready or not, no? Are there any corner cases where waiting for up to 200 ms
will generate valid data, whereas not waiting wont?

> +	}
> +
> +	dev_warn(mma7455->dev, "data not ready\n");
> +
> +	return -EIO;
> +}
> +
[...]
> +static int mma7455_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	unsigned int reg = 0;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (iio_buffer_enabled(indio_dev))
> +			return -EBUSY;
> +
> +		ret = mma7455_drdy(mma7455);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_bulk_read(mma7455->regmap, chan->scan_index * 2,
> +				       &reg, sizeof(s16));

reg is unsigned int while you only read 16 bit. This will cause endianess
issues. If you read a hardware register from a external peripheral always
use __{be,le}{8,16,32} according to the hardware layout and then use the
proper conversion functions {be,le}{8,16,32}_to_cpu() to convert the value
to CPU endianness.

> +		if (ret)
> +			return ret;
> +
> +		*val = sign_extend32(reg, 9);
> +
> +		return IIO_VAL_INT;
> +
[...]
> +static ssize_t mma7455_show_scale_avail(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	return scnprintf(buf, PAGE_SIZE, "0.%u\n", MMA7455_10BIT_SCALE);
> +}

It doesn't make sense to have this at the moment when there is only one
valid scale available.

[...]
> +static int __init mma7455_modinit(void)
> +{
> +	int ret;
> +#if IS_ENABLED(CONFIG_I2C)
> +	ret = i2c_add_driver(&mma7455_i2c_driver);
> +	if (ret)
> +		pr_err("failed to register MMA7455L I2C driver: %d\n", ret);
> +#endif
> +#if IS_ENABLED(CONFIG_SPI_MASTER)
> +	ret = spi_register_driver(&mma7455_spi_driver);
> +	if (ret)
> +		pr_err("failed to register MMA7455L SPI driver: %d\n", ret);
> +#endif

I know there are a fair amount of bad examples in the IIO tree for this,
which do the same thing. But the SPI and the I2C parts should go into
different modules, otherwise you run into issues if one of them is built-in
while the other is built as a module. The bmg160 gyro driver is a good
example on how to do handle this.

> +	return ret;
> +}
> +module_init(mma7455_modinit);


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-17 22:25 [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver Joachim Eastwood
  2015-10-19 10:45 ` Lars-Peter Clausen
@ 2015-10-19 11:00 ` Peter Meerwald
  2015-10-19 12:26   ` Joachim Eastwood
  2015-10-19 11:10 ` Martin Kepplinger
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 36+ messages in thread
From: Peter Meerwald @ 2015-10-19 11:00 UTC (permalink / raw)
  To: Joachim Eastwood; +Cc: jic23, knaack.h, lars, linux-iio


> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
> I2C and SPI bus support. This is a rather simple driver that
> currently doesn't support all the hardware features of MMA7455L.

please add a link to the datasheet

comments below
 
> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---
> 
>  drivers/iio/accel/Kconfig   |  14 ++
>  drivers/iio/accel/Makefile  |   1 +
>  drivers/iio/accel/mma7455.c | 424 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 439 insertions(+)
>  create mode 100644 drivers/iio/accel/mma7455.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index a59047d7657e..8ccb3de85484 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -99,6 +99,20 @@ config KXCJK1013
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called kxcjk-1013.
>  
> +config MMA7455
> +	tristate "Freescale MMA7455L Accelerometer Driver"
> +	depends on I2C || SPI_MASTER
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	select REGMAP_I2C if I2C
> +	select REGMAP_SPI if SPI_MASTER
> +	help
> +	  Say yes here to build support for the Freescale MMA7455L 3-axis
> +	  accelerometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mma7455.
> +
>  config MMA8452
>  	tristate "Freescale MMA8452Q Accelerometer Driver"
>  	depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index ebd2675b2a02..6ac9382761f8 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel.o
>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
> +obj-$(CONFIG_MMA7455)	+= mma7455.o
>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>  
>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
> diff --git a/drivers/iio/accel/mma7455.c b/drivers/iio/accel/mma7455.c
> new file mode 100644
> index 000000000000..fa62f2bcb54a
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455.c
> @@ -0,0 +1,424 @@
> +/*
> + * mma7455.c - Support for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *
> + * Based on MMA8452Q IIO driver
> + * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * UNSUPPORTED hardware features:
> + *  - 8-bit mode with different scales
> + *  - INT1/INT2 interrupts
> + *  - Offset calibration
> + *  - Events
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +
> +#define MMA7455_REG_XOUTL		0x00
> +#define MMA7455_REG_XOUTH		0x01
> +#define MMA7455_REG_YOUTL		0x02
> +#define MMA7455_REG_YOUTH		0x03
> +#define MMA7455_REG_ZOUTL		0x04
> +#define MMA7455_REG_ZOUTH		0x05
> +#define MMA7455_REG_STATUS		0x09
> +#define  MMA7455_STATUS_DRDY		BIT(0)

whitespace inconsistencies

> +#define MMA7455_REG_WHOAMI		0x0f
> +#define  MMA7455_WHOAMI_ID		0x55
> +#define MMA7455_REG_MCTL		0x16
> +#define  MMA7455_MCTL_MODE_STANDBY	0x00
> +#define  MMA7455_MCTL_MODE_MEASURE	0x01
> +#define MMA7455_REG_CTL1		0x18
> +#define  MMA7455_CTL1_DFBW_MASK		BIT(7)
> +#define  MMA7455_CTL1_DFBW_125HZ	BIT(7)
> +#define  MMA7455_CTL1_DFBW_62_5HZ	0
> +#define MMA7455_REG_TW			0x1e
> +
> +/*
> + * When MMA7455 is used in 10-bit it has a fullscale of -8g
> + * corresponding to raw value -512. The userspace interface
> + * uses m/s^2 and we declare micro units.
> + * So scale factor is given by:
> + *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
> + */
> +#define MMA7455_10BIT_SCALE	153229
> +
> +struct mma7455_data {
> +	struct regmap *regmap;
> +	struct device *dev;
> +};
> +
> +static int mma7455_drdy(struct mma7455_data *mma7455)
> +{
> +	unsigned int reg;
> +	int tries = 10;
> +	int ret;
> +
> +	while (tries-- > 0) {
> +		ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (reg & MMA7455_STATUS_DRDY)
> +			return 0;
> +
> +		msleep(20);
> +	}
> +
> +	dev_warn(mma7455->dev, "data not ready\n");
> +
> +	return -EIO;
> +}
> +
> +static irqreturn_t mma7455_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
> +	int ret;
> +
> +	ret = mma7455_drdy(mma7455);
> +	if (ret)
> +		goto done;
> +
> +	ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
> +			       sizeof(s16) * 3);

use le16 to hint which endianness the device registers have

> +	if (ret)
> +		goto done;
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
> +
> +done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mma7455_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	unsigned int reg = 0;

initialization not needed

> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (iio_buffer_enabled(indio_dev))
> +			return -EBUSY;
> +
> +		ret = mma7455_drdy(mma7455);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_bulk_read(mma7455->regmap, chan->scan_index * 2,

it would be clearer to use chan->address instead of computing the register

instead of s16 I suggest to use le16

using regmap_bulk_read(), there is no endianness conversion; the driver 
will only work when chip endianness equals CPU endianness 

> +				       &reg, sizeof(s16));
> +		if (ret)
> +			return ret;
> +
> +		*val = sign_extend32(reg, 9);
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = MMA7455_10BIT_SCALE;
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (reg & MMA7455_CTL1_DFBW_MASK)
> +			*val = 250;
> +		else
> +			*val = 125;
> +
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int mma7455_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	int i;
> +
> +	if (iio_buffer_enabled(indio_dev))
> +		return -EBUSY;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:

check val2 == 0

> +		if (val == 250)
> +			i = MMA7455_CTL1_DFBW_125HZ;
> +		else if (val == 125)
> +			i = MMA7455_CTL1_DFBW_62_5HZ;
> +		else
> +			return -EINVAL;
> +
> +		return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
> +					  MMA7455_CTL1_DFBW_MASK, i);
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		/* In 10-bit mode there is only one scale available */
> +		if (val == 0 && val2 == MMA7455_10BIT_SCALE)
> +			return 0;
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static ssize_t mma7455_show_samp_freq_avail(struct device *dev,
> +					    struct device_attribute *attr,
> +					    char *buf)
> +{
> +	return scnprintf(buf, PAGE_SIZE, "125 250\n");
> +}
> +
> +static ssize_t mma7455_show_scale_avail(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	return scnprintf(buf, PAGE_SIZE, "0.%u\n", MMA7455_10BIT_SCALE);
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma7455_show_samp_freq_avail);
> +static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
> +		       mma7455_show_scale_avail, NULL, 0);
> +
> +static struct attribute *mma7455_attributes[] = {
> +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group mma7455_group = {
> +	.attrs = mma7455_attributes,
> +};
> +
> +static const struct iio_info mma7455_info = {
> +	.attrs = &mma7455_group,
> +	.read_raw = mma7455_read_raw,
> +	.write_raw = mma7455_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +#define MMA7455_CHANNEL(axis, idx) { \
> +	.type = IIO_ACCEL, \
> +	.modified = 1, \
> +	.channel2 = IIO_MOD_##axis, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> +				    BIT(IIO_CHAN_INFO_SCALE), \
> +	.scan_index = idx, \
> +	.scan_type = { \
> +		.sign = 's', \
> +		.realbits = 10, \
> +		.storagebits = 16, \
> +		.shift = 0, \
> +		.endianness = IIO_LE, \
> +	}, \
> +}
> +
> +static const struct iio_chan_spec mma7455_channels[] = {
> +	MMA7455_CHANNEL(X, 0),
> +	MMA7455_CHANNEL(Y, 1),
> +	MMA7455_CHANNEL(Z, 2),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +static const unsigned long mma7455_scan_masks[] = {0x7, 0};
> +
> +static int mma7455_probe(struct device *dev, struct regmap *regmap)
> +{
> +	struct mma7455_data *mma7455;
> +	struct iio_dev *indio_dev;
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(regmap, MMA7455_REG_WHOAMI, &reg);
> +	if (ret) {
> +		dev_err(dev, "unable to read reg\n");
> +		return ret;
> +	}
> +
> +	if (reg != MMA7455_WHOAMI_ID) {
> +		dev_err(dev, "device id mismatch\n");
> +		return -ENODEV;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*mma7455));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, indio_dev);
> +	mma7455 = iio_priv(indio_dev);
> +	mma7455->regmap = regmap;
> +	mma7455->dev = dev;
> +
> +	indio_dev->info = &mma7455_info;
> +	indio_dev->name = "mma7455l";
> +	indio_dev->dev.parent = dev;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mma7455_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(mma7455_channels);
> +	indio_dev->available_scan_masks = mma7455_scan_masks;
> +
> +	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
> +		     MMA7455_MCTL_MODE_MEASURE);
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					 mma7455_trigger_handler, NULL);
> +	if (ret) {
> +		dev_err(dev, "unable to setup triggered buffer\n");
> +		return ret;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(dev, "unable to register device\n");
> +		iio_triggered_buffer_cleanup(indio_dev);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mma7455_remove(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +
> +	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
> +		     MMA7455_MCTL_MODE_STANDBY);
> +
> +	return 0;
> +}
> +
> +static const struct regmap_config mma7455_regmap = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MMA7455_REG_TW,
> +};
> +
> +#if IS_ENABLED(CONFIG_SPI_MASTER)
> +static int mma7455_spi_probe(struct spi_device *spi)
> +{
> +	struct regmap *regmap;
> +
> +	regmap = devm_regmap_init_spi(spi, &mma7455_regmap);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	return mma7455_probe(&spi->dev, regmap);
> +}
> +
> +static int mma7455_spi_remove(struct spi_device *spi)
> +{
> +	return mma7455_remove(&spi->dev);
> +}
> +
> +static const struct spi_device_id mma7455_spi_id[] = {
> +	{ "mma7455", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, mma7455_spi_id);
> +
> +static struct spi_driver mma7455_spi_driver = {
> +	.probe = mma7455_spi_probe,
> +	.remove = mma7455_spi_remove,
> +	.id_table = mma7455_spi_id,
> +	.driver = {
> +		.name = "mma7455-spi",
> +	},
> +};
> +#endif
> +
> +#if IS_ENABLED(CONFIG_I2C)
> +static int mma7455_i2c_probe(struct i2c_client *i2c,
> +			     const struct i2c_device_id *id)
> +{
> +	struct regmap *regmap;
> +
> +	regmap = devm_regmap_init_i2c(i2c, &mma7455_regmap);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	return mma7455_probe(&i2c->dev, regmap);
> +}
> +
> +static int mma7455_i2c_remove(struct i2c_client *i2c)
> +{
> +	return mma7455_remove(&i2c->dev);
> +}
> +
> +static const struct i2c_device_id mma7455_i2c_id[] = {
> +	{ "mma7455", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mma7455_i2c_id);
> +
> +static struct i2c_driver mma7455_i2c_driver = {
> +	.probe = mma7455_i2c_probe,
> +	.remove = mma7455_i2c_remove,
> +	.id_table = mma7455_i2c_id,
> +	.driver = {
> +		.name	= "mma7455-i2c",
> +	},
> +};
> +#endif
> +
> +static int __init mma7455_modinit(void)
> +{
> +	int ret;
> +#if IS_ENABLED(CONFIG_I2C)
> +	ret = i2c_add_driver(&mma7455_i2c_driver);
> +	if (ret)
> +		pr_err("failed to register MMA7455L I2C driver: %d\n", ret);
> +#endif
> +#if IS_ENABLED(CONFIG_SPI_MASTER)
> +	ret = spi_register_driver(&mma7455_spi_driver);
> +	if (ret)
> +		pr_err("failed to register MMA7455L SPI driver: %d\n", ret);
> +#endif
> +	return ret;
> +}
> +module_init(mma7455_modinit);
> +
> +static void __exit mma7455_exit(void)
> +{
> +#if IS_ENABLED(CONFIG_I2C)
> +	i2c_del_driver(&mma7455_i2c_driver);
> +#endif
> +#if IS_ENABLED(CONFIG_SPI_MASTER)
> +	spi_unregister_driver(&mma7455_spi_driver);
> +#endif
> +}
> +module_exit(mma7455_exit);
> +
> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
> +MODULE_DESCRIPTION("Freescale MMA7455L I2C/SPI accelerometer driver");
> +MODULE_LICENSE("GPL v2");
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-17 22:25 [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver Joachim Eastwood
  2015-10-19 10:45 ` Lars-Peter Clausen
  2015-10-19 11:00 ` Peter Meerwald
@ 2015-10-19 11:10 ` Martin Kepplinger
  2015-10-19 12:34   ` Joachim Eastwood
  2015-10-19 19:00 ` [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L " Joachim Eastwood
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 36+ messages in thread
From: Martin Kepplinger @ 2015-10-19 11:10 UTC (permalink / raw)
  To: Joachim Eastwood, jic23, knaack.h, lars, pmeerw; +Cc: linux-iio

Am 2015-10-18 um 00:25 schrieb Joachim Eastwood:
> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
> I2C and SPI bus support. This is a rather simple driver that
> currently doesn't support all the hardware features of MMA7455L.
> 
> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---

Think about adding support for MMA7456L as well. What's the difference
between the two, except for the ID?

thanks
                             martin


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 10:45 ` Lars-Peter Clausen
@ 2015-10-19 12:19   ` Joachim Eastwood
  2015-10-19 12:23     ` Lars-Peter Clausen
  0 siblings, 1 reply; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-19 12:19 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: jic23, knaack.h, pmeerw, linux-iio

Hi Lars-Peter,

On 19 October 2015 at 12:45, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 10/18/2015 12:25 AM, Joachim Eastwood wrote:
>> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
>> I2C and SPI bus support. This is a rather simple driver that
>> currently doesn't support all the hardware features of MMA7455L.
>>
>> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
>>
>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>
> Looks pretty good in general.
>
> [...];
>> +
>> +static int mma7455_drdy(struct mma7455_data *mma7455)
>> +{
>> +     unsigned int reg;
>> +     int tries = 10;
>> +     int ret;
>> +
>> +     while (tries-- > 0) {
>> +             ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
>> +             if (ret)
>> +                     return ret;
>> +
>> +             if (reg & MMA7455_STATUS_DRDY)
>> +                     return 0;
>> +
>> +             msleep(20);
>
> I'm not to sure about this, when the IRQ fires the data should either be
> ready or not, no? Are there any corner cases where waiting for up to 200 ms
> will generate valid data, whereas not waiting wont?

At the moment there is no IRQ support and when used in poll mode I
think checking DRDY is the safest thing to do.
'tries' is probably set a bit high here. I can lower it to 3.
Generating a sample at the lowest sample rate (125 Hz) should take
8ms.


>> +     }
>> +
>> +     dev_warn(mma7455->dev, "data not ready\n");
>> +
>> +     return -EIO;
>> +}
>> +
> [...]
>> +static int mma7455_read_raw(struct iio_dev *indio_dev,
>> +                         struct iio_chan_spec const *chan,
>> +                         int *val, int *val2, long mask)
>> +{
>> +     struct mma7455_data *mma7455 = iio_priv(indio_dev);
>> +     unsigned int reg = 0;
>> +     int ret;
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_RAW:
>> +             if (iio_buffer_enabled(indio_dev))
>> +                     return -EBUSY;
>> +
>> +             ret = mma7455_drdy(mma7455);
>> +             if (ret)
>> +                     return ret;
>> +
>> +             ret = regmap_bulk_read(mma7455->regmap, chan->scan_index * 2,
>> +                                    &reg, sizeof(s16));
>
> reg is unsigned int while you only read 16 bit. This will cause endianess
> issues. If you read a hardware register from a external peripheral always
> use __{be,le}{8,16,32} according to the hardware layout and then use the
> proper conversion functions {be,le}{8,16,32}_to_cpu() to convert the value
> to CPU endianness.

Ok, will do.

>> +             if (ret)
>> +                     return ret;
>> +
>> +             *val = sign_extend32(reg, 9);
>> +
>> +             return IIO_VAL_INT;
>> +
> [...]
>> +static ssize_t mma7455_show_scale_avail(struct device *dev,
>> +                                     struct device_attribute *attr,
>> +                                     char *buf)
>> +{
>> +     return scnprintf(buf, PAGE_SIZE, "0.%u\n", MMA7455_10BIT_SCALE);
>> +}
>
> It doesn't make sense to have this at the moment when there is only one
> valid scale available.

Ok, I'll remove it.


> [...]
>> +static int __init mma7455_modinit(void)
>> +{
>> +     int ret;
>> +#if IS_ENABLED(CONFIG_I2C)
>> +     ret = i2c_add_driver(&mma7455_i2c_driver);
>> +     if (ret)
>> +             pr_err("failed to register MMA7455L I2C driver: %d\n", ret);
>> +#endif
>> +#if IS_ENABLED(CONFIG_SPI_MASTER)
>> +     ret = spi_register_driver(&mma7455_spi_driver);
>> +     if (ret)
>> +             pr_err("failed to register MMA7455L SPI driver: %d\n", ret);
>> +#endif
>
> I know there are a fair amount of bad examples in the IIO tree for this,
> which do the same thing. But the SPI and the I2C parts should go into
> different modules, otherwise you run into issues if one of them is built-in
> while the other is built as a module. The bmg160 gyro driver is a good
> example on how to do handle this.

Ok, I semi-copied from a ASoC CODEC driver. I'll take a look at how
bmg160 handles it.


Thanks for reviewing, I'll implement the changes in the next version.


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 12:19   ` Joachim Eastwood
@ 2015-10-19 12:23     ` Lars-Peter Clausen
  2015-10-19 12:38       ` Joachim Eastwood
  0 siblings, 1 reply; 36+ messages in thread
From: Lars-Peter Clausen @ 2015-10-19 12:23 UTC (permalink / raw)
  To: Joachim Eastwood; +Cc: jic23, knaack.h, pmeerw, linux-iio

On 10/19/2015 02:19 PM, Joachim Eastwood wrote:
>>> +static int __init mma7455_modinit(void)
>>> +{
>>> +     int ret;
>>> +#if IS_ENABLED(CONFIG_I2C)
>>> +     ret = i2c_add_driver(&mma7455_i2c_driver);
>>> +     if (ret)
>>> +             pr_err("failed to register MMA7455L I2C driver: %d\n", ret);
>>> +#endif
>>> +#if IS_ENABLED(CONFIG_SPI_MASTER)
>>> +     ret = spi_register_driver(&mma7455_spi_driver);
>>> +     if (ret)
>>> +             pr_err("failed to register MMA7455L SPI driver: %d\n", ret);
>>> +#endif
>>
>> I know there are a fair amount of bad examples in the IIO tree for this,
>> which do the same thing. But the SPI and the I2C parts should go into
>> different modules, otherwise you run into issues if one of them is built-in
>> while the other is built as a module. The bmg160 gyro driver is a good
>> example on how to do handle this.
> 
> Ok, I semi-copied from a ASoC CODEC driver. I'll take a look at how
> bmg160 handles it.

We deprecated this style in ASoC as well. All new drivers that support both
SPI and I2C always come with a foobar-spi.c and foobar-i2c.c file. Not all
drivers have been updated to this new scheme yet, but I think the majority
follows it by now.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 11:00 ` Peter Meerwald
@ 2015-10-19 12:26   ` Joachim Eastwood
  2015-10-19 12:38     ` Peter Meerwald
  0 siblings, 1 reply; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-19 12:26 UTC (permalink / raw)
  To: Peter Meerwald; +Cc: jic23, knaack.h, Lars-Peter Clausen, linux-iio

Hi Peter,

On 19 October 2015 at 13:00, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>
>> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
>> I2C and SPI bus support. This is a rather simple driver that
>> currently doesn't support all the hardware features of MMA7455L.
>
> please add a link to the datasheet

Sure.

>
> comments below
>
>> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
>>
>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> ---
>>
>>  drivers/iio/accel/Kconfig   |  14 ++
>>  drivers/iio/accel/Makefile  |   1 +
>>  drivers/iio/accel/mma7455.c | 424 ++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 439 insertions(+)
>>  create mode 100644 drivers/iio/accel/mma7455.c
>>
>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>> index a59047d7657e..8ccb3de85484 100644
>> --- a/drivers/iio/accel/Kconfig
>> +++ b/drivers/iio/accel/Kconfig
>> @@ -99,6 +99,20 @@ config KXCJK1013
>>         To compile this driver as a module, choose M here: the module will
>>         be called kxcjk-1013.
>>
>> +config MMA7455
>> +     tristate "Freescale MMA7455L Accelerometer Driver"
>> +     depends on I2C || SPI_MASTER
>> +     select IIO_BUFFER
>> +     select IIO_TRIGGERED_BUFFER
>> +     select REGMAP_I2C if I2C
>> +     select REGMAP_SPI if SPI_MASTER
>> +     help
>> +       Say yes here to build support for the Freescale MMA7455L 3-axis
>> +       accelerometer.
>> +
>> +       To compile this driver as a module, choose M here: the module
>> +       will be called mma7455.
>> +
>>  config MMA8452
>>       tristate "Freescale MMA8452Q Accelerometer Driver"
>>       depends on I2C
>> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
>> index ebd2675b2a02..6ac9382761f8 100644
>> --- a/drivers/iio/accel/Makefile
>> +++ b/drivers/iio/accel/Makefile
>> @@ -8,6 +8,7 @@ obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel.o
>>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>>  obj-$(CONFIG_KXSD9)  += kxsd9.o
>> +obj-$(CONFIG_MMA7455)        += mma7455.o
>>  obj-$(CONFIG_MMA8452)        += mma8452.o
>>
>>  obj-$(CONFIG_MMA9551_CORE)   += mma9551_core.o
>> diff --git a/drivers/iio/accel/mma7455.c b/drivers/iio/accel/mma7455.c
>> new file mode 100644
>> index 000000000000..fa62f2bcb54a
>> --- /dev/null
>> +++ b/drivers/iio/accel/mma7455.c
>> @@ -0,0 +1,424 @@
>> +/*
>> + * mma7455.c - Support for Freescale MMA7455L 3-axis 10-bit accelerometer
>> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
>> + *
>> + * Based on MMA8452Q IIO driver
>> + * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * UNSUPPORTED hardware features:
>> + *  - 8-bit mode with different scales
>> + *  - INT1/INT2 interrupts
>> + *  - Offset calibration
>> + *  - Events
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/i2c.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/spi/spi.h>
>> +
>> +#define MMA7455_REG_XOUTL            0x00
>> +#define MMA7455_REG_XOUTH            0x01
>> +#define MMA7455_REG_YOUTL            0x02
>> +#define MMA7455_REG_YOUTH            0x03
>> +#define MMA7455_REG_ZOUTL            0x04
>> +#define MMA7455_REG_ZOUTH            0x05
>> +#define MMA7455_REG_STATUS           0x09
>> +#define  MMA7455_STATUS_DRDY         BIT(0)
>
> whitespace inconsistencies

This is actually intentional and used to show that it is a field in a
register. If you don't like it I can remove it.

>> +#define MMA7455_REG_WHOAMI           0x0f
>> +#define  MMA7455_WHOAMI_ID           0x55
>> +#define MMA7455_REG_MCTL             0x16
>> +#define  MMA7455_MCTL_MODE_STANDBY   0x00
>> +#define  MMA7455_MCTL_MODE_MEASURE   0x01
>> +#define MMA7455_REG_CTL1             0x18
>> +#define  MMA7455_CTL1_DFBW_MASK              BIT(7)
>> +#define  MMA7455_CTL1_DFBW_125HZ     BIT(7)
>> +#define  MMA7455_CTL1_DFBW_62_5HZ    0
>> +#define MMA7455_REG_TW                       0x1e
>> +
>> +/*
>> + * When MMA7455 is used in 10-bit it has a fullscale of -8g
>> + * corresponding to raw value -512. The userspace interface
>> + * uses m/s^2 and we declare micro units.
>> + * So scale factor is given by:
>> + *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
>> + */
>> +#define MMA7455_10BIT_SCALE  153229
>> +
>> +struct mma7455_data {
>> +     struct regmap *regmap;
>> +     struct device *dev;
>> +};
>> +
>> +static int mma7455_drdy(struct mma7455_data *mma7455)
>> +{
>> +     unsigned int reg;
>> +     int tries = 10;
>> +     int ret;
>> +
>> +     while (tries-- > 0) {
>> +             ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
>> +             if (ret)
>> +                     return ret;
>> +
>> +             if (reg & MMA7455_STATUS_DRDY)
>> +                     return 0;
>> +
>> +             msleep(20);
>> +     }
>> +
>> +     dev_warn(mma7455->dev, "data not ready\n");
>> +
>> +     return -EIO;
>> +}
>> +
>> +static irqreturn_t mma7455_trigger_handler(int irq, void *p)
>> +{
>> +     struct iio_poll_func *pf = p;
>> +     struct iio_dev *indio_dev = pf->indio_dev;
>> +     struct mma7455_data *mma7455 = iio_priv(indio_dev);
>> +     u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
>> +     int ret;
>> +
>> +     ret = mma7455_drdy(mma7455);
>> +     if (ret)
>> +             goto done;
>> +
>> +     ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
>> +                            sizeof(s16) * 3);
>
> use le16 to hint which endianness the device registers have

OK.

>> +     if (ret)
>> +             goto done;
>> +
>> +     iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
>> +
>> +done:
>> +     iio_trigger_notify_done(indio_dev->trig);
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static int mma7455_read_raw(struct iio_dev *indio_dev,
>> +                         struct iio_chan_spec const *chan,
>> +                         int *val, int *val2, long mask)
>> +{
>> +     struct mma7455_data *mma7455 = iio_priv(indio_dev);
>> +     unsigned int reg = 0;
>
> initialization not needed

I think it may complain when used in regmap_bulk_read() later. I'll check.


>> +     int ret;
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_RAW:
>> +             if (iio_buffer_enabled(indio_dev))
>> +                     return -EBUSY;
>> +
>> +             ret = mma7455_drdy(mma7455);
>> +             if (ret)
>> +                     return ret;
>> +
>> +             ret = regmap_bulk_read(mma7455->regmap, chan->scan_index * 2,
>
> it would be clearer to use chan->address instead of computing the register

ah, I didn't noticed the address field in chan_spec. I'll use that in
the next version.

>
> instead of s16 I suggest to use le16

OK.

> using regmap_bulk_read(), there is no endianness conversion; the driver
> will only work when chip endianness equals CPU endianness

I'll add conversion using the to_cpu-functions in the next version.

>
>> +                                    &reg, sizeof(s16));
>> +             if (ret)
>> +                     return ret;
>> +
>> +             *val = sign_extend32(reg, 9);
>> +
>> +             return IIO_VAL_INT;
>> +
>> +     case IIO_CHAN_INFO_SCALE:
>> +             *val = 0;
>> +             *val2 = MMA7455_10BIT_SCALE;
>> +
>> +             return IIO_VAL_INT_PLUS_MICRO;
>> +
>> +     case IIO_CHAN_INFO_SAMP_FREQ:
>> +             ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
>> +             if (ret)
>> +                     return ret;
>> +
>> +             if (reg & MMA7455_CTL1_DFBW_MASK)
>> +                     *val = 250;
>> +             else
>> +                     *val = 125;
>> +
>> +             return IIO_VAL_INT;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static int mma7455_write_raw(struct iio_dev *indio_dev,
>> +                          struct iio_chan_spec const *chan,
>> +                          int val, int val2, long mask)
>> +{
>> +     struct mma7455_data *mma7455 = iio_priv(indio_dev);
>> +     int i;
>> +
>> +     if (iio_buffer_enabled(indio_dev))
>> +             return -EBUSY;
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_SAMP_FREQ:
>
> check val2 == 0

OK.

>> +             if (val == 250)
>> +                     i = MMA7455_CTL1_DFBW_125HZ;
>> +             else if (val == 125)
>> +                     i = MMA7455_CTL1_DFBW_62_5HZ;
>> +             else
>> +                     return -EINVAL;
>> +
>> +             return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
>> +                                       MMA7455_CTL1_DFBW_MASK, i);
>> +
>> +     case IIO_CHAN_INFO_SCALE:
>> +             /* In 10-bit mode there is only one scale available */
>> +             if (val == 0 && val2 == MMA7455_10BIT_SCALE)
>> +                     return 0;
>> +             break;
>> +     }
>> +
>> +     return -EINVAL;
>> +}


Thanks for reviewing, I'll implement the changes in the next version.


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 11:10 ` Martin Kepplinger
@ 2015-10-19 12:34   ` Joachim Eastwood
  2015-10-19 12:56     ` Martin Kepplinger
  0 siblings, 1 reply; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-19 12:34 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: jic23, knaack.h, Lars-Peter Clausen, Peter Meerwald, linux-iio

Hi Martin,

On 19 October 2015 at 13:10, Martin Kepplinger <martink@posteo.de> wrote:
> Am 2015-10-18 um 00:25 schrieb Joachim Eastwood:
>> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
>> I2C and SPI bus support. This is a rather simple driver that
>> currently doesn't support all the hardware features of MMA7455L.
>>
>> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
>>
>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> ---
>
> Think about adding support for MMA7456L as well. What's the difference
> between the two, except for the ID?

I think the driver should work with MMA7456L as well.
I have tried to figure out the difference between the two devices, but
the Freescale doesn't list them. One another annoying thing is that
the ID isn't actually stated in the data sheet (!). So for MMA7456L I
would guess it is 0x56, but there is no way to be sure.


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 12:23     ` Lars-Peter Clausen
@ 2015-10-19 12:38       ` Joachim Eastwood
  0 siblings, 0 replies; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-19 12:38 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: jic23, knaack.h, Peter Meerwald, linux-iio

On 19 October 2015 at 14:23, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 10/19/2015 02:19 PM, Joachim Eastwood wrote:
>>>> +static int __init mma7455_modinit(void)
>>>> +{
>>>> +     int ret;
>>>> +#if IS_ENABLED(CONFIG_I2C)
>>>> +     ret = i2c_add_driver(&mma7455_i2c_driver);
>>>> +     if (ret)
>>>> +             pr_err("failed to register MMA7455L I2C driver: %d\n", ret);
>>>> +#endif
>>>> +#if IS_ENABLED(CONFIG_SPI_MASTER)
>>>> +     ret = spi_register_driver(&mma7455_spi_driver);
>>>> +     if (ret)
>>>> +             pr_err("failed to register MMA7455L SPI driver: %d\n", ret);
>>>> +#endif
>>>
>>> I know there are a fair amount of bad examples in the IIO tree for this,
>>> which do the same thing. But the SPI and the I2C parts should go into
>>> different modules, otherwise you run into issues if one of them is built-in
>>> while the other is built as a module. The bmg160 gyro driver is a good
>>> example on how to do handle this.
>>
>> Ok, I semi-copied from a ASoC CODEC driver. I'll take a look at how
>> bmg160 handles it.
>
> We deprecated this style in ASoC as well. All new drivers that support both
> SPI and I2C always come with a foobar-spi.c and foobar-i2c.c file. Not all
> drivers have been updated to this new scheme yet, but I think the majority
> follows it by now.

I see.

Only one of the Wolfson CODEC drivers has been split this way in Linus
master it seems and it was one of these I took as a template.


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 12:26   ` Joachim Eastwood
@ 2015-10-19 12:38     ` Peter Meerwald
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Meerwald @ 2015-10-19 12:38 UTC (permalink / raw)
  To: Joachim Eastwood; +Cc: jic23, knaack.h, Lars-Peter Clausen, linux-iio


> >> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
> >> I2C and SPI bus support. This is a rather simple driver that
> >> currently doesn't support all the hardware features of MMA7455L.
> >
> > please add a link to the datasheet
> 
> Sure.
> 
> >
> > comments below
> >
> >> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
> >>
> >> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> >> ---
> >>
> >>  drivers/iio/accel/Kconfig   |  14 ++
> >>  drivers/iio/accel/Makefile  |   1 +
> >>  drivers/iio/accel/mma7455.c | 424 ++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 439 insertions(+)
> >>  create mode 100644 drivers/iio/accel/mma7455.c
> >>
> >> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> >> index a59047d7657e..8ccb3de85484 100644
> >> --- a/drivers/iio/accel/Kconfig
> >> +++ b/drivers/iio/accel/Kconfig
> >> @@ -99,6 +99,20 @@ config KXCJK1013
> >>         To compile this driver as a module, choose M here: the module will
> >>         be called kxcjk-1013.
> >>
> >> +config MMA7455
> >> +     tristate "Freescale MMA7455L Accelerometer Driver"
> >> +     depends on I2C || SPI_MASTER
> >> +     select IIO_BUFFER
> >> +     select IIO_TRIGGERED_BUFFER
> >> +     select REGMAP_I2C if I2C
> >> +     select REGMAP_SPI if SPI_MASTER
> >> +     help
> >> +       Say yes here to build support for the Freescale MMA7455L 3-axis
> >> +       accelerometer.
> >> +
> >> +       To compile this driver as a module, choose M here: the module
> >> +       will be called mma7455.
> >> +
> >>  config MMA8452
> >>       tristate "Freescale MMA8452Q Accelerometer Driver"
> >>       depends on I2C
> >> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> >> index ebd2675b2a02..6ac9382761f8 100644
> >> --- a/drivers/iio/accel/Makefile
> >> +++ b/drivers/iio/accel/Makefile
> >> @@ -8,6 +8,7 @@ obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel.o
> >>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
> >>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
> >>  obj-$(CONFIG_KXSD9)  += kxsd9.o
> >> +obj-$(CONFIG_MMA7455)        += mma7455.o
> >>  obj-$(CONFIG_MMA8452)        += mma8452.o
> >>
> >>  obj-$(CONFIG_MMA9551_CORE)   += mma9551_core.o
> >> diff --git a/drivers/iio/accel/mma7455.c b/drivers/iio/accel/mma7455.c
> >> new file mode 100644
> >> index 000000000000..fa62f2bcb54a
> >> --- /dev/null
> >> +++ b/drivers/iio/accel/mma7455.c
> >> @@ -0,0 +1,424 @@
> >> +/*
> >> + * mma7455.c - Support for Freescale MMA7455L 3-axis 10-bit accelerometer
> >> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> >> + *
> >> + * Based on MMA8452Q IIO driver
> >> + * Copyright 2014 Peter Meerwald <pmeerw@pmeerw.net>
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + *
> >> + * UNSUPPORTED hardware features:
> >> + *  - 8-bit mode with different scales
> >> + *  - INT1/INT2 interrupts
> >> + *  - Offset calibration
> >> + *  - Events
> >> + */
> >> +
> >> +#include <linux/delay.h>
> >> +#include <linux/i2c.h>
> >> +#include <linux/iio/iio.h>
> >> +#include <linux/iio/sysfs.h>
> >> +#include <linux/iio/buffer.h>
> >> +#include <linux/iio/trigger.h>
> >> +#include <linux/iio/trigger_consumer.h>
> >> +#include <linux/iio/triggered_buffer.h>
> >> +#include <linux/module.h>
> >> +#include <linux/regmap.h>
> >> +#include <linux/spi/spi.h>
> >> +
> >> +#define MMA7455_REG_XOUTL            0x00
> >> +#define MMA7455_REG_XOUTH            0x01
> >> +#define MMA7455_REG_YOUTL            0x02
> >> +#define MMA7455_REG_YOUTH            0x03
> >> +#define MMA7455_REG_ZOUTL            0x04
> >> +#define MMA7455_REG_ZOUTH            0x05
> >> +#define MMA7455_REG_STATUS           0x09
> >> +#define  MMA7455_STATUS_DRDY         BIT(0)
> >
> > whitespace inconsistencies
> 
> This is actually intentional and used to show that it is a field in a
> register. If you don't like it I can remove it.

hm, I didn't get it; it's probably a good idea :)
 
> >> +#define MMA7455_REG_WHOAMI           0x0f
> >> +#define  MMA7455_WHOAMI_ID           0x55
> >> +#define MMA7455_REG_MCTL             0x16
> >> +#define  MMA7455_MCTL_MODE_STANDBY   0x00
> >> +#define  MMA7455_MCTL_MODE_MEASURE   0x01
> >> +#define MMA7455_REG_CTL1             0x18
> >> +#define  MMA7455_CTL1_DFBW_MASK              BIT(7)
> >> +#define  MMA7455_CTL1_DFBW_125HZ     BIT(7)
> >> +#define  MMA7455_CTL1_DFBW_62_5HZ    0
> >> +#define MMA7455_REG_TW                       0x1e
> >> +
> >> +/*
> >> + * When MMA7455 is used in 10-bit it has a fullscale of -8g
> >> + * corresponding to raw value -512. The userspace interface
> >> + * uses m/s^2 and we declare micro units.
> >> + * So scale factor is given by:
> >> + *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
> >> + */
> >> +#define MMA7455_10BIT_SCALE  153229
> >> +
> >> +struct mma7455_data {
> >> +     struct regmap *regmap;
> >> +     struct device *dev;
> >> +};
> >> +
> >> +static int mma7455_drdy(struct mma7455_data *mma7455)
> >> +{
> >> +     unsigned int reg;
> >> +     int tries = 10;
> >> +     int ret;
> >> +
> >> +     while (tries-- > 0) {
> >> +             ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
> >> +             if (ret)
> >> +                     return ret;
> >> +
> >> +             if (reg & MMA7455_STATUS_DRDY)
> >> +                     return 0;
> >> +
> >> +             msleep(20);
> >> +     }
> >> +
> >> +     dev_warn(mma7455->dev, "data not ready\n");
> >> +
> >> +     return -EIO;
> >> +}
> >> +
> >> +static irqreturn_t mma7455_trigger_handler(int irq, void *p)
> >> +{
> >> +     struct iio_poll_func *pf = p;
> >> +     struct iio_dev *indio_dev = pf->indio_dev;
> >> +     struct mma7455_data *mma7455 = iio_priv(indio_dev);
> >> +     u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
> >> +     int ret;
> >> +
> >> +     ret = mma7455_drdy(mma7455);
> >> +     if (ret)
> >> +             goto done;
> >> +
> >> +     ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
> >> +                            sizeof(s16) * 3);
> >
> > use le16 to hint which endianness the device registers have
> 
> OK.
> 
> >> +     if (ret)
> >> +             goto done;
> >> +
> >> +     iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
> >> +
> >> +done:
> >> +     iio_trigger_notify_done(indio_dev->trig);
> >> +
> >> +     return IRQ_HANDLED;
> >> +}
> >> +
> >> +static int mma7455_read_raw(struct iio_dev *indio_dev,
> >> +                         struct iio_chan_spec const *chan,
> >> +                         int *val, int *val2, long mask)
> >> +{
> >> +     struct mma7455_data *mma7455 = iio_priv(indio_dev);
> >> +     unsigned int reg = 0;
> >
> > initialization not needed
> 
> I think it may complain when used in regmap_bulk_read() later. I'll check.

yes, better leave it in; not sure how _read() is implemented when less 
then sizeof(unsigned int) is read
 
> 
> >> +     int ret;
> >> +
> >> +     switch (mask) {
> >> +     case IIO_CHAN_INFO_RAW:
> >> +             if (iio_buffer_enabled(indio_dev))
> >> +                     return -EBUSY;
> >> +
> >> +             ret = mma7455_drdy(mma7455);
> >> +             if (ret)
> >> +                     return ret;
> >> +
> >> +             ret = regmap_bulk_read(mma7455->regmap, chan->scan_index * 2,
> >
> > it would be clearer to use chan->address instead of computing the register
> 
> ah, I didn't noticed the address field in chan_spec. I'll use that in
> the next version.
> 
> >
> > instead of s16 I suggest to use le16
> 
> OK.
> 
> > using regmap_bulk_read(), there is no endianness conversion; the driver
> > will only work when chip endianness equals CPU endianness
> 
> I'll add conversion using the to_cpu-functions in the next version.
> 
> >
> >> +                                    &reg, sizeof(s16));
> >> +             if (ret)
> >> +                     return ret;
> >> +
> >> +             *val = sign_extend32(reg, 9);
> >> +
> >> +             return IIO_VAL_INT;
> >> +
> >> +     case IIO_CHAN_INFO_SCALE:
> >> +             *val = 0;
> >> +             *val2 = MMA7455_10BIT_SCALE;
> >> +
> >> +             return IIO_VAL_INT_PLUS_MICRO;
> >> +
> >> +     case IIO_CHAN_INFO_SAMP_FREQ:
> >> +             ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
> >> +             if (ret)
> >> +                     return ret;
> >> +
> >> +             if (reg & MMA7455_CTL1_DFBW_MASK)
> >> +                     *val = 250;
> >> +             else
> >> +                     *val = 125;
> >> +
> >> +             return IIO_VAL_INT;
> >> +     }
> >> +
> >> +     return -EINVAL;
> >> +}
> >> +
> >> +static int mma7455_write_raw(struct iio_dev *indio_dev,
> >> +                          struct iio_chan_spec const *chan,
> >> +                          int val, int val2, long mask)
> >> +{
> >> +     struct mma7455_data *mma7455 = iio_priv(indio_dev);
> >> +     int i;
> >> +
> >> +     if (iio_buffer_enabled(indio_dev))
> >> +             return -EBUSY;
> >> +
> >> +     switch (mask) {
> >> +     case IIO_CHAN_INFO_SAMP_FREQ:
> >
> > check val2 == 0
> 
> OK.
> 
> >> +             if (val == 250)
> >> +                     i = MMA7455_CTL1_DFBW_125HZ;
> >> +             else if (val == 125)
> >> +                     i = MMA7455_CTL1_DFBW_62_5HZ;
> >> +             else
> >> +                     return -EINVAL;
> >> +
> >> +             return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
> >> +                                       MMA7455_CTL1_DFBW_MASK, i);
> >> +
> >> +     case IIO_CHAN_INFO_SCALE:
> >> +             /* In 10-bit mode there is only one scale available */
> >> +             if (val == 0 && val2 == MMA7455_10BIT_SCALE)
> >> +                     return 0;
> >> +             break;
> >> +     }
> >> +
> >> +     return -EINVAL;
> >> +}
> 
> 
> Thanks for reviewing, I'll implement the changes in the next version.
> 
> 
> regards,
> Joachim Eastwood
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 12:34   ` Joachim Eastwood
@ 2015-10-19 12:56     ` Martin Kepplinger
  2015-10-19 13:43       ` Joachim Eastwood
  0 siblings, 1 reply; 36+ messages in thread
From: Martin Kepplinger @ 2015-10-19 12:56 UTC (permalink / raw)
  To: Joachim Eastwood
  Cc: jic23, knaack.h, Lars-Peter Clausen, Peter Meerwald, linux-iio

Am 2015-10-19 um 14:34 schrieb Joachim Eastwood:
> Hi Martin,
> 
> On 19 October 2015 at 13:10, Martin Kepplinger <martink@posteo.de> wrote:
>> Am 2015-10-18 um 00:25 schrieb Joachim Eastwood:
>>> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
>>> I2C and SPI bus support. This is a rather simple driver that
>>> currently doesn't support all the hardware features of MMA7455L.
>>>
>>> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
>>>
>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>>> ---
>>
>> Think about adding support for MMA7456L as well. What's the difference
>> between the two, except for the ID?
> 
> I think the driver should work with MMA7456L as well.
> I have tried to figure out the difference between the two devices, but
> the Freescale doesn't list them. One another annoying thing is that
> the ID isn't actually stated in the data sheet (!). So for MMA7456L I
> would guess it is 0x56, but there is no way to be sure.
> 

You're right, that's messy. I asked freescale about it and we'll see
if/when there's an answer.

thanks
                      martin



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 12:56     ` Martin Kepplinger
@ 2015-10-19 13:43       ` Joachim Eastwood
  2015-10-19 14:09         ` Martin Kepplinger
  0 siblings, 1 reply; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-19 13:43 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: jic23, knaack.h, Lars-Peter Clausen, Peter Meerwald, linux-iio

On 19 October 2015 at 14:56, Martin Kepplinger <martink@posteo.de> wrote:
> Am 2015-10-19 um 14:34 schrieb Joachim Eastwood:
>> Hi Martin,
>>
>> On 19 October 2015 at 13:10, Martin Kepplinger <martink@posteo.de> wrote:
>>> Am 2015-10-18 um 00:25 schrieb Joachim Eastwood:
>>>> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
>>>> I2C and SPI bus support. This is a rather simple driver that
>>>> currently doesn't support all the hardware features of MMA7455L.
>>>>
>>>> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
>>>>
>>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>>>> ---
>>>
>>> Think about adding support for MMA7456L as well. What's the difference
>>> between the two, except for the ID?
>>
>> I think the driver should work with MMA7456L as well.
>> I have tried to figure out the difference between the two devices, but
>> the Freescale doesn't list them. One another annoying thing is that
>> the ID isn't actually stated in the data sheet (!). So for MMA7456L I
>> would guess it is 0x56, but there is no way to be sure.
>>
>
> You're right, that's messy. I asked freescale about it and we'll see
> if/when there's an answer.

ah, nice. Thanks for taking the time to doing that.


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 13:43       ` Joachim Eastwood
@ 2015-10-19 14:09         ` Martin Kepplinger
  2015-10-19 14:14           ` Lars-Peter Clausen
  0 siblings, 1 reply; 36+ messages in thread
From: Martin Kepplinger @ 2015-10-19 14:09 UTC (permalink / raw)
  To: Joachim Eastwood
  Cc: jic23, knaack.h, Lars-Peter Clausen, Peter Meerwald, linux-iio

Am 2015-10-19 um 15:43 schrieb Joachim Eastwood:
> On 19 October 2015 at 14:56, Martin Kepplinger <martink@posteo.de> wrote:
>> Am 2015-10-19 um 14:34 schrieb Joachim Eastwood:
>>> Hi Martin,
>>>
>>> On 19 October 2015 at 13:10, Martin Kepplinger <martink@posteo.de> wrote:
>>>> Am 2015-10-18 um 00:25 schrieb Joachim Eastwood:
>>>>> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
>>>>> I2C and SPI bus support. This is a rather simple driver that
>>>>> currently doesn't support all the hardware features of MMA7455L.
>>>>>
>>>>> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
>>>>>
>>>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>>>>> ---
>>>>
>>>> Think about adding support for MMA7456L as well. What's the difference
>>>> between the two, except for the ID?
>>>
>>> I think the driver should work with MMA7456L as well.
>>> I have tried to figure out the difference between the two devices, but
>>> the Freescale doesn't list them. One another annoying thing is that
>>> the ID isn't actually stated in the data sheet (!). So for MMA7456L I
>>> would guess it is 0x56, but there is no way to be sure.
>>>
>>
>> You're right, that's messy. I asked freescale about it and we'll see
>> if/when there's an answer.
> 
> ah, nice. Thanks for taking the time to doing that.
> 

MMA7456L's WHO_AM_I value should also be 0x55, see
https://community.freescale.com/thread/378035

You could think about renaming the driver to mma745xl which is this
"series".

At least you can add MMA7456L to your Kconfig and all your docs.

                        martin


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver
  2015-10-19 14:09         ` Martin Kepplinger
@ 2015-10-19 14:14           ` Lars-Peter Clausen
  0 siblings, 0 replies; 36+ messages in thread
From: Lars-Peter Clausen @ 2015-10-19 14:14 UTC (permalink / raw)
  To: Martin Kepplinger, Joachim Eastwood
  Cc: jic23, knaack.h, Peter Meerwald, linux-iio

On 10/19/2015 04:09 PM, Martin Kepplinger wrote:
> Am 2015-10-19 um 15:43 schrieb Joachim Eastwood:
>> On 19 October 2015 at 14:56, Martin Kepplinger <martink@posteo.de> wrote:
>>> Am 2015-10-19 um 14:34 schrieb Joachim Eastwood:
>>>> Hi Martin,
>>>>
>>>> On 19 October 2015 at 13:10, Martin Kepplinger <martink@posteo.de> wrote:
>>>>> Am 2015-10-18 um 00:25 schrieb Joachim Eastwood:
>>>>>> Add support for Freescale MMA7455L 3-axis in 10-bit mode with both
>>>>>> I2C and SPI bus support. This is a rather simple driver that
>>>>>> currently doesn't support all the hardware features of MMA7455L.
>>>>>>
>>>>>> Tested on Embedded Artists' LPC4357 Dev Kit using I2C bus.
>>>>>>
>>>>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>>>>>> ---
>>>>>
>>>>> Think about adding support for MMA7456L as well. What's the difference
>>>>> between the two, except for the ID?
>>>>
>>>> I think the driver should work with MMA7456L as well.
>>>> I have tried to figure out the difference between the two devices, but
>>>> the Freescale doesn't list them. One another annoying thing is that
>>>> the ID isn't actually stated in the data sheet (!). So for MMA7456L I
>>>> would guess it is 0x56, but there is no way to be sure.
>>>>
>>>
>>> You're right, that's messy. I asked freescale about it and we'll see
>>> if/when there's an answer.
>>
>> ah, nice. Thanks for taking the time to doing that.
>>
> 
> MMA7456L's WHO_AM_I value should also be 0x55, see
> https://community.freescale.com/thread/378035
> 
> You could think about renaming the driver to mma745xl which is this
> "series".

Please no X's in driver names. That's always a mess if multiple different
types of devices match the name. E.g. lets say they release a MMA7459 which
is a gyro or whatever in the future.

Just name the driver after the first device supported.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-17 22:25 [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver Joachim Eastwood
                   ` (2 preceding siblings ...)
  2015-10-19 11:10 ` Martin Kepplinger
@ 2015-10-19 19:00 ` Joachim Eastwood
  2015-10-19 21:07   ` Joachim Eastwood
                     ` (2 more replies)
  2015-10-20 20:50 ` [PATCH v3] " Joachim Eastwood
  2015-10-31 12:49 ` [PATCH v4] " Joachim Eastwood
  5 siblings, 3 replies; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-19 19:00 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw; +Cc: Joachim Eastwood, martink, linux-iio

Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
I2C and SPI bus. This rather simple driver that currently doesn't
support all the hardware features of MMA7455L/MMA7456L.

Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.

Data sheets for the two devices can be found here:
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
Hi,

This version address the comments from Lars-Peter Clausen, Peter
Meerwald and Martin Kepplinger. Thanks for all the constructive
feedback!

Changes since v1:
* limit retries to 3 in mma7455_drdy
* remove mma7455_show_scale_avail
* use chan->address instead of chan->scan_index for reg addr
* check that val2 is 0 when setting sample freq
* use __le16 to hint about endianess in mma7455_trigger_handler
* fix endianess in mma7455_read_raw function
* add mma7456 id
* split it into several source files to support both i2c and spi

I compared the register summary for MMA7455L/MMA7456L and I am
unable to find any difference at all.

 drivers/iio/accel/Kconfig        |  22 +++
 drivers/iio/accel/Makefile       |   5 +
 drivers/iio/accel/mma7455.h      |  20 +++
 drivers/iio/accel/mma7455_core.c | 321 +++++++++++++++++++++++++++++++++++++++
 drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
 drivers/iio/accel/mma7455_spi.c  |  53 +++++++
 6 files changed, 478 insertions(+)
 create mode 100644 drivers/iio/accel/mma7455.h
 create mode 100644 drivers/iio/accel/mma7455_core.c
 create mode 100644 drivers/iio/accel/mma7455_i2c.c
 create mode 100644 drivers/iio/accel/mma7455_spi.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index a59047d7657e..3c87a4ae317a 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -99,6 +99,28 @@ config KXCJK1013
 	  To compile this driver as a module, choose M here: the module will
 	  be called kxcjk-1013.
 
+config MMA7455
+	tristate "Freescale MMA7455L/MMA7456L Accelerometer Driver"
+	depends on I2C || SPI_MASTER
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	select MMA7455_I2C if I2C
+	select MMA7455_SPI if SPI
+	help
+	  Say yes here to build support for the Freescale MMA7455L and
+	  MMA7456L 3-axis accelerometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mma7455_i2c or mma7455_spi.
+
+config MMA7455_I2C
+	tristate
+	select REGMAP_I2C
+
+config MMA7455_SPI
+	tristate
+	select REGMAP_SPI
+
 config MMA8452
 	tristate "Freescale MMA8452Q Accelerometer Driver"
 	depends on I2C
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index ebd2675b2a02..dfb9289393fb 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -8,6 +8,11 @@ obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel.o
 obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
 obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
 obj-$(CONFIG_KXSD9)	+= kxsd9.o
+
+obj-$(CONFIG_MMA7455)		+= mma7455_core.o
+obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
+obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
+
 obj-$(CONFIG_MMA8452)	+= mma8452.o
 
 obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
new file mode 100644
index 000000000000..8fae9345da88
--- /dev/null
+++ b/drivers/iio/accel/mma7455.h
@@ -0,0 +1,20 @@
+/*
+ * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __MMA7455_H
+#define __MMA7455_H
+
+extern const struct regmap_config mma7455_core_regmap;
+
+int mma7455_core_probe(struct device *dev, struct regmap *regmap,
+		       const char *name);
+int mma7455_core_remove(struct device *dev);
+
+#endif
diff --git a/drivers/iio/accel/mma7455_core.c b/drivers/iio/accel/mma7455_core.c
new file mode 100644
index 000000000000..dbe5db9a8322
--- /dev/null
+++ b/drivers/iio/accel/mma7455_core.c
@@ -0,0 +1,321 @@
+/*
+ * IIO accel core driver for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * UNSUPPORTED hardware features:
+ *  - 8-bit mode with different scales
+ *  - INT1/INT2 interrupts
+ *  - Offset calibration
+ *  - Events
+ */
+
+#include <linux/delay.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "mma7455.h"
+
+#define MMA7455_REG_XOUTL		0x00
+#define MMA7455_REG_XOUTH		0x01
+#define MMA7455_REG_YOUTL		0x02
+#define MMA7455_REG_YOUTH		0x03
+#define MMA7455_REG_ZOUTL		0x04
+#define MMA7455_REG_ZOUTH		0x05
+#define MMA7455_REG_STATUS		0x09
+#define  MMA7455_STATUS_DRDY		BIT(0)
+#define MMA7455_REG_WHOAMI		0x0f
+#define  MMA7455_WHOAMI_ID		0x55
+#define MMA7455_REG_MCTL		0x16
+#define  MMA7455_MCTL_MODE_STANDBY	0x00
+#define  MMA7455_MCTL_MODE_MEASURE	0x01
+#define MMA7455_REG_CTL1		0x18
+#define  MMA7455_CTL1_DFBW_MASK		BIT(7)
+#define  MMA7455_CTL1_DFBW_125HZ	BIT(7)
+#define  MMA7455_CTL1_DFBW_62_5HZ	0
+#define MMA7455_REG_TW			0x1e
+
+/*
+ * When MMA7455 is used in 10-bit it has a fullscale of -8g
+ * corresponding to raw value -512. The userspace interface
+ * uses m/s^2 and we declare micro units.
+ * So scale factor is given by:
+ *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
+ */
+#define MMA7455_10BIT_SCALE	153229
+
+struct mma7455_data {
+	struct regmap *regmap;
+	struct device *dev;
+};
+
+static int mma7455_drdy(struct mma7455_data *mma7455)
+{
+	unsigned int reg;
+	int tries = 3;
+	int ret;
+
+	while (tries-- > 0) {
+		ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
+		if (ret)
+			return ret;
+
+		if (reg & MMA7455_STATUS_DRDY)
+			return 0;
+
+		msleep(20);
+	}
+
+	dev_warn(mma7455->dev, "data not ready\n");
+
+	return -EIO;
+}
+
+static irqreturn_t mma7455_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
+	int ret;
+
+	ret = mma7455_drdy(mma7455);
+	if (ret)
+		goto done;
+
+	ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
+			       sizeof(__le16) * 3);
+	if (ret)
+		goto done;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int mma7455_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	unsigned int reg;
+	__le16 data;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (iio_buffer_enabled(indio_dev))
+			return -EBUSY;
+
+		ret = mma7455_drdy(mma7455);
+		if (ret)
+			return ret;
+
+		ret = regmap_bulk_read(mma7455->regmap, chan->address, &data,
+				       sizeof(data));
+		if (ret)
+			return ret;
+
+		*val = sign_extend32(le16_to_cpu(data), 9);
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = MMA7455_10BIT_SCALE;
+
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
+		if (ret)
+			return ret;
+
+		if (reg & MMA7455_CTL1_DFBW_MASK)
+			*val = 250;
+		else
+			*val = 125;
+
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int mma7455_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	int i;
+
+	if (iio_buffer_enabled(indio_dev))
+		return -EBUSY;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (val == 250 && val2 == 0)
+			i = MMA7455_CTL1_DFBW_125HZ;
+		else if (val == 125 && val2 == 0)
+			i = MMA7455_CTL1_DFBW_62_5HZ;
+		else
+			return -EINVAL;
+
+		return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
+					  MMA7455_CTL1_DFBW_MASK, i);
+
+	case IIO_CHAN_INFO_SCALE:
+		/* In 10-bit mode there is only one scale available */
+		if (val == 0 && val2 == MMA7455_10BIT_SCALE)
+			return 0;
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t mma7455_show_samp_freq_avail(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "125 250\n");
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma7455_show_samp_freq_avail);
+
+static struct attribute *mma7455_attributes[] = {
+	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group mma7455_group = {
+	.attrs = mma7455_attributes,
+};
+
+static const struct iio_info mma7455_info = {
+	.attrs = &mma7455_group,
+	.read_raw = mma7455_read_raw,
+	.write_raw = mma7455_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+#define MMA7455_CHANNEL(axis, idx) { \
+	.type = IIO_ACCEL, \
+	.modified = 1, \
+	.address = MMA7455_REG_##axis##OUTL,\
+	.channel2 = IIO_MOD_##axis, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+				    BIT(IIO_CHAN_INFO_SCALE), \
+	.scan_index = idx, \
+	.scan_type = { \
+		.sign = 's', \
+		.realbits = 10, \
+		.storagebits = 16, \
+		.endianness = IIO_LE, \
+	}, \
+}
+
+static const struct iio_chan_spec mma7455_channels[] = {
+	MMA7455_CHANNEL(X, 0),
+	MMA7455_CHANNEL(Y, 1),
+	MMA7455_CHANNEL(Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const unsigned long mma7455_scan_masks[] = {0x7, 0};
+
+const struct regmap_config mma7455_core_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MMA7455_REG_TW,
+};
+EXPORT_SYMBOL_GPL(mma7455_core_regmap);
+
+int mma7455_core_probe(struct device *dev, struct regmap *regmap,
+		       const char *name)
+{
+	struct mma7455_data *mma7455;
+	struct iio_dev *indio_dev;
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(regmap, MMA7455_REG_WHOAMI, &reg);
+	if (ret) {
+		dev_err(dev, "unable to read reg\n");
+		return ret;
+	}
+
+	if (reg != MMA7455_WHOAMI_ID) {
+		dev_err(dev, "device id mismatch\n");
+		return -ENODEV;
+	}
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*mma7455));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, indio_dev);
+	mma7455 = iio_priv(indio_dev);
+	mma7455->regmap = regmap;
+	mma7455->dev = dev;
+
+	indio_dev->info = &mma7455_info;
+	indio_dev->name = name;
+	indio_dev->dev.parent = dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = mma7455_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mma7455_channels);
+	indio_dev->available_scan_masks = mma7455_scan_masks;
+
+	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
+		     MMA7455_MCTL_MODE_MEASURE);
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					 mma7455_trigger_handler, NULL);
+	if (ret) {
+		dev_err(dev, "unable to setup triggered buffer\n");
+		return ret;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "unable to register device\n");
+		iio_triggered_buffer_cleanup(indio_dev);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mma7455_core_probe);
+
+int mma7455_core_remove(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
+		     MMA7455_MCTL_MODE_STANDBY);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mma7455_core_remove);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L core accelerometer driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/mma7455_i2c.c b/drivers/iio/accel/mma7455_i2c.c
new file mode 100644
index 000000000000..2363d96718ef
--- /dev/null
+++ b/drivers/iio/accel/mma7455_i2c.c
@@ -0,0 +1,57 @@
+/*
+ * IIO accel I2C driver for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "mma7455.h"
+
+static int mma7455_i2c_probe(struct i2c_client *i2c,
+			     const struct i2c_device_id *id)
+{
+	struct regmap *regmap;
+	const char *name = NULL;
+
+	regmap = devm_regmap_init_i2c(i2c, &mma7455_core_regmap);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	if (id)
+		name = id->name;
+
+	return mma7455_core_probe(&i2c->dev, regmap, name);
+}
+
+static int mma7455_i2c_remove(struct i2c_client *i2c)
+{
+	return mma7455_core_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id mma7455_i2c_ids[] = {
+	{ "mma7455", 0 },
+	{ "mma7456", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mma7455_i2c_id);
+
+static struct i2c_driver mma7455_i2c_driver = {
+	.probe = mma7455_i2c_probe,
+	.remove = mma7455_i2c_remove,
+	.id_table = mma7455_i2c_ids,
+	.driver = {
+		.name	= "mma7455-i2c",
+	},
+};
+module_i2c_driver(mma7455_i2c_driver);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L I2C accelerometer driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/mma7455_spi.c b/drivers/iio/accel/mma7455_spi.c
new file mode 100644
index 000000000000..a84d6707cf6e
--- /dev/null
+++ b/drivers/iio/accel/mma7455_spi.c
@@ -0,0 +1,53 @@
+/*
+ * IIO accel SPI driver for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "mma7455.h"
+
+static int mma7455_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_spi(spi, &mma7455_core_regmap);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return mma7455_core_probe(&spi->dev, regmap, id->name);
+}
+
+static int mma7455_spi_remove(struct spi_device *spi)
+{
+	return mma7455_core_remove(&spi->dev);
+}
+
+static const struct spi_device_id mma7455_spi_ids[] = {
+	{ "mma7455", 0 },
+	{ "mma7456", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, mma7455_spi_id);
+
+static struct spi_driver mma7455_spi_driver = {
+	.probe = mma7455_spi_probe,
+	.remove = mma7455_spi_remove,
+	.id_table = mma7455_spi_ids,
+	.driver = {
+		.name = "mma7455-spi",
+	},
+};
+module_spi_driver(mma7455_spi_driver);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L SPI accelerometer driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-19 19:00 ` [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L " Joachim Eastwood
@ 2015-10-19 21:07   ` Joachim Eastwood
  2015-10-20  7:48   ` Martin Kepplinger
  2015-10-20  8:05   ` Lars-Peter Clausen
  2 siblings, 0 replies; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-19 21:07 UTC (permalink / raw)
  To: jic23, knaack.h, lars, Peter Meerwald
  Cc: Joachim Eastwood, martink, linux-iio

On 19 October 2015 at 21:00, Joachim Eastwood <manabian@gmail.com> wrote:
> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
> I2C and SPI bus. This rather simple driver that currently doesn't
> support all the hardware features of MMA7455L/MMA7456L.
>
> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>
> Data sheets for the two devices can be found here:
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---
> Hi,
>
> This version address the comments from Lars-Peter Clausen, Peter
> Meerwald and Martin Kepplinger. Thanks for all the constructive
> feedback!
>
> Changes since v1:
> * limit retries to 3 in mma7455_drdy
> * remove mma7455_show_scale_avail
> * use chan->address instead of chan->scan_index for reg addr
> * check that val2 is 0 when setting sample freq
> * use __le16 to hint about endianess in mma7455_trigger_handler
> * fix endianess in mma7455_read_raw function
> * add mma7456 id
> * split it into several source files to support both i2c and spi
>
> I compared the register summary for MMA7455L/MMA7456L and I am
> unable to find any difference at all.
>
>  drivers/iio/accel/Kconfig        |  22 +++
>  drivers/iio/accel/Makefile       |   5 +
>  drivers/iio/accel/mma7455.h      |  20 +++
>  drivers/iio/accel/mma7455_core.c | 321 +++++++++++++++++++++++++++++++++++++++
>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>  6 files changed, 478 insertions(+)
>  create mode 100644 drivers/iio/accel/mma7455.h
>  create mode 100644 drivers/iio/accel/mma7455_core.c
>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>  create mode 100644 drivers/iio/accel/mma7455_spi.c

> +static const struct i2c_device_id mma7455_i2c_ids[] = {
> +       { "mma7455", 0 },
> +       { "mma7456", 0 },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(i2c, mma7455_i2c_id);

> +static const struct spi_device_id mma7455_spi_ids[] = {
> +       { "mma7455", 0 },
> +       { "mma7456", 0 },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(spi, mma7455_spi_id);

Seems like I forgot to do a modular and the kbuild bot caught me.
Renamed to *_ids, but forgot to update variable in MODULE_DEVICE_TABLE macro...

I'll send out a new version after people have had a chance to comment
on any other issues.


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-19 19:00 ` [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L " Joachim Eastwood
  2015-10-19 21:07   ` Joachim Eastwood
@ 2015-10-20  7:48   ` Martin Kepplinger
  2015-10-20 11:03     ` Joachim Eastwood
  2015-10-20  8:05   ` Lars-Peter Clausen
  2 siblings, 1 reply; 36+ messages in thread
From: Martin Kepplinger @ 2015-10-20  7:48 UTC (permalink / raw)
  To: Joachim Eastwood, jic23, knaack.h, lars, pmeerw; +Cc: linux-iio

Am 2015-10-19 um 21:00 schrieb Joachim Eastwood:
> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
> I2C and SPI bus. This rather simple driver that currently doesn't
> support all the hardware features of MMA7455L/MMA7456L.
> 
> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
> 
> Data sheets for the two devices can be found here:
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---
> Hi,
> 
> This version address the comments from Lars-Peter Clausen, Peter
> Meerwald and Martin Kepplinger. Thanks for all the constructive
> feedback!
> 
> Changes since v1:
> * limit retries to 3 in mma7455_drdy
> * remove mma7455_show_scale_avail
> * use chan->address instead of chan->scan_index for reg addr
> * check that val2 is 0 when setting sample freq
> * use __le16 to hint about endianess in mma7455_trigger_handler
> * fix endianess in mma7455_read_raw function
> * add mma7456 id
> * split it into several source files to support both i2c and spi
> 
> I compared the register summary for MMA7455L/MMA7456L and I am
> unable to find any difference at all.
> 
>  drivers/iio/accel/Kconfig        |  22 +++
>  drivers/iio/accel/Makefile       |   5 +
>  drivers/iio/accel/mma7455.h      |  20 +++
>  drivers/iio/accel/mma7455_core.c | 321 +++++++++++++++++++++++++++++++++++++++
>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>  6 files changed, 478 insertions(+)
>  create mode 100644 drivers/iio/accel/mma7455.h
>  create mode 100644 drivers/iio/accel/mma7455_core.c
>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>  create mode 100644 drivers/iio/accel/mma7455_spi.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index a59047d7657e..3c87a4ae317a 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -99,6 +99,28 @@ config KXCJK1013
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called kxcjk-1013.
>  
> +config MMA7455
> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer Driver"
> +	depends on I2C || SPI_MASTER
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	select MMA7455_I2C if I2C
> +	select MMA7455_SPI if SPI
> +	help
> +	  Say yes here to build support for the Freescale MMA7455L and
> +	  MMA7456L 3-axis accelerometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mma7455_i2c or mma7455_spi.
> +
> +config MMA7455_I2C
> +	tristate
> +	select REGMAP_I2C
> +
> +config MMA7455_SPI
> +	tristate
> +	select REGMAP_SPI
> +
>  config MMA8452
>  	tristate "Freescale MMA8452Q Accelerometer Driver"
>  	depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index ebd2675b2a02..dfb9289393fb 100644
> --- a/drivers/iio/accel/Makefile

You don't base your work on top of the current -next tree. But that's
where this most likely will go, if integrated. Please make it apply to
linux-next.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-19 19:00 ` [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L " Joachim Eastwood
  2015-10-19 21:07   ` Joachim Eastwood
  2015-10-20  7:48   ` Martin Kepplinger
@ 2015-10-20  8:05   ` Lars-Peter Clausen
  2015-10-20 11:00     ` Joachim Eastwood
  2 siblings, 1 reply; 36+ messages in thread
From: Lars-Peter Clausen @ 2015-10-20  8:05 UTC (permalink / raw)
  To: Joachim Eastwood, jic23, knaack.h, pmeerw; +Cc: martink, linux-iio

On 10/19/2015 09:00 PM, Joachim Eastwood wrote:
[...]
> +config MMA7455
> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer Driver"
> +	depends on I2C || SPI_MASTER
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	select MMA7455_I2C if I2C
> +	select MMA7455_SPI if SPI

Ah, damm, I shouldn't have said bmg160 is a good example without actually
looking at its Kconfig entry. Only saw the C files and those looked good.

This is still problematic. The depends on clause allows this driver to be
built-in if either I2C is built-in or SPI_MASTER is built-in. And then the
respective I2C and SPI modules will also be selected as built-in even though
the I2C symbol is only selected as a module. So you now have built-in code
that tries to reference code that is only available as a module.

A better solution is to make the Kconfig entries for the I2C and SPI modules
user selectable and then let them select the main module.

> +	help
> +	  Say yes here to build support for the Freescale MMA7455L and
> +	  MMA7456L 3-axis accelerometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mma7455_i2c or mma7455_spi.
> +
> +config MMA7455_I2C
> +	tristate
> +	select REGMAP_I2C
> +
> +config MMA7455_SPI
> +	tristate
> +	select REGMAP_SPI
> +


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-20  8:05   ` Lars-Peter Clausen
@ 2015-10-20 11:00     ` Joachim Eastwood
  2015-10-20 11:05       ` Lars-Peter Clausen
  0 siblings, 1 reply; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-20 11:00 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: jic23, knaack.h, Peter Meerwald, Martin Kepplinger, linux-iio

On 20 October 2015 at 10:05, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 10/19/2015 09:00 PM, Joachim Eastwood wrote:
> [...]
>> +config MMA7455
>> +     tristate "Freescale MMA7455L/MMA7456L Accelerometer Driver"
>> +     depends on I2C || SPI_MASTER
>> +     select IIO_BUFFER
>> +     select IIO_TRIGGERED_BUFFER
>> +     select MMA7455_I2C if I2C
>> +     select MMA7455_SPI if SPI
>
> Ah, damm, I shouldn't have said bmg160 is a good example without actually
> looking at its Kconfig entry. Only saw the C files and those looked good.
>
> This is still problematic. The depends on clause allows this driver to be
> built-in if either I2C is built-in or SPI_MASTER is built-in. And then the
> respective I2C and SPI modules will also be selected as built-in even though
> the I2C symbol is only selected as a module. So you now have built-in code
> that tries to reference code that is only available as a module.
>
> A better solution is to make the Kconfig entries for the I2C and SPI modules
> user selectable and then let them select the main module.

Something like this:

config MMA7455
        tristate

config MMA7455_I2C
        tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
        depends on I2C
        select IIO_BUFFER
        select IIO_TRIGGERED_BUFFER
        select MMA7455
        select REGMAP_I2C
        help
            ...

config MMA7455_SPI
        tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
        depends on SPI_MASTER
        select IIO_BUFFER
        select IIO_TRIGGERED_BUFFER
        select MMA7455
        select REGMAP_SPI
        help
            ...

regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-20  7:48   ` Martin Kepplinger
@ 2015-10-20 11:03     ` Joachim Eastwood
  2015-10-25 11:29       ` Jonathan Cameron
  0 siblings, 1 reply; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-20 11:03 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: jic23, knaack.h, Lars-Peter Clausen, Peter Meerwald, linux-iio

On 20 October 2015 at 09:48, Martin Kepplinger <martink@posteo.de> wrote:
> Am 2015-10-19 um 21:00 schrieb Joachim Eastwood:
>> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
>> I2C and SPI bus. This rather simple driver that currently doesn't
>> support all the hardware features of MMA7455L/MMA7456L.
>>
>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>>
>> Data sheets for the two devices can be found here:
>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>
>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> ---
>> Hi,
>>
>> This version address the comments from Lars-Peter Clausen, Peter
>> Meerwald and Martin Kepplinger. Thanks for all the constructive
>> feedback!
>>
>> Changes since v1:
>> * limit retries to 3 in mma7455_drdy
>> * remove mma7455_show_scale_avail
>> * use chan->address instead of chan->scan_index for reg addr
>> * check that val2 is 0 when setting sample freq
>> * use __le16 to hint about endianess in mma7455_trigger_handler
>> * fix endianess in mma7455_read_raw function
>> * add mma7456 id
>> * split it into several source files to support both i2c and spi
>>
>> I compared the register summary for MMA7455L/MMA7456L and I am
>> unable to find any difference at all.
>>
>>  drivers/iio/accel/Kconfig        |  22 +++
>>  drivers/iio/accel/Makefile       |   5 +
>>  drivers/iio/accel/mma7455.h      |  20 +++
>>  drivers/iio/accel/mma7455_core.c | 321 +++++++++++++++++++++++++++++++++++++++
>>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>>  6 files changed, 478 insertions(+)
>>  create mode 100644 drivers/iio/accel/mma7455.h
>>  create mode 100644 drivers/iio/accel/mma7455_core.c
>>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>>  create mode 100644 drivers/iio/accel/mma7455_spi.c
>>
>>  config MMA8452
>>       tristate "Freescale MMA8452Q Accelerometer Driver"
>>       depends on I2C
>> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
>> index ebd2675b2a02..dfb9289393fb 100644
>> --- a/drivers/iio/accel/Makefile
>
> You don't base your work on top of the current -next tree. But that's
> where this most likely will go, if integrated. Please make it apply to
> linux-next.

Sure I can based it off linux-next or would one of the branches on
git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git be better?


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-20 11:00     ` Joachim Eastwood
@ 2015-10-20 11:05       ` Lars-Peter Clausen
  2015-10-20 11:52         ` Joachim Eastwood
  0 siblings, 1 reply; 36+ messages in thread
From: Lars-Peter Clausen @ 2015-10-20 11:05 UTC (permalink / raw)
  To: Joachim Eastwood
  Cc: jic23, knaack.h, Peter Meerwald, Martin Kepplinger, linux-iio

On 10/20/2015 01:00 PM, Joachim Eastwood wrote:
> On 20 October 2015 at 10:05, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 10/19/2015 09:00 PM, Joachim Eastwood wrote:
>> [...]
>>> +config MMA7455
>>> +     tristate "Freescale MMA7455L/MMA7456L Accelerometer Driver"
>>> +     depends on I2C || SPI_MASTER
>>> +     select IIO_BUFFER
>>> +     select IIO_TRIGGERED_BUFFER
>>> +     select MMA7455_I2C if I2C
>>> +     select MMA7455_SPI if SPI
>>
>> Ah, damm, I shouldn't have said bmg160 is a good example without actually
>> looking at its Kconfig entry. Only saw the C files and those looked good.
>>
>> This is still problematic. The depends on clause allows this driver to be
>> built-in if either I2C is built-in or SPI_MASTER is built-in. And then the
>> respective I2C and SPI modules will also be selected as built-in even though
>> the I2C symbol is only selected as a module. So you now have built-in code
>> that tries to reference code that is only available as a module.
>>
>> A better solution is to make the Kconfig entries for the I2C and SPI modules
>> user selectable and then let them select the main module.
> 
> Something like this:
> 
> config MMA7455
>         tristate
> 
> config MMA7455_I2C
>         tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
>         depends on I2C
>         select IIO_BUFFER
>         select IIO_TRIGGERED_BUFFER
>         select MMA7455
>         select REGMAP_I2C
>         help
>             ...
> 
> config MMA7455_SPI
>         tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
>         depends on SPI_MASTER
>         select IIO_BUFFER
>         select IIO_TRIGGERED_BUFFER
>         select MMA7455
>         select REGMAP_SPI
>         help
>             ...

Yes, but move the select IIO_BUFFER and IIO_TRIGGERED_BUFFER to the shared
symbol.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-20 11:05       ` Lars-Peter Clausen
@ 2015-10-20 11:52         ` Joachim Eastwood
  0 siblings, 0 replies; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-20 11:52 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: jic23, knaack.h, Peter Meerwald, Martin Kepplinger, linux-iio

On 20 October 2015 at 13:05, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 10/20/2015 01:00 PM, Joachim Eastwood wrote:
>> On 20 October 2015 at 10:05, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>> On 10/19/2015 09:00 PM, Joachim Eastwood wrote:
>>> [...]
>>>> +config MMA7455
>>>> +     tristate "Freescale MMA7455L/MMA7456L Accelerometer Driver"
>>>> +     depends on I2C || SPI_MASTER
>>>> +     select IIO_BUFFER
>>>> +     select IIO_TRIGGERED_BUFFER
>>>> +     select MMA7455_I2C if I2C
>>>> +     select MMA7455_SPI if SPI
>>>
>>> Ah, damm, I shouldn't have said bmg160 is a good example without actually
>>> looking at its Kconfig entry. Only saw the C files and those looked good.
>>>
>>> This is still problematic. The depends on clause allows this driver to be
>>> built-in if either I2C is built-in or SPI_MASTER is built-in. And then the
>>> respective I2C and SPI modules will also be selected as built-in even though
>>> the I2C symbol is only selected as a module. So you now have built-in code
>>> that tries to reference code that is only available as a module.
>>>
>>> A better solution is to make the Kconfig entries for the I2C and SPI modules
>>> user selectable and then let them select the main module.
>>
>> Something like this:
>>
>> config MMA7455
>>         tristate
>>
>> config MMA7455_I2C
>>         tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
>>         depends on I2C
>>         select IIO_BUFFER
>>         select IIO_TRIGGERED_BUFFER
>>         select MMA7455
>>         select REGMAP_I2C
>>         help
>>             ...
>>
>> config MMA7455_SPI
>>         tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
>>         depends on SPI_MASTER
>>         select IIO_BUFFER
>>         select IIO_TRIGGERED_BUFFER
>>         select MMA7455
>>         select REGMAP_SPI
>>         help
>>             ...
>
> Yes, but move the select IIO_BUFFER and IIO_TRIGGERED_BUFFER to the shared
> symbol.

ah, yes, that is better. Thanks.


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v3] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-17 22:25 [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver Joachim Eastwood
                   ` (3 preceding siblings ...)
  2015-10-19 19:00 ` [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L " Joachim Eastwood
@ 2015-10-20 20:50 ` Joachim Eastwood
  2015-10-25 11:45   ` Jonathan Cameron
  2015-10-31 12:49 ` [PATCH v4] " Joachim Eastwood
  5 siblings, 1 reply; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-20 20:50 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw; +Cc: Joachim Eastwood, martink, linux-iio

Add support for Freescale MMA7455L/MMA7456L 3-axis accelerometer
in 10-bit mode for I2C and SPI bus. This rather simple driver that
currently doesn't support all the hardware features of MMA7455L/
MMA7456L.

Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.

Data sheets for the two devices can be found here:
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
Hi,

This version address the comments from Lars-Peter Clausen and
Martin Kepplinger. Plus a build fix for modular and a rebase.

Changes since v2:
* fix id variable name in MODULE_DEVICE_TABLE
* make MMA7455_{I2C,SPI} symbols selectable
* rebase on linux-next (tag next-20151020)

Changes since v1:
* limit retries to 3 in mma7455_drdy
* remove mma7455_show_scale_avail
* use chan->address instead of chan->scan_index for reg addr
* check that val2 is 0 when setting sample freq
* use __le16 to hint about endianess in mma7455_trigger_handler
* fix endianess in mma7455_read_raw function
* add mma7456 id
* split it into several source files to support both i2c and spi


 drivers/iio/accel/Kconfig        |  29 ++++
 drivers/iio/accel/Makefile       |   5 +
 drivers/iio/accel/mma7455.h      |  20 +++
 drivers/iio/accel/mma7455_core.c | 321 +++++++++++++++++++++++++++++++++++++++
 drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
 drivers/iio/accel/mma7455_spi.c  |  53 +++++++
 6 files changed, 485 insertions(+)
 create mode 100644 drivers/iio/accel/mma7455.h
 create mode 100644 drivers/iio/accel/mma7455_core.c
 create mode 100644 drivers/iio/accel/mma7455_i2c.c
 create mode 100644 drivers/iio/accel/mma7455_spi.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 969428dd6329..728a7761aaa6 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -107,6 +107,35 @@ config KXCJK1013
 	  To compile this driver as a module, choose M here: the module will
 	  be called kxcjk-1013.
 
+config MMA7455
+	tristate
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+
+config MMA7455_I2C
+	tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
+	depends on I2C
+	select MMA7455
+	select REGMAP_I2C
+	help
+	  Say yes here to build support for the Freescale MMA7455L and
+	  MMA7456L 3-axis accelerometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mma7455_i2c.
+
+config MMA7455_SPI
+	tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
+	depends on SPI_MASTER
+	select MMA7455
+	select REGMAP_SPI
+	help
+	  Say yes here to build support for the Freescale MMA7455L and
+	  MMA7456L 3-axis accelerometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mma7455_spi.
+
 config MMA8452
 	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
 	depends on I2C
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 7925f166e6e9..7ea21f8b7d98 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -10,6 +10,11 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
 obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
 obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
 obj-$(CONFIG_KXSD9)	+= kxsd9.o
+
+obj-$(CONFIG_MMA7455)		+= mma7455_core.o
+obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
+obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
+
 obj-$(CONFIG_MMA8452)	+= mma8452.o
 
 obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
new file mode 100644
index 000000000000..8fae9345da88
--- /dev/null
+++ b/drivers/iio/accel/mma7455.h
@@ -0,0 +1,20 @@
+/*
+ * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __MMA7455_H
+#define __MMA7455_H
+
+extern const struct regmap_config mma7455_core_regmap;
+
+int mma7455_core_probe(struct device *dev, struct regmap *regmap,
+		       const char *name);
+int mma7455_core_remove(struct device *dev);
+
+#endif
diff --git a/drivers/iio/accel/mma7455_core.c b/drivers/iio/accel/mma7455_core.c
new file mode 100644
index 000000000000..dbe5db9a8322
--- /dev/null
+++ b/drivers/iio/accel/mma7455_core.c
@@ -0,0 +1,321 @@
+/*
+ * IIO accel core driver for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * UNSUPPORTED hardware features:
+ *  - 8-bit mode with different scales
+ *  - INT1/INT2 interrupts
+ *  - Offset calibration
+ *  - Events
+ */
+
+#include <linux/delay.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "mma7455.h"
+
+#define MMA7455_REG_XOUTL		0x00
+#define MMA7455_REG_XOUTH		0x01
+#define MMA7455_REG_YOUTL		0x02
+#define MMA7455_REG_YOUTH		0x03
+#define MMA7455_REG_ZOUTL		0x04
+#define MMA7455_REG_ZOUTH		0x05
+#define MMA7455_REG_STATUS		0x09
+#define  MMA7455_STATUS_DRDY		BIT(0)
+#define MMA7455_REG_WHOAMI		0x0f
+#define  MMA7455_WHOAMI_ID		0x55
+#define MMA7455_REG_MCTL		0x16
+#define  MMA7455_MCTL_MODE_STANDBY	0x00
+#define  MMA7455_MCTL_MODE_MEASURE	0x01
+#define MMA7455_REG_CTL1		0x18
+#define  MMA7455_CTL1_DFBW_MASK		BIT(7)
+#define  MMA7455_CTL1_DFBW_125HZ	BIT(7)
+#define  MMA7455_CTL1_DFBW_62_5HZ	0
+#define MMA7455_REG_TW			0x1e
+
+/*
+ * When MMA7455 is used in 10-bit it has a fullscale of -8g
+ * corresponding to raw value -512. The userspace interface
+ * uses m/s^2 and we declare micro units.
+ * So scale factor is given by:
+ *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
+ */
+#define MMA7455_10BIT_SCALE	153229
+
+struct mma7455_data {
+	struct regmap *regmap;
+	struct device *dev;
+};
+
+static int mma7455_drdy(struct mma7455_data *mma7455)
+{
+	unsigned int reg;
+	int tries = 3;
+	int ret;
+
+	while (tries-- > 0) {
+		ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
+		if (ret)
+			return ret;
+
+		if (reg & MMA7455_STATUS_DRDY)
+			return 0;
+
+		msleep(20);
+	}
+
+	dev_warn(mma7455->dev, "data not ready\n");
+
+	return -EIO;
+}
+
+static irqreturn_t mma7455_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
+	int ret;
+
+	ret = mma7455_drdy(mma7455);
+	if (ret)
+		goto done;
+
+	ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
+			       sizeof(__le16) * 3);
+	if (ret)
+		goto done;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int mma7455_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	unsigned int reg;
+	__le16 data;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (iio_buffer_enabled(indio_dev))
+			return -EBUSY;
+
+		ret = mma7455_drdy(mma7455);
+		if (ret)
+			return ret;
+
+		ret = regmap_bulk_read(mma7455->regmap, chan->address, &data,
+				       sizeof(data));
+		if (ret)
+			return ret;
+
+		*val = sign_extend32(le16_to_cpu(data), 9);
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = MMA7455_10BIT_SCALE;
+
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
+		if (ret)
+			return ret;
+
+		if (reg & MMA7455_CTL1_DFBW_MASK)
+			*val = 250;
+		else
+			*val = 125;
+
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int mma7455_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	int i;
+
+	if (iio_buffer_enabled(indio_dev))
+		return -EBUSY;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (val == 250 && val2 == 0)
+			i = MMA7455_CTL1_DFBW_125HZ;
+		else if (val == 125 && val2 == 0)
+			i = MMA7455_CTL1_DFBW_62_5HZ;
+		else
+			return -EINVAL;
+
+		return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
+					  MMA7455_CTL1_DFBW_MASK, i);
+
+	case IIO_CHAN_INFO_SCALE:
+		/* In 10-bit mode there is only one scale available */
+		if (val == 0 && val2 == MMA7455_10BIT_SCALE)
+			return 0;
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t mma7455_show_samp_freq_avail(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "125 250\n");
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma7455_show_samp_freq_avail);
+
+static struct attribute *mma7455_attributes[] = {
+	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group mma7455_group = {
+	.attrs = mma7455_attributes,
+};
+
+static const struct iio_info mma7455_info = {
+	.attrs = &mma7455_group,
+	.read_raw = mma7455_read_raw,
+	.write_raw = mma7455_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+#define MMA7455_CHANNEL(axis, idx) { \
+	.type = IIO_ACCEL, \
+	.modified = 1, \
+	.address = MMA7455_REG_##axis##OUTL,\
+	.channel2 = IIO_MOD_##axis, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+				    BIT(IIO_CHAN_INFO_SCALE), \
+	.scan_index = idx, \
+	.scan_type = { \
+		.sign = 's', \
+		.realbits = 10, \
+		.storagebits = 16, \
+		.endianness = IIO_LE, \
+	}, \
+}
+
+static const struct iio_chan_spec mma7455_channels[] = {
+	MMA7455_CHANNEL(X, 0),
+	MMA7455_CHANNEL(Y, 1),
+	MMA7455_CHANNEL(Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const unsigned long mma7455_scan_masks[] = {0x7, 0};
+
+const struct regmap_config mma7455_core_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MMA7455_REG_TW,
+};
+EXPORT_SYMBOL_GPL(mma7455_core_regmap);
+
+int mma7455_core_probe(struct device *dev, struct regmap *regmap,
+		       const char *name)
+{
+	struct mma7455_data *mma7455;
+	struct iio_dev *indio_dev;
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(regmap, MMA7455_REG_WHOAMI, &reg);
+	if (ret) {
+		dev_err(dev, "unable to read reg\n");
+		return ret;
+	}
+
+	if (reg != MMA7455_WHOAMI_ID) {
+		dev_err(dev, "device id mismatch\n");
+		return -ENODEV;
+	}
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*mma7455));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, indio_dev);
+	mma7455 = iio_priv(indio_dev);
+	mma7455->regmap = regmap;
+	mma7455->dev = dev;
+
+	indio_dev->info = &mma7455_info;
+	indio_dev->name = name;
+	indio_dev->dev.parent = dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = mma7455_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mma7455_channels);
+	indio_dev->available_scan_masks = mma7455_scan_masks;
+
+	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
+		     MMA7455_MCTL_MODE_MEASURE);
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					 mma7455_trigger_handler, NULL);
+	if (ret) {
+		dev_err(dev, "unable to setup triggered buffer\n");
+		return ret;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "unable to register device\n");
+		iio_triggered_buffer_cleanup(indio_dev);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mma7455_core_probe);
+
+int mma7455_core_remove(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
+		     MMA7455_MCTL_MODE_STANDBY);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mma7455_core_remove);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L core accelerometer driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/mma7455_i2c.c b/drivers/iio/accel/mma7455_i2c.c
new file mode 100644
index 000000000000..ce4ad0d45f98
--- /dev/null
+++ b/drivers/iio/accel/mma7455_i2c.c
@@ -0,0 +1,57 @@
+/*
+ * IIO accel I2C driver for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "mma7455.h"
+
+static int mma7455_i2c_probe(struct i2c_client *i2c,
+			     const struct i2c_device_id *id)
+{
+	struct regmap *regmap;
+	const char *name = NULL;
+
+	regmap = devm_regmap_init_i2c(i2c, &mma7455_core_regmap);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	if (id)
+		name = id->name;
+
+	return mma7455_core_probe(&i2c->dev, regmap, name);
+}
+
+static int mma7455_i2c_remove(struct i2c_client *i2c)
+{
+	return mma7455_core_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id mma7455_i2c_ids[] = {
+	{ "mma7455", 0 },
+	{ "mma7456", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mma7455_i2c_ids);
+
+static struct i2c_driver mma7455_i2c_driver = {
+	.probe = mma7455_i2c_probe,
+	.remove = mma7455_i2c_remove,
+	.id_table = mma7455_i2c_ids,
+	.driver = {
+		.name	= "mma7455-i2c",
+	},
+};
+module_i2c_driver(mma7455_i2c_driver);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L I2C accelerometer driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/mma7455_spi.c b/drivers/iio/accel/mma7455_spi.c
new file mode 100644
index 000000000000..5dc0bb75d289
--- /dev/null
+++ b/drivers/iio/accel/mma7455_spi.c
@@ -0,0 +1,53 @@
+/*
+ * IIO accel SPI driver for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "mma7455.h"
+
+static int mma7455_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_spi(spi, &mma7455_core_regmap);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return mma7455_core_probe(&spi->dev, regmap, id->name);
+}
+
+static int mma7455_spi_remove(struct spi_device *spi)
+{
+	return mma7455_core_remove(&spi->dev);
+}
+
+static const struct spi_device_id mma7455_spi_ids[] = {
+	{ "mma7455", 0 },
+	{ "mma7456", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, mma7455_spi_ids);
+
+static struct spi_driver mma7455_spi_driver = {
+	.probe = mma7455_spi_probe,
+	.remove = mma7455_spi_remove,
+	.id_table = mma7455_spi_ids,
+	.driver = {
+		.name = "mma7455-spi",
+	},
+};
+module_spi_driver(mma7455_spi_driver);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L SPI accelerometer driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-20 11:03     ` Joachim Eastwood
@ 2015-10-25 11:29       ` Jonathan Cameron
  0 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2015-10-25 11:29 UTC (permalink / raw)
  To: Joachim Eastwood, Martin Kepplinger
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald, linux-iio

On 20/10/15 12:03, Joachim Eastwood wrote:
> On 20 October 2015 at 09:48, Martin Kepplinger <martink@posteo.de> wrote:
>> Am 2015-10-19 um 21:00 schrieb Joachim Eastwood:
>>> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
>>> I2C and SPI bus. This rather simple driver that currently doesn't
>>> support all the hardware features of MMA7455L/MMA7456L.
>>>
>>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>>>
>>> Data sheets for the two devices can be found here:
>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>>
>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>>> ---
>>> Hi,
>>>
>>> This version address the comments from Lars-Peter Clausen, Peter
>>> Meerwald and Martin Kepplinger. Thanks for all the constructive
>>> feedback!
>>>
>>> Changes since v1:
>>> * limit retries to 3 in mma7455_drdy
>>> * remove mma7455_show_scale_avail
>>> * use chan->address instead of chan->scan_index for reg addr
>>> * check that val2 is 0 when setting sample freq
>>> * use __le16 to hint about endianess in mma7455_trigger_handler
>>> * fix endianess in mma7455_read_raw function
>>> * add mma7456 id
>>> * split it into several source files to support both i2c and spi
>>>
>>> I compared the register summary for MMA7455L/MMA7456L and I am
>>> unable to find any difference at all.
>>>
>>>  drivers/iio/accel/Kconfig        |  22 +++
>>>  drivers/iio/accel/Makefile       |   5 +
>>>  drivers/iio/accel/mma7455.h      |  20 +++
>>>  drivers/iio/accel/mma7455_core.c | 321 +++++++++++++++++++++++++++++++++++++++
>>>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>>>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>>>  6 files changed, 478 insertions(+)
>>>  create mode 100644 drivers/iio/accel/mma7455.h
>>>  create mode 100644 drivers/iio/accel/mma7455_core.c
>>>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>>>  create mode 100644 drivers/iio/accel/mma7455_spi.c
>>>
>>>  config MMA8452
>>>       tristate "Freescale MMA8452Q Accelerometer Driver"
>>>       depends on I2C
>>> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
>>> index ebd2675b2a02..dfb9289393fb 100644
>>> --- a/drivers/iio/accel/Makefile
>>
>> You don't base your work on top of the current -next tree. But that's
>> where this most likely will go, if integrated. Please make it apply to
>> linux-next.
> 
> Sure I can based it off linux-next or would one of the branches on
> git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git be better?
It doesn't matter that much unless lots of work is being done on a
given driver.  Either linux-next or the togreg branch of the above.

The testing branch usually contains the latest stuff but may rebase
as it's really there for automated builds.

Actually I don't really mind basing on last full kernel release either
or on staging-next which is our upstream.

At worst you get a tiny bit of merge fun.

Jonathan

> 
> 
> regards,
> Joachim Eastwood
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-20 20:50 ` [PATCH v3] " Joachim Eastwood
@ 2015-10-25 11:45   ` Jonathan Cameron
  2015-10-29 17:34     ` Joachim Eastwood
  0 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2015-10-25 11:45 UTC (permalink / raw)
  To: Joachim Eastwood, knaack.h, lars, pmeerw; +Cc: martink, linux-iio

On 20/10/15 21:50, Joachim Eastwood wrote:
> Add support for Freescale MMA7455L/MMA7456L 3-axis accelerometer
> in 10-bit mode for I2C and SPI bus. This rather simple driver that
> currently doesn't support all the hardware features of MMA7455L/
> MMA7456L.
> 
> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
> 
> Data sheets for the two devices can be found here:
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Looks pretty good to me.  A few comments inline.
If we hadn't unfortunately just missed the merge window for IIO
I'd have probably taken it as is, but now we have plenty of time
to tidy up a few corners without effecting when people get to play
with it in distros.

Jonathan
> ---
> Hi,
> 
> This version address the comments from Lars-Peter Clausen and
> Martin Kepplinger. Plus a build fix for modular and a rebase.
> 
> Changes since v2:
> * fix id variable name in MODULE_DEVICE_TABLE
> * make MMA7455_{I2C,SPI} symbols selectable
> * rebase on linux-next (tag next-20151020)
> 
> Changes since v1:
> * limit retries to 3 in mma7455_drdy
> * remove mma7455_show_scale_avail
> * use chan->address instead of chan->scan_index for reg addr
> * check that val2 is 0 when setting sample freq
> * use __le16 to hint about endianess in mma7455_trigger_handler
> * fix endianess in mma7455_read_raw function
> * add mma7456 id
> * split it into several source files to support both i2c and spi
> 
> 
>  drivers/iio/accel/Kconfig        |  29 ++++
>  drivers/iio/accel/Makefile       |   5 +
>  drivers/iio/accel/mma7455.h      |  20 +++
>  drivers/iio/accel/mma7455_core.c | 321 +++++++++++++++++++++++++++++++++++++++
>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>  6 files changed, 485 insertions(+)
>  create mode 100644 drivers/iio/accel/mma7455.h
>  create mode 100644 drivers/iio/accel/mma7455_core.c
>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>  create mode 100644 drivers/iio/accel/mma7455_spi.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 969428dd6329..728a7761aaa6 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -107,6 +107,35 @@ config KXCJK1013
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called kxcjk-1013.
>  
> +config MMA7455
> +	tristate
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +
> +config MMA7455_I2C
> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
> +	depends on I2C
> +	select MMA7455
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to build support for the Freescale MMA7455L and
> +	  MMA7456L 3-axis accelerometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mma7455_i2c.
> +
> +config MMA7455_SPI
> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
> +	depends on SPI_MASTER
> +	select MMA7455
> +	select REGMAP_SPI
> +	help
> +	  Say yes here to build support for the Freescale MMA7455L and
> +	  MMA7456L 3-axis accelerometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mma7455_spi.
> +
>  config MMA8452
>  	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
>  	depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 7925f166e6e9..7ea21f8b7d98 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -10,6 +10,11 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
> +
> +obj-$(CONFIG_MMA7455)		+= mma7455_core.o
> +obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
> +obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
> +
>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>  
>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
> diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
> new file mode 100644
> index 000000000000..8fae9345da88
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455.h
> @@ -0,0 +1,20 @@
> +/*
> + * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#ifndef __MMA7455_H
> +#define __MMA7455_H
> +
> +extern const struct regmap_config mma7455_core_regmap;
> +
> +int mma7455_core_probe(struct device *dev, struct regmap *regmap,
> +		       const char *name);
> +int mma7455_core_remove(struct device *dev);
> +
> +#endif
> diff --git a/drivers/iio/accel/mma7455_core.c b/drivers/iio/accel/mma7455_core.c
> new file mode 100644
> index 000000000000..dbe5db9a8322
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455_core.c
> @@ -0,0 +1,321 @@
> +/*
> + * IIO accel core driver for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * UNSUPPORTED hardware features:
> + *  - 8-bit mode with different scales
> + *  - INT1/INT2 interrupts
> + *  - Offset calibration
> + *  - Events
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include "mma7455.h"
> +
> +#define MMA7455_REG_XOUTL		0x00
> +#define MMA7455_REG_XOUTH		0x01
> +#define MMA7455_REG_YOUTL		0x02
> +#define MMA7455_REG_YOUTH		0x03
> +#define MMA7455_REG_ZOUTL		0x04
> +#define MMA7455_REG_ZOUTH		0x05
> +#define MMA7455_REG_STATUS		0x09
> +#define  MMA7455_STATUS_DRDY		BIT(0)
> +#define MMA7455_REG_WHOAMI		0x0f
> +#define  MMA7455_WHOAMI_ID		0x55
> +#define MMA7455_REG_MCTL		0x16
> +#define  MMA7455_MCTL_MODE_STANDBY	0x00
> +#define  MMA7455_MCTL_MODE_MEASURE	0x01
> +#define MMA7455_REG_CTL1		0x18
> +#define  MMA7455_CTL1_DFBW_MASK		BIT(7)
> +#define  MMA7455_CTL1_DFBW_125HZ	BIT(7)
> +#define  MMA7455_CTL1_DFBW_62_5HZ	0
> +#define MMA7455_REG_TW			0x1e
> +
> +/*
> + * When MMA7455 is used in 10-bit it has a fullscale of -8g
> + * corresponding to raw value -512. The userspace interface
> + * uses m/s^2 and we declare micro units.
> + * So scale factor is given by:
> + *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
> + */
> +#define MMA7455_10BIT_SCALE	153229
> +
> +struct mma7455_data {
> +	struct regmap *regmap;
> +	struct device *dev;
> +};
> +
> +static int mma7455_drdy(struct mma7455_data *mma7455)
> +{
> +	unsigned int reg;
> +	int tries = 3;
> +	int ret;
> +
> +	while (tries-- > 0) {
> +		ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (reg & MMA7455_STATUS_DRDY)
> +			return 0;
> +
> +		msleep(20);
> +	}
> +
> +	dev_warn(mma7455->dev, "data not ready\n");
> +
> +	return -EIO;
> +}
> +
> +static irqreturn_t mma7455_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
> +	int ret;
> +
> +	ret = mma7455_drdy(mma7455);
> +	if (ret)
> +		goto done;
> +
> +	ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
> +			       sizeof(__le16) * 3);
> +	if (ret)
> +		goto done;
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
> +
> +done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mma7455_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	unsigned int reg;
> +	__le16 data;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (iio_buffer_enabled(indio_dev))
> +			return -EBUSY;
> +
> +		ret = mma7455_drdy(mma7455);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_bulk_read(mma7455->regmap, chan->address, &data,
> +				       sizeof(data));
> +		if (ret)
> +			return ret;
> +
> +		*val = sign_extend32(le16_to_cpu(data), 9);
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = MMA7455_10BIT_SCALE;
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (reg & MMA7455_CTL1_DFBW_MASK)
> +			*val = 250;
> +		else
> +			*val = 125;
> +
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int mma7455_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	int i;
> +
> +	if (iio_buffer_enabled(indio_dev))
> +		return -EBUSY;

Whilst it is probably 'unwise' to prevent changes to samping frequency
and scale whilst the buffer is running, is there a hardware restriction
to prevent us doing so?  If so add a comment here.

> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		if (val == 250 && val2 == 0)
> +			i = MMA7455_CTL1_DFBW_125HZ;
> +		else if (val == 125 && val2 == 0)
> +			i = MMA7455_CTL1_DFBW_62_5HZ;
> +		else
> +			return -EINVAL;
> +
> +		return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
> +					  MMA7455_CTL1_DFBW_MASK, i);
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		/* In 10-bit mode there is only one scale available */
> +		if (val == 0 && val2 == MMA7455_10BIT_SCALE)
> +			return 0;
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static ssize_t mma7455_show_samp_freq_avail(struct device *dev,
> +					    struct device_attribute *attr,
> +					    char *buf)
> +{
> +	return scnprintf(buf, PAGE_SIZE, "125 250\n");
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma7455_show_samp_freq_avail);
There is IIO_CONST_ATTR for this sort of usage, but I guess you might
know this is going to get more complex in future?

> +
> +static struct attribute *mma7455_attributes[] = {
> +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group mma7455_group = {
> +	.attrs = mma7455_attributes,
> +};
> +
> +static const struct iio_info mma7455_info = {
> +	.attrs = &mma7455_group,
> +	.read_raw = mma7455_read_raw,
> +	.write_raw = mma7455_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +#define MMA7455_CHANNEL(axis, idx) { \
> +	.type = IIO_ACCEL, \
> +	.modified = 1, \
> +	.address = MMA7455_REG_##axis##OUTL,\
> +	.channel2 = IIO_MOD_##axis, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> +				    BIT(IIO_CHAN_INFO_SCALE), \
> +	.scan_index = idx, \
> +	.scan_type = { \
> +		.sign = 's', \
> +		.realbits = 10, \
> +		.storagebits = 16, \
> +		.endianness = IIO_LE, \
> +	}, \
> +}
> +
> +static const struct iio_chan_spec mma7455_channels[] = {
> +	MMA7455_CHANNEL(X, 0),
> +	MMA7455_CHANNEL(Y, 1),
> +	MMA7455_CHANNEL(Z, 2),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +static const unsigned long mma7455_scan_masks[] = {0x7, 0};
> +
> +const struct regmap_config mma7455_core_regmap = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MMA7455_REG_TW,
It does seem a little bit of a shame to not make use of more
regmap magic (e.g. caching) but I suppose this is just an initial
implementation of the driver and you can always add that stuff
later!
> +};
> +EXPORT_SYMBOL_GPL(mma7455_core_regmap);
> +
> +int mma7455_core_probe(struct device *dev, struct regmap *regmap,
> +		       const char *name)
> +{
> +	struct mma7455_data *mma7455;
> +	struct iio_dev *indio_dev;
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(regmap, MMA7455_REG_WHOAMI, &reg);
> +	if (ret) {
> +		dev_err(dev, "unable to read reg\n");
> +		return ret;
> +	}
> +
> +	if (reg != MMA7455_WHOAMI_ID) {
> +		dev_err(dev, "device id mismatch\n");
> +		return -ENODEV;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*mma7455));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, indio_dev);
> +	mma7455 = iio_priv(indio_dev);
> +	mma7455->regmap = regmap;
> +	mma7455->dev = dev;
> +
> +	indio_dev->info = &mma7455_info;
> +	indio_dev->name = name;
> +	indio_dev->dev.parent = dev;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mma7455_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(mma7455_channels);
> +	indio_dev->available_scan_masks = mma7455_scan_masks;
> +
> +	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
> +		     MMA7455_MCTL_MODE_MEASURE);
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					 mma7455_trigger_handler, NULL);
> +	if (ret) {
> +		dev_err(dev, "unable to setup triggered buffer\n");
> +		return ret;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(dev, "unable to register device\n");
> +		iio_triggered_buffer_cleanup(indio_dev);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mma7455_core_probe);
> +
> +int mma7455_core_remove(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +
> +	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
> +		     MMA7455_MCTL_MODE_STANDBY);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mma7455_core_remove);
> +
> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
> +MODULE_DESCRIPTION("Freescale MMA7455L core accelerometer driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/accel/mma7455_i2c.c b/drivers/iio/accel/mma7455_i2c.c
> new file mode 100644
> index 000000000000..ce4ad0d45f98
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455_i2c.c
> @@ -0,0 +1,57 @@
> +/*
> + * IIO accel I2C driver for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include "mma7455.h"
> +
> +static int mma7455_i2c_probe(struct i2c_client *i2c,
> +			     const struct i2c_device_id *id)
> +{
> +	struct regmap *regmap;
> +	const char *name = NULL;
> +
> +	regmap = devm_regmap_init_i2c(i2c, &mma7455_core_regmap);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	if (id)
> +		name = id->name;
> +
> +	return mma7455_core_probe(&i2c->dev, regmap, name);
> +}
> +
> +static int mma7455_i2c_remove(struct i2c_client *i2c)
> +{
> +	return mma7455_core_remove(&i2c->dev);
> +}
> +
> +static const struct i2c_device_id mma7455_i2c_ids[] = {
> +	{ "mma7455", 0 },
> +	{ "mma7456", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mma7455_i2c_ids);
> +
> +static struct i2c_driver mma7455_i2c_driver = {
> +	.probe = mma7455_i2c_probe,
> +	.remove = mma7455_i2c_remove,
> +	.id_table = mma7455_i2c_ids,
> +	.driver = {
> +		.name	= "mma7455-i2c",
> +	},
> +};
> +module_i2c_driver(mma7455_i2c_driver);
> +
> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
> +MODULE_DESCRIPTION("Freescale MMA7455L I2C accelerometer driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/accel/mma7455_spi.c b/drivers/iio/accel/mma7455_spi.c
> new file mode 100644
> index 000000000000..5dc0bb75d289
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455_spi.c
> @@ -0,0 +1,53 @@
> +/*
> + * IIO accel SPI driver for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +
> +#include "mma7455.h"
> +
> +static int mma7455_spi_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *id = spi_get_device_id(spi);
> +	struct regmap *regmap;
> +
> +	regmap = devm_regmap_init_spi(spi, &mma7455_core_regmap);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	return mma7455_core_probe(&spi->dev, regmap, id->name);
> +}
> +
> +static int mma7455_spi_remove(struct spi_device *spi)
> +{
> +	return mma7455_core_remove(&spi->dev);
> +}
> +
> +static const struct spi_device_id mma7455_spi_ids[] = {
> +	{ "mma7455", 0 },
> +	{ "mma7456", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, mma7455_spi_ids);
> +
> +static struct spi_driver mma7455_spi_driver = {
> +	.probe = mma7455_spi_probe,
> +	.remove = mma7455_spi_remove,
> +	.id_table = mma7455_spi_ids,
> +	.driver = {
> +		.name = "mma7455-spi",
> +	},
> +};
> +module_spi_driver(mma7455_spi_driver);
> +
> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
> +MODULE_DESCRIPTION("Freescale MMA7455L SPI accelerometer driver");
> +MODULE_LICENSE("GPL v2");
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-25 11:45   ` Jonathan Cameron
@ 2015-10-29 17:34     ` Joachim Eastwood
  2015-10-30 10:00       ` Jonathan Cameron
  0 siblings, 1 reply; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-29 17:34 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald, Martin Kepplinger,
	linux-iio

Hi Jonathan,

On 25 October 2015 at 12:45, Jonathan Cameron <jic23@kernel.org> wrote:
> On 20/10/15 21:50, Joachim Eastwood wrote:
>> Add support for Freescale MMA7455L/MMA7456L 3-axis accelerometer
>> in 10-bit mode for I2C and SPI bus. This rather simple driver that
>> currently doesn't support all the hardware features of MMA7455L/
>> MMA7456L.
>>
>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>>
>> Data sheets for the two devices can be found here:
>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>
>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> Looks pretty good to me.  A few comments inline.
> If we hadn't unfortunately just missed the merge window for IIO
> I'd have probably taken it as is, but now we have plenty of time
> to tidy up a few corners without effecting when people get to play
> with it in distros.

No, problem. I wasn't expecting it to go upstream during the upcoming window.


>> +static int mma7455_write_raw(struct iio_dev *indio_dev,
>> +                          struct iio_chan_spec const *chan,
>> +                          int val, int val2, long mask)
>> +{
>> +     struct mma7455_data *mma7455 = iio_priv(indio_dev);
>> +     int i;
>> +
>> +     if (iio_buffer_enabled(indio_dev))
>> +             return -EBUSY;
>
> Whilst it is probably 'unwise' to prevent changes to samping frequency
> and scale whilst the buffer is running, is there a hardware restriction
> to prevent us doing so?  If so add a comment here.

The data sheet doesn't really say one way or the other. So I think it
should work, but as you say it's probably not a very good idea to
change it during sampling. I'll change to code to allow it.


>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_SAMP_FREQ:
>> +             if (val == 250 && val2 == 0)
>> +                     i = MMA7455_CTL1_DFBW_125HZ;
>> +             else if (val == 125 && val2 == 0)
>> +                     i = MMA7455_CTL1_DFBW_62_5HZ;
>> +             else
>> +                     return -EINVAL;
>> +
>> +             return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
>> +                                       MMA7455_CTL1_DFBW_MASK, i);
>> +
>> +     case IIO_CHAN_INFO_SCALE:
>> +             /* In 10-bit mode there is only one scale available */
>> +             if (val == 0 && val2 == MMA7455_10BIT_SCALE)
>> +                     return 0;
>> +             break;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static ssize_t mma7455_show_samp_freq_avail(struct device *dev,
>> +                                         struct device_attribute *attr,
>> +                                         char *buf)
>> +{
>> +     return scnprintf(buf, PAGE_SIZE, "125 250\n");
>> +}
>> +
>> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma7455_show_samp_freq_avail);
> There is IIO_CONST_ATTR for this sort of usage, but I guess you might
> know this is going to get more complex in future?

ah, nice, didn't know about IIO_CONST_ATTR. The hardware only support
two sample rates so IIO_CONST_ATTR will do just fine.


>> +const struct regmap_config mma7455_core_regmap = {
>> +     .reg_bits = 8,
>> +     .val_bits = 8,
>> +     .max_register = MMA7455_REG_TW,
> It does seem a little bit of a shame to not make use of more
> regmap magic (e.g. caching) but I suppose this is just an initial
> implementation of the driver and you can always add that stuff
> later!

I agree. If more properties like scaling and calibration is added
using caching would be a good idea.


I'll make a new version and send it within a couple of days or do want
me to wait until the upcoming merge window closes?


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-29 17:34     ` Joachim Eastwood
@ 2015-10-30 10:00       ` Jonathan Cameron
  0 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2015-10-30 10:00 UTC (permalink / raw)
  To: Joachim Eastwood, Jonathan Cameron
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald, Martin Kepplinger,
	linux-iio



On 29 October 2015 17:34:13 GMT+00:00, Joachim  Eastwood <manabian@gmail.com> wrote:
>Hi Jonathan,
>
>On 25 October 2015 at 12:45, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 20/10/15 21:50, Joachim Eastwood wrote:
>>> Add support for Freescale MMA7455L/MMA7456L 3-axis accelerometer
>>> in 10-bit mode for I2C and SPI bus. This rather simple driver that
>>> currently doesn't support all the hardware features of MMA7455L/
>>> MMA7456L.
>>>
>>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C
>bus.
>>>
>>> Data sheets for the two devices can be found here:
>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>>
>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> Looks pretty good to me.  A few comments inline.
>> If we hadn't unfortunately just missed the merge window for IIO
>> I'd have probably taken it as is, but now we have plenty of time
>> to tidy up a few corners without effecting when people get to play
>> with it in distros.
>
>No, problem. I wasn't expecting it to go upstream during the upcoming
>window.
>
>
>>> +static int mma7455_write_raw(struct iio_dev *indio_dev,
>>> +                          struct iio_chan_spec const *chan,
>>> +                          int val, int val2, long mask)
>>> +{
>>> +     struct mma7455_data *mma7455 = iio_priv(indio_dev);
>>> +     int i;
>>> +
>>> +     if (iio_buffer_enabled(indio_dev))
>>> +             return -EBUSY;
>>
>> Whilst it is probably 'unwise' to prevent changes to samping
>frequency
>> and scale whilst the buffer is running, is there a hardware
>restriction
>> to prevent us doing so?  If so add a comment here.
>
>The data sheet doesn't really say one way or the other. So I think it
>should work, but as you say it's probably not a very good idea to
>change it during sampling. I'll change to code to allow it.
>
>
>>> +
>>> +     switch (mask) {
>>> +     case IIO_CHAN_INFO_SAMP_FREQ:
>>> +             if (val == 250 && val2 == 0)
>>> +                     i = MMA7455_CTL1_DFBW_125HZ;
>>> +             else if (val == 125 && val2 == 0)
>>> +                     i = MMA7455_CTL1_DFBW_62_5HZ;
>>> +             else
>>> +                     return -EINVAL;
>>> +
>>> +             return regmap_update_bits(mma7455->regmap,
>MMA7455_REG_CTL1,
>>> +                                       MMA7455_CTL1_DFBW_MASK, i);
>>> +
>>> +     case IIO_CHAN_INFO_SCALE:
>>> +             /* In 10-bit mode there is only one scale available */
>>> +             if (val == 0 && val2 == MMA7455_10BIT_SCALE)
>>> +                     return 0;
>>> +             break;
>>> +     }
>>> +
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +static ssize_t mma7455_show_samp_freq_avail(struct device *dev,
>>> +                                         struct device_attribute
>*attr,
>>> +                                         char *buf)
>>> +{
>>> +     return scnprintf(buf, PAGE_SIZE, "125 250\n");
>>> +}
>>> +
>>> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma7455_show_samp_freq_avail);
>> There is IIO_CONST_ATTR for this sort of usage, but I guess you might
>> know this is going to get more complex in future?
>
>ah, nice, didn't know about IIO_CONST_ATTR. The hardware only support
>two sample rates so IIO_CONST_ATTR will do just fine.
>
>
>>> +const struct regmap_config mma7455_core_regmap = {
>>> +     .reg_bits = 8,
>>> +     .val_bits = 8,
>>> +     .max_register = MMA7455_REG_TW,
>> It does seem a little bit of a shame to not make use of more
>> regmap magic (e.g. caching) but I suppose this is just an initial
>> implementation of the driver and you can always add that stuff
>> later!
>
>I agree. If more properties like scaling and calibration is added
>using caching would be a good idea.
>
>
>I'll make a new version and send it within a couple of days or do want
>me to wait until the upcoming merge window closes?
Whenever you are ready.
>
>
>regards,
>Joachim Eastwood

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v4] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-17 22:25 [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver Joachim Eastwood
                   ` (4 preceding siblings ...)
  2015-10-20 20:50 ` [PATCH v3] " Joachim Eastwood
@ 2015-10-31 12:49 ` Joachim Eastwood
  2015-10-31 21:37   ` Martin Kepplinger
  2015-11-01 18:01   ` Jonathan Cameron
  5 siblings, 2 replies; 36+ messages in thread
From: Joachim Eastwood @ 2015-10-31 12:49 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw; +Cc: Joachim Eastwood, martink, linux-iio

Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
I2C and SPI bus. This rather simple driver that currently doesn't
support all the hardware features of MMA7455L/MMA7456L.

Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.

Data sheets for the two devices can be found here:
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
Hi,

This version address the comments from Jonathan Cameron.

Changes since v3:
* use IIO_CONST_ATTR on sampling_frequency_available
* allow changing of rate and scaling while buffer is running

Changes since v2:
* fix id variable name in MODULE_DEVICE_TABLE
* make MMA7455_{I2C,SPI} symbols selectable
* rebase on linux-next (tag next-20151020)

Changes since v1:
* limit retries to 3 in mma7455_drdy
* remove mma7455_show_scale_avail
* use chan->address instead of chan->scan_index for reg addr
* check that val2 is 0 when setting sample freq
* use __le16 to hint about endianess in mma7455_trigger_handler
* fix endianess in mma7455_read_raw function
* add mma7456 id
* split it into several source files to support both i2c and spi


 drivers/iio/accel/Kconfig        |  29 ++++
 drivers/iio/accel/Makefile       |   5 +
 drivers/iio/accel/mma7455.h      |  20 +++
 drivers/iio/accel/mma7455_core.c | 311 +++++++++++++++++++++++++++++++++++++++
 drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
 drivers/iio/accel/mma7455_spi.c  |  53 +++++++
 6 files changed, 475 insertions(+)
 create mode 100644 drivers/iio/accel/mma7455.h
 create mode 100644 drivers/iio/accel/mma7455_core.c
 create mode 100644 drivers/iio/accel/mma7455_i2c.c
 create mode 100644 drivers/iio/accel/mma7455_spi.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 969428dd6329..728a7761aaa6 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -107,6 +107,35 @@ config KXCJK1013
 	  To compile this driver as a module, choose M here: the module will
 	  be called kxcjk-1013.
 
+config MMA7455
+	tristate
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+
+config MMA7455_I2C
+	tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
+	depends on I2C
+	select MMA7455
+	select REGMAP_I2C
+	help
+	  Say yes here to build support for the Freescale MMA7455L and
+	  MMA7456L 3-axis accelerometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mma7455_i2c.
+
+config MMA7455_SPI
+	tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
+	depends on SPI_MASTER
+	select MMA7455
+	select REGMAP_SPI
+	help
+	  Say yes here to build support for the Freescale MMA7455L and
+	  MMA7456L 3-axis accelerometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mma7455_spi.
+
 config MMA8452
 	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
 	depends on I2C
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 7925f166e6e9..7ea21f8b7d98 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -10,6 +10,11 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
 obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
 obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
 obj-$(CONFIG_KXSD9)	+= kxsd9.o
+
+obj-$(CONFIG_MMA7455)		+= mma7455_core.o
+obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
+obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
+
 obj-$(CONFIG_MMA8452)	+= mma8452.o
 
 obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
new file mode 100644
index 000000000000..8fae9345da88
--- /dev/null
+++ b/drivers/iio/accel/mma7455.h
@@ -0,0 +1,20 @@
+/*
+ * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __MMA7455_H
+#define __MMA7455_H
+
+extern const struct regmap_config mma7455_core_regmap;
+
+int mma7455_core_probe(struct device *dev, struct regmap *regmap,
+		       const char *name);
+int mma7455_core_remove(struct device *dev);
+
+#endif
diff --git a/drivers/iio/accel/mma7455_core.c b/drivers/iio/accel/mma7455_core.c
new file mode 100644
index 000000000000..c633cc2c0789
--- /dev/null
+++ b/drivers/iio/accel/mma7455_core.c
@@ -0,0 +1,311 @@
+/*
+ * IIO accel core driver for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * UNSUPPORTED hardware features:
+ *  - 8-bit mode with different scales
+ *  - INT1/INT2 interrupts
+ *  - Offset calibration
+ *  - Events
+ */
+
+#include <linux/delay.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "mma7455.h"
+
+#define MMA7455_REG_XOUTL		0x00
+#define MMA7455_REG_XOUTH		0x01
+#define MMA7455_REG_YOUTL		0x02
+#define MMA7455_REG_YOUTH		0x03
+#define MMA7455_REG_ZOUTL		0x04
+#define MMA7455_REG_ZOUTH		0x05
+#define MMA7455_REG_STATUS		0x09
+#define  MMA7455_STATUS_DRDY		BIT(0)
+#define MMA7455_REG_WHOAMI		0x0f
+#define  MMA7455_WHOAMI_ID		0x55
+#define MMA7455_REG_MCTL		0x16
+#define  MMA7455_MCTL_MODE_STANDBY	0x00
+#define  MMA7455_MCTL_MODE_MEASURE	0x01
+#define MMA7455_REG_CTL1		0x18
+#define  MMA7455_CTL1_DFBW_MASK		BIT(7)
+#define  MMA7455_CTL1_DFBW_125HZ	BIT(7)
+#define  MMA7455_CTL1_DFBW_62_5HZ	0
+#define MMA7455_REG_TW			0x1e
+
+/*
+ * When MMA7455 is used in 10-bit it has a fullscale of -8g
+ * corresponding to raw value -512. The userspace interface
+ * uses m/s^2 and we declare micro units.
+ * So scale factor is given by:
+ *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
+ */
+#define MMA7455_10BIT_SCALE	153229
+
+struct mma7455_data {
+	struct regmap *regmap;
+	struct device *dev;
+};
+
+static int mma7455_drdy(struct mma7455_data *mma7455)
+{
+	unsigned int reg;
+	int tries = 3;
+	int ret;
+
+	while (tries-- > 0) {
+		ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
+		if (ret)
+			return ret;
+
+		if (reg & MMA7455_STATUS_DRDY)
+			return 0;
+
+		msleep(20);
+	}
+
+	dev_warn(mma7455->dev, "data not ready\n");
+
+	return -EIO;
+}
+
+static irqreturn_t mma7455_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
+	int ret;
+
+	ret = mma7455_drdy(mma7455);
+	if (ret)
+		goto done;
+
+	ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
+			       sizeof(__le16) * 3);
+	if (ret)
+		goto done;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int mma7455_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	unsigned int reg;
+	__le16 data;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (iio_buffer_enabled(indio_dev))
+			return -EBUSY;
+
+		ret = mma7455_drdy(mma7455);
+		if (ret)
+			return ret;
+
+		ret = regmap_bulk_read(mma7455->regmap, chan->address, &data,
+				       sizeof(data));
+		if (ret)
+			return ret;
+
+		*val = sign_extend32(le16_to_cpu(data), 9);
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = MMA7455_10BIT_SCALE;
+
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
+		if (ret)
+			return ret;
+
+		if (reg & MMA7455_CTL1_DFBW_MASK)
+			*val = 250;
+		else
+			*val = 125;
+
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int mma7455_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+	int i;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (val == 250 && val2 == 0)
+			i = MMA7455_CTL1_DFBW_125HZ;
+		else if (val == 125 && val2 == 0)
+			i = MMA7455_CTL1_DFBW_62_5HZ;
+		else
+			return -EINVAL;
+
+		return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
+					  MMA7455_CTL1_DFBW_MASK, i);
+
+	case IIO_CHAN_INFO_SCALE:
+		/* In 10-bit mode there is only one scale available */
+		if (val == 0 && val2 == MMA7455_10BIT_SCALE)
+			return 0;
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static IIO_CONST_ATTR(sampling_frequency_available, "125 250");
+
+static struct attribute *mma7455_attributes[] = {
+	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group mma7455_group = {
+	.attrs = mma7455_attributes,
+};
+
+static const struct iio_info mma7455_info = {
+	.attrs = &mma7455_group,
+	.read_raw = mma7455_read_raw,
+	.write_raw = mma7455_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+#define MMA7455_CHANNEL(axis, idx) { \
+	.type = IIO_ACCEL, \
+	.modified = 1, \
+	.address = MMA7455_REG_##axis##OUTL,\
+	.channel2 = IIO_MOD_##axis, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+				    BIT(IIO_CHAN_INFO_SCALE), \
+	.scan_index = idx, \
+	.scan_type = { \
+		.sign = 's', \
+		.realbits = 10, \
+		.storagebits = 16, \
+		.endianness = IIO_LE, \
+	}, \
+}
+
+static const struct iio_chan_spec mma7455_channels[] = {
+	MMA7455_CHANNEL(X, 0),
+	MMA7455_CHANNEL(Y, 1),
+	MMA7455_CHANNEL(Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const unsigned long mma7455_scan_masks[] = {0x7, 0};
+
+const struct regmap_config mma7455_core_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MMA7455_REG_TW,
+};
+EXPORT_SYMBOL_GPL(mma7455_core_regmap);
+
+int mma7455_core_probe(struct device *dev, struct regmap *regmap,
+		       const char *name)
+{
+	struct mma7455_data *mma7455;
+	struct iio_dev *indio_dev;
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(regmap, MMA7455_REG_WHOAMI, &reg);
+	if (ret) {
+		dev_err(dev, "unable to read reg\n");
+		return ret;
+	}
+
+	if (reg != MMA7455_WHOAMI_ID) {
+		dev_err(dev, "device id mismatch\n");
+		return -ENODEV;
+	}
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*mma7455));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, indio_dev);
+	mma7455 = iio_priv(indio_dev);
+	mma7455->regmap = regmap;
+	mma7455->dev = dev;
+
+	indio_dev->info = &mma7455_info;
+	indio_dev->name = name;
+	indio_dev->dev.parent = dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = mma7455_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mma7455_channels);
+	indio_dev->available_scan_masks = mma7455_scan_masks;
+
+	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
+		     MMA7455_MCTL_MODE_MEASURE);
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					 mma7455_trigger_handler, NULL);
+	if (ret) {
+		dev_err(dev, "unable to setup triggered buffer\n");
+		return ret;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "unable to register device\n");
+		iio_triggered_buffer_cleanup(indio_dev);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mma7455_core_probe);
+
+int mma7455_core_remove(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct mma7455_data *mma7455 = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
+		     MMA7455_MCTL_MODE_STANDBY);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mma7455_core_remove);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L core accelerometer driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/mma7455_i2c.c b/drivers/iio/accel/mma7455_i2c.c
new file mode 100644
index 000000000000..ce4ad0d45f98
--- /dev/null
+++ b/drivers/iio/accel/mma7455_i2c.c
@@ -0,0 +1,57 @@
+/*
+ * IIO accel I2C driver for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "mma7455.h"
+
+static int mma7455_i2c_probe(struct i2c_client *i2c,
+			     const struct i2c_device_id *id)
+{
+	struct regmap *regmap;
+	const char *name = NULL;
+
+	regmap = devm_regmap_init_i2c(i2c, &mma7455_core_regmap);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	if (id)
+		name = id->name;
+
+	return mma7455_core_probe(&i2c->dev, regmap, name);
+}
+
+static int mma7455_i2c_remove(struct i2c_client *i2c)
+{
+	return mma7455_core_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id mma7455_i2c_ids[] = {
+	{ "mma7455", 0 },
+	{ "mma7456", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mma7455_i2c_ids);
+
+static struct i2c_driver mma7455_i2c_driver = {
+	.probe = mma7455_i2c_probe,
+	.remove = mma7455_i2c_remove,
+	.id_table = mma7455_i2c_ids,
+	.driver = {
+		.name	= "mma7455-i2c",
+	},
+};
+module_i2c_driver(mma7455_i2c_driver);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L I2C accelerometer driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/mma7455_spi.c b/drivers/iio/accel/mma7455_spi.c
new file mode 100644
index 000000000000..5dc0bb75d289
--- /dev/null
+++ b/drivers/iio/accel/mma7455_spi.c
@@ -0,0 +1,53 @@
+/*
+ * IIO accel SPI driver for Freescale MMA7455L 3-axis 10-bit accelerometer
+ * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "mma7455.h"
+
+static int mma7455_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_spi(spi, &mma7455_core_regmap);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return mma7455_core_probe(&spi->dev, regmap, id->name);
+}
+
+static int mma7455_spi_remove(struct spi_device *spi)
+{
+	return mma7455_core_remove(&spi->dev);
+}
+
+static const struct spi_device_id mma7455_spi_ids[] = {
+	{ "mma7455", 0 },
+	{ "mma7456", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, mma7455_spi_ids);
+
+static struct spi_driver mma7455_spi_driver = {
+	.probe = mma7455_spi_probe,
+	.remove = mma7455_spi_remove,
+	.id_table = mma7455_spi_ids,
+	.driver = {
+		.name = "mma7455-spi",
+	},
+};
+module_spi_driver(mma7455_spi_driver);
+
+MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
+MODULE_DESCRIPTION("Freescale MMA7455L SPI accelerometer driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v4] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-31 12:49 ` [PATCH v4] " Joachim Eastwood
@ 2015-10-31 21:37   ` Martin Kepplinger
  2015-11-01 18:01     ` Jonathan Cameron
  2015-11-01 18:01   ` Jonathan Cameron
  1 sibling, 1 reply; 36+ messages in thread
From: Martin Kepplinger @ 2015-10-31 21:37 UTC (permalink / raw)
  To: Joachim Eastwood, jic23, knaack.h, lars, pmeerw; +Cc: linux-iio

Am 2015-10-31 um 13:49 schrieb Joachim Eastwood:
> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
> I2C and SPI bus. This rather simple driver that currently doesn't
> support all the hardware features of MMA7455L/MMA7456L.
> 
> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
> 
> Data sheets for the two devices can be found here:
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---
> Hi,
> 
> This version address the comments from Jonathan Cameron.
> 
> Changes since v3:
> * use IIO_CONST_ATTR on sampling_frequency_available
> * allow changing of rate and scaling while buffer is running
> 
> Changes since v2:
> * fix id variable name in MODULE_DEVICE_TABLE
> * make MMA7455_{I2C,SPI} symbols selectable
> * rebase on linux-next (tag next-20151020)
> 
> Changes since v1:
> * limit retries to 3 in mma7455_drdy
> * remove mma7455_show_scale_avail
> * use chan->address instead of chan->scan_index for reg addr
> * check that val2 is 0 when setting sample freq
> * use __le16 to hint about endianess in mma7455_trigger_handler
> * fix endianess in mma7455_read_raw function
> * add mma7456 id
> * split it into several source files to support both i2c and spi
> 
> 
>  drivers/iio/accel/Kconfig        |  29 ++++
>  drivers/iio/accel/Makefile       |   5 +
>  drivers/iio/accel/mma7455.h      |  20 +++
>  drivers/iio/accel/mma7455_core.c | 311 +++++++++++++++++++++++++++++++++++++++
>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>  6 files changed, 475 insertions(+)
>  create mode 100644 drivers/iio/accel/mma7455.h
>  create mode 100644 drivers/iio/accel/mma7455_core.c
>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>  create mode 100644 drivers/iio/accel/mma7455_spi.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 969428dd6329..728a7761aaa6 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -107,6 +107,35 @@ config KXCJK1013
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called kxcjk-1013.
>  
> +config MMA7455
> +	tristate
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +
> +config MMA7455_I2C
> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
> +	depends on I2C
> +	select MMA7455
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to build support for the Freescale MMA7455L and
> +	  MMA7456L 3-axis accelerometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mma7455_i2c.
> +
> +config MMA7455_SPI
> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
> +	depends on SPI_MASTER
> +	select MMA7455
> +	select REGMAP_SPI
> +	help
> +	  Say yes here to build support for the Freescale MMA7455L and
> +	  MMA7456L 3-axis accelerometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mma7455_spi.
> +
>  config MMA8452
>  	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
>  	depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 7925f166e6e9..7ea21f8b7d98 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -10,6 +10,11 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
> +
> +obj-$(CONFIG_MMA7455)		+= mma7455_core.o
> +obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
> +obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
> +
>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>  
>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
> diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
> new file mode 100644
> index 000000000000..8fae9345da88
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455.h
> @@ -0,0 +1,20 @@
> +/*
> + * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *

Even though they are very similar (internal technological differences),
you could mention both supported chips here and in the other files. Btw,
I would also s/header/driver here.

That is, in case you get to do another version.

thanks
                           martin

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-31 21:37   ` Martin Kepplinger
@ 2015-11-01 18:01     ` Jonathan Cameron
  2015-11-02 11:07       ` Martin Kepplinger
  0 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2015-11-01 18:01 UTC (permalink / raw)
  To: Martin Kepplinger, Joachim Eastwood, knaack.h, lars, pmeerw; +Cc: linux-iio

On 31/10/15 21:37, Martin Kepplinger wrote:
> Am 2015-10-31 um 13:49 schrieb Joachim Eastwood:
>> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
>> I2C and SPI bus. This rather simple driver that currently doesn't
>> support all the hardware features of MMA7455L/MMA7456L.
>>
>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>>
>> Data sheets for the two devices can be found here:
>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>
>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> ---
>> Hi,
>>
>> This version address the comments from Jonathan Cameron.
>>
>> Changes since v3:
>> * use IIO_CONST_ATTR on sampling_frequency_available
>> * allow changing of rate and scaling while buffer is running
>>
>> Changes since v2:
>> * fix id variable name in MODULE_DEVICE_TABLE
>> * make MMA7455_{I2C,SPI} symbols selectable
>> * rebase on linux-next (tag next-20151020)
>>
>> Changes since v1:
>> * limit retries to 3 in mma7455_drdy
>> * remove mma7455_show_scale_avail
>> * use chan->address instead of chan->scan_index for reg addr
>> * check that val2 is 0 when setting sample freq
>> * use __le16 to hint about endianess in mma7455_trigger_handler
>> * fix endianess in mma7455_read_raw function
>> * add mma7456 id
>> * split it into several source files to support both i2c and spi
>>
>>
>>  drivers/iio/accel/Kconfig        |  29 ++++
>>  drivers/iio/accel/Makefile       |   5 +
>>  drivers/iio/accel/mma7455.h      |  20 +++
>>  drivers/iio/accel/mma7455_core.c | 311 +++++++++++++++++++++++++++++++++++++++
>>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>>  6 files changed, 475 insertions(+)
>>  create mode 100644 drivers/iio/accel/mma7455.h
>>  create mode 100644 drivers/iio/accel/mma7455_core.c
>>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>>  create mode 100644 drivers/iio/accel/mma7455_spi.c
>>
>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>> index 969428dd6329..728a7761aaa6 100644
>> --- a/drivers/iio/accel/Kconfig
>> +++ b/drivers/iio/accel/Kconfig
>> @@ -107,6 +107,35 @@ config KXCJK1013
>>  	  To compile this driver as a module, choose M here: the module will
>>  	  be called kxcjk-1013.
>>  
>> +config MMA7455
>> +	tristate
>> +	select IIO_BUFFER
>> +	select IIO_TRIGGERED_BUFFER
>> +
>> +config MMA7455_I2C
>> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
>> +	depends on I2C
>> +	select MMA7455
>> +	select REGMAP_I2C
>> +	help
>> +	  Say yes here to build support for the Freescale MMA7455L and
>> +	  MMA7456L 3-axis accelerometer.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called mma7455_i2c.
>> +
>> +config MMA7455_SPI
>> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
>> +	depends on SPI_MASTER
>> +	select MMA7455
>> +	select REGMAP_SPI
>> +	help
>> +	  Say yes here to build support for the Freescale MMA7455L and
>> +	  MMA7456L 3-axis accelerometer.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called mma7455_spi.
>> +
>>  config MMA8452
>>  	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
>>  	depends on I2C
>> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
>> index 7925f166e6e9..7ea21f8b7d98 100644
>> --- a/drivers/iio/accel/Makefile
>> +++ b/drivers/iio/accel/Makefile
>> @@ -10,6 +10,11 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
>>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
>> +
>> +obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>> +obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
>> +obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
>> +
>>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>>  
>>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
>> diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
>> new file mode 100644
>> index 000000000000..8fae9345da88
>> --- /dev/null
>> +++ b/drivers/iio/accel/mma7455.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
>> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
>> + *
> 
> Even though they are very similar (internal technological differences),
> you could mention both supported chips here and in the other files. Btw,
> I would also s/header/driver here.
> 
> That is, in case you get to do another version.
> 
Hi Martin,

I can see where you are coming from, but typically I'd actually advocate
mentioning the list of supported parts in as few a places as possible as
they tend to get out of sync if we end up with a driver supporting lots
of parts.  We have gotten round this in the past but saying something
like **** and similar or *** and compatible but that's always a bit clunky.

I'll switch header to driver in the application of the patch.

Jonathan


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-10-31 12:49 ` [PATCH v4] " Joachim Eastwood
  2015-10-31 21:37   ` Martin Kepplinger
@ 2015-11-01 18:01   ` Jonathan Cameron
  2015-11-01 18:02     ` Jonathan Cameron
  1 sibling, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2015-11-01 18:01 UTC (permalink / raw)
  To: Joachim Eastwood, knaack.h, lars, pmeerw; +Cc: martink, linux-iio

On 31/10/15 12:49, Joachim Eastwood wrote:
> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
> I2C and SPI bus. This rather simple driver that currently doesn't
> support all the hardware features of MMA7455L/MMA7456L.
> 
> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
> 
> Data sheets for the two devices can be found here:
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
A few really trivial gripes I'll deal with when applying.

+ the s/header/driver thing Martin picked up on.

Jonathan
> ---
> Hi,
> 
> This version address the comments from Jonathan Cameron.
> 
> Changes since v3:
> * use IIO_CONST_ATTR on sampling_frequency_available
> * allow changing of rate and scaling while buffer is running
> 
> Changes since v2:
> * fix id variable name in MODULE_DEVICE_TABLE
> * make MMA7455_{I2C,SPI} symbols selectable
> * rebase on linux-next (tag next-20151020)
> 
> Changes since v1:
> * limit retries to 3 in mma7455_drdy
> * remove mma7455_show_scale_avail
> * use chan->address instead of chan->scan_index for reg addr
> * check that val2 is 0 when setting sample freq
> * use __le16 to hint about endianess in mma7455_trigger_handler
> * fix endianess in mma7455_read_raw function
> * add mma7456 id
> * split it into several source files to support both i2c and spi
> 
> 
>  drivers/iio/accel/Kconfig        |  29 ++++
>  drivers/iio/accel/Makefile       |   5 +
>  drivers/iio/accel/mma7455.h      |  20 +++
>  drivers/iio/accel/mma7455_core.c | 311 +++++++++++++++++++++++++++++++++++++++
>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>  6 files changed, 475 insertions(+)
>  create mode 100644 drivers/iio/accel/mma7455.h
>  create mode 100644 drivers/iio/accel/mma7455_core.c
>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>  create mode 100644 drivers/iio/accel/mma7455_spi.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 969428dd6329..728a7761aaa6 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -107,6 +107,35 @@ config KXCJK1013
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called kxcjk-1013.
>  
> +config MMA7455
> +	tristate
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +
> +config MMA7455_I2C
> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
> +	depends on I2C
> +	select MMA7455
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to build support for the Freescale MMA7455L and
> +	  MMA7456L 3-axis accelerometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mma7455_i2c.
> +
> +config MMA7455_SPI
> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
> +	depends on SPI_MASTER
> +	select MMA7455
> +	select REGMAP_SPI
> +	help
> +	  Say yes here to build support for the Freescale MMA7455L and
> +	  MMA7456L 3-axis accelerometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mma7455_spi.
> +
>  config MMA8452
>  	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
>  	depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 7925f166e6e9..7ea21f8b7d98 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -10,6 +10,11 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
> +
> +obj-$(CONFIG_MMA7455)		+= mma7455_core.o
> +obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
> +obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
> +
>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>  
>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
> diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
> new file mode 100644
> index 000000000000..8fae9345da88
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455.h
> @@ -0,0 +1,20 @@
> +/*
> + * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
and another one...
> + *
> + */
> +
> +#ifndef __MMA7455_H
> +#define __MMA7455_H
> +
> +extern const struct regmap_config mma7455_core_regmap;
> +
> +int mma7455_core_probe(struct device *dev, struct regmap *regmap,
> +		       const char *name);
> +int mma7455_core_remove(struct device *dev);
> +
> +#endif
> diff --git a/drivers/iio/accel/mma7455_core.c b/drivers/iio/accel/mma7455_core.c
> new file mode 100644
> index 000000000000..c633cc2c0789
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455_core.c
> @@ -0,0 +1,311 @@
> +/*
> + * IIO accel core driver for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * UNSUPPORTED hardware features:
> + *  - 8-bit mode with different scales
> + *  - INT1/INT2 interrupts
> + *  - Offset calibration
> + *  - Events
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include "mma7455.h"
> +
> +#define MMA7455_REG_XOUTL		0x00
> +#define MMA7455_REG_XOUTH		0x01
> +#define MMA7455_REG_YOUTL		0x02
> +#define MMA7455_REG_YOUTH		0x03
> +#define MMA7455_REG_ZOUTL		0x04
> +#define MMA7455_REG_ZOUTH		0x05
> +#define MMA7455_REG_STATUS		0x09
> +#define  MMA7455_STATUS_DRDY		BIT(0)
> +#define MMA7455_REG_WHOAMI		0x0f
> +#define  MMA7455_WHOAMI_ID		0x55
> +#define MMA7455_REG_MCTL		0x16
> +#define  MMA7455_MCTL_MODE_STANDBY	0x00
> +#define  MMA7455_MCTL_MODE_MEASURE	0x01
> +#define MMA7455_REG_CTL1		0x18
> +#define  MMA7455_CTL1_DFBW_MASK		BIT(7)
> +#define  MMA7455_CTL1_DFBW_125HZ	BIT(7)
> +#define  MMA7455_CTL1_DFBW_62_5HZ	0
> +#define MMA7455_REG_TW			0x1e
> +
> +/*
> + * When MMA7455 is used in 10-bit it has a fullscale of -8g
> + * corresponding to raw value -512. The userspace interface
> + * uses m/s^2 and we declare micro units.
> + * So scale factor is given by:
> + *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
> + */
> +#define MMA7455_10BIT_SCALE	153229
> +
> +struct mma7455_data {
> +	struct regmap *regmap;
> +	struct device *dev;
> +};
> +
> +static int mma7455_drdy(struct mma7455_data *mma7455)
> +{
> +	unsigned int reg;
> +	int tries = 3;
> +	int ret;
> +
> +	while (tries-- > 0) {
> +		ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (reg & MMA7455_STATUS_DRDY)
> +			return 0;
> +
> +		msleep(20);
> +	}
> +
> +	dev_warn(mma7455->dev, "data not ready\n");
> +
> +	return -EIO;
> +}
> +
> +static irqreturn_t mma7455_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
> +	int ret;
> +
> +	ret = mma7455_drdy(mma7455);
> +	if (ret)
> +		goto done;
> +
> +	ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
> +			       sizeof(__le16) * 3);
> +	if (ret)
> +		goto done;
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
> +
> +done:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mma7455_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	unsigned int reg;
> +	__le16 data;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (iio_buffer_enabled(indio_dev))
> +			return -EBUSY;
> +
> +		ret = mma7455_drdy(mma7455);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_bulk_read(mma7455->regmap, chan->address, &data,
> +				       sizeof(data));
> +		if (ret)
> +			return ret;
> +
> +		*val = sign_extend32(le16_to_cpu(data), 9);
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = MMA7455_10BIT_SCALE;
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (reg & MMA7455_CTL1_DFBW_MASK)
> +			*val = 250;
> +		else
> +			*val = 125;
> +
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int mma7455_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +	int i;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		if (val == 250 && val2 == 0)
> +			i = MMA7455_CTL1_DFBW_125HZ;
> +		else if (val == 125 && val2 == 0)
> +			i = MMA7455_CTL1_DFBW_62_5HZ;
> +		else
> +			return -EINVAL;
> +
> +		return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
> +					  MMA7455_CTL1_DFBW_MASK, i);
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		/* In 10-bit mode there is only one scale available */
> +		if (val == 0 && val2 == MMA7455_10BIT_SCALE)
> +			return 0;
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static IIO_CONST_ATTR(sampling_frequency_available, "125 250");
> +
> +static struct attribute *mma7455_attributes[] = {
> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group mma7455_group = {
> +	.attrs = mma7455_attributes,
> +};
> +
> +static const struct iio_info mma7455_info = {
> +	.attrs = &mma7455_group,
> +	.read_raw = mma7455_read_raw,
> +	.write_raw = mma7455_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +#define MMA7455_CHANNEL(axis, idx) { \
> +	.type = IIO_ACCEL, \
> +	.modified = 1, \
> +	.address = MMA7455_REG_##axis##OUTL,\
> +	.channel2 = IIO_MOD_##axis, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> +				    BIT(IIO_CHAN_INFO_SCALE), \
> +	.scan_index = idx, \
> +	.scan_type = { \
> +		.sign = 's', \
> +		.realbits = 10, \
> +		.storagebits = 16, \
> +		.endianness = IIO_LE, \
> +	}, \
> +}
> +
> +static const struct iio_chan_spec mma7455_channels[] = {
> +	MMA7455_CHANNEL(X, 0),
> +	MMA7455_CHANNEL(Y, 1),
> +	MMA7455_CHANNEL(Z, 2),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +static const unsigned long mma7455_scan_masks[] = {0x7, 0};
> +
> +const struct regmap_config mma7455_core_regmap = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MMA7455_REG_TW,
> +};
> +EXPORT_SYMBOL_GPL(mma7455_core_regmap);
> +
> +int mma7455_core_probe(struct device *dev, struct regmap *regmap,
> +		       const char *name)
> +{
> +	struct mma7455_data *mma7455;
> +	struct iio_dev *indio_dev;
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(regmap, MMA7455_REG_WHOAMI, &reg);
> +	if (ret) {
> +		dev_err(dev, "unable to read reg\n");
> +		return ret;
> +	}
> +
> +	if (reg != MMA7455_WHOAMI_ID) {
> +		dev_err(dev, "device id mismatch\n");
> +		return -ENODEV;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*mma7455));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, indio_dev);
> +	mma7455 = iio_priv(indio_dev);
> +	mma7455->regmap = regmap;
> +	mma7455->dev = dev;
> +
> +	indio_dev->info = &mma7455_info;
> +	indio_dev->name = name;
> +	indio_dev->dev.parent = dev;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mma7455_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(mma7455_channels);
> +	indio_dev->available_scan_masks = mma7455_scan_masks;
> +
> +	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
> +		     MMA7455_MCTL_MODE_MEASURE);
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					 mma7455_trigger_handler, NULL);
> +	if (ret) {
> +		dev_err(dev, "unable to setup triggered buffer\n");
> +		return ret;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(dev, "unable to register device\n");
> +		iio_triggered_buffer_cleanup(indio_dev);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mma7455_core_probe);
> +
> +int mma7455_core_remove(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +
> +	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
> +		     MMA7455_MCTL_MODE_STANDBY);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mma7455_core_remove);
> +
> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
> +MODULE_DESCRIPTION("Freescale MMA7455L core accelerometer driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/accel/mma7455_i2c.c b/drivers/iio/accel/mma7455_i2c.c
> new file mode 100644
> index 000000000000..ce4ad0d45f98
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455_i2c.c
> @@ -0,0 +1,57 @@
> +/*
> + * IIO accel I2C driver for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
And this one (see below, I review backwards as drivers usually make
more sense that way around).
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include "mma7455.h"
> +
> +static int mma7455_i2c_probe(struct i2c_client *i2c,
> +			     const struct i2c_device_id *id)
> +{
> +	struct regmap *regmap;
> +	const char *name = NULL;
> +
> +	regmap = devm_regmap_init_i2c(i2c, &mma7455_core_regmap);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	if (id)
> +		name = id->name;
> +
> +	return mma7455_core_probe(&i2c->dev, regmap, name);
> +}
> +
> +static int mma7455_i2c_remove(struct i2c_client *i2c)
> +{
> +	return mma7455_core_remove(&i2c->dev);
> +}
> +
> +static const struct i2c_device_id mma7455_i2c_ids[] = {
> +	{ "mma7455", 0 },
> +	{ "mma7456", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, mma7455_i2c_ids);
> +
> +static struct i2c_driver mma7455_i2c_driver = {
> +	.probe = mma7455_i2c_probe,
> +	.remove = mma7455_i2c_remove,
> +	.id_table = mma7455_i2c_ids,
> +	.driver = {
> +		.name	= "mma7455-i2c",
> +	},
> +};
> +module_i2c_driver(mma7455_i2c_driver);
> +
> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
> +MODULE_DESCRIPTION("Freescale MMA7455L I2C accelerometer driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/accel/mma7455_spi.c b/drivers/iio/accel/mma7455_spi.c
> new file mode 100644
> index 000000000000..5dc0bb75d289
> --- /dev/null
> +++ b/drivers/iio/accel/mma7455_spi.c
> @@ -0,0 +1,53 @@
> +/*
> + * IIO accel SPI driver for Freescale MMA7455L 3-axis 10-bit accelerometer
> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
My quest against blank lines with no purpose continues.  I'll edit this
one out in the appy.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +
> +#include "mma7455.h"
> +
> +static int mma7455_spi_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *id = spi_get_device_id(spi);
> +	struct regmap *regmap;
> +
> +	regmap = devm_regmap_init_spi(spi, &mma7455_core_regmap);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	return mma7455_core_probe(&spi->dev, regmap, id->name);
> +}
> +
> +static int mma7455_spi_remove(struct spi_device *spi)
> +{
> +	return mma7455_core_remove(&spi->dev);
> +}
> +
> +static const struct spi_device_id mma7455_spi_ids[] = {
> +	{ "mma7455", 0 },
> +	{ "mma7456", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, mma7455_spi_ids);
> +
> +static struct spi_driver mma7455_spi_driver = {
> +	.probe = mma7455_spi_probe,
> +	.remove = mma7455_spi_remove,
> +	.id_table = mma7455_spi_ids,
> +	.driver = {
> +		.name = "mma7455-spi",
> +	},
> +};
> +module_spi_driver(mma7455_spi_driver);
> +
> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
> +MODULE_DESCRIPTION("Freescale MMA7455L SPI accelerometer driver");
> +MODULE_LICENSE("GPL v2");
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-11-01 18:01   ` Jonathan Cameron
@ 2015-11-01 18:02     ` Jonathan Cameron
  2015-11-03 22:17       ` Joachim Eastwood
  0 siblings, 1 reply; 36+ messages in thread
From: Jonathan Cameron @ 2015-11-01 18:02 UTC (permalink / raw)
  To: Joachim Eastwood, knaack.h, lars, pmeerw; +Cc: martink, linux-iio

On 01/11/15 18:01, Jonathan Cameron wrote:
> On 31/10/15 12:49, Joachim Eastwood wrote:
>> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
>> I2C and SPI bus. This rather simple driver that currently doesn't
>> support all the hardware features of MMA7455L/MMA7456L.
>>
>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>>
>> Data sheets for the two devices can be found here:
>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>
>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> A few really trivial gripes I'll deal with when applying.
> 
> + the s/header/driver thing Martin picked up on.
Forgot the important bit:

Applied to the togreg branch of iio.git - initially pushed out as
testing for the autobuilders to play with it.

Jonathan
> 
> Jonathan
>> ---
>> Hi,
>>
>> This version address the comments from Jonathan Cameron.
>>
>> Changes since v3:
>> * use IIO_CONST_ATTR on sampling_frequency_available
>> * allow changing of rate and scaling while buffer is running
>>
>> Changes since v2:
>> * fix id variable name in MODULE_DEVICE_TABLE
>> * make MMA7455_{I2C,SPI} symbols selectable
>> * rebase on linux-next (tag next-20151020)
>>
>> Changes since v1:
>> * limit retries to 3 in mma7455_drdy
>> * remove mma7455_show_scale_avail
>> * use chan->address instead of chan->scan_index for reg addr
>> * check that val2 is 0 when setting sample freq
>> * use __le16 to hint about endianess in mma7455_trigger_handler
>> * fix endianess in mma7455_read_raw function
>> * add mma7456 id
>> * split it into several source files to support both i2c and spi
>>
>>
>>  drivers/iio/accel/Kconfig        |  29 ++++
>>  drivers/iio/accel/Makefile       |   5 +
>>  drivers/iio/accel/mma7455.h      |  20 +++
>>  drivers/iio/accel/mma7455_core.c | 311 +++++++++++++++++++++++++++++++++++++++
>>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>>  6 files changed, 475 insertions(+)
>>  create mode 100644 drivers/iio/accel/mma7455.h
>>  create mode 100644 drivers/iio/accel/mma7455_core.c
>>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>>  create mode 100644 drivers/iio/accel/mma7455_spi.c
>>
>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>> index 969428dd6329..728a7761aaa6 100644
>> --- a/drivers/iio/accel/Kconfig
>> +++ b/drivers/iio/accel/Kconfig
>> @@ -107,6 +107,35 @@ config KXCJK1013
>>  	  To compile this driver as a module, choose M here: the module will
>>  	  be called kxcjk-1013.
>>  
>> +config MMA7455
>> +	tristate
>> +	select IIO_BUFFER
>> +	select IIO_TRIGGERED_BUFFER
>> +
>> +config MMA7455_I2C
>> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
>> +	depends on I2C
>> +	select MMA7455
>> +	select REGMAP_I2C
>> +	help
>> +	  Say yes here to build support for the Freescale MMA7455L and
>> +	  MMA7456L 3-axis accelerometer.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called mma7455_i2c.
>> +
>> +config MMA7455_SPI
>> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
>> +	depends on SPI_MASTER
>> +	select MMA7455
>> +	select REGMAP_SPI
>> +	help
>> +	  Say yes here to build support for the Freescale MMA7455L and
>> +	  MMA7456L 3-axis accelerometer.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called mma7455_spi.
>> +
>>  config MMA8452
>>  	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
>>  	depends on I2C
>> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
>> index 7925f166e6e9..7ea21f8b7d98 100644
>> --- a/drivers/iio/accel/Makefile
>> +++ b/drivers/iio/accel/Makefile
>> @@ -10,6 +10,11 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
>>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
>> +
>> +obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>> +obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
>> +obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
>> +
>>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>>  
>>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
>> diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
>> new file mode 100644
>> index 000000000000..8fae9345da88
>> --- /dev/null
>> +++ b/drivers/iio/accel/mma7455.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
>> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
> and another one...
>> + *
>> + */
>> +
>> +#ifndef __MMA7455_H
>> +#define __MMA7455_H
>> +
>> +extern const struct regmap_config mma7455_core_regmap;
>> +
>> +int mma7455_core_probe(struct device *dev, struct regmap *regmap,
>> +		       const char *name);
>> +int mma7455_core_remove(struct device *dev);
>> +
>> +#endif
>> diff --git a/drivers/iio/accel/mma7455_core.c b/drivers/iio/accel/mma7455_core.c
>> new file mode 100644
>> index 000000000000..c633cc2c0789
>> --- /dev/null
>> +++ b/drivers/iio/accel/mma7455_core.c
>> @@ -0,0 +1,311 @@
>> +/*
>> + * IIO accel core driver for Freescale MMA7455L 3-axis 10-bit accelerometer
>> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * UNSUPPORTED hardware features:
>> + *  - 8-bit mode with different scales
>> + *  - INT1/INT2 interrupts
>> + *  - Offset calibration
>> + *  - Events
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "mma7455.h"
>> +
>> +#define MMA7455_REG_XOUTL		0x00
>> +#define MMA7455_REG_XOUTH		0x01
>> +#define MMA7455_REG_YOUTL		0x02
>> +#define MMA7455_REG_YOUTH		0x03
>> +#define MMA7455_REG_ZOUTL		0x04
>> +#define MMA7455_REG_ZOUTH		0x05
>> +#define MMA7455_REG_STATUS		0x09
>> +#define  MMA7455_STATUS_DRDY		BIT(0)
>> +#define MMA7455_REG_WHOAMI		0x0f
>> +#define  MMA7455_WHOAMI_ID		0x55
>> +#define MMA7455_REG_MCTL		0x16
>> +#define  MMA7455_MCTL_MODE_STANDBY	0x00
>> +#define  MMA7455_MCTL_MODE_MEASURE	0x01
>> +#define MMA7455_REG_CTL1		0x18
>> +#define  MMA7455_CTL1_DFBW_MASK		BIT(7)
>> +#define  MMA7455_CTL1_DFBW_125HZ	BIT(7)
>> +#define  MMA7455_CTL1_DFBW_62_5HZ	0
>> +#define MMA7455_REG_TW			0x1e
>> +
>> +/*
>> + * When MMA7455 is used in 10-bit it has a fullscale of -8g
>> + * corresponding to raw value -512. The userspace interface
>> + * uses m/s^2 and we declare micro units.
>> + * So scale factor is given by:
>> + *       g * 8 * 1e6 / 512 = 153228.90625, with g = 9.80665
>> + */
>> +#define MMA7455_10BIT_SCALE	153229
>> +
>> +struct mma7455_data {
>> +	struct regmap *regmap;
>> +	struct device *dev;
>> +};
>> +
>> +static int mma7455_drdy(struct mma7455_data *mma7455)
>> +{
>> +	unsigned int reg;
>> +	int tries = 3;
>> +	int ret;
>> +
>> +	while (tries-- > 0) {
>> +		ret = regmap_read(mma7455->regmap, MMA7455_REG_STATUS, &reg);
>> +		if (ret)
>> +			return ret;
>> +
>> +		if (reg & MMA7455_STATUS_DRDY)
>> +			return 0;
>> +
>> +		msleep(20);
>> +	}
>> +
>> +	dev_warn(mma7455->dev, "data not ready\n");
>> +
>> +	return -EIO;
>> +}
>> +
>> +static irqreturn_t mma7455_trigger_handler(int irq, void *p)
>> +{
>> +	struct iio_poll_func *pf = p;
>> +	struct iio_dev *indio_dev = pf->indio_dev;
>> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
>> +	u8 buf[16]; /* 3 x 16-bit channels + padding + ts */
>> +	int ret;
>> +
>> +	ret = mma7455_drdy(mma7455);
>> +	if (ret)
>> +		goto done;
>> +
>> +	ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf,
>> +			       sizeof(__le16) * 3);
>> +	if (ret)
>> +		goto done;
>> +
>> +	iio_push_to_buffers_with_timestamp(indio_dev, buf, iio_get_time_ns());
>> +
>> +done:
>> +	iio_trigger_notify_done(indio_dev->trig);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int mma7455_read_raw(struct iio_dev *indio_dev,
>> +			    struct iio_chan_spec const *chan,
>> +			    int *val, int *val2, long mask)
>> +{
>> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
>> +	unsigned int reg;
>> +	__le16 data;
>> +	int ret;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		if (iio_buffer_enabled(indio_dev))
>> +			return -EBUSY;
>> +
>> +		ret = mma7455_drdy(mma7455);
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = regmap_bulk_read(mma7455->regmap, chan->address, &data,
>> +				       sizeof(data));
>> +		if (ret)
>> +			return ret;
>> +
>> +		*val = sign_extend32(le16_to_cpu(data), 9);
>> +
>> +		return IIO_VAL_INT;
>> +
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = 0;
>> +		*val2 = MMA7455_10BIT_SCALE;
>> +
>> +		return IIO_VAL_INT_PLUS_MICRO;
>> +
>> +	case IIO_CHAN_INFO_SAMP_FREQ:
>> +		ret = regmap_read(mma7455->regmap, MMA7455_REG_CTL1, &reg);
>> +		if (ret)
>> +			return ret;
>> +
>> +		if (reg & MMA7455_CTL1_DFBW_MASK)
>> +			*val = 250;
>> +		else
>> +			*val = 125;
>> +
>> +		return IIO_VAL_INT;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int mma7455_write_raw(struct iio_dev *indio_dev,
>> +			     struct iio_chan_spec const *chan,
>> +			     int val, int val2, long mask)
>> +{
>> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
>> +	int i;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_SAMP_FREQ:
>> +		if (val == 250 && val2 == 0)
>> +			i = MMA7455_CTL1_DFBW_125HZ;
>> +		else if (val == 125 && val2 == 0)
>> +			i = MMA7455_CTL1_DFBW_62_5HZ;
>> +		else
>> +			return -EINVAL;
>> +
>> +		return regmap_update_bits(mma7455->regmap, MMA7455_REG_CTL1,
>> +					  MMA7455_CTL1_DFBW_MASK, i);
>> +
>> +	case IIO_CHAN_INFO_SCALE:
>> +		/* In 10-bit mode there is only one scale available */
>> +		if (val == 0 && val2 == MMA7455_10BIT_SCALE)
>> +			return 0;
>> +		break;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static IIO_CONST_ATTR(sampling_frequency_available, "125 250");
>> +
>> +static struct attribute *mma7455_attributes[] = {
>> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
>> +	NULL
>> +};
>> +
>> +static const struct attribute_group mma7455_group = {
>> +	.attrs = mma7455_attributes,
>> +};
>> +
>> +static const struct iio_info mma7455_info = {
>> +	.attrs = &mma7455_group,
>> +	.read_raw = mma7455_read_raw,
>> +	.write_raw = mma7455_write_raw,
>> +	.driver_module = THIS_MODULE,
>> +};
>> +
>> +#define MMA7455_CHANNEL(axis, idx) { \
>> +	.type = IIO_ACCEL, \
>> +	.modified = 1, \
>> +	.address = MMA7455_REG_##axis##OUTL,\
>> +	.channel2 = IIO_MOD_##axis, \
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
>> +				    BIT(IIO_CHAN_INFO_SCALE), \
>> +	.scan_index = idx, \
>> +	.scan_type = { \
>> +		.sign = 's', \
>> +		.realbits = 10, \
>> +		.storagebits = 16, \
>> +		.endianness = IIO_LE, \
>> +	}, \
>> +}
>> +
>> +static const struct iio_chan_spec mma7455_channels[] = {
>> +	MMA7455_CHANNEL(X, 0),
>> +	MMA7455_CHANNEL(Y, 1),
>> +	MMA7455_CHANNEL(Z, 2),
>> +	IIO_CHAN_SOFT_TIMESTAMP(3),
>> +};
>> +
>> +static const unsigned long mma7455_scan_masks[] = {0x7, 0};
>> +
>> +const struct regmap_config mma7455_core_regmap = {
>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +	.max_register = MMA7455_REG_TW,
>> +};
>> +EXPORT_SYMBOL_GPL(mma7455_core_regmap);
>> +
>> +int mma7455_core_probe(struct device *dev, struct regmap *regmap,
>> +		       const char *name)
>> +{
>> +	struct mma7455_data *mma7455;
>> +	struct iio_dev *indio_dev;
>> +	unsigned int reg;
>> +	int ret;
>> +
>> +	ret = regmap_read(regmap, MMA7455_REG_WHOAMI, &reg);
>> +	if (ret) {
>> +		dev_err(dev, "unable to read reg\n");
>> +		return ret;
>> +	}
>> +
>> +	if (reg != MMA7455_WHOAMI_ID) {
>> +		dev_err(dev, "device id mismatch\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*mma7455));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	dev_set_drvdata(dev, indio_dev);
>> +	mma7455 = iio_priv(indio_dev);
>> +	mma7455->regmap = regmap;
>> +	mma7455->dev = dev;
>> +
>> +	indio_dev->info = &mma7455_info;
>> +	indio_dev->name = name;
>> +	indio_dev->dev.parent = dev;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +	indio_dev->channels = mma7455_channels;
>> +	indio_dev->num_channels = ARRAY_SIZE(mma7455_channels);
>> +	indio_dev->available_scan_masks = mma7455_scan_masks;
>> +
>> +	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
>> +		     MMA7455_MCTL_MODE_MEASURE);
>> +
>> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> +					 mma7455_trigger_handler, NULL);
>> +	if (ret) {
>> +		dev_err(dev, "unable to setup triggered buffer\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = iio_device_register(indio_dev);
>> +	if (ret) {
>> +		dev_err(dev, "unable to register device\n");
>> +		iio_triggered_buffer_cleanup(indio_dev);
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(mma7455_core_probe);
>> +
>> +int mma7455_core_remove(struct device *dev)
>> +{
>> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +	struct mma7455_data *mma7455 = iio_priv(indio_dev);
>> +
>> +	iio_device_unregister(indio_dev);
>> +	iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +	regmap_write(mma7455->regmap, MMA7455_REG_MCTL,
>> +		     MMA7455_MCTL_MODE_STANDBY);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(mma7455_core_remove);
>> +
>> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
>> +MODULE_DESCRIPTION("Freescale MMA7455L core accelerometer driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/iio/accel/mma7455_i2c.c b/drivers/iio/accel/mma7455_i2c.c
>> new file mode 100644
>> index 000000000000..ce4ad0d45f98
>> --- /dev/null
>> +++ b/drivers/iio/accel/mma7455_i2c.c
>> @@ -0,0 +1,57 @@
>> +/*
>> + * IIO accel I2C driver for Freescale MMA7455L 3-axis 10-bit accelerometer
>> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
> And this one (see below, I review backwards as drivers usually make
> more sense that way around).
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "mma7455.h"
>> +
>> +static int mma7455_i2c_probe(struct i2c_client *i2c,
>> +			     const struct i2c_device_id *id)
>> +{
>> +	struct regmap *regmap;
>> +	const char *name = NULL;
>> +
>> +	regmap = devm_regmap_init_i2c(i2c, &mma7455_core_regmap);
>> +	if (IS_ERR(regmap))
>> +		return PTR_ERR(regmap);
>> +
>> +	if (id)
>> +		name = id->name;
>> +
>> +	return mma7455_core_probe(&i2c->dev, regmap, name);
>> +}
>> +
>> +static int mma7455_i2c_remove(struct i2c_client *i2c)
>> +{
>> +	return mma7455_core_remove(&i2c->dev);
>> +}
>> +
>> +static const struct i2c_device_id mma7455_i2c_ids[] = {
>> +	{ "mma7455", 0 },
>> +	{ "mma7456", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, mma7455_i2c_ids);
>> +
>> +static struct i2c_driver mma7455_i2c_driver = {
>> +	.probe = mma7455_i2c_probe,
>> +	.remove = mma7455_i2c_remove,
>> +	.id_table = mma7455_i2c_ids,
>> +	.driver = {
>> +		.name	= "mma7455-i2c",
>> +	},
>> +};
>> +module_i2c_driver(mma7455_i2c_driver);
>> +
>> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
>> +MODULE_DESCRIPTION("Freescale MMA7455L I2C accelerometer driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/iio/accel/mma7455_spi.c b/drivers/iio/accel/mma7455_spi.c
>> new file mode 100644
>> index 000000000000..5dc0bb75d289
>> --- /dev/null
>> +++ b/drivers/iio/accel/mma7455_spi.c
>> @@ -0,0 +1,53 @@
>> +/*
>> + * IIO accel SPI driver for Freescale MMA7455L 3-axis 10-bit accelerometer
>> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
> My quest against blank lines with no purpose continues.  I'll edit this
> one out in the appy.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/spi/spi.h>
>> +
>> +#include "mma7455.h"
>> +
>> +static int mma7455_spi_probe(struct spi_device *spi)
>> +{
>> +	const struct spi_device_id *id = spi_get_device_id(spi);
>> +	struct regmap *regmap;
>> +
>> +	regmap = devm_regmap_init_spi(spi, &mma7455_core_regmap);
>> +	if (IS_ERR(regmap))
>> +		return PTR_ERR(regmap);
>> +
>> +	return mma7455_core_probe(&spi->dev, regmap, id->name);
>> +}
>> +
>> +static int mma7455_spi_remove(struct spi_device *spi)
>> +{
>> +	return mma7455_core_remove(&spi->dev);
>> +}
>> +
>> +static const struct spi_device_id mma7455_spi_ids[] = {
>> +	{ "mma7455", 0 },
>> +	{ "mma7456", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(spi, mma7455_spi_ids);
>> +
>> +static struct spi_driver mma7455_spi_driver = {
>> +	.probe = mma7455_spi_probe,
>> +	.remove = mma7455_spi_remove,
>> +	.id_table = mma7455_spi_ids,
>> +	.driver = {
>> +		.name = "mma7455-spi",
>> +	},
>> +};
>> +module_spi_driver(mma7455_spi_driver);
>> +
>> +MODULE_AUTHOR("Joachim Eastwood <manabian@gmail.com>");
>> +MODULE_DESCRIPTION("Freescale MMA7455L SPI accelerometer driver");
>> +MODULE_LICENSE("GPL v2");
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-11-01 18:01     ` Jonathan Cameron
@ 2015-11-02 11:07       ` Martin Kepplinger
  2015-11-08 15:47         ` Jonathan Cameron
  0 siblings, 1 reply; 36+ messages in thread
From: Martin Kepplinger @ 2015-11-02 11:07 UTC (permalink / raw)
  To: Jonathan Cameron, Joachim Eastwood, knaack.h, lars, pmeerw; +Cc: linux-iio

Am 2015-11-01 um 19:01 schrieb Jonathan Cameron:
> On 31/10/15 21:37, Martin Kepplinger wrote:
>> Am 2015-10-31 um 13:49 schrieb Joachim Eastwood:
>>> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
>>> I2C and SPI bus. This rather simple driver that currently doesn't
>>> support all the hardware features of MMA7455L/MMA7456L.
>>>
>>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>>>
>>> Data sheets for the two devices can be found here:
>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>>
>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>>> ---
>>> Hi,
>>>
>>> This version address the comments from Jonathan Cameron.
>>>
>>> Changes since v3:
>>> * use IIO_CONST_ATTR on sampling_frequency_available
>>> * allow changing of rate and scaling while buffer is running
>>>
>>> Changes since v2:
>>> * fix id variable name in MODULE_DEVICE_TABLE
>>> * make MMA7455_{I2C,SPI} symbols selectable
>>> * rebase on linux-next (tag next-20151020)
>>>
>>> Changes since v1:
>>> * limit retries to 3 in mma7455_drdy
>>> * remove mma7455_show_scale_avail
>>> * use chan->address instead of chan->scan_index for reg addr
>>> * check that val2 is 0 when setting sample freq
>>> * use __le16 to hint about endianess in mma7455_trigger_handler
>>> * fix endianess in mma7455_read_raw function
>>> * add mma7456 id
>>> * split it into several source files to support both i2c and spi
>>>
>>>
>>>  drivers/iio/accel/Kconfig        |  29 ++++
>>>  drivers/iio/accel/Makefile       |   5 +
>>>  drivers/iio/accel/mma7455.h      |  20 +++
>>>  drivers/iio/accel/mma7455_core.c | 311 +++++++++++++++++++++++++++++++++++++++
>>>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>>>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>>>  6 files changed, 475 insertions(+)
>>>  create mode 100644 drivers/iio/accel/mma7455.h
>>>  create mode 100644 drivers/iio/accel/mma7455_core.c
>>>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>>>  create mode 100644 drivers/iio/accel/mma7455_spi.c
>>>
>>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>>> index 969428dd6329..728a7761aaa6 100644
>>> --- a/drivers/iio/accel/Kconfig
>>> +++ b/drivers/iio/accel/Kconfig
>>> @@ -107,6 +107,35 @@ config KXCJK1013
>>>  	  To compile this driver as a module, choose M here: the module will
>>>  	  be called kxcjk-1013.
>>>  
>>> +config MMA7455
>>> +	tristate
>>> +	select IIO_BUFFER
>>> +	select IIO_TRIGGERED_BUFFER
>>> +
>>> +config MMA7455_I2C
>>> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
>>> +	depends on I2C
>>> +	select MMA7455
>>> +	select REGMAP_I2C
>>> +	help
>>> +	  Say yes here to build support for the Freescale MMA7455L and
>>> +	  MMA7456L 3-axis accelerometer.
>>> +
>>> +	  To compile this driver as a module, choose M here: the module
>>> +	  will be called mma7455_i2c.
>>> +
>>> +config MMA7455_SPI
>>> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
>>> +	depends on SPI_MASTER
>>> +	select MMA7455
>>> +	select REGMAP_SPI
>>> +	help
>>> +	  Say yes here to build support for the Freescale MMA7455L and
>>> +	  MMA7456L 3-axis accelerometer.
>>> +
>>> +	  To compile this driver as a module, choose M here: the module
>>> +	  will be called mma7455_spi.
>>> +
>>>  config MMA8452
>>>  	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
>>>  	depends on I2C
>>> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
>>> index 7925f166e6e9..7ea21f8b7d98 100644
>>> --- a/drivers/iio/accel/Makefile
>>> +++ b/drivers/iio/accel/Makefile
>>> @@ -10,6 +10,11 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
>>>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>>>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>>>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
>>> +
>>> +obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>>> +obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
>>> +obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
>>> +
>>>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>>>  
>>>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
>>> diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
>>> new file mode 100644
>>> index 000000000000..8fae9345da88
>>> --- /dev/null
>>> +++ b/drivers/iio/accel/mma7455.h
>>> @@ -0,0 +1,20 @@
>>> +/*
>>> + * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
>>> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
>>> + *
>>
>> Even though they are very similar (internal technological differences),
>> you could mention both supported chips here and in the other files. Btw,
>> I would also s/header/driver here.
>>
>> That is, in case you get to do another version.
>>
> Hi Martin,
> 
> I can see where you are coming from, but typically I'd actually advocate
> mentioning the list of supported parts in as few a places as possible as
> they tend to get out of sync if we end up with a driver supporting lots
> of parts.  We have gotten round this in the past but saying something
> like **** and similar or *** and compatible but that's always a bit clunky.

I see. Kconfig should probably contain all supported products, so
refering to Kconfig would be scalable...

In this case, I don't really fear that this driver will support more
chips. These 2 are all (that are very similar / equal) there are, and
are relatively old, not "recommended" by the vendor anymore.

Nothing I don't like about the driver, just thoughts.

                     martin

> 
> I'll switch header to driver in the application of the patch.
> 
> Jonathan
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-11-01 18:02     ` Jonathan Cameron
@ 2015-11-03 22:17       ` Joachim Eastwood
  2015-11-08 15:49         ` Jonathan Cameron
  0 siblings, 1 reply; 36+ messages in thread
From: Joachim Eastwood @ 2015-11-03 22:17 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald, Martin Kepplinger,
	linux-iio

Hi Jonathan,

On 1 November 2015 at 19:02, Jonathan Cameron <jic23@kernel.org> wrote:
> On 01/11/15 18:01, Jonathan Cameron wrote:
>> On 31/10/15 12:49, Joachim Eastwood wrote:
>>> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
>>> I2C and SPI bus. This rather simple driver that currently doesn't
>>> support all the hardware features of MMA7455L/MMA7456L.
>>>
>>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>>>
>>> Data sheets for the two devices can be found here:
>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>>
>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> A few really trivial gripes I'll deal with when applying.
>>
>> + the s/header/driver thing Martin picked up on.
> Forgot the important bit:
>
> Applied to the togreg branch of iio.git - initially pushed out as
> testing for the autobuilders to play with it.

Thanks for fixing up and applying.

I can't seem to find it in your testing branch on
kernel/git/jic23/iio.git, though.


regards,
Joachim Eastwood

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-11-02 11:07       ` Martin Kepplinger
@ 2015-11-08 15:47         ` Jonathan Cameron
  0 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2015-11-08 15:47 UTC (permalink / raw)
  To: Martin Kepplinger, Joachim Eastwood, knaack.h, lars, pmeerw; +Cc: linux-iio

On 02/11/15 11:07, Martin Kepplinger wrote:
> Am 2015-11-01 um 19:01 schrieb Jonathan Cameron:
>> On 31/10/15 21:37, Martin Kepplinger wrote:
>>> Am 2015-10-31 um 13:49 schrieb Joachim Eastwood:
>>>> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
>>>> I2C and SPI bus. This rather simple driver that currently doesn't
>>>> support all the hardware features of MMA7455L/MMA7456L.
>>>>
>>>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>>>>
>>>> Data sheets for the two devices can be found here:
>>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>>>
>>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>>>> ---
>>>> Hi,
>>>>
>>>> This version address the comments from Jonathan Cameron.
>>>>
>>>> Changes since v3:
>>>> * use IIO_CONST_ATTR on sampling_frequency_available
>>>> * allow changing of rate and scaling while buffer is running
>>>>
>>>> Changes since v2:
>>>> * fix id variable name in MODULE_DEVICE_TABLE
>>>> * make MMA7455_{I2C,SPI} symbols selectable
>>>> * rebase on linux-next (tag next-20151020)
>>>>
>>>> Changes since v1:
>>>> * limit retries to 3 in mma7455_drdy
>>>> * remove mma7455_show_scale_avail
>>>> * use chan->address instead of chan->scan_index for reg addr
>>>> * check that val2 is 0 when setting sample freq
>>>> * use __le16 to hint about endianess in mma7455_trigger_handler
>>>> * fix endianess in mma7455_read_raw function
>>>> * add mma7456 id
>>>> * split it into several source files to support both i2c and spi
>>>>
>>>>
>>>>  drivers/iio/accel/Kconfig        |  29 ++++
>>>>  drivers/iio/accel/Makefile       |   5 +
>>>>  drivers/iio/accel/mma7455.h      |  20 +++
>>>>  drivers/iio/accel/mma7455_core.c | 311 +++++++++++++++++++++++++++++++++++++++
>>>>  drivers/iio/accel/mma7455_i2c.c  |  57 +++++++
>>>>  drivers/iio/accel/mma7455_spi.c  |  53 +++++++
>>>>  6 files changed, 475 insertions(+)
>>>>  create mode 100644 drivers/iio/accel/mma7455.h
>>>>  create mode 100644 drivers/iio/accel/mma7455_core.c
>>>>  create mode 100644 drivers/iio/accel/mma7455_i2c.c
>>>>  create mode 100644 drivers/iio/accel/mma7455_spi.c
>>>>
>>>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>>>> index 969428dd6329..728a7761aaa6 100644
>>>> --- a/drivers/iio/accel/Kconfig
>>>> +++ b/drivers/iio/accel/Kconfig
>>>> @@ -107,6 +107,35 @@ config KXCJK1013
>>>>  	  To compile this driver as a module, choose M here: the module will
>>>>  	  be called kxcjk-1013.
>>>>  
>>>> +config MMA7455
>>>> +	tristate
>>>> +	select IIO_BUFFER
>>>> +	select IIO_TRIGGERED_BUFFER
>>>> +
>>>> +config MMA7455_I2C
>>>> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer I2C Driver"
>>>> +	depends on I2C
>>>> +	select MMA7455
>>>> +	select REGMAP_I2C
>>>> +	help
>>>> +	  Say yes here to build support for the Freescale MMA7455L and
>>>> +	  MMA7456L 3-axis accelerometer.
>>>> +
>>>> +	  To compile this driver as a module, choose M here: the module
>>>> +	  will be called mma7455_i2c.
>>>> +
>>>> +config MMA7455_SPI
>>>> +	tristate "Freescale MMA7455L/MMA7456L Accelerometer SPI Driver"
>>>> +	depends on SPI_MASTER
>>>> +	select MMA7455
>>>> +	select REGMAP_SPI
>>>> +	help
>>>> +	  Say yes here to build support for the Freescale MMA7455L and
>>>> +	  MMA7456L 3-axis accelerometer.
>>>> +
>>>> +	  To compile this driver as a module, choose M here: the module
>>>> +	  will be called mma7455_spi.
>>>> +
>>>>  config MMA8452
>>>>  	tristate "Freescale MMA8452Q and similar Accelerometers Driver"
>>>>  	depends on I2C
>>>> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
>>>> index 7925f166e6e9..7ea21f8b7d98 100644
>>>> --- a/drivers/iio/accel/Makefile
>>>> +++ b/drivers/iio/accel/Makefile
>>>> @@ -10,6 +10,11 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
>>>>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>>>>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>>>>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
>>>> +
>>>> +obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>>>> +obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
>>>> +obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
>>>> +
>>>>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>>>>  
>>>>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
>>>> diff --git a/drivers/iio/accel/mma7455.h b/drivers/iio/accel/mma7455.h
>>>> new file mode 100644
>>>> index 000000000000..8fae9345da88
>>>> --- /dev/null
>>>> +++ b/drivers/iio/accel/mma7455.h
>>>> @@ -0,0 +1,20 @@
>>>> +/*
>>>> + * IIO accel header for Freescale MMA7455L 3-axis 10-bit accelerometer
>>>> + * Copyright 2015 Joachim Eastwood <manabian@gmail.com>
>>>> + *
>>>
>>> Even though they are very similar (internal technological differences),
>>> you could mention both supported chips here and in the other files. Btw,
>>> I would also s/header/driver here.
>>>
>>> That is, in case you get to do another version.
>>>
>> Hi Martin,
>>
>> I can see where you are coming from, but typically I'd actually advocate
>> mentioning the list of supported parts in as few a places as possible as
>> they tend to get out of sync if we end up with a driver supporting lots
>> of parts.  We have gotten round this in the past but saying something
>> like **** and similar or *** and compatible but that's always a bit clunky.
> 
> I see. Kconfig should probably contain all supported products, so
> refering to Kconfig would be scalable...
Yes, could reference to Kconfig - though typically they are all only listed
in the help description as the list rapidly gets too long to go in the menu
item name.
> 
> In this case, I don't really fear that this driver will support more
> chips. These 2 are all (that are very similar / equal) there are, and
> are relatively old, not "recommended" by the vendor anymore.
Indeed, seems unlikely unless someone gets some old freescale IP in the future ;)
> 
> Nothing I don't like about the driver, just thoughts.
> 
>                      martin
> 
>>
>> I'll switch header to driver in the application of the patch.
>>
>> Jonathan
>>
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v4] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver
  2015-11-03 22:17       ` Joachim Eastwood
@ 2015-11-08 15:49         ` Jonathan Cameron
  0 siblings, 0 replies; 36+ messages in thread
From: Jonathan Cameron @ 2015-11-08 15:49 UTC (permalink / raw)
  To: Joachim Eastwood
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald, Martin Kepplinger,
	linux-iio

On 03/11/15 22:17, Joachim Eastwood wrote:
> Hi Jonathan,
> 
> On 1 November 2015 at 19:02, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 01/11/15 18:01, Jonathan Cameron wrote:
>>> On 31/10/15 12:49, Joachim Eastwood wrote:
>>>> Add support for Freescale MMA7455L/MMA7456L 3-axis in 10-bit mode for
>>>> I2C and SPI bus. This rather simple driver that currently doesn't
>>>> support all the hardware features of MMA7455L/MMA7456L.
>>>>
>>>> Tested on Embedded Artist's LPC4357 Dev Kit with MMA7455L on I2C bus.
>>>>
>>>> Data sheets for the two devices can be found here:
>>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7455L.pdf
>>>> http://cache.freescale.com/files/sensors/doc/data_sheet/MMA7456L.pdf
>>>>
>>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>>> A few really trivial gripes I'll deal with when applying.
>>>
>>> + the s/header/driver thing Martin picked up on.
>> Forgot the important bit:
>>
>> Applied to the togreg branch of iio.git - initially pushed out as
>> testing for the autobuilders to play with it.
> 
> Thanks for fixing up and applying.
> 
> I can't seem to find it in your testing branch on
> kernel/git/jic23/iio.git, though.
Sorry, more than likely I failed to push out. Once my local build
tests are done I'll be pushing out today.

Have a habit of reaching the end of a review time when someone shouts
dinner then failing to remember to come back later and push stuff out.

oops.

Jonathan
> 
> 
> regards,
> Joachim Eastwood
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2015-11-08 15:49 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-17 22:25 [PATCH] iio: accel: add Freescale MMA7455L 3-axis accelerometer driver Joachim Eastwood
2015-10-19 10:45 ` Lars-Peter Clausen
2015-10-19 12:19   ` Joachim Eastwood
2015-10-19 12:23     ` Lars-Peter Clausen
2015-10-19 12:38       ` Joachim Eastwood
2015-10-19 11:00 ` Peter Meerwald
2015-10-19 12:26   ` Joachim Eastwood
2015-10-19 12:38     ` Peter Meerwald
2015-10-19 11:10 ` Martin Kepplinger
2015-10-19 12:34   ` Joachim Eastwood
2015-10-19 12:56     ` Martin Kepplinger
2015-10-19 13:43       ` Joachim Eastwood
2015-10-19 14:09         ` Martin Kepplinger
2015-10-19 14:14           ` Lars-Peter Clausen
2015-10-19 19:00 ` [PATCH v2] iio: accel: add Freescale MMA7455L/MMA7456L " Joachim Eastwood
2015-10-19 21:07   ` Joachim Eastwood
2015-10-20  7:48   ` Martin Kepplinger
2015-10-20 11:03     ` Joachim Eastwood
2015-10-25 11:29       ` Jonathan Cameron
2015-10-20  8:05   ` Lars-Peter Clausen
2015-10-20 11:00     ` Joachim Eastwood
2015-10-20 11:05       ` Lars-Peter Clausen
2015-10-20 11:52         ` Joachim Eastwood
2015-10-20 20:50 ` [PATCH v3] " Joachim Eastwood
2015-10-25 11:45   ` Jonathan Cameron
2015-10-29 17:34     ` Joachim Eastwood
2015-10-30 10:00       ` Jonathan Cameron
2015-10-31 12:49 ` [PATCH v4] " Joachim Eastwood
2015-10-31 21:37   ` Martin Kepplinger
2015-11-01 18:01     ` Jonathan Cameron
2015-11-02 11:07       ` Martin Kepplinger
2015-11-08 15:47         ` Jonathan Cameron
2015-11-01 18:01   ` Jonathan Cameron
2015-11-01 18:02     ` Jonathan Cameron
2015-11-03 22:17       ` Joachim Eastwood
2015-11-08 15:49         ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).