Re: [PATCH 1/3] usb: gadget: function: phonet: balance usb_ep_disable calls

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 8971 bytes --]

On Fri, Feb 20, 2015 at 01:09:48AM +0100, Pali Rohár wrote:
> On Thursday 05 February 2015 13:38:58 Pali Rohár wrote:
> > On Tuesday 03 February 2015 20:57:11 Pali Rohár wrote:
> > > On Tuesday 03 February 2015 20:35:25 Felipe Balbi wrote:
> > > > On Tue, Feb 03, 2015 at 08:27:52PM +0100, Pali Rohár wrote:
> > > > > On Tuesday 03 February 2015 20:18:59 Felipe Balbi wrote:
> > > > > > On Tue, Feb 03, 2015 at 05:17:28PM +0100, Pali Rohár
> > 
> > wrote:
> > > > > > > On Tuesday 03 February 2015 16:43:45 Felipe Balbi
> > 
> > wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > On Tue, Feb 03, 2015 at 04:31:51PM +0100, Pali
> > > > > > > > Rohár
> > > 
> > > wrote:
> > > > > > > > > On Tuesday 03 February 2015 00:15:19 Felipe
> > > > > > > > > Balbi
> > > 
> > > wrote:
> > > > > > > > > > f_phonet's ->set_alt() method will call
> > > > > > > > > > usb_ep_disable() potentially on an endpoint
> > > > > > > > > > which is already disabled. That's something
> > > > > > > > > > the gadget/function driver must guarantee
> > > > > > > > > > that it's always balanced.
> > > > > > > > > > 
> > > > > > > > > > In order to balance the calls, just make sure
> > > > > > > > > > the endpoint was enabled before by means of
> > > > > > > > > > checking the validity of driver_data.
> > > > > > > > > > 
> > > > > > > > > > Reported-by: Pali Rohár <pali.rohar@gmail.com>
> > > > > > > > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > > > > > > > > ---
> > > > > > > > > 
> > > > > > > > > Your patches cause that kernel does not print
> > > > > > > > > any error message to n900 screen anymore and
> > > > > > > > > reboot device in 10 seconds. I did not loaded
> > > > > > > > > any external modules.
> > > > > > > > 
> > > > > > > > > In qemu I see this crash in early boot:
> > > > > > > > alright, so n900's working fine. I'll wait until
> > > > > > > > you debug qemu a little more, thank you
> > > > > > > 
> > > > > > > NO! It does not working, see ^^^^. It break n900
> > > > > > > totally!
> > > > > > 
> > > > > > settle down a bit more. I don't have the HW you have
> > > > > > and things are working fine on boards I _do_ have,
> > > > > > there's not much more I can do to help without you
> > > > > > doing your homework. Debug a bit more and bring more
> > > > > > information as to what's going on, until then you're
> > > > > > on your own.
> > > > > 
> > > > > And what more do you need? It crash on my n900 and also
> > > > > in qemu. I sent you kernel crash dump from qemu which
> > > > > introduced *your* patches. Before applying your patches
> > > > > there was no crash in early boot stage.
> > > > > 
> > > > > In current state I review all 3 patches as:
> > > > > 
> > > > > Rejected-by: Pali Rohár <pali.rohar@gmail.com>
> > > > > [It breaks booting Nokia N900 device]
> > > > 
> > > > next step, figure why it's broken. Working just fine here
> > > > on AM335x which has the same musb IP.
> > > 
> > > Why is broken? That is easy. You send 3 patches which broke
> > > it.
> > 
> > Actually when I reverted only that patch which adds line:
> > 
> > pm_runtime_irq_safe(musb->controller)
> > 
> > then early boot crash disappeared.
> > 
> > But other two patches did not fixed support for external .ko
> > gadget modules. State is same -- crash after modprobe.
> 
> Here is crash from qemu when musb is compiled into kernel:
> 
> [    0.641662] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [    0.642211] pgd = c0004000
> [    0.642425] [00000000] *pgd=00000000
> [    0.642913] Internal error: Oops: 80000005 [#1] PREEMPT ARM
> [    0.643371] Modules linked in:
> [    0.643737] CPU: 0 PID: 1 Comm: swapper Not tainted 3.19.0-rc5+ #329
> [    0.644195] Hardware name: Nokia RX-51 board
> [    0.644531] task: cf8a8000 ti: cf8ac000 task.ti: cf8ac000
> [    0.644958] PC is at 0x0
> [    0.645263] LR is at omap2430_runtime_resume+0x80/0x100
> [    0.645660] pc : [<00000000>]    lr : [<c02ccebc>]    psr: 60000113
> [    0.645660] sp : cf8adda0  ip : 00000001  fp : c0059c64
> [    0.646423] r10: cf8adde8  r9 : cf9b5884  r8 : cf9b58cc
> [    0.646789] r7 : 00000004  r6 : cf93ee10  r5 : c06ac84c  r4 : cf83a010
> [    0.647216] r3 : 00000000  r2 : c0565716  r1 : 00000414  r0 : fa0ab000
> [    0.647735] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
> [    0.648254] Control: 10c53c7d  Table: 80004059  DAC: 00000015
> [    0.648651] Process swapper (pid: 1, stack limit = 0xcf8ac238)
> [    0.649078] Stack: (0xcf8adda0 to 0xcf8ae000)
> [    0.649444] dda0: c0022428 cf9b5810 cf93ee10 c026bb44 00000001 c026d3e0 00000000 cf9b5810
> [    0.650085] ddc0: 00000000 c026d4a0 cf9b5810 cf9b5810 00000000 c026e9a4 c0441790 cfa29410
> [    0.650634] dde0: cfb33800 c0441790 cfa29410 cfa2b700 00000000 60000113 cfa2b6c0 cfa29410
> [    0.651153] de00: c0660c80 0000006c c06210c4 c05de6d4 00000000 c026ee74 cfa2c180 cfa29410
> [    0.651702] de20: cfa2b6c0 c026eed4 cf83a010 c02c53f8 cfa29410 0000006c fa0ab000 ffffffed
> [    0.652252] de40: cfa29410 c0660c80 c0660c80 c062cfe0 c06210c4 c05de6d4 00000000 c0267ad8
> [    0.652770] de60: 00000000 cfa29410 00000000 c026624c 00000000 cfa29410 c0660c80 c0660c80
> [    0.653320] de80: 00000000 c0266478 00000000 cfa29410 cfa29444 c02664f0 c0660c80 cf8adea8
> [    0.653839] dea0: c0266490 c0264dd0 cf88cb8c cfa2c2b0 c0660c80 c0660c80 cfb32b80 c065aad0
> [    0.654388] dec0: 00000000 c0265b80 c044162c c044162d 00000000 c0660c80 cfb37580 00000000
> [    0.654937] dee0: c062cfe0 c0266e28 c0267a38 c0605a5c cfb37580 c00088fc c05de6d4 c004ae84
> [    0.655517] df00: c05acdcc cfcb6cd3 00000000 c0566a48 a0000113 c05acdcc 00000006 00000075
> [    0.656036] df20: 00000006 c004af28 00000075 00000006 00000006 c05de6d4 cfcb6cc3 cfcb6cd2
> [    0.656585] df40: 00000000 00000006 c0614670 00000006 c0614674 c0614654 c0670680 00000075
> [    0.657104] df60: c06210c4 c05decfc 00000006 00000006 c05de6d4 c06488d8 c0620c8c c0620c8c
> [    0.657653] df80: 00000000 00000000 00000000 00000000 00000000 c05ded94 cf8ac000 00000000
> [    0.658172] dfa0: c03f9ff0 c03f9ff8 00000000 c000ddd8 00000000 00000000 00000000 00000000
> [    0.658721] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    0.659271] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> [    0.660186] [<c02ccebc>] (omap2430_runtime_resume) from [<c026bb44>] 
> (pm_generic_runtime_resume+0x2c/0x40)
> [    0.660919] [<c026bb44>] (pm_generic_runtime_resume) from [<c026d3e0>] (__rpm_callback+0x8c/0xdc)
> [    0.661529] [<c026d3e0>] (__rpm_callback) from [<c026d4a0>] (rpm_callback+0x70/0x88)
> [    0.662048] [<c026d4a0>] (rpm_callback) from [<c026e9a4>] (rpm_resume+0x514/0x704)
> [    0.662567] [<c026e9a4>] (rpm_resume) from [<c026ee74>] (__pm_runtime_resume+0x4c/0x90)
> [    0.663116] [<c026ee74>] (__pm_runtime_resume) from [<c026eed4>] (pm_runtime_irq_safe+0x1c/0x74)
> [    0.663757] [<c026eed4>] (pm_runtime_irq_safe) from [<c02c53f8>] (musb_init_controller+0x50/0x60c)
> [    0.664367] [<c02c53f8>] (musb_init_controller) from [<c0267ad8>] (platform_drv_probe+0x48/0x90)
> [    0.664947] [<c0267ad8>] (platform_drv_probe) from [<c026624c>] (really_probe+0xac/0x1e0)
> [    0.665496] [<c026624c>] (really_probe) from [<c0266478>] (driver_probe_device+0x30/0x48)
> [    0.666046] [<c0266478>] (driver_probe_device) from [<c02664f0>] (__driver_attach+0x60/0x84)
> [    0.666595] [<c02664f0>] (__driver_attach) from [<c0264dd0>] (bus_for_each_dev+0x50/0x84)
> [    0.667144] [<c0264dd0>] (bus_for_each_dev) from [<c0265b80>] (bus_add_driver+0xac/0x1bc)
> [    0.667694] [<c0265b80>] (bus_add_driver) from [<c0266e28>] (driver_register+0x9c/0xe0)
> [    0.668212] [<c0266e28>] (driver_register) from [<c00088fc>] (do_one_initcall+0x100/0x1b0)
> [    0.668762] [<c00088fc>] (do_one_initcall) from [<c05decfc>] (do_basic_setup+0x88/0xc0)
> [    0.669311] [<c05decfc>] (do_basic_setup) from [<c05ded94>] (kernel_init_freeable+0x60/0xfc)
> [    0.669891] [<c05ded94>] (kernel_init_freeable) from [<c03f9ff8>] (kernel_init+0x8/0xe4)
> [    0.670410] [<c03f9ff8>] (kernel_init) from [<c000ddd8>] (ret_from_fork+0x14/0x3c)
> [    0.670989] Code: bad PC value
> [    0.671752] ---[ end trace 087e5b16cf36deef ]---
> [    0.672210] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> [    0.672210] 
> [    0.672882] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> [    0.672882]
> 
> Reason why it crashes is because when function
> omap2430_runtime_resume() is called pointer to functions 
> musb_readl and musb_writel are both NULL. And so NULL pointer
> dereference.

https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=1861a2c60351a390272b3395f4d88480cdfd9e58

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]