The patchset adds support for usb functionality of Hikey960, includes:
- dwc3 driver for Hisilicon Kirin Soc hi3660
- usb driver for HiKey960 board
- some adjustment in dwc3, usb gadget and typec driver
- dts for support usb of HiKey960
Yu Chen (12):
dt-bindings: usb: add support for dwc3 controller on HiSilicon SoCs
dt-bindings: phy: Add support for HiSilicon's hi3660 USB PHY
dt-bindings: misc: Add bindings for HiSilicon usb hub and data role
switch functionality on HiKey960
usb: dwc3: dwc3-hisi: Add code for dwc3 of Hisilicon Soc Platform
usb: dwc3: Add two quirks for Hisilicon Kirin Soc Platform
phy: Add usb phy support for hi3660 Soc of Hisilicon
usb: roles: Find the usb role switch by also matching against the
device node
usb: roles: Add usb role switch notifier.
usb: dwc3: Registering a role switch in the DRD code.
hikey960: Support usb functionality of Hikey960
usb: gadget: Add configfs attribuite for controling
match_existing_only
dts: hi3660: Add support for usb on Hikey960
.../bindings/misc/hisilicon-hikey-usb.txt | 36 +++
.../devicetree/bindings/phy/phy-hi3660-usb3.txt | 21 ++
.../devicetree/bindings/usb/dwc3-hisi.txt | 67 +++++
MAINTAINERS | 10 +
arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 56 ++++
arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 72 +++++
drivers/misc/Kconfig | 7 +
drivers/misc/Makefile | 1 +
drivers/misc/hisi_hikey_usb.c | 198 ++++++++++++
drivers/phy/hisilicon/Kconfig | 10 +
drivers/phy/hisilicon/Makefile | 1 +
drivers/phy/hisilicon/phy-hi3660-usb3.c | 237 +++++++++++++++
drivers/usb/common/roles.c | 60 +++-
drivers/usb/dwc3/Kconfig | 8 +
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/core.c | 43 +++
drivers/usb/dwc3/core.h | 9 +
drivers/usb/dwc3/drd.c | 49 +++
drivers/usb/dwc3/dwc3-hisi.c | 335 +++++++++++++++++++++
drivers/usb/dwc3/gadget.c | 2 +-
drivers/usb/gadget/configfs.c | 32 ++
include/linux/usb/role.h | 46 +++
22 files changed, 1299 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
create mode 100644 Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
create mode 100644 Documentation/devicetree/bindings/usb/dwc3-hisi.txt
create mode 100644 drivers/misc/hisi_hikey_usb.c
create mode 100644 drivers/phy/hisilicon/phy-hi3660-usb3.c
create mode 100644 drivers/usb/dwc3/dwc3-hisi.c
--
2.15.0-rc2
This patch adds binding documentation for supporting the hi3660 usb
phy on boards like the HiKey960.
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Binghui Wang <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
--
v1: Fix some format error as suggested by Rob.
--
---
.../devicetree/bindings/phy/phy-hi3660-usb3.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
diff --git a/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
new file mode 100644
index 000000000000..8ea52d51aa13
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
@@ -0,0 +1,21 @@
+Hisilicon hi3660 usb PHY
+-----------------------
+
+Required properties:
+- compatible: should be "hisilicon,hi3660-usb-phy"
+- #phy-cells: must be 0
+- hisilicon,pericrg-syscon: phandle of syscon used to control phy.
+- hisilicon,pctrl-syscon: phandle of syscon used to control phy.
+- hisilicon,usb3-otg-bc-syscon: phandle of syscon used to control phy.
+- hisilicon,eye-diagram-param: parameter set for phy
+Refer to phy/phy-bindings.txt for the generic PHY binding properties
+
+Example:
+ usb-phy {
+ compatible = "hisilicon,hi3660-usb-phy";
+ #phy-cells = <0>;
+ hisilicon,pericrg-syscon = <&crg_ctrl>;
+ hisilicon,pctrl-syscon = <&pctrl>;
+ hisilicon,usb3-otg-bc-syscon = <&usb3_otg_bc>;
+ hisilicon,eye-diagram-param = <0x22466e4>;
+ };
--
2.15.0-rc2
This patch adds binding descriptions to support the dwc3 controller
on HiSilicon SoCs and boards like the HiKey960.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: John Stultz <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
---
.../devicetree/bindings/usb/dwc3-hisi.txt | 67 ++++++++++++++++++++++
1 file changed, 67 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/dwc3-hisi.txt
diff --git a/Documentation/devicetree/bindings/usb/dwc3-hisi.txt b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
new file mode 100644
index 000000000000..d32d2299a0a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
@@ -0,0 +1,67 @@
+HiSilicon DWC3 USB SoC controller
+
+This file documents the parameters for the dwc3-hisi driver.
+
+Required properties:
+- compatible: should be "hisilicon,hi3660-dwc3"
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Specify clock names
+- resets: list of phandle and reset specifier pairs.
+
+Sub-nodes:
+The dwc3 core should be added as subnode to HiSilicon DWC3 as shown in the
+example below. The DT binding details of dwc3 can be found in:
+Documentation/devicetree/bindings/usb/dwc3.txt
+
+Example:
+ usb3: hisi_dwc3 {
+ compatible = "hisilicon,hi3660-dwc3";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
+ <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
+ clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
+ assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
+ assigned-clock-rates = <229000000>;
+ resets = <&crg_rst 0x90 8>,
+ <&crg_rst 0x90 7>,
+ <&crg_rst 0x90 6>,
+ <&crg_rst 0x90 5>;
+
+ dwc3: dwc3@ff100000 {
+ compatible = "snps,dwc3";
+ reg = <0x0 0xff100000 0x0 0x100000>;
+ interrupts = <0 159 4>, <0 161 4>;
+ phys = <&usb_phy>;
+ phy-names = "usb3-phy";
+ dr_mode = "otg";
+ maximum-speed = "super-speed";
+ phy_type = "utmi";
+ snps,dis-del-phy-power-chg-quirk;
+ snps,lfps_filter_quirk;
+ snps,dis_u2_susphy_quirk;
+ snps,dis_u3_susphy_quirk;
+ snps,tx_de_emphasis_quirk;
+ snps,tx_de_emphasis = <1>;
+ snps,dis_enblslpm_quirk;
+ snps,gctl-reset-quirk;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dwc3_role_switch: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&rt1711h_ep>;
+ };
+
+ dwc3_role_switch_notify: endpoint@1 {
+ reg = <1>;
+ remote-endpoint = <&hikey_usb_ep>;
+ };
+ };
+ };
+ };
--
2.15.0-rc2
This driver handles usb phy power on and shutdown for hi3660 Soc of
Hisilicon.
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Pengcheng Li <[email protected]>
Cc: Jianguo Sun <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Jiancheng Xue <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Binghui Wang <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
--
v1: Remove unused code and add comment for time delay as suggested by
Kishon Vijay Abraham I.
--
---
MAINTAINERS | 2 +
drivers/phy/hisilicon/Kconfig | 10 ++
drivers/phy/hisilicon/Makefile | 1 +
drivers/phy/hisilicon/phy-hi3660-usb3.c | 237 ++++++++++++++++++++++++++++++++
4 files changed, 250 insertions(+)
create mode 100644 drivers/phy/hisilicon/phy-hi3660-usb3.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 1b6b3026804c..1c09c2ca516d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15471,7 +15471,9 @@ M: Binghui Wang <[email protected]>
L: [email protected]
S: Maintained
F: Documentation/devicetree/bindings/usb/dwc3-hisi.txt
+F: Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
F: drivers/usb/dwc3/dwc3-hisi.c
+F: drivers/phy/hisilicon/phy-hi3660-usb3.c
USB ISP116X DRIVER
M: Olav Kongas <[email protected]>
diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig
index b40ee54a1a50..3c142f08987c 100644
--- a/drivers/phy/hisilicon/Kconfig
+++ b/drivers/phy/hisilicon/Kconfig
@@ -12,6 +12,16 @@ config PHY_HI6220_USB
To compile this driver as a module, choose M here.
+config PHY_HI3660_USB
+ tristate "hi3660 USB PHY support"
+ depends on (ARCH_HISI && ARM64) || COMPILE_TEST
+ select GENERIC_PHY
+ select MFD_SYSCON
+ help
+ Enable this to support the HISILICON HI3660 USB PHY.
+
+ To compile this driver as a module, choose M here.
+
config PHY_HISTB_COMBPHY
tristate "HiSilicon STB SoCs COMBPHY support"
depends on (ARCH_HISI && ARM64) || COMPILE_TEST
diff --git a/drivers/phy/hisilicon/Makefile b/drivers/phy/hisilicon/Makefile
index f662a4fe18d8..75ba64e2faf8 100644
--- a/drivers/phy/hisilicon/Makefile
+++ b/drivers/phy/hisilicon/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_PHY_HI6220_USB) += phy-hi6220-usb.o
+obj-$(CONFIG_PHY_HI3660_USB) += phy-hi3660-usb3.o
obj-$(CONFIG_PHY_HISTB_COMBPHY) += phy-histb-combphy.o
obj-$(CONFIG_PHY_HISI_INNO_USB2) += phy-hisi-inno-usb2.o
obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o
diff --git a/drivers/phy/hisilicon/phy-hi3660-usb3.c b/drivers/phy/hisilicon/phy-hi3660-usb3.c
new file mode 100644
index 000000000000..102966e49e01
--- /dev/null
+++ b/drivers/phy/hisilicon/phy-hi3660-usb3.c
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Phy provider for USB 3.0 controller on HiSilicon 3660 platform
+ *
+ * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ * http://www.huawei.com
+ *
+ * Authors: Yu Chen <[email protected]>
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+
+#define PERI_CRG_CLK_EN4 (0x40)
+#define PERI_CRG_CLK_DIS4 (0x44)
+#define GT_CLK_USB3OTG_REF BIT(0)
+#define GT_ACLK_USB3OTG BIT(1)
+
+#define PERI_CRG_RSTEN4 (0x90)
+#define PERI_CRG_RSTDIS4 (0x94)
+#define IP_RST_USB3OTGPHY_POR BIT(3)
+#define IP_RST_USB3OTG BIT(5)
+
+#define PERI_CRG_ISODIS (0x148)
+#define USB_REFCLK_ISO_EN BIT(25)
+
+#define PCTRL_PERI_CTRL3 (0x10)
+#define PCTRL_PERI_CTRL3_MSK_START (16)
+#define USB_TCXO_EN BIT(1)
+
+#define PCTRL_PERI_CTRL24 (0x64)
+#define SC_CLK_USB3PHY_3MUX1_SEL BIT(25)
+
+#define USBOTG3_CTRL0 (0x00)
+#define SC_USB3PHY_ABB_GT_EN BIT(15)
+
+#define USBOTG3_CTRL2 (0x08)
+#define USBOTG3CTRL2_POWERDOWN_HSP BIT(0)
+#define USBOTG3CTRL2_POWERDOWN_SSP BIT(1)
+
+#define USBOTG3_CTRL3 (0x0C)
+#define USBOTG3_CTRL3_VBUSVLDEXT BIT(6)
+#define USBOTG3_CTRL3_VBUSVLDEXTSEL BIT(5)
+
+#define USBOTG3_CTRL4 (0x10)
+
+#define USBOTG3_CTRL7 (0x1c)
+#define REF_SSP_EN BIT(16)
+
+#define HI3660_USB_DEFAULT_PHY_PARAM (0x1c466e3)
+
+struct hi3660_priv {
+ struct device *dev;
+ struct regmap *peri_crg;
+ struct regmap *pctrl;
+ struct regmap *otg_bc;
+ u32 eye_diagram_param;
+
+ u32 peri_crg_offset;
+ u32 pctrl_offset;
+ u32 otg_bc_offset;
+};
+
+static int hi3660_phy_init(struct phy *phy)
+{
+ struct hi3660_priv *priv = phy_get_drvdata(phy);
+ u32 val, mask;
+ int ret;
+
+ /* usb refclk iso disable */
+ ret = regmap_write(priv->peri_crg, PERI_CRG_ISODIS, USB_REFCLK_ISO_EN);
+ if (ret)
+ goto out;
+
+ /* enable usb_tcxo_en */
+ val = USB_TCXO_EN | (USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START);
+ ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3, val);
+ if (ret)
+ goto out;
+
+ /* assert phy */
+ val = IP_RST_USB3OTGPHY_POR | IP_RST_USB3OTG;
+ ret = regmap_write(priv->peri_crg, PERI_CRG_RSTEN4, val);
+ if (ret)
+ goto out;
+
+ /* enable phy ref clk */
+ val = SC_USB3PHY_ABB_GT_EN;
+ mask = val;
+ ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL0, mask, val);
+ if (ret)
+ goto out;
+
+ val = REF_SSP_EN;
+ mask = val;
+ ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL7, mask, val);
+ if (ret)
+ goto out;
+
+ /* exit from IDDQ mode */
+ mask = USBOTG3CTRL2_POWERDOWN_HSP | USBOTG3CTRL2_POWERDOWN_SSP;
+ ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL2, mask, 0);
+ if (ret)
+ goto out;
+
+ /* delay for exit from IDDQ mode */
+ usleep_range(100, 100);
+
+ /* deassert phy */
+ val = IP_RST_USB3OTGPHY_POR | IP_RST_USB3OTG;
+ ret = regmap_write(priv->peri_crg, PERI_CRG_RSTDIS4, val);
+ if (ret)
+ goto out;
+
+ /* delay for phy deasserted */
+ usleep_range(10000, 15000);
+
+ /* fake vbus valid signal */
+ val = USBOTG3_CTRL3_VBUSVLDEXT | USBOTG3_CTRL3_VBUSVLDEXTSEL;
+ mask = val;
+ ret = regmap_update_bits(priv->otg_bc, USBOTG3_CTRL3, mask, val);
+ if (ret)
+ goto out;
+
+ /* delay for vbus valid */
+ usleep_range(100, 100);
+
+ ret = regmap_write(priv->otg_bc, USBOTG3_CTRL4,
+ priv->eye_diagram_param);
+ if (ret)
+ goto out;
+
+ return 0;
+out:
+ dev_err(priv->dev, "failed to init phy ret: %d\n", ret);
+ return ret;
+}
+
+static int hi3660_phy_exit(struct phy *phy)
+{
+ struct hi3660_priv *priv = phy_get_drvdata(phy);
+ u32 val;
+ int ret;
+
+ /* assert phy */
+ val = IP_RST_USB3OTGPHY_POR;
+ ret = regmap_write(priv->peri_crg, PERI_CRG_RSTEN4, val);
+ if (ret)
+ goto out;
+
+ /* disable usb_tcxo_en */
+ val = USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START;
+ ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3, val);
+ if (ret)
+ goto out;
+
+ return 0;
+out:
+ dev_err(priv->dev, "failed to exit phy ret: %d\n", ret);
+ return ret;
+}
+
+static struct phy_ops hi3660_phy_ops = {
+ .init = hi3660_phy_init,
+ .exit = hi3660_phy_exit,
+ .owner = THIS_MODULE,
+};
+
+static int hi3660_phy_probe(struct platform_device *pdev)
+{
+ struct phy_provider *phy_provider;
+ struct device *dev = &pdev->dev;
+ struct phy *phy;
+ struct hi3660_priv *priv;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = dev;
+ priv->peri_crg = syscon_regmap_lookup_by_phandle(dev->of_node,
+ "hisilicon,pericrg-syscon");
+ if (IS_ERR(priv->peri_crg)) {
+ dev_err(dev, "no hisilicon,pericrg-syscon\n");
+ return PTR_ERR(priv->peri_crg);
+ }
+
+ priv->pctrl = syscon_regmap_lookup_by_phandle(dev->of_node,
+ "hisilicon,pctrl-syscon");
+ if (IS_ERR(priv->pctrl)) {
+ dev_err(dev, "no hisilicon,pctrl-syscon\n");
+ return PTR_ERR(priv->pctrl);
+ }
+
+ priv->otg_bc = syscon_regmap_lookup_by_phandle(dev->of_node,
+ "hisilicon,usb3-otg-bc-syscon");
+ if (IS_ERR(priv->otg_bc)) {
+ dev_err(dev, "no hisilicon,usb3-otg-bc-syscon\n");
+ return PTR_ERR(priv->otg_bc);
+ }
+
+ if (of_property_read_u32(dev->of_node, "hisilicon,eye-diagram-param",
+ &(priv->eye_diagram_param)))
+ priv->eye_diagram_param = HI3660_USB_DEFAULT_PHY_PARAM;
+
+ phy = devm_phy_create(dev, NULL, &hi3660_phy_ops);
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
+
+ phy_set_drvdata(phy, priv);
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id hi3660_phy_of_match[] = {
+ {.compatible = "hisilicon,hi3660-usb-phy",},
+ { },
+};
+MODULE_DEVICE_TABLE(of, hi3660_phy_of_match);
+
+static struct platform_driver hi3660_phy_driver = {
+ .probe = hi3660_phy_probe,
+ .driver = {
+ .name = "hi3660-usb-phy",
+ .of_match_table = hi3660_phy_of_match,
+ }
+};
+module_platform_driver(hi3660_phy_driver);
+
+MODULE_AUTHOR("Yu Chen <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Hilisicon Hi3660 USB3 PHY Driver");
--
2.15.0-rc2
This patch adds notifier for drivers want to be informed of the usb role switch.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Heikki Krogerus <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: John Stultz <[email protected]>
Signed-off-by: Heikki Krogerus <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
--
v0:
The patch is provided by Heikki Krogerus. I modified and test it
on Hikey960 platform.
--
---
drivers/usb/common/roles.c | 20 +++++++++++++++++++-
include/linux/usb/role.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 0f48090c5c30..bfd7a988b64c 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -21,6 +21,7 @@ struct usb_role_switch {
struct device dev;
struct mutex lock; /* device lock*/
enum usb_role role;
+ struct blocking_notifier_head nh;
/* From descriptor */
struct device *usb2_port;
@@ -50,8 +51,10 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
mutex_lock(&sw->lock);
ret = sw->set(sw->dev.parent, role);
- if (!ret)
+ if (!ret) {
sw->role = role;
+ blocking_notifier_call_chain(&sw->nh, role, NULL);
+ }
mutex_unlock(&sw->lock);
@@ -59,6 +62,20 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
}
EXPORT_SYMBOL_GPL(usb_role_switch_set_role);
+int usb_role_switch_register_notifier(struct usb_role_switch *sw,
+ struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&sw->nh, nb);
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_register_notifier);
+
+int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
+ struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&sw->nh, nb);
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_unregister_notifier);
+
/**
* usb_role_switch_get_role - Get the USB role for a switch
* @sw: USB role switch
@@ -297,6 +314,7 @@ usb_role_switch_register(struct device *parent,
return ERR_PTR(-ENOMEM);
mutex_init(&sw->lock);
+ BLOCKING_INIT_NOTIFIER_HEAD(&sw->nh);
sw->allow_userspace_control = desc->allow_userspace_control;
sw->usb2_port = desc->usb2_port;
diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index edc51be4a77c..c97d4be91ada 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -40,6 +40,8 @@ struct usb_role_switch_desc {
bool allow_userspace_control;
};
+
+#if IS_ENABLED(CONFIG_USB_ROLE_SWITCH)
int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role);
enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw);
struct usb_role_switch *usb_role_switch_get(struct device *dev);
@@ -49,5 +51,49 @@ struct usb_role_switch *
usb_role_switch_register(struct device *parent,
const struct usb_role_switch_desc *desc);
void usb_role_switch_unregister(struct usb_role_switch *sw);
+int usb_role_switch_register_notifier(struct usb_role_switch *sw,
+ struct notifier_block *nb);
+int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
+ struct notifier_block *nb);
+#else
+static inline int usb_role_switch_set_role(struct usb_role_switch *sw,
+ enum usb_role role)
+{
+ return 0;
+}
+
+static inline enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
+{
+ return USB_ROLE_NONE;
+}
+
+static inline struct usb_role_switch *usb_role_switch_get(struct device *dev)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline void usb_role_switch_put(struct usb_role_switch *sw) { }
+
+static inline struct usb_role_switch *
+usb_role_switch_register(struct device *parent,
+ const struct usb_role_switch_desc *desc)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline void usb_role_switch_unregister(struct usb_role_switch *sw) { }
+
+static int usb_role_switch_register_notifier(struct usb_role_switch *sw,
+ struct notifier_block *nb)
+{
+ return -ENODEV;
+}
+
+static int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
+ struct notifier_block *nb)
+{
+ return -ENODEV;
+}
+#endif
#endif /* __LINUX_USB_ROLE_H */
--
2.15.0-rc2
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: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
Signed-off-by: Heikki Krogerus <[email protected]>
--
v0:
The patch is provided by Heikki Krogerus. I modified and test it
on Hikey960 platform.
--
---
drivers/usb/dwc3/core.h | 2 ++
drivers/usb/dwc3/drd.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index b3cea95f7720..47e388878e9a 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>
@@ -1057,6 +1058,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 218371f985ca..3155121e6d29 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -463,6 +463,48 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
return edev;
}
+static int dwc3_usb_role_switch_set(struct device *dev, enum usb_role role)
+{
+ u32 mode;
+
+ pr_info("%s:set role %d\n", __func__, role);
+
+ 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_HOST;
+ break;
+ };
+
+ dwc3_set_mode(dev_get_drvdata(dev), 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);
+ role = dwc->current_otg_role;
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
+ return role;
+
+}
+
+static const struct usb_role_switch_desc dwc3_role_switch = {
+ .set = dwc3_usb_role_switch_set,
+ .get = dwc3_usb_role_switch_get,
+};
+
int dwc3_drd_init(struct dwc3 *dwc)
{
int ret, irq;
@@ -511,6 +553,11 @@ int dwc3_drd_init(struct dwc3 *dwc)
dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
}
+ dwc->role_sw = usb_role_switch_register(dwc->dev, &dwc3_role_switch);
+ if (ret) {
+ dwc3_drd_exit(dwc);
+ return PTR_ERR(dwc->role_sw);
+ }
return 0;
}
@@ -546,4 +593,6 @@ void dwc3_drd_exit(struct dwc3 *dwc)
if (!dwc->edev)
free_irq(dwc->otg_irq, dwc);
+
+ usb_role_switch_unregister(dwc->role_sw);
}
--
2.15.0-rc2
This patch adds code for supporting find usb role switch by matching against
the device node described using of_graph.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Heikki Krogerus <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: John Stultz <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
---
drivers/usb/common/roles.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 99116af07f1d..0f48090c5c30 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -12,6 +12,8 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
static struct class *role_class;
@@ -100,6 +102,38 @@ static void *usb_role_switch_match(struct device_connection *con, int ep,
return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
}
+static int __switch_match_by_of_node(struct device *dev, const void *name)
+{
+ if (!dev->parent || !dev->parent->of_node)
+ return 0;
+
+ return of_node_name_eq(dev->parent->of_node, (const char *)name);
+}
+
+static void *of_graph_find_match_by_type(struct device *dev, const char *ep_type)
+{
+ struct device_node *ep;
+ struct device_node *remote_parent;
+ struct device *role_switch;
+
+ for_each_endpoint_of_node(dev_of_node(dev), ep) {
+ if (!ep->type || strcmp(ep->type, ep_type))
+ continue;
+
+ remote_parent = of_graph_get_remote_port_parent(ep);
+ if (!remote_parent || !remote_parent->name)
+ continue;
+
+ role_switch = class_find_device(role_class, NULL,
+ remote_parent->name, __switch_match_by_of_node);
+
+ return role_switch ? to_role_switch(role_switch) :
+ ERR_PTR(-EPROBE_DEFER);
+ }
+
+ return NULL;
+}
+
/**
* usb_role_switch_get - Find USB role switch linked with the caller
* @dev: The caller device
@@ -114,6 +148,12 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev)
sw = device_connection_find_match(dev, "usb-role-switch", NULL,
usb_role_switch_match);
+ if (!IS_ERR_OR_NULL(sw)) {
+ WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+ return sw;
+ }
+
+ sw = of_graph_find_match_by_type(dev, "usb-role-switch");
if (!IS_ERR_OR_NULL(sw))
WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
--
2.15.0-rc2
Currently the "match_existing_only" of usb_gadget_driver in configfs is
set to one which is not flexible.
Dwc3 udc will be removed when usb core switch to host mode. This causes
failure of writing name of dwc3 udc to configfs's UDC attribuite.
To fix this we need to add a way to change the config of
"match_existing_only".
This patch adds a configfs attribuite for controling match_existing_only
which allow user to config "match_existing_only".
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Binghui Wang <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
---
drivers/usb/gadget/configfs.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 025129942894..c82b7bc98dea 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -291,6 +291,36 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item,
return ret;
}
+static ssize_t gadget_driver_match_existing_only_store(struct config_item *item,
+ const char *page, size_t len)
+{
+ struct usb_gadget_driver *gadget_driver =
+ &(to_gadget_info(item)->composite.gadget_driver);
+ bool match_existing_only;
+ int ret;
+
+ ret = kstrtobool(page, &match_existing_only);
+ if (ret)
+ return ret;
+
+ if (match_existing_only)
+ gadget_driver->match_existing_only = 1;
+ else
+ gadget_driver->match_existing_only = 0;
+
+ return len;
+}
+
+static ssize_t gadget_driver_match_existing_only_show(struct config_item *item,
+ char *page)
+{
+ struct usb_gadget_driver *gadget_driver =
+ &(to_gadget_info(item)->composite.gadget_driver);
+ bool match_existing_only = !!gadget_driver->match_existing_only;
+
+ return sprintf(page, "%s\n", match_existing_only ? "true" : "false");
+}
+
CONFIGFS_ATTR(gadget_dev_desc_, bDeviceClass);
CONFIGFS_ATTR(gadget_dev_desc_, bDeviceSubClass);
CONFIGFS_ATTR(gadget_dev_desc_, bDeviceProtocol);
@@ -300,6 +330,7 @@ CONFIGFS_ATTR(gadget_dev_desc_, idProduct);
CONFIGFS_ATTR(gadget_dev_desc_, bcdDevice);
CONFIGFS_ATTR(gadget_dev_desc_, bcdUSB);
CONFIGFS_ATTR(gadget_dev_desc_, UDC);
+CONFIGFS_ATTR(gadget_, driver_match_existing_only);
static struct configfs_attribute *gadget_root_attrs[] = {
&gadget_dev_desc_attr_bDeviceClass,
@@ -311,6 +342,7 @@ static struct configfs_attribute *gadget_root_attrs[] = {
&gadget_dev_desc_attr_bcdDevice,
&gadget_dev_desc_attr_bcdUSB,
&gadget_dev_desc_attr_UDC,
+ &gadget_attr_driver_match_existing_only,
NULL,
};
--
2.15.0-rc2
This driver handles usb hub power on and typeC port event of HiKey960 board:
1)DP&DM switching between usb hub and typeC port base on typeC port
state
2)Control power of usb hub on Hikey960
3)Control vbus of typeC port
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Binghui Wang <[email protected]>
Cc: Heikki Krogerus <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
--
v1:
1) Using gpiod API with the gpios.
2) Removing registering usb role switch.
3) Registering usb role switch notifier.
--
---
drivers/misc/Kconfig | 7 ++
drivers/misc/Makefile | 1 +
drivers/misc/hisi_hikey_usb.c | 198 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 206 insertions(+)
create mode 100644 drivers/misc/hisi_hikey_usb.c
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 3726eacdf65d..8e04fc87b685 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -513,6 +513,13 @@ config MISC_RTSX
tristate
default MISC_RTSX_PCI || MISC_RTSX_USB
+config HISI_HIKEY_USB
+ tristate "USB functionality of HiSilicon Hikey Platform"
+ depends on GPIOLIB
+ default n
+ help
+ If you say yes here you get support for usb functionality of HiSilicon Hikey Platform.
+
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 af22bbc3d00c..387dd302815c 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
obj-$(CONFIG_OCXL) += ocxl/
obj-$(CONFIG_MISC_RTSX) += cardreader/
+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..f320ed878e44
--- /dev/null
+++ b/drivers/misc/hisi_hikey_usb.c
@@ -0,0 +1,198 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * hisi_hikey_usb.c
+ *
+ * Copyright (c) Hisilicon Tech. Co., Ltd. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/gpio/consumer.h>
+#include <linux/usb/role.h>
+#include <linux/notifier.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
+
+struct hisi_hikey_usb {
+ struct gpio_desc *otg_switch;
+ struct gpio_desc *typec_vbus;
+ struct gpio_desc *hub_vbus;
+ int typec_vbus_enable_val;
+
+ struct usb_role_switch *role_sw;
+ struct notifier_block nb;
+};
+
+static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value)
+{
+ pr_debug("%s: set hub power %d\n", __func__, 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)
+{
+ const char *switch_to_str = (switch_to == USB_SWITCH_TO_HUB) ?
+ "hub" : "typec";
+
+ pr_debug("%s: switch to %s\n", __func__, switch_to_str);
+ 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)
+{
+ pr_debug("%s: set typec vbus gpio to %d\n", __func__, value);
+ gpiod_set_value_cansleep(hisi_hikey_usb->typec_vbus, value);
+}
+
+static int hisi_hikey_role_switch(struct notifier_block *nb,
+ unsigned long state, void *data)
+{
+ struct hisi_hikey_usb *hisi_hikey_usb;
+
+ if (!nb)
+ return -EINVAL;
+
+ hisi_hikey_usb = container_of(nb, struct hisi_hikey_usb, nb);
+
+ pr_info("%s:set typec state to %lu\n", __func__, state);
+ switch (state) {
+ case USB_ROLE_NONE:
+ usb_typec_power_ctrl(hisi_hikey_usb,
+ !hisi_hikey_usb->typec_vbus_enable_val);
+ 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:
+ usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
+ usb_typec_power_ctrl(hisi_hikey_usb,
+ hisi_hikey_usb->typec_vbus_enable_val);
+ break;
+ case USB_ROLE_DEVICE:
+ hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_OFF);
+ usb_typec_power_ctrl(hisi_hikey_usb,
+ !hisi_hikey_usb->typec_vbus_enable_val);
+ usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int hisi_hikey_usb_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *root = dev->of_node;
+ struct hisi_hikey_usb *hisi_hikey_usb;
+ int ret;
+
+ hisi_hikey_usb = devm_kzalloc(dev, sizeof(*hisi_hikey_usb), GFP_KERNEL);
+ if (!hisi_hikey_usb)
+ return -ENOMEM;
+
+ hisi_hikey_usb->nb.notifier_call = hisi_hikey_role_switch;
+
+ ret = of_property_read_u32(root, "typc-vbus-enable-val",
+ &hisi_hikey_usb->typec_vbus_enable_val);
+ if (ret) {
+ pr_info("%s: typc_vbus_enable_val can't get\n", __func__);
+ hisi_hikey_usb->typec_vbus_enable_val = 1;
+ }
+
+ hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
+ hisi_hikey_usb->typec_vbus_enable_val ?
+ GPIOD_OUT_LOW : GPIOD_OUT_HIGH);
+ if (IS_ERR_OR_NULL(hisi_hikey_usb->typec_vbus)) {
+ if (!hisi_hikey_usb->typec_vbus)
+ ret = -EINVAL;
+ else
+ ret = PTR_ERR(hisi_hikey_usb->typec_vbus);
+ return ret;
+ }
+
+ hisi_hikey_usb->otg_switch = devm_gpiod_get(dev, "otg-switch", GPIOD_IN);
+ if (IS_ERR(hisi_hikey_usb->otg_switch))
+ return PTR_ERR(hisi_hikey_usb->otg_switch);
+
+ hisi_hikey_usb->hub_vbus = devm_gpiod_get(dev, "hub-vdd33-en",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(hisi_hikey_usb->hub_vbus))
+ return PTR_ERR(hisi_hikey_usb->hub_vbus);
+
+ hisi_hikey_usb->role_sw = usb_role_switch_get(dev);
+ if (IS_ERR_OR_NULL(hisi_hikey_usb->role_sw)) {
+ pr_err("%s: usb_role_switch_get failed\n", __func__);
+ if (!hisi_hikey_usb->role_sw)
+ ret = -ENOENT;
+ else
+ ret = PTR_ERR(hisi_hikey_usb->role_sw);
+ return ret;
+ }
+
+ ret = usb_role_switch_register_notifier(hisi_hikey_usb->role_sw,
+ &hisi_hikey_usb->nb);
+ if (ret) {
+ usb_role_switch_put(hisi_hikey_usb->role_sw);
+ return ret;
+ }
+
+ 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);
+
+ usb_role_switch_unregister_notifier(hisi_hikey_usb->role_sw,
+ &hisi_hikey_usb->nb);
+
+ usb_role_switch_put(hisi_hikey_usb->role_sw);
+
+ return 0;
+}
+
+static const struct of_device_id id_table_hisi_hikey_usb[] = {
+ {.compatible = "hisilicon,gpio_hubv1"},
+ {.compatible = "hisilicon,hikey960_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 = of_match_ptr(id_table_hisi_hikey_usb),
+
+ },
+};
+
+module_platform_driver(hisi_hikey_usb_driver);
+
+MODULE_AUTHOR("Yu Chen <[email protected]>");
+MODULE_DESCRIPTION("Driver Support for USB functionality of Hikey");
+MODULE_LICENSE("GPL v2");
--
2.15.0-rc2
This patch adds support for usb on Hikey960.
Cc: Wei Xu <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: [email protected]
Cc: John Stultz <[email protected]>
Cc: Binghui Wang <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
---
arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 56 ++++++++++++++++++
arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 72 +++++++++++++++++++++++
2 files changed, 128 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
index c98bcbc8dfba..066c558154ea 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
@@ -13,6 +13,7 @@
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/input.h>
#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/usb/pd.h>
/ {
model = "HiKey960";
@@ -196,6 +197,28 @@
method = "smc";
};
};
+
+ hisi_hikey_usb: hisi_hikey_usb {
+ compatible = "hisilicon,hikey960_usb";
+ typec-vbus-gpios = <&gpio25 2 0>;
+ typc-vbus-enable-val = <1>;
+ otg-switch-gpios = <&gpio25 6 0>;
+ hub-vdd33-en-gpios = <&gpio5 6 0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&usbhub5734_pmx_func>;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hikey_usb_ep: endpoint@0 {
+ reg = <0>;
+ device_type = "usb-role-switch";
+ remote-endpoint = <&dwc3_role_switch_notify>;
+ };
+ };
+ };
+
};
/*
@@ -526,6 +549,39 @@
&i2c1 {
status = "okay";
+ rt1711h: rt1711h@4e {
+ compatible = "richtek,rt1711h";
+ reg = <0x4e>;
+ status = "ok";
+ interrupt-parent = <&gpio27>;
+ interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb_cfg_func>;
+
+ usb_con: connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ data-role = "dual";
+ power-role = "dual";
+ try-power-role = "sink";
+ source-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)>;
+ sink-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)
+ PDO_VAR(5000, 5000, 1000)>;
+ op-sink-microwatt = <10000000>;
+ };
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rt1711h_ep: endpoint@0 {
+ reg = <0>;
+ device_type = "usb-role-switch";
+ remote-endpoint = <&dwc3_role_switch>;
+ };
+ };
+ };
+
adv7533: adv7533@39 {
status = "ok";
compatible = "adi,adv7533";
diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index f432b0a88c65..e6583bd7efdb 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -349,6 +349,12 @@
#clock-cells = <1>;
};
+ pmctrl: pmctrl@fff31000 {
+ compatible = "hisilicon,hi3660-pmctrl", "syscon";
+ reg = <0x0 0xfff31000 0x0 0x1000>;
+ #clock-cells = <1>;
+ };
+
pmuctrl: crg_ctrl@fff34000 {
compatible = "hisilicon,hi3660-pmuctrl", "syscon";
reg = <0x0 0xfff34000 0x0 0x1000>;
@@ -1122,5 +1128,71 @@
};
};
};
+
+ usb3_otg_bc: usb3_otg_bc@ff200000 {
+ compatible = "syscon";
+ reg = <0x0 0xff200000 0x0 0x1000>;
+ };
+
+ usb_phy: usb-phy {
+ compatible = "hisilicon,hi3660-usb-phy";
+ #phy-cells = <0>;
+ hisilicon,pericrg-syscon = <&crg_ctrl>;
+ hisilicon,pctrl-syscon = <&pctrl>;
+ hisilicon,usb3-otg-bc-syscon = <&usb3_otg_bc>;
+ hisilicon,eye-diagram-param = <0x22466e4>;
+ };
+
+ usb3: hisi_dwc3 {
+ compatible = "hisilicon,hi3660-dwc3";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
+ <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
+ clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
+
+ assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
+ assigned-clock-rates = <229000000>;
+ resets = <&crg_rst 0x90 8>,
+ <&crg_rst 0x90 7>,
+ <&crg_rst 0x90 6>,
+ <&crg_rst 0x90 5>;
+
+ dwc3: dwc3@ff100000 {
+ compatible = "snps,dwc3";
+ reg = <0x0 0xff100000 0x0 0x100000>;
+ interrupts = <0 159 4>, <0 161 4>;
+ phys = <&usb_phy>;
+ phy-names = "usb3-phy";
+ dr_mode = "otg";
+ maximum-speed = "super-speed";
+ phy_type = "utmi";
+ snps,dis-del-phy-power-chg-quirk;
+ snps,lfps_filter_quirk;
+ snps,dis_u2_susphy_quirk;
+ snps,dis_u3_susphy_quirk;
+ snps,tx_de_emphasis_quirk;
+ snps,tx_de_emphasis = <1>;
+ snps,dis_enblslpm_quirk;
+ snps,gctl-reset-quirk;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dwc3_role_switch: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&rt1711h_ep>;
+ };
+
+ dwc3_role_switch_notify: endpoint@1 {
+ reg = <1>;
+ remote-endpoint = <&hikey_usb_ep>;
+ };
+ };
+ };
+ };
};
};
--
2.15.0-rc2
This driver handles the poweron and shutdown of dwc3 core
on Hisilicon Soc Platform.
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Binghui Wang <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
---
MAINTAINERS | 8 ++
drivers/usb/dwc3/Kconfig | 8 ++
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/dwc3-hisi.c | 335 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 352 insertions(+)
create mode 100644 drivers/usb/dwc3/dwc3-hisi.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6c3fbbb361f8..1b6b3026804c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15465,6 +15465,14 @@ L: [email protected]
S: Maintained
F: drivers/usb/roles/intel-xhci-usb-role-switch.c
+USB IP DRIVER FOR HISILICON KIRIN
+M: Yu Chen <[email protected]>
+M: Binghui Wang <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/usb/dwc3-hisi.txt
+F: drivers/usb/dwc3/dwc3-hisi.c
+
USB ISP116X DRIVER
M: Olav Kongas <[email protected]>
L: [email protected]
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 1a0404fda596..09d105a2b53a 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -123,4 +123,12 @@ config USB_DWC3_QCOM
for peripheral mode support.
Say 'Y' or 'M' if you have one such device.
+config USB_DWC3_HISI
+ tristate "HiSilicon Kirin Platforms"
+ depends on ((ARCH_HISI && ARM64) || COMPILE_TEST) && OF
+ default USB_DWC3
+ help
+ Support USB2/3 functionality in HiSilicon Kirin platforms.
+ Say 'Y' or 'M' if you have one such device.
+
endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 6e3ef6144e5d..15781455e3f0 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -50,3 +50,4 @@ obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o
obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o
obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o
obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
+obj-$(CONFIG_USB_DWC3_HISI) += dwc3-hisi.o
diff --git a/drivers/usb/dwc3/dwc3-hisi.c b/drivers/usb/dwc3/dwc3-hisi.c
new file mode 100644
index 000000000000..6e9787d420a7
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-hisi.c
@@ -0,0 +1,335 @@
+// SPDX-License-Identifier: GPL-2.0+
+/**
+ * dwc3-hisi.c - Support for dwc3 platform devices on HiSilicon platforms
+ *
+ * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ * http://www.huawei.com
+ *
+ * Authors: Yu Chen <[email protected]>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/extcon.h>
+#include <linux/regulator/consumer.h>
+#include <linux/pinctrl/consumer.h>
+
+struct dwc3_hisi {
+ struct device *dev;
+ struct clk **clks;
+ int num_clocks;
+ struct reset_control **rstcs;
+ int num_rstcs;
+};
+
+struct dwc3_hisi *g_dwc3_hisi;
+
+static int dwc3_hisi_init_clks(struct dwc3_hisi *dwc3_hisi,
+ struct device_node *np)
+{
+ struct device *dev = dwc3_hisi->dev;
+ int i;
+
+ dwc3_hisi->num_clocks = of_clk_get_parent_count(np);
+ if (!dwc3_hisi->num_clocks)
+ return -ENOENT;
+
+ dwc3_hisi->clks = devm_kcalloc(dev, dwc3_hisi->num_clocks,
+ sizeof(struct clk *), GFP_KERNEL);
+ if (!dwc3_hisi->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < dwc3_hisi->num_clocks; i++) {
+ struct clk *clk;
+
+ clk = of_clk_get(np, i);
+ if (IS_ERR(clk)) {
+ while (--i >= 0)
+ clk_put(dwc3_hisi->clks[i]);
+
+ return PTR_ERR(clk);
+ }
+
+ dwc3_hisi->clks[i] = clk;
+ }
+
+ return 0;
+}
+
+static int dwc3_hisi_enable_clks(struct dwc3_hisi *dwc3_hisi)
+{
+ int i;
+ int ret;
+
+ for (i = 0; i < dwc3_hisi->num_clocks; i++) {
+ ret = clk_prepare_enable(dwc3_hisi->clks[i]);
+ if (ret < 0) {
+ while (--i >= 0)
+ clk_disable_unprepare(dwc3_hisi->clks[i]);
+
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static void dwc3_hisi_disable_clks(struct dwc3_hisi *dwc3_hisi)
+{
+ int i;
+
+ for (i = 0; i < dwc3_hisi->num_clocks; i++)
+ clk_disable_unprepare(dwc3_hisi->clks[i]);
+}
+
+static int dwc3_hisi_get_rstcs(struct dwc3_hisi *dwc3_hisi,
+ struct device_node *np)
+{
+ struct device *dev = dwc3_hisi->dev;
+ int i;
+
+ dwc3_hisi->num_rstcs = of_count_phandle_with_args(np,
+ "resets", "#reset-cells");
+ if (!dwc3_hisi->num_rstcs)
+ return -ENOENT;
+
+ dwc3_hisi->rstcs = devm_kcalloc(dev, dwc3_hisi->num_rstcs,
+ sizeof(struct reset_control *), GFP_KERNEL);
+ if (!dwc3_hisi->rstcs)
+ return -ENOMEM;
+
+ for (i = 0; i < dwc3_hisi->num_rstcs; i++) {
+ struct reset_control *rstc;
+
+ rstc = of_reset_control_get_shared_by_index(np, i);
+ if (IS_ERR(rstc)) {
+ while (--i >= 0)
+ reset_control_put(dwc3_hisi->rstcs[i]);
+ return PTR_ERR(rstc);
+ }
+
+ dwc3_hisi->rstcs[i] = rstc;
+ }
+
+ return 0;
+}
+
+static int dwc3_hisi_reset_control_assert(struct dwc3_hisi *dwc3_hisi)
+{
+ int i, ret;
+
+ for (i = dwc3_hisi->num_rstcs - 1; i >= 0 ; i--) {
+ ret = reset_control_assert(dwc3_hisi->rstcs[i]);
+ if (ret) {
+ while (--i >= 0)
+ reset_control_deassert(dwc3_hisi->rstcs[i]);
+ return ret;
+ }
+ udelay(10);
+ }
+
+ return 0;
+}
+
+static int dwc3_hisi_reset_control_deassert(struct dwc3_hisi *dwc3_hisi)
+{
+ int i, ret;
+
+ for (i = 0; i < dwc3_hisi->num_rstcs; i++) {
+ ret = reset_control_deassert(dwc3_hisi->rstcs[i]);
+ if (ret) {
+ while (--i >= 0)
+ reset_control_assert(dwc3_hisi->rstcs[i]);
+ return ret;
+ }
+ udelay(10);
+ }
+
+ return 0;
+}
+
+static int dwc3_hisi_probe(struct platform_device *pdev)
+{
+ struct dwc3_hisi *dwc3_hisi;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+
+ int ret;
+ int i;
+
+ dwc3_hisi = devm_kzalloc(dev, sizeof(*dwc3_hisi), GFP_KERNEL);
+ if (!dwc3_hisi)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, dwc3_hisi);
+ dwc3_hisi->dev = dev;
+
+ ret = dwc3_hisi_init_clks(dwc3_hisi, np);
+ if (ret) {
+ dev_err(dev, "could not get clocks\n");
+ return ret;
+ }
+
+ ret = dwc3_hisi_enable_clks(dwc3_hisi);
+ if (ret) {
+ dev_err(dev, "could not enable clocks\n");
+ goto err_put_clks;
+ }
+
+ ret = dwc3_hisi_get_rstcs(dwc3_hisi, np);
+ if (ret) {
+ dev_err(dev, "could not get reset controllers\n");
+ goto err_disable_clks;
+ }
+ ret = dwc3_hisi_reset_control_deassert(dwc3_hisi);
+ if (ret) {
+ dev_err(dev, "reset control deassert failed\n");
+ goto err_put_rstcs;
+ }
+
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ pm_runtime_get_sync(dev);
+
+ ret = of_platform_populate(np, NULL, NULL, dev);
+ if (ret) {
+ dev_err(dev, "failed to add dwc3 core\n");
+ goto err_reset_assert;
+ }
+
+ dev_err(dev, "finish dwc3 hisi probe\n");
+
+ g_dwc3_hisi = dwc3_hisi;
+ return 0;
+
+err_reset_assert:
+ ret = dwc3_hisi_reset_control_assert(dwc3_hisi);
+ if (ret)
+ dev_err(dev, "reset control assert failed\n");
+err_put_rstcs:
+ for (i = 0; i < dwc3_hisi->num_rstcs; i++)
+ reset_control_put(dwc3_hisi->rstcs[i]);
+err_disable_clks:
+ dwc3_hisi_disable_clks(dwc3_hisi);
+err_put_clks:
+ for (i = 0; i < dwc3_hisi->num_clocks; i++)
+ clk_put(dwc3_hisi->clks[i]);
+
+ return ret;
+}
+
+static int dwc3_hisi_remove(struct platform_device *pdev)
+{
+ struct dwc3_hisi *dwc3_hisi = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+ int i, ret;
+
+ of_platform_depopulate(dev);
+
+ ret = dwc3_hisi_reset_control_assert(dwc3_hisi);
+ if (ret) {
+ dev_err(dev, "reset control assert failed\n");
+ return ret;
+ }
+
+ for (i = 0; i < dwc3_hisi->num_clocks; i++) {
+ clk_disable_unprepare(dwc3_hisi->clks[i]);
+ clk_put(dwc3_hisi->clks[i]);
+ }
+
+ pm_runtime_put_sync(dev);
+ pm_runtime_disable(dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int dwc3_hisi_suspend(struct device *dev)
+{
+ struct dwc3_hisi *dwc3_hisi = dev_get_drvdata(dev);
+ int ret;
+
+ dev_info(dev, "%s\n", __func__);
+
+ ret = dwc3_hisi_reset_control_assert(dwc3_hisi);
+ if (ret) {
+ dev_err(dev, "reset control assert failed\n");
+ return ret;
+ }
+
+ dwc3_hisi_disable_clks(dwc3_hisi);
+
+ pinctrl_pm_select_default_state(dev);
+
+ return 0;
+}
+
+static int dwc3_hisi_resume(struct device *dev)
+{
+ struct dwc3_hisi *dwc3_hisi = dev_get_drvdata(dev);
+ int ret;
+
+ dev_info(dev, "%s\n", __func__);
+ pinctrl_pm_select_default_state(dev);
+
+ ret = dwc3_hisi_enable_clks(dwc3_hisi);
+ if (ret) {
+ dev_err(dev, "could not enable clocks\n");
+ return ret;
+ }
+
+ ret = dwc3_hisi_reset_control_deassert(dwc3_hisi);
+ if (ret) {
+ dev_err(dev, "reset control deassert failed\n");
+ return ret;
+ }
+
+ /* Wait for clock stable */
+ msleep(100);
+
+ return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(dwc3_hisi_dev_pm_ops,
+ dwc3_hisi_suspend, dwc3_hisi_resume);
+
+static const struct of_device_id dwc3_hisi_match[] = {
+ { .compatible = "hisilicon,hi3660-dwc3" },
+ { /* sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, dwc3_hisi_match);
+
+static struct platform_driver dwc3_hisi_driver = {
+ .probe = dwc3_hisi_probe,
+ .remove = dwc3_hisi_remove,
+ .driver = {
+ .name = "usb-hisi-dwc3",
+ .of_match_table = dwc3_hisi_match,
+ .pm = &dwc3_hisi_dev_pm_ops,
+ },
+};
+
+module_platform_driver(dwc3_hisi_driver);
+
+MODULE_AUTHOR("Yu Chen <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("DesignWare USB3 HiSilicon Glue Layer");
--
2.15.0-rc2
This patch adds binding documentation to support usb hub and usb
data role switch of Hisilicon HiKey&HiKey960 Board.
Cc: Sergei Shtylyov <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Binghui Wang <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
--
v1:
1) Fix some format errors as suggested by Sergei.
2) Modify gpio description to use gpiod API.
--
---
.../bindings/misc/hisilicon-hikey-usb.txt | 36 ++++++++++++++++++++++
1 file changed, 36 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
new file mode 100644
index 000000000000..f3413d851e18
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
@@ -0,0 +1,36 @@
+Support usb hub and usb data role switch of Hisilicon HiKey&HiKey960 Board.
+
+-----------------------------
+
+Required properties:
+- compatible: "hisilicon,gpio-hubv1","hisilicon,hikey960-usb"
+- typc-vbus-int-gpio: gpio to control the vbus of typeC port
+- typc-vbus-enable-val: gpio value that enable the vbus of typeC port
+- otg-gpio: gpio to switch DP&DM between the hub and typeC port
+- hub-vdd33-en-gpio: gpio to enable the power of hub
+- pinctrl-0: pinctrl config
+
+Example
+-----
+
+ hisi_hikey_usb: hisi_hikey_usb {
+ compatible = "hisilicon,hikey960-usb";
+ typec-vbus-gpios = <&gpio25 2 0>;
+ typc-vbus-enable-val = <1>;
+ otg-switch-gpios = <&gpio25 6 0>;
+ hub-vdd33-en-gpios = <&gpio5 6 0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&usbhub5734_pmx_func>;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hikey_usb_ep: endpoint@0 {
+ reg = <0>;
+ device_type = "usb-role-switch";
+ remote-endpoint = <&dwc3_role_switch_notify>;
+ };
+ };
+
+ };
--
2.15.0-rc2
There are tow quirks for DesignWare USB3 DRD Core of Hisilicon Kirin Soc.
1)SPLIT_BOUNDARY_DISABLE should be set for Host mode
2)A GCTL soft reset should be executed when switch mode
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Binghui Wang <[email protected]>
Signed-off-by: Yu Chen <[email protected]>
---
drivers/usb/dwc3/core.c | 43 +++++++++++++++++++++++++++++++++++++++++++
drivers/usb/dwc3/core.h | 7 +++++++
drivers/usb/dwc3/gadget.c | 2 +-
3 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index becfbb87f791..b539bcc45d3b 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -111,11 +111,25 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
dwc->current_dr_role = mode;
}
+static void dwc3_gctl_core_soft_reset(struct dwc3 *dwc)
+{
+ int reg;
+
+ reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+ reg |= (DWC3_GCTL_CORESOFTRESET);
+ dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+
+ reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+ reg &= ~(DWC3_GCTL_CORESOFTRESET);
+ dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+}
+
static void __dwc3_set_mode(struct work_struct *work)
{
struct dwc3 *dwc = work_to_dwc(work);
unsigned long flags;
int ret;
+ int reg;
if (dwc->dr_mode != USB_DR_MODE_OTG)
return;
@@ -155,6 +169,10 @@ static void __dwc3_set_mode(struct work_struct *work)
dwc3_set_prtcap(dwc, dwc->desired_dr_role);
+ /* Execute a GCTL Core Soft Reset when switch mode */
+ if (dwc->gctl_reset_quirk)
+ dwc3_gctl_core_soft_reset(dwc);
+
spin_unlock_irqrestore(&dwc->lock, flags);
switch (dwc->desired_dr_role) {
@@ -168,6 +186,11 @@ static void __dwc3_set_mode(struct work_struct *work)
phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
phy_calibrate(dwc->usb2_generic_phy);
+ if (dwc->dis_split_quirk) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
+ reg |= DWC3_GUCTL3_SPLITDISABLE;
+ dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
+ }
}
break;
case DWC3_GCTL_PRTCAP_DEVICE:
@@ -1298,6 +1321,11 @@ static void dwc3_get_properties(struct dwc3 *dwc)
dwc->dis_metastability_quirk = device_property_read_bool(dev,
"snps,dis_metastability_quirk");
+ dwc->dis_split_quirk = device_property_read_bool(dev,
+ "snps,dis-split-quirk");
+ dwc->gctl_reset_quirk = device_property_read_bool(dev,
+ "snps,gctl-reset-quirk");
+
dwc->lpm_nyet_threshold = lpm_nyet_threshold;
dwc->tx_de_emphasis = tx_de_emphasis;
@@ -1815,10 +1843,25 @@ static int dwc3_resume(struct device *dev)
return 0;
}
+
+static void dwc3_complete(struct device *dev)
+{
+ struct dwc3 *dwc = dev_get_drvdata(dev);
+ u32 reg;
+
+ if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST &&
+ dwc->dis_split_quirk) {
+ dev_dbg(dwc->dev, "set DWC3_GUCTL3_SPLITDISABLE\n");
+ reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
+ reg |= DWC3_GUCTL3_SPLITDISABLE;
+ dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
+ }
+}
#endif /* CONFIG_PM_SLEEP */
static const struct dev_pm_ops dwc3_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
+ .complete = dwc3_complete,
SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
dwc3_runtime_idle)
};
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 5bfb62533e0f..b3cea95f7720 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -135,6 +135,7 @@
#define DWC3_GEVNTCOUNT(n) (0xc40c + ((n) * 0x10))
#define DWC3_GHWPARAMS8 0xc600
+#define DWC3_GUCTL3 0xc60c
#define DWC3_GFLADJ 0xc630
/* Device Registers */
@@ -359,6 +360,9 @@
/* Global User Control Register 2 */
#define DWC3_GUCTL2_RST_ACTBITLATER BIT(14)
+/* Global User Control Register 3 */
+#define DWC3_GUCTL3_SPLITDISABLE BIT(14)
+
/* Device Configuration Register */
#define DWC3_DCFG_DEVADDR(addr) ((addr) << 3)
#define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f)
@@ -1168,6 +1172,9 @@ struct dwc3 {
unsigned dis_metastability_quirk:1;
+ unsigned dis_split_quirk:1;
+ unsigned gctl_reset_quirk:1;
+
u16 imod_interval;
};
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 679c12e14522..cd54ad3eaf01 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -269,7 +269,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd,
{
const struct usb_endpoint_descriptor *desc = dep->endpoint.desc;
struct dwc3 *dwc = dep->dwc;
- u32 timeout = 1000;
+ u32 timeout = 5000;
u32 saved_config = 0;
u32 reg;
--
2.15.0-rc2
On Mon, Dec 3, 2018 at 5:48 AM Yu Chen <[email protected]> wrote:
>
> There are tow quirks for DesignWare USB3 DRD Core of Hisilicon Kirin Soc.
> 1)SPLIT_BOUNDARY_DISABLE should be set for Host mode
> 2)A GCTL soft reset should be executed when switch mode
> +static void dwc3_gctl_core_soft_reset(struct dwc3 *dwc)
> +{
> + int reg;
u32? int for register value looks confusing a bit.
> + reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> + reg |= (DWC3_GCTL_CORESOFTRESET);
> + dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> +
> + reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> + reg &= ~(DWC3_GCTL_CORESOFTRESET);
> + dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> +}
> + int reg;
Ditto.
--
With Best Regards,
Andy Shevchenko
On Mon, Dec 3, 2018 at 5:48 AM Yu Chen <[email protected]> wrote:
>
> This driver handles the poweron and shutdown of dwc3 core
> on Hisilicon Soc Platform.
> +config USB_DWC3_HISI
> + tristate "HiSilicon Kirin Platforms"
> + depends on ((ARCH_HISI && ARM64) || COMPILE_TEST) && OF
It would be easy to read if
depends on OF
would be on a separate line
> + default USB_DWC3
> + help
> + Support USB2/3 functionality in HiSilicon Kirin platforms.
> + Say 'Y' or 'M' if you have one such device.
> +++ b/drivers/usb/dwc3/dwc3-hisi.c
> @@ -0,0 +1,335 @@
> +// SPDX-License-Identifier: GPL-2.0+
You need to talk to your manager and fix License correspondingly.
Currently there is an ambigous reading.
> +/**
Why /** ?
> + * dwc3-hisi.c - Support for dwc3 platform devices on HiSilicon platforms
Usually put filename in the file is not the best idea.
> + *
> + * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
> + * http://www.huawei.com
> + *
> + * Authors: Yu Chen <[email protected]>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 of
> + * the License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
The idea of SPDX is exactly to remove the above boilerplate text.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/extcon.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/pinctrl/consumer.h>
It would be easy to maintain ordered list in the future.
> + while (--i >= 0)
while (i--)
> + while (--i >= 0)
while (i--)
> + while (--i >= 0)
while (i--)
> + while (--i >= 0)
while (i--)
> + while (--i >= 0)
> +static int dwc3_hisi_probe(struct platform_device *pdev)
> +{
> + struct dwc3_hisi *dwc3_hisi;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> +
Redundant blank line
> + int ret;
> + int i;
> +}
> +#ifdef CONFIG_PM_SLEEP
> +static int dwc3_hisi_suspend(struct device *dev)
Don't know the usual practice here, but you can just use
__maybe_unused and remove this #ifdef.
> +{
> + struct dwc3_hisi *dwc3_hisi = dev_get_drvdata(dev);
> + int ret;
> +
> + dev_info(dev, "%s\n", __func__);
Noise
> +}
> +static int dwc3_hisi_resume(struct device *dev)
__maybe_unused ?
> +{
> + dev_info(dev, "%s\n", __func__);
Noise.
> + /* Wait for clock stable */
> + msleep(100);
Don't you have any hardware notification that clocks are stable?
(Something like PLL locked?)
> +
> + return 0;
> +}
> +#endif /* CONFIG_PM_SLEEP */
> +static const struct of_device_id dwc3_hisi_match[] = {
> + { .compatible = "hisilicon,hi3660-dwc3" },
> + { /* sentinel */ },
No need comma for terminator line.
> +};
--
With Best Regards,
Andy Shevchenko
On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
>
> This driver handles usb phy power on and shutdown for hi3660 Soc of
> Hisilicon.
> +// SPDX-License-Identifier: GPL-2.0+
Ambigous license through the code reading.
> +#define PERI_CRG_CLK_EN4 (0x40)
> +#define PERI_CRG_CLK_DIS4 (0x44)
> +#define GT_CLK_USB3OTG_REF BIT(0)
> +#define GT_ACLK_USB3OTG BIT(1)
> +
> +#define PERI_CRG_RSTEN4 (0x90)
> +#define PERI_CRG_RSTDIS4 (0x94)
> +#define IP_RST_USB3OTGPHY_POR BIT(3)
> +#define IP_RST_USB3OTG BIT(5)
> +
> +#define PERI_CRG_ISODIS (0x148)
> +#define USB_REFCLK_ISO_EN BIT(25)
> +
> +#define PCTRL_PERI_CTRL3 (0x10)
> +#define PCTRL_PERI_CTRL3_MSK_START (16)
> +#define USB_TCXO_EN BIT(1)
> +
> +#define PCTRL_PERI_CTRL24 (0x64)
> +#define SC_CLK_USB3PHY_3MUX1_SEL BIT(25)
> +
> +#define USBOTG3_CTRL0 (0x00)
> +#define SC_USB3PHY_ABB_GT_EN BIT(15)
> +
> +#define USBOTG3_CTRL2 (0x08)
> +#define USBOTG3CTRL2_POWERDOWN_HSP BIT(0)
> +#define USBOTG3CTRL2_POWERDOWN_SSP BIT(1)
> +
> +#define USBOTG3_CTRL3 (0x0C)
> +#define USBOTG3_CTRL3_VBUSVLDEXT BIT(6)
> +#define USBOTG3_CTRL3_VBUSVLDEXTSEL BIT(5)
> +
> +#define USBOTG3_CTRL4 (0x10)
> +
> +#define USBOTG3_CTRL7 (0x1c)
> +#define REF_SSP_EN BIT(16)
> +
> +#define HI3660_USB_DEFAULT_PHY_PARAM (0x1c466e3)
A lot of redundant parens.
> +static const struct of_device_id hi3660_phy_of_match[] = {
> + {.compatible = "hisilicon,hi3660-usb-phy",},
> + { },
No comma needed.
> +};
--
With Best Regards,
Andy Shevchenko
On Mon, Dec 3, 2018 at 5:45 AM Yu Chen <[email protected]> wrote:
>
> This patch adds code for supporting find usb role switch by matching against
> the device node described using of_graph.
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
Keep it ordered.
> +static int __switch_match_by_of_node(struct device *dev, const void *name)
> +{
> + if (!dev->parent || !dev->parent->of_node)
> + return 0;
> +
> + return of_node_name_eq(dev->parent->of_node, (const char *)name);
> +}
I think Heikki has introduced some more generic function for this.
> sw = device_connection_find_match(dev, "usb-role-switch", NULL,
> usb_role_switch_match);
> + sw = of_graph_find_match_by_type(dev, "usb-role-switch");
Why this is here?! device_connection_find_match() should take care for OF case.
--
With Best Regards,
Andy Shevchenko
On 2018/12/3 16:02, Andy Shevchenko wrote:
> On Mon, Dec 3, 2018 at 5:48 AM Yu Chen <[email protected]> wrote:
>>
>> There are tow quirks for DesignWare USB3 DRD Core of Hisilicon Kirin Soc.
>> 1)SPLIT_BOUNDARY_DISABLE should be set for Host mode
>> 2)A GCTL soft reset should be executed when switch mode
>
>> +static void dwc3_gctl_core_soft_reset(struct dwc3 *dwc)
>> +{
>
>> + int reg;
>
> u32? int for register value looks confusing a bit.
Yes, it should be u32.
>
>> + reg = dwc3_readl(dwc->regs, DWC3_GCTL);
>> + reg |= (DWC3_GCTL_CORESOFTRESET);
>> + dwc3_writel(dwc->regs, DWC3_GCTL, reg);
>> +
>> + reg = dwc3_readl(dwc->regs, DWC3_GCTL);
>> + reg &= ~(DWC3_GCTL_CORESOFTRESET);
>> + dwc3_writel(dwc->regs, DWC3_GCTL, reg);
>> +}
>
>> + int reg;
>
> Ditto.
>
Hi,
On 2018/12/3 16:12, Andy Shevchenko wrote:
> On Mon, Dec 3, 2018 at 5:48 AM Yu Chen <[email protected]> wrote:
>>
>> This driver handles the poweron and shutdown of dwc3 core
>> on Hisilicon Soc Platform.
>
>> +config USB_DWC3_HISI
>> + tristate "HiSilicon Kirin Platforms"
>> + depends on ((ARCH_HISI && ARM64) || COMPILE_TEST) && OF
>
> It would be easy to read if
> depends on OF
> would be on a separate line
>
>> + default USB_DWC3
>> + help
>> + Support USB2/3 functionality in HiSilicon Kirin platforms.
>> + Say 'Y' or 'M' if you have one such device.
>
>> +++ b/drivers/usb/dwc3/dwc3-hisi.c
>> @@ -0,0 +1,335 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>
> You need to talk to your manager and fix License correspondingly.
> Currently there is an ambigous reading.
>
>> +/**
>
> Why /** ?
>
>> + * dwc3-hisi.c - Support for dwc3 platform devices on HiSilicon platforms
>
> Usually put filename in the file is not the best idea.
>
>> + *
>> + * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
>> + * http://www.huawei.com
>> + *
>> + * Authors: Yu Chen <[email protected]>
>
>> + *
>> + * This program is free software: you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 of
>> + * the License as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>
> The idea of SPDX is exactly to remove the above boilerplate text.
>
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +#include <linux/extcon.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/pinctrl/consumer.h>
>
> It would be easy to maintain ordered list in the future.
>
>> + while (--i >= 0)
>
> while (i--)
>
>> + while (--i >= 0)
>
> while (i--)
>
>> + while (--i >= 0)
>
> while (i--)
>
>> + while (--i >= 0)
>
> while (i--)
>
>> + while (--i >= 0)
>
>> +static int dwc3_hisi_probe(struct platform_device *pdev)
>> +{
>> + struct dwc3_hisi *dwc3_hisi;
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>
>> +
>
> Redundant blank line
>
>> + int ret;
>> + int i;
>
>> +}
>
>> +#ifdef CONFIG_PM_SLEEP
>> +static int dwc3_hisi_suspend(struct device *dev)
>
> Don't know the usual practice here, but you can just use
> __maybe_unused and remove this #ifdef.
>
>> +{
>> + struct dwc3_hisi *dwc3_hisi = dev_get_drvdata(dev);
>> + int ret;
>> +
>
>> + dev_info(dev, "%s\n", __func__);
>
> Noise
>
>> +}
>
>> +static int dwc3_hisi_resume(struct device *dev)
>
> __maybe_unused ?
>
>> +{
>
>> + dev_info(dev, "%s\n", __func__);
>
> Noise.
>
>> + /* Wait for clock stable */
>> + msleep(100);
>
> Don't you have any hardware notification that clocks are stable?
> (Something like PLL locked?)
>
>> +
>> + return 0;
>> +}
>> +#endif /* CONFIG_PM_SLEEP */
>
>> +static const struct of_device_id dwc3_hisi_match[] = {
>> + { .compatible = "hisilicon,hi3660-dwc3" },
>> + { /* sentinel */ },
>
> No need comma for terminator line.
>
>> +};
>
Thanks a lot for your advice! I will check and fix the patch.
Chen Yu
Hello!
On 03.12.2018 6:45, Yu Chen wrote:
> This patch adds binding descriptions to support the dwc3 controller
> on HiSilicon SoCs and boards like the HiKey960.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: John Stultz <[email protected]>
> Signed-off-by: Yu Chen <[email protected]>
> ---
> .../devicetree/bindings/usb/dwc3-hisi.txt | 67 ++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3-hisi.txt b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
> new file mode 100644
> index 000000000000..d32d2299a0a1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
> @@ -0,0 +1,67 @@
> +HiSilicon DWC3 USB SoC controller
> +
> +This file documents the parameters for the dwc3-hisi driver.
> +
> +Required properties:
> +- compatible: should be "hisilicon,hi3660-dwc3"
> +- clocks: A list of phandle + clock-specifier pairs for the
> + clocks listed in clock-names
> +- clock-names: Specify clock names
> +- resets: list of phandle and reset specifier pairs.
> +
> +Sub-nodes:
> +The dwc3 core should be added as subnode to HiSilicon DWC3 as shown in the
> +example below. The DT binding details of dwc3 can be found in:
> +Documentation/devicetree/bindings/usb/dwc3.txt
> +
> +Example:
> + usb3: hisi_dwc3 {
> + compatible = "hisilicon,hi3660-dwc3";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> + assigned-clock-rates = <229000000>;
> + resets = <&crg_rst 0x90 8>,
> + <&crg_rst 0x90 7>,
> + <&crg_rst 0x90 6>,
> + <&crg_rst 0x90 5>;
> +
> + dwc3: dwc3@ff100000 {
According to the DT spec, the node names should be generic, not chip
specific, i.e. usb@ff100000 in this case.
[...]
MBR, Sergei
Hi,
On 2018/12/3 16:35, Sergei Shtylyov wrote:
> Hello!
>
> On 03.12.2018 6:45, Yu Chen wrote:
>
>> This patch adds binding descriptions to support the dwc3 controller
>> on HiSilicon SoCs and boards like the HiKey960.
>>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: John Stultz <[email protected]>
>> Signed-off-by: Yu Chen <[email protected]>
>> ---
>> .../devicetree/bindings/usb/dwc3-hisi.txt | 67 ++++++++++++++++++++++
>> 1 file changed, 67 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3-hisi.txt b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>> new file mode 100644
>> index 000000000000..d32d2299a0a1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>> @@ -0,0 +1,67 @@
>> +HiSilicon DWC3 USB SoC controller
>> +
>> +This file documents the parameters for the dwc3-hisi driver.
>> +
>> +Required properties:
>> +- compatible: should be "hisilicon,hi3660-dwc3"
>> +- clocks: A list of phandle + clock-specifier pairs for the
>> + clocks listed in clock-names
>> +- clock-names: Specify clock names
>> +- resets: list of phandle and reset specifier pairs.
>> +
>> +Sub-nodes:
>> +The dwc3 core should be added as subnode to HiSilicon DWC3 as shown in the
>> +example below. The DT binding details of dwc3 can be found in:
>> +Documentation/devicetree/bindings/usb/dwc3.txt
>> +
>> +Example:
>> + usb3: hisi_dwc3 {
>> + compatible = "hisilicon,hi3660-dwc3";
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>> +
>> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
>> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
>> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>> + assigned-clock-rates = <229000000>;
>> + resets = <&crg_rst 0x90 8>,
>> + <&crg_rst 0x90 7>,
>> + <&crg_rst 0x90 6>,
>> + <&crg_rst 0x90 5>;
>> +
>> + dwc3: dwc3@ff100000 {
>
> According to the DT spec, the node names should be generic, not chip specific, i.e. usb@ff100000 in this case.
>
Do you mean it should be usb@ff100000: dwc3@ff100000 ?
Thanks!
> [...]
>
> MBR, Sergei
>
> .
>
On 03.12.2018 11:51, Chen Yu wrote:
>>> This patch adds binding descriptions to support the dwc3 controller
>>> on HiSilicon SoCs and boards like the HiKey960.
>>>
>>> Cc: Greg Kroah-Hartman <[email protected]>
>>> Cc: Rob Herring <[email protected]>
>>> Cc: Mark Rutland <[email protected]>
>>> Cc: John Stultz <[email protected]>
>>> Signed-off-by: Yu Chen <[email protected]>
>>> ---
>>> .../devicetree/bindings/usb/dwc3-hisi.txt | 67 ++++++++++++++++++++++
>>> 1 file changed, 67 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3-hisi.txt b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>> new file mode 100644
>>> index 000000000000..d32d2299a0a1
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>> @@ -0,0 +1,67 @@
>>> +HiSilicon DWC3 USB SoC controller
>>> +
>>> +This file documents the parameters for the dwc3-hisi driver.
>>> +
>>> +Required properties:
>>> +- compatible: should be "hisilicon,hi3660-dwc3"
>>> +- clocks: A list of phandle + clock-specifier pairs for the
>>> + clocks listed in clock-names
>>> +- clock-names: Specify clock names
>>> +- resets: list of phandle and reset specifier pairs.
>>> +
>>> +Sub-nodes:
>>> +The dwc3 core should be added as subnode to HiSilicon DWC3 as shown in the
>>> +example below. The DT binding details of dwc3 can be found in:
>>> +Documentation/devicetree/bindings/usb/dwc3.txt
>>> +
>>> +Example:
>>> + usb3: hisi_dwc3 {
>>> + compatible = "hisilicon,hi3660-dwc3";
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + ranges;
>>> +
>>> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
>>> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>>> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
>>> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>>> + assigned-clock-rates = <229000000>;
>>> + resets = <&crg_rst 0x90 8>,
>>> + <&crg_rst 0x90 7>,
>>> + <&crg_rst 0x90 6>,
>>> + <&crg_rst 0x90 5>;
>>> +
>>> + dwc3: dwc3@ff100000 {
>>
>> According to the DT spec, the node names should be generic, not chip specific, i.e. usb@ff100000 in this case.
>>
>
> Do you mean it should be usb@ff100000: dwc3@ff100000 ?
dwc3: usb@ff100000
"dwc3:" is a label, not name.
> Thanks!
>
>> [...]
MBR, Sergei
On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
>
> 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.
> + pr_info("%s:set role %d\n", __func__, role);
Noise!
> + return role;
> +
Redundant blank line.
> +}
--
With Best Regards,
Andy Shevchenko
On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
>
> This driver handles usb hub power on and typeC port event of HiKey960 board:
> 1)DP&DM switching between usb hub and typeC port base on typeC port
> state
> 2)Control power of usb hub on Hikey960
> 3)Control vbus of typeC port
> +config HISI_HIKEY_USB
> + tristate "USB functionality of HiSilicon Hikey Platform"
> + depends on GPIOLIB
> + default n
No, Linus is not happy about this.
Default n _is_ a default.
> + help
> + If you say yes here you get support for usb functionality of HiSilicon Hikey Platform.
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * hisi_hikey_usb.c
> + *
> + * Copyright (c) Hisilicon Tech. Co., Ltd. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
Same comments to the above lines.
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/usb/role.h>
> +#include <linux/notifier.h>
Order?
> + pr_debug("%s: set hub power %d\n", __func__, value);
Noise.
> + pr_debug("%s: switch to %s\n", __func__, switch_to_str);
Noise.
> + pr_debug("%s: set typec vbus gpio to %d\n", __func__, value);
Noise.
> + if (!nb)
> + return -EINVAL;
On which conditions this happen?
> + pr_info("%s:set typec state to %lu\n", __func__, state);
Noise!
Guys, you perhaps can read about tracepoints and function tracer
facilities in the kernel.
> +static int hisi_hikey_usb_probe(struct platform_device *pdev)
> +{
> + pr_info("%s: typc_vbus_enable_val can't get\n", __func__);
Noise!
> + if (IS_ERR_OR_NULL(hisi_hikey_usb->typec_vbus)) {
So, this check is redundant. You are repeating it below.
> + if (!hisi_hikey_usb->typec_vbus)
> + ret = -EINVAL;
> + else
> + ret = PTR_ERR(hisi_hikey_usb->typec_vbus);
> + return ret;
> + }
> + if (IS_ERR_OR_NULL(hisi_hikey_usb->role_sw)) {
> + pr_err("%s: usb_role_switch_get failed\n", __func__);
Noise and same comment to the conditional check as above
> + if (!hisi_hikey_usb->role_sw)
> + ret = -ENOENT;
> + else
> + ret = PTR_ERR(hisi_hikey_usb->role_sw);
> + return ret;
> + }
> + .of_match_table = of_match_ptr(id_table_hisi_hikey_usb),
Does it compiles for non-OF case? Why macro is in use?
--
With Best Regards,
Andy Shevchenko
On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
>
> Currently the "match_existing_only" of usb_gadget_driver in configfs is
> set to one which is not flexible.
> Dwc3 udc will be removed when usb core switch to host mode. This causes
> failure of writing name of dwc3 udc to configfs's UDC attribuite.
> To fix this we need to add a way to change the config of
> "match_existing_only".
> This patch adds a configfs attribuite for controling match_existing_only
> which allow user to config "match_existing_only".
> +static ssize_t gadget_driver_match_existing_only_store(struct config_item *item,
> + const char *page, size_t len)
> +{
> + struct usb_gadget_driver *gadget_driver =
> + &(to_gadget_info(item)->composite.gadget_driver);
It would be easier for reader to see two lines with two variables
instead of this.
> + bool match_existing_only;
> + int ret;
> +
> + ret = kstrtobool(page, &match_existing_only);
> + if (ret)
> + return ret;
> +
> + if (match_existing_only)
> + gadget_driver->match_existing_only = 1;
> + else
> + gadget_driver->match_existing_only = 0;
gadget_driver->match_existing_only = match_existing_only;
But the question rather why is it not direct parameter to kstrtobool?
> +
> + return len;
> +}
> + struct usb_gadget_driver *gadget_driver =
> + &(to_gadget_info(item)->composite.gadget_driver);
Make it neat.
--
With Best Regards,
Andy Shevchenko
Hi,
On 2018/12/3 16:59, Sergei Shtylyov wrote:
> On 03.12.2018 11:51, Chen Yu wrote:
>
>>>> This patch adds binding descriptions to support the dwc3 controller
>>>> on HiSilicon SoCs and boards like the HiKey960.
>>>>
>>>> Cc: Greg Kroah-Hartman <[email protected]>
>>>> Cc: Rob Herring <[email protected]>
>>>> Cc: Mark Rutland <[email protected]>
>>>> Cc: John Stultz <[email protected]>
>>>> Signed-off-by: Yu Chen <[email protected]>
>>>> ---
>>>> .../devicetree/bindings/usb/dwc3-hisi.txt | 67 ++++++++++++++++++++++
>>>> 1 file changed, 67 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3-hisi.txt b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>>> new file mode 100644
>>>> index 000000000000..d32d2299a0a1
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>>> @@ -0,0 +1,67 @@
>>>> +HiSilicon DWC3 USB SoC controller
>>>> +
>>>> +This file documents the parameters for the dwc3-hisi driver.
>>>> +
>>>> +Required properties:
>>>> +- compatible: should be "hisilicon,hi3660-dwc3"
>>>> +- clocks: A list of phandle + clock-specifier pairs for the
>>>> + clocks listed in clock-names
>>>> +- clock-names: Specify clock names
>>>> +- resets: list of phandle and reset specifier pairs.
>>>> +
>>>> +Sub-nodes:
>>>> +The dwc3 core should be added as subnode to HiSilicon DWC3 as shown in the
>>>> +example below. The DT binding details of dwc3 can be found in:
>>>> +Documentation/devicetree/bindings/usb/dwc3.txt
>>>> +
>>>> +Example:
>>>> + usb3: hisi_dwc3 {
>>>> + compatible = "hisilicon,hi3660-dwc3";
>>>> + #address-cells = <2>;
>>>> + #size-cells = <2>;
>>>> + ranges;
>>>> +
>>>> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
>>>> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>>>> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
>>>> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>>>> + assigned-clock-rates = <229000000>;
>>>> + resets = <&crg_rst 0x90 8>,
>>>> + <&crg_rst 0x90 7>,
>>>> + <&crg_rst 0x90 6>,
>>>> + <&crg_rst 0x90 5>;
>>>> +
>>>> + dwc3: dwc3@ff100000 {
>>>
>>> According to the DT spec, the node names should be generic, not chip specific, i.e. usb@ff100000 in this case.
>>>
>>
>> Do you mean it should be usb@ff100000: dwc3@ff100000 ?
>
> dwc3: usb@ff100000
>
> "dwc3:" is a label, not name.
I use the node name "dwc3@ff100000" according to Documentation/devicetree/bindings/usb/dwc3.txt
and documentations of vendor drivers, i.e. qcom,dwc3.txt, rockchip,dwc3.txt.
In these documentations, the dwc3 sub-node name uses "dwc3@xxxxxxxx".
I think it is better to be same as the other vendor's dwc3 drivers.
>
>> Thanks!
>>
>>> [...]
>
> MBR, Sergei
>
> .
>
On 2018/12/3 17:15, Andy Shevchenko wrote:
> On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
>>
>> 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.
>
>> + pr_info("%s:set role %d\n", __func__, role);
>
> Noise!
>
>> + return role;
>> +
>
> Redundant blank line.
>
>> +}
>
OK. Thanks!
Chen Yu
On 03.12.2018 04:45, Yu Chen wrote:
> Currently the "match_existing_only" of usb_gadget_driver in configfs is
> set to one which is not flexible.
> Dwc3 udc will be removed when usb core switch to host mode. This causes
> failure of writing name of dwc3 udc to configfs's UDC attribuite.
> To fix this we need to add a way to change the config of
> "match_existing_only".
> This patch adds a configfs attribuite for controling match_existing_only
> which allow user to config "match_existing_only".
>
To be honest I strongly disagree with that patch.
This attribute was intended for build-in gadgets to allow user to decide
whether probe should fail or gadget should wait for UDC (used when
gadget is built-in). For ConfigFS we expect the UDC to always exist
prior to binding a gadget to it. If UDC goes away from what ever reason
gadget should be unbound.
So what this patch does in my opinion is abusing the attribute and
hacking the kernel instead of creating a simple udev rule that whenever
dwc3 appears and it gadget should be enabled write its name to UDC
attribute.
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
Hi,
On Mon, Dec 03, 2018 at 10:17:20AM +0200, Andy Shevchenko wrote:
> On Mon, Dec 3, 2018 at 5:45 AM Yu Chen <[email protected]> wrote:
> >
> > This patch adds code for supporting find usb role switch by matching against
> > the device node described using of_graph.
>
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > #include <linux/slab.h>
> > +#include <linux/of.h>
> > +#include <linux/of_graph.h>
>
> Keep it ordered.
>
> > +static int __switch_match_by_of_node(struct device *dev, const void *name)
> > +{
> > + if (!dev->parent || !dev->parent->of_node)
> > + return 0;
> > +
> > + return of_node_name_eq(dev->parent->of_node, (const char *)name);
> > +}
>
> I think Heikki has introduced some more generic function for this.
Yes, for this I'm proposing a new function fwnode_get_name() that
should allow us to support ACPI as well as OF platforms:
https://lkml.org/lkml/2018/11/28/784
> > sw = device_connection_find_match(dev, "usb-role-switch", NULL,
> > usb_role_switch_match);
>
> > + sw = of_graph_find_match_by_type(dev, "usb-role-switch");
>
> Why this is here?! device_connection_find_match() should take care for OF case.
Exactly. device_connection_find_match() needs to parse the graph (OF
and ACPI), and I already made proposal (RFC) for that:
https://lkml.org/lkml/2018/10/24/619
I have prepared a more finalized version of those patches. I've been
waiting for that fwnode_get_name() to be accepted before sending it
out since we are going to need that function, but I'll send a new RFC
if nothing happens soon with that fwnode_get_name() series.
Br,
--
heikki
On Mon, Dec 03, 2018 at 11:45:11AM +0800, Yu Chen wrote:
> This patch adds notifier for drivers want to be informed of the usb role switch.
I think in this case it would be good to explain a little for what we
need the notifier for.
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Heikki Krogerus <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> Cc: John Stultz <[email protected]>
> Signed-off-by: Heikki Krogerus <[email protected]>
> Signed-off-by: Yu Chen <[email protected]>
>
> --
> v0:
> The patch is provided by Heikki Krogerus. I modified and test it
> on Hikey960 platform.
I don't think I actually made a "patch" for this. The "diff" I made
showing the idea does not count, so you should not sign off the patch
with my address.
In this case I believe something like "Suggested-by" should be used
instead of SoB tag:
Suggested-by: Heikki Krogerus <[email protected]>
Side note: If this was from me, then there should be "from" line with
my address as well.
> --
> ---
> drivers/usb/common/roles.c | 20 +++++++++++++++++++-
> include/linux/usb/role.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
> index 0f48090c5c30..bfd7a988b64c 100644
> --- a/drivers/usb/common/roles.c
> +++ b/drivers/usb/common/roles.c
> @@ -21,6 +21,7 @@ struct usb_role_switch {
> struct device dev;
> struct mutex lock; /* device lock*/
> enum usb_role role;
> + struct blocking_notifier_head nh;
>
> /* From descriptor */
> struct device *usb2_port;
> @@ -50,8 +51,10 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
> mutex_lock(&sw->lock);
>
> ret = sw->set(sw->dev.parent, role);
> - if (!ret)
> + if (!ret) {
> sw->role = role;
> + blocking_notifier_call_chain(&sw->nh, role, NULL);
> + }
>
> mutex_unlock(&sw->lock);
>
> @@ -59,6 +62,20 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
> }
> EXPORT_SYMBOL_GPL(usb_role_switch_set_role);
>
> +int usb_role_switch_register_notifier(struct usb_role_switch *sw,
> + struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(&sw->nh, nb);
> +}
> +EXPORT_SYMBOL_GPL(usb_role_switch_register_notifier);
> +
> +int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
> + struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_unregister(&sw->nh, nb);
> +}
> +EXPORT_SYMBOL_GPL(usb_role_switch_unregister_notifier);
> +
> /**
> * usb_role_switch_get_role - Get the USB role for a switch
> * @sw: USB role switch
> @@ -297,6 +314,7 @@ usb_role_switch_register(struct device *parent,
> return ERR_PTR(-ENOMEM);
>
> mutex_init(&sw->lock);
> + BLOCKING_INIT_NOTIFIER_HEAD(&sw->nh);
>
> sw->allow_userspace_control = desc->allow_userspace_control;
> sw->usb2_port = desc->usb2_port;
> diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
> index edc51be4a77c..c97d4be91ada 100644
> --- a/include/linux/usb/role.h
> +++ b/include/linux/usb/role.h
> @@ -40,6 +40,8 @@ struct usb_role_switch_desc {
> bool allow_userspace_control;
> };
>
> +
> +#if IS_ENABLED(CONFIG_USB_ROLE_SWITCH)
> int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role);
> enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw);
> struct usb_role_switch *usb_role_switch_get(struct device *dev);
> @@ -49,5 +51,49 @@ struct usb_role_switch *
> usb_role_switch_register(struct device *parent,
> const struct usb_role_switch_desc *desc);
> void usb_role_switch_unregister(struct usb_role_switch *sw);
> +int usb_role_switch_register_notifier(struct usb_role_switch *sw,
> + struct notifier_block *nb);
> +int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
> + struct notifier_block *nb);
> +#else
> +static inline int usb_role_switch_set_role(struct usb_role_switch *sw,
> + enum usb_role role)
> +{
> + return 0;
> +}
> +
> +static inline enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
> +{
> + return USB_ROLE_NONE;
> +}
> +
> +static inline struct usb_role_switch *usb_role_switch_get(struct device *dev)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> +static inline void usb_role_switch_put(struct usb_role_switch *sw) { }
> +
> +static inline struct usb_role_switch *
> +usb_role_switch_register(struct device *parent,
> + const struct usb_role_switch_desc *desc)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> +static inline void usb_role_switch_unregister(struct usb_role_switch *sw) { }
> +
> +static int usb_role_switch_register_notifier(struct usb_role_switch *sw,
> + struct notifier_block *nb)
> +{
> + return -ENODEV;
> +}
> +
> +static int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
> + struct notifier_block *nb)
> +{
> + return -ENODEV;
> +}
> +#endif
>
> #endif /* __LINUX_USB_ROLE_H */
> --
> 2.15.0-rc2
thanks,
--
heikki
On Mon, Dec 03, 2018 at 11:45:12AM +0800, Yu Chen wrote:
> 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: Felipe Balbi <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Yu Chen <[email protected]>
> Signed-off-by: Heikki Krogerus <[email protected]>
>
> --
> v0:
> The patch is provided by Heikki Krogerus. I modified and test it
> on Hikey960 platform.
Either make me as the original author by adding a "from" line with my
address, or just use that "Suggested-by" tag with this patch as well.
thanks,
--
heikki
On 2018/12/3 20:38, Heikki Krogerus wrote:
> On Mon, Dec 03, 2018 at 11:45:11AM +0800, Yu Chen wrote:
>> This patch adds notifier for drivers want to be informed of the usb role switch.
>
> I think in this case it would be good to explain a little for what we
> need the notifier for.
>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: Heikki Krogerus <[email protected]>
>> Cc: Hans de Goede <[email protected]>
>> Cc: Andy Shevchenko <[email protected]>
>> Cc: John Stultz <[email protected]>
>> Signed-off-by: Heikki Krogerus <[email protected]>
>> Signed-off-by: Yu Chen <[email protected]>
>>
>> --
>> v0:
>> The patch is provided by Heikki Krogerus. I modified and test it
>> on Hikey960 platform.
>
> I don't think I actually made a "patch" for this. The "diff" I made
> showing the idea does not count, so you should not sign off the patch
> with my address.
>
> In this case I believe something like "Suggested-by" should be used
> instead of SoB tag:
>
> Suggested-by: Heikki Krogerus <[email protected]>
>
> Side note: If this was from me, then there should be "from" line with
> my address as well.
>
OK. Thanks!
>> --
>> ---
>> drivers/usb/common/roles.c | 20 +++++++++++++++++++-
>> include/linux/usb/role.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
>> index 0f48090c5c30..bfd7a988b64c 100644
>> --- a/drivers/usb/common/roles.c
>> +++ b/drivers/usb/common/roles.c
>> @@ -21,6 +21,7 @@ struct usb_role_switch {
>> struct device dev;
>> struct mutex lock; /* device lock*/
>> enum usb_role role;
>> + struct blocking_notifier_head nh;
>>
>> /* From descriptor */
>> struct device *usb2_port;
>> @@ -50,8 +51,10 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
>> mutex_lock(&sw->lock);
>>
>> ret = sw->set(sw->dev.parent, role);
>> - if (!ret)
>> + if (!ret) {
>> sw->role = role;
>> + blocking_notifier_call_chain(&sw->nh, role, NULL);
>> + }
>>
>> mutex_unlock(&sw->lock);
>>
>> @@ -59,6 +62,20 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
>> }
>> EXPORT_SYMBOL_GPL(usb_role_switch_set_role);
>>
>> +int usb_role_switch_register_notifier(struct usb_role_switch *sw,
>> + struct notifier_block *nb)
>> +{
>> + return blocking_notifier_chain_register(&sw->nh, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(usb_role_switch_register_notifier);
>> +
>> +int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
>> + struct notifier_block *nb)
>> +{
>> + return blocking_notifier_chain_unregister(&sw->nh, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(usb_role_switch_unregister_notifier);
>> +
>> /**
>> * usb_role_switch_get_role - Get the USB role for a switch
>> * @sw: USB role switch
>> @@ -297,6 +314,7 @@ usb_role_switch_register(struct device *parent,
>> return ERR_PTR(-ENOMEM);
>>
>> mutex_init(&sw->lock);
>> + BLOCKING_INIT_NOTIFIER_HEAD(&sw->nh);
>>
>> sw->allow_userspace_control = desc->allow_userspace_control;
>> sw->usb2_port = desc->usb2_port;
>> diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
>> index edc51be4a77c..c97d4be91ada 100644
>> --- a/include/linux/usb/role.h
>> +++ b/include/linux/usb/role.h
>> @@ -40,6 +40,8 @@ struct usb_role_switch_desc {
>> bool allow_userspace_control;
>> };
>>
>> +
>> +#if IS_ENABLED(CONFIG_USB_ROLE_SWITCH)
>> int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role);
>> enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw);
>> struct usb_role_switch *usb_role_switch_get(struct device *dev);
>> @@ -49,5 +51,49 @@ struct usb_role_switch *
>> usb_role_switch_register(struct device *parent,
>> const struct usb_role_switch_desc *desc);
>> void usb_role_switch_unregister(struct usb_role_switch *sw);
>> +int usb_role_switch_register_notifier(struct usb_role_switch *sw,
>> + struct notifier_block *nb);
>> +int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
>> + struct notifier_block *nb);
>> +#else
>> +static inline int usb_role_switch_set_role(struct usb_role_switch *sw,
>> + enum usb_role role)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
>> +{
>> + return USB_ROLE_NONE;
>> +}
>> +
>> +static inline struct usb_role_switch *usb_role_switch_get(struct device *dev)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static inline void usb_role_switch_put(struct usb_role_switch *sw) { }
>> +
>> +static inline struct usb_role_switch *
>> +usb_role_switch_register(struct device *parent,
>> + const struct usb_role_switch_desc *desc)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static inline void usb_role_switch_unregister(struct usb_role_switch *sw) { }
>> +
>> +static int usb_role_switch_register_notifier(struct usb_role_switch *sw,
>> + struct notifier_block *nb)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> +static int usb_role_switch_unregister_notifier(struct usb_role_switch *sw,
>> + struct notifier_block *nb)
>> +{
>> + return -ENODEV;
>> +}
>> +#endif
>>
>> #endif /* __LINUX_USB_ROLE_H */
>> --
>> 2.15.0-rc2
>
> thanks,
>
On 2018/12/3 17:25, Andy Shevchenko wrote:
> On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
>>
>> Currently the "match_existing_only" of usb_gadget_driver in configfs is
>> set to one which is not flexible.
>> Dwc3 udc will be removed when usb core switch to host mode. This causes
>> failure of writing name of dwc3 udc to configfs's UDC attribuite.
>> To fix this we need to add a way to change the config of
>> "match_existing_only".
>> This patch adds a configfs attribuite for controling match_existing_only
>> which allow user to config "match_existing_only".
>
>> +static ssize_t gadget_driver_match_existing_only_store(struct config_item *item,
>> + const char *page, size_t len)
>> +{
>
>> + struct usb_gadget_driver *gadget_driver =
>> + &(to_gadget_info(item)->composite.gadget_driver);
>
> It would be easier for reader to see two lines with two variables
> instead of this.
OK.
>
>> + bool match_existing_only;
>> + int ret;
>> +
>> + ret = kstrtobool(page, &match_existing_only);
>> + if (ret)
>> + return ret;
>> +
>
>> + if (match_existing_only)
>> + gadget_driver->match_existing_only = 1;
>> + else
>> + gadget_driver->match_existing_only = 0;
>
> gadget_driver->match_existing_only = match_existing_only;
> But the question rather why is it not direct parameter to kstrtobool?
>
I wrote the code in this way to avoid type conversion.
>> +
>> + return len;
>> +}
>
>> + struct usb_gadget_driver *gadget_driver =
>> + &(to_gadget_info(item)->composite.gadget_driver);
>
> Make it neat.
>
OK.
Hi,
On 2018/12/3 17:23, Andy Shevchenko wrote:
> On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
>>
>> This driver handles usb hub power on and typeC port event of HiKey960 board:
>> 1)DP&DM switching between usb hub and typeC port base on typeC port
>> state
>> 2)Control power of usb hub on Hikey960
>> 3)Control vbus of typeC port
>
>> +config HISI_HIKEY_USB
>> + tristate "USB functionality of HiSilicon Hikey Platform"
>> + depends on GPIOLIB
>
>> + default n
>
> No, Linus is not happy about this.
> Default n _is_ a default.
OK.
>
>> + help
>> + If you say yes here you get support for usb functionality of HiSilicon Hikey Platform.
>
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * hisi_hikey_usb.c
>> + *
>> + * Copyright (c) Hisilicon Tech. Co., Ltd. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>
> Same comments to the above lines.
>
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/usb/role.h>
>> +#include <linux/notifier.h>
>
> Order?
>
>> + pr_debug("%s: set hub power %d\n", __func__, value);
>
> Noise.
>
>> + pr_debug("%s: switch to %s\n", __func__, switch_to_str);
>
> Noise.
>
>> + pr_debug("%s: set typec vbus gpio to %d\n", __func__, value);
>
> Noise.
>
>> + if (!nb)
>> + return -EINVAL;
>
> On which conditions this happen?
>
>> + pr_info("%s:set typec state to %lu\n", __func__, state);
>
> Noise!
>
> Guys, you perhaps can read about tracepoints and function tracer
> facilities in the kernel.
>
>> +static int hisi_hikey_usb_probe(struct platform_device *pdev)
>> +{
>
>> + pr_info("%s: typc_vbus_enable_val can't get\n", __func__);
>
> Noise!
OK.
>
>> + if (IS_ERR_OR_NULL(hisi_hikey_usb->typec_vbus)) {
>
> So, this check is redundant. You are repeating it below.
>
>> + if (!hisi_hikey_usb->typec_vbus)
>> + ret = -EINVAL;
>> + else
>> + ret = PTR_ERR(hisi_hikey_usb->typec_vbus);
>> + return ret;
>> + }
OK.
>
>> + if (IS_ERR_OR_NULL(hisi_hikey_usb->role_sw)) {
>
>> + pr_err("%s: usb_role_switch_get failed\n", __func__);
>
> Noise and same comment to the conditional check as above
>
>> + if (!hisi_hikey_usb->role_sw)
>> + ret = -ENOENT;
>> + else
>> + ret = PTR_ERR(hisi_hikey_usb->role_sw);
>> + return ret;
>> + }
>
>> + .of_match_table = of_match_ptr(id_table_hisi_hikey_usb),
>
> Does it compiles for non-OF case? Why macro is in use?
>
OK. I will add the "CONFIG_OF".
On Mon, Dec 03, 2018 at 11:45:12AM +0800, Yu Chen wrote:
> 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: Felipe Balbi <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Yu Chen <[email protected]>
> Signed-off-by: Heikki Krogerus <[email protected]>
>
> --
> v0:
> The patch is provided by Heikki Krogerus. I modified and test it
> on Hikey960 platform.
> --
> ---
> drivers/usb/dwc3/core.h | 2 ++
> drivers/usb/dwc3/drd.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 51 insertions(+)
You need to select USB_ROLE_SWITCH in Kconfig:
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 1a0404fda596..3a0cb9f1f38a 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -42,6 +42,7 @@ config USB_DWC3_DUAL_ROLE
bool "Dual Role mode"
depends on ((USB=y || USB=USB_DWC3) && (USB_GADGET=y || USB_GADGET=USB_DWC3))
depends on (EXTCON=y || EXTCON=USB_DWC3)
+ select USB_ROLE_SWITCH
help
This is the default mode of working of DWC3 controller where
both host and gadget features are enabled.
--
heikki
On Mon, Dec 03, 2018 at 11:45:15AM +0800, Yu Chen wrote:
> This patch adds support for usb on Hikey960.
>
> Cc: Wei Xu <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: [email protected]
> Cc: John Stultz <[email protected]>
> Cc: Binghui Wang <[email protected]>
> Signed-off-by: Yu Chen <[email protected]>
> ---
> arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 56 ++++++++++++++++++
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 72 +++++++++++++++++++++++
> 2 files changed, 128 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> index c98bcbc8dfba..066c558154ea 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> @@ -13,6 +13,7 @@
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/input.h>
> #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/usb/pd.h>
>
> / {
> model = "HiKey960";
> @@ -196,6 +197,28 @@
> method = "smc";
> };
> };
> +
> + hisi_hikey_usb: hisi_hikey_usb {
> + compatible = "hisilicon,hikey960_usb";
> + typec-vbus-gpios = <&gpio25 2 0>;
> + typc-vbus-enable-val = <1>;
> + otg-switch-gpios = <&gpio25 6 0>;
> + hub-vdd33-en-gpios = <&gpio5 6 0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&usbhub5734_pmx_func>;
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + hikey_usb_ep: endpoint@0 {
> + reg = <0>;
> + device_type = "usb-role-switch";
> + remote-endpoint = <&dwc3_role_switch_notify>;
> + };
> + };
> + };
> +
> };
>
> /*
> @@ -526,6 +549,39 @@
> &i2c1 {
> status = "okay";
>
> + rt1711h: rt1711h@4e {
> + compatible = "richtek,rt1711h";
> + reg = <0x4e>;
> + status = "ok";
> + interrupt-parent = <&gpio27>;
> + interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&usb_cfg_func>;
> +
> + usb_con: connector {
> + compatible = "usb-c-connector";
> + label = "USB-C";
> + data-role = "dual";
> + power-role = "dual";
> + try-power-role = "sink";
> + source-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)>;
> + sink-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)
> + PDO_VAR(5000, 5000, 1000)>;
> + op-sink-microwatt = <10000000>;
> + };
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rt1711h_ep: endpoint@0 {
> + reg = <0>;
> + device_type = "usb-role-switch";
device_type is deprecated, no?
In any case, I don't think we can use the endpoint subnode for
matching. To play it safe, I think we should use the remote port
parent for matching. See below..
> + remote-endpoint = <&dwc3_role_switch>;
> + };
> + };
> + };
> +
> adv7533: adv7533@39 {
> status = "ok";
> compatible = "adi,adv7533";
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> index f432b0a88c65..e6583bd7efdb 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
> @@ -349,6 +349,12 @@
> #clock-cells = <1>;
> };
>
> + pmctrl: pmctrl@fff31000 {
> + compatible = "hisilicon,hi3660-pmctrl", "syscon";
> + reg = <0x0 0xfff31000 0x0 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> pmuctrl: crg_ctrl@fff34000 {
> compatible = "hisilicon,hi3660-pmuctrl", "syscon";
> reg = <0x0 0xfff34000 0x0 0x1000>;
> @@ -1122,5 +1128,71 @@
> };
> };
> };
> +
> + usb3_otg_bc: usb3_otg_bc@ff200000 {
> + compatible = "syscon";
> + reg = <0x0 0xff200000 0x0 0x1000>;
> + };
> +
> + usb_phy: usb-phy {
> + compatible = "hisilicon,hi3660-usb-phy";
> + #phy-cells = <0>;
> + hisilicon,pericrg-syscon = <&crg_ctrl>;
> + hisilicon,pctrl-syscon = <&pctrl>;
> + hisilicon,usb3-otg-bc-syscon = <&usb3_otg_bc>;
> + hisilicon,eye-diagram-param = <0x22466e4>;
> + };
> +
> + usb3: hisi_dwc3 {
> + compatible = "hisilicon,hi3660-dwc3";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
> +
> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> + assigned-clock-rates = <229000000>;
> + resets = <&crg_rst 0x90 8>,
> + <&crg_rst 0x90 7>,
> + <&crg_rst 0x90 6>,
> + <&crg_rst 0x90 5>;
> +
> + dwc3: dwc3@ff100000 {
> + compatible = "snps,dwc3";
Can we use the compatible property?
compatible = "snps,dwc3", "usb-role-switch"
> + reg = <0x0 0xff100000 0x0 0x100000>;
> + interrupts = <0 159 4>, <0 161 4>;
> + phys = <&usb_phy>;
> + phy-names = "usb3-phy";
> + dr_mode = "otg";
> + maximum-speed = "super-speed";
> + phy_type = "utmi";
> + snps,dis-del-phy-power-chg-quirk;
> + snps,lfps_filter_quirk;
> + snps,dis_u2_susphy_quirk;
> + snps,dis_u3_susphy_quirk;
> + snps,tx_de_emphasis_quirk;
> + snps,tx_de_emphasis = <1>;
> + snps,dis_enblslpm_quirk;
> + snps,gctl-reset-quirk;
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + dwc3_role_switch: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&rt1711h_ep>;
> + };
> +
> + dwc3_role_switch_notify: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <&hikey_usb_ep>;
> + };
> + };
> + };
> + };
> };
> };
> --
> 2.15.0-rc2
--
heikki
On Tue, Dec 4, 2018 at 3:40 AM Chen Yu <[email protected]> wrote:
> On 2018/12/3 17:23, Andy Shevchenko wrote:
> > On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
> >> + .of_match_table = of_match_ptr(id_table_hisi_hikey_usb),
> >
> > Does it compiles for non-OF case? Why macro is in use?
> OK. I will add the "CONFIG_OF".
Why? Is this driver supposed to work without DT?
--
With Best Regards,
Andy Shevchenko
On 2018/12/4 18:54, Heikki Krogerus wrote:
> On Mon, Dec 03, 2018 at 11:45:12AM +0800, Yu Chen wrote:
>> 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: Felipe Balbi <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Signed-off-by: Yu Chen <[email protected]>
>> Signed-off-by: Heikki Krogerus <[email protected]>
>>
>> --
>> v0:
>> The patch is provided by Heikki Krogerus. I modified and test it
>> on Hikey960 platform.
>> --
>> ---
>> drivers/usb/dwc3/core.h | 2 ++
>> drivers/usb/dwc3/drd.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 51 insertions(+)
>
> You need to select USB_ROLE_SWITCH in Kconfig:
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 1a0404fda596..3a0cb9f1f38a 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -42,6 +42,7 @@ config USB_DWC3_DUAL_ROLE
> bool "Dual Role mode"
> depends on ((USB=y || USB=USB_DWC3) && (USB_GADGET=y || USB_GADGET=USB_DWC3))
> depends on (EXTCON=y || EXTCON=USB_DWC3)
> + select USB_ROLE_SWITCH
> help
> This is the default mode of working of DWC3 controller where
> both host and gadget features are enabled.
>
OK.
Thanks!
Hi,
On 2018/12/5 1:47, Andy Shevchenko wrote:
> On Tue, Dec 4, 2018 at 3:40 AM Chen Yu <[email protected]> wrote:
>> On 2018/12/3 17:23, Andy Shevchenko wrote:
>>> On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
>
>>>> + .of_match_table = of_match_ptr(id_table_hisi_hikey_usb),
>>>
>>> Does it compiles for non-OF case? Why macro is in use?
>
>> OK. I will add the "CONFIG_OF".
> Why? Is this driver supposed to work without DT?
>
No. This driver should depend on OF.
Can I solve this by add "dpends on OF" in Kconfig?
On Wed, Dec 5, 2018 at 3:57 AM Chen Yu <[email protected]> wrote:
> On 2018/12/5 1:47, Andy Shevchenko wrote:
> > On Tue, Dec 4, 2018 at 3:40 AM Chen Yu <[email protected]> wrote:
> >> On 2018/12/3 17:23, Andy Shevchenko wrote:
> >>> On Mon, Dec 3, 2018 at 5:47 AM Yu Chen <[email protected]> wrote:
> >
> >>>> + .of_match_table = of_match_ptr(id_table_hisi_hikey_usb),
> >>>
> >>> Does it compiles for non-OF case? Why macro is in use?
> >
> >> OK. I will add the "CONFIG_OF".
> > Why? Is this driver supposed to work without DT?
> >
> No. This driver should depend on OF.
> Can I solve this by add "dpends on OF" in Kconfig?
Yes and don't forget to drop unneeded macro(s) like of_match_ptr() above.
--
With Best Regards,
Andy Shevchenko
On Mon, Dec 03, 2018 at 05:28:56PM +0800, Chen Yu wrote:
> Hi,
>
> On 2018/12/3 16:59, Sergei Shtylyov wrote:
> > On 03.12.2018 11:51, Chen Yu wrote:
> >
> >>>> This patch adds binding descriptions to support the dwc3 controller
> >>>> on HiSilicon SoCs and boards like the HiKey960.
> >>>>
> >>>> Cc: Greg Kroah-Hartman <[email protected]>
> >>>> Cc: Rob Herring <[email protected]>
> >>>> Cc: Mark Rutland <[email protected]>
> >>>> Cc: John Stultz <[email protected]>
> >>>> Signed-off-by: Yu Chen <[email protected]>
> >>>> ---
> >>>> ?? .../devicetree/bindings/usb/dwc3-hisi.txt????????? | 67 ++++++++++++++++++++++
> >>>> ?? 1 file changed, 67 insertions(+)
> >>>> ?? create mode 100644 Documentation/devicetree/bindings/usb/dwc3-hisi.txt
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3-hisi.txt b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
> >>>> new file mode 100644
> >>>> index 000000000000..d32d2299a0a1
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
> >>>> @@ -0,0 +1,67 @@
> >>>> +HiSilicon DWC3 USB SoC controller
> >>>> +
> >>>> +This file documents the parameters for the dwc3-hisi driver.
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible:??? should be "hisilicon,hi3660-dwc3"
> >>>> +- clocks:??? A list of phandle + clock-specifier pairs for the
> >>>> +??????? clocks listed in clock-names
> >>>> +- clock-names:??? Specify clock names
> >>>> +- resets:??? list of phandle and reset specifier pairs.
> >>>> +
> >>>> +Sub-nodes:
> >>>> +The dwc3 core should be added as subnode to HiSilicon DWC3 as shown in the
> >>>> +example below. The DT binding details of dwc3 can be found in:
> >>>> +Documentation/devicetree/bindings/usb/dwc3.txt
> >>>> +
> >>>> +Example:
> >>>> +??? usb3: hisi_dwc3 {
> >>>> +??????? compatible = "hisilicon,hi3660-dwc3";
> >>>> +??????? #address-cells = <2>;
> >>>> +??????? #size-cells = <2>;
> >>>> +??????? ranges;
> >>>> +
> >>>> +??????? clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
> >>>> +???????????? <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> >>>> +??????? clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
> >>>> +??????? assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> >>>> +??????? assigned-clock-rates = <229000000>;
> >>>> +??????? resets = <&crg_rst 0x90 8>,
> >>>> +???????????? <&crg_rst 0x90 7>,
> >>>> +???????????? <&crg_rst 0x90 6>,
> >>>> +???????????? <&crg_rst 0x90 5>;
> >>>> +
> >>>> +??????? dwc3: dwc3@ff100000 {
Please combine these into a single node. Unless you have a wrapper with
registers, you don't need these 2 nodes. Clocks and reset can go in the
dwc3 node.
> >>>
> >>> ???? According to the DT spec, the node names should be generic, not chip specific, i.e. usb@ff100000 in this case.
> >>>
> >>
> >> Do you mean it should be usb@ff100000: dwc3@ff100000 ?
> >
> > ????dwc3: usb@ff100000
> >
> > ?? "dwc3:" is a label, not name.
>
> I use the node name "dwc3@ff100000" according to Documentation/devicetree/bindings/usb/dwc3.txt
> and documentations of vendor drivers, i.e. qcom,dwc3.txt, rockchip,dwc3.txt.
>
> In these documentations, the dwc3 sub-node name uses "dwc3@xxxxxxxx".
>
> I think it is better to be same as the other vendor's dwc3 drivers.
It's not. The other bindings are wrong. Follow the DT Spec.
Rob
On Mon, Dec 03, 2018 at 11:45:06AM +0800, Yu Chen wrote:
> This patch adds binding documentation to support usb hub and usb
> data role switch of Hisilicon HiKey&HiKey960 Board.
>
> Cc: Sergei Shtylyov <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: John Stultz <[email protected]>
> Cc: Binghui Wang <[email protected]>
> Signed-off-by: Yu Chen <[email protected]>
> --
> v1:
> 1) Fix some format errors as suggested by Sergei.
> 2) Modify gpio description to use gpiod API.
> --
> ---
> .../bindings/misc/hisilicon-hikey-usb.txt | 36 ++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
>
> diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
> new file mode 100644
> index 000000000000..f3413d851e18
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
> @@ -0,0 +1,36 @@
> +Support usb hub and usb data role switch of Hisilicon HiKey&HiKey960 Board.
s/&/ & /
Some trivial comments below, but this all needs to use the USB connector
binding for the OTG and host ports and then we need to figure out how
the switching fits into that.
> +
> +-----------------------------
> +
> +Required properties:
> +- compatible: "hisilicon,gpio-hubv1","hisilicon,hikey960-usb"
> +- typc-vbus-int-gpio: gpio to control the vbus of typeC port
> +- typc-vbus-enable-val: gpio value that enable the vbus of typeC port
HiKey at least doesn't have a TypeC port.
> +- otg-gpio: gpio to switch DP&DM between the hub and typeC port
> +- hub-vdd33-en-gpio: gpio to enable the power of hub
-gpios for all these.
> +- pinctrl-0: pinctrl config
> +
> +Example
> +-----
> +
> + hisi_hikey_usb: hisi_hikey_usb {
> + compatible = "hisilicon,hikey960-usb";
> + typec-vbus-gpios = <&gpio25 2 0>;
> + typc-vbus-enable-val = <1>;
> + otg-switch-gpios = <&gpio25 6 0>;
> + hub-vdd33-en-gpios = <&gpio5 6 0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&usbhub5734_pmx_func>;
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + hikey_usb_ep: endpoint@0 {
> + reg = <0>;
> + device_type = "usb-role-switch";
Drop device_type.
> + remote-endpoint = <&dwc3_role_switch_notify>;
> + };
> + };
> +
> + };
> --
> 2.15.0-rc2
>
On Mon, Dec 03, 2018 at 11:45:05AM +0800, Yu Chen wrote:
> This patch adds binding documentation for supporting the hi3660 usb
> phy on boards like the HiKey960.
>
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: John Stultz <[email protected]>
> Cc: Binghui Wang <[email protected]>
> Signed-off-by: Yu Chen <[email protected]>
> --
> v1: Fix some format error as suggested by Rob.
> --
> ---
> .../devicetree/bindings/phy/phy-hi3660-usb3.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
> new file mode 100644
> index 000000000000..8ea52d51aa13
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
> @@ -0,0 +1,21 @@
> +Hisilicon hi3660 usb PHY
s/usb/USB/
> +-----------------------
> +
> +Required properties:
> +- compatible: should be "hisilicon,hi3660-usb-phy"
> +- #phy-cells: must be 0
> +- hisilicon,pericrg-syscon: phandle of syscon used to control phy.
> +- hisilicon,pctrl-syscon: phandle of syscon used to control phy.
> +- hisilicon,usb3-otg-bc-syscon: phandle of syscon used to control phy.
> +- hisilicon,eye-diagram-param: parameter set for phy
> +Refer to phy/phy-bindings.txt for the generic PHY binding properties
Doesn't a phy need some clocks?
> +
> +Example:
> + usb-phy {
> + compatible = "hisilicon,hi3660-usb-phy";
> + #phy-cells = <0>;
> + hisilicon,pericrg-syscon = <&crg_ctrl>;
> + hisilicon,pctrl-syscon = <&pctrl>;
> + hisilicon,usb3-otg-bc-syscon = <&usb3_otg_bc>;
Is one of these the "main" registers for the phy? If so, then make this
a child of that syscon node.
> + hisilicon,eye-diagram-param = <0x22466e4>;
> + };
> --
> 2.15.0-rc2
>
Hi,
On 2018/12/19 22:09, Rob Herring wrote:
> On Mon, Dec 03, 2018 at 05:28:56PM +0800, Chen Yu wrote:
>> Hi,
>>
>> On 2018/12/3 16:59, Sergei Shtylyov wrote:
>>> On 03.12.2018 11:51, Chen Yu wrote:
>>>
>>>>>> This patch adds binding descriptions to support the dwc3 controller
>>>>>> on HiSilicon SoCs and boards like the HiKey960.
>>>>>>
>>>>>> Cc: Greg Kroah-Hartman <[email protected]>
>>>>>> Cc: Rob Herring <[email protected]>
>>>>>> Cc: Mark Rutland <[email protected]>
>>>>>> Cc: John Stultz <[email protected]>
>>>>>> Signed-off-by: Yu Chen <[email protected]>
>>>>>> ---
>>>>>> .../devicetree/bindings/usb/dwc3-hisi.txt | 67 ++++++++++++++++++++++
>>>>>> 1 file changed, 67 insertions(+)
>>>>>> create mode 100644 Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3-hisi.txt b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>>>>> new file mode 100644
>>>>>> index 000000000000..d32d2299a0a1
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc3-hisi.txt
>>>>>> @@ -0,0 +1,67 @@
>>>>>> +HiSilicon DWC3 USB SoC controller
>>>>>> +
>>>>>> +This file documents the parameters for the dwc3-hisi driver.
>>>>>> +
>>>>>> +Required properties:
>>>>>> +- compatible: should be "hisilicon,hi3660-dwc3"
>>>>>> +- clocks: A list of phandle + clock-specifier pairs for the
>>>>>> + clocks listed in clock-names
>>>>>> +- clock-names: Specify clock names
>>>>>> +- resets: list of phandle and reset specifier pairs.
>>>>>> +
>>>>>> +Sub-nodes:
>>>>>> +The dwc3 core should be added as subnode to HiSilicon DWC3 as shown in the
>>>>>> +example below. The DT binding details of dwc3 can be found in:
>>>>>> +Documentation/devicetree/bindings/usb/dwc3.txt
>>>>>> +
>>>>>> +Example:
>>>>>> + usb3: hisi_dwc3 {
>>>>>> + compatible = "hisilicon,hi3660-dwc3";
>>>>>> + #address-cells = <2>;
>>>>>> + #size-cells = <2>;
>>>>>> + ranges;
>>>>>> +
>>>>>> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
>>>>>> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>>>>>> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
>>>>>> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>>>>>> + assigned-clock-rates = <229000000>;
>>>>>> + resets = <&crg_rst 0x90 8>,
>>>>>> + <&crg_rst 0x90 7>,
>>>>>> + <&crg_rst 0x90 6>,
>>>>>> + <&crg_rst 0x90 5>;
>>>>>> +
>>>>>> + dwc3: dwc3@ff100000 {
>
> Please combine these into a single node. Unless you have a wrapper with
> registers, you don't need these 2 nodes. Clocks and reset can go in the
> dwc3 node.
Yes, clocks can go in the dwc3 node, but there are four resets need to be
deasserted for dwc3 usb of Hi3660 Soc and there is only one reset handled
in dwc3 driver.
>
>>>>>
>>>>> According to the DT spec, the node names should be generic, not chip specific, i.e. usb@ff100000 in this case.
>>>>>
>>>>
>>>> Do you mean it should be usb@ff100000: dwc3@ff100000 ?
>>>
>>> dwc3: usb@ff100000
>>>
>>> "dwc3:" is a label, not name.
>>
>> I use the node name "dwc3@ff100000" according to Documentation/devicetree/bindings/usb/dwc3.txt
>> and documentations of vendor drivers, i.e. qcom,dwc3.txt, rockchip,dwc3.txt.
>>
>> In these documentations, the dwc3 sub-node name uses "dwc3@xxxxxxxx".
>>
>> I think it is better to be same as the other vendor's dwc3 drivers.
>
> It's not. The other bindings are wrong. Follow the DT Spec.
>
> Rob
>
> .
>
Hi,
On 2018/12/19 22:14, Rob Herring wrote:
> On Mon, Dec 03, 2018 at 11:45:05AM +0800, Yu Chen wrote:
>> This patch adds binding documentation for supporting the hi3660 usb
>> phy on boards like the HiKey960.
>>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: John Stultz <[email protected]>
>> Cc: Binghui Wang <[email protected]>
>> Signed-off-by: Yu Chen <[email protected]>
>> --
>> v1: Fix some format error as suggested by Rob.
>> --
>> ---
>> .../devicetree/bindings/phy/phy-hi3660-usb3.txt | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
>> new file mode 100644
>> index 000000000000..8ea52d51aa13
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
>> @@ -0,0 +1,21 @@
>> +Hisilicon hi3660 usb PHY
>
> s/usb/USB/
OK.
>
>> +-----------------------
>> +
>> +Required properties:
>> +- compatible: should be "hisilicon,hi3660-usb-phy"
>> +- #phy-cells: must be 0
>> +- hisilicon,pericrg-syscon: phandle of syscon used to control phy.
>> +- hisilicon,pctrl-syscon: phandle of syscon used to control phy.
>> +- hisilicon,usb3-otg-bc-syscon: phandle of syscon used to control phy.
>> +- hisilicon,eye-diagram-param: parameter set for phy
>> +Refer to phy/phy-bindings.txt for the generic PHY binding properties
>
> Doesn't a phy need some clocks?
>
Yes, the clocks of USB phy currently is enabled by config the
pctrl-syscon registers and pericrg-syscon.
>> +
>> +Example:
>> + usb-phy {
>> + compatible = "hisilicon,hi3660-usb-phy";
>> + #phy-cells = <0>;
>> + hisilicon,pericrg-syscon = <&crg_ctrl>;
>> + hisilicon,pctrl-syscon = <&pctrl>;
>> + hisilicon,usb3-otg-bc-syscon = <&usb3_otg_bc>;
>
> Is one of these the "main" registers for the phy? If so, then make this
> a child of that syscon node.
>
Ok, the usb3-otg-bc-syscon is the "main" registers for the phy.
>> + hisilicon,eye-diagram-param = <0x22466e4>;
>> + };
>> --
>> 2.15.0-rc2
>>
>
> .
>
On 2018/12/19 22:21, Rob Herring wrote:
> On Mon, Dec 03, 2018 at 11:45:06AM +0800, Yu Chen wrote:
>> This patch adds binding documentation to support usb hub and usb
>> data role switch of Hisilicon HiKey&HiKey960 Board.
>>
>> Cc: Sergei Shtylyov <[email protected]>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: John Stultz <[email protected]>
>> Cc: Binghui Wang <[email protected]>
>> Signed-off-by: Yu Chen <[email protected]>
>> --
>> v1:
>> 1) Fix some format errors as suggested by Sergei.
>> 2) Modify gpio description to use gpiod API.
>> --
>> ---
>> .../bindings/misc/hisilicon-hikey-usb.txt | 36 ++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
>> new file mode 100644
>> index 000000000000..f3413d851e18
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.txt
>> @@ -0,0 +1,36 @@
>> +Support usb hub and usb data role switch of Hisilicon HiKey&HiKey960 Board.
>
> s/&/ & /
>
OK.
> Some trivial comments below, but this all needs to use the USB connector
> binding for the OTG and host ports and then we need to figure out how
> the switching fits into that.
>
>> +
>> +-----------------------------
>> +
>> +Required properties:
>> +- compatible: "hisilicon,gpio-hubv1","hisilicon,hikey960-usb"
>> +- typc-vbus-int-gpio: gpio to control the vbus of typeC port
>> +- typc-vbus-enable-val: gpio value that enable the vbus of typeC port
>
> HiKey at least doesn't have a TypeC port.
>Yes. I will re-check the driver for Hikey.
>> +- otg-gpio: gpio to switch DP&DM between the hub and typeC port
>> +- hub-vdd33-en-gpio: gpio to enable the power of hub
>
> -gpios for all these.
OK.
>
>> +- pinctrl-0: pinctrl config
>> +
>> +Example
>> +-----
>> +
>> + hisi_hikey_usb: hisi_hikey_usb {
>> + compatible = "hisilicon,hikey960-usb";
>> + typec-vbus-gpios = <&gpio25 2 0>;
>> + typc-vbus-enable-val = <1>;
>> + otg-switch-gpios = <&gpio25 6 0>;
>> + hub-vdd33-en-gpios = <&gpio5 6 0>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&usbhub5734_pmx_func>;
>> +
>> + port {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + hikey_usb_ep: endpoint@0 {
>> + reg = <0>;
>> + device_type = "usb-role-switch";
>
> Drop device_type.
Currently the device_type is used for usb-role-switch match, but it
will be removed after Heikki Krogerus's patch.
>
>> + remote-endpoint = <&dwc3_role_switch_notify>;
>> + };
>> + };
>> +
>> + };
>> --
>> 2.15.0-rc2
>>
>
> .
>
Thanks!
Hi,
Rob Herring <[email protected]> writes:
>> >>>> +Example:
>> >>>> + usb3: hisi_dwc3 {
>> >>>> + compatible = "hisilicon,hi3660-dwc3";
>> >>>> + #address-cells = <2>;
>> >>>> + #size-cells = <2>;
>> >>>> + ranges;
>> >>>> +
>> >>>> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
>> >>>> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>> >>>> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
>> >>>> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
>> >>>> + assigned-clock-rates = <229000000>;
>> >>>> + resets = <&crg_rst 0x90 8>,
>> >>>> + <&crg_rst 0x90 7>,
>> >>>> + <&crg_rst 0x90 6>,
>> >>>> + <&crg_rst 0x90 5>;
>> >>>> +
>> >>>> + dwc3: dwc3@ff100000 {
>
> Please combine these into a single node. Unless you have a wrapper with
> registers, you don't need these 2 nodes. Clocks and reset can go in the
> dwc3 node.
>
>> >>>
>> >>> According to the DT spec, the node names should be generic, not chip specific, i.e. usb@ff100000 in this case.
>> >>>
>> >>
>> >> Do you mean it should be usb@ff100000: dwc3@ff100000 ?
>> >
>> > dwc3: usb@ff100000
>> >
>> > "dwc3:" is a label, not name.
>>
>> I use the node name "dwc3@ff100000" according to Documentation/devicetree/bindings/usb/dwc3.txt
>> and documentations of vendor drivers, i.e. qcom,dwc3.txt, rockchip,dwc3.txt.
>>
>> In these documentations, the dwc3 sub-node name uses "dwc3@xxxxxxxx".
>>
>> I think it is better to be same as the other vendor's dwc3 drivers.
>
> It's not. The other bindings are wrong. Follow the DT Spec.
what's wrong about them? They clearly describe the HW:
1) a company-specific glue/adaptation/integration IP
2) a generic licensed IP inside it
dwc3.ko is compatible with Synopsys' documentation and there's only one
incarnation of dwc3. Everything that can be detected in runtime, we do
so. Everything that can't, we use quirk flags. Keep in mind dwc3.ko is
also used as is by non-DT systems where we can't simply change a
compatible flag.
--
balbi
On Thu, Dec 20, 2018 at 12:46 AM Felipe Balbi
<[email protected]> wrote:
>
>
> Hi,
>
> Rob Herring <[email protected]> writes:
> >> >>>> +Example:
> >> >>>> + usb3: hisi_dwc3 {
> >> >>>> + compatible = "hisilicon,hi3660-dwc3";
> >> >>>> + #address-cells = <2>;
> >> >>>> + #size-cells = <2>;
> >> >>>> + ranges;
> >> >>>> +
> >> >>>> + clocks = <&crg_ctrl HI3660_CLK_ABB_USB>,
> >> >>>> + <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> >> >>>> + clock-names = "clk_usb3phy_ref", "aclk_usb3otg";
> >> >>>> + assigned-clocks = <&crg_ctrl HI3660_ACLK_GATE_USB3OTG>;
> >> >>>> + assigned-clock-rates = <229000000>;
> >> >>>> + resets = <&crg_rst 0x90 8>,
> >> >>>> + <&crg_rst 0x90 7>,
> >> >>>> + <&crg_rst 0x90 6>,
> >> >>>> + <&crg_rst 0x90 5>;
> >> >>>> +
> >> >>>> + dwc3: dwc3@ff100000 {
> >
> > Please combine these into a single node. Unless you have a wrapper with
> > registers, you don't need these 2 nodes. Clocks and reset can go in the
> > dwc3 node.
> >
> >> >>>
> >> >>> According to the DT spec, the node names should be generic, not chip specific, i.e. usb@ff100000 in this case.
> >> >>>
> >> >>
> >> >> Do you mean it should be usb@ff100000: dwc3@ff100000 ?
> >> >
> >> > dwc3: usb@ff100000
> >> >
> >> > "dwc3:" is a label, not name.
> >>
> >> I use the node name "dwc3@ff100000" according to Documentation/devicetree/bindings/usb/dwc3.txt
> >> and documentations of vendor drivers, i.e. qcom,dwc3.txt, rockchip,dwc3.txt.
> >>
> >> In these documentations, the dwc3 sub-node name uses "dwc3@xxxxxxxx".
> >>
> >> I think it is better to be same as the other vendor's dwc3 drivers.
> >
> > It's not. The other bindings are wrong. Follow the DT Spec.
>
> what's wrong about them? They clearly describe the HW:
>
> 1) a company-specific glue/adaptation/integration IP
> 2) a generic licensed IP inside it
That is *every* licensed IP block and DWC3 is the oddball where we did
this 2 node thing. It is not a pattern we should continue. If there's
registers in the wrapper, then yes, having 2 nodes makes sense. But
just additional clocks or resets, no. I would guess these extra clocks
and resets are inter-connect related and are needed as an artifact of
not describing and managing inter-connects.
I can just as easily argue it doesn't describe the hardware. I'm
pretty sure the DWC3 has clocks and resets yet there are none in the
DWC3 node. How can it operate with no clocks?
> dwc3.ko is compatible with Synopsys' documentation and there's only one
> incarnation of dwc3. Everything that can be detected in runtime, we do
> so. Everything that can't, we use quirk flags. Keep in mind dwc3.ko is
> also used as is by non-DT systems where we can't simply change a
> compatible flag.
Linux driver architecture doesn't dictate bindings.
Rob
Hi Heikki,
On 2018/12/3 20:25, Heikki Krogerus wrote:
> Hi,
>
> On Mon, Dec 03, 2018 at 10:17:20AM +0200, Andy Shevchenko wrote:
>> On Mon, Dec 3, 2018 at 5:45 AM Yu Chen <[email protected]> wrote:
>>>
>>> This patch adds code for supporting find usb role switch by matching against
>>> the device node described using of_graph.
>>
>>> #include <linux/module.h>
>>> #include <linux/mutex.h>
>>> #include <linux/slab.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_graph.h>
>>
>> Keep it ordered.
>>
>>> +static int __switch_match_by_of_node(struct device *dev, const void *name)
>>> +{
>>> + if (!dev->parent || !dev->parent->of_node)
>>> + return 0;
>>> +
>>> + return of_node_name_eq(dev->parent->of_node, (const char *)name);
>>> +}
>>
>> I think Heikki has introduced some more generic function for this.
>
> Yes, for this I'm proposing a new function fwnode_get_name() that
> should allow us to support ACPI as well as OF platforms:
> https://lkml.org/lkml/2018/11/28/784
>
>>> sw = device_connection_find_match(dev, "usb-role-switch", NULL,
>>> usb_role_switch_match);
>>
>>> + sw = of_graph_find_match_by_type(dev, "usb-role-switch");
>>
>> Why this is here?! device_connection_find_match() should take care for OF case.
>
> Exactly. device_connection_find_match() needs to parse the graph (OF
> and ACPI), and I already made proposal (RFC) for that:
> https://lkml.org/lkml/2018/10/24/619
>
> I have prepared a more finalized version of those patches. I've been
> waiting for that fwnode_get_name() to be accepted before sending it
> out since we are going to need that function, but I'll send a new RFC
> if nothing happens soon with that fwnode_get_name() series.
>
>
> Br,
>
Have the fwnode_get_name() patches been accepted?
And I will be grateful if you can tell me your plan about the
https://lkml.org/lkml/2018/10/24/619 patches.
thanks
- Yu Chen
Hi,
On Mon, Jan 21, 2019 at 04:36:13PM +0800, Chen Yu wrote:
> Have the fwnode_get_name() patches been accepted?
No, I didn't have a user for the function anymore:
https://lkml.org/lkml/2018/12/17/284
> And I will be grateful if you can tell me your plan about the
> https://lkml.org/lkml/2018/10/24/619 patches.
The plan is to send a new version of them together with the
fwnode_get_name() patch. I can send them this week.
thanks,
--
heikki
On Mon, Jan 21, 2019 at 03:51:35PM +0200, Heikki Krogerus wrote:
> Hi,
>
> On Mon, Jan 21, 2019 at 04:36:13PM +0800, Chen Yu wrote:
> > Have the fwnode_get_name() patches been accepted?
>
> No, I didn't have a user for the function anymore:
> https://lkml.org/lkml/2018/12/17/284
>
> > And I will be grateful if you can tell me your plan about the
> > https://lkml.org/lkml/2018/10/24/619 patches.
>
> The plan is to send a new version of them together with the
> fwnode_get_name() patch. I can send them this week.
I'm about to send the graph support for the device connection API, but
just letting you know that I did not need the fwnode_get_name() in the
end in that series unfortunately.
Br,
--
heikki