Re: [PATCH 1/4] dt-bindings: usb: add Realtek RTS5453H Type-C port controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] dt-bindings: usb: rts5453h uses addressed connector pattern for
  single port
- [Low] dt-bindings: usb: rts5453h unconditionally requires
  try-power-role
--

commit bf789180bfcb329a121d357c95111fa527217ccf
Author: Gary Yang <gary.yang@cixtech.com>

dt-bindings: usb: add Realtek RTS5453H Type-C port controller

This commit adds the Device Tree binding schema for the Realtek RTS5453H
I2C-connected USB Type-C port controller.

> diff --git a/Documentation/devicetree/bindings/usb/realtek,rts5453h.yaml b/Documentation/devicetree/bindings/usb/realtek,rts5453h.yaml
> new file mode 100644
> index 0000000000000..535aa23794273
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/realtek,rts5453h.yaml
> @@ -0,0 +1,126 @@

[ ... ]

> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  interrupts:
> +    maxItems: 1
> +
> +patternProperties:
> +  '^connector@[0-9a-f]+$':
> +    type: object
> +    $ref: /schemas/connector/usb-connector.yaml#
> +    unevaluatedProperties: false
> +    description:
> +      The USB Type-C connector managed by this controller.
> +
> +    required:
> +      - reg
> +      - data-role
> +      - power-role
> +      - try-power-role

[Severity: Low]
Should try-power-role be optional here instead of required?

In usb-connector.yaml and typec_get_fw_cap(), try-power-role only applies
when power-role is "dual" and is optional even for dual-role ports.
Requiring it unconditionally forces fixed source-only or sink-only ports
(including the example below with power-role = "source") and dual-role
ports without a preferred role to specify an inapplicable property.

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#address-cells'
> +  - '#size-cells'
> +  - interrupts

[Severity: Low]
Since the RTS5453H manages a single USB Type-C connector, should this
define a single "connector" object property in properties (and list
"connector" in required) instead of using patternProperties with unit
addresses?

Using '^connector@[0-9a-f]+$' with '#address-cells', '#size-cells', and
reg allows multiple connector child nodes at arbitrary unit addresses
while leaving the connector node itself optional on the controller.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107011012.3655873-1-gary.yang@cixtech.com?part=1