devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Add driver support for ESWIN EIC7700 SoC USB controller
@ 2025-09-15  8:53 caohang
  2025-09-15  9:10 ` [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 " caohang
  2025-09-15  9:10 ` [PATCH v3 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver caohang
  0 siblings, 2 replies; 9+ messages in thread
From: caohang @ 2025-09-15  8:53 UTC (permalink / raw)
  To: gregkh, robh, krzk+dt, conor+dt, Thinh.Nguyen, p.zabel,
	linux-kernel, linux-usb, devicetree
  Cc: ningyu, linmin, pinkesh.vaghela, Hang Cao

From: Hang Cao <caohang@eswincomputing.com>

Add support for ESWIN EIC7700 USB driver controller.

Those series of patches depend on config option patch[1]
- [1] https://lore.kernel.org/lkml/20250616112316.3833343-1-pinkesh.vaghela@einfochips.com/

Changes in v3->v2:
- Updates: eswin,eic7700-usb.yaml
  - Sort the attributes according to the DTS coding style.
  - Remove the #address-cells and #size-cells attributes.
  - Fold the child node into the parent.
  - Update commit message.

- Updates: dwc3-eic7700.c
  - Use dwc3 core as a library.
  - Add system and runtime pm.
  - Use pm_ptr and remove the __maybe_unused tags.
  - Add new author name
  - Add prepare and complete function
  - Update commit message.
- Link to V2: https://lore.kernel.org/lkml/20250730073953.1623-1-zhangsenchuan@eswincomputing.com/

Changes in v2->v1:
- Updates: eswin,eic7700-usb.yaml
  - Drop the redundant descriptions.
  - Supplement the constraints of resets.
  - Replace "eswin,hsp_sp_csr" with "eswin,hsp-sp-csr"
    and add items description.
  - Drop numa-node-id, This is not necessary.
  - Add patternProperties and match the rules defined
    in the "snps,dwc3.yaml" file.
  - Add "#address-cells" "#size-cells".
  - Update the space indentation, remove the redundant labels,
    and sort the attributes according to the DTS encoding style.
  - Drop the "status = "disabled" attribute.
  - Update the common usb node names and fold the child
    nodes into the parent nodes.
  - The warning detected by the robot has been resolved.

- Updates: dwc3-eic7700.c
  - Remove dwc3_mode_show dwc3_mode_store dwc3_eswin_get_extcon_dev,
    dwc3_eswin_device_notifier and dwc3_eswin_host_notifier, usb role
    detection and switching are not supported.
  - Remove the hub-rst attribute, remove the dwc3_hub_rst_show and
    dwc3_hub_rst_store functions, this feature is not supported.
  - Use syscon_regmap_lookup_by_phandle_args instead of the
    syscon_regmap_lookup_by_phandle function.
  - Use dev_err_probe in probe function.
  - Drop mutex_lock, which is not required.
  - Remove clk_prepare_enable and of_clk_get, and manage multiple
    clocks using devm_clk_bulk_get_all_enabled.
  - Remove the device_init_wakeup related functions, which were
    used incorrectly.
  - Remove MODULE_ALIAS, which is used incorrectly.
  - The warning detected by the robot has been resolved.
- Link to V1: https://lore.kernel.org/lkml/20250516095237.1516-1-zhangsenchuan@eswincomputing.com/

Hang Cao (2):
  dt-bindings: usb: Add ESWIN EIC7700 USB controller
  usb: dwc3: eic7700: Add EIC7700 USB driver

 .../bindings/usb/eswin,eic7700-usb.yaml       |  99 +++++++
 drivers/usb/dwc3/Kconfig                      |  11 +
 drivers/usb/dwc3/Makefile                     |   1 +
 drivers/usb/dwc3/dwc3-eic7700.c               | 261 ++++++++++++++++++
 4 files changed, 372 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
 create mode 100644 drivers/usb/dwc3/dwc3-eic7700.c

--
2.34.1


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

* [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 USB controller
  2025-09-15  8:53 [PATCH v3 0/2] Add driver support for ESWIN EIC7700 SoC USB controller caohang
@ 2025-09-15  9:10 ` caohang
  2025-09-15 17:34   ` Conor Dooley
  2025-09-15  9:10 ` [PATCH v3 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver caohang
  1 sibling, 1 reply; 9+ messages in thread
From: caohang @ 2025-09-15  9:10 UTC (permalink / raw)
  To: gregkh, robh, krzk+dt, conor+dt, Thinh.Nguyen, p.zabel,
	linux-kernel, linux-usb, devicetree
  Cc: ningyu, linmin, pinkesh.vaghela, Hang Cao, Senchuan Zhang

From: Hang Cao <caohang@eswincomputing.com>

Add Device Tree binding documentation for the ESWIN EIC7700
usb controller module.

Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
Signed-off-by: Hang Cao <caohang@eswincomputing.com>
---
 .../bindings/usb/eswin,eic7700-usb.yaml       | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml

diff --git a/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml b/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
new file mode 100644
index 000000000000..37797b85f417
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/eswin,eic7700-usb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ESWIN EIC7700 SoC Usb Controller
+
+maintainers:
+  - Wei Yang <yangwei1@eswincomputing.com>
+  - Senchuan Zhang <zhangsenchuan@eswincomputing.com>
+  - Hang Cao <caohang@eswincomputing.com>
+
+description:
+  The Usb controller on EIC7700 SoC.
+
+allOf:
+  - $ref: snps,dwc3-common.yaml#
+
+properties:
+  compatible:
+    const: eswin,eic7700-dwc3
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-names:
+    items:
+      - const: peripheral
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: aclk
+      - const: cfg
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    items:
+      - const: vaux
+
+  eswin,hsp-sp-csr:
+    description:
+      HSP CSR is to control and get status of different high-speed peripherals
+      (such as Ethernet, USB, SATA, etc.) via register, which can close
+      module's clock,reset module independently and tune board-level's
+      parameters of PHY, etc.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to HSP Register Controller hsp_sp_csr node.
+          - description: usb bus register offset.
+          - description: axi low power register offset.
+          - description: vbus frequency register offset.
+          - description: mpll register offset.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - interrupt-names
+  - resets
+  - reset-names
+  - eswin,hsp-sp-csr
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    usb@50480000 {
+        compatible = "eswin,eic7700-dwc3";
+        reg = <0x50480000 0x10000>;
+        clocks = <&clock 170>,
+                 <&clock 171>;
+        clock-names = "aclk", "cfg";
+        interrupt-parent = <&plic>;
+        interrupts = <85>;
+        interrupt-names = "peripheral";
+        resets = <&reset 84>;
+        reset-names = "vaux";
+        dr_mode = "peripheral";
+        maximum-speed = "high-speed";
+        phy_type = "utmi";
+        snps,dis_enblslpm_quirk;
+        snps,dis-u2-freeclk-exists-quirk;
+        snps,dis_u2_susphy_quirk;
+        snps,dis-del-phy-power-chg-quirk;
+        snps,parkmode-disable-ss-quirk;
+        eswin,hsp-sp-csr = <&hsp_sp_csr 0x800 0x818 0x83c 0x840>;
+    };
-- 
2.34.1


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

* [PATCH v3 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver
  2025-09-15  8:53 [PATCH v3 0/2] Add driver support for ESWIN EIC7700 SoC USB controller caohang
  2025-09-15  9:10 ` [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 " caohang
@ 2025-09-15  9:10 ` caohang
  2025-09-15 17:31   ` kernel test robot
  2025-09-19 21:14   ` Thinh Nguyen
  1 sibling, 2 replies; 9+ messages in thread
From: caohang @ 2025-09-15  9:10 UTC (permalink / raw)
  To: gregkh, robh, krzk+dt, conor+dt, Thinh.Nguyen, p.zabel,
	linux-kernel, linux-usb, devicetree
  Cc: ningyu, linmin, pinkesh.vaghela, Hang Cao, Senchuan Zhang

From: Hang Cao <caohang@eswincomputing.com>

Add the EIC7700 usb driver, which is responsible for
identifying,configuring and connecting usb devices.

Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
Signed-off-by: Hang Cao <caohang@eswincomputing.com>
---
 drivers/usb/dwc3/Kconfig        |  11 ++
 drivers/usb/dwc3/Makefile       |   1 +
 drivers/usb/dwc3/dwc3-eic7700.c | 261 ++++++++++++++++++++++++++++++++
 3 files changed, 273 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-eic7700.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 310d182e10b5..2ae1817987d2 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -189,4 +189,15 @@ config USB_DWC3_RTK
 	  or dual-role mode.
 	  Say 'Y' or 'M' if you have such device.
 
+config USB_DWC3_EIC7700
+	tristate "Eswin Platforms"
+	depends on ARCH_ESWIN || COMPILE_TEST
+	default USB_DWC3
+	depends on OF
+	help
+	  The usb controller on eic7700 SoC.
+	  support of USB2/3 functionality
+	  in Eswin 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 830e6c9e5fe0..05f582103f8b 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -57,3 +57,4 @@ obj-$(CONFIG_USB_DWC3_IMX8MP)		+= dwc3-imx8mp.o
 obj-$(CONFIG_USB_DWC3_XILINX)		+= dwc3-xilinx.o
 obj-$(CONFIG_USB_DWC3_OCTEON)		+= dwc3-octeon.o
 obj-$(CONFIG_USB_DWC3_RTK)		+= dwc3-rtk.o
+obj-$(CONFIG_USB_DWC3_EIC7700)		+= dwc3-eic7700.o
diff --git a/drivers/usb/dwc3/dwc3-eic7700.c b/drivers/usb/dwc3/dwc3-eic7700.c
new file mode 100644
index 000000000000..7d3d051fd0bb
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-eic7700.c
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ESWIN Specific Glue layer
+ *
+ * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
+ *
+ * Authors: Wei Yang <yangwei1@eswincomputing.com>
+ *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
+ *          Hang Cao <caohang@eswincomputing.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/usb.h>
+
+#include "glue.h"
+
+#define HSP_USB_VBUS_FSEL		0x2a
+#define HSP_USB_MPLL_DEFAULT		0x0
+
+#define HSP_USB_BUS_FILTER_EN		BIT(0)
+#define HSP_USB_BUS_CLKEN_GM		BIT(9)
+#define HSP_USB_BUS_CLKEN_GS		BIT(16)
+#define HSP_USB_BUS_SW_RST		BIT(24)
+#define HSP_USB_BUS_CLK_EN		BIT(28)
+
+#define HSP_USB_AXI_LP_XM_CSYSREQ	BIT(0)
+#define HSP_USB_AXI_LP_XS_CSYSREQ	BIT(16)
+
+struct dwc3_eswin {
+	struct device *dev;
+	struct dwc3 dwc;
+	struct clk_bulk_data *clks;
+	int num_clks;
+	struct reset_control *vaux_rst;
+};
+
+#define to_dwc3_eswin(d) container_of((d), struct dwc3_eswin, dwc)
+
+static int dwc_usb_clk_init(struct device *dev)
+{
+	struct regmap *regmap;
+	u32 hsp_usb_vbus_freq;
+	u32 hsp_usb_axi_lp;
+	u32 hsp_usb_bus;
+	u32 hsp_usb_mpll;
+	u32 args[4];
+
+	regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
+						      "eswin,hsp-sp-csr",
+						      ARRAY_SIZE(args), args);
+	if (IS_ERR(regmap)) {
+		dev_err(dev, "No hsp-sp-csr phandle specified\n");
+		return PTR_ERR(regmap);
+	}
+
+	hsp_usb_bus       = args[0];
+	hsp_usb_axi_lp    = args[1];
+	hsp_usb_vbus_freq = args[2];
+	hsp_usb_mpll      = args[3];
+
+	/*
+	 * usb clock init,ref clock is 24M below need to be set to satisfy usb
+	 * phy requirement(125M)
+	 */
+	regmap_write(regmap, hsp_usb_vbus_freq, HSP_USB_VBUS_FSEL);
+	regmap_write(regmap, hsp_usb_mpll, HSP_USB_MPLL_DEFAULT);
+
+	/* reset usb core and usb phy */
+	regmap_write(regmap, hsp_usb_bus, HSP_USB_BUS_FILTER_EN |
+		     HSP_USB_BUS_CLKEN_GM | HSP_USB_BUS_CLKEN_GS |
+		     HSP_USB_BUS_SW_RST | HSP_USB_BUS_CLK_EN);
+	regmap_write(regmap, hsp_usb_axi_lp, HSP_USB_AXI_LP_XM_CSYSREQ |
+		     HSP_USB_AXI_LP_XS_CSYSREQ);
+
+	return 0;
+}
+
+static int dwc3_eswin_probe(struct platform_device *pdev)
+{
+	struct dwc3_probe_data probe_data = {};
+	struct device *dev = &pdev->dev;
+	struct dwc3_eswin *eswin;
+	struct resource *res;
+	int ret;
+
+	eswin = devm_kzalloc(dev, sizeof(*eswin), GFP_KERNEL);
+	if (!eswin)
+		return -ENOMEM;
+
+	eswin->dev = dev;
+	eswin->num_clks = devm_clk_bulk_get_all_enabled(dev, &eswin->clks);
+	if (eswin->num_clks < 0)
+		return dev_err_probe(dev, eswin->num_clks,
+			"Failed to get usb clocks\n");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return dev_err_probe(dev, -ENODEV,
+			"Missing memory resource\n");
+
+	eswin->vaux_rst = devm_reset_control_get(dev, "vaux");
+	if (IS_ERR(eswin->vaux_rst))
+		return dev_err_probe(dev, PTR_ERR(eswin->vaux_rst),
+		"Failed to get vaux reset\n");
+
+	ret = reset_control_deassert(eswin->vaux_rst);
+	if (ret)
+		return dev_err_probe(dev, ret,
+			"Failed to deassert reset\n");
+
+	ret = dwc_usb_clk_init(dev);
+	if (ret) {
+		dev_err(dev, "Failed to clk init: %d\n", ret);
+		goto reset_assert;
+	}
+
+	eswin->dwc.dev = dev;
+	probe_data.dwc = &eswin->dwc;
+	probe_data.res = res;
+	probe_data.ignore_clocks_and_resets = true;
+	ret = dwc3_core_probe(&probe_data);
+	if (ret) {
+		ret = dev_err_probe(dev, ret, "Failed to register DWC3 Core\n");
+		goto reset_assert;
+	}
+
+	return 0;
+
+reset_assert:
+	reset_control_assert(eswin->vaux_rst);
+
+	return ret;
+}
+
+static void dwc3_eswin_remove(struct platform_device *pdev)
+{
+	struct dwc3 *dwc = platform_get_drvdata(pdev);
+	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
+
+	dwc3_core_remove(&eswin->dwc);
+
+	reset_control_assert(eswin->vaux_rst);
+}
+
+static void dwc3_eswin_complete(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+
+	dwc3_pm_complete(dwc);
+}
+
+static int dwc3_eswin_prepare(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+
+	return dwc3_pm_prepare(dwc);
+}
+
+static int dwc3_eswin_pm_suspend(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
+	int ret;
+
+	ret = dwc3_pm_suspend(&eswin->dwc);
+	if (ret)
+		return ret;
+
+	clk_bulk_disable_unprepare(eswin->num_clks, eswin->clks);
+
+	return 0;
+}
+
+static int dwc3_eswin_pm_resume(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(eswin->num_clks, eswin->clks);
+	if (ret) {
+		dev_err(dev, "Failed to enable clocks: %d\n", ret);
+		return ret;
+	}
+
+	return dwc3_pm_resume(&eswin->dwc);
+}
+
+static int dwc3_eswin_runtime_suspend(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
+	int ret;
+
+	ret = dwc3_runtime_suspend(&eswin->dwc);
+	if (ret)
+		return ret;
+
+	clk_bulk_disable_unprepare(eswin->num_clks, eswin->clks);
+	return 0;
+}
+
+static int dwc3_eswin_runtime_resume(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(eswin->num_clks, eswin->clks);
+	if (ret) {
+		dev_err(dev, "Failed to enable clocks: %d\n", ret);
+		return ret;
+	}
+
+	return dwc3_runtime_resume(&eswin->dwc);
+}
+
+static int dwc3_eswin_runtime_idle(struct device *dev)
+{
+	return dwc3_runtime_idle(dev_get_drvdata(dev));
+}
+
+static const struct dev_pm_ops dwc3_eswin_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(dwc3_eswin_pm_suspend, dwc3_eswin_pm_resume)
+	SET_RUNTIME_PM_OPS(dwc3_eswin_runtime_suspend,
+			   dwc3_eswin_runtime_resume, dwc3_eswin_runtime_idle)
+	.complete = pm_sleep_ptr(dwc3_eswin_complete),
+	.prepare = pm_sleep_ptr(dwc3_eswin_prepare),
+};
+
+static const struct of_device_id eswin_dwc3_match[] = {
+	{ .compatible = "eswin,eic7700-dwc3" },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, eswin_dwc3_match);
+
+static struct platform_driver dwc3_eswin_driver = {
+	.probe = dwc3_eswin_probe,
+	.remove = dwc3_eswin_remove,
+	.driver = {
+		.name = "eic7700-dwc3",
+		.pm	= pm_ptr(&dwc3_eswin_dev_pm_ops),
+		.of_match_table = eswin_dwc3_match,
+	},
+};
+
+module_platform_driver(dwc3_eswin_driver);
+
+MODULE_AUTHOR("Wei Yang <yangwei1@eswincomputing.com");
+MODULE_AUTHOR("Senchuan Zhang <zhangsenchuan@eswincomputing.com");
+MODULE_AUTHOR("Hang Cao <caohang@eswincomputing.com");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("DesignWare USB3 ESWIN Glue Layer");
-- 
2.34.1


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

* Re: [PATCH v3 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver
  2025-09-15  9:10 ` [PATCH v3 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver caohang
@ 2025-09-15 17:31   ` kernel test robot
  2025-09-19 21:14   ` Thinh Nguyen
  1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2025-09-15 17:31 UTC (permalink / raw)
  To: caohang, gregkh, robh, krzk+dt, conor+dt, Thinh.Nguyen, p.zabel,
	linux-kernel, linux-usb, devicetree
  Cc: oe-kbuild-all, ningyu, linmin, pinkesh.vaghela, Hang Cao,
	Senchuan Zhang

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus robh/for-next pza/reset/next linus/master v6.17-rc6 next-20250912]
[cannot apply to pza/imx-drm/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/caohang-eswincomputing-com/dt-bindings-usb-Add-ESWIN-EIC7700-USB-controller/20250915-171407
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20250915091051.2148-1-caohang%40eswincomputing.com
patch subject: [PATCH v3 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver
config: i386-buildonly-randconfig-003-20250915 (https://download.01.org/0day-ci/archive/20250916/202509160138.P7gRM9Bt-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250916/202509160138.P7gRM9Bt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509160138.P7gRM9Bt-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/usb/dwc3/dwc3-eic7700.c:225:12: warning: 'dwc3_eswin_runtime_idle' defined but not used [-Wunused-function]
     225 | static int dwc3_eswin_runtime_idle(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/usb/dwc3/dwc3-eic7700.c:210:12: warning: 'dwc3_eswin_runtime_resume' defined but not used [-Wunused-function]
     210 | static int dwc3_eswin_runtime_resume(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/usb/dwc3/dwc3-eic7700.c:196:12: warning: 'dwc3_eswin_runtime_suspend' defined but not used [-Wunused-function]
     196 | static int dwc3_eswin_runtime_suspend(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/usb/dwc3/dwc3-eic7700.c:181:12: warning: 'dwc3_eswin_pm_resume' defined but not used [-Wunused-function]
     181 | static int dwc3_eswin_pm_resume(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~
>> drivers/usb/dwc3/dwc3-eic7700.c:166:12: warning: 'dwc3_eswin_pm_suspend' defined but not used [-Wunused-function]
     166 | static int dwc3_eswin_pm_suspend(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~~


vim +/dwc3_eswin_runtime_idle +225 drivers/usb/dwc3/dwc3-eic7700.c

   165	
 > 166	static int dwc3_eswin_pm_suspend(struct device *dev)
   167	{
   168		struct dwc3 *dwc = dev_get_drvdata(dev);
   169		struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
   170		int ret;
   171	
   172		ret = dwc3_pm_suspend(&eswin->dwc);
   173		if (ret)
   174			return ret;
   175	
   176		clk_bulk_disable_unprepare(eswin->num_clks, eswin->clks);
   177	
   178		return 0;
   179	}
   180	
 > 181	static int dwc3_eswin_pm_resume(struct device *dev)
   182	{
   183		struct dwc3 *dwc = dev_get_drvdata(dev);
   184		struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
   185		int ret;
   186	
   187		ret = clk_bulk_prepare_enable(eswin->num_clks, eswin->clks);
   188		if (ret) {
   189			dev_err(dev, "Failed to enable clocks: %d\n", ret);
   190			return ret;
   191		}
   192	
   193		return dwc3_pm_resume(&eswin->dwc);
   194	}
   195	
 > 196	static int dwc3_eswin_runtime_suspend(struct device *dev)
   197	{
   198		struct dwc3 *dwc = dev_get_drvdata(dev);
   199		struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
   200		int ret;
   201	
   202		ret = dwc3_runtime_suspend(&eswin->dwc);
   203		if (ret)
   204			return ret;
   205	
   206		clk_bulk_disable_unprepare(eswin->num_clks, eswin->clks);
   207		return 0;
   208	}
   209	
 > 210	static int dwc3_eswin_runtime_resume(struct device *dev)
   211	{
   212		struct dwc3 *dwc = dev_get_drvdata(dev);
   213		struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
   214		int ret;
   215	
   216		ret = clk_bulk_prepare_enable(eswin->num_clks, eswin->clks);
   217		if (ret) {
   218			dev_err(dev, "Failed to enable clocks: %d\n", ret);
   219			return ret;
   220		}
   221	
   222		return dwc3_runtime_resume(&eswin->dwc);
   223	}
   224	
 > 225	static int dwc3_eswin_runtime_idle(struct device *dev)
   226	{
   227		return dwc3_runtime_idle(dev_get_drvdata(dev));
   228	}
   229	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 USB controller
  2025-09-15  9:10 ` [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 " caohang
@ 2025-09-15 17:34   ` Conor Dooley
  2025-09-23  4:40     ` Hang Cao
  0 siblings, 1 reply; 9+ messages in thread
From: Conor Dooley @ 2025-09-15 17:34 UTC (permalink / raw)
  To: caohang
  Cc: gregkh, robh, krzk+dt, conor+dt, Thinh.Nguyen, p.zabel,
	linux-kernel, linux-usb, devicetree, ningyu, linmin,
	pinkesh.vaghela, Senchuan Zhang

[-- Attachment #1: Type: text/plain, Size: 3901 bytes --]

On Mon, Sep 15, 2025 at 05:10:24PM +0800, caohang@eswincomputing.com wrote:
> From: Hang Cao <caohang@eswincomputing.com>
> 
> Add Device Tree binding documentation for the ESWIN EIC7700
> usb controller module.
> 
> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> Signed-off-by: Hang Cao <caohang@eswincomputing.com>
> ---
>  .../bindings/usb/eswin,eic7700-usb.yaml       | 99 +++++++++++++++++++
>  1 file changed, 99 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml b/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
> new file mode 100644
> index 000000000000..37797b85f417
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/eswin,eic7700-usb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ESWIN EIC7700 SoC Usb Controller
> +
> +maintainers:
> +  - Wei Yang <yangwei1@eswincomputing.com>
> +  - Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> +  - Hang Cao <caohang@eswincomputing.com>
> +
> +description:
> +  The Usb controller on EIC7700 SoC.
> +
> +allOf:
> +  - $ref: snps,dwc3-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: eswin,eic7700-dwc3
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-names:
> +    items:
> +      - const: peripheral
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: aclk
> +      - const: cfg
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    items:
> +      - const: vaux
> +
> +  eswin,hsp-sp-csr:
> +    description:
> +      HSP CSR is to control and get status of different high-speed peripherals
> +      (such as Ethernet, USB, SATA, etc.) via register, which can close
> +      module's clock,reset module independently and tune board-level's
> +      parameters of PHY, etc.
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to HSP Register Controller hsp_sp_csr node.
> +          - description: usb bus register offset.
> +          - description: axi low power register offset.
> +          - description: vbus frequency register offset.
> +          - description: mpll register offset.

As I mentioned on the shdci binding patch, I'm not happy with the
justification for this phandle. What exactly is the clock that this
controls and why does it not have a dedicated clock-controller driver
and reset-controller driver?

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interrupt-names
> +  - resets
> +  - reset-names
> +  - eswin,hsp-sp-csr
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    usb@50480000 {
> +        compatible = "eswin,eic7700-dwc3";
> +        reg = <0x50480000 0x10000>;
> +        clocks = <&clock 170>,
> +                 <&clock 171>;
> +        clock-names = "aclk", "cfg";
> +        interrupt-parent = <&plic>;
> +        interrupts = <85>;
> +        interrupt-names = "peripheral";
> +        resets = <&reset 84>;
> +        reset-names = "vaux";
> +        dr_mode = "peripheral";
> +        maximum-speed = "high-speed";
> +        phy_type = "utmi";
> +        snps,dis_enblslpm_quirk;
> +        snps,dis-u2-freeclk-exists-quirk;
> +        snps,dis_u2_susphy_quirk;
> +        snps,dis-del-phy-power-chg-quirk;
> +        snps,parkmode-disable-ss-quirk;
> +        eswin,hsp-sp-csr = <&hsp_sp_csr 0x800 0x818 0x83c 0x840>;
> +    };
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v3 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver
  2025-09-15  9:10 ` [PATCH v3 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver caohang
  2025-09-15 17:31   ` kernel test robot
@ 2025-09-19 21:14   ` Thinh Nguyen
  1 sibling, 0 replies; 9+ messages in thread
From: Thinh Nguyen @ 2025-09-19 21:14 UTC (permalink / raw)
  To: caohang@eswincomputing.com
  Cc: gregkh@linuxfoundation.org, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, Thinh Nguyen, p.zabel@pengutronix.de,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	devicetree@vger.kernel.org, ningyu@eswincomputing.com,
	linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com,
	Senchuan Zhang

On Mon, Sep 15, 2025, caohang@eswincomputing.com wrote:
> From: Hang Cao <caohang@eswincomputing.com>
> 
> Add the EIC7700 usb driver, which is responsible for
> identifying,configuring and connecting usb devices.
> 
> Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> Signed-off-by: Hang Cao <caohang@eswincomputing.com>
> ---
>  drivers/usb/dwc3/Kconfig        |  11 ++
>  drivers/usb/dwc3/Makefile       |   1 +
>  drivers/usb/dwc3/dwc3-eic7700.c | 261 ++++++++++++++++++++++++++++++++
>  3 files changed, 273 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-eic7700.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 310d182e10b5..2ae1817987d2 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -189,4 +189,15 @@ config USB_DWC3_RTK
>  	  or dual-role mode.
>  	  Say 'Y' or 'M' if you have such device.
>  
> +config USB_DWC3_EIC7700
> +	tristate "Eswin Platforms"
> +	depends on ARCH_ESWIN || COMPILE_TEST
> +	default USB_DWC3
> +	depends on OF
> +	help
> +	  The usb controller on eic7700 SoC.
> +	  support of USB2/3 functionality
> +	  in Eswin 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 830e6c9e5fe0..05f582103f8b 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -57,3 +57,4 @@ obj-$(CONFIG_USB_DWC3_IMX8MP)		+= dwc3-imx8mp.o
>  obj-$(CONFIG_USB_DWC3_XILINX)		+= dwc3-xilinx.o
>  obj-$(CONFIG_USB_DWC3_OCTEON)		+= dwc3-octeon.o
>  obj-$(CONFIG_USB_DWC3_RTK)		+= dwc3-rtk.o
> +obj-$(CONFIG_USB_DWC3_EIC7700)		+= dwc3-eic7700.o
> diff --git a/drivers/usb/dwc3/dwc3-eic7700.c b/drivers/usb/dwc3/dwc3-eic7700.c
> new file mode 100644
> index 000000000000..7d3d051fd0bb
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-eic7700.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ESWIN Specific Glue layer
> + *
> + * Copyright 2025, Beijing ESWIN Computing Technology Co., Ltd.
> + *
> + * Authors: Wei Yang <yangwei1@eswincomputing.com>
> + *          Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> + *          Hang Cao <caohang@eswincomputing.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/usb.h>
> +
> +#include "glue.h"
> +
> +#define HSP_USB_VBUS_FSEL		0x2a
> +#define HSP_USB_MPLL_DEFAULT		0x0
> +
> +#define HSP_USB_BUS_FILTER_EN		BIT(0)
> +#define HSP_USB_BUS_CLKEN_GM		BIT(9)
> +#define HSP_USB_BUS_CLKEN_GS		BIT(16)
> +#define HSP_USB_BUS_SW_RST		BIT(24)
> +#define HSP_USB_BUS_CLK_EN		BIT(28)
> +
> +#define HSP_USB_AXI_LP_XM_CSYSREQ	BIT(0)
> +#define HSP_USB_AXI_LP_XS_CSYSREQ	BIT(16)
> +
> +struct dwc3_eswin {
> +	struct device *dev;
> +	struct dwc3 dwc;
> +	struct clk_bulk_data *clks;
> +	int num_clks;
> +	struct reset_control *vaux_rst;
> +};
> +
> +#define to_dwc3_eswin(d) container_of((d), struct dwc3_eswin, dwc)
> +
> +static int dwc_usb_clk_init(struct device *dev)
> +{
> +	struct regmap *regmap;
> +	u32 hsp_usb_vbus_freq;
> +	u32 hsp_usb_axi_lp;
> +	u32 hsp_usb_bus;
> +	u32 hsp_usb_mpll;
> +	u32 args[4];
> +
> +	regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
> +						      "eswin,hsp-sp-csr",
> +						      ARRAY_SIZE(args), args);
> +	if (IS_ERR(regmap)) {
> +		dev_err(dev, "No hsp-sp-csr phandle specified\n");
> +		return PTR_ERR(regmap);
> +	}
> +
> +	hsp_usb_bus       = args[0];
> +	hsp_usb_axi_lp    = args[1];
> +	hsp_usb_vbus_freq = args[2];
> +	hsp_usb_mpll      = args[3];
> +
> +	/*
> +	 * usb clock init,ref clock is 24M below need to be set to satisfy usb
> +	 * phy requirement(125M)
> +	 */
> +	regmap_write(regmap, hsp_usb_vbus_freq, HSP_USB_VBUS_FSEL);
> +	regmap_write(regmap, hsp_usb_mpll, HSP_USB_MPLL_DEFAULT);
> +
> +	/* reset usb core and usb phy */
> +	regmap_write(regmap, hsp_usb_bus, HSP_USB_BUS_FILTER_EN |
> +		     HSP_USB_BUS_CLKEN_GM | HSP_USB_BUS_CLKEN_GS |
> +		     HSP_USB_BUS_SW_RST | HSP_USB_BUS_CLK_EN);
> +	regmap_write(regmap, hsp_usb_axi_lp, HSP_USB_AXI_LP_XM_CSYSREQ |
> +		     HSP_USB_AXI_LP_XS_CSYSREQ);
> +
> +	return 0;
> +}
> +
> +static int dwc3_eswin_probe(struct platform_device *pdev)
> +{
> +	struct dwc3_probe_data probe_data = {};
> +	struct device *dev = &pdev->dev;
> +	struct dwc3_eswin *eswin;
> +	struct resource *res;
> +	int ret;
> +
> +	eswin = devm_kzalloc(dev, sizeof(*eswin), GFP_KERNEL);
> +	if (!eswin)
> +		return -ENOMEM;
> +
> +	eswin->dev = dev;
> +	eswin->num_clks = devm_clk_bulk_get_all_enabled(dev, &eswin->clks);
> +	if (eswin->num_clks < 0)
> +		return dev_err_probe(dev, eswin->num_clks,
> +			"Failed to get usb clocks\n");
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return dev_err_probe(dev, -ENODEV,
> +			"Missing memory resource\n");
> +
> +	eswin->vaux_rst = devm_reset_control_get(dev, "vaux");
> +	if (IS_ERR(eswin->vaux_rst))
> +		return dev_err_probe(dev, PTR_ERR(eswin->vaux_rst),
> +		"Failed to get vaux reset\n");
> +
> +	ret = reset_control_deassert(eswin->vaux_rst);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +			"Failed to deassert reset\n");
> +
> +	ret = dwc_usb_clk_init(dev);
> +	if (ret) {
> +		dev_err(dev, "Failed to clk init: %d\n", ret);
> +		goto reset_assert;
> +	}
> +
> +	eswin->dwc.dev = dev;
> +	probe_data.dwc = &eswin->dwc;
> +	probe_data.res = res;
> +	probe_data.ignore_clocks_and_resets = true;
> +	ret = dwc3_core_probe(&probe_data);
> +	if (ret) {
> +		ret = dev_err_probe(dev, ret, "Failed to register DWC3 Core\n");
> +		goto reset_assert;
> +	}
> +
> +	return 0;
> +
> +reset_assert:
> +	reset_control_assert(eswin->vaux_rst);
> +
> +	return ret;
> +}
> +
> +static void dwc3_eswin_remove(struct platform_device *pdev)
> +{
> +	struct dwc3 *dwc = platform_get_drvdata(pdev);
> +	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
> +
> +	dwc3_core_remove(&eswin->dwc);
> +
> +	reset_control_assert(eswin->vaux_rst);
> +}
> +
> +static void dwc3_eswin_complete(struct device *dev)
> +{
> +	struct dwc3 *dwc = dev_get_drvdata(dev);
> +
> +	dwc3_pm_complete(dwc);
> +}
> +
> +static int dwc3_eswin_prepare(struct device *dev)
> +{
> +	struct dwc3 *dwc = dev_get_drvdata(dev);
> +
> +	return dwc3_pm_prepare(dwc);
> +}
> +
> +static int dwc3_eswin_pm_suspend(struct device *dev)
> +{
> +	struct dwc3 *dwc = dev_get_drvdata(dev);
> +	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
> +	int ret;
> +
> +	ret = dwc3_pm_suspend(&eswin->dwc);
> +	if (ret)
> +		return ret;
> +
> +	clk_bulk_disable_unprepare(eswin->num_clks, eswin->clks);
> +
> +	return 0;
> +}
> +
> +static int dwc3_eswin_pm_resume(struct device *dev)
> +{
> +	struct dwc3 *dwc = dev_get_drvdata(dev);
> +	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(eswin->num_clks, eswin->clks);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable clocks: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return dwc3_pm_resume(&eswin->dwc);
> +}
> +
> +static int dwc3_eswin_runtime_suspend(struct device *dev)
> +{
> +	struct dwc3 *dwc = dev_get_drvdata(dev);
> +	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
> +	int ret;
> +
> +	ret = dwc3_runtime_suspend(&eswin->dwc);
> +	if (ret)
> +		return ret;
> +
> +	clk_bulk_disable_unprepare(eswin->num_clks, eswin->clks);
> +	return 0;
> +}
> +
> +static int dwc3_eswin_runtime_resume(struct device *dev)
> +{
> +	struct dwc3 *dwc = dev_get_drvdata(dev);
> +	struct dwc3_eswin *eswin = to_dwc3_eswin(dwc);
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(eswin->num_clks, eswin->clks);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable clocks: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return dwc3_runtime_resume(&eswin->dwc);
> +}
> +
> +static int dwc3_eswin_runtime_idle(struct device *dev)
> +{
> +	return dwc3_runtime_idle(dev_get_drvdata(dev));
> +}
> +
> +static const struct dev_pm_ops dwc3_eswin_dev_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_eswin_pm_suspend, dwc3_eswin_pm_resume)
> +	SET_RUNTIME_PM_OPS(dwc3_eswin_runtime_suspend,
> +			   dwc3_eswin_runtime_resume, dwc3_eswin_runtime_idle)
> +	.complete = pm_sleep_ptr(dwc3_eswin_complete),
> +	.prepare = pm_sleep_ptr(dwc3_eswin_prepare),
> +};
> +
> +static const struct of_device_id eswin_dwc3_match[] = {
> +	{ .compatible = "eswin,eic7700-dwc3" },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, eswin_dwc3_match);
> +
> +static struct platform_driver dwc3_eswin_driver = {
> +	.probe = dwc3_eswin_probe,
> +	.remove = dwc3_eswin_remove,
> +	.driver = {
> +		.name = "eic7700-dwc3",
> +		.pm	= pm_ptr(&dwc3_eswin_dev_pm_ops),
> +		.of_match_table = eswin_dwc3_match,
> +	},
> +};
> +
> +module_platform_driver(dwc3_eswin_driver);
> +
> +MODULE_AUTHOR("Wei Yang <yangwei1@eswincomputing.com");
> +MODULE_AUTHOR("Senchuan Zhang <zhangsenchuan@eswincomputing.com");
> +MODULE_AUTHOR("Hang Cao <caohang@eswincomputing.com");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("DesignWare USB3 ESWIN Glue Layer");
> -- 
> 2.34.1
> 

Can you look into dwc3_generic_plat[1] and see if you can enhance it to
support your platform? Greg has already picked up those changes[2].

[1] https://lore.kernel.org/linux-usb/20250913-dwc3_generic-v8-0-b50f81f05f95@linux.dev/

[2] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=e0b6dc00c701e600e655417aab1e100b73de821a

Thanks,
Thinh

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

* Re: Re: [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 USB controller
  2025-09-15 17:34   ` Conor Dooley
@ 2025-09-23  4:40     ` Hang Cao
  2025-09-24 19:55       ` Conor Dooley
  0 siblings, 1 reply; 9+ messages in thread
From: Hang Cao @ 2025-09-23  4:40 UTC (permalink / raw)
  To: Conor Dooley
  Cc: gregkh, robh, krzk+dt, conor+dt, Thinh.Nguyen, p.zabel,
	linux-kernel, linux-usb, devicetree, ningyu, linmin,
	pinkesh.vaghela, Senchuan Zhang

[-- Attachment #1: Type: text/plain, Size: 5830 bytes --]

Hi, Conor Dooley
    First of all, thank you very much for your detailed and professional review work. Additionally, we sincerely apologize 
for any confusion caused to you.I will try to explain the content of the design framework.

> > From: Hang Cao <caohang@eswincomputing.com>
> > 
> > Add Device Tree binding documentation for the ESWIN EIC7700
> > usb controller module.
> > 
> > Signed-off-by: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > Signed-off-by: Hang Cao <caohang@eswincomputing.com>
> > ---
> >  .../bindings/usb/eswin,eic7700-usb.yaml       | 99 +++++++++++++++++++
> >  1 file changed, 99 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml b/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
> > new file mode 100644
> > index 000000000000..37797b85f417
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
> > @@ -0,0 +1,99 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/eswin,eic7700-usb.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ESWIN EIC7700 SoC Usb Controller
> > +
> > +maintainers:
> > +  - Wei Yang <yangwei1@eswincomputing.com>
> > +  - Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > +  - Hang Cao <caohang@eswincomputing.com>
> > +
> > +description:
> > +  The Usb controller on EIC7700 SoC.
> > +
> > +allOf:
> > +  - $ref: snps,dwc3-common.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: eswin,eic7700-dwc3
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: peripheral
> > +
> > +  clocks:
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    items:
> > +      - const: aclk
> > +      - const: cfg
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  reset-names:
> > +    items:
> > +      - const: vaux
> > +
> > +  eswin,hsp-sp-csr:
> > +    description:
> > +      HSP CSR is to control and get status of different high-speed peripherals
> > +      (such as Ethernet, USB, SATA, etc.) via register, which can close
> > +      module's clock,reset module independently and tune board-level's
> > +      parameters of PHY, etc.
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +      - items:
> > +          - description: phandle to HSP Register Controller hsp_sp_csr node.
> > +          - description: usb bus register offset.
> > +          - description: axi low power register offset.
> > +          - description: vbus frequency register offset.
> > +          - description: mpll register offset.
> 
> As I mentioned on the shdci binding patch, I'm not happy with the
> justification for this phandle. What exactly is the clock that this
> controls and why does it not have a dedicated clock-controller driver
> and reset-controller driver?
> 
In the current design framework, the clock can be divided into two parts: 
1. The top-clock, which is used to manage and control the clocks of various subsystems (such as HSP, GPU, NPU, etc.); 
2. The subsystem clocks managed independently by each subsystem.
The top-clock is a standard clock design(featuring gate, divider, and mux functions) that has been registered in the 
common clock framework,with a dedicated clock controller driver.

The subsystem clocks managed by subsystems are controlled and configured through the CSR (Control and Status Register) 
of each respective subsystem. For example, the HSP subsystem uses the eswin,hsp-sp-csr. Additionally, this CSR is
 responsible for managing startup functions, performing independent reset of specific modules, and adjusting 
PHY parameters to achieve board-level tuning (for USB/SATA interfaces, etc.).

The top-clock manages the global clocks of subsystems. Taking the HSP subsystem as an example, the top-clock
 configures the hsp_aclk_ctrl and hsp_cfg_ctrl of HSP subsystem only.
In contrast, the subsystem clocks are managed via their own CSRs. For instance, the USB ref clock used in the USB module of 
the HSP subsystem can only be configured through the hsp-csr, and cannot be set via the top-clock controller driver.
As for the reset function, it is not integrated into a dedicated controller driver either, for reasons similar to those of the 
clock management mentioned above.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - interrupts
> > +  - interrupt-names
> > +  - resets
> > +  - reset-names
> > +  - eswin,hsp-sp-csr
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    usb@50480000 {
> > +        compatible = "eswin,eic7700-dwc3";
> > +        reg = <0x50480000 0x10000>;
> > +        clocks = <&clock 170>,
> > +                 <&clock 171>;
> > +        clock-names = "aclk", "cfg";
> > +        interrupt-parent = <&plic>;
> > +        interrupts = <85>;
> > +        interrupt-names = "peripheral";
> > +        resets = <&reset 84>;
> > +        reset-names = "vaux";
> > +        dr_mode = "peripheral";
> > +        maximum-speed = "high-speed";
> > +        phy_type = "utmi";
> > +        snps,dis_enblslpm_quirk;
> > +        snps,dis-u2-freeclk-exists-quirk;
> > +        snps,dis_u2_susphy_quirk;
> > +        snps,dis-del-phy-power-chg-quirk;
> > +        snps,parkmode-disable-ss-quirk;
> > +        eswin,hsp-sp-csr = <&hsp_sp_csr 0x800 0x818 0x83c 0x840>;
> > +    };
> > -- 
> > 2.34.1
> > 

I wonder if this explanation has addressed your doubts. If you have any other questions, please feel free to further communicate
 with us. Thank you a lot.

Best regards,
Hang Cao

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

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

* Re: Re: [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 USB controller
  2025-09-23  4:40     ` Hang Cao
@ 2025-09-24 19:55       ` Conor Dooley
  2025-09-26  8:15         ` Hang Cao
  0 siblings, 1 reply; 9+ messages in thread
From: Conor Dooley @ 2025-09-24 19:55 UTC (permalink / raw)
  To: Hang Cao
  Cc: gregkh, robh, krzk+dt, conor+dt, Thinh.Nguyen, p.zabel,
	linux-kernel, linux-usb, devicetree, ningyu, linmin,
	pinkesh.vaghela, Senchuan Zhang

[-- Attachment #1: Type: text/plain, Size: 3393 bytes --]

On Tue, Sep 23, 2025 at 12:40:46PM +0800, Hang Cao wrote:
> > > From: Hang Cao <caohang@eswincomputing.com>
> > > +  eswin,hsp-sp-csr:
> > > +    description:
> > > +      HSP CSR is to control and get status of different high-speed peripherals
> > > +      (such as Ethernet, USB, SATA, etc.) via register, which can close
> > > +      module's clock,reset module independently and tune board-level's
> > > +      parameters of PHY, etc.
> > > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > +    items:
> > > +      - items:
> > > +          - description: phandle to HSP Register Controller hsp_sp_csr node.
> > > +          - description: usb bus register offset.
> > > +          - description: axi low power register offset.
> > > +          - description: vbus frequency register offset.
> > > +          - description: mpll register offset.
> > 
> > As I mentioned on the shdci binding patch, I'm not happy with the
> > justification for this phandle. What exactly is the clock that this
> > controls and why does it not have a dedicated clock-controller driver
> > and reset-controller driver?
> > 
> In the current design framework, the clock can be divided into two parts: 
> 1. The top-clock, which is used to manage and control the clocks of various subsystems (such as HSP, GPU, NPU, etc.); 
> 2. The subsystem clocks managed independently by each subsystem.
> The top-clock is a standard clock design(featuring gate, divider, and mux functions) that has been registered in the 
> common clock framework,with a dedicated clock controller driver.
> 
> The subsystem clocks managed by subsystems are controlled and configured through the CSR (Control and Status Register) 
> of each respective subsystem. For example, the HSP subsystem uses the eswin,hsp-sp-csr. Additionally, this CSR is
>  responsible for managing startup functions, performing independent reset of specific modules, and adjusting 
> PHY parameters to achieve board-level tuning (for USB/SATA interfaces, etc.).

Unlike the use of the HSP in the sdhci driver, where it appears to be
setting bits that indicate stability (according to your colleague) what
you say here (and what is done in the driver on the reset side in
particular) seems like something that should be handled by a dedicated
driver. "independent reset of specific modules" is the domain of
reset-controller drivers. What are the other modules for which the HSP
has resets? Does it have clocks for other modules too?

> The top-clock manages the global clocks of subsystems. Taking the HSP subsystem as an example, the top-clock
>  configures the hsp_aclk_ctrl and hsp_cfg_ctrl of HSP subsystem only.

> In contrast, the subsystem clocks are managed via their own CSRs. For instance, the USB ref clock used in the USB module of 
> the HSP subsystem can only be configured through the hsp-csr, and cannot be set via the top-clock controller driver.
> As for the reset function, it is not integrated into a dedicated controller driver either, for reasons similar to those of the 
> clock management mentioned above.

That just sounds to me like the hsp-csr needs to become both a
reset-controller and a clock-controller! It's not unusual to have more
than one clock-controller in an device, the top-clock being a
clock-controller does not mean that the HSP also cannot be one.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: Re: Re: [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 USB controller
  2025-09-24 19:55       ` Conor Dooley
@ 2025-09-26  8:15         ` Hang Cao
  0 siblings, 0 replies; 9+ messages in thread
From: Hang Cao @ 2025-09-26  8:15 UTC (permalink / raw)
  To: Conor Dooley
  Cc: gregkh, robh, krzk+dt, conor+dt, Thinh.Nguyen, p.zabel,
	linux-kernel, linux-usb, devicetree, ningyu, linmin,
	pinkesh.vaghela, Senchuan Zhang

[-- Attachment #1: Type: text/plain, Size: 3736 bytes --]

Hi, Conor Dooley
    We got your point, and your suggestions sound very reasonable. I'll coordinate with colleagues 
responsible for clock and reset functions to try and implement dedicated drivers as soon as possible, 
separating this part from hsp-csr.

Best regards,
Hang Cao

> On Tue, Sep 23, 2025 at 12:40:46PM +0800, Hang Cao wrote:
> > > > From: Hang Cao <caohang@eswincomputing.com>
> > > > +  eswin,hsp-sp-csr:
> > > > +    description:
> > > > +      HSP CSR is to control and get status of different high-speed peripherals
> > > > +      (such as Ethernet, USB, SATA, etc.) via register, which can close
> > > > +      module's clock,reset module independently and tune board-level's
> > > > +      parameters of PHY, etc.
> > > > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > +    items:
> > > > +      - items:
> > > > +          - description: phandle to HSP Register Controller hsp_sp_csr node.
> > > > +          - description: usb bus register offset.
> > > > +          - description: axi low power register offset.
> > > > +          - description: vbus frequency register offset.
> > > > +          - description: mpll register offset.
> > > 
> > > As I mentioned on the shdci binding patch, I'm not happy with the
> > > justification for this phandle. What exactly is the clock that this
> > > controls and why does it not have a dedicated clock-controller driver
> > > and reset-controller driver?
> > > 
> > In the current design framework, the clock can be divided into two parts: 
> > 1. The top-clock, which is used to manage and control the clocks of various subsystems (such as HSP, GPU, NPU, etc.); 
> > 2. The subsystem clocks managed independently by each subsystem.
> > The top-clock is a standard clock design(featuring gate, divider, and mux functions) that has been registered in the 
> > common clock framework,with a dedicated clock controller driver.
> > 
> > The subsystem clocks managed by subsystems are controlled and configured through the CSR (Control and Status Register) 
> > of each respective subsystem. For example, the HSP subsystem uses the eswin,hsp-sp-csr. Additionally, this CSR is
> >  responsible for managing startup functions, performing independent reset of specific modules, and adjusting 
> > PHY parameters to achieve board-level tuning (for USB/SATA interfaces, etc.).
> 
> Unlike the use of the HSP in the sdhci driver, where it appears to be
> setting bits that indicate stability (according to your colleague) what
> you say here (and what is done in the driver on the reset side in
> particular) seems like something that should be handled by a dedicated
> driver. "independent reset of specific modules" is the domain of
> reset-controller drivers. What are the other modules for which the HSP
> has resets? Does it have clocks for other modules too?
> 
> > The top-clock manages the global clocks of subsystems. Taking the HSP subsystem as an example, the top-clock
> >  configures the hsp_aclk_ctrl and hsp_cfg_ctrl of HSP subsystem only.
> 
> > In contrast, the subsystem clocks are managed via their own CSRs. For instance, the USB ref clock used in the USB module of 
> > the HSP subsystem can only be configured through the hsp-csr, and cannot be set via the top-clock controller driver.
> > As for the reset function, it is not integrated into a dedicated controller driver either, for reasons similar to those of the 
> > clock management mentioned above.
> 
> That just sounds to me like the hsp-csr needs to become both a
> reset-controller and a clock-controller! It's not unusual to have more
> than one clock-controller in an device, the top-clock being a
> clock-controller does not mean that the HSP also cannot be one.
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 235 bytes --]

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

end of thread, other threads:[~2025-09-26  8:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-15  8:53 [PATCH v3 0/2] Add driver support for ESWIN EIC7700 SoC USB controller caohang
2025-09-15  9:10 ` [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 " caohang
2025-09-15 17:34   ` Conor Dooley
2025-09-23  4:40     ` Hang Cao
2025-09-24 19:55       ` Conor Dooley
2025-09-26  8:15         ` Hang Cao
2025-09-15  9:10 ` [PATCH v3 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver caohang
2025-09-15 17:31   ` kernel test robot
2025-09-19 21:14   ` Thinh Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).