linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/8] HiKey960 USB support
@ 2019-12-12  1:42 John Stultz
  2019-12-12  1:42 ` [PATCH v7 1/8] usb: dwc3: Registering a role switch in the DRD code John Stultz
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: John Stultz @ 2019-12-12  1:42 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
	ShuFan Lee, Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun,
	Yu Chen, Felipe Balbi, Hans de Goede, Andy Shevchenko, Jun Li,
	Valentin Schneider, Guillaume Gardet, Jack Pham, linux-usb,
	devicetree

Just another round here trying to push forward a patch series
submitted originally by Yu Chen to get HiKey960 dev-board's USB
functionality working.

The full patchset (including dts changes not submitted here) can
be found here:
https://git.linaro.org/people/john.stultz/android-dev.git/log/?id=2ae2c2a9087b52857dd47313a169c7348261cbca

I'd greatly appreciate any feedback or thoughts!

thanks
-john

New in v7:
* Guillaume Gardet reported issues with CONFIG_USB_ROLE_SWITCH=m
  which to fix required I use IS_ENABLED instead of just #ifdef
  on that config name.
* Added MAINTAINERS file for usb hub misc driver
* Switched the usb hub misc driver bindings to YAML

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: ShuFan Lee <shufan_lee@richtek.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
Cc: Jack Pham <jackp@codeaurora.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org

John Stultz (5):
  dt-bindings: usb: generic: Add role-switch-default-mode binding
  usb: dwc3: Add support for role-switch-default-mode binding
  dt-bindings: usb: dwc3: Allow clock list & resets to be more flexible
  usb: dwc3: Rework clock initialization to be more flexible
  usb: dwc3: Rework resets initialization to be more flexible

Yu Chen (3):
  usb: dwc3: Registering a role switch in the DRD code.
  dt-bindings: misc: Add bindings for HiSilicon usb hub and data role
    switch functionality on HiKey960
  misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on
    Hikey960

 .../bindings/misc/hisilicon-hikey-usb.yaml    |  85 +++++++++
 .../devicetree/bindings/usb/dwc3.txt          |   5 +-
 .../devicetree/bindings/usb/generic.txt       |   6 +
 MAINTAINERS                                   |   7 +
 drivers/misc/Kconfig                          |   9 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/hisi_hikey_usb.c                 | 178 ++++++++++++++++++
 drivers/usb/dwc3/core.c                       |  22 +--
 drivers/usb/dwc3/core.h                       |   6 +
 drivers/usb/dwc3/drd.c                        |  96 +++++++++-
 10 files changed, 396 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
 create mode 100644 drivers/misc/hisi_hikey_usb.c

-- 
2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v7 1/8] usb: dwc3: Registering a role switch in the DRD code.
  2019-12-12  1:42 [PATCH v7 0/8] HiKey960 USB support John Stultz
@ 2019-12-12  1:42 ` John Stultz
  2019-12-12  1:42 ` [PATCH v7 2/8] dt-bindings: usb: generic: Add role-switch-default-mode binding John Stultz
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2019-12-12  1:42 UTC (permalink / raw)
  To: lkml
  Cc: Yu Chen, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
	ShuFan Lee, Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun,
	Felipe Balbi, Hans de Goede, Andy Shevchenko, Jun Li,
	Valentin Schneider, Guillaume Gardet, Jack Pham, linux-usb,
	devicetree, John Stultz

From: Yu Chen <chenyu56@huawei.com>

The Type-C drivers use USB role switch API to inform the
system about the negotiated data role, so registering a role
switch in the DRD code in order to support platforms with
USB Type-C connectors.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: ShuFan Lee <shufan_lee@richtek.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
Cc: Jack Pham <jackp@codeaurora.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Yu Chen <chenyu56@huawei.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2: Fix role_sw and role_switch_default_mode descriptions as
    reported by kbuild test robot <lkp@intel.com>

v3: Split out the role-switch-default-host logic into its own
    patch

v5: Drop selecting CONFIG_USB_ROLE_SWITCH & ifdef dependent code

v6: Fix build issue Reported-by: kbuild test robot <lkp@intel.com>

v7: Minor fix for CONFIG_USB_ROLE_SWITCH=m case not building
    the role switch handling code, reported by
    Guillaume Gardet <Guillaume.Gardet@arm.com>
---
 drivers/usb/dwc3/core.h |  3 ++
 drivers/usb/dwc3/drd.c  | 77 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1c8b349379af..6f19e9891767 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -25,6 +25,7 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg.h>
+#include <linux/usb/role.h>
 #include <linux/ulpi/interface.h>
 
 #include <linux/phy/phy.h>
@@ -951,6 +952,7 @@ struct dwc3_scratchpad_array {
  * @hsphy_mode: UTMI phy mode, one of following:
  *		- USBPHY_INTERFACE_MODE_UTMI
  *		- USBPHY_INTERFACE_MODE_UTMIW
+ * @role_sw: usb_role_switch handle
  * @usb2_phy: pointer to USB2 PHY
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY
@@ -1084,6 +1086,7 @@ struct dwc3 {
 	struct extcon_dev	*edev;
 	struct notifier_block	edev_nb;
 	enum usb_phy_interface	hsphy_mode;
+	struct usb_role_switch	*role_sw;
 
 	u32			fladj;
 	u32			irq_gadget;
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index c946d64142ad..331c6e997f0c 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -476,6 +476,73 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
 	return edev;
 }
 
+#if IS_ENABLED(CONFIG_USB_ROLE_SWITCH)
+#define ROLE_SWITCH 1
+static int dwc3_usb_role_switch_set(struct device *dev, enum usb_role role)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	u32 mode;
+
+	switch (role) {
+	case USB_ROLE_HOST:
+		mode = DWC3_GCTL_PRTCAP_HOST;
+		break;
+	case USB_ROLE_DEVICE:
+		mode = DWC3_GCTL_PRTCAP_DEVICE;
+		break;
+	default:
+		mode = DWC3_GCTL_PRTCAP_DEVICE;
+		break;
+	}
+
+	dwc3_set_mode(dwc, mode);
+	return 0;
+}
+
+static enum usb_role dwc3_usb_role_switch_get(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	unsigned long flags;
+	enum usb_role role;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	switch (dwc->current_dr_role) {
+	case DWC3_GCTL_PRTCAP_HOST:
+		role = USB_ROLE_HOST;
+		break;
+	case DWC3_GCTL_PRTCAP_DEVICE:
+		role = USB_ROLE_DEVICE;
+		break;
+	case DWC3_GCTL_PRTCAP_OTG:
+		role = dwc->current_otg_role;
+		break;
+	default:
+		role = USB_ROLE_DEVICE;
+		break;
+	}
+	spin_unlock_irqrestore(&dwc->lock, flags);
+	return role;
+}
+
+static int dwc3_setup_role_switch(struct dwc3 *dwc)
+{
+	struct usb_role_switch_desc dwc3_role_switch = {NULL};
+
+	dwc3_role_switch.fwnode = dev_fwnode(dwc->dev);
+	dwc3_role_switch.set = dwc3_usb_role_switch_set;
+	dwc3_role_switch.get = dwc3_usb_role_switch_get;
+	dwc->role_sw = usb_role_switch_register(dwc->dev, &dwc3_role_switch);
+	if (IS_ERR(dwc->role_sw))
+		return PTR_ERR(dwc->role_sw);
+
+	dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+	return 0;
+}
+#else
+#define ROLE_SWITCH 0
+#define dwc3_setup_role_switch(x) 0
+#endif
+
 int dwc3_drd_init(struct dwc3 *dwc)
 {
 	int ret, irq;
@@ -484,7 +551,12 @@ int dwc3_drd_init(struct dwc3 *dwc)
 	if (IS_ERR(dwc->edev))
 		return PTR_ERR(dwc->edev);
 
-	if (dwc->edev) {
+	if (ROLE_SWITCH &&
+	    device_property_read_bool(dwc->dev, "usb-role-switch")) {
+		ret = dwc3_setup_role_switch(dwc);
+		if (ret < 0)
+			return ret;
+	} else if (dwc->edev) {
 		dwc->edev_nb.notifier_call = dwc3_drd_notifier;
 		ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
 					       &dwc->edev_nb);
@@ -531,6 +603,9 @@ void dwc3_drd_exit(struct dwc3 *dwc)
 {
 	unsigned long flags;
 
+	if (dwc->role_sw)
+		usb_role_switch_unregister(dwc->role_sw);
+
 	if (dwc->edev)
 		extcon_unregister_notifier(dwc->edev, EXTCON_USB_HOST,
 					   &dwc->edev_nb);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 2/8] dt-bindings: usb: generic: Add role-switch-default-mode binding
  2019-12-12  1:42 [PATCH v7 0/8] HiKey960 USB support John Stultz
  2019-12-12  1:42 ` [PATCH v7 1/8] usb: dwc3: Registering a role switch in the DRD code John Stultz
@ 2019-12-12  1:42 ` John Stultz
  2019-12-12  1:42 ` [PATCH v7 3/8] usb: dwc3: Add support for " John Stultz
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2019-12-12  1:42 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
	ShuFan Lee, Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun,
	Yu Chen, Felipe Balbi, Hans de Goede, Andy Shevchenko, Jun Li,
	Valentin Schneider, Guillaume Gardet, Jack Pham, linux-usb,
	devicetree

Add binding to configure the default role the controller
assumes is host mode when the usb role is USB_ROLE_NONE.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: ShuFan Lee <shufan_lee@richtek.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
Cc: Jack Pham <jackp@codeaurora.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v5: Switch to string rather then a bool
---
 Documentation/devicetree/bindings/usb/generic.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
index cf5a1ad456e6..dd733fa81fad 100644
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ b/Documentation/devicetree/bindings/usb/generic.txt
@@ -34,6 +34,12 @@ Optional properties:
 			the USB data role (USB host or USB device) for a given
 			USB connector, such as Type-C, Type-B(micro).
 			see connector/usb-connector.txt.
+ - role-switch-default-mode: indicating if usb-role-switch is enabled, the
+			device default operation mode of controller while usb
+			role is USB_ROLE_NONE. Valid arguments are "host" and
+			"peripheral". Defaults to "peripheral" if not
+			specified.
+
 
 This is an attribute to a USB controller such as:
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 3/8] usb: dwc3: Add support for role-switch-default-mode binding
  2019-12-12  1:42 [PATCH v7 0/8] HiKey960 USB support John Stultz
  2019-12-12  1:42 ` [PATCH v7 1/8] usb: dwc3: Registering a role switch in the DRD code John Stultz
  2019-12-12  1:42 ` [PATCH v7 2/8] dt-bindings: usb: generic: Add role-switch-default-mode binding John Stultz
@ 2019-12-12  1:42 ` John Stultz
  2019-12-12  1:42 ` [PATCH v7 4/8] dt-bindings: usb: dwc3: Allow clock list & resets to be more flexible John Stultz
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2019-12-12  1:42 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
	ShuFan Lee, Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun,
	Yu Chen, Felipe Balbi, Hans de Goede, Andy Shevchenko, Jun Li,
	Valentin Schneider, Guillaume Gardet, Jack Pham, linux-usb,
	devicetree

Support the new role-switch-default-mode binding for configuring
the default role the controller assumes as when the usb role is
USB_ROLE_NONE

This patch was split out from a larger patch originally by
Yu Chen <chenyu56@huawei.com>

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: ShuFan Lee <shufan_lee@richtek.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
Cc: Jack Pham <jackp@codeaurora.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v3: Split this patch out from addition of usb-role-switch
    handling
v5: Reworked to use string based role-switch-default-mode
---
 drivers/usb/dwc3/core.h |  3 +++
 drivers/usb/dwc3/drd.c  | 25 ++++++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6f19e9891767..3c879c9ab1aa 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -953,6 +953,8 @@ struct dwc3_scratchpad_array {
  *		- USBPHY_INTERFACE_MODE_UTMI
  *		- USBPHY_INTERFACE_MODE_UTMIW
  * @role_sw: usb_role_switch handle
+ * @role_switch_default_mode: default operation mode of controller while
+ *			usb role is USB_ROLE_NONE.
  * @usb2_phy: pointer to USB2 PHY
  * @usb3_phy: pointer to USB3 PHY
  * @usb2_generic_phy: pointer to USB2 PHY
@@ -1087,6 +1089,7 @@ struct dwc3 {
 	struct notifier_block	edev_nb;
 	enum usb_phy_interface	hsphy_mode;
 	struct usb_role_switch	*role_sw;
+	enum usb_dr_mode	role_switch_default_mode;
 
 	u32			fladj;
 	u32			irq_gadget;
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 331c6e997f0c..db68d48c2267 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -491,7 +491,10 @@ static int dwc3_usb_role_switch_set(struct device *dev, enum usb_role role)
 		mode = DWC3_GCTL_PRTCAP_DEVICE;
 		break;
 	default:
-		mode = DWC3_GCTL_PRTCAP_DEVICE;
+		if (dwc->role_switch_default_mode == USB_DR_MODE_HOST)
+			mode = DWC3_GCTL_PRTCAP_HOST;
+		else
+			mode = DWC3_GCTL_PRTCAP_DEVICE;
 		break;
 	}
 
@@ -517,7 +520,10 @@ static enum usb_role dwc3_usb_role_switch_get(struct device *dev)
 		role = dwc->current_otg_role;
 		break;
 	default:
-		role = USB_ROLE_DEVICE;
+		if (dwc->role_switch_default_mode == USB_DR_MODE_HOST)
+			role = USB_ROLE_HOST;
+		else
+			role = USB_ROLE_DEVICE;
 		break;
 	}
 	spin_unlock_irqrestore(&dwc->lock, flags);
@@ -527,6 +533,19 @@ static enum usb_role dwc3_usb_role_switch_get(struct device *dev)
 static int dwc3_setup_role_switch(struct dwc3 *dwc)
 {
 	struct usb_role_switch_desc dwc3_role_switch = {NULL};
+	const char *str;
+	u32 mode;
+	int ret;
+
+	ret = device_property_read_string(dwc->dev, "role-switch-default-mode",
+					  &str);
+	if (ret >= 0  && !strncmp(str, "host", strlen("host"))) {
+		dwc->role_switch_default_mode = USB_DR_MODE_HOST;
+		mode = DWC3_GCTL_PRTCAP_HOST;
+	} else {
+		dwc->role_switch_default_mode = USB_DR_MODE_PERIPHERAL;
+		mode = DWC3_GCTL_PRTCAP_DEVICE;
+	}
 
 	dwc3_role_switch.fwnode = dev_fwnode(dwc->dev);
 	dwc3_role_switch.set = dwc3_usb_role_switch_set;
@@ -535,7 +554,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc)
 	if (IS_ERR(dwc->role_sw))
 		return PTR_ERR(dwc->role_sw);
 
-	dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+	dwc3_set_mode(dwc, mode);
 	return 0;
 }
 #else
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 4/8] dt-bindings: usb: dwc3: Allow clock list & resets to be more flexible
  2019-12-12  1:42 [PATCH v7 0/8] HiKey960 USB support John Stultz
                   ` (2 preceding siblings ...)
  2019-12-12  1:42 ` [PATCH v7 3/8] usb: dwc3: Add support for " John Stultz
@ 2019-12-12  1:42 ` John Stultz
  2019-12-12  1:42 ` [PATCH v7 5/8] usb: dwc3: Rework clock initialization " John Stultz
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2019-12-12  1:42 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
	ShuFan Lee, Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun,
	Yu Chen, Felipe Balbi, Hans de Goede, Andy Shevchenko, Jun Li,
	Valentin Schneider, Guillaume Gardet, Jack Pham, linux-usb,
	devicetree

Rather then adding another device specific binding to support
hikey960, Rob Herring suggested we expand the current dwc3
binding to allow for variable numbers of clocks and resets.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: ShuFan Lee <shufan_lee@richtek.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
Cc: Jack Pham <jackp@codeaurora.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Suggested-by: Rob Herring <robh@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 Documentation/devicetree/bindings/usb/dwc3.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index 66780a47ad85..29768b0ca923 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -7,7 +7,8 @@ Required properties:
  - compatible: must be "snps,dwc3"
  - reg : Address and length of the register set for the device
  - interrupts: Interrupts used by the dwc3 controller.
- - clock-names: should contain "ref", "bus_early", "suspend"
+ - clock-names: list of clock names. Ideally should be "ref",
+                "bus_early", "suspend" but may be less or more.
  - clocks: list of phandle and clock specifier pairs corresponding to
            entries in the clock-names property.
 
@@ -36,7 +37,7 @@ Optional properties:
  - phys: from the *Generic PHY* bindings
  - phy-names: from the *Generic PHY* bindings; supported names are "usb2-phy"
 	or "usb3-phy".
- - resets: a single pair of phandle and reset specifier
+ - resets: set of phandle and reset specifier pairs
  - snps,usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
  - snps,usb3_lpm_capable: determines if platform is USB3 LPM capable
  - snps,dis-start-transfer-quirk: when set, disable isoc START TRANSFER command
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 5/8] usb: dwc3: Rework clock initialization to be more flexible
  2019-12-12  1:42 [PATCH v7 0/8] HiKey960 USB support John Stultz
                   ` (3 preceding siblings ...)
  2019-12-12  1:42 ` [PATCH v7 4/8] dt-bindings: usb: dwc3: Allow clock list & resets to be more flexible John Stultz
@ 2019-12-12  1:42 ` John Stultz
  2019-12-12  1:42 ` [PATCH v7 6/8] usb: dwc3: Rework resets " John Stultz
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2019-12-12  1:42 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
	ShuFan Lee, Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun,
	Yu Chen, Felipe Balbi, Hans de Goede, Andy Shevchenko, Jun Li,
	Valentin Schneider, Guillaume Gardet, Jack Pham, linux-usb,
	devicetree

The dwc3 core binding specifies three clocks:
  ref, bus_early, and suspend

which are all controlled in the driver together.

However some variants of the hardware my not have all three
clks, or some may have more. Usually this was handled by using
the dwc3-of-simple glue driver, but that resulted in a
proliferation of bindings for for every variant, when the only
difference was the clocks and resets lists.

So this patch reworks the reading of the clks from the dts to
use devm_clk_bulk_get_all() will will fetch all the clocks
specified in the dts together.

This patch was recommended by Rob Herring <robh@kernel.org>
as an alternative to creating multiple bindings for each variant
of hardware.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: ShuFan Lee <shufan_lee@richtek.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
Cc: Jack Pham <jackp@codeaurora.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v3: Rework dwc3 core rather then adding another dwc-of-simple
    binding.
v6: Re-introduce this patch, on Rob's suggestion
---
 drivers/usb/dwc3/core.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index f561c6c9e8a9..c6316d4b7593 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -289,12 +289,6 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
 	return 0;
 }
 
-static const struct clk_bulk_data dwc3_core_clks[] = {
-	{ .id = "ref" },
-	{ .id = "bus_early" },
-	{ .id = "suspend" },
-};
-
 /*
  * dwc3_frame_length_adjustment - Adjusts frame length if required
  * @dwc3: Pointer to our controller context structure
@@ -1438,11 +1432,6 @@ static int dwc3_probe(struct platform_device *pdev)
 	if (!dwc)
 		return -ENOMEM;
 
-	dwc->clks = devm_kmemdup(dev, dwc3_core_clks, sizeof(dwc3_core_clks),
-				 GFP_KERNEL);
-	if (!dwc->clks)
-		return -ENOMEM;
-
 	dwc->dev = dev;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1478,17 +1467,18 @@ static int dwc3_probe(struct platform_device *pdev)
 		return PTR_ERR(dwc->reset);
 
 	if (dev->of_node) {
-		dwc->num_clks = ARRAY_SIZE(dwc3_core_clks);
-
-		ret = devm_clk_bulk_get(dev, dwc->num_clks, dwc->clks);
+		ret = devm_clk_bulk_get_all(dev, &dwc->clks);
 		if (ret == -EPROBE_DEFER)
 			return ret;
 		/*
 		 * Clocks are optional, but new DT platforms should support all
 		 * clocks as required by the DT-binding.
 		 */
-		if (ret)
+		if (ret < 0)
 			dwc->num_clks = 0;
+		else
+			dwc->num_clks = ret;
+
 	}
 
 	ret = reset_control_deassert(dwc->reset);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 6/8] usb: dwc3: Rework resets initialization to be more flexible
  2019-12-12  1:42 [PATCH v7 0/8] HiKey960 USB support John Stultz
                   ` (4 preceding siblings ...)
  2019-12-12  1:42 ` [PATCH v7 5/8] usb: dwc3: Rework clock initialization " John Stultz
@ 2019-12-12  1:42 ` John Stultz
  2019-12-12  1:42 ` [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960 John Stultz
  2019-12-12  1:42 ` [PATCH v7 8/8] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960 John Stultz
  7 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2019-12-12  1:42 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
	ShuFan Lee, Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun,
	Yu Chen, Felipe Balbi, Hans de Goede, Andy Shevchenko, Jun Li,
	Valentin Schneider, Guillaume Gardet, Jack Pham, linux-usb,
	devicetree

The dwc3 core binding specifies one reset.

However some variants of the hardware may have more. Previously
this was handled by using the dwc3-of-simple glue driver, but
that resulted in a proliferation of bindings for for every
variant, when the only difference was the clocks and resets
lists.

So this patch reworks the reading of the resets to fetch all the
resets specified in the dts together.

This patch was recommended by Rob Herring <robh@kernel.org>
as an alternative to creating multiple bindings for each variant
of hardware.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: ShuFan Lee <shufan_lee@richtek.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
Cc: Jack Pham <jackp@codeaurora.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v3: Rework dwc3 core rather then adding another dwc-of-simple
    binding.
v6: Re-introduce this patch, on Rob's suggestion
---
 drivers/usb/dwc3/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c6316d4b7593..19504b907476 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1462,7 +1462,7 @@ static int dwc3_probe(struct platform_device *pdev)
 
 	dwc3_get_properties(dwc);
 
-	dwc->reset = devm_reset_control_get_optional_shared(dev, NULL);
+	dwc->reset = devm_reset_control_array_get(dev, true, true);
 	if (IS_ERR(dwc->reset))
 		return PTR_ERR(dwc->reset);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
  2019-12-12  1:42 [PATCH v7 0/8] HiKey960 USB support John Stultz
                   ` (5 preceding siblings ...)
  2019-12-12  1:42 ` [PATCH v7 6/8] usb: dwc3: Rework resets " John Stultz
@ 2019-12-12  1:42 ` John Stultz
  2019-12-18 16:37   ` Rob Herring
  2019-12-12  1:42 ` [PATCH v7 8/8] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960 John Stultz
  7 siblings, 1 reply; 13+ messages in thread
From: John Stultz @ 2019-12-12  1:42 UTC (permalink / raw)
  To: lkml
  Cc: Yu Chen, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
	ShuFan Lee, Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun,
	Felipe Balbi, Hans de Goede, Andy Shevchenko, Jun Li,
	Valentin Schneider, Guillaume Gardet, Jack Pham, linux-usb,
	devicetree, John Stultz

From: Yu Chen <chenyu56@huawei.com>

This patch adds binding documentation to support usb hub and usb
data role switch of Hisilicon HiKey960 Board.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: ShuFan Lee <shufan_lee@richtek.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
Cc: Jack Pham <jackp@codeaurora.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Yu Chen <chenyu56@huawei.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v3: Reworked as usb-role-switch intermediary

v7: Switched over to YAML dt binding description
---
 .../bindings/misc/hisilicon-hikey-usb.yaml    | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml

diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
new file mode 100644
index 000000000000..1fc3b198ef73
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Linaro Ltd.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/misc/hisilicon-hikey-usb.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: HiKey960 onboard USB GPIO Hub
+
+maintainers:
+  - John Stultz <john.stultz@linaro.org>
+
+description: |
+  Supports the onboard HiKey960 USB GPIO hub, which acts as a
+  role-switch intermediary to detect the state of the USB-C
+  port, to switch the hub into dual-role USB-C or host mode,
+  which enables the onboard USB-A host ports.
+
+  Schematics about the hub can be found here:
+    https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf
+
+properties:
+  compatible:
+    items:
+      - const: hisilicon,gpio_hubv1
+
+  typec-vbus-gpios:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the typec-vbus gpio
+
+  otg-switch-gpios:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the otg-switch gpio
+
+  hub-vdd33-en-gpios:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the hub 3.3v power enablement gpio
+
+  usb-role-switch:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: Support role switch.
+
+  port:
+    description: |
+      any connector to the data bus of this controller should be modelled
+      using the OF graph bindings specified, if the "usb-role-switch"
+      property is used. Note for this driver, two ports are supported,
+      the first being the endpoint that will be notified by this driver,
+      and the second being the endpoint that notifies this driver of a
+      role switch.
+
+
+required:
+  - compatible
+  - typec-vbus-gpios
+  - otg-switch-gpios
+  - hub-vdd33-en-gpios
+  - usb-role-switch
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    hisi_hikey_usb: hisi_hikey_usb {
+        compatible = "hisilicon,gpio_hubv1";
+        typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>;
+        otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
+        hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+        usb-role-switch;
+
+        port {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            hikey_usb_ep0: endpoint@0 {
+                reg = <0>;
+                remote-endpoint = <&dwc3_role_switch>;
+            };
+            hikey_usb_ep1: endpoint@1 {
+                reg = <1>;
+                remote-endpoint = <&rt1711h_ep>;
+            };
+        };
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 8/8] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960
  2019-12-12  1:42 [PATCH v7 0/8] HiKey960 USB support John Stultz
                   ` (6 preceding siblings ...)
  2019-12-12  1:42 ` [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960 John Stultz
@ 2019-12-12  1:42 ` John Stultz
  7 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2019-12-12  1:42 UTC (permalink / raw)
  To: lkml
  Cc: Yu Chen, Greg Kroah-Hartman, Rob Herring, Mark Rutland,
	ShuFan Lee, Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun,
	Felipe Balbi, Hans de Goede, Andy Shevchenko, Jun Li,
	Valentin Schneider, Guillaume Gardet, Jack Pham, linux-usb,
	devicetree, John Stultz

From: Yu Chen <chenyu56@huawei.com>

The HiKey960 has a fairly complex USB configuration due to it
needing to support a USB-C port for host/device mode and multiple
USB-A ports in host mode, all using a single USB controller.

See schematics here:
  https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf

This driver acts as a usb-role-switch intermediary, intercepting
the role switch notifications from the tcpm code, and passing
them on to the dwc3 core.

In doing so, it also controls the onboard hub and power gpios in
order to properly route the data lines between the USB-C port
and the onboard hub to the USB-A ports.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
CC: ShuFan Lee <shufan_lee@richtek.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Yu Chen <chenyu56@huawei.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Jun Li <lijun.kernel@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
Cc: Jack Pham <jackp@codeaurora.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Yu Chen <chenyu56@huawei.com>
[jstultz: Major rework to make the driver a usb-role-switch
          intermediary]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v3:
* Major rework to make the driver a usb-role-switch intermediary
  rather then trying to do notifier callbacks from the role switch
  logic
v7:
* Add MAINTAINERS entry and more verbose Kconfig description
---
 MAINTAINERS                   |   7 ++
 drivers/misc/Kconfig          |   9 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/hisi_hikey_usb.c | 178 ++++++++++++++++++++++++++++++++++
 4 files changed, 195 insertions(+)
 create mode 100644 drivers/misc/hisi_hikey_usb.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bd5847e802de..ae5bebc2b3e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7422,6 +7422,13 @@ F:	include/uapi/linux/if_hippi.h
 F:	net/802/hippi.c
 F:	drivers/net/hippi/
 
+HIKEY960 ONBOARD USB GPIO HUB DRIVER
+M:	John Stultz <john.stultz@linaro.org>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	drivers/misc/hisi_hikey_usb.c
+F:	Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
+
 HISILICON SECURITY ENGINE V2 DRIVER (SEC2)
 M:	Zaibo Xu <xuzaibo@huawei.com>
 L:	linux-crypto@vger.kernel.org
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 7f0d48f406e3..563492f67be3 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -465,6 +465,15 @@ config PVPANIC
 	  a paravirtualized device provided by QEMU; it lets a virtual machine
 	  (guest) communicate panic events to the host.
 
+config HISI_HIKEY_USB
+	tristate "USB GPIO Hub on HiSilicon Hikey960 Platform"
+	depends on OF && GPIOLIB
+	help
+	  If you say yes here this adds support for the on-board USB gpio hub
+	  found on the HiKey960, which is necssary to support switching between
+	  the dual-role USB-C port and the USB-A host ports using only one USB
+	  controller.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c1860d35dc7e..e5e85ad0dd57 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -57,3 +57,4 @@ obj-y				+= cardreader/
 obj-$(CONFIG_PVPANIC)   	+= pvpanic.o
 obj-$(CONFIG_HABANA_AI)		+= habanalabs/
 obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
+obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
new file mode 100644
index 000000000000..32015bc9ccf6
--- /dev/null
+++ b/drivers/misc/hisi_hikey_usb.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for usb functionality of Hikey series boards
+ * based on Hisilicon Kirin Soc.
+ *
+ * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ *		http://www.huawei.com
+ *
+ * Authors: Yu Chen <chenyu56@huawei.com>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/usb/role.h>
+
+#define DEVICE_DRIVER_NAME "hisi_hikey_usb"
+
+#define HUB_VBUS_POWER_ON 1
+#define HUB_VBUS_POWER_OFF 0
+#define USB_SWITCH_TO_HUB 1
+#define USB_SWITCH_TO_TYPEC 0
+#define TYPEC_VBUS_POWER_ON 1
+#define TYPEC_VBUS_POWER_OFF 0
+
+struct hisi_hikey_usb {
+	struct gpio_desc *otg_switch;
+	struct gpio_desc *typec_vbus;
+	struct gpio_desc *hub_vbus;
+
+	struct usb_role_switch *hub_role_sw;
+	struct usb_role_switch *dev_role_sw;
+	struct notifier_block nb;
+};
+
+static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value)
+{
+	gpiod_set_value_cansleep(hisi_hikey_usb->hub_vbus, value);
+}
+
+static void usb_switch_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
+			    int switch_to)
+{
+	gpiod_set_value_cansleep(hisi_hikey_usb->otg_switch, switch_to);
+}
+
+static void usb_typec_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
+				 int value)
+{
+	gpiod_set_value_cansleep(hisi_hikey_usb->typec_vbus, value);
+}
+
+static int hub_usb_role_switch_set(struct device *dev, enum usb_role role)
+{
+	struct hisi_hikey_usb *hisi_hikey_usb = dev_get_drvdata(dev);
+
+	if (!hisi_hikey_usb || !hisi_hikey_usb->dev_role_sw)
+		return -EINVAL;
+
+	switch (role) {
+	case USB_ROLE_NONE:
+		usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_OFF);
+		usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_HUB);
+		hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_ON);
+		break;
+	case USB_ROLE_HOST:
+		hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_OFF);
+		usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
+		usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_ON);
+		break;
+	case USB_ROLE_DEVICE:
+		hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_OFF);
+		usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_OFF);
+		usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
+		break;
+	default:
+		break;
+	}
+
+	return usb_role_switch_set_role(hisi_hikey_usb->dev_role_sw, role);
+}
+
+static enum usb_role hub_usb_role_switch_get(struct device *dev)
+{
+	struct hisi_hikey_usb *hisi_hikey_usb = dev_get_drvdata(dev);
+
+	if (!hisi_hikey_usb || !hisi_hikey_usb->dev_role_sw)
+		return -EINVAL;
+
+	return usb_role_switch_get_role(hisi_hikey_usb->dev_role_sw);
+}
+
+static int hisi_hikey_usb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hisi_hikey_usb *hisi_hikey_usb;
+	struct usb_role_switch_desc hub_role_switch = {NULL};
+
+	hisi_hikey_usb = devm_kzalloc(dev, sizeof(*hisi_hikey_usb), GFP_KERNEL);
+	if (!hisi_hikey_usb)
+		return -ENOMEM;
+
+	hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
+						    GPIOD_OUT_LOW);
+	if (IS_ERR(hisi_hikey_usb->typec_vbus))
+		return PTR_ERR(hisi_hikey_usb->typec_vbus);
+
+	hisi_hikey_usb->otg_switch = devm_gpiod_get(dev, "otg-switch",
+						    GPIOD_OUT_HIGH);
+	if (IS_ERR(hisi_hikey_usb->otg_switch))
+		return PTR_ERR(hisi_hikey_usb->otg_switch);
+
+	/* hub-vdd33-en is optional */
+	hisi_hikey_usb->hub_vbus = devm_gpiod_get_optional(dev, "hub-vdd33-en",
+							   GPIOD_OUT_HIGH);
+	if (IS_ERR(hisi_hikey_usb->hub_vbus))
+		return PTR_ERR(hisi_hikey_usb->hub_vbus);
+
+	hisi_hikey_usb->dev_role_sw = usb_role_switch_get(dev);
+	if (!hisi_hikey_usb->dev_role_sw)
+		return -EPROBE_DEFER;
+	if (IS_ERR(hisi_hikey_usb->dev_role_sw))
+		return PTR_ERR(hisi_hikey_usb->dev_role_sw);
+
+	hub_role_switch.fwnode = dev_fwnode(dev);
+	hub_role_switch.set = hub_usb_role_switch_set;
+	hub_role_switch.get = hub_usb_role_switch_get;
+	hisi_hikey_usb->hub_role_sw = usb_role_switch_register(dev,
+							&hub_role_switch);
+
+	if (IS_ERR(hisi_hikey_usb->hub_role_sw)) {
+		usb_role_switch_put(hisi_hikey_usb->dev_role_sw);
+		return PTR_ERR(hisi_hikey_usb->hub_role_sw);
+	}
+
+	platform_set_drvdata(pdev, hisi_hikey_usb);
+
+	return 0;
+}
+
+static int  hisi_hikey_usb_remove(struct platform_device *pdev)
+{
+	struct hisi_hikey_usb *hisi_hikey_usb = platform_get_drvdata(pdev);
+
+	if (hisi_hikey_usb->hub_role_sw)
+		usb_role_switch_unregister(hisi_hikey_usb->hub_role_sw);
+
+	if (hisi_hikey_usb->dev_role_sw)
+		usb_role_switch_put(hisi_hikey_usb->dev_role_sw);
+
+	return 0;
+}
+
+static const struct of_device_id id_table_hisi_hikey_usb[] = {
+	{.compatible = "hisilicon,gpio_hubv1"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, id_table_hisi_hikey_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 = id_table_hisi_hikey_usb,
+	},
+};
+
+module_platform_driver(hisi_hikey_usb_driver);
+
+MODULE_AUTHOR("Yu Chen <chenyu56@huawei.com>");
+MODULE_DESCRIPTION("Driver Support for USB functionality of Hikey");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
  2019-12-12  1:42 ` [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960 John Stultz
@ 2019-12-18 16:37   ` Rob Herring
  2019-12-18 17:21     ` John Stultz
  0 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2019-12-18 16:37 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Yu Chen, Greg Kroah-Hartman, Mark Rutland, ShuFan Lee,
	Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun, Felipe Balbi,
	Hans de Goede, Andy Shevchenko, Jun Li, Valentin Schneider,
	Guillaume Gardet, Jack Pham, linux-usb, devicetree

On Thu, Dec 12, 2019 at 01:42:32AM +0000, John Stultz wrote:
> From: Yu Chen <chenyu56@huawei.com>
> 
> This patch adds binding documentation to support usb hub and usb
> data role switch of Hisilicon HiKey960 Board.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> CC: ShuFan Lee <shufan_lee@richtek.com>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Yu Chen <chenyu56@huawei.com>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Jun Li <lijun.kernel@gmail.com>
> Cc: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
> Cc: Jack Pham <jackp@codeaurora.org>
> Cc: linux-usb@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Yu Chen <chenyu56@huawei.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v3: Reworked as usb-role-switch intermediary
> 
> v7: Switched over to YAML dt binding description
> ---
>  .../bindings/misc/hisilicon-hikey-usb.yaml    | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
> new file mode 100644
> index 000000000000..1fc3b198ef73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
> @@ -0,0 +1,85 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2019 Linaro Ltd.
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/misc/hisilicon-hikey-usb.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: HiKey960 onboard USB GPIO Hub
> +
> +maintainers:
> +  - John Stultz <john.stultz@linaro.org>
> +
> +description: |
> +  Supports the onboard HiKey960 USB GPIO hub, which acts as a
> +  role-switch intermediary to detect the state of the USB-C
> +  port, to switch the hub into dual-role USB-C or host mode,
> +  which enables the onboard USB-A host ports.

Honestly I'm torn between whatever works for you because this is pretty 
"special" dev board design and it should more accurately match the 
hardware design. I think we can do the later and it doesn't really need 
anything new.

> +
> +  Schematics about the hub can be found here:
> +    https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: hisilicon,gpio_hubv1

As a whole this is HiSilicon specific, but really it is not. It's really 
just a hub, a mux, and connectors for which we have bindings for. I 
think you need to model the actual hub in DT. We have 2 ways already to 
describe hubs in DT: a I2C device or USB device. 

AIUI, the board looks something like this:

ctrl -> mux --> hub -> type-a connector
            +-> type-c connector

If the hub I2C is not used, then you could do something like this:

ctrl {
    mux-controls = <&usb_gpio_mux>;
    connector@0 {
	// type C connector binding
    };
    hub@1 {
	// USB device binding
    };
};

Or if I2C is used and the hub is under the I2C controller:

ctrl {
    port@0 {
        mux-controls = <&usb_gpio_mux>;
        endpoint@0 { // mux state 0
		remote-endpoint = <&usb_c_connector_port>;
	};
        endpoint@1 { // mux state 1
		remote-endpoint = <&usb_hub_port>;
	};
};

The only new bindings you really need are adding 'mux-controls' to the 
USB host controller and the hub binding (we already have a few).

If the USB2 and USB3 signals come from 2 different host controller 
nodes, then I think it will need to look like the 2nd case regardless 
of I2C. (It's strange that USB3 was not routed to Type-C connector. Can 
you do USB2 on Type-C and USB3 on hub simultaneously? You need USB2 to 
enumerate, right?)

> +
> +  typec-vbus-gpios:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the typec-vbus gpio

This should be modeled as a GPIO regulator, and belongs as part of a 
connector node. See bindings/connector/usb-connector.txt.

> +
> +  otg-switch-gpios:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the otg-switch gpio

This would be the gpio-mux binding instead.

> +
> +  hub-vdd33-en-gpios:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the hub 3.3v power enablement gpio

This should be modeled as a GPIO regulator. 

What about the reset line on the hub?

> +
> +  usb-role-switch:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: Support role switch.

This normally is a controller property. Role switch is foreign to the 
hub, so doesn't really belong there for sure.

> +
> +  port:
> +    description: |
> +      any connector to the data bus of this controller should be modelled
> +      using the OF graph bindings specified, if the "usb-role-switch"
> +      property is used. Note for this driver, two ports are supported,
> +      the first being the endpoint that will be notified by this driver,
> +      and the second being the endpoint that notifies this driver of a
> +      role switch.
> +
> +
> +required:
> +  - compatible
> +  - typec-vbus-gpios
> +  - otg-switch-gpios
> +  - hub-vdd33-en-gpios
> +  - usb-role-switch
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hisi_hikey_usb: hisi_hikey_usb {
> +        compatible = "hisilicon,gpio_hubv1";
> +        typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>;
> +        otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>;
> +        hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> +        usb-role-switch;
> +
> +        port {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            hikey_usb_ep0: endpoint@0 {
> +                reg = <0>;
> +                remote-endpoint = <&dwc3_role_switch>;
> +            };
> +            hikey_usb_ep1: endpoint@1 {
> +                reg = <1>;
> +                remote-endpoint = <&rt1711h_ep>;
> +            };
> +        };
> +    };
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
  2019-12-18 16:37   ` Rob Herring
@ 2019-12-18 17:21     ` John Stultz
  2019-12-18 19:56       ` Rob Herring
  0 siblings, 1 reply; 13+ messages in thread
From: John Stultz @ 2019-12-18 17:21 UTC (permalink / raw)
  To: Rob Herring
  Cc: lkml, Yu Chen, Greg Kroah-Hartman, Mark Rutland, ShuFan Lee,
	Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun, Felipe Balbi,
	Hans de Goede, Andy Shevchenko, Jun Li, Valentin Schneider,
	Guillaume Gardet, Jack Pham, Linux USB List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Wed, Dec 18, 2019 at 8:37 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Dec 12, 2019 at 01:42:32AM +0000, John Stultz wrote:
> > From: Yu Chen <chenyu56@huawei.com>
> >
> > This patch adds binding documentation to support usb hub and usb
> > data role switch of Hisilicon HiKey960 Board.
> >
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > CC: ShuFan Lee <shufan_lee@richtek.com>
> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > Cc: Yu Chen <chenyu56@huawei.com>
> > Cc: Felipe Balbi <balbi@kernel.org>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Cc: Jun Li <lijun.kernel@gmail.com>
> > Cc: Valentin Schneider <valentin.schneider@arm.com>
> > Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
> > Cc: Jack Pham <jackp@codeaurora.org>
> > Cc: linux-usb@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Yu Chen <chenyu56@huawei.com>
> > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > ---
> > v3: Reworked as usb-role-switch intermediary
> >
> > v7: Switched over to YAML dt binding description
> > ---
> >  .../bindings/misc/hisilicon-hikey-usb.yaml    | 85 +++++++++++++++++++
> >  1 file changed, 85 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
> > new file mode 100644
> > index 000000000000..1fc3b198ef73
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
> > @@ -0,0 +1,85 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2019 Linaro Ltd.
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/misc/hisilicon-hikey-usb.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: HiKey960 onboard USB GPIO Hub
> > +
> > +maintainers:
> > +  - John Stultz <john.stultz@linaro.org>
> > +
> > +description: |
> > +  Supports the onboard HiKey960 USB GPIO hub, which acts as a
> > +  role-switch intermediary to detect the state of the USB-C
> > +  port, to switch the hub into dual-role USB-C or host mode,
> > +  which enables the onboard USB-A host ports.
>
> Honestly I'm torn between whatever works for you because this is pretty
> "special" dev board design and it should more accurately match the
> hardware design. I think we can do the later and it doesn't really need
> anything new.
>
> > +
> > +  Schematics about the hub can be found here:
> > +    https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: hisilicon,gpio_hubv1
>
> As a whole this is HiSilicon specific, but really it is not. It's really
> just a hub, a mux, and connectors for which we have bindings for. I
> think you need to model the actual hub in DT. We have 2 ways already to
> describe hubs in DT: a I2C device or USB device.
>
> AIUI, the board looks something like this:
>
> ctrl -> mux --> hub -> type-a connector
>             +-> type-c connector
>
> If the hub I2C is not used, then you could do something like this:
>
> ctrl {
>     mux-controls = <&usb_gpio_mux>;
>     connector@0 {
>         // type C connector binding
>     };
>     hub@1 {
>         // USB device binding
>     };
> };

I can't say I totally grok all this, but I'll go digging to try to
better understand it.
I don't believe there is any I2C involved here, so I'll try the
approach you outline above.



> Or if I2C is used and the hub is under the I2C controller:
>
> ctrl {
>     port@0 {
>         mux-controls = <&usb_gpio_mux>;
>         endpoint@0 { // mux state 0
>                 remote-endpoint = <&usb_c_connector_port>;
>         };
>         endpoint@1 { // mux state 1
>                 remote-endpoint = <&usb_hub_port>;
>         };
> };
>
> The only new bindings you really need are adding 'mux-controls' to the
> USB host controller and the hub binding (we already have a few).
>
> If the USB2 and USB3 signals come from 2 different host controller
> nodes, then I think it will need to look like the 2nd case regardless
> of I2C. (It's strange that USB3 was not routed to Type-C connector. Can
> you do USB2 on Type-C and USB3 on hub simultaneously? You need USB2 to
> enumerate, right?)

Yea, it is strange, and I unfortunately don't know why only USB2 was
exported to the type-c connector.
And to my knowledge, you cannot use both the type-c and hub simultaneously.


> > +
> > +  typec-vbus-gpios:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the typec-vbus gpio
>
> This should be modeled as a GPIO regulator, and belongs as part of a
> connector node. See bindings/connector/usb-connector.txt.
>
> > +
> > +  otg-switch-gpios:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the otg-switch gpio
>
> This would be the gpio-mux binding instead.
>
> > +
> > +  hub-vdd33-en-gpios:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle to the hub 3.3v power enablement gpio
>
> This should be modeled as a GPIO regulator.
>
> What about the reset line on the hub?

Unknown. I don't have any details on that.


> > +
> > +  usb-role-switch:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description: Support role switch.
>
> This normally is a controller property. Role switch is foreign to the
> hub, so doesn't really belong there for sure.

So this part was critical to being able to get role switch
notification from the connector and to properly switch modes without
adding extra notifier gunk from the previous patch that folks didn't
like.

Trying to understand further,  your suggestion here is to re-model the
binding, as gpio regulators and gpio muxes, and use a usb-connector
node to describe them,  but I'm missing how I connect that to the
driver implementation I have? Is the idea to extend the rt1711h and
dwc3 drivers further to support the mux/hub bit (this part is fairly
foggy to me), completely removing the need for the misc driver?

I did take an attempt at something similar with an earlier iteration
of the patch set, where I was trying to move the vbus-gpio as a
gpio-regulator to be controlled by the rt1711h/tpcm core, but that
approach didn't work properly and Hans suggested I just go back to the
approach submitted here:
  https://lkml.org/lkml/2019/10/22/42

thanks
-john

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
  2019-12-18 17:21     ` John Stultz
@ 2019-12-18 19:56       ` Rob Herring
  2020-01-22 18:28         ` John Stultz
  0 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2019-12-18 19:56 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Yu Chen, Greg Kroah-Hartman, Mark Rutland, ShuFan Lee,
	Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun, Felipe Balbi,
	Hans de Goede, Andy Shevchenko, Jun Li, Valentin Schneider,
	Guillaume Gardet, Jack Pham, Linux USB List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Wed, Dec 18, 2019 at 11:21 AM John Stultz <john.stultz@linaro.org> wrote:
>
> On Wed, Dec 18, 2019 at 8:37 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Dec 12, 2019 at 01:42:32AM +0000, John Stultz wrote:
> > > From: Yu Chen <chenyu56@huawei.com>
> > >
> > > This patch adds binding documentation to support usb hub and usb
> > > data role switch of Hisilicon HiKey960 Board.
> > >
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > CC: ShuFan Lee <shufan_lee@richtek.com>
> > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > > Cc: Yu Chen <chenyu56@huawei.com>
> > > Cc: Felipe Balbi <balbi@kernel.org>
> > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > Cc: Jun Li <lijun.kernel@gmail.com>
> > > Cc: Valentin Schneider <valentin.schneider@arm.com>
> > > Cc: Guillaume Gardet <Guillaume.Gardet@arm.com>
> > > Cc: Jack Pham <jackp@codeaurora.org>
> > > Cc: linux-usb@vger.kernel.org
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: Yu Chen <chenyu56@huawei.com>
> > > Signed-off-by: John Stultz <john.stultz@linaro.org>
> > > ---
> > > v3: Reworked as usb-role-switch intermediary
> > >
> > > v7: Switched over to YAML dt binding description
> > > ---
> > >  .../bindings/misc/hisilicon-hikey-usb.yaml    | 85 +++++++++++++++++++
> > >  1 file changed, 85 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
> > > new file mode 100644
> > > index 000000000000..1fc3b198ef73
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
> > > @@ -0,0 +1,85 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +# Copyright 2019 Linaro Ltd.
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/misc/hisilicon-hikey-usb.yaml#"
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > +
> > > +title: HiKey960 onboard USB GPIO Hub
> > > +
> > > +maintainers:
> > > +  - John Stultz <john.stultz@linaro.org>
> > > +
> > > +description: |
> > > +  Supports the onboard HiKey960 USB GPIO hub, which acts as a
> > > +  role-switch intermediary to detect the state of the USB-C
> > > +  port, to switch the hub into dual-role USB-C or host mode,
> > > +  which enables the onboard USB-A host ports.
> >
> > Honestly I'm torn between whatever works for you because this is pretty
> > "special" dev board design and it should more accurately match the
> > hardware design. I think we can do the later and it doesn't really need
> > anything new.
> >
> > > +
> > > +  Schematics about the hub can be found here:
> > > +    https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - const: hisilicon,gpio_hubv1
> >
> > As a whole this is HiSilicon specific, but really it is not. It's really
> > just a hub, a mux, and connectors for which we have bindings for. I
> > think you need to model the actual hub in DT. We have 2 ways already to
> > describe hubs in DT: a I2C device or USB device.
> >
> > AIUI, the board looks something like this:
> >
> > ctrl -> mux --> hub -> type-a connector
> >             +-> type-c connector
> >
> > If the hub I2C is not used, then you could do something like this:
> >
> > ctrl {
> >     mux-controls = <&usb_gpio_mux>;
> >     connector@0 {
> >         // type C connector binding
> >     };
> >     hub@1 {
> >         // USB device binding
> >     };
> > };
>
> I can't say I totally grok all this, but I'll go digging to try to
> better understand it.
> I don't believe there is any I2C involved here, so I'll try the
> approach you outline above.

Well, it is there in the schematics.

> > Or if I2C is used and the hub is under the I2C controller:
> >
> > ctrl {
> >     port@0 {
> >         mux-controls = <&usb_gpio_mux>;
> >         endpoint@0 { // mux state 0
> >                 remote-endpoint = <&usb_c_connector_port>;
> >         };
> >         endpoint@1 { // mux state 1
> >                 remote-endpoint = <&usb_hub_port>;
> >         };
> > };
> >
> > The only new bindings you really need are adding 'mux-controls' to the
> > USB host controller and the hub binding (we already have a few).
> >
> > If the USB2 and USB3 signals come from 2 different host controller
> > nodes, then I think it will need to look like the 2nd case regardless
> > of I2C. (It's strange that USB3 was not routed to Type-C connector. Can
> > you do USB2 on Type-C and USB3 on hub simultaneously? You need USB2 to
> > enumerate, right?)
>
> Yea, it is strange, and I unfortunately don't know why only USB2 was
> exported to the type-c connector.
> And to my knowledge, you cannot use both the type-c and hub simultaneously.
>
>
> > > +
> > > +  typec-vbus-gpios:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description: phandle to the typec-vbus gpio
> >
> > This should be modeled as a GPIO regulator, and belongs as part of a
> > connector node. See bindings/connector/usb-connector.txt.
> >
> > > +
> > > +  otg-switch-gpios:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description: phandle to the otg-switch gpio
> >
> > This would be the gpio-mux binding instead.
> >
> > > +
> > > +  hub-vdd33-en-gpios:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description: phandle to the hub 3.3v power enablement gpio
> >
> > This should be modeled as a GPIO regulator.
> >
> > What about the reset line on the hub?
>
> Unknown. I don't have any details on that.

You might just be getting lucky that it is pulled to the right state.


> > > +  usb-role-switch:
> > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > +    description: Support role switch.
> >
> > This normally is a controller property. Role switch is foreign to the
> > hub, so doesn't really belong there for sure.
>
> So this part was critical to being able to get role switch
> notification from the connector and to properly switch modes without
> adding extra notifier gunk from the previous patch that folks didn't
> like.
>
> Trying to understand further,  your suggestion here is to re-model the
> binding, as gpio regulators and gpio muxes, and use a usb-connector
> node to describe them,  but I'm missing how I connect that to the
> driver implementation I have?

Good question, but that shouldn't really dictate your binding design.

> Is the idea to extend the rt1711h and
> dwc3 drivers further to support the mux/hub bit (this part is fairly
> foggy to me), completely removing the need for the misc driver?

I imagine that you need some driver to determine the state of the mux.
Perhaps a usb-mux driver which is instantiated by the host controller
driver when it sees a mux-controls property. Sorry, haven't looked at
the driver side of this at all.

> I did take an attempt at something similar with an earlier iteration
> of the patch set, where I was trying to move the vbus-gpio as a
> gpio-regulator to be controlled by the rt1711h/tpcm core, but that
> approach didn't work properly and Hans suggested I just go back to the
> approach submitted here:
>   https://lkml.org/lkml/2019/10/22/42

I don't see why that would matter. If you need to sense the Vbus
state, then you do need a GPIO typically. But for an enable line, it's
just another level of abstraction.

Rob

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
  2019-12-18 19:56       ` Rob Herring
@ 2020-01-22 18:28         ` John Stultz
  0 siblings, 0 replies; 13+ messages in thread
From: John Stultz @ 2020-01-22 18:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: lkml, Yu Chen, Greg Kroah-Hartman, Mark Rutland, ShuFan Lee,
	Heikki Krogerus, Suzuki K Poulose, Chunfeng Yun, Felipe Balbi,
	Hans de Goede, Andy Shevchenko, Jun Li, Valentin Schneider,
	Guillaume Gardet, Jack Pham, Linux USB List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

(Sorry for the slow reply. The holidays and other priorities struck
and I'm only now just getting back to this!)

On Wed, Dec 18, 2019 at 11:57 AM Rob Herring <robh@kernel.org> wrote:
> On Wed, Dec 18, 2019 at 11:21 AM John Stultz <john.stultz@linaro.org> wrote:
> > On Wed, Dec 18, 2019 at 8:37 AM Rob Herring <robh@kernel.org> wrote:
> > > As a whole this is HiSilicon specific, but really it is not. It's really
> > > just a hub, a mux, and connectors for which we have bindings for. I
> > > think you need to model the actual hub in DT. We have 2 ways already to
> > > describe hubs in DT: a I2C device or USB device.
> > >
> > > AIUI, the board looks something like this:
> > >
> > > ctrl -> mux --> hub -> type-a connector
> > >             +-> type-c connector
> > >
> > > If the hub I2C is not used, then you could do something like this:
> > >
> > > ctrl {
> > >     mux-controls = <&usb_gpio_mux>;
> > >     connector@0 {
> > >         // type C connector binding
> > >     };
> > >     hub@1 {
> > >         // USB device binding
> > >     };
> > > };
> >
> > I can't say I totally grok all this, but I'll go digging to try to
> > better understand it.
> > I don't believe there is any I2C involved here, so I'll try the
> > approach you outline above.
>
> Well, it is there in the schematics.

Fair point. Though at least for the USB5734 hub, I don't believe we've
made use of the i2c on it (at least that I can see).  From the
existing driver, the control is basically just 3 GPIOs:  type-c power,
hub power, and the mux/switch.

Trying to get my head around your suggestion:
ctrl {
    mux-controls = <&usb_gpio_mux>;
    connector@0 {
        // type C connector binding
    };
    hub@1 {
        // USB device binding
    };
};

The usb_gpio_mux would be the gpio mux switch.

For the "type C connector binding", that would probably be the rt1711h
type-c chip.

For the "USB device binding" that would be a yet to be implemented
USB5734 hub driver, I'm guessing?

Though I'm not sure I understand how the hub driver gets a signal to
power-on/power-off its gpio-regulator from the mux state?  I'm maybe
still missing some details on the mux infrastructure.

> > > The only new bindings you really need are adding 'mux-controls' to the
> > > USB host controller and the hub binding (we already have a few).

So this is a little confusing to me. Why is the host controller
involved?  It seems to me all of this is down-stream of the
controller, and it's just taking whatever the switch gives it.

I think the switch needs to be signalled from the rt1711h type-c
connector (when it detects the cable and determines the role). That
said, I'm not sure how it would think to control the mux in that case
(as that's pretty special case for this specific hardware). That's why
we're using the role notifier intermediary trick in the current code.

So I guess we could still have the roll notifier intermediary driver
which then controls the mux?

I know that's more a driver implementation detail and not really
hardware description, :) but I'm just trying to figure out how it's
going to come together and actually work.

> > Is the idea to extend the rt1711h and
> > dwc3 drivers further to support the mux/hub bit (this part is fairly
> > foggy to me), completely removing the need for the misc driver?
>
> I imagine that you need some driver to determine the state of the mux.
> Perhaps a usb-mux driver which is instantiated by the host controller
> driver when it sees a mux-controls property. Sorry, haven't looked at
> the driver side of this at all.
>
> > I did take an attempt at something similar with an earlier iteration
> > of the patch set, where I was trying to move the vbus-gpio as a
> > gpio-regulator to be controlled by the rt1711h/tpcm core, but that
> > approach didn't work properly and Hans suggested I just go back to the
> > approach submitted here:
> >   https://lkml.org/lkml/2019/10/22/42
>
> I don't see why that would matter. If you need to sense the Vbus
> state, then you do need a GPIO typically. But for an enable line, it's
> just another level of abstraction.

My concern is that I suspect the issue we had then was that the order
and the timing that we switch the 3 GPIOs ends up being important. In
the current implementation we can adjust them linearly together, where
as when I took a stab at having the vbus gpio was modeled as a
regulator and controlled independently by the rt1711h, the typec state
machine would get confused as I'm guessing the mux/switch wasn't where
it expected it to be when it wanted to change the state of the type-c
vbus.

Thoughts?

Thanks again for the review and feedback. And sorry to let so much
time (and mental context) pass.
-john

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-01-22 18:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-12  1:42 [PATCH v7 0/8] HiKey960 USB support John Stultz
2019-12-12  1:42 ` [PATCH v7 1/8] usb: dwc3: Registering a role switch in the DRD code John Stultz
2019-12-12  1:42 ` [PATCH v7 2/8] dt-bindings: usb: generic: Add role-switch-default-mode binding John Stultz
2019-12-12  1:42 ` [PATCH v7 3/8] usb: dwc3: Add support for " John Stultz
2019-12-12  1:42 ` [PATCH v7 4/8] dt-bindings: usb: dwc3: Allow clock list & resets to be more flexible John Stultz
2019-12-12  1:42 ` [PATCH v7 5/8] usb: dwc3: Rework clock initialization " John Stultz
2019-12-12  1:42 ` [PATCH v7 6/8] usb: dwc3: Rework resets " John Stultz
2019-12-12  1:42 ` [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960 John Stultz
2019-12-18 16:37   ` Rob Herring
2019-12-18 17:21     ` John Stultz
2019-12-18 19:56       ` Rob Herring
2020-01-22 18:28         ` John Stultz
2019-12-12  1:42 ` [PATCH v7 8/8] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960 John Stultz

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).