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@xxxxxxxxxxxxxxx [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
