> On 01/10/16 18:15, Lorenzo Bianconi wrote: >> On Oct 01, Jonathan Cameron wrote: >>> 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> >> >> Hi Jonathan, >> >> thanks for the review :) >> >>> 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! >> >> Running a couple of tests I verified the ODR is shared between humidity and >> temperature sensor, so I think we can use just one iio_dev structure for both >> sensors and define a temperature channel and humidity one. This will simplify >> the triggered mode support. What do you think? Do you prefer to just support >> oneshot mode or modify the trigger/buffer support? > For this one a combined device would be nice. > > If it's straight forward then then modified trigger/buffer support option would be > the ideal. If not, I'd be happy enough with oneshot mode only at these rates. > > Up to you ;) > > Guessing you are several versions later you've already gone one way or the > other though. > > Sorry I didn't get to this during the week. > No worries :) I sent out a v3 yesterday morning with trigger/buffer support since it was quite straight forward. Regards, Lorenzo > Jonathan >> >> Best regards, >> Lorenzo >> >>> >>> 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"); >>>> >>> >> > -- UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch; unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp; umount; make clean; sleep -- 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
