All of lore.kernel.org
 help / color / mirror / Atom feed
From: Prashant Malani <pmalani@chromium.org>
To: Stephen Boyd <swboyd@chromium.org>
Cc: "Heikki Krogerus" <heikki.krogerus@linux.intel.com>,
	"Andrzej Hajda" <andrzej.hajda@intel.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"David Airlie" <airlied@linux.ie>,
	dri-devel@lists.freedesktop.org,
	"Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	"Jernej Skrabec" <jernej.skrabec@gmail.com>,
	devicetree@vger.kernel.org,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Nícolas F . R . A . Prado" <nfraprado@collabora.com>,
	"Jonas Karlman" <jonas@kwiboo.se>,
	"Allen Chen" <allen.chen@ite.com.tw>,
	"Pin-yen Lin" <treapking@chromium.org>,
	"Maxime Ripard" <maxime@cerno.tech>,
	"Hsin-Yi Wang" <hsinyi@chromium.org>,
	"Xin Ji" <xji@analogixsemi.com>,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Linux USB List" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Robert Foss" <robert.foss@linaro.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
	"José Expósito" <jose.exposito89@gmail.com>
Subject: Re: [PATCH v5 1/9] dt-bindings: usb: Add Type-C switch binding
Date: Wed, 29 Jun 2022 15:55:10 -0700	[thread overview]
Message-ID: <CACeCKacJnnk4_dXEX7XiboOWrYpfAcE=ukP63agVAYUxWR9Vbg@mail.gmail.com> (raw)
In-Reply-To: <CAE-0n53ers881LOTCEmKDDxJQt+5vvXJSURs=o6TcOiR5m_EAw@mail.gmail.com>

On Wed, Jun 29, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> > What device controls the switching in this case? Again, block diagrams
> > please if you want advice on what the binding should look like.
>
> My understanding is there are 4 DP lanes on it6505 and two lanes are
> connected to one usb-c-connector and the other two lanes are connected
> to a different usb-c-connector. The IT6505 driver will send DP out on
> the associated two DP lanes depending on which usb-c-connector has DP
> pins assigned by the typec manager.
>
>                                                      +-------+
>                                                      |       |
>           +--------+                            /----+ usb-c |
>           | IT6505 |                           / /---+       |
>           |        +------------ lane 0 ------/ /    |       |
>           |        +------------ lane 1 -------/     +-------+
>  DPI -----+        |
>           |        |                                 +-------+
>           |        |                                 |       |
>           |        +------------ lane 2 -------------+ usb-c |
>           |        +------------ lane 3 -------------+       |
>           |        |                                 |       |
>           +--------+                                 +-------+
>
> The bridge is a mux that steers DP to one or the other usb-c-connector
> based on what the typec manager decides.
>
> I would expect this to be described with the existing port binding in
> the it6505 node. The binding would need to be extended to describe the
> output side.
>
>         bridge@5c {
>             compatible = "ite,it6505";

We'll need a top level "mode-switch" property here.
>             ...
>
>             ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
>                 port@0 {
>                     reg = <0>;
>                     it6505_in: endpoint {
>                         remote-endpoint = <&dpi_out>;
>                     };
>                 };
>
>                 port@1 {
>                     #address-cells = <1>;
>                     #size-cells = <0>;
>                     reg = <1>;
>
>                     it6505_out_lanes_01: endpoint@0 {
>                         reg = <0>
>                         data-lanes = <0 1>;
>                         remote-endpoint = <&typec0>;
>                     };
>
>                     it6505_out_lanes_23: endpoint@1 {
>                         reg = <1>
>                         data-lanes = <2 3>;
>                         remote-endpoint = <&typec1>;
>                     };
>                 };
>             };
>         };
>
>         usb-c-connector {
>             compatible = "usb-c-connector";
>             ....
>             ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
>                 port@1 {
>                     reg = <1>;
>                     typec0: endpoint {
>                         remote-endpoint = <&it6505_out_lanes_01>;
>                     };
>                 };
>             };
>         };

We can adopt this binding, but from what I gathered in this thread, that
shouldn't be done, because IT6505 isn't meant to be aware of Type-C
connections at all.

>
> I don't see the benefit to making a genericish binding for typec
> switches, even if the hardware has typec awareness like anx7625. It
> looks like the graph binding can already handle what we need. By putting
> it in the top-level ports node we have one way to describe the
> input/output of the device instead of describing it in the top-level in
> the display connector case and the child typec switch node in the usb c
> connector case.

Ack, I'll drop the generic binding for future revisions.

>
> I think the difficulty comes from the combinatorial explosion of
> possible configurations. As evidenced here, hardware engineers can take
> a DP bridge and use it as a DP mux as long as the bridge has lane
> control. Or they can take a device like anx7625 and ignore the USB
> aspect and use the internal crosspoint switch as a DP mux. The anx7625
> part could be a MIPI-to-DP display bridge plus mux that is connected to
> two dp-connectors, in which case typec isn't even involved, but we could
> mux between two dp connectors.

Each containing a single DP lane, right?
I think that will not be a valid configuration, since there is only 1 HPD
pin (so it's assuming both DP lanes go to the same DP sink).

But yes, your larger point is valid: h/w engineers can repurpose these
bridges in ways the datasheet doesn't originally anticipate.

>
> Also, the typec framework would like to simply walk the graph from the
> usb-c-connector looking for nodes that have 'mode-switch' or
> 'orientation-switch' properties and treat those devices as the typec
> switches for the connector. This means that we have to add these typec
> properties like 'mode-switch' to something like the IT6505 bridge
> binding, which is a little awkward. I wonder if those properties aren't
> really required. Would it be sufficient if the framework could walk the
> graph and look for registered typec switches in the kernel that have a
> matching of_node?

My interpretation of the current mode-switch search code [1] is that
a top level property of "mode-switch" is required.

[1] https://elixir.bootlin.com/linux/v5.19-rc4/source/drivers/usb/typec/mux.c#L347

WARNING: multiple messages have this Message-ID (diff)
From: Prashant Malani <pmalani@chromium.org>
To: Stephen Boyd <swboyd@chromium.org>
Cc: "Pin-yen Lin" <treapking@chromium.org>,
	"Rob Herring" <robh@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Linux USB List" <linux-usb@vger.kernel.org>,
	"Benson Leung" <bleung@chromium.org>,
	"Heikki Krogerus" <heikki.krogerus@linux.intel.com>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	"Nícolas F . R . A . Prado" <nfraprado@collabora.com>,
	"Allen Chen" <allen.chen@ite.com.tw>,
	"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>,
	"Robert Foss" <robert.foss@linaro.org>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Xin Ji" <xji@analogixsemi.com>
Subject: Re: [PATCH v5 1/9] dt-bindings: usb: Add Type-C switch binding
Date: Wed, 29 Jun 2022 15:55:10 -0700	[thread overview]
Message-ID: <CACeCKacJnnk4_dXEX7XiboOWrYpfAcE=ukP63agVAYUxWR9Vbg@mail.gmail.com> (raw)
In-Reply-To: <CAE-0n53ers881LOTCEmKDDxJQt+5vvXJSURs=o6TcOiR5m_EAw@mail.gmail.com>

On Wed, Jun 29, 2022 at 2:58 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> > What device controls the switching in this case? Again, block diagrams
> > please if you want advice on what the binding should look like.
>
> My understanding is there are 4 DP lanes on it6505 and two lanes are
> connected to one usb-c-connector and the other two lanes are connected
> to a different usb-c-connector. The IT6505 driver will send DP out on
> the associated two DP lanes depending on which usb-c-connector has DP
> pins assigned by the typec manager.
>
>                                                      +-------+
>                                                      |       |
>           +--------+                            /----+ usb-c |
>           | IT6505 |                           / /---+       |
>           |        +------------ lane 0 ------/ /    |       |
>           |        +------------ lane 1 -------/     +-------+
>  DPI -----+        |
>           |        |                                 +-------+
>           |        |                                 |       |
>           |        +------------ lane 2 -------------+ usb-c |
>           |        +------------ lane 3 -------------+       |
>           |        |                                 |       |
>           +--------+                                 +-------+
>
> The bridge is a mux that steers DP to one or the other usb-c-connector
> based on what the typec manager decides.
>
> I would expect this to be described with the existing port binding in
> the it6505 node. The binding would need to be extended to describe the
> output side.
>
>         bridge@5c {
>             compatible = "ite,it6505";

We'll need a top level "mode-switch" property here.
>             ...
>
>             ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
>                 port@0 {
>                     reg = <0>;
>                     it6505_in: endpoint {
>                         remote-endpoint = <&dpi_out>;
>                     };
>                 };
>
>                 port@1 {
>                     #address-cells = <1>;
>                     #size-cells = <0>;
>                     reg = <1>;
>
>                     it6505_out_lanes_01: endpoint@0 {
>                         reg = <0>
>                         data-lanes = <0 1>;
>                         remote-endpoint = <&typec0>;
>                     };
>
>                     it6505_out_lanes_23: endpoint@1 {
>                         reg = <1>
>                         data-lanes = <2 3>;
>                         remote-endpoint = <&typec1>;
>                     };
>                 };
>             };
>         };
>
>         usb-c-connector {
>             compatible = "usb-c-connector";
>             ....
>             ports {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
>                 port@1 {
>                     reg = <1>;
>                     typec0: endpoint {
>                         remote-endpoint = <&it6505_out_lanes_01>;
>                     };
>                 };
>             };
>         };

We can adopt this binding, but from what I gathered in this thread, that
shouldn't be done, because IT6505 isn't meant to be aware of Type-C
connections at all.

>
> I don't see the benefit to making a genericish binding for typec
> switches, even if the hardware has typec awareness like anx7625. It
> looks like the graph binding can already handle what we need. By putting
> it in the top-level ports node we have one way to describe the
> input/output of the device instead of describing it in the top-level in
> the display connector case and the child typec switch node in the usb c
> connector case.

Ack, I'll drop the generic binding for future revisions.

>
> I think the difficulty comes from the combinatorial explosion of
> possible configurations. As evidenced here, hardware engineers can take
> a DP bridge and use it as a DP mux as long as the bridge has lane
> control. Or they can take a device like anx7625 and ignore the USB
> aspect and use the internal crosspoint switch as a DP mux. The anx7625
> part could be a MIPI-to-DP display bridge plus mux that is connected to
> two dp-connectors, in which case typec isn't even involved, but we could
> mux between two dp connectors.

Each containing a single DP lane, right?
I think that will not be a valid configuration, since there is only 1 HPD
pin (so it's assuming both DP lanes go to the same DP sink).

But yes, your larger point is valid: h/w engineers can repurpose these
bridges in ways the datasheet doesn't originally anticipate.

>
> Also, the typec framework would like to simply walk the graph from the
> usb-c-connector looking for nodes that have 'mode-switch' or
> 'orientation-switch' properties and treat those devices as the typec
> switches for the connector. This means that we have to add these typec
> properties like 'mode-switch' to something like the IT6505 bridge
> binding, which is a little awkward. I wonder if those properties aren't
> really required. Would it be sufficient if the framework could walk the
> graph and look for registered typec switches in the kernel that have a
> matching of_node?

My interpretation of the current mode-switch search code [1] is that
a top level property of "mode-switch" is required.

[1] https://elixir.bootlin.com/linux/v5.19-rc4/source/drivers/usb/typec/mux.c#L347

  reply	other threads:[~2022-06-29 22:55 UTC|newest]

Thread overview: 100+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-22 17:34 [PATCH v5 0/9] usb: typec: Introduce typec-switch binding Prashant Malani
2022-06-22 17:34 ` Prashant Malani
2022-06-22 17:34 ` [PATCH v5 1/9] dt-bindings: usb: Add Type-C switch binding Prashant Malani
2022-06-22 17:34   ` Prashant Malani
2022-06-23 18:30   ` Stephen Boyd
2022-06-23 18:30     ` Stephen Boyd
2022-06-23 19:08     ` Prashant Malani
2022-06-23 19:08       ` Prashant Malani
2022-06-23 23:14       ` Stephen Boyd
2022-06-23 23:14         ` Stephen Boyd
2022-06-24  0:35         ` Prashant Malani
2022-06-24  0:35           ` Prashant Malani
2022-06-24  1:24           ` Prashant Malani
2022-06-24  1:24             ` Prashant Malani
2022-06-24  2:13           ` Stephen Boyd
2022-06-24  2:13             ` Stephen Boyd
2022-06-24  2:48             ` Prashant Malani
2022-06-24  2:48               ` Prashant Malani
2022-06-24 19:50               ` Stephen Boyd
2022-06-24 19:50                 ` Stephen Boyd
2022-06-24 21:41                 ` Prashant Malani
2022-06-24 21:41                   ` Prashant Malani
2022-06-25  1:21                   ` Stephen Boyd
2022-06-25  1:21                     ` Stephen Boyd
2022-06-25 20:13                   ` Krzysztof Kozlowski
2022-06-25 20:13                     ` Krzysztof Kozlowski
2022-06-27 21:04   ` Rob Herring
2022-06-27 21:04     ` Rob Herring
2022-06-27 21:43     ` Prashant Malani
2022-06-27 21:43       ` Prashant Malani
2022-06-28 18:23       ` Rob Herring
2022-06-28 18:23         ` Rob Herring
2022-06-29 14:33         ` Pin-yen Lin
2022-06-29 14:33           ` Pin-yen Lin
2022-06-29 15:00           ` Pin-yen Lin
2022-06-29 15:00             ` Pin-yen Lin
2022-06-29 17:58             ` Rob Herring
2022-06-29 17:58               ` Rob Herring
2022-06-29 21:58               ` Stephen Boyd
2022-06-29 21:58                 ` Stephen Boyd
2022-06-29 22:55                 ` Prashant Malani [this message]
2022-06-29 22:55                   ` Prashant Malani
2022-06-29 23:55                   ` Stephen Boyd
2022-06-29 23:55                     ` Stephen Boyd
2022-06-30 17:10                     ` Prashant Malani
2022-06-30 17:10                       ` Prashant Malani
2022-07-12 17:45                       ` Rob Herring
2022-07-12 17:45                         ` Rob Herring
2022-07-13 21:58                         ` Prashant Malani
2022-07-13 21:58                           ` Prashant Malani
2022-09-02  7:41                         ` Prashant Malani
2022-09-02  7:41                           ` Prashant Malani
2022-09-16 18:21                           ` Prashant Malani
2022-09-16 18:21                             ` Prashant Malani
2022-10-03  3:42                             ` Pin-yen Lin
2022-10-03  3:42                               ` Pin-yen Lin
2022-06-22 17:34 ` [PATCH v5 2/9] dt-bindings: drm/bridge: anx7625: Add mode-switch support Prashant Malani
2022-06-22 17:34   ` Prashant Malani
2022-06-22 17:34 ` [PATCH v5 3/9] drm/bridge: anx7625: Register number of Type C switches Prashant Malani
2022-06-22 17:34   ` Prashant Malani
2022-06-22 17:34 ` [PATCH v5 4/9] drm/bridge: anx7625: Register Type-C mode switches Prashant Malani
2022-06-22 17:34   ` Prashant Malani
2022-06-22 17:34 ` [PATCH v5 5/9] drm/bridge: anx7625: Add typec_mux_set callback function Prashant Malani
2022-06-22 17:34   ` Prashant Malani
2022-06-28 19:25   ` Stephen Boyd
2022-06-28 19:25     ` Stephen Boyd
2022-06-28 19:48     ` Prashant Malani
2022-06-28 19:48       ` Prashant Malani
2022-06-28 20:40       ` Stephen Boyd
2022-06-28 20:40         ` Stephen Boyd
2022-06-28 20:56         ` Prashant Malani
2022-06-28 20:56           ` Prashant Malani
2022-06-30 23:21           ` Stephen Boyd
2022-06-30 23:21             ` Stephen Boyd
2022-06-30 23:38             ` Prashant Malani
2022-06-30 23:38               ` Prashant Malani
2022-07-06 18:26               ` Prashant Malani
2022-07-06 18:26                 ` Prashant Malani
2022-07-07  0:17                 ` Stephen Boyd
2022-07-07  0:17                   ` Stephen Boyd
2022-07-12 10:22                   ` Pin-yen Lin
2022-07-12 10:22                     ` Pin-yen Lin
2022-06-22 17:34 ` [PATCH v5 6/9] dt/bindings: drm/bridge: it6505: Add mode-switch support Prashant Malani
2022-06-22 17:34   ` Prashant Malani
2022-06-23 18:24   ` Stephen Boyd
2022-06-23 18:24     ` Stephen Boyd
2022-06-23 18:37     ` Prashant Malani
2022-06-23 18:37       ` Prashant Malani
2022-06-23 19:08       ` Stephen Boyd
2022-06-23 19:08         ` Stephen Boyd
2022-06-23 19:15         ` Prashant Malani
2022-06-23 19:15           ` Prashant Malani
2022-06-22 17:34 ` [PATCH v5 7/9] drm/bridge: it6505: Register number of Type C switches Prashant Malani
2022-06-22 17:34   ` Prashant Malani
2022-06-27 21:05   ` Rob Herring
2022-06-27 21:05     ` Rob Herring
2022-06-22 17:34 ` [PATCH v5 8/9] drm/bridge: it6505: Register Type-C mode switches Prashant Malani
2022-06-22 17:34   ` Prashant Malani
2022-06-22 17:34 ` [PATCH v5 9/9] drm/bridge: it6505: Add typec_mux_set callback function Prashant Malani
2022-06-22 17:34   ` Prashant Malani

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='CACeCKacJnnk4_dXEX7XiboOWrYpfAcE=ukP63agVAYUxWR9Vbg@mail.gmail.com' \
    --to=pmalani@chromium.org \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@linux.ie \
    --cc=allen.chen@ite.com.tw \
    --cc=andrzej.hajda@intel.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --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=krzysztof.kozlowski@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=robert.foss@linaro.org \
    --cc=sam@ravnborg.org \
    --cc=swboyd@chromium.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: link
Be 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.