[PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Stanley Chang]

Realtek DHC RTD SoCs integrate dwc3 IP and has some customizations to
support different generations of SoCs.

Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
v1 to v2 change:
    Remove the the code about the property realtek,enable-l4icg.
    Select USB_ROLE_SWITCH in Kconfig.
    Add dependency OF and ARCH_REALTEK in Kconfig.
---
 drivers/usb/dwc3/Kconfig    |  12 +
 drivers/usb/dwc3/Makefile   |   1 +
 drivers/usb/dwc3/dwc3-rtk.c | 499 ++++++++++++++++++++++++++++++++++++
 3 files changed, 512 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-rtk.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index be954a9abbe0..baa6a38b2f13 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -168,4 +168,16 @@ config USB_DWC3_AM62
 	  The Designware Core USB3 IP is programmed to operate in
 	  in USB 2.0 mode only.
 	  Say 'Y' or 'M' here if you have one such device
+
+config USB_DWC3_RTK
+	tristate "Realtek DWC3 Platform Driver"
+	depends on OF && ARCH_REALTEK
+	default USB_DWC3
+	select USB_ROLE_SWITCH
+	help
+	  RTK DHC RTD SoCs with DesignWare Core USB3 IP inside,
+	  and IP Core configured for USB 2.0 and USB 3.0 in host
+	  or dual-role mode.
+	  Say 'Y' or 'M' if you have such device.
+
 endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 9f66bd82b639..38d19e4369a5 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -54,3 +54,4 @@ obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
 obj-$(CONFIG_USB_DWC3_QCOM)		+= dwc3-qcom.o
 obj-$(CONFIG_USB_DWC3_IMX8MP)		+= dwc3-imx8mp.o
 obj-$(CONFIG_USB_DWC3_XILINX)		+= dwc3-xilinx.o
+obj-$(CONFIG_USB_DWC3_RTK)		+= dwc3-rtk.o
diff --git a/drivers/usb/dwc3/dwc3-rtk.c b/drivers/usb/dwc3/dwc3-rtk.c
new file mode 100644
index 000000000000..5c2b2f4e422d
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-rtk.c
@@ -0,0 +1,499 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dwc3-rtk.c - Realtek DWC3 Specific Glue layer
+ *
+ * Copyright (C) 2023 Realtek Semiconductor Corporation
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/suspend.h>
+#include <linux/sys_soc.h>
+#include <linux/usb/otg.h>
+#include <linux/usb/of.h>
+#include <linux/usb/role.h>
+
+#include "core.h"
+
+#define WRAP_CTR_REG  0x0
+#define DISABLE_MULTI_REQ BIT(1)
+#define DESC_R2W_MULTI_DISABLE BIT(9)
+#define FORCE_PIPE3_PHY_STATUS_TO_0 BIT(13)
+
+#define WRAP_USB2_PHY_UTMI_REG 0x8
+#define TXHSVM_EN BIT(3)
+
+#define WRAP_PHY_PIPE_REG 0xC
+#define RESET_DISABLE_PIPE3_P0 BIT(0)
+#define CLOCK_ENABLE_FOR_PIPE3_PCLK BIT(1)
+
+#define WRAP_USB_HMAC_CTR0_REG 0x60
+#define U3PORT_DIS BIT(8)
+
+#define WRAP_USB2_PHY_REG  0x70
+#define USB2_PHY_EN_PHY_PLL_PORT0 BIT(12)
+#define USB2_PHY_EN_PHY_PLL_PORT1 BIT(13)
+#define USB2_PHY_SWITCH_MASK 0x707
+#define USB2_PHY_SWITCH_DEVICE 0x0
+#define USB2_PHY_SWITCH_HOST 0x606
+
+#define WRAP_APHY_REG 0x128
+#define USB3_MBIAS_ENABLE BIT(1)
+
+#define WRAP_USB_DBUS_PWR_CTRL_REG 0x160
+#define DBUS_PWR_CTRL_EN BIT(0)
+
+struct dwc3_rtk {
+	struct device *dev;
+	void __iomem *regs;
+	size_t regs_size;
+
+	struct dwc3 *dwc;
+
+	int cur_dr_mode; /* current dr mode */
+	bool support_drd_mode; /* if support Host/device switch */
+	struct usb_role_switch *role_switch;
+
+	bool unlink_usb3_port;
+	bool disable_usb3_phy;
+};
+
+static void switch_usb2_dr_mode(struct dwc3_rtk *rtk, int dr_mode)
+{
+	switch (dr_mode) {
+	case USB_DR_MODE_PERIPHERAL:
+		writel(USB2_PHY_SWITCH_DEVICE |
+			    (~USB2_PHY_SWITCH_MASK &
+			      readl(rtk->regs + WRAP_USB2_PHY_REG)),
+			    rtk->regs + WRAP_USB2_PHY_REG);
+		break;
+	case USB_DR_MODE_HOST:
+		writel(USB2_PHY_SWITCH_HOST |
+			    (~USB2_PHY_SWITCH_MASK &
+			      readl(rtk->regs + WRAP_USB2_PHY_REG)),
+			    rtk->regs + WRAP_USB2_PHY_REG);
+		break;
+	default:
+		dev_dbg(rtk->dev, "%s: dr_mode=%d\n", __func__, dr_mode);
+		break;
+	}
+}
+
+static void switch_dwc3_dr_mode(struct dwc3_rtk *rtk, int dr_mode)
+{
+	if (!rtk->dwc->role_sw)
+		goto out;
+
+	switch (dr_mode) {
+	case USB_DR_MODE_PERIPHERAL:
+		usb_role_switch_set_role(rtk->dwc->role_sw, USB_ROLE_DEVICE);
+		break;
+	case USB_DR_MODE_HOST:
+		usb_role_switch_set_role(rtk->dwc->role_sw, USB_ROLE_HOST);
+		break;
+	default:
+		dev_dbg(rtk->dev, "%s dr_mode=%d\n", __func__, dr_mode);
+		break;
+	}
+
+out:
+	return;
+}
+
+static int dwc3_rtk_get_dr_mode(struct dwc3_rtk *rtk)
+{
+	enum usb_role role;
+
+	role = rtk->cur_dr_mode;
+
+	if (rtk->dwc && rtk->dwc->role_sw)
+		role = usb_role_switch_get_role(rtk->dwc->role_sw);
+	else
+		dev_dbg(rtk->dev, "%s not usb_role_switch role=%d\n", __func__, role);
+
+	return role;
+}
+
+static int dwc3_rtk_set_dr_mode(struct dwc3_rtk *rtk, int dr_mode)
+{
+	if (!rtk->support_drd_mode)
+		return rtk->cur_dr_mode;
+
+	rtk->cur_dr_mode = dr_mode;
+
+	switch_dwc3_dr_mode(rtk, dr_mode);
+	mdelay(10);
+	switch_usb2_dr_mode(rtk, dr_mode);
+
+	return rtk->cur_dr_mode;
+}
+
+#if IS_ENABLED(CONFIG_USB_ROLE_SWITCH)
+static int dwc3_usb_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
+{
+	struct dwc3_rtk *rtk = usb_role_switch_get_drvdata(sw);
+
+	switch (role) {
+	case USB_ROLE_HOST:
+		dwc3_rtk_set_dr_mode(rtk, USB_DR_MODE_HOST);
+		break;
+	case USB_ROLE_DEVICE:
+		dwc3_rtk_set_dr_mode(rtk, USB_DR_MODE_PERIPHERAL);
+		break;
+	default:
+		dwc3_rtk_set_dr_mode(rtk, 0);
+	}
+
+	return 0;
+}
+
+static enum usb_role dwc3_usb_role_switch_get(struct usb_role_switch *sw)
+{
+	struct dwc3_rtk *rtk = usb_role_switch_get_drvdata(sw);
+	enum usb_role role = USB_ROLE_NONE;
+	int dr_mode;
+
+	dr_mode = dwc3_rtk_get_dr_mode(rtk);
+	switch (dr_mode) {
+	case USB_DR_MODE_HOST:
+		role = USB_ROLE_HOST;
+		break;
+	case USB_DR_MODE_PERIPHERAL:
+		role = USB_ROLE_DEVICE;
+		break;
+	default:
+		dev_dbg(rtk->dev, "%s dr_mode=%d", __func__, dr_mode);
+		break;
+	}
+	return role;
+}
+
+static int dwc3_rtk_setup_role_switch(struct dwc3_rtk *rtk)
+{
+	struct usb_role_switch_desc dwc3_role_switch = {NULL};
+
+	dwc3_role_switch.name = strchrnul(dev_name(rtk->dev), '.') + 1;
+	dwc3_role_switch.driver_data = rtk;
+	dwc3_role_switch.allow_userspace_control = true;
+	dwc3_role_switch.fwnode = dev_fwnode(rtk->dev);
+	dwc3_role_switch.set = dwc3_usb_role_switch_set;
+	dwc3_role_switch.get = dwc3_usb_role_switch_get;
+	rtk->role_switch = usb_role_switch_register(rtk->dev, &dwc3_role_switch);
+	if (IS_ERR(rtk->role_switch))
+		return PTR_ERR(rtk->role_switch);
+
+	return 0;
+}
+
+static int dwc3_rtk_remove_role_switch(struct dwc3_rtk *rtk)
+{
+	if (rtk->role_switch)
+		usb_role_switch_unregister(rtk->role_switch);
+
+	rtk->role_switch = NULL;
+
+	return 0;
+}
+#else
+#define dwc3_rtk_setup_role_switch(x) 0
+#define dwc3_rtk_remove_role_switch(x) 0
+#endif
+
+static int dwc3_rtk_init(struct dwc3_rtk *rtk)
+{
+	struct device *dev = rtk->dev;
+	void __iomem *regs = rtk->regs;
+	const struct soc_device_attribute rtk_soc_kylin_a00[] = {
+		{ .family = "Realtek Kylin", .revision = "A00", },
+		{ /* empty */ } };
+	const struct soc_device_attribute rtk_soc_kylin[] = {
+		{ .family = "Realtek Kylin", }, { /* empty */ } };
+	const struct soc_device_attribute rtk_soc_hercules[] = {
+		{ .family = "Realtek Hercules", }, { /* empty */ } };
+	const struct soc_device_attribute rtk_soc_hw_clock_gating_quirk[] = {
+		{ .family = "Realtek Kylin", },
+		{ .family = "Realtek Hercules", },
+		{ .family = "Realtek Thor", },
+		{ .family = "Realtek Hank", },
+		{ .family = "Realtek Groot", },
+		{ .family = "Realtek Stark", },
+		{ /* empty */ } };
+
+	if (soc_device_match(rtk_soc_kylin_a00)) {
+		writel(DISABLE_MULTI_REQ | readl(regs + WRAP_CTR_REG),
+		       regs + WRAP_CTR_REG);
+		dev_info(dev, "[bug fixed] 1295/1296 A00: add workaround to disable multiple request for D-Bus");
+	}
+
+	if (soc_device_match(rtk_soc_hercules)) {
+		writel(USB2_PHY_EN_PHY_PLL_PORT1 |
+		       readl(regs + WRAP_USB2_PHY_REG),
+		       regs + WRAP_USB2_PHY_REG);
+		dev_info(dev, "[bug fixed] 1395 add workaround to disable usb2 port 2 suspend!");
+	}
+
+	writel(TXHSVM_EN | readl(regs + WRAP_USB2_PHY_UTMI_REG),
+	       regs + WRAP_USB2_PHY_UTMI_REG);
+
+	if (rtk->unlink_usb3_port) {
+		void __iomem *reg;
+		int val;
+
+		reg =  rtk->regs + WRAP_USB_HMAC_CTR0_REG;
+		val = U3PORT_DIS | readl(reg);
+		writel(val, reg);
+	}
+
+	if (rtk->disable_usb3_phy) {
+		void __iomem *reg;
+		int val;
+
+		reg = rtk->regs + WRAP_CTR_REG;
+		val = FORCE_PIPE3_PHY_STATUS_TO_0 | readl(reg);
+		writel(val, reg);
+
+		reg = rtk->regs + WRAP_PHY_PIPE_REG;
+		val = ~CLOCK_ENABLE_FOR_PIPE3_PCLK & readl(reg);
+		val |= RESET_DISABLE_PIPE3_P0;
+		writel(val, reg);
+
+		reg =  rtk->regs + WRAP_USB_HMAC_CTR0_REG;
+		val = U3PORT_DIS | readl(reg);
+		writel(val, reg);
+
+		reg = rtk->regs + WRAP_APHY_REG;
+		val = ~USB3_MBIAS_ENABLE & readl(reg);
+		writel(val, reg);
+
+		dev_info(rtk->dev, "%s: disable usb 3.0 phy\n", __func__);
+	}
+
+	if (!soc_device_match(rtk_soc_kylin)) {
+		void __iomem *reg;
+		int val;
+
+		reg = rtk->regs + WRAP_USB_DBUS_PWR_CTRL_REG;
+		if (soc_device_match(rtk_soc_hw_clock_gating_quirk))
+			reg += 0x4;
+
+		val = DBUS_PWR_CTRL_EN | readl(reg);
+		writel(val, reg);
+	}
+
+	writel(DESC_R2W_MULTI_DISABLE | readl(regs + WRAP_CTR_REG),
+	       regs + WRAP_CTR_REG);
+
+	/* Set phy Dp/Dm initial state to host mode to avoid the Dp glitch */
+	writel(USB2_PHY_SWITCH_HOST |
+	       (~USB2_PHY_SWITCH_MASK &
+	       readl(rtk->regs + WRAP_USB2_PHY_REG)),
+	       rtk->regs + WRAP_USB2_PHY_REG);
+
+	return 0;
+}
+
+static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
+{
+	struct device *dev = rtk->dev;
+	struct device_node *node = dev->of_node;
+	struct platform_device *dwc3_pdev;
+	struct device *dwc3_dev;
+	struct device_node *dwc3_node;
+	int dr_mode;
+	int ret = 0;
+
+	ret = dwc3_rtk_init(rtk);
+	if (ret)
+		return -EINVAL;
+
+	ret = of_platform_populate(node, NULL, NULL, dev);
+	if (ret) {
+		dev_err(dev, "failed to add dwc3 core\n");
+		return ret;
+	}
+
+	dwc3_node = of_get_compatible_child(node, "snps,dwc3");
+	if (!dwc3_node) {
+		dev_err(dev, "failed to find dwc3 core node\n");
+		return -ENODEV;
+	}
+
+	dwc3_pdev = of_find_device_by_node(dwc3_node);
+	if (!dwc3_pdev) {
+		dev_err(dev, "failed to find dwc3 core platform_device\n");
+		of_node_put(dwc3_node);
+		return -ENODEV;
+	}
+
+	dwc3_dev = &dwc3_pdev->dev;
+	rtk->dwc = platform_get_drvdata(dwc3_pdev);
+	if (!rtk->dwc) {
+		dev_err(dev, "failed to find dwc3 core\n");
+		of_node_put(dwc3_node);
+		return -ENODEV;
+	}
+
+	dr_mode = usb_get_dr_mode(dwc3_dev);
+	if (dr_mode != rtk->dwc->dr_mode) {
+		dev_info(dev, "dts set dr_mode=%d, but dwc3 set dr_mode=%d\n",
+			 dr_mode, rtk->dwc->dr_mode);
+		dr_mode = rtk->dwc->dr_mode;
+	}
+
+	rtk->cur_dr_mode = dr_mode;
+
+	if (device_property_read_bool(dwc3_dev, "usb-role-switch"))
+		rtk->support_drd_mode = true;
+	else
+		rtk->support_drd_mode = false;
+
+	if (rtk->support_drd_mode) {
+		dwc3_rtk_setup_role_switch(rtk);
+		rtk->cur_dr_mode = dwc3_rtk_get_dr_mode(rtk);
+	}
+
+	switch_usb2_dr_mode(rtk, rtk->cur_dr_mode);
+
+	of_node_put(dwc3_node);
+	platform_device_put(dwc3_pdev);
+
+	return ret;
+}
+
+static int dwc3_rtk_probe(struct platform_device *pdev)
+{
+	struct dwc3_rtk *rtk;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct resource *res;
+	void __iomem *regs;
+	int ret = 0;
+	unsigned long probe_time = jiffies;
+
+	rtk = devm_kzalloc(dev, sizeof(*rtk), GFP_KERNEL);
+	if (!rtk) {
+		ret = -ENOMEM;
+		goto err1;
+	}
+
+	platform_set_drvdata(pdev, rtk);
+
+	rtk->dev = dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "missing memory resource\n");
+		ret = -ENODEV;
+		goto err1;
+	}
+
+	regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(regs)) {
+		ret = PTR_ERR(regs);
+		goto err1;
+	}
+
+	rtk->regs = regs;
+	rtk->regs_size = resource_size(res);
+
+	rtk->unlink_usb3_port = false;
+	if (of_property_read_bool(node, "realtek,unlink-usb3-port"))
+		rtk->unlink_usb3_port = true;
+
+	rtk->disable_usb3_phy = false;
+	if (of_property_read_bool(node, "realtek,disable-usb3-phy"))
+		rtk->disable_usb3_phy = true;
+
+	ret = dwc3_rtk_probe_dwc3_core(rtk);
+	if (ret) {
+		dev_err(dev, "%s failed to add dwc3 core\n", __func__);
+		goto err1;
+	}
+
+	dev_dbg(dev, "%s ok! (take %d ms)\n", __func__,
+		jiffies_to_msecs(jiffies - probe_time));
+
+	return 0;
+
+err1:
+	return ret;
+}
+
+static void dwc3_rtk_remove(struct platform_device *pdev)
+{
+	struct dwc3_rtk *rtk = platform_get_drvdata(pdev);
+
+	rtk->dwc = NULL;
+
+	if (rtk->support_drd_mode)
+		dwc3_rtk_remove_role_switch(rtk);
+
+	of_platform_depopulate(rtk->dev);
+}
+
+static void dwc3_rtk_shutdown(struct platform_device *pdev)
+{
+	struct dwc3_rtk *rtk = platform_get_drvdata(pdev);
+
+	of_platform_depopulate(rtk->dev);
+}
+
+static const struct of_device_id rtk_dwc3_match[] = {
+	{ .compatible = "realtek,rtd-dwc3" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rtk_dwc3_match);
+
+#ifdef CONFIG_PM_SLEEP
+static int dwc3_rtk_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int dwc3_rtk_resume(struct device *dev)
+{
+	struct dwc3_rtk *rtk = dev_get_drvdata(dev);
+
+	dwc3_rtk_init(rtk);
+
+	switch_usb2_dr_mode(rtk, rtk->cur_dr_mode);
+
+	/* runtime set active to reflect active state. */
+	pm_runtime_disable(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops dwc3_rtk_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(dwc3_rtk_suspend, dwc3_rtk_resume)
+};
+
+#define DEV_PM_OPS	(&dwc3_rtk_dev_pm_ops)
+#else
+#define DEV_PM_OPS	NULL
+#endif /* CONFIG_PM_SLEEP */
+
+static struct platform_driver dwc3_rtk_driver = {
+	.probe		= dwc3_rtk_probe,
+	.remove_new	= dwc3_rtk_remove,
+	.driver		= {
+		.name	= "rtk-dwc3",
+		.of_match_table = rtk_dwc3_match,
+		.pm	= DEV_PM_OPS,
+	},
+	.shutdown	= dwc3_rtk_shutdown,
+};
+
+module_platform_driver(dwc3_rtk_driver);
+
+MODULE_AUTHOR("Stanley Chang <stanley_chang@realtek.com>");
+MODULE_DESCRIPTION("DesignWare USB3 Realtek Glue Layer");
+MODULE_ALIAS("platform:rtk-dwc3");
+MODULE_LICENSE("GPL");
+MODULE_SOFTDEP("pre: phy_rtk_usb2 phy_rtk_usb3");
-- 
2.34.1

[PATCH v2 2/2] dt-bindings: usb: dwc3: Add Realtek DHC RTD SoC DWC3 USB

[Stanley Chang]

Document the DWC3 USB bindings for Realtek SoCs.

Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
---
v1 to v2 change:
    Revise the subject.
    Rename the file.
    Fix dtschema warnings.
    Remove the property realtek,enable-l4icg.
    Drop status.
---
 .../bindings/usb/realtek,rtd-dwc3.yaml        | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/realtek,rtd-dwc3.yaml

diff --git a/Documentation/devicetree/bindings/usb/realtek,rtd-dwc3.yaml b/Documentation/devicetree/bindings/usb/realtek,rtd-dwc3.yaml
new file mode 100644
index 000000000000..8b9050724b97
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/realtek,rtd-dwc3.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Realtek Semiconductor Corporation
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/realtek,rtd-dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Realtek DWC3 USB SoC Controller Glue
+
+maintainers:
+  - Stanley Chang <stanley_chang@realtek.com>
+
+description:
+  The Realtek DHC SoC embeds a DWC3 USB IP Core configured for USB 2.0
+  and USB 3.0 in host or dual-role mode.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - realtek,rtd1295-dwc3
+          - realtek,rtd1315e-dwc3
+          - realtek,rtd1319-dwc3
+          - realtek,rtd1319d-dwc3
+          - realtek,rtd1395-dwc3
+          - realtek,rtd1619-dwc3
+          - realtek,rtd1619b-dwc3
+      - const: realtek,rtd-dwc3
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
+  ranges: true
+
+  realtek,unlink-usb3-port:
+    description: Disable link between USB 3.0 PHY and USB mac.
+      Only for RTD1619 SoC, if the board design support maximum 2.0 speed.
+    type: boolean
+
+  realtek,disable-usb3-phy:
+    description: Close USB 3.0 PHY if the board design not support USB 3.0.
+    type: boolean
+
+patternProperties:
+  "^usb@[0-9a-f]+$":
+    $ref: snps,dwc3.yaml#
+    description: Required child node
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - ranges
+
+allOf:
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - realtek,rtd1619-dwc3
+    then:
+      properties:
+        realtek,unlink-usb3-port: false
+
+additionalProperties: false
+
+examples:
+  - |
+    usb@98013e00 {
+        compatible = "realtek,rtd1319d-dwc3", "realtek,rtd-dwc3";
+        reg = <0x98013e00 0x200>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges;
+        realtek,disable-usb3-phy;
+
+        usb@98050000 {
+            compatible = "snps,dwc3";
+            reg = <0x98050000 0x9000>;
+            interrupts = <0 94 4>;
+            phys = <&usb2phy &usb3phy>;
+            phy-names = "usb2-phy", "usb3-phy";
+            dr_mode = "otg";
+            usb-role-switch;
+            role-switch-default-mode = "host";
+            snps,dis_u2_susphy_quirk;
+            snps,parkmode-disable-ss-quirk;
+            snps,parkmode-disable-hs-quirk;
+            maximum-speed = "high-speed";
+        };
+    };
-- 
2.34.1

Re: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Thinh Nguyen]

Hi,

On Tue, Aug 01, 2023, Stanley Chang wrote:
> Realtek DHC RTD SoCs integrate dwc3 IP and has some customizations to
> support different generations of SoCs.
> 
> Signed-off-by: Stanley Chang <stanley_chang@realtek.com>
> ---
> v1 to v2 change:
>     Remove the the code about the property realtek,enable-l4icg.
>     Select USB_ROLE_SWITCH in Kconfig.
>     Add dependency OF and ARCH_REALTEK in Kconfig.
> ---
>  drivers/usb/dwc3/Kconfig    |  12 +
>  drivers/usb/dwc3/Makefile   |   1 +
>  drivers/usb/dwc3/dwc3-rtk.c | 499 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 512 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-rtk.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index be954a9abbe0..baa6a38b2f13 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -168,4 +168,16 @@ config USB_DWC3_AM62
>  	  The Designware Core USB3 IP is programmed to operate in
>  	  in USB 2.0 mode only.
>  	  Say 'Y' or 'M' here if you have one such device
> +
> +config USB_DWC3_RTK
> +	tristate "Realtek DWC3 Platform Driver"
> +	depends on OF && ARCH_REALTEK
> +	default USB_DWC3
> +	select USB_ROLE_SWITCH
> +	help
> +	  RTK DHC RTD SoCs with DesignWare Core USB3 IP inside,
> +	  and IP Core configured for USB 2.0 and USB 3.0 in host
> +	  or dual-role mode.
> +	  Say 'Y' or 'M' if you have such device.
> +
>  endif
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 9f66bd82b639..38d19e4369a5 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -54,3 +54,4 @@ obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
>  obj-$(CONFIG_USB_DWC3_QCOM)		+= dwc3-qcom.o
>  obj-$(CONFIG_USB_DWC3_IMX8MP)		+= dwc3-imx8mp.o
>  obj-$(CONFIG_USB_DWC3_XILINX)		+= dwc3-xilinx.o
> +obj-$(CONFIG_USB_DWC3_RTK)		+= dwc3-rtk.o
> diff --git a/drivers/usb/dwc3/dwc3-rtk.c b/drivers/usb/dwc3/dwc3-rtk.c
> new file mode 100644
> index 000000000000..5c2b2f4e422d
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-rtk.c
> @@ -0,0 +1,499 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * dwc3-rtk.c - Realtek DWC3 Specific Glue layer
> + *
> + * Copyright (C) 2023 Realtek Semiconductor Corporation
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/suspend.h>
> +#include <linux/sys_soc.h>
> +#include <linux/usb/otg.h>
> +#include <linux/usb/of.h>
> +#include <linux/usb/role.h>
> +
> +#include "core.h"
> +
> +#define WRAP_CTR_REG  0x0
> +#define DISABLE_MULTI_REQ BIT(1)
> +#define DESC_R2W_MULTI_DISABLE BIT(9)
> +#define FORCE_PIPE3_PHY_STATUS_TO_0 BIT(13)
> +
> +#define WRAP_USB2_PHY_UTMI_REG 0x8
> +#define TXHSVM_EN BIT(3)
> +
> +#define WRAP_PHY_PIPE_REG 0xC
> +#define RESET_DISABLE_PIPE3_P0 BIT(0)
> +#define CLOCK_ENABLE_FOR_PIPE3_PCLK BIT(1)
> +
> +#define WRAP_USB_HMAC_CTR0_REG 0x60
> +#define U3PORT_DIS BIT(8)
> +
> +#define WRAP_USB2_PHY_REG  0x70
> +#define USB2_PHY_EN_PHY_PLL_PORT0 BIT(12)
> +#define USB2_PHY_EN_PHY_PLL_PORT1 BIT(13)
> +#define USB2_PHY_SWITCH_MASK 0x707
> +#define USB2_PHY_SWITCH_DEVICE 0x0
> +#define USB2_PHY_SWITCH_HOST 0x606
> +
> +#define WRAP_APHY_REG 0x128
> +#define USB3_MBIAS_ENABLE BIT(1)
> +
> +#define WRAP_USB_DBUS_PWR_CTRL_REG 0x160
> +#define DBUS_PWR_CTRL_EN BIT(0)
> +
> +struct dwc3_rtk {
> +	struct device *dev;
> +	void __iomem *regs;
> +	size_t regs_size;
> +
> +	struct dwc3 *dwc;
> +
> +	int cur_dr_mode; /* current dr mode */
> +	bool support_drd_mode; /* if support Host/device switch */

I think we may not need this and do away the boolean support_drd_mode.

> +	struct usb_role_switch *role_switch;
> +
> +	bool unlink_usb3_port;
> +	bool disable_usb3_phy;
> +};
> +
> +static void switch_usb2_dr_mode(struct dwc3_rtk *rtk, int dr_mode)
> +{
> +	switch (dr_mode) {
> +	case USB_DR_MODE_PERIPHERAL:
> +		writel(USB2_PHY_SWITCH_DEVICE |
> +			    (~USB2_PHY_SWITCH_MASK &
> +			      readl(rtk->regs + WRAP_USB2_PHY_REG)),
> +			    rtk->regs + WRAP_USB2_PHY_REG);
> +		break;
> +	case USB_DR_MODE_HOST:
> +		writel(USB2_PHY_SWITCH_HOST |
> +			    (~USB2_PHY_SWITCH_MASK &
> +			      readl(rtk->regs + WRAP_USB2_PHY_REG)),
> +			    rtk->regs + WRAP_USB2_PHY_REG);
> +		break;
> +	default:
> +		dev_dbg(rtk->dev, "%s: dr_mode=%d\n", __func__, dr_mode);
> +		break;
> +	}
> +}
> +
> +static void switch_dwc3_dr_mode(struct dwc3_rtk *rtk, int dr_mode)
> +{
> +	if (!rtk->dwc->role_sw)
> +		goto out;
> +
> +	switch (dr_mode) {
> +	case USB_DR_MODE_PERIPHERAL:
> +		usb_role_switch_set_role(rtk->dwc->role_sw, USB_ROLE_DEVICE);
> +		break;
> +	case USB_DR_MODE_HOST:
> +		usb_role_switch_set_role(rtk->dwc->role_sw, USB_ROLE_HOST);
> +		break;
> +	default:
> +		dev_dbg(rtk->dev, "%s dr_mode=%d\n", __func__, dr_mode);
> +		break;
> +	}
> +
> +out:
> +	return;
> +}
> +
> +static int dwc3_rtk_get_dr_mode(struct dwc3_rtk *rtk)
> +{
> +	enum usb_role role;
> +
> +	role = rtk->cur_dr_mode;
> +
> +	if (rtk->dwc && rtk->dwc->role_sw)
> +		role = usb_role_switch_get_role(rtk->dwc->role_sw);
> +	else
> +		dev_dbg(rtk->dev, "%s not usb_role_switch role=%d\n", __func__, role);
> +
> +	return role;
> +}
> +
> +static int dwc3_rtk_set_dr_mode(struct dwc3_rtk *rtk, int dr_mode)

Why return the mode rather than status if the setting? You're not
checking the return of this function in the caller anyway.

> +{
> +	if (!rtk->support_drd_mode)
> +		return rtk->cur_dr_mode;
> +
> +	rtk->cur_dr_mode = dr_mode;
> +
> +	switch_dwc3_dr_mode(rtk, dr_mode);
> +	mdelay(10);
> +	switch_usb2_dr_mode(rtk, dr_mode);
> +
> +	return rtk->cur_dr_mode;
> +}
> +
> +#if IS_ENABLED(CONFIG_USB_ROLE_SWITCH)
> +static int dwc3_usb_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
> +{
> +	struct dwc3_rtk *rtk = usb_role_switch_get_drvdata(sw);
> +
> +	switch (role) {
> +	case USB_ROLE_HOST:
> +		dwc3_rtk_set_dr_mode(rtk, USB_DR_MODE_HOST);
> +		break;
> +	case USB_ROLE_DEVICE:
> +		dwc3_rtk_set_dr_mode(rtk, USB_DR_MODE_PERIPHERAL);
> +		break;
> +	default:
> +		dwc3_rtk_set_dr_mode(rtk, 0);
> +	}
> +
> +	return 0;
> +}
> +
> +static enum usb_role dwc3_usb_role_switch_get(struct usb_role_switch *sw)
> +{
> +	struct dwc3_rtk *rtk = usb_role_switch_get_drvdata(sw);
> +	enum usb_role role = USB_ROLE_NONE;
> +	int dr_mode;
> +
> +	dr_mode = dwc3_rtk_get_dr_mode(rtk);
> +	switch (dr_mode) {
> +	case USB_DR_MODE_HOST:
> +		role = USB_ROLE_HOST;
> +		break;
> +	case USB_DR_MODE_PERIPHERAL:
> +		role = USB_ROLE_DEVICE;
> +		break;
> +	default:
> +		dev_dbg(rtk->dev, "%s dr_mode=%d", __func__, dr_mode);
> +		break;
> +	}
> +	return role;
> +}
> +
> +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk *rtk)

Any reason why we're doing the role switch here and not what's
implemented from the core?

> +{
> +	struct usb_role_switch_desc dwc3_role_switch = {NULL};
> +
> +	dwc3_role_switch.name = strchrnul(dev_name(rtk->dev), '.') + 1;
> +	dwc3_role_switch.driver_data = rtk;
> +	dwc3_role_switch.allow_userspace_control = true;
> +	dwc3_role_switch.fwnode = dev_fwnode(rtk->dev);
> +	dwc3_role_switch.set = dwc3_usb_role_switch_set;
> +	dwc3_role_switch.get = dwc3_usb_role_switch_get;
> +	rtk->role_switch = usb_role_switch_register(rtk->dev, &dwc3_role_switch);
> +	if (IS_ERR(rtk->role_switch))
> +		return PTR_ERR(rtk->role_switch);
> +
> +	return 0;
> +}
> +
> +static int dwc3_rtk_remove_role_switch(struct dwc3_rtk *rtk)
> +{
> +	if (rtk->role_switch)
> +		usb_role_switch_unregister(rtk->role_switch);
> +
> +	rtk->role_switch = NULL;
> +
> +	return 0;
> +}
> +#else
> +#define dwc3_rtk_setup_role_switch(x) 0
> +#define dwc3_rtk_remove_role_switch(x) 0
> +#endif
> +
> +static int dwc3_rtk_init(struct dwc3_rtk *rtk)
> +{
> +	struct device *dev = rtk->dev;
> +	void __iomem *regs = rtk->regs;
> +	const struct soc_device_attribute rtk_soc_kylin_a00[] = {
> +		{ .family = "Realtek Kylin", .revision = "A00", },
> +		{ /* empty */ } };
> +	const struct soc_device_attribute rtk_soc_kylin[] = {
> +		{ .family = "Realtek Kylin", }, { /* empty */ } };
> +	const struct soc_device_attribute rtk_soc_hercules[] = {
> +		{ .family = "Realtek Hercules", }, { /* empty */ } };
> +	const struct soc_device_attribute rtk_soc_hw_clock_gating_quirk[] = {
> +		{ .family = "Realtek Kylin", },
> +		{ .family = "Realtek Hercules", },
> +		{ .family = "Realtek Thor", },
> +		{ .family = "Realtek Hank", },
> +		{ .family = "Realtek Groot", },
> +		{ .family = "Realtek Stark", },
> +		{ /* empty */ } };
> +
> +	if (soc_device_match(rtk_soc_kylin_a00)) {
> +		writel(DISABLE_MULTI_REQ | readl(regs + WRAP_CTR_REG),
> +		       regs + WRAP_CTR_REG);
> +		dev_info(dev, "[bug fixed] 1295/1296 A00: add workaround to disable multiple request for D-Bus");
> +	}
> +
> +	if (soc_device_match(rtk_soc_hercules)) {
> +		writel(USB2_PHY_EN_PHY_PLL_PORT1 |
> +		       readl(regs + WRAP_USB2_PHY_REG),
> +		       regs + WRAP_USB2_PHY_REG);
> +		dev_info(dev, "[bug fixed] 1395 add workaround to disable usb2 port 2 suspend!");
> +	}
> +
> +	writel(TXHSVM_EN | readl(regs + WRAP_USB2_PHY_UTMI_REG),
> +	       regs + WRAP_USB2_PHY_UTMI_REG);
> +
> +	if (rtk->unlink_usb3_port) {
> +		void __iomem *reg;
> +		int val;
> +
> +		reg =  rtk->regs + WRAP_USB_HMAC_CTR0_REG;
> +		val = U3PORT_DIS | readl(reg);
> +		writel(val, reg);
> +	}
> +
> +	if (rtk->disable_usb3_phy) {
> +		void __iomem *reg;
> +		int val;
> +
> +		reg = rtk->regs + WRAP_CTR_REG;
> +		val = FORCE_PIPE3_PHY_STATUS_TO_0 | readl(reg);
> +		writel(val, reg);
> +
> +		reg = rtk->regs + WRAP_PHY_PIPE_REG;
> +		val = ~CLOCK_ENABLE_FOR_PIPE3_PCLK & readl(reg);
> +		val |= RESET_DISABLE_PIPE3_P0;
> +		writel(val, reg);
> +
> +		reg =  rtk->regs + WRAP_USB_HMAC_CTR0_REG;
> +		val = U3PORT_DIS | readl(reg);
> +		writel(val, reg);
> +
> +		reg = rtk->regs + WRAP_APHY_REG;
> +		val = ~USB3_MBIAS_ENABLE & readl(reg);
> +		writel(val, reg);
> +
> +		dev_info(rtk->dev, "%s: disable usb 3.0 phy\n", __func__);
> +	}
> +
> +	if (!soc_device_match(rtk_soc_kylin)) {
> +		void __iomem *reg;
> +		int val;
> +
> +		reg = rtk->regs + WRAP_USB_DBUS_PWR_CTRL_REG;
> +		if (soc_device_match(rtk_soc_hw_clock_gating_quirk))
> +			reg += 0x4;
> +
> +		val = DBUS_PWR_CTRL_EN | readl(reg);
> +		writel(val, reg);
> +	}
> +
> +	writel(DESC_R2W_MULTI_DISABLE | readl(regs + WRAP_CTR_REG),
> +	       regs + WRAP_CTR_REG);
> +
> +	/* Set phy Dp/Dm initial state to host mode to avoid the Dp glitch */
> +	writel(USB2_PHY_SWITCH_HOST |
> +	       (~USB2_PHY_SWITCH_MASK &
> +	       readl(rtk->regs + WRAP_USB2_PHY_REG)),
> +	       rtk->regs + WRAP_USB2_PHY_REG);
> +
> +	return 0;
> +}
> +
> +static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
> +{
> +	struct device *dev = rtk->dev;
> +	struct device_node *node = dev->of_node;
> +	struct platform_device *dwc3_pdev;
> +	struct device *dwc3_dev;
> +	struct device_node *dwc3_node;
> +	int dr_mode;
> +	int ret = 0;
> +
> +	ret = dwc3_rtk_init(rtk);
> +	if (ret)
> +		return -EINVAL;
> +
> +	ret = of_platform_populate(node, NULL, NULL, dev);
> +	if (ret) {
> +		dev_err(dev, "failed to add dwc3 core\n");
> +		return ret;
> +	}
> +
> +	dwc3_node = of_get_compatible_child(node, "snps,dwc3");
> +	if (!dwc3_node) {
> +		dev_err(dev, "failed to find dwc3 core node\n");
> +		return -ENODEV;
> +	}
> +
> +	dwc3_pdev = of_find_device_by_node(dwc3_node);
> +	if (!dwc3_pdev) {
> +		dev_err(dev, "failed to find dwc3 core platform_device\n");
> +		of_node_put(dwc3_node);
> +		return -ENODEV;
> +	}
> +
> +	dwc3_dev = &dwc3_pdev->dev;
> +	rtk->dwc = platform_get_drvdata(dwc3_pdev);
> +	if (!rtk->dwc) {
> +		dev_err(dev, "failed to find dwc3 core\n");
> +		of_node_put(dwc3_node);
> +		return -ENODEV;
> +	}
> +
> +	dr_mode = usb_get_dr_mode(dwc3_dev);
> +	if (dr_mode != rtk->dwc->dr_mode) {
> +		dev_info(dev, "dts set dr_mode=%d, but dwc3 set dr_mode=%d\n",
> +			 dr_mode, rtk->dwc->dr_mode);
> +		dr_mode = rtk->dwc->dr_mode;
> +	}
> +
> +	rtk->cur_dr_mode = dr_mode;
> +
> +	if (device_property_read_bool(dwc3_dev, "usb-role-switch"))
> +		rtk->support_drd_mode = true;
> +	else
> +		rtk->support_drd_mode = false;
> +
> +	if (rtk->support_drd_mode) {
> +		dwc3_rtk_setup_role_switch(rtk);
> +		rtk->cur_dr_mode = dwc3_rtk_get_dr_mode(rtk);
> +	}
> +
> +	switch_usb2_dr_mode(rtk, rtk->cur_dr_mode);
> +
> +	of_node_put(dwc3_node);
> +	platform_device_put(dwc3_pdev);
> +
> +	return ret;
> +}
> +
> +static int dwc3_rtk_probe(struct platform_device *pdev)
> +{
> +	struct dwc3_rtk *rtk;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct resource *res;
> +	void __iomem *regs;
> +	int ret = 0;
> +	unsigned long probe_time = jiffies;
> +
> +	rtk = devm_kzalloc(dev, sizeof(*rtk), GFP_KERNEL);
> +	if (!rtk) {
> +		ret = -ENOMEM;
> +		goto err1;
> +	}
> +
> +	platform_set_drvdata(pdev, rtk);
> +
> +	rtk->dev = dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(dev, "missing memory resource\n");
> +		ret = -ENODEV;
> +		goto err1;
> +	}
> +
> +	regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(regs)) {
> +		ret = PTR_ERR(regs);
> +		goto err1;
> +	}
> +
> +	rtk->regs = regs;
> +	rtk->regs_size = resource_size(res);
> +
> +	rtk->unlink_usb3_port = false;
> +	if (of_property_read_bool(node, "realtek,unlink-usb3-port"))
> +		rtk->unlink_usb3_port = true;
> +
> +	rtk->disable_usb3_phy = false;
> +	if (of_property_read_bool(node, "realtek,disable-usb3-phy"))
> +		rtk->disable_usb3_phy = true;
> +
> +	ret = dwc3_rtk_probe_dwc3_core(rtk);
> +	if (ret) {
> +		dev_err(dev, "%s failed to add dwc3 core\n", __func__);
> +		goto err1;
> +	}
> +
> +	dev_dbg(dev, "%s ok! (take %d ms)\n", __func__,
> +		jiffies_to_msecs(jiffies - probe_time));
> +
> +	return 0;
> +
> +err1:
> +	return ret;
> +}
> +
> +static void dwc3_rtk_remove(struct platform_device *pdev)
> +{
> +	struct dwc3_rtk *rtk = platform_get_drvdata(pdev);
> +
> +	rtk->dwc = NULL;
> +
> +	if (rtk->support_drd_mode)
> +		dwc3_rtk_remove_role_switch(rtk);
> +
> +	of_platform_depopulate(rtk->dev);
> +}
> +
> +static void dwc3_rtk_shutdown(struct platform_device *pdev)
> +{
> +	struct dwc3_rtk *rtk = platform_get_drvdata(pdev);
> +
> +	of_platform_depopulate(rtk->dev);
> +}
> +
> +static const struct of_device_id rtk_dwc3_match[] = {
> +	{ .compatible = "realtek,rtd-dwc3" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rtk_dwc3_match);
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int dwc3_rtk_suspend(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static int dwc3_rtk_resume(struct device *dev)
> +{
> +	struct dwc3_rtk *rtk = dev_get_drvdata(dev);
> +
> +	dwc3_rtk_init(rtk);
> +
> +	switch_usb2_dr_mode(rtk, rtk->cur_dr_mode);
> +
> +	/* runtime set active to reflect active state. */
> +	pm_runtime_disable(dev);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops dwc3_rtk_dev_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_rtk_suspend, dwc3_rtk_resume)
> +};
> +
> +#define DEV_PM_OPS	(&dwc3_rtk_dev_pm_ops)
> +#else
> +#define DEV_PM_OPS	NULL
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static struct platform_driver dwc3_rtk_driver = {
> +	.probe		= dwc3_rtk_probe,
> +	.remove_new	= dwc3_rtk_remove,
> +	.driver		= {
> +		.name	= "rtk-dwc3",
> +		.of_match_table = rtk_dwc3_match,
> +		.pm	= DEV_PM_OPS,
> +	},
> +	.shutdown	= dwc3_rtk_shutdown,
> +};
> +
> +module_platform_driver(dwc3_rtk_driver);
> +
> +MODULE_AUTHOR("Stanley Chang <stanley_chang@realtek.com>");
> +MODULE_DESCRIPTION("DesignWare USB3 Realtek Glue Layer");
> +MODULE_ALIAS("platform:rtk-dwc3");
> +MODULE_LICENSE("GPL");

I'm not familiar with licensing much, but can the SPDX header indicates
different version than the module license?

> +MODULE_SOFTDEP("pre: phy_rtk_usb2 phy_rtk_usb3");
> -- 
> 2.34.1
> 

BR,
Thinh

Re: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Greg Kroah-Hartman]

On Wed, Aug 02, 2023 at 01:14:09AM +0000, Thinh Nguyen wrote:
> > +MODULE_LICENSE("GPL");
> 
> I'm not familiar with licensing much, but can the SPDX header indicates
> different version than the module license?

They match, so I do not understand the issue.  Look at module.h for a
list of what the different strings of MODULE_LICENSE() mean for details.

thanks,

greg k-h

RE: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Stanley Chang[昌育德]]

Hi Thinh,

> > +struct dwc3_rtk {
> > +     struct device *dev;
> > +     void __iomem *regs;
> > +     size_t regs_size;
> > +
> > +     struct dwc3 *dwc;
> > +
> > +     int cur_dr_mode; /* current dr mode */
> > +     bool support_drd_mode; /* if support Host/device switch */
> 
> I think we may not need this and do away the boolean support_drd_mode.

Yes, the initial flow should be simplified as
@@ -346,12 +342,7 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)

        rtk->cur_dr_mode = dr_mode;

-       if (device_property_read_bool(dwc3_dev, "usb-role-switch"))
-               rtk->support_drd_mode = true;
-       else
-               rtk->support_drd_mode = false;
-
-       if (rtk->support_drd_mode) {
+       if (device_property_read_bool(dwc3_dev, "usb-role-switch")) {
                dwc3_rtk_setup_role_switch(rtk);
                rtk->cur_dr_mode = dwc3_rtk_get_dr_mode(rtk);
        }

> > +static int dwc3_rtk_set_dr_mode(struct dwc3_rtk *rtk, int dr_mode)
> 
> Why return the mode rather than status if the setting? You're not checking the
> return of this function in the caller anyway.

You are right, this return value is unnecessary.
I will remove it.

> > +{
> > +     if (!rtk->support_drd_mode)
> > +             return rtk->cur_dr_mode;
> > +
> > +     rtk->cur_dr_mode = dr_mode;
> > +
> > +     switch_dwc3_dr_mode(rtk, dr_mode);
> > +     mdelay(10);
> > +     switch_usb2_dr_mode(rtk, dr_mode);
> > +
> > +     return rtk->cur_dr_mode;
> > +}
> > +

> > +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk *rtk)
> 
> Any reason why we're doing the role switch here and not what's implemented
> from the core?
> 
Because we have to set the usb 2.0 phy mode through switch_usb2_dr_mode in the function dwc3_rtk_set_dr_mode.
In fact, switch_dwc3_dr_mode will use the role switching implemented by core.

> > +
> > +module_platform_driver(dwc3_rtk_driver);
> > +
> > +MODULE_AUTHOR("Stanley Chang <stanley_chang@realtek.com>");
> > +MODULE_DESCRIPTION("DesignWare USB3 Realtek Glue Layer");
> > +MODULE_ALIAS("platform:rtk-dwc3");
> > +MODULE_LICENSE("GPL");
> 
> I'm not familiar with licensing much, but can the SPDX header indicates
> different version than the module license?
> 
Thanks Greg for your comment.
Either GPL or GPL v2 are suitable for our source code.

Thanks,
Stanley

Re: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Thinh Nguyen]

On Wed, Aug 02, 2023, Greg Kroah-Hartman wrote:
> On Wed, Aug 02, 2023 at 01:14:09AM +0000, Thinh Nguyen wrote:
> > > +MODULE_LICENSE("GPL");
> > 
> > I'm not familiar with licensing much, but can the SPDX header indicates
> > different version than the module license?
> 
> They match, so I do not understand the issue.  Look at module.h for a
> list of what the different strings of MODULE_LICENSE() mean for details.
> 

Thanks for the confirmation and pointing where to look.

Thinh

Re: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Thinh Nguyen]

On Wed, Aug 02, 2023, Stanley Chang[昌育德] wrote:
> 
> > > +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk *rtk)
> > 
> > Any reason why we're doing the role switch here and not what's implemented
> > from the core?
> > 
> Because we have to set the usb 2.0 phy mode through switch_usb2_dr_mode in the function dwc3_rtk_set_dr_mode.
> In fact, switch_dwc3_dr_mode will use the role switching implemented by core.
> 

I don't think this is a good way to go about it. Do you (the glue
driver) create a role switch structure and register to the same role
switch device that the dwc3 core would register later? I did not expect
that to work at all.

How about we create a couple ops callbacks to dwc3 structure. One to
initialize gadget and the other for host. Should any platform specific
device need some intialization before initializing the specific role, we
can invoke these callbacks.

e.g.:

struct dwc3_glue_ops {
	int (*init_gadget)(struct dwc3 *dwc);
	int (*init_host)(struct dwc3 *dwc);
}

struct dwc3 {
	..
	const struct dwc3_glue_ops	*ops;
}


static int dwc3_glue_init_gadget(struct dwc3 *dwc)
{
	if (!dwc->ops->init_gadget)
		return 0;

	return dwc->ops->init_gadget(dwc);
}

static int dwc3_glue_init_host(struct dwc3 *dwc)
{
	if (!dwc->ops->init_host)
		return 0;

	return dwc->ops->init_host(dwc);
}


Your glue driver would implement these callbacks. In dwc3_gadget_init()
and dwc3_host_init(), we can call these callbacks at start. I admit that
this may not be the greatest solution since it would require the glue
driver to alloc and register a dwc3 platform device. But I think should
be ok. If anyone else can chime in for more idea, it'd be great.

Thanks,
Thinh

RE: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Stanley Chang[昌育德]]

Hi Thinh,

> > > > +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk *rtk)
> > >
> > > Any reason why we're doing the role switch here and not what's
> > > implemented from the core?
> > >
> > Because we have to set the usb 2.0 phy mode through switch_usb2_dr_mode
> in the function dwc3_rtk_set_dr_mode.
> > In fact, switch_dwc3_dr_mode will use the role switching implemented by
> core.
> >
> 
> I don't think this is a good way to go about it. Do you (the glue
> driver) create a role switch structure and register to the same role switch
> device that the dwc3 core would register later? I did not expect that to work at
> all.
> 
In our application, this role switch in the glue driver works fine.

> How about we create a couple ops callbacks to dwc3 structure. One to
> initialize gadget and the other for host. Should any platform specific device
> need some intialization before initializing the specific role, we can invoke
> these callbacks.
> 
> e.g.:
> 
> struct dwc3_glue_ops {
>         int (*init_gadget)(struct dwc3 *dwc);
>         int (*init_host)(struct dwc3 *dwc); }
> 
> struct dwc3 {
>         ..
>         const struct dwc3_glue_ops      *ops;
> }
> 
> 
> static int dwc3_glue_init_gadget(struct dwc3 *dwc) {
>         if (!dwc->ops->init_gadget)
>                 return 0;
> 
>         return dwc->ops->init_gadget(dwc); }
> 
> static int dwc3_glue_init_host(struct dwc3 *dwc) {
>         if (!dwc->ops->init_host)
>                 return 0;
> 
>         return dwc->ops->init_host(dwc); }
> 
> 
> Your glue driver would implement these callbacks. In dwc3_gadget_init() and
> dwc3_host_init(), we can call these callbacks at start. I admit that this may not
> be the greatest solution since it would require the glue driver to alloc and
> register a dwc3 platform device. But I think should be ok. If anyone else can
> chime in for more idea, it'd be great.


Thanks for your advice.
Here is a solution. As you're concerned, it must register callbacks with the core driver. It looks a bit complicated.
I noticed the phy_set_mode api.
Maybe I can try to move switch_usb2_dr_mode to phy driver.

phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);

Thanks,
Stanley

Re: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Thinh Nguyen]

On Thu, Aug 03, 2023, Stanley Chang[昌育德] wrote:
> Hi Thinh,
> 
> > > > > +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk *rtk)
> > > >
> > > > Any reason why we're doing the role switch here and not what's
> > > > implemented from the core?
> > > >
> > > Because we have to set the usb 2.0 phy mode through switch_usb2_dr_mode
> > in the function dwc3_rtk_set_dr_mode.
> > > In fact, switch_dwc3_dr_mode will use the role switching implemented by
> > core.
> > >
> > 
> > I don't think this is a good way to go about it. Do you (the glue
> > driver) create a role switch structure and register to the same role switch
> > device that the dwc3 core would register later? I did not expect that to work at
> > all.
> > 
> In our application, this role switch in the glue driver works fine.

Is the order of operation between the switch from the core vs the glue
determinate? Also, which operation should happen first? It's not clear
how you handle it here.

> 
> > How about we create a couple ops callbacks to dwc3 structure. One to
> > initialize gadget and the other for host. Should any platform specific device
> > need some intialization before initializing the specific role, we can invoke
> > these callbacks.
> > 
> > e.g.:
> > 
> > struct dwc3_glue_ops {
> >         int (*init_gadget)(struct dwc3 *dwc);
> >         int (*init_host)(struct dwc3 *dwc); }
> > 
> > struct dwc3 {
> >         ..
> >         const struct dwc3_glue_ops      *ops;
> > }
> > 
> > 
> > static int dwc3_glue_init_gadget(struct dwc3 *dwc) {
> >         if (!dwc->ops->init_gadget)
> >                 return 0;
> > 
> >         return dwc->ops->init_gadget(dwc); }
> > 
> > static int dwc3_glue_init_host(struct dwc3 *dwc) {
> >         if (!dwc->ops->init_host)
> >                 return 0;
> > 
> >         return dwc->ops->init_host(dwc); }
> > 
> > 
> > Your glue driver would implement these callbacks. In dwc3_gadget_init() and
> > dwc3_host_init(), we can call these callbacks at start. I admit that this may not
> > be the greatest solution since it would require the glue driver to alloc and
> > register a dwc3 platform device. But I think should be ok. If anyone else can
> > chime in for more idea, it'd be great.
> 
> 
> Thanks for your advice.
> Here is a solution. As you're concerned, it must register callbacks with the core driver. It looks a bit complicated.
> I noticed the phy_set_mode api.
> Maybe I can try to move switch_usb2_dr_mode to phy driver.
> 
> phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
> phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
> 

I'm concern if there are other unwanted side effects this may have to
other platforms.

BR,
Thinh

RE: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Stanley Chang[昌育德]]

Hi Thinh,

> > > > > > +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk *rtk)
> > > > >
> > > > > Any reason why we're doing the role switch here and not what's
> > > > > implemented from the core?
> > > > >
> > > > Because we have to set the usb 2.0 phy mode through
> > > > switch_usb2_dr_mode
> > > in the function dwc3_rtk_set_dr_mode.
> > > > In fact, switch_dwc3_dr_mode will use the role switching
> > > > implemented by
> > > core.
> > > >
> > >
> > > I don't think this is a good way to go about it. Do you (the glue
> > > driver) create a role switch structure and register to the same role
> > > switch device that the dwc3 core would register later? I did not
> > > expect that to work at all.
> > >
> > In our application, this role switch in the glue driver works fine.
> 
> Is the order of operation between the switch from the core vs the glue
> determinate? Also, which operation should happen first? It's not clear how you
> handle it here.

We have a type c driver that first calls role switch in glue.
The role switching function in glue will call the role switching function in core.

Type c driver setting switch role function:
      |----> Set role in the glue
                   |----> Set roles in the core
                   |----> switch_usb2_dr_mode in glue

> >
> > > How about we create a couple ops callbacks to dwc3 structure. One to
> > > initialize gadget and the other for host. Should any platform
> > > specific device need some intialization before initializing the
> > > specific role, we can invoke these callbacks.
> > >
> > > e.g.:
> > >
> > > struct dwc3_glue_ops {
> > >         int (*init_gadget)(struct dwc3 *dwc);
> > >         int (*init_host)(struct dwc3 *dwc); }
> > >
> > > struct dwc3 {
> > >         ..
> > >         const struct dwc3_glue_ops      *ops;
> > > }
> > >
> > >
> > > static int dwc3_glue_init_gadget(struct dwc3 *dwc) {
> > >         if (!dwc->ops->init_gadget)
> > >                 return 0;
> > >
> > >         return dwc->ops->init_gadget(dwc); }
> > >
> > > static int dwc3_glue_init_host(struct dwc3 *dwc) {
> > >         if (!dwc->ops->init_host)
> > >                 return 0;
> > >
> > >         return dwc->ops->init_host(dwc); }
> > >
> > >
> > > Your glue driver would implement these callbacks. In
> > > dwc3_gadget_init() and dwc3_host_init(), we can call these callbacks
> > > at start. I admit that this may not be the greatest solution since
> > > it would require the glue driver to alloc and register a dwc3
> > > platform device. But I think should be ok. If anyone else can chime in for
> more idea, it'd be great.
> >
> >
> > Thanks for your advice.
> > Here is a solution. As you're concerned, it must register callbacks with the
> core driver. It looks a bit complicated.
> > I noticed the phy_set_mode api.
> > Maybe I can try to move switch_usb2_dr_mode to phy driver.
> >
> > phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
> > phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
> >
> 
> I'm concern if there are other unwanted side effects this may have to other
> platforms.


I didn't add any new phy_set_mode to dwc3/core.
I mean use existing callback, as follows
https://elixir.bootlin.com/linux/v6.4.8/source/drivers/usb/dwc3/core.c#L203


Thanks,
Stanley

Re: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Thinh Nguyen]

On Mon, Aug 07, 2023, Stanley Chang[昌育德] wrote:
> Hi Thinh,
> 
> > > > > > > +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk *rtk)
> > > > > >
> > > > > > Any reason why we're doing the role switch here and not what's
> > > > > > implemented from the core?
> > > > > >
> > > > > Because we have to set the usb 2.0 phy mode through
> > > > > switch_usb2_dr_mode
> > > > in the function dwc3_rtk_set_dr_mode.
> > > > > In fact, switch_dwc3_dr_mode will use the role switching
> > > > > implemented by
> > > > core.
> > > > >
> > > >
> > > > I don't think this is a good way to go about it. Do you (the glue
> > > > driver) create a role switch structure and register to the same role
> > > > switch device that the dwc3 core would register later? I did not
> > > > expect that to work at all.
> > > >
> > > In our application, this role switch in the glue driver works fine.
> > 
> > Is the order of operation between the switch from the core vs the glue
> > determinate? Also, which operation should happen first? It's not clear how you
> > handle it here.
> 
> We have a type c driver that first calls role switch in glue.
> The role switching function in glue will call the role switching function in core.

How do we know that? You're registering to the same switch device,
wouldn't the switch event notify the switch work in both the glue and
the core? Perhaps I'm missing something here, but is there some
mechanism that waits for the switch work from the glue to complete
before the core.

> 
> Type c driver setting switch role function:
>       |----> Set role in the glue
>                    |----> Set roles in the core
>                    |----> switch_usb2_dr_mode in glue
> 
> > >
> > > > How about we create a couple ops callbacks to dwc3 structure. One to
> > > > initialize gadget and the other for host. Should any platform
> > > > specific device need some intialization before initializing the
> > > > specific role, we can invoke these callbacks.
> > > >
> > > > e.g.:
> > > >
> > > > struct dwc3_glue_ops {
> > > >         int (*init_gadget)(struct dwc3 *dwc);
> > > >         int (*init_host)(struct dwc3 *dwc); }
> > > >
> > > > struct dwc3 {
> > > >         ..
> > > >         const struct dwc3_glue_ops      *ops;
> > > > }
> > > >
> > > >
> > > > static int dwc3_glue_init_gadget(struct dwc3 *dwc) {
> > > >         if (!dwc->ops->init_gadget)
> > > >                 return 0;
> > > >
> > > >         return dwc->ops->init_gadget(dwc); }
> > > >
> > > > static int dwc3_glue_init_host(struct dwc3 *dwc) {
> > > >         if (!dwc->ops->init_host)
> > > >                 return 0;
> > > >
> > > >         return dwc->ops->init_host(dwc); }
> > > >
> > > >
> > > > Your glue driver would implement these callbacks. In
> > > > dwc3_gadget_init() and dwc3_host_init(), we can call these callbacks
> > > > at start. I admit that this may not be the greatest solution since
> > > > it would require the glue driver to alloc and register a dwc3
> > > > platform device. But I think should be ok. If anyone else can chime in for
> > more idea, it'd be great.
> > >
> > >
> > > Thanks for your advice.
> > > Here is a solution. As you're concerned, it must register callbacks with the
> > core driver. It looks a bit complicated.
> > > I noticed the phy_set_mode api.
> > > Maybe I can try to move switch_usb2_dr_mode to phy driver.
> > >
> > > phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
> > > phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
> > >
> > 
> > I'm concern if there are other unwanted side effects this may have to other
> > platforms.
> 
> 
> I didn't add any new phy_set_mode to dwc3/core.
> I mean use existing callback, as follows
> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.4.8/source/drivers/usb/dwc3/core.c*L203__;Iw!!A4F2R9G_pg!dOK1h9Z03CfmtwWlNePk-TiEdHQjaP8qMmY4BgMfYm0yTi5NThLqK_POOzdM89tSz2zDRkfGG9BSzb78zkLFhttxoefU5w$ 
> 

Ah ok, this seems like a better option.

Thanks,
Thinh

RE: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Stanley Chang[昌育德]]

Hi Thinh,

> On Mon, Aug 07, 2023, Stanley Chang[昌育德] wrote:
> > Hi Thinh,
> >
> > > > > > > > +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk
> > > > > > > > +*rtk)
> > > > > > >
> > > > > > > Any reason why we're doing the role switch here and not
> > > > > > > what's implemented from the core?
> > > > > > >
> > > > > > Because we have to set the usb 2.0 phy mode through
> > > > > > switch_usb2_dr_mode
> > > > > in the function dwc3_rtk_set_dr_mode.
> > > > > > In fact, switch_dwc3_dr_mode will use the role switching
> > > > > > implemented by
> > > > > core.
> > > > > >
> > > > >
> > > > > I don't think this is a good way to go about it. Do you (the
> > > > > glue
> > > > > driver) create a role switch structure and register to the same
> > > > > role switch device that the dwc3 core would register later? I
> > > > > did not expect that to work at all.
> > > > >
> > > > In our application, this role switch in the glue driver works fine.
> > >
> > > Is the order of operation between the switch from the core vs the
> > > glue determinate? Also, which operation should happen first? It's
> > > not clear how you handle it here.
> >
> > We have a type c driver that first calls role switch in glue.
> > The role switching function in glue will call the role switching function in
> core.
> 
> How do we know that? You're registering to the same switch device, wouldn't
> the switch event notify the switch work in both the glue and the core? Perhaps
> I'm missing something here, but is there some mechanism that waits for the
> switch work from the glue to complete before the core.

First, the switches in glue and core are different devices.
In the type-c driver, we only notify the switch in the glue.
Then, the switch in the glue calls the switch in the core to change roles.
This is a sequential process. So there is no order problem.

Thanks,
Stanley

Re: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

[Thinh Nguyen]

On Tue, Aug 08, 2023, Stanley Chang[昌育德] wrote:
> Hi Thinh,
> 
> > On Mon, Aug 07, 2023, Stanley Chang[昌育德] wrote:
> > > Hi Thinh,
> > >
> > > > > > > > > +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk
> > > > > > > > > +*rtk)
> > > > > > > >
> > > > > > > > Any reason why we're doing the role switch here and not
> > > > > > > > what's implemented from the core?
> > > > > > > >
> > > > > > > Because we have to set the usb 2.0 phy mode through
> > > > > > > switch_usb2_dr_mode
> > > > > > in the function dwc3_rtk_set_dr_mode.
> > > > > > > In fact, switch_dwc3_dr_mode will use the role switching
> > > > > > > implemented by
> > > > > > core.
> > > > > > >
> > > > > >
> > > > > > I don't think this is a good way to go about it. Do you (the
> > > > > > glue
> > > > > > driver) create a role switch structure and register to the same
> > > > > > role switch device that the dwc3 core would register later? I
> > > > > > did not expect that to work at all.
> > > > > >
> > > > > In our application, this role switch in the glue driver works fine.
> > > >
> > > > Is the order of operation between the switch from the core vs the
> > > > glue determinate? Also, which operation should happen first? It's
> > > > not clear how you handle it here.
> > >
> > > We have a type c driver that first calls role switch in glue.
> > > The role switching function in glue will call the role switching function in
> > core.
> > 
> > How do we know that? You're registering to the same switch device, wouldn't
> > the switch event notify the switch work in both the glue and the core? Perhaps
> > I'm missing something here, but is there some mechanism that waits for the
> > switch work from the glue to complete before the core.
> 
> First, the switches in glue and core are different devices.
> In the type-c driver, we only notify the switch in the glue.
> Then, the switch in the glue calls the switch in the core to change roles.
> This is a sequential process. So there is no order problem.
> 

I see. That makes sense now. I thought you were trying to register to
the same switch device. I need to look into your implementation in more
detail.

Thanks,
Thinh