Re: [PATCH 1/2] dt-bindings: usb: fsa4480: Add an optional audio port/endpoint

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: usb: fsa4480: conflicting usb-switch-ports.yaml ref in allOf
--

commit bdd98503313bd1c358f2a30a54c1cf8d1fe906dd
Author: Val Packett <val@packett.cool>

dt-bindings: usb: fsa4480: Add an optional audio port/endpoint

This commit updates the fcs,fsa4480 binding to support a multi-port graph
layout so that hotplug notifications can be delivered to an audio codec
driver through a second port while keeping the single-port layout valid.

> diff --git a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml b/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
> index 89b1fb90aeebc..9dfe95b6adf35 100644
> --- a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
> +++ b/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
> @@ -9,31 +9,11 @@ title: ON Semiconductor Analog Audio Switch
>  maintainers:
>    - Bjorn Andersson <bjorn.andersson@linaro.org>
>  
> -properties:
> -  compatible:
> -    oneOf:
> -      - const: fcs,fsa4480
> -      - items:
> -          - enum:
> -              - ocs,ocp96011
> -          - const: fcs,fsa4480
> -
> -  reg:
> -    maxItems: 1
> -
> -  interrupts:
> -    maxItems: 1
> -
> -  vcc-supply:
> -    description: power supply (2.7V-5.5V)
> -
> -  mode-switch: true
> -  orientation-switch: true
> -
> -  port:
> +$defs:
> +  controller-port:
>      $ref: /schemas/graph.yaml#/$defs/port-base
>      description:
> -      A port node to link the FSA4480 to a TypeC controller for the purpose of
> +      A port node to link the FSA4480 to a Type-C controller for the purpose of
>        handling altmode muxing and orientation switching.
>      unevaluatedProperties: false
>  
> @@ -69,15 +49,56 @@ properties:
>                    - AUX+ connected to SBU2
>                    - AUX- connected to SBU1
>  
> +properties:

[ ... ]

> +  port:
> +    $ref: "#/$defs/controller-port"
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    properties:
> +      port@0:
> +        $ref: "#/$defs/controller-port"
> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        description:
> +          A port node to link the FSA4480 to an audio codec in order to
> +          notify it whenever an analog audio adapter is plugged/unplugged.
> +        unevaluatedProperties: false
> +
>  required:
>    - compatible
>    - reg
> -  - port
>  
>  allOf:
>    - $ref: usb-switch.yaml#
>    - $ref: usb-switch-ports.yaml#
>  
> +oneOf:
> +  - required:
> +      - port
> +  - required:
> +      - ports

[Severity: Medium]
Does keeping usb-switch-ports.yaml# in allOf conflict with the custom ports
schema defined here?

In Documentation/devicetree/bindings/usb/usb-switch-ports.yaml, ports is
defined for SuperSpeed switches:

  ports:
    $ref: /schemas/graph.yaml#/properties/ports
    properties:
      port@0:
        $ref: /schemas/graph.yaml#/properties/port
        description:
          Super Speed (SS) Output endpoint to the Type-C connector

      port@1:
        $ref: /schemas/graph.yaml#/$defs/port-base
        description:
          Super Speed (SS) Input endpoint from the Super-Speed PHY
        ...
        properties:
          endpoint:
            $ref: /schemas/graph.yaml#/$defs/endpoint-base
            ...
            properties:
              data-lanes:
                minItems: 1
                maxItems: 8

Because allOf requires validation against both schemas, port@0 in
usb-switch-ports.yaml references /schemas/graph.yaml#/properties/port, which
enforces unevaluatedProperties: false on endpoint with no data-lanes property
(as noted in commit 11e80d371bbb ("dt-bindings: usb: fcs,fsa4480: Fix default
port schema reference")). That rejects valid data-lanes properties inside
ports/port@0/endpoint (such as on ocs,ocp96011 in
arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts when converted to ports),
defeating #/$defs/controller-port on port@0.

At the same time, port@1 here uses /schemas/graph.yaml#/$defs/port-base
instead of /schemas/graph.yaml#/properties/port, which leaves port@1/endpoint
without unevaluatedProperties: false and inherits the SuperSpeed data-lanes
definition from usb-switch-ports.yaml, while ports does not require port@0.

Following commit bd8c3ce6d7a2 ("dt-bindings: usb: switch: split out ports
definition") for bindings with custom ports layouts, should
usb-switch-ports.yaml# be dropped from allOf, port@1 changed to reference
/schemas/graph.yaml#/properties/port, and required: [port@0] added under
ports?

> +
>  additionalProperties: false

[ ... ]

>
>
>

 base-commit: 90d9f76ccb72ec024ef476b4229f3dd1957d7b26
-- 
2.51.2

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018225906.1265299-1-val@packett.cool?part=1