[PATCHv6] staging/iio/adc: change the MXS touchscreen driver implementation

Commit Message

Juergen Borleis Feb. 24, 2014, 1:16 p.m. UTC

Hi Alexandre,

> While I don't have much experience with the TS part of the code but I
> can investigate if you don't have any idea.

can you please test the following patch?
After releasing the touchscreen the internal state machine was left in a
wrong state.


Regards,
Juergen

Comments

Dan Carpenter Feb. 24, 2014, 1:33 p.m. UTC | #1

On Mon, Feb 24, 2014 at 02:16:24PM +0100, Juergen Beisert wrote:
> Hi Alexandre,
> 
> > While I don't have much experience with the TS part of the code but I
> > can investigate if you don't have any idea.
> 
> can you please test the following patch?
> After releasing the touchscreen the internal state machine was left in a
> wrong state.
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index e2dd783..558a76c 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -698,6 +698,7 @@ static void mxs_lradc_finish_touch_event(struct mxs_lradc *lradc, bool valid)
>         }
> 
>         /* if it is released, wait for the next touch via IRQ */
> +       lradc->cur_plate = LRADC_TOUCH;
>         mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
>         mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
>  }

I am confused.  Why don't you send this like a normal patch?

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Alexandre Belloni Feb. 24, 2014, 2:26 p.m. UTC | #2

Hi Juergen,

On 24/02/2014 at 14:16:24 +0100, Juergen Beisert wrote :
> Hi Alexandre,
> 
> > While I don't have much experience with the TS part of the code but I
> > can investigate if you don't have any idea.
> 
> can you please test the following patch?
> After releasing the touchscreen the internal state machine was left in a
> wrong state.
> 

This seems to solve the issue. You can send the patch with my Tested-by.

It is probably worth trying to get it in 3.14.

Thanks !

> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index e2dd783..558a76c 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -698,6 +698,7 @@ static void mxs_lradc_finish_touch_event(struct mxs_lradc *lradc, bool valid)
>         }
> 
>         /* if it is released, wait for the next touch via IRQ */
> +       lradc->cur_plate = LRADC_TOUCH;
>         mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
>         mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
>  }
> 
> Regards,
> Juergen
> 
> -- 
> Pengutronix e.K.                              | Juergen Beisert             |
> Linux Solutions for Science and Industry      | Phone: [phone]-5128 |
> Peiner Str. 6-8, 31137 Hildesheim, Germany    | Fax:   [phone]-5555 |
> Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

Juergen Borleis Feb. 24, 2014, 2:38 p.m. UTC | #3

Hi Dan,

On Monday 24 February 2014 14:33:16 Dan Carpenter wrote:
> On Mon, Feb 24, 2014 at 02:16:24PM +0100, Juergen Beisert wrote:
> > > While I don't have much experience with the TS part of the code but I
> > > can investigate if you don't have any idea.
> >
> > can you please test the following patch?
> > After releasing the touchscreen the internal state machine was left in a
> > wrong state.
> >
> > diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> > index e2dd783..558a76c 100644
> > --- a/drivers/staging/iio/adc/mxs-lradc.c  
> > +++ b/drivers/staging/iio/adc/mxs-lradc.c
> > @@ -698,6 +698,7 @@ static void mxs_lradc_finish_touch_event(struct
> > mxs_lradc *lradc, bool valid) }
> >
> >         /* if it is released, wait for the next touch via IRQ */
> > +       lradc->cur_plate = LRADC_TOUCH;
> >         mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
> >         mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
> >         } 
>
> I am confused.  Why don't you send this like a normal patch?

For testing fist. But you are right, patch will follow.

Regards,
Juergen

Dan Carpenter Feb. 24, 2014, 2:47 p.m. UTC | #4

On Mon, Feb 24, 2014 at 03:38:26PM +0100, Juergen Beisert wrote:
> Hi Dan,
> 
> On Monday 24 February 2014 14:33:16 Dan Carpenter wrote:
> > On Mon, Feb 24, 2014 at 02:16:24PM +0100, Juergen Beisert wrote:
> > > > While I don't have much experience with the TS part of the code but I
> > > > can investigate if you don't have any idea.
> > >
> > > can you please test the following patch?
> > > After releasing the touchscreen the internal state machine was left in a
> > > wrong state.
> > >
> > > diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> > > index e2dd783..558a76c 100644
> > > --- a/drivers/staging/iio/adc/mxs-lradc.c  
> > > +++ b/drivers/staging/iio/adc/mxs-lradc.c
> > > @@ -698,6 +698,7 @@ static void mxs_lradc_finish_touch_event(struct
> > > mxs_lradc *lradc, bool valid) }
> > >
> > >         /* if it is released, wait for the next touch via IRQ */
> > > +       lradc->cur_plate = LRADC_TOUCH;
> > >         mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
> > >         mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
> > >         } 
> >
> > I am confused.  Why don't you send this like a normal patch?
> 
> For testing fist. But you are right, patch will follow.
> 

Never mind, I suck at reading.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jonathan Cameron Feb. 24, 2014, 5:14 p.m. UTC | #5

On February 24, 2014 2:26:58 PM GMT+00:00, Alexandre Belloni <[e-mail]> wrote:
>Hi Juergen,
>
>On 24/02/2014 at 14:16:24 +0100, Juergen Beisert wrote :
>> Hi Alexandre,
>> 
>> > While I don't have much experience with the TS part of the code but
>I
>> > can investigate if you don't have any idea.
>> 
>> can you please test the following patch?
>> After releasing the touchscreen the internal state machine was left
>in a
>> wrong state.
>> 
>
>This seems to solve the issue. You can send the patch with my
>Tested-by.
>
>It is probably worth trying to get it in 3.14.
Definitely. There is time.
>
>Thanks !
>
>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
>b/drivers/staging/iio/adc/mxs-lradc.c
>> index e2dd783..558a76c 100644
>> --- a/drivers/staging/iio/adc/mxs-lradc.c
>> +++ b/drivers/staging/iio/adc/mxs-lradc.c
>> @@ -698,6 +698,7 @@ static void mxs_lradc_finish_touch_event(struct
>mxs_lradc *lradc, bool valid)
>>         }
>> 
>>         /* if it is released, wait for the next touch via IRQ */
>> +       lradc->cur_plate = LRADC_TOUCH;
>>         mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ,
>LRADC_CTRL1);
>>         mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN,
>LRADC_CTRL1);
>>  }
>> 
>> Regards,
>> Juergen
>> 
>> -- 
>> Pengutronix e.K.                              | Juergen Beisert      
>      |
>> Linux Solutions for Science and Industry      | Phone:
>[phone]-5128 |
>> Peiner Str. 6-8, 31137 Hildesheim, Germany    | Fax:  
>[phone]-5555 |
>> Amtsgericht Hildesheim, HRA 2686              |
>http://www.pengutronix.de/  |

Patch

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index e2dd783..558a76c 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -698,6 +698,7 @@  static void mxs_lradc_finish_touch_event(struct mxs_lradc *lradc, bool valid)
        }

        /* if it is released, wait for the next touch via IRQ */
+       lradc->cur_plate = LRADC_TOUCH;
        mxs_lradc_reg_clear(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ, LRADC_CTRL1);
        mxs_lradc_reg_set(lradc, LRADC_CTRL1_TOUCH_DETECT_IRQ_EN, LRADC_CTRL1);
 }