From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felipe Balbi Subject: Re: [PATCH v2 1/1] usb: gadget: f_fs: Add support for SuperSpeed Mode Date: Tue, 1 Oct 2013 09:37:31 -0500 Message-ID: <20131001143731.GV1476@radagast> 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> Reply-To: Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="PxDrs/Fpf4pPiewX" Return-path: Content-Disposition: inline In-Reply-To: <52493DFE.503-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> Sender: linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Manu Gautam Cc: Paul Zimmerman , "balbi-l0cyMroinI0@public.gmane.org" , Jack Pham , "pheatwol-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" , "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org" List-Id: linux-arm-msm@vger.kernel.org --PxDrs/Fpf4pPiewX Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-usb-owner-u79uwXL29TY@public.gmane.org= nel.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 driv= er > >>>> incompatible with existing userspace applications compiled against o= ld > >>>> 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 reserv= ed, > >>> unused space in our structures for situations where we need to add mo= re > >>> 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: > >=20 > > 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? > >=20 >=20 > 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 followin= g. > This can be used by kernel driver to check if userspace is trying pass > ss desc only or some invalid data.=20 > 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. --=20 balbi --PxDrs/Fpf4pPiewX Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQIcBAEBAgAGBQJSSt4rAAoJEIaOsuA1yqREh4sP/iDGatjcs5i4oF7/S4SI/12g mJtoAyr1fC+Xv+exa+MBNyye+zeFRuQN8bhTRCtPdEUhhrqBWgoIllUdVe76nT5e WRW1ErLKr2HmV5mDdx6Mac2jml3RqGP5cx130253aNaa14IFxBIwnjg9w/CzegxU /99VOUmKWE4QKX/4nv7Ab6QeCUUCourW6wqNydDFmG4eMe1vxd5tPQoUoPD3lMQh CXkOI29CUvZo3xJ+LoptfEEWwY0JwUt6GZODSnjvpR7uQTDS8aXZUIlik65qMVjJ /hUVDfdSbgX6g1tGCf61kUHbh/6NMfV1QNEHPL8eLxcO0bcZjKyPPbrsqeTC3LE5 h3+07ZSq0R+77sLOa0coV0+nxkUDeCMMjj2SAsFK/sN1zwBU0D+fS3ivkarZ/kXZ /OBNlx3+Z2zBel0UGR1Cc8uOxR4Zf2kJHLoisBCsNGp2vOC7aPrR3YB8v+rp7cnT O+ry+r/dAIAzIq4CKiyqy2w+2WT2RbzDoQq57kooMPJX+4ZQessZjeZEILCbf4wM 3SFcy/kqqJTsdmQEtfrHv2iaOMnA+alyja5rruq0vvr3n+d7s78A5I4fxYzBnoFQ vEyBpWGJRaOfFzNZy7NALSVmSmVQbLSsvxbTXrk3Lm8n1nSBzHNmzcK5toW4DR/8 nh3umxihYJhWQfd0pKZM =VZRU -----END PGP SIGNATURE----- --PxDrs/Fpf4pPiewX-- -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html