Re: [PATCH] usb: gadget: f_fs: Fix kernel panic for SuperSpeed

[Jim Lin]

On 2016å04æ25æ 20:01, Felipe Balbi wrote:
> * PGP Signed by an unknown key
>
>
>
>
> > > Is this happening on set_config() ? If that's the case, why is
> > > gadget->speed set to USB_SPEED_SUPER to start with ? Your controller
> > > should already have negotiated highspeed which means
> > > function_descriptors() should have returned highspeed descriptors, not a
> > > NULL superspeed.
> > >
> > > Care to explain why you haven't negotiated Highspeed ? The only thing I
> > > can think of is that you're using a Superspeed-capable peripheral
> > > controller (dwc3?) with maximum-speed set to Superspeed, with a
> > > Superspeed-capable cable connected to an XHCI PC, but loading a
> > > high-speed gadget driver (which you got from Android, written with f_fs)
> > > and this gadget doesn't tell composite that its maximum speed is
> > > Highspeed, instead of super-speed.
> > >
> > > We can add a check, sure, to avoid a kernel oops; however, you should
> > > really fix up the gadget implementation and/or set dwc3's maximum-speed
> > > property accordingly.
> > >
> > > Can you check if this patch makes your scenario work while still being
> > > fully functional ?
> > >
> > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> > > index de9ffd60fcfa..3d3cdc5ed20d 100644
> > > --- a/drivers/usb/gadget/composite.c
> > > +++ b/drivers/usb/gadget/composite.c
> > > @@ -66,20 +66,36 @@ function_descriptors(struct usb_function *f,
> > >    {
> > >    	struct usb_descriptor_header **descriptors;
> > >    
> > > +	/*
> > > +	 * NOTE: we try to help gadget drivers which might not be setting
> > > +	 * max_speed appropriately.
> > > +	 */
> > > +
> > >    	switch (speed) {
> > >    	case USB_SPEED_SUPER_PLUS:
> > >    		descriptors = f->ssp_descriptors;
> > > -		break;
> > > +		if (descriptors)
> > > +			break;
> > > +		/* FALLTHROUGH */
> > >    	case USB_SPEED_SUPER:
> > >    		descriptors = f->ss_descriptors;
> > > -		break;
> > > +		if (descriptors)
> > > +			break;
> > > +		/* FALLTHROUGH */
> > >    	case USB_SPEED_HIGH:
> > >    		descriptors = f->hs_descriptors;
> > > -		break;
> > > +		if (descriptors)
> > > +			break;
> > > +		/* FALLTHROUGH */
> > >    	default:
> > >    		descriptors = f->fs_descriptors;
> > >    	}
> > >    
> > > +	/*
> > > +	 * if we can't find any descriptors at all, then this gadget deserves to
> > > +	 * Oops with a NULL pointer dereference
> > > +	 */
> > > +
> > >    	return descriptors;
> > >    }
> > >    
> > After trying your change, no kernel panic, but SuperSpeed device (device
> > mode) is not enumerated by ubuntu 14.04 USB 3.0 host controller as MTP
> > device with USB3.0 cable.
> what do you get on dmesg on host side ? Are you running dwc3 ? If you
> are, please capture trace logs of the failure:
>
> # mount -t debugfs none /sys/kernel/debug
> # cd /sys/kernel/debug/tracing
> # echo 2048 > buffer_size_kb
> # echo 1 > events/dwc3/enable
>
> (now connect your cable to host pc)
>
> # cp trace /path/to/non-volatile/storage/trace.txt
>
> Please reply with this trace.txt file and dmesg from host side.
This is not running with dwc3.

dmesg from PC host side (after adding your change without my patch):

[17907.984647] usb 6-2: new SuperSpeed USB device number 54 using xhci_hcd
[17908.012036] usb 6-2: No SuperSpeed endpoint companion for config 1  
interface 1 altsetting 0 ep 2: using minimum values
[17908.012040] usb 6-2: No SuperSpeed endpoint companion for config 1  
interface 1 altsetting 0 ep 131: using minimum values
[17908.013652] usb 6-2: New USB device found, idVendor=xxxx, idProduct=xxxx
[17908.013656] usb 6-2: New USB device strings: Mfr=1, Product=2, 
SerialNumber=3
[17908.013658] usb 6-2: Product: xxxxxxx
[17908.013661] usb 6-2: Manufacturer: xxxxxx
[17908.013664] usb 6-2: SerialNumber: 1234567890
[17908.014680] xhci_hcd 0000:05:00.0: ERROR: unexpected command 
completion code 0x11.
[17908.014690] usb 6-2: can't set config #1, error -22

I also attach git log of system/core/adb/usb_linux_client.cpp of Android 
N for your reference.
"
Author: Badhri Jagan Sridharan <Badhri@xxxxxxxxxx>
Date:   Mon Oct 5 13:04:03 2015 -0700

    adbd: Add os descriptor support for adb.

    Eventhough windows does not rely on extended os
    descriptor for adbd, when android usb device is
    configures as a composite device such as mtp+adb,
    windows discards the extended os descriptor even
    if one of the USB function fails to send
    the extended compat descriptor. This results in automatic
    install of MTP driverto fail when Android device is in
    "File Transfer" mode with adb enabled.

https://msdn.microsoft.com/en-us/library/windows/hardware/gg463179.aspx
"

--nvpublic