Linux-PHY Archive on lore.kernel.org

* Re: [PATCH v2 2/4] dt-bindings: phy: nuvoton,ma35d1-usb2-phy: extend for dual-port OTG support
From: Krzysztof Kozlowski @ 2026-06-25  7:58 UTC (permalink / raw)
  To: Joey Lu
  Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Arnd Bergmann, Catalin Marinas, Jacky Huang,
	Shan-Chun Hung, Hui-Ping Chen, Joey Lu, linux-phy, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20260625023958.569299-3-a0987203069@gmail.com>

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

^ permalink raw reply