RE: [PATCH v3] usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS

From: Phil Edworthy
Date: Tue Jul 07 2015 - 04:59:28 EST


Hi Shimoda-san,

On 06 July 2015 08:28, Shimoda-san wrote:
> Hi Phil-san,
>
> > Sent: Thursday, July 02, 2015 7:27 PM
> >
> > These changes allow a PHY driver to trigger a VBUS interrupt and
> > to provide the value of VBUS.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
>
> Thank you for the patch!
>
> However, if I tested this patch, a kernel panic happened when
> I did "rmmod g_mass_storage" on koelsch:
>
> root@192:~/usb# rmmod g_mass_storage
> Unable to handle kernel NULL pointer dereference at virtual address 00000004
> pgd = eebd4bc0
> [00000004] *pgd=00000000
> Internal error: Oops: 205 [#1] SMP ARM
> Modules linked in: g_mass_storage(-) usb_f_mass_storage libcomposite
> CPU: 0 PID: 2496 Comm: rmmod Not tainted 4.1.0-dirty #32
> Hardware name: Generic R8A7791 (Flattened Device Tree)
> task: eeb64ac0 ti: ee022000 task.ti: ee022000
> PC is at usbhs_pkt_pop+0x1c/0x100
> LR is at usbhsg_ep_dequeue+0x28/0x40
> pc : [<c036d934>] lr : [<c036e364>] psr: 20000013
> sp : ee023e70 ip : ee023e98 fp : ee023e94
> r10: 00000000 r9 : ee022000 r8 : c00101e4
> r7 : 00200200 r6 : 00100100 r5 : 00000000 r4 : eeb917f8
> r3 : c036e33c r2 : 00000041 r1 : eeb917f8 r0 : 00000000
> Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> Control: 30c5307d Table: 6ebd4bc0 DAC: fffffffd
> Process rmmod (pid: 2496, stack limit = 0xee022210)
> Stack: (0xee023e70 to 0xee024000)
> 3e60: ee1b1400 eeb917c0 00100100 00200200
> 3e80: c00101e4 ee022000 ee023eac ee023e98 c036e364 c036d924 eeb34180
> eeb341c0
> 3ea0: ee023ecc ee023eb0 bf002f20 c036e348 eeb34180 00000001 ee1b1800
> eeb341b8
> 3ec0: ee023eec ee023ed0 bf002fec bf002e5c ee181600 bf01d624 bed3ff0d
> 00000081
> 3ee0: ee023efc ee023ef0 bf003028 bf002f78 ee023f14 ee023f00 c036fb88
> bf003018
> 3f00: ee181600 bf01d624 ee023f2c ee023f18 c036fc10 c036fb34 bf01d694 7f64cd30
> 3f20: ee023f3c ee023f30 bf0013d0 c036fbbc ee023f4c ee023f40 bf01d26c bf0013c0
> 3f40: ee023fa4 ee023f50 c0090bd8 bf01d248 ee023f6c 616d5f67 735f7373
> 61726f74
> 3f60: 00006567 ee023f70 c00460f4 c052a8c0 c00101e4 ee022010 ee023fb0
> ee022000
> 3f80: ee023fac ee023f90 c00136c4 00046038 00000001 7f64cd00 00000000
> ee023fa8
> 3fa0: c0010040 c0090adc 00000001 7f64cd00 7f64cd30 00000800 ccd5c100 ccd5c100
> 3fc0: 00000001 7f64cd00 bed3ff0d 00000081 7f64c008 00000000 00000002 00000800
> 3fe0: bed3fe0c bed3fb98 b6eeb068 b6e4f05c 60000010 7f64cd30 00000000
> 00000000
> Backtrace:
> [<c036d918>] (usbhs_pkt_pop) from [<c036e364>]
> (usbhsg_ep_dequeue+0x28/0x40)
> r9:ee022000 r8:c00101e4 r7:00200200 r6:00100100 r5:eeb917c0 r4:ee1b1400
> [<c036e33c>] (usbhsg_ep_dequeue) from [<bf002f20>]
> (composite_dev_cleanup+0xd0/0x11c [libcomposite])
> r5:eeb341c0 r4:eeb34180
> [<bf002e50>] (composite_dev_cleanup [libcomposite]) from [<bf002fec>]
> (__composite_unbind+0x80/0xa0 [libcomposite])
> r7:eeb341b8 r6:ee1b1800 r5:00000001 r4:eeb34180
> [<bf002f6c>] (__composite_unbind [libcomposite]) from [<bf003028>]
> (composite_unbind+0x1c/0x20 [libcomposite])
> r7:00000081 r6:bed3ff0d r5:bf01d624 r4:ee181600
> [<bf00300c>] (composite_unbind [libcomposite]) from [<c036fb88>]
> (usb_gadget_remove_driver+0x60/0x88)
> [<c036fb28>] (usb_gadget_remove_driver) from [<c036fc10>]
> (usb_gadget_unregister_driver+0x60/0xa0)
> r5:bf01d624 r4:ee181600
> [<c036fbb0>] (usb_gadget_unregister_driver) from [<bf0013d0>]
> (usb_composite_unregister+0x1c/0x20 [libcomposite])
> r5:7f64cd30 r4:bf01d694
> [<bf0013b4>] (usb_composite_unregister [libcomposite]) from [<bf01d26c>]
> (msg_cleanup+0x30/0x3c [g_mass_storage])
> [<bf01d23c>] (msg_cleanup [g_mass_storage]) from [<c0090bd8>]
> (SyS_delete_module+0x108/0x1c4)
> [<c0090ad0>] (SyS_delete_module) from [<c0010040>]
> (ret_fast_syscall+0x0/0x3c)
> r5:7f64cd00 r4:00000001
> Code: e52de004 e8bd4000 e1a05000 e1a04001 (e5907004)
> ---[ end trace 8152e492b3f02f66 ]---
>
>
> I don't know why this kernel panic happened after I applied this patch.
> But, if I added the following code in the mod_gadget.c, this issue disappeared:
>
> @@ -682,7 +684,8 @@ static int usbhsg_ep_dequeue(struct usb_ep *ep, struct
> usb_r
> struct usbhsg_request *ureq = usbhsg_req_to_ureq(req);
> struct usbhs_pipe *pipe = usbhsg_uep_to_pipe(uep);
>
> - usbhs_pkt_pop(pipe, usbhsg_ureq_to_pkt(ureq));
> + if (pipe)
> + usbhs_pkt_pop(pipe, usbhsg_ureq_to_pkt(ureq));
> usbhsg_queue_pop(uep, ureq, -ECONNRESET);
>
> return 0;

That's odd, I can't see how my patch causes this problem. Do you think
that there has always been a race problem here and my changes make this
happen?

> Best regards,
> Yoshihiro Shimoda
>

Best regards
Phil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/