HiKey960 USB support
v7 cover HiKey960 USB support
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
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
Signature · 1 line
2.17.1
PATCH v7 1/8 usb: dwc3: Registering a role switch in the DRD code.
From: Yu Chen <[email protected]>
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.
---
v2: Fix role_sw and role_switch_default_mode descriptions as
reported by kbuild test robot <[email protected]>
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 <[email protected]>
v7: Minor fix for CONFIG_USB_ROLE_SWITCH=m case not building
the role switch handling code, reported by Guillaume Gardet <[email protected]>
--- drivers/usb/dwc3/core.h | 3 ++ drivers/usb/dwc3/drd.c | 77 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-)
drivers/usb/dwc3/core.h +3 -0
@@ -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;
drivers/usb/dwc3/drd.c +76 -1
95 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v7 2/8 dt-bindings: usb: generic: Add role-switch-default-mode binding
In reply to John StultzAdd binding to configure the default role the controller assumes is host mode when the usb role is USB_ROLE_NONE.
---
v5: Switch to string rather then a bool
--- Documentation/devicetree/bindings/usb/generic.txt | 6 ++++++ 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/usb/generic.txt +6 -0
@@ -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:
Signature · 1 line
2.17.1
PATCH v7 3/8 usb: dwc3: Add support for role-switch-default-mode binding
In reply to John StultzSupport 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 <[email protected]>
---
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(-)
drivers/usb/dwc3/core.h +3 -0
@@ -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;
drivers/usb/dwc3/drd.c +22 -3
@@ -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
Signature · 1 line
2.17.1
PATCH v7 4/8 dt-bindings: usb: dwc3: Allow clock list & resets to be more flexible
In reply to John StultzRather 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.
--- Documentation/devicetree/bindings/usb/dwc3.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/usb/dwc3.txt +3 -2
@@ -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
Signature · 1 line
2.17.1
PATCH v7 5/8 usb: dwc3: Rework clock initialization to be more flexible
In reply to John StultzThe 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 <[email protected]> as an alternative to creating multiple bindings for each variant of hardware.
---
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(-)
drivers/usb/dwc3/core.c +5 -15
@@ -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);
Signature · 1 line
2.17.1
PATCH v7 6/8 usb: dwc3: Rework resets initialization to be more flexible
In reply to John StultzThe 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 <[email protected]> as an alternative to creating multiple bindings for each variant of hardware.
---
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(-)
drivers/usb/dwc3/core.c +1 -1
@@ -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);
Signature · 1 line
2.17.1
PATCH v7 7/8 dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
In reply to John StultzFrom: Yu Chen <[email protected]>
This patch adds binding documentation to support usb hub and usb data role switch of Hisilicon HiKey960 Board.
---
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
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +85 -0
@@ -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 <[email protected]> + +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>; + }; + }; + };
Signature · 1 line
2.17.1
Rob Herring
maintainerRe: [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
Quoted 46 lines · John Stultz
From: Yu Chen <[email protected]>
This patch adds binding documentation to support usb hub and usb data role switch of Hisilicon HiKey960 Board.
Cc Greg Kroah-Hartman <[email protected]>Cc Rob Herring <[email protected]>Cc Mark Rutland <[email protected]>CC ShuFan Lee <[email protected]>Cc Heikki Krogerus <[email protected]>Cc Suzuki K Poulose <[email protected]>Cc Chunfeng Yun <[email protected]>Cc Yu Chen <[email protected]>Cc Felipe Balbi <[email protected]>Cc Hans de Goede <[email protected]>Cc Andy Shevchenko <[email protected]>Cc Jun Li <[email protected]>Cc Valentin Schneider <[email protected]>Cc Guillaume Gardet <[email protected]>Cc Jack Pham <[email protected]>Signed-off-by Yu Chen <[email protected]>Signed-off-by John Stultz <[email protected]>---
v3: Reworked as usb-role-switch intermediaryv7: 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.yamlDocumentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +7
-0@@ -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#" +
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 17
Quoted 10 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +10
-0+title: HiKey960 onboard USB GPIO Hub + +maintainers: + - John Stultz <[email protected]> + +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.
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 25
Quoted 8 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +8
-0+ + 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?)
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 29
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + 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.
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + otg-switch-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the otg-switch gpio
This would be the gpio-mux binding instead.
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 37
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + 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?
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 41
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + 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.
Quoted 47 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +44
-0+ + 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>; + }; + }; + };Signature · 1 line
2.17.1
Re: [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
Quoted 85 lines · Rob Herring
Quoted 46 lines · John Stultz
From: Yu Chen <[email protected]>
This patch adds binding documentation to support usb hub and usb data role switch of Hisilicon HiKey960 Board.
Cc Greg Kroah-Hartman <[email protected]>Cc Rob Herring <[email protected]>Cc Mark Rutland <[email protected]>CC ShuFan Lee <[email protected]>Cc Heikki Krogerus <[email protected]>Cc Suzuki K Poulose <[email protected]>Cc Chunfeng Yun <[email protected]>Cc Yu Chen <[email protected]>Cc Felipe Balbi <[email protected]>Cc Hans de Goede <[email protected]>Cc Andy Shevchenko <[email protected]>Cc Jun Li <[email protected]>Cc Valentin Schneider <[email protected]>Cc Guillaume Gardet <[email protected]>Cc Jack Pham <[email protected]>Signed-off-by Yu Chen <[email protected]>Signed-off-by John Stultz <[email protected]>---
v3: Reworked as usb-role-switch intermediaryv7: 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.yamlDocumentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +7
-0@@ -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#" +Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 17
Quoted 10 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +10
-0+title: HiKey960 onboard USB GPIO Hub + +maintainers: + - John Stultz <[email protected]> + +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.
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 25
Quoted 8 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +8
-0+ + 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_hubv1As 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 connectorIf the hub I2C is not used, then you could do something like this:
Quoted 9 lines
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.
Quoted 13 lines
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>; }; };
Quoted 8 lines
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.
Quoted 15 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 29
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + typec-vbus-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the typec-vbus gpioThis should be modeled as a GPIO regulator, and belongs as part of a connector node. See
bindings/connector/usb-connector.txt.Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + otg-switch-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the otg-switch gpioThis would be the gpio-mux binding instead.
Quoted 8 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 37
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + hub-vdd33-en-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the hub 3.3v power enablement gpioThis should be modeled as a GPIO regulator.
What about the reset line on the hub?
Unknown. I don't have any details on that.
Quoted 7 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 41
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + 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
Rob Herring
maintainerRe: [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
Quoted 97 lines · John Stultz
Quoted 85 lines · Rob Herring
Quoted 46 lines · John Stultz
From: Yu Chen <[email protected]>
This patch adds binding documentation to support usb hub and usb data role switch of Hisilicon HiKey960 Board.
Cc Greg Kroah-Hartman <[email protected]>Cc Rob Herring <[email protected]>Cc Mark Rutland <[email protected]>CC ShuFan Lee <[email protected]>Cc Heikki Krogerus <[email protected]>Cc Suzuki K Poulose <[email protected]>Cc Chunfeng Yun <[email protected]>Cc Yu Chen <[email protected]>Cc Felipe Balbi <[email protected]>Cc Hans de Goede <[email protected]>Cc Andy Shevchenko <[email protected]>Cc Jun Li <[email protected]>Cc Valentin Schneider <[email protected]>Cc Guillaume Gardet <[email protected]>Cc Jack Pham <[email protected]>Signed-off-by Yu Chen <[email protected]>Signed-off-by John Stultz <[email protected]>---
v3: Reworked as usb-role-switch intermediaryv7: 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.yamlDocumentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +7
-0@@ -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#" +Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 17
Quoted 10 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +10
-0+title: HiKey960 onboard USB GPIO Hub + +maintainers: + - John Stultz <[email protected]> + +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.
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 25
Quoted 8 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +8
-0+ + 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_hubv1As 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 connectorIf the hub I2C is not used, then you could do something like this:
Quoted 9 lines
ctrl { mux-controls = <&usb_gpio_mux>; connector@0 { // type C connector binding }; hub@1 { // USB device binding }; };
Quoted 4 lines
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.
Quoted 47 lines
Quoted 13 lines
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>; }; };Quoted 8 lines
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.Quoted 20 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 29
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + typec-vbus-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the typec-vbus gpioThis should be modeled as a GPIO regulator, and belongs as part of a connector node. See
bindings/connector/usb-connector.txt.Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 33
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + otg-switch-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the otg-switch gpioThis would be the gpio-mux binding instead.
Quoted 4 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +4
-0+ + hub-vdd33-en-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the hub 3.3v power enablement gpio
Quoted 5 lines
Quoted 3 lines
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.
Quoted 12 lines
Quoted 6 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml line 41
Quoted 3 lines
Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml new file +3
-0+ 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.
Quoted 4 lines
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.
Quoted 3 lines
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.
Quoted 6 lines
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
Re: [PATCH v7 7/8] dt-bindings: misc: Add bindings for HiSilicon usb hub and data role switch functionality on HiKey960
(Sorry for the slow reply. The holidays and other priorities struck and I'm only now just getting back to this!)
Quoted 24 lines · Rob Herring
Quoted 23 lines · John Stultz
Quoted 21 lines · Rob Herring
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 connectorIf 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 }; };
Quoted 6 lines
Quoted 4 lines
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.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
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.
Quoted 16 lines
Quoted 3 lines
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.
Quoted 6 lines
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
Quoted 3 lines
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
PATCH v7 8/8 misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960
In reply to John StultzFrom: Yu Chen <[email protected]>
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.
[jstultz: Major rework to make the driver a usb-role-switch
intermediary]
---
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
MAINTAINERS +7 -0
@@ -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 <[email protected]> +L: [email protected] +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 <[email protected]> L: [email protected]
drivers/misc/Kconfig +9 -0
@@ -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"
drivers/misc/Makefile +1 -0
@@ -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
drivers/misc/hisi_hikey_usb.c new file +178 -0
178 lines not on this page · raw message
Signature · 1 line
2.17.1