Re: [PATCH 1/2] iio: light: add support to UVIS25 sensor

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

 




On Wed, 25 Oct 2017 20:16:08 +0200
Lorenzo Bianconi <lorenzo.bianconi83@xxxxxxxxx> wrote:

Really minor English comment to start.
add support 'for' UVIS25 sensor



> add support to STMicroelectronics UVIS25 uv sensor
> http://www.st.com/resource/en/datasheet/uvis25.pdf
> 
> - continuos mode support
> - i2c support
> - spi support
> - trigger mode support
> - system PM support

The autoincrement bit in the addresses is a little unusual, but
if feels like this could be neater done using regmap than
by spinning your own infrastructure.

If there is no nasty side effect in always setting autoincrement
then you should be fine just putting that in write_flag_mask and
read_flag_mask.

I've suggested an alternative for the interrupt handling.
Not sure it won't end up more hideous than what you have
but worth playing with perhaps.  Tryrenable was always
there for triggers to deal with odd race conditions
(originally it was a level interrupt connected to an 
edge sensitive pxa27x input on the first board I had ;)

Might work here at the cost of an extra few reads.

Jonathan
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
> ---
>  drivers/iio/light/Kconfig            |  22 +++
>  drivers/iio/light/Makefile           |   6 +
>  drivers/iio/light/st_uvis25.h        |  63 +++++++++
>  drivers/iio/light/st_uvis25_buffer.c | 147 +++++++++++++++++++
>  drivers/iio/light/st_uvis25_core.c   | 264 +++++++++++++++++++++++++++++++++++
>  drivers/iio/light/st_uvis25_i2c.c    |  76 ++++++++++
>  drivers/iio/light/st_uvis25_spi.c    | 109 +++++++++++++++
>  7 files changed, 687 insertions(+)
>  create mode 100644 drivers/iio/light/st_uvis25.h
>  create mode 100644 drivers/iio/light/st_uvis25_buffer.c
>  create mode 100644 drivers/iio/light/st_uvis25_core.c
>  create mode 100644 drivers/iio/light/st_uvis25_i2c.c
>  create mode 100644 drivers/iio/light/st_uvis25_spi.c
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 2356ed9285df..3c5492ec9df6 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -334,6 +334,28 @@ config STK3310
>  	 Choosing M will build the driver as a module. If so, the module
>  	 will be called stk3310.
>  
> +config ST_UVIS25
> +	tristate "STMicroelectronics UVIS25 sensor driver"
> +	depends on (I2C || SPI)
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	select ST_UVIS25_I2C if (I2C)
> +	select ST_UVIS25_SPI if (SPI_MASTER)
> +	help
> +	  Say yes here to build support for STMicroelectronics UVIS25
> +	  uv sensor
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called st_uvis25.
> +
> +config ST_UVIS25_I2C
> +	tristate
> +	depends on ST_UVIS25
> +
> +config ST_UVIS25_SPI
> +	tristate
> +	depends on ST_UVIS25
> +
>  config TCS3414
>  	tristate "TAOS TCS3414 digital color sensor"
>  	depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index fa32fa459e2e..971d316cba5f 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -32,6 +32,12 @@ obj-$(CONFIG_RPR0521)		+= rpr0521.o
>  obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
>  obj-$(CONFIG_SI1145)		+= si1145.o
>  obj-$(CONFIG_STK3310)          += stk3310.o
> +
> +st_uvis25-y := st_uvis25_core.o st_uvis25_buffer.o
Why bother with the split?  I'd just have one file for both of
these.  There isn't enough code to justify it for readability reasons.

I thought I was going to find it as a config dependency
(was going to suggest you always built if it was ;)

> +obj-$(CONFIG_ST_UVIS25)		+= st_uvis25.o
> +obj-$(CONFIG_ST_UVIS25_I2C)	+= st_uvis25_i2c.o
> +obj-$(CONFIG_ST_UVIS25_SPI)	+= st_uvis25_spi.o
> +
>  obj-$(CONFIG_TCS3414)		+= tcs3414.o
>  obj-$(CONFIG_TCS3472)		+= tcs3472.o
>  obj-$(CONFIG_TSL2583)		+= tsl2583.o
> diff --git a/drivers/iio/light/st_uvis25.h b/drivers/iio/light/st_uvis25.h
> new file mode 100644
> index 000000000000..d444a73e743f
> --- /dev/null
> +++ b/drivers/iio/light/st_uvis25.h
> @@ -0,0 +1,63 @@
> +/*
> + * STMicroelectronics uvis25 sensor driver
> + *
> + * Copyright 2017 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#ifndef ST_UVIS25_H
> +#define ST_UVIS25_H
> +
> +#define ST_UVIS25_DEV_NAME		"uvis25"
> +
> +#include <linux/iio/iio.h>
> +
> +#define ST_UVIS25_RX_MAX_LENGTH		8
> +#define ST_UVIS25_TX_MAX_LENGTH		8
> +
> +struct st_uvis25_transfer_buffer {
> +	u8 rx_buf[ST_UVIS25_RX_MAX_LENGTH];
> +	u8 tx_buf[ST_UVIS25_TX_MAX_LENGTH] ____cacheline_aligned;
> +};
> +
> +struct st_uvis25_transfer_function {
> +	int (*read)(struct device *dev, u8 addr, int len, u8 *data);
> +	int (*write)(struct device *dev, u8 addr, int len, u8 *data);
> +};
> +
> +/**
> + * struct st_uvis25_hw - ST UVIS25 sensor instance
> + * @dev: Pointer to instance of struct device (I2C or SPI).
> + * @lock: Mutex to protect read and write operations.
> + * @trig: The trigger in use by the driver.
> + * @enabled: Status of the sensor (false->off, true->on).
> + * @irq: Device interrupt line (I2C or SPI).
> + * @tf: Transfer function structure used by I/O operations.
> + * @tb: Transfer buffers used by SPI I/O operations.
> + */
> +struct st_uvis25_hw {
> +	struct device *dev;
> +
> +	struct mutex lock;
> +	struct iio_trigger *trig;
> +	bool enabled;
> +	int irq;
> +
> +	const struct st_uvis25_transfer_function *tf;
> +	struct st_uvis25_transfer_buffer tb;
> +};
> +
> +extern const struct dev_pm_ops st_uvis25_pm_ops;
> +
> +int st_uvis25_write_with_mask(struct st_uvis25_hw *hw, u8 addr,
> +			      u8 mask, u8 val);
> +int st_uvis25_set_enable(struct st_uvis25_hw *hw, bool enable);
> +int st_uvis25_probe(struct device *dev, int irq,
> +		    const struct st_uvis25_transfer_function *tf_ops);
> +int st_uvis25_allocate_buffer(struct iio_dev *iio_dev);
> +int st_uvis25_allocate_trigger(struct iio_dev *iio_dev);
> +
> +#endif /* ST_UVIS25_H */
> diff --git a/drivers/iio/light/st_uvis25_buffer.c b/drivers/iio/light/st_uvis25_buffer.c
> new file mode 100644
> index 000000000000..06b95287ca98
> --- /dev/null
> +++ b/drivers/iio/light/st_uvis25_buffer.c
> @@ -0,0 +1,147 @@
> +/*
> + * STMicroelectronics uvis25 sensor driver
> + *
> + * Copyright 2017 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/buffer.h>
> +
> +#include "st_uvis25.h"
> +
> +#define ST_UVIS25_REG_CTRL3_ADDR	0x22
> +#define ST_UVIS25_REG_HL_MASK		BIT(7)
> +#define ST_UVIS25_REG_STATUS_ADDR	0x27
> +#define ST_UVIS25_REG_UV_DA_MASK	BIT(0)
> +
> +static irqreturn_t st_uvis25_trigger_handler_thread(int irq, void *private)
> +{
> +	struct st_uvis25_hw *hw = private;
> +	u8 status;
> +	int err;
> +
> +	err = hw->tf->read(hw->dev, ST_UVIS25_REG_STATUS_ADDR, sizeof(status),
> +			   &status);
> +	if (err < 0)
> +		return IRQ_HANDLED;
> +
> +	if (!(status & ST_UVIS25_REG_UV_DA_MASK))
> +		return IRQ_NONE;
> +
> +	iio_trigger_poll_chained(hw->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +int st_uvis25_allocate_trigger(struct iio_dev *iio_dev)
> +{
> +	struct st_uvis25_hw *hw = iio_priv(iio_dev);
> +	bool irq_active_low = false;
> +	unsigned long irq_type;
> +	int err;
> +
> +	irq_type = irqd_get_trigger_type(irq_get_irq_data(hw->irq));
> +
> +	switch (irq_type) {
> +	case IRQF_TRIGGER_HIGH:
> +	case IRQF_TRIGGER_RISING:
> +		break;
> +	case IRQF_TRIGGER_LOW:
> +	case IRQF_TRIGGER_FALLING:
> +		irq_active_low = true;
> +		break;
> +	default:
> +		dev_info(hw->dev,
> +			 "mode %lx unsupported, using IRQF_TRIGGER_RISING\n",
> +			 irq_type);
> +		irq_type = IRQF_TRIGGER_RISING;
> +		break;
> +	}
> +
> +	err = st_uvis25_write_with_mask(hw, ST_UVIS25_REG_CTRL3_ADDR,
> +					ST_UVIS25_REG_HL_MASK, irq_active_low);
> +	if (err < 0)
> +		return err;
> +
> +	err = devm_request_threaded_irq(hw->dev, hw->irq, NULL,
> +					st_uvis25_trigger_handler_thread,
> +					irq_type | IRQF_ONESHOT,
> +					iio_dev->name, hw);
> +	if (err) {
> +		dev_err(hw->dev, "failed to request trigger irq %d\n",
> +			hw->irq);
> +		return err;
> +	}
> +
> +	hw->trig = devm_iio_trigger_alloc(hw->dev, "%s-trigger",
> +					  iio_dev->name);
> +	if (!hw->trig)
> +		return -ENOMEM;
> +
> +	iio_trigger_set_drvdata(hw->trig, iio_dev);
> +	hw->trig->dev.parent = hw->dev;
> +
> +	return devm_iio_trigger_register(hw->dev, hw->trig);
> +}
> +
> +static int st_uvis25_buffer_preenable(struct iio_dev *iio_dev)
> +{
> +	return st_uvis25_set_enable(iio_priv(iio_dev), true);
> +}
> +
> +static int st_uvis25_buffer_postdisable(struct iio_dev *iio_dev)
> +{
> +	return st_uvis25_set_enable(iio_priv(iio_dev), false);
> +}
> +
> +static const struct iio_buffer_setup_ops st_uvis25_buffer_ops = {
> +	.preenable = st_uvis25_buffer_preenable,
> +	.postenable = iio_triggered_buffer_postenable,
> +	.predisable = iio_triggered_buffer_predisable,
> +	.postdisable = st_uvis25_buffer_postdisable,
> +};
> +
> +static irqreturn_t st_uvis25_buffer_handler_thread(int irq, void *p)
> +{
> +	u8 buffer[ALIGN(sizeof(u8), sizeof(s64)) + sizeof(s64)];
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *iio_dev = pf->indio_dev;
> +	struct st_uvis25_hw *hw = iio_priv(iio_dev);
> +	int err;
> +
> +	err = hw->tf->read(hw->dev, iio_dev->channels[0].address, sizeof(u8),
> +			   buffer);
> +	if (err < 0)
> +		goto out;
> +
> +	iio_push_to_buffers_with_timestamp(iio_dev, buffer,
> +					   iio_get_time_ns(iio_dev));
> +
> +out:
> +	iio_trigger_notify_done(hw->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +int st_uvis25_allocate_buffer(struct iio_dev *iio_dev)
> +{
> +	struct st_uvis25_hw *hw = iio_priv(iio_dev);
> +
> +	return devm_iio_triggered_buffer_setup(hw->dev, iio_dev, NULL,
> +					       st_uvis25_buffer_handler_thread,
> +					       &st_uvis25_buffer_ops);
> +}
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>");
> +MODULE_DESCRIPTION("STMicroelectronics uvis25 buffer driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/light/st_uvis25_core.c b/drivers/iio/light/st_uvis25_core.c
> new file mode 100644
> index 000000000000..08247092dfff
> --- /dev/null
> +++ b/drivers/iio/light/st_uvis25_core.c
> @@ -0,0 +1,264 @@
> +/*
> + * STMicroelectronics uvis25 sensor driver
> + *
> + * Copyright 2017 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/delay.h>
> +#include <linux/pm.h>
> +#include <linux/interrupt.h>
> +
> +#include "st_uvis25.h"
> +
> +#define ST_UVIS25_REG_WHOAMI_ADDR	0x0f
> +#define ST_UVIS25_REG_WHOAMI_VAL	0xca
> +#define ST_UVIS25_REG_CTRL1_ADDR	0x20
> +#define ST_UVIS25_REG_ODR_MASK		BIT(0)
> +#define ST_UVIS25_REG_BDU_MASK		BIT(1)
> +#define ST_UVIS25_REG_CTRL2_ADDR	0x21
> +#define ST_UVIS25_REG_BOOT_MASK		BIT(7)
> +
> +static const struct iio_chan_spec st_uvis25_channels[] = {
> +	{
> +		.type = IIO_UVINDEX,
> +		.address = 0x28,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 8,
> +			.storagebits = 8,
> +		},
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
> +static int st_uvis25_check_whoami(struct st_uvis25_hw *hw)
> +{
> +	u8 data;
> +	int err;
> +
> +	err = hw->tf->read(hw->dev, ST_UVIS25_REG_WHOAMI_ADDR, sizeof(data),
> +			   &data);
> +	if (err < 0) {
> +		dev_err(hw->dev, "failed to read whoami register\n");
> +		return err;
> +	}
> +
> +	if (data != ST_UVIS25_REG_WHOAMI_VAL) {
> +		dev_err(hw->dev, "wrong whoami {%02x vs %02x}\n",
> +			data, ST_UVIS25_REG_WHOAMI_VAL);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +int st_uvis25_write_with_mask(struct st_uvis25_hw *hw, u8 addr, u8 mask, u8 val)
> +{
> +	u8 data;
> +	int err;
> +
> +	mutex_lock(&hw->lock);

This stuff comes for free in regmap... 

> +
> +	err = hw->tf->read(hw->dev, addr, sizeof(data), &data);
> +	if (err < 0) {
> +		dev_err(hw->dev, "failed to read %02x register\n", addr);
> +		goto unlock;
> +	}
> +
> +	data = (data & ~mask) | ((val << __ffs(mask)) & mask);
> +
> +	err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
> +	if (err < 0)
> +		dev_err(hw->dev, "failed to write %02x register\n", addr);
> +
> +unlock:
> +	mutex_unlock(&hw->lock);
> +
> +	return err < 0 ? err : 0;
> +}
> +
> +int st_uvis25_set_enable(struct st_uvis25_hw *hw, bool enable)
> +{
> +	int err;
> +
> +	err = st_uvis25_write_with_mask(hw, ST_UVIS25_REG_CTRL1_ADDR,
> +					ST_UVIS25_REG_ODR_MASK, enable);
> +	if (err < 0)
> +		return err;
> +
> +	hw->enabled = enable;
> +
> +	return 0;
> +}
> +
> +static int st_uvis25_read_oneshot(struct st_uvis25_hw *hw, u8 addr, int *val)
> +{
> +	u8 data;
> +	int err;
> +
> +	err = st_uvis25_set_enable(hw, true);
> +	if (err < 0)
> +		return err;
> +
> +	msleep(1500);

Could drive this off the interrupt rather than disabling the interrupt?
Would that be a little neater (simple completion here).

> +
> +	err = hw->tf->read(hw->dev, addr, sizeof(data), &data);
> +	if (err < 0)
> +		return err;

Is there a potential race here if for some reason we managed to
got to sleep for another conversion?  I think to be completely
safe you need force an additional read after the disable (or
will that fail to clear the data ready?

> +
> +	st_uvis25_set_enable(hw, false);
> +
> +	*val = data;
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int st_uvis25_read_raw(struct iio_dev *iio_dev,
> +			      struct iio_chan_spec const *ch,
> +			      int *val, int *val2, long mask)
> +{
> +	int ret;
> +
> +	ret = iio_device_claim_direct_mode(iio_dev);
> +	if (ret)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_PROCESSED: {
> +		struct st_uvis25_hw *hw = iio_priv(iio_dev);
> +
> +		/*
> +		 * mask irq line during oneshot read since the sensor
> +		 * does not export the capability to disable data-ready line
> +		 * in the register map and it is enabled by default.
> +		 * If the line is unmasked during read_raw() it will be set
> +		 * active and never reset since the trigger is disabled
> +		 */

Nasty but well documented...

I wonder if there is a nicer way to handle this... If we leave it on
the issues is that we end up with the status being checked by the interrupt
handler for the trigger (harmless if a waste of time) then the trigger
being fired (with nothing associated with it).  No consumers are attached
so we call notify done for all of them and finally that will result in
a try reenable. You could supply one of those that results in a read
to the device.  It think that would always deal with your case of
the data ready getting stuck high..

(Not totally sure though as it's been a while since I dealt with a
sensor with this particular issue).

> +		if (hw->irq > 0)
> +			disable_irq(hw->irq);
> +		ret = st_uvis25_read_oneshot(hw, ch->address, val);
> +		if (hw->irq > 0)
> +			enable_irq(hw->irq);
> +		break;
> +	}
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	iio_device_release_direct_mode(iio_dev);
> +
> +	return ret;
> +}
> +
> +static const struct iio_info st_uvis25_info = {
> +	.read_raw = st_uvis25_read_raw,
> +};
> +
> +static const unsigned long st_uvis25_scan_masks[] = { 0x1, 0x0 };
> +
> +static int st_uvis25_init_sensor(struct st_uvis25_hw *hw)
> +{
> +	int err;
> +
> +	err = st_uvis25_write_with_mask(hw, ST_UVIS25_REG_CTRL2_ADDR,
> +					ST_UVIS25_REG_BOOT_MASK, 1);
> +	if (err < 0)
> +		return err;
> +
> +	msleep(2000);
> +
> +	return st_uvis25_write_with_mask(hw, ST_UVIS25_REG_CTRL1_ADDR,
> +					 ST_UVIS25_REG_BDU_MASK, 1);
> +}
> +
> +int st_uvis25_probe(struct device *dev, int irq,
> +		    const struct st_uvis25_transfer_function *tf_ops)
> +{
> +	struct st_uvis25_hw *hw;
> +	struct iio_dev *iio_dev;
> +	int err;
> +
> +	iio_dev = devm_iio_device_alloc(dev, sizeof(*hw));
> +	if (!iio_dev)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(dev, (void *)iio_dev);
> +
> +	hw = iio_priv(iio_dev);
> +	hw->dev = dev;
> +	hw->irq = irq;
> +	hw->tf = tf_ops;
> +
> +	mutex_init(&hw->lock);
> +
> +	err = st_uvis25_check_whoami(hw);
> +	if (err < 0)
> +		return err;
> +
> +	iio_dev->modes = INDIO_DIRECT_MODE;
> +	iio_dev->dev.parent = hw->dev;
> +	iio_dev->available_scan_masks = st_uvis25_scan_masks;
> +	iio_dev->channels = st_uvis25_channels;
> +	iio_dev->num_channels = ARRAY_SIZE(st_uvis25_channels);
> +	iio_dev->name = ST_UVIS25_DEV_NAME;
> +	iio_dev->info = &st_uvis25_info;
> +
> +	err = st_uvis25_init_sensor(hw);
> +	if (err < 0)
> +		return err;
> +
> +	if (hw->irq > 0) {
> +		err = st_uvis25_allocate_buffer(iio_dev);
> +		if (err < 0)
> +			return err;
> +
> +		err = st_uvis25_allocate_trigger(iio_dev);
> +		if (err)
> +			return err;
> +	}
> +
> +	return devm_iio_device_register(hw->dev, iio_dev);
> +}
> +EXPORT_SYMBOL(st_uvis25_probe);
> +
> +static int __maybe_unused st_uvis25_suspend(struct device *dev)
> +{
> +	struct iio_dev *iio_dev = dev_get_drvdata(dev);
> +	struct st_uvis25_hw *hw = iio_priv(iio_dev);
> +
> +	return st_uvis25_write_with_mask(hw, ST_UVIS25_REG_CTRL1_ADDR,
> +					 ST_UVIS25_REG_ODR_MASK, 0);
> +}
> +
> +static int __maybe_unused st_uvis25_resume(struct device *dev)
> +{
> +	struct iio_dev *iio_dev = dev_get_drvdata(dev);
> +	struct st_uvis25_hw *hw = iio_priv(iio_dev);
> +	int err = 0;
> +
> +	if (hw->enabled)
> +		err = st_uvis25_write_with_mask(hw, ST_UVIS25_REG_CTRL1_ADDR,
> +						ST_UVIS25_REG_ODR_MASK, 1);
> +
> +	return err;
> +}
> +
> +const struct dev_pm_ops st_uvis25_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(st_uvis25_suspend, st_uvis25_resume)
> +};
> +EXPORT_SYMBOL(st_uvis25_pm_ops);
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>");
> +MODULE_DESCRIPTION("STMicroelectronics uvis25 sensor driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/light/st_uvis25_i2c.c b/drivers/iio/light/st_uvis25_i2c.c
> new file mode 100644
> index 000000000000..0d70d866a190
> --- /dev/null
> +++ b/drivers/iio/light/st_uvis25_i2c.c
> @@ -0,0 +1,76 @@
> +/*
> + * STMicroelectronics uvis25 i2c driver
> + *
> + * Copyright 2017 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +
> +#include "st_uvis25.h"
> +
> +#define I2C_AUTO_INCREMENT	0x80
> +
> +static int st_uvis25_i2c_read(struct device *dev, u8 addr, int len, u8 *data)
> +{
> +	if (len > 1)
> +		addr |= I2C_AUTO_INCREMENT;
> +
> +	return i2c_smbus_read_i2c_block_data_or_emulated(to_i2c_client(dev),
> +							 addr, len, data);
> +}
> +
> +static int st_uvis25_i2c_write(struct device *dev, u8 addr, int len, u8 *data)
> +{
> +	if (len > 1)
> +		addr |= I2C_AUTO_INCREMENT;
> +
> +	return i2c_smbus_write_i2c_block_data(to_i2c_client(dev), addr,
> +					      len, data);
> +}
> +
> +static const struct st_uvis25_transfer_function st_uvis25_transfer_fn = {
> +	.read = st_uvis25_i2c_read,
> +	.write = st_uvis25_i2c_write,
> +};
> +
> +static int st_uvis25_i2c_probe(struct i2c_client *client,
> +			       const struct i2c_device_id *id)
> +{
> +	return st_uvis25_probe(&client->dev, client->irq,
> +			       &st_uvis25_transfer_fn);
> +}
> +
> +static const struct of_device_id st_uvis25_i2c_of_match[] = {
> +	{ .compatible = "st,uvis25", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, st_uvis25_i2c_of_match);
> +
> +static const struct i2c_device_id st_uvis25_i2c_id_table[] = {
> +	{ ST_UVIS25_DEV_NAME },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, st_uvis25_i2c_id_table);
> +
> +static struct i2c_driver st_uvis25_driver = {
> +	.driver = {
> +		.name = "st_uvis25_i2c",
> +		.pm = &st_uvis25_pm_ops,
> +		.of_match_table = of_match_ptr(st_uvis25_i2c_of_match),
> +	},
> +	.probe = st_uvis25_i2c_probe,
> +	.id_table = st_uvis25_i2c_id_table,
> +};
> +module_i2c_driver(st_uvis25_driver);
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>");
> +MODULE_DESCRIPTION("STMicroelectronics uvis25 i2c driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/light/st_uvis25_spi.c b/drivers/iio/light/st_uvis25_spi.c
> new file mode 100644
> index 000000000000..be67d9e7564b
> --- /dev/null
> +++ b/drivers/iio/light/st_uvis25_spi.c
> @@ -0,0 +1,109 @@
> +/*
> + * STMicroelectronics uvis25 spi driver
> + *
> + * Copyright 2017 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/slab.h>
> +
> +#include "st_uvis25.h"
> +
> +#define SENSORS_SPI_READ	0x80
> +#define SPI_AUTO_INCREMENT	0x40
> +
> +static int st_uvis25_spi_read(struct device *dev, u8 addr, int len, u8 *data)

Hmm.. Maybe a good case for regmap?

> +{
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct iio_dev *iio_dev = spi_get_drvdata(spi);
> +	struct st_uvis25_hw *hw = iio_priv(iio_dev);
> +	int err;
> +
> +	struct spi_transfer xfers[] = {
> +		{
> +			.tx_buf = hw->tb.tx_buf,
> +			.bits_per_word = 8,
> +			.len = 1,
> +		},
> +		{
> +			.rx_buf = hw->tb.rx_buf,
> +			.bits_per_word = 8,
> +			.len = len,
> +		}
> +	};
> +
> +	if (len > 1)
> +		addr |= SPI_AUTO_INCREMENT;
> +	hw->tb.tx_buf[0] = addr | SENSORS_SPI_READ;
> +
> +	err = spi_sync_transfer(spi, xfers,  ARRAY_SIZE(xfers));
> +	if (err < 0)
> +		return err;
> +
> +	memcpy(data, hw->tb.rx_buf, len * sizeof(u8));
> +
> +	return len;
> +}
> +
> +static int st_uvis25_spi_write(struct device *dev, u8 addr, int len, u8 *data)
> +{
> +	struct iio_dev *iio_dev;
> +	struct st_uvis25_hw *hw;
> +	struct spi_device *spi;
> +
> +	if (len >= ST_UVIS25_TX_MAX_LENGTH)
> +		return -ENOMEM;
> +
> +	spi = to_spi_device(dev);
> +	iio_dev = spi_get_drvdata(spi);
> +	hw = iio_priv(iio_dev);
Could could just explicitly have the elements of this chain
that are used in local variables.

hw = iio_priv(spi_get_drvdata(spi));

up to you though..
> +
> +	hw->tb.tx_buf[0] = addr;
> +	memcpy(&hw->tb.tx_buf[1], data, len);
> +
> +	return spi_write(spi, hw->tb.tx_buf, len + 1);
> +}
> +
> +static const struct st_uvis25_transfer_function st_uvis25_transfer_fn = {
> +	.read = st_uvis25_spi_read,
> +	.write = st_uvis25_spi_write,
> +};
> +
> +static int st_uvis25_spi_probe(struct spi_device *spi)
> +{
> +	return st_uvis25_probe(&spi->dev, spi->irq,
> +			       &st_uvis25_transfer_fn);
> +}
> +
> +static const struct of_device_id st_uvis25_spi_of_match[] = {
> +	{ .compatible = "st,uvis25", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, st_uvis25_spi_of_match);
> +
> +static const struct spi_device_id st_uvis25_spi_id_table[] = {
> +	{ ST_UVIS25_DEV_NAME },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, st_uvis25_spi_id_table);
> +
> +static struct spi_driver st_uvis25_driver = {
> +	.driver = {
> +		.name = "st_uvis25_spi",
> +		.pm = &st_uvis25_pm_ops,
> +		.of_match_table = of_match_ptr(st_uvis25_spi_of_match),
> +	},
> +	.probe = st_uvis25_spi_probe,
> +	.id_table = st_uvis25_spi_id_table,
> +};
> +module_spi_driver(st_uvis25_driver);
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>");
> +MODULE_DESCRIPTION("STMicroelectronics uvis25 spi driver");
> +MODULE_LICENSE("GPL v2");

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux