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: [v1,2/2] usb:cdns3 Add Cadence USB3 DRD Driver From: Felipe Balbi Message-Id: <87a7lbme3m.fsf@linux.intel.com> Date: Wed, 12 Dec 2018 08:52:13 +0200 To: Pawel Laszczak , "devicetree@vger.kernel.org" , Kishon Vijay Abraham I Cc: "gregkh@linuxfoundation.org" , "linux-usb@vger.kernel.org" , "rogerq@ti.com" , "linux-kernel@vger.kernel.org" , Alan Douglas , "jbergsagel@ti.com" , "nsekhar@ti.com" , "nm@ti.com" , Suresh Punnoose , "peter.chen@nxp.com" , Pawel Jez , Rahul Kumar List-ID: SGkKClBhd2VsIExhc3pjemFrIDxwYXdlbGxAY2FkZW5jZS5jb20+IHdyaXRlczoKPj4+ICsJY2Ru cy0+cGh5ID0gZGV2bV9waHlfZ2V0KGRldiwgImNkbnMzLHVzYnBoeSIpOwo+Pj4gKwlpZiAoSVNf RVJSKGNkbnMtPnBoeSkpIHsKPj4+ICsJCXJldCA9IFBUUl9FUlIoY2Rucy0+cGh5KTsKPj4+ICsJ CWlmIChyZXQgPT0gLUVOT1NZUyB8fCByZXQgPT0gLUVOT0RFVikgewo+Pgo+PkFyZSB5b3Ugc3Vy ZSB5b3UgY2FuIGdldCBFTk9TWVMgaGVyZT8gSGF2ZSB5b3UgY2hlY2tlZCBvdXRwdXQgb2YKPj5j aGVja3BhdGNoIC0tc3RyaWN0Pwo+Pi06ODUyOiBXQVJOSU5HOiBFTk9TWVMgbWVhbnMgJ2ludmFs aWQgc3lzY2FsbCBucicgYW5kIG5vdGhpbmcgZWxzZQo+Cj4gWWVzIHRoaXMgZXJyb3IgY29kZSBj YW4gYmUgcmV0dXJuZWQgYnkgcmVsYXRlZCB0byBwaHkgZnVuY3Rpb24gaWYKPiBDT05GSUdfR0VO RVJJQ19QSFkgaXMgZGlzYWJsZWQuCj4KPiBJIGhhdmUgc2VlbiB0aGlzIHdhcm5pbmcgaW4gb3V0 cHV0IG9mIGNoZWNrcGF0Y2ggLS1zdHJpY3QgLgoKS2lzaG9uLCBzZWVtcyBsaWtlIHlvdSBzaG91 bGRuJ3QgYmUgdXNpbmcgdGhhdCBlcnJvciB2YWx1ZS4KCjxzbmlwPgoKPj4+ICsJY2FzZSBVU0Jf UkVRX1NFVF9JU09DSF9ERUxBWToKPj4+ICsJCXNwcmludGYoc3RyLCAiU2V0IElzb2Nocm9ub3Vz IERlbGF5IERlbGF5OiAlZCBucyIsIHdWYWx1ZSk7Cj4+PiArCQlicmVhazsKPj4+ICsJZGVmYXVs dDoKPj4+ICsJCXNwcmludGYoc3RyLAo+Pj4gKwkJCSJTRVRVUCBCUlQ6ICUwMnggQlI6ICUwMngg VjogJTA0eCBJOiAlMDR4IEw6ICUwNHhcbiIsCj4+PiArCQkJYlJlcXVlc3RUeXBlLCBiUmVxdWVz dCwKPj4+ICsJCQl3VmFsdWUsIHdJbmRleCwgd0xlbmd0aCk7Cj4+PiArCX0KPj4+ICsKPj4+ICsJ cmV0dXJuIHN0cjsKPj4+ICt9Cj4+Cj4+QWxsIG9mIHRoZXNlIGFyZSBhIGZsYXQgb3V0IGNvcHkg b2YgZHdjMydzIGltcGxlbWVudGF0aW9uLiBJdCdzIG11Y2gsCj4+bXVjaCBiZXR0ZXIgdG8gdHVy biBkd2MzJ3MgaW1wbGVtZW50YXRpb24gaW50byBhIGdlbmVyaWMsIHJldXNhYmxlCj4+bGlicmFy eSBmdW5jdGlvbiB0aGVuIHNwaW5uaW5nIHlvdXIgb3duIGFzIGEgZHVwbGljYXRlZCBlZmZvcnQu Cj4gSSBhZ3JlZSwgCj4gSXQgd291bGQgYmUgbmljZSB0byBoYXZlIGEgc2V0IG9mIGRlY29kaW5n IGZ1bmN0aW9uICBpbiBzb21lIGdlbmVyaWMgbGlicmFyeS4gSXQgY291bGQgYmUgdXNlZCAKPiBh bHNvIGJ5IG90aGVyIGRyaXZlcnMuCj4gV2hvIHNob3VsZCBkbyB0aGlzID8KCnNpbmNlIHlvdSdy ZSB0aGUgZmlyc3QgdG8gcmV1c2UgaXQsIHRoZW4gaXQgc2hvdWxkIGJlIHlvdSA6LSkKCj4+PiAr c3RhdGljIGludCBjZG5zM19yZXFfZXAwX3NldF9jb25maWd1cmF0aW9uKHN0cnVjdCBjZG5zM19k ZXZpY2UgKnByaXZfZGV2LAo+Pj4gKwkJCQkJICAgc3RydWN0IHVzYl9jdHJscmVxdWVzdCAqY3Ry bF9yZXEpCj4+PiArewo+Pj4gKwllbnVtIHVzYl9kZXZpY2Vfc3RhdGUgZGV2aWNlX3N0YXRlID0g cHJpdl9kZXYtPmdhZGdldC5zdGF0ZTsKPj4+ICsJc3RydWN0IGNkbnMzX2VuZHBvaW50ICpwcml2 X2VwOwo+Pj4gKwl1MzIgY29uZmlnID0gbGUxNl90b19jcHUoY3RybF9yZXEtPndWYWx1ZSk7Cj4+ PiArCWludCByZXN1bHQgPSAwOwo+Pj4gKwlpbnQgaTsKPj4+ICsKPj4+ICsJc3dpdGNoIChkZXZp Y2Vfc3RhdGUpIHsKPj4+ICsJY2FzZSBVU0JfU1RBVEVfQUREUkVTUzoKPj4+ICsJCS8qIENvbmZp Z3VyZSBub24tY29udHJvbCBFUHMgKi8KPj4+ICsJCWZvciAoaSA9IDA7IGkgPCBDRE5TM19FTkRQ T0lOVFNfTUFYX0NPVU5UOyBpKyspIHsKPj4+ICsJCQlwcml2X2VwID0gcHJpdl9kZXYtPmVwc1tp XTsKPj4+ICsJCQlpZiAoIXByaXZfZXApCj4+PiArCQkJCWNvbnRpbnVlOwo+Pj4gKwo+Pj4gKwkJ CWlmIChwcml2X2VwLT5mbGFncyAmIEVQX0NMQUlNRUQpCj4+PiArCQkJCWNkbnMzX2VwX2NvbmZp Zyhwcml2X2VwKTsKPj4+ICsJCX0KPj4+ICsKPj4+ICsJCXJlc3VsdCA9IGNkbnMzX2VwMF9kZWxl Z2F0ZV9yZXEocHJpdl9kZXYsIGN0cmxfcmVxKTsKPj4+ICsKPj4+ICsJCWlmIChyZXN1bHQpCj4+ PiArCQkJcmV0dXJuIHJlc3VsdDsKPj4+ICsKPj4+ICsJCWlmIChjb25maWcpIHsKPj4+ICsJCQlj ZG5zM19zZXRfaHdfY29uZmlndXJhdGlvbihwcml2X2Rldik7Cj4+PiArCQl9IGVsc2Ugewo+Pj4g KwkJCWNkbnMzX2dhZGdldF91bmNvbmZpZyhwcml2X2Rldik7Cj4+PiArCQkJdXNiX2dhZGdldF9z ZXRfc3RhdGUoJnByaXZfZGV2LT5nYWRnZXQsCj4+PiArCQkJCQkgICAgIFVTQl9TVEFURV9BRERS RVNTKTsKPj4KPj50aGlzIGlzIHdyb25nLiBJZiBhZGRyZXNzIGlzIHplcm8sIHN0YXRlIHNob3Vs ZCBiZSBkZWZhdWx0LCBub3QKPj5hZGRyZXNzZWQuIEFkZHJlc3NlZCB3b3VsZCBiZSB1c2VkIG9u IHRoZSBvdGhlciBicmFuY2ggaGVyZSwgd2hlbiB5b3UKPj5oYXZlIGEgbm9uLXplcm8gYWRkcmVz cwo+Cj4gSWYgYWRkcmVzcyBpcyB6ZXJvIHRoZW4gc3RhdGUgc2hvdWxkIGJlIFVTQl9TVEFURV9E RUZBVUxULiBEcml2ZXIKPiBlbnRlcnMgdG8gdGhpcyBicmFuY2ggb25seSBpZiBnYWRnZXQuc3Rh dGUgPSBVU0JfU1RBVEVfQUREUkVTUwo+IChhZGRyZXNzICE9IDApLiBUaGlzIHN0YXRlIGNhbiBi ZSBjaGFuZ2VkIGluIGNvbXBvc2l0ZS5jIGZpbGUgYWZ0ZXIKPiBkZWxlZ2F0aW5nIHJlcXVlc3Qg dG8gZ2FkZ2V0IGRyaXZlci4gQmVjYXVzZSB0aGlzIHN0YXRlIGNvdWxkIGJlCj4gY2hhbmdlZCBk cml2ZXIgc2ltcGxlIHJlc3RvcmUgVVNCX1NUQVRFX0FERFJFU1MgaWYgY29uZmlnID0gMC4KCm5l dmVybWluZCwgSSByZWFkIHRoaXMgYXMgYmVpbmcgdGhlIGhhbmRsZXIgZm9yIFNldCBBZGRyZXNz LiBUaGlzIGlzIFNldApDb25maWcsIGluc3RlYWQuCgo+Pj4gKwkJfQo+Pj4gKwkJYnJlYWs7Cj4+ PiArCWNhc2UgVVNCX1NUQVRFX0NPTkZJR1VSRUQ6Cj4+Cj4+d2hlcmUgZG8geW91IHNldCB0aGlz IHN0YXRlPwo+IEl0J3Mgc2V0IGluIHNldF9jb25maWcgaW4gY29tcG9zaXRlLmMgZmlsZS4gCgpp bmRlZWQKCj4+PiArCWNhc2UgVVNCX0RFVklDRV9URVNUX01PREU6Cj4+PiArCQlpZiAoc3RhdGUg IT0gVVNCX1NUQVRFX0NPTkZJR1VSRUQgfHwgc3BlZWQgPiBVU0JfU1BFRURfSElHSCkKPj4+ICsJ CQlyZXR1cm4gLUVJTlZBTDsKPj4+ICsKPj4+ICsJCXRtb2RlID0gbGUxNl90b19jcHUoY3RybC0+ d0luZGV4KTsKPj4+ICsKPj4+ICsJCWlmICghc2V0IHx8ICh0bW9kZSAmIDB4ZmYpICE9IDApCj4+ PiArCQkJcmV0dXJuIC1FSU5WQUw7Cj4+PiArCj4+PiArCQlzd2l0Y2ggKHRtb2RlID4+IDgpIHsK Pj4+ICsJCWNhc2UgVEVTVF9KOgo+Pj4gKwkJY2FzZSBURVNUX0s6Cj4+PiArCQljYXNlIFRFU1Rf U0UwX05BSzoKPj4+ICsJCWNhc2UgVEVTVF9QQUNLRVQ6Cj4+PiArCQkJY2RuczNfc2V0X3JlZ2lz dGVyX2JpdCgmcHJpdl9kZXYtPnJlZ3MtPnVzYl9jbWQsCj4+PiArCQkJCQkgICAgICAgVVNCX0NN RF9TVE1PREUgfAo+Pj4gKwkJCQkJICAgICAgIFVTQl9TVFNfVE1PREVfU0VMKHRtb2RlIC0gMSkp Owo+Pgo+PkknbSA5MCUgc3VyZSB0aGlzIHdvbid0IHdvcmsuIFRoZXJlJ3MgYSByZWFzb24gd2h5 IHdlIG9ubHkgZW50ZXIgdGhlCj4+cmVxdWVzdGVkIHRlc3QgbW9kZSBmcm9tIHN0YXR1cyBzdGFn ZS4gSG93IGhhdmUgeW91IHRlc3RlZCB0aGlzPwo+Cj4gRnJvbSBVU0Igc3BlYzoKPiAiVGhlIHRy YW5zaXRpb24gdG8gdGVzdCBtb2RlIG11c3QgYmUgY29tcGxldGUgbm8gbGF0ZXIgdGhhbiAzIG1z IGFmdGVyIHRoZSBjb21wbGV0aW9uIG9mIHRoZSBzdGF0dXMgc3RhZ2Ugb2YgdGhlCj4gcmVxdWVz dC4iCgpleGFjdGx5LCBfYWZ0ZXJfIGNvbXBsZXRpb24gb2Ygc3RhdHVzIHN0YWdlIDotKQoKPiBC dXQgSSBkb24ndCByZW1lbWJlciBhbnkgaXNzdWVzIHdpdGggdGhpcyB0ZXN0IG9uIG90aGVyIG91 cnMKPiBkcml2ZXJzLiBNYXliZSBzdGF0dXMgc3RhZ2UgaXMgYXJtZWQgaW4gdGhpcyBjYXNlIGJ5 IGNvbnRyb2xsZXIuIEkKPiBoYXZlIHRvIGNvbmZpcm0gaG93IGl0IHdvcmtzIHdpdGggaGFyZHdh cmUgdGVhbS4gIERyaXZlciBkb2Vzbid0IGtub3cKPiB3aGVuIHN0YXR1cyBzdGFnZSB3YXMgY29t cGxldGVkLiBXZSBkb24ndCBoYXZlIGFueSBldmVudCBvbiBzdGF0dXMKPiBzdGFnZSBjb21wbGV0 aW9uLiAgSSBoYXZlbid0IGNoZWNrZWQgaXQgeWV0IHdpdGggdGVzdGVyIG9uIHRoaXMKPiBkcml2 ZXIuCgpMZXQncyBjb25zaWRlciB0aGUgc2NlbmFyaW8gd2hlcmUgeW91J3JlIHJlcXVlc3Rpbmcg VGVzdF9QYWNrZXQgbW9kZS4gSWYKeW91IHN0YXJ0IHRyYW5zbWl0dGluZyB0aGUgdGVzdCBwYXR0 ZXJuIGZyb20gc2V0dXAgc3RhZ2UsIGhvdyBjYW4geW91CmV2ZW4gdHJhbnNtaXQgc3RhdHVzIHN0 YWdlPwoKPj4+ICt2b2lkIGNkbnMzX2dhZGdldF91bmNvbmZpZyhzdHJ1Y3QgY2RuczNfZGV2aWNl ICpwcml2X2RldikKPj4+ICt7Cj4+PiArCS8qIFJFU0VUIENPTkZJR1VSQVRJT04gKi8KPj4+ICsJ d3JpdGVsKFVTQl9DT05GX0NGR1JTVCwgJnByaXZfZGV2LT5yZWdzLT51c2JfY29uZik7Cj4+PiAr Cj4+PiArCWNkbnMzX2FsbG93X2VuYWJsZV9sMShwcml2X2RldiwgMCk7Cj4+PiArCXByaXZfZGV2 LT5od19jb25maWd1cmVkX2ZsYWcgPSAwOwo+Pj4gKwlwcml2X2Rldi0+b25jaGlwX21lbV9hbGxv Y2F0ZWRfc2l6ZSA9IDA7Cj4+Cj4+Y2xlYXIgYWxsIHRlc3QgbW9kZXM/IFJlc2V0IGVwMCBtYXhf cGFja2V0X3NpemUgdG8gNTEyPwo+IFRlc3QgbW9kZSBzaG91bGQgYmUgcmVzZXQgYXV0b21hdGlj YWxseSBvbiBleGl0LiAKCm9uIGV4aXQ/IFdoaWNoIGV4aXQ/IERpZCB5b3UgdGVzdCB0aGlzIG9u IFVTQiBSZXNldD8gRGlkIHlvdSBydW4gVVNCQ1YKb24gU3VwZXIgYW5kIEhpZ2ggc3BlZWQgd2l0 aCBhbnkgZ2FkZ2V0PwoKPiBXIGNhbid0IGNsZWFyIHRoaXMgbW9kZSBpbiByZWdpc3Rlci4gSXQn cyBXQUMgKHdyaXRlIG9ubHkgYW5kIGF1dG8gY2xlYXIpICBiaXQuCj4gVGhpcyBmdW5jdGlvbiBv bmx5IHJlc2V0IGVuZHBvaW50IGNvbmZpZ3VyYXRpb24gaW4gY29udHJvbGxlciByZWdpc3Rlci4g Cj4gRXAwIG1heF9wYWNrZXRfc2l6ZSBzaG91bGQgYmUgdW5jaGFuZ2VkIGhlcmUuICBFcDAgbWF4 X3BhY2tldCBpdCdzIHVwZGF0ZWQgb24gCj4gQ29ubmVjdC9EaXNjb25uZWN0L1Jlc2V0IGV2ZW50 cy4gIAoKcmlnaHQsIGFuZCB0aGlzIGlzIGNhbGxlZCBmb3IgYm90aCByZXNldCBhbmQgZGlzY29u bmVjdCAoc2VlIGJlbG93KS4gSWYKeW91J3JlIHRlbGxpbmcgbWUgdGhhdCB0ZXN0IG1vZGUgYW5k IGVwMCB3TWF4UGFja2V0U2l6ZSBpcyBoYW5kbGVkCmVsc2V3aGVyZSwgdGhhdCdzIGZpbmUuCgor CS8qIERpc2Nvbm5lY3Rpb24gZGV0ZWN0ZWQgKi8KKwlpZiAodXNiX2lzdHMgJiAoVVNCX0lTVFNf RElTMkkgfCBVU0JfSVNUU19ESVNJKSkgeworCQlpZiAocHJpdl9kZXYtPmdhZGdldF9kcml2ZXIg JiYKKwkJICAgIHByaXZfZGV2LT5nYWRnZXRfZHJpdmVyLT5kaXNjb25uZWN0KSB7CisJCQlzcGlu X3VubG9jaygmcHJpdl9kZXYtPmxvY2spOworCQkJcHJpdl9kZXYtPmdhZGdldF9kcml2ZXItPmRp c2Nvbm5lY3QoJnByaXZfZGV2LT5nYWRnZXQpOworCQkJc3Bpbl9sb2NrKCZwcml2X2Rldi0+bG9j ayk7CisJCX0KKworCQlwcml2X2Rldi0+Z2FkZ2V0LnNwZWVkID0gVVNCX1NQRUVEX1VOS05PV047 CisJCXVzYl9nYWRnZXRfc2V0X3N0YXRlKCZwcml2X2Rldi0+Z2FkZ2V0LCBVU0JfU1RBVEVfTk9U QVRUQUNIRUQpOworCQljZG5zM19nYWRnZXRfdW5jb25maWcocHJpdl9kZXYpOworCX0KKworCS8q IHJlc2V0Ki8KKwlpZiAodXNiX2lzdHMgJiAoVVNCX0lTVFNfVVdSRVNJIHwgVVNCX0lTVFNfVUhS RVNJIHwgVVNCX0lTVFNfVTJSRVNJKSkgeworCQkvKnJlYWQgYWdhaW4gdG8gY2hlY2sgdGhlIGFj dHVhbGwgc3BlZWQqLworCQlzcGVlZCA9IGNkbnMzX2dldF9zcGVlZChwcml2X2Rldik7CisJCXVz Yl9nYWRnZXRfc2V0X3N0YXRlKCZwcml2X2Rldi0+Z2FkZ2V0LCBVU0JfU1RBVEVfREVGQVVMVCk7 CisJCXByaXZfZGV2LT5nYWRnZXQuc3BlZWQgPSBzcGVlZDsKKwkJY2RuczNfZ2FkZ2V0X3VuY29u ZmlnKHByaXZfZGV2KTsKKwkJY2RuczNfZXAwX2NvbmZpZyhwcml2X2Rldik7CisJfQoKCj4gTWF5 YmUgdGhpcyBmdW5jdGlvbiBzaG91bGQgYmUgY2FsbGVkIGNkbnMzX2h3X3Jlc2V0X2Vwc19jb25m aWcuIAoKcGVyaGFwcwoKPiBJdCBkb2Vzbid0IHVuY29uZmlndXJlIHdob2xlIGdhZGdldCBidXQg b25seSByZXNldCBlbmRwb2ludHMgY29uZmlndXJhdGlvbiBrZXB0IGJ5IAo+IGNvbnRyb2xsZXIu Cj4gSSB3aWxsIGNoYW5nZSB0aGlzIGZ1bmN0aW9uLiAKCm9rCgo+Pj4gK3N0YXRpYyBpcnFyZXR1 cm5fdCBjZG5zM19kZXZpY2VfaXJxX2hhbmRsZXIoaW50IGlycSwgdm9pZCAqZGF0YSkKPj4+ICt7 Cj4+PiArCXN0cnVjdCBjZG5zM19kZXZpY2UgKnByaXZfZGV2Owo+Pj4gKwlzdHJ1Y3QgY2RuczMg KmNkbnMgPSBkYXRhOwo+Pj4gKwlpcnFyZXR1cm5fdCByZXQgPSBJUlFfTk9ORTsKPj4+ICsJdW5z aWduZWQgbG9uZyBmbGFnczsKPj4+ICsJdTMyIHJlZzsKPj4+ICsKPj4+ICsJcHJpdl9kZXYgPSBj ZG5zLT5nYWRnZXRfZGV2Owo+Pj4gKwlzcGluX2xvY2tfaXJxc2F2ZSgmcHJpdl9kZXYtPmxvY2ss IGZsYWdzKTsKPj4KPj55b3UncmUgYWxyZWFkeSBydW5uaW5nIGluIGhhcmRpcnEgY29udGV4dC4g V2h5IGRvIHlvdSBuZWVkIHRoaXMgbG9jayBhdAo+PmFsbD8gSSB3b3VsZCBiZSBiZXR0ZXIgdG8g dXNlIHRoZSBoYXJkaXJxIGhhbmRsZXIgdG8gbWFzayB5b3VyCj4+aW50ZXJydXB0cywgc28gdGhl eSBkb24ndCBmaXJlIGFnYWluLCB0aGVuIHVzZWQgdGhlIHRvcC1oYWxmIChzb2Z0aXJxKQo+Pmhh bmRsZXIgdG8gYWN0dWFsbHkgaGFuZGxlIHRoZSBpbnRlcnJ1cHRzLgo+Cj4gWWVzLCBzcGluX2xv Y2tfaXJxc2F2ZSBpcyBub3QgbmVjZXNzYXJ5IGhlcmUuIAo+Cj4gRG8geW91IG1lYW4gcmVwbGFj aW5nIGRldm1fcmVxdWVzdF9pcnEgd2l0aCBhIHJlcXVlc3RfdGhyZWFkZWRfaXJxID8KPiBJIGhh dmUgc2luZ2xlIGludGVycnVwdCBsaW5lIHNoYXJlZCBiZXR3ZWVuICBIb3N0LCBEcml2ZXIsIERS RC9PVEcuIAo+IEknbSBub3Qgc3VyZSBpZiBpdCB3aWxsIHdvcmsgbW9yZSBlZmZpY2llbnRseS4g CgpUaGUgd2hvbGUgaWRlYSBmb3IgcnVubmluZyB2ZXJ5IGxpdHRsZSBpbiBoYXJkaXJxIGNvbnRl eHQgaXMgdG8gZ2l2ZSB0aGUKc2NoZWR1bGVyIGEgY2hhbmNlIHRvIGRlY2lkZSB3aGF0IHNob3Vs ZCBydW4uIFRoaXMgaXMgaW1wb3J0YW50IHRvCnJlZHVjZSBsYXRlbmN5IHdoZW4gcnVubmluZyB3 aXRoIFJUIHBhdGNoc2V0IGFwcGxpZWQsIGZvcgpleGFtcGxlLiBIb3dldmVyLCBJJ2xsIGdpdmUg eW91IHRoYXQsIGl0J3MgYSBtaW5vciByZXF1aXJlbWVudC4gSXQncwpqdXN0IHRoYXQsIHRvIG1l LCBpdCdzIGEgc21hbGwgZGV0YWlsIHRoYXQncyBlYXN5IHRvIGltcGxlbWVudC4KCj4+PiArCS8q IGNoZWNrIFVTQiBkZXZpY2UgaW50ZXJydXB0ICovCj4+PiArCXJlZyA9IHJlYWRsKCZwcml2X2Rl di0+cmVncy0+dXNiX2lzdHMpOwo+Pj4gKwl3cml0ZWwocmVnLCAmcHJpdl9kZXYtPnJlZ3MtPnVz Yl9pc3RzKTsKPj4+ICsKPj4+ICsJaWYgKHJlZykgewo+Pj4gKwkJZGV2X2RiZyhwcml2X2Rldi0+ ZGV2LCAiSVJROiB1c2JfaXN0czogJTA4WFxuIiwgcmVnKTsKPj4KPj5JIHN0cm9uZ2x5IGFkdmlz ZSBhZ2FpbnN0IHVzaW5nIGRldl9kYmcoKSBmb3IgZGVidWdnaW5nLiBFdmVuIG1vcmUgc28KPj5p bnNpZGUgeW91ciBJUlEgaGFuZGxlci4KPiBPaywgSXQncyBub3QgbmVjZXNzYXJ5IGluIHRoaXMg cGxhY2UsIGVzcGVjaWFsbHksIHRoYXQgdGhlcmUgaXMgaW52b2tlZCB0cmFjZSBwb2ludCAKPiBp bnNpZGUgY2RuczNfY2hlY2tfdXNiX2ludGVycnVwdF9wcm9jZWVkIHdoaWNoIG1ha2VzIHRoZSBz YW1lLgoKb3ZlcmhlYWQuIFRyYWNlcG9pbnRzIGFyZSByZWFsbHksIHJlYWxseSBsb3cgb3Zlcmhl YWQuIFRoZSBzdHJpbmcKZGVjb2RpbmcgZG9lc24ndCBoYXBwZW4gdW50aWwgeW91IHJlYWQgdGhl IHRyYWNlIGZpbGUuCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felipe Balbi Subject: RE: [PATCH v1 2/2] usb:cdns3 Add Cadence USB3 DRD Driver Date: Wed, 12 Dec 2018 08:52:13 +0200 Message-ID: <87a7lbme3m.fsf@linux.intel.com> References: <1544445555-17325-1-git-send-email-pawell@cadence.com> <1544445555-17325-3-git-send-email-pawell@cadence.com> <87h8fkmfar.fsf@linux.intel.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 , "devicetree@vger.kernel.org" , Kishon Vijay Abraham I Cc: "gregkh@linuxfoundation.org" , "linux-usb@vger.kernel.org" , "rogerq@ti.com" , "linux-kernel@vger.kernel.org" , Alan Douglas , "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: >>> + cdns->phy =3D devm_phy_get(dev, "cdns3,usbphy"); >>> + if (IS_ERR(cdns->phy)) { >>> + ret =3D PTR_ERR(cdns->phy); >>> + if (ret =3D=3D -ENOSYS || ret =3D=3D -ENODEV) { >> >>Are you sure you can get ENOSYS here? Have you checked output of >>checkpatch --strict? >>-:852: WARNING: ENOSYS means 'invalid syscall nr' and nothing else > > Yes this error code can be returned by related to phy function if > CONFIG_GENERIC_PHY is disabled. > > I have seen this warning in output of checkpatch --strict . Kishon, seems like you shouldn't be using that error value. >>> + case USB_REQ_SET_ISOCH_DELAY: >>> + sprintf(str, "Set Isochronous Delay Delay: %d ns", wValue); >>> + break; >>> + default: >>> + sprintf(str, >>> + "SETUP BRT: %02x BR: %02x V: %04x I: %04x L: %04x\n", >>> + bRequestType, bRequest, >>> + wValue, wIndex, wLength); >>> + } >>> + >>> + return str; >>> +} >> >>All of these are a flat out copy of dwc3's implementation. It's much, >>much better to turn dwc3's implementation into a generic, reusable >>library function then spinning your own as a duplicated effort. > I agree,=20 > It would be nice to have a set of decoding function in some generic libr= ary. It could be used=20 > also by other drivers. > Who should do this ? since you're the first to reuse it, then it should be you :-) >>> +static int cdns3_req_ep0_set_configuration(struct cdns3_device *priv_d= ev, >>> + struct usb_ctrlrequest *ctrl_req) >>> +{ >>> + enum usb_device_state device_state =3D priv_dev->gadget.state; >>> + struct cdns3_endpoint *priv_ep; >>> + u32 config =3D le16_to_cpu(ctrl_req->wValue); >>> + int result =3D 0; >>> + int i; >>> + >>> + switch (device_state) { >>> + case USB_STATE_ADDRESS: >>> + /* Configure non-control EPs */ >>> + for (i =3D 0; i < CDNS3_ENDPOINTS_MAX_COUNT; i++) { >>> + priv_ep =3D priv_dev->eps[i]; >>> + if (!priv_ep) >>> + continue; >>> + >>> + if (priv_ep->flags & EP_CLAIMED) >>> + cdns3_ep_config(priv_ep); >>> + } >>> + >>> + result =3D cdns3_ep0_delegate_req(priv_dev, ctrl_req); >>> + >>> + if (result) >>> + return result; >>> + >>> + if (config) { >>> + cdns3_set_hw_configuration(priv_dev); >>> + } else { >>> + cdns3_gadget_unconfig(priv_dev); >>> + usb_gadget_set_state(&priv_dev->gadget, >>> + USB_STATE_ADDRESS); >> >>this is wrong. If address is zero, state should be default, not >>addressed. Addressed would be used on the other branch here, when you >>have a non-zero address > > If address is zero then state should be USB_STATE_DEFAULT. Driver > enters to this branch only if gadget.state =3D USB_STATE_ADDRESS > (address !=3D 0). This state can be changed in composite.c file after > delegating request to gadget driver. Because this state could be > changed driver simple restore USB_STATE_ADDRESS if config =3D 0. nevermind, I read this as being the handler for Set Address. This is Set Config, instead. >>> + } >>> + break; >>> + case USB_STATE_CONFIGURED: >> >>where do you set this state? > It's set in set_config in composite.c file.=20 indeed >>> + case USB_DEVICE_TEST_MODE: >>> + if (state !=3D USB_STATE_CONFIGURED || speed > USB_SPEED_HIGH) >>> + return -EINVAL; >>> + >>> + tmode =3D le16_to_cpu(ctrl->wIndex); >>> + >>> + if (!set || (tmode & 0xff) !=3D 0) >>> + return -EINVAL; >>> + >>> + switch (tmode >> 8) { >>> + case TEST_J: >>> + case TEST_K: >>> + case TEST_SE0_NAK: >>> + case TEST_PACKET: >>> + cdns3_set_register_bit(&priv_dev->regs->usb_cmd, >>> + USB_CMD_STMODE | >>> + USB_STS_TMODE_SEL(tmode - 1)); >> >>I'm 90% sure this won't work. There's a reason why we only enter the >>requested test mode from status stage. How have you tested this? > > From USB spec: > "The transition to test mode must be complete no later than 3 ms after th= e completion of the status stage of the > request." exactly, _after_ completion of status stage :-) > But I don't remember any issues with this test on other ours > drivers. Maybe status stage is armed in this case by controller. I > have to confirm how it works with hardware team. Driver doesn't know > when status stage was completed. We don't have any event on status > stage completion. I haven't checked it yet with tester on this > driver. Let's consider the scenario where you're requesting Test_Packet mode. If you start transmitting the test pattern from setup stage, how can you even transmit status stage? >>> +void cdns3_gadget_unconfig(struct cdns3_device *priv_dev) >>> +{ >>> + /* RESET CONFIGURATION */ >>> + writel(USB_CONF_CFGRST, &priv_dev->regs->usb_conf); >>> + >>> + cdns3_allow_enable_l1(priv_dev, 0); >>> + priv_dev->hw_configured_flag =3D 0; >>> + priv_dev->onchip_mem_allocated_size =3D 0; >> >>clear all test modes? Reset ep0 max_packet_size to 512? > Test mode should be reset automatically on exit.=20 on exit? Which exit? Did you test this on USB Reset? Did you run USBCV on Super and High speed with any gadget? > W can't clear this mode in register. It's WAC (write only and auto clear)= bit. > This function only reset endpoint configuration in controller register.=20 > Ep0 max_packet_size should be unchanged here. Ep0 max_packet it's update= d on=20 > Connect/Disconnect/Reset events.=20=20 right, and this is called for both reset and disconnect (see below). If you're telling me that test mode and ep0 wMaxPacketSize is handled elsewhere, that's fine. + /* Disconnection detected */ + if (usb_ists & (USB_ISTS_DIS2I | USB_ISTS_DISI)) { + if (priv_dev->gadget_driver && + priv_dev->gadget_driver->disconnect) { + spin_unlock(&priv_dev->lock); + priv_dev->gadget_driver->disconnect(&priv_dev->gadget); + spin_lock(&priv_dev->lock); + } + + priv_dev->gadget.speed =3D USB_SPEED_UNKNOWN; + usb_gadget_set_state(&priv_dev->gadget, USB_STATE_NOTATTACHED); + cdns3_gadget_unconfig(priv_dev); + } + + /* reset*/ + if (usb_ists & (USB_ISTS_UWRESI | USB_ISTS_UHRESI | USB_ISTS_U2RESI)) { + /*read again to check the actuall speed*/ + speed =3D cdns3_get_speed(priv_dev); + usb_gadget_set_state(&priv_dev->gadget, USB_STATE_DEFAULT); + priv_dev->gadget.speed =3D speed; + cdns3_gadget_unconfig(priv_dev); + cdns3_ep0_config(priv_dev); + } > Maybe this function should be called cdns3_hw_reset_eps_config.=20 perhaps > It doesn't unconfigure whole gadget but only reset endpoints configuratio= n kept by=20 > controller. > I will change this function.=20 ok >>> +static irqreturn_t cdns3_device_irq_handler(int irq, void *data) >>> +{ >>> + struct cdns3_device *priv_dev; >>> + struct cdns3 *cdns =3D data; >>> + irqreturn_t ret =3D IRQ_NONE; >>> + unsigned long flags; >>> + u32 reg; >>> + >>> + priv_dev =3D cdns->gadget_dev; >>> + spin_lock_irqsave(&priv_dev->lock, flags); >> >>you're already running in hardirq context. Why do you need this lock at >>all? I would be better to use the hardirq handler to mask your >>interrupts, so they don't fire again, then used the top-half (softirq) >>handler to actually handle the interrupts. > > Yes, spin_lock_irqsave is not necessary here.=20 > > Do you mean replacing devm_request_irq with a request_threaded_irq ? > I have single interrupt line shared between Host, Driver, DRD/OTG.=20 > I'm not sure if it will work more efficiently.=20 The whole idea for running very little in hardirq context is to give the scheduler a chance to decide what should run. This is important to reduce latency when running with RT patchset applied, for example. However, I'll give you that, it's a minor requirement. It's just that, to me, it's a small detail that's easy to implement. >>> + /* check USB device interrupt */ >>> + reg =3D readl(&priv_dev->regs->usb_ists); >>> + writel(reg, &priv_dev->regs->usb_ists); >>> + >>> + if (reg) { >>> + dev_dbg(priv_dev->dev, "IRQ: usb_ists: %08X\n", reg); >> >>I strongly advise against using dev_dbg() for debugging. Even more so >>inside your IRQ handler. > Ok, It's not necessary in this place, especially, that there is invoked t= race point=20 > inside cdns3_check_usb_interrupt_proceed which makes the same. overhead. Tracepoints are really, really low overhead. The string decoding doesn't happen until you read the trace file. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlwQsB8ACgkQzL64meEa mQZTFBAAzCz5Xjms6g/VgCfwqihk5jT43+4N333dW+KzDW9DpW7FMhEDcMhRt2eO E39KB6wIKE9on6XiNppRkYG9XNzp7MBgLfF74/0NWEfmorsNUUrh1d3xRjXMJMGt woETX8vC10MNq4naAjJMEBqvnFJ3qbjoTZcAVUosVXgVO3F2pxT+lprRg/nN3Uge NEj0tHoFquHUIilw4ySQk4TtJM4+4w5bgusjbOLFqZdnNdpeLrwzksXvjP5Quyir PTYWeP7uFfws/qI2LZW+2igwz/MSNW8cAG7eR28cr+chZaOdogqFoj+6mBlljTJG XQd5jNvZQp4dlD98ZuYkoNVCprGf7O6tgU/Hc7/4Nls9TK1wVzUWj3Vzyep20pEg PRTrA83Qv63zTgIPYntMZ0bmP1i48Vj2gmSrMKjbQ0FlGgEP84pGwJ3SrsuKTPPV HVzi0HfJxGhZKaorKBB3B/isazgBGhxUegM3Jj5P4Cezb2nF2mMRp0ASu8n5jVMy jJN1RHHtqCwSUGxYN7f9BFfrUiLl41ususK5LDGqn07gBRXWq/XM+6SMPWQopGng KRjOEt2pTYsp24eFIBXdTo/49hO9s+JXYiMTI1fUJC8FK4x8Ug/pl+CUyrDArQtE 3JsUi55d83LWtKg9f799oYtx9dV28/aFRCFlJbM34uGmahnodu8= =JcCE -----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.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_PASS,T_MIXED_ES,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 2E609C04EB8 for ; Wed, 12 Dec 2018 06:52:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C7F1D20851 for ; Wed, 12 Dec 2018 06:52:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1544597550; bh=Gw9yZwcerAdqMANRjGQ/u3XhYBF2LO/puCDzmFnoiRo=; h=From:To:Cc:Subject:In-Reply-To:References:Date:List-ID:From; b=VpkLR7m5KDv+y6ddMImjzVFsCimP06e1xx07+KYvlJMwB3rUNzCLOMyw/lyKg52iI mYZpKph7CnaJR7eu4MXCCHOtf0WB1oMq5kCKsQ3BgRBGGh54CRRsMhs03FGB4dgJ6u 0aUvEZOvoSWTbgEX6fzlAK6p2frrPsCP7F9jO6DA= DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C7F1D20851 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726551AbeLLGw2 (ORCPT ); Wed, 12 Dec 2018 01:52:28 -0500 Received: from mga17.intel.com ([192.55.52.151]:36004 "EHLO mga17.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726242AbeLLGw2 (ORCPT ); Wed, 12 Dec 2018 01:52:28 -0500 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Dec 2018 22:52:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,343,1539673200"; d="asc'?scan'208";a="109701464" Received: from pipin.fi.intel.com (HELO localhost) ([10.237.72.175]) by orsmga003.jf.intel.com with ESMTP; 11 Dec 2018 22:52:23 -0800 From: Felipe Balbi To: Pawel Laszczak , "devicetree\@vger.kernel.org" , Kishon Vijay Abraham I Cc: "gregkh\@linuxfoundation.org" , "linux-usb\@vger.kernel.org" , "rogerq\@ti.com" , "linux-kernel\@vger.kernel.org" , Alan Douglas , "jbergsagel\@ti.com" , "nsekhar\@ti.com" , "nm\@ti.com" , Suresh Punnoose , "peter.chen\@nxp.com" , Pawel Jez , Rahul Kumar Subject: RE: [PATCH v1 2/2] usb:cdns3 Add Cadence USB3 DRD Driver In-Reply-To: References: <1544445555-17325-1-git-send-email-pawell@cadence.com> <1544445555-17325-3-git-send-email-pawell@cadence.com> <87h8fkmfar.fsf@linux.intel.com> Date: Wed, 12 Dec 2018 08:52:13 +0200 Message-ID: <87a7lbme3m.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: >>> + cdns->phy =3D devm_phy_get(dev, "cdns3,usbphy"); >>> + if (IS_ERR(cdns->phy)) { >>> + ret =3D PTR_ERR(cdns->phy); >>> + if (ret =3D=3D -ENOSYS || ret =3D=3D -ENODEV) { >> >>Are you sure you can get ENOSYS here? Have you checked output of >>checkpatch --strict? >>-:852: WARNING: ENOSYS means 'invalid syscall nr' and nothing else > > Yes this error code can be returned by related to phy function if > CONFIG_GENERIC_PHY is disabled. > > I have seen this warning in output of checkpatch --strict . Kishon, seems like you shouldn't be using that error value. >>> + case USB_REQ_SET_ISOCH_DELAY: >>> + sprintf(str, "Set Isochronous Delay Delay: %d ns", wValue); >>> + break; >>> + default: >>> + sprintf(str, >>> + "SETUP BRT: %02x BR: %02x V: %04x I: %04x L: %04x\n", >>> + bRequestType, bRequest, >>> + wValue, wIndex, wLength); >>> + } >>> + >>> + return str; >>> +} >> >>All of these are a flat out copy of dwc3's implementation. It's much, >>much better to turn dwc3's implementation into a generic, reusable >>library function then spinning your own as a duplicated effort. > I agree,=20 > It would be nice to have a set of decoding function in some generic libr= ary. It could be used=20 > also by other drivers. > Who should do this ? since you're the first to reuse it, then it should be you :-) >>> +static int cdns3_req_ep0_set_configuration(struct cdns3_device *priv_d= ev, >>> + struct usb_ctrlrequest *ctrl_req) >>> +{ >>> + enum usb_device_state device_state =3D priv_dev->gadget.state; >>> + struct cdns3_endpoint *priv_ep; >>> + u32 config =3D le16_to_cpu(ctrl_req->wValue); >>> + int result =3D 0; >>> + int i; >>> + >>> + switch (device_state) { >>> + case USB_STATE_ADDRESS: >>> + /* Configure non-control EPs */ >>> + for (i =3D 0; i < CDNS3_ENDPOINTS_MAX_COUNT; i++) { >>> + priv_ep =3D priv_dev->eps[i]; >>> + if (!priv_ep) >>> + continue; >>> + >>> + if (priv_ep->flags & EP_CLAIMED) >>> + cdns3_ep_config(priv_ep); >>> + } >>> + >>> + result =3D cdns3_ep0_delegate_req(priv_dev, ctrl_req); >>> + >>> + if (result) >>> + return result; >>> + >>> + if (config) { >>> + cdns3_set_hw_configuration(priv_dev); >>> + } else { >>> + cdns3_gadget_unconfig(priv_dev); >>> + usb_gadget_set_state(&priv_dev->gadget, >>> + USB_STATE_ADDRESS); >> >>this is wrong. If address is zero, state should be default, not >>addressed. Addressed would be used on the other branch here, when you >>have a non-zero address > > If address is zero then state should be USB_STATE_DEFAULT. Driver > enters to this branch only if gadget.state =3D USB_STATE_ADDRESS > (address !=3D 0). This state can be changed in composite.c file after > delegating request to gadget driver. Because this state could be > changed driver simple restore USB_STATE_ADDRESS if config =3D 0. nevermind, I read this as being the handler for Set Address. This is Set Config, instead. >>> + } >>> + break; >>> + case USB_STATE_CONFIGURED: >> >>where do you set this state? > It's set in set_config in composite.c file.=20 indeed >>> + case USB_DEVICE_TEST_MODE: >>> + if (state !=3D USB_STATE_CONFIGURED || speed > USB_SPEED_HIGH) >>> + return -EINVAL; >>> + >>> + tmode =3D le16_to_cpu(ctrl->wIndex); >>> + >>> + if (!set || (tmode & 0xff) !=3D 0) >>> + return -EINVAL; >>> + >>> + switch (tmode >> 8) { >>> + case TEST_J: >>> + case TEST_K: >>> + case TEST_SE0_NAK: >>> + case TEST_PACKET: >>> + cdns3_set_register_bit(&priv_dev->regs->usb_cmd, >>> + USB_CMD_STMODE | >>> + USB_STS_TMODE_SEL(tmode - 1)); >> >>I'm 90% sure this won't work. There's a reason why we only enter the >>requested test mode from status stage. How have you tested this? > > From USB spec: > "The transition to test mode must be complete no later than 3 ms after th= e completion of the status stage of the > request." exactly, _after_ completion of status stage :-) > But I don't remember any issues with this test on other ours > drivers. Maybe status stage is armed in this case by controller. I > have to confirm how it works with hardware team. Driver doesn't know > when status stage was completed. We don't have any event on status > stage completion. I haven't checked it yet with tester on this > driver. Let's consider the scenario where you're requesting Test_Packet mode. If you start transmitting the test pattern from setup stage, how can you even transmit status stage? >>> +void cdns3_gadget_unconfig(struct cdns3_device *priv_dev) >>> +{ >>> + /* RESET CONFIGURATION */ >>> + writel(USB_CONF_CFGRST, &priv_dev->regs->usb_conf); >>> + >>> + cdns3_allow_enable_l1(priv_dev, 0); >>> + priv_dev->hw_configured_flag =3D 0; >>> + priv_dev->onchip_mem_allocated_size =3D 0; >> >>clear all test modes? Reset ep0 max_packet_size to 512? > Test mode should be reset automatically on exit.=20 on exit? Which exit? Did you test this on USB Reset? Did you run USBCV on Super and High speed with any gadget? > W can't clear this mode in register. It's WAC (write only and auto clear)= bit. > This function only reset endpoint configuration in controller register.=20 > Ep0 max_packet_size should be unchanged here. Ep0 max_packet it's update= d on=20 > Connect/Disconnect/Reset events.=20=20 right, and this is called for both reset and disconnect (see below). If you're telling me that test mode and ep0 wMaxPacketSize is handled elsewhere, that's fine. + /* Disconnection detected */ + if (usb_ists & (USB_ISTS_DIS2I | USB_ISTS_DISI)) { + if (priv_dev->gadget_driver && + priv_dev->gadget_driver->disconnect) { + spin_unlock(&priv_dev->lock); + priv_dev->gadget_driver->disconnect(&priv_dev->gadget); + spin_lock(&priv_dev->lock); + } + + priv_dev->gadget.speed =3D USB_SPEED_UNKNOWN; + usb_gadget_set_state(&priv_dev->gadget, USB_STATE_NOTATTACHED); + cdns3_gadget_unconfig(priv_dev); + } + + /* reset*/ + if (usb_ists & (USB_ISTS_UWRESI | USB_ISTS_UHRESI | USB_ISTS_U2RESI)) { + /*read again to check the actuall speed*/ + speed =3D cdns3_get_speed(priv_dev); + usb_gadget_set_state(&priv_dev->gadget, USB_STATE_DEFAULT); + priv_dev->gadget.speed =3D speed; + cdns3_gadget_unconfig(priv_dev); + cdns3_ep0_config(priv_dev); + } > Maybe this function should be called cdns3_hw_reset_eps_config.=20 perhaps > It doesn't unconfigure whole gadget but only reset endpoints configuratio= n kept by=20 > controller. > I will change this function.=20 ok >>> +static irqreturn_t cdns3_device_irq_handler(int irq, void *data) >>> +{ >>> + struct cdns3_device *priv_dev; >>> + struct cdns3 *cdns =3D data; >>> + irqreturn_t ret =3D IRQ_NONE; >>> + unsigned long flags; >>> + u32 reg; >>> + >>> + priv_dev =3D cdns->gadget_dev; >>> + spin_lock_irqsave(&priv_dev->lock, flags); >> >>you're already running in hardirq context. Why do you need this lock at >>all? I would be better to use the hardirq handler to mask your >>interrupts, so they don't fire again, then used the top-half (softirq) >>handler to actually handle the interrupts. > > Yes, spin_lock_irqsave is not necessary here.=20 > > Do you mean replacing devm_request_irq with a request_threaded_irq ? > I have single interrupt line shared between Host, Driver, DRD/OTG.=20 > I'm not sure if it will work more efficiently.=20 The whole idea for running very little in hardirq context is to give the scheduler a chance to decide what should run. This is important to reduce latency when running with RT patchset applied, for example. However, I'll give you that, it's a minor requirement. It's just that, to me, it's a small detail that's easy to implement. >>> + /* check USB device interrupt */ >>> + reg =3D readl(&priv_dev->regs->usb_ists); >>> + writel(reg, &priv_dev->regs->usb_ists); >>> + >>> + if (reg) { >>> + dev_dbg(priv_dev->dev, "IRQ: usb_ists: %08X\n", reg); >> >>I strongly advise against using dev_dbg() for debugging. Even more so >>inside your IRQ handler. > Ok, It's not necessary in this place, especially, that there is invoked t= race point=20 > inside cdns3_check_usb_interrupt_proceed which makes the same. overhead. Tracepoints are really, really low overhead. The string decoding doesn't happen until you read the trace file. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlwQsB8ACgkQzL64meEa mQZTFBAAzCz5Xjms6g/VgCfwqihk5jT43+4N333dW+KzDW9DpW7FMhEDcMhRt2eO E39KB6wIKE9on6XiNppRkYG9XNzp7MBgLfF74/0NWEfmorsNUUrh1d3xRjXMJMGt woETX8vC10MNq4naAjJMEBqvnFJ3qbjoTZcAVUosVXgVO3F2pxT+lprRg/nN3Uge NEj0tHoFquHUIilw4ySQk4TtJM4+4w5bgusjbOLFqZdnNdpeLrwzksXvjP5Quyir PTYWeP7uFfws/qI2LZW+2igwz/MSNW8cAG7eR28cr+chZaOdogqFoj+6mBlljTJG XQd5jNvZQp4dlD98ZuYkoNVCprGf7O6tgU/Hc7/4Nls9TK1wVzUWj3Vzyep20pEg PRTrA83Qv63zTgIPYntMZ0bmP1i48Vj2gmSrMKjbQ0FlGgEP84pGwJ3SrsuKTPPV HVzi0HfJxGhZKaorKBB3B/isazgBGhxUegM3Jj5P4Cezb2nF2mMRp0ASu8n5jVMy jJN1RHHtqCwSUGxYN7f9BFfrUiLl41ususK5LDGqn07gBRXWq/XM+6SMPWQopGng KRjOEt2pTYsp24eFIBXdTo/49hO9s+JXYiMTI1fUJC8FK4x8Ug/pl+CUyrDArQtE 3JsUi55d83LWtKg9f799oYtx9dV28/aFRCFlJbM34uGmahnodu8= =JcCE -----END PGP SIGNATURE----- --=-=-=--