| Message ID | 1550596338-24220-2-git-send-email-stefan.popa@analog.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: imu: adis16480: Add support for ADIS1649x family of devices | expand |
On Tue, 19 Feb 2019 19:12:13 +0200 Stefan Popa <stefan.popa@analog.com> wrote: > The FNCTIO_CTRL register, Bits[3:0] provide three configuration options > for the data ready function: on/off, polarity, and DIOx line. The > factory default assigns DIO2 as a positive polarity, data ready signal. > > The adis16480_enable_irq() function, overwrites this configuration when > it enables/disables the data ready pin by only setting BIT[3]. > As a result, the data ready signal becomes DIO1 pin which is assigned as > negative polarity. > > This patch reads the FNCTIO_CTRL register and creates a mask, such that > only data ready enable (BIT[3]) will be modified when > adis16480_enable_irq function is called. So this is potentially a problem. As I read this, we just changed the default. So a device that has been relying on this 'bug' for a long time will now not work as it will be expecting the interrupt on the wrong physical pin. So, whilst it might seem logical to let the device stay with it's default we can't do it because the defacto Linux default is the other choice. The delights of having to support old 'bugs' :) Jonathan > > Signed-off-by: Stefan Popa <stefan.popa@analog.com> > --- > drivers/iio/imu/adis16480.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > index a27fe20..d222188 100644 > --- a/drivers/iio/imu/adis16480.c > +++ b/drivers/iio/imu/adis16480.c > @@ -9,6 +9,7 @@ > * > */ > > +#include <linux/bitfield.h> > #include <linux/interrupt.h> > #include <linux/delay.h> > #include <linux/mutex.h> > @@ -107,6 +108,10 @@ > #define ADIS16480_FIR_COEF_C(x) ADIS16480_FIR_COEF(0x09, (x)) > #define ADIS16480_FIR_COEF_D(x) ADIS16480_FIR_COEF(0x0B, (x)) > > +/* ADIS16480_REG_FNCTIO_CTRL */ > +#define ADIS16480_DRDY_EN_MSK BIT(3) > +#define ADIS16480_DRDY_EN(x) FIELD_PREP(ADIS16480_DRDY_EN_MSK, x) > + > struct adis16480_chip_info { > unsigned int num_channels; > const struct iio_chan_spec *channels; > @@ -741,8 +746,17 @@ static int adis16480_stop_device(struct iio_dev *indio_dev) > > static int adis16480_enable_irq(struct adis *adis, bool enable) > { > - return adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, > - enable ? BIT(3) : 0); > + uint16_t val; > + int ret; > + > + ret = adis_read_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, &val); > + if (ret < 0) > + return ret; > + > + val &= ~ADIS16480_DRDY_EN_MSK; > + val |= ADIS16480_DRDY_EN(enable); > + > + return adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, val); > } > > static int adis16480_initial_setup(struct iio_dev *indio_dev)
diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c index a27fe20..d222188 100644 --- a/drivers/iio/imu/adis16480.c +++ b/drivers/iio/imu/adis16480.c @@ -9,6 +9,7 @@ * */ +#include <linux/bitfield.h> #include <linux/interrupt.h> #include <linux/delay.h> #include <linux/mutex.h> @@ -107,6 +108,10 @@ #define ADIS16480_FIR_COEF_C(x) ADIS16480_FIR_COEF(0x09, (x)) #define ADIS16480_FIR_COEF_D(x) ADIS16480_FIR_COEF(0x0B, (x)) +/* ADIS16480_REG_FNCTIO_CTRL */ +#define ADIS16480_DRDY_EN_MSK BIT(3) +#define ADIS16480_DRDY_EN(x) FIELD_PREP(ADIS16480_DRDY_EN_MSK, x) + struct adis16480_chip_info { unsigned int num_channels; const struct iio_chan_spec *channels; @@ -741,8 +746,17 @@ static int adis16480_stop_device(struct iio_dev *indio_dev) static int adis16480_enable_irq(struct adis *adis, bool enable) { - return adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, - enable ? BIT(3) : 0); + uint16_t val; + int ret; + + ret = adis_read_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, &val); + if (ret < 0) + return ret; + + val &= ~ADIS16480_DRDY_EN_MSK; + val |= ADIS16480_DRDY_EN(enable); + + return adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, val); } static int adis16480_initial_setup(struct iio_dev *indio_dev)
The FNCTIO_CTRL register, Bits[3:0] provide three configuration options for the data ready function: on/off, polarity, and DIOx line. The factory default assigns DIO2 as a positive polarity, data ready signal. The adis16480_enable_irq() function, overwrites this configuration when it enables/disables the data ready pin by only setting BIT[3]. As a result, the data ready signal becomes DIO1 pin which is assigned as negative polarity. This patch reads the FNCTIO_CTRL register and creates a mask, such that only data ready enable (BIT[3]) will be modified when adis16480_enable_irq function is called. Signed-off-by: Stefan Popa <stefan.popa@analog.com> --- drivers/iio/imu/adis16480.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)