From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v3,2/6] usb:common Separated decoding functions from dwc3 driver. From: Felipe Balbi Message-Id: <87tvhimyl8.fsf@linux.intel.com> Date: Tue, 05 Feb 2019 10:25:07 +0200 To: Pawel Laszczak , Greg KH Cc: "devicetree@vger.kernel.org" , "mark.rutland@arm.com" , "linux-usb@vger.kernel.org" , "hdegoede@redhat.com" , "heikki.krogerus@linux.intel.com" , "andy.shevchenko@gmail.com" , "robh+dt@kernel.org" , "rogerq@ti.com" , "linux-kernel@vger.kernel.org" , "jbergsagel@ti.com" , "nsekhar@ti.com" , "nm@ti.com" , Suresh Punnoose , "peter.chen@nxp.com" , Pawel Jez , Rahul Kumar List-ID: SGksCgpQYXdlbCBMYXN6Y3phayA8cGF3ZWxsQGNhZGVuY2UuY29tPiB3cml0ZXM6Cj4+T24gVGh1 LCBKYW4gMzEsIDIwMTkgYXQgMTE6NTI6MjlBTSArMDAwMCwgUGF3ZWwgTGFzemN6YWsgd3JvdGU6 Cj4+PiBQYXRjaCBtb3ZlcyBzb21lIGRlY29kaW5nIGZ1bmN0aW9ucyBmcm9tIGRyaXZlci91c2Iv ZHdjMy9kZWJ1Zy5oIGRyaXZlcgo+Pj4gdG8gZHJpdmVyL3VzYi9jb21tb24vZGVidWcuYyBmaWxl LiBUaGVzZSBtb3ZlZCBmdW5jdGlvbnMgaW5jbHVkZToKPj4+ICAgICBkd2MzX2RlY29kZV9nZXRf c3RhdHVzCj4+PiAgICAgZHdjM19kZWNvZGVfc2V0X2NsZWFyX2ZlYXR1cmUKPj4+ICAgICBkd2Mz X2RlY29kZV9zZXRfYWRkcmVzcwo+Pj4gICAgIGR3YzNfZGVjb2RlX2dldF9zZXRfZGVzY3JpcHRv cgo+Pj4gICAgIGR3YzNfZGVjb2RlX2dldF9jb25maWd1cmF0aW9uCj4+PiAgICAgZHdjM19kZWNv ZGVfc2V0X2NvbmZpZ3VyYXRpb24KPj4+ICAgICBkd2MzX2RlY29kZV9nZXRfaW50Zgo+Pj4gICAg IGR3YzNfZGVjb2RlX3NldF9pbnRmCj4+PiAgICAgZHdjM19kZWNvZGVfc3luY2hfZnJhbWUKPj4+ ICAgICBkd2MzX2RlY29kZV9zZXRfc2VsCj4+PiAgICAgZHdjM19kZWNvZGVfc2V0X2lzb2NoX2Rl bGF5Cj4+PiAgICAgZHdjM19kZWNvZGVfY3RybAo+Pj4KPj4+IFRoZXNlIGZ1bmN0aW9ucyBhcmUg dXNlZCBhbHNvIGluIGlucm9kdWNlZCBjZG5zMyBkcml2ZXIuCj4+Pgo+Pj4gQWxsIGZ1bmN0aW9u cyBwcmVmaXhlcyB3ZXJlIGNoYW5nZWQgZnJvbSBkd2MzIHRvIHVzYi4KPj4KPj5JY2ssIHdoeT8K Pgo+IEJlY2F1c2UgQ0ROUzMgZHJpdmVyIGluIG9uZSBvZiB0aGUgcHJldmlvdXMgdmVyc2lvbiBo YWQgaW1wbGVtZW50ZWQgdmVyeSBzaW1pbGFyIGZ1bmN0aW9uIGFzIGR3YzMgYW5kIEZlbGlwZSBz dWdnZXN0ZWQgdGhhdCB3ZSBzaG91bGQgCj4gbWFrZSBjb21tb24gZmlsZSB3aXRoIHRoZXNlIGZ1 bmN0aW9ucy4gSGUgYWxzbyBzdWdnZXN0ZWQgdGhhdCB0aGlzIGZ1bmN0aW9uIGFsc28gY291bGQg YmUgdXNlZCBvbiBob3N0IHNpZGUuIAoKcmlnaHQsIGhvc3QgY29udHJvbGxlcnMgY2FuIG1ha2Ug dXNlIG9mIENvbnRyb2wgdHJhbnNmZXIgZGVjb2RpbmcuYQoKPj4+IEFsc28sIGZ1bmN0aW9uJ3Mg cGFyYW1ldGVycyBoYXMgYmVlbiBleHRlbmRlZCBhY2NvcmRpbmcgdG8gdGhlIG5hbWUKPj4+IG9m IGZpZWxkcyBpbiBzdGFuZGFyZCBTRVRVUCBwYWNrZXQuCj4+PiBBZGRpdGlvbmFsbHksIHBhdGNo IGFkZHMgdXNiX2RlY29kZV9jdHJsIGZ1bmN0aW9uIHRvCj4+PiBpbmNsdWRlL2xpbnV4L3VzYi9j aDkuaCBmaWxlLgo+Pgo+PldoeSBjaDkuaD8gIEl0J3Mgbm90IHNvbWV0aGluZyB0aGF0IGlzIHNw ZWNpZmllZCBpbiB0aGUgc3BlYywgaXQncyBhCj4+dXNiLXNwZWNpZmljIHRoaW5nIDopCj4gV2h5 IG5vdCA/Cj4KPiBTaW1pbGFyIGFzIHVzYl9zdGF0ZV9zdHJpbmcgZnVuY3Rpb24gZnJvbSBpbmNs dWRlL2xpbnV4L3VzYi9jaDkuaCB3aGljaCAKPiAiIFJldHVybnMgaHVtYW4gcmVhZGFibGUgbmFt ZSBmb3IgdGhlIHN0YXRlIiwgdGhlIHVzYl9kZWNvZGVfY3RybCBmdW5jdGlvbiAKPiBtYWtlIHRo ZSBzYW1lIGJ1dCBmb3Igc3RhbmRhcmQgVVNCIHJlcXVlc3QuIAoKcmlnaHQsIEkgd291bGQgc2F5 IHVzYl9zdGF0ZV9zdHJpbmcoKSBzaG91bGQgYmUgbW92ZWQgZWxzZXdoZXJlLiBjaDkuaAppcywg cmVhbGx5LCBqdXN0IHdoYXQncyBkZXNjcmliZWQgaW4gY2hhcHRlciA5IG9mIHRoZSB1c2Igc3Bl Y2lmaWNhdGlvbi4KCj4+QWxzbywgdGhlIGFwaSBmb3IgdGhhdCBmdW5jdGlvbiBpcyBub3Qgb2su ICBJZiB5b3UgYXJlIGdvaW5nIHRvIG1ha2UKPj50aGlzIHNvbWV0aGluZyB0aGF0IHRoZSB3aG9s ZSBrZXJuZWwgY2FuIGNhbGwsIHlvdSBoYXZlIHRvIGZpeCBpdCB1cDoKPj4KPj4+ICsvKioKPj4+ ICsgKiB1c2JfZGVjb2RlX2N0cmwgLSBSZXR1cm5zIGh1bWFuIHJlYWRhYmxlIHJlcHJlc2VudGF0 aW9uIG9mIGNvbnRyb2wgcmVxdWVzdC4KPj4+ICsgKiBAc3RyOiBidWZmZXIgdG8gcmV0dXJuIGEg aHVtYW4tcmVhZGFibGUgcmVwcmVzZW50YXRpb24gb2YgY29udHJvbCByZXF1ZXN0Lgo+Pj4gKyAq ICAgICAgIFRoaXMgYnVmZmVyIHNob3VsZCBoYXZlIGFib3V0IDIwMCBieXRlcy4KPj4KPj4iYWJv dXQgMjAwIGJ5dGVzIiBpcyBub3QgdmVyeSBzcGVjaWZpYy4KPj4KPj5QYXNzIGluIHRoZSBsZW5n dGggc28gd2Uga25vdyB3ZSBkb24ndCBvdmVyZmxvdyBpdC4KPgo+IEkgZGlkbid0IHdhbnQgdG8g Y2hhbmdlIHRvIG11Y2ggdGhpcyBjb2RlLCBiZWNhdXNlIGl0J3Mgbm90IG15IGNvZGUuIAo+IEkn bSBub3Qgc3VyZSBpZiBJIHNob3VsZCA/CgpJIGhhdmUgYSBwYXRjaCBmb3IgdGhhdCwgdGhlbiB5 b3UgY2FuIHN0YXJ0IG9uIHRvcCBvZiBpdC4gSSdsbCBzZW5kIGl0CnNvb24gYWZ0ZXIgdGVzdGlu Zy4K From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felipe Balbi Subject: RE: [PATCH v3 2/6] usb:common Separated decoding functions from dwc3 driver. Date: Tue, 05 Feb 2019 10:25:07 +0200 Message-ID: <87tvhimyl8.fsf@linux.intel.com> References: <1548935553-452-1-git-send-email-pawell@cadence.com> <1548935553-452-3-git-send-email-pawell@cadence.com> <20190204114502.GA28608@kroah.com> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Pawel Laszczak , Greg KH Cc: "devicetree@vger.kernel.org" , "mark.rutland@arm.com" , "linux-usb@vger.kernel.org" , "hdegoede@redhat.com" , "heikki.krogerus@linux.intel.com" , "andy.shevchenko@gmail.com" , "robh+dt@kernel.org" , "rogerq@ti.com" , "linux-kernel@vger.kernel.org" , "jbergsagel@ti.com" , "nsekhar@ti.com" , "nm@ti.com" , Suresh Punnoose , "peter.chen@nxp.com" , Pawel Jez , Rahul Kumar List-Id: devicetree@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Pawel Laszczak writes: >>On Thu, Jan 31, 2019 at 11:52:29AM +0000, Pawel Laszczak wrote: >>> Patch moves some decoding functions from driver/usb/dwc3/debug.h driver >>> to driver/usb/common/debug.c file. These moved functions include: >>> dwc3_decode_get_status >>> dwc3_decode_set_clear_feature >>> dwc3_decode_set_address >>> dwc3_decode_get_set_descriptor >>> dwc3_decode_get_configuration >>> dwc3_decode_set_configuration >>> dwc3_decode_get_intf >>> dwc3_decode_set_intf >>> dwc3_decode_synch_frame >>> dwc3_decode_set_sel >>> dwc3_decode_set_isoch_delay >>> dwc3_decode_ctrl >>> >>> These functions are used also in inroduced cdns3 driver. >>> >>> All functions prefixes were changed from dwc3 to usb. >> >>Ick, why? > > Because CDNS3 driver in one of the previous version had implemented very = similar function as dwc3 and Felipe suggested that we should=20 > make common file with these functions. He also suggested that this functi= on also could be used on host side.=20 right, host controllers can make use of Control transfer decoding.a >>> Also, function's parameters has been extended according to the name >>> of fields in standard SETUP packet. >>> Additionally, patch adds usb_decode_ctrl function to >>> include/linux/usb/ch9.h file. >> >>Why ch9.h? It's not something that is specified in the spec, it's a >>usb-specific thing :) > Why not ? > > Similar as usb_state_string function from include/linux/usb/ch9.h which=20 > " Returns human readable name for the state", the usb_decode_ctrl functio= n=20 > make the same but for standard USB request.=20 right, I would say usb_state_string() should be moved elsewhere. ch9.h is, really, just what's described in chapter 9 of the usb specification. >>Also, the api for that function is not ok. If you are going to make >>this something that the whole kernel can call, you have to fix it up: >> >>> +/** >>> + * usb_decode_ctrl - Returns human readable representation of control = request. >>> + * @str: buffer to return a human-readable representation of control r= equest. >>> + * This buffer should have about 200 bytes. >> >>"about 200 bytes" is not very specific. >> >>Pass in the length so we know we don't overflow it. > > I didn't want to change to much this code, because it's not my code.=20 > I'm not sure if I should ? I have a patch for that, then you can start on top of it. I'll send it soon after testing. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlxZSGMACgkQzL64meEa mQbMGg//VXLDsETbEraPdlFH6KERmPC/PHNq6LpXI9OUu4q5DGk/vfHEuKb64Sr5 crkH64hxvxCswj3qCIlV4wr9iikEeijvV9bCwl4jVv1wqyaVZB7kXu501jUBHe8y XM0S9SI6jXdbByG0z4pLXe075IXQamzbGPtwUxOPiAkBeHXlc0wljzJFf1SNCbl0 7W2JHpWlzkwEFyQ8cl/rl+P7tM3T1oBufijqTLTdUlqXrYB12yJxnIIvWMIRXQnn uzGWIhPQql2sbefretjJWeArTA6yE7yNXh8fscmWvXoL1FudSRsKwjg2C2kySfhH Cq1PozYk+JEbIK1ZDuFqAoVd3ikro7D2MMwOrLCH/bRIi/jLvtHFBOor9fdHPaGq wZJYUX1uHje3HxoDB2RC2zn8CbIZrauGygL7nxp6keVbCZ+8u6FZbo25hgyP4Sxi 7XKHdBXJt7xV3l5L1O0lx/ispMLeAGRLpndRfoq8jVM+mFfPUigwlkjFw/qxhmcM zj1uKmCqLgE+TssyMIrDyIyFFHHmm822+rzkDtkEmoSlLa4Qb9f/+u2SOYLDSzDn QqsATtd7hE+7868iu9jLY2y8tWU0FqUTsRZfj8hE/a89y708F5ZR6HEgddW7SKOC QGyg8Xlqt+qlWoZC1wcUkw0e+xm1kWFrRvNYb3LA3grdeRlaS44= =iEDt -----END PGP SIGNATURE----- --=-=-=-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 25509C282CB for ; Tue, 5 Feb 2019 08:25:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E5F012145D for ; Tue, 5 Feb 2019 08:25:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727138AbfBEIZU (ORCPT ); Tue, 5 Feb 2019 03:25:20 -0500 Received: from mga05.intel.com ([192.55.52.43]:23356 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725898AbfBEIZT (ORCPT ); Tue, 5 Feb 2019 03:25:19 -0500 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Feb 2019 00:25:19 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,562,1539673200"; d="asc'?scan'208";a="317740704" Received: from pipin.fi.intel.com (HELO localhost) ([10.237.72.175]) by fmsmga005.fm.intel.com with ESMTP; 05 Feb 2019 00:25:15 -0800 From: Felipe Balbi To: Pawel Laszczak , Greg KH Cc: "devicetree\@vger.kernel.org" , "mark.rutland\@arm.com" , "linux-usb\@vger.kernel.org" , "hdegoede\@redhat.com" , "heikki.krogerus\@linux.intel.com" , "andy.shevchenko\@gmail.com" , "robh+dt\@kernel.org" , "rogerq\@ti.com" , "linux-kernel\@vger.kernel.org" , "jbergsagel\@ti.com" , "nsekhar\@ti.com" , "nm\@ti.com" , Suresh Punnoose , "peter.chen\@nxp.com" , Pawel Jez , Rahul Kumar Subject: RE: [PATCH v3 2/6] usb:common Separated decoding functions from dwc3 driver. In-Reply-To: References: <1548935553-452-1-git-send-email-pawell@cadence.com> <1548935553-452-3-git-send-email-pawell@cadence.com> <20190204114502.GA28608@kroah.com> Date: Tue, 05 Feb 2019 10:25:07 +0200 Message-ID: <87tvhimyl8.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Pawel Laszczak writes: >>On Thu, Jan 31, 2019 at 11:52:29AM +0000, Pawel Laszczak wrote: >>> Patch moves some decoding functions from driver/usb/dwc3/debug.h driver >>> to driver/usb/common/debug.c file. These moved functions include: >>> dwc3_decode_get_status >>> dwc3_decode_set_clear_feature >>> dwc3_decode_set_address >>> dwc3_decode_get_set_descriptor >>> dwc3_decode_get_configuration >>> dwc3_decode_set_configuration >>> dwc3_decode_get_intf >>> dwc3_decode_set_intf >>> dwc3_decode_synch_frame >>> dwc3_decode_set_sel >>> dwc3_decode_set_isoch_delay >>> dwc3_decode_ctrl >>> >>> These functions are used also in inroduced cdns3 driver. >>> >>> All functions prefixes were changed from dwc3 to usb. >> >>Ick, why? > > Because CDNS3 driver in one of the previous version had implemented very = similar function as dwc3 and Felipe suggested that we should=20 > make common file with these functions. He also suggested that this functi= on also could be used on host side.=20 right, host controllers can make use of Control transfer decoding.a >>> Also, function's parameters has been extended according to the name >>> of fields in standard SETUP packet. >>> Additionally, patch adds usb_decode_ctrl function to >>> include/linux/usb/ch9.h file. >> >>Why ch9.h? It's not something that is specified in the spec, it's a >>usb-specific thing :) > Why not ? > > Similar as usb_state_string function from include/linux/usb/ch9.h which=20 > " Returns human readable name for the state", the usb_decode_ctrl functio= n=20 > make the same but for standard USB request.=20 right, I would say usb_state_string() should be moved elsewhere. ch9.h is, really, just what's described in chapter 9 of the usb specification. >>Also, the api for that function is not ok. If you are going to make >>this something that the whole kernel can call, you have to fix it up: >> >>> +/** >>> + * usb_decode_ctrl - Returns human readable representation of control = request. >>> + * @str: buffer to return a human-readable representation of control r= equest. >>> + * This buffer should have about 200 bytes. >> >>"about 200 bytes" is not very specific. >> >>Pass in the length so we know we don't overflow it. > > I didn't want to change to much this code, because it's not my code.=20 > I'm not sure if I should ? I have a patch for that, then you can start on top of it. I'll send it soon after testing. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlxZSGMACgkQzL64meEa mQbMGg//VXLDsETbEraPdlFH6KERmPC/PHNq6LpXI9OUu4q5DGk/vfHEuKb64Sr5 crkH64hxvxCswj3qCIlV4wr9iikEeijvV9bCwl4jVv1wqyaVZB7kXu501jUBHe8y XM0S9SI6jXdbByG0z4pLXe075IXQamzbGPtwUxOPiAkBeHXlc0wljzJFf1SNCbl0 7W2JHpWlzkwEFyQ8cl/rl+P7tM3T1oBufijqTLTdUlqXrYB12yJxnIIvWMIRXQnn uzGWIhPQql2sbefretjJWeArTA6yE7yNXh8fscmWvXoL1FudSRsKwjg2C2kySfhH Cq1PozYk+JEbIK1ZDuFqAoVd3ikro7D2MMwOrLCH/bRIi/jLvtHFBOor9fdHPaGq wZJYUX1uHje3HxoDB2RC2zn8CbIZrauGygL7nxp6keVbCZ+8u6FZbo25hgyP4Sxi 7XKHdBXJt7xV3l5L1O0lx/ispMLeAGRLpndRfoq8jVM+mFfPUigwlkjFw/qxhmcM zj1uKmCqLgE+TssyMIrDyIyFFHHmm822+rzkDtkEmoSlLa4Qb9f/+u2SOYLDSzDn QqsATtd7hE+7868iu9jLY2y8tWU0FqUTsRZfj8hE/a89y708F5ZR6HEgddW7SKOC QGyg8Xlqt+qlWoZC1wcUkw0e+xm1kWFrRvNYb3LA3grdeRlaS44= =iEDt -----END PGP SIGNATURE----- --=-=-=--