It looks like you're going to have to respin the patchset so I'm adding my nits even though it's a bit late. > +static int ad7091r_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode) > +{ > + int ret; > + > + switch (mode) { > + case AD7091R_MODE_SAMPLE: > + ret = regmap_update_bits(st->map, AD7091R_REG_CONF, > + AD7091R_REG_CONF_MODE_MASK, 0); > + break; > + case AD7091R_MODE_COMMAND: > + ret = regmap_update_bits(st->map, AD7091R_REG_CONF, > + AD7091R_REG_CONF_MODE_MASK, > + AD7091R_REG_CONF_CMD); > + break; > + case AD7091R_MODE_AUTOCYCLE: > + ret = regmap_update_bits(st->map, AD7091R_REG_CONF, > + AD7091R_REG_CONF_MODE_MASK, > + AD7091R_REG_CONF_AUTO); > + break; > + default: > + ret = -EINVAL; > + break; > + } This would look even better as: switch (mode) { case AD7091R_MODE_SAMPLE: conf = 0; break; case AD7091R_MODE_COMMAND: conf = AD7091R_REG_CONF_CMD; break; case AD7091R_MODE_AUTOCYCLE: conf = AD7091R_REG_CONF_AUTO; break; default: return -EINVAL; } ret = regmap_update_bits(st->map, AD7091R_REG_CONF, AD7091R_REG_CONF_MODE_MASK, conf); if (ret) return ret; st->mode = mode; return 0; > +int ad7091r_probe(struct device *dev, const char *name, > + const struct ad7091r_chip_info *chip_info, > + struct regmap *map, int irq) > +{ > + struct iio_dev *iio_dev; > + struct ad7091r_state *st; > + int ret; > + > + iio_dev = devm_iio_device_alloc(dev, sizeof(*st)); > + if (!iio_dev) > + return -ENOMEM; > + > + st = iio_priv(iio_dev); > + st->dev = dev; > + st->chip_info = chip_info; > + st->map = map; > + > + iio_dev->dev.parent = dev; > + iio_dev->name = name; > + iio_dev->info = &ad7091r_info; > + iio_dev->modes = INDIO_DIRECT_MODE; > + > + iio_dev->num_channels = chip_info->num_channels; > + iio_dev->channels = chip_info->channels; > + > + if (irq) { > + ret = devm_request_threaded_irq(dev, irq, NULL, > + ad7091r_event_handler, > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, st); > + if (ret) > + return ret; > + } > + > + /* Use command mode by default to convert only desired channels*/ > + ret = ad7091r_set_mode(st, AD7091R_MODE_COMMAND); > + if (ret < 0) if (ret) { > + return ret; > + > + return iio_device_register(iio_dev); > +} > +EXPORT_SYMBOL_GPL(ad7091r_probe); [ snip ] > +#include <linux/i2c.h> > +#include <linux/iio/iio.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > + > +#include "ad7091r-base.h" > + > +static const struct iio_event_spec ad7091r5_events[] = { > + { > + .type = IIO_EV_TYPE_THRESH, > + .dir = IIO_EV_DIR_RISING, > + .mask_separate = BIT(IIO_EV_INFO_VALUE) | > + BIT(IIO_EV_INFO_ENABLE), This would be more clear if it were aligned like so: .mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE), > + }, > + { > + .type = IIO_EV_TYPE_THRESH, > + .dir = IIO_EV_DIR_FALLING, > + .mask_separate = BIT(IIO_EV_INFO_VALUE) | > + BIT(IIO_EV_INFO_ENABLE), .mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE), > + }, > + { > + .type = IIO_EV_TYPE_THRESH, > + .dir = IIO_EV_DIR_EITHER, > + .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS), > + }, > +}; > + regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel