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

From: Pali RohÃr
Date: Thu Feb 19 2015 - 19:09:57 EST


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@xxxxxxxxx>
> > > > > > > > > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > > > > > > > > ---
> > > > > > > >
> > > > > > > > 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@xxxxxxxxx>
> > > > [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.

--
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.