From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756882AbcEEKgI (ORCPT ); Thu, 5 May 2016 06:36:08 -0400 Received: from nat-hk.nvidia.com ([203.18.50.4]:10804 "EHLO hkmmgate102.nvidia.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756581AbcEEKgG convert rfc822-to-8bit (ORCPT ); Thu, 5 May 2016 06:36:06 -0400 X-PGP-Universal: processed; by hkpgpgate102.nvidia.com on Thu, 05 May 2016 03:36:02 -0700 Subject: Re: [PATCH] usb: gadget: f_fs: Fix kernel panic for SuperSpeed To: Felipe Balbi References: <1461321780-3226-1-git-send-email-jilin@nvidia.com> <87bn51uagb.fsf@intel.com> <571E0058.6020007@nvidia.com> <87r3dtrj7b.fsf@intel.com> <571F2BA3.6040209@nvidia.com> <871t5plya5.fsf@intel.com> <57234522.9000607@nvidia.com> <87bn4siq5c.fsf@intel.com> <5729ADC9.3030704@nvidia.com> <87shxyds7o.fsf@intel.com> CC: , From: Jim Lin Message-ID: <572B2208.7010804@nvidia.com> Date: Thu, 5 May 2016 18:35:52 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <87shxyds7o.fsf@intel.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8BIT X-Originating-IP: [10.19.108.174] X-ClientProxiedBy: DRBGMAIL104.nvidia.com (10.18.16.23) To HKMAIL103.nvidia.com (10.18.16.12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年05月04日 18:37, Felipe Balbi wrote: > * PGP Signed by an unknown key > > > Hi, > > Jim Lin writes: > > > >>>> 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