Re: [PATCH] tsl2563: fixed bug with disabling interrupts

Re: [PATCH] tsl2563: fixed bug with disabling interrupts

[Jonathan Cameron]

cc'd iio mailing list and Jon Brenner.
Being lazy I haven't dug out the datahsheet to check this (my board has a broken
track to the interrupt line so can't easily test this!)

> In tsl_2563_write_interrupt_config and tsl2562_remove, interrupts are not
> disabled where they should be. This seems to be from a mistake of using |=
> instead of &= in 2 lines of code.
> 
> Signed-off-by: Derek Basehore <dbasehore@chromium.org>
> ---
>  drivers/staging/iio/light/tsl2563.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2563.c b/drivers/staging/iio/light/tsl2563.c
> index 954ca2c..3f72543 100644
> --- a/drivers/staging/iio/light/tsl2563.c
> +++ b/drivers/staging/iio/light/tsl2563.c
> @@ -652,7 +652,7 @@ static int tsl2563_write_interrupt_config(struct iio_dev *indio_dev,
>  	}
>  
>  	if (!state && (chip->intr & 0x30)) {
> -		chip->intr |= ~0x30;
> +		chip->intr &= ~0x30;
>  		ret = i2c_smbus_write_byte_data(chip->client,
>  						TSL2563_CMD | TSL2563_REG_INT,
>  						chip->intr);
> @@ -814,7 +814,7 @@ static int __devexit tsl2563_remove(struct i2c_client *client)
>  	if (!chip->int_enabled)
>  		cancel_delayed_work(&chip->poweroff_work);
>  	/* Ensure that interrupts are disabled - then flush any bottom halves */
> -	chip->intr |= ~0x30;
> +	chip->intr &= ~0x30;
>  	i2c_smbus_write_byte_data(chip->client, TSL2563_CMD | TSL2563_REG_INT,
>  				  chip->intr);
>  	flush_scheduled_work();
> 

Re: [PATCH] tsl2563: fixed bug with disabling interrupts

[Derek Basehore]

[-- Attachment #1: Type: text/plain, Size: 2143 bytes --]

Here's the link to the data sheet

http://cfnewsads.thomasnet.com/pnn-pdf/478523.pdf

We don't actually use this functionality for the ambient light sensor, so
it's untested right now. I just happened to notice this while reading
through the driver. The register in question has specifics starting on page
15.

Thanks


On Sat, Oct 20, 2012 at 2:49 AM, Jonathan Cameron <jic23@kernel.org> wrote:

> cc'd iio mailing list and Jon Brenner.
> Being lazy I haven't dug out the datahsheet to check this (my board has a
> broken
> track to the interrupt line so can't easily test this!)
>
> > In tsl_2563_write_interrupt_config and tsl2562_remove, interrupts are not
> > disabled where they should be. This seems to be from a mistake of using
> |=
> > instead of &= in 2 lines of code.
> >
> > Signed-off-by: Derek Basehore <dbasehore@chromium.org>
> > ---
> >  drivers/staging/iio/light/tsl2563.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/iio/light/tsl2563.c
> b/drivers/staging/iio/light/tsl2563.c
> > index 954ca2c..3f72543 100644
> > --- a/drivers/staging/iio/light/tsl2563.c
> > +++ b/drivers/staging/iio/light/tsl2563.c
> > @@ -652,7 +652,7 @@ static int tsl2563_write_interrupt_config(struct
> iio_dev *indio_dev,
> >       }
> >
> >       if (!state && (chip->intr & 0x30)) {
> > -             chip->intr |= ~0x30;
> > +             chip->intr &= ~0x30;
> >               ret = i2c_smbus_write_byte_data(chip->client,
> >                                               TSL2563_CMD |
> TSL2563_REG_INT,
> >                                               chip->intr);
> > @@ -814,7 +814,7 @@ static int __devexit tsl2563_remove(struct
> i2c_client *client)
> >       if (!chip->int_enabled)
> >               cancel_delayed_work(&chip->poweroff_work);
> >       /* Ensure that interrupts are disabled - then flush any bottom
> halves */
> > -     chip->intr |= ~0x30;
> > +     chip->intr &= ~0x30;
> >       i2c_smbus_write_byte_data(chip->client, TSL2563_CMD |
> TSL2563_REG_INT,
> >                                 chip->intr);
> >       flush_scheduled_work();
> >
>



-- 
Derek

[-- Attachment #2: Type: text/html, Size: 2966 bytes --]