* [PATCH v3] iio: add driver for Freescale MMA9551L
@ 2014-11-17 15:03 Vlad Dogaru
2014-11-23 10:58 ` Hartmut Knaack
0 siblings, 1 reply; 3+ messages in thread
From: Vlad Dogaru @ 2014-11-17 15:03 UTC (permalink / raw)
To: linux-iio; +Cc: Vlad Dogaru, Irina Tirdea
Add support for Freescale MMA9551L Intelligent Motion-Sensing Platform.
The driver supports raw reads for acceleration and inclination, as well
as configuring an inclination rate-of-change events. The events can be
used similarly to an Android sensor Tilt event.
The specifications can be downloaded from:
http://www.freescale.com/files/sensors/doc/ref_manual/MMA955xLSWRM.pdf
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Signed-off-by: Vlad Dogaru <vlad.dogaru@intel.com>
---
Changes since v2:
- all three events can be enabled at the same time, each using a separate
GPIO line.
- simplify GPIO config code.
- style fixes as suggested by Jonathan.
Changes since v1:
- invert logic in mma9551_set_device_state(). Setting the sleep bit to 1 puts
the device into sleep.
- do not use scan_index for channels, since we don't support buffering.
- style fixes as suggested by Peter.
drivers/iio/accel/Kconfig | 10 +
drivers/iio/accel/Makefile | 1 +
drivers/iio/accel/mma9551.c | 960 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 971 insertions(+)
create mode 100644 drivers/iio/accel/mma9551.c
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 9b9be87..d80616d 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -105,4 +105,14 @@ config KXCJK1013
To compile this driver as a module, choose M here: the module will
be called kxcjk-1013.
+config MMA9551
+ tristate "Freescale MMA9551L Intelligent Motion-Sensing Platform Driver"
+ depends on I2C
+ help
+ Say yes here to build support for the Freescale MMA9551L
+ Intelligent Motion-Sensing Platform Driver.
+
+ To compile this driver as a module, choose M here: the module
+ will be called mma9551.
+
endmenu
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index a593996..de5b9cb 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
obj-$(CONFIG_KXSD9) += kxsd9.o
obj-$(CONFIG_MMA8452) += mma8452.o
+obj-$(CONFIG_MMA9551) += mma9551.o
obj-$(CONFIG_IIO_ST_ACCEL_3AXIS) += st_accel.o
st_accel-y := st_accel_core.o
diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
new file mode 100644
index 0000000..d677042
--- /dev/null
+++ b/drivers/iio/accel/mma9551.c
@@ -0,0 +1,960 @@
+/*
+ * Freescale MMA9551L Intelligent Motion-Sensing Platform driver
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
+
+#define MMA9551_DRV_NAME "mma9551"
+#define MMA9551_IRQ_NAME "mma9551_event"
+#define MMA9551_GPIO_NAME "mma9551_int"
+#define MMA9551_GPIO_COUNT 4
+
+/* Applications IDs */
+#define MMA9551_APPID_VERSION 0x00
+#define MMA9551_APPID_GPIO 0x03
+#define MMA9551_APPID_AFE 0x06
+#define MMA9551_APPID_TILT 0x0B
+#define MMA9551_APPID_SLEEP_WAKE 0x12
+#define MMA9551_APPID_RESET 0x17
+#define MMA9551_APPID_NONE 0xff
+
+/* Command masks for mailbox write command */
+#define MMA9551_CMD_READ_VERSION_INFO 0x00
+#define MMA9551_CMD_READ_CONFIG 0x10
+#define MMA9551_CMD_WRITE_CONFIG 0x20
+#define MMA9551_CMD_READ_STATUS 0x30
+
+enum mma9551_gpio_pin {
+ mma9551_gpio6 = 0,
+ mma9551_gpio7,
+ mma9551_gpio8,
+ mma9551_gpio9,
+ mma9551_gpio_max = mma9551_gpio9,
+};
+
+/* Mailbox read command */
+#define MMA9551_RESPONSE_COCO BIT(7)
+
+/* Error-Status codes returned in mailbox read command */
+#define MMA9551_MCI_ERROR_NONE 0x00
+#define MMA9551_MCI_ERROR_PARAM 0x04
+#define MMA9551_MCI_INVALID_COUNT 0x19
+#define MMA9551_MCI_ERROR_COMMAND 0x1C
+#define MMA9551_MCI_ERROR_INVALID_LENGTH 0x21
+#define MMA9551_MCI_ERROR_FIFO_BUSY 0x22
+#define MMA9551_MCI_ERROR_FIFO_ALLOCATED 0x23
+#define MMA9551_MCI_ERROR_FIFO_OVERSIZE 0x24
+
+/* GPIO Application */
+#define MMA9551_GPIO_POL_MSB 0x08
+#define MMA9551_GPIO_POL_LSB 0x09
+
+/* Sleep/Wake application */
+#define MMA9551_SLEEP_CFG 0x06
+#define MMA9551_SLEEP_CFG_SNCEN BIT(0)
+#define MMA9551_SLEEP_CFG_SCHEN BIT(2)
+
+/* AFE application */
+#define MMA9551_AFE_X_ACCEL_REG 0x00
+#define MMA9551_AFE_Y_ACCEL_REG 0x02
+#define MMA9551_AFE_Z_ACCEL_REG 0x04
+
+/* Tilt application (inclination in IIO terms). */
+#define MMA9551_TILT_XZ_ANG_REG 0x00
+#define MMA9551_TILT_YZ_ANG_REG 0x01
+#define MMA9551_TILT_XY_ANG_REG 0x02
+#define MMA9551_TILT_XY_QUAD_SHIFT 0
+#define MMA9551_TILT_YZ_QUAD_SHIFT 2
+#define MMA9551_TILT_XZ_QUAD_SHIFT 4
+#define MMA9551_TILT_ANGFLG BIT(7)
+#define MMA9551_TILT_QUAD_REG 0x03
+#define MMA9551_TILT_CFG_REG 0x01
+#define MMA9551_TILT_ANG_THRESH_MASK 0x0f
+
+/* Tilt events are mapped to the first three GPIO pins. */
+enum mma9551_tilt_axis {
+ mma9551_x = 0,
+ mma9551_y,
+ mma9551_z,
+};
+
+/*
+ * A response is composed of:
+ * - control registers: MB0-3
+ * - data registers: MB4-31
+ *
+ * A request is composed of:
+ * - mbox to write to (always 0)
+ * - control registers: MB1-4
+ * - data registers: MB5-31
+ */
+#define MMA9551_MAILBOX_CTRL_REGS 4
+#define MMA9551_MAX_MAILBOX_DATA_REGS 28
+#define MMA9551_MAILBOX_REGS 32
+
+#define MMA9551_I2C_READ_RETRIES 5
+#define MMA9551_I2C_READ_DELAY 50 /* us */
+
+struct mma9551_mbox_request {
+ u8 start_mbox; /* Always 0. */
+ u8 app_id;
+ /*
+ * See Section 5.3.1 of the MMA955xL Software Reference Manual.
+ *
+ * Bit 7: reserved, always 0
+ * Bits 6-4: command
+ * Bits 3-0: upper bits of register offset
+ */
+ u8 cmd_off;
+ u8 lower_off;
+ u8 nbytes;
+ u8 buf[MMA9551_MAX_MAILBOX_DATA_REGS - 1];
+} __packed;
+
+struct mma9551_mbox_response {
+ u8 app_id;
+ /*
+ * See Section 5.3.3 of the MMA955xL Software Reference Manual.
+ *
+ * Bit 7: COCO
+ * Bits 6-0: Error code.
+ */
+ u8 coco_err;
+ u8 nbytes;
+ u8 req_bytes;
+ u8 buf[MMA9551_MAX_MAILBOX_DATA_REGS];
+} __packed;
+
+struct mma9551_version_info {
+ __be32 device_id;
+ u8 rom_version[2];
+ u8 fw_version[2];
+ u8 hw_version[2];
+ u8 fw_build[2];
+};
+
+struct mma9551_data {
+ struct i2c_client *client;
+ struct mutex mutex;
+ int event_enabled[3];
+ int irqs[MMA9551_GPIO_COUNT];
+};
+
+static int mma9551_transfer(struct i2c_client *client,
+ u8 app_id, u8 command, u16 offset,
+ u8 *inbytes, int num_inbytes,
+ u8 *outbytes, int num_outbytes)
+{
+ struct mma9551_mbox_request req;
+ struct mma9551_mbox_response rsp;
+ struct i2c_msg in, out;
+ u8 req_len, err_code;
+ int ret;
+ int retries;
+
+ if (offset >= 1 << 12) {
+ dev_err(&client->dev, "register offset too large\n");
+ return -EINVAL;
+ }
+
+ req_len = 1 + MMA9551_MAILBOX_CTRL_REGS + num_inbytes;
+ req.start_mbox = 0;
+ req.app_id = app_id;
+ req.cmd_off = command | (offset >> 8);
+ req.lower_off = offset;
+
+ if (command == MMA9551_CMD_WRITE_CONFIG)
+ req.nbytes = num_inbytes;
+ else
+ req.nbytes = num_outbytes;
+ if (num_inbytes)
+ memcpy(req.buf, inbytes, num_inbytes);
+
+ out.addr = client->addr;
+ out.flags = 0;
+ out.len = req_len;
+ out.buf = (u8 *)&req;
+
+ ret = i2c_transfer(client->adapter, &out, 1);
+ if (ret < 0) {
+ dev_err(&client->dev, "i2c write failed\n");
+ return ret;
+ }
+
+ retries = MMA9551_I2C_READ_RETRIES;
+ do {
+ udelay(MMA9551_I2C_READ_DELAY);
+
+ in.addr = client->addr;
+ in.flags = I2C_M_RD;
+ in.len = sizeof(rsp);
+ in.buf = (u8 *)&rsp;
+
+ ret = i2c_transfer(client->adapter, &in, 1);
+ if (ret < 0) {
+ dev_err(&client->dev, "i2c read failed\n");
+ return ret;
+ }
+
+ if (rsp.coco_err & MMA9551_RESPONSE_COCO)
+ break;
+ } while (--retries > 0);
+
+ if (retries == 0) {
+ dev_err(&client->dev,
+ "timed out while waiting for command response\n");
+ return -ETIMEDOUT;
+ }
+
+ if (rsp.app_id != app_id) {
+ dev_err(&client->dev,
+ "app_id mismatch in response got %02x expected %02x\n",
+ rsp.app_id, app_id);
+ return -EINVAL;
+ }
+
+ err_code = rsp.coco_err & ~MMA9551_RESPONSE_COCO;
+ if (err_code != MMA9551_MCI_ERROR_NONE) {
+ dev_err(&client->dev, "read returned error %x\n", err_code);
+ return -EINVAL;
+ }
+
+ if (rsp.nbytes != rsp.req_bytes) {
+ dev_err(&client->dev,
+ "output length mismatch got %d expected %d\n",
+ rsp.nbytes, num_outbytes);
+ return -EINVAL;
+ }
+
+ if (num_outbytes)
+ memcpy(outbytes, rsp.buf, num_outbytes);
+
+ return 0;
+}
+
+static int mma9551_read_config_byte(struct i2c_client *client, u8 app_id,
+ u16 reg, u8 *val)
+{
+ return mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
+ reg, NULL, 0, val, 1);
+}
+
+static int mma9551_write_config_byte(struct i2c_client *client, u8 app_id,
+ u16 reg, u8 val)
+{
+ return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG, reg,
+ &val, 1, NULL, 0);
+}
+
+static int mma9551_read_status_byte(struct i2c_client *client, u8 app_id,
+ u16 reg, u8 *val)
+{
+ return mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
+ reg, NULL, 0, val, 1);
+}
+
+static int mma9551_read_status_word(struct i2c_client *client, u8 app_id,
+ u16 reg, u16 *val)
+{
+ int ret;
+ __be16 v;
+
+ ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
+ reg, NULL, 0, (u8 *)&v, 2);
+ *val = be16_to_cpu(v);
+
+ return ret;
+}
+
+static int mma9551_update_config_bits(struct i2c_client *client, u8 app_id,
+ u8 reg, u8 mask, u8 val)
+{
+ int ret;
+ u8 tmp, orig;
+
+ ret = mma9551_read_config_byte(client, app_id, reg, &orig);
+ if (ret < 0)
+ return ret;
+
+ tmp = orig & ~mask;
+ tmp |= val & mask;
+
+ if (tmp == orig)
+ return 0;
+
+ return mma9551_write_config_byte(client, app_id, reg, tmp);
+}
+
+/*
+ * The polarity parameter is described in section 6.2.2, page 66, of the
+ * Software Reference Manual. Basically, polarity=0 means the interrupt
+ * line has the same value as the selected bit, while polarity=1 means
+ * the line is inverted.
+ */
+static int mma9551_gpio_config(struct i2c_client *client,
+ enum mma9551_gpio_pin pin,
+ u8 app_id, u8 bitnum, int polarity)
+{
+ u8 reg, pol_mask, pol_val;
+ int ret;
+
+ if (pin > mma9551_gpio_max) {
+ dev_err(&client->dev, "bad GPIO pin\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Pin 6 is configured by regs 0x00 and 0x01, pin 7 by 0x02 and
+ * 0x03, and so on.
+ */
+ reg = pin * 2;
+
+ ret = mma9551_write_config_byte(client, MMA9551_APPID_GPIO,
+ reg, app_id);
+ if (ret < 0) {
+ dev_err(&client->dev, "error setting GPIO app_id\n");
+ return ret;
+ }
+
+ ret = mma9551_write_config_byte(client, MMA9551_APPID_GPIO,
+ reg + 1, bitnum);
+ if (ret < 0) {
+ dev_err(&client->dev, "error setting GPIO bit number\n");
+ return ret;
+ }
+
+ switch (pin) {
+ case mma9551_gpio6:
+ reg = MMA9551_GPIO_POL_LSB;
+ pol_mask = 1 << 6;
+ break;
+ case mma9551_gpio7:
+ reg = MMA9551_GPIO_POL_LSB;
+ pol_mask = 1 << 7;
+ break;
+ case mma9551_gpio8:
+ reg = MMA9551_GPIO_POL_MSB;
+ pol_mask = 1 << 0;
+ break;
+ case mma9551_gpio9:
+ reg = MMA9551_GPIO_POL_MSB;
+ pol_mask = 1 << 1;
+ break;
+ }
+ pol_val = polarity ? pol_mask : 0;
+
+ ret = mma9551_update_config_bits(client, MMA9551_APPID_GPIO, reg,
+ pol_mask, pol_val);
+ if (ret < 0)
+ dev_err(&client->dev, "error setting GPIO polarity\n");
+
+ return ret;
+}
+
+static int mma9551_read_version(struct i2c_client *client)
+{
+ struct mma9551_version_info info;
+ int ret;
+
+ ret = mma9551_transfer(client, MMA9551_APPID_VERSION, 0x00, 0x00,
+ NULL, 0, (u8 *)&info, sizeof(info));
+ if (ret < 0)
+ return ret;
+
+ dev_info(&client->dev, "Device ID 0x%x, firmware version %02x.%02x\n",
+ be32_to_cpu(info.device_id), info.fw_version[0],
+ info.fw_version[1]);
+
+ return 0;
+}
+
+/*
+ * Use 'false' as the second parameter to cause the device to enter
+ * sleep.
+ */
+static int mma9551_set_device_state(struct i2c_client *client,
+ bool enable)
+{
+ return mma9551_update_config_bits(client, MMA9551_APPID_SLEEP_WAKE,
+ MMA9551_SLEEP_CFG,
+ MMA9551_SLEEP_CFG_SNCEN,
+ enable ? 0 : MMA9551_SLEEP_CFG_SNCEN);
+}
+
+static int mma9551_read_incli_chan(struct i2c_client *client,
+ const struct iio_chan_spec *chan,
+ int *val)
+{
+ u8 quad_shift, quad_mask, angle, quadrant;
+ u16 reg_addr;
+ int ret;
+
+ switch (chan->channel2) {
+ case IIO_MOD_X:
+ reg_addr = MMA9551_TILT_YZ_ANG_REG;
+ quad_shift = MMA9551_TILT_YZ_QUAD_SHIFT;
+ break;
+ case IIO_MOD_Y:
+ reg_addr = MMA9551_TILT_XZ_ANG_REG;
+ quad_shift = MMA9551_TILT_XZ_QUAD_SHIFT;
+ break;
+ case IIO_MOD_Z:
+ reg_addr = MMA9551_TILT_XY_ANG_REG;
+ quad_shift = MMA9551_TILT_XY_QUAD_SHIFT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = mma9551_read_status_byte(client, MMA9551_APPID_TILT,
+ reg_addr, &angle);
+ if (ret < 0)
+ return ret;
+
+ ret = mma9551_read_status_byte(client, MMA9551_APPID_TILT,
+ MMA9551_TILT_QUAD_REG, &quadrant);
+ if (ret < 0)
+ return ret;
+
+ angle &= ~MMA9551_TILT_ANGFLG;
+ quad_mask = 0x03 << quad_shift;
+ quadrant = (quadrant & quad_mask) >> quad_shift;
+
+ if (quadrant == 1 || quadrant == 3)
+ *val = 90 * (quadrant + 1) - angle;
+ else
+ *val = angle + 90 * quadrant;
+
+ return IIO_VAL_INT;
+}
+
+static int mma9551_read_accel_chan(struct i2c_client *client,
+ const struct iio_chan_spec *chan,
+ int *val, int *val2)
+{
+ u16 reg_addr;
+ s16 raw_accel;
+ int ret;
+
+ switch (chan->channel2) {
+ case IIO_MOD_X:
+ reg_addr = MMA9551_AFE_X_ACCEL_REG;
+ break;
+ case IIO_MOD_Y:
+ reg_addr = MMA9551_AFE_Y_ACCEL_REG;
+ break;
+ case IIO_MOD_Z:
+ reg_addr = MMA9551_AFE_Z_ACCEL_REG;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = mma9551_read_status_word(client, MMA9551_APPID_AFE,
+ reg_addr, &raw_accel);
+ if (ret < 0)
+ return ret;
+
+ *val = raw_accel;
+
+ return IIO_VAL_INT;
+}
+
+static int mma9551_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct mma9551_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_PROCESSED:
+ switch (chan->type) {
+ case IIO_INCLI:
+ mutex_lock(&data->mutex);
+ ret = mma9551_read_incli_chan(data->client,
+ chan, val);
+ mutex_unlock(&data->mutex);
+ return ret;
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_RAW:
+ switch (chan->type) {
+ case IIO_ACCEL:
+ mutex_lock(&data->mutex);
+ ret = mma9551_read_accel_chan(data->client,
+ chan, val, val2);
+ mutex_unlock(&data->mutex);
+ return ret;
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_ACCEL:
+ *val = 0;
+ *val2 = 2440;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mma9551_read_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir)
+{
+ struct mma9551_data *data = iio_priv(indio_dev);
+
+ switch (chan->type) {
+ case IIO_INCLI:
+ /* IIO counts axes from 1, because IIO_NO_MOD is 0. */
+ return data->event_enabled[chan->channel2 - 1];
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mma9551_config_incli_event(struct iio_dev *indio_dev,
+ enum iio_modifier axis,
+ int state)
+{
+ struct mma9551_data *data = iio_priv(indio_dev);
+ enum mma9551_tilt_axis mma_axis;
+ int ret;
+
+ /* IIO counts axes from 1, because IIO_NO_MOD is 0. */
+ mma_axis = axis - 1;
+
+ if (data->event_enabled[mma_axis] == state)
+ return 0;
+
+ if (state == 0) {
+ ret = mma9551_gpio_config(data->client, mma_axis,
+ MMA9551_APPID_NONE, 0, 0);
+ if (ret < 0)
+ return ret;
+ } else {
+ int bitnum;
+
+ /* Bit 7 of each angle register holds the angle flag. */
+ switch (axis) {
+ case IIO_MOD_X:
+ bitnum = 7 + 8 * MMA9551_TILT_YZ_ANG_REG;
+ break;
+ case IIO_MOD_Y:
+ bitnum = 7 + 8 * MMA9551_TILT_XZ_ANG_REG;
+ break;
+ case IIO_MOD_Z:
+ bitnum = 7 + 8 * MMA9551_TILT_XY_ANG_REG;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = mma9551_gpio_config(data->client, mma_axis,
+ MMA9551_APPID_TILT, bitnum, 0);
+ if (ret < 0)
+ return ret;
+ }
+
+ data->event_enabled[mma_axis] = state;
+
+ return ret;
+}
+
+static int mma9551_write_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ int state)
+{
+ struct mma9551_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (chan->type) {
+ case IIO_INCLI:
+ mutex_lock(&data->mutex);
+ ret = mma9551_config_incli_event(indio_dev,
+ chan->channel2, state);
+ mutex_unlock(&data->mutex);
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mma9551_write_event_value(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int val, int val2)
+{
+ struct mma9551_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (chan->type) {
+ case IIO_INCLI:
+ if (val2 != 0 || val < 1 || val > 10)
+ return -EINVAL;
+ mutex_lock(&data->mutex);
+ ret = mma9551_update_config_bits(data->client,
+ MMA9551_APPID_TILT,
+ MMA9551_TILT_CFG_REG,
+ MMA9551_TILT_ANG_THRESH_MASK,
+ val);
+ mutex_unlock(&data->mutex);
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mma9551_read_event_value(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int *val, int *val2)
+{
+ struct mma9551_data *data = iio_priv(indio_dev);
+ int ret;
+ u8 tmp;
+
+ switch (chan->type) {
+ case IIO_INCLI:
+ mutex_lock(&data->mutex);
+ ret = mma9551_read_config_byte(data->client,
+ MMA9551_APPID_TILT,
+ MMA9551_TILT_CFG_REG, &tmp);
+ mutex_unlock(&data->mutex);
+ if (ret < 0)
+ return ret;
+ *val = tmp & MMA9551_TILT_ANG_THRESH_MASK;
+ *val2 = 0;
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_event_spec mma9551_incli_event = {
+ .type = IIO_EV_TYPE_ROC,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_separate = BIT(IIO_EV_INFO_ENABLE),
+ .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE),
+};
+
+#define MMA9551_ACCEL_CHANNEL(axis) { \
+ .type = IIO_ACCEL, \
+ .modified = 1, \
+ .channel2 = axis, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+}
+
+#define MMA9551_INCLI_CHANNEL(axis) { \
+ .type = IIO_INCLI, \
+ .modified = 1, \
+ .channel2 = axis, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
+ .event_spec = &mma9551_incli_event, \
+ .num_event_specs = 1, \
+}
+
+static const struct iio_chan_spec mma9551_channels[] = {
+ MMA9551_ACCEL_CHANNEL(IIO_MOD_X),
+ MMA9551_ACCEL_CHANNEL(IIO_MOD_Y),
+ MMA9551_ACCEL_CHANNEL(IIO_MOD_Z),
+
+ MMA9551_INCLI_CHANNEL(IIO_MOD_X),
+ MMA9551_INCLI_CHANNEL(IIO_MOD_Y),
+ MMA9551_INCLI_CHANNEL(IIO_MOD_Z),
+};
+
+static const struct iio_info mma9551_info = {
+ .driver_module = THIS_MODULE,
+ .read_raw = mma9551_read_raw,
+ .read_event_config = mma9551_read_event_config,
+ .write_event_config = mma9551_write_event_config,
+ .read_event_value = mma9551_read_event_value,
+ .write_event_value = mma9551_write_event_value,
+};
+
+static irqreturn_t mma9551_event_handler(int irq, void *private)
+{
+ struct iio_dev *indio_dev = private;
+ struct mma9551_data *data = iio_priv(indio_dev);
+ int mma_axis, ret, i;
+ u16 reg;
+ u8 val;
+
+ mutex_lock(&data->mutex);
+
+ mma_axis = -1;
+ for (i = 0; i < 3; i++)
+ if (irq == data->irqs[i]) {
+ mma_axis = i;
+ break;
+ }
+
+ if (mma_axis == -1) {
+ /* IRQ was triggered on 4th line, which we don't use. */
+ dev_warn(&data->client->dev,
+ "irq triggered on unused line %d\n", data->irqs[3]);
+ goto out;
+ }
+
+ switch (mma_axis) {
+ case mma9551_x:
+ reg = MMA9551_TILT_YZ_ANG_REG;
+ break;
+ case mma9551_y:
+ reg = MMA9551_TILT_XZ_ANG_REG;
+ break;
+ case mma9551_z:
+ reg = MMA9551_TILT_XY_ANG_REG;
+ break;
+ }
+
+ /*
+ * Read the angle even though we don't use it, otherwise we
+ * won't get any further interrupts.
+ */
+ ret = mma9551_read_status_byte(data->client, MMA9551_APPID_TILT,
+ reg, &val);
+ if (ret < 0) {
+ dev_err(&data->client->dev,
+ "error %d reading tilt register in IRQ\n", ret);
+ goto out;
+ }
+
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_INCLI, 0,
+ (mma_axis + 1),
+ IIO_EV_TYPE_ROC,
+ IIO_EV_DIR_RISING),
+ iio_get_time_ns());
+
+out:
+ mutex_unlock(&data->mutex);
+
+ return IRQ_HANDLED;
+}
+
+static int mma9551_init(struct mma9551_data *data)
+{
+ int ret;
+
+ ret = mma9551_read_version(data->client);
+ if (ret)
+ return ret;
+
+ /* Power on chip and enable doze mode. */
+ ret = mma9551_update_config_bits(data->client,
+ MMA9551_APPID_SLEEP_WAKE,
+ MMA9551_SLEEP_CFG,
+ MMA9551_SLEEP_CFG_SCHEN | MMA9551_SLEEP_CFG_SNCEN,
+ MMA9551_SLEEP_CFG_SCHEN);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int mma9551_gpio_probe(struct iio_dev *indio_dev)
+{
+ struct gpio_desc *gpio;
+ int i, ret;
+ struct mma9551_data *data = iio_priv(indio_dev);
+ struct device *dev = &data->client->dev;
+
+ for (i = 0; i < MMA9551_GPIO_COUNT; i++) {
+ gpio = devm_gpiod_get_index(dev, MMA9551_GPIO_NAME, i);
+ if (IS_ERR(gpio)) {
+ dev_err(dev, "acpi gpio get index failed\n");
+ return PTR_ERR(gpio);
+ }
+
+ ret = gpiod_direction_input(gpio);
+ if (ret)
+ return ret;
+
+ data->irqs[i] = gpiod_to_irq(gpio);
+ ret = devm_request_threaded_irq(dev, data->irqs[i],
+ NULL, mma9551_event_handler,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ MMA9551_IRQ_NAME, indio_dev);
+ if (ret < 0) {
+ dev_err(dev, "request irq %d failed\n", data->irqs[i]);
+ return ret;
+ }
+
+ dev_dbg(dev, "gpio resource, no:%d irq:%d\n",
+ desc_to_gpio(gpio), data->irqs[i]);
+ }
+
+ return 0;
+}
+
+static const char *mma9551_match_acpi_device(struct device *dev)
+{
+ const struct acpi_device_id *id;
+
+ id = acpi_match_device(dev->driver->acpi_match_table, dev);
+ if (!id)
+ return NULL;
+
+ return dev_name(dev);
+}
+
+static int mma9551_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct mma9551_data *data;
+ struct iio_dev *indio_dev;
+ const char *name = NULL;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+
+ if (id)
+ name = id->name;
+ else if (ACPI_HANDLE(&client->dev))
+ name = mma9551_match_acpi_device(&client->dev);
+
+ ret = mma9551_init(data);
+ if (ret < 0)
+ return ret;
+
+ mutex_init(&data->mutex);
+ memset(data->event_enabled, 0, sizeof(data->event_enabled));
+
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->channels = mma9551_channels;
+ indio_dev->num_channels = ARRAY_SIZE(mma9551_channels);
+ indio_dev->name = name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &mma9551_info;
+
+ ret = mma9551_gpio_probe(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ ret = iio_device_register(indio_dev);
+ if (ret < 0) {
+ dev_err(&client->dev, "unable to register iio device\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int mma9551_remove(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct mma9551_data *data = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+ mutex_lock(&data->mutex);
+ mma9551_set_device_state(data->client, false);
+ mutex_unlock(&data->mutex);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mma9551_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+ struct mma9551_data *data = iio_priv(indio_dev);
+
+ mutex_lock(&data->mutex);
+ mma9551_set_device_state(data->client, false);
+ mutex_unlock(&data->mutex);
+
+ return 0;
+}
+
+static int mma9551_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+ struct mma9551_data *data = iio_priv(indio_dev);
+
+ mutex_lock(&data->mutex);
+ mma9551_set_device_state(data->client, true);
+ mutex_unlock(&data->mutex);
+
+ return 0;
+}
+#else
+#define mma9551_suspend NULL
+#define mma9551_resume NULL
+#endif
+
+static const struct dev_pm_ops mma9551_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(mma9551_suspend, mma9551_resume)
+};
+
+static const struct acpi_device_id mma9551_acpi_match[] = {
+ {"MMA9551", 0},
+ {},
+};
+
+MODULE_DEVICE_TABLE(acpi, mma9551_acpi_match);
+
+static const struct i2c_device_id mma9551_id[] = {
+ {"mma9551", 0},
+ {}
+};
+
+MODULE_DEVICE_TABLE(i2c, mma9551_id);
+
+static struct i2c_driver mma9551_driver = {
+ .driver = {
+ .name = MMA9551_DRV_NAME,
+ .acpi_match_table = ACPI_PTR(mma9551_acpi_match),
+ .pm = &mma9551_pm_ops,
+ },
+ .probe = mma9551_probe,
+ .remove = mma9551_remove,
+ .id_table = mma9551_id,
+};
+
+module_i2c_driver(mma9551_driver);
+
+MODULE_AUTHOR("Irina Tirdea <irina.tirdea@intel.com>");
+MODULE_AUTHOR("Vlad Dogaru <vlad.dogaru@intel.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MMA9551L motion-sensing platform driver");
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] iio: add driver for Freescale MMA9551L
2014-11-17 15:03 [PATCH v3] iio: add driver for Freescale MMA9551L Vlad Dogaru
@ 2014-11-23 10:58 ` Hartmut Knaack
2014-11-24 9:22 ` Vlad Dogaru
0 siblings, 1 reply; 3+ messages in thread
From: Hartmut Knaack @ 2014-11-23 10:58 UTC (permalink / raw)
To: Vlad Dogaru, linux-iio; +Cc: Irina Tirdea
Vlad Dogaru schrieb am 17.11.2014 16:03:
> Add support for Freescale MMA9551L Intelligent Motion-Sensing Platform.
> The driver supports raw reads for acceleration and inclination, as well
> as configuring an inclination rate-of-change events. The events can be
> used similarly to an Android sensor Tilt event.
>
> The specifications can be downloaded from:
> http://www.freescale.com/files/sensors/doc/ref_manual/MMA955xLSWRM.pdf
>
Looks very good. Just a few minor cleanup recommendations inline.
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Signed-off-by: Vlad Dogaru <vlad.dogaru@intel.com>
> ---
> Changes since v2:
> - all three events can be enabled at the same time, each using a separate
> GPIO line.
> - simplify GPIO config code.
> - style fixes as suggested by Jonathan.
>
> Changes since v1:
> - invert logic in mma9551_set_device_state(). Setting the sleep bit to 1 puts
> the device into sleep.
> - do not use scan_index for channels, since we don't support buffering.
> - style fixes as suggested by Peter.
>
> drivers/iio/accel/Kconfig | 10 +
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/mma9551.c | 960 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 971 insertions(+)
> create mode 100644 drivers/iio/accel/mma9551.c
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 9b9be87..d80616d 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -105,4 +105,14 @@ config KXCJK1013
> To compile this driver as a module, choose M here: the module will
> be called kxcjk-1013.
>
> +config MMA9551
> + tristate "Freescale MMA9551L Intelligent Motion-Sensing Platform Driver"
> + depends on I2C
> + help
> + Say yes here to build support for the Freescale MMA9551L
> + Intelligent Motion-Sensing Platform Driver.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called mma9551.
> +
> endmenu
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index a593996..de5b9cb 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
> obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
> obj-$(CONFIG_KXSD9) += kxsd9.o
> obj-$(CONFIG_MMA8452) += mma8452.o
> +obj-$(CONFIG_MMA9551) += mma9551.o
>
> obj-$(CONFIG_IIO_ST_ACCEL_3AXIS) += st_accel.o
> st_accel-y := st_accel_core.o
> diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
> new file mode 100644
> index 0000000..d677042
> --- /dev/null
> +++ b/drivers/iio/accel/mma9551.c
> @@ -0,0 +1,960 @@
> +/*
> + * Freescale MMA9551L Intelligent Motion-Sensing Platform driver
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/slab.h>
> +#include <linux/acpi.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
> +
> +#define MMA9551_DRV_NAME "mma9551"
> +#define MMA9551_IRQ_NAME "mma9551_event"
> +#define MMA9551_GPIO_NAME "mma9551_int"
> +#define MMA9551_GPIO_COUNT 4
> +
> +/* Applications IDs */
> +#define MMA9551_APPID_VERSION 0x00
> +#define MMA9551_APPID_GPIO 0x03
> +#define MMA9551_APPID_AFE 0x06
> +#define MMA9551_APPID_TILT 0x0B
> +#define MMA9551_APPID_SLEEP_WAKE 0x12
> +#define MMA9551_APPID_RESET 0x17
> +#define MMA9551_APPID_NONE 0xff
> +
> +/* Command masks for mailbox write command */
> +#define MMA9551_CMD_READ_VERSION_INFO 0x00
> +#define MMA9551_CMD_READ_CONFIG 0x10
> +#define MMA9551_CMD_WRITE_CONFIG 0x20
> +#define MMA9551_CMD_READ_STATUS 0x30
> +
> +enum mma9551_gpio_pin {
> + mma9551_gpio6 = 0,
> + mma9551_gpio7,
> + mma9551_gpio8,
> + mma9551_gpio9,
> + mma9551_gpio_max = mma9551_gpio9,
> +};
> +
> +/* Mailbox read command */
> +#define MMA9551_RESPONSE_COCO BIT(7)
> +
> +/* Error-Status codes returned in mailbox read command */
> +#define MMA9551_MCI_ERROR_NONE 0x00
> +#define MMA9551_MCI_ERROR_PARAM 0x04
> +#define MMA9551_MCI_INVALID_COUNT 0x19
> +#define MMA9551_MCI_ERROR_COMMAND 0x1C
> +#define MMA9551_MCI_ERROR_INVALID_LENGTH 0x21
> +#define MMA9551_MCI_ERROR_FIFO_BUSY 0x22
> +#define MMA9551_MCI_ERROR_FIFO_ALLOCATED 0x23
> +#define MMA9551_MCI_ERROR_FIFO_OVERSIZE 0x24
> +
> +/* GPIO Application */
> +#define MMA9551_GPIO_POL_MSB 0x08
> +#define MMA9551_GPIO_POL_LSB 0x09
> +
> +/* Sleep/Wake application */
> +#define MMA9551_SLEEP_CFG 0x06
> +#define MMA9551_SLEEP_CFG_SNCEN BIT(0)
> +#define MMA9551_SLEEP_CFG_SCHEN BIT(2)
> +
> +/* AFE application */
> +#define MMA9551_AFE_X_ACCEL_REG 0x00
> +#define MMA9551_AFE_Y_ACCEL_REG 0x02
> +#define MMA9551_AFE_Z_ACCEL_REG 0x04
> +
> +/* Tilt application (inclination in IIO terms). */
> +#define MMA9551_TILT_XZ_ANG_REG 0x00
> +#define MMA9551_TILT_YZ_ANG_REG 0x01
> +#define MMA9551_TILT_XY_ANG_REG 0x02
> +#define MMA9551_TILT_XY_QUAD_SHIFT 0
> +#define MMA9551_TILT_YZ_QUAD_SHIFT 2
> +#define MMA9551_TILT_XZ_QUAD_SHIFT 4
> +#define MMA9551_TILT_ANGFLG BIT(7)
This flag is part of the angular registers, so better move it up there.
> +#define MMA9551_TILT_QUAD_REG 0x03
This line should be put on top of the _QUAD_SHIFTs.
> +#define MMA9551_TILT_CFG_REG 0x01
> +#define MMA9551_TILT_ANG_THRESH_MASK 0x0f
This may be represented using GENMASK - just a matter of taste.
> +
> +/* Tilt events are mapped to the first three GPIO pins. */
> +enum mma9551_tilt_axis {
> + mma9551_x = 0,
> + mma9551_y,
> + mma9551_z,
> +};
> +
> +/*
> + * A response is composed of:
> + * - control registers: MB0-3
> + * - data registers: MB4-31
> + *
> + * A request is composed of:
> + * - mbox to write to (always 0)
> + * - control registers: MB1-4
> + * - data registers: MB5-31
> + */
> +#define MMA9551_MAILBOX_CTRL_REGS 4
> +#define MMA9551_MAX_MAILBOX_DATA_REGS 28
> +#define MMA9551_MAILBOX_REGS 32
> +
> +#define MMA9551_I2C_READ_RETRIES 5
> +#define MMA9551_I2C_READ_DELAY 50 /* us */
> +
> +struct mma9551_mbox_request {
> + u8 start_mbox; /* Always 0. */
> + u8 app_id;
> + /*
> + * See Section 5.3.1 of the MMA955xL Software Reference Manual.
> + *
> + * Bit 7: reserved, always 0
> + * Bits 6-4: command
> + * Bits 3-0: upper bits of register offset
> + */
> + u8 cmd_off;
> + u8 lower_off;
> + u8 nbytes;
> + u8 buf[MMA9551_MAX_MAILBOX_DATA_REGS - 1];
> +} __packed;
> +
> +struct mma9551_mbox_response {
> + u8 app_id;
> + /*
> + * See Section 5.3.3 of the MMA955xL Software Reference Manual.
> + *
> + * Bit 7: COCO
> + * Bits 6-0: Error code.
> + */
> + u8 coco_err;
> + u8 nbytes;
> + u8 req_bytes;
> + u8 buf[MMA9551_MAX_MAILBOX_DATA_REGS];
> +} __packed;
> +
> +struct mma9551_version_info {
> + __be32 device_id;
> + u8 rom_version[2];
> + u8 fw_version[2];
> + u8 hw_version[2];
> + u8 fw_build[2];
> +};
> +
> +struct mma9551_data {
> + struct i2c_client *client;
> + struct mutex mutex;
> + int event_enabled[3];
> + int irqs[MMA9551_GPIO_COUNT];
> +};
> +
> +static int mma9551_transfer(struct i2c_client *client,
> + u8 app_id, u8 command, u16 offset,
> + u8 *inbytes, int num_inbytes,
> + u8 *outbytes, int num_outbytes)
> +{
> + struct mma9551_mbox_request req;
> + struct mma9551_mbox_response rsp;
> + struct i2c_msg in, out;
> + u8 req_len, err_code;
> + int ret;
> + int retries;
These two int definitions could be consolidated in one line.
> +
> + if (offset >= 1 << 12) {
> + dev_err(&client->dev, "register offset too large\n");
> + return -EINVAL;
> + }
> +
> + req_len = 1 + MMA9551_MAILBOX_CTRL_REGS + num_inbytes;
> + req.start_mbox = 0;
> + req.app_id = app_id;
> + req.cmd_off = command | (offset >> 8);
> + req.lower_off = offset;
> +
> + if (command == MMA9551_CMD_WRITE_CONFIG)
> + req.nbytes = num_inbytes;
> + else
> + req.nbytes = num_outbytes;
> + if (num_inbytes)
> + memcpy(req.buf, inbytes, num_inbytes);
> +
> + out.addr = client->addr;
> + out.flags = 0;
> + out.len = req_len;
> + out.buf = (u8 *)&req;
> +
> + ret = i2c_transfer(client->adapter, &out, 1);
> + if (ret < 0) {
> + dev_err(&client->dev, "i2c write failed\n");
> + return ret;
> + }
> +
> + retries = MMA9551_I2C_READ_RETRIES;
> + do {
> + udelay(MMA9551_I2C_READ_DELAY);
> +
> + in.addr = client->addr;
> + in.flags = I2C_M_RD;
> + in.len = sizeof(rsp);
> + in.buf = (u8 *)&rsp;
> +
> + ret = i2c_transfer(client->adapter, &in, 1);
> + if (ret < 0) {
> + dev_err(&client->dev, "i2c read failed\n");
> + return ret;
> + }
> +
> + if (rsp.coco_err & MMA9551_RESPONSE_COCO)
> + break;
> + } while (--retries > 0);
> +
> + if (retries == 0) {
> + dev_err(&client->dev,
> + "timed out while waiting for command response\n");
> + return -ETIMEDOUT;
> + }
> +
> + if (rsp.app_id != app_id) {
> + dev_err(&client->dev,
> + "app_id mismatch in response got %02x expected %02x\n",
> + rsp.app_id, app_id);
> + return -EINVAL;
> + }
> +
> + err_code = rsp.coco_err & ~MMA9551_RESPONSE_COCO;
> + if (err_code != MMA9551_MCI_ERROR_NONE) {
> + dev_err(&client->dev, "read returned error %x\n", err_code);
> + return -EINVAL;
> + }
> +
> + if (rsp.nbytes != rsp.req_bytes) {
> + dev_err(&client->dev,
> + "output length mismatch got %d expected %d\n",
> + rsp.nbytes, num_outbytes);
Don't you mean rsp.req_bytes instead of num_outbytes here?
> + return -EINVAL;
> + }
> +
> + if (num_outbytes)
> + memcpy(outbytes, rsp.buf, num_outbytes);
> +
> + return 0;
> +}
> +
> +static int mma9551_read_config_byte(struct i2c_client *client, u8 app_id,
> + u16 reg, u8 *val)
> +{
> + return mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
> + reg, NULL, 0, val, 1);
> +}
> +
> +static int mma9551_write_config_byte(struct i2c_client *client, u8 app_id,
> + u16 reg, u8 val)
> +{
> + return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG, reg,
> + &val, 1, NULL, 0);
> +}
> +
> +static int mma9551_read_status_byte(struct i2c_client *client, u8 app_id,
> + u16 reg, u8 *val)
> +{
> + return mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
> + reg, NULL, 0, val, 1);
> +}
> +
> +static int mma9551_read_status_word(struct i2c_client *client, u8 app_id,
> + u16 reg, u16 *val)
> +{
> + int ret;
> + __be16 v;
> +
> + ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
> + reg, NULL, 0, (u8 *)&v, 2);
> + *val = be16_to_cpu(v);
> +
> + return ret;
> +}
> +
> +static int mma9551_update_config_bits(struct i2c_client *client, u8 app_id,
> + u8 reg, u8 mask, u8 val)
u16 was used for reg previously, would be more consistent to stick to it.
> +{
> + int ret;
> + u8 tmp, orig;
> +
> + ret = mma9551_read_config_byte(client, app_id, reg, &orig);
> + if (ret < 0)
> + return ret;
> +
> + tmp = orig & ~mask;
> + tmp |= val & mask;
> +
> + if (tmp == orig)
> + return 0;
> +
> + return mma9551_write_config_byte(client, app_id, reg, tmp);
> +}
> +
> +/*
> + * The polarity parameter is described in section 6.2.2, page 66, of the
> + * Software Reference Manual. Basically, polarity=0 means the interrupt
> + * line has the same value as the selected bit, while polarity=1 means
> + * the line is inverted.
> + */
> +static int mma9551_gpio_config(struct i2c_client *client,
> + enum mma9551_gpio_pin pin,
> + u8 app_id, u8 bitnum, int polarity)
> +{
> + u8 reg, pol_mask, pol_val;
> + int ret;
> +
> + if (pin > mma9551_gpio_max) {
> + dev_err(&client->dev, "bad GPIO pin\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Pin 6 is configured by regs 0x00 and 0x01, pin 7 by 0x02 and
> + * 0x03, and so on.
> + */
> + reg = pin * 2;
> +
> + ret = mma9551_write_config_byte(client, MMA9551_APPID_GPIO,
> + reg, app_id);
> + if (ret < 0) {
> + dev_err(&client->dev, "error setting GPIO app_id\n");
> + return ret;
> + }
> +
> + ret = mma9551_write_config_byte(client, MMA9551_APPID_GPIO,
> + reg + 1, bitnum);
> + if (ret < 0) {
> + dev_err(&client->dev, "error setting GPIO bit number\n");
> + return ret;
> + }
> +
> + switch (pin) {
> + case mma9551_gpio6:
> + reg = MMA9551_GPIO_POL_LSB;
> + pol_mask = 1 << 6;
> + break;
> + case mma9551_gpio7:
> + reg = MMA9551_GPIO_POL_LSB;
> + pol_mask = 1 << 7;
> + break;
> + case mma9551_gpio8:
> + reg = MMA9551_GPIO_POL_MSB;
> + pol_mask = 1 << 0;
> + break;
> + case mma9551_gpio9:
> + reg = MMA9551_GPIO_POL_MSB;
> + pol_mask = 1 << 1;
> + break;
> + }
> + pol_val = polarity ? pol_mask : 0;
> +
> + ret = mma9551_update_config_bits(client, MMA9551_APPID_GPIO, reg,
> + pol_mask, pol_val);
> + if (ret < 0)
> + dev_err(&client->dev, "error setting GPIO polarity\n");
> +
> + return ret;
> +}
> +
> +static int mma9551_read_version(struct i2c_client *client)
> +{
> + struct mma9551_version_info info;
> + int ret;
> +
> + ret = mma9551_transfer(client, MMA9551_APPID_VERSION, 0x00, 0x00,
> + NULL, 0, (u8 *)&info, sizeof(info));
> + if (ret < 0)
> + return ret;
> +
> + dev_info(&client->dev, "Device ID 0x%x, firmware version %02x.%02x\n",
> + be32_to_cpu(info.device_id), info.fw_version[0],
> + info.fw_version[1]);
> +
> + return 0;
> +}
> +
> +/*
> + * Use 'false' as the second parameter to cause the device to enter
> + * sleep.
> + */
> +static int mma9551_set_device_state(struct i2c_client *client,
> + bool enable)
> +{
> + return mma9551_update_config_bits(client, MMA9551_APPID_SLEEP_WAKE,
> + MMA9551_SLEEP_CFG,
> + MMA9551_SLEEP_CFG_SNCEN,
> + enable ? 0 : MMA9551_SLEEP_CFG_SNCEN);
This will fit exactly 80 chars, if indented the normal way.
> +}
> +
> +static int mma9551_read_incli_chan(struct i2c_client *client,
> + const struct iio_chan_spec *chan,
> + int *val)
> +{
> + u8 quad_shift, quad_mask, angle, quadrant;
> + u16 reg_addr;
> + int ret;
> +
> + switch (chan->channel2) {
> + case IIO_MOD_X:
> + reg_addr = MMA9551_TILT_YZ_ANG_REG;
> + quad_shift = MMA9551_TILT_YZ_QUAD_SHIFT;
> + break;
> + case IIO_MOD_Y:
> + reg_addr = MMA9551_TILT_XZ_ANG_REG;
> + quad_shift = MMA9551_TILT_XZ_QUAD_SHIFT;
> + break;
> + case IIO_MOD_Z:
> + reg_addr = MMA9551_TILT_XY_ANG_REG;
> + quad_shift = MMA9551_TILT_XY_QUAD_SHIFT;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = mma9551_read_status_byte(client, MMA9551_APPID_TILT,
> + reg_addr, &angle);
> + if (ret < 0)
> + return ret;
> +
> + ret = mma9551_read_status_byte(client, MMA9551_APPID_TILT,
> + MMA9551_TILT_QUAD_REG, &quadrant);
> + if (ret < 0)
> + return ret;
> +
> + angle &= ~MMA9551_TILT_ANGFLG;
> + quad_mask = 0x03 << quad_shift;
> + quadrant = (quadrant & quad_mask) >> quad_shift;
quad_mask is unnecessary. Just do:
quadrant = (quadrant >> quad_shift) & 0x03;
> +
> + if (quadrant == 1 || quadrant == 3)
> + *val = 90 * (quadrant + 1) - angle;
> + else
> + *val = angle + 90 * quadrant;
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int mma9551_read_accel_chan(struct i2c_client *client,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2)
> +{
> + u16 reg_addr;
> + s16 raw_accel;
> + int ret;
> +
> + switch (chan->channel2) {
> + case IIO_MOD_X:
> + reg_addr = MMA9551_AFE_X_ACCEL_REG;
> + break;
> + case IIO_MOD_Y:
> + reg_addr = MMA9551_AFE_Y_ACCEL_REG;
> + break;
> + case IIO_MOD_Z:
> + reg_addr = MMA9551_AFE_Z_ACCEL_REG;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = mma9551_read_status_word(client, MMA9551_APPID_AFE,
> + reg_addr, &raw_accel);
> + if (ret < 0)
> + return ret;
> +
> + *val = raw_accel;
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int mma9551_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct mma9551_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_PROCESSED:
> + switch (chan->type) {
> + case IIO_INCLI:
> + mutex_lock(&data->mutex);
> + ret = mma9551_read_incli_chan(data->client,
> + chan, val);
No need to wrap this line.
> + mutex_unlock(&data->mutex);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + mutex_lock(&data->mutex);
> + ret = mma9551_read_accel_chan(data->client,
> + chan, val, val2);
> + mutex_unlock(&data->mutex);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + *val = 0;
> + *val2 = 2440;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int mma9551_read_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir)
> +{
> + struct mma9551_data *data = iio_priv(indio_dev);
> +
> + switch (chan->type) {
> + case IIO_INCLI:
> + /* IIO counts axes from 1, because IIO_NO_MOD is 0. */
> + return data->event_enabled[chan->channel2 - 1];
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int mma9551_config_incli_event(struct iio_dev *indio_dev,
> + enum iio_modifier axis,
> + int state)
> +{
> + struct mma9551_data *data = iio_priv(indio_dev);
> + enum mma9551_tilt_axis mma_axis;
> + int ret;
> +
> + /* IIO counts axes from 1, because IIO_NO_MOD is 0. */
> + mma_axis = axis - 1;
> +
> + if (data->event_enabled[mma_axis] == state)
> + return 0;
> +
> + if (state == 0) {
> + ret = mma9551_gpio_config(data->client, mma_axis,
> + MMA9551_APPID_NONE, 0, 0);
> + if (ret < 0)
> + return ret;
> + } else {
> + int bitnum;
> +
> + /* Bit 7 of each angle register holds the angle flag. */
> + switch (axis) {
> + case IIO_MOD_X:
> + bitnum = 7 + 8 * MMA9551_TILT_YZ_ANG_REG;
> + break;
> + case IIO_MOD_Y:
> + bitnum = 7 + 8 * MMA9551_TILT_XZ_ANG_REG;
> + break;
> + case IIO_MOD_Z:
> + bitnum = 7 + 8 * MMA9551_TILT_XY_ANG_REG;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = mma9551_gpio_config(data->client, mma_axis,
> + MMA9551_APPID_TILT, bitnum, 0);
> + if (ret < 0)
> + return ret;
> + }
> +
> + data->event_enabled[mma_axis] = state;
> +
> + return ret;
> +}
> +
> +static int mma9551_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + int state)
> +{
> + struct mma9551_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (chan->type) {
> + case IIO_INCLI:
> + mutex_lock(&data->mutex);
> + ret = mma9551_config_incli_event(indio_dev,
> + chan->channel2, state);
> + mutex_unlock(&data->mutex);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int mma9551_write_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int val, int val2)
> +{
> + struct mma9551_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (chan->type) {
> + case IIO_INCLI:
> + if (val2 != 0 || val < 1 || val > 10)
> + return -EINVAL;
> + mutex_lock(&data->mutex);
> + ret = mma9551_update_config_bits(data->client,
> + MMA9551_APPID_TILT,
> + MMA9551_TILT_CFG_REG,
> + MMA9551_TILT_ANG_THRESH_MASK,
> + val);
> + mutex_unlock(&data->mutex);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int mma9551_read_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int *val, int *val2)
> +{
> + struct mma9551_data *data = iio_priv(indio_dev);
> + int ret;
> + u8 tmp;
> +
> + switch (chan->type) {
> + case IIO_INCLI:
> + mutex_lock(&data->mutex);
> + ret = mma9551_read_config_byte(data->client,
> + MMA9551_APPID_TILT,
> + MMA9551_TILT_CFG_REG, &tmp);
> + mutex_unlock(&data->mutex);
> + if (ret < 0)
> + return ret;
> + *val = tmp & MMA9551_TILT_ANG_THRESH_MASK;
> + *val2 = 0;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_event_spec mma9551_incli_event = {
> + .type = IIO_EV_TYPE_ROC,
> + .dir = IIO_EV_DIR_RISING,
> + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> + .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE),
> +};
> +
> +#define MMA9551_ACCEL_CHANNEL(axis) { \
> + .type = IIO_ACCEL, \
> + .modified = 1, \
> + .channel2 = axis, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +}
> +
> +#define MMA9551_INCLI_CHANNEL(axis) { \
> + .type = IIO_INCLI, \
> + .modified = 1, \
> + .channel2 = axis, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> + .event_spec = &mma9551_incli_event, \
> + .num_event_specs = 1, \
> +}
> +
> +static const struct iio_chan_spec mma9551_channels[] = {
> + MMA9551_ACCEL_CHANNEL(IIO_MOD_X),
> + MMA9551_ACCEL_CHANNEL(IIO_MOD_Y),
> + MMA9551_ACCEL_CHANNEL(IIO_MOD_Z),
> +
> + MMA9551_INCLI_CHANNEL(IIO_MOD_X),
> + MMA9551_INCLI_CHANNEL(IIO_MOD_Y),
> + MMA9551_INCLI_CHANNEL(IIO_MOD_Z),
> +};
> +
> +static const struct iio_info mma9551_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = mma9551_read_raw,
> + .read_event_config = mma9551_read_event_config,
> + .write_event_config = mma9551_write_event_config,
> + .read_event_value = mma9551_read_event_value,
> + .write_event_value = mma9551_write_event_value,
> +};
> +
> +static irqreturn_t mma9551_event_handler(int irq, void *private)
> +{
> + struct iio_dev *indio_dev = private;
> + struct mma9551_data *data = iio_priv(indio_dev);
> + int mma_axis, ret, i;
> + u16 reg;
> + u8 val;
> +
> + mutex_lock(&data->mutex);
> +
> + mma_axis = -1;
This value could be assigned already during declaration.
> + for (i = 0; i < 3; i++)
> + if (irq == data->irqs[i]) {
> + mma_axis = i;
> + break;
> + }
> +
> + if (mma_axis == -1) {
> + /* IRQ was triggered on 4th line, which we don't use. */
> + dev_warn(&data->client->dev,
> + "irq triggered on unused line %d\n", data->irqs[3]);
> + goto out;
> + }
> +
> + switch (mma_axis) {
> + case mma9551_x:
> + reg = MMA9551_TILT_YZ_ANG_REG;
> + break;
> + case mma9551_y:
> + reg = MMA9551_TILT_XZ_ANG_REG;
> + break;
> + case mma9551_z:
> + reg = MMA9551_TILT_XY_ANG_REG;
> + break;
> + }
> +
> + /*
> + * Read the angle even though we don't use it, otherwise we
> + * won't get any further interrupts.
> + */
> + ret = mma9551_read_status_byte(data->client, MMA9551_APPID_TILT,
> + reg, &val);
> + if (ret < 0) {
> + dev_err(&data->client->dev,
> + "error %d reading tilt register in IRQ\n", ret);
> + goto out;
> + }
> +
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_INCLI, 0,
> + (mma_axis + 1),
> + IIO_EV_TYPE_ROC,
> + IIO_EV_DIR_RISING),
Why not put more parameters in every line? Would save some space.
> + iio_get_time_ns());
> +
> +out:
> + mutex_unlock(&data->mutex);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int mma9551_init(struct mma9551_data *data)
> +{
> + int ret;
> +
> + ret = mma9551_read_version(data->client);
> + if (ret)
> + return ret;
> +
> + /* Power on chip and enable doze mode. */
> + ret = mma9551_update_config_bits(data->client,
> + MMA9551_APPID_SLEEP_WAKE,
> + MMA9551_SLEEP_CFG,
> + MMA9551_SLEEP_CFG_SCHEN | MMA9551_SLEEP_CFG_SNCEN,
> + MMA9551_SLEEP_CFG_SCHEN);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
Just return mma9551_update_config_bits().
> +}
> +
> +static int mma9551_gpio_probe(struct iio_dev *indio_dev)
> +{
> + struct gpio_desc *gpio;
> + int i, ret;
> + struct mma9551_data *data = iio_priv(indio_dev);
> + struct device *dev = &data->client->dev;
> +
> + for (i = 0; i < MMA9551_GPIO_COUNT; i++) {
> + gpio = devm_gpiod_get_index(dev, MMA9551_GPIO_NAME, i);
> + if (IS_ERR(gpio)) {
> + dev_err(dev, "acpi gpio get index failed\n");
> + return PTR_ERR(gpio);
> + }
> +
> + ret = gpiod_direction_input(gpio);
> + if (ret)
> + return ret;
> +
> + data->irqs[i] = gpiod_to_irq(gpio);
> + ret = devm_request_threaded_irq(dev, data->irqs[i],
> + NULL, mma9551_event_handler,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + MMA9551_IRQ_NAME, indio_dev);
> + if (ret < 0) {
> + dev_err(dev, "request irq %d failed\n", data->irqs[i]);
> + return ret;
> + }
> +
> + dev_dbg(dev, "gpio resource, no:%d irq:%d\n",
> + desc_to_gpio(gpio), data->irqs[i]);
> + }
> +
> + return 0;
> +}
> +
> +static const char *mma9551_match_acpi_device(struct device *dev)
> +{
> + const struct acpi_device_id *id;
> +
> + id = acpi_match_device(dev->driver->acpi_match_table, dev);
> + if (!id)
> + return NULL;
> +
> + return dev_name(dev);
> +}
> +
> +static int mma9551_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct mma9551_data *data;
> + struct iio_dev *indio_dev;
> + const char *name = NULL;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + data->client = client;
> +
> + if (id)
> + name = id->name;
> + else if (ACPI_HANDLE(&client->dev))
> + name = mma9551_match_acpi_device(&client->dev);
> +
> + ret = mma9551_init(data);
> + if (ret < 0)
> + return ret;
> +
> + mutex_init(&data->mutex);
> + memset(data->event_enabled, 0, sizeof(data->event_enabled));
What for? devm_iio_device_alloc already fills that memory region with zeros.
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->channels = mma9551_channels;
> + indio_dev->num_channels = ARRAY_SIZE(mma9551_channels);
> + indio_dev->name = name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &mma9551_info;
> +
> + ret = mma9551_gpio_probe(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret < 0) {
> + dev_err(&client->dev, "unable to register iio device\n");
> + return ret;
> + }
> +
> + return 0;
What about an error-out-path for powering off the device if something fails after mma9551_init()?
> +}
> +
> +static int mma9551_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct mma9551_data *data = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> + mutex_lock(&data->mutex);
> + mma9551_set_device_state(data->client, false);
> + mutex_unlock(&data->mutex);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mma9551_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct mma9551_data *data = iio_priv(indio_dev);
> +
> + mutex_lock(&data->mutex);
> + mma9551_set_device_state(data->client, false);
> + mutex_unlock(&data->mutex);
> +
> + return 0;
> +}
> +
> +static int mma9551_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct mma9551_data *data = iio_priv(indio_dev);
> +
> + mutex_lock(&data->mutex);
> + mma9551_set_device_state(data->client, true);
> + mutex_unlock(&data->mutex);
> +
> + return 0;
> +}
> +#else
> +#define mma9551_suspend NULL
> +#define mma9551_resume NULL
> +#endif
> +
> +static const struct dev_pm_ops mma9551_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(mma9551_suspend, mma9551_resume)
> +};
> +
> +static const struct acpi_device_id mma9551_acpi_match[] = {
> + {"MMA9551", 0},
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, mma9551_acpi_match);
> +
> +static const struct i2c_device_id mma9551_id[] = {
> + {"mma9551", 0},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, mma9551_id);
> +
> +static struct i2c_driver mma9551_driver = {
> + .driver = {
> + .name = MMA9551_DRV_NAME,
> + .acpi_match_table = ACPI_PTR(mma9551_acpi_match),
> + .pm = &mma9551_pm_ops,
> + },
> + .probe = mma9551_probe,
> + .remove = mma9551_remove,
> + .id_table = mma9551_id,
> +};
> +
> +module_i2c_driver(mma9551_driver);
> +
> +MODULE_AUTHOR("Irina Tirdea <irina.tirdea@intel.com>");
> +MODULE_AUTHOR("Vlad Dogaru <vlad.dogaru@intel.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("MMA9551L motion-sensing platform driver");
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] iio: add driver for Freescale MMA9551L
2014-11-23 10:58 ` Hartmut Knaack
@ 2014-11-24 9:22 ` Vlad Dogaru
0 siblings, 0 replies; 3+ messages in thread
From: Vlad Dogaru @ 2014-11-24 9:22 UTC (permalink / raw)
To: Hartmut Knaack; +Cc: linux-iio, Irina Tirdea
On Sun, Nov 23, 2014 at 11:58:41AM +0100, Hartmut Knaack wrote:
> Vlad Dogaru schrieb am 17.11.2014 16:03:
> > Add support for Freescale MMA9551L Intelligent Motion-Sensing Platform.
> > The driver supports raw reads for acceleration and inclination, as well
> > as configuring an inclination rate-of-change events. The events can be
> > used similarly to an Android sensor Tilt event.
> >
> > The specifications can be downloaded from:
> > http://www.freescale.com/files/sensors/doc/ref_manual/MMA955xLSWRM.pdf
> >
> Looks very good. Just a few minor cleanup recommendations inline.
Hopefully I haven't missed anything. Will send out v4 soon.
As always, thank you for the detailed review.
> > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> > Signed-off-by: Vlad Dogaru <vlad.dogaru@intel.com>
> > ---
> > Changes since v2:
> > - all three events can be enabled at the same time, each using a separate
> > GPIO line.
> > - simplify GPIO config code.
> > - style fixes as suggested by Jonathan.
> >
> > Changes since v1:
> > - invert logic in mma9551_set_device_state(). Setting the sleep bit to 1 puts
> > the device into sleep.
> > - do not use scan_index for channels, since we don't support buffering.
> > - style fixes as suggested by Peter.
> >
> > drivers/iio/accel/Kconfig | 10 +
> > drivers/iio/accel/Makefile | 1 +
> > drivers/iio/accel/mma9551.c | 960 ++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 971 insertions(+)
> > create mode 100644 drivers/iio/accel/mma9551.c
> >
> > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> > index 9b9be87..d80616d 100644
> > --- a/drivers/iio/accel/Kconfig
> > +++ b/drivers/iio/accel/Kconfig
> > @@ -105,4 +105,14 @@ config KXCJK1013
> > To compile this driver as a module, choose M here: the module will
> > be called kxcjk-1013.
> >
> > +config MMA9551
> > + tristate "Freescale MMA9551L Intelligent Motion-Sensing Platform Driver"
> > + depends on I2C
> > + help
> > + Say yes here to build support for the Freescale MMA9551L
> > + Intelligent Motion-Sensing Platform Driver.
> > +
> > + To compile this driver as a module, choose M here: the module
> > + will be called mma9551.
> > +
> > endmenu
> > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> > index a593996..de5b9cb 100644
> > --- a/drivers/iio/accel/Makefile
> > +++ b/drivers/iio/accel/Makefile
> > @@ -9,6 +9,7 @@ obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
> > obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
> > obj-$(CONFIG_KXSD9) += kxsd9.o
> > obj-$(CONFIG_MMA8452) += mma8452.o
> > +obj-$(CONFIG_MMA9551) += mma9551.o
> >
> > obj-$(CONFIG_IIO_ST_ACCEL_3AXIS) += st_accel.o
> > st_accel-y := st_accel_core.o
> > diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
> > new file mode 100644
> > index 0000000..d677042
> > --- /dev/null
> > +++ b/drivers/iio/accel/mma9551.c
> > @@ -0,0 +1,960 @@
> > +/*
> > + * Freescale MMA9551L Intelligent Motion-Sensing Platform driver
> > + * Copyright (c) 2014, Intel Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> > + * more details.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/i2c.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/slab.h>
> > +#include <linux/acpi.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/iio/events.h>
> > +
> > +#define MMA9551_DRV_NAME "mma9551"
> > +#define MMA9551_IRQ_NAME "mma9551_event"
> > +#define MMA9551_GPIO_NAME "mma9551_int"
> > +#define MMA9551_GPIO_COUNT 4
> > +
> > +/* Applications IDs */
> > +#define MMA9551_APPID_VERSION 0x00
> > +#define MMA9551_APPID_GPIO 0x03
> > +#define MMA9551_APPID_AFE 0x06
> > +#define MMA9551_APPID_TILT 0x0B
> > +#define MMA9551_APPID_SLEEP_WAKE 0x12
> > +#define MMA9551_APPID_RESET 0x17
> > +#define MMA9551_APPID_NONE 0xff
> > +
> > +/* Command masks for mailbox write command */
> > +#define MMA9551_CMD_READ_VERSION_INFO 0x00
> > +#define MMA9551_CMD_READ_CONFIG 0x10
> > +#define MMA9551_CMD_WRITE_CONFIG 0x20
> > +#define MMA9551_CMD_READ_STATUS 0x30
> > +
> > +enum mma9551_gpio_pin {
> > + mma9551_gpio6 = 0,
> > + mma9551_gpio7,
> > + mma9551_gpio8,
> > + mma9551_gpio9,
> > + mma9551_gpio_max = mma9551_gpio9,
> > +};
> > +
> > +/* Mailbox read command */
> > +#define MMA9551_RESPONSE_COCO BIT(7)
> > +
> > +/* Error-Status codes returned in mailbox read command */
> > +#define MMA9551_MCI_ERROR_NONE 0x00
> > +#define MMA9551_MCI_ERROR_PARAM 0x04
> > +#define MMA9551_MCI_INVALID_COUNT 0x19
> > +#define MMA9551_MCI_ERROR_COMMAND 0x1C
> > +#define MMA9551_MCI_ERROR_INVALID_LENGTH 0x21
> > +#define MMA9551_MCI_ERROR_FIFO_BUSY 0x22
> > +#define MMA9551_MCI_ERROR_FIFO_ALLOCATED 0x23
> > +#define MMA9551_MCI_ERROR_FIFO_OVERSIZE 0x24
> > +
> > +/* GPIO Application */
> > +#define MMA9551_GPIO_POL_MSB 0x08
> > +#define MMA9551_GPIO_POL_LSB 0x09
> > +
> > +/* Sleep/Wake application */
> > +#define MMA9551_SLEEP_CFG 0x06
> > +#define MMA9551_SLEEP_CFG_SNCEN BIT(0)
> > +#define MMA9551_SLEEP_CFG_SCHEN BIT(2)
> > +
> > +/* AFE application */
> > +#define MMA9551_AFE_X_ACCEL_REG 0x00
> > +#define MMA9551_AFE_Y_ACCEL_REG 0x02
> > +#define MMA9551_AFE_Z_ACCEL_REG 0x04
> > +
> > +/* Tilt application (inclination in IIO terms). */
> > +#define MMA9551_TILT_XZ_ANG_REG 0x00
> > +#define MMA9551_TILT_YZ_ANG_REG 0x01
> > +#define MMA9551_TILT_XY_ANG_REG 0x02
> > +#define MMA9551_TILT_XY_QUAD_SHIFT 0
> > +#define MMA9551_TILT_YZ_QUAD_SHIFT 2
> > +#define MMA9551_TILT_XZ_QUAD_SHIFT 4
> > +#define MMA9551_TILT_ANGFLG BIT(7)
> This flag is part of the angular registers, so better move it up there.
> > +#define MMA9551_TILT_QUAD_REG 0x03
> This line should be put on top of the _QUAD_SHIFTs.
Makes sense, will fix both in v4.
> > +#define MMA9551_TILT_CFG_REG 0x01
> > +#define MMA9551_TILT_ANG_THRESH_MASK 0x0f
> This may be represented using GENMASK - just a matter of taste.
Neat. Didn't know about GENMASK.
> > +
> > +/* Tilt events are mapped to the first three GPIO pins. */
> > +enum mma9551_tilt_axis {
> > + mma9551_x = 0,
> > + mma9551_y,
> > + mma9551_z,
> > +};
> > +
> > +/*
> > + * A response is composed of:
> > + * - control registers: MB0-3
> > + * - data registers: MB4-31
> > + *
> > + * A request is composed of:
> > + * - mbox to write to (always 0)
> > + * - control registers: MB1-4
> > + * - data registers: MB5-31
> > + */
> > +#define MMA9551_MAILBOX_CTRL_REGS 4
> > +#define MMA9551_MAX_MAILBOX_DATA_REGS 28
> > +#define MMA9551_MAILBOX_REGS 32
> > +
> > +#define MMA9551_I2C_READ_RETRIES 5
> > +#define MMA9551_I2C_READ_DELAY 50 /* us */
> > +
> > +struct mma9551_mbox_request {
> > + u8 start_mbox; /* Always 0. */
> > + u8 app_id;
> > + /*
> > + * See Section 5.3.1 of the MMA955xL Software Reference Manual.
> > + *
> > + * Bit 7: reserved, always 0
> > + * Bits 6-4: command
> > + * Bits 3-0: upper bits of register offset
> > + */
> > + u8 cmd_off;
> > + u8 lower_off;
> > + u8 nbytes;
> > + u8 buf[MMA9551_MAX_MAILBOX_DATA_REGS - 1];
> > +} __packed;
> > +
> > +struct mma9551_mbox_response {
> > + u8 app_id;
> > + /*
> > + * See Section 5.3.3 of the MMA955xL Software Reference Manual.
> > + *
> > + * Bit 7: COCO
> > + * Bits 6-0: Error code.
> > + */
> > + u8 coco_err;
> > + u8 nbytes;
> > + u8 req_bytes;
> > + u8 buf[MMA9551_MAX_MAILBOX_DATA_REGS];
> > +} __packed;
> > +
> > +struct mma9551_version_info {
> > + __be32 device_id;
> > + u8 rom_version[2];
> > + u8 fw_version[2];
> > + u8 hw_version[2];
> > + u8 fw_build[2];
> > +};
> > +
> > +struct mma9551_data {
> > + struct i2c_client *client;
> > + struct mutex mutex;
> > + int event_enabled[3];
> > + int irqs[MMA9551_GPIO_COUNT];
> > +};
> > +
> > +static int mma9551_transfer(struct i2c_client *client,
> > + u8 app_id, u8 command, u16 offset,
> > + u8 *inbytes, int num_inbytes,
> > + u8 *outbytes, int num_outbytes)
> > +{
> > + struct mma9551_mbox_request req;
> > + struct mma9551_mbox_response rsp;
> > + struct i2c_msg in, out;
> > + u8 req_len, err_code;
> > + int ret;
> > + int retries;
> These two int definitions could be consolidated in one line.
> > +
> > + if (offset >= 1 << 12) {
> > + dev_err(&client->dev, "register offset too large\n");
> > + return -EINVAL;
> > + }
> > +
> > + req_len = 1 + MMA9551_MAILBOX_CTRL_REGS + num_inbytes;
> > + req.start_mbox = 0;
> > + req.app_id = app_id;
> > + req.cmd_off = command | (offset >> 8);
> > + req.lower_off = offset;
> > +
> > + if (command == MMA9551_CMD_WRITE_CONFIG)
> > + req.nbytes = num_inbytes;
> > + else
> > + req.nbytes = num_outbytes;
> > + if (num_inbytes)
> > + memcpy(req.buf, inbytes, num_inbytes);
> > +
> > + out.addr = client->addr;
> > + out.flags = 0;
> > + out.len = req_len;
> > + out.buf = (u8 *)&req;
> > +
> > + ret = i2c_transfer(client->adapter, &out, 1);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "i2c write failed\n");
> > + return ret;
> > + }
> > +
> > + retries = MMA9551_I2C_READ_RETRIES;
> > + do {
> > + udelay(MMA9551_I2C_READ_DELAY);
> > +
> > + in.addr = client->addr;
> > + in.flags = I2C_M_RD;
> > + in.len = sizeof(rsp);
> > + in.buf = (u8 *)&rsp;
> > +
> > + ret = i2c_transfer(client->adapter, &in, 1);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "i2c read failed\n");
> > + return ret;
> > + }
> > +
> > + if (rsp.coco_err & MMA9551_RESPONSE_COCO)
> > + break;
> > + } while (--retries > 0);
> > +
> > + if (retries == 0) {
> > + dev_err(&client->dev,
> > + "timed out while waiting for command response\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + if (rsp.app_id != app_id) {
> > + dev_err(&client->dev,
> > + "app_id mismatch in response got %02x expected %02x\n",
> > + rsp.app_id, app_id);
> > + return -EINVAL;
> > + }
> > +
> > + err_code = rsp.coco_err & ~MMA9551_RESPONSE_COCO;
> > + if (err_code != MMA9551_MCI_ERROR_NONE) {
> > + dev_err(&client->dev, "read returned error %x\n", err_code);
> > + return -EINVAL;
> > + }
> > +
> > + if (rsp.nbytes != rsp.req_bytes) {
> > + dev_err(&client->dev,
> > + "output length mismatch got %d expected %d\n",
> > + rsp.nbytes, num_outbytes);
> Don't you mean rsp.req_bytes instead of num_outbytes here?
Yes, I do. Nice catch!
> > + return -EINVAL;
> > + }
> > +
> > + if (num_outbytes)
> > + memcpy(outbytes, rsp.buf, num_outbytes);
> > +
> > + return 0;
> > +}
> > +
> > +static int mma9551_read_config_byte(struct i2c_client *client, u8 app_id,
> > + u16 reg, u8 *val)
> > +{
> > + return mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
> > + reg, NULL, 0, val, 1);
> > +}
> > +
> > +static int mma9551_write_config_byte(struct i2c_client *client, u8 app_id,
> > + u16 reg, u8 val)
> > +{
> > + return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG, reg,
> > + &val, 1, NULL, 0);
> > +}
> > +
> > +static int mma9551_read_status_byte(struct i2c_client *client, u8 app_id,
> > + u16 reg, u8 *val)
> > +{
> > + return mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
> > + reg, NULL, 0, val, 1);
> > +}
> > +
> > +static int mma9551_read_status_word(struct i2c_client *client, u8 app_id,
> > + u16 reg, u16 *val)
> > +{
> > + int ret;
> > + __be16 v;
> > +
> > + ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
> > + reg, NULL, 0, (u8 *)&v, 2);
> > + *val = be16_to_cpu(v);
> > +
> > + return ret;
> > +}
> > +
> > +static int mma9551_update_config_bits(struct i2c_client *client, u8 app_id,
> > + u8 reg, u8 mask, u8 val)
> u16 was used for reg previously, would be more consistent to stick to it.
Right, will fix.
> > +{
> > + int ret;
> > + u8 tmp, orig;
> > +
> > + ret = mma9551_read_config_byte(client, app_id, reg, &orig);
> > + if (ret < 0)
> > + return ret;
> > +
> > + tmp = orig & ~mask;
> > + tmp |= val & mask;
> > +
> > + if (tmp == orig)
> > + return 0;
> > +
> > + return mma9551_write_config_byte(client, app_id, reg, tmp);
> > +}
> > +
> > +/*
> > + * The polarity parameter is described in section 6.2.2, page 66, of the
> > + * Software Reference Manual. Basically, polarity=0 means the interrupt
> > + * line has the same value as the selected bit, while polarity=1 means
> > + * the line is inverted.
> > + */
> > +static int mma9551_gpio_config(struct i2c_client *client,
> > + enum mma9551_gpio_pin pin,
> > + u8 app_id, u8 bitnum, int polarity)
> > +{
> > + u8 reg, pol_mask, pol_val;
> > + int ret;
> > +
> > + if (pin > mma9551_gpio_max) {
> > + dev_err(&client->dev, "bad GPIO pin\n");
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > + * Pin 6 is configured by regs 0x00 and 0x01, pin 7 by 0x02 and
> > + * 0x03, and so on.
> > + */
> > + reg = pin * 2;
> > +
> > + ret = mma9551_write_config_byte(client, MMA9551_APPID_GPIO,
> > + reg, app_id);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "error setting GPIO app_id\n");
> > + return ret;
> > + }
> > +
> > + ret = mma9551_write_config_byte(client, MMA9551_APPID_GPIO,
> > + reg + 1, bitnum);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "error setting GPIO bit number\n");
> > + return ret;
> > + }
> > +
> > + switch (pin) {
> > + case mma9551_gpio6:
> > + reg = MMA9551_GPIO_POL_LSB;
> > + pol_mask = 1 << 6;
> > + break;
> > + case mma9551_gpio7:
> > + reg = MMA9551_GPIO_POL_LSB;
> > + pol_mask = 1 << 7;
> > + break;
> > + case mma9551_gpio8:
> > + reg = MMA9551_GPIO_POL_MSB;
> > + pol_mask = 1 << 0;
> > + break;
> > + case mma9551_gpio9:
> > + reg = MMA9551_GPIO_POL_MSB;
> > + pol_mask = 1 << 1;
> > + break;
> > + }
> > + pol_val = polarity ? pol_mask : 0;
> > +
> > + ret = mma9551_update_config_bits(client, MMA9551_APPID_GPIO, reg,
> > + pol_mask, pol_val);
> > + if (ret < 0)
> > + dev_err(&client->dev, "error setting GPIO polarity\n");
> > +
> > + return ret;
> > +}
> > +
> > +static int mma9551_read_version(struct i2c_client *client)
> > +{
> > + struct mma9551_version_info info;
> > + int ret;
> > +
> > + ret = mma9551_transfer(client, MMA9551_APPID_VERSION, 0x00, 0x00,
> > + NULL, 0, (u8 *)&info, sizeof(info));
> > + if (ret < 0)
> > + return ret;
> > +
> > + dev_info(&client->dev, "Device ID 0x%x, firmware version %02x.%02x\n",
> > + be32_to_cpu(info.device_id), info.fw_version[0],
> > + info.fw_version[1]);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Use 'false' as the second parameter to cause the device to enter
> > + * sleep.
> > + */
> > +static int mma9551_set_device_state(struct i2c_client *client,
> > + bool enable)
> > +{
> > + return mma9551_update_config_bits(client, MMA9551_APPID_SLEEP_WAKE,
> > + MMA9551_SLEEP_CFG,
> > + MMA9551_SLEEP_CFG_SNCEN,
> > + enable ? 0 : MMA9551_SLEEP_CFG_SNCEN);
> This will fit exactly 80 chars, if indented the normal way.
I've been switching things around here and they didn't fit previously.
Will fix.
> > +}
> > +
> > +static int mma9551_read_incli_chan(struct i2c_client *client,
> > + const struct iio_chan_spec *chan,
> > + int *val)
> > +{
> > + u8 quad_shift, quad_mask, angle, quadrant;
> > + u16 reg_addr;
> > + int ret;
> > +
> > + switch (chan->channel2) {
> > + case IIO_MOD_X:
> > + reg_addr = MMA9551_TILT_YZ_ANG_REG;
> > + quad_shift = MMA9551_TILT_YZ_QUAD_SHIFT;
> > + break;
> > + case IIO_MOD_Y:
> > + reg_addr = MMA9551_TILT_XZ_ANG_REG;
> > + quad_shift = MMA9551_TILT_XZ_QUAD_SHIFT;
> > + break;
> > + case IIO_MOD_Z:
> > + reg_addr = MMA9551_TILT_XY_ANG_REG;
> > + quad_shift = MMA9551_TILT_XY_QUAD_SHIFT;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + ret = mma9551_read_status_byte(client, MMA9551_APPID_TILT,
> > + reg_addr, &angle);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = mma9551_read_status_byte(client, MMA9551_APPID_TILT,
> > + MMA9551_TILT_QUAD_REG, &quadrant);
> > + if (ret < 0)
> > + return ret;
> > +
> > + angle &= ~MMA9551_TILT_ANGFLG;
> > + quad_mask = 0x03 << quad_shift;
> > + quadrant = (quadrant & quad_mask) >> quad_shift;
> quad_mask is unnecessary. Just do:
> quadrant = (quadrant >> quad_shift) & 0x03;
That does seems easier to read, will fix.
> > +
> > + if (quadrant == 1 || quadrant == 3)
> > + *val = 90 * (quadrant + 1) - angle;
> > + else
> > + *val = angle + 90 * quadrant;
> > +
> > + return IIO_VAL_INT;
> > +}
> > +
> > +static int mma9551_read_accel_chan(struct i2c_client *client,
> > + const struct iio_chan_spec *chan,
> > + int *val, int *val2)
> > +{
> > + u16 reg_addr;
> > + s16 raw_accel;
> > + int ret;
> > +
> > + switch (chan->channel2) {
> > + case IIO_MOD_X:
> > + reg_addr = MMA9551_AFE_X_ACCEL_REG;
> > + break;
> > + case IIO_MOD_Y:
> > + reg_addr = MMA9551_AFE_Y_ACCEL_REG;
> > + break;
> > + case IIO_MOD_Z:
> > + reg_addr = MMA9551_AFE_Z_ACCEL_REG;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + ret = mma9551_read_status_word(client, MMA9551_APPID_AFE,
> > + reg_addr, &raw_accel);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = raw_accel;
> > +
> > + return IIO_VAL_INT;
> > +}
> > +
> > +static int mma9551_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long mask)
> > +{
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > + int ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_PROCESSED:
> > + switch (chan->type) {
> > + case IIO_INCLI:
> > + mutex_lock(&data->mutex);
> > + ret = mma9551_read_incli_chan(data->client,
> > + chan, val);
> No need to wrap this line.
Will fix (along with other style issues).
> > + mutex_unlock(&data->mutex);
> > + return ret;
> > + default:
> > + return -EINVAL;
> > + }
> > + case IIO_CHAN_INFO_RAW:
> > + switch (chan->type) {
> > + case IIO_ACCEL:
> > + mutex_lock(&data->mutex);
> > + ret = mma9551_read_accel_chan(data->client,
> > + chan, val, val2);
> > + mutex_unlock(&data->mutex);
> > + return ret;
> > + default:
> > + return -EINVAL;
> > + }
> > + case IIO_CHAN_INFO_SCALE:
> > + switch (chan->type) {
> > + case IIO_ACCEL:
> > + *val = 0;
> > + *val2 = 2440;
> > + return IIO_VAL_INT_PLUS_MICRO;
> > + default:
> > + return -EINVAL;
> > + }
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static int mma9551_read_event_config(struct iio_dev *indio_dev,
> > + const struct iio_chan_spec *chan,
> > + enum iio_event_type type,
> > + enum iio_event_direction dir)
> > +{
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > +
> > + switch (chan->type) {
> > + case IIO_INCLI:
> > + /* IIO counts axes from 1, because IIO_NO_MOD is 0. */
> > + return data->event_enabled[chan->channel2 - 1];
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static int mma9551_config_incli_event(struct iio_dev *indio_dev,
> > + enum iio_modifier axis,
> > + int state)
> > +{
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > + enum mma9551_tilt_axis mma_axis;
> > + int ret;
> > +
> > + /* IIO counts axes from 1, because IIO_NO_MOD is 0. */
> > + mma_axis = axis - 1;
> > +
> > + if (data->event_enabled[mma_axis] == state)
> > + return 0;
> > +
> > + if (state == 0) {
> > + ret = mma9551_gpio_config(data->client, mma_axis,
> > + MMA9551_APPID_NONE, 0, 0);
> > + if (ret < 0)
> > + return ret;
> > + } else {
> > + int bitnum;
> > +
> > + /* Bit 7 of each angle register holds the angle flag. */
> > + switch (axis) {
> > + case IIO_MOD_X:
> > + bitnum = 7 + 8 * MMA9551_TILT_YZ_ANG_REG;
> > + break;
> > + case IIO_MOD_Y:
> > + bitnum = 7 + 8 * MMA9551_TILT_XZ_ANG_REG;
> > + break;
> > + case IIO_MOD_Z:
> > + bitnum = 7 + 8 * MMA9551_TILT_XY_ANG_REG;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + ret = mma9551_gpio_config(data->client, mma_axis,
> > + MMA9551_APPID_TILT, bitnum, 0);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + data->event_enabled[mma_axis] = state;
> > +
> > + return ret;
> > +}
> > +
> > +static int mma9551_write_event_config(struct iio_dev *indio_dev,
> > + const struct iio_chan_spec *chan,
> > + enum iio_event_type type,
> > + enum iio_event_direction dir,
> > + int state)
> > +{
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > + int ret;
> > +
> > + switch (chan->type) {
> > + case IIO_INCLI:
> > + mutex_lock(&data->mutex);
> > + ret = mma9551_config_incli_event(indio_dev,
> > + chan->channel2, state);
> > + mutex_unlock(&data->mutex);
> > + return ret;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static int mma9551_write_event_value(struct iio_dev *indio_dev,
> > + const struct iio_chan_spec *chan,
> > + enum iio_event_type type,
> > + enum iio_event_direction dir,
> > + enum iio_event_info info,
> > + int val, int val2)
> > +{
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > + int ret;
> > +
> > + switch (chan->type) {
> > + case IIO_INCLI:
> > + if (val2 != 0 || val < 1 || val > 10)
> > + return -EINVAL;
> > + mutex_lock(&data->mutex);
> > + ret = mma9551_update_config_bits(data->client,
> > + MMA9551_APPID_TILT,
> > + MMA9551_TILT_CFG_REG,
> > + MMA9551_TILT_ANG_THRESH_MASK,
> > + val);
> > + mutex_unlock(&data->mutex);
> > + return ret;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static int mma9551_read_event_value(struct iio_dev *indio_dev,
> > + const struct iio_chan_spec *chan,
> > + enum iio_event_type type,
> > + enum iio_event_direction dir,
> > + enum iio_event_info info,
> > + int *val, int *val2)
> > +{
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > + int ret;
> > + u8 tmp;
> > +
> > + switch (chan->type) {
> > + case IIO_INCLI:
> > + mutex_lock(&data->mutex);
> > + ret = mma9551_read_config_byte(data->client,
> > + MMA9551_APPID_TILT,
> > + MMA9551_TILT_CFG_REG, &tmp);
> > + mutex_unlock(&data->mutex);
> > + if (ret < 0)
> > + return ret;
> > + *val = tmp & MMA9551_TILT_ANG_THRESH_MASK;
> > + *val2 = 0;
> > + return IIO_VAL_INT;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static const struct iio_event_spec mma9551_incli_event = {
> > + .type = IIO_EV_TYPE_ROC,
> > + .dir = IIO_EV_DIR_RISING,
> > + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> > + .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE),
> > +};
> > +
> > +#define MMA9551_ACCEL_CHANNEL(axis) { \
> > + .type = IIO_ACCEL, \
> > + .modified = 1, \
> > + .channel2 = axis, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> > +}
> > +
> > +#define MMA9551_INCLI_CHANNEL(axis) { \
> > + .type = IIO_INCLI, \
> > + .modified = 1, \
> > + .channel2 = axis, \
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> > + .event_spec = &mma9551_incli_event, \
> > + .num_event_specs = 1, \
> > +}
> > +
> > +static const struct iio_chan_spec mma9551_channels[] = {
> > + MMA9551_ACCEL_CHANNEL(IIO_MOD_X),
> > + MMA9551_ACCEL_CHANNEL(IIO_MOD_Y),
> > + MMA9551_ACCEL_CHANNEL(IIO_MOD_Z),
> > +
> > + MMA9551_INCLI_CHANNEL(IIO_MOD_X),
> > + MMA9551_INCLI_CHANNEL(IIO_MOD_Y),
> > + MMA9551_INCLI_CHANNEL(IIO_MOD_Z),
> > +};
> > +
> > +static const struct iio_info mma9551_info = {
> > + .driver_module = THIS_MODULE,
> > + .read_raw = mma9551_read_raw,
> > + .read_event_config = mma9551_read_event_config,
> > + .write_event_config = mma9551_write_event_config,
> > + .read_event_value = mma9551_read_event_value,
> > + .write_event_value = mma9551_write_event_value,
> > +};
> > +
> > +static irqreturn_t mma9551_event_handler(int irq, void *private)
> > +{
> > + struct iio_dev *indio_dev = private;
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > + int mma_axis, ret, i;
> > + u16 reg;
> > + u8 val;
> > +
> > + mutex_lock(&data->mutex);
> > +
> > + mma_axis = -1;
> This value could be assigned already during declaration.
I thought it would make sense to keep it near the loop where it's
assigned, but I don't have a strong preference. I'll move the
assignment up with the declaration.
> > + for (i = 0; i < 3; i++)
> > + if (irq == data->irqs[i]) {
> > + mma_axis = i;
> > + break;
> > + }
> > +
> > + if (mma_axis == -1) {
> > + /* IRQ was triggered on 4th line, which we don't use. */
> > + dev_warn(&data->client->dev,
> > + "irq triggered on unused line %d\n", data->irqs[3]);
> > + goto out;
> > + }
> > +
> > + switch (mma_axis) {
> > + case mma9551_x:
> > + reg = MMA9551_TILT_YZ_ANG_REG;
> > + break;
> > + case mma9551_y:
> > + reg = MMA9551_TILT_XZ_ANG_REG;
> > + break;
> > + case mma9551_z:
> > + reg = MMA9551_TILT_XY_ANG_REG;
> > + break;
> > + }
> > +
> > + /*
> > + * Read the angle even though we don't use it, otherwise we
> > + * won't get any further interrupts.
> > + */
> > + ret = mma9551_read_status_byte(data->client, MMA9551_APPID_TILT,
> > + reg, &val);
> > + if (ret < 0) {
> > + dev_err(&data->client->dev,
> > + "error %d reading tilt register in IRQ\n", ret);
> > + goto out;
> > + }
> > +
> > + iio_push_event(indio_dev,
> > + IIO_MOD_EVENT_CODE(IIO_INCLI, 0,
> > + (mma_axis + 1),
> > + IIO_EV_TYPE_ROC,
> > + IIO_EV_DIR_RISING),
> Why not put more parameters in every line? Would save some space.
> > + iio_get_time_ns());
> > +
> > +out:
> > + mutex_unlock(&data->mutex);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int mma9551_init(struct mma9551_data *data)
> > +{
> > + int ret;
> > +
> > + ret = mma9551_read_version(data->client);
> > + if (ret)
> > + return ret;
> > +
> > + /* Power on chip and enable doze mode. */
> > + ret = mma9551_update_config_bits(data->client,
> > + MMA9551_APPID_SLEEP_WAKE,
> > + MMA9551_SLEEP_CFG,
> > + MMA9551_SLEEP_CFG_SCHEN | MMA9551_SLEEP_CFG_SNCEN,
> > + MMA9551_SLEEP_CFG_SCHEN);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return 0;
> Just return mma9551_update_config_bits().
> > +}
> > +
> > +static int mma9551_gpio_probe(struct iio_dev *indio_dev)
> > +{
> > + struct gpio_desc *gpio;
> > + int i, ret;
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > + struct device *dev = &data->client->dev;
> > +
> > + for (i = 0; i < MMA9551_GPIO_COUNT; i++) {
> > + gpio = devm_gpiod_get_index(dev, MMA9551_GPIO_NAME, i);
> > + if (IS_ERR(gpio)) {
> > + dev_err(dev, "acpi gpio get index failed\n");
> > + return PTR_ERR(gpio);
> > + }
> > +
> > + ret = gpiod_direction_input(gpio);
> > + if (ret)
> > + return ret;
> > +
> > + data->irqs[i] = gpiod_to_irq(gpio);
> > + ret = devm_request_threaded_irq(dev, data->irqs[i],
> > + NULL, mma9551_event_handler,
> > + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> > + MMA9551_IRQ_NAME, indio_dev);
> > + if (ret < 0) {
> > + dev_err(dev, "request irq %d failed\n", data->irqs[i]);
> > + return ret;
> > + }
> > +
> > + dev_dbg(dev, "gpio resource, no:%d irq:%d\n",
> > + desc_to_gpio(gpio), data->irqs[i]);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const char *mma9551_match_acpi_device(struct device *dev)
> > +{
> > + const struct acpi_device_id *id;
> > +
> > + id = acpi_match_device(dev->driver->acpi_match_table, dev);
> > + if (!id)
> > + return NULL;
> > +
> > + return dev_name(dev);
> > +}
> > +
> > +static int mma9551_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct mma9551_data *data;
> > + struct iio_dev *indio_dev;
> > + const char *name = NULL;
> > + int ret;
> > +
> > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> > + if (!indio_dev)
> > + return -ENOMEM;
> > +
> > + data = iio_priv(indio_dev);
> > + i2c_set_clientdata(client, indio_dev);
> > + data->client = client;
> > +
> > + if (id)
> > + name = id->name;
> > + else if (ACPI_HANDLE(&client->dev))
> > + name = mma9551_match_acpi_device(&client->dev);
> > +
> > + ret = mma9551_init(data);
> > + if (ret < 0)
> > + return ret;
> > +
> > + mutex_init(&data->mutex);
> > + memset(data->event_enabled, 0, sizeof(data->event_enabled));
> What for? devm_iio_device_alloc already fills that memory region with zeros.
I didn't know that, thanks for pointing it out.
> > +
> > + indio_dev->dev.parent = &client->dev;
> > + indio_dev->channels = mma9551_channels;
> > + indio_dev->num_channels = ARRAY_SIZE(mma9551_channels);
> > + indio_dev->name = name;
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > + indio_dev->info = &mma9551_info;
> > +
> > + ret = mma9551_gpio_probe(indio_dev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = iio_device_register(indio_dev);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "unable to register iio device\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> What about an error-out-path for powering off the device if something fails after mma9551_init()?
Good point.
> > +}
> > +
> > +static int mma9551_remove(struct i2c_client *client)
> > +{
> > + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > +
> > + iio_device_unregister(indio_dev);
> > + mutex_lock(&data->mutex);
> > + mma9551_set_device_state(data->client, false);
> > + mutex_unlock(&data->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int mma9551_suspend(struct device *dev)
> > +{
> > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > +
> > + mutex_lock(&data->mutex);
> > + mma9551_set_device_state(data->client, false);
> > + mutex_unlock(&data->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static int mma9551_resume(struct device *dev)
> > +{
> > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> > + struct mma9551_data *data = iio_priv(indio_dev);
> > +
> > + mutex_lock(&data->mutex);
> > + mma9551_set_device_state(data->client, true);
> > + mutex_unlock(&data->mutex);
> > +
> > + return 0;
> > +}
> > +#else
> > +#define mma9551_suspend NULL
> > +#define mma9551_resume NULL
> > +#endif
> > +
> > +static const struct dev_pm_ops mma9551_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(mma9551_suspend, mma9551_resume)
> > +};
> > +
> > +static const struct acpi_device_id mma9551_acpi_match[] = {
> > + {"MMA9551", 0},
> > + {},
> > +};
> > +
> > +MODULE_DEVICE_TABLE(acpi, mma9551_acpi_match);
> > +
> > +static const struct i2c_device_id mma9551_id[] = {
> > + {"mma9551", 0},
> > + {}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(i2c, mma9551_id);
> > +
> > +static struct i2c_driver mma9551_driver = {
> > + .driver = {
> > + .name = MMA9551_DRV_NAME,
> > + .acpi_match_table = ACPI_PTR(mma9551_acpi_match),
> > + .pm = &mma9551_pm_ops,
> > + },
> > + .probe = mma9551_probe,
> > + .remove = mma9551_remove,
> > + .id_table = mma9551_id,
> > +};
> > +
> > +module_i2c_driver(mma9551_driver);
> > +
> > +MODULE_AUTHOR("Irina Tirdea <irina.tirdea@intel.com>");
> > +MODULE_AUTHOR("Vlad Dogaru <vlad.dogaru@intel.com>");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("MMA9551L motion-sensing platform driver");
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-24 9:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-17 15:03 [PATCH v3] iio: add driver for Freescale MMA9551L Vlad Dogaru
2014-11-23 10:58 ` Hartmut Knaack
2014-11-24 9:22 ` Vlad Dogaru
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox