[PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Constantin Musca]

Minimal implementation of an IIO driver for the Freescale
MMA7660FC 3-axis accelerometer. Datasheet:
http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf

Includes:
- ACPI support;
- read_raw for x,y,z axes;
- reading and setting the scale (range) parameter.
- power management

Signed-off-by: Constantin Musca <constantin.musca@intel.com>
---
 drivers/iio/accel/Kconfig   |  10 ++
 drivers/iio/accel/Makefile  |   2 +
 drivers/iio/accel/mma7660.c | 269 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 281 insertions(+)
 create mode 100644 drivers/iio/accel/mma7660.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index e4a758c..1df6361 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -136,6 +136,16 @@ config MMA7455_SPI
 	  To compile this driver as a module, choose M here: the module
 	  will be called mma7455_spi.
 
+config MMA7660
+	tristate "Freescale MMA7660FC 3-Axis Accelerometer Driver"
+	depends on I2C
+	help
+	  Say yes here to get support for the Freescale MMA7660FC 3-Axis
+	  accelerometer.
+
+	  Choosing M will build the driver as a module. If so, the module
+	  will be called mma7660.
+
 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 71b6794..ba1165f 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -15,6 +15,8 @@ obj-$(CONFIG_MMA7455)		+= mma7455_core.o
 obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
 obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
 
+obj-$(CONFIG_MMA7660)	+= mma7660.o
+
 obj-$(CONFIG_MMA8452)	+= mma8452.o
 
 obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
diff --git a/drivers/iio/accel/mma7660.c b/drivers/iio/accel/mma7660.c
new file mode 100644
index 0000000..94b2999
--- /dev/null
+++ b/drivers/iio/accel/mma7660.c
@@ -0,0 +1,269 @@
+/**
+ * Freescale MMA7660FC 3-Axis Accelerometer
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * IIO driver for Freescale MMA7660FC; 7-bit I2C address: 0x4c.
+ */
+
+#include <linux/acpi.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define MMA7660_DRIVER_NAME	"mma7660"
+
+#define MMA7660_REG_XOUT	0x00
+#define MMA7660_REG_YOUT	0x01
+#define MMA7660_REG_ZOUT	0x02
+#define MMA7660_REG_OUT_BIT_ALERT	BIT(6)
+
+#define MMA7660_REG_MODE	0x07
+#define MMA7660_REG_MODE_BIT_MODE	BIT(0)
+#define MMA7660_REG_MODE_BIT_TON	BIT(2)
+
+#define MMA7660_I2C_READ_RETRIES	5
+
+/*
+ * The accelerometer has one measurement range:
+ *
+ * -1.5g - +1.5g (6-bit, signed)
+ *
+ * scale = (1.5 + 1.5) * 9.81 / (2^6 - 1)	= 0.467142857
+ */
+
+#define MMA7660_SCALE_AVAIL	"0.467142857"
+
+const int mma7660_nscale = 467142857;
+
+#define MMA7660_CHANNEL(reg, axis) {	\
+	.type = IIO_ACCEL,	\
+	.address = reg,	\
+	.modified = 1,	\
+	.channel2 = IIO_MOD_##axis,	\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+}
+
+static const struct iio_chan_spec mma7660_channels[] = {
+	MMA7660_CHANNEL(MMA7660_REG_XOUT, X),
+	MMA7660_CHANNEL(MMA7660_REG_YOUT, Y),
+	MMA7660_CHANNEL(MMA7660_REG_ZOUT, Z),
+};
+
+enum mma7660_mode {
+	MMA7660_MODE_STANDBY,
+	MMA7660_MODE_ACTIVE
+};
+
+struct mma7660_data {
+	struct i2c_client *client;
+	struct mutex lock;
+	enum mma7660_mode mode;
+};
+
+static IIO_CONST_ATTR(in_accel_scale_available, MMA7660_SCALE_AVAIL);
+
+static struct attribute *mma7660_attributes[] = {
+	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group mma7660_attribute_group = {
+	.attrs = mma7660_attributes
+};
+
+static int mma7660_set_mode(struct mma7660_data *data,
+				enum mma7660_mode mode)
+{
+	int ret;
+	struct i2c_client *client = data->client;
+
+	if (mode == data->mode)
+		return 0;
+
+	ret = i2c_smbus_read_byte_data(client, MMA7660_REG_MODE);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to read sensor mode\n");
+		return ret;
+	}
+
+	if (mode == MMA7660_MODE_ACTIVE) {
+		ret &= ~MMA7660_REG_MODE_BIT_TON;
+		ret |= MMA7660_REG_MODE_BIT_MODE;
+	} else {
+		ret &= ~MMA7660_REG_MODE_BIT_TON;
+		ret &= ~MMA7660_REG_MODE_BIT_MODE;
+	}
+
+	ret = i2c_smbus_write_byte_data(client, MMA7660_REG_MODE, ret);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to change sensor mode\n");
+		return ret;
+	}
+
+	data->mode = mode;
+	return ret;
+}
+
+static int mma7660_read_accel(struct mma7660_data *data, u8 address)
+{
+	int ret, retries = MMA7660_I2C_READ_RETRIES;
+	struct i2c_client *client = data->client;
+
+	do {
+		ret = i2c_smbus_read_byte_data(client, address);
+		if (ret < 0) {
+			dev_err(&client->dev, "register read failed\n");
+			return ret;
+		}
+	} while (retries-- > 0 && ret & MMA7660_REG_OUT_BIT_ALERT);
+
+	if (ret & MMA7660_REG_OUT_BIT_ALERT) {
+		dev_err(&client->dev, "all register read retries failed\n");
+		return -ETIMEDOUT;
+	}
+
+	return ret;
+}
+
+static int mma7660_read_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int *val, int *val2, long mask)
+{
+	struct mma7660_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&data->lock);
+		ret = mma7660_read_accel(data, chan->address);
+		mutex_unlock(&data->lock);
+		if (ret < 0)
+			return ret;
+		*val = sign_extend32(ret, 5);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = mma7660_nscale;
+		return IIO_VAL_INT_PLUS_NANO;
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info mma7660_info = {
+	.driver_module	= THIS_MODULE,
+	.read_raw		= mma7660_read_raw,
+	.attrs			= &mma7660_attribute_group,
+};
+
+static int mma7660_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	int ret;
+	struct iio_dev *indio_dev;
+	struct mma7660_data *data;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev) {
+		dev_err(&client->dev, "iio allocation failed!\n");
+		return -ENOMEM;
+	}
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+	i2c_set_clientdata(client, indio_dev);
+	mutex_init(&data->lock);
+	data->mode = MMA7660_MODE_STANDBY;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &mma7660_info;
+	indio_dev->name = MMA7660_DRIVER_NAME;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = mma7660_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mma7660_channels);
+
+	ret = mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0) {
+		dev_err(&client->dev, "device_register failed\n");
+		mma7660_set_mode(data, MMA7660_MODE_STANDBY);
+	}
+
+	return ret;
+}
+
+static int mma7660_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+
+	return mma7660_set_mode(iio_priv(indio_dev), MMA7660_MODE_STANDBY);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mma7660_suspend(struct device *dev)
+{
+	struct mma7660_data *data;
+
+	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+
+	return mma7660_set_mode(data, MMA7660_MODE_STANDBY);
+}
+
+static int mma7660_resume(struct device *dev)
+{
+	struct mma7660_data *data;
+
+	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+
+	return mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
+}
+
+static SIMPLE_DEV_PM_OPS(mma7660_pm_ops, mma7660_suspend, mma7660_resume);
+
+#define MMA7660_PM_OPS (&mma7660_pm_ops)
+#else
+#define MMA7660_PM_OPS NULL
+#endif
+
+static const struct i2c_device_id mma7660_i2c_id[] = {
+	{"mma7660", 0},
+	{}
+};
+
+static const struct acpi_device_id mma7660_acpi_id[] = {
+	{"MMA7660", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(acpi, mma7660_acpi_id);
+
+static struct i2c_driver mma7660_driver = {
+	.driver = {
+		.name = "mma7660",
+		.pm = MMA7660_PM_OPS,
+		.acpi_match_table = ACPI_PTR(mma7660_acpi_id),
+	},
+	.probe		= mma7660_probe,
+	.remove		= mma7660_remove,
+	.id_table	= mma7660_i2c_id,
+};
+
+module_i2c_driver(mma7660_driver);
+
+MODULE_AUTHOR("Constantin Musca <constantin.musca@intel.com>");
+MODULE_DESCRIPTION("Freescale MMA7660FC 3-Axis Accelerometer driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Martin Kepplinger]

Am 2016-04-29 um 14:19 schrieb Constantin Musca:
> Minimal implementation of an IIO driver for the Freescale
> MMA7660FC 3-axis accelerometer. Datasheet:
> http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf

it's nxp.com now, which would probably be valid for a longer time.

Theoretically there would be a way to add it to mma8452, but with quite
some rewriting of register definitions. I guess it's ok to start over
here to keep it simple. If everyone is aware of this, feel free to add
my reviewed-by.

Are you planning to support it's orienation detection via iio in the future?

                      martin

> 
> Includes:
> - ACPI support;
> - read_raw for x,y,z axes;
> - reading and setting the scale (range) parameter.
> - power management
> 
> Signed-off-by: Constantin Musca <constantin.musca@intel.com>
> ---
>  drivers/iio/accel/Kconfig   |  10 ++
>  drivers/iio/accel/Makefile  |   2 +
>  drivers/iio/accel/mma7660.c | 269 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 281 insertions(+)
>  create mode 100644 drivers/iio/accel/mma7660.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index e4a758c..1df6361 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -136,6 +136,16 @@ config MMA7455_SPI
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called mma7455_spi.
>  
> +config MMA7660
> +	tristate "Freescale MMA7660FC 3-Axis Accelerometer Driver"
> +	depends on I2C
> +	help
> +	  Say yes here to get support for the Freescale MMA7660FC 3-Axis
> +	  accelerometer.
> +
> +	  Choosing M will build the driver as a module. If so, the module
> +	  will be called mma7660.
> +
>  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 71b6794..ba1165f 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -15,6 +15,8 @@ obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>  obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
>  obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
>  
> +obj-$(CONFIG_MMA7660)	+= mma7660.o
> +
>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>  
>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
> diff --git a/drivers/iio/accel/mma7660.c b/drivers/iio/accel/mma7660.c
> new file mode 100644
> index 0000000..94b2999
> --- /dev/null
> +++ b/drivers/iio/accel/mma7660.c
> @@ -0,0 +1,269 @@
> +/**
> + * Freescale MMA7660FC 3-Axis Accelerometer
> + *
> + * Copyright (c) 2016, Intel Corporation.
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * IIO driver for Freescale MMA7660FC; 7-bit I2C address: 0x4c.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define MMA7660_DRIVER_NAME	"mma7660"
> +
> +#define MMA7660_REG_XOUT	0x00
> +#define MMA7660_REG_YOUT	0x01
> +#define MMA7660_REG_ZOUT	0x02
> +#define MMA7660_REG_OUT_BIT_ALERT	BIT(6)
> +
> +#define MMA7660_REG_MODE	0x07
> +#define MMA7660_REG_MODE_BIT_MODE	BIT(0)
> +#define MMA7660_REG_MODE_BIT_TON	BIT(2)
> +
> +#define MMA7660_I2C_READ_RETRIES	5
> +
> +/*
> + * The accelerometer has one measurement range:
> + *
> + * -1.5g - +1.5g (6-bit, signed)
> + *
> + * scale = (1.5 + 1.5) * 9.81 / (2^6 - 1)	= 0.467142857
> + */
> +
> +#define MMA7660_SCALE_AVAIL	"0.467142857"
> +
> +const int mma7660_nscale = 467142857;
> +
> +#define MMA7660_CHANNEL(reg, axis) {	\
> +	.type = IIO_ACCEL,	\
> +	.address = reg,	\
> +	.modified = 1,	\
> +	.channel2 = IIO_MOD_##axis,	\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +}
> +
> +static const struct iio_chan_spec mma7660_channels[] = {
> +	MMA7660_CHANNEL(MMA7660_REG_XOUT, X),
> +	MMA7660_CHANNEL(MMA7660_REG_YOUT, Y),
> +	MMA7660_CHANNEL(MMA7660_REG_ZOUT, Z),
> +};
> +
> +enum mma7660_mode {
> +	MMA7660_MODE_STANDBY,
> +	MMA7660_MODE_ACTIVE
> +};
> +
> +struct mma7660_data {
> +	struct i2c_client *client;
> +	struct mutex lock;
> +	enum mma7660_mode mode;
> +};
> +
> +static IIO_CONST_ATTR(in_accel_scale_available, MMA7660_SCALE_AVAIL);
> +
> +static struct attribute *mma7660_attributes[] = {
> +	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group mma7660_attribute_group = {
> +	.attrs = mma7660_attributes
> +};
> +
> +static int mma7660_set_mode(struct mma7660_data *data,
> +				enum mma7660_mode mode)
> +{
> +	int ret;
> +	struct i2c_client *client = data->client;
> +
> +	if (mode == data->mode)
> +		return 0;
> +
> +	ret = i2c_smbus_read_byte_data(client, MMA7660_REG_MODE);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to read sensor mode\n");
> +		return ret;
> +	}
> +
> +	if (mode == MMA7660_MODE_ACTIVE) {
> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
> +		ret |= MMA7660_REG_MODE_BIT_MODE;
> +	} else {
> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
> +		ret &= ~MMA7660_REG_MODE_BIT_MODE;
> +	}
> +
> +	ret = i2c_smbus_write_byte_data(client, MMA7660_REG_MODE, ret);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to change sensor mode\n");
> +		return ret;
> +	}
> +
> +	data->mode = mode;
> +	return ret;
> +}
> +
> +static int mma7660_read_accel(struct mma7660_data *data, u8 address)
> +{
> +	int ret, retries = MMA7660_I2C_READ_RETRIES;
> +	struct i2c_client *client = data->client;
> +
> +	do {
> +		ret = i2c_smbus_read_byte_data(client, address);
> +		if (ret < 0) {
> +			dev_err(&client->dev, "register read failed\n");
> +			return ret;
> +		}
> +	} while (retries-- > 0 && ret & MMA7660_REG_OUT_BIT_ALERT);
> +
> +	if (ret & MMA7660_REG_OUT_BIT_ALERT) {
> +		dev_err(&client->dev, "all register read retries failed\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return ret;
> +}
> +
> +static int mma7660_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct mma7660_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&data->lock);
> +		ret = mma7660_read_accel(data, chan->address);
> +		mutex_unlock(&data->lock);
> +		if (ret < 0)
> +			return ret;
> +		*val = sign_extend32(ret, 5);
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = mma7660_nscale;
> +		return IIO_VAL_INT_PLUS_NANO;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info mma7660_info = {
> +	.driver_module	= THIS_MODULE,
> +	.read_raw		= mma7660_read_raw,
> +	.attrs			= &mma7660_attribute_group,
> +};
> +
> +static int mma7660_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev;
> +	struct mma7660_data *data;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev) {
> +		dev_err(&client->dev, "iio allocation failed!\n");
> +		return -ENOMEM;
> +	}
> +
> +	data = iio_priv(indio_dev);
> +	data->client = client;
> +	i2c_set_clientdata(client, indio_dev);
> +	mutex_init(&data->lock);
> +	data->mode = MMA7660_MODE_STANDBY;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &mma7660_info;
> +	indio_dev->name = MMA7660_DRIVER_NAME;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mma7660_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(mma7660_channels);
> +
> +	ret = mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "device_register failed\n");
> +		mma7660_set_mode(data, MMA7660_MODE_STANDBY);
> +	}
> +
> +	return ret;
> +}
> +
> +static int mma7660_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	return mma7660_set_mode(iio_priv(indio_dev), MMA7660_MODE_STANDBY);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mma7660_suspend(struct device *dev)
> +{
> +	struct mma7660_data *data;
> +
> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return mma7660_set_mode(data, MMA7660_MODE_STANDBY);
> +}
> +
> +static int mma7660_resume(struct device *dev)
> +{
> +	struct mma7660_data *data;
> +
> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mma7660_pm_ops, mma7660_suspend, mma7660_resume);
> +
> +#define MMA7660_PM_OPS (&mma7660_pm_ops)
> +#else
> +#define MMA7660_PM_OPS NULL
> +#endif
> +
> +static const struct i2c_device_id mma7660_i2c_id[] = {
> +	{"mma7660", 0},
> +	{}
> +};
> +
> +static const struct acpi_device_id mma7660_acpi_id[] = {
> +	{"MMA7660", 0},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, mma7660_acpi_id);
> +
> +static struct i2c_driver mma7660_driver = {
> +	.driver = {
> +		.name = "mma7660",
> +		.pm = MMA7660_PM_OPS,
> +		.acpi_match_table = ACPI_PTR(mma7660_acpi_id),
> +	},
> +	.probe		= mma7660_probe,
> +	.remove		= mma7660_remove,
> +	.id_table	= mma7660_i2c_id,
> +};
> +
> +module_i2c_driver(mma7660_driver);
> +
> +MODULE_AUTHOR("Constantin Musca <constantin.musca@intel.com>");
> +MODULE_DESCRIPTION("Freescale MMA7660FC 3-Axis Accelerometer driver");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Constantin Musca]

Thanks. We will add orientation detection support in case there will be 
a requirement for this feature in the future.

Constantin

On 29.04.2016 16:08, Martin Kepplinger wrote:
> Am 2016-04-29 um 14:19 schrieb Constantin Musca:
>> Minimal implementation of an IIO driver for the Freescale
>> MMA7660FC 3-axis accelerometer. Datasheet:
>> http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf
> it's nxp.com now, which would probably be valid for a longer time.
>
> Theoretically there would be a way to add it to mma8452, but with quite
> some rewriting of register definitions. I guess it's ok to start over
> here to keep it simple. If everyone is aware of this, feel free to add
> my reviewed-by.
>
> Are you planning to support it's orienation detection via iio in the future?
>
>                        martin
>
>> Includes:
>> - ACPI support;
>> - read_raw for x,y,z axes;
>> - reading and setting the scale (range) parameter.
>> - power management
>>
>> Signed-off-by: Constantin Musca <constantin.musca@intel.com>
>> ---
>>   drivers/iio/accel/Kconfig   |  10 ++
>>   drivers/iio/accel/Makefile  |   2 +
>>   drivers/iio/accel/mma7660.c | 269 ++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 281 insertions(+)
>>   create mode 100644 drivers/iio/accel/mma7660.c
>>
>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>> index e4a758c..1df6361 100644
>> --- a/drivers/iio/accel/Kconfig
>> +++ b/drivers/iio/accel/Kconfig
>> @@ -136,6 +136,16 @@ config MMA7455_SPI
>>   	  To compile this driver as a module, choose M here: the module
>>   	  will be called mma7455_spi.
>>   
>> +config MMA7660
>> +	tristate "Freescale MMA7660FC 3-Axis Accelerometer Driver"
>> +	depends on I2C
>> +	help
>> +	  Say yes here to get support for the Freescale MMA7660FC 3-Axis
>> +	  accelerometer.
>> +
>> +	  Choosing M will build the driver as a module. If so, the module
>> +	  will be called mma7660.
>> +
>>   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 71b6794..ba1165f 100644
>> --- a/drivers/iio/accel/Makefile
>> +++ b/drivers/iio/accel/Makefile
>> @@ -15,6 +15,8 @@ obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>>   obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
>>   obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
>>   
>> +obj-$(CONFIG_MMA7660)	+= mma7660.o
>> +
>>   obj-$(CONFIG_MMA8452)	+= mma8452.o
>>   
>>   obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
>> diff --git a/drivers/iio/accel/mma7660.c b/drivers/iio/accel/mma7660.c
>> new file mode 100644
>> index 0000000..94b2999
>> --- /dev/null
>> +++ b/drivers/iio/accel/mma7660.c
>> @@ -0,0 +1,269 @@
>> +/**
>> + * Freescale MMA7660FC 3-Axis Accelerometer
>> + *
>> + * Copyright (c) 2016, Intel Corporation.
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License. See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * IIO driver for Freescale MMA7660FC; 7-bit I2C address: 0x4c.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +#define MMA7660_DRIVER_NAME	"mma7660"
>> +
>> +#define MMA7660_REG_XOUT	0x00
>> +#define MMA7660_REG_YOUT	0x01
>> +#define MMA7660_REG_ZOUT	0x02
>> +#define MMA7660_REG_OUT_BIT_ALERT	BIT(6)
>> +
>> +#define MMA7660_REG_MODE	0x07
>> +#define MMA7660_REG_MODE_BIT_MODE	BIT(0)
>> +#define MMA7660_REG_MODE_BIT_TON	BIT(2)
>> +
>> +#define MMA7660_I2C_READ_RETRIES	5
>> +
>> +/*
>> + * The accelerometer has one measurement range:
>> + *
>> + * -1.5g - +1.5g (6-bit, signed)
>> + *
>> + * scale = (1.5 + 1.5) * 9.81 / (2^6 - 1)	= 0.467142857
>> + */
>> +
>> +#define MMA7660_SCALE_AVAIL	"0.467142857"
>> +
>> +const int mma7660_nscale = 467142857;
>> +
>> +#define MMA7660_CHANNEL(reg, axis) {	\
>> +	.type = IIO_ACCEL,	\
>> +	.address = reg,	\
>> +	.modified = 1,	\
>> +	.channel2 = IIO_MOD_##axis,	\
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
>> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
>> +}
>> +
>> +static const struct iio_chan_spec mma7660_channels[] = {
>> +	MMA7660_CHANNEL(MMA7660_REG_XOUT, X),
>> +	MMA7660_CHANNEL(MMA7660_REG_YOUT, Y),
>> +	MMA7660_CHANNEL(MMA7660_REG_ZOUT, Z),
>> +};
>> +
>> +enum mma7660_mode {
>> +	MMA7660_MODE_STANDBY,
>> +	MMA7660_MODE_ACTIVE
>> +};
>> +
>> +struct mma7660_data {
>> +	struct i2c_client *client;
>> +	struct mutex lock;
>> +	enum mma7660_mode mode;
>> +};
>> +
>> +static IIO_CONST_ATTR(in_accel_scale_available, MMA7660_SCALE_AVAIL);
>> +
>> +static struct attribute *mma7660_attributes[] = {
>> +	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
>> +	NULL,
>> +};
>> +
>> +static const struct attribute_group mma7660_attribute_group = {
>> +	.attrs = mma7660_attributes
>> +};
>> +
>> +static int mma7660_set_mode(struct mma7660_data *data,
>> +				enum mma7660_mode mode)
>> +{
>> +	int ret;
>> +	struct i2c_client *client = data->client;
>> +
>> +	if (mode == data->mode)
>> +		return 0;
>> +
>> +	ret = i2c_smbus_read_byte_data(client, MMA7660_REG_MODE);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "failed to read sensor mode\n");
>> +		return ret;
>> +	}
>> +
>> +	if (mode == MMA7660_MODE_ACTIVE) {
>> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
>> +		ret |= MMA7660_REG_MODE_BIT_MODE;
>> +	} else {
>> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
>> +		ret &= ~MMA7660_REG_MODE_BIT_MODE;
>> +	}
>> +
>> +	ret = i2c_smbus_write_byte_data(client, MMA7660_REG_MODE, ret);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "failed to change sensor mode\n");
>> +		return ret;
>> +	}
>> +
>> +	data->mode = mode;
>> +	return ret;
>> +}
>> +
>> +static int mma7660_read_accel(struct mma7660_data *data, u8 address)
>> +{
>> +	int ret, retries = MMA7660_I2C_READ_RETRIES;
>> +	struct i2c_client *client = data->client;
>> +
>> +	do {
>> +		ret = i2c_smbus_read_byte_data(client, address);
>> +		if (ret < 0) {
>> +			dev_err(&client->dev, "register read failed\n");
>> +			return ret;
>> +		}
>> +	} while (retries-- > 0 && ret & MMA7660_REG_OUT_BIT_ALERT);
>> +
>> +	if (ret & MMA7660_REG_OUT_BIT_ALERT) {
>> +		dev_err(&client->dev, "all register read retries failed\n");
>> +		return -ETIMEDOUT;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int mma7660_read_raw(struct iio_dev *indio_dev,
>> +				struct iio_chan_spec const *chan,
>> +				int *val, int *val2, long mask)
>> +{
>> +	struct mma7660_data *data = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		mutex_lock(&data->lock);
>> +		ret = mma7660_read_accel(data, chan->address);
>> +		mutex_unlock(&data->lock);
>> +		if (ret < 0)
>> +			return ret;
>> +		*val = sign_extend32(ret, 5);
>> +		return IIO_VAL_INT;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = 0;
>> +		*val2 = mma7660_nscale;
>> +		return IIO_VAL_INT_PLUS_NANO;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static const struct iio_info mma7660_info = {
>> +	.driver_module	= THIS_MODULE,
>> +	.read_raw		= mma7660_read_raw,
>> +	.attrs			= &mma7660_attribute_group,
>> +};
>> +
>> +static int mma7660_probe(struct i2c_client *client,
>> +			const struct i2c_device_id *id)
>> +{
>> +	int ret;
>> +	struct iio_dev *indio_dev;
>> +	struct mma7660_data *data;
>> +
>> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> +	if (!indio_dev) {
>> +		dev_err(&client->dev, "iio allocation failed!\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	data = iio_priv(indio_dev);
>> +	data->client = client;
>> +	i2c_set_clientdata(client, indio_dev);
>> +	mutex_init(&data->lock);
>> +	data->mode = MMA7660_MODE_STANDBY;
>> +
>> +	indio_dev->dev.parent = &client->dev;
>> +	indio_dev->info = &mma7660_info;
>> +	indio_dev->name = MMA7660_DRIVER_NAME;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +	indio_dev->channels = mma7660_channels;
>> +	indio_dev->num_channels = ARRAY_SIZE(mma7660_channels);
>> +
>> +	ret = mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = iio_device_register(indio_dev);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "device_register failed\n");
>> +		mma7660_set_mode(data, MMA7660_MODE_STANDBY);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int mma7660_remove(struct i2c_client *client)
>> +{
>> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> +
>> +	iio_device_unregister(indio_dev);
>> +
>> +	return mma7660_set_mode(iio_priv(indio_dev), MMA7660_MODE_STANDBY);
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int mma7660_suspend(struct device *dev)
>> +{
>> +	struct mma7660_data *data;
>> +
>> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +	return mma7660_set_mode(data, MMA7660_MODE_STANDBY);
>> +}
>> +
>> +static int mma7660_resume(struct device *dev)
>> +{
>> +	struct mma7660_data *data;
>> +
>> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +	return mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(mma7660_pm_ops, mma7660_suspend, mma7660_resume);
>> +
>> +#define MMA7660_PM_OPS (&mma7660_pm_ops)
>> +#else
>> +#define MMA7660_PM_OPS NULL
>> +#endif
>> +
>> +static const struct i2c_device_id mma7660_i2c_id[] = {
>> +	{"mma7660", 0},
>> +	{}
>> +};
>> +
>> +static const struct acpi_device_id mma7660_acpi_id[] = {
>> +	{"MMA7660", 0},
>> +	{}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(acpi, mma7660_acpi_id);
>> +
>> +static struct i2c_driver mma7660_driver = {
>> +	.driver = {
>> +		.name = "mma7660",
>> +		.pm = MMA7660_PM_OPS,
>> +		.acpi_match_table = ACPI_PTR(mma7660_acpi_id),
>> +	},
>> +	.probe		= mma7660_probe,
>> +	.remove		= mma7660_remove,
>> +	.id_table	= mma7660_i2c_id,
>> +};
>> +
>> +module_i2c_driver(mma7660_driver);
>> +
>> +MODULE_AUTHOR("Constantin Musca <constantin.musca@intel.com>");
>> +MODULE_DESCRIPTION("Freescale MMA7660FC 3-Axis Accelerometer driver");
>> +MODULE_LICENSE("GPL v2");
>>

Re: [PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Jonathan Cameron]

On 29/04/16 14:08, Martin Kepplinger wrote:
> Am 2016-04-29 um 14:19 schrieb Constantin Musca:
>> Minimal implementation of an IIO driver for the Freescale
>> MMA7660FC 3-axis accelerometer. Datasheet:
>> http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf
> 
> it's nxp.com now, which would probably be valid for a longer time.
> 
> Theoretically there would be a way to add it to mma8452, but with quite
> some rewriting of register definitions. I guess it's ok to start over
> here to keep it simple. If everyone is aware of this, feel free to add
> my reviewed-by.
Martin - formal tags please! and in the right place...
> 
> Are you planning to support it's orienation detection via iio in the future?
> 
>                       martin
> 
>>
>> Includes:
>> - ACPI support;
>> - read_raw for x,y,z axes;
>> - reading and setting the scale (range) parameter.
>> - power management
>>
>> Signed-off-by: Constantin Musca <constantin.musca@intel.com>
Reviewed-by: Martin Klepplinger <martink@posteo.de>

!!

>> ---
>>  drivers/iio/accel/Kconfig   |  10 ++
>>  drivers/iio/accel/Makefile  |   2 +
>>  drivers/iio/accel/mma7660.c | 269 ++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 281 insertions(+)
>>  create mode 100644 drivers/iio/accel/mma7660.c
>>
>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>> index e4a758c..1df6361 100644
>> --- a/drivers/iio/accel/Kconfig
>> +++ b/drivers/iio/accel/Kconfig
>> @@ -136,6 +136,16 @@ config MMA7455_SPI
>>  	  To compile this driver as a module, choose M here: the module
>>  	  will be called mma7455_spi.
>>  
>> +config MMA7660
>> +	tristate "Freescale MMA7660FC 3-Axis Accelerometer Driver"
>> +	depends on I2C
>> +	help
>> +	  Say yes here to get support for the Freescale MMA7660FC 3-Axis
>> +	  accelerometer.
>> +
>> +	  Choosing M will build the driver as a module. If so, the module
>> +	  will be called mma7660.
>> +
>>  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 71b6794..ba1165f 100644
>> --- a/drivers/iio/accel/Makefile
>> +++ b/drivers/iio/accel/Makefile
>> @@ -15,6 +15,8 @@ obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>>  obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
>>  obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
>>  
>> +obj-$(CONFIG_MMA7660)	+= mma7660.o
>> +
>>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>>  
>>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
>> diff --git a/drivers/iio/accel/mma7660.c b/drivers/iio/accel/mma7660.c
>> new file mode 100644
>> index 0000000..94b2999
>> --- /dev/null
>> +++ b/drivers/iio/accel/mma7660.c
>> @@ -0,0 +1,269 @@
>> +/**
>> + * Freescale MMA7660FC 3-Axis Accelerometer
>> + *
>> + * Copyright (c) 2016, Intel Corporation.
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License. See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * IIO driver for Freescale MMA7660FC; 7-bit I2C address: 0x4c.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +#define MMA7660_DRIVER_NAME	"mma7660"
>> +
>> +#define MMA7660_REG_XOUT	0x00
>> +#define MMA7660_REG_YOUT	0x01
>> +#define MMA7660_REG_ZOUT	0x02
>> +#define MMA7660_REG_OUT_BIT_ALERT	BIT(6)
>> +
>> +#define MMA7660_REG_MODE	0x07
>> +#define MMA7660_REG_MODE_BIT_MODE	BIT(0)
>> +#define MMA7660_REG_MODE_BIT_TON	BIT(2)
>> +
>> +#define MMA7660_I2C_READ_RETRIES	5
>> +
>> +/*
>> + * The accelerometer has one measurement range:
>> + *
>> + * -1.5g - +1.5g (6-bit, signed)
>> + *
>> + * scale = (1.5 + 1.5) * 9.81 / (2^6 - 1)	= 0.467142857
>> + */
>> +
>> +#define MMA7660_SCALE_AVAIL	"0.467142857"
>> +
>> +const int mma7660_nscale = 467142857;
>> +
>> +#define MMA7660_CHANNEL(reg, axis) {	\
>> +	.type = IIO_ACCEL,	\
>> +	.address = reg,	\
>> +	.modified = 1,	\
>> +	.channel2 = IIO_MOD_##axis,	\
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
>> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
>> +}
>> +
>> +static const struct iio_chan_spec mma7660_channels[] = {
>> +	MMA7660_CHANNEL(MMA7660_REG_XOUT, X),
>> +	MMA7660_CHANNEL(MMA7660_REG_YOUT, Y),
>> +	MMA7660_CHANNEL(MMA7660_REG_ZOUT, Z),
>> +};
>> +
>> +enum mma7660_mode {
>> +	MMA7660_MODE_STANDBY,
>> +	MMA7660_MODE_ACTIVE
>> +};
>> +
>> +struct mma7660_data {
>> +	struct i2c_client *client;
>> +	struct mutex lock;
>> +	enum mma7660_mode mode;
>> +};
>> +
>> +static IIO_CONST_ATTR(in_accel_scale_available, MMA7660_SCALE_AVAIL);
>> +
>> +static struct attribute *mma7660_attributes[] = {
>> +	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
>> +	NULL,
>> +};
>> +
>> +static const struct attribute_group mma7660_attribute_group = {
>> +	.attrs = mma7660_attributes
>> +};
>> +
>> +static int mma7660_set_mode(struct mma7660_data *data,
>> +				enum mma7660_mode mode)
>> +{
>> +	int ret;
>> +	struct i2c_client *client = data->client;
>> +
>> +	if (mode == data->mode)
>> +		return 0;
>> +
>> +	ret = i2c_smbus_read_byte_data(client, MMA7660_REG_MODE);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "failed to read sensor mode\n");
>> +		return ret;
>> +	}
>> +
>> +	if (mode == MMA7660_MODE_ACTIVE) {
>> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
>> +		ret |= MMA7660_REG_MODE_BIT_MODE;
>> +	} else {
>> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
>> +		ret &= ~MMA7660_REG_MODE_BIT_MODE;
>> +	}
>> +
>> +	ret = i2c_smbus_write_byte_data(client, MMA7660_REG_MODE, ret);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "failed to change sensor mode\n");
>> +		return ret;
>> +	}
>> +
>> +	data->mode = mode;
>> +	return ret;
>> +}
>> +
>> +static int mma7660_read_accel(struct mma7660_data *data, u8 address)
>> +{
>> +	int ret, retries = MMA7660_I2C_READ_RETRIES;
>> +	struct i2c_client *client = data->client;
>> +
>> +	do {
>> +		ret = i2c_smbus_read_byte_data(client, address);
>> +		if (ret < 0) {
>> +			dev_err(&client->dev, "register read failed\n");
>> +			return ret;
>> +		}
>> +	} while (retries-- > 0 && ret & MMA7660_REG_OUT_BIT_ALERT);
>> +
>> +	if (ret & MMA7660_REG_OUT_BIT_ALERT) {
>> +		dev_err(&client->dev, "all register read retries failed\n");
>> +		return -ETIMEDOUT;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int mma7660_read_raw(struct iio_dev *indio_dev,
>> +				struct iio_chan_spec const *chan,
>> +				int *val, int *val2, long mask)
>> +{
>> +	struct mma7660_data *data = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		mutex_lock(&data->lock);
>> +		ret = mma7660_read_accel(data, chan->address);
>> +		mutex_unlock(&data->lock);
>> +		if (ret < 0)
>> +			return ret;
>> +		*val = sign_extend32(ret, 5);
>> +		return IIO_VAL_INT;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = 0;
>> +		*val2 = mma7660_nscale;
>> +		return IIO_VAL_INT_PLUS_NANO;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static const struct iio_info mma7660_info = {
>> +	.driver_module	= THIS_MODULE,
>> +	.read_raw		= mma7660_read_raw,
>> +	.attrs			= &mma7660_attribute_group,
>> +};
>> +
>> +static int mma7660_probe(struct i2c_client *client,
>> +			const struct i2c_device_id *id)
>> +{
>> +	int ret;
>> +	struct iio_dev *indio_dev;
>> +	struct mma7660_data *data;
>> +
>> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> +	if (!indio_dev) {
>> +		dev_err(&client->dev, "iio allocation failed!\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	data = iio_priv(indio_dev);
>> +	data->client = client;
>> +	i2c_set_clientdata(client, indio_dev);
>> +	mutex_init(&data->lock);
>> +	data->mode = MMA7660_MODE_STANDBY;
>> +
>> +	indio_dev->dev.parent = &client->dev;
>> +	indio_dev->info = &mma7660_info;
>> +	indio_dev->name = MMA7660_DRIVER_NAME;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +	indio_dev->channels = mma7660_channels;
>> +	indio_dev->num_channels = ARRAY_SIZE(mma7660_channels);
>> +
>> +	ret = mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = iio_device_register(indio_dev);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "device_register failed\n");
>> +		mma7660_set_mode(data, MMA7660_MODE_STANDBY);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int mma7660_remove(struct i2c_client *client)
>> +{
>> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> +
>> +	iio_device_unregister(indio_dev);
>> +
>> +	return mma7660_set_mode(iio_priv(indio_dev), MMA7660_MODE_STANDBY);
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int mma7660_suspend(struct device *dev)
>> +{
>> +	struct mma7660_data *data;
>> +
>> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +	return mma7660_set_mode(data, MMA7660_MODE_STANDBY);
>> +}
>> +
>> +static int mma7660_resume(struct device *dev)
>> +{
>> +	struct mma7660_data *data;
>> +
>> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +	return mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(mma7660_pm_ops, mma7660_suspend, mma7660_resume);
>> +
>> +#define MMA7660_PM_OPS (&mma7660_pm_ops)
>> +#else
>> +#define MMA7660_PM_OPS NULL
>> +#endif
>> +
>> +static const struct i2c_device_id mma7660_i2c_id[] = {
>> +	{"mma7660", 0},
>> +	{}
>> +};
>> +
>> +static const struct acpi_device_id mma7660_acpi_id[] = {
>> +	{"MMA7660", 0},
>> +	{}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(acpi, mma7660_acpi_id);
>> +
>> +static struct i2c_driver mma7660_driver = {
>> +	.driver = {
>> +		.name = "mma7660",
>> +		.pm = MMA7660_PM_OPS,
>> +		.acpi_match_table = ACPI_PTR(mma7660_acpi_id),
>> +	},
>> +	.probe		= mma7660_probe,
>> +	.remove		= mma7660_remove,
>> +	.id_table	= mma7660_i2c_id,
>> +};
>> +
>> +module_i2c_driver(mma7660_driver);
>> +
>> +MODULE_AUTHOR("Constantin Musca <constantin.musca@intel.com>");
>> +MODULE_DESCRIPTION("Freescale MMA7660FC 3-Axis 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
> 

Re: [PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Jonathan Cameron]

On 29/04/16 13:19, Constantin Musca wrote:
> Minimal implementation of an IIO driver for the Freescale
> MMA7660FC 3-axis accelerometer. Datasheet:
> http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf
> 
> Includes:
> - ACPI support;
> - read_raw for x,y,z axes;
> - reading and setting the scale (range) parameter.
> - power management
> 
> Signed-off-by: Constantin Musca <constantin.musca@intel.com>
Couple of trivial bits inline as well as that request to update the link above
if possible...

Why the retries? (I'm on a train without internet access for quite a few
hours yet hence I can't dig into the datasheet!)

Anyhow, even if it's obvious from the datasheet, that sort of 'magic' needs
an explanation comment...

Jonathan
> ---
>  drivers/iio/accel/Kconfig   |  10 ++
>  drivers/iio/accel/Makefile  |   2 +
>  drivers/iio/accel/mma7660.c | 269 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 281 insertions(+)
>  create mode 100644 drivers/iio/accel/mma7660.c
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index e4a758c..1df6361 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -136,6 +136,16 @@ config MMA7455_SPI
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called mma7455_spi.
>  
> +config MMA7660
> +	tristate "Freescale MMA7660FC 3-Axis Accelerometer Driver"
> +	depends on I2C
> +	help
> +	  Say yes here to get support for the Freescale MMA7660FC 3-Axis
> +	  accelerometer.
> +
> +	  Choosing M will build the driver as a module. If so, the module
> +	  will be called mma7660.
> +
>  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 71b6794..ba1165f 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -15,6 +15,8 @@ obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>  obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
>  obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
>  
> +obj-$(CONFIG_MMA7660)	+= mma7660.o
> +
>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>  
>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
> diff --git a/drivers/iio/accel/mma7660.c b/drivers/iio/accel/mma7660.c
> new file mode 100644
> index 0000000..94b2999
> --- /dev/null
> +++ b/drivers/iio/accel/mma7660.c
> @@ -0,0 +1,269 @@
> +/**
> + * Freescale MMA7660FC 3-Axis Accelerometer
> + *
> + * Copyright (c) 2016, Intel Corporation.
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * IIO driver for Freescale MMA7660FC; 7-bit I2C address: 0x4c.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define MMA7660_DRIVER_NAME	"mma7660"
> +
> +#define MMA7660_REG_XOUT	0x00
> +#define MMA7660_REG_YOUT	0x01
> +#define MMA7660_REG_ZOUT	0x02
> +#define MMA7660_REG_OUT_BIT_ALERT	BIT(6)
> +
> +#define MMA7660_REG_MODE	0x07
> +#define MMA7660_REG_MODE_BIT_MODE	BIT(0)
> +#define MMA7660_REG_MODE_BIT_TON	BIT(2)
> +
> +#define MMA7660_I2C_READ_RETRIES	5
> +
> +/*
> + * The accelerometer has one measurement range:
> + *
> + * -1.5g - +1.5g (6-bit, signed)
> + *
> + * scale = (1.5 + 1.5) * 9.81 / (2^6 - 1)	= 0.467142857
> + */
> +
> +#define MMA7660_SCALE_AVAIL	"0.467142857"
> +
> +const int mma7660_nscale = 467142857;
> +
> +#define MMA7660_CHANNEL(reg, axis) {	\
> +	.type = IIO_ACCEL,	\
> +	.address = reg,	\
> +	.modified = 1,	\
> +	.channel2 = IIO_MOD_##axis,	\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +}
> +
> +static const struct iio_chan_spec mma7660_channels[] = {
> +	MMA7660_CHANNEL(MMA7660_REG_XOUT, X),
> +	MMA7660_CHANNEL(MMA7660_REG_YOUT, Y),
> +	MMA7660_CHANNEL(MMA7660_REG_ZOUT, Z),
> +};
> +
> +enum mma7660_mode {
> +	MMA7660_MODE_STANDBY,
> +	MMA7660_MODE_ACTIVE
> +};
> +
> +struct mma7660_data {
> +	struct i2c_client *client;
> +	struct mutex lock;
> +	enum mma7660_mode mode;
> +};
> +
> +static IIO_CONST_ATTR(in_accel_scale_available, MMA7660_SCALE_AVAIL);
> +
> +static struct attribute *mma7660_attributes[] = {
> +	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group mma7660_attribute_group = {
> +	.attrs = mma7660_attributes
> +};
> +
> +static int mma7660_set_mode(struct mma7660_data *data,
> +				enum mma7660_mode mode)
> +{
> +	int ret;
> +	struct i2c_client *client = data->client;
> +
> +	if (mode == data->mode)
> +		return 0;
> +
> +	ret = i2c_smbus_read_byte_data(client, MMA7660_REG_MODE);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to read sensor mode\n");
> +		return ret;
> +	}
> +
> +	if (mode == MMA7660_MODE_ACTIVE) {
> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
> +		ret |= MMA7660_REG_MODE_BIT_MODE;
> +	} else {
> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
> +		ret &= ~MMA7660_REG_MODE_BIT_MODE;
> +	}
> +
> +	ret = i2c_smbus_write_byte_data(client, MMA7660_REG_MODE, ret);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to change sensor mode\n");
> +		return ret;
> +	}
> +
> +	data->mode = mode;
blank line here (slightly prefered)...
> +	return ret;
> +}
> +
> +static int mma7660_read_accel(struct mma7660_data *data, u8 address)
> +{
> +	int ret, retries = MMA7660_I2C_READ_RETRIES;
> +	struct i2c_client *client = data->client;
> +
This needs a comment to explain why a number of tries might be needed. 
> +	do {
> +		ret = i2c_smbus_read_byte_data(client, address);
> +		if (ret < 0) {
> +			dev_err(&client->dev, "register read failed\n");
> +			return ret;
> +		}
> +	} while (retries-- > 0 && ret & MMA7660_REG_OUT_BIT_ALERT);
> +
> +	if (ret & MMA7660_REG_OUT_BIT_ALERT) {
> +		dev_err(&client->dev, "all register read retries failed\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return ret;
> +}
> +
> +static int mma7660_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct mma7660_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&data->lock);
> +		ret = mma7660_read_accel(data, chan->address);
> +		mutex_unlock(&data->lock);
> +		if (ret < 0)
> +			return ret;
> +		*val = sign_extend32(ret, 5);
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = mma7660_nscale;
> +		return IIO_VAL_INT_PLUS_NANO;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info mma7660_info = {
> +	.driver_module	= THIS_MODULE,
> +	.read_raw		= mma7660_read_raw,
> +	.attrs			= &mma7660_attribute_group,
> +};
> +
> +static int mma7660_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev;
> +	struct mma7660_data *data;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev) {
> +		dev_err(&client->dev, "iio allocation failed!\n");
> +		return -ENOMEM;
> +	}
> +
> +	data = iio_priv(indio_dev);
> +	data->client = client;
> +	i2c_set_clientdata(client, indio_dev);
> +	mutex_init(&data->lock);
> +	data->mode = MMA7660_MODE_STANDBY;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &mma7660_info;
> +	indio_dev->name = MMA7660_DRIVER_NAME;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mma7660_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(mma7660_channels);
> +
> +	ret = mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "device_register failed\n");
> +		mma7660_set_mode(data, MMA7660_MODE_STANDBY);
> +	}
> +
> +	return ret;
> +}
> +
> +static int mma7660_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	return mma7660_set_mode(iio_priv(indio_dev), MMA7660_MODE_STANDBY);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mma7660_suspend(struct device *dev)
> +{
> +	struct mma7660_data *data;
> +
> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return mma7660_set_mode(data, MMA7660_MODE_STANDBY);
> +}
> +
> +static int mma7660_resume(struct device *dev)
> +{
> +	struct mma7660_data *data;
> +
> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mma7660_pm_ops, mma7660_suspend, mma7660_resume);
> +
> +#define MMA7660_PM_OPS (&mma7660_pm_ops)
> +#else
> +#define MMA7660_PM_OPS NULL
> +#endif
> +
> +static const struct i2c_device_id mma7660_i2c_id[] = {
> +	{"mma7660", 0},
> +	{}
> +};
> +
> +static const struct acpi_device_id mma7660_acpi_id[] = {
> +	{"MMA7660", 0},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, mma7660_acpi_id);
> +
> +static struct i2c_driver mma7660_driver = {
> +	.driver = {
> +		.name = "mma7660",
> +		.pm = MMA7660_PM_OPS,
> +		.acpi_match_table = ACPI_PTR(mma7660_acpi_id),
> +	},
> +	.probe		= mma7660_probe,
> +	.remove		= mma7660_remove,
> +	.id_table	= mma7660_i2c_id,
> +};
> +
> +module_i2c_driver(mma7660_driver);
> +
> +MODULE_AUTHOR("Constantin Musca <constantin.musca@intel.com>");
> +MODULE_DESCRIPTION("Freescale MMA7660FC 3-Axis Accelerometer driver");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Daniel Baluta]

On Sun, May 1, 2016 at 9:56 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 29/04/16 13:19, Constantin Musca wrote:
>> Minimal implementation of an IIO driver for the Freescale
>> MMA7660FC 3-axis accelerometer. Datasheet:
>> http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf
>>
>> Includes:
>> - ACPI support;
>> - read_raw for x,y,z axes;
>> - reading and setting the scale (range) parameter.
>> - power management
>>
>> Signed-off-by: Constantin Musca <constantin.musca@intel.com>
> Couple of trivial bits inline as well as that request to update the link above
> if possible...
>
> Why the retries? (I'm on a train without internet access for quite a few
> hours yet hence I can't dig into the datasheet!)

I suggested the retries because it's quite unsafe to infinitely loop
in the kernel
on a hardware condition. We can lockup the kernel if there is some sort of
hardware problem.


>
> Anyhow, even if it's obvious from the datasheet, that sort of 'magic' needs
> an explanation comment...

Re: [PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Jonathan Cameron]

On 3 May 2016 09:29:26 CEST, Daniel Baluta <daniel.baluta@intel.com> wrote:
>On Sun, May 1, 2016 at 9:56 PM, Jonathan Cameron <jic23@kernel.org>
>wrote:
>> On 29/04/16 13:19, Constantin Musca wrote:
>>> Minimal implementation of an IIO driver for the Freescale
>>> MMA7660FC 3-axis accelerometer. Datasheet:
>>>
>http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf
>>>
>>> Includes:
>>> - ACPI support;
>>> - read_raw for x,y,z axes;
>>> - reading and setting the scale (range) parameter.
>>> - power management
>>>
>>> Signed-off-by: Constantin Musca <constantin.musca@intel.com>
>> Couple of trivial bits inline as well as that request to update the
>link above
>> if possible...
>>
>> Why the retries? (I'm on a train without internet access for quite a
>few
>> hours yet hence I can't dig into the datasheet!)
>
>I suggested the retries because it's quite unsafe to infinitely loop
>in the kernel
>on a hardware condition. We can lockup the kernel if there is some sort
>of
>hardware problem.
Agreed but why try more than once? Needs a comment...
>
>
>>
>> Anyhow, even if it's obvious from the datasheet, that sort of 'magic'
>needs
>> an explanation comment...

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

Re: [PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Daniel Baluta]

On Tue, May 3, 2016 at 1:43 PM, Jonathan Cameron
<jic23@jic23.retrosnub.co.uk> wrote:
>
>
> On 3 May 2016 09:29:26 CEST, Daniel Baluta <daniel.baluta@intel.com> wrote:
>>On Sun, May 1, 2016 at 9:56 PM, Jonathan Cameron <jic23@kernel.org>
>>wrote:
>>> On 29/04/16 13:19, Constantin Musca wrote:
>>>> Minimal implementation of an IIO driver for the Freescale
>>>> MMA7660FC 3-axis accelerometer. Datasheet:
>>>>
>>http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf
>>>>
>>>> Includes:
>>>> - ACPI support;
>>>> - read_raw for x,y,z axes;
>>>> - reading and setting the scale (range) parameter.
>>>> - power management
>>>>
>>>> Signed-off-by: Constantin Musca <constantin.musca@intel.com>
>>> Couple of trivial bits inline as well as that request to update the
>>link above
>>> if possible...
>>>
>>> Why the retries? (I'm on a train without internet access for quite a
>>few
>>> hours yet hence I can't dig into the datasheet!)
>>
>>I suggested the retries because it's quite unsafe to infinitely loop
>>in the kernel
>>on a hardware condition. We can lockup the kernel if there is some sort
>>of
>>hardware problem.
> Agreed but why try more than once? Needs a comment...
>>
>>
>>>
>>> Anyhow, even if it's obvious from the datasheet, that sort of 'magic'
>>needs
>>> an explanation comment...

Agree we need a comment on that. As for the number of retries I can see
drivers doing from 5 to 100 (re)tries:


humidity/si7005.c
44:     int tries = 50;
55:     while (tries-- > 0) {

temperature/tmp006.c
55:     int tries = 50;
57:     while (tries-- > 0) {

dac/mcp4725.c
76:     int tries = 20;
99:     while (tries--) {
111:    if (tries < 0) {

pressure/mpl3115.c
49:     int ret, tries = 15;
57:     while (tries-- > 0) {

ight/ltr501.c
329:    int tries = 100;
332:    while (tries--) {

proximity/pulsedlight-lidar-lite-v2.c
160:    int tries = 10;

accel/mma9551_core.c
75:#define MMA9551_I2C_READ_RETRIES     5

Daniel.

Re: [PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Jonathan Cameron]

On 03/05/16 11:55, Daniel Baluta wrote:
> On Tue, May 3, 2016 at 1:43 PM, Jonathan Cameron
> <jic23@jic23.retrosnub.co.uk> wrote:
>>
>>
>> On 3 May 2016 09:29:26 CEST, Daniel Baluta <daniel.baluta@intel.com> wrote:
>>> On Sun, May 1, 2016 at 9:56 PM, Jonathan Cameron <jic23@kernel.org>
>>> wrote:
>>>> On 29/04/16 13:19, Constantin Musca wrote:
>>>>> Minimal implementation of an IIO driver for the Freescale
>>>>> MMA7660FC 3-axis accelerometer. Datasheet:
>>>>>
>>> http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf
>>>>>
>>>>> Includes:
>>>>> - ACPI support;
>>>>> - read_raw for x,y,z axes;
>>>>> - reading and setting the scale (range) parameter.
>>>>> - power management
>>>>>
>>>>> Signed-off-by: Constantin Musca <constantin.musca@intel.com>
>>>> Couple of trivial bits inline as well as that request to update the
>>> link above
>>>> if possible...
>>>>
>>>> Why the retries? (I'm on a train without internet access for quite a
>>> few
>>>> hours yet hence I can't dig into the datasheet!)
>>>
>>> I suggested the retries because it's quite unsafe to infinitely loop
>>> in the kernel
>>> on a hardware condition. We can lockup the kernel if there is some sort
>>> of
>>> hardware problem.
>> Agreed but why try more than once? Needs a comment...
>>>
>>>
>>>>
>>>> Anyhow, even if it's obvious from the datasheet, that sort of 'magic'
>>> needs
>>>> an explanation comment...
> 
> Agree we need a comment on that. As for the number of retries I can see
> drivers doing from 5 to 100 (re)tries:
Interacts with the delays in the relevant loops.
> 
> 
> humidity/si7005.c
> 44:     int tries = 50;
> 55:     while (tries-- > 0) {
This one has a 20msec delay so total time is 1 second.
It is polling for a status change as the reading completes (faking an 
interrupt).
> 
> temperature/tmp006.c
> 55:     int tries = 50;
> 57:     while (tries-- > 0) {
100msec sleep so 5 seconds (feels a bit long now you mention it.)

> 
> dac/mcp4725.c
> 76:     int tries = 20;
> 99:     while (tries--) {
> 111:    if (tries < 0) {
Curious overkill - comment says up to 50msecs then allows 20 x 20
 = 400 msecs.
> 
> pressure/mpl3115.c
> 49:     int ret, tries = 15;
> 57:     while (tries-- > 0) {
15x20msecs no comment on how long it might take but 300msecs seems fine.
> 
> light/ltr501.c
> 329:    int tries = 100;
> 332:    while (tries--) {
100x25mescs = 2.5secs (rather long) - no comment on what is reasonable...
> 
> proximity/pulsedlight-lidar-lite-v2.c
> 160:    int tries = 10;
polls rather quick 1-2msecs so max 10msecs.
> 
> accel/mma9551_core.c
> 75:#define MMA9551_I2C_READ_RETRIES     5
No real description at all for this one... I don't have the datasheet to hand
an no internet right now.

Definitely room for some improved commenting in some of these drivers
(I clearly wasn't as fussy in the past!)  If anyone wants to datasheet dive
and add missing justifications that would be great.

Jonathan
> 
> Daniel.
> 

Re: [PATCH v3] iio: accel: Add support for Freescale MMA7660FC

[Jonathan Cameron]

On 01/05/16 19:20, Jonathan Cameron wrote:
> On 29/04/16 14:08, Martin Kepplinger wrote:
>> Am 2016-04-29 um 14:19 schrieb Constantin Musca:
>>> Minimal implementation of an IIO driver for the Freescale
>>> MMA7660FC 3-axis accelerometer. Datasheet:
>>> http://www.freescale.com.cn/files/sensors/doc/data_sheet/MMA7660FC.pdf
>>
>> it's nxp.com now, which would probably be valid for a longer time.
>>
>> Theoretically there would be a way to add it to mma8452, but with quite
>> some rewriting of register definitions. I guess it's ok to start over
>> here to keep it simple. If everyone is aware of this, feel free to add
>> my reviewed-by.
> Martin - formal tags please! and in the right place...
>>
>> Are you planning to support it's orienation detection via iio in the future?
>>
>>                       martin
>>
>>>
>>> Includes:
>>> - ACPI support;
>>> - read_raw for x,y,z axes;
>>> - reading and setting the scale (range) parameter.
>>> - power management
>>>
>>> Signed-off-by: Constantin Musca <constantin.musca@intel.com>
> Reviewed-by: Martin Klepplinger <martink@posteo.de>
> 
> !!
I nearly missed this when actually applying the patch - just added
the tag...
> 
>>> ---
>>>  drivers/iio/accel/Kconfig   |  10 ++
>>>  drivers/iio/accel/Makefile  |   2 +
>>>  drivers/iio/accel/mma7660.c | 269 ++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 281 insertions(+)
>>>  create mode 100644 drivers/iio/accel/mma7660.c
>>>
>>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>>> index e4a758c..1df6361 100644
>>> --- a/drivers/iio/accel/Kconfig
>>> +++ b/drivers/iio/accel/Kconfig
>>> @@ -136,6 +136,16 @@ config MMA7455_SPI
>>>  	  To compile this driver as a module, choose M here: the module
>>>  	  will be called mma7455_spi.
>>>  
>>> +config MMA7660
>>> +	tristate "Freescale MMA7660FC 3-Axis Accelerometer Driver"
>>> +	depends on I2C
>>> +	help
>>> +	  Say yes here to get support for the Freescale MMA7660FC 3-Axis
>>> +	  accelerometer.
>>> +
>>> +	  Choosing M will build the driver as a module. If so, the module
>>> +	  will be called mma7660.
>>> +
>>>  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 71b6794..ba1165f 100644
>>> --- a/drivers/iio/accel/Makefile
>>> +++ b/drivers/iio/accel/Makefile
>>> @@ -15,6 +15,8 @@ obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>>>  obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
>>>  obj-$(CONFIG_MMA7455_SPI)	+= mma7455_spi.o
>>>  
>>> +obj-$(CONFIG_MMA7660)	+= mma7660.o
>>> +
>>>  obj-$(CONFIG_MMA8452)	+= mma8452.o
>>>  
>>>  obj-$(CONFIG_MMA9551_CORE)	+= mma9551_core.o
>>> diff --git a/drivers/iio/accel/mma7660.c b/drivers/iio/accel/mma7660.c
>>> new file mode 100644
>>> index 0000000..94b2999
>>> --- /dev/null
>>> +++ b/drivers/iio/accel/mma7660.c
>>> @@ -0,0 +1,269 @@
>>> +/**
>>> + * Freescale MMA7660FC 3-Axis Accelerometer
>>> + *
>>> + * Copyright (c) 2016, Intel Corporation.
>>> + *
>>> + * This file is subject to the terms and conditions of version 2 of
>>> + * the GNU General Public License. See the file COPYING in the main
>>> + * directory of this archive for more details.
>>> + *
>>> + * IIO driver for Freescale MMA7660FC; 7-bit I2C address: 0x4c.
>>> + */
>>> +
>>> +#include <linux/acpi.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/module.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +
>>> +#define MMA7660_DRIVER_NAME	"mma7660"
>>> +
>>> +#define MMA7660_REG_XOUT	0x00
>>> +#define MMA7660_REG_YOUT	0x01
>>> +#define MMA7660_REG_ZOUT	0x02
>>> +#define MMA7660_REG_OUT_BIT_ALERT	BIT(6)
>>> +
>>> +#define MMA7660_REG_MODE	0x07
>>> +#define MMA7660_REG_MODE_BIT_MODE	BIT(0)
>>> +#define MMA7660_REG_MODE_BIT_TON	BIT(2)
>>> +
>>> +#define MMA7660_I2C_READ_RETRIES	5
>>> +
>>> +/*
>>> + * The accelerometer has one measurement range:
>>> + *
>>> + * -1.5g - +1.5g (6-bit, signed)
>>> + *
>>> + * scale = (1.5 + 1.5) * 9.81 / (2^6 - 1)	= 0.467142857
>>> + */
>>> +
>>> +#define MMA7660_SCALE_AVAIL	"0.467142857"
>>> +
>>> +const int mma7660_nscale = 467142857;
>>> +
>>> +#define MMA7660_CHANNEL(reg, axis) {	\
>>> +	.type = IIO_ACCEL,	\
>>> +	.address = reg,	\
>>> +	.modified = 1,	\
>>> +	.channel2 = IIO_MOD_##axis,	\
>>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
>>> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
>>> +}
>>> +
>>> +static const struct iio_chan_spec mma7660_channels[] = {
>>> +	MMA7660_CHANNEL(MMA7660_REG_XOUT, X),
>>> +	MMA7660_CHANNEL(MMA7660_REG_YOUT, Y),
>>> +	MMA7660_CHANNEL(MMA7660_REG_ZOUT, Z),
>>> +};
>>> +
>>> +enum mma7660_mode {
>>> +	MMA7660_MODE_STANDBY,
>>> +	MMA7660_MODE_ACTIVE
>>> +};
>>> +
>>> +struct mma7660_data {
>>> +	struct i2c_client *client;
>>> +	struct mutex lock;
>>> +	enum mma7660_mode mode;
>>> +};
>>> +
>>> +static IIO_CONST_ATTR(in_accel_scale_available, MMA7660_SCALE_AVAIL);
>>> +
>>> +static struct attribute *mma7660_attributes[] = {
>>> +	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
>>> +	NULL,
>>> +};
>>> +
>>> +static const struct attribute_group mma7660_attribute_group = {
>>> +	.attrs = mma7660_attributes
>>> +};
>>> +
>>> +static int mma7660_set_mode(struct mma7660_data *data,
>>> +				enum mma7660_mode mode)
>>> +{
>>> +	int ret;
>>> +	struct i2c_client *client = data->client;
>>> +
>>> +	if (mode == data->mode)
>>> +		return 0;
>>> +
>>> +	ret = i2c_smbus_read_byte_data(client, MMA7660_REG_MODE);
>>> +	if (ret < 0) {
>>> +		dev_err(&client->dev, "failed to read sensor mode\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	if (mode == MMA7660_MODE_ACTIVE) {
>>> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
>>> +		ret |= MMA7660_REG_MODE_BIT_MODE;
>>> +	} else {
>>> +		ret &= ~MMA7660_REG_MODE_BIT_TON;
>>> +		ret &= ~MMA7660_REG_MODE_BIT_MODE;
>>> +	}
>>> +
>>> +	ret = i2c_smbus_write_byte_data(client, MMA7660_REG_MODE, ret);
>>> +	if (ret < 0) {
>>> +		dev_err(&client->dev, "failed to change sensor mode\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	data->mode = mode;
>>> +	return ret;
>>> +}
>>> +
>>> +static int mma7660_read_accel(struct mma7660_data *data, u8 address)
>>> +{
>>> +	int ret, retries = MMA7660_I2C_READ_RETRIES;
>>> +	struct i2c_client *client = data->client;
>>> +
>>> +	do {
>>> +		ret = i2c_smbus_read_byte_data(client, address);
>>> +		if (ret < 0) {
>>> +			dev_err(&client->dev, "register read failed\n");
>>> +			return ret;
>>> +		}
>>> +	} while (retries-- > 0 && ret & MMA7660_REG_OUT_BIT_ALERT);
>>> +
>>> +	if (ret & MMA7660_REG_OUT_BIT_ALERT) {
>>> +		dev_err(&client->dev, "all register read retries failed\n");
>>> +		return -ETIMEDOUT;
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int mma7660_read_raw(struct iio_dev *indio_dev,
>>> +				struct iio_chan_spec const *chan,
>>> +				int *val, int *val2, long mask)
>>> +{
>>> +	struct mma7660_data *data = iio_priv(indio_dev);
>>> +	int ret;
>>> +
>>> +	switch (mask) {
>>> +	case IIO_CHAN_INFO_RAW:
>>> +		mutex_lock(&data->lock);
>>> +		ret = mma7660_read_accel(data, chan->address);
>>> +		mutex_unlock(&data->lock);
>>> +		if (ret < 0)
>>> +			return ret;
>>> +		*val = sign_extend32(ret, 5);
>>> +		return IIO_VAL_INT;
>>> +	case IIO_CHAN_INFO_SCALE:
>>> +		*val = 0;
>>> +		*val2 = mma7660_nscale;
>>> +		return IIO_VAL_INT_PLUS_NANO;
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +
>>> +static const struct iio_info mma7660_info = {
>>> +	.driver_module	= THIS_MODULE,
>>> +	.read_raw		= mma7660_read_raw,
>>> +	.attrs			= &mma7660_attribute_group,
>>> +};
>>> +
>>> +static int mma7660_probe(struct i2c_client *client,
>>> +			const struct i2c_device_id *id)
>>> +{
>>> +	int ret;
>>> +	struct iio_dev *indio_dev;
>>> +	struct mma7660_data *data;
>>> +
>>> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>>> +	if (!indio_dev) {
>>> +		dev_err(&client->dev, "iio allocation failed!\n");
>>> +		return -ENOMEM;
>>> +	}
>>> +
>>> +	data = iio_priv(indio_dev);
>>> +	data->client = client;
>>> +	i2c_set_clientdata(client, indio_dev);
>>> +	mutex_init(&data->lock);
>>> +	data->mode = MMA7660_MODE_STANDBY;
>>> +
>>> +	indio_dev->dev.parent = &client->dev;
>>> +	indio_dev->info = &mma7660_info;
>>> +	indio_dev->name = MMA7660_DRIVER_NAME;
>>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>>> +	indio_dev->channels = mma7660_channels;
>>> +	indio_dev->num_channels = ARRAY_SIZE(mma7660_channels);
>>> +
>>> +	ret = mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	ret = iio_device_register(indio_dev);
>>> +	if (ret < 0) {
>>> +		dev_err(&client->dev, "device_register failed\n");
>>> +		mma7660_set_mode(data, MMA7660_MODE_STANDBY);
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int mma7660_remove(struct i2c_client *client)
>>> +{
>>> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>>> +
>>> +	iio_device_unregister(indio_dev);
>>> +
>>> +	return mma7660_set_mode(iio_priv(indio_dev), MMA7660_MODE_STANDBY);
>>> +}
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int mma7660_suspend(struct device *dev)
>>> +{
>>> +	struct mma7660_data *data;
>>> +
>>> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>>> +
>>> +	return mma7660_set_mode(data, MMA7660_MODE_STANDBY);
>>> +}
>>> +
>>> +static int mma7660_resume(struct device *dev)
>>> +{
>>> +	struct mma7660_data *data;
>>> +
>>> +	data = iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>>> +
>>> +	return mma7660_set_mode(data, MMA7660_MODE_ACTIVE);
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(mma7660_pm_ops, mma7660_suspend, mma7660_resume);
>>> +
>>> +#define MMA7660_PM_OPS (&mma7660_pm_ops)
>>> +#else
>>> +#define MMA7660_PM_OPS NULL
>>> +#endif
>>> +
>>> +static const struct i2c_device_id mma7660_i2c_id[] = {
>>> +	{"mma7660", 0},
>>> +	{}
>>> +};
>>> +
>>> +static const struct acpi_device_id mma7660_acpi_id[] = {
>>> +	{"MMA7660", 0},
>>> +	{}
>>> +};
>>> +
>>> +MODULE_DEVICE_TABLE(acpi, mma7660_acpi_id);
>>> +
>>> +static struct i2c_driver mma7660_driver = {
>>> +	.driver = {
>>> +		.name = "mma7660",
>>> +		.pm = MMA7660_PM_OPS,
>>> +		.acpi_match_table = ACPI_PTR(mma7660_acpi_id),
>>> +	},
>>> +	.probe		= mma7660_probe,
>>> +	.remove		= mma7660_remove,
>>> +	.id_table	= mma7660_i2c_id,
>>> +};
>>> +
>>> +module_i2c_driver(mma7660_driver);
>>> +
>>> +MODULE_AUTHOR("Constantin Musca <constantin.musca@intel.com>");
>>> +MODULE_DESCRIPTION("Freescale MMA7660FC 3-Axis 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
>>
> 
> --
> 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
>