Vlad Dogaru schrieb am 26.11.2014 um 13:50: > Supports buffering, IIO events and changing sampling frequency. > > Datasheet available at: > http://www.semtech.com/images/datasheet/sx9500_ag.pdf > Please find some issues and suggestions inline. > Signed-off-by: Vlad Dogaru <vlad.dogaru@xxxxxxxxx> > --- > Changes since v1: > - report raw readings of the channels instead of just 0 or 1. > - add a new Kconfig section for proximity and leave the lightning sensor in its > own one. > > drivers/iio/proximity/Kconfig | 14 + > drivers/iio/proximity/Makefile | 1 + > drivers/iio/proximity/sx9500.c | 755 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 770 insertions(+) > create mode 100644 drivers/iio/proximity/sx9500.c > > diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig > index 0c8cdf5..623fb03 100644 > --- a/drivers/iio/proximity/Kconfig > +++ b/drivers/iio/proximity/Kconfig > @@ -17,3 +17,17 @@ config AS3935 > module will be called as3935 > > endmenu > + > +menu "Proximity sensors" > + > +config SX9500 > + tristate "SX9500 Semtech proximity sensor" > + select IIO_BUFFER > + select IIO_TRIGGERED_BUFFER > + select REGMAP_I2C > + depends on I2C > + help > + Say Y here to build a driver for Semtech's SX9500 capacitive > + proximity/button sensor. It can also be built as a module, called... > + > +endmenu > diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile > index 743adee..9818dc5 100644 > --- a/drivers/iio/proximity/Makefile > +++ b/drivers/iio/proximity/Makefile > @@ -4,3 +4,4 @@ > > # When adding new entries keep the list in alphabetical order > obj-$(CONFIG_AS3935) += as3935.o > +obj-$(CONFIG_SX9500) += sx9500.o > diff --git a/drivers/iio/proximity/sx9500.c b/drivers/iio/proximity/sx9500.c > new file mode 100644 > index 0000000..07f2a2e > --- /dev/null > +++ b/drivers/iio/proximity/sx9500.c > @@ -0,0 +1,755 @@ > +/* > + * Copyright (c) 2014 Intel Corporation > + * > + * Driver for Semtech's SX9500 capacitive proximity/button solution. > + * Datasheet available at > + * <http://www.semtech.com/images/datasheet/sx9500.pdf>. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published by > + * the Free Software Foundation. > + */ > + > +#include <linux/kernel.h> > +#include <linux/slab.h> > +#include <linux/module.h> > +#include <linux/i2c.h> > +#include <linux/irq.h> > +#include <linux/acpi.h> > +#include <linux/gpio/consumer.h> > +#include <linux/regmap.h> > + > +#include <linux/iio/iio.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/sysfs.h> > +#include <linux/iio/events.h> > +#include <linux/iio/trigger.h> > +#include <linux/iio/triggered_buffer.h> > +#include <linux/iio/trigger_consumer.h> > + > +#define SX9500_DRIVER_NAME "sx9500" > +#define SX9500_IRQ_NAME "sx9500_event" > +#define SX9500_GPIO_NAME "sx9500_gpio" > + > +/* Register definitions. */ > +#define SX9500_REG_IRQ_SRC 0x00 > +#define SX9500_REG_STAT 0x01 > +#define SX9500_REG_IRQ_MSK 0x03 > + > +#define SX9500_REG_PROX_CTRL0 0x06 > +#define SX9500_REG_PROX_CTRL1 0x07 > +#define SX9500_REG_PROX_CTRL2 0x08 > +#define SX9500_REG_PROX_CTRL3 0x09 > +#define SX9500_REG_PROX_CTRL4 0x0a > +#define SX9500_REG_PROX_CTRL5 0x0b > +#define SX9500_REG_PROX_CTRL6 0x0c > +#define SX9500_REG_PROX_CTRL7 0x0d > +#define SX9500_REG_PROX_CTRL8 0x0e > + > +#define SX9500_REG_SENSOR_SEL 0x20 > +#define SX9500_REG_USE_MSB 0x21 > +#define SX9500_REG_USE_LSB 0x22 > +#define SX9500_REG_AVG_MSB 0x23 > +#define SX9500_REG_AVG_LSB 0x24 > +#define SX9500_REG_DIFF_MSB 0x25 > +#define SX9500_REG_DIFF_LSB 0x26 > +#define SX9500_REG_OFFSET_MSB 0x27 > +#define SX9500_REG_OFFSET_LSB 0x28 > + > +#define SX9500_REG_RESET 0x7f > + > +/* Write this to REG_RESET to do a soft reset. */ > +#define SX9500_SOFT_RESET 0xde > + > +#define SX9500_SCAN_PERIOD_MASK 0x70 Could be a GENMASK(6, 4) > + > +/* > + * These serve for identifying IRQ source in the IRQ_SRC register, and > + * also for masking the IRQs in the IRQ_MSK register. > + */ > +#define SX9500_CLOSE_IRQ BIT(6) > +#define SX9500_FAR_IRQ BIT(5) > +#define SX9500_CONVDONE_IRQ BIT(3) > + > +#define SX9500_PROXSTAT_SHIFT 4 > + > +#define SX9500_NUM_CHANNELS 4 > + > +struct sx9500_data { > + struct mutex mutex; > + struct i2c_client *client; > + struct iio_trigger *trig; > + struct regmap *regmap; > + /* > + * Last reading of the proximity status for each channel. We > + * only send an event to user space when this changes. > + */ > + u8 prox_stat[SX9500_NUM_CHANNELS]; prox_stat should be bool. > + bool event_enabled[SX9500_NUM_CHANNELS]; > + bool trigger_enabled; > +}; > + > +static const struct iio_event_spec sx9500_events[] = { > + { > + .type = IIO_EV_TYPE_THRESH, > + .dir = IIO_EV_DIR_EITHER, > + .mask_separate = BIT(IIO_EV_INFO_ENABLE), > + }, > +}; > + > +#define SX9500_CHANNEL(idx) \ > + { \ > + .type = IIO_PROXIMITY, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \ > + .indexed = 1, \ > + .channel = idx, \ > + .event_spec = sx9500_events, \ > + .num_event_specs = 1, \ = ARRAY_SIZE(sx9500_events)? > + .scan_index = idx, \ > + .scan_type = { \ > + .sign = 'u', \ Datasheet says: Signed, 2's complement format. > + .realbits = 16, \ > + .storagebits = 16, \ > + .shift = 0, \ > + }, \ > + } > + > +static const struct iio_chan_spec sx9500_channels[] = { > + SX9500_CHANNEL(0), > + SX9500_CHANNEL(1), > + SX9500_CHANNEL(2), > + SX9500_CHANNEL(3), > + IIO_CHAN_SOFT_TIMESTAMP(4), > +}; > + > +static const struct { > + int val; > + int val2; > + unsigned int bits; > +} sx9500_samp_freq_table[] = { > + {2, 500000, 0x70}, > + {3, 333333, 0x60}, > + {5, 0, 0x50}, > + {6, 666666, 0x40}, > + {8, 333333, 0x30}, > + {11, 111111, 0x20}, > + {16, 666666, 0x10}, > + {33, 333333, 0x00}, > +}; In reverse order, bits could be dropped from the struct and be used as the index instead (with the 4 bit shift applied in access functions). > + > +static const struct regmap_range sx9500_writable_reg_ranges[] = { > + regmap_reg_range(SX9500_REG_IRQ_MSK, SX9500_REG_IRQ_MSK), > + regmap_reg_range(SX9500_REG_PROX_CTRL0, SX9500_REG_PROX_CTRL8), > + regmap_reg_range(SX9500_REG_SENSOR_SEL, SX9500_REG_SENSOR_SEL), > + regmap_reg_range(SX9500_REG_OFFSET_MSB, SX9500_REG_OFFSET_LSB), > + regmap_reg_range(SX9500_REG_RESET, SX9500_REG_RESET), > +}; > + > +static const struct regmap_access_table sx9500_writeable_regs = { > + .yes_ranges = sx9500_writable_reg_ranges, > + .n_yes_ranges = ARRAY_SIZE(sx9500_writable_reg_ranges), > +}; > + > +/* > + * All allocated registers are readable, so we just list unallocated > + * ones. > + */ > +static const struct regmap_range sx9500_non_readable_reg_ranges[] = { > + regmap_reg_range(SX9500_REG_STAT + 1, SX9500_REG_STAT + 1), > + regmap_reg_range(SX9500_REG_IRQ_MSK + 1, SX9500_REG_PROX_CTRL0 - 1), > + regmap_reg_range(SX9500_REG_PROX_CTRL8 + 1, SX9500_REG_SENSOR_SEL - 1), > + regmap_reg_range(SX9500_REG_OFFSET_LSB + 1, SX9500_REG_RESET - 1), > +}; > + > +static const struct regmap_access_table sx9500_readable_regs = { > + .no_ranges = sx9500_non_readable_reg_ranges, > + .n_no_ranges = ARRAY_SIZE(sx9500_non_readable_reg_ranges), > +}; > + > +static const struct regmap_range sx9500_volatile_reg_ranges[] = { > + regmap_reg_range(SX9500_REG_IRQ_SRC, SX9500_REG_STAT), > + regmap_reg_range(SX9500_REG_USE_MSB, SX9500_REG_OFFSET_LSB), > + regmap_reg_range(SX9500_REG_RESET, SX9500_REG_RESET), > +}; > + > +static const struct regmap_access_table sx9500_volatile_regs = { > + .yes_ranges = sx9500_volatile_reg_ranges, > + .n_yes_ranges = ARRAY_SIZE(sx9500_volatile_reg_ranges), > +}; > + > +static const struct regmap_config sx9500_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + > + .max_register = SX9500_REG_RESET, > + .cache_type = REGCACHE_RBTREE, > + > + .wr_table = &sx9500_writeable_regs, > + .rd_table = &sx9500_readable_regs, > + .volatile_table = &sx9500_volatile_regs, > +}; > + > +static int sx9500_read_proximity(struct sx9500_data *data, > + const struct iio_chan_spec *chan, > + int *val) > +{ > + int ret; > + __be16 regval; > + > + ret = regmap_write(data->regmap, SX9500_REG_SENSOR_SEL, chan->channel); > + if (ret < 0) > + return ret; > + > + ret = regmap_bulk_read(data->regmap, SX9500_REG_USE_MSB, ®val, 2); > + if (ret < 0) > + return ret; > + > + *val = 32767 - (s16)be16_to_cpu(regval); Unless I'm missing something, it should be: *val = (s16)be16_to_cpu(regval); > + > + return IIO_VAL_INT; > +} > + > +static int sx9500_read_samp_freq(struct sx9500_data *data, > + int *val, int *val2) > +{ > + int i, ret; > + unsigned int regval; > + > + mutex_lock(&data->mutex); > + > + ret = regmap_read(data->regmap, SX9500_REG_PROX_CTRL0, ®val); mutex_unlock could be moved up here. > + if (ret < 0) > + goto out; > + With my suggested format of sx9500_samp_freq_table, it could be as simple as that: regval = (regval & SX9500_SCAN_PERIOD_MASK) >> 4; *val = sx9500_samp_freq_table[regval].val; *val2 = sx9500_samp_freq_table[regval].val2; ret = IIO_VAL_INT_PLUS_MICRO; > + ret = -EINVAL; > + for (i = 0; i < ARRAY_SIZE(sx9500_samp_freq_table); i++) > + if (sx9500_samp_freq_table[i].bits == > + (regval & SX9500_SCAN_PERIOD_MASK)) { > + *val = sx9500_samp_freq_table[i].val; > + *val2 = sx9500_samp_freq_table[i].val2; > + ret = IIO_VAL_INT_PLUS_MICRO; > + break; > + } > + > +out: > + mutex_unlock(&data->mutex); > + > + return ret; > +} > + > +static int sx9500_read_raw(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + int *val, int *val2, long mask) > +{ > + struct sx9500_data *data = iio_priv(indio_dev); > + int ret; > + > + switch (chan->type) { > + case IIO_PROXIMITY: > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + if (iio_buffer_enabled(indio_dev)) > + return -EBUSY; > + mutex_lock(&data->mutex); > + ret = sx9500_read_proximity(data, chan, val); > + mutex_unlock(&data->mutex); > + return ret; > + case IIO_CHAN_INFO_SAMP_FREQ: > + return sx9500_read_samp_freq(data, val, val2); > + default: > + return -EINVAL; > + } > + default: > + return -EINVAL; > + } > +} > + > +static int sx9500_set_samp_freq(struct sx9500_data *data, > + int val, int val2) > +{ > + int i, ret, idx; > + Similar to the _write_frequency approach in ad799x.c, this could be done without idx. > + idx = -1; > + for (i = 0; i < ARRAY_SIZE(sx9500_samp_freq_table); i++) > + if (val == sx9500_samp_freq_table[i].val && > + val2 == sx9500_samp_freq_table[i].val2) { > + idx = i; Just break in case of a match. > + break; > + } > + > + if (idx == -1) If we had no match, i will be equal to ARRAY_SIZE(sx9500_samp_freq_table). > + return -EINVAL; > + > + mutex_lock(&data->mutex); > + > + ret = regmap_update_bits(data->regmap, SX9500_REG_PROX_CTRL0, > + SX9500_SCAN_PERIOD_MASK, > + sx9500_samp_freq_table[idx].bits); Using my suggested version, this would change to: ret = regmap_update_bits(data->regmap, SX9500_REG_PROX_CTRL0, SX9500_SCAN_PERIOD_MASK, i << 4); > + > + mutex_unlock(&data->mutex); > + > + return ret; > +} > + > +static int sx9500_write_raw(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + int val, int val2, long mask) > +{ > + struct sx9500_data *data = iio_priv(indio_dev); > + > + switch (chan->type) { > + case IIO_PROXIMITY: > + switch (mask) { > + case IIO_CHAN_INFO_SAMP_FREQ: > + return sx9500_set_samp_freq(data, val, val2); > + default: > + return -EINVAL; > + } > + default: > + return -EINVAL; > + } > +} > + > +static irqreturn_t sx9500_irq_handler(int irq, void *private) > +{ > + struct iio_dev *indio_dev = private; > + struct sx9500_data *data = iio_priv(indio_dev); > + > + if (data->trigger_enabled) > + iio_trigger_poll(data->trig); > + > + /* > + * Even if no event is enabled, we need to wake the thread to > + * clear the interrupt state by reading SX9500_REG_IRQ_SRC. It Double whitespace. > + * is not possible to do that here because regmap_read takes a > + * mutex. > + */ > + return IRQ_WAKE_THREAD; > +} > + > +static irqreturn_t sx9500_irq_thread_handler(int irq, void *private) > +{ > + struct iio_dev *indio_dev = private; > + struct sx9500_data *data = iio_priv(indio_dev); > + int ret, chan; chan could be unsigned. > + unsigned int val; > + > + mutex_lock(&data->mutex); > + > + ret = regmap_read(data->regmap, SX9500_REG_IRQ_SRC, &val); > + if (ret < 0) { > + dev_err(&data->client->dev, "i2c transfer error in irq\n"); > + goto out; > + } > + > + if (!(val & (SX9500_CLOSE_IRQ | SX9500_FAR_IRQ))) > + goto out; > + > + ret = regmap_read(data->regmap, SX9500_REG_STAT, &val); > + if (ret < 0) { > + dev_err(&data->client->dev, "i2c transfer error in irq\n"); > + goto out; > + } > + > + val >>= SX9500_PROXSTAT_SHIFT; > + for (chan = 0; chan < SX9500_NUM_CHANNELS; chan++) { > + int dir; > + u64 ev; > + u8 new_prox = !(val & (1 << chan)); new_prox should be bool. (1 << chan) could be expressed as BIT(chan). Why the negation? If PROXSTATx gets set, doesn't this indicate, that an object gets close to the sensor? Shouldn't such an event cause IIO_EV_DIR_RISING? > + > + if (!data->event_enabled[chan]) > + continue; > + if (new_prox == data->prox_stat[chan]) > + /* No change on this channel. */ > + continue; > + > + dir = new_prox ? IIO_EV_DIR_RISING : > + IIO_EV_DIR_FALLING; > + ev = IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, > + chan, > + IIO_EV_TYPE_THRESH, > + dir); > + iio_push_event(indio_dev, ev, iio_get_time_ns()); > + data->prox_stat[chan] = new_prox; > + } > + > +out: > + mutex_unlock(&data->mutex); > + > + return IRQ_HANDLED; > +} > + > +static int sx9500_read_event_config(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir) > +{ > + struct sx9500_data *data = iio_priv(indio_dev); > + > + if (chan->type != IIO_PROXIMITY) > + return -EINVAL; > + if (type != IIO_EV_TYPE_THRESH) > + return -EINVAL; > + if (dir != IIO_EV_DIR_EITHER) > + return -EINVAL; These checks could be consolidated to just one if and one return. > + > + return data->event_enabled[chan->channel]; > +} > + > +static int sx9500_write_event_config(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + int state) > +{ > + struct sx9500_data *data = iio_priv(indio_dev); > + int ret; > + int i; i could be grouped up with ret or irqmask. > + bool any_active; any_active could be initialized with false. > + unsigned int irqmask; > + > + if (chan->type != IIO_PROXIMITY) > + return -EINVAL; > + if (type != IIO_EV_TYPE_THRESH) > + return -EINVAL; > + if (dir != IIO_EV_DIR_EITHER) > + return -EINVAL; These checks could also be consolidated. > + > + mutex_lock(&data->mutex); > + > + data->event_enabled[chan->channel] = state; > + > + any_active = 0; true or false should be used for bools, but better initialize during declaration. > + for (i = 0; i < SX9500_NUM_CHANNELS; i++) > + if (data->event_enabled[chan->channel]) { Don't you mean data->event_enabled[i]? > + any_active = true; > + break; > + } > + > + irqmask = SX9500_CLOSE_IRQ | SX9500_FAR_IRQ; > + ret = regmap_update_bits(data->regmap, SX9500_REG_IRQ_MSK, > + irqmask, any_active ? irqmask : 0); > + > + mutex_unlock(&data->mutex); > + > + return ret; > +} > + > +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL( > + "2.500000 3.333333 5 6.666666 8.333333 11.111111 16.666666 33.333333"); > + > +static struct attribute *sx9500_attributes[] = { > + &iio_const_attr_sampling_frequency_available.dev_attr.attr, > + NULL, > +}; > + > +static const struct attribute_group sx9500_attribute_group = { > + .attrs = sx9500_attributes, > +}; > + > +static const struct iio_info sx9500_info = { > + .driver_module = THIS_MODULE, > + .attrs = &sx9500_attribute_group, > + .read_raw = &sx9500_read_raw, > + .write_raw = &sx9500_write_raw, > + .read_event_config = &sx9500_read_event_config, > + .write_event_config = &sx9500_write_event_config, > +}; > + > +static int sx9500_set_trigger_state(struct iio_trigger *trig, > + bool state) > +{ > + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig); > + struct sx9500_data *data = iio_priv(indio_dev); > + int ret; > + > + mutex_lock(&data->mutex); > + > + ret = regmap_update_bits(data->regmap, SX9500_REG_IRQ_MSK, > + SX9500_CONVDONE_IRQ, > + state ? SX9500_CONVDONE_IRQ : 0); > + data->trigger_enabled = state; Only set data->trigger_enabled if regmap_update_bits was successful. > + > + mutex_unlock(&data->mutex); > + > + return ret; > +} > + > +static const struct iio_trigger_ops sx9500_trigger_ops = { > + .set_trigger_state = sx9500_set_trigger_state, > + .owner = THIS_MODULE, > +}; > + > +static irqreturn_t sx9500_trigger_handler(int irq, void *private) > +{ > + struct iio_poll_func *pf = private; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct sx9500_data *data = iio_priv(indio_dev); > + s16 *buf; > + int val, bit, ret, i = 0; > + > + buf = kmalloc(indio_dev->scan_bytes, GFP_KERNEL); Use kzalloc for a clean buffer? > + if (!buf) { > + dev_err(&data->client->dev, > + "failed to allocate memory in trigger handler\n"); > + return IRQ_HANDLED; > + } > + > + mutex_lock(&data->mutex); > + > + for_each_set_bit(bit, indio_dev->buffer->scan_mask, > + indio_dev->masklength) { > + ret = sx9500_read_proximity(data, &indio_dev->channels[bit], > + &val); > + if (ret < 0) > + goto out; > + > + buf[i++] = (s16)val; Which need is there to treat val as s16? > + } > + > + iio_push_to_buffers_with_timestamp(indio_dev, buf, > + iio_get_time_ns()); > + > +out: > + mutex_unlock(&data->mutex); > + > + kfree(buf); > + > + iio_trigger_notify_done(indio_dev->trig); > + > + return IRQ_HANDLED; > +} > + > +struct sx9500_reg_default { > + u8 reg; > + u8 def; > +}; > + > +static const struct sx9500_reg_default sx9500_default_regs[] = { > + { > + .reg = SX9500_REG_PROX_CTRL1, > + /* Shield enabled, small range. */ > + .def = 0x43, > + }, > + { > + .reg = SX9500_REG_PROX_CTRL2, > + /* x8 gain, 167kHz frequency, finest resolution. */ > + .def = 0x77, > + }, > + { > + .reg = SX9500_REG_PROX_CTRL3, > + /* Doze enabled, 2x scan period doze, no raw filter. */ > + .def = 0x40, > + }, > + { > + .reg = SX9500_REG_PROX_CTRL4, > + /* Average threshold. */ > + .def = 0x30, > + }, > + { > + .reg = SX9500_REG_PROX_CTRL5, > + /* > + * Debouncer off, lowest average negative filter, > + * highest average postive filter. > + */ > + .def = 0x0f, > + }, > + { > + .reg = SX9500_REG_PROX_CTRL6, > + /* Proximity detection threshold: 400 */ > + .def = 0x0e, .def should be 0x11. > + }, > + { > + .reg = SX9500_REG_PROX_CTRL7, > + /* > + * No automatic compensation, compensate each pin > + * independently, proximity hysteresis: 32, close > + * debouncer off, far debouncer off. > + */ > + .def = 0x00, > + }, > + { > + .reg = SX9500_REG_PROX_CTRL8, > + /* No stuck timeout, no periodic compensation. */ > + .def = 0x00, > + }, > + { > + .reg = SX9500_REG_PROX_CTRL0, > + /* Scan period: 30ms, all sensors enabled. */ > + .def = 0x0f, > + }, > +}; > + > +static int sx9500_init_device(struct iio_dev *indio_dev) > +{ > + struct sx9500_data *data = iio_priv(indio_dev); > + int ret, i, num_defaults; > + unsigned int val; > + > + ret = regmap_write(data->regmap, SX9500_REG_IRQ_MSK, 0); > + if (ret < 0) > + return ret; > + > + ret = regmap_write(data->regmap, SX9500_REG_RESET, > + SX9500_SOFT_RESET); > + if (ret < 0) > + return ret; > + > + ret = regmap_read(data->regmap, SX9500_REG_IRQ_SRC, &val); > + if (ret < 0) > + return ret; > + > + num_defaults = ARRAY_SIZE(sx9500_default_regs); num_defaults doesn't serve an essential purpose. > + for (i = 0; i < num_defaults; i++) { > + ret = regmap_write(data->regmap, > + sx9500_default_regs[i].reg, > + sx9500_default_regs[i].def); > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} > + > +static int sx9500_gpio_probe(struct i2c_client *client, > + struct sx9500_data *data) > +{ > + struct device *dev; > + struct gpio_desc *gpio; > + int ret; > + > + if (!client) > + return -EINVAL; > + > + dev = &client->dev; > + > + /* data ready gpio interrupt pin */ > + gpio = devm_gpiod_get_index(dev, SX9500_GPIO_NAME, 0); > + if (IS_ERR(gpio)) { > + dev_err(dev, "acpi gpio get index failed\n"); > + return PTR_ERR(gpio); > + } > + > + ret = gpiod_direction_input(gpio); > + if (ret) > + return ret; > + > + ret = gpiod_to_irq(gpio); > + > + dev_dbg(dev, "GPIO resource, no:%d irq:%d\n", desc_to_gpio(gpio), ret); > + > + return ret; > +} > + > +static int sx9500_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + int ret; > + struct iio_dev *indio_dev; > + struct sx9500_data *data; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (indio_dev == NULL) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + data->client = client; > + mutex_init(&data->mutex); > + data->trigger_enabled = false; > + > + data->regmap = devm_regmap_init_i2c(client, &sx9500_regmap_config); > + if (IS_ERR(data->regmap)) > + return PTR_ERR(data->regmap); > + > + sx9500_init_device(indio_dev); > + > + indio_dev->dev.parent = &client->dev; > + indio_dev->name = id->name; > + indio_dev->channels = sx9500_channels; > + indio_dev->num_channels = ARRAY_SIZE(sx9500_channels); > + indio_dev->info = &sx9500_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + if (client->irq <= 0) > + client->irq = sx9500_gpio_probe(client, data); > + > + if (client->irq > 0) { > + ret = devm_request_threaded_irq(&client->dev, client->irq, > + sx9500_irq_handler, sx9500_irq_thread_handler, > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, > + SX9500_IRQ_NAME, indio_dev); > + if (ret < 0) > + return ret; > + > + data->trig = devm_iio_trigger_alloc(&client->dev, > + "%s-dev%d", indio_dev->name, indio_dev->id); > + if (!data->trig) > + return -ENOMEM; > + > + data->trig->dev.parent = &client->dev; > + data->trig->ops = &sx9500_trigger_ops; > + iio_trigger_set_drvdata(data->trig, indio_dev); > + > + ret = iio_trigger_register(data->trig); > + if (ret) > + return ret; > + } > + > + ret = iio_triggered_buffer_setup(indio_dev, NULL, > + sx9500_trigger_handler, NULL); > + if (ret < 0) > + goto out_trigger_unregister; > + > + ret = iio_device_register(indio_dev); > + if (ret < 0) > + goto out_buffer_cleanup; > + > + return 0; > + > +out_buffer_cleanup: > + iio_triggered_buffer_cleanup(indio_dev); > +out_trigger_unregister: Trigger was only registered, if client->irq > 0. > + iio_trigger_unregister(data->trig); > + > + return ret; > +} i2c_set_clientdata() is missing in _probe. > + > +static int sx9500_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + struct sx9500_data *data = iio_priv(indio_dev); > + > + iio_device_unregister(indio_dev); > + iio_triggered_buffer_cleanup(indio_dev); Trigger was only registered, if client->irq > 0. > + iio_trigger_unregister(data->trig); > + > + return 0; > +} > + > +static const struct acpi_device_id sx9500_acpi_match[] = { > + {"SSX9500", 0}, /* TODO is there a better ACPI handle? */ Is this a typo? Shouldn't it be "SX9500"? > + { }, > +}; > +MODULE_DEVICE_TABLE(acpi, sx9500_acpi_match); > + > +static const struct i2c_device_id sx9500_id[] = { > + {"sx9500", 0}, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, sx9500_id); > + > +static struct i2c_driver sx9500_driver = { > + .driver = { > + .name = SX9500_DRIVER_NAME, > + .acpi_match_table = ACPI_PTR(sx9500_acpi_match), > + }, > + .probe = sx9500_probe, > + .remove = sx9500_remove, > + .id_table = sx9500_id, > +}; > +module_i2c_driver(sx9500_driver); > + > +MODULE_AUTHOR("Vlad Dogaru <vlad.dogaru@xxxxxxxxx>"); > +MODULE_DESCRIPTION("Driver for Semtech SX9500 proximity sensor"); > +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
