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: Heikki Krogerus Message-Id: <20181030101635.GC14534@kuha.fi.intel.com> Date: Tue, 30 Oct 2018 12:16:35 +0200 To: Chen Yu 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: T24gVHVlLCBPY3QgMzAsIDIwMTggYXQgMTA6NTA6MjJBTSArMDgwMCwgQ2hlbiBZdSB3cm90ZToK PiA+IEkgdGhpbmsgeW91IGhhdmUgdG9vIG1hbnkgdGhpbmdzIGludGVncmF0ZWQgaW50byB0aGlz IG9uZSBkcml2ZXIuIElNTwo+ID4gaXQgd291bGQgYXQgbGVhc3QgYmUgYmV0dGVyIHRvIGp1c3Qg bGV0IHRoZSBUeXBlLUMgcG9ydCBkcml2ZXIgdGFrZQo+ID4gY2FyZSBvZiBWQlVTIGxpa2UgSSBt ZW50aW9uZWQgYWJvdmUuIEknbSBhbHNvIHdvbmRlcmluZyBpZiBpdCB3b3VsZAo+ID4gbWFrZSBz ZW5zZSB0byBoYW5kbGUgdGhlIHJvbGUgc3dpdGNoIGFuZCB0aGUgImh1YiIgaW4gdGhlaXIgb3du Cj4gPiBkcml2ZXJzLCBidXQgSSBkb24ndCBrbm93IGVub3VnaCBhYm91dCB5b3VyIHBsYXRmb3Jt IGF0IHRoaXMgcG9pbnQgdG8KPiA+IHNheSBmb3Igc3VyZS4KPiAKPiBUaGFua3MgZm9yIHlvdXIg YWR2aWNlISBUaGUgSGlLZXkgOTYwIGRldmVsb3BtZW50IHBsYXRmb3JtIGlzIGJhc2VkIGFyb3Vu ZCB0aGUgSHVhd2VpIEtpcmluIDk2MC4KPiBUaGUgSGlrZXk5NjAgRGV2ZWxvcG1lbnQgQm9hcmQg c3VwcG9ydHMgdGhyZWUgVVNCIGhvc3QgcG9ydCB2aWEgYSBVU0IgaHViIChVMTgwMyBVU0I1NzM0 KS4KPiBUaGUgSGlrZXk5NjAgRGV2ZWxvcG1lbnQgQm9hcmQgYWxzbyBpbXBsZW1lbnRzIGEgVVNC Mi4wIHR5cGVDIE9URyBwb3J0Lj8/Cj4gVGhlIERwIGFuZCBEbSBvZiBTb2MgY2FuIGJlIHN3aXRj aGVkIGJldHdlZW4gdGhlIHR5cGVDIHBvcnQgYW5kIHRoZSBVU0IgaHViLgo+IElmIHRoZXJlIGlz IG5vIGNhYmxlIG9uIHRoZSB0eXBlQyBwb3J0LCB0aGVuIGR3YzMgY29yZSBvZiBTb2Mgd2lsbCBi ZSBzd2l0Y2ggdG8gaG9zdCBtb2RlIGFuZCB0aGUKPiBkcml2ZXIgb2YgdGhpcyBwYXRjaCB3aWxs IHN3aXRjaCBEcCBhbmQgRHAgdG8gdGhlIGh1Yi4gVGhlIGRyaXZlciBhbHNvIHBvd2VyIG9uIHRo ZSBodWIgaW4gdGhlIG1lYW50aW1lLgoKVGhhbmsgeW91IGZvciB0aGUgZXhwbGFuYXRpb24uIEkg Z290IHRoZSBwaWN0dXJlIG5vdy4gSSByZWFsaXplZCB0aGF0CnRoZXJlIGlzIHNvbWUgb25saW5l IGluZm9ybWF0aW9uIGZvciB0aGlzIGJvYXJkOgpodHRwczovL3d3dy45NmJvYXJkcy5vcmcvZG9j dW1lbnRhdGlvbi9jb25zdW1lci9oaWtleS9oaWtleTk2MC9oYXJkd2FyZS1kb2NzL2hhcmR3YXJl LXVzZXItbWFudWFsLm1kLmh0bWwjdXNiLXBvcnRzCgpTbyB0aGF0IG11eCBpcyBhY3R1YWxseSBf bm90XyBzd2l0Y2hpbmcgYmV0d2VlbiB0aGUgaG9zdCBhbmQgZGV2aWNlCm1vZGVzLCBidXQgaW5z dGVhZCwgaXQncyBzd2l0Y2hpbmcgYmV0d2VlbiBUeXBlLUMgYW5kIFR5cGUtQQpjb25uZWN0b3Jz IChJJ20gc2tpcHBpbmcgdGhlIGh1YiwgYXMgaXQncyBpcnJlbGV2YW50IGZyb20gdGhlIFBvVyBv Zgp0aGUgbXV4KSwgc28gSSd2ZSBtaXN1bmRlcnN0b29kLiBBbmQgeWVzLCB5b3UgZGlkIHNheSB0 aGF0IGl0IGlzCnN3aXRjaGluZyBiZXR3ZWVuIGNvbm5lY3RvcnMgaW4gdGhlIGNvbW1pdCBtZXNz YWdlLCBidXQgSSBnb3QgY29uZnVzZWQKYmVjYXVzZSB5b3UgYXJlIHJlZ2lzdGVycyBhIHJvbGUg c3dpdGNoLgoKSSBkb24ndCB0aGluayB5b3Ugc2hvdWxkIHJlZ2lzdGVyIGEgcm9sZSBzd2l0Y2gg ZnJvbSB0aGlzIGRyaXZlci4gVGhpcwpkcml2ZXIgaXMgbm90IHJlYWxseSByZXByZXNlbnRpbmcg VVNCIHJvbGUgc3dpdGNoLiBUaGUgbXV4IG9uIHRoaXMKYm9hcmQgaXMgc29tZXRoaW5nIGVsc2Uu IEluc3RlYWQgeW91IHNob3VsZCByZWdpc3RlciB0aGUgcm9sZSBzd2l0Y2gKZnJvbSB0aGUgZHdj MyBkcmQgY29kZS4gVGhhdCBpcyB0aGUgcGFydCB0aGF0IGlzIHJlcHJlc2VudGluZyB0aGUgcm9s ZQpzd2l0Y2ggaGVyZS4gSSBhY3R1YWxseSB0aGluayB0aGF0IHdlIHNob3VsZCBkbyB0aGF0IGlu IGFueSBjYXNlLiBUaGUKZHdjMyBkcmQgY29kZSBzaG91bGQgYWx3YXlzIHJlZ2lzdGVyIGEgcm9s ZSBzd2l0Y2guCgpXZSBjYW4gc29sdmUgdGhlIHByb2JsZW0gb2YgZ2V0dGluZyB0aGUgcm9sZSBj aGFuZ2UgZXZlbnRzIGluIHRoaXMKZHJpdmVyIGJ5IGFkZGluZyBub3RpZmljYXRpb24gY2hhaW4g dG8gc3RydWN0IHVzYl9yb2xlX3N3aXRjaCAoY2hlY2sKdGhlIGF0dGFjaGVkIGRpZmYpLiBZb3Ug d291bGQgdGhlbiBqdXN0IG5lZWQgdG8gY2FsbAp1c2Jfcm9sZV9zd2l0Y2hfZ2V0KCkgYW5kIHVz Yl9yb2xlX3N3aXRjaF9yZWdpc3Rlcl9ub3RpZmllcigpLCBhbmQKd2FpdCBmb3IgdGhvc2Ugbm90 aWZpY2F0aW9ucy4gVGhlIGV4dGNvbiBkZXZpY2UgZG9lcyBub3Qgc2VydmUgYW55CnB1cnBvc2Ug YW55bW9yZS4KClRoaXMgZHJpdmVyIHdvdWxkIGNvbnRpbnVlIHRvIHRha2UgY2FyZSBvZiB0aGUg aHViIHBvd2VyaW5nIGFuZCB0aGUKbXV4LCBhbmQgYWxzbyB0aGUgVkJVUyBsaWtlIGJlZm9yZS4K CgpiciwKCmRpZmYgLS1naXQgYS9kcml2ZXJzL3VzYi9jb21tb24vcm9sZXMuYyBiL2RyaXZlcnMv dXNiL2NvbW1vbi9yb2xlcy5jCmluZGV4IGJiNTJlMDA2ZDIwMy4uMjg4MTc3N2MxNmU1IDEwMDY0 NAotLS0gYS9kcml2ZXJzL3VzYi9jb21tb24vcm9sZXMuYworKysgYi9kcml2ZXJzL3VzYi9jb21t b24vcm9sZXMuYwpAQCAtMjAsNiArMjAsNyBAQCBzdHJ1Y3QgdXNiX3JvbGVfc3dpdGNoIHsKIAlz dHJ1Y3QgZGV2aWNlIGRldjsKIAlzdHJ1Y3QgbXV0ZXggbG9jazsgLyogZGV2aWNlIGxvY2sqLwog CWVudW0gdXNiX3JvbGUgcm9sZTsKKwlzdHJ1Y3QgYmxvY2tpbmdfbm90aWZpZXJfaGVhZCBuaDsK IAogCS8qIEZyb20gZGVzY3JpcHRvciAqLwogCXN0cnVjdCBkZXZpY2UgKnVzYjJfcG9ydDsKQEAg LTQ5LDggKzUwLDEwIEBAIGludCB1c2Jfcm9sZV9zd2l0Y2hfc2V0X3JvbGUoc3RydWN0IHVzYl9y b2xlX3N3aXRjaCAqc3csIGVudW0gdXNiX3JvbGUgcm9sZSkKIAltdXRleF9sb2NrKCZzdy0+bG9j ayk7CiAKIAlyZXQgPSBzdy0+c2V0KHN3LT5kZXYucGFyZW50LCByb2xlKTsKLQlpZiAoIXJldCkK KwlpZiAoIXJldCkgewogCQlzdy0+cm9sZSA9IHJvbGU7CisJCWJsb2NraW5nX25vdGlmaWVyX2Nh bGxfY2hhaW4oJnN3LT5uaCwgcm9sZSwgTlVMTCk7CisJfQogCiAJbXV0ZXhfdW5sb2NrKCZzdy0+ bG9jayk7CiAKQEAgLTExMCw2ICsxMTMsMjAgQEAgc3RhdGljIHZvaWQgKnVzYl9yb2xlX3N3aXRj aF9tYXRjaChzdHJ1Y3QgZGV2aWNlX2Nvbm5lY3Rpb24gKmNvbiwgaW50IGVwLAogCXJldHVybiBk ZXYgPyB0b19yb2xlX3N3aXRjaChkZXYpIDogRVJSX1BUUigtRVBST0JFX0RFRkVSKTsKIH0KIAor aW50IHVzYl9yb2xlX3N3aXRjaF9yZWdpc3Rlcl9ub3RpZmllcihzdHJ1Y3QgdXNiX3JvbGVfc3dp dGNoICpzdywKKwkJCQkgICAgICBzdHJ1Y3Qgbm90aWZpZXJfYmxvY2sgKm5iKQoreworCXJldHVy biBibG9ja2luZ19ub3RpZmllcl9jaGFpbl9yZWdpc3Rlcigmc3ctPm5oLCBuYik7Cit9CitFWFBP UlRfU1lNQk9MX0dQTCh1c2Jfcm9sZV9zd2l0Y2hfcmVnaXN0ZXJfbm90aWZpZXIpOworCitpbnQg dXNiX3JvbGVfc3dpdGNoX3VucmVnaXN0ZXJfbm90aWZpZXIoc3RydWN0IHVzYl9yb2xlX3N3aXRj aCAqc3csCisJCQkJCXN0cnVjdCBub3RpZmllcl9ibG9jayAqbmIpCit7CisJcmV0dXJuIGJsb2Nr aW5nX25vdGlmaWVyX2NoYWluX3VucmVnaXN0ZXIoJnN3LT5uaCwgbmIpOworfQorRVhQT1JUX1NZ TUJPTF9HUEwodXNiX3JvbGVfc3dpdGNoX3VucmVnaXN0ZXJfbm90aWZpZXIpOworCiAvKioKICAq IHVzYl9yb2xlX3N3aXRjaF9nZXQgLSBGaW5kIFVTQiByb2xlIHN3aXRjaCBsaW5rZWQgd2l0aCB0 aGUgY2FsbGVyCiAgKiBAZGV2OiBUaGUgY2FsbGVyIGRldmljZQpAQCAtMjY3LDYgKzI4NCw3IEBA IHVzYl9yb2xlX3N3aXRjaF9yZWdpc3RlcihzdHJ1Y3QgZGV2aWNlICpwYXJlbnQsCiAJCXJldHVy biBFUlJfUFRSKC1FTk9NRU0pOwogCiAJbXV0ZXhfaW5pdCgmc3ctPmxvY2spOworCUJMT0NLSU5H X0lOSVRfTk9USUZJRVJfSEVBRCgmc3ctPm5oKTsKIAogCXN3LT5hbGxvd191c2Vyc3BhY2VfY29u dHJvbCA9IGRlc2MtPmFsbG93X3VzZXJzcGFjZV9jb250cm9sOwogCXN3LT51c2IyX3BvcnQgPSBk ZXNjLT51c2IyX3BvcnQ7Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heikki Krogerus Subject: Re: [PATCH 07/10] hikey960: Support usb functionality of Hikey960 Date: Tue, 30 Oct 2018 12:16:35 +0200 Message-ID: <20181030101635.GC14534@kuha.fi.intel.com> References: <20181027095820.40056-1-chenyu56@huawei.com> <20181027095820.40056-8-chenyu56@huawei.com> <20181029143040.GB14534@kuha.fi.intel.com> <33a6ac59-b545-e2c8-2bb7-0a5460fcf5e9@huawei.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="YiEDa0DAkWCtVeE4" Return-path: Content-Disposition: inline In-Reply-To: <33a6ac59-b545-e2c8-2bb7-0a5460fcf5e9@huawei.com> Sender: linux-kernel-owner@vger.kernel.org To: Chen Yu 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 --YiEDa0DAkWCtVeE4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Oct 30, 2018 at 10:50:22AM +0800, Chen Yu wrote: > > 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. Thank you for the explanation. I got the picture now. I realized that there is some online information for this board: https://www.96boards.org/documentation/consumer/hikey/hikey960/hardware-docs/hardware-user-manual.md.html#usb-ports So that mux is actually _not_ switching between the host and device modes, but instead, it's switching between Type-C and Type-A connectors (I'm skipping the hub, as it's irrelevant from the PoW of the mux), so I've misunderstood. And yes, you did say that it is switching between connectors in the commit message, but I got confused because you are registers a role switch. I don't think you should register a role switch from this driver. This driver is not really representing USB role switch. The mux on this board is something else. Instead you should register the role switch from the dwc3 drd code. That is the part that is representing the role switch here. I actually think that we should do that in any case. The dwc3 drd code should always register a role switch. We can solve the problem of getting the role change events in this driver by adding notification chain to struct usb_role_switch (check the attached diff). You would then just need to call usb_role_switch_get() and usb_role_switch_register_notifier(), and wait for those notifications. The extcon device does not serve any purpose anymore. This driver would continue to take care of the hub powering and the mux, and also the VBUS like before. br, -- heikki --YiEDa0DAkWCtVeE4 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="roles.diff" diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c index bb52e006d203..2881777c16e5 100644 --- a/drivers/usb/common/roles.c +++ b/drivers/usb/common/roles.c @@ -20,6 +20,7 @@ struct usb_role_switch { struct device dev; struct mutex lock; /* device lock*/ enum usb_role role; + struct blocking_notifier_head nh; /* From descriptor */ struct device *usb2_port; @@ -49,8 +50,10 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role) mutex_lock(&sw->lock); ret = sw->set(sw->dev.parent, role); - if (!ret) + if (!ret) { sw->role = role; + blocking_notifier_call_chain(&sw->nh, role, NULL); + } mutex_unlock(&sw->lock); @@ -110,6 +113,20 @@ static void *usb_role_switch_match(struct device_connection *con, int ep, return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER); } +int usb_role_switch_register_notifier(struct usb_role_switch *sw, + struct notifier_block *nb) +{ + return blocking_notifier_chain_register(&sw->nh, nb); +} +EXPORT_SYMBOL_GPL(usb_role_switch_register_notifier); + +int usb_role_switch_unregister_notifier(struct usb_role_switch *sw, + struct notifier_block *nb) +{ + return blocking_notifier_chain_unregister(&sw->nh, nb); +} +EXPORT_SYMBOL_GPL(usb_role_switch_unregister_notifier); + /** * usb_role_switch_get - Find USB role switch linked with the caller * @dev: The caller device @@ -267,6 +284,7 @@ usb_role_switch_register(struct device *parent, return ERR_PTR(-ENOMEM); mutex_init(&sw->lock); + BLOCKING_INIT_NOTIFIER_HEAD(&sw->nh); sw->allow_userspace_control = desc->allow_userspace_control; sw->usb2_port = desc->usb2_port; --YiEDa0DAkWCtVeE4--