From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ezequiel Garcia Subject: Re: [RFC/PATCH] usb: musb: Prevent musb_am335x from being removed Date: Mon, 12 May 2014 11:08:35 -0300 Message-ID: <20140512140835.GB20360@arch.cereza> References: <1399570060-28939-1-git-send-email-ezequiel@vanguardiasur.com.ar> <5370552C.40506@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-yh0-f46.google.com ([209.85.213.46]:51292 "EHLO mail-yh0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758466AbaELOJJ (ORCPT ); Mon, 12 May 2014 10:09:09 -0400 Received: by mail-yh0-f46.google.com with SMTP id 29so6298134yhl.19 for ; Mon, 12 May 2014 07:09:09 -0700 (PDT) Content-Disposition: inline In-Reply-To: <5370552C.40506@ti.com> Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: George Cherian , Tony Lindgren , Felipe Balbi , Sebastian Andrzej Siewior Cc: linux-usb@vger.kernel.org, linux-omap@vger.kernel.org, Guido =?iso-8859-1?Q?Mart=EDnez?= , Greg Kroah-Hartman , yegorslists@googlemail.com On 12 May 10:29 AM, George Cherian wrote: > On 5/8/2014 10:57 PM, Ezequiel Garcia wrote: > >At probe time, the musb_am335x driver registers 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 in the musb_am335x driver device struct. > > > >However, when musb_dsps is loaded, its devices are attached to the musb_am335x > >driver device struct 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 be enforcing the removal of the devices registered by > >musb_am335x *only*, instead of all the 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. > > > >Just 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 : [] lr : [] 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 > >[] (am335x_shutdown) from [] (dsps_musb_exit+0x3c/0x4c [musb_dsps]) > >[] (dsps_musb_exit [musb_dsps]) from [] (musb_shutdown+0x80/0x90 [musb_hdrc]) > >[] (musb_shutdown [musb_hdrc]) from [] (musb_remove+0x24/0x68 [musb_hdrc]) > >[] (musb_remove [musb_hdrc]) from [] (platform_drv_remove+0x18/0x1c) > >[] (platform_drv_remove) from [] (__device_release_driver+0x70/0xc8) > >[] (__device_release_driver) from [] (device_release_driver+0x20/0x2c) > >[] (device_release_driver) from [] (bus_remove_device+0xdc/0x10c) > >[] (bus_remove_device) from [] (device_del+0x104/0x198) > >[] (device_del) from [] (platform_device_del+0x14/0x9c) > >[] (platform_device_del) from [] (platform_device_unregister+0xc/0x20) > >[] (platform_device_unregister) from [] (dsps_remove+0x18/0x38 [musb_dsps]) > >[] (dsps_remove [musb_dsps]) from [] (platform_drv_remove+0x18/0x1c) > >[] (platform_drv_remove) from [] (__device_release_driver+0x70/0xc8) > >[] (__device_release_driver) from [] (device_release_driver+0x20/0x2c) > >[] (device_release_driver) from [] (bus_remove_device+0xdc/0x10c) > >[] (bus_remove_device) from [] (device_del+0x104/0x198) > >[] (device_del) from [] (device_unregister+0xc/0x20) > >[] (device_unregister) from [] (of_remove_populated_child+0xc/0x14 [musb_am335x]) > >[] (of_remove_populated_child [musb_am335x]) from [] (device_for_each_child+0x44/0x70) > >[] (device_for_each_child) from [] (am335x_child_remove+0x18/0x30 [musb_am335x]) > >[] (am335x_child_remove [musb_am335x]) from [] (platform_drv_remove+0x18/0x1c) > >[] (platform_drv_remove) from [] (__device_release_driver+0x70/0xc8) > >[] (__device_release_driver) from [] (driver_detach+0xb4/0xb8) > >[] (driver_detach) from [] (bus_remove_driver+0x4c/0xa0) > >[] (bus_remove_driver) from [] (SyS_delete_module+0x128/0x1cc) > >[] (SyS_delete_module) from [] (ret_fast_syscall+0x0/0x48) > > > >Signed-off-by: Ezequiel Garcia > Acked-by: George Cherian > Thanks! The module removal was introduced together with the driver in this commit: commit 97238b35d5bbb5d5312d83c30a429824b777619f Author: Sebastian Andrzej Siewior Date: Fri Jul 5 14:51:33 2013 +0200 usb: musb: dsps: use proper child nodes AFAICS, by using git tag --contains, the above was merged since v3.12. Should I send a patch and Cc stable properly? -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar