diff mbox series

[13/22] dt-bindings: chrome: Add google,cros-ec-typec-switch binding

Message ID 20240210070934.2549994-14-swboyd@chromium.org (mailing list archive)
State New, archived
Headers show
Series platform/chrome: Add DT USB/DP muxing/topology to Trogdor | expand

Commit Message

Stephen Boyd Feb. 10, 2024, 7:09 a.m. UTC
Add a binding for the USB type-c switch controls found on some ChromeOS
Embedded Controllers (ECs). When this device is a mode switch, it takes
one DisplayPort (DP) port as input and some number (possibly zero) of
USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
a usb-c-connector. When this device is an orientation switch, it
redirects the DP lanes to the proper USB type-c SSTRX lanes.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Lee Jones <lee@kernel.org>
Cc: Benson Leung <bleung@chromium.org>
Cc: Guenter Roeck <groeck@chromium.org>
Cc: Prashant Malani <pmalani@chromium.org>
Cc: Tzung-Bi Shih <tzungbi@kernel.org>
Cc: <devicetree@vger.kernel.org>
Cc: <chrome-platform@lists.linux.dev>
Cc: Pin-yen Lin <treapking@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 .../chrome/google,cros-ec-typec-switch.yaml   | 365 ++++++++++++++++++
 .../bindings/mfd/google,cros-ec.yaml          |   5 +
 2 files changed, 370 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml

Comments

Krzysztof Kozlowski Feb. 11, 2024, 1:34 p.m. UTC | #1
On 10/02/2024 08:09, Stephen Boyd wrote:
> Add a binding for the USB type-c switch controls found on some ChromeOS
> Embedded Controllers (ECs). When this device is a mode switch, it takes
> one DisplayPort (DP) port as input and some number (possibly zero) of
> USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
> lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
> a usb-c-connector. When this device is an orientation switch, it
> redirects the DP lanes to the proper USB type-c SSTRX lanes.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Prashant Malani <pmalani@chromium.org>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Pin-yen Lin <treapking@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  .../chrome/google,cros-ec-typec-switch.yaml   | 365 ++++++++++++++++++
>  .../bindings/mfd/google,cros-ec.yaml          |   5 +
>  2 files changed, 370 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> 
> diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> new file mode 100644
> index 000000000000..17a0ba928f5d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> @@ -0,0 +1,365 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/chrome/google,cros-ec-typec-switch.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Google Chrome OS EC(Embedded Controller) USB Type C Switch
> +
> +maintainers:
> +  - Benson Leung <bleung@chromium.org>
> +  - Prashant Malani <pmalani@chromium.org>
> +  - Stephen Boyd <swboyd@chromium.org>
> +
> +description:
> +  Chrome OS devices have an Embedded Controller(EC) which has access to USB
> +  Type C switching. This node is intended to allow the OS to control Type C
> +  signal muxing for USB-C orientation and alternate modes. The node for this
> +  device should be under a cros-ec node like google,cros-ec-spi.
> +

If this is USB Type C switch, then you miss reference to
usb-switch.yaml, but then ports look a bit different.

> +properties:
> +  compatible:
> +    const: google,cros-ec-typec-switch
> +
> +  mode-switch:
> +    description: Indicates this device controls altmode switching
> +    type: boolean
> +
> +  orientation-switch:
> +    description: Indicates this device controls orientation switching
> +    type: boolean
> +
> +  mux-gpios:
> +    description: GPIOs indicating which way the DP mux is steered

missing maxItems

> +
> +  no-hpd:
> +    description: Indicates this device doesn't signal HPD for DisplayPort
> +    type: boolean
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        description: Input port to receive DisplayPort (DP) data
> +        unevaluatedProperties: false
> +
> +        properties:
> +          endpoint@0:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description: DisplayPort data
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                description: |
> +                  An array of physical DP data lane indexes
> +                  - 0 is DP ML0 lane
> +                  - 1 is DP ML1 lane
> +                  - 2 is DP ML2 lane
> +                  - 3 is DP ML3 lane
> +                oneOf:
> +                  - items:
> +                      - const: 0
> +                      - const: 1
> +                  - items:
> +                      - const: 0
> +                      - const: 1
> +                      - const: 2
> +                      - const: 3
> +
> +        required:
> +          - endpoint@0
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port to receive USB SuperSpeed (SS) data
> +        properties:
> +          endpoint@0:
> +            $ref: /schemas/graph.yaml#/properties/endpoint
> +            description: USB SS data
> +
> +          endpoint@1:
> +            $ref: /schemas/graph.yaml#/properties/endpoint
> +            description: USB SS data
> +
> +        anyOf:
> +          - required:
> +              - endpoint@0
> +          - required:
> +              - endpoint@1
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Output port for USB-C data
> +        properties:
> +          endpoint@0:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description: USB-C data
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                description: |
> +                  An array of physical USB-C data lane indexes.
> +                  - 0 is SSRX1 lane
> +                  - 1 is SSTX1 lane
> +                  - 2 is SSTX2 lane
> +                  - 3 is SSRX2 lane
> +                minItems: 4
> +                maxItems: 4
> +                items:
> +                  maximum: 3
> +
> +          endpoint@1:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description: USB-C data for EC's 1st type-c port
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                description: |
> +                  An array of physical USB-C data lane indexes.
> +                  - 0 is SSRX1 lane
> +                  - 1 is SSTX1 lane
> +                  - 2 is SSTX2 lane
> +                  - 3 is SSRX2 lane
> +                minItems: 4
> +                maxItems: 4
> +                items:
> +                  maximum: 3
> +
> +          endpoint@2:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description: USB-C data for EC's 2nd type-c port
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                description: |
> +                  An array of physical USB-C data lane indexes.
> +                  - 0 is SSRX1 lane
> +                  - 1 is SSTX1 lane
> +                  - 2 is SSTX2 lane
> +                  - 3 is SSRX2 lane
> +                minItems: 4
> +                maxItems: 4
> +                items:
> +                  maximum: 3
> +
> +          endpoint@3:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description: USB-C data for EC's 3rd type-c port
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                description: |
> +                  An array of physical USB-C data lane indexes.
> +                  - 0 is SSRX1 lane
> +                  - 1 is SSTX1 lane
> +                  - 2 is SSTX2 lane
> +                  - 3 is SSRX2 lane
> +                minItems: 4
> +                maxItems: 4
> +                items:
> +                  maximum: 3
> +
> +          endpoint@4:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description: USB-C data for EC's 4th type-c port
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                description: |
> +                  An array of physical USB-C data lane indexes.
> +                  - 0 is SSRX1 lane
> +                  - 1 is SSTX1 lane
> +                  - 2 is SSTX2 lane
> +                  - 3 is SSRX2 lane
> +                minItems: 4
> +                maxItems: 4
> +                items:
> +                  maximum: 3
> +
> +          endpoint@5:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description: USB-C data for EC's 5th type-c port
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                description: |
> +                  An array of physical USB-C data lane indexes.
> +                  - 0 is SSRX1 lane
> +                  - 1 is SSTX1 lane
> +                  - 2 is SSTX2 lane
> +                  - 3 is SSRX2 lane
> +                minItems: 4
> +                maxItems: 4
> +                items:
> +                  maximum: 3
> +
> +          endpoint@6:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description: USB-C data for EC's 6th type-c port
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                description: |
> +                  An array of physical USB-C data lane indexes.
> +                  - 0 is SSRX1 lane
> +                  - 1 is SSTX1 lane
> +                  - 2 is SSTX2 lane
> +                  - 3 is SSRX2 lane
> +                minItems: 4
> +                maxItems: 4
> +                items:
> +                  maximum: 3
> +
> +          endpoint@7:
> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +            description: USB-C data for EC's 7th type-c port
> +            unevaluatedProperties: false
> +            properties:
> +              data-lanes:
> +                $ref: /schemas/types.yaml#/definitions/uint32-array
> +                description: |
> +                  An array of physical USB-C data lane indexes.
> +                  - 0 is SSRX1 lane
> +                  - 1 is SSTX1 lane
> +                  - 2 is SSTX2 lane
> +                  - 3 is SSRX2 lane
> +                minItems: 4
> +                maxItems: 4
> +                items:
> +                  maximum: 3
> +
> +        anyOf:
> +          - required:
> +              - endpoint@0

I don't get what you want to say here. This anyOf should have no effect.

> +          - required:
> +              - endpoint@1
> +          - required:
> +              - endpoint@2
> +          - required:
> +              - endpoint@3
> +          - required:
> +              - endpoint@4
> +          - required:
> +              - endpoint@5
> +          - required:
> +              - endpoint@6
> +          - required:
> +              - endpoint@7
> +
> +    required:
> +      - port@2
> +    anyOf:
> +      - required:
> +          - port@0
> +      - required:
> +          - port@1

Same problem here

> +
> +required:
> +  - compatible
> +  - ports
> +
> +allOf:
> +  - if:
> +      properties:
> +        no-hpd: true

I don't understand this either. What is it for? Where did you see such
syntax?


> +      required:
> +        - no-hpd> +    then:
> +      properties:
> +        ports:
> +          required:
> +            - port@0
> +  - if:
> +      properties:
> +        mode-switch: true
> +      required:
> +        - mode-switch
> +    then:
> +      properties:
> +        ports:
> +          required:
> +            - port@0
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      cros_ec: ec@0 {
> +        compatible = "google,cros-ec-spi";
> +        reg = <0>;
> +        interrupts = <35 0>;

Same concerns about interrupts.

> +
> +        typec-switch {
> +          compatible = "google,cros-ec-typec-switch";
> +          mode-switch;
> +          orientation-switch;
> +
> +          ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +              reg = <0>;
> +              #address-cells = <1>;
> +              #size-cells = <0>;
> +              dp_in: endpoint@0 {
> +                reg = <0>;
> +                remote-endpoint = <&dp_phy>;
> +                data-lanes = <0 1>;
> +              };
> +            };
> +
> +            port@1 {
> +              reg = <1>;
> +              #address-cells = <1>;
> +              #size-cells = <0>;
> +
> +              usb_in_0: endpoint@0 {
> +                reg = <0>;
> +                remote-endpoint = <&usb_ss_0_out>;
> +              };
> +
> +              usb_in_1: endpoint@1 {
> +                reg = <1>;
> +                remote-endpoint = <&usb_ss_1_out>;
> +              };
> +            };
> +
> +            port@2 {
> +              reg = <2>;
> +              #address-cells = <1>;
> +              #size-cells = <0>;
> +
> +              cros_typec_c0_ss: endpoint@0 {
> +                reg = <0>;
> +                remote-endpoint = <&usb_c0_ss>;
> +              };
> +
> +              cros_typec_c1_ss: endpoint@1 {
> +                reg = <1>;
> +                remote-endpoint = <&usb_c1_ss>;
> +              };
> +            };
> +          };
> +        };
> +      };
> +    };
> +...
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index ded396b28fba..563c51a4a39c 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -164,6 +164,10 @@ patternProperties:
>      type: object
>      $ref: /schemas/extcon/extcon-usbc-cros-ec.yaml#
>  
> +  "^typec-switch[0-9]*$":

Missing - after typec-switch (e.g. typec-switch-1)

> +    type: object
> +    $ref: /schemas/chrome/google,cros-ec-typec-switch.yaml#


Best regards,
Krzysztof
Krzysztof Kozlowski Feb. 11, 2024, 1:35 p.m. UTC | #2
On 10/02/2024 08:09, Stephen Boyd wrote:
> Add a binding for the USB type-c switch controls found on some ChromeOS
> Embedded Controllers (ECs). When this device is a mode switch, it takes
> one DisplayPort (DP) port as input and some number (possibly zero) of
> USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
> lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
> a usb-c-connector. When this device is an orientation switch, it
> redirects the DP lanes to the proper USB type-c SSTRX lanes.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Prashant Malani <pmalani@chromium.org>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Pin-yen Lin <treapking@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  .../chrome/google,cros-ec-typec-switch.yaml   | 365 ++++++++++++++++++
>  .../bindings/mfd/google,cros-ec.yaml          |   5 +
>  2 files changed, 370 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml

Ah, and wrong placement. There is no hardware called "chrome", please
don't stuff things there. USB switches go to other USB switches (git
grep usb-switch.yaml will give you hints).

Best regards,
Krzysztof
Stephen Boyd Feb. 15, 2024, 1:52 a.m. UTC | #3
Quoting Krzysztof Kozlowski (2024-02-11 05:34:25)
> On 10/02/2024 08:09, Stephen Boyd wrote:
> > Add a binding for the USB type-c switch controls found on some ChromeOS
> > Embedded Controllers (ECs). When this device is a mode switch, it takes
> > one DisplayPort (DP) port as input and some number (possibly zero) of
> > USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
> > lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
> > a usb-c-connector. When this device is an orientation switch, it
> > redirects the DP lanes to the proper USB type-c SSTRX lanes.
> >
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > Cc: Conor Dooley <conor+dt@kernel.org>
> > Cc: Lee Jones <lee@kernel.org>
> > Cc: Benson Leung <bleung@chromium.org>
> > Cc: Guenter Roeck <groeck@chromium.org>
> > Cc: Prashant Malani <pmalani@chromium.org>
> > Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> > Cc: <devicetree@vger.kernel.org>
> > Cc: <chrome-platform@lists.linux.dev>
> > Cc: Pin-yen Lin <treapking@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >  .../chrome/google,cros-ec-typec-switch.yaml   | 365 ++++++++++++++++++
> >  .../bindings/mfd/google,cros-ec.yaml          |   5 +
> >  2 files changed, 370 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> > new file mode 100644
> > index 000000000000..17a0ba928f5d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> > @@ -0,0 +1,365 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/chrome/google,cros-ec-typec-switch.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Google Chrome OS EC(Embedded Controller) USB Type C Switch
> > +
> > +maintainers:
> > +  - Benson Leung <bleung@chromium.org>
> > +  - Prashant Malani <pmalani@chromium.org>
> > +  - Stephen Boyd <swboyd@chromium.org>
> > +
> > +description:
> > +  Chrome OS devices have an Embedded Controller(EC) which has access to USB
> > +  Type C switching. This node is intended to allow the OS to control Type C
> > +  signal muxing for USB-C orientation and alternate modes. The node for this
> > +  device should be under a cros-ec node like google,cros-ec-spi.
> > +
>
> If this is USB Type C switch, then you miss reference to
> usb-switch.yaml, but then ports look a bit different.

Thanks for the pointer. I suspect that's in linux-next only? I'm going
to move this to the cros-ec-typec.yaml file but I'll still need some
sort of property like 'mode-switch' or 'orientation-switch' to describe
what needs to be done in the kernel by changing lane assignments in the
drm_bridge.

One problem with those properties is that they're boolean for the whole
device. If I have a google,cros-ec-typec node that has two input DP
ports and two output USB-C ports then it may be that one port needs
orientation switching and the other only needs to do mode switching.
This needs to be expressed somehow and a top-level boolean property
doesn't do that. It could be part of the DP endpoint node itself.

Also I don't know how to indicate that mode switching can't be changed
here directly. For example, on Trogdor the mode is switched by the EC,
and the kernel can't change the mode. Changing DP lane assignments isn't
going to change the situation either. The mode is still going to be
something like DP+USB, or just DP, etc. Maybe there needs to be a
different property, like 'dp-mode-lane-assignment = <array of ports>',
that indicates which DP ports need to do lane assignment or
'dp-orientation-lane-assignment = <array of ports>' for orientation
control. Either way, I'm saying that 'mode-switch' and
'orientation-switch' properties don't make any sense here. I was using
them to wedge the code into the typec switch callbacks, but if I move
this into the cros-ec-typec driver then I don't need them.

>
> > +
> > +  no-hpd:
> > +    description: Indicates this device doesn't signal HPD for DisplayPort
> > +    type: boolean
> > +
> > +  ports:
[...]
> > +
> > +          endpoint@7:
> > +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> > +            description: USB-C data for EC's 7th type-c port
> > +            unevaluatedProperties: false
> > +            properties:
> > +              data-lanes:
> > +                $ref: /schemas/types.yaml#/definitions/uint32-array
> > +                description: |
> > +                  An array of physical USB-C data lane indexes.
> > +                  - 0 is SSRX1 lane
> > +                  - 1 is SSTX1 lane
> > +                  - 2 is SSTX2 lane
> > +                  - 3 is SSRX2 lane
> > +                minItems: 4
> > +                maxItems: 4
> > +                items:
> > +                  maximum: 3
> > +
> > +        anyOf:
> > +          - required:
> > +              - endpoint@0
>
> I don't get what you want to say here. This anyOf should have no effect.

I'm trying to say that there should be at least one usb-c data lane
output endpoint if there's a port@2 (usb-c output).

>
> > +          - required:
> > +              - endpoint@1
> > +          - required:
> > +              - endpoint@2
[...]
>
> > +
> > +required:
> > +  - compatible
> > +  - ports
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        no-hpd: true
>
> I don't understand this either. What is it for? Where did you see such
> syntax?

This is saying that if the no-hpd property is present then the port@0
port (DP input port) is required. Otherwise no-hpd doesn't really make
any sense because this device isn't muxing DP onto USB type-c connectors.

I found this syntax by searching the schema website and reading this
page[1]. The last yellow note about "country" not being a required
property led me to this syntax.

[1] https://json-schema.org/understanding-json-schema/reference/conditionals#ifthenelse
Krzysztof Kozlowski Feb. 15, 2024, 8:34 a.m. UTC | #4
On 15/02/2024 02:52, Stephen Boyd wrote:
> Quoting Krzysztof Kozlowski (2024-02-11 05:34:25)
>> On 10/02/2024 08:09, Stephen Boyd wrote:
>>> Add a binding for the USB type-c switch controls found on some ChromeOS
>>> Embedded Controllers (ECs). When this device is a mode switch, it takes
>>> one DisplayPort (DP) port as input and some number (possibly zero) of
>>> USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
>>> lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
>>> a usb-c-connector. When this device is an orientation switch, it
>>> redirects the DP lanes to the proper USB type-c SSTRX lanes.
>>>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>>> Cc: Conor Dooley <conor+dt@kernel.org>
>>> Cc: Lee Jones <lee@kernel.org>
>>> Cc: Benson Leung <bleung@chromium.org>
>>> Cc: Guenter Roeck <groeck@chromium.org>
>>> Cc: Prashant Malani <pmalani@chromium.org>
>>> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
>>> Cc: <devicetree@vger.kernel.org>
>>> Cc: <chrome-platform@lists.linux.dev>
>>> Cc: Pin-yen Lin <treapking@chromium.org>
>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>> ---
>>>  .../chrome/google,cros-ec-typec-switch.yaml   | 365 ++++++++++++++++++
>>>  .../bindings/mfd/google,cros-ec.yaml          |   5 +
>>>  2 files changed, 370 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
>>> new file mode 100644
>>> index 000000000000..17a0ba928f5d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
>>> @@ -0,0 +1,365 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/chrome/google,cros-ec-typec-switch.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Google Chrome OS EC(Embedded Controller) USB Type C Switch
>>> +
>>> +maintainers:
>>> +  - Benson Leung <bleung@chromium.org>
>>> +  - Prashant Malani <pmalani@chromium.org>
>>> +  - Stephen Boyd <swboyd@chromium.org>
>>> +
>>> +description:
>>> +  Chrome OS devices have an Embedded Controller(EC) which has access to USB
>>> +  Type C switching. This node is intended to allow the OS to control Type C
>>> +  signal muxing for USB-C orientation and alternate modes. The node for this
>>> +  device should be under a cros-ec node like google,cros-ec-spi.
>>> +
>>
>> If this is USB Type C switch, then you miss reference to
>> usb-switch.yaml, but then ports look a bit different.
> 
> Thanks for the pointer. I suspect that's in linux-next only? I'm going

Yes, Greg's tree.

> to move this to the cros-ec-typec.yaml file but I'll still need some
> sort of property like 'mode-switch' or 'orientation-switch' to describe
> what needs to be done in the kernel by changing lane assignments in the
> drm_bridge.
> 
> One problem with those properties is that they're boolean for the whole
> device. If I have a google,cros-ec-typec node that has two input DP
> ports and two output USB-C ports then it may be that one port needs
> orientation switching and the other only needs to do mode switching.
> This needs to be expressed somehow and a top-level boolean property
> doesn't do that. It could be part of the DP endpoint node itself.

Maybe it can be added to the connector binding as well.

> 
> Also I don't know how to indicate that mode switching can't be changed
> here directly. For example, on Trogdor the mode is switched by the EC,
> and the kernel can't change the mode. Changing DP lane assignments isn't
> going to change the situation either. The mode is still going to be
> something like DP+USB, or just DP, etc. Maybe there needs to be a
> different property, like 'dp-mode-lane-assignment = <array of ports>',
> that indicates which DP ports need to do lane assignment or
> 'dp-orientation-lane-assignment = <array of ports>' for orientation
> control. Either way, I'm saying that 'mode-switch' and
> 'orientation-switch' properties don't make any sense here. I was using
> them to wedge the code into the typec switch callbacks, but if I move
> this into the cros-ec-typec driver then I don't need them.

OK

> 
>>
>>> +
>>> +  no-hpd:
>>> +    description: Indicates this device doesn't signal HPD for DisplayPort
>>> +    type: boolean
>>> +
>>> +  ports:
> [...]
>>> +
>>> +          endpoint@7:
>>> +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>> +            description: USB-C data for EC's 7th type-c port
>>> +            unevaluatedProperties: false
>>> +            properties:
>>> +              data-lanes:
>>> +                $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +                description: |
>>> +                  An array of physical USB-C data lane indexes.
>>> +                  - 0 is SSRX1 lane
>>> +                  - 1 is SSTX1 lane
>>> +                  - 2 is SSTX2 lane
>>> +                  - 3 is SSRX2 lane
>>> +                minItems: 4
>>> +                maxItems: 4
>>> +                items:
>>> +                  maximum: 3
>>> +
>>> +        anyOf:
>>> +          - required:
>>> +              - endpoint@0
>>
>> I don't get what you want to say here. This anyOf should have no effect.
> 
> I'm trying to say that there should be at least one usb-c data lane
> output endpoint if there's a port@2 (usb-c output).

Wait, my bad, this is good. At least one endpoint will be required.

> 
>>
>>> +          - required:
>>> +              - endpoint@1
>>> +          - required:
>>> +              - endpoint@2
> [...]
>>
>>> +
>>> +required:
>>> +  - compatible
>>> +  - ports
>>> +
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        no-hpd: true
>>
>> I don't understand this either. What is it for? Where did you see such
>> syntax?
> 
> This is saying that if the no-hpd property is present then the port@0
> port (DP input port) is required. Otherwise no-hpd doesn't really make
> any sense because this device isn't muxing DP onto USB type-c connectors.

Then you only need if: required: like here:

https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/net/qcom,ipa.yaml#L174

> 
> I found this syntax by searching the schema website and reading this
> page[1]. The last yellow note about "country" not being a required
> property led me to this syntax.
> 
> [1] https://json-schema.org/understanding-json-schema/reference/conditionals#ifthenelse

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
new file mode 100644
index 000000000000..17a0ba928f5d
--- /dev/null
+++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
@@ -0,0 +1,365 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/chrome/google,cros-ec-typec-switch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Google Chrome OS EC(Embedded Controller) USB Type C Switch
+
+maintainers:
+  - Benson Leung <bleung@chromium.org>
+  - Prashant Malani <pmalani@chromium.org>
+  - Stephen Boyd <swboyd@chromium.org>
+
+description:
+  Chrome OS devices have an Embedded Controller(EC) which has access to USB
+  Type C switching. This node is intended to allow the OS to control Type C
+  signal muxing for USB-C orientation and alternate modes. The node for this
+  device should be under a cros-ec node like google,cros-ec-spi.
+
+properties:
+  compatible:
+    const: google,cros-ec-typec-switch
+
+  mode-switch:
+    description: Indicates this device controls altmode switching
+    type: boolean
+
+  orientation-switch:
+    description: Indicates this device controls orientation switching
+    type: boolean
+
+  mux-gpios:
+    description: GPIOs indicating which way the DP mux is steered
+
+  no-hpd:
+    description: Indicates this device doesn't signal HPD for DisplayPort
+    type: boolean
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        description: Input port to receive DisplayPort (DP) data
+        unevaluatedProperties: false
+
+        properties:
+          endpoint@0:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description: DisplayPort data
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description: |
+                  An array of physical DP data lane indexes
+                  - 0 is DP ML0 lane
+                  - 1 is DP ML1 lane
+                  - 2 is DP ML2 lane
+                  - 3 is DP ML3 lane
+                oneOf:
+                  - items:
+                      - const: 0
+                      - const: 1
+                  - items:
+                      - const: 0
+                      - const: 1
+                      - const: 2
+                      - const: 3
+
+        required:
+          - endpoint@0
+
+      port@1:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description:
+          Input port to receive USB SuperSpeed (SS) data
+        properties:
+          endpoint@0:
+            $ref: /schemas/graph.yaml#/properties/endpoint
+            description: USB SS data
+
+          endpoint@1:
+            $ref: /schemas/graph.yaml#/properties/endpoint
+            description: USB SS data
+
+        anyOf:
+          - required:
+              - endpoint@0
+          - required:
+              - endpoint@1
+
+      port@2:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description:
+          Output port for USB-C data
+        properties:
+          endpoint@0:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description: USB-C data
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description: |
+                  An array of physical USB-C data lane indexes.
+                  - 0 is SSRX1 lane
+                  - 1 is SSTX1 lane
+                  - 2 is SSTX2 lane
+                  - 3 is SSRX2 lane
+                minItems: 4
+                maxItems: 4
+                items:
+                  maximum: 3
+
+          endpoint@1:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description: USB-C data for EC's 1st type-c port
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description: |
+                  An array of physical USB-C data lane indexes.
+                  - 0 is SSRX1 lane
+                  - 1 is SSTX1 lane
+                  - 2 is SSTX2 lane
+                  - 3 is SSRX2 lane
+                minItems: 4
+                maxItems: 4
+                items:
+                  maximum: 3
+
+          endpoint@2:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description: USB-C data for EC's 2nd type-c port
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description: |
+                  An array of physical USB-C data lane indexes.
+                  - 0 is SSRX1 lane
+                  - 1 is SSTX1 lane
+                  - 2 is SSTX2 lane
+                  - 3 is SSRX2 lane
+                minItems: 4
+                maxItems: 4
+                items:
+                  maximum: 3
+
+          endpoint@3:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description: USB-C data for EC's 3rd type-c port
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description: |
+                  An array of physical USB-C data lane indexes.
+                  - 0 is SSRX1 lane
+                  - 1 is SSTX1 lane
+                  - 2 is SSTX2 lane
+                  - 3 is SSRX2 lane
+                minItems: 4
+                maxItems: 4
+                items:
+                  maximum: 3
+
+          endpoint@4:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description: USB-C data for EC's 4th type-c port
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description: |
+                  An array of physical USB-C data lane indexes.
+                  - 0 is SSRX1 lane
+                  - 1 is SSTX1 lane
+                  - 2 is SSTX2 lane
+                  - 3 is SSRX2 lane
+                minItems: 4
+                maxItems: 4
+                items:
+                  maximum: 3
+
+          endpoint@5:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description: USB-C data for EC's 5th type-c port
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description: |
+                  An array of physical USB-C data lane indexes.
+                  - 0 is SSRX1 lane
+                  - 1 is SSTX1 lane
+                  - 2 is SSTX2 lane
+                  - 3 is SSRX2 lane
+                minItems: 4
+                maxItems: 4
+                items:
+                  maximum: 3
+
+          endpoint@6:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description: USB-C data for EC's 6th type-c port
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description: |
+                  An array of physical USB-C data lane indexes.
+                  - 0 is SSRX1 lane
+                  - 1 is SSTX1 lane
+                  - 2 is SSTX2 lane
+                  - 3 is SSRX2 lane
+                minItems: 4
+                maxItems: 4
+                items:
+                  maximum: 3
+
+          endpoint@7:
+            $ref: /schemas/graph.yaml#/$defs/endpoint-base
+            description: USB-C data for EC's 7th type-c port
+            unevaluatedProperties: false
+            properties:
+              data-lanes:
+                $ref: /schemas/types.yaml#/definitions/uint32-array
+                description: |
+                  An array of physical USB-C data lane indexes.
+                  - 0 is SSRX1 lane
+                  - 1 is SSTX1 lane
+                  - 2 is SSTX2 lane
+                  - 3 is SSRX2 lane
+                minItems: 4
+                maxItems: 4
+                items:
+                  maximum: 3
+
+        anyOf:
+          - required:
+              - endpoint@0
+          - required:
+              - endpoint@1
+          - required:
+              - endpoint@2
+          - required:
+              - endpoint@3
+          - required:
+              - endpoint@4
+          - required:
+              - endpoint@5
+          - required:
+              - endpoint@6
+          - required:
+              - endpoint@7
+
+    required:
+      - port@2
+    anyOf:
+      - required:
+          - port@0
+      - required:
+          - port@1
+
+required:
+  - compatible
+  - ports
+
+allOf:
+  - if:
+      properties:
+        no-hpd: true
+      required:
+        - no-hpd
+    then:
+      properties:
+        ports:
+          required:
+            - port@0
+  - if:
+      properties:
+        mode-switch: true
+      required:
+        - mode-switch
+    then:
+      properties:
+        ports:
+          required:
+            - port@0
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      cros_ec: ec@0 {
+        compatible = "google,cros-ec-spi";
+        reg = <0>;
+        interrupts = <35 0>;
+
+        typec-switch {
+          compatible = "google,cros-ec-typec-switch";
+          mode-switch;
+          orientation-switch;
+
+          ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+              reg = <0>;
+              #address-cells = <1>;
+              #size-cells = <0>;
+              dp_in: endpoint@0 {
+                reg = <0>;
+                remote-endpoint = <&dp_phy>;
+                data-lanes = <0 1>;
+              };
+            };
+
+            port@1 {
+              reg = <1>;
+              #address-cells = <1>;
+              #size-cells = <0>;
+
+              usb_in_0: endpoint@0 {
+                reg = <0>;
+                remote-endpoint = <&usb_ss_0_out>;
+              };
+
+              usb_in_1: endpoint@1 {
+                reg = <1>;
+                remote-endpoint = <&usb_ss_1_out>;
+              };
+            };
+
+            port@2 {
+              reg = <2>;
+              #address-cells = <1>;
+              #size-cells = <0>;
+
+              cros_typec_c0_ss: endpoint@0 {
+                reg = <0>;
+                remote-endpoint = <&usb_c0_ss>;
+              };
+
+              cros_typec_c1_ss: endpoint@1 {
+                reg = <1>;
+                remote-endpoint = <&usb_c1_ss>;
+              };
+            };
+          };
+        };
+      };
+    };
+...
diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index ded396b28fba..563c51a4a39c 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -164,6 +164,10 @@  patternProperties:
     type: object
     $ref: /schemas/extcon/extcon-usbc-cros-ec.yaml#
 
+  "^typec-switch[0-9]*$":
+    type: object
+    $ref: /schemas/chrome/google,cros-ec-typec-switch.yaml#
+
 required:
   - compatible
 
@@ -227,6 +231,7 @@  allOf:
         "^i2c-tunnel[0-9]*$": false
         "^regulator@[0-9]+$": false
         "^extcon[0-9]*$": false
+        "^typec-switch[0-9]*$": false
 
       # Using additionalProperties: false here and
       # listing true properties doesn't work