Re: [PATCH v3] iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
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@xxxxxxxxx>
> + *
> + * 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@xxxxxxxxx>
> + *
> + * 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@xxxxxxxxx>");
> +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@xxxxxxxxx>
> + *
> + * 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@xxxxxxxxx>");
> +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@xxxxxxxxx>
> + *
> + * 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@xxxxxxxxx>");
> +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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux