linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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;

             reply	other threads:[~2018-10-30 10:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-30 10:16 Heikki Krogerus [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-10-30  2:50 [07/10] hikey960: Support usb functionality of Hikey960 Yu Chen
2018-10-29 14:30 Heikki Krogerus
2018-10-27  9:58 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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).