| Message ID | 4607844.sdhchmu5cM@wasted.cogentembedded.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Thu, Nov 19, 2015 at 01:39:51AM +0300, Sergei Shtylyov wrote: > When the R8A7795 support was added to the driver, little attention was paid > to the ravb_open() error path: free_irq() for the EMAC interrupt was called > uncoditionally, unlike request_irq(), and in a wrong order as well... > As a result, on the R-Car gen2 SoCs I started getting the following in case > of a device opening error: > > WARNING: CPU: 0 PID: 1 at kernel/irq/manage.c:1448 __free_irq+0x8c/0x228() > Trying to free already-free IRQ 0 > Modules linked in: > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.4.0-rc1-dirty #1005 > Hardware name: Generic R8A7791 (Flattened Device Tree) > Backtrace: > [<c0013818>] (dump_backtrace) from [<c00139b4>] (show_stack+0x18/0x1c) > r6:c063cdd6 r5:00000009 r4:00000000 r3:00204140 > [<c001399c>] (show_stack) from [<c022a578>] (dump_stack+0x74/0x90) > [<c022a504>] (dump_stack) from [<c0025f04>] (warn_slowpath_common+0x8c/0xb8) > r4:ef04fd38 r3:c0714770 > [<c0025e78>] (warn_slowpath_common) from [<c0025fd4>] (warn_slowpath_fmt+0x38/0x40) > r8:ee8ad800 r7:ef0030a0 r6:00000000 r5:00000000 r4:ef003040 > [<c0025fa0>] (warn_slowpath_fmt) from [<c0064cc0>] (__free_irq+0x8c/0x228) > r3:00000000 r2:c063ce9f > [<c0064c34>] (__free_irq) from [<c0064ecc>] (free_irq+0x70/0xa4) > r10:0000016b r8:00000000 r7:00000000 r6:ee8ad800 r5:00000000 r4:ef003040 > [<c0064e5c>] (free_irq) from [<c033472c>] (ravb_open+0x224/0x274) > r7:fffffffe r6:00000000 r5:fffffffe r4:ee8ad800 > [<c0334508>] (ravb_open) from [<c041ac78>] (__dev_open+0x84/0x104) > r7:ee8ad830 r6:c0566334 r5:00000000 r4:ee8ad800 > [<c041abf4>] (__dev_open) from [<c041af08>] (__dev_change_flags+0x94/0x13c) > r7:00001002 r6:00000001 r5:00001003 r4:ee8ad800 > [<c041ae74>] (__dev_change_flags) from [<c041afe8>] (dev_change_flags+0x20/0x50) > r7:c072e6e0 r6:00000138 r5:00001002 r4:ee8ad800 > [<c041afc8>] (dev_change_flags) from [<c06ec06c>] (ip_auto_config+0x174/0xfb8) > r8:00001002 r7:c072e6e0 r6:c0703344 r5:00000001 r4:ee8ad800 r3:00000101 > [<c06ebef8>] (ip_auto_config) from [<c000a810>] (do_one_initcall+0x100/0x1cc) > r10:c06fb83c r9:00000000 r8:c06ebef8 r7:c0736000 r6:c0710918 r5:c0710918 > r4:ef2f8f80 > [<c000a710>] (do_one_initcall) from [<c06ccddc>] (kernel_init_freeable+0x11c/0x1 > ec) > r10:c06fb83c r9:00000000 r8:0000009a r7:c0736000 r6:c0706bf0 r5:c06fb834 > r4:00000007 > [<c06cccc0>] (kernel_init_freeable) from [<c0514c54>] (kernel_init+0x14/0xec) > r10:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c0514c40 r4:c0736000 > [<c0514c40>] (kernel_init) from [<c0010458>] (ret_from_fork+0x14/0x3c) > r4:00000000 r3:ef04e000 > > Fix up the free_irq() call order and add a new label on the error path. > > Fixes: 22d4df8ff3a3 ("ravb: Add support for r8a7795 SoC") > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Thanks for fixing my mistake. Acked-by: Simon Horman <horms+renesas@verge.net.au> -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Date: Thu, 19 Nov 2015 01:39:51 +0300 > When the R8A7795 support was added to the driver, little attention was paid > to the ravb_open() error path: free_irq() for the EMAC interrupt was called > uncoditionally, unlike request_irq(), and in a wrong order as well... > As a result, on the R-Car gen2 SoCs I started getting the following in case > of a device opening error: ... > Fix up the free_irq() call order and add a new label on the error path. > > Fixes: 22d4df8ff3a3 ("ravb: Add support for r8a7795 SoC") > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: net/drivers/net/ethernet/renesas/ravb_main.c =================================================================== --- net.orig/drivers/net/ethernet/renesas/ravb_main.c +++ net/drivers/net/ethernet/renesas/ravb_main.c @@ -1225,7 +1225,7 @@ static int ravb_open(struct net_device * /* Device init */ error = ravb_dmac_init(ndev); if (error) - goto out_free_irq; + goto out_free_irq2; ravb_emac_init(ndev); /* Initialise PTP Clock driver */ @@ -1243,9 +1243,11 @@ static int ravb_open(struct net_device * out_ptp_stop: /* Stop PTP Clock driver */ ravb_ptp_stop(ndev); +out_free_irq2: + if (priv->chip_id == RCAR_GEN3) + free_irq(priv->emac_irq, ndev); out_free_irq: free_irq(ndev->irq, ndev); - free_irq(priv->emac_irq, ndev); out_napi_off: napi_disable(&priv->napi[RAVB_NC]); napi_disable(&priv->napi[RAVB_BE]);
When the R8A7795 support was added to the driver, little attention was paid to the ravb_open() error path: free_irq() for the EMAC interrupt was called uncoditionally, unlike request_irq(), and in a wrong order as well... As a result, on the R-Car gen2 SoCs I started getting the following in case of a device opening error: WARNING: CPU: 0 PID: 1 at kernel/irq/manage.c:1448 __free_irq+0x8c/0x228() Trying to free already-free IRQ 0 Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.4.0-rc1-dirty #1005 Hardware name: Generic R8A7791 (Flattened Device Tree) Backtrace: [<c0013818>] (dump_backtrace) from [<c00139b4>] (show_stack+0x18/0x1c) r6:c063cdd6 r5:00000009 r4:00000000 r3:00204140 [<c001399c>] (show_stack) from [<c022a578>] (dump_stack+0x74/0x90) [<c022a504>] (dump_stack) from [<c0025f04>] (warn_slowpath_common+0x8c/0xb8) r4:ef04fd38 r3:c0714770 [<c0025e78>] (warn_slowpath_common) from [<c0025fd4>] (warn_slowpath_fmt+0x38/0x40) r8:ee8ad800 r7:ef0030a0 r6:00000000 r5:00000000 r4:ef003040 [<c0025fa0>] (warn_slowpath_fmt) from [<c0064cc0>] (__free_irq+0x8c/0x228) r3:00000000 r2:c063ce9f [<c0064c34>] (__free_irq) from [<c0064ecc>] (free_irq+0x70/0xa4) r10:0000016b r8:00000000 r7:00000000 r6:ee8ad800 r5:00000000 r4:ef003040 [<c0064e5c>] (free_irq) from [<c033472c>] (ravb_open+0x224/0x274) r7:fffffffe r6:00000000 r5:fffffffe r4:ee8ad800 [<c0334508>] (ravb_open) from [<c041ac78>] (__dev_open+0x84/0x104) r7:ee8ad830 r6:c0566334 r5:00000000 r4:ee8ad800 [<c041abf4>] (__dev_open) from [<c041af08>] (__dev_change_flags+0x94/0x13c) r7:00001002 r6:00000001 r5:00001003 r4:ee8ad800 [<c041ae74>] (__dev_change_flags) from [<c041afe8>] (dev_change_flags+0x20/0x50) r7:c072e6e0 r6:00000138 r5:00001002 r4:ee8ad800 [<c041afc8>] (dev_change_flags) from [<c06ec06c>] (ip_auto_config+0x174/0xfb8) r8:00001002 r7:c072e6e0 r6:c0703344 r5:00000001 r4:ee8ad800 r3:00000101 [<c06ebef8>] (ip_auto_config) from [<c000a810>] (do_one_initcall+0x100/0x1cc) r10:c06fb83c r9:00000000 r8:c06ebef8 r7:c0736000 r6:c0710918 r5:c0710918 r4:ef2f8f80 [<c000a710>] (do_one_initcall) from [<c06ccddc>] (kernel_init_freeable+0x11c/0x1 ec) r10:c06fb83c r9:00000000 r8:0000009a r7:c0736000 r6:c0706bf0 r5:c06fb834 r4:00000007 [<c06cccc0>] (kernel_init_freeable) from [<c0514c54>] (kernel_init+0x14/0xec) r10:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c0514c40 r4:c0736000 [<c0514c40>] (kernel_init) from [<c0010458>] (ret_from_fork+0x14/0x3c) r4:00000000 r3:ef04e000 Fix up the free_irq() call order and add a new label on the error path. Fixes: 22d4df8ff3a3 ("ravb: Add support for r8a7795 SoC") Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- The patch is against DaveM's 'net.git' repo. drivers/net/ethernet/renesas/ravb_main.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html