On 26/09/16 20:55, Lorenzo Bianconi wrote: > Add support to STM HTS221 humidity + temperature sensor > > http://www.st.com/resource/en/datasheet/hts221.pdf > > - continuos mode support > - i2c support > - spi support > - trigger mode support > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@xxxxxx> This is an odd device for something so seemingly simple. It would have been helpful to have a little bit of text explaining that here. Key point is that the two sensors both operate with data ready interrupts, but not necessarily at the same frequency. Hence the multiple IIO devices. Why it makes sense to bother doing this in a device that runs at a 'maximum' of 12.5Hz is beyond me but that's what the hardware does ;( Hmm. Good old hardware designers make something lovely and flexible. Actual result, a bit of hardware that doesn't generate data in a form that anyone actually wants to use. Normally you explicitly want temperature and humidity to be paired. One really stupid question that I can't figure out from the datasheet. Do the average values effect the output rate? (or are they a moving window). I think I'd have been cynical on this device and just not supported triggered / buffered reading at all. Used it in oneshot mode. It's slow, so not as though we need to do buffered data flows. Still if you want to do it the hard way then fair enough! Jonathan > --- > drivers/iio/humidity/Kconfig | 2 + > drivers/iio/humidity/Makefile | 1 + > drivers/iio/humidity/hts221/Kconfig | 23 + > drivers/iio/humidity/hts221/Makefile | 6 + > drivers/iio/humidity/hts221/hts221.h | 106 ++++ > drivers/iio/humidity/hts221/hts221_buffer.c | 227 ++++++++ > drivers/iio/humidity/hts221/hts221_core.c | 804 ++++++++++++++++++++++++++++ > drivers/iio/humidity/hts221/hts221_i2c.c | 135 +++++ > drivers/iio/humidity/hts221/hts221_spi.c | 148 +++++ > 9 files changed, 1452 insertions(+) > create mode 100644 drivers/iio/humidity/hts221/Kconfig > create mode 100644 drivers/iio/humidity/hts221/Makefile > create mode 100644 drivers/iio/humidity/hts221/hts221.h > create mode 100644 drivers/iio/humidity/hts221/hts221_buffer.c > create mode 100644 drivers/iio/humidity/hts221/hts221_core.c > create mode 100644 drivers/iio/humidity/hts221/hts221_i2c.c > create mode 100644 drivers/iio/humidity/hts221/hts221_spi.c > > diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig > index b17e2e2..4ce75da 100644 > --- a/drivers/iio/humidity/Kconfig > +++ b/drivers/iio/humidity/Kconfig > @@ -69,4 +69,6 @@ config SI7020 > To compile this driver as a module, choose M here: the module > will be called si7020. > > +source "drivers/iio/humidity/hts221/Kconfig" > + > endmenu > diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile > index 4a73442..bdd5cd0 100644 > --- a/drivers/iio/humidity/Makefile > +++ b/drivers/iio/humidity/Makefile > @@ -8,3 +8,4 @@ obj-$(CONFIG_HDC100X) += hdc100x.o > obj-$(CONFIG_HTU21) += htu21.o > obj-$(CONFIG_SI7005) += si7005.o > obj-$(CONFIG_SI7020) += si7020.o > +obj-$(CONFIG_IIO_HTS221) += hts221/ > diff --git a/drivers/iio/humidity/hts221/Kconfig b/drivers/iio/humidity/hts221/Kconfig > new file mode 100644 > index 0000000..95b40e0 > --- /dev/null > +++ b/drivers/iio/humidity/hts221/Kconfig > @@ -0,0 +1,23 @@ > + > +config IIO_HTS221 > + tristate "STMicroelectronics HTS221 sensor Driver" > + depends on (I2C || SPI) && SYSFS > + select IIO_BUFFER > + select IIO_TRIGGERED_BUFFER > + select IIO_HTS221_I2C if (I2C) > + select IIO_HTS221_SPI if (SPI_MASTER) > + help > + Say yes here to build support for STMicroelectronics HTS221 > + temperature-humidity sensor > + > + To compile this driver as a module, choose M here: the module > + will be called hts221. > + > +config IIO_HTS221_I2C > + tristate > + depends on IIO_HTS221 > + > +config IIO_HTS221_SPI > + tristate > + depends on IIO_HTS221 > + > diff --git a/drivers/iio/humidity/hts221/Makefile b/drivers/iio/humidity/hts221/Makefile > new file mode 100644 > index 0000000..6fdfc6a > --- /dev/null > +++ b/drivers/iio/humidity/hts221/Makefile > @@ -0,0 +1,6 @@ > +hts221-y := hts221_core.o > +hts221-$(CONFIG_IIO_BUFFER) += hts221_buffer.o > + > +obj-$(CONFIG_IIO_HTS221) += hts221.o > +obj-$(CONFIG_IIO_HTS221_I2C) += hts221_i2c.o > +obj-$(CONFIG_IIO_HTS221_SPI) += hts221_spi.o > diff --git a/drivers/iio/humidity/hts221/hts221.h b/drivers/iio/humidity/hts221/hts221.h > new file mode 100644 > index 0000000..23cf5b2 > --- /dev/null > +++ b/drivers/iio/humidity/hts221/hts221.h > @@ -0,0 +1,106 @@ > +/* > + * STMicroelectronics hts221 sensor driver > + * > + * Copyright 2016 STMicroelectronics Inc. > + * > + * Lorenzo Bianconi <lorenzo.bianconi@xxxxxx> > + * > + * Licensed under the GPL-2. > + */ > + > +#ifndef HTS221_H > +#define HTS221_H > + > +#define HTS221_DEV_NAME "hts221" > + > +#include <linux/iio/iio.h> > + > +#if defined(CONFIG_IIO_HTS221_SPI) || \ > + defined(CONFIG_IIO_HTS221_SPI_MODULE) > +#define HTS221_RX_MAX_LENGTH 500 > +#define HTS221_TX_MAX_LENGTH 500 > + > +struct hts221_transfer_buffer { > + u8 rx_buf[HTS221_RX_MAX_LENGTH]; > + u8 tx_buf[HTS221_TX_MAX_LENGTH] ____cacheline_aligned; > +}; > +#endif /* CONFIG_IIO_HTS221_SPI */ > + > +struct hts221_transfer_function { > + int (*read)(struct device *dev, u8 addr, int len, u8 *data); > + int (*write)(struct device *dev, u8 addr, int len, u8 *data); > +}; > + > +#define HTS221_AVG_DEPTH 8 > +struct hts221_avg_avl { > + u16 avg; > + u8 val; > +}; > + > +enum hts221_sensor_type { > + HTS221_SENSOR_H, > + HTS221_SENSOR_T, > + HTS221_SENSOR_MAX, > +}; > + > +struct hts221_sensor { > + struct hts221_dev *dev; > + struct iio_trigger *trig; > + > + enum hts221_sensor_type type; > + bool enabled; > + u8 odr, cur_avg_idx; > + int slope, b_gen; > + > + u8 drdy_data_mask; > + u8 buffer[2]; > +}; > + > +struct hts221_dev { > + const char *name; > + struct device *dev; > + int irq; > + struct mutex lock; > + > + struct iio_dev *iio_devs[HTS221_SENSOR_MAX]; > + > + s64 hw_timestamp; > + > + const struct hts221_transfer_function *tf; > +#if defined(CONFIG_IIO_HTS221_SPI) || \ > + defined(CONFIG_IIO_HTS221_SPI_MODULE) > + struct hts221_transfer_buffer tb; > +#endif /* CONFIG_IIO_HTS221_SPI */ I'd be cynical. It's tiny, leave this there even in the i2c case... > +}; > + > +int hts221_config_drdy(struct hts221_sensor *sensor, bool enable); > +int hts221_probe(struct hts221_dev *dev); > +int hts221_remove(struct hts221_dev *dev); > +int hts221_sensor_power_on(struct hts221_sensor *sensor); > +int hts221_sensor_power_off(struct hts221_sensor *sensor); > +#ifdef CONFIG_IIO_BUFFER > +int hts221_allocate_buffers(struct hts221_dev *dev); > +void hts221_deallocate_buffers(struct hts221_dev *dev); > +int hts221_allocate_triggers(struct hts221_dev *dev); > +void hts221_deallocate_triggers(struct hts221_dev *dev); > +#else > +static inline int hts221_allocate_buffers(struct hts221_dev *dev) > +{ > + return 0; > +} > + > +static inline void hts221_deallocate_buffers(struct hts221_dev *dev) > +{ > +} > + > +static inline int hts221_allocate_triggers(struct hts221_dev *dev) > +{ > + return 0; > +} > + > +static inline void hts221_deallocate_triggers(struct hts221_dev *dev) > +{ > +} > +#endif /* CONFIG_IIO_BUFFER */ > + > +#endif /* HTS221_H */ > diff --git a/drivers/iio/humidity/hts221/hts221_buffer.c b/drivers/iio/humidity/hts221/hts221_buffer.c > new file mode 100644 > index 0000000..31ac29c > --- /dev/null > +++ b/drivers/iio/humidity/hts221/hts221_buffer.c > @@ -0,0 +1,227 @@ > +/* > + * STMicroelectronics hts221 sensor driver > + * > + * Copyright 2016 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/iio.h> > +#include <linux/iio/trigger.h> > +#include <linux/interrupt.h> > +#include <linux/iio/events.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/triggered_buffer.h> > +#include <linux/iio/buffer.h> > + > +#include "hts221.h" > + > +#define REG_STATUS_ADDR 0x27 > + > +int hts221_trig_set_state(struct iio_trigger *trig, bool state) > +{ > + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig); > + struct hts221_sensor *sensor = iio_priv(indio_dev); > + > + return hts221_config_drdy(sensor, state); > +} > + > +static const struct iio_trigger_ops hts221_trigger_ops = { > + .owner = THIS_MODULE, > + .set_trigger_state = hts221_trig_set_state, > +}; > + > +static irqreturn_t hts221_trigger_handler_th(int irq, void *private) > +{ > + struct hts221_dev *dev = (struct hts221_dev *)private; > + > + dev->hw_timestamp = iio_get_time_ns(dev->iio_devs[0]); Hmm. Might be on the wrong clock base for the particular device. You'll probably have to grab one for each of the IIO devices then use the right one later. > + > + return IRQ_WAKE_THREAD; > +} > + > +static irqreturn_t hts221_trigger_handler_bh(int irq, void *private) > +{ > + u8 status; > + int i, err; > + struct hts221_sensor *sensor; > + struct iio_chan_spec const *ch; > + struct hts221_dev *dev = (struct hts221_dev *)private; > + > + err = dev->tf->read(dev->dev, REG_STATUS_ADDR, 1, &status); > + if (err < 0) > + return IRQ_HANDLED; > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + sensor = iio_priv(dev->iio_devs[i]); > + > + if (status & sensor->drdy_data_mask) { > + ch = dev->iio_devs[i]->channels; > + err = dev->tf->read(dev->dev, ch[0].address, 2, > + sensor->buffer); > + if (err < 0) > + continue; > + > + iio_trigger_poll_chained(sensor->trig); > + } > + } > + > + return IRQ_HANDLED; Shouldn't return IRQ handled if it wasn't us... i.e. no bits are set. Someone may want to support shared irqs in the future. > +} > + > +int hts221_allocate_triggers(struct hts221_dev *dev) > +{ > + int i, err, count = 0; > + unsigned long irq_type; > + struct hts221_sensor *sensor; > + > + irq_type = irqd_get_trigger_type(irq_get_irq_data(dev->irq)); > + > + switch (irq_type) { > + case IRQF_TRIGGER_HIGH: > + case IRQF_TRIGGER_RISING: > + break; > + default: > + dev_info(dev->dev, > + "mode %lx unsupported, use IRQF_TRIGGER_RISING\n", using (use would tell them to change it, not that the driver is going to do it on it's own). > + irq_type); > + irq_type = IRQF_TRIGGER_RISING; > + break; > + } > + > + err = devm_request_threaded_irq(dev->dev, dev->irq, > + hts221_trigger_handler_th, > + hts221_trigger_handler_bh, > + irq_type | IRQF_ONESHOT, > + dev->name, dev); > + if (err) { > + dev_err(dev->dev, "failed to request trigger irq %d\n", > + dev->irq); > + return err; > + } > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + sensor = iio_priv(dev->iio_devs[i]); > + sensor->trig = iio_trigger_alloc("%s-trigger", > + dev->iio_devs[i]->name); > + if (!sensor->trig) { > + err = -ENOMEM; > + goto iio_trigger_error; > + } > + > + iio_trigger_set_drvdata(sensor->trig, dev->iio_devs[i]); > + sensor->trig->ops = &hts221_trigger_ops; > + sensor->trig->dev.parent = dev->dev; > + > + err = iio_trigger_register(sensor->trig); > + if (err < 0) { > + dev_err(dev->dev, "failed to register iio trigger\n"); > + goto iio_trigger_error; > + } > + dev->iio_devs[i]->trig = iio_trigger_get(sensor->trig); > + count++; > + } > + > + return 0; > + > +iio_trigger_error: > + for (i = count - 1; i >= 0; i--) { > + sensor = iio_priv(dev->iio_devs[i]); > + iio_trigger_unregister(sensor->trig); > + } > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + sensor = iio_priv(dev->iio_devs[i]); > + iio_trigger_free(sensor->trig); > + } > + > + return err; > +} > + > +void hts221_deallocate_triggers(struct hts221_dev *dev) > +{ > + int i; > + struct hts221_sensor *sensor; > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + sensor = iio_priv(dev->iio_devs[i]); > + iio_trigger_unregister(sensor->trig); > + iio_trigger_free(sensor->trig); > + } > +} > + > +static int hts221_buffer_preenable(struct iio_dev *indio_dev) > +{ > + return hts221_sensor_power_on(iio_priv(indio_dev)); > +} > + > +static int hts221_buffer_postdisable(struct iio_dev *indio_dev) > +{ > + return hts221_sensor_power_off(iio_priv(indio_dev)); > +} > + > +static const struct iio_buffer_setup_ops hts221_buffer_ops = { > + .preenable = hts221_buffer_preenable, > + .postenable = iio_triggered_buffer_postenable, > + .predisable = iio_triggered_buffer_predisable, > + .postdisable = hts221_buffer_postdisable, > +}; > + > +static irqreturn_t hts221_buffer_handler_bh(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *iio_dev = pf->indio_dev; > + struct hts221_sensor *sensor = iio_priv(iio_dev); > + u8 out_data[iio_dev->scan_bytes]; > + > + if (iio_dev->active_scan_mask && > + test_bit(0, iio_dev->active_scan_mask)) > + memcpy(out_data, sensor->buffer, 2); > + > + iio_push_to_buffers_with_timestamp(iio_dev, out_data, > + sensor->dev->hw_timestamp); > + > + iio_trigger_notify_done(sensor->trig); > + > + return IRQ_HANDLED; > +} > + > +int hts221_allocate_buffers(struct hts221_dev *dev) > +{ > + int i, err, count = 0; > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + err = iio_triggered_buffer_setup(dev->iio_devs[i], NULL, > + hts221_buffer_handler_bh, > + &hts221_buffer_ops); > + if (err < 0) > + goto iio_buffer_error; > + count++; > + } > + > + return 0; > + > +iio_buffer_error: > + for (i = count - 1; i >= 0; i--) > + iio_triggered_buffer_cleanup(dev->iio_devs[i]); > + > + return err; > +} > + > +void hts221_deallocate_buffers(struct hts221_dev *dev) > +{ > + int i; > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) > + iio_triggered_buffer_cleanup(dev->iio_devs[i]); > +} > + > +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>"); > +MODULE_DESCRIPTION("STMicroelectronics hts221 buffer driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/humidity/hts221/hts221_core.c b/drivers/iio/humidity/hts221/hts221_core.c > new file mode 100644 > index 0000000..0beac47 > --- /dev/null > +++ b/drivers/iio/humidity/hts221/hts221_core.c > @@ -0,0 +1,804 @@ > +/* > + * STMicroelectronics hts221 sensor driver > + * > + * Copyright 2016 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 <asm/unaligned.h> > + > +#include "hts221.h" > + > +#define REG_WHOAMI_ADDR 0x0f > +#define REG_WHOAMI_VAL 0xbc > + > +#define REG_CNTRL1_ADDR 0x20 > +#define REG_CNTRL2_ADDR 0x21 > +#define REG_CNTRL3_ADDR 0x22 > + > +#define REG_H_AVG_ADDR 0x10 > +#define REG_T_AVG_ADDR 0x10 > +#define REG_H_OUT_L 0x28 > +#define REG_T_OUT_L 0x2a > + > +#define H_AVG_MASK 0x07 > +#define T_AVG_MASK 0x38 > + > +#define ODR_MASK 0x87 > +#define BDU_MASK 0x04 > + > +#define DRDY_MASK 0x04 > + > +#define ENABLE_SENSOR 0x80 > + A more unique prefix for all defines will make future clashes far less likely. so HTS221_H_AVG_4 etc > +#define H_AVG_4 0x00 /* 0.4 %RH */ > +#define H_AVG_8 0x01 /* 0.3 %RH */ > +#define H_AVG_16 0x02 /* 0.2 %RH */ > +#define H_AVG_32 0x03 /* 0.15 %RH */ > +#define H_AVG_64 0x04 /* 0.1 %RH */ > +#define H_AVG_128 0x05 /* 0.07 %RH */ > +#define H_AVG_256 0x06 /* 0.05 %RH */ > +#define H_AVG_512 0x07 /* 0.03 %RH */ > + > +#define T_AVG_2 0x00 /* 0.08 degC */ > +#define T_AVG_4 0x08 /* 0.05 degC */ > +#define T_AVG_8 0x10 /* 0.04 degC */ > +#define T_AVG_16 0x18 /* 0.03 degC */ > +#define T_AVG_32 0x20 /* 0.02 degC */ > +#define T_AVG_64 0x28 /* 0.015 degC */ > +#define T_AVG_128 0x30 /* 0.01 degC */ > +#define T_AVG_256 0x38 /* 0.007 degC */ > + > +/* caldata registers */ > +#define REG_0RH_CAL_X_H 0x36 > +#define REG_1RH_CAL_X_H 0x3a > +#define REG_0RH_CAL_Y_H 0x30 > +#define REG_1RH_CAL_Y_H 0x31 > +#define REG_0T_CAL_X_L 0x3c > +#define REG_1T_CAL_X_L 0x3e > +#define REG_0T_CAL_Y_H 0x32 > +#define REG_1T_CAL_Y_H 0x33 > +#define REG_T1_T0_CAL_Y_H 0x35 > + > +struct hts221_odr { > + u32 hz; > + u8 val; > +}; > + > +struct hts221_avg { > + u8 addr; > + u8 mask; > + struct hts221_avg_avl avg_avl[HTS221_AVG_DEPTH]; > +}; > + > +static const struct hts221_odr hts221_odr_table[] = { > + { 1, 0x01 }, /* 1Hz */ > + { 7, 0x02 }, /* 7Hz */ > + { 13, 0x03 }, /* 12.5 HZ */ > +}; > + > +static const struct hts221_avg hts221_avg_list[] = { > + { > + .addr = REG_H_AVG_ADDR, > + .mask = H_AVG_MASK, > + .avg_avl = { > + { 4, H_AVG_4 }, > + { 8, H_AVG_8 }, > + { 16, H_AVG_16 }, > + { 32, H_AVG_32 }, > + { 64, H_AVG_64 }, > + { 128, H_AVG_128 }, > + { 256, H_AVG_256 }, > + { 512, H_AVG_512 }, > + }, > + }, > + { > + .addr = REG_T_AVG_ADDR, > + .mask = T_AVG_MASK, > + .avg_avl = { > + { 2, T_AVG_2 }, > + { 4, T_AVG_4 }, > + { 8, T_AVG_8 }, > + { 16, T_AVG_16 }, > + { 32, T_AVG_32 }, > + { 64, T_AVG_64 }, > + { 128, T_AVG_128 }, > + { 256, T_AVG_256 }, > + }, > + }, > +}; > + > +static const struct iio_chan_spec hts221_h_channels[] = { > + { > + .type = IIO_HUMIDITYRELATIVE, > + .address = REG_H_OUT_L, > + .modified = 0, > + .channel2 = IIO_NO_MOD, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_OFFSET) | > + BIT(IIO_CHAN_INFO_SCALE), > + .scan_index = 0, > + .scan_type = { > + .sign = 's', > + .realbits = 16, > + .storagebits = 16, > + .endianness = IIO_LE, > + }, > + }, > + IIO_CHAN_SOFT_TIMESTAMP(1), > +}; > + > +static const struct iio_chan_spec hts221_t_channels[] = { > + { > + .type = IIO_TEMP, > + .address = REG_T_OUT_L, No need to specify modified or channel2 as they will be allocated to 0 anyway and there is no semantic meaning in setting them again (unlike scan_index where there is) > + .modified = 0, > + .channel2 = IIO_NO_MOD, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_OFFSET) | > + BIT(IIO_CHAN_INFO_SCALE), > + .scan_index = 0, > + .scan_type = { > + .sign = 's', > + .realbits = 16, > + .storagebits = 16, > + .endianness = IIO_LE, > + }, > + }, > + IIO_CHAN_SOFT_TIMESTAMP(1), > +}; > + > +static int hts221_write_with_mask(struct hts221_dev *dev, u8 addr, u8 mask, > + u8 val) > +{ > + u8 data; > + int err; > + > + mutex_lock(&dev->lock); > + > + err = dev->tf->read(dev->dev, addr, 1, &data); > + if (err < 0) { > + dev_err(dev->dev, "failed to read %02x register\n", addr); > + mutex_unlock(&dev->lock); > + > + return err; > + } > + > + data = (data & ~mask) | (val & mask); > + > + err = dev->tf->write(dev->dev, addr, 1, &data); > + if (err < 0) { > + dev_err(dev->dev, "failed to write %02x register\n", addr); > + mutex_unlock(&dev->lock); > + > + return err; > + } > + > + mutex_unlock(&dev->lock); > + > + return 0; > +} > + > +static int hts221_check_whoami(struct hts221_dev *dev) > +{ > + u8 data; > + int err; > + > + err = dev->tf->read(dev->dev, REG_WHOAMI_ADDR, 1, &data); > + if (err < 0) { > + dev_err(dev->dev, "failed to read whoami register\n"); > + return err; > + } > + > + if (data != REG_WHOAMI_VAL) { > + dev_err(dev->dev, "wrong whoami {%02x-%02x}\n", > + data, REG_WHOAMI_VAL); > + return -ENODEV; > + } > + > + return 0; > +} > + > +int hts221_config_drdy(struct hts221_sensor *sensor, bool enable) > +{ > + int i; > + struct hts221_sensor *cur_sensor; > + struct hts221_dev *dev = sensor->dev; > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + cur_sensor = iio_priv(dev->iio_devs[i]); > + > + if (cur_sensor == sensor) > + continue; > + > + if (!cur_sensor->enabled) > + break; > + } > + > + if (i < HTS221_SENSOR_MAX) { > + u8 val = (enable) ? 0x04 : 0; > + > + return hts221_write_with_mask(dev, REG_CNTRL3_ADDR, > + DRDY_MASK, val); > + } else { > + return 0; > + } > +} > + > +static int hts221_update_odr(struct hts221_dev *dev, u8 odr) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(hts221_odr_table); i++) > + if (hts221_odr_table[i].hz == odr) > + break; > + > + if (i == ARRAY_SIZE(hts221_odr_table)) > + return -EINVAL; > + > + return hts221_write_with_mask(dev, REG_CNTRL1_ADDR, ODR_MASK, > + ENABLE_SENSOR | BDU_MASK | > + hts221_odr_table[i].val); > +} > + > +static int hts221_sensor_update_odr(struct hts221_sensor *sensor, u8 odr) > +{ > + int i; > + struct hts221_sensor *cur_sensor; > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + cur_sensor = iio_priv(sensor->dev->iio_devs[i]); > + > + if (cur_sensor == sensor) > + continue; > + > + if (cur_sensor->enabled && cur_sensor->odr >= odr) > + return 0; > + } > + > + return hts221_update_odr(sensor->dev, odr); > +} > + > +static int hts221_update_avg(struct hts221_sensor *sensor, u16 val) > +{ > + int i, err; > + const struct hts221_avg *avg = &hts221_avg_list[sensor->type]; > + > + for (i = 0; i < HTS221_AVG_DEPTH; i++) > + if (avg->avg_avl[i].avg == val) > + break; > + > + if (i == HTS221_AVG_DEPTH) > + return -EINVAL; > + > + err = hts221_write_with_mask(sensor->dev, avg->addr, avg->mask, > + avg->avg_avl[i].val); > + if (err < 0) > + return err; > + > + sensor->cur_avg_idx = i; > + > + return 0; > +} > + > +static ssize_t > +hts221_sysfs_get_h_avg_val(struct device *device, > + struct device_attribute *attr, char *buf) > +{ > + int idx, val; > + struct iio_dev *indio_dev = dev_get_drvdata(device); > + struct hts221_sensor *sensor = iio_priv(indio_dev); > + > + idx = sensor->cur_avg_idx; > + val = hts221_avg_list[HTS221_SENSOR_H].avg_avl[idx].avg; > + > + return sprintf(buf, "%d\n", val); > +} > + > +static ssize_t > +hts221_sysfs_set_h_avg_val(struct device *device, > + struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + int err; > + unsigned int val; > + struct iio_dev *indio_dev = dev_get_drvdata(device); > + struct hts221_sensor *sensor = iio_priv(indio_dev); > + > + err = kstrtoint(buf, 10, &val); > + if (err < 0) > + return err; > + > + err = hts221_update_avg(sensor, (u16)val); > + > + return err < 0 ? err : size; > +} > + > +static ssize_t > +hts221_sysfs_get_t_avg_val(struct device *device, > + struct device_attribute *attr, char *buf) > +{ > + int idx, val; > + struct iio_dev *indio_dev = dev_get_drvdata(device); > + struct hts221_sensor *sensor = iio_priv(indio_dev); > + > + idx = sensor->cur_avg_idx; > + val = hts221_avg_list[HTS221_SENSOR_T].avg_avl[idx].avg; > + > + return sprintf(buf, "%d\n", val); > +} > + > +static ssize_t > +hts221_sysfs_set_t_avg_val(struct device *device, > + struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + int err; > + unsigned int val; > + struct iio_dev *indio_dev = dev_get_drvdata(device); > + struct hts221_sensor *sensor = iio_priv(indio_dev); > + > + err = kstrtoint(buf, 10, &val); > + if (err < 0) > + return err; > + > + err = hts221_update_avg(sensor, (u16)val); > + > + return err < 0 ? err : size; > +} > + > +static ssize_t hts221_sysfs_sampling_freq(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + int i; > + ssize_t len = 0; > + > + for (i = 0; i < ARRAY_SIZE(hts221_odr_table); i++) > + len += scnprintf(buf + len, PAGE_SIZE - len, "%d ", > + hts221_odr_table[i].hz); > + buf[len - 1] = '\n'; > + > + return len; > +} > + > +static ssize_t > +hts221_sysfs_get_sampling_frequency(struct device *device, > + struct device_attribute *attr, char *buf) > +{ > + struct iio_dev *indio_dev = dev_get_drvdata(device); > + struct hts221_sensor *sensor = iio_priv(indio_dev); > + > + return sprintf(buf, "%d\n", sensor->odr); > +} > + > +static ssize_t > +hts221_sysfs_set_sampling_frequency(struct device *device, > + struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + int err; > + unsigned int odr; > + struct iio_dev *indio_dev = dev_get_drvdata(device); > + struct hts221_sensor *sensor = iio_priv(indio_dev); > + > + err = kstrtoint(buf, 10, &odr); > + if (err < 0) > + return err; > + > + err = hts221_sensor_update_odr(sensor, odr); > + if (!err) > + sensor->odr = odr; > + > + return err < 0 ? err : size; > +} > + > +int hts221_sensor_power_on(struct hts221_sensor *sensor) > +{ > + int err, idx, val; > + > + idx = sensor->cur_avg_idx; > + val = hts221_avg_list[sensor->type].avg_avl[idx].avg; > + err = hts221_update_avg(sensor, val); > + if (err < 0) > + return err; > + > + err = hts221_sensor_update_odr(sensor, sensor->odr); > + if (err < 0) > + return err; > + > + sensor->enabled = true; > + > + return 0; > +} > + > +static int hts221_dev_power_off(struct hts221_dev *dev) > +{ > + u8 data[] = {0x00, 0x00}; > + > + return dev->tf->write(dev->dev, REG_CNTRL1_ADDR, 2, data); > +} > + > +int hts221_sensor_power_off(struct hts221_sensor *sensor) > +{ > + int i; > + struct hts221_sensor *cur_sensor; > + struct hts221_dev *dev = sensor->dev; > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + cur_sensor = iio_priv(dev->iio_devs[i]); > + if (cur_sensor == sensor) > + continue; > + > + if (cur_sensor->enabled) > + break; > + } > + > + if (i == HTS221_SENSOR_MAX) > + hts221_dev_power_off(dev); > + > + sensor->enabled = false; > + > + return 0; > +} > + > +static int hts221_parse_caldata(struct hts221_sensor *sensor) > +{ > + int err, *slope, *b_gen; > + u8 addr_x0, addr_x1; > + s16 cal_x0, cal_x1, cal_y0, cal_y1; > + struct hts221_dev *dev = sensor->dev; > + > + switch (sensor->type) { > + case HTS221_SENSOR_H: > + addr_x0 = REG_0RH_CAL_X_H; > + addr_x1 = REG_1RH_CAL_X_H; > + > + cal_y1 = 0; > + cal_y0 = 0; > + err = dev->tf->read(dev->dev, REG_0RH_CAL_Y_H, 1, > + (u8 *)&cal_y0); > + if (err < 0) > + return err; > + > + err = dev->tf->read(dev->dev, REG_1RH_CAL_Y_H, 1, > + (u8 *)&cal_y1); > + if (err < 0) > + return err; > + break; > + case HTS221_SENSOR_T: { > + u8 cal0, cal1; > + > + addr_x0 = REG_0T_CAL_X_L; > + addr_x1 = REG_1T_CAL_X_L; > + > + err = dev->tf->read(dev->dev, REG_0T_CAL_Y_H, 1, &cal0); > + if (err < 0) > + return err; > + > + err = dev->tf->read(dev->dev, REG_T1_T0_CAL_Y_H, 1, &cal1); > + if (err < 0) > + return err; > + cal_y0 = (le16_to_cpu(cal1 & 0x3) << 8) | cal0; > + > + err = dev->tf->read(dev->dev, REG_1T_CAL_Y_H, 1, &cal0); > + if (err < 0) > + return err; > + > + err = dev->tf->read(dev->dev, REG_T1_T0_CAL_Y_H, 1, &cal1); > + if (err < 0) > + return err; > + cal_y1 = (le16_to_cpu((cal1 & 0xc) >> 2) << 8) | cal0; > + break; > + } > + default: > + return -ENODEV; > + } > + > + err = dev->tf->read(dev->dev, addr_x0, 2, (u8 *)&cal_x0); > + if (err < 0) > + return err; > + cal_x0 = le32_to_cpu(cal_x0); > + > + err = dev->tf->read(dev->dev, addr_x1, 2, (u8 *)&cal_x1); > + if (err < 0) > + return err; > + cal_x1 = le32_to_cpu(cal_x1); > + > + slope = &sensor->slope; > + b_gen = &sensor->b_gen; > + > + *slope = ((cal_y1 - cal_y0) * 8000) / (cal_x1 - cal_x0); > + *b_gen = (((s32)cal_x1 * cal_y0 - (s32)cal_x0 * cal_y1) * 1000) / > + (cal_x1 - cal_x0); > + *b_gen *= 8; > + > + return 0; > +} > + > +static int hts221_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *ch, > + int *val, int *val2, long mask) > +{ > + int ret; > + struct hts221_sensor *sensor = iio_priv(indio_dev); > + struct hts221_dev *dev = sensor->dev; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: { > + u8 data[2]; > + > + mutex_lock(&indio_dev->mlock); Use the iio_claim_direct_mode functions rather than hand rolling it. > + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) { > + mutex_unlock(&indio_dev->mlock); > + return -EBUSY; > + } > + I'd roll this up into a separate function with the unlocking outside. Will lead to neater code. > + ret = hts221_sensor_power_on(sensor); > + if (ret < 0) { > + mutex_unlock(&indio_dev->mlock); > + return ret; > + } > + > + msleep(50); > + > + ret = dev->tf->read(dev->dev, ch->address, 2, data); > + if (ret < 0) { > + mutex_unlock(&indio_dev->mlock); > + return ret; > + } > + > + ret = hts221_sensor_power_off(sensor); > + if (ret < 0) { > + mutex_unlock(&indio_dev->mlock); > + return ret; > + } > + > + *val = (s16)get_unaligned_le16(data); > + ret = IIO_VAL_INT; > + mutex_unlock(&indio_dev->mlock); > + > + break; > + } > + case IIO_CHAN_INFO_SCALE: { > + s64 tmp; > + s32 rem, div, data = sensor->slope; > + > + switch (ch->type) { > + case IIO_HUMIDITYRELATIVE: { > + div = (1 << 4) * 1000; > + break; > + } > + case IIO_TEMP: { > + div = (1 << 6) * 1000; > + break; > + } > + default: > + return -EINVAL; > + } > + > + tmp = div_s64(data * 1000000000LL, div); > + tmp = div_s64_rem(tmp, 1000000000LL, &rem); > + > + *val = tmp; > + *val2 = rem; > + ret = IIO_VAL_INT_PLUS_NANO; > + break; > + } > + case IIO_CHAN_INFO_OFFSET: { > + s64 tmp; > + s32 rem, div = sensor->slope, data = sensor->b_gen; > + > + tmp = div_s64(data * 1000000000LL, div); > + tmp = div_s64_rem(tmp, 1000000000LL, &rem); > + > + *val = tmp; > + *val2 = abs(rem); > + ret = IIO_VAL_INT_PLUS_NANO; > + break; > + } > + default: > + ret = -EINVAL; > + break; > + } > + > + return ret; > +} > + > +static IIO_CONST_ATTR(humidityrelative_avg_sample_available, > + "4 8 16 32 64 128 256 512"); > +static IIO_DEVICE_ATTR(humidityrelative_avg_sample, S_IWUSR | S_IRUGO, > + hts221_sysfs_get_h_avg_val, > + hts221_sysfs_set_h_avg_val, 0); > +static IIO_CONST_ATTR(temp_avg_sample_available, > + "2 4 8 16 32 64 128 256"); > +static IIO_DEVICE_ATTR(temp_avg_sample, S_IWUSR | S_IRUGO, > + hts221_sysfs_get_t_avg_val, > + hts221_sysfs_set_t_avg_val, 0); > + > +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hts221_sysfs_sampling_freq); > +static IIO_DEV_ATTR_SAMP_FREQ(S_IWUSR | S_IRUGO, > + hts221_sysfs_get_sampling_frequency, > + hts221_sysfs_set_sampling_frequency); > + > +static struct attribute *hts221_h_attributes[] = { > + &iio_dev_attr_sampling_frequency_available.dev_attr.attr, > + &iio_dev_attr_sampling_frequency.dev_attr.attr, Please use the info_mask_shared_by_all element for sampling_frequency. > + &iio_const_attr_humidityrelative_avg_sample_available.dev_attr.attr, > + &iio_dev_attr_humidityrelative_avg_sample.dev_attr.attr, This is oversampling whatever they have decided to call it in the datasheet. ABI for that is standard... The sampling frequency and this will be annoying tied together but keeping to standard abi will make it worth the pain.. Usual trick is to cache the requested sampling frequency and go for the best match possible when the oversampling ratio is changed. > + NULL, > +}; > + > +static const struct attribute_group hts221_h_attribute_group = { > + .attrs = hts221_h_attributes, > +}; > + > +static const struct iio_info hts221_h_info = { > + .driver_module = THIS_MODULE, > + .attrs = &hts221_h_attribute_group, > + .read_raw = &hts221_read_raw, > +}; > + > +static struct attribute *hts221_t_attributes[] = { > + &iio_dev_attr_sampling_frequency_available.dev_attr.attr, > + &iio_dev_attr_sampling_frequency.dev_attr.attr, > + &iio_const_attr_temp_avg_sample_available.dev_attr.attr, > + &iio_dev_attr_temp_avg_sample.dev_attr.attr, > + NULL, > +}; > + > +static const struct attribute_group hts221_t_attribute_group = { > + .attrs = hts221_t_attributes, > +}; > + > +static const struct iio_info hts221_t_info = { > + .driver_module = THIS_MODULE, > + .attrs = &hts221_t_attribute_group, > + .read_raw = hts221_read_raw, > +}; > + > +static struct iio_dev *hts221_alloc_iio_sensor(struct hts221_dev *dev, > + enum hts221_sensor_type type) > +{ > + struct iio_dev *iio_dev; > + struct hts221_sensor *sensor; > + > + iio_dev = iio_device_alloc(sizeof(*sensor)); > + if (!iio_dev) > + return NULL; > + > + iio_dev->modes = INDIO_DIRECT_MODE; > + iio_dev->dev.parent = dev->dev; > + > + sensor = iio_priv(iio_dev); > + sensor->type = type; > + sensor->dev = dev; > + sensor->odr = hts221_odr_table[0].hz; > + > + switch (type) { > + case HTS221_SENSOR_H: > + iio_dev->channels = hts221_h_channels; > + iio_dev->num_channels = ARRAY_SIZE(hts221_h_channels); > + iio_dev->name = "hts221_rh"; > + iio_dev->info = &hts221_h_info; > + sensor->drdy_data_mask = 0x02; > + break; > + case HTS221_SENSOR_T: > + iio_dev->channels = hts221_t_channels; > + iio_dev->num_channels = ARRAY_SIZE(hts221_t_channels); > + iio_dev->name = "hts221_temp"; > + iio_dev->info = &hts221_t_info; > + sensor->drdy_data_mask = 0x01; > + break; > + default: > + iio_device_free(iio_dev); > + return NULL; > + } > + > + return iio_dev; > +} > + > +int hts221_probe(struct hts221_dev *dev) > +{ > + int i, err, count = 0; > + struct hts221_sensor *sensor; > + > + mutex_init(&dev->lock); > + > + err = hts221_check_whoami(dev); > + if (err < 0) > + return err; > + > + err = hts221_update_odr(dev, 1); > + if (err < 0) > + return err; > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + dev->iio_devs[i] = hts221_alloc_iio_sensor(dev, i); > + if (!dev->iio_devs[i]) { > + err = -ENOMEM; > + goto power_off; > + } > + sensor = iio_priv(dev->iio_devs[i]); > + > + err = hts221_update_avg(sensor, > + hts221_avg_list[i].avg_avl[3].avg); > + if (err < 0) > + goto power_off; > + > + err = hts221_parse_caldata(sensor); > + if (err < 0) > + goto power_off; > + } > + > + err = hts221_dev_power_off(dev); > + if (err < 0) > + goto iio_device_free; > + > + if (dev->irq > 0) { > + err = hts221_allocate_buffers(dev); > + if (err < 0) > + goto iio_device_free; > + > + err = hts221_allocate_triggers(dev); > + if (err) { > + hts221_deallocate_buffers(dev); > + goto iio_device_free; > + } > + } > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) { > + err = iio_device_register(dev->iio_devs[i]); > + if (err < 0) > + goto iio_register_err; > + count++; > + } > + > + return 0; > + > +iio_register_err: > + for (i = count - 1; i >= 0; i--) > + iio_device_unregister(dev->iio_devs[i]); > +power_off: > + hts221_dev_power_off(dev); > +iio_device_free: > + for (i = 0; i < HTS221_SENSOR_MAX; i++) > + if (dev->iio_devs[i]) > + iio_device_free(dev->iio_devs[i]); > + > + return err; > +} > +EXPORT_SYMBOL(hts221_probe); > + > +int hts221_remove(struct hts221_dev *dev) > +{ > + int i, err; > + > + err = hts221_dev_power_off(dev); > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) > + iio_device_unregister(dev->iio_devs[i]); Ideal would be for remove to precisely reverse the ordering of probe. That includes running these loops the other way around. > + > + if (dev->irq > 0) { > + hts221_deallocate_triggers(dev); > + hts221_deallocate_buffers(dev); > + } > + > + for (i = 0; i < HTS221_SENSOR_MAX; i++) > + iio_device_free(dev->iio_devs[i]); Nothing prevents you using devm_iio_device_alloc thus allowing you to drop the need to free here and in the error path above. > + > + return err; > +} > +EXPORT_SYMBOL(hts221_remove); > + > +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>"); > +MODULE_DESCRIPTION("STMicroelectronics hts221 sensor driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/humidity/hts221/hts221_i2c.c b/drivers/iio/humidity/hts221/hts221_i2c.c > new file mode 100644 > index 0000000..c02c6d2 > --- /dev/null > +++ b/drivers/iio/humidity/hts221/hts221_i2c.c > @@ -0,0 +1,135 @@ > +/* > + * STMicroelectronics hts221 i2c driver > + * > + * Copyright 2016 STMicroelectronics Inc. > + * > + * Lorenzo Bianconi <lorenzo.bianconi@xxxxxx> > + * > + * Licensed under the GPL-2. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/i2c.h> > +#include <linux/slab.h> > +#include "hts221.h" > + > +#define I2C_AUTO_INCREMENT 0x80 > + > +static int hts221_i2c_read(struct device *dev, u8 addr, int len, u8 *data) > +{ > + struct i2c_msg msg[2]; > + struct i2c_client *client = to_i2c_client(dev); > + > + if (len > 1) > + addr |= I2C_AUTO_INCREMENT; > + > + msg[0].addr = client->addr; > + msg[0].flags = client->flags; > + msg[0].len = 1; > + msg[0].buf = &addr; > + > + msg[1].addr = client->addr; > + msg[1].flags = client->flags | I2C_M_RD; > + msg[1].len = len; > + msg[1].buf = data; > + > + return i2c_transfer(client->adapter, msg, 2); > +} > + > +static int hts221_i2c_write(struct device *dev, u8 addr, int len, u8 *data) > +{ > + u8 send[len + 1]; > + struct i2c_msg msg; > + struct i2c_client *client = to_i2c_client(dev); > + > + if (len > 1) > + addr |= I2C_AUTO_INCREMENT; > + > + send[0] = addr; > + memcpy(&send[1], data, len * sizeof(u8)); > + > + msg.addr = client->addr; > + msg.flags = client->flags; > + msg.len = len + 1; > + msg.buf = send; > + > + return i2c_transfer(client->adapter, &msg, 1); > +} > + > +static const struct hts221_transfer_function hts221_transfer_fn = { > + .read = hts221_i2c_read, > + .write = hts221_i2c_write, > +}; > + > +static int hts221_i2c_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + int err; > + struct hts221_dev *dev; > + > + dev = kzalloc(sizeof(*dev), GFP_KERNEL); > + if (!dev) > + return -ENOMEM; > + > + i2c_set_clientdata(client, dev); > + dev->name = client->name; > + dev->dev = &client->dev; > + dev->irq = client->irq; > + dev->tf = &hts221_transfer_fn; > + > + err = hts221_probe(dev); > + if (err < 0) { > + kfree(dev); > + return err; > + } > + > + dev_info(&client->dev, "sensor probed\n"); > + > + return 0; > +} > + > +static int hts221_i2c_remove(struct i2c_client *client) > +{ > + int err; > + struct hts221_dev *dev = i2c_get_clientdata(client); > + > + err = hts221_remove(dev); > + if (err < 0) > + return err; > + > + dev_info(&client->dev, "sensor removed\n"); Again, too much info. > + > + return 0; > +} > + > +#ifdef CONFIG_OF > +static const struct of_device_id hts221_i2c_of_match[] = { > + { .compatible = "st,hts221", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, hts221_i2c_of_match); > +#endif /* CONFIG_OF */ > + > +static const struct i2c_device_id hts221_i2c_id_table[] = { > + { HTS221_DEV_NAME }, > + {}, > +}; > +MODULE_DEVICE_TABLE(i2c, hts221_i2c_id_table); > + > +static struct i2c_driver hts221_driver = { > + .driver = { > + .name = "hts221_i2c", > +#ifdef CONFIG_OF > + .of_match_table = hts221_i2c_of_match, > +#endif /* CONFIG_OF */ As with SPI, there are cleaner ways of doing this. Look at other drivers. > + }, > + .probe = hts221_i2c_probe, > + .remove = hts221_i2c_remove, > + .id_table = hts221_i2c_id_table, > +}; > +module_i2c_driver(hts221_driver); > + > +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>"); > +MODULE_DESCRIPTION("STMicroelectronics hts221 i2c driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/humidity/hts221/hts221_spi.c b/drivers/iio/humidity/hts221/hts221_spi.c > new file mode 100644 > index 0000000..10056c5 > --- /dev/null > +++ b/drivers/iio/humidity/hts221/hts221_spi.c > @@ -0,0 +1,148 @@ > +/* > + * STMicroelectronics hts221 spi driver > + * > + * Copyright 2016 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 "hts221.h" > + > +#define SENSORS_SPI_READ 0x80 > +#define SPI_AUTO_INCREMENT 0x40 > + > +static int hts221_spi_read(struct device *device, u8 addr, int len, u8 *data) > +{ > + int err; > + struct spi_device *spi = to_spi_device(device); > + struct hts221_dev *dev = spi_get_drvdata(spi); > + > + struct spi_transfer xfers[] = { > + { > + .tx_buf = dev->tb.tx_buf, > + .bits_per_word = 8, > + .len = 1, > + }, > + { > + .rx_buf = dev->tb.rx_buf, > + .bits_per_word = 8, > + .len = len, > + } > + }; > + > + if (len > 1) > + addr |= SPI_AUTO_INCREMENT; > + dev->tb.tx_buf[0] = addr | SENSORS_SPI_READ; > + > + err = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers)); > + if (err < 0) > + return err; > + > + memcpy(data, dev->tb.rx_buf, len * sizeof(u8)); > + > + return len; > +} > + > +static int hts221_spi_write(struct device *device, u8 addr, int len, u8 *data) > +{ > + struct spi_device *spi = to_spi_device(device); > + struct hts221_dev *dev = spi_get_drvdata(spi); > + > + struct spi_transfer xfers = { > + .tx_buf = dev->tb.tx_buf, > + .bits_per_word = 8, > + .len = len + 1, > + }; > + > + if (len >= HTS221_TX_MAX_LENGTH) > + return -ENOMEM; > + > + if (len > 1) > + addr |= SPI_AUTO_INCREMENT; > + dev->tb.tx_buf[0] = addr; > + memcpy(&dev->tb.tx_buf[1], data, len); > + > + return spi_sync_transfer(spi, &xfers, 1); > +} > + > +static const struct hts221_transfer_function hts221_transfer_fn = { > + .read = hts221_spi_read, > + .write = hts221_spi_write, > +}; > + > +static int hts221_spi_probe(struct spi_device *spi) > +{ > + int err; > + struct hts221_dev *dev; > + > + dev = kzalloc(sizeof(*dev), GFP_KERNEL); > + if (!dev) > + return -ENOMEM; > + > + spi_set_drvdata(spi, dev); > + dev->name = spi->modalias; > + dev->dev = &spi->dev; > + dev->irq = spi->irq; > + dev->tf = &hts221_transfer_fn; > + > + err = hts221_probe(dev); > + if (err < 0) { > + kfree(dev); Having dropped th dv_info, can use drop this return outside the brackets. > + return err; > + } > + > + dev_info(&spi->dev, "sensor probed\n"); No useful information that isn't available elsewhere, please drop this dev_info. > + > + return 0; > +} > + > +static int hts221_spi_remove(struct spi_device *spi) > +{ > + int err; > + struct hts221_dev *dev = spi_get_drvdata(spi); > + > + err = hts221_remove(dev); > + if (err < 0) > + return err; > + > + dev_info(&spi->dev, "sensor removed\n"); No useful information that can't be trivially obtained from sysfs, so drop this dev_info. > + > + return 0; > +} > + Not worth the ifdefs to save a tiny amount of space here. > +#ifdef CONFIG_OF > +static const struct of_device_id hts221_spi_of_match[] = { > + { .compatible = "st,hts221", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, hts221_spi_of_match); > +#endif /* CONFIG_OF */ > + > +static const struct spi_device_id hts221_spi_id_table[] = { > + { HTS221_DEV_NAME }, > + {}, > +}; > +MODULE_DEVICE_TABLE(spi, hts221_spi_id_table); > + > +static struct spi_driver hts221_driver = { > + .driver = { > + .name = "hts221_spi", > +#ifdef CONFIG_OF > + .of_match_table = hts221_spi_of_match, > +#endif /* CONFIG_OF */ use the of_match_pointer macro that I think has the same effect and is cleaner. > + }, > + .probe = hts221_spi_probe, > + .remove = hts221_spi_remove, > + .id_table = hts221_spi_id_table, > +}; > +module_spi_driver(hts221_driver); > + > +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>"); > +MODULE_DESCRIPTION("STMicroelectronics hts221 spi driver"); > +MODULE_LICENSE("GPL v2"); > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
