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: Wed, 02 Oct 2013 10:06:19 +0530 Message-ID: <524BA2C3.5050007@codeaurora.org> References: <1379678152-9617-1-git-send-email-mgautam@codeaurora.org> <52415BAC.6030708@codeaurora.org> <20130925204030.GW10746@radagast> <5243DD3A.8080603@codeaurora.org> <52493DFE.503@codeaurora.org> <20131001143731.GV1476@radagast> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.11.231]:50087 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751231Ab3JBEg2 (ORCPT ); Wed, 2 Oct 2013 00:36:28 -0400 In-Reply-To: <20131001143731.GV1476@radagast> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: balbi@ti.com Cc: Paul Zimmerman , Jack Pham , "pheatwol@codeaurora.org" , "linux-usb@vger.kernel.org" , "linux-arm-msm@vger.kernel.org" , "gregkh@linuxfoundation.org" On 10/1/2013 8:07 PM, Felipe Balbi wrote: > Hi, > > On Mon, Sep 30, 2013 at 02:31:50PM +0530, Manu Gautam wrote: >> On 9/28/2013 1:52 AM, Paul Zimmerman wrote: >>>> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of Manu Gautam >>>> Sent: Thursday, September 26, 2013 12:08 AM >>>> >>>> On 9/26/2013 2:10 AM, Felipe Balbi wrote: >>>>> >>>>> 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: >>> >>> Just a suggestion: Instead of a new interface for SuperSpeed, why not >>> just add the new fields to the end of the ffs_data struct? And have the >>> functions that copy the struct to/from userspace check the 'len' value >>> passed in, and only handle the SuperSpeed stuff if the length indicates >>> it is new userspace? >>> >> >> Initially I thought on similar lines but then adding a new interface for >> SS looked cleaner to me. But, your suggestion also make sense as we can >> avoid extra system call and the same interface can be extended later. >> One more thing we can do is to add a magic number after hs_desc (i.e. at >> the end of existing ffs_data) to specify that ss_descriptors are following. >> This can be used by kernel driver to check if userspace is trying pass >> ss desc only or some invalid data. >> Felipe: Your recommendation on this? > > We need to have some more people look at this. I remember there were > always some concerns about Chris architecture when doing such changes. > Can you please add appropriate folks to this thread who can check this as well? -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation