diff mbox

input: wrong irq requested in edt-ft5x06.c

Message ID 1348066434-2168-1-git-send-email-sbabic@denx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Stefano Babic Sept. 19, 2012, 2:53 p.m. UTC
The probe function checks for integrity the pdata->irq_pin,
but then does not request this line for interrupt.
For this reason, no interrupts are generated.

Tested on a AM3517 board with EP0700M06

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: Simon Budig <simon.budig@kernelconcepts.de>
CC: Guenter Roeck <linux@roeck-us.net>
---
 drivers/input/touchscreen/edt-ft5x06.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Dmitry Torokhov Sept. 19, 2012, 4:58 p.m. UTC | #1
Hi Stefano,

On Wed, Sep 19, 2012 at 04:53:54PM +0200, Stefano Babic wrote:
> The probe function checks for integrity the pdata->irq_pin,
> but then does not request this line for interrupt.
> For this reason, no interrupts are generated.
> 
> Tested on a AM3517 board with EP0700M06

Why is your board code does not set client->irq properly?

Thanks.

> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> CC: Simon Budig <simon.budig@kernelconcepts.de>
> CC: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/input/touchscreen/edt-ft5x06.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index b06a5e3..9608281 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -788,7 +788,8 @@ static int __devinit edt_ft5x06_ts_probe(struct i2c_client *client,
>  	input_set_drvdata(input, tsdata);
>  	i2c_set_clientdata(client, tsdata);
>  
> -	error = request_threaded_irq(client->irq, NULL, edt_ft5x06_ts_isr,
> +	error = request_threaded_irq(gpio_to_irq(pdata->irq_pin), NULL,
> +				     edt_ft5x06_ts_isr,
>  				     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>  				     client->name, tsdata);
>  	if (error) {
> -- 
> 1.7.9.5
>
Stefano Babic Sept. 20, 2012, 8:18 a.m. UTC | #2
On 19/09/2012 18:58, Dmitry Torokhov wrote:
> Hi Stefano,
> 

Hi Dmitry,

> On Wed, Sep 19, 2012 at 04:53:54PM +0200, Stefano Babic wrote:
>> The probe function checks for integrity the pdata->irq_pin,
>> but then does not request this line for interrupt.
>> For this reason, no interrupts are generated.
>>
>> Tested on a AM3517 board with EP0700M06
> 
> Why is your board code does not set client->irq properly?

My concern is related that there are two different setup for the irq.
Near the client structure, the driver uses a an own platform data
structure edt_ft5x06_platform_data, where one filed is irq_pin. In whole
driver the pdata->irq_pin is used, and client->irq is used only for
requesting the irq.

But they contain the same information, as the irq number can be get easy
from irq_pin with gpio_to_irq(). Having both, it is possible to set them
to different values, and this is wrong.

Regards,
Stefano
Simon Budig Sept. 20, 2012, 8:31 a.m. UTC | #3
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/20/2012 10:18 AM, Stefano Babic wrote:
> But they contain the same information, as the irq number can be get
> easy from irq_pin with gpio_to_irq(). Having both, it is possible
> to set them to different values, and this is wrong.

gpio_to_irq is not available on all platforms. There was a discussion
on that topic quite a while ago. Moving the irq setup code to the
board file was the conclusion from that discussion.

Bye,
         Simon


- -- 
       Simon Budig                        kernel concepts GmbH
       simon.budig@kernelconcepts.de      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iEYEARECAAYFAlBa1FQACgkQO2O/RXesiHDBOACfamvKFxTYUR9TdVSLhTobthoG
KKwAniPJtTiufxLrATH+n8DwlzuPsok0
=QqF7
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefano Babic Sept. 20, 2012, 8:56 a.m. UTC | #4
On 20/09/2012 10:31, Simon Budig wrote:
> On 09/20/2012 10:18 AM, Stefano Babic wrote:
>> But they contain the same information, as the irq number can be
>> get easy from irq_pin with gpio_to_irq(). Having both, it is
>> possible to set them to different values, and this is wrong.
> 
> gpio_to_irq is not available on all platforms. There was a
> discussion on that topic quite a while ago.

I missed this issue, thanks !

> Moving the irq setup code to the board file was the conclusion from
> that discussion.

Understood, thanks.

Regards,
Stefano
diff mbox

Patch

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index b06a5e3..9608281 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -788,7 +788,8 @@  static int __devinit edt_ft5x06_ts_probe(struct i2c_client *client,
 	input_set_drvdata(input, tsdata);
 	i2c_set_clientdata(client, tsdata);
 
-	error = request_threaded_irq(client->irq, NULL, edt_ft5x06_ts_isr,
+	error = request_threaded_irq(gpio_to_irq(pdata->irq_pin), NULL,
+				     edt_ft5x06_ts_isr,
 				     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 				     client->name, tsdata);
 	if (error) {