[PATCH v2 0/4] phy: nuvoton: extend MA35D1 USB2 PHY driver for dual-port OTG support

[PATCH v2 0/4] phy: nuvoton: extend MA35D1 USB2 PHY driver for dual-port OTG support

[Joey Lu]

The MA35D1 SoC has two USB PHY ports managed by a shared hardware block:

  - PHY0 (USB0): OTG port shared between the DWC2 gadget controller and
    the EHCI0/OHCI0 host controllers.  A hardware mux automatically routes
    USB0 signals to the correct controller based on the USB ID pin.

  - PHY1 (USB1): dedicated host-only port for EHCI1/OHCI1.

A previous series [1] added a separate phy-ma35d1-otg.c driver for this.
Following reviewer suggestion to reuse the existing phy-ma35d1-usb2.c
driver rather than introduce a new one, that series has been dropped and
this series instead extends the existing driver.

Changes in this series:

  Patch 1 (new) adds simple-mfd support to the nuvoton,ma35d1-reset
  syscon binding.  The sys node needs to act as an MFD parent so that
  usb-phy@60 can be its DT child.  This patch is a prerequisite for
  patch 2 and has no functional impact on existing users of the syscon.

  Patch 2 updates the nuvoton,ma35d1-usb2-phy binding: the PHY node
  becomes a child of the syscon node (reg = <0x60 0x14>), nuvoton,sys
  phandle and clocks are removed, and #phy-cells changes from 0 to 1
  for per-port selection.  Optional nuvoton,rcalcode and
  nuvoton,oc-active-high properties are added.

  Patch 3 updates the MA35D1 DTS: sys gains simple-mfd, usb-phy@60 is
  added as a syscon child, and DWC2/EHCI0/EHCI1/OHCI0/OHCI1 nodes are
  added.  Board files enable the nodes and add pinctrl for the HSUSB
  signals (VBUSVLD, PWREN, OVC).

  Patch 4 extends phy-ma35d1-usb2.c: a loop creates two struct phy
  objects; a unified .init handles both ports with parametric register
  macros; clock management is removed (each controller gates its own
  clock); a read-only USB role switch is registered for PHY0 reporting
  the USB ID pin via PWRONOTP[16].

Changes since v1:

  - New patch 1: nuvoton,ma35d1-reset binding extended for simple-mfd
    and child node support; required as a prerequisite for the PHY
    binding which places usb-phy@60 as a syscon child.

  - Patch 2 (was patch 1): nuvoton,rcalcode description updated to
    clarify that both PHY entries are individually optional; example
    reg corrected from 4-cell to 2-cell format.

  - Patch 3 (was patch 2): ehci1 node address corrected to lowercase
    (401c0000).

  - Patch 4 (was patch 3): register definition section header comments
    added; HOST_READY and DEV_READY macro comments made more
    descriptive; ma35_role_switch_exit() added to the PHY provider
    registration error path to prevent a role switch leak.

Link: [1] https://lore.kernel.org/linux-phy/20260604101220.1092822-1-a0987203069@gmail.com/T/#t

Joey Lu (4):
  dt-bindings: reset: nuvoton,ma35d1-reset: add simple-mfd and child
    node support
  dt-bindings: phy: nuvoton,ma35d1-usb2-phy: extend for dual-port OTG
    support
  arm64: dts: nuvoton: ma35d1: add USB controllers and dual-port PHY
    node
  phy: nuvoton: phy-ma35d1-usb2: extend to dual-port with OTG support

 .../bindings/phy/nuvoton,ma35d1-usb2-phy.yaml |  62 +++-
 .../bindings/reset/nuvoton,ma35d1-reset.yaml  |  13 +-
 .../boot/dts/nuvoton/ma35d1-iot-512m.dts      |  36 +++
 .../boot/dts/nuvoton/ma35d1-som-256m.dts      |  36 +++
 arch/arm64/boot/dts/nuvoton/ma35d1.dtsi       |  68 ++++-
 drivers/phy/nuvoton/phy-ma35d1-usb2.c         | 267 +++++++++++++-----
 6 files changed, 391 insertions(+), 91 deletions(-)

-- 
2.43.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 1/4] dt-bindings: reset: nuvoton,ma35d1-reset: add simple-mfd and child node support

[Joey Lu]

The MA35D1 system-management syscon node hosts the USB PHY register
block at offset 0x60.  To model usb-phy@60 as a DT child of the syscon
node the binding must allow:

  - simple-mfd as an optional third compatible so the MFD core can
    instantiate child platform devices.

  - #address-cells and #size-cells (each const: 1) so child nodes can
    carry a reg property.

  - An open child-node pattern (patternProperties "^.*@[0-9a-f]+$")
    to pass dt-schema validation.

Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
 .../bindings/reset/nuvoton,ma35d1-reset.yaml        | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
index 3ce7dcecd87a..1fda7e8f4b5d 100644
--- a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
+++ b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
@@ -19,6 +19,8 @@ properties:
     items:
       - const: nuvoton,ma35d1-reset
       - const: syscon
+      - const: simple-mfd
+    minItems: 2
 
   reg:
     maxItems: 1
@@ -26,6 +28,16 @@ properties:
   '#reset-cells':
     const: 1
 
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
+patternProperties:
+  "^.*@[0-9a-f]+$":
+    type: object
+
 required:
   - compatible
   - reg
@@ -43,4 +55,3 @@ examples:
         #reset-cells = <1>;
     };
 ...
-
-- 
2.43.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 1/4] dt-bindings: reset: nuvoton,ma35d1-reset: add simple-mfd and child node support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The child node pattern forces a unit address and `reg` property, but the intended child device's binding strictly prohibits `reg`, creating a validation Catch-22.
--

commit 64487b3a088a3cb8b25dc58df4266bfb284945b3
Author: Joey Lu <a0987203069@gmail.com>

dt-bindings: reset: nuvoton,ma35d1-reset: add simple-mfd and child node support

This commit updates the MA35D1 reset binding to allow simple-mfd and child
nodes. It adds #address-cells, #size-cells, and a pattern property to support
instantiating children like the USB PHY.

> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
> index 3ce7dcecd87ae..1fda7e8f4b5d1 100644
> --- a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
[ ... ]
> @@ -26,6 +28,16 @@ properties:
>    '#reset-cells':
>      const: 1
>  
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +patternProperties:
> +  "^.*@[0-9a-f]+$":
> +    type: object

[Severity: Medium]
This pattern property forces child nodes to have a unit address, which
inherently requires a reg property to avoid a dtc warning ("node has a unit 
name, but no reg or ranges property").

However, looking at the child PHY binding 
(Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy.yaml), it 
strictly prohibits reg via additionalProperties: false.

If we provide the reg property required by the parent's unit address, it will
fail the PHY schema validation.

Additionally, the PHY driver doesn't seem to act as a true MFD child. Looking
at drivers/phy/nuvoton/phy-ma35d1-usb2.c:ma35_usb_phy_probe():

    p_phy->sysreg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "nuvoton,sys");

The driver hardcodes its offset and uses a phandle instead of expecting a reg 
property.

Could this create a validation failure for the child PHY node, and are the
bindings and the driver structurally out of sync here?

> +
>  required:
>    - compatible
>    - reg

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625023958.569299-1-a0987203069@gmail.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 1/4] dt-bindings: reset: nuvoton,ma35d1-reset: add simple-mfd and child node support

[Krzysztof Kozlowski]

On Thu, Jun 25, 2026 at 10:39:55AM +0800, Joey Lu wrote:
> The MA35D1 system-management syscon node hosts the USB PHY register
> block at offset 0x60.  To model usb-phy@60 as a DT child of the syscon
> node the binding must allow:

Explain why do you need child node. If you have fixed device @0x60, you do
not need DT child node at all. Compatible implies that child existence.


> 
>   - simple-mfd as an optional third compatible so the MFD core can
>     instantiate child platform devices.
> 
>   - #address-cells and #size-cells (each const: 1) so child nodes can
>     carry a reg property.
> 
>   - An open child-node pattern (patternProperties "^.*@[0-9a-f]+$")
>     to pass dt-schema validation.

No. Do not explain what you did - we can read the diff. You must explain
WHY you are doing that.

> 
> Signed-off-by: Joey Lu <a0987203069@gmail.com>
> ---
>  .../bindings/reset/nuvoton,ma35d1-reset.yaml        | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
> index 3ce7dcecd87a..1fda7e8f4b5d 100644
> --- a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
> @@ -19,6 +19,8 @@ properties:
>      items:
>        - const: nuvoton,ma35d1-reset
>        - const: syscon
> +      - const: simple-mfd
> +    minItems: 2
>  
>    reg:
>      maxItems: 1
> @@ -26,6 +28,16 @@ properties:
>    '#reset-cells':
>      const: 1
>  
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +patternProperties:
> +  "^.*@[0-9a-f]+$":

This must be specific.

> +    type: object

Missing ref and additionalProps. Please look at other simple-mfd.

> +
>  required:
>    - compatible
>    - reg
> @@ -43,4 +55,3 @@ examples:
>          #reset-cells = <1>;
>      };
>  ...
> -
> -- 
> 2.43.0
> 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 2/4] dt-bindings: phy: nuvoton,ma35d1-usb2-phy: extend for dual-port OTG support

[Joey Lu]

The MA35D1 has two USB PHY ports managed by the same hardware block:

  - PHY0 (index 0): OTG port shared between the DWC2 gadget controller
    and EHCI0/OHCI0 host controllers.  A hardware mux follows the USB
    ID pin automatically.

  - PHY1 (index 1): dedicated host-only port for EHCI1/OHCI1.

Extend the existing binding to cover both ports:

  - The PHY node is now a child of the system-management syscon node
    with a reg property.  The nuvoton,sys phandle and clocks properties
    are removed; the driver derives the regmap from its parent, and
    clock gating is owned by each individual USB controller.

  - #phy-cells changes from 0 to 1: the cell selects the PHY port.

  - Two optional board-tuning properties are added: nuvoton,rcalcode
    for per-port resistor trim and nuvoton,oc-active-high for
    over-current polarity.

Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
 .../bindings/phy/nuvoton,ma35d1-usb2-phy.yaml | 62 ++++++++++++++-----
 1 file changed, 48 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy.yaml b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy.yaml
index fff858c909a0..a20d03c80932 100644
--- a/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy.yaml
@@ -8,38 +8,72 @@ title: Nuvoton MA35D1 USB2 phy
 
 maintainers:
   - Hui-Ping Chen <hpchen0nvt@gmail.com>
+  - Joey Lu <yclu4@nuvoton.com>
+
+description:
+  USB 2.0 PHY for the Nuvoton MA35D1 SoC. The PHY node is a child of the
+  system-management syscon node and covers both PHY ports.
+
+  PHY0 (index 0) is the OTG port whose signals are routed to either the DWC2
+  gadget controller or the EHCI0/OHCI0 host controller by a hardware mux that
+  follows the USB ID pin automatically.
+
+  PHY1 (index 1) is a dedicated host-only port used by EHCI1/OHCI1.
 
 properties:
   compatible:
     enum:
       - nuvoton,ma35d1-usb2-phy
 
+  reg:
+    maxItems: 1
+
   "#phy-cells":
-    const: 0
+    const: 1
+    description:
+      The single cell selects the PHY port. 0 selects the OTG port (USB0,
+      shared with DWC2 gadget controller) and 1 selects the host-only port
+      (USB1).
 
-  clocks:
-    maxItems: 1
+  nuvoton,rcalcode:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 2
+    items:
+      minimum: 0
+      maximum: 15
+    description:
+      Resistor calibration trim codes for PHY0 and PHY1 respectively.
+      Each 4-bit value is written to the RCALCODE field in USBPMISCR and
+      adjusts the PHY's internal termination resistance. Both entries are
+      optional; when absent the hardware reset default is used.
 
-  nuvoton,sys:
-    $ref: /schemas/types.yaml#/definitions/phandle
+  nuvoton,oc-active-high:
+    type: boolean
     description:
-      phandle to syscon for checking the PHY clock status.
+      When present, the over-current detect input from the VBUS power switch
+      is treated as active-high. The default (property absent) is active-low.
+      This setting is shared by both USB host ports.
 
 required:
   - compatible
+  - reg
   - "#phy-cells"
-  - clocks
-  - nuvoton,sys
 
 additionalProperties: false
 
 examples:
   - |
-    #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+    system-management@40460000 {
+        compatible = "nuvoton,ma35d1-reset", "syscon", "simple-mfd";
+        reg = <0x40460000 0x200>;
+        #reset-cells = <1>;
+        #address-cells = <1>;
+        #size-cells = <1>;
 
-    usb_phy: usb-phy {
-        compatible = "nuvoton,ma35d1-usb2-phy";
-        clocks = <&clk USBD_GATE>;
-        nuvoton,sys = <&sys>;
-        #phy-cells = <0>;
+        usb-phy@60 {
+            compatible = "nuvoton,ma35d1-usb2-phy";
+            reg = <0x60 0x14>;
+            #phy-cells = <1>;
+        };
     };
-- 
2.43.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/4] dt-bindings: phy: nuvoton,ma35d1-usb2-phy: extend for dual-port OTG support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The commit message claims multiple driver behavioral changes that are missing from the patch, leaving the driver incompatible with the new binding.
--

commit f557b8622653f3df69b040020ac61c52fd0498d6
Author: Joey Lu <a0987203069@gmail.com>

dt-bindings: phy: nuvoton,ma35d1-usb2-phy: extend for dual-port OTG support

This commit updates the device tree binding for the MA35D1 USB2 PHY to support
dual-port OTG. It changes the PHY node to be a child of a syscon node, removes
the nuvoton,sys and clocks properties, and updates #phy-cells to 1.

>       - The PHY node is now a child of the system-management syscon node
>         with a reg property.  The nuvoton,sys phandle and clocks properties
>         are removed; the driver derives the regmap from its parent, and
>         clock gating is owned by each individual USB controller.
>     
>       - #phy-cells changes from 0 to 1: the cell selects the PHY port.

> diff --git a/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy.yaml b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy.yaml
> index fff858c909a06..a20d03c80932b 100644
> --- a/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy.yaml
[ ... ]
> @@ -8,38 +8,72 @@ title: Nuvoton MA35D1 USB2 phy
[ ... ]
>    "#phy-cells":
> -    const: 0
> +    const: 1
> +    description:
> +      The single cell selects the PHY port. 0 selects the OTG port (USB0,
> +      shared with DWC2 gadget controller) and 1 selects the host-only port
> +      (USB1).
>  
> -  clocks:
> -    maxItems: 1
>  
> -  nuvoton,sys:
> -    $ref: /schemas/types.yaml#/definitions/phandle
> +  nuvoton,rcalcode:

[Severity: High]
Does this patch introduce a regression by updating the binding without the
corresponding driver updates?

The commit message indicates the driver derives the regmap from its parent and
handles the #phy-cells change, but this patch does not contain driver
modifications.

If a device tree is updated to comply with this new binding, will the existing
driver fail to probe? Looking at ma35_usb_phy_probe() in
drivers/phy/nuvoton/phy-ma35d1-usb2.c, it unconditionally requires the
nuvoton,sys property:

drivers/phy/nuvoton/phy-ma35d1-usb2.c:ma35_usb_phy_probe() {
    ...
    p_phy->sysreg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
                                                    "nuvoton,sys");
    if (IS_ERR(p_phy->sysreg))
        ...
}

Additionally, the unmodified driver still uses of_phy_simple_xlate and
registers a single PHY, which is structurally incompatible with the binding
change to #phy-cells = <1>:

drivers/phy/nuvoton/phy-ma35d1-usb2.c:ma35_usb_phy_probe() {
    ...
    provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
    ...
}

Should the driver changes be included in this patch or submitted together as
a series to avoid breaking USB initialization at boot time for compliant device
trees?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625023958.569299-1-a0987203069@gmail.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/4] dt-bindings: phy: nuvoton,ma35d1-usb2-phy: extend for dual-port OTG support

[Krzysztof Kozlowski]

On Thu, Jun 25, 2026 at 10:39:56AM +0800, Joey Lu wrote:
>  properties:
>    compatible:
>      enum:
>        - nuvoton,ma35d1-usb2-phy
>  
> +  reg:
> +    maxItems: 1
> +
>    "#phy-cells":
> -    const: 0
> +    const: 1
> +    description:
> +      The single cell selects the PHY port. 0 selects the OTG port (USB0,
> +      shared with DWC2 gadget controller) and 1 selects the host-only port
> +      (USB1).
>  
> -  clocks:
> -    maxItems: 1

This is odd, considering that parent does not have clocks. So explain me
this:
1. USB PHY needed clocks.
2. You extend USB PHY to cover second part.
3. That extension for second part means that clocks are not needed.
Really, how? How is it possible in hardware?

> +  nuvoton,rcalcode:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 2

You should require two values. I understand that any PHY is optional,
thus you skip the entry, so how would you provide value for PHY1 only?

> +    items:
> +      minimum: 0
> +      maximum: 15
> +    description:
> +      Resistor calibration trim codes for PHY0 and PHY1 respectively.
> +      Each 4-bit value is written to the RCALCODE field in USBPMISCR and
> +      adjusts the PHY's internal termination resistance. Both entries are
> +      optional; when absent the hardware reset default is used.
>  
> -  nuvoton,sys:
> -    $ref: /schemas/types.yaml#/definitions/phandle
> +  nuvoton,oc-active-high:
> +    type: boolean
>      description:
> -      phandle to syscon for checking the PHY clock status.
> +      When present, the over-current detect input from the VBUS power switch
> +      is treated as active-high. The default (property absent) is active-low.
> +      This setting is shared by both USB host ports.
>  
>  required:
>    - compatible
> +  - reg

That's ABI break which was not explained in the commit msg - neither
specifying impact nor actually providing reasons why you break ABI.

And honestly, you have no resources here except the address, so now it
is clear that this should be folded into parent. See DTS101 talk slides.

>    - "#phy-cells"
> -  - clocks
> -  - nuvoton,sys
>  
>  additionalProperties: false
>  
>  examples:
>    - |
> -    #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
> +    system-management@40460000 {
> +        compatible = "nuvoton,ma35d1-reset", "syscon", "simple-mfd";
> +        reg = <0x40460000 0x200>;
> +        #reset-cells = <1>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;

Drop. Keep only child node and make parent binding example complete.

>  
> -    usb_phy: usb-phy {
> -        compatible = "nuvoton,ma35d1-usb2-phy";
> -        clocks = <&clk USBD_GATE>;
> -        nuvoton,sys = <&sys>;
> -        #phy-cells = <0>;
> +        usb-phy@60 {
> +            compatible = "nuvoton,ma35d1-usb2-phy";
> +            reg = <0x60 0x14>;
> +            #phy-cells = <1>;
> +        };
>      };
> -- 
> 2.43.0
> 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 3/4] arm64: dts: nuvoton: ma35d1: add USB controllers and dual-port PHY node

[Joey Lu]

Add device tree nodes for the MA35D1 USB subsystem:

  - sys node gains simple-mfd + address/size-cells so it can contain
    the usb-phy@60 child.

  - usb-phy@60 is added as a child of sys, using the combined
    nuvoton,ma35d1-usb2-phy driver with #phy-cells = <1>.  No clock
    properties: clock gating is handled by each controller node.

  - DWC2 gadget (usb@40200000), EHCI0/1, and OHCI0/1 nodes are
    added.  Each controller names its clock gate directly and
    references the PHY by index (0 for the OTG port, 1 for the
    dedicated host port).

  - Board files (ma35d1-som-256m.dts, ma35d1-iot-512m.dts) enable the
    PHY, dwc2, ehci0/1, and ohci0/1 nodes and add pinctrl for the
    HSUSB signals (VBUSVLD, PWREN, OVC).

Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
 .../boot/dts/nuvoton/ma35d1-iot-512m.dts      | 36 ++++++++++
 .../boot/dts/nuvoton/ma35d1-som-256m.dts      | 36 ++++++++++
 arch/arm64/boot/dts/nuvoton/ma35d1.dtsi       | 68 ++++++++++++++++++-
 3 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts
index 9482bec1aa57..32fea36da7f4 100644
--- a/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1-iot-512m.dts
@@ -95,6 +95,16 @@ pinctrl_uart14: uart14-pins {
 			power-source = <1>;
 		};
 	};
+
+	hsusb {
+		pinctrl_hsusb: hsusb-pins {
+			nuvoton,pins = <5 15 1>, /* VBUSVLD */
+				       <11 12 9>,    /* PWREN */
+				       <11 13 9>;    /* OVC */
+			bias-disable;
+			power-source = <1>;
+		};
+	};
 };
 
 &uart0 {
@@ -126,3 +136,29 @@ &uart14 {
 	pinctrl-0 = <&pinctrl_uart14>;
 	status = "okay";
 };
+
+&usb_phy {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hsusb>;
+	status = "okay";
+};
+
+&usb {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&ohci0 {
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
index f6f20a17e501..85d1c5db8bd9 100644
--- a/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
@@ -98,6 +98,16 @@ pinctrl_uart16: uart16-pins {
 			power-source = <1>;
 		};
 	};
+
+	hsusb {
+		pinctrl_hsusb: hsusb-pins {
+			nuvoton,pins = <5 15 1>, /* VBUSVLD */
+				       <11 12 9>,    /* PWREN */
+				       <11 13 9>;    /* OVC */
+			bias-disable;
+			power-source = <1>;
+		};
+	};
 };
 
 &uart0 {
@@ -129,3 +139,29 @@ &uart16 {
 	pinctrl-0 = <&pinctrl_uart16>;
 	status = "okay";
 };
+
+&usb_phy {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hsusb>;
+	status = "okay";
+};
+
+&usb {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&ohci0 {
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
index e51b98f5bdce..a6a354f28311 100644
--- a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
@@ -83,9 +83,18 @@ soc {
 		ranges;
 
 		sys: system-management@40460000 {
-			compatible = "nuvoton,ma35d1-reset", "syscon";
+			compatible = "nuvoton,ma35d1-reset", "syscon", "simple-mfd";
 			reg = <0x0 0x40460000 0x0 0x200>;
 			#reset-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			usb_phy: usb-phy@60 {
+				compatible = "nuvoton,ma35d1-usb2-phy";
+				reg = <0x60 0x14>;
+				#phy-cells = <1>;
+				status = "disabled";
+			};
 		};
 
 		clk: clock-controller@40460200 {
@@ -379,5 +388,62 @@ uart16: serial@40880000 {
 			clocks = <&clk UART16_GATE>;
 			status = "disabled";
 		};
+
+		usb: usb@40200000 {
+			compatible = "snps,dwc2";
+			reg = <0x0 0x40200000 0x0 0x1000>;
+			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk USBD_GATE>;
+			clock-names = "otg";
+			phys = <&usb_phy 0>;
+			phy-names = "usb2-phy";
+			dr_mode = "peripheral";
+			g-np-tx-fifo-size = <16>;
+			g-rx-fifo-size = <0x100>;
+			g-tx-fifo-size = <256 256 64 64 64 32 32 32>;
+			status = "disabled";
+		};
+
+		ehci0: usb@40140000 {
+			compatible = "generic-ehci";
+			reg = <0x0 0x40140000 0x0 0x1000>;
+			interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk HUSBH0_GATE>;
+			phys = <&usb_phy 0>;
+			phy-names = "usb";
+			companion = <&ohci0>;
+			status = "disabled";
+		};
+
+		ehci1: usb@401c0000 {
+			compatible = "generic-ehci";
+			reg = <0x0 0x401c0000 0x0 0x1000>;
+			interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk HUSBH1_GATE>;
+			phys = <&usb_phy 1>;
+			phy-names = "usb";
+			companion = <&ohci1>;
+			status = "disabled";
+		};
+
+		ohci0: usb@40150000 {
+			compatible = "generic-ohci";
+			reg = <0x0 0x40150000 0x0 0x1000>;
+			interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk HUSBH0_GATE>;
+			phys = <&usb_phy 0>;
+			phy-names = "usb";
+			status = "disabled";
+		};
+
+		ohci1: usb@401d0000 {
+			compatible = "generic-ohci";
+			reg = <0x0 0x401d0000 0x0 0x1000>;
+			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk HUSBH1_GATE>;
+			phys = <&usb_phy 1>;
+			phy-names = "usb";
+			status = "disabled";
+		};
 	};
 };
-- 
2.43.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 3/4] arm64: dts: nuvoton: ma35d1: add USB controllers and dual-port PHY node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The PHY driver fails to probe unconditionally because the new device tree node lacks required properties.
- [High] The PHY driver does not support multi-PHY indexing, causing the second USB port to remain uninitialized.
- [Low] The `sys` node lacks a `ranges` property, breaking standard device tree address translation for its children.
--

commit 9dc588096b739f5b70f171c1eb14764b230dfb57
Author: Joey Lu <a0987203069@gmail.com>

arm64: dts: nuvoton: ma35d1: add USB controllers and dual-port PHY node

This commit adds device tree nodes for the MA35D1 USB subsystem, including
the simple-mfd sys node, the usb-phy child node, and the DWC2, EHCI0/1, 
and OHCI0/1 controllers. It also enables them and sets up pinctrl in the 
board files.

> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> index e51b98f5bdce4..a6a354f283117 100644
> --- a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> @@ -83,9 +83,18 @@ soc {
>  		ranges;
>  
>  		sys: system-management@40460000 {
> -			compatible = "nuvoton,ma35d1-reset", "syscon";
> +			compatible = "nuvoton,ma35d1-reset", "syscon", "simple-mfd";
>  			reg = <0x0 0x40460000 0x0 0x200>;
>  			#reset-cells = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;

[Severity: Low]
Since this sys node is acting as a bus for usb_phy and now defines 
#address-cells and #size-cells, does it also need a "ranges;" property?

Standard device tree specification requires a ranges property in the
parent to map the child's address space. Omitting it here breaks standard
device tree address translation and can trigger dtbs_check warnings.

> +
> +			usb_phy: usb-phy@60 {
> +				compatible = "nuvoton,ma35d1-usb2-phy";
> +				reg = <0x60 0x14>;
> +				#phy-cells = <1>;
> +				status = "disabled";
> +			};

[Severity: High]
Will this PHY node fail to probe since it lacks the "nuvoton,sys" and
"clocks" properties?

Looking at ma35_usb_phy_probe() in drivers/phy/nuvoton/phy-ma35d1-usb2.c,
it unconditionally requests both of these:

    p_phy->sysreg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
                                                    "nuvoton,sys");
    if (IS_ERR(p_phy->sysreg))
        return dev_err_probe(&pdev->dev, PTR_ERR(p_phy->sysreg),
                             "Failed to get SYS registers\n");

Because they are omitted from this node, won't the lookup return -ENODEV
and cause all dependent USB controllers (DWC2, EHCI0/1, OHCI0/1) to defer 
indefinitely?

>  		};
>  
>  		clk: clock-controller@40460200 {

[ ... ]

> +		ehci1: usb@401c0000 {
> +			compatible = "generic-ehci";
> +			reg = <0x0 0x401c0000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk HUSBH1_GATE>;
> +			phys = <&usb_phy 1>;

[Severity: High]
Does the current driver actually support indexing the PHY like this?

The device tree maps controllers to <&usb_phy 0> and <&usb_phy 1>, but 
looking at ma35_usb_phy_probe(), the driver registers with a simple xlate
that ignores the index:

    provider = devm_of_phy_provider_register(&pdev->dev,
                                             of_phy_simple_xlate);

Since of_phy_simple_xlate returns the first PHY object for all requests, 
and the power routines like ma35_usb_phy_power_on() are hardcoded to only 
manipulate PHY0 bits:

    regmap_update_bits(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, 0x7,
                       (PHY0POR | PHY0SUSPEND));

Will the hardware for PHY1 (the dedicated host port) remain completely 
unpowered and fail to initialize when ehci1 and ohci1 attempt to use it?

> +			phy-names = "usb";
> +			companion = <&ohci1>;
> +			status = "disabled";
> +		};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625023958.569299-1-a0987203069@gmail.com?part=3

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v2 4/4] phy: nuvoton: phy-ma35d1-usb2: extend to dual-port with OTG support

[Joey Lu]

The existing driver handled only PHY0 in device mode (DWC2 gadget).
Extend it to manage both PHY ports and integrate OTG support, per
reviewer suggestion to reuse the existing driver rather than add a
separate one.

The MA35D1 SoC has two USB PHY ports:

  - PHY0 (USB0): OTG port shared between the DWC2 gadget controller
    and EHCI0/OHCI0 host controllers.  A hardware mux in the SoC
    automatically routes the USB0 signals to the appropriate
    controller based on the USB ID pin state.

  - PHY1 (USB1): dedicated host-only port for EHCI1/OHCI1.

Key changes:

  Dual-port support
    A loop in probe() creates two struct phy objects, one per port,
    each with its own phy_set_drvdata() context.  A custom xlate
    function selects the correct phy by the single #phy-cells argument.

  Unified .init callback
    A single ma35_usb_phy_init() handles both ports using parametric
    register macros (USBPMISCR_PHY_*(n)).  If the SUSPEND bit is
    already set the init is skipped entirely, preventing the shared
    PHY0 from being reset while a live link is active.  On cold boot,
    PHY0 polls for either host-mode clocks (HSTCKSTB + CK12MSTB) or
    device-mode clock (DEVCKSTB) since the hardware selects the role
    automatically; PHY1 polls for host-mode clocks only.

  Clock management removed
    .power_on/.power_off and all struct clk handling are removed.
    Each USB controller (DWC2, EHCI, OHCI) already gates its own
    clock directly through its DTS clocks binding.  Having the PHY
    driver redundantly enable the same gates added unnecessary
    coupling without benefit.

  OTG role switch for PHY0
    A read-only USB role switch is registered, reporting the current
    OTG role by reading the USB ID pin state from PWRONOTP[16].
    .set returns -EOPNOTSUPP since the hardware mux is fully
    automatic.  allow_userspace_control is kept true to preserve the
    sysfs attribute for observation; writes are rejected by .set.

  syscon regmap via parent
    The driver obtains the regmap by calling
    syscon_node_to_regmap(pdev->dev.parent->of_node), removing the
    need for the nuvoton,sys phandle.

Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
 drivers/phy/nuvoton/phy-ma35d1-usb2.c | 267 ++++++++++++++++++--------
 1 file changed, 192 insertions(+), 75 deletions(-)

diff --git a/drivers/phy/nuvoton/phy-ma35d1-usb2.c b/drivers/phy/nuvoton/phy-ma35d1-usb2.c
index 9a459b700ed4..19242b10cee3 100644
--- a/drivers/phy/nuvoton/phy-ma35d1-usb2.c
+++ b/drivers/phy/nuvoton/phy-ma35d1-usb2.c
@@ -1,11 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (C) 2024 Nuvoton Technology Corp.
+ * Nuvoton MA35D1 USB 2.0 PHY driver
+ *
+ * Supports PHY0 (USB0 OTG port, shared between DWC2 gadget and EHCI0/OHCI0)
+ * and PHY1 (USB1 host-only port, used by EHCI1/OHCI1). The hardware mux on
+ * PHY0 switches automatically via the USB ID pin.
+ *
+ * Copyright (C) 2026 Nuvoton Technology Corp.
  */
 #include <linux/bitfield.h>
-#include <linux/clk.h>
 #include <linux/delay.h>
-#include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
@@ -13,131 +17,244 @@
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/usb/role.h>
 
-/* USB PHY Miscellaneous Control Register */
-#define MA35_SYS_REG_USBPMISCR	0x60
-#define PHY0POR			BIT(0)  /* PHY Power-On Reset Control Bit */
-#define PHY0SUSPEND			BIT(1)  /* PHY Suspend; 0: suspend, 1: operaion */
-#define PHY0COMN			BIT(2)  /* PHY Common Block Power-Down Control */
-#define PHY0DEVCKSTB			BIT(10) /* PHY 60 MHz UTMI clock stable bit */
+#define MA35_SYS_PWRONOTP		0x04
+#define PWRONOTP_USBP0ID		BIT(16)	/* USB0 ID pin state */
+
+#define MA35_SYS_USBPMISCR		0x60
+#define USBPMISCR_PHY_POR(n)		BIT(0  + (n) * 16)
+#define USBPMISCR_PHY_SUSPEND(n)	BIT(1  + (n) * 16)
+#define USBPMISCR_PHY_COMN(n)		BIT(2  + (n) * 16)
+#define USBPMISCR_PHY_HSTCKSTB(n)	BIT(8  + (n) * 16)
+#define USBPMISCR_PHY_CK12MSTB(n)	BIT(9  + (n) * 16)
+#define USBPMISCR_PHY_DEVCKSTB(n)	BIT(10 + (n) * 16)
+/* Mask for control bits (POR, SUSPEND, COMN) of one PHY */
+#define USBPMISCR_PHY_CTL_MASK(n)	(0x7u << ((n) * 16))
+/* Host-mode ready: SUSPEND set */
+#define USBPMISCR_PHY_HOST_READY(n)	(USBPMISCR_PHY_SUSPEND(n)  | \
+					 USBPMISCR_PHY_HSTCKSTB(n) | \
+					 USBPMISCR_PHY_CK12MSTB(n))
+/* Device-mode ready: SUSPEND set */
+#define USBPMISCR_PHY_DEV_READY(n)	(USBPMISCR_PHY_SUSPEND(n)  | \
+					 USBPMISCR_PHY_DEVCKSTB(n))
+/* RCALCODE: 4-bit resistor trim at bits [15:12] (PHY0) or [31:28] (PHY1) */
+#define USBPMISCR_RCAL_SHIFT(n)		(12 + (n) * 16)
+#define USBPMISCR_RCAL_MASK(n)		GENMASK(USBPMISCR_RCAL_SHIFT(n) + 3, \
+						USBPMISCR_RCAL_SHIFT(n))
+
+#define MA35_SYS_MISCFCR0		0x70
+/* Bit 12: USB host over-current detect polarity (shared, both ports) */
+#define MISCFCR0_UHOVRCURH		BIT(12)
+
+#define MA35_PHY_NUM			2
+
+struct ma35_phy_port {
+	struct phy *phy;
+	unsigned int idx;
+};
 
 struct ma35_usb_phy {
-	struct clk *clk;
 	struct device *dev;
 	struct regmap *sysreg;
+	struct ma35_phy_port port[MA35_PHY_NUM];
+	struct usb_role_switch *role_sw;
 };
 
-static int ma35_usb_phy_power_on(struct phy *phy)
+static int ma35_usb_phy_init(struct phy *phy)
 {
-	struct ma35_usb_phy *p_phy = phy_get_drvdata(phy);
+	struct ma35_phy_port *port = phy_get_drvdata(phy);
+	struct ma35_usb_phy *p = container_of(port - port->idx,
+					      struct ma35_usb_phy, port[0]);
+	unsigned int n = port->idx;
 	unsigned int val;
 	int ret;
 
-	ret = clk_prepare_enable(p_phy->clk);
-	if (ret < 0) {
-		dev_err(p_phy->dev, "Failed to enable PHY clock: %d\n", ret);
-		return ret;
-	}
+	regmap_read(p->sysreg, MA35_SYS_USBPMISCR, &val);
 
-	regmap_read(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, &val);
-	if (val & PHY0SUSPEND) {
-		/*
-		 * USB PHY0 is in operation mode already
-		 * make sure USB PHY 60 MHz UTMI Interface Clock ready
-		 */
-		ret = regmap_read_poll_timeout(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, val,
-						val & PHY0DEVCKSTB, 10, 1000);
-		if (ret == 0)
-			return 0;
-	}
+	if (val & USBPMISCR_PHY_SUSPEND(n))
+		return 0;
 
-	/*
-	 * reset USB PHY0.
-	 * wait until USB PHY0 60 MHz UTMI Interface Clock ready
-	 */
-	regmap_update_bits(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, 0x7, (PHY0POR | PHY0SUSPEND));
+	regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
+			   USBPMISCR_PHY_CTL_MASK(n),
+			   USBPMISCR_PHY_POR(n) | USBPMISCR_PHY_SUSPEND(n));
 	udelay(20);
 
-	/* make USB PHY0 enter operation mode */
-	regmap_update_bits(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, 0x7, PHY0SUSPEND);
+	regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
+			   USBPMISCR_PHY_CTL_MASK(n),
+			   USBPMISCR_PHY_SUSPEND(n));
+
+	if (n == 0) {
+		ret = regmap_read_poll_timeout(p->sysreg, MA35_SYS_USBPMISCR,
+					       val,
+					       ((val & USBPMISCR_PHY_HOST_READY(0)) ==
+						USBPMISCR_PHY_HOST_READY(0)) ||
+					       ((val & USBPMISCR_PHY_DEV_READY(0)) ==
+						USBPMISCR_PHY_DEV_READY(0)),
+					       10, 1000);
+	} else {
+		ret = regmap_read_poll_timeout(p->sysreg, MA35_SYS_USBPMISCR,
+					       val,
+					       (val & USBPMISCR_PHY_HOST_READY(n)) ==
+					       USBPMISCR_PHY_HOST_READY(n),
+					       10, 1000);
+	}
 
-	/* make sure USB PHY 60 MHz UTMI Interface Clock ready */
-	ret = regmap_read_poll_timeout(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, val,
-					val & PHY0DEVCKSTB, 10, 1000);
-	if (ret == -ETIMEDOUT) {
-		dev_err(p_phy->dev, "Check PHY clock, Timeout: %d\n", ret);
-		clk_disable_unprepare(p_phy->clk);
+	if (ret) {
+		dev_err(p->dev, "USB PHY%u clock not stable (USBPMISCR=0x%08x)\n",
+			n, val);
 		return ret;
 	}
 
 	return 0;
 }
 
-static int ma35_usb_phy_power_off(struct phy *phy)
+static const struct phy_ops ma35_usb_phy_ops = {
+	.init		= ma35_usb_phy_init,
+	.owner		= THIS_MODULE,
+};
+
+static int ma35_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
+{
+	return -EOPNOTSUPP;
+}
+
+static enum usb_role ma35_role_sw_get(struct usb_role_switch *sw)
+{
+	struct ma35_usb_phy *p = usb_role_switch_get_drvdata(sw);
+	u32 val;
+
+	regmap_read(p->sysreg, MA35_SYS_PWRONOTP, &val);
+
+	return (val & PWRONOTP_USBP0ID) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+}
+
+static int ma35_role_switch_init(struct platform_device *pdev,
+				 struct ma35_usb_phy *p)
 {
-	struct ma35_usb_phy *p_phy = phy_get_drvdata(phy);
+	struct usb_role_switch_desc sw_desc = {0};
+
+	sw_desc.set = ma35_role_sw_set;
+	sw_desc.get = ma35_role_sw_get;
+	sw_desc.allow_userspace_control = true;
+	sw_desc.driver_data = p;
+	sw_desc.fwnode = dev_fwnode(&pdev->dev);
+
+	p->role_sw = usb_role_switch_register(&pdev->dev, &sw_desc);
+	if (IS_ERR(p->role_sw))
+		return dev_err_probe(&pdev->dev, PTR_ERR(p->role_sw),
+				     "failed to register role switch\n");
 
-	clk_disable_unprepare(p_phy->clk);
 	return 0;
 }
 
-static const struct phy_ops ma35_usb_phy_ops = {
-	.power_on = ma35_usb_phy_power_on,
-	.power_off = ma35_usb_phy_power_off,
-	.owner = THIS_MODULE,
-};
+static void ma35_role_switch_exit(struct ma35_usb_phy *p)
+{
+	if (p->role_sw) {
+		usb_role_switch_unregister(p->role_sw);
+		p->role_sw = NULL;
+	}
+}
+
+static struct phy *ma35_usb_phy_xlate(struct device *dev,
+				      const struct of_phandle_args *args)
+{
+	struct ma35_usb_phy *p = dev_get_drvdata(dev);
+
+	if (args->args[0] >= MA35_PHY_NUM)
+		return ERR_PTR(-EINVAL);
+
+	return p->port[args->args[0]].phy;
+}
 
 static int ma35_usb_phy_probe(struct platform_device *pdev)
 {
 	struct phy_provider *provider;
-	struct ma35_usb_phy *p_phy;
-	struct phy *phy;
+	struct ma35_usb_phy *p;
+	int n, ret;
+	u32 code;
 
-	p_phy = devm_kzalloc(&pdev->dev, sizeof(*p_phy), GFP_KERNEL);
-	if (!p_phy)
+	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
+	if (!p)
 		return -ENOMEM;
 
-	p_phy->dev = &pdev->dev;
-	platform_set_drvdata(pdev, p_phy);
+	p->dev = &pdev->dev;
+	platform_set_drvdata(pdev, p);
+
+	p->sysreg = syscon_node_to_regmap(pdev->dev.parent->of_node);
+	if (IS_ERR(p->sysreg))
+		return dev_err_probe(&pdev->dev, PTR_ERR(p->sysreg),
+				     "failed to get parent SYS regmap\n");
+
+	for (n = 0; n < MA35_PHY_NUM; n++) {
+		if (of_property_read_u32_index(pdev->dev.of_node,
+					       "nuvoton,rcalcode", n, &code))
+			continue;
 
-	p_phy->sysreg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "nuvoton,sys");
-	if (IS_ERR(p_phy->sysreg))
-		return dev_err_probe(&pdev->dev, PTR_ERR(p_phy->sysreg),
-				     "Failed to get SYS registers\n");
+		if (code > 15)
+			return dev_err_probe(&pdev->dev, -EINVAL,
+					     "rcalcode[%d] %u out of range (0-15)\n",
+					     n, code);
 
-	p_phy->clk = of_clk_get(pdev->dev.of_node, 0);
-	if (IS_ERR(p_phy->clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(p_phy->clk),
-				     "failed to find usb_phy clock\n");
+		regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
+				   USBPMISCR_RCAL_MASK(n),
+				   code << USBPMISCR_RCAL_SHIFT(n));
+	}
+
+	if (of_property_read_bool(pdev->dev.of_node, "nuvoton,oc-active-high"))
+		regmap_update_bits(p->sysreg, MA35_SYS_MISCFCR0,
+				   MISCFCR0_UHOVRCURH, MISCFCR0_UHOVRCURH);
+
+	for (n = 0; n < MA35_PHY_NUM; n++) {
+		p->port[n].idx = n;
 
-	phy = devm_phy_create(&pdev->dev, NULL, &ma35_usb_phy_ops);
-	if (IS_ERR(phy))
-		return dev_err_probe(&pdev->dev, PTR_ERR(phy), "Failed to create PHY\n");
+		p->port[n].phy = devm_phy_create(&pdev->dev, pdev->dev.of_node,
+						 &ma35_usb_phy_ops);
+		if (IS_ERR(p->port[n].phy))
+			return dev_err_probe(&pdev->dev, PTR_ERR(p->port[n].phy),
+					     "failed to create PHY%d\n", n);
 
-	phy_set_drvdata(phy, p_phy);
+		phy_set_drvdata(p->port[n].phy, &p->port[n]);
+	}
+
+	ret = ma35_role_switch_init(pdev, p);
+	if (ret)
+		return ret;
 
-	provider = devm_of_phy_provider_register(&pdev->dev, of_phy_simple_xlate);
-	if (IS_ERR(provider))
+	provider = devm_of_phy_provider_register(&pdev->dev, ma35_usb_phy_xlate);
+	if (IS_ERR(provider)) {
+		ma35_role_switch_exit(p);
 		return dev_err_probe(&pdev->dev, PTR_ERR(provider),
-				     "Failed to register PHY provider\n");
+				     "failed to register PHY provider\n");
+	}
+
 	return 0;
 }
 
+static void ma35_usb_phy_remove(struct platform_device *pdev)
+{
+	struct ma35_usb_phy *p = platform_get_drvdata(pdev);
+
+	ma35_role_switch_exit(p);
+}
+
 static const struct of_device_id ma35_usb_phy_of_match[] = {
-	{ .compatible = "nuvoton,ma35d1-usb2-phy", },
-	{ },
+	{ .compatible = "nuvoton,ma35d1-usb2-phy" },
+	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ma35_usb_phy_of_match);
 
 static struct platform_driver ma35_usb_phy_driver = {
 	.probe		= ma35_usb_phy_probe,
-	.driver	= {
-		.name	= "ma35d1-usb2-phy",
-		.of_match_table = ma35_usb_phy_of_match,
+	.remove		= ma35_usb_phy_remove,
+	.driver		= {
+		.name		= "ma35d1-usb2-phy",
+		.of_match_table	= ma35_usb_phy_of_match,
 	},
 };
 module_platform_driver(ma35_usb_phy_driver);
 
 MODULE_DESCRIPTION("Nuvoton ma35d1 USB2.0 PHY driver");
 MODULE_AUTHOR("Hui-Ping Chen <hpchen0nvt@gmail.com>");
+MODULE_AUTHOR("Joey Lu <a0987203069@gmail.com>");
 MODULE_LICENSE("GPL");
-- 
2.43.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 4/4] phy: nuvoton: phy-ma35d1-usb2: extend to dual-port with OTG support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The custom `ma35_usb_phy_xlate` callback accesses `args->args[0]` without verifying `args->args_count`, leading to a potential read of uninitialized stack memory.
- [High] The driver breaks Device Tree backward compatibility by unconditionally relying on the parent's `of_node` for the syscon regmap, removing the legacy 'nuvoton,sys' phandle fallback.
- [High] The driver fails to explicitly clear the 'MISCFCR0_UHOVRCURH' bit if the 'nuvoton,oc-active-high' property is absent, leading to persistent incorrect hardware state across soft reboots.
- [High] The driver incorrectly skips PHY clock stabilization checks if the `SUSPEND` bit is already set, potentially leading to hardware hangs.
- [Medium] The variable 'val' in `ma35_role_sw_get` is used uninitialized if the `regmap_read` call fails.
--

commit ff3f6d83cfb95f48a64d4c533549a7465a41d150
Author: Joey Lu <a0987203069@gmail.com>

phy: nuvoton: phy-ma35d1-usb2: extend to dual-port with OTG support

This commit extends the driver to support both PHY ports and integrates OTG
role switching. It introduces a custom xlate function, unifies the init
callback, removes redundant clock management, and registers a USB role switch.

> --- a/drivers/phy/nuvoton/phy-ma35d1-usb2.c
> +++ b/drivers/phy/nuvoton/phy-ma35d1-usb2.c
[ ... ]
> +static int ma35_usb_phy_init(struct phy *phy)
> +{
> +	struct ma35_phy_port *port = phy_get_drvdata(phy);
> +	struct ma35_usb_phy *p = container_of(port - port->idx,
> +					      struct ma35_usb_phy, port[0]);
> +	unsigned int n = port->idx;
> +	unsigned int val;
> +	int ret;
> +
> +	regmap_read(p->sysreg, MA35_SYS_USBPMISCR, &val);
> +
> +	if (val & USBPMISCR_PHY_SUSPEND(n))
> +		return 0;

[Severity: High]
Does returning early here skip necessary PHY clock stabilization checks?

The previous implementation specifically polled the clock stable bit
even when the suspend bit was already set. By returning immediately,
the USB controller might start accessing the PHY before its newly ungated
clock has fully stabilized, potentially resulting in bus hangs.

[ ... ]
> +static enum usb_role ma35_role_sw_get(struct usb_role_switch *sw)
> +{
> +	struct ma35_usb_phy *p = usb_role_switch_get_drvdata(sw);
> +	u32 val;
> +
> +	regmap_read(p->sysreg, MA35_SYS_PWRONOTP, &val);
> +
> +	return (val & PWRONOTP_USBP0ID) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
> +}

[Severity: Medium]
Is it possible for val to be evaluated uninitialized here?

If regmap_read() encounters an error, it returns a non-zero error code
and leaves val unmodified. This could cause the function to return an
incorrect USB role based on stack garbage.

[ ... ]
> +static struct phy *ma35_usb_phy_xlate(struct device *dev,
> +				      const struct of_phandle_args *args)
> +{
> +	struct ma35_usb_phy *p = dev_get_drvdata(dev);
> +
> +	if (args->args[0] >= MA35_PHY_NUM)
> +		return ERR_PTR(-EINVAL);

[Severity: High]
Does this code safely handle device trees where the number of phy cells is zero?

If args_count is zero, args->args would be uninitialized stack memory,
which could result in returning an incorrect PHY port or out-of-bounds
access. Should there be a check for args->args_count >= 1?

[ ... ]
> +static int ma35_usb_phy_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	p->sysreg = syscon_node_to_regmap(pdev->dev.parent->of_node);
> +	if (IS_ERR(p->sysreg))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(p->sysreg),
> +				     "failed to get parent SYS regmap\n");

[Severity: High]
Does this break Device Tree backward compatibility?

The old implementation relied on a "nuvoton,sys" phandle fallback which has
been removed here. Systems using older device tree binaries might fail to
probe the USB PHY on kernel upgrades if the node isn't a direct child of
the syscon.

[ ... ]
> +	if (of_property_read_bool(pdev->dev.of_node, "nuvoton,oc-active-high"))
> +		regmap_update_bits(p->sysreg, MA35_SYS_MISCFCR0,
> +				   MISCFCR0_UHOVRCURH, MISCFCR0_UHOVRCURH);

[Severity: High]
If the "nuvoton,oc-active-high" property is absent, should this code explicitly
clear the MISCFCR0_UHOVRCURH bit?

Without an else branch to clear the bit, a soft reboot could incorrectly
inherit an active-high state set by a bootloader or previous kernel.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625023958.569299-1-a0987203069@gmail.com?part=4

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy