From: Stephen Boyd <swboyd@chromium.org> To: Prashant Malani <pmalani@chromium.org>, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Cc: bleung@chromium.org, heikki.krogerus@linux.intel.com, "Nícolas F . R . A . Prado" <nfraprado@collabora.com>, "Andrzej Hajda" <andrzej.hajda@intel.com>, "Daniel Vetter" <daniel@ffwll.ch>, "David Airlie" <airlied@linux.ie>, devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Hsin-Yi Wang" <hsinyi@chromium.org>, "Jernej Skrabec" <jernej.skrabec@gmail.com>, "Jonas Karlman" <jonas@kwiboo.se>, "José Expósito" <jose.exposito89@gmail.com>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>, "Maxime Ripard" <maxime@cerno.tech>, "Neil Armstrong" <narmstrong@baylibre.com>, "Pin-Yen Lin" <treapking@chromium.org>, "Robert Foss" <robert.foss@linaro.org>, "Rob Herring" <robh+dt@kernel.org>, "Sam Ravnborg" <sam@ravnborg.org>, "Thomas Zimmermann" <tzimmermann@suse.de>, "Xin Ji" <xji@analogixsemi.com> Subject: Re: [PATCH v4 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support Date: Thu, 16 Jun 2022 00:42:01 -0700 [thread overview] Message-ID: <CAE-0n53ub30HXB325wPoMB4C3n4j_9FWnNu5AmtYgU3PBvs8mQ@mail.gmail.com> (raw) In-Reply-To: <20220615172129.1314056-5-pmalani@chromium.org> Quoting Prashant Malani (2022-06-15 10:20:20) > > .../display/bridge/analogix,anx7625.yaml | 64 +++++++++++++++++++ > 1 file changed, 64 insertions(+) Can this file get a link to the product brief[1]? It helps to quickly find the block diagram. > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > index 35a48515836e..bc6f7644db31 100644 > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > @@ -105,6 +105,34 @@ properties: > - port@0 > - port@1 > > + switches: > + type: object > + description: Set of switches controlling DisplayPort traffic on > + outgoing RX/TX lanes to Type C ports. > + additionalProperties: false > + > + properties: > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + patternProperties: > + '^switch@[01]$': > + $ref: /schemas/usb/typec-switch.yaml# > + unevaluatedProperties: false > + > + properties: > + reg: > + maxItems: 1 > + > + required: > + - reg > + > + required: > + - switch@0 > + > required: > - compatible > - reg > @@ -167,5 +195,41 @@ examples: > }; > }; > }; > + switches { Is "switches" a bus? > + #address-cells = <1>; > + #size-cells = <0>; > + switch@0 { > + compatible = "typec-switch"; Is this compatible matched against a driver that's populated on this "switches" bus? > + reg = <0>; > + mode-switch; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + anx_typec0: endpoint { > + remote-endpoint = <&typec_port0>; > + }; > + }; > + }; I was expecting to see these simply be more ports in the existing graph binding of this device, and then have the 'mode-switch' or 'orientation-switch' properties be at the same level as the compatible string "analogix,anx7625". Here's the reasoning, based on looking at the product brief and the existing binding/implementation. Looking at the only existing implementation of this binding upstream in mt8183-kukui-jacuzzi.dtsi it looks like one of these typec ports is actually the same physically as the 'anx7625_out' endpoint (reg address of 1) that is already defined in the binding. It seems that MIPI DSI/DPI comes in and is output through 2 lanes, SSRX2 and SSTX2 according to the product brief[1], and that is connected to some eDP panel ("auo,b116xw03"). Presumably that is the same as anx_typec1 in this patch? I suspect the USB3.1 input is not connected on this board, and thus the crosspoint switch is never used, nor the SSRX1/SSTX1 pins. The existing binding defines the MIPI DSI/DPI input as port0 and two of the four lanes of output that is probably by default connected to the "DisplayPort Transmitter" as port1 because that's how the crosspoint switch comes out of reset. That leaves the USB3.1 input possibly needing a port in the ports binding, and the other two lanes of output needing a port in the ports binding to describe their connection to the downstream device. And finally information about if the crosspoint switch needs to be registered with the typec framework to do typec things, which can be achieved by the presence of the 'mode-switch' property. On a board like kukui-jacuzzi these new properties and ports wouldn't be specified, because what is there is already sufficient. If this chip is connected to a usb-c-connector then I'd expect to see a connection from the output ports in the graph binding to the connector node's ports. There aren't any ports in the usb-c-connector binding though from what I see. I believe there's also one more use case here where USB3.1 or MIPI DSI/DPI is connected on the input side and this device is used to steer USB3.1 or DP through the crosspoint switch to either of the two output pairs. This last scenario means that we have to describe both output pairs, SSRX1/SSTX1 and SSRX2/SSTX2, as different ports in the binding so they can be connected to different usb-c-connectors if the hardware engineer wired the output pins that way. TL;DR: Can we add 'mode-switch' as an optional property and two more ports at address 2 and 3 for the USB3.1 input and the SSRX1/SSTX1 pair respectively to the existing graph part of this binding? > + }; > + 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>; > + }; > + }; > + }; > + }; > + }; > }; [1] https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <swboyd@chromium.org> To: Prashant Malani <pmalani@chromium.org>, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Cc: heikki.krogerus@linux.intel.com, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Neil Armstrong" <narmstrong@baylibre.com>, "David Airlie" <airlied@linux.ie>, dri-devel@lists.freedesktop.org, "Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>, "Andrzej Hajda" <andrzej.hajda@intel.com>, "Sam Ravnborg" <sam@ravnborg.org>, "Jernej Skrabec" <jernej.skrabec@gmail.com>, devicetree@vger.kernel.org, "Nícolas F . R . A . Prado" <nfraprado@collabora.com>, "Jonas Karlman" <jonas@kwiboo.se>, "Pin-Yen Lin" <treapking@chromium.org>, "Rob Herring" <robh+dt@kernel.org>, "Maxime Ripard" <maxime@cerno.tech>, "Hsin-Yi Wang" <hsinyi@chromium.org>, "Xin Ji" <xji@analogixsemi.com>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Robert Foss" <robert.foss@linaro.org>, "Thomas Zimmermann" <tzimmermann@suse.de>, "José Expósito" <jose.exposito89@gmail.com> Subject: Re: [PATCH v4 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support Date: Thu, 16 Jun 2022 00:42:01 -0700 [thread overview] Message-ID: <CAE-0n53ub30HXB325wPoMB4C3n4j_9FWnNu5AmtYgU3PBvs8mQ@mail.gmail.com> (raw) In-Reply-To: <20220615172129.1314056-5-pmalani@chromium.org> Quoting Prashant Malani (2022-06-15 10:20:20) > > .../display/bridge/analogix,anx7625.yaml | 64 +++++++++++++++++++ > 1 file changed, 64 insertions(+) Can this file get a link to the product brief[1]? It helps to quickly find the block diagram. > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > index 35a48515836e..bc6f7644db31 100644 > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml > @@ -105,6 +105,34 @@ properties: > - port@0 > - port@1 > > + switches: > + type: object > + description: Set of switches controlling DisplayPort traffic on > + outgoing RX/TX lanes to Type C ports. > + additionalProperties: false > + > + properties: > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + patternProperties: > + '^switch@[01]$': > + $ref: /schemas/usb/typec-switch.yaml# > + unevaluatedProperties: false > + > + properties: > + reg: > + maxItems: 1 > + > + required: > + - reg > + > + required: > + - switch@0 > + > required: > - compatible > - reg > @@ -167,5 +195,41 @@ examples: > }; > }; > }; > + switches { Is "switches" a bus? > + #address-cells = <1>; > + #size-cells = <0>; > + switch@0 { > + compatible = "typec-switch"; Is this compatible matched against a driver that's populated on this "switches" bus? > + reg = <0>; > + mode-switch; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + anx_typec0: endpoint { > + remote-endpoint = <&typec_port0>; > + }; > + }; > + }; I was expecting to see these simply be more ports in the existing graph binding of this device, and then have the 'mode-switch' or 'orientation-switch' properties be at the same level as the compatible string "analogix,anx7625". Here's the reasoning, based on looking at the product brief and the existing binding/implementation. Looking at the only existing implementation of this binding upstream in mt8183-kukui-jacuzzi.dtsi it looks like one of these typec ports is actually the same physically as the 'anx7625_out' endpoint (reg address of 1) that is already defined in the binding. It seems that MIPI DSI/DPI comes in and is output through 2 lanes, SSRX2 and SSTX2 according to the product brief[1], and that is connected to some eDP panel ("auo,b116xw03"). Presumably that is the same as anx_typec1 in this patch? I suspect the USB3.1 input is not connected on this board, and thus the crosspoint switch is never used, nor the SSRX1/SSTX1 pins. The existing binding defines the MIPI DSI/DPI input as port0 and two of the four lanes of output that is probably by default connected to the "DisplayPort Transmitter" as port1 because that's how the crosspoint switch comes out of reset. That leaves the USB3.1 input possibly needing a port in the ports binding, and the other two lanes of output needing a port in the ports binding to describe their connection to the downstream device. And finally information about if the crosspoint switch needs to be registered with the typec framework to do typec things, which can be achieved by the presence of the 'mode-switch' property. On a board like kukui-jacuzzi these new properties and ports wouldn't be specified, because what is there is already sufficient. If this chip is connected to a usb-c-connector then I'd expect to see a connection from the output ports in the graph binding to the connector node's ports. There aren't any ports in the usb-c-connector binding though from what I see. I believe there's also one more use case here where USB3.1 or MIPI DSI/DPI is connected on the input side and this device is used to steer USB3.1 or DP through the crosspoint switch to either of the two output pairs. This last scenario means that we have to describe both output pairs, SSRX1/SSTX1 and SSRX2/SSTX2, as different ports in the binding so they can be connected to different usb-c-connectors if the hardware engineer wired the output pins that way. TL;DR: Can we add 'mode-switch' as an optional property and two more ports at address 2 and 3 for the USB3.1 input and the SSRX1/SSTX1 pair respectively to the existing graph part of this binding? > + }; > + 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>; > + }; > + }; > + }; > + }; > + }; > }; [1] https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
next prev parent reply other threads:[~2022-06-16 7:42 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-06-15 17:20 [PATCH v4 0/7] usb: typec: Introduce typec-switch binding Prashant Malani 2022-06-15 17:20 ` Prashant Malani 2022-06-15 17:20 ` [PATCH v4 1/7] usb: typec: mux: Allow muxes to specify mode-switch Prashant Malani 2022-06-15 17:20 ` Prashant Malani 2022-06-15 17:20 ` [PATCH v4 2/7] usb: typec: mux: Add CONFIG guards for functions Prashant Malani 2022-06-15 17:20 ` Prashant Malani 2022-06-16 6:22 ` Heikki Krogerus 2022-06-16 6:22 ` Heikki Krogerus 2022-06-15 17:20 ` [PATCH v4 3/7] dt-bindings: usb: Add Type-C switch binding Prashant Malani 2022-06-15 17:20 ` Prashant Malani 2022-06-22 14:52 ` Krzysztof Kozlowski 2022-06-22 14:52 ` Krzysztof Kozlowski 2022-06-15 17:20 ` [PATCH v4 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support Prashant Malani 2022-06-15 17:20 ` Prashant Malani 2022-06-15 17:51 ` Krzysztof Kozlowski 2022-06-15 17:51 ` Krzysztof Kozlowski 2022-06-16 7:42 ` Stephen Boyd [this message] 2022-06-16 7:42 ` Stephen Boyd 2022-06-16 8:54 ` Prashant Malani 2022-06-16 8:54 ` Prashant Malani 2022-06-16 19:34 ` Rob Herring 2022-06-16 19:34 ` Rob Herring 2022-06-16 19:57 ` Prashant Malani 2022-06-16 19:57 ` Prashant Malani 2022-06-21 21:12 ` Prashant Malani 2022-06-21 21:12 ` Prashant Malani 2022-06-15 17:20 ` [PATCH v4 5/7] drm/bridge: anx7625: Register number of Type C switches Prashant Malani 2022-06-15 17:20 ` Prashant Malani 2022-06-15 17:20 ` [PATCH v4 6/7] drm/bridge: anx7625: Register Type-C mode switches Prashant Malani 2022-06-15 17:20 ` Prashant Malani 2022-06-15 17:20 ` [PATCH v4 7/7] drm/bridge: anx7625: Add typec_mux_set callback function Prashant Malani 2022-06-15 17:20 ` Prashant Malani 2022-06-15 18:13 ` [PATCH v4 0/7] usb: typec: Introduce typec-switch binding Prashant Malani 2022-06-15 18:13 ` Prashant Malani 2022-06-21 13:17 ` Greg Kroah-Hartman 2022-06-21 13:17 ` Greg Kroah-Hartman 2022-06-22 14:53 ` Krzysztof Kozlowski 2022-06-22 14:53 ` Krzysztof Kozlowski 2022-06-22 15:11 ` Greg Kroah-Hartman 2022-06-22 15:11 ` Greg Kroah-Hartman 2022-06-22 17:52 ` Prashant Malani 2022-06-22 17:52 ` Prashant Malani 2022-06-16 8:55 ` AngeloGioacchino Del Regno 2022-06-16 8:55 ` AngeloGioacchino Del Regno
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAE-0n53ub30HXB325wPoMB4C3n4j_9FWnNu5AmtYgU3PBvs8mQ@mail.gmail.com \ --to=swboyd@chromium.org \ --cc=Laurent.pinchart@ideasonboard.com \ --cc=airlied@linux.ie \ --cc=andrzej.hajda@intel.com \ --cc=bleung@chromium.org \ --cc=daniel@ffwll.ch \ --cc=devicetree@vger.kernel.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=gregkh@linuxfoundation.org \ --cc=heikki.krogerus@linux.intel.com \ --cc=hsinyi@chromium.org \ --cc=jernej.skrabec@gmail.com \ --cc=jonas@kwiboo.se \ --cc=jose.exposito89@gmail.com \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=maxime@cerno.tech \ --cc=narmstrong@baylibre.com \ --cc=nfraprado@collabora.com \ --cc=pmalani@chromium.org \ --cc=robert.foss@linaro.org \ --cc=robh+dt@kernel.org \ --cc=sam@ravnborg.org \ --cc=treapking@chromium.org \ --cc=tzimmermann@suse.de \ --cc=xji@analogixsemi.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.