| Message ID | 20190114035621.5252-3-martin@martingkelly.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio:bmi160: add drdy interrupt support | expand |
On Sun, 13 Jan 2019 19:56:18 -0800 Martin Kelly <martin@martingkelly.com> wrote: > From: Martin Kelly <martin@martingkelly.com> > > Add interrupt support for the data ready signal on the BMI160, which fires > an interrupt whenever new accelerometer/gyroscope data is ready to read. Various minor comments inline. Mostly fine, but one big 'oddity'. I think we only ever request 1 irq, but we have support in here for configuring the chip to generate two at the same time. Doesn't make much sense as it stands... Am I missing something? Jonathan > > Signed-off-by: Martin Kelly <martin@martingkelly.com> > --- > drivers/iio/imu/bmi160/Makefile | 1 + > drivers/iio/imu/bmi160/bmi160.h | 13 +- > drivers/iio/imu/bmi160/bmi160_core.c | 271 +++++++++++++++++++++++++++++++- > drivers/iio/imu/bmi160/bmi160_i2c.c | 3 +- > drivers/iio/imu/bmi160/bmi160_spi.c | 2 +- > drivers/iio/imu/bmi160/bmi160_trigger.c | 60 +++++++ > 6 files changed, 341 insertions(+), 9 deletions(-) > create mode 100644 drivers/iio/imu/bmi160/bmi160_trigger.c > > diff --git a/drivers/iio/imu/bmi160/Makefile b/drivers/iio/imu/bmi160/Makefile > index 10365e493ae2..19e1720c6fe9 100644 > --- a/drivers/iio/imu/bmi160/Makefile > +++ b/drivers/iio/imu/bmi160/Makefile > @@ -2,5 +2,6 @@ > # Makefile for Bosch BMI160 IMU > # > obj-$(CONFIG_BMI160) += bmi160_core.o > +obj-$(CONFIG_BMI160) += bmi160_trigger.o > obj-$(CONFIG_BMI160_I2C) += bmi160_i2c.o > obj-$(CONFIG_BMI160_SPI) += bmi160_spi.o > diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h > index 2351049d930b..0c5e67e0d35b 100644 > --- a/drivers/iio/imu/bmi160/bmi160.h > +++ b/drivers/iio/imu/bmi160/bmi160.h > @@ -2,9 +2,20 @@ > #ifndef BMI160_H_ > #define BMI160_H_ > > +#include <linux/iio/iio.h> > + > +struct bmi160_data { > + struct regmap *regmap; > + struct iio_trigger *trig; > +}; > + > extern const struct regmap_config bmi160_regmap_config; > > int bmi160_core_probe(struct device *dev, struct regmap *regmap, > - const char *name, bool use_spi); > + const char *name, bool use_spi, int irq); > + > +int bmi160_enable_irq(struct regmap *regmap, bool enable); > + > +int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type); > > #endif /* BMI160_H_ */ > diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c > index ce61026d84c3..e119965e64a3 100644 > --- a/drivers/iio/imu/bmi160/bmi160_core.c > +++ b/drivers/iio/imu/bmi160/bmi160_core.c > @@ -6,12 +6,14 @@ > * > * IIO core driver for BMI160, with support for I2C/SPI busses > * > - * TODO: magnetometer, interrupts, hardware FIFO > + * TODO: magnetometer, hardware FIFO > */ > #include <linux/module.h> > #include <linux/regmap.h> > #include <linux/acpi.h> > #include <linux/delay.h> > +#include <linux/irq.h> > +#include <linux/of_irq.h> > > #include <linux/iio/iio.h> > #include <linux/iio/triggered_buffer.h> > @@ -61,8 +63,32 @@ > #define BMI160_CMD_GYRO_PM_FAST_STARTUP 0x17 > #define BMI160_CMD_SOFTRESET 0xB6 > > +#define BMI160_REG_INT_EN 0x51 > +#define BMI160_DRDY_INT_EN BIT(4) > + > +#define BMI160_REG_INT_OUT_CTRL 0x53 > +#define BMI160_INT_OUT_CTRL_MASK 0x0f > +#define BMI160_INT1_OUT_CTRL_SHIFT 0 > +#define BMI160_INT2_OUT_CTRL_SHIFT 4 > +#define BMI160_LEVEL_TRIGGERED BIT(0) > +#define BMI160_ACTIVE_HIGH BIT(1) > +#define BMI160_OPEN_DRAIN BIT(2) > +#define BMI160_OUTPUT_EN BIT(3) > + > +#define BMI160_REG_INT_LATCH 0x54 > +#define BMI160_INT1_LATCH_MASK BIT(4) > +#define BMI160_INT2_LATCH_MASK BIT(5) > + > +/* INT1 and INT2 are in the opposite order as in INT_OUT_CTRL! */ > +#define BMI160_REG_INT_MAP 0x56 > +#define BMI160_INT1_MAP_DRDY_EN 0x80 > +#define BMI160_INT2_MAP_DRDY_EN 0x08 > + > #define BMI160_REG_DUMMY 0x7F > > +#define BMI160_NORMAL_WRITE_USLEEP 2 > +#define BMI160_SUSPENDED_WRITE_USLEEP 450 > + > #define BMI160_ACCEL_PMU_MIN_USLEEP 3800 > #define BMI160_GYRO_PMU_MIN_USLEEP 80000 > #define BMI160_SOFTRESET_USLEEP 1000 > @@ -105,8 +131,9 @@ enum bmi160_sensor_type { > BMI160_NUM_SENSORS /* must be last */ > }; > > -struct bmi160_data { > - struct regmap *regmap; > +enum bmi160_int_pin { > + BMI160_PIN_INT1, > + BMI160_PIN_INT2 > }; > > const struct regmap_config bmi160_regmap_config = { > @@ -495,7 +522,232 @@ static const char *bmi160_match_acpi_device(struct device *dev) > return dev_name(dev); > } > > -static int bmi160_chip_init(struct bmi160_data *data, bool use_spi) > +static int bmi160_write_conf_reg(struct regmap *regmap, unsigned int reg, > + unsigned int mask, unsigned int bits, > + unsigned int write_usleep) > +{ > + int ret; > + unsigned int val; > + > + ret = regmap_read(regmap, reg, &val); > + if (ret < 0) > + return ret; regmap is 0 on success, so if (ret) preferred. Makes the question on whether this can return a positive number that I raise below more obvious. > + > + val = (val & ~mask) | bits; > + > + ret = regmap_write(regmap, reg, val); > + if (ret < 0) if (ret) > + return ret; > + > + /* > + * We need to wait after writing before we can write again. See the > + * datasheet, page 93. > + */ > + usleep_range(write_usleep, write_usleep + 1000); > + > + return 0; > +} > + > +static int bmi160_config_pin(struct regmap *regmap, enum bmi160_int_pin pin, > + bool open_drain, u8 irq_mask, > + unsigned long write_usleep) > +{ > + int ret; > + u8 int_out_ctrl_shift; > + u8 int_latch_mask; > + u8 int_map_mask; > + u8 int_out_ctrl_mask; > + u8 int_out_ctrl_bits; > + > + switch (pin) { > + case BMI160_PIN_INT1: > + int_out_ctrl_shift = BMI160_INT1_OUT_CTRL_SHIFT; > + int_latch_mask = BMI160_INT1_LATCH_MASK; > + int_map_mask = BMI160_INT1_MAP_DRDY_EN; > + break; > + case BMI160_PIN_INT2: > + int_out_ctrl_shift = BMI160_INT2_OUT_CTRL_SHIFT; > + int_latch_mask = BMI160_INT2_LATCH_MASK; > + int_map_mask = BMI160_INT2_MAP_DRDY_EN; > + break; > + } > + int_out_ctrl_mask = BMI160_INT_OUT_CTRL_MASK << int_out_ctrl_shift; > + > + /* > + * Enable the requested pin with the right settings: > + * - Push-pull/open-drain > + * - Active low/high > + * - Edge/level triggered > + */ > + int_out_ctrl_bits = 0; > + int_out_ctrl_bits |= BMI160_OUTPUT_EN; Combine the two lines above. > + if (open_drain) > + /* Default is push-pull. */ > + int_out_ctrl_bits |= BMI160_OPEN_DRAIN; > + int_out_ctrl_bits |= irq_mask; > + int_out_ctrl_bits <<= int_out_ctrl_shift; > + > + ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_OUT_CTRL, > + int_out_ctrl_mask, int_out_ctrl_bits, > + write_usleep); > + if (ret < 0) if (ret) See both above an below! > + return ret; > + > + /* Set the pin to input mode with no latching. */ > + ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_LATCH, > + int_latch_mask, int_latch_mask, > + write_usleep); > + if (ret < 0) > + return ret; if (ret) > + > + /* Map interrupts to the requested pin. */ > + ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_MAP, > + int_map_mask, int_map_mask, > + write_usleep); > + if (ret < 0) > + return ret; Can ret be greater than 0? I checked, nope. return bmi160_write_conf_reg. (note btw that I review backwards as code makes more sense that way to me!) > + > + return 0; > +} > + > +int bmi160_enable_irq(struct regmap *regmap, bool enable) > +{ > + int ret; > + unsigned int enable_bit; = 0; > + > + if (enable) > + enable_bit = BMI160_DRDY_INT_EN; > + else > + enable_bit = 0; Then drop the else. > + > + ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_EN, > + BMI160_DRDY_INT_EN, enable_bit, > + BMI160_NORMAL_WRITE_USLEEP); > + return bmi160... > + return ret; > +} > +EXPORT_SYMBOL(bmi160_enable_irq); > + > +static void bmi160_parse_irqname(struct device_node *of_node, int irq, > + bool *int1_enabled, bool *int2_enabled) > +{ > + int ret; > + > + /* of_irq_get_byname returns the IRQ number if the entry is found. */ > + ret = of_irq_get_byname(of_node, "INT1"); > + if (ret == irq) { > + *int1_enabled = true; > + *int2_enabled = false; > + return; > + } > + > + ret = of_irq_get_byname(of_node, "INT2"); > + if (ret == irq) { > + *int1_enabled = false; > + *int2_enabled = true; > + return; > + } > + > + ret = of_irq_get_byname(of_node, "BOTH"); ? I'm not following this. Hopefully the DT docs will make it clear. > + if (ret == irq) { > + *int1_enabled = true; > + *int2_enabled = true; > + return; > + } > + > + /* No interrupt name entries found. */ > + *int1_enabled = false; > + *int2_enabled = false; > +} > + > +static int bmi160_config_device_irq(struct iio_dev *indio_dev, > + int irq, int irq_type) > +{ > + int ret; > + bool int1_enabled; > + bool int1_open_drain; > + bool int2_enabled; > + bool int2_open_drain; > + u8 irq_mask; > + struct bmi160_data *data = iio_priv(indio_dev); > + struct device *dev = regmap_get_device(data->regmap); > + > + /* Edge-triggered, active-low is the default if we set all zeroes. */ > + if (irq_type == IRQF_TRIGGER_RISING) > + irq_mask = BMI160_ACTIVE_HIGH | BMI160_LEVEL_TRIGGERED; > + else if (irq_type == IRQF_TRIGGER_FALLING) > + irq_mask = BMI160_LEVEL_TRIGGERED; > + else if (irq_type == IRQF_TRIGGER_HIGH) > + irq_mask = BMI160_ACTIVE_HIGH; > + else if (irq_type == IRQF_TRIGGER_LOW) > + irq_mask = 0; > + else { > + dev_err(&indio_dev->dev, > + "Invalid interrupt type 0x%x specified\n", irq_type); > + return -EINVAL; > + } > + > + bmi160_parse_irqname(dev->of_node, irq, &int1_enabled, &int2_enabled); > + > + if (!int1_enabled && !int2_enabled) { > + dev_err(&indio_dev->dev, "IRQ specified but not routed anywhere. bmi160,int1-enabled or bmi160,int2-enabled must be set."); > + return -EINVAL; > + } > + > + int1_open_drain = of_property_read_bool(dev->of_node, > + "bmi160,int1-open-drain"); > + int2_open_drain = of_property_read_bool(dev->of_node, > + "bmi160,int2-open-drain"); > + This is fine for this section, but a little confusing as we don't actually register for two interrupts. > + if (int1_enabled) { > + ret = bmi160_config_pin(data->regmap, BMI160_PIN_INT1, > + int1_open_drain, irq_mask, > + BMI160_NORMAL_WRITE_USLEEP); > + if (ret < 0) { > + dev_err(&indio_dev->dev, "Failed to configure INT1 IRQ pin"); > + return ret; > + } > + } > + > + if (int2_enabled) { > + ret = bmi160_config_pin(data->regmap, BMI160_PIN_INT2, > + int2_open_drain, irq_mask, > + BMI160_NORMAL_WRITE_USLEEP); > + if (ret < 0) { > + dev_err(&indio_dev->dev, "Failed to configure INT2 IRQ pin"); > + return ret; > + } > + } > + > + return 0; > +} > + > +static int bmi160_setup_irq(struct iio_dev *indio_dev, int irq) > +{ > + struct irq_data *desc; > + u32 irq_type; > + int ret; > + > + desc = irq_get_irq_data(irq); > + if (!desc) { > + dev_warn(&indio_dev->dev, "Could not find IRQ %d\n", irq); > + return -EINVAL; > + } > + > + irq_type = irqd_get_trigger_type(desc); > + > + ret = bmi160_config_device_irq(indio_dev, irq, irq_type); > + if (ret < 0) > + return ret; > + > + ret = bmi160_probe_trigger(indio_dev, irq, irq_type); > + if (ret < 0) > + return ret; > + > + return 0; > +} > + > +static int bmi160_chip_init(struct bmi160_data *data, bool use_spi, int irq) > { > int ret; > unsigned int val; > @@ -548,7 +800,7 @@ static void bmi160_chip_uninit(void *data) > } > > int bmi160_core_probe(struct device *dev, struct regmap *regmap, > - const char *name, bool use_spi) > + const char *name, bool use_spi, int irq) > { > struct iio_dev *indio_dev; > struct bmi160_data *data; > @@ -561,8 +813,9 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap, > data = iio_priv(indio_dev); > dev_set_drvdata(dev, indio_dev); > data->regmap = regmap; > + data->trig = NULL; Given that data is allocated as part of the iio_device_alloc, its already set to null. No need to do so again as it's a fairly obvious default for a pointer (hence no 'documentation' benefit here). > > - ret = bmi160_chip_init(data, use_spi); > + ret = bmi160_chip_init(data, use_spi, irq); > if (ret < 0) > return ret; > > @@ -585,6 +838,12 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap, > if (ret < 0) > return ret; > > + if (irq) { > + ret = bmi160_setup_irq(indio_dev, irq); > + if (ret < 0) > + return ret; > + } > + > ret = devm_iio_device_register(dev, indio_dev); > if (ret < 0) > return ret; > diff --git a/drivers/iio/imu/bmi160/bmi160_i2c.c b/drivers/iio/imu/bmi160/bmi160_i2c.c > index e36f5e82d400..98467d73c887 100644 > --- a/drivers/iio/imu/bmi160/bmi160_i2c.c > +++ b/drivers/iio/imu/bmi160/bmi160_i2c.c > @@ -32,7 +32,8 @@ static int bmi160_i2c_probe(struct i2c_client *client, > if (id) > name = id->name; > > - return bmi160_core_probe(&client->dev, regmap, name, false); > + return bmi160_core_probe(&client->dev, regmap, > + name, false, client->irq); > } > > static const struct i2c_device_id bmi160_i2c_id[] = { > diff --git a/drivers/iio/imu/bmi160/bmi160_spi.c b/drivers/iio/imu/bmi160/bmi160_spi.c > index c19e3df35559..23e323518873 100644 > --- a/drivers/iio/imu/bmi160/bmi160_spi.c > +++ b/drivers/iio/imu/bmi160/bmi160_spi.c > @@ -24,7 +24,7 @@ static int bmi160_spi_probe(struct spi_device *spi) > (int)PTR_ERR(regmap)); > return PTR_ERR(regmap); > } > - return bmi160_core_probe(&spi->dev, regmap, id->name, true); > + return bmi160_core_probe(&spi->dev, regmap, id->name, true, spi->irq); > } > > static const struct spi_device_id bmi160_spi_id[] = { > diff --git a/drivers/iio/imu/bmi160/bmi160_trigger.c b/drivers/iio/imu/bmi160/bmi160_trigger.c > new file mode 100644 > index 000000000000..96e254eb2f19 > --- /dev/null > +++ b/drivers/iio/imu/bmi160/bmi160_trigger.c > @@ -0,0 +1,60 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * BMI160 - Bosch IMU, trigger/IRQ bits > + * > + * Copyright (c) 2019, Martin Kelly. > + */ It's nice to break up code into relevant files etc, but this one is rather short. Perhaps just put it in the core file? Feel free to add a second copyright notice to that file whilst doing so to reflect this non-trivial addition. > + > +#include <linux/interrupt.h> > +#include <linux/regmap.h> > + > +#include <linux/iio/iio.h> > +#include <linux/iio/trigger.h> > + > +#include "bmi160.h" > + > +static int bmi160_data_rdy_trigger_set_state(struct iio_trigger *trig, > + bool enable) > +{ > + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig); > + struct bmi160_data *data = iio_priv(indio_dev); > + > + return bmi160_enable_irq(data->regmap, enable); > +} > + > +static const struct iio_trigger_ops bmi160_trigger_ops = { > + .set_trigger_state = &bmi160_data_rdy_trigger_set_state, > +}; > + > +int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type) > +{ > + struct bmi160_data *data = iio_priv(indio_dev); > + int ret; > + > + data->trig = devm_iio_trigger_alloc(&indio_dev->dev, "%s-dev%d", > + indio_dev->name, indio_dev->id); > + > + if (data->trig == NULL) > + return -ENOMEM; > + > + ret = devm_request_irq(&indio_dev->dev, irq, > + &iio_trigger_generic_data_rdy_poll, > + irq_type, "bmi160", data->trig); > + if (ret < 0) > + return ret; > + > + data->trig->dev.parent = regmap_get_device(data->regmap); > + data->trig->ops = &bmi160_trigger_ops; > + iio_trigger_set_drvdata(data->trig, indio_dev); > + > + ret = devm_iio_trigger_register(&indio_dev->dev, data->trig); > + if (ret < 0) > + return ret; > + > + indio_dev->trig = iio_trigger_get(data->trig); > + if (ret < 0) > + return ret; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(bmi160_probe_trigger);
On 1/19/19 9:17 AM, Jonathan Cameron wrote: > On Sun, 13 Jan 2019 19:56:18 -0800 > Martin Kelly <martin@martingkelly.com> wrote: > >> From: Martin Kelly <martin@martingkelly.com> >> >> Add interrupt support for the data ready signal on the BMI160, which fires >> an interrupt whenever new accelerometer/gyroscope data is ready to read. > > Various minor comments inline. Mostly fine, but one big 'oddity'. > > I think we only ever request 1 irq, but we have support in here for > configuring the chip to generate two at the same time. Doesn't make > much sense as it stands... Am I missing something? > > Jonathan > Yeah, I initially wrote it not allowing for a "BOTH" mapping but added it afterwards in order to support what the hardware allows. However, thinking about it more, the hardware allows this so that you can map different types of interrupts onto the two pins. So, I agree with you that with just a DRDY interrupt, there's no reason for this mapping, so I've dropped it. Thanks for the other comments; I have made all the changes you suggested and sent a v2. I also added a cleanup patch in the v2 to change the other invocations of "if (ret < 0)" to "if (ret)", as there were a number of them in the driver prior to this change.
diff --git a/drivers/iio/imu/bmi160/Makefile b/drivers/iio/imu/bmi160/Makefile index 10365e493ae2..19e1720c6fe9 100644 --- a/drivers/iio/imu/bmi160/Makefile +++ b/drivers/iio/imu/bmi160/Makefile @@ -2,5 +2,6 @@ # Makefile for Bosch BMI160 IMU # obj-$(CONFIG_BMI160) += bmi160_core.o +obj-$(CONFIG_BMI160) += bmi160_trigger.o obj-$(CONFIG_BMI160_I2C) += bmi160_i2c.o obj-$(CONFIG_BMI160_SPI) += bmi160_spi.o diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h index 2351049d930b..0c5e67e0d35b 100644 --- a/drivers/iio/imu/bmi160/bmi160.h +++ b/drivers/iio/imu/bmi160/bmi160.h @@ -2,9 +2,20 @@ #ifndef BMI160_H_ #define BMI160_H_ +#include <linux/iio/iio.h> + +struct bmi160_data { + struct regmap *regmap; + struct iio_trigger *trig; +}; + extern const struct regmap_config bmi160_regmap_config; int bmi160_core_probe(struct device *dev, struct regmap *regmap, - const char *name, bool use_spi); + const char *name, bool use_spi, int irq); + +int bmi160_enable_irq(struct regmap *regmap, bool enable); + +int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type); #endif /* BMI160_H_ */ diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c index ce61026d84c3..e119965e64a3 100644 --- a/drivers/iio/imu/bmi160/bmi160_core.c +++ b/drivers/iio/imu/bmi160/bmi160_core.c @@ -6,12 +6,14 @@ * * IIO core driver for BMI160, with support for I2C/SPI busses * - * TODO: magnetometer, interrupts, hardware FIFO + * TODO: magnetometer, hardware FIFO */ #include <linux/module.h> #include <linux/regmap.h> #include <linux/acpi.h> #include <linux/delay.h> +#include <linux/irq.h> +#include <linux/of_irq.h> #include <linux/iio/iio.h> #include <linux/iio/triggered_buffer.h> @@ -61,8 +63,32 @@ #define BMI160_CMD_GYRO_PM_FAST_STARTUP 0x17 #define BMI160_CMD_SOFTRESET 0xB6 +#define BMI160_REG_INT_EN 0x51 +#define BMI160_DRDY_INT_EN BIT(4) + +#define BMI160_REG_INT_OUT_CTRL 0x53 +#define BMI160_INT_OUT_CTRL_MASK 0x0f +#define BMI160_INT1_OUT_CTRL_SHIFT 0 +#define BMI160_INT2_OUT_CTRL_SHIFT 4 +#define BMI160_LEVEL_TRIGGERED BIT(0) +#define BMI160_ACTIVE_HIGH BIT(1) +#define BMI160_OPEN_DRAIN BIT(2) +#define BMI160_OUTPUT_EN BIT(3) + +#define BMI160_REG_INT_LATCH 0x54 +#define BMI160_INT1_LATCH_MASK BIT(4) +#define BMI160_INT2_LATCH_MASK BIT(5) + +/* INT1 and INT2 are in the opposite order as in INT_OUT_CTRL! */ +#define BMI160_REG_INT_MAP 0x56 +#define BMI160_INT1_MAP_DRDY_EN 0x80 +#define BMI160_INT2_MAP_DRDY_EN 0x08 + #define BMI160_REG_DUMMY 0x7F +#define BMI160_NORMAL_WRITE_USLEEP 2 +#define BMI160_SUSPENDED_WRITE_USLEEP 450 + #define BMI160_ACCEL_PMU_MIN_USLEEP 3800 #define BMI160_GYRO_PMU_MIN_USLEEP 80000 #define BMI160_SOFTRESET_USLEEP 1000 @@ -105,8 +131,9 @@ enum bmi160_sensor_type { BMI160_NUM_SENSORS /* must be last */ }; -struct bmi160_data { - struct regmap *regmap; +enum bmi160_int_pin { + BMI160_PIN_INT1, + BMI160_PIN_INT2 }; const struct regmap_config bmi160_regmap_config = { @@ -495,7 +522,232 @@ static const char *bmi160_match_acpi_device(struct device *dev) return dev_name(dev); } -static int bmi160_chip_init(struct bmi160_data *data, bool use_spi) +static int bmi160_write_conf_reg(struct regmap *regmap, unsigned int reg, + unsigned int mask, unsigned int bits, + unsigned int write_usleep) +{ + int ret; + unsigned int val; + + ret = regmap_read(regmap, reg, &val); + if (ret < 0) + return ret; + + val = (val & ~mask) | bits; + + ret = regmap_write(regmap, reg, val); + if (ret < 0) + return ret; + + /* + * We need to wait after writing before we can write again. See the + * datasheet, page 93. + */ + usleep_range(write_usleep, write_usleep + 1000); + + return 0; +} + +static int bmi160_config_pin(struct regmap *regmap, enum bmi160_int_pin pin, + bool open_drain, u8 irq_mask, + unsigned long write_usleep) +{ + int ret; + u8 int_out_ctrl_shift; + u8 int_latch_mask; + u8 int_map_mask; + u8 int_out_ctrl_mask; + u8 int_out_ctrl_bits; + + switch (pin) { + case BMI160_PIN_INT1: + int_out_ctrl_shift = BMI160_INT1_OUT_CTRL_SHIFT; + int_latch_mask = BMI160_INT1_LATCH_MASK; + int_map_mask = BMI160_INT1_MAP_DRDY_EN; + break; + case BMI160_PIN_INT2: + int_out_ctrl_shift = BMI160_INT2_OUT_CTRL_SHIFT; + int_latch_mask = BMI160_INT2_LATCH_MASK; + int_map_mask = BMI160_INT2_MAP_DRDY_EN; + break; + } + int_out_ctrl_mask = BMI160_INT_OUT_CTRL_MASK << int_out_ctrl_shift; + + /* + * Enable the requested pin with the right settings: + * - Push-pull/open-drain + * - Active low/high + * - Edge/level triggered + */ + int_out_ctrl_bits = 0; + int_out_ctrl_bits |= BMI160_OUTPUT_EN; + if (open_drain) + /* Default is push-pull. */ + int_out_ctrl_bits |= BMI160_OPEN_DRAIN; + int_out_ctrl_bits |= irq_mask; + int_out_ctrl_bits <<= int_out_ctrl_shift; + + ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_OUT_CTRL, + int_out_ctrl_mask, int_out_ctrl_bits, + write_usleep); + if (ret < 0) + return ret; + + /* Set the pin to input mode with no latching. */ + ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_LATCH, + int_latch_mask, int_latch_mask, + write_usleep); + if (ret < 0) + return ret; + + /* Map interrupts to the requested pin. */ + ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_MAP, + int_map_mask, int_map_mask, + write_usleep); + if (ret < 0) + return ret; + + return 0; +} + +int bmi160_enable_irq(struct regmap *regmap, bool enable) +{ + int ret; + unsigned int enable_bit; + + if (enable) + enable_bit = BMI160_DRDY_INT_EN; + else + enable_bit = 0; + + ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_EN, + BMI160_DRDY_INT_EN, enable_bit, + BMI160_NORMAL_WRITE_USLEEP); + + return ret; +} +EXPORT_SYMBOL(bmi160_enable_irq); + +static void bmi160_parse_irqname(struct device_node *of_node, int irq, + bool *int1_enabled, bool *int2_enabled) +{ + int ret; + + /* of_irq_get_byname returns the IRQ number if the entry is found. */ + ret = of_irq_get_byname(of_node, "INT1"); + if (ret == irq) { + *int1_enabled = true; + *int2_enabled = false; + return; + } + + ret = of_irq_get_byname(of_node, "INT2"); + if (ret == irq) { + *int1_enabled = false; + *int2_enabled = true; + return; + } + + ret = of_irq_get_byname(of_node, "BOTH"); + if (ret == irq) { + *int1_enabled = true; + *int2_enabled = true; + return; + } + + /* No interrupt name entries found. */ + *int1_enabled = false; + *int2_enabled = false; +} + +static int bmi160_config_device_irq(struct iio_dev *indio_dev, + int irq, int irq_type) +{ + int ret; + bool int1_enabled; + bool int1_open_drain; + bool int2_enabled; + bool int2_open_drain; + u8 irq_mask; + struct bmi160_data *data = iio_priv(indio_dev); + struct device *dev = regmap_get_device(data->regmap); + + /* Edge-triggered, active-low is the default if we set all zeroes. */ + if (irq_type == IRQF_TRIGGER_RISING) + irq_mask = BMI160_ACTIVE_HIGH | BMI160_LEVEL_TRIGGERED; + else if (irq_type == IRQF_TRIGGER_FALLING) + irq_mask = BMI160_LEVEL_TRIGGERED; + else if (irq_type == IRQF_TRIGGER_HIGH) + irq_mask = BMI160_ACTIVE_HIGH; + else if (irq_type == IRQF_TRIGGER_LOW) + irq_mask = 0; + else { + dev_err(&indio_dev->dev, + "Invalid interrupt type 0x%x specified\n", irq_type); + return -EINVAL; + } + + bmi160_parse_irqname(dev->of_node, irq, &int1_enabled, &int2_enabled); + + if (!int1_enabled && !int2_enabled) { + dev_err(&indio_dev->dev, "IRQ specified but not routed anywhere. bmi160,int1-enabled or bmi160,int2-enabled must be set."); + return -EINVAL; + } + + int1_open_drain = of_property_read_bool(dev->of_node, + "bmi160,int1-open-drain"); + int2_open_drain = of_property_read_bool(dev->of_node, + "bmi160,int2-open-drain"); + + if (int1_enabled) { + ret = bmi160_config_pin(data->regmap, BMI160_PIN_INT1, + int1_open_drain, irq_mask, + BMI160_NORMAL_WRITE_USLEEP); + if (ret < 0) { + dev_err(&indio_dev->dev, "Failed to configure INT1 IRQ pin"); + return ret; + } + } + + if (int2_enabled) { + ret = bmi160_config_pin(data->regmap, BMI160_PIN_INT2, + int2_open_drain, irq_mask, + BMI160_NORMAL_WRITE_USLEEP); + if (ret < 0) { + dev_err(&indio_dev->dev, "Failed to configure INT2 IRQ pin"); + return ret; + } + } + + return 0; +} + +static int bmi160_setup_irq(struct iio_dev *indio_dev, int irq) +{ + struct irq_data *desc; + u32 irq_type; + int ret; + + desc = irq_get_irq_data(irq); + if (!desc) { + dev_warn(&indio_dev->dev, "Could not find IRQ %d\n", irq); + return -EINVAL; + } + + irq_type = irqd_get_trigger_type(desc); + + ret = bmi160_config_device_irq(indio_dev, irq, irq_type); + if (ret < 0) + return ret; + + ret = bmi160_probe_trigger(indio_dev, irq, irq_type); + if (ret < 0) + return ret; + + return 0; +} + +static int bmi160_chip_init(struct bmi160_data *data, bool use_spi, int irq) { int ret; unsigned int val; @@ -548,7 +800,7 @@ static void bmi160_chip_uninit(void *data) } int bmi160_core_probe(struct device *dev, struct regmap *regmap, - const char *name, bool use_spi) + const char *name, bool use_spi, int irq) { struct iio_dev *indio_dev; struct bmi160_data *data; @@ -561,8 +813,9 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap, data = iio_priv(indio_dev); dev_set_drvdata(dev, indio_dev); data->regmap = regmap; + data->trig = NULL; - ret = bmi160_chip_init(data, use_spi); + ret = bmi160_chip_init(data, use_spi, irq); if (ret < 0) return ret; @@ -585,6 +838,12 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap, if (ret < 0) return ret; + if (irq) { + ret = bmi160_setup_irq(indio_dev, irq); + if (ret < 0) + return ret; + } + ret = devm_iio_device_register(dev, indio_dev); if (ret < 0) return ret; diff --git a/drivers/iio/imu/bmi160/bmi160_i2c.c b/drivers/iio/imu/bmi160/bmi160_i2c.c index e36f5e82d400..98467d73c887 100644 --- a/drivers/iio/imu/bmi160/bmi160_i2c.c +++ b/drivers/iio/imu/bmi160/bmi160_i2c.c @@ -32,7 +32,8 @@ static int bmi160_i2c_probe(struct i2c_client *client, if (id) name = id->name; - return bmi160_core_probe(&client->dev, regmap, name, false); + return bmi160_core_probe(&client->dev, regmap, + name, false, client->irq); } static const struct i2c_device_id bmi160_i2c_id[] = { diff --git a/drivers/iio/imu/bmi160/bmi160_spi.c b/drivers/iio/imu/bmi160/bmi160_spi.c index c19e3df35559..23e323518873 100644 --- a/drivers/iio/imu/bmi160/bmi160_spi.c +++ b/drivers/iio/imu/bmi160/bmi160_spi.c @@ -24,7 +24,7 @@ static int bmi160_spi_probe(struct spi_device *spi) (int)PTR_ERR(regmap)); return PTR_ERR(regmap); } - return bmi160_core_probe(&spi->dev, regmap, id->name, true); + return bmi160_core_probe(&spi->dev, regmap, id->name, true, spi->irq); } static const struct spi_device_id bmi160_spi_id[] = { diff --git a/drivers/iio/imu/bmi160/bmi160_trigger.c b/drivers/iio/imu/bmi160/bmi160_trigger.c new file mode 100644 index 000000000000..96e254eb2f19 --- /dev/null +++ b/drivers/iio/imu/bmi160/bmi160_trigger.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * BMI160 - Bosch IMU, trigger/IRQ bits + * + * Copyright (c) 2019, Martin Kelly. + */ + +#include <linux/interrupt.h> +#include <linux/regmap.h> + +#include <linux/iio/iio.h> +#include <linux/iio/trigger.h> + +#include "bmi160.h" + +static int bmi160_data_rdy_trigger_set_state(struct iio_trigger *trig, + bool enable) +{ + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig); + struct bmi160_data *data = iio_priv(indio_dev); + + return bmi160_enable_irq(data->regmap, enable); +} + +static const struct iio_trigger_ops bmi160_trigger_ops = { + .set_trigger_state = &bmi160_data_rdy_trigger_set_state, +}; + +int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type) +{ + struct bmi160_data *data = iio_priv(indio_dev); + int ret; + + data->trig = devm_iio_trigger_alloc(&indio_dev->dev, "%s-dev%d", + indio_dev->name, indio_dev->id); + + if (data->trig == NULL) + return -ENOMEM; + + ret = devm_request_irq(&indio_dev->dev, irq, + &iio_trigger_generic_data_rdy_poll, + irq_type, "bmi160", data->trig); + if (ret < 0) + return ret; + + data->trig->dev.parent = regmap_get_device(data->regmap); + data->trig->ops = &bmi160_trigger_ops; + iio_trigger_set_drvdata(data->trig, indio_dev); + + ret = devm_iio_trigger_register(&indio_dev->dev, data->trig); + if (ret < 0) + return ret; + + indio_dev->trig = iio_trigger_get(data->trig); + if (ret < 0) + return ret; + + return 0; +} +EXPORT_SYMBOL_GPL(bmi160_probe_trigger);