Forums

Hi Max,

Our USB engineer is on travel and he may not be able to look into this issue this week. I have created a bug ticket for tracking this issue.

https://cqweb.ext.ti.com/cqweb/main?command=GenerateMainFrame&service=CQ&schema=SDO-Web&contextid=SDOWP&entityID=SDOCM00091275&entityDefName=IncidentReport&username=readonly&password=readonly

Thanks,

Sekhar

Hi Sekhar

Thanks for opening the ticket, in the meanwhile I've tried to debug a little bit. Please not that musb driver cause crash if built with -DDEBUG: I had to remove dev_dbg call in cppi41dma_Interrupt function. Then I've built usb musb, core and gadget with -DDEBUG.

The composite_disconnect function is called by usb_gadget_remove_driver. But in PERIPHERAL mode case it is called a second time due to a DISCONNECT interrupt: at this time the usb_composite_dev struct associated to gadget seems to have been already cleaned, but composite_disconnect does not do any check on cdev not null, causing the crash when it calls spin_lock_irqsave(&cdev->lock, flags).

Gadget code seems a little bit weak from this point of view, but I am not sure this is the right place to fix: something in musb driver code flow seems changed, and maybe it is not the right one. Maybe that fix would mask other potential issues. I am looking for your final patch.

Logs follow, hope can help to understand.

Regards

Max

OTG case (fine):

root@am335x-evm:~# rmmod g_mass_storage
[   66.481057] g_mass_storage musb-hdrc.0: unregistering UDC driver [musb-hdrc]
[   66.488581] g cf9151a0 cdev cec3e840 cdev->config bf007b18 composite->disconnect   (null)!
[   66.497224]  gadget: composite_disconnect gadget cf9151a0 cdev cec3e840 ...
[   66.504494]  gadget: reset config
[   66.507967]  gadget: unbind function 'Mass Storage Function'/cfa67ec0
[   66.514696]  gadget: unbind
[   66.519081] musb-hdrc musb-hdrc.0: Descriptor (8f8ac000) popped from teardown completion queue
[   66.528145] musb-hdrc musb-hdrc.0: ep1out request cec3e9c0, 0/512 fault -104
[   66.535500]  gadget: bulk_out_complete --> -104, 0/31
[   66.540850]  gadget: reset interface
[   66.544599] musb-hdrc musb-hdrc.0: ep1in: abort DMA --> 0
[   66.550239] musb-hdrc musb-hdrc.0: ep1in
[   66.554340] musb-hdrc musb-hdrc.0: ep1out: abort DMA --> 0
[   66.560059] musb-hdrc musb-hdrc.0: ep1out
[   66.570104]  lun0: close backing file
[   66.574147]  lun1: close backing file
[   66.578030] musb-hdrc musb-hdrc.0: START musb_gadget_stop...
[   66.583956] musb-hdrc musb-hdrc.0: HNP: Stopping in unknown state b_peripheral
[   66.591491] musb-hdrc musb-hdrc.0: gadget D+ pullup off
[   66.599485] musb-hdrc musb-hdrc.0: HDRC disabled
[   66.604300] musb-hdrc musb-hdrc.0: UNDEFINED active, deleting timer
[   66.610851] musb-hdrc musb-hdrc.0: unregistering driver g_mass_storage
[   66.617659] musb-hdrc musb-hdrc.0: UNDEFINED inactive, starting idle timer for 10 ms
[   66.625759] musb-hdrc musb-hdrc.0: Poll devctl 98 (UNDEFINED)
[   66.631753] musb-hdrc musb-hdrc.0: remove, state 1
[   66.636770] musb-hdrc musb-hdrc.0: roothub graceful disconnect
[   66.642871] usb usb2: USB disconnect, device number 1
[   66.648137] usb usb2: unregistering device
[   66.652418] usb usb2: unregistering interface 2-0:1.0
[   66.657808] musb-hdrc musb-hdrc.0: shutdown urb cfb91340 ep1in-intr
[   66.664467] usb usb2: usb_disable_device nuking all URBs
[   66.672872] musb-hdrc musb-hdrc.0: HDRC disabled
[   66.677692] musb-hdrc musb-hdrc.0: UNDEFINED inactive, starting idle timer for 10 ms
[   66.685789] musb-hdrc musb-hdrc.0: USB bus 2 deregistered
[   66.691461] musb-hdrc musb-hdrc.0: ... musb_gadget_stop END
[   66.697286] musb-hdrc musb-hdrc.0: Poll devctl 98 (UNDEFINED)

PERIPHERAL case (crash):

root@am335x-evm:~# rmmod g_mass_storage
[   70.707391] g_mass_storage musb-hdrc.0: unregistering UDC driver [musb-hdrc]
[   70.714925] g cf9150b8 cdev cee20f00 cdev->config bf007b18 composite->disconnect   (null)!
[   70.723564]  gadget: composite_disconnect gadget cf9150b8 cdev cee20f00 ...
[   70.730828]  gadget: reset config
[   70.734310]  gadget: unbind function 'Mass Storage Function'/cf3b30c0
[   70.741028]  gadget: unbind
[   70.745408] musb-hdrc musb-hdrc.0: Descriptor (8f8ac000) popped from teardown completion queue
[   70.754475] musb-hdrc musb-hdrc.0: ep1out request ce47d900, 0/512 fault -104
[   70.761831]  gadget: bulk_out_complete --> -104, 0/31
[   70.767162]  gadget: reset interface
[   70.770898] musb-hdrc musb-hdrc.0: ep1in: abort DMA --> 0
[   70.776552] musb-hdrc musb-hdrc.0: ep1in
[   70.780646] musb-hdrc musb-hdrc.0: ep1out: abort DMA --> 0
[   70.786379] musb-hdrc musb-hdrc.0: ep1out
[   70.796494]  lun0: close backing file
[   70.800452]  lun1: close backing file
[   70.804413] musb-hdrc musb-hdrc.0: START musb_gadget_stop...
[   70.810348] musb-hdrc musb-hdrc.0: usbintr (20) epintr(0)
[   70.815981] musb-hdrc musb-hdrc.0: ** IRQ peripheral usb0020 tx0000 rx0000
[   70.823152] musb-hdrc musb-hdrc.0: <== Power=f0, DevCtl=98, int_usb=0x20
[   70.830142] musb-hdrc musb-hdrc.0: DISCONNECT (b_peripheral) as Peripheral, devctl 98
[   70.838309] musb g_mass_storage gadget disconnected.
[   70.845847] musb-hdrc musb-hdrc.0: devctl 98
[   70.850300] musb-hdrc musb-hdrc.0: musb_g_disconnect calling composite_disconnect...
[   70.858379] g cf9150b8 cdev   (null) cdev->config 00000001 composite->disconnect 00000002!
[   70.866999] [ERROR] composite_disconnect gadget cf9150b8 cdev   (null) : return to avoid crash!
[   70.876114] musb-hdrc musb-hdrc.0: usbintr (1) epintr(0)
[   70.881658] musb-hdrc musb-hdrc.0: ** IRQ peripheral usb0001 tx0000 rx0000
[   70.888828] musb-hdrc musb-hdrc.0: <== Power=e0, DevCtl=99, int_usb=0x1
[   70.895728] musb-hdrc musb-hdrc.0: SUSPEND (b_idle) devctl 99 power e0
[   70.902572] musb-hdrc musb-hdrc.0: gadget D+ pullup off
[   70.910568] musb-hdrc musb-hdrc.0: HDRC disabled
[   70.915382] musb-hdrc musb-hdrc.0: unregistering driver g_mass_storage
[   70.924759] musb-hdrc musb-hdrc.0: HDRC disabled
[   70.929575] musb-hdrc musb-hdrc.0: ... musb_gadget_stop END

Max,

Thanks for sharing the details. I will look into this and update sometime next week

Ajay

Hi Ajay, any news on this issue? Thanks, regards.

Max

I can dig up the logs if it helps you (as a working data point), but this worked fine for us on the AM335x EVM in the default PSP mode with USB0 switching g_ether.ko between g_mass_storage.ko.

Did you defconfig the initial PSP 04.06.00.07?

Seemed to make a big difference in USB (and other kernel) stability when I did.

I can dig up the logs if it helps you (as a working data point), but this worked fine for us on the AM335x EVM in the default PSP mode with USB0 switching g_ether.ko between g_mass_storage.ko.

Do you mean you cannot reproduce the issue with AM335x EVM and PSP 04.06.00.07 original code, or even after changing the USB0 role configuration from OTG to PERIPHERAL in board-am335xevm.c file? I know everything is fine with OTG configuration, I'm facing the issue just if USB0 is configured in PERIPHERAL mode.

Did you defconfig the initial PSP 04.06.00.07?
Seemed to make a big difference in USB (and other kernel) stability when I did.

What do you mean exactly with "to defconfig the initial PSP 04.06.00.07"? I've run the usual mrproper and then am335x_evm_defconfig... correct?

>>I'm facing the issue just if USB0 is configured in PERIPHERAL mode.

Sorry, once we understood the default EVM config HW doc here:

http://processors.wiki.ti.com/index.php/AM335X_USB_Driver_User_Guide_04.06.00#Port_mode_configuration

and SW documented below, we realized we only need OTG on USB0.  Haven't tried peripheral mode.

>> I've run the usual mrproper and then am335x_evm_defconfig... correct?

Yup, that's it.

A  key Linux improvement  in PSP-04.06.00.07 came from the file arch/arm/mach-omap2/board-am335xevm.c:

static struct omap_musb_board_data musb_board_data = {
        .interface_type = MUSB_INTERFACE_ULPI,
        /*
         * mode[0:3] = USB0PORT's mode
         * mode[4:7] = USB1PORT's mode
         * AM335X beta EVM has USB0 in OTG mode and USB1 in host mode.
         */

Please use below patch which will come in next version of TI PSP release.

----------------------------------------------------------------------------------------------------

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 305d843..47da5ed 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -2120,11 +2120,6 @@ void musb_g_disconnect(struct musb *musb)
        (void) musb_gadget_vbus_draw(&musb->g, 0);

        musb->g.speed = USB_SPEED_UNKNOWN;
-       if (musb->gadget_driver && musb->gadget_driver->disconnect) {
-               spin_unlock(&musb->lock);
-               musb->gadget_driver->disconnect(&musb->g);
-               spin_lock(&musb->lock);
-       }

        switch (musb->xceiv->state) {
        default:
-----------------------------------------------------------------------------------------------------------------------------

I am checking this with musb maintainer and will update if any change.

Ajay

Hi Ajay,

I've patched just udc_core.c as suggested, without changing musb driver code as in previously proposed patch: rmmod does not cause NULL pointer deference any longer, and everything seems working fine in HOST+PERIPHERAL modes such as in HOST+OTG modes case.

Thanks for your support.

Regards,

Max