| Message ID | 20220607190131.1647511-5-pmalani@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: typec: Introduce typec-switch binding | expand |
On 07/06/2022 21:00, Prashant Malani wrote: > Analogix 7625 can be used in systems to switch USB Type-C DisplayPort > alternate mode lane traffic between 2 Type-C ports. > > Update the binding to accommodate this usage by introducing a switch > property. > > Signed-off-by: Prashant Malani <pmalani@chromium.org> > --- > .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > index 35a48515836e..7e1f655ddfcc 100644 > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > @@ -105,6 +105,26 @@ properties: > - port@0 > - port@1 > > + switches: > + type: object > + description: Set of switches controlling DisplayPort traffic on > + outgoing RX/TX lanes to Type C ports. > + > + properties: > + switch: You allow only one switch with such schema, so no need for "switches"... > + $ref: /schemas/usb/typec-switch.yaml# > + maxItems: 2 Are you sure this works? what are you limiting here with maxItems? I think you wanted patternProperties... > + > + properties: > + reg: > + maxItems: 1 > + > + required: > + - reg > + > + required: > + - switch@0 This does not match the property. You also need unevaluatedProperties:false > + > required: > - compatible > - reg > @@ -167,5 +187,41 @@ examples: > }; > }; > }; > + switches { > + #address-cells = <1>; > + #size-cells = <0>; > + switch@0 { > + compatible = "typec-switch"; > + reg = <0>; > + mode-switch; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + anx_typec0: endpoint { > + remote-endpoint = <&typec_port0>; Messed up indentation. Your previous patch should also switch to 4-space as recommended by schema coding style. > + }; > + }; > + }; > + }; > + switch@1 { > + compatible = "typec-switch"; > + reg = <1>; > + mode-switch; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + anx_typec1: endpoint { > + remote-endpoint = <&typec_port1>; Ditto. > + }; > + }; > + }; > + }; > + }; > }; > }; Best regards, Krzysztof
Hi Krzysztof, Thank you for looking at the patch. On Jun 08 11:24, Krzysztof Kozlowski wrote: > On 07/06/2022 21:00, Prashant Malani wrote: > > Analogix 7625 can be used in systems to switch USB Type-C DisplayPort > > alternate mode lane traffic between 2 Type-C ports. > > > > Update the binding to accommodate this usage by introducing a switch > > property. > > > > Signed-off-by: Prashant Malani <pmalani@chromium.org> > > --- > > .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++ > > 1 file changed, 56 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > index 35a48515836e..7e1f655ddfcc 100644 > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > @@ -105,6 +105,26 @@ properties: > > - port@0 > > - port@1 > > > > + switches: > > + type: object > > + description: Set of switches controlling DisplayPort traffic on > > + outgoing RX/TX lanes to Type C ports. > > + > > + properties: > > + switch: > > You allow only one switch with such schema, so no need for "switches"... See below comment (summary: we'd like to allow 1 or 2 switches). > > > + $ref: /schemas/usb/typec-switch.yaml# > > + maxItems: 2 > > Are you sure this works? what are you limiting here with maxItems? I > think you wanted patternProperties... Yeah, I might not have used the DT syntax correctly here. What I'm aiming for is: "switches" should can contain 1 or 2 "switch" nodes. 2 is the maximum (limitation of the hardware). > > > + > > + properties: > > + reg: > > + maxItems: 1 > > + > > + required: > > + - reg > > + > > + required: > > + - switch@0 > > This does not match the property. > > You also need unevaluatedProperties:false Ack, will update this in the next version. > > > > + > > required: > > - compatible > > - reg > > @@ -167,5 +187,41 @@ examples: > > }; > > }; > > }; > > + switches { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + switch@0 { > > + compatible = "typec-switch"; > > + reg = <0>; > > + mode-switch; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + port@0 { > > + reg = <0>; > > + anx_typec0: endpoint { > > + remote-endpoint = <&typec_port0>; > > Messed up indentation. Your previous patch should also switch to 4-space > as recommended by schema coding style. Sorry about that, will fix up the indentation in the next version. > > > + }; > > + }; > > + }; > > + }; > > + switch@1 { > > + compatible = "typec-switch"; > > + reg = <1>; > > + mode-switch; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + port@0 { > > + reg = <0>; > > + anx_typec1: endpoint { > > + remote-endpoint = <&typec_port1>; > > Ditto. > > > + }; > > + }; > > + }; > > + }; > > + }; > > }; > > }; > > > Best regards, > Krzysztof
On Wed, Jun 8, 2022 at 10:08 AM Prashant Malani <pmalani@chromium.org> wrote: > > Hi Krzysztof, > > Thank you for looking at the patch. > > On Jun 08 11:24, Krzysztof Kozlowski wrote: > > On 07/06/2022 21:00, Prashant Malani wrote: > > > Analogix 7625 can be used in systems to switch USB Type-C DisplayPort > > > alternate mode lane traffic between 2 Type-C ports. > > > > > > Update the binding to accommodate this usage by introducing a switch > > > property. > > > > > > Signed-off-by: Prashant Malani <pmalani@chromium.org> > > > --- > > > .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++ > > > 1 file changed, 56 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > index 35a48515836e..7e1f655ddfcc 100644 > > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > > > @@ -105,6 +105,26 @@ properties: > > > - port@0 > > > - port@1 > > > > > > + switches: > > > + type: object > > > + description: Set of switches controlling DisplayPort traffic on > > > + outgoing RX/TX lanes to Type C ports. > > > + > > > + properties: > > > + switch: > > > > You allow only one switch with such schema, so no need for "switches"... > > See below comment (summary: we'd like to allow 1 or 2 switches). > > > > > + $ref: /schemas/usb/typec-switch.yaml# > > > + maxItems: 2 > > > > Are you sure this works? what are you limiting here with maxItems? I > > think you wanted patternProperties... > > Yeah, I might not have used the DT syntax correctly here. > What I'm aiming for is: > "switches" should can contain 1 or 2 "switch" nodes. > 2 is the maximum (limitation of the hardware). > > > > > > + > > > + properties: > > > + reg: > > > + maxItems: 1 > > > + > > > + required: > > > + - reg > > > + > > > + required: > > > + - switch@0 > > > > This does not match the property. > > > > You also need unevaluatedProperties:false > > Ack, will update this in the next version. Actually, could you kindly clarify which of the two needs this? "switches" or "switch" ? I interpreted "switch" as requiring it, but I thought it better to confirm. > > > > > > > > + > > > required: > > > - compatible > > > - reg > > > @@ -167,5 +187,41 @@ examples: > > > }; > > > }; > > > }; > > > + switches { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + switch@0 { > > > + compatible = "typec-switch"; > > > + reg = <0>; > > > + mode-switch; > > > + > > > + ports { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + port@0 { > > > + reg = <0>; > > > + anx_typec0: endpoint { > > > + remote-endpoint = <&typec_port0>; > > > > Messed up indentation. Your previous patch should also switch to 4-space > > as recommended by schema coding style. > > Sorry about that, will fix up the indentation in the next version. > > > > > > + }; > > > + }; > > > + }; > > > + }; > > > + switch@1 { > > > + compatible = "typec-switch"; > > > + reg = <1>; > > > + mode-switch; > > > + > > > + ports { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + port@0 { > > > + reg = <0>; > > > + anx_typec1: endpoint { > > > + remote-endpoint = <&typec_port1>; > > > > Ditto. > > > > > + }; > > > + }; > > > + }; > > > + }; > > > + }; > > > }; > > > }; > > > > > > Best regards, > > Krzysztof
On 08/06/2022 23:56, Prashant Malani wrote: > On Wed, Jun 8, 2022 at 10:08 AM Prashant Malani <pmalani@chromium.org> wrote: >> >> Hi Krzysztof, >> >> Thank you for looking at the patch. >> >> On Jun 08 11:24, Krzysztof Kozlowski wrote: >>> On 07/06/2022 21:00, Prashant Malani wrote: >>>> Analogix 7625 can be used in systems to switch USB Type-C DisplayPort >>>> alternate mode lane traffic between 2 Type-C ports. >>>> >>>> Update the binding to accommodate this usage by introducing a switch >>>> property. >>>> >>>> Signed-off-by: Prashant Malani <pmalani@chromium.org> >>>> --- >>>> .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++ >>>> 1 file changed, 56 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml >>>> index 35a48515836e..7e1f655ddfcc 100644 >>>> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml >>>> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml >>>> @@ -105,6 +105,26 @@ properties: >>>> - port@0 >>>> - port@1 >>>> >>>> + switches: >>>> + type: object >>>> + description: Set of switches controlling DisplayPort traffic on >>>> + outgoing RX/TX lanes to Type C ports. >>>> + >>>> + properties: >>>> + switch: >>> >>> You allow only one switch with such schema, so no need for "switches"... >> >> See below comment (summary: we'd like to allow 1 or 2 switches). >>> >>>> + $ref: /schemas/usb/typec-switch.yaml# >>>> + maxItems: 2 >>> >>> Are you sure this works? what are you limiting here with maxItems? I >>> think you wanted patternProperties... >> >> Yeah, I might not have used the DT syntax correctly here. >> What I'm aiming for is: >> "switches" should can contain 1 or 2 "switch" nodes. >> 2 is the maximum (limitation of the hardware). >> >>> >>>> + >>>> + properties: >>>> + reg: >>>> + maxItems: 1 >>>> + >>>> + required: >>>> + - reg >>>> + >>>> + required: >>>> + - switch@0 >>> >>> This does not match the property. >>> >>> You also need unevaluatedProperties:false >> >> Ack, will update this in the next version. > > Actually, could you kindly clarify which of the two needs this? > "switches" or "switch" ? > I interpreted "switch" as requiring it, but I thought it better to confirm. Depends what do you want to have there. If two properties called "switch", then "switches" is ok. However old code had only one property thus switches with maximum one switch is a bit weird. Looking at example you wanted to switch@[01], so you need to use patternProperties. Best regards, Krzysztof
On Wed, Jun 8, 2022 at 11:41 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/06/2022 23:56, Prashant Malani wrote: > > On Wed, Jun 8, 2022 at 10:08 AM Prashant Malani <pmalani@chromium.org> wrote: > >> > >> Hi Krzysztof, > >> > >> Thank you for looking at the patch. > >> > >> On Jun 08 11:24, Krzysztof Kozlowski wrote: > >>> On 07/06/2022 21:00, Prashant Malani wrote: > >>>> Analogix 7625 can be used in systems to switch USB Type-C DisplayPort > >>>> alternate mode lane traffic between 2 Type-C ports. > >>>> > >>>> Update the binding to accommodate this usage by introducing a switch > >>>> property. > >>>> > >>>> Signed-off-by: Prashant Malani <pmalani@chromium.org> > >>>> --- > >>>> .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++ > >>>> 1 file changed, 56 insertions(+) > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > >>>> index 35a48515836e..7e1f655ddfcc 100644 > >>>> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > >>>> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > >>>> @@ -105,6 +105,26 @@ properties: > >>>> - port@0 > >>>> - port@1 > >>>> > >>>> + switches: > >>>> + type: object > >>>> + description: Set of switches controlling DisplayPort traffic on > >>>> + outgoing RX/TX lanes to Type C ports. > >>>> + > >>>> + properties: > >>>> + switch: > >>> > >>> You allow only one switch with such schema, so no need for "switches"... > >> > >> See below comment (summary: we'd like to allow 1 or 2 switches). > >>> > >>>> + $ref: /schemas/usb/typec-switch.yaml# > >>>> + maxItems: 2 > >>> > >>> Are you sure this works? what are you limiting here with maxItems? I > >>> think you wanted patternProperties... > >> > >> Yeah, I might not have used the DT syntax correctly here. > >> What I'm aiming for is: > >> "switches" should can contain 1 or 2 "switch" nodes. > >> 2 is the maximum (limitation of the hardware). > >> > >>> > >>>> + > >>>> + properties: > >>>> + reg: > >>>> + maxItems: 1 > >>>> + > >>>> + required: > >>>> + - reg > >>>> + > >>>> + required: > >>>> + - switch@0 > >>> > >>> This does not match the property. > >>> > >>> You also need unevaluatedProperties:false > >> > >> Ack, will update this in the next version. > > > > Actually, could you kindly clarify which of the two needs this? > > "switches" or "switch" ? > > I interpreted "switch" as requiring it, but I thought it better to confirm. > > Depends what do you want to have there. If two properties called > "switch", then "switches" is ok. However old code had only one property > thus switches with maximum one switch is a bit weird. > > Looking at example you wanted to switch@[01], so you need to use > patternProperties. Thanks for the suggestion. I've made the change in v2. Regards, -Prashant
diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml index 35a48515836e..7e1f655ddfcc 100644 --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml @@ -105,6 +105,26 @@ properties: - port@0 - port@1 + switches: + type: object + description: Set of switches controlling DisplayPort traffic on + outgoing RX/TX lanes to Type C ports. + + properties: + switch: + $ref: /schemas/usb/typec-switch.yaml# + maxItems: 2 + + properties: + reg: + maxItems: 1 + + required: + - reg + + required: + - switch@0 + required: - compatible - reg @@ -167,5 +187,41 @@ examples: }; }; }; + switches { + #address-cells = <1>; + #size-cells = <0>; + switch@0 { + compatible = "typec-switch"; + reg = <0>; + mode-switch; + + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + anx_typec0: endpoint { + remote-endpoint = <&typec_port0>; + }; + }; + }; + }; + switch@1 { + compatible = "typec-switch"; + reg = <1>; + mode-switch; + + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + anx_typec1: endpoint { + remote-endpoint = <&typec_port1>; + }; + }; + }; + }; + }; }; };
Analogix 7625 can be used in systems to switch USB Type-C DisplayPort alternate mode lane traffic between 2 Type-C ports. Update the binding to accommodate this usage by introducing a switch property. Signed-off-by: Prashant Malani <pmalani@chromium.org> --- .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+)