[PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[G, Manjunath Kondaiah]

The failure exit paths seems to be wrong in probe function.
This patch corrects exit failure paths for error handling
cases.

Boot warning incase of request irq failure:

[    1.553985] twl4030_keypad twl4030_keypad: request_irq failed for irq no=369
[    1.561157] ------------[ cut here ]------------
[    1.565795] WARNING: at kernel/irq/manage.c:899 __free_irq+0x88/0x164()
[    1.572418] Trying to free already-free IRQ 369
[    1.576965] Modules linked in:
[    1.580047] [<c00470ec>] (unwind_backtrace+0x0/0xe4) from [<c0078b5c>] (warn_slowpath_common+0x4c/0x64)
[    1.589477] [<c0078b5c>] (warn_slowpath_common+0x4c/0x64) from [<c0078bf4>] (warn_slowpath_fmt+0x2c/0x3c)
[    1.599060] [<c0078bf4>] (warn_slowpath_fmt+0x2c/0x3c) from [<c00adb90>] (__free_irq+0x88/0x164)
[    1.607849] [<c00adb90>] (__free_irq+0x88/0x164) from [<c00adca8>] (free_irq+0x3c/0x5c)
[    1.615875] [<c00adca8>] (free_irq+0x3c/0x5c) from [<c043ab2c>] (twl4030_kp_probe+0x308/0x374)
[    1.624511] [<c043ab2c>] (twl4030_kp_probe+0x308/0x374) from [<c023c338>] (platform_drv_probe+0x14/0x18)
[    1.634033] [<c023c338>] (platform_drv_probe+0x14/0x18) from [<c023b4dc>] (driver_probe_device+0xc8/0x184)
[    1.643707] [<c023b4dc>] (driver_probe_device+0xc8/0x184) from [<c023b600>] (__driver_attach+0x68/0x8c)
[    1.653106] [<c023b600>] (__driver_attach+0x68/0x8c) from [<c023ad34>] (bus_for_each_dev+0x48/0x74)
[    1.662170] [<c023ad34>] (bus_for_each_dev+0x48/0x74) from [<c023a690>] (bus_add_driver+0x9c/0x210)
[    1.671234] [<c023a690>] (bus_add_driver+0x9c/0x210) from [<c023b8f8>] (driver_register+0xa8/0x134)
[    1.680297] [<c023b8f8>] (driver_register+0xa8/0x134) from [<c0041340>] (do_one_initcall+0x58/0x1b4)
[    1.689453] [<c0041340>] (do_one_initcall+0x58/0x1b4) from [<c0008574>] (kernel_init+0x98/0x150)
[    1.698272] [<c0008574>] (kernel_init+0x98/0x150) from [<c0042970>] (kernel_thread_exit+0x0/0x8)
[    1.707214] ---[ end trace 6559b322ad3cbdfe ]---
[    1.718292] twl4030_keypad: probe of twl4030_keypad failed with error -16

Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com>
Cc: linux-input@vger.kernel.org
Cc: Dmitry Torokhov <dtor@mail.ru>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-input@vger.kernel.org
Cc: Tony Lindgren <tony@atomide.com>
---
 version v1 : initial patch
 version v2 : CC'ed input subsystem and arm kernel mailing lists.

 drivers/input/keyboard/twl4030_keypad.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/twl4030_keypad.c b/drivers/input/keyboard/twl4030_keypad.c
index fb16b5e..39a9f30 100644
--- a/drivers/input/keyboard/twl4030_keypad.c
+++ b/drivers/input/keyboard/twl4030_keypad.c
@@ -347,8 +347,7 @@ static int __devinit twl4030_kp_probe(struct platform_device *pdev)
 	kp = kzalloc(sizeof(*kp), GFP_KERNEL);
 	input = input_allocate_device();
 	if (!kp || !input) {
-		error = -ENOMEM;
-		goto err1;
+		return -ENOMEM;
 	}
 
 	/* Get the debug Device */
@@ -406,23 +405,22 @@ static int __devinit twl4030_kp_probe(struct platform_device *pdev)
 	if (error) {
 		dev_info(kp->dbg_dev, "request_irq failed for irq no=%d\n",
 			kp->irq);
-		goto err3;
+		goto err2;
 	}
 
 	/* Enable KP and TO interrupts now. */
 	reg = (u8) ~(KEYP_IMR1_KP | KEYP_IMR1_TO);
 	if (twl4030_kpwrite_u8(kp, reg, KEYP_IMR1)) {
 		error = -EIO;
-		goto err4;
+		goto err3;
 	}
 
 	platform_set_drvdata(pdev, kp);
 	return 0;
 
-err4:
+err3:
 	/* mask all events - we don't care about the result */
 	(void) twl4030_kpwrite_u8(kp, 0xff, KEYP_IMR1);
-err3:
 	free_irq(kp->irq, NULL);
 err2:
 	input_unregister_device(input);
-- 
1.7.0.4

Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[Ameya Palande]

Hi Manjunath,

On Tue, 2010-09-21 at 13:49 +0200, ext G, Manjunath Kondaiah wrote:
> The failure exit paths seems to be wrong in probe function.
> This patch corrects exit failure paths for error handling
> cases.

https://patchwork.kernel.org/patch/160551/
Any comments on this?

Cheers,
Ameya.

RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[G, Manjunath Kondaiah]

Hi,

> -----Original Message-----
> From: Ameya Palande [mailto:ameya.palande@nokia.com] 
> Sent: Tuesday, September 21, 2010 7:04 PM
> To: G, Manjunath Kondaiah
> Cc: linux-omap@vger.kernel.org; linux-input@vger.kernel.org; 
> Dmitry Torokhov; linux-arm-kernel@lists.infradead.org; Tony Lindgren
> Subject: Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> 
> Hi Manjunath,
> 
> On Tue, 2010-09-21 at 13:49 +0200, ext G, Manjunath Kondaiah wrote:
> > The failure exit paths seems to be wrong in probe function.
> > This patch corrects exit failure paths for error handling cases.
> 
> https://patchwork.kernel.org/patch/160551/
> Any comments on this?

Looks fine. Sorry, I didn't look at the change. This version seems to 
be better. 

-Manjunath

Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[Dmitry Torokhov]

Hi,

On Tue, Sep 21, 2010 at 07:11:01PM +0530, G, Manjunath Kondaiah wrote:
> 
> Hi,
> 
> > -----Original Message-----
> > From: Ameya Palande [mailto:ameya.palande@nokia.com] 
> > Sent: Tuesday, September 21, 2010 7:04 PM
> > To: G, Manjunath Kondaiah
> > Cc: linux-omap@vger.kernel.org; linux-input@vger.kernel.org; 
> > Dmitry Torokhov; linux-arm-kernel@lists.infradead.org; Tony Lindgren
> > Subject: Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> > 
> > Hi Manjunath,
> > 
> > On Tue, 2010-09-21 at 13:49 +0200, ext G, Manjunath Kondaiah wrote:
> > > The failure exit paths seems to be wrong in probe function.
> > > This patch corrects exit failure paths for error handling cases.
> > 

And also adds memory leak...


> > https://patchwork.kernel.org/patch/160551/
> > Any comments on this?
> 
> Looks fine. Sorry, I didn't look at the change. This version seems to 
> be better. 
> 

I do not understand why we need to separate memory allocations. It looks
like the minimal patch should be like one below.

Thanks.

-- 
Dmitry


Input: twl4030_keypad - fix error handling path

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

We should not try to call free_irq() when request_irq() failed.

Reported-by: G, Manjunath Kondaiah <manjugk@ti.com>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/keyboard/twl4030_keypad.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)


diff --git a/drivers/input/keyboard/twl4030_keypad.c b/drivers/input/keyboard/twl4030_keypad.c
index fb16b5e..09bef79 100644
--- a/drivers/input/keyboard/twl4030_keypad.c
+++ b/drivers/input/keyboard/twl4030_keypad.c
@@ -406,23 +406,22 @@ static int __devinit twl4030_kp_probe(struct platform_device *pdev)
 	if (error) {
 		dev_info(kp->dbg_dev, "request_irq failed for irq no=%d\n",
 			kp->irq);
-		goto err3;
+		goto err2;
 	}
 
 	/* Enable KP and TO interrupts now. */
 	reg = (u8) ~(KEYP_IMR1_KP | KEYP_IMR1_TO);
 	if (twl4030_kpwrite_u8(kp, reg, KEYP_IMR1)) {
 		error = -EIO;
-		goto err4;
+		goto err3;
 	}
 
 	platform_set_drvdata(pdev, kp);
 	return 0;
 
-err4:
+err3:
 	/* mask all events - we don't care about the result */
 	(void) twl4030_kpwrite_u8(kp, 0xff, KEYP_IMR1);
-err3:
 	free_irq(kp->irq, NULL);
 err2:
 	input_unregister_device(input);

RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[G, Manjunath Kondaiah]

Hi Dmitry,

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] 
> Sent: Tuesday, September 21, 2010 9:57 PM
> To: G, Manjunath Kondaiah
> Cc: Ameya Palande; linux-omap@vger.kernel.org; 
> linux-input@vger.kernel.org; 
> linux-arm-kernel@lists.infradead.org; Tony Lindgren
> Subject: Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> 
> Hi,
> 
> On Tue, Sep 21, 2010 at 07:11:01PM +0530, G, Manjunath Kondaiah wrote:
> > 
> > Hi,
> > 
> > > -----Original Message-----
> > > From: Ameya Palande [mailto:ameya.palande@nokia.com]
> > > Sent: Tuesday, September 21, 2010 7:04 PM
> > > To: G, Manjunath Kondaiah
> > > Cc: linux-omap@vger.kernel.org; 
> linux-input@vger.kernel.org; Dmitry 
> > > Torokhov; linux-arm-kernel@lists.infradead.org; Tony Lindgren
> > > Subject: Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in 
> > > probe
> > > 
> > > Hi Manjunath,
> > > 
> > > On Tue, 2010-09-21 at 13:49 +0200, ext G, Manjunath 
> Kondaiah wrote:
> > > > The failure exit paths seems to be wrong in probe function.
> > > > This patch corrects exit failure paths for error handling cases.
> > > 
> 
> And also adds memory leak...
> 
> 
> > > https://patchwork.kernel.org/patch/160551/
> > > Any comments on this?
> > 
> > Looks fine. Sorry, I didn't look at the change. This 
> version seems to 
> > be better.
> > 
> 
> I do not understand why we need to separate memory 
> allocations. It looks
> like the minimal patch should be like one below.

Thanks. I am ok with this minimal patch.

-Manjunath

> 
> Thanks.
> 
> -- 
> Dmitry
> 
> 
> Input: twl4030_keypad - fix error handling path
> 
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> We should not try to call free_irq() when request_irq() failed.
> 
> Reported-by: G, Manjunath Kondaiah <manjugk@ti.com>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 
>  drivers/input/keyboard/twl4030_keypad.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/drivers/input/keyboard/twl4030_keypad.c 
> b/drivers/input/keyboard/twl4030_keypad.c
> index fb16b5e..09bef79 100644
> --- a/drivers/input/keyboard/twl4030_keypad.c
> +++ b/drivers/input/keyboard/twl4030_keypad.c
> @@ -406,23 +406,22 @@ static int __devinit 
> twl4030_kp_probe(struct platform_device *pdev)
>  	if (error) {
>  		dev_info(kp->dbg_dev, "request_irq failed for 
> irq no=%d\n",
>  			kp->irq);
> -		goto err3;
> +		goto err2;
>  	}
>  
>  	/* Enable KP and TO interrupts now. */
>  	reg = (u8) ~(KEYP_IMR1_KP | KEYP_IMR1_TO);
>  	if (twl4030_kpwrite_u8(kp, reg, KEYP_IMR1)) {
>  		error = -EIO;
> -		goto err4;
> +		goto err3;
>  	}
>  
>  	platform_set_drvdata(pdev, kp);
>  	return 0;
>  
> -err4:
> +err3:
>  	/* mask all events - we don't care about the result */
>  	(void) twl4030_kpwrite_u8(kp, 0xff, KEYP_IMR1);
> -err3:
>  	free_irq(kp->irq, NULL);
>  err2:
>  	input_unregister_device(input);
> 

RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[Datta, Shubhrajyoti]

> -----Original Message-----
> From: linux-input-owner@vger.kernel.org [mailto:linux-input-
> owner@vger.kernel.org] On Behalf Of G, Manjunath Kondaiah
> Sent: Tuesday, September 21, 2010 5:20 PM
> To: linux-omap@vger.kernel.org
> Cc: linux-input@vger.kernel.org; Dmitry Torokhov; linux-arm-
> kernel@lists.infradead.org; Tony Lindgren
> Subject: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> 
> The failure exit paths seems to be wrong in probe function.
> This patch corrects exit failure paths for error handling
> cases.
> 
> Boot warning incase of request irq failure:
> 
> [    1.553985] twl4030_keypad twl4030_keypad: request_irq failed for irq
> no=369
> [    1.561157] ------------[ cut here ]------------
> [    1.565795] WARNING: at kernel/irq/manage.c:899 __free_irq+0x88/0x164()
> [    1.572418] Trying to free already-free IRQ 369
> [    1.576965] Modules linked in:
> [    1.580047] [<c00470ec>] (unwind_backtrace+0x0/0xe4) from [<c0078b5c>]
> (warn_slowpath_common+0x4c/0x64)
> [    1.589477] [<c0078b5c>] (warn_slowpath_common+0x4c/0x64) from
> [<c0078bf4>] (warn_slowpath_fmt+0x2c/0x3c)
> [    1.599060] [<c0078bf4>] (warn_slowpath_fmt+0x2c/0x3c) from
> [<c00adb90>] (__free_irq+0x88/0x164)
> [    1.607849] [<c00adb90>] (__free_irq+0x88/0x164) from [<c00adca8>]
> (free_irq+0x3c/0x5c)
> [    1.615875] [<c00adca8>] (free_irq+0x3c/0x5c) from [<c043ab2c>]
> (twl4030_kp_probe+0x308/0x374)
> [    1.624511] [<c043ab2c>] (twl4030_kp_probe+0x308/0x374) from
> [<c023c338>] (platform_drv_probe+0x14/0x18)
> [    1.634033] [<c023c338>] (platform_drv_probe+0x14/0x18) from
> [<c023b4dc>] (driver_probe_device+0xc8/0x184)
> [    1.643707] [<c023b4dc>] (driver_probe_device+0xc8/0x184) from
> [<c023b600>] (__driver_attach+0x68/0x8c)
> [    1.653106] [<c023b600>] (__driver_attach+0x68/0x8c) from [<c023ad34>]
> (bus_for_each_dev+0x48/0x74)
> [    1.662170] [<c023ad34>] (bus_for_each_dev+0x48/0x74) from [<c023a690>]
> (bus_add_driver+0x9c/0x210)
> [    1.671234] [<c023a690>] (bus_add_driver+0x9c/0x210) from [<c023b8f8>]
> (driver_register+0xa8/0x134)
> [    1.680297] [<c023b8f8>] (driver_register+0xa8/0x134) from [<c0041340>]
> (do_one_initcall+0x58/0x1b4)
> [    1.689453] [<c0041340>] (do_one_initcall+0x58/0x1b4) from [<c0008574>]
> (kernel_init+0x98/0x150)
> [    1.698272] [<c0008574>] (kernel_init+0x98/0x150) from [<c0042970>]
> (kernel_thread_exit+0x0/0x8)
> [    1.707214] ---[ end trace 6559b322ad3cbdfe ]---
> [    1.718292] twl4030_keypad: probe of twl4030_keypad failed with error -
> 16
> 
> Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com>
> Cc: linux-input@vger.kernel.org
> Cc: Dmitry Torokhov <dtor@mail.ru>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-input@vger.kernel.org
> Cc: Tony Lindgren <tony@atomide.com>
> ---
>  version v1 : initial patch
>  version v2 : CC'ed input subsystem and arm kernel mailing lists.
> 
>  drivers/input/keyboard/twl4030_keypad.c |   10 ++++------
>  1 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/keyboard/twl4030_keypad.c
> b/drivers/input/keyboard/twl4030_keypad.c
> index fb16b5e..39a9f30 100644
> --- a/drivers/input/keyboard/twl4030_keypad.c
> +++ b/drivers/input/keyboard/twl4030_keypad.c
> @@ -347,8 +347,7 @@ static int __devinit twl4030_kp_probe(struct
> platform_device *pdev)
>  	kp = kzalloc(sizeof(*kp), GFP_KERNEL);
>  	input = input_allocate_device();
>  	if (!kp || !input) {
> -		error = -ENOMEM;
> -		goto err1;
Wont you leak memory here ?


> +		return -ENOMEM;
>  	}
> 
>  	/* Get the debug Device */
> @@ -406,23 +405,22 @@ static int __devinit twl4030_kp_probe(struct
> platform_device *pdev)
>  	if (error) {
>  		dev_info(kp->dbg_dev, "request_irq failed for irq no=%d\n",
>  			kp->irq);
> -		goto err3;
> +		goto err2;
>  	}
> 
>  	/* Enable KP and TO interrupts now. */
>  	reg = (u8) ~(KEYP_IMR1_KP | KEYP_IMR1_TO);
>  	if (twl4030_kpwrite_u8(kp, reg, KEYP_IMR1)) {
>  		error = -EIO;
> -		goto err4;
> +		goto err3;
>  	}
> 
>  	platform_set_drvdata(pdev, kp);
>  	return 0;
> 
> -err4:
> +err3:
>  	/* mask all events - we don't care about the result */
>  	(void) twl4030_kpwrite_u8(kp, 0xff, KEYP_IMR1);
> -err3:
>  	free_irq(kp->irq, NULL);
>  err2:
>  	input_unregister_device(input);
> --
> 1.7.0.4
> 
> --
> 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

RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[G, Manjunath Kondaiah]

 

> -----Original Message-----
> From: Datta, Shubhrajyoti 
> Sent: Friday, September 24, 2010 5:00 PM
> To: G, Manjunath Kondaiah; linux-omap@vger.kernel.org
> Cc: linux-input@vger.kernel.org; Dmitry Torokhov; 
> linux-arm-kernel@lists.infradead.org; Tony Lindgren
> Subject: RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> 
> 
> 
> > -----Original Message-----
> > From: linux-input-owner@vger.kernel.org [mailto:linux-input- 
> > owner@vger.kernel.org] On Behalf Of G, Manjunath Kondaiah
> > Sent: Tuesday, September 21, 2010 5:20 PM
> > To: linux-omap@vger.kernel.org
> > Cc: linux-input@vger.kernel.org; Dmitry Torokhov; linux-arm- 
> > kernel@lists.infradead.org; Tony Lindgren
> > Subject: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> > 
> > The failure exit paths seems to be wrong in probe function.
> > This patch corrects exit failure paths for error handling cases.
> > 
> > Boot warning incase of request irq failure:
> > 
> > [    1.553985] twl4030_keypad twl4030_keypad: request_irq 
> failed for irq
> > no=369
> > [    1.561157] ------------[ cut here ]------------
> > [    1.565795] WARNING: at kernel/irq/manage.c:899 
> __free_irq+0x88/0x164()
> > [    1.572418] Trying to free already-free IRQ 369
> > [    1.576965] Modules linked in:
> > [    1.580047] [<c00470ec>] (unwind_backtrace+0x0/0xe4) 
> from [<c0078b5c>]
> > (warn_slowpath_common+0x4c/0x64)
> > [    1.589477] [<c0078b5c>] (warn_slowpath_common+0x4c/0x64) from
> > [<c0078bf4>] (warn_slowpath_fmt+0x2c/0x3c)
> > [    1.599060] [<c0078bf4>] (warn_slowpath_fmt+0x2c/0x3c) from
> > [<c00adb90>] (__free_irq+0x88/0x164)
> > [    1.607849] [<c00adb90>] (__free_irq+0x88/0x164) from 
> [<c00adca8>]
> > (free_irq+0x3c/0x5c)
> > [    1.615875] [<c00adca8>] (free_irq+0x3c/0x5c) from [<c043ab2c>]
> > (twl4030_kp_probe+0x308/0x374)
> > [    1.624511] [<c043ab2c>] (twl4030_kp_probe+0x308/0x374) from
> > [<c023c338>] (platform_drv_probe+0x14/0x18)
> > [    1.634033] [<c023c338>] (platform_drv_probe+0x14/0x18) from
> > [<c023b4dc>] (driver_probe_device+0xc8/0x184)
> > [    1.643707] [<c023b4dc>] (driver_probe_device+0xc8/0x184) from
> > [<c023b600>] (__driver_attach+0x68/0x8c)
> > [    1.653106] [<c023b600>] (__driver_attach+0x68/0x8c) 
> from [<c023ad34>]
> > (bus_for_each_dev+0x48/0x74)
> > [    1.662170] [<c023ad34>] (bus_for_each_dev+0x48/0x74) 
> from [<c023a690>]
> > (bus_add_driver+0x9c/0x210)
> > [    1.671234] [<c023a690>] (bus_add_driver+0x9c/0x210) 
> from [<c023b8f8>]
> > (driver_register+0xa8/0x134)
> > [    1.680297] [<c023b8f8>] (driver_register+0xa8/0x134) 
> from [<c0041340>]
> > (do_one_initcall+0x58/0x1b4)
> > [    1.689453] [<c0041340>] (do_one_initcall+0x58/0x1b4) 
> from [<c0008574>]
> > (kernel_init+0x98/0x150)
> > [    1.698272] [<c0008574>] (kernel_init+0x98/0x150) from 
> [<c0042970>]
> > (kernel_thread_exit+0x0/0x8)
> > [    1.707214] ---[ end trace 6559b322ad3cbdfe ]---
> > [    1.718292] twl4030_keypad: probe of twl4030_keypad 
> failed with error -
> > 16
> > 
> > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com>
> > Cc: linux-input@vger.kernel.org
> > Cc: Dmitry Torokhov <dtor@mail.ru>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-input@vger.kernel.org
> > Cc: Tony Lindgren <tony@atomide.com>
> > ---
> >  version v1 : initial patch
> >  version v2 : CC'ed input subsystem and arm kernel mailing lists.
> > 
> >  drivers/input/keyboard/twl4030_keypad.c |   10 ++++------
> >  1 files changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/input/keyboard/twl4030_keypad.c
> > b/drivers/input/keyboard/twl4030_keypad.c
> > index fb16b5e..39a9f30 100644
> > --- a/drivers/input/keyboard/twl4030_keypad.c
> > +++ b/drivers/input/keyboard/twl4030_keypad.c
> > @@ -347,8 +347,7 @@ static int __devinit twl4030_kp_probe(struct 
> > platform_device *pdev)
> >  	kp = kzalloc(sizeof(*kp), GFP_KERNEL);
> >  	input = input_allocate_device();
> >  	if (!kp || !input) {
> > -		error = -ENOMEM;
> > -		goto err1;
> Wont you leak memory here ?

Already catpured this comment and dmitry has posted alternate patch at:
http://www.spinics.net/lists/arm-kernel/msg99053.html

-Manjunath

RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[Datta, Shubhrajyoti]

> -----Original Message-----
> From: G, Manjunath Kondaiah
> Sent: Friday, September 24, 2010 5:11 PM
> To: Datta, Shubhrajyoti; linux-omap@vger.kernel.org
> Cc: linux-input@vger.kernel.org; Dmitry Torokhov; linux-arm-
> kernel@lists.infradead.org; Tony Lindgren
> Subject: RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> 
> 
> 
> > -----Original Message-----
> > From: Datta, Shubhrajyoti
> > Sent: Friday, September 24, 2010 5:00 PM
> > To: G, Manjunath Kondaiah; linux-omap@vger.kernel.org
> > Cc: linux-input@vger.kernel.org; Dmitry Torokhov;
> > linux-arm-kernel@lists.infradead.org; Tony Lindgren
> > Subject: RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> >
> >
> >
> > > -----Original Message-----
> > > From: linux-input-owner@vger.kernel.org [mailto:linux-input-
> > > owner@vger.kernel.org] On Behalf Of G, Manjunath Kondaiah
> > > Sent: Tuesday, September 21, 2010 5:20 PM
> > > To: linux-omap@vger.kernel.org
> > > Cc: linux-input@vger.kernel.org; Dmitry Torokhov; linux-arm-
> > > kernel@lists.infradead.org; Tony Lindgren
> > > Subject: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> > >
> > > The failure exit paths seems to be wrong in probe function.
> > > This patch corrects exit failure paths for error handling cases.
> > >
> > > Boot warning incase of request irq failure:
> > >
> > > [    1.553985] twl4030_keypad twl4030_keypad: request_irq
> > failed for irq
> > > no=369
> > > [    1.561157] ------------[ cut here ]------------
> > > [    1.565795] WARNING: at kernel/irq/manage.c:899
> > __free_irq+0x88/0x164()
> > > [    1.572418] Trying to free already-free IRQ 369
> > > [    1.576965] Modules linked in:
> > > [    1.580047] [<c00470ec>] (unwind_backtrace+0x0/0xe4)
> > from [<c0078b5c>]
> > > (warn_slowpath_common+0x4c/0x64)
> > > [    1.589477] [<c0078b5c>] (warn_slowpath_common+0x4c/0x64) from
> > > [<c0078bf4>] (warn_slowpath_fmt+0x2c/0x3c)
> > > [    1.599060] [<c0078bf4>] (warn_slowpath_fmt+0x2c/0x3c) from
> > > [<c00adb90>] (__free_irq+0x88/0x164)
> > > [    1.607849] [<c00adb90>] (__free_irq+0x88/0x164) from
> > [<c00adca8>]
> > > (free_irq+0x3c/0x5c)
> > > [    1.615875] [<c00adca8>] (free_irq+0x3c/0x5c) from [<c043ab2c>]
> > > (twl4030_kp_probe+0x308/0x374)
> > > [    1.624511] [<c043ab2c>] (twl4030_kp_probe+0x308/0x374) from
> > > [<c023c338>] (platform_drv_probe+0x14/0x18)
> > > [    1.634033] [<c023c338>] (platform_drv_probe+0x14/0x18) from
> > > [<c023b4dc>] (driver_probe_device+0xc8/0x184)
> > > [    1.643707] [<c023b4dc>] (driver_probe_device+0xc8/0x184) from
> > > [<c023b600>] (__driver_attach+0x68/0x8c)
> > > [    1.653106] [<c023b600>] (__driver_attach+0x68/0x8c)
> > from [<c023ad34>]
> > > (bus_for_each_dev+0x48/0x74)
> > > [    1.662170] [<c023ad34>] (bus_for_each_dev+0x48/0x74)
> > from [<c023a690>]
> > > (bus_add_driver+0x9c/0x210)
> > > [    1.671234] [<c023a690>] (bus_add_driver+0x9c/0x210)
> > from [<c023b8f8>]
> > > (driver_register+0xa8/0x134)
> > > [    1.680297] [<c023b8f8>] (driver_register+0xa8/0x134)
> > from [<c0041340>]
> > > (do_one_initcall+0x58/0x1b4)
> > > [    1.689453] [<c0041340>] (do_one_initcall+0x58/0x1b4)
> > from [<c0008574>]
> > > (kernel_init+0x98/0x150)
> > > [    1.698272] [<c0008574>] (kernel_init+0x98/0x150) from
> > [<c0042970>]
> > > (kernel_thread_exit+0x0/0x8)
> > > [    1.707214] ---[ end trace 6559b322ad3cbdfe ]---
> > > [    1.718292] twl4030_keypad: probe of twl4030_keypad
> > failed with error -
> > > 16
> > >
> > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com>
> > > Cc: linux-input@vger.kernel.org
> > > Cc: Dmitry Torokhov <dtor@mail.ru>
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > Cc: linux-input@vger.kernel.org
> > > Cc: Tony Lindgren <tony@atomide.com>
> > > ---
> > >  version v1 : initial patch
> > >  version v2 : CC'ed input subsystem and arm kernel mailing lists.
> > >
> > >  drivers/input/keyboard/twl4030_keypad.c |   10 ++++------
> > >  1 files changed, 4 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/input/keyboard/twl4030_keypad.c
> > > b/drivers/input/keyboard/twl4030_keypad.c
> > > index fb16b5e..39a9f30 100644
> > > --- a/drivers/input/keyboard/twl4030_keypad.c
> > > +++ b/drivers/input/keyboard/twl4030_keypad.c
> > > @@ -347,8 +347,7 @@ static int __devinit twl4030_kp_probe(struct
> > > platform_device *pdev)
> > >  	kp = kzalloc(sizeof(*kp), GFP_KERNEL);
> > >  	input = input_allocate_device();
> > >  	if (!kp || !input) {
> > > -		error = -ENOMEM;
> > > -		goto err1;
> > Wont you leak memory here ?
> 
> Already catpured this comment and dmitry has posted alternate patch at:
> http://www.spinics.net/lists/arm-kernel/msg99053.html
Apologies missed it.
 
> 
> -Manjunath

RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[G, Manjunath Kondaiah]

Dmitry,
As agreed, can you push this patch?
https://patchwork.kernel.org/patch/197442/

-Manjunath

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] 
> Sent: Tuesday, September 21, 2010 9:57 PM
> To: G, Manjunath Kondaiah
> Cc: Ameya Palande; linux-omap@vger.kernel.org; 
> linux-input@vger.kernel.org; 
> linux-arm-kernel@lists.infradead.org; Tony Lindgren
> Subject: Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> 
> Hi,
> 
> On Tue, Sep 21, 2010 at 07:11:01PM +0530, G, Manjunath Kondaiah wrote:
> > 
> > Hi,
> > 
> > > -----Original Message-----
> > > From: Ameya Palande [mailto:ameya.palande@nokia.com]
> > > Sent: Tuesday, September 21, 2010 7:04 PM
> > > To: G, Manjunath Kondaiah
> > > Cc: linux-omap@vger.kernel.org; 
> linux-input@vger.kernel.org; Dmitry 
> > > Torokhov; linux-arm-kernel@lists.infradead.org; Tony Lindgren
> > > Subject: Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in 
> > > probe
> > > 
> > > Hi Manjunath,
> > > 
> > > On Tue, 2010-09-21 at 13:49 +0200, ext G, Manjunath 
> Kondaiah wrote:
> > > > The failure exit paths seems to be wrong in probe function.
> > > > This patch corrects exit failure paths for error handling cases.
> > > 
> 
> And also adds memory leak...
> 
> 
> > > https://patchwork.kernel.org/patch/160551/
> > > Any comments on this?
> > 
> > Looks fine. Sorry, I didn't look at the change. This 
> version seems to 
> > be better.
> > 
> 
> I do not understand why we need to separate memory 
> allocations. It looks
> like the minimal patch should be like one below.
> 
> Thanks.
> 
> -- 
> Dmitry
> 
> 
> Input: twl4030_keypad - fix error handling path
> 
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> We should not try to call free_irq() when request_irq() failed.
> 
> Reported-by: G, Manjunath Kondaiah <manjugk@ti.com>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 
>  drivers/input/keyboard/twl4030_keypad.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/drivers/input/keyboard/twl4030_keypad.c 
> b/drivers/input/keyboard/twl4030_keypad.c
> index fb16b5e..09bef79 100644
> --- a/drivers/input/keyboard/twl4030_keypad.c
> +++ b/drivers/input/keyboard/twl4030_keypad.c
> @@ -406,23 +406,22 @@ static int __devinit 
> twl4030_kp_probe(struct platform_device *pdev)
>  	if (error) {
>  		dev_info(kp->dbg_dev, "request_irq failed for 
> irq no=%d\n",
>  			kp->irq);
> -		goto err3;
> +		goto err2;
>  	}
>  
>  	/* Enable KP and TO interrupts now. */
>  	reg = (u8) ~(KEYP_IMR1_KP | KEYP_IMR1_TO);
>  	if (twl4030_kpwrite_u8(kp, reg, KEYP_IMR1)) {
>  		error = -EIO;
> -		goto err4;
> +		goto err3;
>  	}
>  
>  	platform_set_drvdata(pdev, kp);
>  	return 0;
>  
> -err4:
> +err3:
>  	/* mask all events - we don't care about the result */
>  	(void) twl4030_kpwrite_u8(kp, 0xff, KEYP_IMR1);
> -err3:
>  	free_irq(kp->irq, NULL);
>  err2:
>  	input_unregister_device(input);
> 

Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[Dmitry Torokhov]

On Mon, Oct 04, 2010 at 01:32:29PM +0530, G, Manjunath Kondaiah wrote:
> Dmitry,
> As agreed, can you push this patch?
> https://patchwork.kernel.org/patch/197442/
> 

Manjunath,

It is queued for 2.6.37 (take a peek at 'next' branch of my tree) - I do
not believe that the failure is critical enough for .36.

Thanks.

-- 
Dmitry

RE: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe

[G, Manjunath Kondaiah]

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] 
> Sent: Monday, October 04, 2010 11:59 PM
> To: G, Manjunath Kondaiah
> Cc: Ameya Palande; linux-omap@vger.kernel.org; 
> linux-input@vger.kernel.org; 
> linux-arm-kernel@lists.infradead.org; Tony Lindgren
> Subject: Re: [PATCH v2] OMAP3: Keypad: Fix failure exit path in probe
> 
> On Mon, Oct 04, 2010 at 01:32:29PM +0530, G, Manjunath Kondaiah wrote:
> > Dmitry,
> > As agreed, can you push this patch?
> > https://patchwork.kernel.org/patch/197442/
> > 
> 
> Manjunath,
> 
> It is queued for 2.6.37 (take a peek at 'next' branch of my 
> tree) - I do not believe that the failure is critical enough for .36.

Thanks for queuing this patch for 2.6.37

-Manjunath