Re: [PATCH] iio: adis16480: fix FNCTIO_CTRL corruption when enabling IRQ

[Lars-Peter Clausen <lars@metafoo.de>]

Hi,

Hm, right. But the intention of the driver is to use DIO1. Changing this to
DIO2 by default will break all existing users.

This change should be part of a patch which allows to configure which
interrupt output to use and also the interrupt polarity.

- Lars

On 03/09/2016 12:39 PM, Vlad Banea wrote:
> Hi,
> 
> Thanks for your answer. I'm using this driver with the ADIS16485 device and
> the default value for this register is 0x000D:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ADIS16485.pdf
> (Table 89)
> 
> When the driver enables the interrupt, the Data Ready Line selection and
> polarity are changed, and I never receive the interrupt.
> 
> Vlad
> 
> 
> On Wed, Mar 9, 2016 at 4:22 AM, Lars-Peter Clausen <lars@metafoo.de
> <mailto:lars@metafoo.de>> wrote:
> 
>     On 03/09/2016 06:28 AM, Vlad Banea wrote:
>     > Enabling the IRQ should leave all other settings in the FNCTIO_CTRL
>     > register untouched: read the whole register, toggle just the enable bit,
>     > before writing it back.
> 
>     Hi,
> 
>     Thanks for the patch. Looks good in general, but it's not a fix. The driver
>     does not write this register anywhere else and the reset default value is
>     0x00. So we don't corrupt any other settings since, 0x00 and BIT(3) are the
>     only two settings the driver does at the moment.
> 
>     If the patch is in preparation of future changes that are going to set/clear
>     other bits of the register this should be noted in the commit message.
> 
>     The reason I'm so pedantic here is because fix generally means that the
>     patch needs to be backported to older kernel versions, which is not the case
>     here.
> 
>     - Lars
> 
> 
>     > ---
>     >  drivers/iio/imu/adis16480.c | 15 +++++++++++++--
>     >  1 file changed, 13 insertions(+), 2 deletions(-)
>     >
>     > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
>     > index b94bfd3..8473859 100644
>     > --- a/drivers/iio/imu/adis16480.c
>     > +++ b/drivers/iio/imu/adis16480.c
>     > @@ -738,8 +738,19 @@ 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);
>     > +     u16 fnctio_ctrl;
>     > +     int ret;
>     > +
>     > +     ret = adis_read_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL,
>     &fnctio_ctrl);
>     > +     if (ret < 0)
>     > +             return ret;
>     > +
>     > +     if (enable)
>     > +             fnctio_ctrl |= BIT(3);
>     > +     else
>     > +             fnctio_ctrl &= ~BIT(3);
>     > +
>     > +     return adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL,
>     fnctio_ctrl);
>     >  }
>     >
>     >  static int adis16480_initial_setup(struct iio_dev *indio_dev)
>     >
> 
>