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

[Jim Lin]

On 2016å04æ29æ 19:57, Felipe Balbi wrote:
> * PGP Signed by an unknown key
>
>
> Hi,
>
> Jim Lin <jilin@xxxxxxxxxx> writes:
> > On 2016å04æ28æ 20:21, Felipe Balbi wrote:
> > > > 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
> > > > "
> > > Okay, cool. Can you check that you're limitting your controller's speed
> > > to high-speed ?
> > Let's focus on original patch.
> > Could you help to explain why we need below d->Reserved1 checking?
> > Now the question is that
> >
> > https://msdn.microsoft.com/en-us/library/windows/hardware/gg463179.aspx
> >
> > Page 7 of OS_Desc_CompatID.doc
> > defines reserved field to be 1 and
> > below code will think that os_desc is invalid because d->Reserved1 is 1.
> >
> >
> > 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.

--nvpublic