Re: [PATCH v4 10/14] dt-bindings: usb: mediatek,mtk-xhci: Switch to ports for USB connections

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] DT ABI Breakage: Reverting the top-level `port` property and replacing it with `ports` fails to update existing in-tree device tree users, causing schema validation failures.
- [Medium] Incomplete and incorrect property constraints: The descriptions for `port@1` and `port@2` strictly assume a maximum of 1 SS port and 1 HS port, which contradicts hardware with multiple ports.
--

--- Patch [10]: [PATCH v4 10/14] dt-bindings: usb: mediatek,mtk-xhci: Switch to ports for USB connections ---
commit 2e9e786003304eb48defeb647a256e455bd0ffa3
Author: Chen-Yu Tsai <wenst@chromium.org>

dt-bindings: usb: mediatek,mtk-xhci: Switch to ports for USB connections

This commit replaces the single 'port' node with a 'ports' node containing
'port@1' and 'port@2' to represent the USB connections. It maps port 1 to SS
and port 2 to HS depending on controller capabilities.

> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> index 231e6f35a986f..d6c75bd20b785 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -107,10 +107,6 @@ properties:
>        - description: USB3/SS(P) PHY
>        - description: USB2/HS PHY
>  
> -  port:
> -    $ref: /schemas/graph.yaml#/properties/port
> -    description: Super Speed (SS) Output endpoint to a Type-C connector
> -

[Severity: High]
Does removing this property break schema validation for existing device tree
files? 

For example, arch/arm64/boot/dts/mediatek/mt8390-genio-common.dtsi uses
the port property under the &xhci1 node:

arch/arm64/boot/dts/mediatek/mt8390-genio-common.dtsi:
	port {
		xhci_ss_ep: endpoint {
			remote-endpoint = <&typec_con_ss>;
		};
	};

Running make dtbs_check with this commit seems like it will result in
'undocumented property' errors for the old port node.

Should the in-tree users be updated alongside this binding change, or should
backward compatibility be maintained?

>    vusb33-supply:
>      description: Regulator of USB AVDD3.3v
>  
[ ... ]
> @@ -188,6 +184,19 @@ properties:
>    "#size-cells":
>      const: 0
>  
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Super Speed (SS) data bus if SS-capable;
> +          otherwise High Speed (HS) data bus.
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: High Speed (HS) data bus if controller is SS-capable.

[Severity: Medium]
Do these port definitions fully account for controllers with multiple ports of
the same speed?

The hardware supports up to 9 PHYs in the existing phys property array,
accommodating multiple ports on a single controller (e.g., 2 HS ports and 1 SS
port on ssusb1). Defining only port@1 and port@2 restricts the schema to a
2-port layout.

If a developer defines a valid port@3 for a second HS port, won't it fail
schema validation? Could these descriptions be broadened or the properties
expanded to accommodate multi-port controllers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709095726.704448-1-wenst@chromium.org?part=10