| 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
| ||
|
Message-ID: <CACeCKadSCXZo3E4JZiwxFn_4CH3KDfQkk=xRrxSqCEWAgYhV6Q@mail.gmail.com>
Date: Thu, 16 Jun 2022 01:54:36 -0700
From: Prashant Malani <pmalani@...omium.org>
To: Stephen Boyd <swboyd@...omium.org>
Cc: linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
bleung@...omium.org, heikki.krogerus@...ux.intel.com,
Nícolas F . R . A . Prado
<nfraprado@...labora.com>, Andrzej Hajda <andrzej.hajda@...el.com>,
Daniel Vetter <daniel@...ll.ch>,
David Airlie <airlied@...ux.ie>, devicetree@...r.kernel.org,
dri-devel@...ts.freedesktop.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Hsin-Yi Wang <hsinyi@...omium.org>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Jonas Karlman <jonas@...boo.se>,
José Expósito <jose.exposito89@...il.com>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Maxime Ripard <maxime@...no.tech>,
Neil Armstrong <narmstrong@...libre.com>,
Pin-Yen Lin <treapking@...omium.org>,
Robert Foss <robert.foss@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Sam Ravnborg <sam@...nborg.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Xin Ji <xji@...logixsemi.com>
Subject: Re: [PATCH v4 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support
On Thu, Jun 16, 2022 at 12:42 AM Stephen Boyd <swboyd@...omium.org> wrote:
>
> 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.
Sure, but I don't really think that should be included in this patch
(or series).
I'd be happy to submit a separate patch once this series is resolved.
>
> >
> > 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?
No.
>
> > + #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?
No. Patch 6/7 has the implementation details on how the anx driver
performs the enumeration of switches.
>
> > + 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?
Sorry, but I got lost midway through the preceding explanation. The binding
can always add additional ports to each "switch" to accomplish the
graph connections
you are alluding to (if the driver needs/uses it, which I don't think
this one does at present).
Adding extra ports to existing ports gets tricky from a mode-switch
enumeration perspective (which
ports should have the modes switches, which shouldn't? Do you follow
the remote end points for each port
and see which one is a Type C connector? What if we add an
intermediate switch device in the future?)
Having a dedicated "switch" binding makes this consistent and easy
(port0 will always have the end-point for the switch).
While there may be more than 1 valid approach here, I believe the
current one is appropriate.
>
> > + };
> > + 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
Powered by blists - more mailing lists