[PATCH v5 0/2] Add Google Tensor SoC USB PHY support

[PATCH v5 0/2] Add Google Tensor SoC USB PHY support

[Roy Luo]

This series introduces USB 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. The controller
support is sent as a separate patch series.

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 v5:
- Add usb3 registers/clks/resets to binding as suggested by Krzysztof
  Kozlowski. This ensures completeness of the binding, though the
  driver has not yet ultilized the resources. The usb3 clks and resets
  are optional if usb2-only operation is desired, this is denoted by
  minItems and descriptions in the clocks and resets properties.
  Additionally, rename existing binding entries for consistency and to
  better differntiate between usb2 and usb3.
- Move the description of the phy select to phy-cells in binding as
  suggested by Krzysztof Kozlowski.
Link to v4: https://lore.kernel.org/linux-phy/20251017235159.2417576-1-royluo@google.com

Changes in v4:
- Separate controller and phy changes into two distinct patch series.
- Remove usb2only mode configuration and the corresponding usb_top_cfg
  reg (moved to controller)
- Add more descriptions to dp_top reg to indicate it's not DP specific.
- Add u2phy_apb clk/reset
Link to v3: https://lore.kernel.org/linux-usb/20251010201607.1190967-1-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 (2):
  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      | 127 ++++++++
 drivers/phy/Kconfig                           |  13 +
 drivers/phy/Makefile                          |   1 +
 drivers/phy/phy-google-usb.c                  | 271 ++++++++++++++++++
 4 files changed, 412 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
 create mode 100644 drivers/phy/phy-google-usb.c


base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
-- 
2.51.1.851.g4ebd6896fd-goog

[PATCH v5 1/2] dt-bindings: phy: google: Add Google Tensor G5 USB PHY

[Roy Luo]

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      | 127 ++++++++++++++++++
 1 file changed, 127 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..8a590036fbac
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
@@ -0,0 +1,127 @@
+# 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.
+
+properties:
+  compatible:
+    const: google,gs5-usb-phy
+
+  reg:
+    items:
+      - description: USB3.2/DisplayPort combo PHY core registers.
+      - description: USB3.2/DisplayPort combo PHY Type-C Assist registers.
+      - description: USB2 PHY configuration registers.
+      - description: USB3.2/DisplayPort combo PHY top-level registers.
+
+  reg-names:
+    items:
+      - const: usb3_core
+      - const: usb3_tca
+      - const: usb2_cfg
+      - const: usb3_top
+
+  "#phy-cells":
+    description: |
+      The phandle's argument in the PHY specifier selects one of the three
+      following PHY interfaces.
+      - 0 for USB high-speed.
+      - 1 for USB super-speed.
+      - 2 for DisplayPort.
+    const: 1
+
+  clocks:
+    minItems: 2
+    items:
+      - description: USB2 PHY clock.
+      - description: USB2 PHY APB clock.
+      - description: USB3.2/DisplayPort combo PHY clock.
+      - description: USB3.2/DisplayPort combo PHY firmware clock.
+    description:
+      USB3 clocks are optional if the device is intended for USB2-only
+      operation.
+
+  clock-names:
+    minItems: 2
+    items:
+      - const: usb2
+      - const: usb2_apb
+      - const: usb3
+      - const: usb3_fw
+
+  resets:
+    minItems: 2
+    items:
+      - description: USB2 PHY reset.
+      - description: USB2 PHY APB reset.
+      - description: USB3.2/DisplayPort combo PHY reset.
+    description:
+      USB3 clocks are optional if the device is intended for USB2-only
+      operation.
+
+  reset-names:
+    minItems: 2
+    items:
+      - const: usb2
+      - const: usb2_apb
+      - const: usb3
+
+  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
+  - clock-names
+  - resets
+  - reset-names
+  - 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 0x0c410000 0 0x20000>,
+                  <0 0x0c430000 0 0x1000>,
+                  <0 0x0c450014 0 0xc>,
+                  <0 0x0c637000 0 0xa0>;
+            reg-names = "usb3_core", "usb3_tca", "usb2_cfg", "usb3_top";
+            #phy-cells = <1>;
+            clocks = <&hsion_usb2_phy_clk>, <&hsion_u2phy_apb_clk>;
+            clock-names = "usb2", "usb2_apb";
+            resets = <&hsion_resets_usb2_phy>,
+                     <&hsion_resets_u2phy_apb>;
+            reset-names = "usb2", "usb2_apb";
+            power-domains = <&hsio_n_usb_pd>;
+            orientation-switch;
+        };
+    };
+...
-- 
2.51.1.851.g4ebd6896fd-goog

[PATCH v5 2/2] phy: Add Google Tensor SoC USB PHY driver

[Roy Luo]

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          |  13 ++
 drivers/phy/Makefile         |   1 +
 drivers/phy/phy-google-usb.c | 271 +++++++++++++++++++++++++++++++++++
 3 files changed, 285 insertions(+)
 create mode 100644 drivers/phy/phy-google-usb.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 58c911e1b2d2..be237847efed 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -101,6 +101,19 @@ 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
+	select GENERIC_PHY
+	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 DRD 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..02c6e9f2912e
--- /dev/null
+++ b/drivers/phy/phy-google-usb.c
@@ -0,0 +1,271 @@
+// 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)
+
+enum google_usb_phy_id {
+	GOOGLE_USB2_PHY,
+	GOOGLE_USB_PHY_NUM,
+};
+
+struct google_usb_phy_instance {
+	int index;
+	struct phy *phy;
+	int num_clks;
+	struct clk_bulk_data *clks;
+	struct reset_control *rsts;
+};
+
+struct google_usb_phy {
+	struct device *dev;
+	void __iomem *usb2_cfg_base;
+	void __iomem *usb3_top_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->usb3_top_base + USBCS_PHY_CFG1_OFFSET);
+		reg &= ~USBCS_PHY_CFG1_SYS_VBUSVALID;
+		writel(reg, gphy->usb3_top_base + USBCS_PHY_CFG1_OFFSET);
+	} else {
+		reg = readl(gphy->usb3_top_base + USBCS_PHY_CFG1_OFFSET);
+		reg |= USBCS_PHY_CFG1_SYS_VBUSVALID;
+		writel(reg, gphy->usb3_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);
+
+	reg = readl(gphy->usb2_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->usb2_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+
+	reg = readl(gphy->usb2_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->usb2_cfg_base + USBCS_USB2PHY_CFG19_OFFSET);
+
+	set_vbus_valid(gphy);
+
+	ret = clk_bulk_prepare_enable(inst->num_clks, inst->clks);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(inst->rsts);
+	if (ret) {
+		clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
+		return ret;
+	}
+
+	reg = readl(gphy->usb2_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+	reg |= USBCS_USB2PHY_CFG21_PHY_ENABLE;
+	writel(reg, gphy->usb2_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->usb2_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+	reg &= ~USBCS_USB2PHY_CFG21_PHY_ENABLE;
+	writel(reg, gphy->usb2_cfg_base + USBCS_USB2PHY_CFG21_OFFSET);
+
+	reset_control_assert(inst->rsts);
+	clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
+
+	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->usb2_cfg_base = devm_platform_ioremap_resource_byname(pdev,
+								    "usb2_cfg");
+	if (IS_ERR(gphy->usb2_cfg_base))
+		return dev_err_probe(dev, PTR_ERR(gphy->usb2_cfg_base),
+				    "invalid usb2 cfg\n");
+
+	gphy->usb3_top_base = devm_platform_ioremap_resource_byname(pdev,
+								    "usb3_top");
+	if (IS_ERR(gphy->usb3_top_base))
+		return dev_err_probe(dev, PTR_ERR(gphy->usb3_top_base),
+				    "invalid usb3 top\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);
+	ret = devm_clk_bulk_get_all_enabled(dev, &inst->clks);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to get u2 phy clks\n");
+	inst->num_clks = ret;
+
+	inst->rsts = devm_reset_control_array_get_exclusive(dev);
+	if (IS_ERR(inst->rsts))
+		return dev_err_probe(dev, PTR_ERR(inst->rsts),
+				     "failed to get u2 phy resets\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.1.851.g4ebd6896fd-goog

Re: [PATCH v5 1/2] dt-bindings: phy: google: Add Google Tensor G5 USB PHY

[Krzysztof Kozlowski]

On Wed, Oct 29, 2025 at 09:40:31PM +0000, Roy Luo wrote:
> 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      | 127 ++++++++++++++++++
>  1 file changed, 127 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..8a590036fbac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
> @@ -0,0 +1,127 @@
> +# 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.
> +
> +properties:
> +  compatible:
> +    const: google,gs5-usb-phy
> +
> +  reg:
> +    items:
> +      - description: USB3.2/DisplayPort combo PHY core registers.
> +      - description: USB3.2/DisplayPort combo PHY Type-C Assist registers.
> +      - description: USB2 PHY configuration registers.
> +      - description: USB3.2/DisplayPort combo PHY top-level registers.
> +
> +  reg-names:
> +    items:
> +      - const: usb3_core
> +      - const: usb3_tca
> +      - const: usb2_cfg
> +      - const: usb3_top

These prefixes are redundant. Also, you are still referencing here
completely different devices. MMIO of IP blocks do not have size of 0xc
and they do not span over other blocks (0x0c410000 and then next one is
0x0c637000).


> +            reg = <0 0x0c410000 0 0x20000>,
> +                  <0 0x0c430000 0 0x1000>,
> +                  <0 0x0c450014 0 0xc>,
> +                  <0 0x0c637000 0 0xa0>;
> +
> +  "#phy-cells":
> +    description: |
> +      The phandle's argument in the PHY specifier selects one of the three
> +      following PHY interfaces.
> +      - 0 for USB high-speed.
> +      - 1 for USB super-speed.
> +      - 2 for DisplayPort.
> +    const: 1
> +
> +  clocks:
> +    minItems: 2
> +    items:
> +      - description: USB2 PHY clock.
> +      - description: USB2 PHY APB clock.
> +      - description: USB3.2/DisplayPort combo PHY clock.
> +      - description: USB3.2/DisplayPort combo PHY firmware clock.
> +    description:
> +      USB3 clocks are optional if the device is intended for USB2-only
> +      operation.

No, they are not. SoCs are not made that internal wires are being
disconnected when you solder it to a different board.

I stopped reviewing here.

You are making unusual, unexpected big changes after v4. At v4 you
received only few nits because the review process was about to finish.

Now you rewrite everything so you ask me to re-review from scratch.

> +
> +  clock-names:
> +    minItems: 2
> +    items:
> +      - const: usb2
> +      - const: usb2_apb
> +      - const: usb3
> +      - const: usb3_fw

Again, what's with the prefixes? apb is bus clock, so how you could have
bus clock for usb2 and usb3? This means you have two busses, so two
devices.

> +
> +  resets:
> +    minItems: 2
> +    items:
> +      - description: USB2 PHY reset.
> +      - description: USB2 PHY APB reset.
> +      - description: USB3.2/DisplayPort combo PHY reset.
> +    description:
> +      USB3 clocks are optional if the device is intended for USB2-only
> +      operation.
> +
> +  reset-names:
> +    minItems: 2
> +    items:
> +      - const: usb2
> +      - const: usb2_apb
> +      - const: usb3

And here?

This all looks like downstream code.

Best regards,
Krzysztof

Re: [PATCH v5 1/2] dt-bindings: phy: google: Add Google Tensor G5 USB PHY

[Roy Luo]

On Thu, Oct 30, 2025 at 12:37 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, Oct 29, 2025 at 09:40:31PM +0000, Roy Luo wrote:
> > 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      | 127 ++++++++++++++++++
> >  1 file changed, 127 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..8a590036fbac
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
> > @@ -0,0 +1,127 @@
> > +# 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.
> > +
> > +properties:
> > +  compatible:
> > +    const: google,gs5-usb-phy
> > +
> > +  reg:
> > +    items:
> > +      - description: USB3.2/DisplayPort combo PHY core registers.
> > +      - description: USB3.2/DisplayPort combo PHY Type-C Assist registers.
> > +      - description: USB2 PHY configuration registers.
> > +      - description: USB3.2/DisplayPort combo PHY top-level registers.
> > +
> > +  reg-names:
> > +    items:
> > +      - const: usb3_core
> > +      - const: usb3_tca
> > +      - const: usb2_cfg
> > +      - const: usb3_top
>
> These prefixes are redundant. Also, you are still referencing here
> completely different devices. MMIO of IP blocks do not have size of 0xc
> and they do not span over other blocks (0x0c410000 and then next one is
> 0x0c637000).

I'd like to explain why MMIO of IP blocks looks discontinuous.
As outlined in the description, this device contains two SNPS PHY IPs
including eUSB2 PHY and USB3.2/DP combo PHY, and is integrated
with the SNPS DWC3 USB controller. A top-level subsystem wrapper
sits above the PHYs and controller. This wrapper integrates these IPs
and is where the Tensor-specific implementation resides. It's essential
to touch these wrapper registers to control the underlying SNSP IPs.
Unfortunately, the top-level wrapper's MMIO space lacks a clear
boundary between these IPs. Specifically, the registers required to
configure a particular IP are not always adjacent to that IP, and in
some cases, multiple IPs may even share the same address space.

The following is the register layout overview:
- 0xc400000: Dedicated address space for DWC3 controller IP.
- 0xc410000: Dedicated address space for USB3.2/DP combo PHY IP.
- 0cc440000: Dedicated address space for the eUSB2 PHY IP.
                      While this is not in use, it should perhaps be
called out in
                      the binding for completeness.
- 0xc450000: This address range contains top-level wrapper registers
                      and its space is shared by two devices: the DWC3
                      controller and the eUSB2 PHY.
                      It includes control registers for the DWC3 controller
                      (e.g. hibernation control and interrupt registers) and
                      the eUSB2 PHY (e.g. registers for USB2 PHY
                      frequency configuration).
                      Because the space is shared, the MMIO range for the
                      PHY becomes fragmented and is only allocated a size
                      of 0xc, as the remaining registers in this range are
                      assigned to the DWC3 controller.
- 0xc460000: This address range contains registers for other blocks
                      within the same top-level wrapper (such as PCIe PHY
                      and DP controller) which are not relevant to USB.
- 0xc637000: Another region of top-level wrapper registers.
                      This area is relevant to both the eUSB2 PHY IP
                      (e.g. control register for vbus valid) and USB3.2/DP
                      combo PHY (e.g. registers relevant to PHY firmware).

Thanks for taking the time to go through this wall of text.
This is definitely not an ideal register layout, but I'm open
to any suggestions on how best to address this fragmentation.
If discontinuous MMIO space is a concern, does it make sense to
make the wrapper registers a syscon node so that it can be
shared by multiple devices?

>
>
> > +            reg = <0 0x0c410000 0 0x20000>,
> > +                  <0 0x0c430000 0 0x1000>,
> > +                  <0 0x0c450014 0 0xc>,
> > +                  <0 0x0c637000 0 0xa0>;
> > +
> > +  "#phy-cells":
> > +    description: |
> > +      The phandle's argument in the PHY specifier selects one of the three
> > +      following PHY interfaces.
> > +      - 0 for USB high-speed.
> > +      - 1 for USB super-speed.
> > +      - 2 for DisplayPort.
> > +    const: 1
> > +
> > +  clocks:
> > +    minItems: 2
> > +    items:
> > +      - description: USB2 PHY clock.
> > +      - description: USB2 PHY APB clock.
> > +      - description: USB3.2/DisplayPort combo PHY clock.
> > +      - description: USB3.2/DisplayPort combo PHY firmware clock.
> > +    description:
> > +      USB3 clocks are optional if the device is intended for USB2-only
> > +      operation.
>
> No, they are not. SoCs are not made that internal wires are being
> disconnected when you solder it to a different board.
>
> I stopped reviewing here.
>
> You are making unusual, unexpected big changes after v4. At v4 you
> received only few nits because the review process was about to finish.
>
> Now you rewrite everything so you ask me to re-review from scratch.

Apologies for the trouble, my intent was to address your feedback on v4
by describing the USB3/DP PHY block for completeness.
Like mentioned earlier, this device contains two underlying IPs: eUSB2
PHY and USB3.2/DP combo PHY. The device can operate in USB2-only
mode by initializing just the eUSB2 block without touching the USB3
PHY block - but not the other way around. The v4 patch reflected this
USB2-only configuration.
I tried to support the USB 2.0-only configuration in the binding by
making the USB 3.0 clocks and resets optional. However, if I
understand your comment correctly, the binding should describe
FULL hardware capability. I will make USB3 resources mandatory
in the next version, please let me know if I've misunderstood it.

>
> > +
> > +  clock-names:
> > +    minItems: 2
> > +    items:
> > +      - const: usb2
> > +      - const: usb2_apb
> > +      - const: usb3
> > +      - const: usb3_fw
>
> Again, what's with the prefixes? apb is bus clock, so how you could have
> bus clock for usb2 and usb3? This means you have two busses, so two
> devices.

The prefixes are to differentiate the clocks and resets for the
underlying two SNPS IP as outlined in the device description.
- eUSB2 PHY IP needs clocks and resets for the phy itself
  and its apb bus.
- USB3.2/DP combo PHY has its own clocks and resets for
  the phy, plus it needs a clock for its PHY firmware.
Technically these are two separate IPs, but they're deeply
integrated together so that they share top-level wrapper
register space (see the register layout above), and they
have implicit hardware dependency like mentioned earlier
(USB2 PHY can work without USB3 PHY, but not vice-versa),
hence I'm describing them in the same device.

Thanks,
Roy Luo

>
> > +
> > +  resets:
> > +    minItems: 2
> > +    items:
> > +      - description: USB2 PHY reset.
> > +      - description: USB2 PHY APB reset.
> > +      - description: USB3.2/DisplayPort combo PHY reset.
> > +    description:
> > +      USB3 clocks are optional if the device is intended for USB2-only
> > +      operation.
> > +
> > +  reset-names:
> > +    minItems: 2
> > +    items:
> > +      - const: usb2
> > +      - const: usb2_apb
> > +      - const: usb3
>
> And here?
>
> This all looks like downstream code.
>
> Best regards,
> Krzysztof
>

Re: [PATCH v5 1/2] dt-bindings: phy: google: Add Google Tensor G5 USB PHY

[Krzysztof Kozlowski]

On 01/11/2025 00:45, Roy Luo wrote:
> On Thu, Oct 30, 2025 at 12:37 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Wed, Oct 29, 2025 at 09:40:31PM +0000, Roy Luo wrote:
>>> 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      | 127 ++++++++++++++++++
>>>  1 file changed, 127 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..8a590036fbac
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
>>> @@ -0,0 +1,127 @@
>>> +# 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.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: google,gs5-usb-phy
>>> +
>>> +  reg:
>>> +    items:
>>> +      - description: USB3.2/DisplayPort combo PHY core registers.
>>> +      - description: USB3.2/DisplayPort combo PHY Type-C Assist registers.
>>> +      - description: USB2 PHY configuration registers.
>>> +      - description: USB3.2/DisplayPort combo PHY top-level registers.
>>> +
>>> +  reg-names:
>>> +    items:
>>> +      - const: usb3_core
>>> +      - const: usb3_tca
>>> +      - const: usb2_cfg
>>> +      - const: usb3_top
>>
>> These prefixes are redundant. Also, you are still referencing here
>> completely different devices. MMIO of IP blocks do not have size of 0xc
>> and they do not span over other blocks (0x0c410000 and then next one is
>> 0x0c637000).
> 
> I'd like to explain why MMIO of IP blocks looks discontinuous.
> As outlined in the description, this device contains two SNPS PHY IPs
> including eUSB2 PHY and USB3.2/DP combo PHY, and is integrated
> with the SNPS DWC3 USB controller. A top-level subsystem wrapper
> sits above the PHYs and controller. This wrapper integrates these IPs
> and is where the Tensor-specific implementation resides. It's essential
> to touch these wrapper registers to control the underlying SNSP IPs.
> Unfortunately, the top-level wrapper's MMIO space lacks a clear
> boundary between these IPs. Specifically, the registers required to
> configure a particular IP are not always adjacent to that IP, and in
> some cases, multiple IPs may even share the same address space.
> 
> The following is the register layout overview:
> - 0xc400000: Dedicated address space for DWC3 controller IP.
> - 0xc410000: Dedicated address space for USB3.2/DP combo PHY IP.
> - 0cc440000: Dedicated address space for the eUSB2 PHY IP.
>                       While this is not in use, it should perhaps be
> called out in
>                       the binding for completeness.
> - 0xc450000: This address range contains top-level wrapper registers
>                       and its space is shared by two devices: the DWC3
>                       controller and the eUSB2 PHY.
>                       It includes control registers for the DWC3 controller
>                       (e.g. hibernation control and interrupt registers) and
>                       the eUSB2 PHY (e.g. registers for USB2 PHY
>                       frequency configuration).
>                       Because the space is shared, the MMIO range for the
>                       PHY becomes fragmented and is only allocated a size
>                       of 0xc, as the remaining registers in this range are
>                       assigned to the DWC3 controller.
> - 0xc460000: This address range contains registers for other blocks
>                       within the same top-level wrapper (such as PCIe PHY
>                       and DP controller) which are not relevant to USB.
> - 0xc637000: Another region of top-level wrapper registers.
>                       This area is relevant to both the eUSB2 PHY IP
>                       (e.g. control register for vbus valid) and USB3.2/DP
>                       combo PHY (e.g. registers relevant to PHY firmware).

To me it all feels like you pick up individual registers from the
common, miscellaneous register region aka syscon.

And if that's the case then you create a narrowly constrained binding
which won't work with next generations where hardware engineers decide
to make that shared region a bigger syscon.

> 
> Thanks for taking the time to go through this wall of text.
> This is definitely not an ideal register layout, but I'm open
> to any suggestions on how best to address this fragmentation.
> If discontinuous MMIO space is a concern, does it make sense to
> make the wrapper registers a syscon node so that it can be
> shared by multiple devices?

0xc450014 looks like that. 0x0c637000, depends how other devices really
use it.

You should post somewhere full DTS for clarity. It's not a requirement
but it actually can answer several questions.

> 
>>
>>
>>> +            reg = <0 0x0c410000 0 0x20000>,
>>> +                  <0 0x0c430000 0 0x1000>,
>>> +                  <0 0x0c450014 0 0xc>,
>>> +                  <0 0x0c637000 0 0xa0>;
>>> +
>>> +  "#phy-cells":
>>> +    description: |
>>> +      The phandle's argument in the PHY specifier selects one of the three
>>> +      following PHY interfaces.
>>> +      - 0 for USB high-speed.
>>> +      - 1 for USB super-speed.
>>> +      - 2 for DisplayPort.
>>> +    const: 1
>>> +
>>> +  clocks:
>>> +    minItems: 2
>>> +    items:
>>> +      - description: USB2 PHY clock.
>>> +      - description: USB2 PHY APB clock.
>>> +      - description: USB3.2/DisplayPort combo PHY clock.
>>> +      - description: USB3.2/DisplayPort combo PHY firmware clock.
>>> +    description:
>>> +      USB3 clocks are optional if the device is intended for USB2-only
>>> +      operation.
>>
>> No, they are not. SoCs are not made that internal wires are being
>> disconnected when you solder it to a different board.
>>
>> I stopped reviewing here.
>>
>> You are making unusual, unexpected big changes after v4. At v4 you
>> received only few nits because the review process was about to finish.
>>
>> Now you rewrite everything so you ask me to re-review from scratch.
> 
> Apologies for the trouble, my intent was to address your feedback on v4
> by describing the USB3/DP PHY block for completeness.
> Like mentioned earlier, this device contains two underlying IPs: eUSB2
> PHY and USB3.2/DP combo PHY. The device can operate in USB2-only
> mode by initializing just the eUSB2 block without touching the USB3
> PHY block - but not the other way around. The v4 patch reflected this
> USB2-only configuration.

You describe the device in your SoC. This SoC either has both or has
not. The case of "can operate in USB2-only mode" is simply not real.

> I tried to support the USB 2.0-only configuration in the binding by
> making the USB 3.0 clocks and resets optional. However, if I
> understand your comment correctly, the binding should describe
> FULL hardware capability. I will make USB3 resources mandatory
> in the next version, please let me know if I've misunderstood it.

Yes, binding should describe complete hardware picture, so with USB3 and
all wires/signals being required.

> 
>>
>>> +
>>> +  clock-names:
>>> +    minItems: 2
>>> +    items:
>>> +      - const: usb2
>>> +      - const: usb2_apb
>>> +      - const: usb3
>>> +      - const: usb3_fw
>>
>> Again, what's with the prefixes? apb is bus clock, so how you could have
>> bus clock for usb2 and usb3? This means you have two busses, so two
>> devices.
> 
> The prefixes are to differentiate the clocks and resets for the
> underlying two SNPS IP as outlined in the device description.
> - eUSB2 PHY IP needs clocks and resets for the phy itself
>   and its apb bus.
> - USB3.2/DP combo PHY has its own clocks and resets for
>   the phy, plus it needs a clock for its PHY firmware.

If you have two bus clocks I think you have two separate devices...

> Technically these are two separate IPs, but they're deeply
> integrated together so that they share top-level wrapper
> register space (see the register layout above), and they
> have implicit hardware dependency like mentioned earlier
> (USB2 PHY can work without USB3 PHY, but not vice-versa),
> hence I'm describing them in the same device.

But okay, if that's the case naming is fine.


Best regards,
Krzysztof

Re: [PATCH v5 1/2] dt-bindings: phy: google: Add Google Tensor G5 USB PHY

[Roy Luo]

On Wed, Nov 5, 2025 at 3:35 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 01/11/2025 00:45, Roy Luo wrote:
> > On Thu, Oct 30, 2025 at 12:37 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Wed, Oct 29, 2025 at 09:40:31PM +0000, Roy Luo wrote:
> >>> 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      | 127 ++++++++++++++++++
> >>>  1 file changed, 127 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..8a590036fbac
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
> >>> @@ -0,0 +1,127 @@
> >>> +# 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.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    const: google,gs5-usb-phy
> >>> +
> >>> +  reg:
> >>> +    items:
> >>> +      - description: USB3.2/DisplayPort combo PHY core registers.
> >>> +      - description: USB3.2/DisplayPort combo PHY Type-C Assist registers.
> >>> +      - description: USB2 PHY configuration registers.
> >>> +      - description: USB3.2/DisplayPort combo PHY top-level registers.
> >>> +
> >>> +  reg-names:
> >>> +    items:
> >>> +      - const: usb3_core
> >>> +      - const: usb3_tca
> >>> +      - const: usb2_cfg
> >>> +      - const: usb3_top
> >>
> >> These prefixes are redundant. Also, you are still referencing here
> >> completely different devices. MMIO of IP blocks do not have size of 0xc
> >> and they do not span over other blocks (0x0c410000 and then next one is
> >> 0x0c637000).
> >
> > I'd like to explain why MMIO of IP blocks looks discontinuous.
> > As outlined in the description, this device contains two SNPS PHY IPs
> > including eUSB2 PHY and USB3.2/DP combo PHY, and is integrated
> > with the SNPS DWC3 USB controller. A top-level subsystem wrapper
> > sits above the PHYs and controller. This wrapper integrates these IPs
> > and is where the Tensor-specific implementation resides. It's essential
> > to touch these wrapper registers to control the underlying SNSP IPs.
> > Unfortunately, the top-level wrapper's MMIO space lacks a clear
> > boundary between these IPs. Specifically, the registers required to
> > configure a particular IP are not always adjacent to that IP, and in
> > some cases, multiple IPs may even share the same address space.
> >
> > The following is the register layout overview:
> > - 0xc400000: Dedicated address space for DWC3 controller IP.
> > - 0xc410000: Dedicated address space for USB3.2/DP combo PHY IP.
> > - 0cc440000: Dedicated address space for the eUSB2 PHY IP.
> >                       While this is not in use, it should perhaps be
> > called out in
> >                       the binding for completeness.
> > - 0xc450000: This address range contains top-level wrapper registers
> >                       and its space is shared by two devices: the DWC3
> >                       controller and the eUSB2 PHY.
> >                       It includes control registers for the DWC3 controller
> >                       (e.g. hibernation control and interrupt registers) and
> >                       the eUSB2 PHY (e.g. registers for USB2 PHY
> >                       frequency configuration).
> >                       Because the space is shared, the MMIO range for the
> >                       PHY becomes fragmented and is only allocated a size
> >                       of 0xc, as the remaining registers in this range are
> >                       assigned to the DWC3 controller.
> > - 0xc460000: This address range contains registers for other blocks
> >                       within the same top-level wrapper (such as PCIe PHY
> >                       and DP controller) which are not relevant to USB.
> > - 0xc637000: Another region of top-level wrapper registers.
> >                       This area is relevant to both the eUSB2 PHY IP
> >                       (e.g. control register for vbus valid) and USB3.2/DP
> >                       combo PHY (e.g. registers relevant to PHY firmware).
>
> To me it all feels like you pick up individual registers from the
> common, miscellaneous register region aka syscon.
>
> And if that's the case then you create a narrowly constrained binding
> which won't work with next generations where hardware engineers decide
> to make that shared region a bigger syscon.

Ack. Will go with syscon.

>
> >
> > Thanks for taking the time to go through this wall of text.
> > This is definitely not an ideal register layout, but I'm open
> > to any suggestions on how best to address this fragmentation.
> > If discontinuous MMIO space is a concern, does it make sense to
> > make the wrapper registers a syscon node so that it can be
> > shared by multiple devices?
>
> 0xc450014 looks like that. 0x0c637000, depends how other devices really
> use it.

Agree it makes sense to make region 0xc450000 a syscon node provided
it's shared by two distinct devices (the controller and the PHY). I will
implement this change in the next version.
A heads up, I will also need to send a new version for the corresponding
dwc3 controller binding patch that has already been reviewed [1] to reflect
this syscon change.

As for region 0x0c637000, this range is exclusive to this PHY device, which
includes both eUSB2 PHY and USB3.2/DP combo PHY. (It would be a
different story if the USB2 PHY and the USB3 PHY were to be treated as
two distinct devices.) Therefore, I'm hesitant to convert this region to a
syscon node. I recommend keeping the region as-is and add a more
detailed description for this reg entry to clarify that this is top-level
subsystem wrapper registers distinct from the core IP's register space.

[1] https://lore.kernel.org/linux-usb/20251017233459.2409975-2-royluo@google.com/

>
> You should post somewhere full DTS for clarity. It's not a requirement
> but it actually can answer several questions.

Yes, I can definitely share the DTS I'm using to test this PHY and controller
patch series. Could you recommend the most appropriate way to do so?
I came across a previous patch thread [2] that used gist link for sharing code
snippets and logs. Is that generally considered acceptable?

[2] https://lore.kernel.org/all/20240715120936.1150314-1-s-vadapalli@ti.com/

>
> >
> >>
> >>
> >>> +            reg = <0 0x0c410000 0 0x20000>,
> >>> +                  <0 0x0c430000 0 0x1000>,
> >>> +                  <0 0x0c450014 0 0xc>,
> >>> +                  <0 0x0c637000 0 0xa0>;
> >>> +
> >>> +  "#phy-cells":
> >>> +    description: |
> >>> +      The phandle's argument in the PHY specifier selects one of the three
> >>> +      following PHY interfaces.
> >>> +      - 0 for USB high-speed.
> >>> +      - 1 for USB super-speed.
> >>> +      - 2 for DisplayPort.
> >>> +    const: 1
> >>> +
> >>> +  clocks:
> >>> +    minItems: 2
> >>> +    items:
> >>> +      - description: USB2 PHY clock.
> >>> +      - description: USB2 PHY APB clock.
> >>> +      - description: USB3.2/DisplayPort combo PHY clock.
> >>> +      - description: USB3.2/DisplayPort combo PHY firmware clock.
> >>> +    description:
> >>> +      USB3 clocks are optional if the device is intended for USB2-only
> >>> +      operation.
> >>
> >> No, they are not. SoCs are not made that internal wires are being
> >> disconnected when you solder it to a different board.
> >>
> >> I stopped reviewing here.
> >>
> >> You are making unusual, unexpected big changes after v4. At v4 you
> >> received only few nits because the review process was about to finish.
> >>
> >> Now you rewrite everything so you ask me to re-review from scratch.
> >
> > Apologies for the trouble, my intent was to address your feedback on v4
> > by describing the USB3/DP PHY block for completeness.
> > Like mentioned earlier, this device contains two underlying IPs: eUSB2
> > PHY and USB3.2/DP combo PHY. The device can operate in USB2-only
> > mode by initializing just the eUSB2 block without touching the USB3
> > PHY block - but not the other way around. The v4 patch reflected this
> > USB2-only configuration.
>
> You describe the device in your SoC. This SoC either has both or has
> not. The case of "can operate in USB2-only mode" is simply not real.

Ack.

>
> > I tried to support the USB 2.0-only configuration in the binding by
> > making the USB 3.0 clocks and resets optional. However, if I
> > understand your comment correctly, the binding should describe
> > FULL hardware capability. I will make USB3 resources mandatory
> > in the next version, please let me know if I've misunderstood it.
>
> Yes, binding should describe complete hardware picture, so with USB3 and
> all wires/signals being required.

Ack.

>
> >
> >>
> >>> +
> >>> +  clock-names:
> >>> +    minItems: 2
> >>> +    items:
> >>> +      - const: usb2
> >>> +      - const: usb2_apb
> >>> +      - const: usb3
> >>> +      - const: usb3_fw
> >>
> >> Again, what's with the prefixes? apb is bus clock, so how you could have
> >> bus clock for usb2 and usb3? This means you have two busses, so two
> >> devices.
> >
> > The prefixes are to differentiate the clocks and resets for the
> > underlying two SNPS IP as outlined in the device description.
> > - eUSB2 PHY IP needs clocks and resets for the phy itself
> >   and its apb bus.
> > - USB3.2/DP combo PHY has its own clocks and resets for
> >   the phy, plus it needs a clock for its PHY firmware.
>
> If you have two bus clocks I think you have two separate devices...
>
> > Technically these are two separate IPs, but they're deeply
> > integrated together so that they share top-level wrapper
> > register space (see the register layout above), and they
> > have implicit hardware dependency like mentioned earlier
> > (USB2 PHY can work without USB3 PHY, but not vice-versa),
> > hence I'm describing them in the same device.
>
> But okay, if that's the case naming is fine.

Thanks,
Roy Luo

>
>
> Best regards,
> Krzysztof

Re: [PATCH v5 1/2] dt-bindings: phy: google: Add Google Tensor G5 USB PHY

[Doug Anderson]

Hi,

On Wed, Oct 29, 2025 at 2:40 PM Roy Luo <royluo@google.com> wrote:
>
> 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      | 127 ++++++++++++++++++
>  1 file changed, 127 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..8a590036fbac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/google,gs5-usb-phy.yaml
> @@ -0,0 +1,127 @@
> +# 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.
> +
> +properties:
> +  compatible:
> +    const: google,gs5-usb-phy

FWIW, we've had some rather heated bikeshedding at Google about the
use of "gs5" to refer to this processor.

* The processor is almost exclusively referred to as "lga" in code at Google.

* The processor's code name is "laguna".

* Nobody is aware of the processor being referred to as "gs5"
internally. Though this is the 5th Google Silicon ("GS") processor, so
it makes some sense, "gs5" is not really an official name for it. At
least one person pointed to the fact that it's a tad bit confusing
that the first generation Tensor processor is called "gs101" upstream
and the fifth generation is called "gs5".

* Some folks proposed "gs501" to match the "gs101, gs201, ..." trend.
The first two Tensor processors were definitely called "gs101" and
"gs201" and the next two were referred to as "gs301" and "gs401" in
some internal docs, though this was discouraged. The processor in
Pixel 10 was never called "gs501" internally as far as I can tell.


In any case, it's a bit of a mess. The straw poll I took seemed to
land on "lga" being the preferred name to continue to refer to this
processor in upstream code. Would it be possible to change from "gs5"
to "lga" here? The "laguna" code name for this processor is well known
publicly and it's generally quite common to refer to processors (and
boards) by their codenames, since codenames are often available sooner
than marketing names and also less likely to change. Indeed, I was
even CCed on a change recently where there were plans to move away
from a processor ID and back to a codename [1].


[1] http://lore.kernel.org/r/20251030-rename-dts-2-v1-2-80c0b81c4d77@oss.qualcomm.com