Received-SPF: pass (google.com: domain of linux-kernel+bounces-139944-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Thu, 11 Apr 2024 09:13:12 +0200
Message-Id: <D0H3VE6RLM2I.MK2NT1P9N02O@fairphone.com>
Subject: Re: [PATCH v2] dt-bindings: usb: add common Type-C USB Switch
 schema
From: "Luca Weiss" <luca.weiss@fairphone.com>
To: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>, "Greg
 Kroah-Hartman" <gregkh@linuxfoundation.org>, "Rob Herring"
 <robh+dt@kernel.org>, "Krzysztof Kozlowski"
 <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>,
 "Bjorn Andersson" <andersson@kernel.org>, "Konrad Dybcio"
 <konrad.dybcio@linaro.org>, "Neil Armstrong" <neil.armstrong@linaro.org>,
 <linux-usb@vger.kernel.org>, <devicetree@vger.kernel.org>,
 <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>
References: <20240122094406.32198-1-krzysztof.kozlowski@linaro.org>
In-Reply-To: <20240122094406.32198-1-krzysztof.kozlowski@linaro.org>

On Mon Jan 22, 2024 at 10:44 AM CET, Krzysztof Kozlowski wrote:
> Several bindings implement parts of Type-C USB orientation and mode
> switching, and retiming.  Keep definition of such properties in one
> place, new usb-switch schema, to avoid duplicate defines.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
>
> Changes in v2:
> 1. Fix language typos handle->handler (Luca)
> 2. Drop debugging left-over (Luca)
> ---
>  .../devicetree/bindings/usb/fcs,fsa4480.yaml  | 12 ++--
>  .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 12 ++--
>  .../devicetree/bindings/usb/nxp,ptn36502.yaml | 12 ++--
>  .../bindings/usb/onnn,nb7vpq904m.yaml         | 13 ++--
>  .../bindings/usb/qcom,wcd939x-usbss.yaml      | 12 ++--
>  .../devicetree/bindings/usb/usb-switch.yaml   | 67 +++++++++++++++++++
>  6 files changed, 92 insertions(+), 36 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb-switch.yaml
>
> diff --git a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml b/Doc=
umentation/devicetree/bindings/usb/fcs,fsa4480.yaml
> index f9410eb76a62..8b25b9a01ced 100644
> --- a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
> +++ b/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
> @@ -27,13 +27,8 @@ properties:
>    vcc-supply:
>      description: power supply (2.7V-5.5V)
> =20
> -  mode-switch:
> -    description: Flag the port as possible handle of altmode switching
> -    type: boolean
> -
> -  orientation-switch:
> -    description: Flag the port as possible handler of orientation switch=
ing
> -    type: boolean
> +  mode-switch: true
> +  orientation-switch: true
> =20
>    port:
>      $ref: /schemas/graph.yaml#/$defs/port-base
> @@ -79,6 +74,9 @@ required:
>    - reg
>    - port
> =20
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
> =20
>  examples:

Hi Krzysztof,

This patch seems to break validation for fsa4480 if data-lanes is set in
the endpoint like the following

diff --git a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml b/Docum=
entation/devicetree/bindings/usb/fcs,fsa4480.yaml
index f9410eb76a62..3aa03fd65556 100644
--- a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
+++ b/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
@@ -102,6 +102,7 @@ examples:
           port {
             fsa4480_ept: endpoint {
               remote-endpoint =3D <&typec_controller>;
+              data-lanes =3D <0 1>;
             };
           };
         };

Similar to how it's already used on qcom/qcm6490-fairphone-fp5.dts

I'm guessing the 'port' definition in the common schema somehow
disallows the fsa4480 schema from describing it further?

Regards
Luca


> diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Do=
cumentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> index d3b2b666ec2a..88e1607cf053 100644
> --- a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> +++ b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml
> @@ -33,13 +33,8 @@ properties:
>    vcc-supply:
>      description: power supply
> =20
> -  mode-switch:
> -    description: Flag the port as possible handle of altmode switching
> -    type: boolean
> -
> -  orientation-switch:
> -    description: Flag the port as possible handler of orientation switch=
ing
> -    type: boolean
> +  mode-switch: true
> +  orientation-switch: true
> =20
>    port:
>      $ref: /schemas/graph.yaml#/properties/port
> @@ -54,6 +49,9 @@ required:
>    - orientation-switch
>    - port
> =20
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
> =20
>  examples:
> diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn36502.yaml b/Do=
cumentation/devicetree/bindings/usb/nxp,ptn36502.yaml
> index eee548ac1abe..d805dde80796 100644
> --- a/Documentation/devicetree/bindings/usb/nxp,ptn36502.yaml
> +++ b/Documentation/devicetree/bindings/usb/nxp,ptn36502.yaml
> @@ -20,13 +20,8 @@ properties:
>    vdd18-supply:
>      description: Power supply for VDD18 pin
> =20
> -  retimer-switch:
> -    description: Flag the port as possible handle of SuperSpeed signals =
retiming
> -    type: boolean
> -
> -  orientation-switch:
> -    description: Flag the port as possible handler of orientation switch=
ing
> -    type: boolean
> +  orientation-switch: true
> +  retimer-switch: true
> =20
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -49,6 +44,9 @@ required:
>    - compatible
>    - reg
> =20
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
> =20
>  examples:
> diff --git a/Documentation/devicetree/bindings/usb/onnn,nb7vpq904m.yaml b=
/Documentation/devicetree/bindings/usb/onnn,nb7vpq904m.yaml
> index c0201da002f6..589914d22bf2 100644
> --- a/Documentation/devicetree/bindings/usb/onnn,nb7vpq904m.yaml
> +++ b/Documentation/devicetree/bindings/usb/onnn,nb7vpq904m.yaml
> @@ -21,14 +21,8 @@ properties:
>      description: power supply (1.8V)
> =20
>    enable-gpios: true
> -
> -  retimer-switch:
> -    description: Flag the port as possible handle of SuperSpeed signals =
retiming
> -    type: boolean
> -
> -  orientation-switch:
> -    description: Flag the port as possible handler of orientation switch=
ing
> -    type: boolean
> +  orientation-switch: true
> +  retimer-switch: true
> =20
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -95,6 +89,9 @@ required:
>    - compatible
>    - reg
> =20
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
> =20
>  examples:
> diff --git a/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yam=
l b/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml
> index 7ddfd3313a18..96346723f3e9 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml
> +++ b/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml
> @@ -35,13 +35,8 @@ properties:
>    vdd-supply:
>      description: USBSS VDD power supply
> =20
> -  mode-switch:
> -    description: Flag the port as possible handle of altmode switching
> -    type: boolean
> -
> -  orientation-switch:
> -    description: Flag the port as possible handler of orientation switch=
ing
> -    type: boolean
> +  mode-switch: true
> +  orientation-switch: true
> =20
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -63,6 +58,9 @@ required:
>    - reg
>    - ports
> =20
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
> =20
>  examples:
> diff --git a/Documentation/devicetree/bindings/usb/usb-switch.yaml b/Docu=
mentation/devicetree/bindings/usb/usb-switch.yaml
> new file mode 100644
> index 000000000000..da76118e73a5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb-switch.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/usb-switch.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: USB Orientation and Mode Switches Common Properties
> +
> +maintainers:
> +  - Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> +
> +description:
> +  Common properties for devices handling USB mode and orientation switch=
ing.
> +
> +properties:
> +  mode-switch:
> +    description: Possible handler of altmode switching
> +    type: boolean
> +
> +  orientation-switch:
> +    description: Possible handler of orientation switching
> +    type: boolean
> +
> +  retimer-switch:
> +    description: Possible handler of SuperSpeed signals retiming
> +    type: boolean
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description:
> +      A port node to link the device to a TypeC controller for the purpo=
se of
> +      handling altmode muxing and orientation switching.
> +
> +  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
> +        unevaluatedProperties: false
> +
> +        properties:
> +          endpoint:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                minItems: 1
> +                maxItems: 8
> +                uniqueItems: true
> +                items:
> +                  maximum: 8
> +
> +oneOf:
> +  - required:
> +      - port
> +  - required:
> +      - ports
> +
> +additionalProperties: true