linux-kernel - Re: [PATCH v2] dt-bindings: usb: add common Type-C USB Switch schema

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 11 Apr 2024 09:25:25 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Luca Weiss <luca.weiss@...rphone.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konrad.dybcio@...aro.org>,
 Neil Armstrong <neil.armstrong@...aro.org>, linux-usb@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v2] dt-bindings: usb: add common Type-C USB Switch schema

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@...aro.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