On 12/13/2012 01:11 PM, Denis CIOCCA wrote:
> This patch add generic library for STMicroelectronics 3-axis sensors.
Nice piece of work. Couple of little bits and bobs, but basically there
as far as I am concerned...
(just the spi buffer alignment stuff and it really is just a case of
making sure they are the last element in the chunk of memory created
as mallocs are always a whole number of cachelines.)
> ---
> drivers/iio/common/Kconfig | 1 +
> drivers/iio/common/Makefile | 1 +
> drivers/iio/common/st_sensors/Kconfig | 30 ++
> drivers/iio/common/st_sensors/Makefile | 9 +
> drivers/iio/common/st_sensors/st_sensors_buffer.c | 115 +++++++
> drivers/iio/common/st_sensors/st_sensors_core.c | 335 ++++++++++++++++++++
> drivers/iio/common/st_sensors/st_sensors_i2c.c | 78 +++++
> drivers/iio/common/st_sensors/st_sensors_spi.c | 124 +++++++
> drivers/iio/common/st_sensors/st_sensors_trigger.c | 76 +++++
> include/linux/iio/common/st_sensors.h | 246 ++++++++++++++
> 10 files changed, 1015 insertions(+), 0 deletions(-)
> create mode 100644 drivers/iio/common/st_sensors/Kconfig
> create mode 100644 drivers/iio/common/st_sensors/Makefile
> create mode 100644 drivers/iio/common/st_sensors/st_sensors_buffer.c
> create mode 100644 drivers/iio/common/st_sensors/st_sensors_core.c
> create mode 100644 drivers/iio/common/st_sensors/st_sensors_i2c.c
> create mode 100644 drivers/iio/common/st_sensors/st_sensors_spi.c
> create mode 100644 drivers/iio/common/st_sensors/st_sensors_trigger.c
> create mode 100644 include/linux/iio/common/st_sensors.h
>
> diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
> index ed45ee5..0b6e97d 100644
> --- a/drivers/iio/common/Kconfig
> +++ b/drivers/iio/common/Kconfig
> @@ -3,3 +3,4 @@
> #
>
> source "drivers/iio/common/hid-sensors/Kconfig"
> +source "drivers/iio/common/st_sensors/Kconfig"
> diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
> index 8158400..c2352be 100644
> --- a/drivers/iio/common/Makefile
> +++ b/drivers/iio/common/Makefile
> @@ -7,3 +7,4 @@
> #
>
> obj-y += hid-sensors/
> +obj-y += st_sensors/
> diff --git a/drivers/iio/common/st_sensors/Kconfig b/drivers/iio/common/st_sensors/Kconfig
> new file mode 100644
> index 0000000..ddcfcc6
> --- /dev/null
> +++ b/drivers/iio/common/st_sensors/Kconfig
> @@ -0,0 +1,30 @@
> +#
> +# Hid Sensor common modules
> +#
> +menu "STMicroelectronics sensors library"
> +
> +config ST_SENSORS_CORE
> + tristate "Common modules for all ST sensors IIO drivers"
> + help
> + Say yes here to build support for ST sensors to use
> + common core functions.
> +
> +config ST_SENSORS_I2C
> + tristate "I2C common library for ST Sensor IIO drivers"
> + help
> + Say yes here to build support for ST sensors to use
> + common i2c functions.
> +
> +config ST_SENSORS_SPI
> + tristate "SPI common library for ST Sensor IIO drivers"
> + help
> + Say yes here to build support for ST sensors to use
> + common spi functions.
> +
> +config ST_SENSORS_TRIGGERED_BUFFER
> + tristate "trigger and buffer common library for ST Sensor IIO drivers"
> + help
> + Say yes here to build support for ST sensors to use
> + common trigger and buffer functions.
> +
> +endmenu
> diff --git a/drivers/iio/common/st_sensors/Makefile b/drivers/iio/common/st_sensors/Makefile
> new file mode 100644
> index 0000000..a191e65
> --- /dev/null
> +++ b/drivers/iio/common/st_sensors/Makefile
> @@ -0,0 +1,9 @@
> +#
> +# Makefile for the STMicroelectronics sensor common modules.
> +#
> +
> +obj-$(CONFIG_ST_SENSORS_CORE) += st_sensors_core.o
> +obj-$(CONFIG_ST_SENSORS_I2C) += st_sensors_i2c.o
> +obj-$(CONFIG_ST_SENSORS_SPI) += st_sensors_spi.o
> +obj-$(CONFIG_ST_SENSORS_TRIGGERED_BUFFER) += st_sensors_triggered_buffer.o
> +st_sensors_triggered_buffer-y := st_sensors_trigger.o st_sensors_buffer.o
> diff --git a/drivers/iio/common/st_sensors/st_sensors_buffer.c b/drivers/iio/common/st_sensors/st_sensors_buffer.c
> new file mode 100644
> index 0000000..b636751
> --- /dev/null
> +++ b/drivers/iio/common/st_sensors/st_sensors_buffer.c
> @@ -0,0 +1,115 @@
> +/*
> + * STMicroelectronics sensors buffer library driver
> + *
> + * Copyright 2012 STMicroelectronics Inc.
> + *
> + * Denis Ciocca <denis.ciocca@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/interrupt.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/irqreturn.h>
> +
> +#include <linux/iio/common/st_sensors.h>
> +
> +int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
> +{
> + int i, n = 0, len;
> + u8 addr[ST_SENSORS_NUMBER_DATA_CHANNELS];
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + for (i = 0; i < ST_SENSORS_NUMBER_DATA_CHANNELS; i++) {
> + if (test_bit(i, indio_dev->active_scan_mask)) {
> + addr[n] = indio_dev->channels[i].address;
> + n++;
> + }
> + }
> + switch (n) {
> + case 1:
> + len = sdata->tf.read_multiple_byte(&sdata->tf, sdata->dev,
> + addr[0], ST_SENSORS_BYTE_FOR_CHANNEL, buf,
> + sdata->multiread_bit);
> + break;
> + case 2:
> + if ((addr[1] - addr[0]) == ST_SENSORS_BYTE_FOR_CHANNEL) {
> + len = sdata->tf.read_multiple_byte(&sdata->tf,
> + sdata->dev, addr[0],
> + ST_SENSORS_BYTE_FOR_CHANNEL*n,
> + buf, sdata->multiread_bit);
> + } else {
> + u8 rx_array[ST_SENSORS_BYTE_FOR_CHANNEL*
> + ST_SENSORS_NUMBER_DATA_CHANNELS];
> + len = sdata->tf.read_multiple_byte(&sdata->tf,
> + sdata->dev, addr[0],
> + ST_SENSORS_BYTE_FOR_CHANNEL*
> + ST_SENSORS_NUMBER_DATA_CHANNELS,
> + rx_array, sdata->multiread_bit);
> + if (len < 0)
> + goto read_data_channels_error;
> +
> + for (i = 0; i < n * ST_SENSORS_NUMBER_DATA_CHANNELS;
> + i++) {
> + if (i < n)
> + buf[i] = rx_array[i];
> + else
> + buf[i] = rx_array[n + i];
> + }
> + len = ST_SENSORS_BYTE_FOR_CHANNEL*n;
> + }
> + break;
> + case 3:
> + len = sdata->tf.read_multiple_byte(&sdata->tf, sdata->dev,
> + addr[0], ST_SENSORS_BYTE_FOR_CHANNEL*
> + ST_SENSORS_NUMBER_DATA_CHANNELS,
> + buf, sdata->multiread_bit);
> + break;
> + default:
> + len = -EINVAL;
> + goto read_data_channels_error;
> + }
> + if (len != ST_SENSORS_BYTE_FOR_CHANNEL*n) {
> + len = -EIO;
> + goto read_data_channels_error;
> + }
> +
> +read_data_channels_error:
> + return len;
> +}
> +EXPORT_SYMBOL(st_sensors_get_buffer_element);
> +
> +irqreturn_t st_sensors_trigger_handler(int irq, void *p)
> +{
> + int len;
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + len = st_sensors_get_buffer_element(indio_dev, sdata->buffer_data);
> + if (len < 0)
> + goto st_sensors_get_buffer_element_error;
> +
> + if (indio_dev->scan_timestamp)
> + *(s64 *)((u8 *)sdata->buffer_data +
> + ALIGN(len, sizeof(s64))) = pf->timestamp;
> +
> + iio_push_to_buffers(indio_dev, sdata->buffer_data);
> +
> +st_sensors_get_buffer_element_error:
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(st_sensors_trigger_handler);
> +
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca@xxxxxx>");
> +MODULE_DESCRIPTION("STMicroelectronics ST-sensors buffer");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> new file mode 100644
> index 0000000..2b00ed7
> --- /dev/null
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -0,0 +1,335 @@
> +/*
> + * STMicroelectronics sensors core library driver
> + *
> + * Copyright 2012 STMicroelectronics Inc.
> + *
> + * Denis Ciocca <denis.ciocca@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +
> +#include <linux/iio/common/st_sensors.h>
> +
> +
> +#define ST_SENSORS_WAI_ADDRESS 0x0f
> +
> +int st_sensors_write_data_with_mask(struct iio_dev *indio_dev, u8 reg_addr,
> + u8 mask, short num_bit, u8 data)
> +{
> + int err;
> + u8 new_data;
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + err = sdata->tf.read_byte(&sdata->tf, sdata->dev, reg_addr, &new_data);
> + if (err < 0)
> + goto st_sensors_write_data_with_mask_error;
> +
> + new_data = ((new_data & (~mask)) | ((data << __ffs(mask)) & mask));
> + err = sdata->tf.write_byte(&sdata->tf, sdata->dev, reg_addr, new_data);
> +
> +st_sensors_write_data_with_mask_error:
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_write_data_with_mask);
> +
> +int st_sensors_get_sampling_frequency_avl(struct iio_dev *indio_dev,
> + const struct st_sensor_odr_avl *odr_avl, char *buf)
> +{
> + int i, len = 0;
> +
> + mutex_lock(&indio_dev->mlock);
> + for (i = 0; i < ST_SENSORS_ODR_LIST_MAX; i++) {
> + if (odr_avl[i].hz == 0)
> + break;
> +
> + len += sprintf(buf + len, "%d ", odr_avl[i].hz);
> + }
> + mutex_unlock(&indio_dev->mlock);
> + buf[len - 1] = '\n';
> +
> + return len;
> +}
> +EXPORT_SYMBOL(st_sensors_get_sampling_frequency_avl);
> +
> +int st_sensors_get_fullscale_avl(struct iio_dev *indio_dev,
> + const struct st_sensor_fullscale_avl *fullscale_avl, char *buf)
> +{
> + int i, len = 0;
> +
> + mutex_lock(&indio_dev->mlock);
> + for (i = 0; i < ST_SENSORS_FULLSCALE_AVL_MAX; i++) {
> + if (fullscale_avl[i].num == 0)
> + break;
> +
> + len += sprintf(buf + len, "0.%06u ", fullscale_avl[i].gain);
> + }
> + mutex_unlock(&indio_dev->mlock);
> + buf[len - 1] = '\n';
> +
> + return len;
> +}
> +EXPORT_SYMBOL(st_sensors_get_fullscale_avl);
> +
> +static int st_sensors_match_odr(const struct st_sensors *sensor,
> + unsigned int odr, struct st_sensor_odr_avl *odr_out)
> +{
> + int i, ret = -EINVAL;
> +
> + for (i = 0; i < ST_SENSORS_ODR_LIST_MAX; i++) {
> + if ((sensor->odr.odr_avl[i].hz == odr) &&
> + (sensor->odr.odr_avl[i].hz != 0)) {
> + odr_out->hz = sensor->odr.odr_avl[i].hz;
> + odr_out->value = sensor->odr.odr_avl[i].value;
> + ret = 0;
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +int st_sensors_set_odr(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, unsigned int odr)
> +{
> + int err;
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct st_sensor_odr_avl odr_out;
> +
> + err = st_sensors_match_odr(sensor, odr, &odr_out);
> + if (err < 0)
> + goto st_sensors_match_odr_error;
> +
> + if ((sensor->odr.addr == sensor->pw.addr) &&
> + (sensor->odr.mask == sensor->pw.mask)) {
> + if (sdata->enabled == true) {
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sensor->odr.addr, sensor->odr.mask,
> + sensor->odr.num_bit,
> + odr_out.value);
> + } else {
> + err = 0;
> + }
> + } else {
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sensor->odr.addr, sensor->odr.mask,
> + sensor->odr.num_bit, odr_out.value);
> + }
> + if (err >= 0)
> + sdata->odr = odr_out.hz;
> +
> +st_sensors_match_odr_error:
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_set_odr);
> +
> +static int st_sensors_match_fs(const struct st_sensors *sensor,
> + unsigned int fs, struct st_sensor_fullscale_avl *fs_out)
> +{
> + int i, ret = -EINVAL;
> +
> + for (i = 0; i < ST_SENSORS_FULLSCALE_AVL_MAX; i++) {
> + if ((sensor->fs.fs_avl[i].num == fs) &&
> + (sensor->fs.fs_avl[i].num != 0)) {
> + fs_out->num = sensor->fs.fs_avl[i].num;
> + fs_out->gain = sensor->fs.fs_avl[i].gain;
> + fs_out->value = sensor->fs.fs_avl[i].value;
why not
fs_out = &sensor->fs.fs_avl[i];
(obviously with the relevant changes to the call sites.)
> + ret = 0;
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +int st_sensors_set_fullscale(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, unsigned int fs)
> +{
> + int err;
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct st_sensor_fullscale_avl fs_out;
> +
> + err = st_sensors_match_fs(sensor, fs, &fs_out);
> + if (err < 0)
> + goto st_accel_set_fullscale_error;
> +
> + err = st_sensors_write_data_with_mask(indio_dev, sensor->fs.addr,
> + sensor->fs.mask, sensor->fs.num_bit, sensor->fs.fs_avl->value);
> + if (err < 0)
> + goto st_accel_set_fullscale_error;
> +
> + sdata->fullscale = sensor->fs.fs_avl->num;
> + sdata->gain = sensor->fs.fs_avl->gain;
> + sdata->gain2 = sensor->fs.fs_avl->gain2;
Is there any reason not to just have fs_avl in sdata
so have a struct st_sensor_fullscale_avl *fs_avl in sdata and
then just set the pointer here?
> + return err;
> +
> +st_accel_set_fullscale_error:
> + dev_err(&indio_dev->dev, "failed to set new fullscale.\n");
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_set_fullscale);
> +
> +int st_sensors_set_enable(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, bool enable)
> +{
> + int err = -EINVAL;
> + bool found;
> + u8 tmp_value;
> + struct st_sensor_odr_avl odr_out;
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + if (enable) {
> + found = false;
> + tmp_value = sensor->pw.value_on;
> + if ((sensor->odr.addr == sensor->pw.addr) &&
> + (sensor->odr.mask == sensor->pw.mask)) {
> + err = st_sensors_match_odr(sensor,
> + sdata->odr, &odr_out);
> + if (err < 0)
> + goto set_enable_error;
> + tmp_value = odr_out.value;
> + found = true;
> + }
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sensor->pw.addr, sensor->pw.mask,
> + sensor->pw.num_bit, tmp_value);
> + if (err < 0)
> + goto set_enable_error;
> + sdata->enabled = true;
> + if (found)
> + sdata->odr = odr_out.hz;
> + } else {
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sensor->pw.addr, sensor->pw.mask,
> + sensor->pw.num_bit, sensor->pw.value_off);
> + if (err < 0)
> + goto set_enable_error;
> + sdata->enabled = false;
> + }
> +
> +set_enable_error:
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_set_enable);
> +
> +int st_sensors_set_axis_enable(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, u8 axis_enable)
> +{
> + return st_sensors_write_data_with_mask(indio_dev,
> + sensor->enable_axis.addr, sensor->enable_axis.mask,
> + 3, axis_enable);
> +}
> +EXPORT_SYMBOL(st_sensors_set_axis_enable);
> +
> +int st_sensors_init_sensor(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor)
> +{
> + int err;
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + err = st_sensors_set_enable(indio_dev, sensor, false);
> + if (err < 0)
> + goto init_error;
> +
> + err = st_sensors_set_fullscale(indio_dev, sensor, sdata->fullscale);
> + if (err < 0)
> + goto init_error;
> +
> + err = st_sensors_set_odr(indio_dev, sensor, sdata->odr);
> + if (err < 0)
> + goto init_error;
> +
> + /* set BDU */
> + err = st_sensors_write_data_with_mask(indio_dev, sensor->bdu.addr,
> + sensor->bdu.mask, 1, true);
> + if (err < 0)
> + goto init_error;
> +
> + err = st_sensors_set_axis_enable(indio_dev, sensor,
> + ST_SENSORS_ENABLE_ALL_CHANNELS);
> +
> +init_error:
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_init_sensor);
> +
> +int st_sensors_set_dataready_irq(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, bool enable)
> +{
> + int err;
> +
I'd like to see some comments in here as the flow / naming is not entirely
obvious.
> + if (sensor->drdy_irq.ig1.en_addr > 0) {
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sensor->drdy_irq.ig1.en_addr,
> + sensor->drdy_irq.ig1.en_mask, 1, (int)enable);
> + if (err < 0)
> + goto st_accel_set_dataready_irq_error;
> + }
> +
> + if (sensor->drdy_irq.ig1.latch_mask_addr > 0) {
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sensor->drdy_irq.ig1.latch_mask_addr,
> + sensor->drdy_irq.ig1.latching_mask, 1,
> + (int)enable);
> + if (err < 0)
> + goto st_accel_set_dataready_irq_error;
> + }
> +
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sensor->drdy_irq.addr,
> + sensor->drdy_irq.mask, 1, (int)enable);
> +
> +st_accel_set_dataready_irq_error:
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_set_dataready_irq);
> +
> +int st_sensors_set_fullscale_by_gain(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, int scale)
> +{
> + int err = -EINVAL, i;
> +
> + for (i = 0; i < ST_SENSORS_FULLSCALE_AVL_MAX; i++) {
> + if ((sensor->fs.fs_avl[i].gain == scale) &&
> + (sensor->fs.fs_avl[i].gain != 0)) {
> + err = 0;
> + break;
> + }
> + }
> + if (err < 0)
> + goto st_sensors_match_scale_error;
> +
> + err = st_sensors_set_fullscale(indio_dev,
> + sensor, sensor->fs.fs_avl[i].num);
> +
> +st_sensors_match_scale_error:
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_set_fullscale_by_gain);
> +
> +int st_sensors_read_axis_data(struct iio_dev *indio_dev, u8 ch_addr, int *data)
> +{
> + int err;
> + u8 outdata[ST_SENSORS_BYTE_FOR_CHANNEL];
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + err = sdata->tf.read_multiple_byte(&sdata->tf, sdata->dev,
> + ch_addr, ST_SENSORS_BYTE_FOR_CHANNEL,
> + outdata, sdata->multiread_bit);
> + if (err < 0)
> + goto read_error;
> +
> + *data = ((s16)le16_to_cpup((u16 *)outdata));
> +
> +read_error:
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_read_axis_data);
> +
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca@xxxxxx>");
> +MODULE_DESCRIPTION("STMicroelectronics ST-sensors core");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> new file mode 100644
> index 0000000..fa4bc7d
> --- /dev/null
> +++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> @@ -0,0 +1,78 @@
> +/*
> + * STMicroelectronics sensors i2c library driver
> + *
> + * Copyright 2012 STMicroelectronics Inc.
> + *
> + * Denis Ciocca <denis.ciocca@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +
> +#include <linux/iio/common/st_sensors.h>
> +
> +
> +#define ST_SENSORS_I2C_MULTIREAD 0x80
> +
> +unsigned int st_sensors_i2c_get_irq(struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct i2c_client *client = to_i2c_client(sdata->dev);
> +
> + return client->irq;
> +}
> +
> +static int st_sensors_i2c_read_byte(struct st_sensor_transfer *st,
> + struct device *dev, u8 reg_addr, u8 *res_byte)
> +{
> + int err;
> +
> + err = i2c_smbus_read_byte_data(to_i2c_client(dev), reg_addr);
> + if (err < 0)
> + goto st_accel_i2c_read_byte_error;
> +
> + *res_byte = err & 0xff;
> +
> +st_accel_i2c_read_byte_error:
> + return err < 0 ? err : 0;
> +}
> +
> +static int st_sensors_i2c_read_multiple_byte(struct st_sensor_transfer *st,
> + struct device *dev, u8 reg_addr, int len, u8 *data, bool multiread_bit)
> +{
> + if (multiread_bit == true)
> + reg_addr |= ST_SENSORS_I2C_MULTIREAD;
> +
> + return i2c_smbus_read_i2c_block_data(to_i2c_client(dev),
> + reg_addr, len, data);
> +}
> +
> +static int st_sensors_i2c_write_byte(struct st_sensor_transfer *st,
> + struct device *dev, u8 reg_addr, u8 data)
> +{
> + return i2c_smbus_write_byte_data(to_i2c_client(dev), reg_addr, data);
> +}
> +
> +void st_sensors_i2c_configure(struct iio_dev *indio_dev,
> + struct i2c_client *client, struct st_sensor_data *sdata)
> +{
> + i2c_set_clientdata(client, indio_dev);
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->name = client->name;
> +
> + mutex_init(&sdata->tf.buf_lock);
> + sdata->tf.read_byte = st_sensors_i2c_read_byte;
> + sdata->tf.write_byte = st_sensors_i2c_write_byte;
> + sdata->tf.read_multiple_byte = st_sensors_i2c_read_multiple_byte;
> + sdata->get_irq_data_ready = st_sensors_i2c_get_irq;
I review backwards through code so the same comments as written for the
spi equivalent apply here...
> +}
> +EXPORT_SYMBOL(st_sensors_i2c_configure);
> +
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca@xxxxxx>");
> +MODULE_DESCRIPTION("STMicroelectronics ST-sensors i2c driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/common/st_sensors/st_sensors_spi.c b/drivers/iio/common/st_sensors/st_sensors_spi.c
> new file mode 100644
> index 0000000..82fa633
> --- /dev/null
> +++ b/drivers/iio/common/st_sensors/st_sensors_spi.c
> @@ -0,0 +1,124 @@
> +/*
> + * STMicroelectronics sensors spi library driver
> + *
> + * Copyright 2012 STMicroelectronics Inc.
> + *
> + * Denis Ciocca <denis.ciocca@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +
> +#include <linux/iio/common/st_sensors.h>
> +
> +
> +#define ST_SENSORS_SPI_MULTIREAD 0xc0
> +#define ST_SENSORS_SPI_READ 0x80
> +
> +unsigned int st_sensors_spi_get_irq(struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct spi_device *spi = to_spi_device(sdata->dev);
> +
> + return spi->irq;
I'd roll this up slightly and drop the intermediate variable.
return to_spi_device(sdata->dev)->irq;
> +}
> +
> +static int st_sensors_spi_read(struct st_sensor_transfer *st,
> + struct device *dev, u8 reg_addr, int len, u8 *data, bool multiread_bit)
> +{
> + struct spi_message msg;
> + int err;
> +
> + struct spi_transfer xfers[] = {
> + {
> + .tx_buf = st->tx_buf,
> + .bits_per_word = 8,
> + .len = 1,
> + },
> + {
> + .rx_buf = st->rx_buf,
> + .bits_per_word = 8,
> + .len = len,
> + }
> + };
> +
> + mutex_lock(&st->buf_lock);
> + if ((multiread_bit == true) && (len > 1))
> + st->tx_buf[0] = reg_addr | ST_SENSORS_SPI_MULTIREAD;
> + else
> + st->tx_buf[0] = reg_addr | ST_SENSORS_SPI_READ;
> +
> + spi_message_init(&msg);
> + spi_message_add_tail(&xfers[0], &msg);
> + spi_message_add_tail(&xfers[1], &msg);
> + err = spi_sync(to_spi_device(dev), &msg);
> + if (err)
> + goto acc_spi_read_error;
> +
> + memcpy(data, st->rx_buf, len*sizeof(u8));
> + mutex_unlock(&st->buf_lock);
> + return len;
> +
> +acc_spi_read_error:
> + return err;
> +}
> +
> +static int st_sensors_spi_read_byte(struct st_sensor_transfer *st,
> + struct device *dev, u8 reg_addr, u8 *res_byte)
> +{
> + return st_sensors_spi_read(st, dev, reg_addr, 1, res_byte, false);
> +}
> +
> +static int st_sensors_spi_read_multiple_byte(struct st_sensor_transfer *st,
> + struct device *dev, u8 reg_addr, int len, u8 *data, bool multiread_bit)
> +{
> + return st_sensors_spi_read(st, dev, reg_addr, len, data, multiread_bit);
> +}
> +
> +static int st_sensors_spi_write_byte(struct st_sensor_transfer *st,
> + struct device *dev, u8 reg_addr, u8 data)
> +{
> + struct spi_message msg;
> + int err;
> +
> + struct spi_transfer xfers = {
> + .tx_buf = st->tx_buf,
> + .bits_per_word = 8,
> + .len = 2,
> + };
> +
> + mutex_lock(&st->buf_lock);
> + st->tx_buf[0] = reg_addr;
> + st->tx_buf[1] = data;
> +
> + spi_message_init(&msg);
> + spi_message_add_tail(&xfers, &msg);
> + err = spi_sync(to_spi_device(dev), &msg);
> + mutex_unlock(&st->buf_lock);
> +
> + return err;
> +}
> +
> +void st_sensors_spi_configure(struct iio_dev *indio_dev,
> + struct spi_device *spi, struct st_sensor_data *sdata)
> +{
> + spi_set_drvdata(spi, indio_dev);
> +
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->name = spi->modalias;
> +
> + mutex_init(&sdata->tf.buf_lock);
Silly though it may seem I thik I'd prefer the mutex init in
st_sensors_init_sensor as it isn't spi or i2c specific whereas
everything else in here is...
> + sdata->tf.read_byte = st_sensors_spi_read_byte;
> + sdata->tf.write_byte = st_sensors_spi_write_byte;
> + sdata->tf.read_multiple_byte = st_sensors_spi_read_multiple_byte;
Might be worth wrapping these callbacks up in a const static structure rather
than along side the dynamic data in tf. (like we do with struct iio_info within
struct iio_dev)
> + sdata->get_irq_data_ready = st_sensors_spi_get_irq;
> +}
> +EXPORT_SYMBOL(st_sensors_spi_configure);
> +
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca@xxxxxx>");
> +MODULE_DESCRIPTION("STMicroelectronics ST-sensors spi driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> new file mode 100644
> index 0000000..6d43b13
> --- /dev/null
> +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> @@ -0,0 +1,76 @@
> +/*
> + * STMicroelectronics sensors trigger library driver
> + *
> + * Copyright 2012 STMicroelectronics Inc.
> + *
> + * Denis Ciocca <denis.ciocca@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/interrupt.h>
> +
> +#include <linux/iio/common/st_sensors.h>
> +
> +int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
> + const struct iio_trigger_ops *trig_ops)
> +{
> + int err;
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + sdata->trig = iio_trigger_alloc("%s-trigger", indio_dev->name);
> + if (sdata->trig == NULL) {
> + err = -ENOMEM;
> + dev_err(&indio_dev->dev, "failed to allocate iio trigger.\n");
> + goto iio_trigger_alloc_error;
> + }
> +
> + err = request_threaded_irq(sdata->get_irq_data_ready(indio_dev),
> + iio_trigger_generic_data_rdy_poll,
> + NULL,
> + IRQF_TRIGGER_RISING,
> + sdata->trig->name,
> + sdata->trig);
> + if (err)
> + goto request_irq_error;
> +
> + sdata->trig->private_data = indio_dev;
> + sdata->trig->ops = trig_ops;
> + sdata->trig->dev.parent = sdata->dev;
> +
> + err = iio_trigger_register(sdata->trig);
> + if (err < 0) {
> + dev_err(&indio_dev->dev, "failed to register iio trigger.\n");
> + goto iio_trigger_register_error;
> + }
> + indio_dev->trig = sdata->trig;
> +
> + return 0;
> +
> +iio_trigger_register_error:
> + free_irq(sdata->get_irq_data_ready(indio_dev), sdata->trig);
> +request_irq_error:
> + iio_trigger_free(sdata->trig);
> +iio_trigger_alloc_error:
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_allocate_trigger);
> +
> +void st_sensors_deallocate_trigger(struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + iio_trigger_unregister(sdata->trig);
> + free_irq(sdata->get_irq_data_ready(indio_dev), sdata->trig);
> + iio_trigger_free(sdata->trig);
> +}
> +EXPORT_SYMBOL(st_sensors_deallocate_trigger);
> +
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca@xxxxxx>");
> +MODULE_DESCRIPTION("STMicroelectronics ST-sensors trigger");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> new file mode 100644
> index 0000000..c0f2acc
> --- /dev/null
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -0,0 +1,246 @@
> +/*
> + * STMicroelectronics sensors library driver
> + *
> + * Copyright 2012 STMicroelectronics Inc.
> + *
> + * Denis Ciocca <denis.ciocca@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#ifndef ST_SENSORS_H
> +#define ST_SENSORS_H
> +
> +#include <linux/i2c.h>
> +#include <linux/spi/spi.h>
> +#include <linux/irqreturn.h>
> +#include <linux/iio/trigger.h>
> +
> +
> +#define ST_SENSORS_TX_MAX_LENGHT 2
> +#define ST_SENSORS_RX_MAX_LENGHT 6
> +
> +#define ST_SENSORS_ODR_LIST_MAX 10
> +#define ST_SENSORS_FULLSCALE_AVL_MAX 10
> +
> +#define ST_SENSORS_NUMBER_ALL_CHANNELS 4
> +#define ST_SENSORS_NUMBER_DATA_CHANNELS 3
> +#define ST_SENSORS_ENABLE_ALL_CHANNELS 0x07
> +#define ST_SENSORS_BYTE_FOR_CHANNEL 2
> +#define ST_SENSORS_SCAN_X 0
> +#define ST_SENSORS_SCAN_Y 1
> +#define ST_SENSORS_SCAN_Z 2
> +#define ST_SENSORS_DEFAULT_12_REALBITS 12
> +#define ST_SENSORS_DEFAULT_16_REALBITS 16
> +#define ST_SENSORS_DEFAULT_POWER_ON_VALUE 0x01
> +#define ST_SENSORS_DEFAULT_POWER_OFF_VALUE 0x00
> +#define ST_SENSORS_DEFAULT_WAI_ADDRESS 0x0f
> +#define ST_SENSORS_DEFAULT_AXIS_ADDR 0x20
> +#define ST_SENSORS_DEFAULT_AXIS_MASK 0x07
> +#define ST_SENSORS_DEFAULT_AXIS_N_BIT 3
> +
> +#define ST_SENSORS_LSM_CHANNELS(device_type, index, mod, endian, bits, addr) \
> +{ \
> + .type = device_type, \
> + .modified = 1, \
> + .info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | \
> + IIO_CHAN_INFO_SCALE_SEPARATE_BIT, \
> + .scan_index = index, \
> + .channel2 = mod, \
> + .address = addr, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = bits, \
> + .shift = 16 - bits, \
> + .storagebits = 16, \
> + .endianness = endian, \
> + }, \
> +}
> +
> +struct st_sensor_odr_avl {
> + unsigned int hz;
> + u8 value;
> +};
> +
> +struct st_sensor_odr {
> + u8 addr;
> + u8 mask;
> + short num_bit;
> + struct st_sensor_odr_avl odr_avl[ST_SENSORS_ODR_LIST_MAX];
> +};
> +
> +struct st_sensor_power {
> + u8 addr;
> + u8 mask;
> + unsigned short num_bit;
> + u8 value_off;
> + u8 value_on;
> +};
> +
> +struct st_sensor_axis {
> + u8 addr;
> + u8 mask;
> +};
> +
> +struct st_sensor_fullscale_avl {
> + unsigned int num;
> + u8 value;
> + unsigned int gain;
> + unsigned int gain2;
> +};
> +
> +struct st_sensor_fullscale {
> + u8 addr;
> + u8 mask;
> + unsigned short num_bit;
> + struct st_sensor_fullscale_avl fs_avl[ST_SENSORS_FULLSCALE_AVL_MAX];
> +};
> +
> +struct st_sensor_bdu {
> + u8 addr;
> + u8 mask;
> +};
> +
> +struct st_sensor_interrupt_generator {
> + u8 en_addr;
> + u8 latch_mask_addr;
> + u8 en_mask;
> + u8 latching_mask;
> +};
> +
> +struct st_sensor_data_ready_irq {
> + u8 addr;
> + u8 mask;
> + struct st_sensor_interrupt_generator ig1;
I'd just embed the definition in here
i.e. (assuming I remember the syntax correctly...)
struct st_sensor_data_read_irq {
u8 addr;
u8 mask;
struct {
u8 en_addr;
u8 latch_mask_addr;
u8 en_mask;
u8 latching_mask;
} ig1;
};
> +};
> +
> +/**
> + * struct st_sensor_transfer - ST sensor device I/O struct
> + * @buf_lock: Mutex to protect rx and tx buffers.
> + * @tx_buf: Buffer used by SPI transfer function to send data to the sensors.
> + * This buffer is used to avoid DMA not-aligned issue.
> + * @rx_buf: Buffer used by SPI transfer to receive data from sensors.
> + * This buffer is used to avoid DMA not-aligned issue.
> + * @read_byte: Function used to read one byte.
> + * @write_byte: Function used to write one byte.
> + * @read_multiple_byte: Function used to read multiple byte.
> + */
> +struct st_sensor_transfer {
> + struct mutex buf_lock;
Move these to the end of the structure and all is fine
(as long as this is the last structure in any containing one as well -
see below).
Also only need to force alignment of the tx_buf
(as the rx_buf will pack after it and any hardware
that corrupts that is plain mad)
> + u8 tx_buf[ST_SENSORS_TX_MAX_LENGHT] ____cacheline_aligned;
> + u8 rx_buf[ST_SENSORS_RX_MAX_LENGHT] ____cacheline_aligned;
> + int (*read_byte) (struct st_sensor_transfer *st, struct device *dev,
> + u8 reg_addr, u8 *res_byte);
> + int (*write_byte) (struct st_sensor_transfer *st, struct device *dev,
> + u8 reg_addr, u8 data);
> + int (*read_multiple_byte) (struct st_sensor_transfer *st,
> + struct device *dev, u8 reg_addr, int len, u8 *data,
> + bool multiread_bit);
> +};
> +
> +/**
> + * struct st_sensor_data - ST sensor device status
> + * @dev: Pointer to instance of struct device (I2C or SPI).
> + * @trig: The trigger in use by the core driver.
> + * @enabled: Status of the sensor (false->off, true->on).
> + * @multiread_bit: Use or not particular bit for [I2C/SPI] multiread.
> + * @index: Number used to point the sensor being used in the st_sensors struct.
> + * @buffer_data: Data used by buffer part.
> + * @fullscale: Maximum range of measure by the sensor.
> + * @gain: Sensitivity of the sensor [m/s^2/LSB].
> + * @odr: Output data rate of the sensor [Hz].
> + * @tf: Transfer function structure used by I/O operations.
> + */
> +struct st_sensor_data {
> + struct device *dev;
> + struct iio_trigger *trig;
> +
> + bool enabled;
> + bool multiread_bit;
> +
> + short index;
> +
> + char *buffer_data;
> +
> + unsigned int fullscale;
> + unsigned int gain;
> + unsigned int gain2;
> + unsigned int odr;
> +
This will need to also be at the end of this structure to ensure
that the below callback pointer doesn't end up on the same cacheline as
the tx_buf and rx_buf. A comment saying that this is a requirement here
will also make future bugs much less likely.
> + struct st_sensor_transfer tf;
> +
> + unsigned int (*get_irq_data_ready) (struct iio_dev *indio_dev);
> +};
> +
> +/**
> + * struct st_sensors - ST sensors list
> + * @wai: Contents of WhoAmI register.
> + * @ch: IIO channels for the sensor.
> + * @odr: Output data rate register and odr list available.
> + * @pw: Power register of the sensor.
> + * @enable_axis: Enable one or more axis of the sensor.
> + * @fs: Full scale register and fs list available.
> + * @bdu: Block data update register.
> + * @drdy_irq: Data ready register of the sensor.
> + * @multi_read_bit: Use or not particular bit for [I2C/SPI] multiread.
> + */
> +struct st_sensors {
> + u8 wai;
> + struct iio_chan_spec *ch;
> + struct st_sensor_odr odr;
> + struct st_sensor_power pw;
> + struct st_sensor_axis enable_axis;
> + struct st_sensor_fullscale fs;
> + struct st_sensor_bdu bdu;
> + struct st_sensor_data_ready_irq drdy_irq;
> + bool multi_read_bit;
> +};
> +
Either break these out to separate headers or deal with the fact that
spi or i2c might not be present in a given build with ifdef fun and
games.
> +void st_sensors_spi_configure(struct iio_dev *indio_dev,
> + struct spi_device *spi, struct st_sensor_data *sdata);
> +
> +void st_sensors_i2c_configure(struct iio_dev *indio_dev,
> + struct i2c_client *client, struct st_sensor_data *sdata);
> +
> +int st_sensors_write_data_with_mask(struct iio_dev *indio_dev, u8 reg_addr,
> + u8 mask, short num_bit, u8 data);
> +
> +int st_sensors_get_sampling_frequency_avl(struct iio_dev *indio_dev,
> + const struct st_sensor_odr_avl *odr_avl, char *buf);
> +
> +int st_sensors_get_fullscale_avl(struct iio_dev *indio_dev,
> + const struct st_sensor_fullscale_avl *fullscale_avl, char *buf);
> +
> +int st_sensors_set_odr(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, unsigned int odr);
> +
> +int st_sensors_set_fullscale(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, unsigned int fs);
> +
> +int st_sensors_set_enable(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, bool enable);
> +
> +int st_sensors_set_axis_enable(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, u8 axis_enable);
> +
> +int st_sensors_init_sensor(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor);
> +
> +int st_sensors_set_dataready_irq(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, bool enable);
> +
> +int st_sensors_set_fullscale_by_gain(struct iio_dev *indio_dev,
> + const struct st_sensors *sensor, int scale);
> +
> +int st_sensors_read_axis_data(struct iio_dev *indio_dev, u8 ch_addr, int *data);
> +
> +int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
> + const struct iio_trigger_ops *trig_ops);
> +
> +void st_sensors_deallocate_trigger(struct iio_dev *indio_dev);
> +
> +int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf);
> +
> +irqreturn_t st_sensors_trigger_handler(int irq, void *p);
> +
> +#endif /* ST_SENSORS_H */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
