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: [RFC,v2,08/15] usb:cdns3: Implements device operations part of the API From: Roger Quadros Message-Id: <5BFE8883.7090802@ti.com> Date: Wed, 28 Nov 2018 14:22:27 +0200 To: Pawel Laszczak , devicetree@vger.kernel.org Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, adouglas@cadence.com, jbergsagel@ti.com, nsekhar@ti.com, nm@ti.com, sureshp@cadence.com, peter.chen@nxp.com, pjez@cadence.com, kurahul@cadence.com, Felipe Balbi List-ID: T24gMTgvMTEvMTggMTI6MDksIFBhd2VsIExhc3pjemFrIHdyb3RlOgo+IFBhdGNoIGFkZHMgaW1w bGVtZW50YXRpb24gY2FsbGJhY2sgZnVuY3Rpb24gZGVmaW5lZCBpbgo+IHVzYl9nYWRnZXRfb3Bz IG9iamVjdC4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBQYXdlbCBMYXN6Y3phayA8cGF3ZWxsQGNhZGVu Y2UuY29tPgo+IC0tLQo+ICBkcml2ZXJzL3VzYi9jZG5zMy9nYWRnZXQuYyB8IDI0OSArKysrKysr KysrKysrKysrKysrKysrKysrKysrKysrKysrKystCj4gIDEgZmlsZSBjaGFuZ2VkLCAyNDcgaW5z ZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy91c2Iv Y2RuczMvZ2FkZ2V0LmMgYi9kcml2ZXJzL3VzYi9jZG5zMy9nYWRnZXQuYwo+IGluZGV4IDM3NmI2 OGIxM2QxYi4uNzAyYTA1ZmFhNjY0IDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvdXNiL2NkbnMzL2dh ZGdldC5jCj4gKysrIGIvZHJpdmVycy91c2IvY2RuczMvZ2FkZ2V0LmMKPiBAQCAtMTcsNiArMTcs MzYgQEAKPiAgI2luY2x1ZGUgImdhZGdldC1leHBvcnQuaCIKPiAgI2luY2x1ZGUgImdhZGdldC5o Igo+ICAKPiArLyoqCj4gKyAqIGNkbnMzX2hhbmRzaGFrZSAtIHNwaW4gcmVhZGluZyAgdW50aWwg aGFuZHNoYWtlIGNvbXBsZXRlcyBvciBmYWlscwo+ICsgKiBAcHRyOiBhZGRyZXNzIG9mIGRldmlj ZSBjb250cm9sbGVyIHJlZ2lzdGVyIHRvIGJlIHJlYWQKPiArICogQG1hc2s6IGJpdHMgdG8gbG9v ayBhdCBpbiByZXN1bHQgb2YgcmVhZAo+ICsgKiBAZG9uZTogdmFsdWUgb2YgdGhvc2UgYml0cyB3 aGVuIGhhbmRzaGFrZSBzdWNjZWVkcwo+ICsgKiBAdXNlYzogdGltZW91dCBpbiBtaWNyb3NlY29u ZHMKPiArICoKPiArICogUmV0dXJucyBuZWdhdGl2ZSBlcnJubywgb3IgemVybyBvbiBzdWNjZXNz Cj4gKyAqCj4gKyAqIFN1Y2Nlc3MgaGFwcGVucyB3aGVuIHRoZSAibWFzayIgYml0cyBoYXZlIHRo ZSBzcGVjaWZpZWQgdmFsdWUgKGhhcmR3YXJlCj4gKyAqIGhhbmRzaGFrZSBkb25lKS4gVGhlcmUg YXJlIHR3byBmYWlsdXJlIG1vZGVzOiAidXNlYyIgaGF2ZSBwYXNzZWQgKG1ham9yCj4gKyAqIGhh cmR3YXJlIGZsYWtlb3V0KSwgb3IgdGhlIHJlZ2lzdGVyIHJlYWRzIGFzIGFsbC1vbmVzIChoYXJk d2FyZSByZW1vdmVkKS4KPiArICovCj4gK2ludCBjZG5zM19oYW5kc2hha2Uodm9pZCBfX2lvbWVt ICpwdHIsIHUzMiBtYXNrLCB1MzIgZG9uZSwgaW50IHVzZWMpCj4gK3sKPiArCXUzMglyZXN1bHQ7 Cj4gKwo+ICsJZG8gewo+ICsJCXJlc3VsdCA9IHJlYWRsKHB0cik7Cj4gKwkJaWYgKHJlc3VsdCA9 PSB+KHUzMikwKQkvKiBjYXJkIHJlbW92ZWQgKi8KPiArCQkJcmV0dXJuIC1FTk9ERVY7CgpJcyB0 aGlzIGFwcGxpY2FibGUgdG8gYWxsIHJlZ2lzdGVycz8KV2hhdCBpcyBtZWFudCBieSBjYXJkIHJl bW92ZWQ/IFdlJ3JlIG5vdCBjb25uZWN0ZWQgdG8gaG9zdD8KCmhvdyBkb2VzIEVQIHJlc2V0IGJl aGF2ZSB3aGVuIHRoZXJlIGlzIG5vIFVTQiBjb25uZWN0aW9uPwoKPiArCQlyZXN1bHQgJj0gbWFz azsKPiArCQlpZiAocmVzdWx0ID09IGRvbmUpCj4gKwkJCXJldHVybiAwOwo+ICsJCXVkZWxheSgx KTsKPiArCQl1c2VjLS07Cj4gKwl9IHdoaWxlICh1c2VjID4gMCk7Cj4gKwlyZXR1cm4gLUVUSU1F RE9VVDsKPiArfQo+ICsKPiAgLyoqCj4gICAqIGNkbnMzX3NldF9yZWdpc3Rlcl9iaXQgLSBzZXQg Yml0IGluIGdpdmVuIHJlZ2lzdGVyLgo+ICAgKiBAcHRyOiBhZGRyZXNzIG9mIGRldmljZSBjb250 cm9sbGVyIHJlZ2lzdGVyIHRvIGJlIHJlYWQgYW5kIGNoYW5nZWQKPiBAQCAtNDMsNiArNzMsMjUg QEAgdm9pZCBjZG5zM19zZWxlY3RfZXAoc3RydWN0IGNkbnMzX2RldmljZSAqcHJpdl9kZXYsIHUz MiBlcCkKPiAgCXdyaXRlbChlcCwgJnByaXZfZGV2LT5yZWdzLT5lcF9zZWwpOwo+ICB9Cj4gIAo+ ICtzdGF0aWMgdm9pZCBjZG5zM19mcmVlX3RyYl9wb29sKHN0cnVjdCBjZG5zM19lbmRwb2ludCAq cHJpdl9lcCkKPiArewo+ICsJc3RydWN0IGNkbnMzX2RldmljZSAqcHJpdl9kZXYgPSBwcml2X2Vw LT5jZG5zM19kZXY7Cj4gKwo+ICsJaWYgKHByaXZfZXAtPnRyYl9wb29sKSB7Cj4gKwkJZG1hX2Zy ZWVfY29oZXJlbnQocHJpdl9kZXYtPnN5c2RldiwKPiArCQkJCSAgVFJCX1JJR05fU0laRSwKPiAr CQkJCSAgcHJpdl9lcC0+dHJiX3Bvb2wsIHByaXZfZXAtPnRyYl9wb29sX2RtYSk7Cj4gKwkJcHJp dl9lcC0+dHJiX3Bvb2wgPSBOVUxMOwo+ICsJfQo+ICsKPiArCWlmIChwcml2X2VwLT5hbGlnbmVk X2J1ZmYpIHsKPiArCQlkbWFfZnJlZV9jb2hlcmVudChwcml2X2Rldi0+c3lzZGV2LCBDRE5TM19V TkFMSUdORURfQlVGX1NJWkUsCj4gKwkJCQkgIHByaXZfZXAtPmFsaWduZWRfYnVmZiwKPiArCQkJ CSAgcHJpdl9lcC0+YWxpZ25lZF9kbWFfYWRkcik7Cj4gKwkJcHJpdl9lcC0+YWxpZ25lZF9idWZm ID0gTlVMTDsKPiArCX0KPiArfQo+ICsKPiAgLyoqCj4gICAqIGNkbnMzX2lycV9oYW5kbGVyIC0g aXJxIGxpbmUgaW50ZXJydXB0IGhhbmRsZXIKPiAgICogQGNkbnM6IGNkbnMzIGluc3RhbmNlCj4g QEAgLTU4LDYgKzEwNywxMTQgQEAgc3RhdGljIGlycXJldHVybl90IGNkbnMzX2lycV9oYW5kbGVy X3RocmVhZChzdHJ1Y3QgY2RuczMgKmNkbnMpCj4gIAlyZXR1cm4gcmV0Owo+ICB9Cj4gIAo+ICsv KiBGaW5kIGNvcnJlY3QgZGlyZWN0aW9uIGZvciBIVyBlbmRwb2ludCBhY2NvcmRpbmcgdG8gZGVz Y3JpcHRpb24gKi8KPiArc3RhdGljIGludCBjZG5zM19lcF9kaXJfaXNfY29ycmVjdChzdHJ1Y3Qg dXNiX2VuZHBvaW50X2Rlc2NyaXB0b3IgKmRlc2MsCj4gKwkJCQkgICBzdHJ1Y3QgY2RuczNfZW5k cG9pbnQgKnByaXZfZXApCj4gK3sKPiArCXJldHVybiAocHJpdl9lcC0+ZW5kcG9pbnQuY2Fwcy5k aXJfaW4gJiYgdXNiX2VuZHBvaW50X2Rpcl9pbihkZXNjKSkgfHwKPiArCSAgICAgICAocHJpdl9l cC0+ZW5kcG9pbnQuY2Fwcy5kaXJfb3V0ICYmIHVzYl9lbmRwb2ludF9kaXJfb3V0KGRlc2MpKTsK PiArfQo+ICsKPiArc3RhdGljIHN0cnVjdCBjZG5zM19lbmRwb2ludCAqY2RuczNfZmluZF9hdmFp bGFibGVfc3NfZXAoc3RydWN0IGNkbnMzX2RldmljZSAqcHJpdl9kZXYsCj4gKwkJCQkJCQkgc3Ry dWN0IHVzYl9lbmRwb2ludF9kZXNjcmlwdG9yICpkZXNjKQoKd2h5IGlzIHRoaXMgZnVuY3Rpb24g Y2FsbGVkIHNzX2VwPyBUaGlzIGRvZXNuJ3Qgc2VlbSBsaWtlIG9ubHkgZm9yIHN1cGVyc3BlZWQg ZW5kcG9pbnRzLgoKPiArewo+ICsJc3RydWN0IHVzYl9lcCAqZXA7Cj4gKwlzdHJ1Y3QgY2RuczNf ZW5kcG9pbnQgKnByaXZfZXA7Cj4gKwo+ICsJbGlzdF9mb3JfZWFjaF9lbnRyeShlcCwgJnByaXZf ZGV2LT5nYWRnZXQuZXBfbGlzdCwgZXBfbGlzdCkgewo+ICsJCXVuc2lnbmVkIGxvbmcgbnVtOwo+ ICsJCWludCByZXQ7Cj4gKwkJLyogZXAgbmFtZSBwYXR0ZXJuIGxpa2VzIGVwWGluIG9yIGVwWG91 dCAqLwo+ICsJCWNoYXIgY1syXSA9IHtlcC0+bmFtZVsyXSwgJ1wwJ307Cj4gKwo+ICsJCXJldCA9 IGtzdHJ0b3VsKGMsIDEwLCAmbnVtKTsKPiArCQlpZiAocmV0KQo+ICsJCQlyZXR1cm4gRVJSX1BU UihyZXQpOwo+ICsKPiArCQlwcml2X2VwID0gZXBfdG9fY2RuczNfZXAoZXApOwo+ICsJCWlmIChj ZG5zM19lcF9kaXJfaXNfY29ycmVjdChkZXNjLCBwcml2X2VwKSkgewo+ICsJCQlpZiAoIShwcml2 X2VwLT5mbGFncyAmIEVQX1VTRUQpKSB7Cj4gKwkJCQlwcml2X2VwLT5udW0gID0gbnVtOwo+ICsJ CQkJcHJpdl9lcC0+ZmxhZ3MgfD0gRVBfVVNFRDsKPiArCQkJCXJldHVybiBwcml2X2VwOwo+ICsJ CQl9Cj4gKwkJfQo+ICsJfQo+ICsJcmV0dXJuIEVSUl9QVFIoLUVOT0VOVCk7Cj4gK30KPiArCj4g K3N0YXRpYyBzdHJ1Y3QgdXNiX2VwICpjZG5zM19nYWRnZXRfbWF0Y2hfZXAoc3RydWN0IHVzYl9n YWRnZXQgKmdhZGdldCwKPiArCQkJCQkgICAgc3RydWN0IHVzYl9lbmRwb2ludF9kZXNjcmlwdG9y ICpkZXNjLAo+ICsJCQkJCSAgICBzdHJ1Y3QgdXNiX3NzX2VwX2NvbXBfZGVzY3JpcHRvciAqY29t cF9kZXNjKQo+ICt7Cj4gKwlzdHJ1Y3QgY2RuczNfZGV2aWNlICpwcml2X2RldiA9IGdhZGdldF90 b19jZG5zM19kZXZpY2UoZ2FkZ2V0KTsKPiArCXN0cnVjdCBjZG5zM19lbmRwb2ludCAqcHJpdl9l cDsKPiArCXVuc2lnbmVkIGxvbmcgZmxhZ3M7Cj4gKwo+ICsJcHJpdl9lcCA9IGNkbnMzX2ZpbmRf YXZhaWxhYmxlX3NzX2VwKHByaXZfZGV2LCBkZXNjKTsKPiArCWlmIChJU19FUlIocHJpdl9lcCkp IHsKPiArCQlkZXZfZXJyKCZwcml2X2Rldi0+ZGV2LCAibm8gYXZhaWxhYmxlIGVwXG4iKTsKPiAr CQlyZXR1cm4gTlVMTDsKPiArCX0KPiArCj4gKwlkZXZfZGJnKCZwcml2X2Rldi0+ZGV2LCAibWF0 Y2ggZW5kcG9pbnQ6ICVzXG4iLCBwcml2X2VwLT5uYW1lKTsKPiArCj4gKwlzcGluX2xvY2tfaXJx c2F2ZSgmcHJpdl9kZXYtPmxvY2ssIGZsYWdzKTsKPiArCXByaXZfZXAtPmVuZHBvaW50LmRlc2Mg PSBkZXNjOwo+ICsJcHJpdl9lcC0+ZGlyICA9IHVzYl9lbmRwb2ludF9kaXJfaW4oZGVzYykgPyBV U0JfRElSX0lOIDogVVNCX0RJUl9PVVQ7Cj4gKwlwcml2X2VwLT50eXBlID0gdXNiX2VuZHBvaW50 X3R5cGUoZGVzYyk7Cj4gKwo+ICsJbGlzdF9hZGRfdGFpbCgmcHJpdl9lcC0+ZXBfbWF0Y2hfcGVu ZGluZ19saXN0LAo+ICsJCSAgICAgICZwcml2X2Rldi0+ZXBfbWF0Y2hfbGlzdCk7Cj4gKwlzcGlu X3VubG9ja19pcnFyZXN0b3JlKCZwcml2X2Rldi0+bG9jaywgZmxhZ3MpOwo+ICsJcmV0dXJuICZw cml2X2VwLT5lbmRwb2ludDsKPiArfQoKV2h5IGRvIHlvdSBuZWVkIGEgY3VzdG9tIG1hdGNoX2Vw Pwpkb2Vzbid0IHVzYl9lcF9hdXRvY29uZmlnIHN1ZmZpY2U/CgpZb3UgY2FuIGNoZWNrIGlmIEVQ IGlzIGNsYWltZWQgb3Igbm90IGJ5IGNoZWNraW5nIHRoZSBlcC0+Y2xhaW1lZCBmbGFnLgoKPiAr Cj4gKy8qKgo+ICsgKiBjZG5zM19nYWRnZXRfZ2V0X2ZyYW1lIFJldHVybnMgbnVtYmVyIG9mIGFj dHVhbCBJVFAgZnJhbWUKPiArICogQGdhZGdldDogZ2FkZ2V0IG9iamVjdAo+ICsgKgo+ICsgKiBS ZXR1cm5zIG51bWJlciBvZiBhY3R1YWwgSVRQIGZyYW1lCj4gKyAqLwo+ICtzdGF0aWMgaW50IGNk bnMzX2dhZGdldF9nZXRfZnJhbWUoc3RydWN0IHVzYl9nYWRnZXQgKmdhZGdldCkKPiArewo+ICsJ c3RydWN0IGNkbnMzX2RldmljZSAqcHJpdl9kZXYgPSBnYWRnZXRfdG9fY2RuczNfZGV2aWNlKGdh ZGdldCk7Cj4gKwo+ICsJcmV0dXJuIHJlYWRsKCZwcml2X2Rldi0+cmVncy0+dXNiX2lwdG4pOwo+ ICt9Cj4gKwo+ICtzdGF0aWMgaW50IGNkbnMzX2dhZGdldF93YWtldXAoc3RydWN0IHVzYl9nYWRn ZXQgKmdhZGdldCkKPiArewo+ICsJcmV0dXJuIDA7Cj4gK30KPiArCj4gK3N0YXRpYyBpbnQgY2Ru czNfZ2FkZ2V0X3NldF9zZWxmcG93ZXJlZChzdHJ1Y3QgdXNiX2dhZGdldCAqZ2FkZ2V0LAo+ICsJ CQkJCWludCBpc19zZWxmcG93ZXJlZCkKPiArewo+ICsJc3RydWN0IGNkbnMzX2RldmljZSAqcHJp dl9kZXYgPSBnYWRnZXRfdG9fY2RuczNfZGV2aWNlKGdhZGdldCk7Cj4gKwl1bnNpZ25lZCBsb25n IGZsYWdzOwo+ICsKPiArCXNwaW5fbG9ja19pcnFzYXZlKCZwcml2X2Rldi0+bG9jaywgZmxhZ3Mp Owo+ICsJZ2FkZ2V0LT5pc19zZWxmcG93ZXJlZCA9ICEhaXNfc2VsZnBvd2VyZWQ7Cj4gKwlzcGlu X3VubG9ja19pcnFyZXN0b3JlKCZwcml2X2Rldi0+bG9jaywgZmxhZ3MpOwo+ICsJcmV0dXJuIDA7 Cj4gK30KPiArCj4gK3N0YXRpYyBpbnQgY2RuczNfZ2FkZ2V0X3B1bGx1cChzdHJ1Y3QgdXNiX2dh ZGdldCAqZ2FkZ2V0LCBpbnQgaXNfb24pCj4gK3sKPiArCXN0cnVjdCBjZG5zM19kZXZpY2UgKnBy aXZfZGV2ID0gZ2FkZ2V0X3RvX2NkbnMzX2RldmljZShnYWRnZXQpOwo+ICsKPiArCWlmICghcHJp dl9kZXYtPnN0YXJ0X2dhZGdldCkKPiArCQlyZXR1cm4gMDsKPiArCj4gKwlpZiAoaXNfb24pCj4g KwkJd3JpdGVsKFVTQl9DT05GX0RFVkVOLCAmcHJpdl9kZXYtPnJlZ3MtPnVzYl9jb25mKTsKPiAr CWVsc2UKPiArCQl3cml0ZWwoVVNCX0NPTkZfREVWRFMsICZwcml2X2Rldi0+cmVncy0+dXNiX2Nv bmYpOwo+ICsKPiArCXJldHVybiAwOwo+ICt9Cj4gKwo+ICBzdGF0aWMgdm9pZCBjZG5zM19nYWRn ZXRfY29uZmlnKHN0cnVjdCBjZG5zM19kZXZpY2UgKnByaXZfZGV2KQo+ICB7Cj4gIAlzdHJ1Y3Qg Y2RuczNfdXNiX3JlZ3MgX19pb21lbSAqcmVncyA9IHByaXZfZGV2LT5yZWdzOwo+IEBAIC03NCw2 ICsyMzEsOTUgQEAgc3RhdGljIHZvaWQgY2RuczNfZ2FkZ2V0X2NvbmZpZyhzdHJ1Y3QgY2RuczNf ZGV2aWNlICpwcml2X2RldikKPiAgCXdyaXRlbChVU0JfQ09ORl9ERVZFTiwgJnJlZ3MtPnVzYl9j b25mKTsKPiAgfQo+ICAKPiArLyoqCj4gKyAqIGNkbnMzX2dhZGdldF91ZGNfc3RhcnQgR2FkZ2V0 IHN0YXJ0Cj4gKyAqIEBnYWRnZXQ6IGdhZGdldCBvYmplY3QKPiArICogQGRyaXZlcjogZHJpdmVy IHdoaWNoIG9wZXJhdGVzIG9uIHRoaXMgZ2FkZ2V0Cj4gKyAqCj4gKyAqIFJldHVybnMgMCBvbiBz dWNjZXNzLCBlcnJvciBjb2RlIGVsc2V3aGVyZQo+ICsgKi8KPiArc3RhdGljIGludCBjZG5zM19n YWRnZXRfdWRjX3N0YXJ0KHN0cnVjdCB1c2JfZ2FkZ2V0ICpnYWRnZXQsCj4gKwkJCQkgIHN0cnVj dCB1c2JfZ2FkZ2V0X2RyaXZlciAqZHJpdmVyKQo+ICt7Cj4gKwlzdHJ1Y3QgY2RuczNfZGV2aWNl ICpwcml2X2RldiA9IGdhZGdldF90b19jZG5zM19kZXZpY2UoZ2FkZ2V0KTsKPiArCXVuc2lnbmVk IGxvbmcgZmxhZ3M7Cj4gKwo+ICsJaWYgKHByaXZfZGV2LT5nYWRnZXRfZHJpdmVyKSB7Cj4gKwkJ ZGV2X2VycigmcHJpdl9kZXYtPmRldiwgIiVzIGlzIGFscmVhZHkgYm91bmQgdG8gJXNcbiIsCj4g KwkJCXByaXZfZGV2LT5nYWRnZXQubmFtZSwKPiArCQkJcHJpdl9kZXYtPmdhZGdldF9kcml2ZXIt PmRyaXZlci5uYW1lKTsKPiArCQlyZXR1cm4gLUVCVVNZOwo+ICsJfQoKTm90IHN1cmUgaWYgdGhp cyBjaGVjayBpcyByZXF1aXJlZC4gVURDIGNvcmUgc2hvdWxkIGJlIGRvaW5nIHRoYXQuCgo+ICsK PiArCXNwaW5fbG9ja19pcnFzYXZlKCZwcml2X2Rldi0+bG9jaywgZmxhZ3MpOwo+ICsJcHJpdl9k ZXYtPmdhZGdldF9kcml2ZXIgPSBkcml2ZXI7Cj4gKwlpZiAoIXByaXZfZGV2LT5zdGFydF9nYWRn ZXQpCj4gKwkJZ290byB1bmxvY2s7Cj4gKwo+ICsJY2RuczNfZ2FkZ2V0X2NvbmZpZyhwcml2X2Rl dik7Cj4gK3VubG9jazoKPiArCXNwaW5fdW5sb2NrX2lycXJlc3RvcmUoJnByaXZfZGV2LT5sb2Nr LCBmbGFncyk7Cj4gKwlyZXR1cm4gMDsKPiArfQo+ICsKPiArLyoqCj4gKyAqIGNkbnMzX2dhZGdl dF91ZGNfc3RvcCBTdG9wcyBnYWRnZXQKPiArICogQGdhZGdldDogZ2FkZ2V0IG9iamVjdAo+ICsg Kgo+ICsgKiBSZXR1cm5zIDAKPiArICovCj4gK3N0YXRpYyBpbnQgY2RuczNfZ2FkZ2V0X3VkY19z dG9wKHN0cnVjdCB1c2JfZ2FkZ2V0ICpnYWRnZXQpCj4gK3sKPiArCXN0cnVjdCBjZG5zM19kZXZp Y2UgKnByaXZfZGV2ID0gZ2FkZ2V0X3RvX2NkbnMzX2RldmljZShnYWRnZXQpOwo+ICsJc3RydWN0 IGNkbnMzX2VuZHBvaW50ICpwcml2X2VwLCAqdGVtcF9lcDsKPiArCXUzMiBiRW5kcG9pbnRBZGRy ZXNzOwo+ICsJc3RydWN0IHVzYl9lcCAqZXA7Cj4gKwlpbnQgcmV0ID0gMDsKPiArCWludCBpOwo+ ICsKPiArCXByaXZfZGV2LT5nYWRnZXRfZHJpdmVyID0gTlVMTDsKPiArCWxpc3RfZm9yX2VhY2hf ZW50cnlfc2FmZShwcml2X2VwLCB0ZW1wX2VwLCAmcHJpdl9kZXYtPmVwX21hdGNoX2xpc3QsCj4g KwkJCQkgZXBfbWF0Y2hfcGVuZGluZ19saXN0KSB7Cj4gKwkJbGlzdF9kZWwoJnByaXZfZXAtPmVw X21hdGNoX3BlbmRpbmdfbGlzdCk7Cj4gKwkJcHJpdl9lcC0+ZmxhZ3MgJj0gfkVQX1VTRUQ7Cj4g Kwl9Cj4gKwo+ICsJcHJpdl9kZXYtPm9uY2hpcF9tZW1fYWxsb2NhdGVkX3NpemUgPSAwOwo+ICsJ cHJpdl9kZXYtPm91dF9tZW1faXNfYWxsb2NhdGVkID0gMDsKPiArCXByaXZfZGV2LT5nYWRnZXQu c3BlZWQgPSBVU0JfU1BFRURfVU5LTk9XTjsKPiArCj4gKwlmb3IgKGkgPSAwOyBpIDwgcHJpdl9k ZXYtPmVwX251bXMgOyBpKyspCj4gKwkJY2RuczNfZnJlZV90cmJfcG9vbChwcml2X2Rldi0+ZXBz W2ldKTsKPiArCj4gKwlpZiAoIXByaXZfZGV2LT5zdGFydF9nYWRnZXQpCj4gKwkJcmV0dXJuIDA7 CgpUaGlzIGxvb2tzIHRyaWNreS4gV2h5IGRvIHdlIG5lZWQgdGhpcyBmbGFnPwoKPiArCj4gKwls aXN0X2Zvcl9lYWNoX2VudHJ5KGVwLCAmcHJpdl9kZXYtPmdhZGdldC5lcF9saXN0LCBlcF9saXN0 KSB7Cj4gKwkJcHJpdl9lcCA9IGVwX3RvX2NkbnMzX2VwKGVwKTsKPiArCQliRW5kcG9pbnRBZGRy ZXNzID0gcHJpdl9lcC0+bnVtIHwgcHJpdl9lcC0+ZGlyOwo+ICsJCWNkbnMzX3NlbGVjdF9lcChw cml2X2RldiwgYkVuZHBvaW50QWRkcmVzcyk7Cj4gKwkJd3JpdGVsKEVQX0NNRF9FUFJTVCwgJnBy aXZfZGV2LT5yZWdzLT5lcF9jbWQpOwo+ICsJCXJldCA9IGNkbnMzX2hhbmRzaGFrZSgmcHJpdl9k ZXYtPnJlZ3MtPmVwX2NtZCwKPiArCQkJCSAgICAgIEVQX0NNRF9FUFJTVCwgMCwgMTAwKTsKPiAr CX0KPiArCj4gKwkvKiBkaXNhYmxlIGludGVycnVwdCBmb3IgZGV2aWNlICovCj4gKwl3cml0ZWwo MCwgJnByaXZfZGV2LT5yZWdzLT51c2JfaWVuKTsKPiArCXdyaXRlbChVU0JfQ09ORl9ERVZEUywg JnByaXZfZGV2LT5yZWdzLT51c2JfY29uZik7Cgp3aGVyZSBhcmUgeW91IHJlcXVlc3RpbmcgdGhl IGludGVycnVwdD8gTG9va3MgbGlrZSBpdCBzaG91bGQgYmUgZG9uZSBpbgp1ZGNfc3RhcnQoKSBu bz8KCj4gKwo+ICsJcmV0dXJuIHJldDsKPiArfQoKQ2FuIHdlIGNvbWJpbmUgY2RuczNfZ2FkZ2V0 X3VkY19zdGFydCgpIGFuZCBjZG5zM19nYWRnZXRfdWRjX3N0YXJ0KCkKd2l0aCBjZG5zM19nYWRn ZXRfc3RhcnQoKSBhbmQgY2RuczNfZ2FkZ2V0X3N0b3AoKSByZXNwZWN0aXZlbHkgc28gdGhhdApj ZG5zM19nYWRnZXRfY29uZmlnKCkgYW5kIGNsZWFudXAoKSBpcyBkb25lIGF0IG9uZSBwbGFjZS4K Cj4gKwo+ICtzdGF0aWMgY29uc3Qgc3RydWN0IHVzYl9nYWRnZXRfb3BzIGNkbnMzX2dhZGdldF9v cHMgPSB7Cj4gKwkuZ2V0X2ZyYW1lID0gY2RuczNfZ2FkZ2V0X2dldF9mcmFtZSwKPiArCS53YWtl dXAgPSBjZG5zM19nYWRnZXRfd2FrZXVwLAo+ICsJLnNldF9zZWxmcG93ZXJlZCA9IGNkbnMzX2dh ZGdldF9zZXRfc2VsZnBvd2VyZWQsCj4gKwkucHVsbHVwID0gY2RuczNfZ2FkZ2V0X3B1bGx1cCwK PiArCS51ZGNfc3RhcnQgPSBjZG5zM19nYWRnZXRfdWRjX3N0YXJ0LAo+ICsJLnVkY19zdG9wID0g Y2RuczNfZ2FkZ2V0X3VkY19zdG9wLAo+ICsJLm1hdGNoX2VwID0gY2RuczNfZ2FkZ2V0X21hdGNo X2VwLAo+ICt9Owo+ICsKPiAgLyoqCj4gICAqIGNkbnMzX2luaXRfZXAgSW5pdGlhbGl6ZXMgc29m dHdhcmUgZW5kcG9pbnRzIG9mIGdhZGdldAo+ICAgKiBAY2RuczM6IGV4dGVuZGVkIGdhZGdldCBv YmplY3QKPiBAQCAtMTg0LDggKzQzMCw3IEBAIHN0YXRpYyBpbnQgX19jZG5zM19nYWRnZXRfaW5p dChzdHJ1Y3QgY2RuczMgKmNkbnMpCj4gIAkvKiBmaWxsIGdhZGdldCBmaWVsZHMgKi8KPiAgCXBy aXZfZGV2LT5nYWRnZXQubWF4X3NwZWVkID0gVVNCX1NQRUVEX1NVUEVSOwo+ICAJcHJpdl9kZXYt PmdhZGdldC5zcGVlZCA9IFVTQl9TUEVFRF9VTktOT1dOOwo+IC0JLy9UT0RPOiBBZGQgaW1wbGVt ZW50YXRpb24gb2YgY2RuczNfZ2FkZ2V0X29wcwo+IC0JLy9wcml2X2Rldi0+Z2FkZ2V0Lm9wcyA9 ICZjZG5zM19nYWRnZXRfb3BzOwo+ICsJcHJpdl9kZXYtPmdhZGdldC5vcHMgPSAmY2RuczNfZ2Fk Z2V0X29wczsKPiAgCXByaXZfZGV2LT5nYWRnZXQubmFtZSA9ICJ1c2Itc3MtZ2FkZ2V0IjsKPiAg CXByaXZfZGV2LT5nYWRnZXQuc2dfc3VwcG9ydGVkID0gMTsKPiAgCXByaXZfZGV2LT5pc19jb25u ZWN0ZWQgPSAwOwo+IAoKY2hlZXJzLAotcm9nZXIK From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roger Quadros Subject: Re: [RFC PATCH v2 08/15] usb:cdns3: Implements device operations part of the API Date: Wed, 28 Nov 2018 14:22:27 +0200 Message-ID: <5BFE8883.7090802@ti.com> References: <1542535751-16079-1-git-send-email-pawell@cadence.com> <1542535751-16079-9-git-send-email-pawell@cadence.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1542535751-16079-9-git-send-email-pawell@cadence.com> Sender: linux-kernel-owner@vger.kernel.org To: Pawel Laszczak , devicetree@vger.kernel.org Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, adouglas@cadence.com, jbergsagel@ti.com, nsekhar@ti.com, nm@ti.com, sureshp@cadence.com, peter.chen@nxp.com, pjez@cadence.com, kurahul@cadence.com, Felipe Balbi List-Id: devicetree@vger.kernel.org On 18/11/18 12:09, Pawel Laszczak wrote: > Patch adds implementation callback function defined in > usb_gadget_ops object. > > Signed-off-by: Pawel Laszczak > --- > drivers/usb/cdns3/gadget.c | 249 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 247 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > index 376b68b13d1b..702a05faa664 100644 > --- a/drivers/usb/cdns3/gadget.c > +++ b/drivers/usb/cdns3/gadget.c > @@ -17,6 +17,36 @@ > #include "gadget-export.h" > #include "gadget.h" > > +/** > + * cdns3_handshake - spin reading until handshake completes or fails > + * @ptr: address of device controller register to be read > + * @mask: bits to look at in result of read > + * @done: value of those bits when handshake succeeds > + * @usec: timeout in microseconds > + * > + * Returns negative errno, or zero on success > + * > + * Success happens when the "mask" bits have the specified value (hardware > + * handshake done). There are two failure modes: "usec" have passed (major > + * hardware flakeout), or the register reads as all-ones (hardware removed). > + */ > +int cdns3_handshake(void __iomem *ptr, u32 mask, u32 done, int usec) > +{ > + u32 result; > + > + do { > + result = readl(ptr); > + if (result == ~(u32)0) /* card removed */ > + return -ENODEV; Is this applicable to all registers? What is meant by card removed? We're not connected to host? how does EP reset behave when there is no USB connection? > + result &= mask; > + if (result == done) > + return 0; > + udelay(1); > + usec--; > + } while (usec > 0); > + return -ETIMEDOUT; > +} > + > /** > * cdns3_set_register_bit - set bit in given register. > * @ptr: address of device controller register to be read and changed > @@ -43,6 +73,25 @@ void cdns3_select_ep(struct cdns3_device *priv_dev, u32 ep) > writel(ep, &priv_dev->regs->ep_sel); > } > > +static void cdns3_free_trb_pool(struct cdns3_endpoint *priv_ep) > +{ > + struct cdns3_device *priv_dev = priv_ep->cdns3_dev; > + > + if (priv_ep->trb_pool) { > + dma_free_coherent(priv_dev->sysdev, > + TRB_RIGN_SIZE, > + priv_ep->trb_pool, priv_ep->trb_pool_dma); > + priv_ep->trb_pool = NULL; > + } > + > + if (priv_ep->aligned_buff) { > + dma_free_coherent(priv_dev->sysdev, CDNS3_UNALIGNED_BUF_SIZE, > + priv_ep->aligned_buff, > + priv_ep->aligned_dma_addr); > + priv_ep->aligned_buff = NULL; > + } > +} > + > /** > * cdns3_irq_handler - irq line interrupt handler > * @cdns: cdns3 instance > @@ -58,6 +107,114 @@ static irqreturn_t cdns3_irq_handler_thread(struct cdns3 *cdns) > return ret; > } > > +/* Find correct direction for HW endpoint according to description */ > +static int cdns3_ep_dir_is_correct(struct usb_endpoint_descriptor *desc, > + struct cdns3_endpoint *priv_ep) > +{ > + return (priv_ep->endpoint.caps.dir_in && usb_endpoint_dir_in(desc)) || > + (priv_ep->endpoint.caps.dir_out && usb_endpoint_dir_out(desc)); > +} > + > +static struct cdns3_endpoint *cdns3_find_available_ss_ep(struct cdns3_device *priv_dev, > + struct usb_endpoint_descriptor *desc) why is this function called ss_ep? This doesn't seem like only for superspeed endpoints. > +{ > + struct usb_ep *ep; > + struct cdns3_endpoint *priv_ep; > + > + list_for_each_entry(ep, &priv_dev->gadget.ep_list, ep_list) { > + unsigned long num; > + int ret; > + /* ep name pattern likes epXin or epXout */ > + char c[2] = {ep->name[2], '\0'}; > + > + ret = kstrtoul(c, 10, &num); > + if (ret) > + return ERR_PTR(ret); > + > + priv_ep = ep_to_cdns3_ep(ep); > + if (cdns3_ep_dir_is_correct(desc, priv_ep)) { > + if (!(priv_ep->flags & EP_USED)) { > + priv_ep->num = num; > + priv_ep->flags |= EP_USED; > + return priv_ep; > + } > + } > + } > + return ERR_PTR(-ENOENT); > +} > + > +static struct usb_ep *cdns3_gadget_match_ep(struct usb_gadget *gadget, > + struct usb_endpoint_descriptor *desc, > + struct usb_ss_ep_comp_descriptor *comp_desc) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + struct cdns3_endpoint *priv_ep; > + unsigned long flags; > + > + priv_ep = cdns3_find_available_ss_ep(priv_dev, desc); > + if (IS_ERR(priv_ep)) { > + dev_err(&priv_dev->dev, "no available ep\n"); > + return NULL; > + } > + > + dev_dbg(&priv_dev->dev, "match endpoint: %s\n", priv_ep->name); > + > + spin_lock_irqsave(&priv_dev->lock, flags); > + priv_ep->endpoint.desc = desc; > + priv_ep->dir = usb_endpoint_dir_in(desc) ? USB_DIR_IN : USB_DIR_OUT; > + priv_ep->type = usb_endpoint_type(desc); > + > + list_add_tail(&priv_ep->ep_match_pending_list, > + &priv_dev->ep_match_list); > + spin_unlock_irqrestore(&priv_dev->lock, flags); > + return &priv_ep->endpoint; > +} Why do you need a custom match_ep? doesn't usb_ep_autoconfig suffice? You can check if EP is claimed or not by checking the ep->claimed flag. > + > +/** > + * cdns3_gadget_get_frame Returns number of actual ITP frame > + * @gadget: gadget object > + * > + * Returns number of actual ITP frame > + */ > +static int cdns3_gadget_get_frame(struct usb_gadget *gadget) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + > + return readl(&priv_dev->regs->usb_iptn); > +} > + > +static int cdns3_gadget_wakeup(struct usb_gadget *gadget) > +{ > + return 0; > +} > + > +static int cdns3_gadget_set_selfpowered(struct usb_gadget *gadget, > + int is_selfpowered) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + unsigned long flags; > + > + spin_lock_irqsave(&priv_dev->lock, flags); > + gadget->is_selfpowered = !!is_selfpowered; > + spin_unlock_irqrestore(&priv_dev->lock, flags); > + return 0; > +} > + > +static int cdns3_gadget_pullup(struct usb_gadget *gadget, int is_on) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + > + if (!priv_dev->start_gadget) > + return 0; > + > + if (is_on) > + writel(USB_CONF_DEVEN, &priv_dev->regs->usb_conf); > + else > + writel(USB_CONF_DEVDS, &priv_dev->regs->usb_conf); > + > + return 0; > +} > + > static void cdns3_gadget_config(struct cdns3_device *priv_dev) > { > struct cdns3_usb_regs __iomem *regs = priv_dev->regs; > @@ -74,6 +231,95 @@ static void cdns3_gadget_config(struct cdns3_device *priv_dev) > writel(USB_CONF_DEVEN, ®s->usb_conf); > } > > +/** > + * cdns3_gadget_udc_start Gadget start > + * @gadget: gadget object > + * @driver: driver which operates on this gadget > + * > + * Returns 0 on success, error code elsewhere > + */ > +static int cdns3_gadget_udc_start(struct usb_gadget *gadget, > + struct usb_gadget_driver *driver) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + unsigned long flags; > + > + if (priv_dev->gadget_driver) { > + dev_err(&priv_dev->dev, "%s is already bound to %s\n", > + priv_dev->gadget.name, > + priv_dev->gadget_driver->driver.name); > + return -EBUSY; > + } Not sure if this check is required. UDC core should be doing that. > + > + spin_lock_irqsave(&priv_dev->lock, flags); > + priv_dev->gadget_driver = driver; > + if (!priv_dev->start_gadget) > + goto unlock; > + > + cdns3_gadget_config(priv_dev); > +unlock: > + spin_unlock_irqrestore(&priv_dev->lock, flags); > + return 0; > +} > + > +/** > + * cdns3_gadget_udc_stop Stops gadget > + * @gadget: gadget object > + * > + * Returns 0 > + */ > +static int cdns3_gadget_udc_stop(struct usb_gadget *gadget) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + struct cdns3_endpoint *priv_ep, *temp_ep; > + u32 bEndpointAddress; > + struct usb_ep *ep; > + int ret = 0; > + int i; > + > + priv_dev->gadget_driver = NULL; > + list_for_each_entry_safe(priv_ep, temp_ep, &priv_dev->ep_match_list, > + ep_match_pending_list) { > + list_del(&priv_ep->ep_match_pending_list); > + priv_ep->flags &= ~EP_USED; > + } > + > + priv_dev->onchip_mem_allocated_size = 0; > + priv_dev->out_mem_is_allocated = 0; > + priv_dev->gadget.speed = USB_SPEED_UNKNOWN; > + > + for (i = 0; i < priv_dev->ep_nums ; i++) > + cdns3_free_trb_pool(priv_dev->eps[i]); > + > + if (!priv_dev->start_gadget) > + return 0; This looks tricky. Why do we need this flag? > + > + list_for_each_entry(ep, &priv_dev->gadget.ep_list, ep_list) { > + priv_ep = ep_to_cdns3_ep(ep); > + bEndpointAddress = priv_ep->num | priv_ep->dir; > + cdns3_select_ep(priv_dev, bEndpointAddress); > + writel(EP_CMD_EPRST, &priv_dev->regs->ep_cmd); > + ret = cdns3_handshake(&priv_dev->regs->ep_cmd, > + EP_CMD_EPRST, 0, 100); > + } > + > + /* disable interrupt for device */ > + writel(0, &priv_dev->regs->usb_ien); > + writel(USB_CONF_DEVDS, &priv_dev->regs->usb_conf); where are you requesting the interrupt? Looks like it should be done in udc_start() no? > + > + return ret; > +} Can we combine cdns3_gadget_udc_start() and cdns3_gadget_udc_start() with cdns3_gadget_start() and cdns3_gadget_stop() respectively so that cdns3_gadget_config() and cleanup() is done at one place. > + > +static const struct usb_gadget_ops cdns3_gadget_ops = { > + .get_frame = cdns3_gadget_get_frame, > + .wakeup = cdns3_gadget_wakeup, > + .set_selfpowered = cdns3_gadget_set_selfpowered, > + .pullup = cdns3_gadget_pullup, > + .udc_start = cdns3_gadget_udc_start, > + .udc_stop = cdns3_gadget_udc_stop, > + .match_ep = cdns3_gadget_match_ep, > +}; > + > /** > * cdns3_init_ep Initializes software endpoints of gadget > * @cdns3: extended gadget object > @@ -184,8 +430,7 @@ static int __cdns3_gadget_init(struct cdns3 *cdns) > /* fill gadget fields */ > priv_dev->gadget.max_speed = USB_SPEED_SUPER; > priv_dev->gadget.speed = USB_SPEED_UNKNOWN; > - //TODO: Add implementation of cdns3_gadget_ops > - //priv_dev->gadget.ops = &cdns3_gadget_ops; > + priv_dev->gadget.ops = &cdns3_gadget_ops; > priv_dev->gadget.name = "usb-ss-gadget"; > priv_dev->gadget.sg_supported = 1; > priv_dev->is_connected = 0; > cheers, -roger -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki 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=-8.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,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 EF025C43441 for ; Wed, 28 Nov 2018 12:22:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9943320832 for ; Wed, 28 Nov 2018 12:22:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b="PxEP/LGc" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9943320832 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=ti.com 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 S1728357AbeK1XYI (ORCPT ); Wed, 28 Nov 2018 18:24:08 -0500 Received: from lelv0143.ext.ti.com ([198.47.23.248]:40400 "EHLO lelv0143.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727703AbeK1XYI (ORCPT ); Wed, 28 Nov 2018 18:24:08 -0500 Received: from lelv0265.itg.ti.com ([10.180.67.224]) by lelv0143.ext.ti.com (8.15.2/8.15.2) with ESMTP id wASCMW14075111; Wed, 28 Nov 2018 06:22:32 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1543407752; bh=UEq7tf0CRjQ7r9qUZ2hVopnZQBoFanlYdKDczggAEII=; h=Subject:To:References:CC:From:Date:In-Reply-To; b=PxEP/LGccohFOIcu9t6Z0nOtnVV8gT8ST1mPYOdaQQiAi7cYXKvSlGm5OLrhMUMC4 ZdAdrDgzpALw3pauTrkJxcCl/6oilfrdoDoVCU8Q4v9BZS3AvIsz7q5DMw3b9f/1Jx avdprq13+0YPpLTDhIjL6khVexCzhBF6py+jXiyw= Received: from DFLE110.ent.ti.com (dfle110.ent.ti.com [10.64.6.31]) by lelv0265.itg.ti.com (8.15.2/8.15.2) with ESMTPS id wASCMWuZ011716 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 28 Nov 2018 06:22:32 -0600 Received: from DFLE102.ent.ti.com (10.64.6.23) by DFLE110.ent.ti.com (10.64.6.31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1591.10; Wed, 28 Nov 2018 06:22:31 -0600 Received: from dflp32.itg.ti.com (10.64.6.15) by DFLE102.ent.ti.com (10.64.6.23) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.1.1591.10 via Frontend Transport; Wed, 28 Nov 2018 06:22:31 -0600 Received: from [192.168.2.6] (ileax41-snat.itg.ti.com [10.172.224.153]) by dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id wASCMS9X000893; Wed, 28 Nov 2018 06:22:28 -0600 Subject: Re: [RFC PATCH v2 08/15] usb:cdns3: Implements device operations part of the API To: Pawel Laszczak , References: <1542535751-16079-1-git-send-email-pawell@cadence.com> <1542535751-16079-9-git-send-email-pawell@cadence.com> CC: , , , , , , , , , , , Felipe Balbi From: Roger Quadros Message-ID: <5BFE8883.7090802@ti.com> Date: Wed, 28 Nov 2018 14:22:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1542535751-16079-9-git-send-email-pawell@cadence.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 18/11/18 12:09, Pawel Laszczak wrote: > Patch adds implementation callback function defined in > usb_gadget_ops object. > > Signed-off-by: Pawel Laszczak > --- > drivers/usb/cdns3/gadget.c | 249 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 247 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > index 376b68b13d1b..702a05faa664 100644 > --- a/drivers/usb/cdns3/gadget.c > +++ b/drivers/usb/cdns3/gadget.c > @@ -17,6 +17,36 @@ > #include "gadget-export.h" > #include "gadget.h" > > +/** > + * cdns3_handshake - spin reading until handshake completes or fails > + * @ptr: address of device controller register to be read > + * @mask: bits to look at in result of read > + * @done: value of those bits when handshake succeeds > + * @usec: timeout in microseconds > + * > + * Returns negative errno, or zero on success > + * > + * Success happens when the "mask" bits have the specified value (hardware > + * handshake done). There are two failure modes: "usec" have passed (major > + * hardware flakeout), or the register reads as all-ones (hardware removed). > + */ > +int cdns3_handshake(void __iomem *ptr, u32 mask, u32 done, int usec) > +{ > + u32 result; > + > + do { > + result = readl(ptr); > + if (result == ~(u32)0) /* card removed */ > + return -ENODEV; Is this applicable to all registers? What is meant by card removed? We're not connected to host? how does EP reset behave when there is no USB connection? > + result &= mask; > + if (result == done) > + return 0; > + udelay(1); > + usec--; > + } while (usec > 0); > + return -ETIMEDOUT; > +} > + > /** > * cdns3_set_register_bit - set bit in given register. > * @ptr: address of device controller register to be read and changed > @@ -43,6 +73,25 @@ void cdns3_select_ep(struct cdns3_device *priv_dev, u32 ep) > writel(ep, &priv_dev->regs->ep_sel); > } > > +static void cdns3_free_trb_pool(struct cdns3_endpoint *priv_ep) > +{ > + struct cdns3_device *priv_dev = priv_ep->cdns3_dev; > + > + if (priv_ep->trb_pool) { > + dma_free_coherent(priv_dev->sysdev, > + TRB_RIGN_SIZE, > + priv_ep->trb_pool, priv_ep->trb_pool_dma); > + priv_ep->trb_pool = NULL; > + } > + > + if (priv_ep->aligned_buff) { > + dma_free_coherent(priv_dev->sysdev, CDNS3_UNALIGNED_BUF_SIZE, > + priv_ep->aligned_buff, > + priv_ep->aligned_dma_addr); > + priv_ep->aligned_buff = NULL; > + } > +} > + > /** > * cdns3_irq_handler - irq line interrupt handler > * @cdns: cdns3 instance > @@ -58,6 +107,114 @@ static irqreturn_t cdns3_irq_handler_thread(struct cdns3 *cdns) > return ret; > } > > +/* Find correct direction for HW endpoint according to description */ > +static int cdns3_ep_dir_is_correct(struct usb_endpoint_descriptor *desc, > + struct cdns3_endpoint *priv_ep) > +{ > + return (priv_ep->endpoint.caps.dir_in && usb_endpoint_dir_in(desc)) || > + (priv_ep->endpoint.caps.dir_out && usb_endpoint_dir_out(desc)); > +} > + > +static struct cdns3_endpoint *cdns3_find_available_ss_ep(struct cdns3_device *priv_dev, > + struct usb_endpoint_descriptor *desc) why is this function called ss_ep? This doesn't seem like only for superspeed endpoints. > +{ > + struct usb_ep *ep; > + struct cdns3_endpoint *priv_ep; > + > + list_for_each_entry(ep, &priv_dev->gadget.ep_list, ep_list) { > + unsigned long num; > + int ret; > + /* ep name pattern likes epXin or epXout */ > + char c[2] = {ep->name[2], '\0'}; > + > + ret = kstrtoul(c, 10, &num); > + if (ret) > + return ERR_PTR(ret); > + > + priv_ep = ep_to_cdns3_ep(ep); > + if (cdns3_ep_dir_is_correct(desc, priv_ep)) { > + if (!(priv_ep->flags & EP_USED)) { > + priv_ep->num = num; > + priv_ep->flags |= EP_USED; > + return priv_ep; > + } > + } > + } > + return ERR_PTR(-ENOENT); > +} > + > +static struct usb_ep *cdns3_gadget_match_ep(struct usb_gadget *gadget, > + struct usb_endpoint_descriptor *desc, > + struct usb_ss_ep_comp_descriptor *comp_desc) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + struct cdns3_endpoint *priv_ep; > + unsigned long flags; > + > + priv_ep = cdns3_find_available_ss_ep(priv_dev, desc); > + if (IS_ERR(priv_ep)) { > + dev_err(&priv_dev->dev, "no available ep\n"); > + return NULL; > + } > + > + dev_dbg(&priv_dev->dev, "match endpoint: %s\n", priv_ep->name); > + > + spin_lock_irqsave(&priv_dev->lock, flags); > + priv_ep->endpoint.desc = desc; > + priv_ep->dir = usb_endpoint_dir_in(desc) ? USB_DIR_IN : USB_DIR_OUT; > + priv_ep->type = usb_endpoint_type(desc); > + > + list_add_tail(&priv_ep->ep_match_pending_list, > + &priv_dev->ep_match_list); > + spin_unlock_irqrestore(&priv_dev->lock, flags); > + return &priv_ep->endpoint; > +} Why do you need a custom match_ep? doesn't usb_ep_autoconfig suffice? You can check if EP is claimed or not by checking the ep->claimed flag. > + > +/** > + * cdns3_gadget_get_frame Returns number of actual ITP frame > + * @gadget: gadget object > + * > + * Returns number of actual ITP frame > + */ > +static int cdns3_gadget_get_frame(struct usb_gadget *gadget) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + > + return readl(&priv_dev->regs->usb_iptn); > +} > + > +static int cdns3_gadget_wakeup(struct usb_gadget *gadget) > +{ > + return 0; > +} > + > +static int cdns3_gadget_set_selfpowered(struct usb_gadget *gadget, > + int is_selfpowered) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + unsigned long flags; > + > + spin_lock_irqsave(&priv_dev->lock, flags); > + gadget->is_selfpowered = !!is_selfpowered; > + spin_unlock_irqrestore(&priv_dev->lock, flags); > + return 0; > +} > + > +static int cdns3_gadget_pullup(struct usb_gadget *gadget, int is_on) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + > + if (!priv_dev->start_gadget) > + return 0; > + > + if (is_on) > + writel(USB_CONF_DEVEN, &priv_dev->regs->usb_conf); > + else > + writel(USB_CONF_DEVDS, &priv_dev->regs->usb_conf); > + > + return 0; > +} > + > static void cdns3_gadget_config(struct cdns3_device *priv_dev) > { > struct cdns3_usb_regs __iomem *regs = priv_dev->regs; > @@ -74,6 +231,95 @@ static void cdns3_gadget_config(struct cdns3_device *priv_dev) > writel(USB_CONF_DEVEN, ®s->usb_conf); > } > > +/** > + * cdns3_gadget_udc_start Gadget start > + * @gadget: gadget object > + * @driver: driver which operates on this gadget > + * > + * Returns 0 on success, error code elsewhere > + */ > +static int cdns3_gadget_udc_start(struct usb_gadget *gadget, > + struct usb_gadget_driver *driver) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + unsigned long flags; > + > + if (priv_dev->gadget_driver) { > + dev_err(&priv_dev->dev, "%s is already bound to %s\n", > + priv_dev->gadget.name, > + priv_dev->gadget_driver->driver.name); > + return -EBUSY; > + } Not sure if this check is required. UDC core should be doing that. > + > + spin_lock_irqsave(&priv_dev->lock, flags); > + priv_dev->gadget_driver = driver; > + if (!priv_dev->start_gadget) > + goto unlock; > + > + cdns3_gadget_config(priv_dev); > +unlock: > + spin_unlock_irqrestore(&priv_dev->lock, flags); > + return 0; > +} > + > +/** > + * cdns3_gadget_udc_stop Stops gadget > + * @gadget: gadget object > + * > + * Returns 0 > + */ > +static int cdns3_gadget_udc_stop(struct usb_gadget *gadget) > +{ > + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget); > + struct cdns3_endpoint *priv_ep, *temp_ep; > + u32 bEndpointAddress; > + struct usb_ep *ep; > + int ret = 0; > + int i; > + > + priv_dev->gadget_driver = NULL; > + list_for_each_entry_safe(priv_ep, temp_ep, &priv_dev->ep_match_list, > + ep_match_pending_list) { > + list_del(&priv_ep->ep_match_pending_list); > + priv_ep->flags &= ~EP_USED; > + } > + > + priv_dev->onchip_mem_allocated_size = 0; > + priv_dev->out_mem_is_allocated = 0; > + priv_dev->gadget.speed = USB_SPEED_UNKNOWN; > + > + for (i = 0; i < priv_dev->ep_nums ; i++) > + cdns3_free_trb_pool(priv_dev->eps[i]); > + > + if (!priv_dev->start_gadget) > + return 0; This looks tricky. Why do we need this flag? > + > + list_for_each_entry(ep, &priv_dev->gadget.ep_list, ep_list) { > + priv_ep = ep_to_cdns3_ep(ep); > + bEndpointAddress = priv_ep->num | priv_ep->dir; > + cdns3_select_ep(priv_dev, bEndpointAddress); > + writel(EP_CMD_EPRST, &priv_dev->regs->ep_cmd); > + ret = cdns3_handshake(&priv_dev->regs->ep_cmd, > + EP_CMD_EPRST, 0, 100); > + } > + > + /* disable interrupt for device */ > + writel(0, &priv_dev->regs->usb_ien); > + writel(USB_CONF_DEVDS, &priv_dev->regs->usb_conf); where are you requesting the interrupt? Looks like it should be done in udc_start() no? > + > + return ret; > +} Can we combine cdns3_gadget_udc_start() and cdns3_gadget_udc_start() with cdns3_gadget_start() and cdns3_gadget_stop() respectively so that cdns3_gadget_config() and cleanup() is done at one place. > + > +static const struct usb_gadget_ops cdns3_gadget_ops = { > + .get_frame = cdns3_gadget_get_frame, > + .wakeup = cdns3_gadget_wakeup, > + .set_selfpowered = cdns3_gadget_set_selfpowered, > + .pullup = cdns3_gadget_pullup, > + .udc_start = cdns3_gadget_udc_start, > + .udc_stop = cdns3_gadget_udc_stop, > + .match_ep = cdns3_gadget_match_ep, > +}; > + > /** > * cdns3_init_ep Initializes software endpoints of gadget > * @cdns3: extended gadget object > @@ -184,8 +430,7 @@ static int __cdns3_gadget_init(struct cdns3 *cdns) > /* fill gadget fields */ > priv_dev->gadget.max_speed = USB_SPEED_SUPER; > priv_dev->gadget.speed = USB_SPEED_UNKNOWN; > - //TODO: Add implementation of cdns3_gadget_ops > - //priv_dev->gadget.ops = &cdns3_gadget_ops; > + priv_dev->gadget.ops = &cdns3_gadget_ops; > priv_dev->gadget.name = "usb-ss-gadget"; > priv_dev->gadget.sg_supported = 1; > priv_dev->is_connected = 0; > cheers, -roger -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki