From: Heikki Krogerus <heikki.krogerus@linux.intel.com> To: Chen Yu <chenyu56@huawei.com> 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 <arnd@arndb.de>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, John Stultz <john.stultz@linaro.org> Subject: Re: [PATCH 07/10] hikey960: Support usb functionality of Hikey960 Date: Tue, 30 Oct 2018 12:16:35 +0200 [thread overview] Message-ID: <20181030101635.GC14534@kuha.fi.intel.com> (raw) In-Reply-To: <33a6ac59-b545-e2c8-2bb7-0a5460fcf5e9@huawei.com> [-- Attachment #1: Type: text/plain, Size: 2469 bytes --] 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 [-- Attachment #2: roles.diff --] [-- Type: text/plain, Size: 1662 bytes --] 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;
WARNING: multiple messages have this Message-ID (diff)
From: Heikki Krogerus <heikki.krogerus@linux.intel.com> To: Chen Yu <chenyu56@huawei.com> 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 <arnd@arndb.de>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, John Stultz <john.stultz@linaro.org> Subject: [07/10] hikey960: Support usb functionality of Hikey960 Date: Tue, 30 Oct 2018 12:16:35 +0200 [thread overview] Message-ID: <20181030101635.GC14534@kuha.fi.intel.com> (raw) 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, 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;
next prev parent reply other threads:[~2018-10-30 10:16 UTC|newest] Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-27 9:58 [PATCH 00/10] Add support for usb on Hikey960 Yu Chen 2018-10-27 9:58 ` Yu Chen 2018-10-27 9:58 ` [PATCH 01/10] dt-bindings: usb: add support for dwc3 controller on HiSilicon SoCs Yu Chen 2018-10-27 9:58 ` [01/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 01/10] " Yu Chen 2018-11-12 16:02 ` Rob Herring 2018-11-17 2:29 ` Chen Yu 2018-11-17 2:29 ` [01/10] " Yu Chen 2018-11-17 2:29 ` [PATCH 01/10] " Chen Yu 2018-12-03 16:01 ` Rob Herring 2018-12-03 16:01 ` [01/10] " Rob Herring 2018-12-04 1:03 ` [PATCH 01/10] " Chen Yu 2018-12-04 1:03 ` [01/10] " Yu Chen 2018-12-04 1:03 ` [PATCH 01/10] " Chen Yu 2018-12-14 8:05 ` Chen Yu 2018-12-14 8:05 ` [01/10] " Yu Chen 2018-12-14 8:05 ` [PATCH 01/10] " Chen Yu 2018-10-27 9:58 ` [PATCH 02/10] dt-bindings: phy: Add support for HiSilicon's hi3660 USB PHY Yu Chen 2018-10-27 9:58 ` [02/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 02/10] " Yu Chen 2018-11-12 16:06 ` Rob Herring 2018-10-27 9:58 ` [PATCH 03/10] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960 Yu Chen 2018-10-27 9:58 ` [03/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 03/10] " Yu Chen 2018-10-27 18:06 ` Sergei Shtylyov 2018-10-27 18:06 ` [03/10] " Sergei Shtylyov 2018-10-29 12:25 ` [PATCH 03/10] " Chen Yu 2018-10-29 12:25 ` [03/10] " Yu Chen 2018-10-29 12:25 ` [PATCH 03/10] " Chen Yu 2018-10-27 9:58 ` [PATCH 04/10] usb: dwc3: dwc3-hisi: Add code for dwc3 of Hisilicon Soc Platform Yu Chen 2018-10-27 9:58 ` [04/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 04/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 05/10] usb: dwc3: Add two quirks for Hisilicon Kirin " Yu Chen 2018-10-27 9:58 ` [05/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 05/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 06/10] phy: Add usb phy support for hi3660 Soc of Hisilicon Yu Chen 2018-10-27 9:58 ` [06/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 06/10] " Yu Chen 2018-11-12 9:09 ` Kishon Vijay Abraham I 2018-11-12 9:09 ` [06/10] " Kishon Vijay Abraham I 2018-11-12 9:09 ` [PATCH 06/10] " Kishon Vijay Abraham I 2018-10-27 9:58 ` [PATCH 07/10] hikey960: Support usb functionality of Hikey960 Yu Chen 2018-10-27 9:58 ` [07/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 07/10] " Yu Chen 2018-10-29 14:30 ` Heikki Krogerus 2018-10-29 14:30 ` [07/10] " Heikki Krogerus 2018-10-30 2:50 ` [PATCH 07/10] " Chen Yu 2018-10-30 2:50 ` [07/10] " Yu Chen 2018-10-30 2:50 ` [PATCH 07/10] " Chen Yu 2018-10-30 10:16 ` Heikki Krogerus [this message] 2018-10-30 10:16 ` [07/10] " Heikki Krogerus 2018-10-27 9:58 ` [PATCH 08/10] usb: typec: Add support for usb role switch in rt1711h driver Yu Chen 2018-10-27 9:58 ` [08/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 08/10] " Yu Chen 2018-10-29 11:58 ` Heikki Krogerus 2018-10-29 11:58 ` [08/10] " Heikki Krogerus 2018-10-29 12:22 ` [PATCH 08/10] " Chen Yu 2018-10-29 12:22 ` [08/10] " Yu Chen 2018-10-29 12:22 ` [PATCH 08/10] " Chen Yu 2018-10-27 9:58 ` [PATCH 09/10] usb: gadget: Add configfs attribuite for controling match_existing_only Yu Chen 2018-10-27 9:58 ` [09/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 09/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 10/10] dts: hi3660: Add support for usb on Hikey960 Yu Chen 2018-10-27 9:58 ` Yu Chen 2018-10-27 9:58 ` [10/10] " Yu Chen 2018-10-27 9:58 ` [PATCH 10/10] " Yu Chen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20181030101635.GC14534@kuha.fi.intel.com \ --to=heikki.krogerus@linux.intel.com \ --cc=arnd@arndb.de \ --cc=chenyu56@huawei.com \ --cc=devicetree@vger.kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=john.stultz@linaro.org \ --cc=kongfei@hisilicon.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=suzhuangluan@hisilicon.com \ --cc=wangbinghui@hisilicon.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.