Re: [PATCH 1/2] dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The binding incorrectly represents a multi-port PHY controller as multiple independent nodes instead of a single provider with `#phy-cells = <1>`.
--

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

dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding

Adds the device tree binding documentation for the Nuvoton MA35D1 USB 2.0
OTG PHY driver. The PHY supports two ports (OTG and host-only) and allows
board-specific resistor calibration and over-current detect polarity.

> diff --git a/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml

[ ... ]

> +  nuvoton,sys:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to the system management syscon.
> +          - description: PHY instance index.
> +            enum:
> +              - 0   # USB0, OTG port (shared with DWC2 gadget controller)
> +              - 1   # USB1, host-only port
> +    description:
> +      A phandle to the syscon node covering the SYS register block, with
> +      one argument selecting the PHY instance. Index 0 selects the OTG
> +      port PHY (USB0) and index 1 selects the host-only PHY (USB1).
> +
> +  "#phy-cells":
> +    const: 0

[Severity: Medium]
Should this multi-port PHY controller be represented as a single provider
node with #phy-cells = <1> instead of embedding the index in nuvoton,sys?

Using independent nodes for each port with #phy-cells = <0> sidesteps the
standard consumer argument pattern and forces non-standard node names to
avoid collisions at the same tree level.

[ ... ]

> +  nuvoton,oc-active-high:
> +    type: boolean
> +    description:
> +      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.

Since this property is explicitly described as shared by both ports, does
instantiating separate nodes per port risk conflicting states if the nodes
configure this property differently?

A single PHY provider node would allow shared properties to be defined once,
while consumers could specify their port index via standard phys arguments.

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

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