usb: musb: Fix panic upon musb_am335x module removal
Subsystems (2)
PATCH 1/1 usb: musb: Fix panic upon musb_am335x module removal
At probe time, the musb_am335x driver register its childs by calling of_platform_populate(), which registers all childs in the devicetree hierarchy recursively.
On the other side, the driver's remove() function uses of_device_unregister() to remove each child of musb_am335x's.
However, when musb_dsps is loaded, its devices are attached to the musb_am335x device as musb_am335x childs. Hence, musb_am335x remove() will attempt to unregister the devices registered by musb_dsps, which produces a kernel panic.
In other words, the childs in the "struct device" hierarchy are not the same as the childs in the "devicetree" hierarchy.
Ideally, we should enforce the removal of the devices registered by musb_am335x *only*, instead of all its child devices. However, because of the recursive nature of of_platform_populate, this doesn't seem possible.
Therefore, as the only solution at hand, this commit disables musb_am335x driver removal capability, preventing it from being ever removed. This was originally suggested by Sebastian Siewior:
https://www.mail-archive.com/linux-omap at vger.kernel.org/msg104946.html
And for reference, here's the panic upon module removal:
musb-hdrc musb-hdrc.0.auto: remove, state 4 usb usb1: USB disconnect, device number 1 musb-hdrc musb-hdrc.0.auto: USB bus 1 deregistered
Unable to handle kernel NULL pointer dereference at virtual address 0000008c pgd = de11c000 [0000008c] *pgd=9e174831, *pte=00000000, *ppte=00000000 Internal error: Oops: 17 [#1] ARM Modules linked in: musb_am335x(-) musb_dsps musb_hdrc usbcore usb_common CPU: 0 PID: 623 Comm: modprobe Not tainted 3.15.0-rc4-00001-g24efd13 #69 task: de1b7500 ti: de122000 task.ti: de122000 PC is at am335x_shutdown+0x10/0x28 LR is at am335x_shutdown+0xc/0x28 pc : [<c0327798>] lr : [<c0327794>] psr: a0000013
sp : de123df8 ip : 00000004 fp : 00028f00
r10: 00000000 r9 : de122000 r8 : c000e6c4
r7 : de0e3c10 r6 : de0e3800 r5 : de624010 r4 : de1ec750 r3 : de0e3810 r2 : 00000000 r1 : 00000001 r0 : 00000000
Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 10c5387d Table: 9e11c019 DAC: 00000015 Process modprobe (pid: 623, stack limit = 0xde122240)
Stack: (0xde123df8 to 0xde124000)
3de0: de0e3810 bf054488
3e00: bf05444c de624010 60000013 bf043650 000012fc de624010 de0e3810 bf043a20
3e20: de0e3810 bf04b240 c0635b88 c02ca37c c02ca364 c02c8db0 de1b7500 de0e3844
3e40: de0e3810 c02c8e28 c0635b88 de02824c de0e3810 c02c884c de0e3800 de0e3810
3e60: de0e3818 c02c5b20 bf05417c de0e3800 de0e3800 c0635b88 de0f2410 c02ca838
3e80: bf05417c de0e3800 bf055438 c02ca8cc de0e3c10 bf054194 de0e3c10 c02ca37c
3ea0: c02ca364 c02c8db0 de1b7500 de0e3c44 de0e3c10 c02c8e28 c0635b88 de02824c
3ec0: de0e3c10 c02c884c de0e3c10 de0e3c10 de0e3c18 c02c5b20 de0e3c10 de0e3c10
3ee0: 00000000 bf059000 a0000013 c02c5bc0 00000000 bf05900c de0e3c10 c02c5c48
3f00: de0dd0c0 de1ec970 de0f2410 bf05929c de0f2444 bf05902c de0f2410 c02ca37c
3f20: c02ca364 c02c8db0 bf05929c de0f2410 bf05929c c02c94c8 bf05929c 00000000
3f40: 00000800 c02c8ab4 bf0592e0 c007fc40 c00dd820 6273756d 336d615f 00783533
3f60: c064a0ac de1b7500 de122000 de1b7500 c000e590 00000001 c000e6c4 c0060160
3f80: 00028e70 00028e70 00028ea4 00000081 60000010 00028e70 00028e70 00028ea4
3fa0: 00000081 c000e500 00028e70 00028e70 00028ea4 00000800 becb59f8 00027608
3fc0: 00028e70 00028e70 00028ea4 00000081 00000001 00000001 00000000 00028f00
3fe0: b6e6b6f0 becb59d4 000160e8 b6e6b6fc 60000010 00028ea4 00000000 00000000 [<c0327798>] (am335x_shutdown) from [<bf054488>] (dsps_musb_exit+0x3c/0x4c [musb_dsps]) [<bf054488>] (dsps_musb_exit [musb_dsps]) from [<bf043650>] (musb_shutdown+0x80/0x90 [musb_hdrc]) [<bf043650>] (musb_shutdown [musb_hdrc]) from [<bf043a20>] (musb_remove+0x24/0x68 [musb_hdrc]) [<bf043a20>] (musb_remove [musb_hdrc]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c8e28>] (device_release_driver+0x20/0x2c) [<c02c8e28>] (device_release_driver) from [<c02c884c>] (bus_remove_device+0xdc/0x10c) [<c02c884c>] (bus_remove_device) from [<c02c5b20>] (device_del+0x104/0x198)
[<c02c5b20>] (device_del) from [<c02ca838>] (platform_device_del+0x14/0x9c)
[<c02ca838>] (platform_device_del) from [<c02ca8cc>] (platform_device_unregister+0xc/0x20) [<c02ca8cc>] (platform_device_unregister) from [<bf054194>] (dsps_remove+0x18/0x38 [musb_dsps]) [<bf054194>] (dsps_remove [musb_dsps]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c8e28>] (device_release_driver+0x20/0x2c) [<c02c8e28>] (device_release_driver) from [<c02c884c>] (bus_remove_device+0xdc/0x10c) [<c02c884c>] (bus_remove_device) from [<c02c5b20>] (device_del+0x104/0x198)
[<c02c5b20>] (device_del) from [<c02c5bc0>] (device_unregister+0xc/0x20)
[<c02c5bc0>] (device_unregister) from [<bf05900c>] (of_remove_populated_child+0xc/0x14 [musb_am335x]) [<bf05900c>] (of_remove_populated_child [musb_am335x]) from [<c02c5c48>] (device_for_each_child+0x44/0x70) [<c02c5c48>] (device_for_each_child) from [<bf05902c>] (am335x_child_remove+0x18/0x30 [musb_am335x]) [<bf05902c>] (am335x_child_remove [musb_am335x]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c94c8>] (driver_detach+0xb4/0xb8) [<c02c94c8>] (driver_detach) from [<c02c8ab4>] (bus_remove_driver+0x4c/0xa0)
[<c02c8ab4>] (bus_remove_driver) from [<c007fc40>] (SyS_delete_module+0x128/0x1cc) [<c007fc40>] (SyS_delete_module) from [<c000e500>] (ret_fast_syscall+0x0/0x48)
--- drivers/usb/musb/musb_am335x.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-)
drivers/usb/musb/musb_am335x.c +6 -17
@@ -19,21 +19,6 @@ err:return ret; } -static int of_remove_populated_child (struct device *dev,void *d) -{ -struct platform_device *pdev =to_platform_device (dev); - -of_device_unregister (pdev); -return 0 ; -} - -static int am335x_child_remove (struct platform_device *pdev) -{ -device_for_each_child (&pdev->dev,NULL , of_remove_populated_child); -pm_runtime_disable (&pdev->dev); -return 0 ; -} -static const struct of_device_id am335x_child_of_match[] = { { .compatible ="ti,am33xx-usb" }, { },
@@ -42,13 +27,17 @@ MODULE_DEVICE_TABLE(of, am335x_child_of_match);static struct platform_driver am335x_child_driver = { .probe = am335x_child_probe, - .remove = am335x_child_remove, .driver = { .name ="am335x-usb-childs" , .of_match_table = am335x_child_of_match, }, }; -module_platform_driver (am335x_child_driver); +static int __init am335x_child_init (void ) +{ +return platform_driver_register (&am335x_child_driver); +} +module_init (am335x_child_init); +MODULE_DESCRIPTION ("AM33xx child devices" );MODULE_LICENSE ("GPL v2" );
Signature · 1 line
1.9.1
PATCH 1/1 usb: musb: Fix panic upon musb_am335x module removal
Quoted 148 lines · Ezequiel Garcia
At probe time, the
musb_am335xdriver register its childs by callingof_platform_populate(), which registers all childs in the devicetree hierarchy recursively.On the other side, the driver's
remove()function usesof_device_unregister()to remove each child of musb_am335x's.However, when
musb_dspsis loaded, its devices are attached to themusb_am335xdevice asmusb_am335xchilds. Hence,musb_am335xremove()will attempt to unregister the devices registered bymusb_dsps, which produces a kernel panic.In other words, the childs in the "struct device" hierarchy are not the same as the childs in the "devicetree" hierarchy.
Ideally, we should enforce the removal of the devices registered by
musb_am335x*only*, instead of all its child devices. However, because of the recursive nature ofof_platform_populate, this doesn't seem possible.Therefore, as the only solution at hand, this commit disables
musb_am335xdriver removal capability, preventing it from being ever removed. This was originally suggested by Sebastian Siewior:https://www.mail-archive.com/linux-omap at vger.kernel.org/msg104946.html
And for reference, here's the panic upon module removal:
musb-hdrc musb-hdrc.0.auto: remove, state 4 usb usb1: USB disconnect, device number 1 musb-hdrc musb-hdrc.0.auto: USB bus 1 deregistered
Unable to handle kernel NULL pointer dereference at virtual address 0000008c pgd = de11c000 [0000008c] *pgd=9e174831, *pte=00000000, *ppte=00000000 Internal error: Oops: 17 [#1] ARM Modules linked in: musb_am335x(-) musb_dsps musb_hdrc usbcore usb_common CPU: 0 PID: 623 Comm: modprobe Not tainted 3.15.0-rc4-00001-g24efd13 #69 task: de1b7500 ti: de122000 task.ti: de122000 PC is at am335x_shutdown+0x10/0x28 LR is at am335x_shutdown+0xc/0x28 pc : [<c0327798>] lr : [<c0327794>] psr: a0000013sp :
de123df8ip : 00000004 fp :00028f00
r10: 00000000 r9 :de122000r8 :c000e6c4
r7 :de0e3c10r6 :de0e3800r5 :de624010r4 :de1ec750r3 :de0e3810r2 : 00000000 r1 : 00000001 r0 : 00000000
Flags: NzCv IRQs on FIQs on ModeSVC_32ISA ARM Segment user
Control:10c5387dTable:9e11c019DAC: 00000015 Process modprobe (pid: 623, stack limit =0xde122240)Stack: (0xde123df8 to 0xde124000)3de0:
de0e3810bf054488
3e00:bf05444cde62401060000013bf043650000012fcde624010de0e3810bf043a20
3e20:de0e3810bf04b240c0635b88c02ca37cc02ca364c02c8db0de1b7500de0e3844
3e40:de0e3810c02c8e28c0635b88de02824cde0e3810c02c884cde0e3800de0e3810
3e60:de0e3818c02c5b20bf05417cde0e3800de0e3800c0635b88de0f2410c02ca838
3e80:bf05417cde0e3800bf055438c02ca8ccde0e3c10bf054194de0e3c10c02ca37c
3ea0:c02ca364c02c8db0de1b7500de0e3c44de0e3c10c02c8e28c0635b88de02824c
3ec0:de0e3c10c02c884cde0e3c10de0e3c10de0e3c18c02c5b20de0e3c10de0e3c10
3ee0: 00000000bf059000a0000013c02c5bc000000000bf05900cde0e3c10c02c5c48
3f00:de0dd0c0de1ec970de0f2410bf05929cde0f2444bf05902cde0f2410c02ca37c
3f20:c02ca364c02c8db0bf05929cde0f2410bf05929cc02c94c8bf05929c00000000
3f40: 00000800c02c8ab4bf0592e0c007fc40c00dd8206273756d336d615f00783533
3f60:c064a0acde1b7500de122000de1b7500c000e59000000001c000e6c4c0060160
3f80:00028e7000028e7000028ea400000081 6000001000028e7000028e7000028ea4
3fa0: 00000081c000e50000028e7000028e7000028ea400000800becb59f800027608
3fc0:00028e7000028e7000028ea400000081 00000001 00000001 0000000000028f00
3fe0:b6e6b6f0becb59d4000160e8b6e6b6fc6000001000028ea400000000 00000000 [<c0327798>] (am335x_shutdown) from [<bf054488>] (dsps_musb_exit+0x3c/0x4c[musb_dsps]) [<bf054488>] (dsps_musb_exit[musb_dsps]) from [<bf043650>] (musb_shutdown+0x80/0x90[musb_hdrc]) [<bf043650>] (musb_shutdown[musb_hdrc]) from [<bf043a20>] (musb_remove+0x24/0x68[musb_hdrc]) [<bf043a20>] (musb_remove[musb_hdrc]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c8e28>] (device_release_driver+0x20/0x2c) [<c02c8e28>] (device_release_driver) from [<c02c884c>] (bus_remove_device+0xdc/0x10c) [<c02c884c>] (bus_remove_device) from [<c02c5b20>] (device_del+0x104/0x198)
[<c02c5b20>] (device_del) from [<c02ca838>] (platform_device_del+0x14/0x9c)
[<c02ca838>] (platform_device_del) from [<c02ca8cc>] (platform_device_unregister+0xc/0x20) [<c02ca8cc>] (platform_device_unregister) from [<bf054194>] (dsps_remove+0x18/0x38[musb_dsps]) [<bf054194>] (dsps_remove[musb_dsps]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c8e28>] (device_release_driver+0x20/0x2c) [<c02c8e28>] (device_release_driver) from [<c02c884c>] (bus_remove_device+0xdc/0x10c) [<c02c884c>] (bus_remove_device) from [<c02c5b20>] (device_del+0x104/0x198)
[<c02c5b20>] (device_del) from [<c02c5bc0>] (device_unregister+0xc/0x20)
[<c02c5bc0>] (device_unregister) from [<bf05900c>] (of_remove_populated_child+0xc/0x14[musb_am335x]) [<bf05900c>] (of_remove_populated_child[musb_am335x]) from [<c02c5c48>] (device_for_each_child+0x44/0x70) [<c02c5c48>] (device_for_each_child) from [<bf05902c>] (am335x_child_remove+0x18/0x30[musb_am335x]) [<bf05902c>] (am335x_child_remove[musb_am335x]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c94c8>] (driver_detach+0xb4/0xb8) [<c02c94c8>] (driver_detach) from [<c02c8ab4>] (bus_remove_driver+0x4c/0xa0)
[<c02c8ab4>] (bus_remove_driver) from [<c007fc40>] (SyS_delete_module+0x128/0x1cc) [<c007fc40>] (SyS_delete_module) from [<c000e500>] (ret_fast_syscall+0x0/0x48)Fixes 97238b35d5bb ("usb: musb: dsps: use proper child nodes")Cc <[email protected]> # v3.12+Acked-by George Cherian <[email protected]>Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/usb/musb/musb_am335x.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-)drivers/usb/musb/musb_am335x.c +6
-17@@ -19,21 +19,6 @@ err:return ret; } -static int of_remove_populated_child (struct device *dev,void *d) -{ -struct platform_device *pdev =to_platform_device (dev); - -of_device_unregister (pdev); -return 0 ; -} - -static int am335x_child_remove (struct platform_device *pdev) -{ -device_for_each_child (&pdev->dev,NULL , of_remove_populated_child); -pm_runtime_disable (&pdev->dev); -return 0 ; -} -static const struct of_device_id am335x_child_of_match[] = { { .compatible ="ti,am33xx-usb" }, { },@@ -42,13 +27,17 @@ MODULE_DEVICE_TABLE(of, am335x_child_of_match);static struct platform_driver am335x_child_driver = { .probe = am335x_child_probe, - .remove = am335x_child_remove, .driver = { .name ="am335x-usb-childs" , .of_match_table = am335x_child_of_match, }, }; -module_platform_driver (am335x_child_driver); +static int __init am335x_child_init (void ) +{ +return platform_driver_register (&am335x_child_driver); +} +module_init (am335x_child_init); +MODULE_DESCRIPTION ("AM33xx child devices" );MODULE_LICENSE ("GPL v2" );Signature · 1 line
1.9.1
Felipe,
Any comments on this?
Signature · 2 lines
Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar
PATCH 1/1 usb: musb: Fix panic upon musb_am335x module removal
Hi Felipe,
Quoted 152 lines · Ezequiel Garcia
Quoted 148 lines · Ezequiel Garcia
At probe time, the
musb_am335xdriver register its childs by callingof_platform_populate(), which registers all childs in the devicetree hierarchy recursively.On the other side, the driver's
remove()function usesof_device_unregister()to remove each child of musb_am335x's.However, when
musb_dspsis loaded, its devices are attached to themusb_am335xdevice asmusb_am335xchilds. Hence,musb_am335xremove()will attempt to unregister the devices registered bymusb_dsps, which produces a kernel panic.In other words, the childs in the "struct device" hierarchy are not the same as the childs in the "devicetree" hierarchy.
Ideally, we should enforce the removal of the devices registered by
musb_am335x*only*, instead of all its child devices. However, because of the recursive nature ofof_platform_populate, this doesn't seem possible.Therefore, as the only solution at hand, this commit disables
musb_am335xdriver removal capability, preventing it from being ever removed. This was originally suggested by Sebastian Siewior:https://www.mail-archive.com/linux-omap at vger.kernel.org/msg104946.html
And for reference, here's the panic upon module removal:
musb-hdrc musb-hdrc.0.auto: remove, state 4 usb usb1: USB disconnect, device number 1 musb-hdrc musb-hdrc.0.auto: USB bus 1 deregistered
Unable to handle kernel NULL pointer dereference at virtual address 0000008c pgd = de11c000 [0000008c] *pgd=9e174831, *pte=00000000, *ppte=00000000 Internal error: Oops: 17 [#1] ARM Modules linked in: musb_am335x(-) musb_dsps musb_hdrc usbcore usb_common CPU: 0 PID: 623 Comm: modprobe Not tainted 3.15.0-rc4-00001-g24efd13 #69 task: de1b7500 ti: de122000 task.ti: de122000 PC is at am335x_shutdown+0x10/0x28 LR is at am335x_shutdown+0xc/0x28 pc : [<c0327798>] lr : [<c0327794>] psr: a0000013sp :
de123df8ip : 00000004 fp :00028f00
r10: 00000000 r9 :de122000r8 :c000e6c4
r7 :de0e3c10r6 :de0e3800r5 :de624010r4 :de1ec750r3 :de0e3810r2 : 00000000 r1 : 00000001 r0 : 00000000
Flags: NzCv IRQs on FIQs on ModeSVC_32ISA ARM Segment user
Control:10c5387dTable:9e11c019DAC: 00000015 Process modprobe (pid: 623, stack limit =0xde122240)Stack: (0xde123df8 to 0xde124000)3de0:
de0e3810bf054488
3e00:bf05444cde62401060000013bf043650000012fcde624010de0e3810bf043a20
3e20:de0e3810bf04b240c0635b88c02ca37cc02ca364c02c8db0de1b7500de0e3844
3e40:de0e3810c02c8e28c0635b88de02824cde0e3810c02c884cde0e3800de0e3810
3e60:de0e3818c02c5b20bf05417cde0e3800de0e3800c0635b88de0f2410c02ca838
3e80:bf05417cde0e3800bf055438c02ca8ccde0e3c10bf054194de0e3c10c02ca37c
3ea0:c02ca364c02c8db0de1b7500de0e3c44de0e3c10c02c8e28c0635b88de02824c
3ec0:de0e3c10c02c884cde0e3c10de0e3c10de0e3c18c02c5b20de0e3c10de0e3c10
3ee0: 00000000bf059000a0000013c02c5bc000000000bf05900cde0e3c10c02c5c48
3f00:de0dd0c0de1ec970de0f2410bf05929cde0f2444bf05902cde0f2410c02ca37c
3f20:c02ca364c02c8db0bf05929cde0f2410bf05929cc02c94c8bf05929c00000000
3f40: 00000800c02c8ab4bf0592e0c007fc40c00dd8206273756d336d615f00783533
3f60:c064a0acde1b7500de122000de1b7500c000e59000000001c000e6c4c0060160
3f80:00028e7000028e7000028ea400000081 6000001000028e7000028e7000028ea4
3fa0: 00000081c000e50000028e7000028e7000028ea400000800becb59f800027608
3fc0:00028e7000028e7000028ea400000081 00000001 00000001 0000000000028f00
3fe0:b6e6b6f0becb59d4000160e8b6e6b6fc6000001000028ea400000000 00000000 [<c0327798>] (am335x_shutdown) from [<bf054488>] (dsps_musb_exit+0x3c/0x4c[musb_dsps]) [<bf054488>] (dsps_musb_exit[musb_dsps]) from [<bf043650>] (musb_shutdown+0x80/0x90[musb_hdrc]) [<bf043650>] (musb_shutdown[musb_hdrc]) from [<bf043a20>] (musb_remove+0x24/0x68[musb_hdrc]) [<bf043a20>] (musb_remove[musb_hdrc]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c8e28>] (device_release_driver+0x20/0x2c) [<c02c8e28>] (device_release_driver) from [<c02c884c>] (bus_remove_device+0xdc/0x10c) [<c02c884c>] (bus_remove_device) from [<c02c5b20>] (device_del+0x104/0x198)
[<c02c5b20>] (device_del) from [<c02ca838>] (platform_device_del+0x14/0x9c)
[<c02ca838>] (platform_device_del) from [<c02ca8cc>] (platform_device_unregister+0xc/0x20) [<c02ca8cc>] (platform_device_unregister) from [<bf054194>] (dsps_remove+0x18/0x38[musb_dsps]) [<bf054194>] (dsps_remove[musb_dsps]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c8e28>] (device_release_driver+0x20/0x2c) [<c02c8e28>] (device_release_driver) from [<c02c884c>] (bus_remove_device+0xdc/0x10c) [<c02c884c>] (bus_remove_device) from [<c02c5b20>] (device_del+0x104/0x198)
[<c02c5b20>] (device_del) from [<c02c5bc0>] (device_unregister+0xc/0x20)
[<c02c5bc0>] (device_unregister) from [<bf05900c>] (of_remove_populated_child+0xc/0x14[musb_am335x]) [<bf05900c>] (of_remove_populated_child[musb_am335x]) from [<c02c5c48>] (device_for_each_child+0x44/0x70) [<c02c5c48>] (device_for_each_child) from [<bf05902c>] (am335x_child_remove+0x18/0x30[musb_am335x]) [<bf05902c>] (am335x_child_remove[musb_am335x]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c94c8>] (driver_detach+0xb4/0xb8) [<c02c94c8>] (driver_detach) from [<c02c8ab4>] (bus_remove_driver+0x4c/0xa0)
[<c02c8ab4>] (bus_remove_driver) from [<c007fc40>] (SyS_delete_module+0x128/0x1cc) [<c007fc40>] (SyS_delete_module) from [<c000e500>] (ret_fast_syscall+0x0/0x48)Fixes 97238b35d5bb ("usb: musb: dsps: use proper child nodes")Cc <[email protected]> # v3.12+Acked-by George Cherian <[email protected]>Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/usb/musb/musb_am335x.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-)drivers/usb/musb/musb_am335x.c +6
-17@@ -19,21 +19,6 @@ err:return ret; } -static int of_remove_populated_child (struct device *dev,void *d) -{ -struct platform_device *pdev =to_platform_device (dev); - -of_device_unregister (pdev); -return 0 ; -} - -static int am335x_child_remove (struct platform_device *pdev) -{ -device_for_each_child (&pdev->dev,NULL , of_remove_populated_child); -pm_runtime_disable (&pdev->dev); -return 0 ; -} -static const struct of_device_id am335x_child_of_match[] = { { .compatible ="ti,am33xx-usb" }, { },@@ -42,13 +27,17 @@ MODULE_DEVICE_TABLE(of, am335x_child_of_match);static struct platform_driver am335x_child_driver = { .probe = am335x_child_probe, - .remove = am335x_child_remove, .driver = { .name ="am335x-usb-childs" , .of_match_table = am335x_child_of_match, }, }; -module_platform_driver (am335x_child_driver); +static int __init am335x_child_init (void ) +{ +return platform_driver_register (&am335x_child_driver); +} +module_init (am335x_child_init); +MODULE_DESCRIPTION ("AM33xx child devices" );MODULE_LICENSE ("GPL v2" );Signature · 1 line
1.9.1Felipe,
Quoted 2 lines
Any comments on this?
Any comments on this? I don't like to remove a feature, but as explained above, I don't see any other way to solve this.
The driver is currently broken. I tried using of_platform_depopulate() but failed.
Signature · 2 lines
Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar
PATCH 1/1 usb: musb: Fix panic upon musb_am335x module removal
Quoted 155 lines · Ezequiel Garcia
Hi Felipe,
Quoted 152 lines · Ezequiel Garcia
Quoted 148 lines · Ezequiel Garcia
At probe time, the
musb_am335xdriver register its childs by callingof_platform_populate(), which registers all childs in the devicetree hierarchy recursively.On the other side, the driver's
remove()function usesof_device_unregister()to remove each child of musb_am335x's.However, when
musb_dspsis loaded, its devices are attached to themusb_am335xdevice asmusb_am335xchilds. Hence,musb_am335xremove()will attempt to unregister the devices registered bymusb_dsps, which produces a kernel panic.In other words, the childs in the "struct device" hierarchy are not the same as the childs in the "devicetree" hierarchy.
Ideally, we should enforce the removal of the devices registered by
musb_am335x*only*, instead of all its child devices. However, because of the recursive nature ofof_platform_populate, this doesn't seem possible.Therefore, as the only solution at hand, this commit disables
musb_am335xdriver removal capability, preventing it from being ever removed. This was originally suggested by Sebastian Siewior:https://www.mail-archive.com/linux-omap at vger.kernel.org/msg104946.html
And for reference, here's the panic upon module removal:
musb-hdrc musb-hdrc.0.auto: remove, state 4 usb usb1: USB disconnect, device number 1 musb-hdrc musb-hdrc.0.auto: USB bus 1 deregistered
Unable to handle kernel NULL pointer dereference at virtual address 0000008c pgd = de11c000 [0000008c] *pgd=9e174831, *pte=00000000, *ppte=00000000 Internal error: Oops: 17 [#1] ARM Modules linked in: musb_am335x(-) musb_dsps musb_hdrc usbcore usb_common CPU: 0 PID: 623 Comm: modprobe Not tainted 3.15.0-rc4-00001-g24efd13 #69 task: de1b7500 ti: de122000 task.ti: de122000 PC is at am335x_shutdown+0x10/0x28 LR is at am335x_shutdown+0xc/0x28 pc : [<c0327798>] lr : [<c0327794>] psr: a0000013sp :
de123df8ip : 00000004 fp :00028f00
r10: 00000000 r9 :de122000r8 :c000e6c4
r7 :de0e3c10r6 :de0e3800r5 :de624010r4 :de1ec750r3 :de0e3810r2 : 00000000 r1 : 00000001 r0 : 00000000
Flags: NzCv IRQs on FIQs on ModeSVC_32ISA ARM Segment user
Control:10c5387dTable:9e11c019DAC: 00000015 Process modprobe (pid: 623, stack limit =0xde122240)Stack: (0xde123df8 to 0xde124000)3de0:
de0e3810bf054488
3e00:bf05444cde62401060000013bf043650000012fcde624010de0e3810bf043a20
3e20:de0e3810bf04b240c0635b88c02ca37cc02ca364c02c8db0de1b7500de0e3844
3e40:de0e3810c02c8e28c0635b88de02824cde0e3810c02c884cde0e3800de0e3810
3e60:de0e3818c02c5b20bf05417cde0e3800de0e3800c0635b88de0f2410c02ca838
3e80:bf05417cde0e3800bf055438c02ca8ccde0e3c10bf054194de0e3c10c02ca37c
3ea0:c02ca364c02c8db0de1b7500de0e3c44de0e3c10c02c8e28c0635b88de02824c
3ec0:de0e3c10c02c884cde0e3c10de0e3c10de0e3c18c02c5b20de0e3c10de0e3c10
3ee0: 00000000bf059000a0000013c02c5bc000000000bf05900cde0e3c10c02c5c48
3f00:de0dd0c0de1ec970de0f2410bf05929cde0f2444bf05902cde0f2410c02ca37c
3f20:c02ca364c02c8db0bf05929cde0f2410bf05929cc02c94c8bf05929c00000000
3f40: 00000800c02c8ab4bf0592e0c007fc40c00dd8206273756d336d615f00783533
3f60:c064a0acde1b7500de122000de1b7500c000e59000000001c000e6c4c0060160
3f80:00028e7000028e7000028ea400000081 6000001000028e7000028e7000028ea4
3fa0: 00000081c000e50000028e7000028e7000028ea400000800becb59f800027608
3fc0:00028e7000028e7000028ea400000081 00000001 00000001 0000000000028f00
3fe0:b6e6b6f0becb59d4000160e8b6e6b6fc6000001000028ea400000000 00000000 [<c0327798>] (am335x_shutdown) from [<bf054488>] (dsps_musb_exit+0x3c/0x4c[musb_dsps]) [<bf054488>] (dsps_musb_exit[musb_dsps]) from [<bf043650>] (musb_shutdown+0x80/0x90[musb_hdrc]) [<bf043650>] (musb_shutdown[musb_hdrc]) from [<bf043a20>] (musb_remove+0x24/0x68[musb_hdrc]) [<bf043a20>] (musb_remove[musb_hdrc]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c8e28>] (device_release_driver+0x20/0x2c) [<c02c8e28>] (device_release_driver) from [<c02c884c>] (bus_remove_device+0xdc/0x10c) [<c02c884c>] (bus_remove_device) from [<c02c5b20>] (device_del+0x104/0x198)
[<c02c5b20>] (device_del) from [<c02ca838>] (platform_device_del+0x14/0x9c)
[<c02ca838>] (platform_device_del) from [<c02ca8cc>] (platform_device_unregister+0xc/0x20) [<c02ca8cc>] (platform_device_unregister) from [<bf054194>] (dsps_remove+0x18/0x38[musb_dsps]) [<bf054194>] (dsps_remove[musb_dsps]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c8e28>] (device_release_driver+0x20/0x2c) [<c02c8e28>] (device_release_driver) from [<c02c884c>] (bus_remove_device+0xdc/0x10c) [<c02c884c>] (bus_remove_device) from [<c02c5b20>] (device_del+0x104/0x198)
[<c02c5b20>] (device_del) from [<c02c5bc0>] (device_unregister+0xc/0x20)
[<c02c5bc0>] (device_unregister) from [<bf05900c>] (of_remove_populated_child+0xc/0x14[musb_am335x]) [<bf05900c>] (of_remove_populated_child[musb_am335x]) from [<c02c5c48>] (device_for_each_child+0x44/0x70) [<c02c5c48>] (device_for_each_child) from [<bf05902c>] (am335x_child_remove+0x18/0x30[musb_am335x]) [<bf05902c>] (am335x_child_remove[musb_am335x]) from [<c02ca37c>] (platform_drv_remove+0x18/0x1c) [<c02ca37c>] (platform_drv_remove) from [<c02c8db0>] (__device_release_driver+0x70/0xc8) [<c02c8db0>] (__device_release_driver) from [<c02c94c8>] (driver_detach+0xb4/0xb8) [<c02c94c8>] (driver_detach) from [<c02c8ab4>] (bus_remove_driver+0x4c/0xa0)
[<c02c8ab4>] (bus_remove_driver) from [<c007fc40>] (SyS_delete_module+0x128/0x1cc) [<c007fc40>] (SyS_delete_module) from [<c000e500>] (ret_fast_syscall+0x0/0x48)Fixes 97238b35d5bb ("usb: musb: dsps: use proper child nodes")Cc <[email protected]> # v3.12+Acked-by George Cherian <[email protected]>Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/usb/musb/musb_am335x.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-)drivers/usb/musb/musb_am335x.c +6
-17@@ -19,21 +19,6 @@ err:return ret; } -static int of_remove_populated_child (struct device *dev,void *d) -{ -struct platform_device *pdev =to_platform_device (dev); - -of_device_unregister (pdev); -return 0 ; -} - -static int am335x_child_remove (struct platform_device *pdev) -{ -device_for_each_child (&pdev->dev,NULL , of_remove_populated_child); -pm_runtime_disable (&pdev->dev); -return 0 ; -} -static const struct of_device_id am335x_child_of_match[] = { { .compatible ="ti,am33xx-usb" }, { },@@ -42,13 +27,17 @@ MODULE_DEVICE_TABLE(of, am335x_child_of_match);static struct platform_driver am335x_child_driver = { .probe = am335x_child_probe, - .remove = am335x_child_remove, .driver = { .name ="am335x-usb-childs" , .of_match_table = am335x_child_of_match, }, }; -module_platform_driver (am335x_child_driver); +static int __init am335x_child_init (void ) +{ +return platform_driver_register (&am335x_child_driver); +} +module_init (am335x_child_init); +MODULE_DESCRIPTION ("AM33xx child devices" );MODULE_LICENSE ("GPL v2" );Signature · 1 line
1.9.1Felipe,
Quoted 8 lines
Quoted 2 lines
Any comments on this?
Any comments on this? I don't like to remove a feature, but as explained above, I don't see any other way to solve this.
The driver is currently broken. I tried using
of_platform_depopulate()but failed.
yeah, I know about the problem and, frankly, there's no other way to fix this. Just resend with linux-usb in Cc.
Signature · 8 lines
balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140623/f5184c5b/attachment.sig>
PATCH 1/1 usb: musb: Fix panic upon musb_am335x module removal
Quoted 10 lines · Felipe Balbi
Quoted 6 lines
Any comments on this? I don't like to remove a feature, but as explained above, I don't see any other way to solve this.
The driver is currently broken. I tried using
of_platform_depopulate()but failed.yeah, I know about the problem and, frankly, there's no other way to fix this. Just resend with linux-usb in Cc.
Ouch, forgot about linux-usb on this one! I'm resending on top of v3.16-rc1 (although I think this one already applies).
Thanks for the feedback,
Signature · 2 lines
Ezequiel Garc?a, VanguardiaSur www.vanguardiasur.com.ar
PATCH 1/1 usb: musb: Fix panic upon musb_am335x module removal
Quoted 9 lines · Ezequiel Garc?a
Quoted 7 lines · Felipe Balbi
Quoted 6 lines
Any comments on this? I don't like to remove a feature, but as explained above, I don't see any other way to solve this.
The driver is currently broken. I tried using
of_platform_depopulate()but failed.
Quoted 6 lines
Quoted 3 lines
yeah, I know about the problem and, frankly, there's no other way to fix this. Just resend with linux-usb in Cc.
Ouch, forgot about linux-usb on this one! I'm resending on top of v3.16-rc1 (although I think this one already applies).
thanks, that helps
Quoted 1 line
Thanks for the feedback,
no problem.
Signature · 8 lines
balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140623/30d92a7b/attachment.sig>