diff mbox series

[v2] dt-bindings: usb: add common Type-C USB Switch schema

Message ID 20240122094406.32198-1-krzysztof.kozlowski@linaro.org (mailing list archive)
State Accepted
Commit fd2a052ccd69f3780b96291cacc21089e198d02c
Headers show
Series [v2] dt-bindings: usb: add common Type-C USB Switch schema | expand

Commit Message

Krzysztof Kozlowski Jan. 22, 2024, 9:44 a.m. UTC
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

Comments

Luca Weiss April 11, 2024, 7:13 a.m. UTC | #1
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/Documentation/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)
>  
> -  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 switching
> -    type: boolean
> +  mode-switch: true
> +  orientation-switch: true
>  
>    port:
>      $ref: /schemas/graph.yaml#/$defs/port-base
> @@ -79,6 +74,9 @@ required:
>    - reg
>    - port
>  
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
>  
>  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/Documentation/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 = <&typec_controller>;
+              data-lanes = <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/Documentation/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
>  
> -  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 switching
> -    type: boolean
> +  mode-switch: true
> +  orientation-switch: true
>  
>    port:
>      $ref: /schemas/graph.yaml#/properties/port
> @@ -54,6 +49,9 @@ required:
>    - orientation-switch
>    - port
>  
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
>  
>  examples:
> diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn36502.yaml b/Documentation/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
>  
> -  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 switching
> -    type: boolean
> +  orientation-switch: true
> +  retimer-switch: true
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -49,6 +44,9 @@ required:
>    - compatible
>    - reg
>  
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
>  
>  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)
>  
>    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 switching
> -    type: boolean
> +  orientation-switch: true
> +  retimer-switch: true
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -95,6 +89,9 @@ required:
>    - compatible
>    - reg
>  
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
>  
>  examples:
> diff --git a/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml 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
>  
> -  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 switching
> -    type: boolean
> +  mode-switch: true
> +  orientation-switch: true
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -63,6 +58,9 @@ required:
>    - reg
>    - ports
>  
> +allOf:
> +  - $ref: usb-switch.yaml#
> +
>  additionalProperties: false
>  
>  examples:
> diff --git a/Documentation/devicetree/bindings/usb/usb-switch.yaml b/Documentation/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 switching.
> +
> +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 purpose 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
Krzysztof Kozlowski April 11, 2024, 7:25 a.m. UTC | #2
On 11/04/2024 09:13, Luca Weiss wrote:
> 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/Documentation/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)
>>  
>> -  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 switching
>> -    type: boolean
>> +  mode-switch: true
>> +  orientation-switch: true
>>  
>>    port:
>>      $ref: /schemas/graph.yaml#/$defs/port-base
>> @@ -79,6 +74,9 @@ required:
>>    - reg
>>    - port
>>  
>> +allOf:
>> +  - $ref: usb-switch.yaml#
>> +
>>  additionalProperties: false
>>  
>>  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/Documentation/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 = <&typec_controller>;
> +              data-lanes = <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?

There is no such code in qcm6490-fairphone-fp5.dts. There was no such
code in the example of fsa4480 when I was testing my changes (and
examples should be complete), so this did not pop up.

You right, new schema does not allow extending the port. However the
true question is, why muxing happens on the port to the SoC controller?
The graph in commit msg fad89aa14 shows it happens on the side of the
connector.

Looks like fsa4480 mixes connector with the controller.

Best regards,
Krzysztof
Luca Weiss April 11, 2024, 7:35 a.m. UTC | #3
On Thu Apr 11, 2024 at 9:25 AM CEST, Krzysztof Kozlowski wrote:
> On 11/04/2024 09:13, Luca Weiss wrote:
> > 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/Documentation/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)
> >>  
> >> -  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 switching
> >> -    type: boolean
> >> +  mode-switch: true
> >> +  orientation-switch: true
> >>  
> >>    port:
> >>      $ref: /schemas/graph.yaml#/$defs/port-base
> >> @@ -79,6 +74,9 @@ required:
> >>    - reg
> >>    - port
> >>  
> >> +allOf:
> >> +  - $ref: usb-switch.yaml#
> >> +
> >>  additionalProperties: false
> >>  
> >>  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/Documentation/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 = <&typec_controller>;
> > +              data-lanes = <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?
>
> There is no such code in qcm6490-fairphone-fp5.dts. There was no such
> code in the example of fsa4480 when I was testing my changes (and
> examples should be complete), so this did not pop up.

Right, I'm sorry this is just out-of-tree for now, I've forgotten this.
There's some dependency chain with some unsupported DSC configuration in
DPU for now that blocks upstreaming this.

My tree with these patches is here if you want to take a look:
https://github.com/sc7280-mainline/linux/blob/sc7280-6.8.y/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts#L628

>
> You right, new schema does not allow extending the port. However the
> true question is, why muxing happens on the port to the SoC controller?
> The graph in commit msg fad89aa14 shows it happens on the side of the
> connector.
>
> Looks like fsa4480 mixes connector with the controller.

Could be honestly.. I trust you with knowing better how the ports are
supposed to work.

The property is for telling the fsa4480 driver that essentially the
hardware is wired up the reverse way. So with this info the driver can
handle the orientation switching correctly.

There's another layer to this as explained in the patches there that the
OCP96011 essentially works reversed compared to FSA4480, that's why it's
all a bit of a mess.

Regards
Luca

>
> Best regards,
> Krzysztof
Krzysztof Kozlowski April 13, 2024, 11:31 a.m. UTC | #4
On 11/04/2024 09:35, Luca Weiss wrote:
> On Thu Apr 11, 2024 at 9:25 AM CEST, Krzysztof Kozlowski wrote:
>> On 11/04/2024 09:13, Luca Weiss wrote:
>>> 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/Documentation/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)
>>>>  
>>>> -  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 switching
>>>> -    type: boolean
>>>> +  mode-switch: true
>>>> +  orientation-switch: true
>>>>  
>>>>    port:
>>>>      $ref: /schemas/graph.yaml#/$defs/port-base
>>>> @@ -79,6 +74,9 @@ required:
>>>>    - reg
>>>>    - port
>>>>  
>>>> +allOf:
>>>> +  - $ref: usb-switch.yaml#
>>>> +
>>>>  additionalProperties: false
>>>>  
>>>>  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/Documentation/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 = <&typec_controller>;
>>> +              data-lanes = <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?
>>
>> There is no such code in qcm6490-fairphone-fp5.dts. There was no such
>> code in the example of fsa4480 when I was testing my changes (and
>> examples should be complete), so this did not pop up.
> 
> Right, I'm sorry this is just out-of-tree for now, I've forgotten this.
> There's some dependency chain with some unsupported DSC configuration in
> DPU for now that blocks upstreaming this.
> 
> My tree with these patches is here if you want to take a look:
> https://github.com/sc7280-mainline/linux/blob/sc7280-6.8.y/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts#L628

I think fsa4480 schema is incomplete. I looked at HDK8450 board diagram
and fsa4480 sits between and connects:
1. Type-c connector
2. USB phy or controller
3. DisplayPort controller for altmode

I think nxp,ptn36502 is done correctly. You need three ports and
data-lanes would be on only one of them. usb-switch.yaml schema is ready
for this and assumes data-lanes will be on (2) above.

> 
>>
>> You right, new schema does not allow extending the port. However the
>> true question is, why muxing happens on the port to the SoC controller?
>> The graph in commit msg fad89aa14 shows it happens on the side of the
>> connector.
>>
>> Looks like fsa4480 mixes connector with the controller.
> 
> Could be honestly.. I trust you with knowing better how the ports are
> supposed to work.
> 
> The property is for telling the fsa4480 driver that essentially the
> hardware is wired up the reverse way. So with this info the driver can
> handle the orientation switching correctly.
> 
> There's another layer to this as explained in the patches there that the
> OCP96011 essentially works reversed compared to FSA4480, that's why it's
> all a bit of a mess.

Maybe Bjorn, Dmitry or Neil have some more ideas how this should look
like, but as of now to me it feels we should add "ports" property and
move there to port@1 the data-lanes part of fsa schema.

Driver then should check whether there is port or ports and use
ports->port@1 in the latter case.


Best regards,
Krzysztof
Dmitry Baryshkov April 14, 2024, 12:47 a.m. UTC | #5
On Sat, 13 Apr 2024 at 14:31, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 11/04/2024 09:35, Luca Weiss wrote:
> > On Thu Apr 11, 2024 at 9:25 AM CEST, Krzysztof Kozlowski wrote:
> >> On 11/04/2024 09:13, Luca Weiss wrote:
> >>> 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/Documentation/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)
> >>>>
> >>>> -  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 switching
> >>>> -    type: boolean
> >>>> +  mode-switch: true
> >>>> +  orientation-switch: true
> >>>>
> >>>>    port:
> >>>>      $ref: /schemas/graph.yaml#/$defs/port-base
> >>>> @@ -79,6 +74,9 @@ required:
> >>>>    - reg
> >>>>    - port
> >>>>
> >>>> +allOf:
> >>>> +  - $ref: usb-switch.yaml#
> >>>> +
> >>>>  additionalProperties: false
> >>>>
> >>>>  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/Documentation/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 = <&typec_controller>;
> >>> +              data-lanes = <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?
> >>
> >> There is no such code in qcm6490-fairphone-fp5.dts. There was no such
> >> code in the example of fsa4480 when I was testing my changes (and
> >> examples should be complete), so this did not pop up.
> >
> > Right, I'm sorry this is just out-of-tree for now, I've forgotten this.
> > There's some dependency chain with some unsupported DSC configuration in
> > DPU for now that blocks upstreaming this.
> >
> > My tree with these patches is here if you want to take a look:
> > https://github.com/sc7280-mainline/linux/blob/sc7280-6.8.y/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts#L628
>
> I think fsa4480 schema is incomplete. I looked at HDK8450 board diagram
> and fsa4480 sits between and connects:

I'd second this

> 1. Type-c connector
> 2. USB phy or controller

USB SS PHY or controller

> 3. DisplayPort controller for altmode

Also:
4. Audio HP / Mic pins

>
> I think nxp,ptn36502 is done correctly. You need three ports and
> data-lanes would be on only one of them. usb-switch.yaml schema is ready
> for this and assumes data-lanes will be on (2) above.
>
> >
> >>
> >> You right, new schema does not allow extending the port. However the
> >> true question is, why muxing happens on the port to the SoC controller?
> >> The graph in commit msg fad89aa14 shows it happens on the side of the
> >> connector.
> >>
> >> Looks like fsa4480 mixes connector with the controller.
> >
> > Could be honestly.. I trust you with knowing better how the ports are
> > supposed to work.
> >
> > The property is for telling the fsa4480 driver that essentially the
> > hardware is wired up the reverse way. So with this info the driver can
> > handle the orientation switching correctly.
> >
> > There's another layer to this as explained in the patches there that the
> > OCP96011 essentially works reversed compared to FSA4480, that's why it's
> > all a bit of a mess.
>
> Maybe Bjorn, Dmitry or Neil have some more ideas how this should look
> like, but as of now to me it feels we should add "ports" property and
> move there to port@1 the data-lanes part of fsa schema.

It might be done if this benefits HW description. However I don't
think it is really possible to define the usb-switch schema that
covers all mux and retimer cases.
Let's cover major usecases:
- simple SBU mux, is connected only to the SBU / DP AUX lines.
  - gpio-sbu-mux misses the port connected to DP AUX

- SBU switch. It is connected to SBU lines and to two other ports
(e.g. DP AUX and USB4/PCIe SBU)
  - gpio-sbu-mux in some laptops, not supported yet

- Audio accessory mode MUX. Connects USB-C DP/DM and SBU lines either
to the USB 2.0 controller + DP AUX (or any other usecase) or to the
audio codec
  - FSA4480,
  - WCD939X

- Debug accessory mode MUX. Connects USB-C DP/DM, SS and SBU lines
either to the USB 2.0, USB+DP SS and DP AUX or to the JTAG port
  - Qualcomm EUD?

- USB/DP MUX: Connects USB-C SS and SBU lines either to the USB 3.0 or
to the DP and DP AUX
  - pi3usb30532, no bindings

- USB-C SS retimer. Is connected to USB-C SS lines and and to the SS
lines on the other side
  - no known examples

- USB-C SS + SBU retirmer. Is connected to USB-C SS + SBU lines and
and to the SS + SBU lines on the other side.
  - nb7vpq904m Second SBU port is missing in bindings
  - ptn36502 Second SBU port is missing in bindings

There might be some combinations of these mux/switch/retimers.

Hope this summary helps.

>
> Driver then should check whether there is port or ports and use
> ports->port@1 in the latter case.
>
>
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml b/Documentation/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)
 
-  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 switching
-    type: boolean
+  mode-switch: true
+  orientation-switch: true
 
   port:
     $ref: /schemas/graph.yaml#/$defs/port-base
@@ -79,6 +74,9 @@  required:
   - reg
   - port
 
+allOf:
+  - $ref: usb-switch.yaml#
+
 additionalProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Documentation/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
 
-  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 switching
-    type: boolean
+  mode-switch: true
+  orientation-switch: true
 
   port:
     $ref: /schemas/graph.yaml#/properties/port
@@ -54,6 +49,9 @@  required:
   - orientation-switch
   - port
 
+allOf:
+  - $ref: usb-switch.yaml#
+
 additionalProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn36502.yaml b/Documentation/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
 
-  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 switching
-    type: boolean
+  orientation-switch: true
+  retimer-switch: true
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -49,6 +44,9 @@  required:
   - compatible
   - reg
 
+allOf:
+  - $ref: usb-switch.yaml#
+
 additionalProperties: false
 
 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)
 
   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 switching
-    type: boolean
+  orientation-switch: true
+  retimer-switch: true
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -95,6 +89,9 @@  required:
   - compatible
   - reg
 
+allOf:
+  - $ref: usb-switch.yaml#
+
 additionalProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml 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
 
-  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 switching
-    type: boolean
+  mode-switch: true
+  orientation-switch: true
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -63,6 +58,9 @@  required:
   - reg
   - ports
 
+allOf:
+  - $ref: usb-switch.yaml#
+
 additionalProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/usb/usb-switch.yaml b/Documentation/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 switching.
+
+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 purpose 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