On Sat, 13 Apr 2024 at 14:31, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 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@xxxxxxxxxx> > >>>> > >>>> --- > >>>> > >>>> 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 > -- With best wishes Dmitry