lkml.org 
[lkml]   [2016]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] iio: adis16480: fix FNCTIO_CTRL corruption when enabling IRQ
From
Hi,

Ok, I'll change the patch to keep the old behaviour as the default one
but add a device tree settings for selecting the DIO and polarity as
you said.

Vlad

On Wed, Mar 9, 2016 at 11:41 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> Hi,
>
> The intention was to use DIO1 by default, which the driver does and which
> has been tested. With your patch it uses DIO2 by default, which is the
> correct setting for your board.
>
> - Lars
>
> On 03/09/2016 04:27 PM, Vlad Banea wrote:
>>
>> Hi,
>>
>> Since the driver is never setting the FNCTIO_CTRL register, I'd say the
>> intention is to use the default settings.
>> The clearing of all the other bits as we toggle the interrupt seems
>> unintentional, and that's what the proposed patch addresses. The ADIS16485
>> device is not usable without this patch with the most up to date driver, and
>> becomes fully functional when the patch is applied..
>>
>> I wouldn't mind contributing to allow the driver to fully configure this
>> register, but I'm not sure what user interface this should go through.
>>
>> Thanks
>> Vlad
>>
>>
>>
>> On Wed, Mar 9, 2016 at 7:21 AM, Lars-Peter Clausen <lars@metafoo.de
>> <mailto:lars@metafoo.de>> wrote:
>>
>> 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>
>> > <mailto: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)
>> > >
>> >
>> >
>>
>>
>

\
 
 \ /
  Last update: 2016-03-09 18:41    [W:0.130 / U:0.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site