[PATCH] pcf8574_keypad: use request_devm_threaded_irq

[PATCH] pcf8574_keypad: use request_devm_threaded_irq

[Michael Grzeschik]

We can use the devres API here. Remove the extra cleanup codepath.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/input/misc/pcf8574_keypad.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
index 97f711a..62fcf36 100644
--- a/drivers/input/misc/pcf8574_keypad.c
+++ b/drivers/input/misc/pcf8574_keypad.c
@@ -131,7 +131,8 @@ static int pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_i
 
 	lp->laststate = read_state(lp);
 
-	ret = request_threaded_irq(client->irq, NULL, pcf8574_kp_irq_handler,
+	ret = devm_request_threaded_irq(&client->dev, client->irq,
+				   NULL, pcf8574_kp_irq_handler,
 				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 				   DRV_NAME, lp);
 	if (ret) {
@@ -142,14 +143,12 @@ static int pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_i
 	ret = input_register_device(idev);
 	if (ret) {
 		dev_err(&client->dev, "input_register_device() failed\n");
-		goto fail_free_irq;
+		goto fail_free_device;
 	}
 
 	i2c_set_clientdata(client, lp);
 	return 0;
 
- fail_free_irq:
-	free_irq(client->irq, lp);
  fail_free_device:
 	input_free_device(idev);
  fail_allocate:
-- 
2.1.4

Re: [PATCH] pcf8574_keypad: use request_devm_threaded_irq

[Dmitry Torokhov]

On Fri, Jun 19, 2015 at 12:35:54PM +0200, Michael Grzeschik wrote:
> We can use the devres API here. Remove the extra cleanup codepath.

We could, but not like that (hint: what is the order of releasing
resources with your patch?). Also, calling free_irq() on IRQ requested
by devm_* is not quite valid. Did you test the patch, especially the
remove portion?

> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/input/misc/pcf8574_keypad.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
> index 97f711a..62fcf36 100644
> --- a/drivers/input/misc/pcf8574_keypad.c
> +++ b/drivers/input/misc/pcf8574_keypad.c
> @@ -131,7 +131,8 @@ static int pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_i
>  
>  	lp->laststate = read_state(lp);
>  
> -	ret = request_threaded_irq(client->irq, NULL, pcf8574_kp_irq_handler,
> +	ret = devm_request_threaded_irq(&client->dev, client->irq,
> +				   NULL, pcf8574_kp_irq_handler,
>  				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
>  				   DRV_NAME, lp);
>  	if (ret) {
> @@ -142,14 +143,12 @@ static int pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_i
>  	ret = input_register_device(idev);
>  	if (ret) {
>  		dev_err(&client->dev, "input_register_device() failed\n");
> -		goto fail_free_irq;
> +		goto fail_free_device;
>  	}
>  
>  	i2c_set_clientdata(client, lp);
>  	return 0;
>  
> - fail_free_irq:
> -	free_irq(client->irq, lp);
>   fail_free_device:
>  	input_free_device(idev);
>   fail_allocate:
> -- 
> 2.1.4
> 

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in

Re: [PATCH] pcf8574_keypad: use request_devm_threaded_irq

[Michael Grzeschik]

On Mon, Jun 22, 2015 at 01:55:52PM -0700, Dmitry Torokhov wrote:
> On Fri, Jun 19, 2015 at 12:35:54PM +0200, Michael Grzeschik wrote:
> > We can use the devres API here. Remove the extra cleanup codepath.
> 
> We could, but not like that (hint: what is the order of releasing
> resources with your patch?). Also, calling free_irq() on IRQ requested
> by devm_* is not quite valid. Did you test the patch, especially the
> remove portion?

Doh, thanks for the hints. I will send v2.

Michael

> 
> > 
> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > ---
> >  drivers/input/misc/pcf8574_keypad.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
> > index 97f711a..62fcf36 100644
> > --- a/drivers/input/misc/pcf8574_keypad.c
> > +++ b/drivers/input/misc/pcf8574_keypad.c
> > @@ -131,7 +131,8 @@ static int pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_i
> >  
> >  	lp->laststate = read_state(lp);
> >  
> > -	ret = request_threaded_irq(client->irq, NULL, pcf8574_kp_irq_handler,
> > +	ret = devm_request_threaded_irq(&client->dev, client->irq,
> > +				   NULL, pcf8574_kp_irq_handler,
> >  				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> >  				   DRV_NAME, lp);
> >  	if (ret) {
> > @@ -142,14 +143,12 @@ static int pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_i
> >  	ret = input_register_device(idev);
> >  	if (ret) {
> >  		dev_err(&client->dev, "input_register_device() failed\n");
> > -		goto fail_free_irq;
> > +		goto fail_free_device;
> >  	}
> >  
> >  	i2c_set_clientdata(client, lp);
> >  	return 0;
> >  
> > - fail_free_irq:
> > -	free_irq(client->irq, lp);
> >   fail_free_device:
> >  	input_free_device(idev);
> >   fail_allocate:
> > -- 
> > 2.1.4
> > 
> 
> Thanks.
> 
> -- 
> Dmitry
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in