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

[Jim Lin]

On 2016å05æ04æ 18:37, Felipe Balbi wrote:
> * PGP Signed by an unknown key
>
>
> Hi,
>
> Jim Lin <jilin@xxxxxxxxxx> writes:
>
> <snip>
>
> > > > In f_fs.c
> > > > "
> > > > static int __ffs_data_do_os_desc(enum ffs_os_desc_type type,
> > > >                     struct usb_os_desc_header *h, void *data,
> > > >                     unsigned len, void *priv)
> > > > {
> > > >        struct ffs_data *ffs = priv;
> > > >        u8 length;
> > > >
> > > >        ENTER();
> > > >
> > > >        switch (type) {
> > > >        case FFS_OS_DESC_EXT_COMPAT: {
> > > >            struct usb_ext_compat_desc *d = data;
> > > >            int i;
> > > >
> > > >            if (len < sizeof(*d) ||
> > > >                d->bFirstInterfaceNumber >= ffs->interfaces_count ||
> > > >                d->Reserved1)
> > > >                return -EINVAL;
> > > > "
> > > that's fine, but this is only failing because something else is
> > > returning the wrong set of descriptors (SS vs HS). That's the bug we
> > > want to fix, not work around it.
> > Thanks.
> you're welcome, but to fix that bug we need more information. Why is
> composite.c using the wrong set of descriptors ? What is your setup ?
>
> Are you using an in-kernel gadget ? which one ?
No, our gadget driver is on the way to submit.
> Using configfs or legacy
> gadgets ? gadgetfs ? f_fs ?

>   How to trigger this ? Can you provide
> instructions and (in case of gadgetfs/ffs) code to create a gadget that
> hits this problem ?
Please refer to
https://android.googlesource.com/platform/system/core/+/master/adb/usb_linux_client.cpp
https://android.googlesource.com/device/google/dragon/+/android-6.0.1_r4/init.dragon.usb.rc
https://android.googlesource.com/platform/system/core/+/master/rootdir/init.usb.configfs.rc


Also this is a thought coming from another engineer for your reference.
"

I think Microsoft and linux are contradicting the requirements. 
According MSFT's os descriptor definition, one of the reserved fields 
needs to be set to 1 whereas seems like f_fs.c expects them to be 0. 
(copy pasting from the spec downloaded from: 
https://msdn.microsoft.com/en-us/library/windows/hardware/gg463179.aspx) 
What does upstream think ? Requires some conflict resolution I guess !! 
Since the OS descriptors are from MSFT, I believe upstream has to drop 
the check and I think this patch might be valid..

bFirstInterfaceNumber This field specifies the interface or function 
that is associated with the IDs in this section. To use this function 
section to associate a single-function group of interfaces with a single 
pair of IDs, set bFirstInterfaceNumber to the first interface in the 
group. Then use an IAD in that interfaceâs interface descriptor to 
specify which additional interfaces should be included in the group. The 
interfaces in the group must be consecutively numbered. For details, see 
âSupport for USB Interface Association Descriptor in Windows.â

RESERVED Reserved for system use. Set this value to 0x01.

compatibleID This field contains the value of the compatible ID to be 
associated with this function. Any unused bytes should be filled with 
NULLs. If the function does not have a compatible ID, fill the entire 
field with NULLs.

subCompatibleID This field contains the value of the subcompatible ID to 
be associated with this function. Any remaining bytes should be filled 
with NULLs. If the function does not have a subcompatible ID, fill the 
entire field with NULLs.

RESERVED Reserved. Fill this value with NULLs.
"

--nvpublic