From mboxrd@z Thu Jan 1 00:00:00 1970 From: Manu Gautam Subject: Re: [PATCH v2 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode Date: Thu, 26 Sep 2013 12:37:38 +0530 Message-ID: <5243DD3A.8080603@codeaurora.org> References: <1379678152-9617-1-git-send-email-mgautam@codeaurora.org> <52415BAC.6030708@codeaurora.org> <20130925204030.GW10746@radagast> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.11.231]:43367 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752294Ab3IZHHn (ORCPT ); Thu, 26 Sep 2013 03:07:43 -0400 In-Reply-To: <20130925204030.GW10746@radagast> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: balbi@ti.com Cc: Jack Pham , pheatwol@codeaurora.org, linux-usb@vger.kernel.org, linux-arm-msm@vger.kernel.org, gregkh@linuxfoundation.org On 9/26/2013 2:10 AM, Felipe Balbi wrote: > Hi, > > (please avoid top-posting) > > On Tue, Sep 24, 2013 at 03:00:20PM +0530, Manu Gautam wrote: >> Hi Felipe, >> >> I wanted to mention one point with respect to this patch: Below >> changes in the functionfs.h to add ss_count (super speed descriptors >> count) in desc_header (which is passed from userspace) make the driver >> incompatible with existing userspace applications compiled against old >> header file. Let me know if that is acceptable. We are using this >> driver with Android for adbd (android debug bridge) and these changes >> are required to support adb over Super Speed controllers e.g. DWC3 >> along with changed in adbd to pass SS EP and companion descriptors. > > Good you mentioned, it saves me the trouble of reviewing this patch :-) > > It's not acceptable to break userspace ABI at all. If you want > SuperSpeed support on function fs, we need to figure out a way to do so > without breaking userspace. > > This might mean adding a separate userspace interface to be used with > superspeed. While at that, we might want to add a few bytes of reserved, > unused space in our structures for situations where we need to add more > data into it, just to make it slightly future proof. > Thanks for your reply. As you suggested we can have a different interface for super speed which would be optional to workaround ABI compatibility issue. Let me know if below interface looks fine to you, I will then implement accordingly: diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h index d6b0128..b8cb740 100644 --- a/include/uapi/linux/usb/functionfs.h +++ b/include/uapi/linux/usb/functionfs.h @@ -9,8 +9,9 @@ enum { - FUNCTIONFS_DESCRIPTORS_MAGIC = 1, - FUNCTIONFS_STRINGS_MAGIC = 2 + FUNCTIONFS_DESCRIPTORS_MAGIC = 1, + FUNCTIONFS_STRINGS_MAGIC = 2, + FUNCTIONFS_SS_DESCRIPTORS_MAGIC = 3 }; @@ -60,6 +61,25 @@ struct usb_functionfs_descs_head { * | 2 | payload | | descriptor's payload | */ +struct usb_functionfs_ss_descs_head { + __le32 magic; + __le32 length; + __le32 reserved; + __le32 ss_count; +} __attribute__((packed)); + +/* + * SS Descriptors format: + * + * | off | name | type | description | + * |-----+-----------+--------------+--------------------------------------| + * | 0 | magic | LE32 | FUNCTIONFS_SS_DESCRIPTORS_MAGIC | + * | 4 | length | LE32 | length of the whole data chunk | + * | 8 | ss_count | LE32 | number of super-speed descriptors | + * | 12 | reserved field | + * | 16 | ss_descrs | Descriptor[] | list of super-speed descriptors | + */ + struct usb_functionfs_strings_head { __le32 magic; __le32 length;