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-googHi,
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
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
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
>
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
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
© 2016 - 2025 Red Hat, Inc.