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: [07/10] hikey960: Support usb functionality of Hikey960 From: Yu Chen Message-Id: <33a6ac59-b545-e2c8-2bb7-0a5460fcf5e9@huawei.com> Date: Tue, 30 Oct 2018 10:50:22 +0800 To: Heikki Krogerus Cc: wangbinghui@hisilicon.com, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, suzhuangluan@hisilicon.com, kongfei@hisilicon.com, Arnd Bergmann , Greg Kroah-Hartman , John Stultz List-ID: SGkKCgpPbiAyMDE4LzEwLzI5IDIyOjMwLCBIZWlra2kgS3JvZ2VydXMgd3JvdGU6Cj4gSGksCj4K PiBPbiBTYXQsIE9jdCAyNywgMjAxOCBhdCAwNTo1ODoxN1BNICswODAwLCBZdSBDaGVuIHdyb3Rl Ogo+PiBUaGlzIGRyaXZlciBoYW5kbGVzIHVzYiBodWIgcG93ZXIgb24gYW5kIHR5cGVDIHBvcnQg ZXZlbnQgb2YgSGlLZXk5NjAgYm9hcmQ6Cj4+IDEpRFAmRE0gc3dpdGNoaW5nIGJldHdlZW4gdXNi IGh1YiBhbmQgdHlwZUMgcG9ydCBiYXNlIG9uIHR5cGVDIHBvcnQKPj4gc3RhdGUKPiBCeSAiaHVi IiBkbyB5b3UgbWVhbiB5b3UgaGF2ZSBzb21lIGtpbmQgb2YgYW4gaW50ZWdyYXRlZCBVU0IgaHVi IG9uCj4geW91ciBTb0M/Ck5vLiBUaGUgImh1YiIgaXMgb24gdGhlIGJvYXJkIG9mIEhpS2V5OTYw IGRldmVsb3BtZW50IHBsYXRmb3JtLgoKPj4gMilDb250cm9sIHBvd2VyIG9mIHVzYiBodWIgb24g SGlrZXk5NjAKPj4gMylDb250cm9sIHZidXMgb2YgdHlwZUMgcG9ydAo+PiA0KUhhbmRsZSB0eXBl QyBwb3J0IGV2ZW50IHRvIHN3aXRjaCBkYXRhIHJvbGUKPiBJcyB5b3VyIHJvbGUgc3dpdGNoIGEg ZGlzY3JldGUgY29tcG9uZW50LCBvciBzb21ldGhpbmcgcGFydCBvZiB0aGUgU29DPwpZZXMsIHRo ZSBkd2MzIHVzYiBjb250cm9sbGVyIG9mIHRoZSBTb0MgbmVlZHMgc3dpdGNoIGJldHdlZW4gZGV2 aWNlIGFuZCBob3N0Cm1vZGUgYmFzZWQgb24gdGhlIHR5cGVDIHBvcnQgZXZlbnQuCgo+Cj4+ICsj ZGVmaW5lIFVTQl9TV0lUQ0hfVE9fSFVCIDEKPj4gKyNkZWZpbmUgVVNCX1NXSVRDSF9UT19UWVBF QyAwCj4+ICsKPj4gKyNkZWZpbmUgSU5WQUxJRF9HUElPX1ZBTFVFICgtMSkKPj4gKwo+PiArc3Ry dWN0IGhpc2lfaGlrZXlfdXNiIHsKPj4gKwlpbnQgb3RnX3N3aXRjaF9ncGlvOwo+PiArCWludCB0 eXBlY192YnVzX2dwaW87Cj4+ICsJaW50IHR5cGVjX3ZidXNfZW5hYmxlX3ZhbDsKPj4gKwlpbnQg aHViX3ZidXNfZ3BpbzsKPiBJIHRoaW5rIHlvdSBzaG91bGQgdXNlIHN0cnVjdCBncGlvX2Rlc2Mg YW5kIGdwaW9kXyooKSBBUEkgd2l0aCB0aGUKPiBncGlvcy4KCllvdSBhcmUgcmlnaHQsIEkgd2ls bCBmaXggdGhhdC4gVGhhbmtzIQo+PiArCXN0cnVjdCBleHRjb25fZGV2ICplZGV2Owo+PiArCXN0 cnVjdCB1c2Jfcm9sZV9zd2l0Y2ggKnJvbGVfc3c7Cj4+ICt9Owo+PiArCj4+ICtzdGF0aWMgY29u c3QgdW5zaWduZWQgaW50IHVzYl9leHRjb25fY2FibGVbXSA9IHsKPj4gKwlFWFRDT05fVVNCLAo+ PiArCUVYVENPTl9VU0JfSE9TVCwKPj4gKwlFWFRDT05fTk9ORSwKPj4gK307Cj4+ICsKPj4gK3N0 YXRpYyB2b2lkIGh1Yl9wb3dlcl9jdHJsKHN0cnVjdCBoaXNpX2hpa2V5X3VzYiAqaGlzaV9oaWtl eV91c2IsIGludCB2YWx1ZSkKPj4gK3sKPj4gKwlpbnQgZ3BpbyA9IGhpc2lfaGlrZXlfdXNiLT5o dWJfdmJ1c19ncGlvOwo+PiArCj4+ICsJaWYgKGdwaW9faXNfdmFsaWQoZ3BpbykpCj4+ICsJCWdw aW9fc2V0X3ZhbHVlKGdwaW8sIHZhbHVlKTsKPj4gK30KPj4gKwo+PiArc3RhdGljIHZvaWQgdXNi X3N3aXRjaF9jdHJsKHN0cnVjdCBoaXNpX2hpa2V5X3VzYiAqaGlzaV9oaWtleV91c2IsCj4+ICsJ CWludCBzd2l0Y2hfdG8pCj4+ICt7Cj4+ICsJaW50IGdwaW8gPSBoaXNpX2hpa2V5X3VzYi0+b3Rn X3N3aXRjaF9ncGlvOwo+PiArCWNvbnN0IGNoYXIgKnN3aXRjaF90b19zdHIgPSAoc3dpdGNoX3Rv ID09IFVTQl9TV0lUQ0hfVE9fSFVCKSA/Cj4+ICsJCSJodWIiIDogInR5cGVjIjsKPj4gKwo+PiAr CWlmICghZ3Bpb19pc192YWxpZChncGlvKSkgewo+PiArCQlwcl9lcnIoIiVzOiBvdGdfc3dpdGNo X2dwaW8gaXMgZXJyXG4iLCBfX2Z1bmNfXyk7Cj4+ICsJCXJldHVybjsKPj4gKwl9Cj4+ICsKPj4g KwlpZiAoZ3Bpb19nZXRfdmFsdWUoZ3BpbykgPT0gc3dpdGNoX3RvKSB7Cj4+ICsJCXByX2luZm8o IiVzOiBhbHJlYWR5IHN3aXRjaCB0byAlc1xuIiwgX19mdW5jX18sIHN3aXRjaF90b19zdHIpOwo+ IFRoYXQga2luZCBvZiBwcmludHMgYXJlIHJlYWxseSBqdXN0IG5vaXNlLgo+Cj4+ICsJCXJldHVy bjsKPj4gKwl9Cj4+ICsKPj4gKwlncGlvX2RpcmVjdGlvbl9vdXRwdXQoZ3Bpbywgc3dpdGNoX3Rv KTsKPj4gKwlwcl9pbmZvKCIlczogc3dpdGNoIHRvICVzXG4iLCBfX2Z1bmNfXywgc3dpdGNoX3Rv X3N0cik7Cj4gVGhhdCBpcyBhbHNvIGp1c3Qgbm9pc2UuCj4KPj4gK30KPj4gKwo+PiArc3RhdGlj IHZvaWQgdXNiX3R5cGVjX3Bvd2VyX2N0cmwoc3RydWN0IGhpc2lfaGlrZXlfdXNiICpoaXNpX2hp a2V5X3VzYiwKPj4gKwkJaW50IHZhbHVlKQo+PiArewo+PiArCWludCBncGlvID0gaGlzaV9oaWtl eV91c2ItPnR5cGVjX3ZidXNfZ3BpbzsKPj4gKwo+PiArCWlmICghZ3Bpb19pc192YWxpZChncGlv KSkgewo+PiArCQlwcl9lcnIoIiVzOiB0eXBlYyBwb3dlciBncGlvIGlzIGVyclxuIiwgX19mdW5j X18pOwo+PiArCQlyZXR1cm47Cj4+ICsJfQo+PiArCj4+ICsJaWYgKGdwaW9fZ2V0X3ZhbHVlKGdw aW8pID09IHZhbHVlKSB7Cj4+ICsJCXByX2luZm8oIiVzOiB0eXBlYyBwb3dlciBubyBjaGFuZ2Vc biIsIF9fZnVuY19fKTsKPiBEaXR0by4KPgo+PiArCQlyZXR1cm47Cj4+ICsJfQo+PiArCj4+ICsJ Z3Bpb19kaXJlY3Rpb25fb3V0cHV0KGdwaW8sIHZhbHVlKTsKPj4gKwlwcl9pbmZvKCIlczogc2V0 IHR5cGVjIHZidXMgZ3BpbyB0byAlZFxuIiwgX19mdW5jX18sIHZhbHVlKTsKPiBEaXR0by4KPgo+ PiArfQo+PiArCj4+ICtzdGF0aWMgaW50IGV4dGNvbl9oaXNpX3BkX3NldF9yb2xlKHN0cnVjdCBk ZXZpY2UgKmRldiwgZW51bSB1c2Jfcm9sZSByb2xlKQo+PiArewo+PiArCXN0cnVjdCBoaXNpX2hp a2V5X3VzYiAqaGlzaV9oaWtleV91c2IgPSBkZXZfZ2V0X2RydmRhdGEoZGV2KTsKPj4gKwo+PiAr CWRldl9pbmZvKGRldiwgIiVzOnNldCB1c2Igcm9sZSB0byAlZFxuIiwgX19mdW5jX18sIHJvbGUp Owo+PiArCXN3aXRjaCAocm9sZSkgewo+PiArCWNhc2UgVVNCX1JPTEVfTk9ORToKPj4gKwkJdXNi X3N3aXRjaF9jdHJsKGhpc2lfaGlrZXlfdXNiLCBVU0JfU1dJVENIX1RPX0hVQik7Cj4+ICsJCXVz Yl90eXBlY19wb3dlcl9jdHJsKGhpc2lfaGlrZXlfdXNiLAo+PiArCQkJCSFoaXNpX2hpa2V5X3Vz Yi0+dHlwZWNfdmJ1c19lbmFibGVfdmFsKTsKPj4gKwkJaHViX3Bvd2VyX2N0cmwoaGlzaV9oaWtl eV91c2IsIEhVQl9WQlVTX1BPV0VSX09OKTsKPj4gKwkJZXh0Y29uX3NldF9zdGF0ZV9zeW5jKGhp c2lfaGlrZXlfdXNiLT5lZGV2LCBFWFRDT05fVVNCLCBmYWxzZSk7Cj4+ICsJCWV4dGNvbl9zZXRf c3RhdGVfc3luYyhoaXNpX2hpa2V5X3VzYi0+ZWRldiwgRVhUQ09OX1VTQl9IT1NULAo+PiArCQkJ CXRydWUpOwo+PiArCQlicmVhazsKPj4gKwljYXNlIFVTQl9ST0xFX0hPU1Q6Cj4+ICsJCXVzYl9z d2l0Y2hfY3RybChoaXNpX2hpa2V5X3VzYiwgVVNCX1NXSVRDSF9UT19UWVBFQyk7Cj4+ICsJCXVz Yl90eXBlY19wb3dlcl9jdHJsKGhpc2lfaGlrZXlfdXNiLAo+PiArCQkJCWhpc2lfaGlrZXlfdXNi LT50eXBlY192YnVzX2VuYWJsZV92YWwpOwo+PiArCQlleHRjb25fc2V0X3N0YXRlX3N5bmMoaGlz aV9oaWtleV91c2ItPmVkZXYsIEVYVENPTl9VU0IsIGZhbHNlKTsKPj4gKwkJZXh0Y29uX3NldF9z dGF0ZV9zeW5jKGhpc2lfaGlrZXlfdXNiLT5lZGV2LCBFWFRDT05fVVNCX0hPU1QsCj4+ICsJCQkJ dHJ1ZSk7Cj4+ICsJCWJyZWFrOwo+PiArCWNhc2UgVVNCX1JPTEVfREVWSUNFOgo+PiArCQlodWJf cG93ZXJfY3RybChoaXNpX2hpa2V5X3VzYiwgSFVCX1ZCVVNfUE9XRVJfT0ZGKTsKPj4gKwkJdXNi X3R5cGVjX3Bvd2VyX2N0cmwoaGlzaV9oaWtleV91c2IsCj4+ICsJCQkJaGlzaV9oaWtleV91c2It PnR5cGVjX3ZidXNfZW5hYmxlX3ZhbCk7Cj4+ICsJCXVzYl9zd2l0Y2hfY3RybChoaXNpX2hpa2V5 X3VzYiwgVVNCX1NXSVRDSF9UT19UWVBFQyk7Cj4+ICsJCWV4dGNvbl9zZXRfc3RhdGVfc3luYyho aXNpX2hpa2V5X3VzYi0+ZWRldiwgRVhUQ09OX1VTQl9IT1NULAo+PiArCQkJCWZhbHNlKTsKPj4g KwkJZXh0Y29uX3NldF9zdGF0ZV9zeW5jKGhpc2lfaGlrZXlfdXNiLT5lZGV2LCBFWFRDT05fVVNC LCB0cnVlKTsKPj4gKwkJYnJlYWs7Cj4+ICsJfQo+IElmIEkgdW5kZXJzdG9vZCB0aGUgYWJvdmUg Y29ycmVjdGx5LCB5b3UgYXJlIGNvbnRyb2xsaW5nIHRoZSBWQlVTCj4gYmFzZWQgb24gdGhlIGRh dGEgcm9sZSwgcmlnaHQ/ClllcywgeW91IGFyZSByaWdodC4KPiBXaXRoIFVTQiBUeXBlLUMgY29u bmVjdG9ycyB0aGUgcG93ZXIgYW5kIGRhdGEgcm9sZXMgYXJlIHNlcGFyYXRlLCBzbwo+IHlvdSBz aG91bGQgbm90IGJlIGRvaW5nIHRoYXQuCj4KPiBCdXQgc2luY2UgeW91IGFyZSBjb250cm9sbGlu ZyB0aGUgVkJVUyB3aXRoIGEgc2luZ2xlIEdQSU8sIHdvdWxkbid0IGl0Cj4gYmUgZWFzaWVyIHRv IGp1c3QgZ2l2ZSB0aGUgVHlwZS1DIHBvcnQgY29udHJvbGxlciBkZXZpY2UgdGhhdCBHUElPCj4g cmVzb3VyY2VzIGFuZCBsZXQgdGhlIFR5cGUtQyBkcml2ZXJzIHRha2UgY2FyZSBvZiBpdD8gWW91 IGNvdWxkIGFkZCBhCj4gInNldF92YnVzIiBjYWxsYmFjayB0byBzdHJ1Y3QgdGNwY2lfZGF0YSBh bmQgdGFrZSBjYXJlIG9mIHRoZSBHUElPIGluCj4gdGNwY2lfcnQxNzExaC5jLCBvciBhbHRlcm5h dGl2ZWx5LCBqdXN0IGhhbmRsZSBpdCBpbiB0Y3BjaS5jLgpJIHdpbGwgdHJ5IHRvIGltcGxlbWVu dCB0aGUgInNldF92YnVzIiBjYWxsYmFjay4KCj4+ICsJcmV0dXJuIDA7Cj4+ICt9Cj4+ICsKPj4g K3N0YXRpYyBlbnVtIHVzYl9yb2xlIGV4dGNvbl9oaXNpX3BkX2dldF9yb2xlKHN0cnVjdCBkZXZp Y2UgKmRldikKPj4gK3sKPj4gKwlzdHJ1Y3QgaGlzaV9oaWtleV91c2IgKmhpc2lfaGlrZXlfdXNi ID0gZGV2X2dldF9kcnZkYXRhKGRldik7Cj4+ICsKPj4gKwlyZXR1cm4gdXNiX3JvbGVfc3dpdGNo X2dldF9yb2xlKGhpc2lfaGlrZXlfdXNiLT5yb2xlX3N3KTsKPj4gK30KPj4gKwo+PiArc3RhdGlj IGNvbnN0IHN0cnVjdCB1c2Jfcm9sZV9zd2l0Y2hfZGVzYyBzd19kZXNjID0gewo+PiArCS5zZXQg PSBleHRjb25faGlzaV9wZF9zZXRfcm9sZSwKPj4gKwkuZ2V0ID0gZXh0Y29uX2hpc2lfcGRfZ2V0 X3JvbGUsCj4+ICsJLmFsbG93X3VzZXJzcGFjZV9jb250cm9sID0gdHJ1ZSwKPj4gK307Cj4+ICsK Pj4gK3N0YXRpYyBpbnQgaGlzaV9oaWtleV91c2JfcHJvYmUoc3RydWN0IHBsYXRmb3JtX2Rldmlj ZSAqcGRldikKPj4gK3sKPj4gKwlzdHJ1Y3QgZGV2aWNlICpkZXYgPSAmcGRldi0+ZGV2Owo+PiAr CXN0cnVjdCBkZXZpY2Vfbm9kZSAqcm9vdCA9IGRldi0+b2Zfbm9kZTsKPj4gKwlzdHJ1Y3QgaGlz aV9oaWtleV91c2IgKmhpc2lfaGlrZXlfdXNiOwo+PiArCWludCByZXQ7Cj4+ICsKPj4gKwloaXNp X2hpa2V5X3VzYiA9IGRldm1fa3phbGxvYyhkZXYsIHNpemVvZigqaGlzaV9oaWtleV91c2IpLCBH RlBfS0VSTkVMKTsKPj4gKwlpZiAoIWhpc2lfaGlrZXlfdXNiKQo+PiArCQlyZXR1cm4gLUVOT01F TTsKPj4gKwo+PiArCWRldl9zZXRfbmFtZShkZXYsICJoaXNpX2hpa2V5X3VzYiIpOwo+PiArCj4+ ICsJaGlzaV9oaWtleV91c2ItPmh1Yl92YnVzX2dwaW8gPSBJTlZBTElEX0dQSU9fVkFMVUU7Cj4+ ICsJaGlzaV9oaWtleV91c2ItPm90Z19zd2l0Y2hfZ3BpbyA9IElOVkFMSURfR1BJT19WQUxVRTsK Pj4gKwloaXNpX2hpa2V5X3VzYi0+dHlwZWNfdmJ1c19ncGlvID0gSU5WQUxJRF9HUElPX1ZBTFVF Owo+PiArCj4+ICsJaGlzaV9oaWtleV91c2ItPmh1Yl92YnVzX2dwaW8gPSBvZl9nZXRfbmFtZWRf Z3Bpbyhyb290LAo+PiArCQkJImh1Yl92ZGQzM19lbl9ncGlvIiwgMCk7Cj4+ICsJaWYgKCFncGlv X2lzX3ZhbGlkKGhpc2lfaGlrZXlfdXNiLT5odWJfdmJ1c19ncGlvKSkgewo+PiArCQlwcl9lcnIo IiVzOiBodWJfdmJ1c19ncGlvIGlzIGVyclxuIiwgX19mdW5jX18pOwo+PiArCQlyZXR1cm4gaGlz aV9oaWtleV91c2ItPmh1Yl92YnVzX2dwaW87Cj4+ICsJfQo+PiArCj4+ICsJcmV0ID0gZ3Bpb19y ZXF1ZXN0KGhpc2lfaGlrZXlfdXNiLT5odWJfdmJ1c19ncGlvLCAiaHViX3ZidXNfaW50X2dwaW8i KTsKPj4gKwlpZiAocmV0KSB7Cj4+ICsJCXByX2VycigiJXM6IHJlcXVlc3QgaHViX3ZidXNfZ3Bp byBlcnJcbiIsIF9fZnVuY19fKTsKPj4gKwkJaGlzaV9oaWtleV91c2ItPmh1Yl92YnVzX2dwaW8g PSBJTlZBTElEX0dQSU9fVkFMVUU7Cj4+ICsJCXJldHVybiByZXQ7Cj4+ICsJfQo+PiArCj4+ICsJ cmV0ID0gZ3Bpb19kaXJlY3Rpb25fb3V0cHV0KGhpc2lfaGlrZXlfdXNiLT5odWJfdmJ1c19ncGlv LAo+PiArCQkJSFVCX1ZCVVNfUE9XRVJfT04pOwo+PiArCWlmIChyZXQpIHsKPj4gKwkJcHJfZXJy KCIlczogcG93ZXIgb24gaHViIHZidXMgZXJyXG4iLCBfX2Z1bmNfXyk7Cj4+ICsJCWdvdG8gZnJl ZV9ncGlvMTsKPj4gKwl9Cj4+ICsKPj4gKwloaXNpX2hpa2V5X3VzYi0+dHlwZWNfdmJ1c19ncGlv ID0gb2ZfZ2V0X25hbWVkX2dwaW8ocm9vdCwKPj4gKwkJInR5cGNfdmJ1c19pbnRfZ3Bpbyx0eXBl Yy1ncGlvcyIsIDApOwo+PiArCWlmICghZ3Bpb19pc192YWxpZChoaXNpX2hpa2V5X3VzYi0+dHlw ZWNfdmJ1c19ncGlvKSkgewo+PiArCQlwcl9lcnIoIiVzOiB0eXBlY192YnVzX2dwaW8gaXMgZXJy XG4iLCBfX2Z1bmNfXyk7Cj4+ICsJCXJldCA9IGhpc2lfaGlrZXlfdXNiLT50eXBlY192YnVzX2dw aW87Cj4+ICsJCWdvdG8gZnJlZV9ncGlvMTsKPj4gKwl9Cj4+ICsKPj4gKwlyZXQgPSBncGlvX3Jl cXVlc3QoaGlzaV9oaWtleV91c2ItPnR5cGVjX3ZidXNfZ3BpbywKPj4gKwkJCSJ0eXBjX3ZidXNf aW50X2dwaW8iKTsKPj4gKwlpZiAocmV0KSB7Cj4+ICsJCXByX2VycigiJXM6IHJlcXVlc3QgdHlw ZWNfdmJ1c19ncGlvIGVyclxuIiwgX19mdW5jX18pOwo+PiArCQloaXNpX2hpa2V5X3VzYi0+dHlw ZWNfdmJ1c19ncGlvID0gSU5WQUxJRF9HUElPX1ZBTFVFOwo+PiArCQlnb3RvIGZyZWVfZ3BpbzE7 Cj4+ICsJfQo+PiArCj4+ICsJcmV0ID0gb2ZfcHJvcGVydHlfcmVhZF91MzIocm9vdCwgInR5cGNf dmJ1c19lbmFibGVfdmFsIiwKPj4gKwkJCQkgICAmaGlzaV9oaWtleV91c2ItPnR5cGVjX3ZidXNf ZW5hYmxlX3ZhbCk7Cj4+ICsJaWYgKHJldCkgewo+PiArCQlwcl9lcnIoIiVzOiB0eXBjX3ZidXNf ZW5hYmxlX3ZhbCBjYW4ndCBnZXRcbiIsIF9fZnVuY19fKTsKPj4gKwkJZ290byBmcmVlX2dwaW8y Owo+PiArCX0KPj4gKwo+PiArCWhpc2lfaGlrZXlfdXNiLT50eXBlY192YnVzX2VuYWJsZV92YWwg PQo+PiArCQkhIWhpc2lfaGlrZXlfdXNiLT50eXBlY192YnVzX2VuYWJsZV92YWw7Cj4+ICsKPj4g KwlyZXQgPSBncGlvX2RpcmVjdGlvbl9vdXRwdXQoaGlzaV9oaWtleV91c2ItPnR5cGVjX3ZidXNf Z3BpbywKPj4gKwkJCQkgICAgaGlzaV9oaWtleV91c2ItPnR5cGVjX3ZidXNfZW5hYmxlX3ZhbCk7 Cj4+ICsJaWYgKHJldCkgewo+PiArCQlwcl9lcnIoIiVzOiBwb3dlciBvbiB0eXBlYyB2YnVzIGVy ciIsIF9fZnVuY19fKTsKPj4gKwkJZ290byBmcmVlX2dwaW8yOwo+PiArCX0KPj4gKwo+PiArCWlm IChvZl9kZXZpY2VfaXNfY29tcGF0aWJsZShyb290LCAiaGlzaWxpY29uLGhpa2V5OTYwX3VzYiIp KSB7Cj4gSW5zdGVhZCBvZiB0aGF0IGtpbmQgb2YgY2hlY2tzLCBpc24ndCBpdCBlbm91Z2ggdG8g anVzdCB1c2Ugb3B0aW9uYWwKPiBncGlvcz8KPgo+PiArCQloaXNpX2hpa2V5X3VzYi0+b3RnX3N3 aXRjaF9ncGlvID0gb2ZfZ2V0X25hbWVkX2dwaW8ocm9vdCwKPj4gKwkJCQkib3RnX2dwaW8iLCAw KTsKPj4gKwkJaWYgKCFncGlvX2lzX3ZhbGlkKGhpc2lfaGlrZXlfdXNiLT5vdGdfc3dpdGNoX2dw aW8pKSB7Cj4+ICsJCQlwcl9pbmZvKCIlczogb3RnX3N3aXRjaF9ncGlvIGlzIGVyclxuIiwgX19m dW5jX18pOwo+PiArCQkJZ290byBmcmVlX2dwaW8yOwo+PiArCQl9Cj4+ICsKPj4gKwkJcmV0ID0g Z3Bpb19yZXF1ZXN0KGhpc2lfaGlrZXlfdXNiLT5vdGdfc3dpdGNoX2dwaW8sCj4+ICsJCQkJIm90 Z19zd2l0Y2hfZ3BpbyIpOwo+PiArCQlpZiAocmV0KSB7Cj4+ICsJCQloaXNpX2hpa2V5X3VzYi0+ b3RnX3N3aXRjaF9ncGlvID0gSU5WQUxJRF9HUElPX1ZBTFVFOwo+PiArCQkJcHJfZXJyKCIlczog cmVxdWVzdCB0eXBlY192YnVzX2dwaW8gZXJyXG4iLCBfX2Z1bmNfXyk7Cj4+ICsJCQlnb3RvIGZy ZWVfZ3BpbzI7Cj4+ICsJCX0KPj4gKwl9Cj4+ICsKPj4gKwloaXNpX2hpa2V5X3VzYi0+ZWRldiA9 IGRldm1fZXh0Y29uX2Rldl9hbGxvY2F0ZShkZXYsIHVzYl9leHRjb25fY2FibGUpOwo+PiArCWlm IChJU19FUlIoaGlzaV9oaWtleV91c2ItPmVkZXYpKSB7Cj4+ICsJCWRldl9lcnIoZGV2LCAiZmFp bGVkIHRvIGFsbG9jYXRlIGV4dGNvbiBkZXZpY2VcbiIpOwo+PiArCQlnb3RvIGZyZWVfZ3BpbzI7 Cj4+ICsJfQo+PiArCj4+ICsJcmV0ID0gZGV2bV9leHRjb25fZGV2X3JlZ2lzdGVyKGRldiwgaGlz aV9oaWtleV91c2ItPmVkZXYpOwo+PiArCWlmIChyZXQgPCAwKSB7Cj4+ICsJCWRldl9lcnIoZGV2 LCAiZmFpbGVkIHRvIHJlZ2lzdGVyIGV4dGNvbiBkZXZpY2VcbiIpOwo+PiArCQlnb3RvIGZyZWVf Z3BpbzI7Cj4+ICsJfQo+PiArCWV4dGNvbl9zZXRfc3RhdGUoaGlzaV9oaWtleV91c2ItPmVkZXYs IEVYVENPTl9VU0JfSE9TVCwgdHJ1ZSk7Cj4gSXMgdGhlIHByaW1hcnkgcHVycG9zZSBmb3IgdGhp cyBleHRjb24gZGV2aWNlIHRvIHNhdGlzZnkgdGhlIERSRCBjb2RlCj4gaW4gZHdjMyBkcml2ZXI/ Clllcy4gSSBuZWVkIGl0IHRvIHN3aXRjaCBtb2RlIG9mIGR3YzMuCj4+ICsJaGlzaV9oaWtleV91 c2ItPnJvbGVfc3cgPSB1c2Jfcm9sZV9zd2l0Y2hfcmVnaXN0ZXIoZGV2LCAmc3dfZGVzYyk7Cj4+ ICsJaWYgKElTX0VSUihoaXNpX2hpa2V5X3VzYi0+cm9sZV9zdykpCj4+ICsJCWdvdG8gZnJlZV9n cGlvMjsKPiBJdCBsb29rcyBhIGJpdCBjbHVtc3kgdG8gbWUgdG8gcmVnaXN0ZXIgYm90aCB0aGUg ZXh0Y29uIGRldmljZSBhbmQgdGhlCj4gbXV4IGRldmljZSwgYnV0IEknbSBndWVzc2luZyB5b3Ug bmVlZCB0byBnZXQgYSBub3RpZmljYXRpb24gaW4gZHdjMwo+IGRyaXZlciB3aGVuIHRoZSByb2xl IGNoYW5nZXMsIHJpZ2h0PyBQZXJoYXBzIHdlIHNob3VsZCBzaW1wbHkgYWRkCj4gbm90aWZpY2F0 aW9uIGNoYWluIHRvIHRoZSByb2xlIG11eCBzdHJ1Y3R1cmUuIFRoYXQgY291bGQgcG90ZW50aWFs bHkKPiBhbGxvdyB0aGlzIGtpbmQgb2YgY29kZSB0byBiZSBvcmdhbml6ZWQgYSBiaXQgYmV0dGVy Lgo+Cj4+ICsJcGxhdGZvcm1fc2V0X2RydmRhdGEocGRldiwgaGlzaV9oaWtleV91c2IpOwo+PiAr Cj4+ICsJcmV0dXJuIDA7Cj4+ICsKPj4gK2ZyZWVfZ3BpbzI6Cj4+ICsJaWYgKGdwaW9faXNfdmFs aWQoaGlzaV9oaWtleV91c2ItPnR5cGVjX3ZidXNfZ3BpbykpIHsKPj4gKwkJZ3Bpb19mcmVlKGhp c2lfaGlrZXlfdXNiLT50eXBlY192YnVzX2dwaW8pOwo+PiArCQloaXNpX2hpa2V5X3VzYi0+dHlw ZWNfdmJ1c19ncGlvID0gSU5WQUxJRF9HUElPX1ZBTFVFOwo+PiArCX0KPj4gKwo+PiArZnJlZV9n cGlvMToKPj4gKwlpZiAoZ3Bpb19pc192YWxpZChoaXNpX2hpa2V5X3VzYi0+aHViX3ZidXNfZ3Bp bykpIHsKPj4gKwkJZ3Bpb19mcmVlKGhpc2lfaGlrZXlfdXNiLT5odWJfdmJ1c19ncGlvKTsKPj4g KwkJaGlzaV9oaWtleV91c2ItPmh1Yl92YnVzX2dwaW8gPSBJTlZBTElEX0dQSU9fVkFMVUU7Cj4+ ICsJfQo+PiArCj4+ICsJcmV0dXJuIHJldDsKPj4gK30KPj4gKwo+PiArc3RhdGljIGludCAgaGlz aV9oaWtleV91c2JfcmVtb3ZlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpCj4+ICt7Cj4+ ICsJc3RydWN0IGhpc2lfaGlrZXlfdXNiICpoaXNpX2hpa2V5X3VzYiA9IHBsYXRmb3JtX2dldF9k cnZkYXRhKHBkZXYpOwo+PiArCj4+ICsJaWYgKGdwaW9faXNfdmFsaWQoaGlzaV9oaWtleV91c2It Pm90Z19zd2l0Y2hfZ3BpbykpIHsKPj4gKwkJZ3Bpb19mcmVlKGhpc2lfaGlrZXlfdXNiLT5vdGdf c3dpdGNoX2dwaW8pOwo+PiArCQloaXNpX2hpa2V5X3VzYi0+b3RnX3N3aXRjaF9ncGlvID0gSU5W QUxJRF9HUElPX1ZBTFVFOwo+PiArCX0KPj4gKwo+PiArCWlmIChncGlvX2lzX3ZhbGlkKGhpc2lf aGlrZXlfdXNiLT50eXBlY192YnVzX2dwaW8pKSB7Cj4+ICsJCWdwaW9fZnJlZShoaXNpX2hpa2V5 X3VzYi0+dHlwZWNfdmJ1c19ncGlvKTsKPj4gKwkJaGlzaV9oaWtleV91c2ItPnR5cGVjX3ZidXNf Z3BpbyA9IElOVkFMSURfR1BJT19WQUxVRTsKPj4gKwl9Cj4+ICsKPj4gKwlpZiAoZ3Bpb19pc192 YWxpZChoaXNpX2hpa2V5X3VzYi0+aHViX3ZidXNfZ3BpbykpIHsKPj4gKwkJZ3Bpb19mcmVlKGhp c2lfaGlrZXlfdXNiLT5odWJfdmJ1c19ncGlvKTsKPj4gKwkJaGlzaV9oaWtleV91c2ItPmh1Yl92 YnVzX2dwaW8gPSBJTlZBTElEX0dQSU9fVkFMVUU7Cj4+ICsJfQo+PiArCj4+ICsJdXNiX3JvbGVf c3dpdGNoX3VucmVnaXN0ZXIoaGlzaV9oaWtleV91c2ItPnJvbGVfc3cpOwo+PiArCj4+ICsJcmV0 dXJuIDA7Cj4+ICt9Cj4+ICsKPj4gK3N0YXRpYyBjb25zdCBzdHJ1Y3Qgb2ZfZGV2aWNlX2lkIGlk X3RhYmxlX2hpc2lfaGlrZXlfdXNiW10gPSB7Cj4+ICsJey5jb21wYXRpYmxlID0gImhpc2lsaWNv bixncGlvX2h1YnYxIn0sCj4+ICsJey5jb21wYXRpYmxlID0gImhpc2lsaWNvbixoaWtleTk2MF91 c2IifSwKPj4gKwl7fQo+PiArfTsKPj4gKwo+PiArc3RhdGljIHN0cnVjdCBwbGF0Zm9ybV9kcml2 ZXIgIGhpc2lfaGlrZXlfdXNiX2RyaXZlciA9IHsKPj4gKwkucHJvYmUgPSBoaXNpX2hpa2V5X3Vz Yl9wcm9iZSwKPj4gKwkucmVtb3ZlID0gaGlzaV9oaWtleV91c2JfcmVtb3ZlLAo+PiArCS5kcml2 ZXIgPSB7Cj4+ICsJCS5uYW1lID0gREVWSUNFX0RSSVZFUl9OQU1FLAo+PiArCQkub2ZfbWF0Y2hf dGFibGUgPSBvZl9tYXRjaF9wdHIoaWRfdGFibGVfaGlzaV9oaWtleV91c2IpLAo+PiArCj4+ICsJ fSwKPj4gK307Cj4+ICsKPj4gK21vZHVsZV9wbGF0Zm9ybV9kcml2ZXIoaGlzaV9oaWtleV91c2Jf ZHJpdmVyKTsKPj4gKwo+PiArTU9EVUxFX0FVVEhPUigiWXUgQ2hlbiA8Y2hlbnl1NTZAaHVhd2Vp LmNvbT4iKTsKPj4gK01PRFVMRV9ERVNDUklQVElPTigiRHJpdmVyIFN1cHBvcnQgZm9yIFVTQiBm dW5jdGlvbmFsaXR5IG9mIEhpa2V5Iik7Cj4+ICtNT0RVTEVfTElDRU5TRSgiR1BMIHYyIik7Cj4+ IC0tIAo+PiAyLjE1LjAtcmMyCj4gSSB0aGluayB5b3UgaGF2ZSB0b28gbWFueSB0aGluZ3MgaW50 ZWdyYXRlZCBpbnRvIHRoaXMgb25lIGRyaXZlci4gSU1PCj4gaXQgd291bGQgYXQgbGVhc3QgYmUg YmV0dGVyIHRvIGp1c3QgbGV0IHRoZSBUeXBlLUMgcG9ydCBkcml2ZXIgdGFrZQo+IGNhcmUgb2Yg VkJVUyBsaWtlIEkgbWVudGlvbmVkIGFib3ZlLiBJJ20gYWxzbyB3b25kZXJpbmcgaWYgaXQgd291 bGQKPiBtYWtlIHNlbnNlIHRvIGhhbmRsZSB0aGUgcm9sZSBzd2l0Y2ggYW5kIHRoZSAiaHViIiBp biB0aGVpciBvd24KPiBkcml2ZXJzLCBidXQgSSBkb24ndCBrbm93IGVub3VnaCBhYm91dCB5b3Vy IHBsYXRmb3JtIGF0IHRoaXMgcG9pbnQgdG8KPiBzYXkgZm9yIHN1cmUuCgpUaGFua3MgZm9yIHlv dXIgYWR2aWNlISBUaGUgSGlLZXkgOTYwIGRldmVsb3BtZW50IHBsYXRmb3JtIGlzIGJhc2VkIGFy b3VuZCB0aGUgSHVhd2VpIEtpcmluIDk2MC4KVGhlIEhpa2V5OTYwIERldmVsb3BtZW50IEJvYXJk IHN1cHBvcnRzIHRocmVlIFVTQiBob3N0IHBvcnQgdmlhIGEgVVNCIGh1YiAoVTE4MDMgVVNCNTcz NCkuClRoZSBIaWtleTk2MCBEZXZlbG9wbWVudCBCb2FyZCBhbHNvIGltcGxlbWVudHMgYSBVU0Iy LjAgdHlwZUMgT1RHIHBvcnQuwqAKVGhlIERwIGFuZCBEbSBvZiBTb2MgY2FuIGJlIHN3aXRjaGVk IGJldHdlZW4gdGhlIHR5cGVDIHBvcnQgYW5kIHRoZSBVU0IgaHViLgpJZiB0aGVyZSBpcyBubyBj YWJsZSBvbiB0aGUgdHlwZUMgcG9ydCwgdGhlbiBkd2MzIGNvcmUgb2YgU29jIHdpbGwgYmUgc3dp dGNoIHRvIGhvc3QgbW9kZSBhbmQgdGhlCmRyaXZlciBvZiB0aGlzIHBhdGNoIHdpbGwgc3dpdGNo IERwIGFuZCBEcCB0byB0aGUgaHViLiBUaGUgZHJpdmVyIGFsc28gcG93ZXIgb24gdGhlIGh1YiBp biB0aGUgbWVhbnRpbWUuCj4KPiBiciwKPgo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chen Yu Subject: Re: [PATCH 07/10] hikey960: Support usb functionality of Hikey960 Date: Tue, 30 Oct 2018 10:50:22 +0800 Message-ID: <33a6ac59-b545-e2c8-2bb7-0a5460fcf5e9@huawei.com> References: <20181027095820.40056-1-chenyu56@huawei.com> <20181027095820.40056-8-chenyu56@huawei.com> <20181029143040.GB14534@kuha.fi.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <20181029143040.GB14534@kuha.fi.intel.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Heikki Krogerus Cc: wangbinghui@hisilicon.com, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, suzhuangluan@hisilicon.com, kongfei@hisilicon.com, Arnd Bergmann , Greg Kroah-Hartman , John Stultz List-Id: devicetree@vger.kernel.org Hi On 2018/10/29 22:30, Heikki Krogerus wrote: > Hi, > > On Sat, Oct 27, 2018 at 05:58:17PM +0800, Yu Chen wrote: >> This driver handles usb hub power on and typeC port event of HiKey960 board: >> 1)DP&DM switching between usb hub and typeC port base on typeC port >> state > By "hub" do you mean you have some kind of an integrated USB hub on > your SoC? No. The "hub" is on the board of HiKey960 development platform. >> 2)Control power of usb hub on Hikey960 >> 3)Control vbus of typeC port >> 4)Handle typeC port event to switch data role > Is your role switch a discrete component, or something part of the SoC? Yes, the dwc3 usb controller of the SoC needs switch between device and host mode based on the typeC port event. > >> +#define USB_SWITCH_TO_HUB 1 >> +#define USB_SWITCH_TO_TYPEC 0 >> + >> +#define INVALID_GPIO_VALUE (-1) >> + >> +struct hisi_hikey_usb { >> + int otg_switch_gpio; >> + int typec_vbus_gpio; >> + int typec_vbus_enable_val; >> + int hub_vbus_gpio; > I think you should use struct gpio_desc and gpiod_*() API with the > gpios. You are right, I will fix that. Thanks! >> + struct extcon_dev *edev; >> + struct usb_role_switch *role_sw; >> +}; >> + >> +static const unsigned int usb_extcon_cable[] = { >> + EXTCON_USB, >> + EXTCON_USB_HOST, >> + EXTCON_NONE, >> +}; >> + >> +static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value) >> +{ >> + int gpio = hisi_hikey_usb->hub_vbus_gpio; >> + >> + if (gpio_is_valid(gpio)) >> + gpio_set_value(gpio, value); >> +} >> + >> +static void usb_switch_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, >> + int switch_to) >> +{ >> + int gpio = hisi_hikey_usb->otg_switch_gpio; >> + const char *switch_to_str = (switch_to == USB_SWITCH_TO_HUB) ? >> + "hub" : "typec"; >> + >> + if (!gpio_is_valid(gpio)) { >> + pr_err("%s: otg_switch_gpio is err\n", __func__); >> + return; >> + } >> + >> + if (gpio_get_value(gpio) == switch_to) { >> + pr_info("%s: already switch to %s\n", __func__, switch_to_str); > That kind of prints are really just noise. > >> + return; >> + } >> + >> + gpio_direction_output(gpio, switch_to); >> + pr_info("%s: switch to %s\n", __func__, switch_to_str); > That is also just noise. > >> +} >> + >> +static void usb_typec_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, >> + int value) >> +{ >> + int gpio = hisi_hikey_usb->typec_vbus_gpio; >> + >> + if (!gpio_is_valid(gpio)) { >> + pr_err("%s: typec power gpio is err\n", __func__); >> + return; >> + } >> + >> + if (gpio_get_value(gpio) == value) { >> + pr_info("%s: typec power no change\n", __func__); > Ditto. > >> + return; >> + } >> + >> + gpio_direction_output(gpio, value); >> + pr_info("%s: set typec vbus gpio to %d\n", __func__, value); > Ditto. > >> +} >> + >> +static int extcon_hisi_pd_set_role(struct device *dev, enum usb_role role) >> +{ >> + struct hisi_hikey_usb *hisi_hikey_usb = dev_get_drvdata(dev); >> + >> + dev_info(dev, "%s:set usb role to %d\n", __func__, role); >> + switch (role) { >> + case USB_ROLE_NONE: >> + usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_HUB); >> + usb_typec_power_ctrl(hisi_hikey_usb, >> + !hisi_hikey_usb->typec_vbus_enable_val); >> + hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_ON); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB, false); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB_HOST, >> + true); >> + break; >> + case USB_ROLE_HOST: >> + usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC); >> + usb_typec_power_ctrl(hisi_hikey_usb, >> + hisi_hikey_usb->typec_vbus_enable_val); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB, false); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB_HOST, >> + true); >> + break; >> + case USB_ROLE_DEVICE: >> + hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_OFF); >> + usb_typec_power_ctrl(hisi_hikey_usb, >> + hisi_hikey_usb->typec_vbus_enable_val); >> + usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB_HOST, >> + false); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB, true); >> + break; >> + } > If I understood the above correctly, you are controlling the VBUS > based on the data role, right? Yes, you are right. > With USB Type-C connectors the power and data roles are separate, so > you should not be doing that. > > But since you are controlling the VBUS with a single GPIO, wouldn't it > be easier to just give the Type-C port controller device that GPIO > resources and let the Type-C drivers take care of it? You could add a > "set_vbus" callback to struct tcpci_data and take care of the GPIO in > tcpci_rt1711h.c, or alternatively, just handle it in tcpci.c. I will try to implement the "set_vbus" callback. >> + return 0; >> +} >> + >> +static enum usb_role extcon_hisi_pd_get_role(struct device *dev) >> +{ >> + struct hisi_hikey_usb *hisi_hikey_usb = dev_get_drvdata(dev); >> + >> + return usb_role_switch_get_role(hisi_hikey_usb->role_sw); >> +} >> + >> +static const struct usb_role_switch_desc sw_desc = { >> + .set = extcon_hisi_pd_set_role, >> + .get = extcon_hisi_pd_get_role, >> + .allow_userspace_control = true, >> +}; >> + >> +static int hisi_hikey_usb_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct device_node *root = dev->of_node; >> + struct hisi_hikey_usb *hisi_hikey_usb; >> + int ret; >> + >> + hisi_hikey_usb = devm_kzalloc(dev, sizeof(*hisi_hikey_usb), GFP_KERNEL); >> + if (!hisi_hikey_usb) >> + return -ENOMEM; >> + >> + dev_set_name(dev, "hisi_hikey_usb"); >> + >> + hisi_hikey_usb->hub_vbus_gpio = INVALID_GPIO_VALUE; >> + hisi_hikey_usb->otg_switch_gpio = INVALID_GPIO_VALUE; >> + hisi_hikey_usb->typec_vbus_gpio = INVALID_GPIO_VALUE; >> + >> + hisi_hikey_usb->hub_vbus_gpio = of_get_named_gpio(root, >> + "hub_vdd33_en_gpio", 0); >> + if (!gpio_is_valid(hisi_hikey_usb->hub_vbus_gpio)) { >> + pr_err("%s: hub_vbus_gpio is err\n", __func__); >> + return hisi_hikey_usb->hub_vbus_gpio; >> + } >> + >> + ret = gpio_request(hisi_hikey_usb->hub_vbus_gpio, "hub_vbus_int_gpio"); >> + if (ret) { >> + pr_err("%s: request hub_vbus_gpio err\n", __func__); >> + hisi_hikey_usb->hub_vbus_gpio = INVALID_GPIO_VALUE; >> + return ret; >> + } >> + >> + ret = gpio_direction_output(hisi_hikey_usb->hub_vbus_gpio, >> + HUB_VBUS_POWER_ON); >> + if (ret) { >> + pr_err("%s: power on hub vbus err\n", __func__); >> + goto free_gpio1; >> + } >> + >> + hisi_hikey_usb->typec_vbus_gpio = of_get_named_gpio(root, >> + "typc_vbus_int_gpio,typec-gpios", 0); >> + if (!gpio_is_valid(hisi_hikey_usb->typec_vbus_gpio)) { >> + pr_err("%s: typec_vbus_gpio is err\n", __func__); >> + ret = hisi_hikey_usb->typec_vbus_gpio; >> + goto free_gpio1; >> + } >> + >> + ret = gpio_request(hisi_hikey_usb->typec_vbus_gpio, >> + "typc_vbus_int_gpio"); >> + if (ret) { >> + pr_err("%s: request typec_vbus_gpio err\n", __func__); >> + hisi_hikey_usb->typec_vbus_gpio = INVALID_GPIO_VALUE; >> + goto free_gpio1; >> + } >> + >> + ret = of_property_read_u32(root, "typc_vbus_enable_val", >> + &hisi_hikey_usb->typec_vbus_enable_val); >> + if (ret) { >> + pr_err("%s: typc_vbus_enable_val can't get\n", __func__); >> + goto free_gpio2; >> + } >> + >> + hisi_hikey_usb->typec_vbus_enable_val = >> + !!hisi_hikey_usb->typec_vbus_enable_val; >> + >> + ret = gpio_direction_output(hisi_hikey_usb->typec_vbus_gpio, >> + hisi_hikey_usb->typec_vbus_enable_val); >> + if (ret) { >> + pr_err("%s: power on typec vbus err", __func__); >> + goto free_gpio2; >> + } >> + >> + if (of_device_is_compatible(root, "hisilicon,hikey960_usb")) { > Instead of that kind of checks, isn't it enough to just use optional > gpios? > >> + hisi_hikey_usb->otg_switch_gpio = of_get_named_gpio(root, >> + "otg_gpio", 0); >> + if (!gpio_is_valid(hisi_hikey_usb->otg_switch_gpio)) { >> + pr_info("%s: otg_switch_gpio is err\n", __func__); >> + goto free_gpio2; >> + } >> + >> + ret = gpio_request(hisi_hikey_usb->otg_switch_gpio, >> + "otg_switch_gpio"); >> + if (ret) { >> + hisi_hikey_usb->otg_switch_gpio = INVALID_GPIO_VALUE; >> + pr_err("%s: request typec_vbus_gpio err\n", __func__); >> + goto free_gpio2; >> + } >> + } >> + >> + hisi_hikey_usb->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable); >> + if (IS_ERR(hisi_hikey_usb->edev)) { >> + dev_err(dev, "failed to allocate extcon device\n"); >> + goto free_gpio2; >> + } >> + >> + ret = devm_extcon_dev_register(dev, hisi_hikey_usb->edev); >> + if (ret < 0) { >> + dev_err(dev, "failed to register extcon device\n"); >> + goto free_gpio2; >> + } >> + extcon_set_state(hisi_hikey_usb->edev, EXTCON_USB_HOST, true); > Is the primary purpose for this extcon device to satisfy the DRD code > in dwc3 driver? Yes. I need it to switch mode of dwc3. >> + hisi_hikey_usb->role_sw = usb_role_switch_register(dev, &sw_desc); >> + if (IS_ERR(hisi_hikey_usb->role_sw)) >> + goto free_gpio2; > It looks a bit clumsy to me to register both the extcon device and the > mux device, but I'm guessing you need to get a notification in dwc3 > driver when the role changes, right? Perhaps we should simply add > notification chain to the role mux structure. That could potentially > allow this kind of code to be organized a bit better. > >> + platform_set_drvdata(pdev, hisi_hikey_usb); >> + >> + return 0; >> + >> +free_gpio2: >> + if (gpio_is_valid(hisi_hikey_usb->typec_vbus_gpio)) { >> + gpio_free(hisi_hikey_usb->typec_vbus_gpio); >> + hisi_hikey_usb->typec_vbus_gpio = INVALID_GPIO_VALUE; >> + } >> + >> +free_gpio1: >> + if (gpio_is_valid(hisi_hikey_usb->hub_vbus_gpio)) { >> + gpio_free(hisi_hikey_usb->hub_vbus_gpio); >> + hisi_hikey_usb->hub_vbus_gpio = INVALID_GPIO_VALUE; >> + } >> + >> + return ret; >> +} >> + >> +static int hisi_hikey_usb_remove(struct platform_device *pdev) >> +{ >> + struct hisi_hikey_usb *hisi_hikey_usb = platform_get_drvdata(pdev); >> + >> + if (gpio_is_valid(hisi_hikey_usb->otg_switch_gpio)) { >> + gpio_free(hisi_hikey_usb->otg_switch_gpio); >> + hisi_hikey_usb->otg_switch_gpio = INVALID_GPIO_VALUE; >> + } >> + >> + if (gpio_is_valid(hisi_hikey_usb->typec_vbus_gpio)) { >> + gpio_free(hisi_hikey_usb->typec_vbus_gpio); >> + hisi_hikey_usb->typec_vbus_gpio = INVALID_GPIO_VALUE; >> + } >> + >> + if (gpio_is_valid(hisi_hikey_usb->hub_vbus_gpio)) { >> + gpio_free(hisi_hikey_usb->hub_vbus_gpio); >> + hisi_hikey_usb->hub_vbus_gpio = INVALID_GPIO_VALUE; >> + } >> + >> + usb_role_switch_unregister(hisi_hikey_usb->role_sw); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id id_table_hisi_hikey_usb[] = { >> + {.compatible = "hisilicon,gpio_hubv1"}, >> + {.compatible = "hisilicon,hikey960_usb"}, >> + {} >> +}; >> + >> +static struct platform_driver hisi_hikey_usb_driver = { >> + .probe = hisi_hikey_usb_probe, >> + .remove = hisi_hikey_usb_remove, >> + .driver = { >> + .name = DEVICE_DRIVER_NAME, >> + .of_match_table = of_match_ptr(id_table_hisi_hikey_usb), >> + >> + }, >> +}; >> + >> +module_platform_driver(hisi_hikey_usb_driver); >> + >> +MODULE_AUTHOR("Yu Chen "); >> +MODULE_DESCRIPTION("Driver Support for USB functionality of Hikey"); >> +MODULE_LICENSE("GPL v2"); >> -- >> 2.15.0-rc2 > I think you have too many things integrated into this one driver. IMO > it would at least be better to just let the Type-C port driver take > care of VBUS like I mentioned above. I'm also wondering if it would > make sense to handle the role switch and the "hub" in their own > drivers, but I don't know enough about your platform at this point to > say for sure. Thanks for your advice! The HiKey 960 development platform is based around the Huawei Kirin 960. The Hikey960 Development Board supports three USB host port via a USB hub (U1803 USB5734). The Hikey960 Development Board also implements a USB2.0 typeC OTG port.  The Dp and Dm of Soc can be switched between the typeC port and the USB hub. If there is no cable on the typeC port, then dwc3 core of Soc will be switch to host mode and the driver of this patch will switch Dp and Dp to the hub. The driver also power on the hub in the meantime. > > br, > 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=-0.8 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 E16C6C0044C for ; Tue, 30 Oct 2018 02:50:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9196E20824 for ; Tue, 30 Oct 2018 02:50:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9196E20824 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.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 S1726441AbeJ3LmI (ORCPT ); Tue, 30 Oct 2018 07:42:08 -0400 Received: from szxga05-in.huawei.com ([45.249.212.191]:14159 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725964AbeJ3LmH (ORCPT ); Tue, 30 Oct 2018 07:42:07 -0400 Received: from DGGEMS411-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id E1770DAF4F206; Tue, 30 Oct 2018 10:50:29 +0800 (CST) Received: from [127.0.0.1] (10.142.63.192) by DGGEMS411-HUB.china.huawei.com (10.3.19.211) with Microsoft SMTP Server id 14.3.408.0; Tue, 30 Oct 2018 10:50:25 +0800 CC: , , , , , , Arnd Bergmann , Greg Kroah-Hartman , John Stultz Subject: Re: [PATCH 07/10] hikey960: Support usb functionality of Hikey960 To: Heikki Krogerus References: <20181027095820.40056-1-chenyu56@huawei.com> <20181027095820.40056-8-chenyu56@huawei.com> <20181029143040.GB14534@kuha.fi.intel.com> From: Chen Yu Message-ID: <33a6ac59-b545-e2c8-2bb7-0a5460fcf5e9@huawei.com> Date: Tue, 30 Oct 2018 10:50:22 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20181029143040.GB14534@kuha.fi.intel.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [10.142.63.192] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi On 2018/10/29 22:30, Heikki Krogerus wrote: > Hi, > > On Sat, Oct 27, 2018 at 05:58:17PM +0800, Yu Chen wrote: >> This driver handles usb hub power on and typeC port event of HiKey960 board: >> 1)DP&DM switching between usb hub and typeC port base on typeC port >> state > By "hub" do you mean you have some kind of an integrated USB hub on > your SoC? No. The "hub" is on the board of HiKey960 development platform. >> 2)Control power of usb hub on Hikey960 >> 3)Control vbus of typeC port >> 4)Handle typeC port event to switch data role > Is your role switch a discrete component, or something part of the SoC? Yes, the dwc3 usb controller of the SoC needs switch between device and host mode based on the typeC port event. > >> +#define USB_SWITCH_TO_HUB 1 >> +#define USB_SWITCH_TO_TYPEC 0 >> + >> +#define INVALID_GPIO_VALUE (-1) >> + >> +struct hisi_hikey_usb { >> + int otg_switch_gpio; >> + int typec_vbus_gpio; >> + int typec_vbus_enable_val; >> + int hub_vbus_gpio; > I think you should use struct gpio_desc and gpiod_*() API with the > gpios. You are right, I will fix that. Thanks! >> + struct extcon_dev *edev; >> + struct usb_role_switch *role_sw; >> +}; >> + >> +static const unsigned int usb_extcon_cable[] = { >> + EXTCON_USB, >> + EXTCON_USB_HOST, >> + EXTCON_NONE, >> +}; >> + >> +static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value) >> +{ >> + int gpio = hisi_hikey_usb->hub_vbus_gpio; >> + >> + if (gpio_is_valid(gpio)) >> + gpio_set_value(gpio, value); >> +} >> + >> +static void usb_switch_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, >> + int switch_to) >> +{ >> + int gpio = hisi_hikey_usb->otg_switch_gpio; >> + const char *switch_to_str = (switch_to == USB_SWITCH_TO_HUB) ? >> + "hub" : "typec"; >> + >> + if (!gpio_is_valid(gpio)) { >> + pr_err("%s: otg_switch_gpio is err\n", __func__); >> + return; >> + } >> + >> + if (gpio_get_value(gpio) == switch_to) { >> + pr_info("%s: already switch to %s\n", __func__, switch_to_str); > That kind of prints are really just noise. > >> + return; >> + } >> + >> + gpio_direction_output(gpio, switch_to); >> + pr_info("%s: switch to %s\n", __func__, switch_to_str); > That is also just noise. > >> +} >> + >> +static void usb_typec_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, >> + int value) >> +{ >> + int gpio = hisi_hikey_usb->typec_vbus_gpio; >> + >> + if (!gpio_is_valid(gpio)) { >> + pr_err("%s: typec power gpio is err\n", __func__); >> + return; >> + } >> + >> + if (gpio_get_value(gpio) == value) { >> + pr_info("%s: typec power no change\n", __func__); > Ditto. > >> + return; >> + } >> + >> + gpio_direction_output(gpio, value); >> + pr_info("%s: set typec vbus gpio to %d\n", __func__, value); > Ditto. > >> +} >> + >> +static int extcon_hisi_pd_set_role(struct device *dev, enum usb_role role) >> +{ >> + struct hisi_hikey_usb *hisi_hikey_usb = dev_get_drvdata(dev); >> + >> + dev_info(dev, "%s:set usb role to %d\n", __func__, role); >> + switch (role) { >> + case USB_ROLE_NONE: >> + usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_HUB); >> + usb_typec_power_ctrl(hisi_hikey_usb, >> + !hisi_hikey_usb->typec_vbus_enable_val); >> + hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_ON); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB, false); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB_HOST, >> + true); >> + break; >> + case USB_ROLE_HOST: >> + usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC); >> + usb_typec_power_ctrl(hisi_hikey_usb, >> + hisi_hikey_usb->typec_vbus_enable_val); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB, false); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB_HOST, >> + true); >> + break; >> + case USB_ROLE_DEVICE: >> + hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_OFF); >> + usb_typec_power_ctrl(hisi_hikey_usb, >> + hisi_hikey_usb->typec_vbus_enable_val); >> + usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB_HOST, >> + false); >> + extcon_set_state_sync(hisi_hikey_usb->edev, EXTCON_USB, true); >> + break; >> + } > If I understood the above correctly, you are controlling the VBUS > based on the data role, right? Yes, you are right. > With USB Type-C connectors the power and data roles are separate, so > you should not be doing that. > > But since you are controlling the VBUS with a single GPIO, wouldn't it > be easier to just give the Type-C port controller device that GPIO > resources and let the Type-C drivers take care of it? You could add a > "set_vbus" callback to struct tcpci_data and take care of the GPIO in > tcpci_rt1711h.c, or alternatively, just handle it in tcpci.c. I will try to implement the "set_vbus" callback. >> + return 0; >> +} >> + >> +static enum usb_role extcon_hisi_pd_get_role(struct device *dev) >> +{ >> + struct hisi_hikey_usb *hisi_hikey_usb = dev_get_drvdata(dev); >> + >> + return usb_role_switch_get_role(hisi_hikey_usb->role_sw); >> +} >> + >> +static const struct usb_role_switch_desc sw_desc = { >> + .set = extcon_hisi_pd_set_role, >> + .get = extcon_hisi_pd_get_role, >> + .allow_userspace_control = true, >> +}; >> + >> +static int hisi_hikey_usb_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct device_node *root = dev->of_node; >> + struct hisi_hikey_usb *hisi_hikey_usb; >> + int ret; >> + >> + hisi_hikey_usb = devm_kzalloc(dev, sizeof(*hisi_hikey_usb), GFP_KERNEL); >> + if (!hisi_hikey_usb) >> + return -ENOMEM; >> + >> + dev_set_name(dev, "hisi_hikey_usb"); >> + >> + hisi_hikey_usb->hub_vbus_gpio = INVALID_GPIO_VALUE; >> + hisi_hikey_usb->otg_switch_gpio = INVALID_GPIO_VALUE; >> + hisi_hikey_usb->typec_vbus_gpio = INVALID_GPIO_VALUE; >> + >> + hisi_hikey_usb->hub_vbus_gpio = of_get_named_gpio(root, >> + "hub_vdd33_en_gpio", 0); >> + if (!gpio_is_valid(hisi_hikey_usb->hub_vbus_gpio)) { >> + pr_err("%s: hub_vbus_gpio is err\n", __func__); >> + return hisi_hikey_usb->hub_vbus_gpio; >> + } >> + >> + ret = gpio_request(hisi_hikey_usb->hub_vbus_gpio, "hub_vbus_int_gpio"); >> + if (ret) { >> + pr_err("%s: request hub_vbus_gpio err\n", __func__); >> + hisi_hikey_usb->hub_vbus_gpio = INVALID_GPIO_VALUE; >> + return ret; >> + } >> + >> + ret = gpio_direction_output(hisi_hikey_usb->hub_vbus_gpio, >> + HUB_VBUS_POWER_ON); >> + if (ret) { >> + pr_err("%s: power on hub vbus err\n", __func__); >> + goto free_gpio1; >> + } >> + >> + hisi_hikey_usb->typec_vbus_gpio = of_get_named_gpio(root, >> + "typc_vbus_int_gpio,typec-gpios", 0); >> + if (!gpio_is_valid(hisi_hikey_usb->typec_vbus_gpio)) { >> + pr_err("%s: typec_vbus_gpio is err\n", __func__); >> + ret = hisi_hikey_usb->typec_vbus_gpio; >> + goto free_gpio1; >> + } >> + >> + ret = gpio_request(hisi_hikey_usb->typec_vbus_gpio, >> + "typc_vbus_int_gpio"); >> + if (ret) { >> + pr_err("%s: request typec_vbus_gpio err\n", __func__); >> + hisi_hikey_usb->typec_vbus_gpio = INVALID_GPIO_VALUE; >> + goto free_gpio1; >> + } >> + >> + ret = of_property_read_u32(root, "typc_vbus_enable_val", >> + &hisi_hikey_usb->typec_vbus_enable_val); >> + if (ret) { >> + pr_err("%s: typc_vbus_enable_val can't get\n", __func__); >> + goto free_gpio2; >> + } >> + >> + hisi_hikey_usb->typec_vbus_enable_val = >> + !!hisi_hikey_usb->typec_vbus_enable_val; >> + >> + ret = gpio_direction_output(hisi_hikey_usb->typec_vbus_gpio, >> + hisi_hikey_usb->typec_vbus_enable_val); >> + if (ret) { >> + pr_err("%s: power on typec vbus err", __func__); >> + goto free_gpio2; >> + } >> + >> + if (of_device_is_compatible(root, "hisilicon,hikey960_usb")) { > Instead of that kind of checks, isn't it enough to just use optional > gpios? > >> + hisi_hikey_usb->otg_switch_gpio = of_get_named_gpio(root, >> + "otg_gpio", 0); >> + if (!gpio_is_valid(hisi_hikey_usb->otg_switch_gpio)) { >> + pr_info("%s: otg_switch_gpio is err\n", __func__); >> + goto free_gpio2; >> + } >> + >> + ret = gpio_request(hisi_hikey_usb->otg_switch_gpio, >> + "otg_switch_gpio"); >> + if (ret) { >> + hisi_hikey_usb->otg_switch_gpio = INVALID_GPIO_VALUE; >> + pr_err("%s: request typec_vbus_gpio err\n", __func__); >> + goto free_gpio2; >> + } >> + } >> + >> + hisi_hikey_usb->edev = devm_extcon_dev_allocate(dev, usb_extcon_cable); >> + if (IS_ERR(hisi_hikey_usb->edev)) { >> + dev_err(dev, "failed to allocate extcon device\n"); >> + goto free_gpio2; >> + } >> + >> + ret = devm_extcon_dev_register(dev, hisi_hikey_usb->edev); >> + if (ret < 0) { >> + dev_err(dev, "failed to register extcon device\n"); >> + goto free_gpio2; >> + } >> + extcon_set_state(hisi_hikey_usb->edev, EXTCON_USB_HOST, true); > Is the primary purpose for this extcon device to satisfy the DRD code > in dwc3 driver? Yes. I need it to switch mode of dwc3. >> + hisi_hikey_usb->role_sw = usb_role_switch_register(dev, &sw_desc); >> + if (IS_ERR(hisi_hikey_usb->role_sw)) >> + goto free_gpio2; > It looks a bit clumsy to me to register both the extcon device and the > mux device, but I'm guessing you need to get a notification in dwc3 > driver when the role changes, right? Perhaps we should simply add > notification chain to the role mux structure. That could potentially > allow this kind of code to be organized a bit better. > >> + platform_set_drvdata(pdev, hisi_hikey_usb); >> + >> + return 0; >> + >> +free_gpio2: >> + if (gpio_is_valid(hisi_hikey_usb->typec_vbus_gpio)) { >> + gpio_free(hisi_hikey_usb->typec_vbus_gpio); >> + hisi_hikey_usb->typec_vbus_gpio = INVALID_GPIO_VALUE; >> + } >> + >> +free_gpio1: >> + if (gpio_is_valid(hisi_hikey_usb->hub_vbus_gpio)) { >> + gpio_free(hisi_hikey_usb->hub_vbus_gpio); >> + hisi_hikey_usb->hub_vbus_gpio = INVALID_GPIO_VALUE; >> + } >> + >> + return ret; >> +} >> + >> +static int hisi_hikey_usb_remove(struct platform_device *pdev) >> +{ >> + struct hisi_hikey_usb *hisi_hikey_usb = platform_get_drvdata(pdev); >> + >> + if (gpio_is_valid(hisi_hikey_usb->otg_switch_gpio)) { >> + gpio_free(hisi_hikey_usb->otg_switch_gpio); >> + hisi_hikey_usb->otg_switch_gpio = INVALID_GPIO_VALUE; >> + } >> + >> + if (gpio_is_valid(hisi_hikey_usb->typec_vbus_gpio)) { >> + gpio_free(hisi_hikey_usb->typec_vbus_gpio); >> + hisi_hikey_usb->typec_vbus_gpio = INVALID_GPIO_VALUE; >> + } >> + >> + if (gpio_is_valid(hisi_hikey_usb->hub_vbus_gpio)) { >> + gpio_free(hisi_hikey_usb->hub_vbus_gpio); >> + hisi_hikey_usb->hub_vbus_gpio = INVALID_GPIO_VALUE; >> + } >> + >> + usb_role_switch_unregister(hisi_hikey_usb->role_sw); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id id_table_hisi_hikey_usb[] = { >> + {.compatible = "hisilicon,gpio_hubv1"}, >> + {.compatible = "hisilicon,hikey960_usb"}, >> + {} >> +}; >> + >> +static struct platform_driver hisi_hikey_usb_driver = { >> + .probe = hisi_hikey_usb_probe, >> + .remove = hisi_hikey_usb_remove, >> + .driver = { >> + .name = DEVICE_DRIVER_NAME, >> + .of_match_table = of_match_ptr(id_table_hisi_hikey_usb), >> + >> + }, >> +}; >> + >> +module_platform_driver(hisi_hikey_usb_driver); >> + >> +MODULE_AUTHOR("Yu Chen "); >> +MODULE_DESCRIPTION("Driver Support for USB functionality of Hikey"); >> +MODULE_LICENSE("GPL v2"); >> -- >> 2.15.0-rc2 > I think you have too many things integrated into this one driver. IMO > it would at least be better to just let the Type-C port driver take > care of VBUS like I mentioned above. I'm also wondering if it would > make sense to handle the role switch and the "hub" in their own > drivers, but I don't know enough about your platform at this point to > say for sure. Thanks for your advice! The HiKey 960 development platform is based around the Huawei Kirin 960. The Hikey960 Development Board supports three USB host port via a USB hub (U1803 USB5734). The Hikey960 Development Board also implements a USB2.0 typeC OTG port.  The Dp and Dm of Soc can be switched between the typeC port and the USB hub. If there is no cable on the typeC port, then dwc3 core of Soc will be switch to host mode and the driver of this patch will switch Dp and Dp to the hub. The driver also power on the hub in the meantime. > > br, >