devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Add Google Tensor SoC USB support
@ 2025-10-10 20:16 Roy Luo
  2025-10-10 20:16 ` [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3 Roy Luo
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Roy Luo @ 2025-10-10 20:16 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus
  Cc: Joy Chakraborty, Naveen Kumar, Roy Luo, Badhri Jagan Sridharan,
	linux-phy, devicetree, linux-kernel, linux-usb, linux-arm-kernel,
	linux-samsung-soc

This series introduces USB controller and PHY support for the Google
Tensor G5 SoC (codename: Laguna), a new generation of Google silicon
first launched with Pixel 10 devices.

The Tensor G5 represents a significant architectural overhaul compared
to previous Tensor generations (e.g., gs101), which were based on Samsung
Exynos IP. Although the G5 still utilizes Synopsys IP for the USB
components, the custom top-level integration introduces a completely new
design for clock, reset scheme, register interfaces and programming
sequence, necessitating new drivers and device tree bindings.

The USB subsystem on Tensor G5 integrates a Synopsys DWC3 USB 3.1
DRD-Single Port controller with hibernation support, and a custom PHY
block comprising Synopsys eUSB2 and USB 3.2/DP combo PHYs.

Co-developed-by: Joy Chakraborty <joychakr@google.com>
Signed-off-by: Joy Chakraborty <joychakr@google.com>
Co-developed-by: Naveen Kumar <mnkumar@google.com>
Signed-off-by: Naveen Kumar <mnkumar@google.com>
Signed-off-by: Roy Luo <royluo@google.com>
---
Changes in v3:
- Align binding file name with the compatible string
- Simplify the compatible property in binding to a single const value.
- Add descriptive comments and use item list in binding.
- Rename binding entries for clarity and brevity.
Link to v2: https://lore.kernel.org/linux-usb/20251008060000.3136021-1-royluo@google.com

Changes in v2:
- Reorder patches to present bindings first.
- Update dt binding compatible strings to be SoC-specific (google,gs5-*).
- Better describe the hardware in dt binding commit messages and
  descriptions.
- Adjust PHY driver commit subjects to use correct prefixes ("phy:").
- Move PHY driver from a subdirectory to drivers/phy/.
Link to v1: https://lore.kernel.org/linux-usb/20251006232125.1833979-1-royluo@google.com/
---
Roy Luo (4):
  dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3
  usb: dwc3: Add Google Tensor SoC DWC3 glue driver
  dt-bindings: phy: google: Add Google Tensor G5 USB PHY
  phy: Add Google Tensor SoC USB PHY driver

 .../bindings/phy/google,gs5-usb-phy.yaml      |  88 +++
 .../bindings/usb/google,gs5-dwc3.yaml         | 141 +++++
 drivers/phy/Kconfig                           |  15 +
 drivers/phy/Makefile                          |   1 +
 drivers/phy/phy-google-usb.c                  | 286 +++++++++
 drivers/usb/dwc3/Kconfig                      |  10 +
 drivers/usb/dwc3/Makefile                     |   1 +
 drivers/usb/dwc3/dwc3-google.c                | 597 ++++++++++++++++++
 8 files changed, 1139 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
 create mode 100644 Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
 create mode 100644 drivers/phy/phy-google-usb.c
 create mode 100644 drivers/usb/dwc3/dwc3-google.c


base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
-- 
2.51.0.740.g6adb054d12-goog


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

* [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3
  2025-10-10 20:16 [PATCH v3 0/4] Add Google Tensor SoC USB support Roy Luo
@ 2025-10-10 20:16 ` Roy Luo
  2025-10-11  0:08   ` Krzysztof Kozlowski
  2025-10-10 20:16 ` [PATCH v3 2/4] usb: dwc3: Add Google Tensor SoC DWC3 glue driver Roy Luo
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Roy Luo @ 2025-10-10 20:16 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus
  Cc: Joy Chakraborty, Naveen Kumar, Roy Luo, Badhri Jagan Sridharan,
	linux-phy, devicetree, linux-kernel, linux-usb, linux-arm-kernel,
	linux-samsung-soc

Document the device tree bindings for the DWC3 USB controller found in
Google Tensor SoCs, starting with the G5 generation.

The Tensor G5 silicon represents a complete architectural departure from
previous generations (like gs101), including entirely new clock/reset
schemes, top-level wrapper and register interface. Consequently,
existing Samsung/Exynos DWC3 USB bindings are incompatible, necessitating
this new device tree binding.

The USB controller on Tensor G5 is based on Synopsys DWC3 IP and features
Dual-Role Device single port with hibernation support.

Signed-off-by: Roy Luo <royluo@google.com>
---
 .../bindings/usb/google,gs5-dwc3.yaml         | 141 ++++++++++++++++++
 1 file changed, 141 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml

diff --git a/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
new file mode 100644
index 000000000000..6fadea7f41e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
@@ -0,0 +1,141 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (c) 2025, Google LLC
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/google,gs5-dwc3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Google Tensor Series (G5+) DWC3 USB SoC Controller
+
+maintainers:
+  - Roy Luo <royluo@google.com>
+
+description:
+  Describes the DWC3 USB controller block implemented on Google Tensor SoCs,
+  starting with the G5 generation. Based on Synopsys DWC3 IP, the controller
+  features Dual-Role Device single port with hibernation add-on.
+
+properties:
+  compatible:
+    const: google,gs5-dwc3
+
+  reg:
+    items:
+      - description: Core DWC3 IP registers.
+      - description: USB host controller configuration registers.
+      - description: USB custom interrrupts control registers.
+
+  reg-names:
+    items:
+      - const: dwc3_core
+      - const: host_cfg
+      - const: usbint_cfg
+
+  interrupts:
+    items:
+      - description: Core DWC3 interrupt.
+      - description: High speed power management event for remote wakeup from hibernation.
+      - description: Super speed power management event for remote wakeup from hibernation.
+
+  interrupt-names:
+    items:
+      - const: dwc_usb3
+      - const: hs_pme
+      - const: ss_pme
+
+  clocks:
+    items:
+      - description: Non-sticky module clock.
+      - description: Sticky module clock.
+      - description: USB2 PHY APB clock.
+
+  clock-names:
+    items:
+      - const: non_sticky
+      - const: sticky
+      - const: u2phy_apb
+
+  resets:
+    items:
+      - description: Non-sticky module reset.
+      - description: Sticky module reset.
+      - description: USB2 PHY APB reset.
+      - description: DRD bus reset.
+      - description: Top-level reset.
+
+  reset-names:
+    items:
+      - const: non_sticky
+      - const: sticky
+      - const: u2phy_apb
+      - const: drd_bus
+      - const: top
+
+  power-domains:
+    items:
+      - description: Power switchable domain, the child of top domain.
+          Turning it on puts the controller into full power state,
+          turning it off puts the controller into power gated state.
+      - description: Top domain, the parent of power switchable domain.
+          Turning it on puts the controller into power gated state,
+          turning it off completely shuts off the controller.
+
+  power-domain-names:
+    items:
+      - const: psw
+      - const: top
+
+  iommus:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - power-domains
+  - power-domain-names
+
+allOf:
+  - $ref: snps,dwc3-common.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        usb@c400000 {
+            compatible = "google,gs5-dwc3";
+            reg = <0 0x0c400000  0 0xd060>, <0 0x0c450000 0 0x14>, <0 0x0c450020 0 0x8>;
+            reg-names = "dwc3_core", "host_cfg", "usbint_cfg";
+            interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH 0>,
+                         <GIC_SPI 597 IRQ_TYPE_LEVEL_HIGH 0>,
+                         <GIC_SPI 598 IRQ_TYPE_LEVEL_HIGH 0>;
+            interrupt-names = "dwc_usb3", "hs_pme", "ss_pme";
+            clocks = <&hsion_usbc_non_sticky_clk>,  <&hsion_usbc_sticky_clk>,
+                     <&hsion_u2phy_apb_clk>;
+            clock-names = "non_sticky", "sticky", "u2phy_apb";
+            resets = <&hsion_resets_usbc_non_sticky>, <&hsion_resets_usbc_sticky>,
+                     <&hsion_resets_u2phy_apb>, <&hsion_resets_usb_drd_bus>,
+                     <&hsion_resets_usb_top>;
+            reset-names = "non_sticky", "sticky", "u2phy_apb", "drd_bus", "top";
+            power-domains = <&hsio_n_usb_psw>, <&hsio_n_usb>;
+            power-domain-names = "psw", "top";
+            phys = <&usb_phy 0>;
+            phy-names = "usb2-phy";
+            snps,quirk-frame-length-adjustment = <0x20>;
+            snps,gfladj-refclk-lpm-sel-quirk;
+            snps,incr-burst-type-adjustment = <4>;
+        };
+    };
+...
-- 
2.51.0.740.g6adb054d12-goog


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

* [PATCH v3 2/4] usb: dwc3: Add Google Tensor SoC DWC3 glue driver
  2025-10-10 20:16 [PATCH v3 0/4] Add Google Tensor SoC USB support Roy Luo
  2025-10-10 20:16 ` [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3 Roy Luo
@ 2025-10-10 20:16 ` Roy Luo
  2025-10-15  0:27   ` Thinh Nguyen
  2025-10-10 20:16 ` [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY Roy Luo
  2025-10-10 20:16 ` [PATCH v3 4/4] phy: Add Google Tensor SoC USB PHY driver Roy Luo
  3 siblings, 1 reply; 19+ messages in thread
From: Roy Luo @ 2025-10-10 20:16 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus
  Cc: Joy Chakraborty, Naveen Kumar, Roy Luo, Badhri Jagan Sridharan,
	linux-phy, devicetree, linux-kernel, linux-usb, linux-arm-kernel,
	linux-samsung-soc

Add support for the DWC3 USB controller found on Google Tensor G5.
The controller features dual-role functionality and hibernation.

The primary focus is implementing hibernation support in host mode,
enabling the controller to enter a low-power state (D3). This is
particularly relevant during system power state transition and
runtime power management for power efficiency.
Highlights:
- Align suspend callback with dwc3_suspend_common() for deciding
  between a full teardown and hibernation in host mode.
- Integration with `psw` (power switchable) and `top` power domains,
  managing their states and device links to support hibernation.
- A notifier callback dwc3_google_usb_psw_pd_notifier() for
  `psw` power domain events to manage controller state
  transitions to/from D3.
- Coordination of the `non_sticky` reset during power state
  transitions, asserting it on D3 entry and deasserting on D0 entry
  in hibernation scenario.
- Handling of high-speed and super-speed PME interrupts
  that are generated by remote wakeup during hibernation.

Co-developed-by: Joy Chakraborty <joychakr@google.com>
Signed-off-by: Joy Chakraborty <joychakr@google.com>
Co-developed-by: Naveen Kumar <mnkumar@google.com>
Signed-off-by: Naveen Kumar <mnkumar@google.com>
Signed-off-by: Roy Luo <royluo@google.com>
---
 drivers/usb/dwc3/Kconfig       |  10 +
 drivers/usb/dwc3/Makefile      |   1 +
 drivers/usb/dwc3/dwc3-google.c | 597 +++++++++++++++++++++++++++++++++
 3 files changed, 608 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-google.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 310d182e10b5..467515d5f937 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -189,4 +189,14 @@ config USB_DWC3_RTK
 	  or dual-role mode.
 	  Say 'Y' or 'M' if you have such device.
 
+config USB_DWC3_GOOGLE
+	tristate "Google Platform"
+	depends on OF && COMMON_CLK && RESET_CONTROLLER
+	default n
+	help
+	  Support the DesignWare Core USB3 IP found on Google Tensor
+	  SoCs, starting with the G5 generation. This driver includes
+	  support for hibernation in host mode.
+	  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..a94982630657 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_GOOGLE)		+= dwc3-google.o
diff --git a/drivers/usb/dwc3/dwc3-google.c b/drivers/usb/dwc3/dwc3-google.c
new file mode 100644
index 000000000000..ae4e9ed2a98f
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-google.c
@@ -0,0 +1,597 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dwc3-google.c - Google DWC3 Specific Glue Layer
+ *
+ * Copyright (c) 2025, Google LLC
+ * Author: Roy Luo <royluo@google.com>
+ */
+
+#include <linux/of.h>
+#include <linux/bitfield.h>
+#include <linux/irq.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/pm_domain.h>
+#include <linux/iopoll.h>
+#include "core.h"
+#include "glue.h"
+
+/* HOST CFG registers */
+#define HC_STATUS_OFFSET 0x0
+#define HC_STATUS_CURRENT_POWER_STATE_U2PMU GENMASK(1, 0)
+#define HC_STATUS_CURRENT_POWER_STATE_U3PMU GENMASK(4, 3)
+
+#define HOST_CFG1_OFFSET 0x4
+#define HOST_CFG1_PME_EN BIT(3)
+#define HOST_CFG1_PM_POWER_STATE_REQUEST GENMASK(5, 4)
+#define HOST_CFG1_PM_POWER_STATE_D0 0x0
+#define HOST_CFG1_PM_POWER_STATE_D3 0x3
+
+/* USBINT registers */
+#define USBINT_CFG1_OFFSET 0x0
+#define USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_MSK BIT(2)
+#define USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_MSK BIT(3)
+#define USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_INT_EN BIT(8)
+#define USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_INT_EN BIT(9)
+#define USBINT_CFG1_USBDRD_PME_GEN_U2_INTR_CLR BIT(14)
+#define USBINT_CFG1_USBDRD_PME_GEN_U3_INTR_CLR BIT(15)
+
+#define USBINT_STATUS_OFFSET 0x4
+#define USBINT_STATUS_USBDRD_PME_GEN_U2P_INTR_STS_RAW BIT(2)
+#define USBINT_STATUS_USBDRD_PME_GEN_U3P_INTR_STS_RAW BIT(3)
+
+#define DWC3_GOOGLE_MAX_RESETS	5
+
+struct dwc3_google {
+	struct device		*dev;
+	struct dwc3		dwc;
+	struct clk_bulk_data	*clks;
+	int			num_clks;
+	struct reset_control_bulk_data rsts[DWC3_GOOGLE_MAX_RESETS];
+	int			num_rsts;
+	struct reset_control	*non_sticky_rst;
+	struct device		*usb_psw_pd;
+	struct device_link	*usb_psw_pd_dl;
+	struct notifier_block	usb_psw_pd_nb;
+	struct device		*usb_top_pd;
+	struct device_link	*usb_top_pd_dl;
+	void __iomem		*host_cfg_base;
+	void __iomem		*usbint_cfg_base;
+	int			hs_pme_irq;
+	int			ss_pme_irq;
+	bool			is_hibernation;
+};
+
+#define to_dwc3_google(d) container_of((d), struct dwc3_google, dwc)
+
+static int dwc3_google_rst_init(struct dwc3_google *google)
+{
+	int ret;
+
+	google->num_rsts = 5;
+	google->rsts[0].id = "non_sticky";
+	google->rsts[1].id = "sticky";
+	google->rsts[2].id = "u2phy_apb";
+	google->rsts[3].id = "drd_bus";
+	google->rsts[4].id = "top";
+
+	ret = devm_reset_control_bulk_get_exclusive(google->dev,
+						    google->num_rsts,
+						    google->rsts);
+
+	if (ret < 0)
+		return ret;
+
+	google->non_sticky_rst = google->rsts[0].rstc;
+
+	return 0;
+}
+
+static int dwc3_google_set_pmu_state(struct dwc3_google *google, int state)
+{
+	u32 reg;
+	int ret;
+
+	reg = readl(google->host_cfg_base + HOST_CFG1_OFFSET);
+	reg &= ~HOST_CFG1_PM_POWER_STATE_REQUEST;
+	reg |= (FIELD_PREP(HOST_CFG1_PM_POWER_STATE_REQUEST, state) |
+		HOST_CFG1_PME_EN);
+	writel(reg, google->host_cfg_base + HOST_CFG1_OFFSET);
+
+	ret = readl_poll_timeout(google->host_cfg_base + HC_STATUS_OFFSET, reg,
+				 (FIELD_GET(HC_STATUS_CURRENT_POWER_STATE_U2PMU, reg) == state &&
+				  FIELD_GET(HC_STATUS_CURRENT_POWER_STATE_U3PMU, reg) == state),
+				 10, 10000);
+
+	if (ret)
+		dev_err(google->dev, "failed to set PMU state %d\n", state);
+
+	return ret;
+}
+
+/*
+ * Clear pme interrupts and report their status.
+ * The hardware requires write-1 then write-0 sequence to clear the interrupt bits.
+ */
+static u32 dwc3_google_clear_pme_irqs(struct dwc3_google *google)
+{
+	u32 irq_status, reg_set, reg_clear;
+
+	irq_status = readl(google->usbint_cfg_base + USBINT_STATUS_OFFSET);
+	irq_status &= (USBINT_STATUS_USBDRD_PME_GEN_U2P_INTR_STS_RAW |
+		       USBINT_STATUS_USBDRD_PME_GEN_U3P_INTR_STS_RAW);
+	if (!irq_status)
+		return irq_status;
+
+	reg_set = readl(google->usbint_cfg_base + USBINT_CFG1_OFFSET);
+	reg_clear = reg_set;
+	if (irq_status & USBINT_STATUS_USBDRD_PME_GEN_U2P_INTR_STS_RAW) {
+		reg_set |= USBINT_CFG1_USBDRD_PME_GEN_U2_INTR_CLR;
+		reg_clear &= ~USBINT_CFG1_USBDRD_PME_GEN_U2_INTR_CLR;
+	}
+	if (irq_status & USBINT_STATUS_USBDRD_PME_GEN_U3P_INTR_STS_RAW) {
+		reg_set |= USBINT_CFG1_USBDRD_PME_GEN_U3_INTR_CLR;
+		reg_clear &= ~USBINT_CFG1_USBDRD_PME_GEN_U3_INTR_CLR;
+	}
+
+	writel(reg_set, google->usbint_cfg_base + USBINT_CFG1_OFFSET);
+	writel(reg_clear, google->usbint_cfg_base + USBINT_CFG1_OFFSET);
+
+	return irq_status;
+}
+
+static void dwc3_google_enable_pme_irq(struct dwc3_google *google)
+{
+	u32 reg;
+
+	reg = readl(google->usbint_cfg_base + USBINT_CFG1_OFFSET);
+	reg &= ~(USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_MSK |
+		 USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_MSK);
+	reg |= (USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_INT_EN |
+		USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_INT_EN);
+	writel(reg, google->usbint_cfg_base + USBINT_CFG1_OFFSET);
+
+	enable_irq(google->hs_pme_irq);
+	enable_irq(google->ss_pme_irq);
+	enable_irq_wake(google->hs_pme_irq);
+	enable_irq_wake(google->ss_pme_irq);
+}
+
+static void dwc3_google_disable_pme_irq(struct dwc3_google *google)
+{
+	u32 reg;
+
+	reg = readl(google->usbint_cfg_base + USBINT_CFG1_OFFSET);
+	reg &= ~(USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_INT_EN |
+		 USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_INT_EN);
+	reg |= (USBINT_CFG1_USBDRD_PME_GEN_U2P_INTR_MSK |
+		USBINT_CFG1_USBDRD_PME_GEN_U3P_INTR_MSK);
+	writel(reg, google->usbint_cfg_base + USBINT_CFG1_OFFSET);
+
+	disable_irq_wake(google->hs_pme_irq);
+	disable_irq_wake(google->ss_pme_irq);
+	disable_irq_nosync(google->hs_pme_irq);
+	disable_irq_nosync(google->ss_pme_irq);
+}
+
+static irqreturn_t dwc3_google_resume_irq(int irq, void *data)
+{
+	struct dwc3_google      *google = data;
+	struct dwc3             *dwc = &google->dwc;
+	u32 irq_status, dr_role;
+
+	irq_status = dwc3_google_clear_pme_irqs(google);
+	dr_role = dwc->current_dr_role;
+
+	if (!irq_status || !google->is_hibernation ||
+	    dr_role != DWC3_GCTL_PRTCAP_HOST) {
+		dev_warn(google->dev, "spurious pme irq %d, hibernation %d, dr_role %u\n",
+			 irq, google->is_hibernation, dr_role);
+		return IRQ_HANDLED;
+	}
+
+	if (dwc->xhci)
+		pm_runtime_resume(&dwc->xhci->dev);
+
+	return IRQ_HANDLED;
+}
+
+static int dwc3_google_request_irq(struct dwc3_google *google, struct platform_device *pdev,
+				   const char *irq_name, const char *req_name)
+{
+	int ret;
+	int irq;
+
+	irq = platform_get_irq_byname(pdev, irq_name);
+	if (irq < 0) {
+		dev_err(google->dev, "invalid irq name %s\n", irq_name);
+		return irq;
+	}
+
+	irq_set_status_flags(irq, IRQ_NOAUTOEN);
+	ret = devm_request_threaded_irq(google->dev, irq, NULL,
+					dwc3_google_resume_irq,
+					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+					req_name, google);
+	if (ret < 0) {
+		dev_err(google->dev, "failed to request irq %s\n", req_name);
+		return ret;
+	}
+
+	return irq;
+}
+
+static int dwc3_google_usb_psw_pd_notifier(struct notifier_block *nb, unsigned long action, void *d)
+{
+	struct dwc3_google *google = container_of(nb, struct dwc3_google, usb_psw_pd_nb);
+	int ret;
+
+	if (!google->is_hibernation)
+		return NOTIFY_OK;
+
+	if (action == GENPD_NOTIFY_OFF) {
+		dev_dbg(google->dev, "enter D3 power state\n");
+		dwc3_google_set_pmu_state(google, HOST_CFG1_PM_POWER_STATE_D3);
+		ret = reset_control_assert(google->non_sticky_rst);
+		if (ret)
+			dev_err(google->dev, "non sticky reset assert failed: %d\n", ret);
+	} else if (action == GENPD_NOTIFY_ON) {
+		dev_dbg(google->dev, "enter D0 power state\n");
+		dwc3_google_clear_pme_irqs(google);
+		ret = reset_control_deassert(google->non_sticky_rst);
+		if (ret)
+			dev_err(google->dev, "non sticky reset deassert failed: %d\n", ret);
+		dwc3_google_set_pmu_state(google, HOST_CFG1_PM_POWER_STATE_D0);
+	}
+
+	return NOTIFY_OK;
+}
+
+static void dwc3_google_pm_domain_deinit(struct dwc3_google *google)
+{
+	if (google->usb_top_pd_dl)
+		device_link_del(google->usb_top_pd_dl);
+
+	if (!IS_ERR_OR_NULL(google->usb_top_pd)) {
+		device_set_wakeup_capable(google->usb_top_pd, false);
+		dev_pm_domain_detach(google->usb_top_pd, true);
+	}
+
+	if (google->usb_psw_pd_dl)
+		device_link_del(google->usb_psw_pd_dl);
+
+	if (!IS_ERR_OR_NULL(google->usb_psw_pd)) {
+		dev_pm_genpd_remove_notifier(google->usb_psw_pd);
+		dev_pm_domain_detach(google->usb_psw_pd, true);
+	}
+}
+
+static int dwc3_google_pm_domain_init(struct dwc3_google *google)
+{
+	int ret;
+
+	/*
+	 * Establish PM RUNTIME link between dwc dev and its power domain usb_psw_pd,
+	 * register notifier block to handle hibernation.
+	 */
+	google->usb_psw_pd = dev_pm_domain_attach_by_name(google->dev, "psw");
+	if (IS_ERR_OR_NULL(google->usb_psw_pd)) {
+		dev_err(google->dev, "failed to get usb psw pd");
+		ret = google->usb_psw_pd ? PTR_ERR(google->usb_psw_pd) : -ENODATA;
+		return ret;
+	}
+
+	google->usb_psw_pd_nb.notifier_call = dwc3_google_usb_psw_pd_notifier;
+	ret = dev_pm_genpd_add_notifier(google->usb_psw_pd, &google->usb_psw_pd_nb);
+	if (ret) {
+		dev_err(google->dev, "failed to add usb psw pd notifier");
+		goto err;
+	}
+
+	google->usb_psw_pd_dl = device_link_add(google->dev, google->usb_psw_pd,
+						DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME |
+						DL_FLAG_RPM_ACTIVE);
+	if (!google->usb_psw_pd_dl) {
+		dev_err(google->usb_psw_pd, "failed to add device link");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	/*
+	 * usb_top_pd is the parent power domain of usb_psw_pd. Keeping usb_top_pd on
+	 * while usb_psw_pd is off places the controller in a power-gated state,
+	 * essential for hibernation. Acquire a handle to usb_top_pd and sets it as
+	 * wakeup-capable to allow the domain to be left on during system suspend.
+	 */
+	google->usb_top_pd = dev_pm_domain_attach_by_name(google->dev, "top");
+	if (IS_ERR_OR_NULL(google->usb_top_pd)) {
+		dev_err(google->dev, "failed to get usb top pd");
+		ret = google->usb_top_pd ? PTR_ERR(google->usb_top_pd) : -ENODATA;
+		goto err;
+	}
+	device_set_wakeup_capable(google->usb_top_pd, true);
+
+	google->usb_top_pd_dl = device_link_add(google->dev, google->usb_top_pd,
+						DL_FLAG_STATELESS);
+	if (!google->usb_top_pd_dl) {
+		dev_err(google->usb_top_pd, "failed to add device link");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	return 0;
+
+err:
+	dwc3_google_pm_domain_deinit(google);
+
+	return ret;
+}
+
+static int dwc3_google_probe(struct platform_device *pdev)
+{
+	struct dwc3_probe_data	probe_data = {};
+	struct device		*dev = &pdev->dev;
+	struct dwc3_google	*google;
+	struct resource		*res;
+	int			ret;
+
+	google = devm_kzalloc(&pdev->dev, sizeof(*google), GFP_KERNEL);
+	if (!google)
+		return -ENOMEM;
+
+	google->dev = &pdev->dev;
+
+	ret = dwc3_google_pm_domain_init(google);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "failed to init pdom\n");
+
+	ret = devm_clk_bulk_get_all_enabled(dev, &google->clks);
+	if (ret < 0) {
+		ret = dev_err_probe(&pdev->dev, ret, "failed to get and enable clks\n");
+		goto err_deinit_pdom;
+	}
+	google->num_clks = ret;
+
+	ret = dwc3_google_rst_init(google);
+	if (ret) {
+		ret = dev_err_probe(&pdev->dev, ret, "failed to get resets\n");
+		goto err_deinit_pdom;
+	}
+
+	ret = reset_control_bulk_deassert(google->num_rsts, google->rsts);
+	if (ret) {
+		ret = dev_err_probe(&pdev->dev, ret, "failed to deassert rsts\n");
+		goto err_deinit_pdom;
+	}
+
+	ret = dwc3_google_request_irq(google, pdev, "hs_pme", "USB HS wakeup");
+	if (ret < 0) {
+		ret = dev_err_probe(&pdev->dev, ret, "failed to request hs pme irq");
+		goto err_reset_assert;
+	}
+	google->hs_pme_irq = ret;
+
+	ret = dwc3_google_request_irq(google, pdev, "ss_pme", "USB SS wakeup");
+	if (ret < 0) {
+		ret = dev_err_probe(&pdev->dev, ret, "failed to request ss pme irq");
+		goto err_reset_assert;
+	}
+	google->ss_pme_irq = ret;
+
+	google->host_cfg_base =
+		devm_platform_ioremap_resource_byname(pdev, "host_cfg");
+	if (IS_ERR(google->host_cfg_base)) {
+		ret = dev_err_probe(&pdev->dev, PTR_ERR(google->host_cfg_base),
+				    "invalid host cfg csr\n");
+		goto err_reset_assert;
+	}
+
+	google->usbint_cfg_base =
+		devm_platform_ioremap_resource_byname(pdev, "usbint_cfg");
+	if (IS_ERR(google->usbint_cfg_base)) {
+		ret = dev_err_probe(&pdev->dev, PTR_ERR(google->usbint_cfg_base),
+				    "invalid usbint csr\n");
+		goto err_reset_assert;
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dwc3_core");
+	if (!res) {
+		ret = dev_err_probe(dev, -ENODEV, "invalid dwc3 core memory\n");
+		goto err_reset_assert;
+	}
+
+	device_init_wakeup(dev, true);
+
+	google->dwc.dev = dev;
+	probe_data.dwc = &google->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 err_reset_assert;
+	}
+
+	return 0;
+
+err_reset_assert:
+	reset_control_bulk_assert(google->num_rsts, google->rsts);
+
+err_deinit_pdom:
+	dwc3_google_pm_domain_deinit(google);
+
+	return ret;
+}
+
+static void dwc3_google_remove(struct platform_device *pdev)
+{
+	struct dwc3 *dwc = platform_get_drvdata(pdev);
+	struct dwc3_google *google = to_dwc3_google(dwc);
+
+	dwc3_core_remove(&google->dwc);
+
+	reset_control_bulk_assert(google->num_rsts, google->rsts);
+
+	dwc3_google_pm_domain_deinit(google);
+}
+
+static int dwc3_google_suspend(struct dwc3_google *google, pm_message_t msg)
+{
+	if (pm_runtime_suspended(google->dev))
+		return 0;
+
+	if (google->dwc.current_dr_role == DWC3_GCTL_PRTCAP_HOST) {
+		/*
+		 * Follow dwc3_suspend_common() guidelines for deciding between
+		 * a full teardown and hibernation.
+		 */
+		if (PMSG_IS_AUTO(msg) || device_may_wakeup(google->dev)) {
+			dev_dbg(google->dev, "enter hibernation");
+			pm_runtime_get_sync(google->usb_top_pd);
+			device_wakeup_enable(google->usb_top_pd);
+			dwc3_google_enable_pme_irq(google);
+			google->is_hibernation = true;
+			return 0;
+		}
+	}
+
+	reset_control_bulk_assert(google->num_rsts, google->rsts);
+	clk_bulk_disable_unprepare(google->num_clks, google->clks);
+
+	return 0;
+}
+
+static int dwc3_google_resume(struct dwc3_google *google, pm_message_t msg)
+{
+	int ret;
+
+	if (google->is_hibernation) {
+		dev_dbg(google->dev, "exit hibernation");
+		dwc3_google_disable_pme_irq(google);
+		device_wakeup_disable(google->usb_top_pd);
+		pm_runtime_put_sync(google->usb_top_pd);
+		google->is_hibernation = false;
+		return 0;
+	}
+
+	ret = clk_bulk_prepare_enable(google->num_clks, google->clks);
+	if (ret)
+		return ret;
+
+	ret = reset_control_bulk_deassert(google->num_rsts, google->rsts);
+	if (ret) {
+		clk_bulk_disable_unprepare(google->num_clks, google->clks);
+		return ret;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int dwc3_google_pm_suspend(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	struct dwc3_google *google = to_dwc3_google(dwc);
+	int ret;
+
+	ret = dwc3_pm_suspend(&google->dwc);
+	if (ret)
+		return ret;
+
+	return dwc3_google_suspend(google, PMSG_SUSPEND);
+}
+
+static int dwc3_google_pm_resume(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	struct dwc3_google *google = to_dwc3_google(dwc);
+	int ret;
+
+	ret = dwc3_google_resume(google, PMSG_RESUME);
+	if (ret)
+		return ret;
+
+	return dwc3_pm_resume(&google->dwc);
+}
+
+static void dwc3_google_complete(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+
+	dwc3_pm_complete(dwc);
+}
+
+static int dwc3_google_prepare(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+
+	return dwc3_pm_prepare(dwc);
+}
+#else
+#define dwc3_google_complete NULL
+#define dwc3_google_prepare NULL
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM
+static int dwc3_google_runtime_suspend(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	struct dwc3_google *google = to_dwc3_google(dwc);
+	int ret;
+
+	ret = dwc3_runtime_suspend(&google->dwc);
+	if (ret)
+		return ret;
+
+	return dwc3_google_suspend(google, PMSG_AUTO_SUSPEND);
+}
+
+static int dwc3_google_runtime_resume(struct device *dev)
+{
+	struct dwc3 *dwc = dev_get_drvdata(dev);
+	struct dwc3_google *google = to_dwc3_google(dwc);
+	int ret;
+
+	ret = dwc3_google_resume(google, PMSG_AUTO_RESUME);
+	if (ret)
+		return ret;
+
+	return dwc3_runtime_resume(&google->dwc);
+}
+
+static int dwc3_google_runtime_idle(struct device *dev)
+{
+	return dwc3_runtime_idle(dev_get_drvdata(dev));
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops dwc3_google_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(dwc3_google_pm_suspend, dwc3_google_pm_resume)
+	SET_RUNTIME_PM_OPS(dwc3_google_runtime_suspend, dwc3_google_runtime_resume,
+			   dwc3_google_runtime_idle)
+	.complete = dwc3_google_complete,
+	.prepare = dwc3_google_prepare,
+};
+
+static const struct of_device_id dwc3_google_of_match[] = {
+	{ .compatible = "google,gs5-dwc3" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dwc3_google_of_match);
+
+static struct platform_driver dwc3_google_driver = {
+	.probe		= dwc3_google_probe,
+	.remove		= dwc3_google_remove,
+	.driver		= {
+		.name	= "google-dwc3",
+		.pm	= &dwc3_google_dev_pm_ops,
+		.of_match_table	= dwc3_google_of_match,
+	},
+};
+
+module_platform_driver(dwc3_google_driver);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("DesignWare DWC3 Google Glue Driver");
-- 
2.51.0.740.g6adb054d12-goog


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

* [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY
  2025-10-10 20:16 [PATCH v3 0/4] Add Google Tensor SoC USB support Roy Luo
  2025-10-10 20:16 ` [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3 Roy Luo
  2025-10-10 20:16 ` [PATCH v3 2/4] usb: dwc3: Add Google Tensor SoC DWC3 glue driver Roy Luo
@ 2025-10-10 20:16 ` Roy Luo
  2025-10-11  0:10   ` Krzysztof Kozlowski
  2025-10-10 20:16 ` [PATCH v3 4/4] phy: Add Google Tensor SoC USB PHY driver Roy Luo
  3 siblings, 1 reply; 19+ messages in thread
From: Roy Luo @ 2025-10-10 20:16 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus
  Cc: Joy Chakraborty, Naveen Kumar, Roy Luo, Badhri Jagan Sridharan,
	linux-phy, devicetree, linux-kernel, linux-usb, linux-arm-kernel,
	linux-samsung-soc

Document the device tree bindings for the USB PHY interfaces integrated
with the DWC3 controller on Google Tensor SoCs, starting with G5
generation. The USB PHY on Tensor G5 includes two integrated Synopsys
PHY IPs: the eUSB 2.0 PHY IP and the USB 3.2/DisplayPort combo PHY IP.

Due to a complete architectural overhaul in the Google Tensor G5, the
existing Samsung/Exynos USB PHY binding for older generations of Google
silicons such as gs101 are no longer compatible, necessitating this new
device tree binding.

Signed-off-by: Roy Luo <royluo@google.com>
---
 .../bindings/phy/google,gs5-usb-phy.yaml      | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml b/Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
new file mode 100644
index 000000000000..a40de31ac768
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2025, Google LLC
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/google,gs5-usb-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Google Tensor Series (G5+) USB PHY
+
+maintainers:
+  - Roy Luo <royluo@google.com>
+
+description: |
+  Describes the USB PHY interfaces integrated with the DWC3 USB controller on
+  Google Tensor SoCs, starting with the G5 generation.
+  Two specific PHY IPs from Synopsys are integrated, including eUSB 2.0 PHY IP
+  and USB 3.2/DisplayPort combo PHY IP.
+  The hardware can support three PHY interfaces, which are selected using the
+  first phandle argument in the PHY specifier::
+    0 - USB high-speed.
+    1 - USB super-speed.
+    2 - DisplayPort
+
+properties:
+  compatible:
+    const: google,gs5-usb-phy
+
+  reg:
+    items:
+      - description: USB2 PHY configuration registers.
+      - description: DisplayPort top-level registers.
+      - description: USB top-level configuration registers.
+
+  reg-names:
+    items:
+      - const: u2phy_cfg
+      - const: dp_top
+      - const: usb_top_cfg
+
+  "#phy-cells":
+    const: 1
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  orientation-switch:
+    type: boolean
+    description:
+      Indicates the PHY as a handler of USB Type-C orientation changes
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - "#phy-cells"
+  - clocks
+  - resets
+  - power-domains
+  - orientation-switch
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        usb_phy: usb_phy@c410000 {
+            compatible = "google,gs5-usb-phy";
+            reg = <0 0x0c450014 0 0xc>,
+                  <0 0x0c637000 0 0xa0>,
+                  <0 0x0c45002c 0 0x4>;
+            reg-names = "u2phy_cfg", "dp_top", "usb_top_cfg";
+            #phy-cells = <1>;
+            clocks = <&hsion_usb2_phy_reset_clk>;
+            resets = <&hsion_resets_usb2_phy>;
+            power-domains = <&hsio_n_usb_pd>;
+            orientation-switch;
+        };
+    };
+...
-- 
2.51.0.740.g6adb054d12-goog


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

* [PATCH v3 4/4] phy: Add Google Tensor SoC USB PHY driver
  2025-10-10 20:16 [PATCH v3 0/4] Add Google Tensor SoC USB support Roy Luo
                   ` (2 preceding siblings ...)
  2025-10-10 20:16 ` [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY Roy Luo
@ 2025-10-10 20:16 ` Roy Luo
  3 siblings, 0 replies; 19+ messages in thread
From: Roy Luo @ 2025-10-10 20:16 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus
  Cc: Joy Chakraborty, Naveen Kumar, Roy Luo, Badhri Jagan Sridharan,
	linux-phy, devicetree, linux-kernel, linux-usb, linux-arm-kernel,
	linux-samsung-soc

Support the USB PHY found on Google Tensor G5. This particular USB PHY
supports both high-speed and super-speed operations, and is integrated
with the SNPS DWC3 controller that's also on the SoC.
This initial patch specifically adds functionality for high-speed.

Co-developed-by: Joy Chakraborty <joychakr@google.com>
Signed-off-by: Joy Chakraborty <joychakr@google.com>
Co-developed-by: Naveen Kumar <mnkumar@google.com>
Signed-off-by: Naveen Kumar <mnkumar@google.com>
Signed-off-by: Roy Luo <royluo@google.com>
---
 drivers/phy/Kconfig          |  15 ++
 drivers/phy/Makefile         |   1 +
 drivers/phy/phy-google-usb.c | 286 +++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)
 create mode 100644 drivers/phy/phy-google-usb.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 58c911e1b2d2..a01f91d6e05e 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -101,6 +101,21 @@ config PHY_NXP_PTN3222
 	  schemes. It supports all three USB 2.0 data rates: Low Speed, Full
 	  Speed and High Speed.
 
+config PHY_GOOGLE_USB
+	tristate "Google Tensor SoC USB PHY driver"
+	depends on HAS_IOMEM
+	depends on OF
+	depends on TYPEC
+	depends on USB_DWC3_GOOGLE
+	select GENERIC_PHY
+	default USB_DWC3_GOOGLE
+	help
+	  Enable support for the USB PHY on Google Tensor SoCs, starting with
+	  the G5 generation. This driver provides the PHY interfaces to
+	  interact with the SNPS eUSB2 and USB 3.2/DisplayPort Combo PHY, both
+	  of which are integrated with the DWC3 USB controller.
+	  This driver currently supports USB high-speed.
+
 source "drivers/phy/allwinner/Kconfig"
 source "drivers/phy/amlogic/Kconfig"
 source "drivers/phy/broadcom/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index c670a8dac468..1d7a1331bd19 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_PHY_SNPS_EUSB2)		+= phy-snps-eusb2.o
 obj-$(CONFIG_USB_LGM_PHY)		+= phy-lgm-usb.o
 obj-$(CONFIG_PHY_AIROHA_PCIE)		+= phy-airoha-pcie.o
 obj-$(CONFIG_PHY_NXP_PTN3222)		+= phy-nxp-ptn3222.o
+obj-$(CONFIG_PHY_GOOGLE_USB)		+= phy-google-usb.o
 obj-y					+= allwinner/	\
 					   amlogic/	\
 					   broadcom/	\
diff --git a/drivers/phy/phy-google-usb.c b/drivers/phy/phy-google-usb.c
new file mode 100644
index 000000000000..883abe64300c
--- /dev/null
+++ b/drivers/phy/phy-google-usb.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * phy-google-usb.c - Google USB PHY driver
+ *
+ * Copyright (C) 2025, Google LLC
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/reset.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/mutex.h>
+#include <linux/cleanup.h>
+#include <linux/usb/typec_mux.h>
+
+#define USBCS_USB2PHY_CFG19_OFFSET 0x0
+#define USBCS_USB2PHY_CFG19_PHY_CFG_PLL_FB_DIV GENMASK(19, 8)
+
+#define USBCS_USB2PHY_CFG21_OFFSET 0x8
+#define USBCS_USB2PHY_CFG21_PHY_ENABLE BIT(12)
+#define USBCS_USB2PHY_CFG21_REF_FREQ_SEL GENMASK(15, 13)
+#define USBCS_USB2PHY_CFG21_PHY_TX_DIG_BYPASS_SEL BIT(19)
+
+#define USBCS_PHY_CFG1_OFFSET 0x28
+#define USBCS_PHY_CFG1_SYS_VBUSVALID BIT(17)
+
+#define USBCS_TOP_CTRL_CFG1_OFFSET 0x0
+#define USBCS_TOP_CTRL_CFG1_USB2ONLY_MODE BIT(5)
+
+enum google_usb_phy_id {
+	GOOGLE_USB2_PHY,
+	GOOGLE_USB_PHY_NUM,
+};
+
+struct google_usb_phy_instance {
+	int index;
+	struct phy *phy;
+	struct clk *clk;
+	struct reset_control *rst;
+};
+
+struct google_usb_phy {
+	struct device *dev;
+	void __iomem *u2phy_cfg_base;
+	void __iomem *dp_top_base;
+	void __iomem *usb_top_cfg_base;
+	struct google_usb_phy_instance insts[GOOGLE_USB_PHY_NUM];
+	/* serialize phy access */
+	struct mutex phy_mutex;
+	struct typec_switch_dev *sw;
+	enum typec_orientation orientation;
+};
+
+static inline struct google_usb_phy *to_google_usb_phy(struct google_usb_phy_instance *inst)
+{
+	return container_of(inst, struct google_usb_phy, insts[inst->index]);
+}
+
+static void set_vbus_valid(struct google_usb_phy *gphy)
+{
+	u32 reg;
+
+	if (gphy->orientation == TYPEC_ORIENTATION_NONE) {
+		reg = readl(gphy->dp_top_base + USBCS_PHY_CFG1_OFFSET);
+		reg &= ~USBCS_PHY_CFG1_SYS_VBUSVALID;
+		writel(reg, gphy->dp_top_base + USBCS_PHY_CFG1_OFFSET);
+	} else {
+		reg = readl(gphy->dp_top_base + USBCS_PHY_CFG1_OFFSET);
+		reg |= USBCS_PHY_CFG1_SYS_VBUSVALID;
+		writel(reg, gphy->dp_top_base + USBCS_PHY_CFG1_OFFSET);
+	}
+}
+
+static int google_usb_set_orientation(struct typec_switch_dev *sw,
+				      enum typec_orientation orientation)
+{
+	struct google_usb_phy *gphy = typec_switch_get_drvdata(sw);
+
+	dev_dbg(gphy->dev, "set orientation %d\n", orientation);
+
+	gphy->orientation = orientation;
+
+	if (pm_runtime_suspended(gphy->dev))
+		return 0;
+
+	guard(mutex)(&gphy->phy_mutex);
+
+	set_vbus_valid(gphy);
+
+	return 0;
+}
+
+static int google_usb2_phy_init(struct phy *_phy)
+{
+	struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
+	struct google_usb_phy *gphy = to_google_usb_phy(inst);
+	u32 reg;
+	int ret = 0;
+
+	dev_dbg(gphy->dev, "initializing usb2 phy\n");
+
+	guard(mutex)(&gphy->phy_mutex);
+
+	 /*
+	  * TODO: usb2only mode should be removed once usb3 is supported
+	  */
+	reg = readl(gphy->usb_top_cfg_base + USBCS_TOP_CTRL_CFG1_OFFSET);
+	reg |= USBCS_TOP_CTRL_CFG1_USB2ONLY_MODE;
+	writel(reg, gphy->usb_top_cfg_base + USBCS_TOP_CTRL_CFG1_OFFSET);
+
+	reg = readl(gphy->u2phy_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+	reg &= ~USBCS_USB2PHY_CFG21_PHY_TX_DIG_BYPASS_SEL;
+	reg &= ~USBCS_USB2PHY_CFG21_REF_FREQ_SEL;
+	reg |= FIELD_PREP(USBCS_USB2PHY_CFG21_REF_FREQ_SEL, 0);
+	writel(reg, gphy->u2phy_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+
+	reg = readl(gphy->u2phy_cfg_base + USBCS_USB2PHY_CFG19_OFFSET);
+	reg &= ~USBCS_USB2PHY_CFG19_PHY_CFG_PLL_FB_DIV;
+	reg |= FIELD_PREP(USBCS_USB2PHY_CFG19_PHY_CFG_PLL_FB_DIV, 368);
+	writel(reg, gphy->u2phy_cfg_base + USBCS_USB2PHY_CFG19_OFFSET);
+
+	set_vbus_valid(gphy);
+
+	ret = clk_prepare_enable(inst->clk);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(inst->rst);
+	if (ret) {
+		clk_disable_unprepare(inst->clk);
+		return ret;
+	}
+
+	reg = readl(gphy->u2phy_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+	reg |= USBCS_USB2PHY_CFG21_PHY_ENABLE;
+	writel(reg, gphy->u2phy_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+
+	return ret;
+}
+
+static int google_usb2_phy_exit(struct phy *_phy)
+{
+	struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
+	struct google_usb_phy *gphy = to_google_usb_phy(inst);
+	u32 reg;
+
+	dev_dbg(gphy->dev, "exiting usb2 phy\n");
+
+	guard(mutex)(&gphy->phy_mutex);
+
+	reg = readl(gphy->u2phy_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+	reg &= ~USBCS_USB2PHY_CFG21_PHY_ENABLE;
+	writel(reg, gphy->u2phy_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+
+	reset_control_assert(inst->rst);
+	clk_disable_unprepare(inst->clk);
+
+	return 0;
+}
+
+static const struct phy_ops google_usb2_phy_ops = {
+	.init		= google_usb2_phy_init,
+	.exit		= google_usb2_phy_exit,
+};
+
+static struct phy *google_usb_phy_xlate(struct device *dev,
+					const struct of_phandle_args *args)
+{
+	struct google_usb_phy *gphy = dev_get_drvdata(dev);
+
+	if (args->args[0] >= GOOGLE_USB_PHY_NUM) {
+		dev_err(dev, "invalid PHY index requested from DT\n");
+		return ERR_PTR(-ENODEV);
+	}
+	return gphy->insts[args->args[0]].phy;
+}
+
+static int google_usb_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct google_usb_phy *gphy;
+	struct phy *phy;
+	struct google_usb_phy_instance *inst;
+	struct phy_provider *phy_provider;
+	struct typec_switch_desc sw_desc = { };
+	int ret;
+
+	gphy = devm_kzalloc(dev, sizeof(*gphy), GFP_KERNEL);
+	if (!gphy)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, gphy);
+	gphy->dev = dev;
+
+	ret = devm_mutex_init(dev, &gphy->phy_mutex);
+	if (ret)
+		return ret;
+
+	gphy->u2phy_cfg_base = devm_platform_ioremap_resource_byname(pdev,
+								     "u2phy_cfg");
+	if (IS_ERR(gphy->u2phy_cfg_base))
+		return dev_err_probe(dev, PTR_ERR(gphy->u2phy_cfg_base),
+				    "invalid usb2 cfg csr\n");
+
+	gphy->dp_top_base = devm_platform_ioremap_resource_byname(pdev,
+								  "dp_top");
+	if (IS_ERR(gphy->dp_top_base))
+		return dev_err_probe(dev, PTR_ERR(gphy->dp_top_base),
+				    "invalid dp top csr\n");
+
+	gphy->usb_top_cfg_base = devm_platform_ioremap_resource_byname(pdev,
+								       "usb_top_cfg");
+	if (IS_ERR(gphy->usb_top_cfg_base))
+		return dev_err_probe(dev, PTR_ERR(gphy->usb_top_cfg_base),
+				    "invalid usb top cfg csr\n");
+
+	inst = &gphy->insts[GOOGLE_USB2_PHY];
+	inst->index = GOOGLE_USB2_PHY;
+	phy = devm_phy_create(dev, NULL, &google_usb2_phy_ops);
+	if (IS_ERR(phy))
+		return dev_err_probe(dev, PTR_ERR(phy),
+				     "failed to create usb2 phy instance\n");
+	inst->phy = phy;
+	phy_set_drvdata(phy, inst);
+	inst->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(inst->clk))
+		return dev_err_probe(dev, PTR_ERR(inst->clk),
+				     "failed to get usb2 phy clk\n");
+	inst->rst = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(inst->rst))
+		return dev_err_probe(dev, PTR_ERR(inst->rst),
+				     "failed to get usb2 phy reset\n");
+
+	phy_provider = devm_of_phy_provider_register(dev, google_usb_phy_xlate);
+	if (IS_ERR(phy_provider))
+		return dev_err_probe(dev, PTR_ERR(phy_provider),
+				     "failed to register phy provider\n");
+
+	pm_runtime_enable(dev);
+
+	sw_desc.fwnode = dev_fwnode(dev);
+	sw_desc.drvdata = gphy;
+	sw_desc.name = fwnode_get_name(dev_fwnode(dev));
+	sw_desc.set = google_usb_set_orientation;
+
+	gphy->sw = typec_switch_register(dev, &sw_desc);
+	if (IS_ERR(gphy->sw))
+		return dev_err_probe(dev, PTR_ERR(gphy->sw),
+				     "failed to register typec switch\n");
+
+	return 0;
+}
+
+static void google_usb_phy_remove(struct platform_device *pdev)
+{
+	struct google_usb_phy *gphy = dev_get_drvdata(&pdev->dev);
+
+	typec_switch_unregister(gphy->sw);
+	pm_runtime_disable(&pdev->dev);
+}
+
+static const struct of_device_id google_usb_phy_of_match[] = {
+	{
+		.compatible = "google,gs5-usb-phy",
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, google_usb_phy_of_match);
+
+static struct platform_driver google_usb_phy = {
+	.probe	= google_usb_phy_probe,
+	.remove = google_usb_phy_remove,
+	.driver = {
+		.name		= "google-usb-phy",
+		.of_match_table	= google_usb_phy_of_match,
+	}
+};
+
+module_platform_driver(google_usb_phy);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Google USB phy driver");
-- 
2.51.0.740.g6adb054d12-goog


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

* Re: [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3
  2025-10-10 20:16 ` [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3 Roy Luo
@ 2025-10-11  0:08   ` Krzysztof Kozlowski
  2025-10-14  1:40     ` Roy Luo
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-11  0:08 UTC (permalink / raw)
  To: Roy Luo, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus
  Cc: Joy Chakraborty, Naveen Kumar, Badhri Jagan Sridharan, linux-phy,
	devicetree, linux-kernel, linux-usb, linux-arm-kernel,
	linux-samsung-soc

On 10/10/2025 22:16, Roy Luo wrote:
> Document the device tree bindings for the DWC3 USB controller found in
> Google Tensor SoCs, starting with the G5 generation.
> 
> The Tensor G5 silicon represents a complete architectural departure from
> previous generations (like gs101), including entirely new clock/reset
> schemes, top-level wrapper and register interface. Consequently,
> existing Samsung/Exynos DWC3 USB bindings are incompatible, necessitating
> this new device tree binding.
> 
> The USB controller on Tensor G5 is based on Synopsys DWC3 IP and features
> Dual-Role Device single port with hibernation support.

You still mix, completely unnecessarily, subsystems. For Greg this is
actually even undesired, but regardless don't do this for any cases
because it just makes everything slower or more difficult to apply.

Really, think how maintainers should deal with your patches.

> 
> Signed-off-by: Roy Luo <royluo@google.com>
> ---
>  .../bindings/usb/google,gs5-dwc3.yaml         | 141 ++++++++++++++++++
>  1 file changed, 141 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> new file mode 100644
> index 000000000000..6fadea7f41e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> @@ -0,0 +1,141 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (c) 2025, Google LLC
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/google,gs5-dwc3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Google Tensor Series (G5+) DWC3 USB SoC Controller
> +
> +maintainers:
> +  - Roy Luo <royluo@google.com>
> +
> +description:
> +  Describes the DWC3 USB controller block implemented on Google Tensor SoCs,
> +  starting with the G5 generation. Based on Synopsys DWC3 IP, the controller
> +  features Dual-Role Device single port with hibernation add-on.
> +
> +properties:
> +  compatible:
> +    const: google,gs5-dwc3
> +
> +  reg:
> +    items:
> +      - description: Core DWC3 IP registers.
> +      - description: USB host controller configuration registers.
> +      - description: USB custom interrrupts control registers.
> +
> +  reg-names:
> +    items:
> +      - const: dwc3_core
> +      - const: host_cfg
> +      - const: usbint_cfg
> +
> +  interrupts:
> +    items:
> +      - description: Core DWC3 interrupt.
> +      - description: High speed power management event for remote wakeup from hibernation.
> +      - description: Super speed power management event for remote wakeup from hibernation.

Wrap at 80 (see coding style) or just shorten these.

> +
> +  interrupt-names:
> +    items:
> +      - const: dwc_usb3

So just "core"?

> +      - const: hs_pme
> +      - const: ss_pme
> +
> +  clocks:
> +    items:
> +      - description: Non-sticky module clock.
> +      - description: Sticky module clock.
> +      - description: USB2 PHY APB clock.

This looks wrong. This is not the USB2 phy, so how can it consume APB clock?

> +
> +  clock-names:
> +    items:
> +      - const: non_sticky
> +      - const: sticky
> +      - const: u2phy_apb
> +
> +  resets:
> +    items:
> +      - description: Non-sticky module reset.
> +      - description: Sticky module reset.
> +      - description: USB2 PHY APB reset.

This as well.

> +      - description: DRD bus reset.
> +      - description: Top-level reset.
> +
> +  reset-names:
> +    items:
> +      - const: non_sticky
> +      - const: sticky
> +      - const: u2phy_apb
> +      - const: drd_bus
> +      - const: top


Best regards,
Krzysztof

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

* Re: [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY
  2025-10-10 20:16 ` [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY Roy Luo
@ 2025-10-11  0:10   ` Krzysztof Kozlowski
  2025-10-14  1:46     ` Roy Luo
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-11  0:10 UTC (permalink / raw)
  To: Roy Luo, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus
  Cc: Joy Chakraborty, Naveen Kumar, Badhri Jagan Sridharan, linux-phy,
	devicetree, linux-kernel, linux-usb, linux-arm-kernel,
	linux-samsung-soc

On 10/10/2025 22:16, Roy Luo wrote:
> +  reg:
> +    items:
> +      - description: USB2 PHY configuration registers.
> +      - description: DisplayPort top-level registers.
> +      - description: USB top-level configuration registers.
> +
> +  reg-names:
> +    items:
> +      - const: u2phy_cfg
> +      - const: dp_top
> +      - const: usb_top_cfg
> +
> +  "#phy-cells":
> +    const: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  orientation-switch:
> +    type: boolean
> +    description:
> +      Indicates the PHY as a handler of USB Type-C orientation changes
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - "#phy-cells"
> +  - clocks
> +  - resets
> +  - power-domains
> +  - orientation-switch
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        usb_phy: usb_phy@c410000 {
> +            compatible = "google,gs5-usb-phy";
> +            reg = <0 0x0c450014 0 0xc>,
> +                  <0 0x0c637000 0 0xa0>,

You probably miss DP support and this does not belong here.

> +                  <0 0x0c45002c 0 0x4>;

That's not a separate address space. I really, really doubt that
hardware engineers came with address spaces of one word long.

> +            reg-names = "u2phy_cfg", "dp_top", "usb_top_cfg";
> +            #phy-cells = <1>;
> +            clocks = <&hsion_usb2_phy_reset_clk>;
> +            resets = <&hsion_resets_usb2_phy>;
> +            power-domains = <&hsio_n_usb_pd>;
> +            orientation-switch;
> +        };
> +    };
> +...


Best regards,
Krzysztof

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

* Re: [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3
  2025-10-11  0:08   ` Krzysztof Kozlowski
@ 2025-10-14  1:40     ` Roy Luo
  2025-10-14  8:22       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Roy Luo @ 2025-10-14  1:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy, devicetree, linux-kernel,
	linux-usb, linux-arm-kernel, linux-samsung-soc

On Fri, Oct 10, 2025 at 5:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 10/10/2025 22:16, Roy Luo wrote:
> > Document the device tree bindings for the DWC3 USB controller found in
> > Google Tensor SoCs, starting with the G5 generation.
> >
> > The Tensor G5 silicon represents a complete architectural departure from
> > previous generations (like gs101), including entirely new clock/reset
> > schemes, top-level wrapper and register interface. Consequently,
> > existing Samsung/Exynos DWC3 USB bindings are incompatible, necessitating
> > this new device tree binding.
> >
> > The USB controller on Tensor G5 is based on Synopsys DWC3 IP and features
> > Dual-Role Device single port with hibernation support.
>
> You still mix, completely unnecessarily, subsystems. For Greg this is
> actually even undesired, but regardless don't do this for any cases
> because it just makes everything slower or more difficult to apply.
>
> Really, think how maintainers should deal with your patches.
>

Understood, I will separate the patches into two distinct series: one for
the controller and one for the PHY.
Appreciate the feedback and the explanation.

> >
> > Signed-off-by: Roy Luo <royluo@google.com>
> > ---
> >  .../bindings/usb/google,gs5-dwc3.yaml         | 141 ++++++++++++++++++
> >  1 file changed, 141 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> > new file mode 100644
> > index 000000000000..6fadea7f41e8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> > @@ -0,0 +1,141 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (c) 2025, Google LLC
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/google,gs5-dwc3.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Google Tensor Series (G5+) DWC3 USB SoC Controller
> > +
> > +maintainers:
> > +  - Roy Luo <royluo@google.com>
> > +
> > +description:
> > +  Describes the DWC3 USB controller block implemented on Google Tensor SoCs,
> > +  starting with the G5 generation. Based on Synopsys DWC3 IP, the controller
> > +  features Dual-Role Device single port with hibernation add-on.
> > +
> > +properties:
> > +  compatible:
> > +    const: google,gs5-dwc3
> > +
> > +  reg:
> > +    items:
> > +      - description: Core DWC3 IP registers.
> > +      - description: USB host controller configuration registers.
> > +      - description: USB custom interrrupts control registers.
> > +
> > +  reg-names:
> > +    items:
> > +      - const: dwc3_core
> > +      - const: host_cfg
> > +      - const: usbint_cfg
> > +
> > +  interrupts:
> > +    items:
> > +      - description: Core DWC3 interrupt.
> > +      - description: High speed power management event for remote wakeup from hibernation.
> > +      - description: Super speed power management event for remote wakeup from hibernation.
>
> Wrap at 80 (see coding style) or just shorten these.

Ack, will fix it in the next patch.

>
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: dwc_usb3
>
> So just "core"?

I'd prefer to stick to "dwc_usb3" as that's
1. more expressive by referring to the underlying IP name,
2. consistent with established dwc3 bindings such as
    Documentation/devicetree/bindings/usb/snps,dwc3.yaml,
    Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml,
unless you have a strong preference for the alternative naming.

>
> > +      - const: hs_pme
> > +      - const: ss_pme
> > +
> > +  clocks:
> > +    items:
> > +      - description: Non-sticky module clock.
> > +      - description: Sticky module clock.
> > +      - description: USB2 PHY APB clock.
>
> This looks wrong. This is not the USB2 phy, so how can it consume APB clock?

That's a fair point, I'll look into the necessity and placement of this specific
clk/reset and get back.

Thanks,
Roy Luo

>
> > +
> > +  clock-names:
> > +    items:
> > +      - const: non_sticky
> > +      - const: sticky
> > +      - const: u2phy_apb
> > +
> > +  resets:
> > +    items:
> > +      - description: Non-sticky module reset.
> > +      - description: Sticky module reset.
> > +      - description: USB2 PHY APB reset.
>
> This as well.
>
> > +      - description: DRD bus reset.
> > +      - description: Top-level reset.
> > +
> > +  reset-names:
> > +    items:
> > +      - const: non_sticky
> > +      - const: sticky
> > +      - const: u2phy_apb
> > +      - const: drd_bus
> > +      - const: top
>
>
> Best regards,
> Krzysztof

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

* Re: [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY
  2025-10-11  0:10   ` Krzysztof Kozlowski
@ 2025-10-14  1:46     ` Roy Luo
  2025-10-15 13:05       ` Rob Herring
  0 siblings, 1 reply; 19+ messages in thread
From: Roy Luo @ 2025-10-14  1:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy, devicetree, linux-kernel,
	linux-usb, linux-arm-kernel, linux-samsung-soc

On Fri, Oct 10, 2025 at 5:11 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 10/10/2025 22:16, Roy Luo wrote:
> > +  reg:
> > +    items:
> > +      - description: USB2 PHY configuration registers.
> > +      - description: DisplayPort top-level registers.
> > +      - description: USB top-level configuration registers.
> > +
> > +  reg-names:
> > +    items:
> > +      - const: u2phy_cfg
> > +      - const: dp_top
> > +      - const: usb_top_cfg
> > +
> > +  "#phy-cells":
> > +    const: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  orientation-switch:
> > +    type: boolean
> > +    description:
> > +      Indicates the PHY as a handler of USB Type-C orientation changes
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - "#phy-cells"
> > +  - clocks
> > +  - resets
> > +  - power-domains
> > +  - orientation-switch
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        usb_phy: usb_phy@c410000 {
> > +            compatible = "google,gs5-usb-phy";
> > +            reg = <0 0x0c450014 0 0xc>,
> > +                  <0 0x0c637000 0 0xa0>,
>
> You probably miss DP support and this does not belong here.

This register space isn't solely for DP operation, a significant portion
manages the custom combo PHY. Consequently, this space is essential
even for USB-only operation. We can expect more registers in the space
to be utilized when DP support is added.

While I acknowledge the current name is confusing, it directly reflects
the hardware documentation. We can either adhere to the hardware
documentation's naming or propose a more descriptive alternative.
What's your preference?

>
> > +                  <0 0x0c45002c 0 0x4>;
>
> That's not a separate address space. I really, really doubt that
> hardware engineers came with address spaces of one word long.

I initially created this space to access the usb2only mode register,
which must be programmed when the controller operates in high-speed
only mode without the USB3 PHY initialized. Upon review, I now
believe the controller driver is the better location for this configuration,
as the register logically belongs there and the controller can tell
whether usb3 phy is going to be initialized.

That is, I'm removing this register space in the next patch.

Thanks,
Roy Luo
>
> > +            reg-names = "u2phy_cfg", "dp_top", "usb_top_cfg";
> > +            #phy-cells = <1>;
> > +            clocks = <&hsion_usb2_phy_reset_clk>;
> > +            resets = <&hsion_resets_usb2_phy>;
> > +            power-domains = <&hsio_n_usb_pd>;
> > +            orientation-switch;
> > +        };
> > +    };
> > +...
>
>
> Best regards,
> Krzysztof

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

* Re: [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3
  2025-10-14  1:40     ` Roy Luo
@ 2025-10-14  8:22       ` Krzysztof Kozlowski
  2025-10-15  0:50         ` Roy Luo
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-14  8:22 UTC (permalink / raw)
  To: Roy Luo
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy, devicetree, linux-kernel,
	linux-usb, linux-arm-kernel, linux-samsung-soc

On 14/10/2025 03:40, Roy Luo wrote:
> On Fri, Oct 10, 2025 at 5:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 10/10/2025 22:16, Roy Luo wrote:
>>> Document the device tree bindings for the DWC3 USB controller found in
>>> Google Tensor SoCs, starting with the G5 generation.
>>>
>>> The Tensor G5 silicon represents a complete architectural departure from
>>> previous generations (like gs101), including entirely new clock/reset
>>> schemes, top-level wrapper and register interface. Consequently,
>>> existing Samsung/Exynos DWC3 USB bindings are incompatible, necessitating
>>> this new device tree binding.
>>>
>>> The USB controller on Tensor G5 is based on Synopsys DWC3 IP and features
>>> Dual-Role Device single port with hibernation support.
>>
>> You still mix, completely unnecessarily, subsystems. For Greg this is
>> actually even undesired, but regardless don't do this for any cases
>> because it just makes everything slower or more difficult to apply.
>>
>> Really, think how maintainers should deal with your patches.
>>
> 
> Understood, I will separate the patches into two distinct series: one for
> the controller and one for the PHY.
> Appreciate the feedback and the explanation.
> 
>>>
>>> Signed-off-by: Roy Luo <royluo@google.com>
>>> ---
>>>  .../bindings/usb/google,gs5-dwc3.yaml         | 141 ++++++++++++++++++
>>>  1 file changed, 141 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
>>> new file mode 100644
>>> index 000000000000..6fadea7f41e8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
>>> @@ -0,0 +1,141 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +# Copyright (c) 2025, Google LLC
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/usb/google,gs5-dwc3.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Google Tensor Series (G5+) DWC3 USB SoC Controller
>>> +
>>> +maintainers:
>>> +  - Roy Luo <royluo@google.com>
>>> +
>>> +description:
>>> +  Describes the DWC3 USB controller block implemented on Google Tensor SoCs,
>>> +  starting with the G5 generation. Based on Synopsys DWC3 IP, the controller
>>> +  features Dual-Role Device single port with hibernation add-on.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: google,gs5-dwc3
>>> +
>>> +  reg:
>>> +    items:
>>> +      - description: Core DWC3 IP registers.
>>> +      - description: USB host controller configuration registers.
>>> +      - description: USB custom interrrupts control registers.
>>> +
>>> +  reg-names:
>>> +    items:
>>> +      - const: dwc3_core
>>> +      - const: host_cfg
>>> +      - const: usbint_cfg
>>> +
>>> +  interrupts:
>>> +    items:
>>> +      - description: Core DWC3 interrupt.
>>> +      - description: High speed power management event for remote wakeup from hibernation.
>>> +      - description: Super speed power management event for remote wakeup from hibernation.
>>
>> Wrap at 80 (see coding style) or just shorten these.
> 
> Ack, will fix it in the next patch.
> 
>>
>>> +
>>> +  interrupt-names:
>>> +    items:
>>> +      - const: dwc_usb3
>>
>> So just "core"?
> 
> I'd prefer to stick to "dwc_usb3" as that's
> 1. more expressive by referring to the underlying IP name,


But that's completely redundant name.

> 2. consistent with established dwc3 bindings such as
>     Documentation/devicetree/bindings/usb/snps,dwc3.yaml,

If you use only one interrupt. You don't use one interrupt here.

>     Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml,
> unless you have a strong preference for the alternative naming.

Such namings are discouraged, because they tell absolutely nothing.
Also, schematics or datasheets usually do not use them, either.


Best regards,
Krzysztof

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

* Re: [PATCH v3 2/4] usb: dwc3: Add Google Tensor SoC DWC3 glue driver
  2025-10-10 20:16 ` [PATCH v3 2/4] usb: dwc3: Add Google Tensor SoC DWC3 glue driver Roy Luo
@ 2025-10-15  0:27   ` Thinh Nguyen
  2025-10-15 17:39     ` Roy Luo
  0 siblings, 1 reply; 19+ messages in thread
From: Thinh Nguyen @ 2025-10-15  0:27 UTC (permalink / raw)
  To: Roy Luo
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org

Hi,

On Fri, Oct 10, 2025, Roy Luo wrote:
> +#endif /* CONFIG_PM */
> +
> +static const struct dev_pm_ops dwc3_google_dev_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_google_pm_suspend, dwc3_google_pm_resume)
> +	SET_RUNTIME_PM_OPS(dwc3_google_runtime_suspend, dwc3_google_runtime_resume,
> +			   dwc3_google_runtime_idle)

Can we use the new pm_ptr() and *_PM_OPS macros to get rid of the ifdef
CONFIG_PM guards?

> +	.complete = dwc3_google_complete,
> +	.prepare = dwc3_google_prepare,
> +};
> +
> +static const struct of_device_id dwc3_google_of_match[] = {
> +	{ .compatible = "google,gs5-dwc3" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, dwc3_google_of_match);
> +
> +static struct platform_driver dwc3_google_driver = {
> +	.probe		= dwc3_google_probe,
> +	.remove		= dwc3_google_remove,
> +	.driver		= {
> +		.name	= "google-dwc3",
> +		.pm	= &dwc3_google_dev_pm_ops,
> +		.of_match_table	= dwc3_google_of_match,
> +	},
> +};
> +
> +module_platform_driver(dwc3_google_driver);
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("DesignWare DWC3 Google Glue Driver");
> -- 
> 2.51.0.740.g6adb054d12-goog
> 

Give me some time, and I'll review the rest of this patch.

Thanks,
Thinh

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

* Re: [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3
  2025-10-14  8:22       ` Krzysztof Kozlowski
@ 2025-10-15  0:50         ` Roy Luo
  2025-10-15  8:59           ` Conor Dooley
  0 siblings, 1 reply; 19+ messages in thread
From: Roy Luo @ 2025-10-15  0:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy, devicetree, linux-kernel,
	linux-usb, linux-arm-kernel, linux-samsung-soc

On Tue, Oct 14, 2025 at 1:22 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 14/10/2025 03:40, Roy Luo wrote:
> > On Fri, Oct 10, 2025 at 5:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 10/10/2025 22:16, Roy Luo wrote:
> >>> Document the device tree bindings for the DWC3 USB controller found in
> >>> Google Tensor SoCs, starting with the G5 generation.
> >>>
> >>> The Tensor G5 silicon represents a complete architectural departure from
> >>> previous generations (like gs101), including entirely new clock/reset
> >>> schemes, top-level wrapper and register interface. Consequently,
> >>> existing Samsung/Exynos DWC3 USB bindings are incompatible, necessitating
> >>> this new device tree binding.
> >>>
> >>> The USB controller on Tensor G5 is based on Synopsys DWC3 IP and features
> >>> Dual-Role Device single port with hibernation support.
> >>
> >> You still mix, completely unnecessarily, subsystems. For Greg this is
> >> actually even undesired, but regardless don't do this for any cases
> >> because it just makes everything slower or more difficult to apply.
> >>
> >> Really, think how maintainers should deal with your patches.
> >>
> >
> > Understood, I will separate the patches into two distinct series: one for
> > the controller and one for the PHY.
> > Appreciate the feedback and the explanation.
> >
> >>>
> >>> Signed-off-by: Roy Luo <royluo@google.com>
> >>> ---
> >>>  .../bindings/usb/google,gs5-dwc3.yaml         | 141 ++++++++++++++++++
> >>>  1 file changed, 141 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> >>> new file mode 100644
> >>> index 000000000000..6fadea7f41e8
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> >>> @@ -0,0 +1,141 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >>> +# Copyright (c) 2025, Google LLC
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/usb/google,gs5-dwc3.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Google Tensor Series (G5+) DWC3 USB SoC Controller
> >>> +
> >>> +maintainers:
> >>> +  - Roy Luo <royluo@google.com>
> >>> +
> >>> +description:
> >>> +  Describes the DWC3 USB controller block implemented on Google Tensor SoCs,
> >>> +  starting with the G5 generation. Based on Synopsys DWC3 IP, the controller
> >>> +  features Dual-Role Device single port with hibernation add-on.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    const: google,gs5-dwc3
> >>> +
> >>> +  reg:
> >>> +    items:
> >>> +      - description: Core DWC3 IP registers.
> >>> +      - description: USB host controller configuration registers.
> >>> +      - description: USB custom interrrupts control registers.
> >>> +
> >>> +  reg-names:
> >>> +    items:
> >>> +      - const: dwc3_core
> >>> +      - const: host_cfg
> >>> +      - const: usbint_cfg
> >>> +
> >>> +  interrupts:
> >>> +    items:
> >>> +      - description: Core DWC3 interrupt.
> >>> +      - description: High speed power management event for remote wakeup from hibernation.
> >>> +      - description: Super speed power management event for remote wakeup from hibernation.
> >>
> >> Wrap at 80 (see coding style) or just shorten these.
> >
> > Ack, will fix it in the next patch.
> >
> >>
> >>> +
> >>> +  interrupt-names:
> >>> +    items:
> >>> +      - const: dwc_usb3
> >>
> >> So just "core"?
> >
> > I'd prefer to stick to "dwc_usb3" as that's
> > 1. more expressive by referring to the underlying IP name,
>
>
> But that's completely redundant name.
>
> > 2. consistent with established dwc3 bindings such as
> >     Documentation/devicetree/bindings/usb/snps,dwc3.yaml,
>
> If you use only one interrupt. You don't use one interrupt here.

After looking into it further, I found that the interrupt name "dwc_usb3"
must be used here to adhere to the interrupt naming defined in
"snps,dwc3.yaml".

This requirement stems from the device's corresponding glue driver
utilizing a so-called "flattened" model (see [1] for context). This model
causes the glue driver to probe an underlying "snps,dwc3" device.
Consequently, the core DWC3 interrupt defined here is consumed by
the driver handling the "snps,dwc3" device, making it mandatory to
follow the interrupt naming established in "snps,dwc3.yaml".
Essentially, the interrupts defined here are a mix of vendor specific
implementation (like "hs_pme", "ss_pme") and the DWC3 core in
"snps,dwc3.yaml" ("dwc_usb3").

I don't know if there's a better way to express this implicit dependency
of the core DWC3 interrupt except for documenting it in the binding
description. Any advice would be welcome.

[1] https://lore.kernel.org/all/20250414-dwc3-refactor-v7-0-f015b358722d@oss.qualcomm.com/

Thanks,
Roy Luo

>
> >     Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml,
> > unless you have a strong preference for the alternative naming.
>
> Such namings are discouraged, because they tell absolutely nothing.
> Also, schematics or datasheets usually do not use them, either.
>
>
> Best regards,
> Krzysztof

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

* Re: [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3
  2025-10-15  0:50         ` Roy Luo
@ 2025-10-15  8:59           ` Conor Dooley
  2025-10-15 17:13             ` Roy Luo
  0 siblings, 1 reply; 19+ messages in thread
From: Conor Dooley @ 2025-10-15  8:59 UTC (permalink / raw)
  To: Roy Luo
  Cc: Krzysztof Kozlowski, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Greg Kroah-Hartman, Thinh Nguyen, Philipp Zabel, Peter Griffin,
	André Draszik, Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy, devicetree, linux-kernel,
	linux-usb, linux-arm-kernel, linux-samsung-soc

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

On Tue, Oct 14, 2025 at 05:50:17PM -0700, Roy Luo wrote:
> On Tue, Oct 14, 2025 at 1:22 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On 14/10/2025 03:40, Roy Luo wrote:
> > > On Fri, Oct 10, 2025 at 5:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >>
> > >> On 10/10/2025 22:16, Roy Luo wrote:
> > >>> Document the device tree bindings for the DWC3 USB controller found in
> > >>> Google Tensor SoCs, starting with the G5 generation.
> > >>>
> > >>> The Tensor G5 silicon represents a complete architectural departure from
> > >>> previous generations (like gs101), including entirely new clock/reset
> > >>> schemes, top-level wrapper and register interface. Consequently,
> > >>> existing Samsung/Exynos DWC3 USB bindings are incompatible, necessitating
> > >>> this new device tree binding.
> > >>>
> > >>> The USB controller on Tensor G5 is based on Synopsys DWC3 IP and features
> > >>> Dual-Role Device single port with hibernation support.
> > >>
> > >> You still mix, completely unnecessarily, subsystems. For Greg this is
> > >> actually even undesired, but regardless don't do this for any cases
> > >> because it just makes everything slower or more difficult to apply.
> > >>
> > >> Really, think how maintainers should deal with your patches.
> > >>
> > >
> > > Understood, I will separate the patches into two distinct series: one for
> > > the controller and one for the PHY.
> > > Appreciate the feedback and the explanation.
> > >
> > >>>
> > >>> Signed-off-by: Roy Luo <royluo@google.com>
> > >>> ---
> > >>>  .../bindings/usb/google,gs5-dwc3.yaml         | 141 ++++++++++++++++++
> > >>>  1 file changed, 141 insertions(+)
> > >>>  create mode 100644 Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> > >>> new file mode 100644
> > >>> index 000000000000..6fadea7f41e8
> > >>> --- /dev/null
> > >>> +++ b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> > >>> @@ -0,0 +1,141 @@
> > >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > >>> +# Copyright (c) 2025, Google LLC
> > >>> +%YAML 1.2
> > >>> +---
> > >>> +$id: http://devicetree.org/schemas/usb/google,gs5-dwc3.yaml#
> > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > >>> +
> > >>> +title: Google Tensor Series (G5+) DWC3 USB SoC Controller
> > >>> +
> > >>> +maintainers:
> > >>> +  - Roy Luo <royluo@google.com>
> > >>> +
> > >>> +description:
> > >>> +  Describes the DWC3 USB controller block implemented on Google Tensor SoCs,
> > >>> +  starting with the G5 generation. Based on Synopsys DWC3 IP, the controller
> > >>> +  features Dual-Role Device single port with hibernation add-on.
> > >>> +
> > >>> +properties:
> > >>> +  compatible:
> > >>> +    const: google,gs5-dwc3
> > >>> +
> > >>> +  reg:
> > >>> +    items:
> > >>> +      - description: Core DWC3 IP registers.
> > >>> +      - description: USB host controller configuration registers.
> > >>> +      - description: USB custom interrrupts control registers.
> > >>> +
> > >>> +  reg-names:
> > >>> +    items:
> > >>> +      - const: dwc3_core
> > >>> +      - const: host_cfg
> > >>> +      - const: usbint_cfg
> > >>> +
> > >>> +  interrupts:
> > >>> +    items:
> > >>> +      - description: Core DWC3 interrupt.
> > >>> +      - description: High speed power management event for remote wakeup from hibernation.
> > >>> +      - description: Super speed power management event for remote wakeup from hibernation.
> > >>
> > >> Wrap at 80 (see coding style) or just shorten these.
> > >
> > > Ack, will fix it in the next patch.
> > >
> > >>
> > >>> +
> > >>> +  interrupt-names:
> > >>> +    items:
> > >>> +      - const: dwc_usb3
> > >>
> > >> So just "core"?
> > >
> > > I'd prefer to stick to "dwc_usb3" as that's
> > > 1. more expressive by referring to the underlying IP name,
> >
> >
> > But that's completely redundant name.
> >
> > > 2. consistent with established dwc3 bindings such as
> > >     Documentation/devicetree/bindings/usb/snps,dwc3.yaml,
> >
> > If you use only one interrupt. You don't use one interrupt here.
> 
> After looking into it further, I found that the interrupt name "dwc_usb3"
> must be used here to adhere to the interrupt naming defined in
> "snps,dwc3.yaml".

Did you just chose to not read what Krzysztof said here? It must be used
only when that's the sole interrupt, which he stated is not the case for
your platform.

> This requirement stems from the device's corresponding glue driver
> utilizing a so-called "flattened" model (see [1] for context). This model
> causes the glue driver to probe an underlying "snps,dwc3" device.
> Consequently, the core DWC3 interrupt defined here is consumed by
> the driver handling the "snps,dwc3" device, making it mandatory to
> follow the interrupt naming established in "snps,dwc3.yaml".

I look at the binding and noticed that interrupt-names isn't even a
required property by snps,dwc3.yaml, and this comment about driver
behaviour likely isn't accurate given that the code in for host mode
(and the others are identical) is written so that it will grab the first
interrupt if the specific names it looks for are absent:
| static int dwc3_host_get_irq(struct dwc3 *dwc)
| {
| 	struct platform_device	*dwc3_pdev = to_platform_device(dwc->dev);
| 	int irq;
| 
| 	irq = platform_get_irq_byname_optional(dwc3_pdev, "host");
| 	if (irq > 0) {
| 		dwc3_host_fill_xhci_irq_res(dwc, irq, "host");
| 		goto out;
| 	}
| 
| 	if (irq == -EPROBE_DEFER)
| 		goto out;
| 
| 	irq = platform_get_irq_byname_optional(dwc3_pdev, "dwc_usb3");
| 	if (irq > 0) {
| 		dwc3_host_fill_xhci_irq_res(dwc, irq, "dwc_usb3");
| 		goto out;
| 	}
| 
| 	if (irq == -EPROBE_DEFER)
| 		goto out;
| 
| 	irq = platform_get_irq(dwc3_pdev, 0);
| 	if (irq > 0)
| 		dwc3_host_fill_xhci_irq_res(dwc, irq, NULL);
| 
| out:
| 	return irq;
| }

Since it grabs the first interrupt, as a fallback, in order to support
not having the interrupt-names property, the name of the first interrupt
ultimately doesn't matter at all to this driver (and likely any other
driver written in compliance with the bindings for the dwc3 core).

I'm not here to argue about what the name for the single interrupt
should be (keeping consistency with other devices might actually be
good), but ignoring what a maintainer says and the seemingly providing
an incorrect analysis is annoying. Did you perform the analysis on this
yourself, or did it perhaps come from Gemini?

Thanks,
Conor.

> Essentially, the interrupts defined here are a mix of vendor specific
> implementation (like "hs_pme", "ss_pme") and the DWC3 core in
> "snps,dwc3.yaml" ("dwc_usb3").
> 
> I don't know if there's a better way to express this implicit dependency
> of the core DWC3 interrupt except for documenting it in the binding
> description. Any advice would be welcome.
> 
> [1] https://lore.kernel.org/all/20250414-dwc3-refactor-v7-0-f015b358722d@oss.qualcomm.com/
> 
> Thanks,
> Roy Luo
> 
> >
> > >     Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml,
> > > unless you have a strong preference for the alternative naming.
> >
> > Such namings are discouraged, because they tell absolutely nothing.
> > Also, schematics or datasheets usually do not use them, either.
> >
> >
> > Best regards,
> > Krzysztof

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

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

* Re: [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY
  2025-10-14  1:46     ` Roy Luo
@ 2025-10-15 13:05       ` Rob Herring
  2025-10-15 18:57         ` Roy Luo
  0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2025-10-15 13:05 UTC (permalink / raw)
  To: Roy Luo
  Cc: Krzysztof Kozlowski, Vinod Koul, Kishon Vijay Abraham I,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy, devicetree, linux-kernel,
	linux-usb, linux-arm-kernel, linux-samsung-soc

On Mon, Oct 13, 2025 at 06:46:39PM -0700, Roy Luo wrote:
> On Fri, Oct 10, 2025 at 5:11 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On 10/10/2025 22:16, Roy Luo wrote:
> > > +  reg:
> > > +    items:
> > > +      - description: USB2 PHY configuration registers.
> > > +      - description: DisplayPort top-level registers.
> > > +      - description: USB top-level configuration registers.
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: u2phy_cfg
> > > +      - const: dp_top
> > > +      - const: usb_top_cfg
> > > +
> > > +  "#phy-cells":
> > > +    const: 1
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +
> > > +  resets:
> > > +    maxItems: 1
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +
> > > +  orientation-switch:
> > > +    type: boolean
> > > +    description:
> > > +      Indicates the PHY as a handler of USB Type-C orientation changes
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - reg-names
> > > +  - "#phy-cells"
> > > +  - clocks
> > > +  - resets
> > > +  - power-domains
> > > +  - orientation-switch
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    soc {
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +
> > > +        usb_phy: usb_phy@c410000 {
> > > +            compatible = "google,gs5-usb-phy";
> > > +            reg = <0 0x0c450014 0 0xc>,
> > > +                  <0 0x0c637000 0 0xa0>,
> >
> > You probably miss DP support and this does not belong here.
> 
> This register space isn't solely for DP operation, a significant portion
> manages the custom combo PHY. Consequently, this space is essential
> even for USB-only operation. We can expect more registers in the space
> to be utilized when DP support is added.
> 
> While I acknowledge the current name is confusing, it directly reflects
> the hardware documentation. We can either adhere to the hardware
> documentation's naming or propose a more descriptive alternative.
> What's your preference?
> 
> >
> > > +                  <0 0x0c45002c 0 0x4>;
> >
> > That's not a separate address space. I really, really doubt that
> > hardware engineers came with address spaces of one word long.
> 
> I initially created this space to access the usb2only mode register,
> which must be programmed when the controller operates in high-speed
> only mode without the USB3 PHY initialized. Upon review, I now
> believe the controller driver is the better location for this configuration,
> as the register logically belongs there and the controller can tell
> whether usb3 phy is going to be initialized.
> 
> That is, I'm removing this register space in the next patch.

You are missing the point. What exists from 0x0c450020-2c and 
0x0c450000-0x14 for that matter? Hardware blocks don't just start on 
unaligned boundaries like 0x14 or 0x2c. DT describes the h/w blocks, not 
just nodes of what a driver needs. So if the 0x2c register needs to be 
accessed by the USB driver, that's fine, but the register doesn't go in 
the USB controller node 'reg'. A property with a phandle to the node 
defining all the 0x0c450000 registers and an offset (if needed) is 
typically what we do there. Or you can just find that node by 
compatible.

Rob

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

* Re: [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3
  2025-10-15  8:59           ` Conor Dooley
@ 2025-10-15 17:13             ` Roy Luo
  0 siblings, 0 replies; 19+ messages in thread
From: Roy Luo @ 2025-10-15 17:13 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Krzysztof Kozlowski, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Greg Kroah-Hartman, Thinh Nguyen, Philipp Zabel, Peter Griffin,
	André Draszik, Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy, devicetree, linux-kernel,
	linux-usb, linux-arm-kernel, linux-samsung-soc

On Wed, Oct 15, 2025 at 1:59 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Oct 14, 2025 at 05:50:17PM -0700, Roy Luo wrote:
> > On Tue, Oct 14, 2025 at 1:22 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On 14/10/2025 03:40, Roy Luo wrote:
> > > > On Fri, Oct 10, 2025 at 5:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > >>
> > > >> On 10/10/2025 22:16, Roy Luo wrote:
> > > >>> Document the device tree bindings for the DWC3 USB controller found in
> > > >>> Google Tensor SoCs, starting with the G5 generation.
> > > >>>
> > > >>> The Tensor G5 silicon represents a complete architectural departure from
> > > >>> previous generations (like gs101), including entirely new clock/reset
> > > >>> schemes, top-level wrapper and register interface. Consequently,
> > > >>> existing Samsung/Exynos DWC3 USB bindings are incompatible, necessitating
> > > >>> this new device tree binding.
> > > >>>
> > > >>> The USB controller on Tensor G5 is based on Synopsys DWC3 IP and features
> > > >>> Dual-Role Device single port with hibernation support.
> > > >>
> > > >> You still mix, completely unnecessarily, subsystems. For Greg this is
> > > >> actually even undesired, but regardless don't do this for any cases
> > > >> because it just makes everything slower or more difficult to apply.
> > > >>
> > > >> Really, think how maintainers should deal with your patches.
> > > >>
> > > >
> > > > Understood, I will separate the patches into two distinct series: one for
> > > > the controller and one for the PHY.
> > > > Appreciate the feedback and the explanation.
> > > >
> > > >>>
> > > >>> Signed-off-by: Roy Luo <royluo@google.com>
> > > >>> ---
> > > >>>  .../bindings/usb/google,gs5-dwc3.yaml         | 141 ++++++++++++++++++
> > > >>>  1 file changed, 141 insertions(+)
> > > >>>  create mode 100644 Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> > > >>>
> > > >>> diff --git a/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> > > >>> new file mode 100644
> > > >>> index 000000000000..6fadea7f41e8
> > > >>> --- /dev/null
> > > >>> +++ b/Documentation/devicetree/bindings/usb/google,gs5-dwc3.yaml
> > > >>> @@ -0,0 +1,141 @@
> > > >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > >>> +# Copyright (c) 2025, Google LLC
> > > >>> +%YAML 1.2
> > > >>> +---
> > > >>> +$id: http://devicetree.org/schemas/usb/google,gs5-dwc3.yaml#
> > > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > >>> +
> > > >>> +title: Google Tensor Series (G5+) DWC3 USB SoC Controller
> > > >>> +
> > > >>> +maintainers:
> > > >>> +  - Roy Luo <royluo@google.com>
> > > >>> +
> > > >>> +description:
> > > >>> +  Describes the DWC3 USB controller block implemented on Google Tensor SoCs,
> > > >>> +  starting with the G5 generation. Based on Synopsys DWC3 IP, the controller
> > > >>> +  features Dual-Role Device single port with hibernation add-on.
> > > >>> +
> > > >>> +properties:
> > > >>> +  compatible:
> > > >>> +    const: google,gs5-dwc3
> > > >>> +
> > > >>> +  reg:
> > > >>> +    items:
> > > >>> +      - description: Core DWC3 IP registers.
> > > >>> +      - description: USB host controller configuration registers.
> > > >>> +      - description: USB custom interrrupts control registers.
> > > >>> +
> > > >>> +  reg-names:
> > > >>> +    items:
> > > >>> +      - const: dwc3_core
> > > >>> +      - const: host_cfg
> > > >>> +      - const: usbint_cfg
> > > >>> +
> > > >>> +  interrupts:
> > > >>> +    items:
> > > >>> +      - description: Core DWC3 interrupt.
> > > >>> +      - description: High speed power management event for remote wakeup from hibernation.
> > > >>> +      - description: Super speed power management event for remote wakeup from hibernation.
> > > >>
> > > >> Wrap at 80 (see coding style) or just shorten these.
> > > >
> > > > Ack, will fix it in the next patch.
> > > >
> > > >>
> > > >>> +
> > > >>> +  interrupt-names:
> > > >>> +    items:
> > > >>> +      - const: dwc_usb3
> > > >>
> > > >> So just "core"?
> > > >
> > > > I'd prefer to stick to "dwc_usb3" as that's
> > > > 1. more expressive by referring to the underlying IP name,
> > >
> > >
> > > But that's completely redundant name.
> > >
> > > > 2. consistent with established dwc3 bindings such as
> > > >     Documentation/devicetree/bindings/usb/snps,dwc3.yaml,
> > >
> > > If you use only one interrupt. You don't use one interrupt here.
> >
> > After looking into it further, I found that the interrupt name "dwc_usb3"
> > must be used here to adhere to the interrupt naming defined in
> > "snps,dwc3.yaml".
>
> Did you just chose to not read what Krzysztof said here? It must be used
> only when that's the sole interrupt, which he stated is not the case for
> your platform.
>
> > This requirement stems from the device's corresponding glue driver
> > utilizing a so-called "flattened" model (see [1] for context). This model
> > causes the glue driver to probe an underlying "snps,dwc3" device.
> > Consequently, the core DWC3 interrupt defined here is consumed by
> > the driver handling the "snps,dwc3" device, making it mandatory to
> > follow the interrupt naming established in "snps,dwc3.yaml".
>
> I look at the binding and noticed that interrupt-names isn't even a
> required property by snps,dwc3.yaml, and this comment about driver
> behaviour likely isn't accurate given that the code in for host mode
> (and the others are identical) is written so that it will grab the first
> interrupt if the specific names it looks for are absent:
> | static int dwc3_host_get_irq(struct dwc3 *dwc)
> | {
> |       struct platform_device  *dwc3_pdev = to_platform_device(dwc->dev);
> |       int irq;
> |
> |       irq = platform_get_irq_byname_optional(dwc3_pdev, "host");
> |       if (irq > 0) {
> |               dwc3_host_fill_xhci_irq_res(dwc, irq, "host");
> |               goto out;
> |       }
> |
> |       if (irq == -EPROBE_DEFER)
> |               goto out;
> |
> |       irq = platform_get_irq_byname_optional(dwc3_pdev, "dwc_usb3");
> |       if (irq > 0) {
> |               dwc3_host_fill_xhci_irq_res(dwc, irq, "dwc_usb3");
> |               goto out;
> |       }
> |
> |       if (irq == -EPROBE_DEFER)
> |               goto out;
> |
> |       irq = platform_get_irq(dwc3_pdev, 0);
> |       if (irq > 0)
> |               dwc3_host_fill_xhci_irq_res(dwc, irq, NULL);
> |
> | out:
> |       return irq;
> | }
>
> Since it grabs the first interrupt, as a fallback, in order to support
> not having the interrupt-names property, the name of the first interrupt
> ultimately doesn't matter at all to this driver (and likely any other
> driver written in compliance with the bindings for the dwc3 core).
>
> I'm not here to argue about what the name for the single interrupt
> should be (keeping consistency with other devices might actually be
> good), but ignoring what a maintainer says and the seemingly providing
> an incorrect analysis is annoying. Did you perform the analysis on this
> yourself, or did it perhaps come from Gemini?
>
> Thanks,
> Conor.

Hi Conor,

My apologies for the incorrect analysis. I misinterpreted the code,
which was a big mistake. I can assure you that I do value your
feedback and take every comment seriously.
Thanks for pointing out the error and raising your concern.
This won't happen again.

Thanks,
Roy Luo

>
> > Essentially, the interrupts defined here are a mix of vendor specific
> > implementation (like "hs_pme", "ss_pme") and the DWC3 core in
> > "snps,dwc3.yaml" ("dwc_usb3").
> >
> > I don't know if there's a better way to express this implicit dependency
> > of the core DWC3 interrupt except for documenting it in the binding
> > description. Any advice would be welcome.
> >
> > [1] https://lore.kernel.org/all/20250414-dwc3-refactor-v7-0-f015b358722d@oss.qualcomm.com/
> >
> > Thanks,
> > Roy Luo
> >
> > >
> > > >     Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml,
> > > > unless you have a strong preference for the alternative naming.
> > >
> > > Such namings are discouraged, because they tell absolutely nothing.
> > > Also, schematics or datasheets usually do not use them, either.
> > >
> > >
> > > Best regards,
> > > Krzysztof

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

* Re: [PATCH v3 2/4] usb: dwc3: Add Google Tensor SoC DWC3 glue driver
  2025-10-15  0:27   ` Thinh Nguyen
@ 2025-10-15 17:39     ` Roy Luo
  2025-10-16 22:17       ` Thinh Nguyen
  0 siblings, 1 reply; 19+ messages in thread
From: Roy Luo @ 2025-10-15 17:39 UTC (permalink / raw)
  To: Thinh Nguyen
  Cc: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Philipp Zabel, Peter Griffin, André Draszik, Tudor Ambarus,
	Joy Chakraborty, Naveen Kumar, Badhri Jagan Sridharan,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org

On Tue, Oct 14, 2025 at 5:28 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>
> Hi,
>
> On Fri, Oct 10, 2025, Roy Luo wrote:
> > +#endif /* CONFIG_PM */
> > +
> > +static const struct dev_pm_ops dwc3_google_dev_pm_ops = {
> > +     SET_SYSTEM_SLEEP_PM_OPS(dwc3_google_pm_suspend, dwc3_google_pm_resume)
> > +     SET_RUNTIME_PM_OPS(dwc3_google_runtime_suspend, dwc3_google_runtime_resume,
> > +                        dwc3_google_runtime_idle)
>
> Can we use the new pm_ptr() and *_PM_OPS macros to get rid of the ifdef
> CONFIG_PM guards?

Yes, will replace it with pm_ptr in the next patch.
P.S. The kernel test robot is also complaining about it. [1]

[1] https://lore.kernel.org/linux-usb/202510111335.oyOAs9MB-lkp@intel.com/

>
> > +     .complete = dwc3_google_complete,
> > +     .prepare = dwc3_google_prepare,
> > +};
> > +
> > +static const struct of_device_id dwc3_google_of_match[] = {
> > +     { .compatible = "google,gs5-dwc3" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(of, dwc3_google_of_match);
> > +
> > +static struct platform_driver dwc3_google_driver = {
> > +     .probe          = dwc3_google_probe,
> > +     .remove         = dwc3_google_remove,
> > +     .driver         = {
> > +             .name   = "google-dwc3",
> > +             .pm     = &dwc3_google_dev_pm_ops,
> > +             .of_match_table = dwc3_google_of_match,
> > +     },
> > +};
> > +
> > +module_platform_driver(dwc3_google_driver);
> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("DesignWare DWC3 Google Glue Driver");
> > --
> > 2.51.0.740.g6adb054d12-goog
> >
>
> Give me some time, and I'll review the rest of this patch.

Thanks Thinh! and a heads up, I'm splitting this patset into
two separate series as Krzysztof suggested; one for the
controller and one for the phy, so the series title might
change in the next version.

Regards,
Roy Luo

>
> Thanks,
> Thinh

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

* Re: [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY
  2025-10-15 13:05       ` Rob Herring
@ 2025-10-15 18:57         ` Roy Luo
  2025-10-17 23:57           ` Roy Luo
  0 siblings, 1 reply; 19+ messages in thread
From: Roy Luo @ 2025-10-15 18:57 UTC (permalink / raw)
  To: Rob Herring
  Cc: Krzysztof Kozlowski, Vinod Koul, Kishon Vijay Abraham I,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy, devicetree, linux-kernel,
	linux-usb, linux-arm-kernel, linux-samsung-soc

On Wed, Oct 15, 2025 at 6:05 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Oct 13, 2025 at 06:46:39PM -0700, Roy Luo wrote:
> > On Fri, Oct 10, 2025 at 5:11 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On 10/10/2025 22:16, Roy Luo wrote:
> > > > +  reg:
> > > > +    items:
> > > > +      - description: USB2 PHY configuration registers.
> > > > +      - description: DisplayPort top-level registers.
> > > > +      - description: USB top-level configuration registers.
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - const: u2phy_cfg
> > > > +      - const: dp_top
> > > > +      - const: usb_top_cfg
> > > > +
> > > > +  "#phy-cells":
> > > > +    const: 1
> > > > +
> > > > +  clocks:
> > > > +    maxItems: 1
> > > > +
> > > > +  resets:
> > > > +    maxItems: 1
> > > > +
> > > > +  power-domains:
> > > > +    maxItems: 1
> > > > +
> > > > +  orientation-switch:
> > > > +    type: boolean
> > > > +    description:
> > > > +      Indicates the PHY as a handler of USB Type-C orientation changes
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - reg-names
> > > > +  - "#phy-cells"
> > > > +  - clocks
> > > > +  - resets
> > > > +  - power-domains
> > > > +  - orientation-switch
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    soc {
> > > > +        #address-cells = <2>;
> > > > +        #size-cells = <2>;
> > > > +
> > > > +        usb_phy: usb_phy@c410000 {
> > > > +            compatible = "google,gs5-usb-phy";
> > > > +            reg = <0 0x0c450014 0 0xc>,
> > > > +                  <0 0x0c637000 0 0xa0>,
> > >
> > > You probably miss DP support and this does not belong here.
> >
> > This register space isn't solely for DP operation, a significant portion
> > manages the custom combo PHY. Consequently, this space is essential
> > even for USB-only operation. We can expect more registers in the space
> > to be utilized when DP support is added.
> >
> > While I acknowledge the current name is confusing, it directly reflects
> > the hardware documentation. We can either adhere to the hardware
> > documentation's naming or propose a more descriptive alternative.
> > What's your preference?
> >
> > >
> > > > +                  <0 0x0c45002c 0 0x4>;
> > >
> > > That's not a separate address space. I really, really doubt that
> > > hardware engineers came with address spaces of one word long.
> >
> > I initially created this space to access the usb2only mode register,
> > which must be programmed when the controller operates in high-speed
> > only mode without the USB3 PHY initialized. Upon review, I now
> > believe the controller driver is the better location for this configuration,
> > as the register logically belongs there and the controller can tell
> > whether usb3 phy is going to be initialized.
> >
> > That is, I'm removing this register space in the next patch.
>
> You are missing the point. What exists from 0x0c450020-2c and
> 0x0c450000-0x14 for that matter? Hardware blocks don't just start on
> unaligned boundaries like 0x14 or 0x2c. DT describes the h/w blocks, not

Rob,

Thanks for chiming in. Let me elaborate the register layout here:
The register space 0x0c450000 - 0x00450043 is supposed to
be assigned to the USB controller. However, the USB phy has
to access a small portion of it, i.e. 0x0c450014 - 0x0c450020,
in order to initialize usb2 phy. This is really unfortunate and
makes things more complicated than it should've been.

The current patch is addressing it by splitting the register space:
- USB phy: <0 0x0c450014 0 0xc>
- USB controller: <0 0x0c450000 0 0x14>, <0 0x0c450020 0 0x23>

> just nodes of what a driver needs. So if the 0x2c register needs to be
> accessed by the USB driver, that's fine, but the register doesn't go in
> the USB controller node 'reg'. A property with a phandle to the node
> defining all the 0x0c450000 registers and an offset (if needed) is
> typically what we do there. Or you can just find that node by
> compatible.

Just to make sure we're on the same page, are you suggesting
making the register space a syscon node [1]? something like this:

usb_cfg_csr: usb_cfg_csr@c450000 {
  compatible = "syscon";
  reg = <0 0x0c450000 0x0 0x43>;
};

usb@c400000 {
  ...
  usb-cfg-syscon = <&usb_cfg_csr>;
  ...
};

usb_phy@c637000 {
  ...
  usb-cfg-syscon = <&usb_cfg_csr>;
  ...
}

[1] Documentation/devicetree/bindings/mfd/syscon.yaml

Thanks,
Roy Luo

>
> Rob

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

* Re: [PATCH v3 2/4] usb: dwc3: Add Google Tensor SoC DWC3 glue driver
  2025-10-15 17:39     ` Roy Luo
@ 2025-10-16 22:17       ` Thinh Nguyen
  0 siblings, 0 replies; 19+ messages in thread
From: Thinh Nguyen @ 2025-10-16 22:17 UTC (permalink / raw)
  To: Roy Luo
  Cc: Thinh Nguyen, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Philipp Zabel, Peter Griffin, André Draszik, Tudor Ambarus,
	Joy Chakraborty, Naveen Kumar, Badhri Jagan Sridharan,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org

On Wed, Oct 15, 2025, Roy Luo wrote:
> On Tue, Oct 14, 2025 at 5:28 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> >
> > Give me some time, and I'll review the rest of this patch.
> 
> Thanks Thinh! and a heads up, I'm splitting this patset into
> two separate series as Krzysztof suggested; one for the
> controller and one for the phy, so the series title might
> change in the next version.
> 

Ok. I'll review them after you push out the new changes.

BR,
Thinh

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

* Re: [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY
  2025-10-15 18:57         ` Roy Luo
@ 2025-10-17 23:57           ` Roy Luo
  0 siblings, 0 replies; 19+ messages in thread
From: Roy Luo @ 2025-10-17 23:57 UTC (permalink / raw)
  To: Rob Herring
  Cc: Krzysztof Kozlowski, Vinod Koul, Kishon Vijay Abraham I,
	Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
	Thinh Nguyen, Philipp Zabel, Peter Griffin, André Draszik,
	Tudor Ambarus, Joy Chakraborty, Naveen Kumar,
	Badhri Jagan Sridharan, linux-phy, devicetree, linux-kernel,
	linux-usb, linux-arm-kernel, linux-samsung-soc

On Wed, Oct 15, 2025 at 11:57 AM Roy Luo <royluo@google.com> wrote:
>
> On Wed, Oct 15, 2025 at 6:05 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Mon, Oct 13, 2025 at 06:46:39PM -0700, Roy Luo wrote:
> > > On Fri, Oct 10, 2025 at 5:11 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > >
> > > > On 10/10/2025 22:16, Roy Luo wrote:
> > > > > +  reg:
> > > > > +    items:
> > > > > +      - description: USB2 PHY configuration registers.
> > > > > +      - description: DisplayPort top-level registers.
> > > > > +      - description: USB top-level configuration registers.
> > > > > +
> > > > > +  reg-names:
> > > > > +    items:
> > > > > +      - const: u2phy_cfg
> > > > > +      - const: dp_top
> > > > > +      - const: usb_top_cfg
> > > > > +
> > > > > +  "#phy-cells":
> > > > > +    const: 1
> > > > > +
> > > > > +  clocks:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  resets:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  power-domains:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  orientation-switch:
> > > > > +    type: boolean
> > > > > +    description:
> > > > > +      Indicates the PHY as a handler of USB Type-C orientation changes
> > > > > +
> > > > > +required:
> > > > > +  - compatible
> > > > > +  - reg
> > > > > +  - reg-names
> > > > > +  - "#phy-cells"
> > > > > +  - clocks
> > > > > +  - resets
> > > > > +  - power-domains
> > > > > +  - orientation-switch
> > > > > +
> > > > > +additionalProperties: false
> > > > > +
> > > > > +examples:
> > > > > +  - |
> > > > > +    soc {
> > > > > +        #address-cells = <2>;
> > > > > +        #size-cells = <2>;
> > > > > +
> > > > > +        usb_phy: usb_phy@c410000 {
> > > > > +            compatible = "google,gs5-usb-phy";
> > > > > +            reg = <0 0x0c450014 0 0xc>,
> > > > > +                  <0 0x0c637000 0 0xa0>,
> > > >
> > > > You probably miss DP support and this does not belong here.
> > >
> > > This register space isn't solely for DP operation, a significant portion
> > > manages the custom combo PHY. Consequently, this space is essential
> > > even for USB-only operation. We can expect more registers in the space
> > > to be utilized when DP support is added.
> > >
> > > While I acknowledge the current name is confusing, it directly reflects
> > > the hardware documentation. We can either adhere to the hardware
> > > documentation's naming or propose a more descriptive alternative.
> > > What's your preference?
> > >
> > > >
> > > > > +                  <0 0x0c45002c 0 0x4>;
> > > >
> > > > That's not a separate address space. I really, really doubt that
> > > > hardware engineers came with address spaces of one word long.
> > >
> > > I initially created this space to access the usb2only mode register,
> > > which must be programmed when the controller operates in high-speed
> > > only mode without the USB3 PHY initialized. Upon review, I now
> > > believe the controller driver is the better location for this configuration,
> > > as the register logically belongs there and the controller can tell
> > > whether usb3 phy is going to be initialized.
> > >
> > > That is, I'm removing this register space in the next patch.
> >
> > You are missing the point. What exists from 0x0c450020-2c and
> > 0x0c450000-0x14 for that matter? Hardware blocks don't just start on
> > unaligned boundaries like 0x14 or 0x2c. DT describes the h/w blocks, not
>
> Rob,
>
> Thanks for chiming in. Let me elaborate the register layout here:
> The register space 0x0c450000 - 0x00450043 is supposed to
> be assigned to the USB controller. However, the USB phy has
> to access a small portion of it, i.e. 0x0c450014 - 0x0c450020,
> in order to initialize usb2 phy. This is really unfortunate and
> makes things more complicated than it should've been.
>
> The current patch is addressing it by splitting the register space:
> - USB phy: <0 0x0c450014 0 0xc>
> - USB controller: <0 0x0c450000 0 0x14>, <0 0x0c450020 0 0x23>
>
> > just nodes of what a driver needs. So if the 0x2c register needs to be
> > accessed by the USB driver, that's fine, but the register doesn't go in
> > the USB controller node 'reg'. A property with a phandle to the node
> > defining all the 0x0c450000 registers and an offset (if needed) is
> > typically what we do there. Or you can just find that node by
> > compatible.
>
> Just to make sure we're on the same page, are you suggesting
> making the register space a syscon node [1]? something like this:
>
> usb_cfg_csr: usb_cfg_csr@c450000 {
>   compatible = "syscon";
>   reg = <0 0x0c450000 0x0 0x43>;
> };
>
> usb@c400000 {
>   ...
>   usb-cfg-syscon = <&usb_cfg_csr>;
>   ...
> };
>
> usb_phy@c637000 {
>   ...
>   usb-cfg-syscon = <&usb_cfg_csr>;
>   ...
> }
>
> [1] Documentation/devicetree/bindings/mfd/syscon.yaml
>
> Thanks,
> Roy Luo
>

Hi Rob,

I'm sending out a new version of the patchset.
Although this specific comment isn't fully resolved yet, I've integrated
enough new content and fixes into this and other patches that I believe
it's ready for another round of review.

I'm happy to continue the discussion on this specific point here or in
the new patchset. Here are the links to the new patches FYI:
- controller https://lore.kernel.org/linux-usb/20251017233459.2409975-1-royluo@google.com
- phy https://lore.kernel.org/linux-phy/20251017235159.2417576-1-royluo@google.com

Thanks,
Roy Luo

> >
> > Rob

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

end of thread, other threads:[~2025-10-17 23:58 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-10 20:16 [PATCH v3 0/4] Add Google Tensor SoC USB support Roy Luo
2025-10-10 20:16 ` [PATCH v3 1/4] dt-bindings: usb: dwc3: Add Google Tensor G5 DWC3 Roy Luo
2025-10-11  0:08   ` Krzysztof Kozlowski
2025-10-14  1:40     ` Roy Luo
2025-10-14  8:22       ` Krzysztof Kozlowski
2025-10-15  0:50         ` Roy Luo
2025-10-15  8:59           ` Conor Dooley
2025-10-15 17:13             ` Roy Luo
2025-10-10 20:16 ` [PATCH v3 2/4] usb: dwc3: Add Google Tensor SoC DWC3 glue driver Roy Luo
2025-10-15  0:27   ` Thinh Nguyen
2025-10-15 17:39     ` Roy Luo
2025-10-16 22:17       ` Thinh Nguyen
2025-10-10 20:16 ` [PATCH v3 3/4] dt-bindings: phy: google: Add Google Tensor G5 USB PHY Roy Luo
2025-10-11  0:10   ` Krzysztof Kozlowski
2025-10-14  1:46     ` Roy Luo
2025-10-15 13:05       ` Rob Herring
2025-10-15 18:57         ` Roy Luo
2025-10-17 23:57           ` Roy Luo
2025-10-10 20:16 ` [PATCH v3 4/4] phy: Add Google Tensor SoC USB PHY driver Roy Luo

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