On Thu, Jul 7, 2022 at 8:17 AM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > Quoting Prashant Malani (2022-07-06 11:26:19) > > > > Stephen, any pending concerns? > > No more pending concerns. > > > If not,I will post a v6 series with the suggested changes: > > - Drop typec-switch binding; instead add a new top-level port with > > end-points for each Type-C connector's switch. > > - Drop it6505 patches. > > - Squash anx7625 driver patches into one patch. > > - Add a comment mentioning that we aren't registering the orientation-switch. We've been working on these changes, and the new DT node looks like this: ``` anx_bridge_dp: anx7625-dp@58 { [...] mode-switch; ports { [...] typec_switches: port@2 { #adderss-cells = <1>; #size-cells = <0>; reg = <2>; anx_typec0: endpoint@0 { reg = <0>; remote-endpoint = <&typec_port0>; }; anx_typec1: endpoint@1 { reg = <1>; remote-endpoint = <&typec_port1>; }; }; }; ``` However we found some issues with that approach: 1. The current typec mux API does not allow us to put muxes into `ports` directly. `fwnode_typec_mux_get` searches for the parent node behind the port(s) nodes, so we cannot register the muxes with the port nodes unless we change the interface. 2. We need a compatible string between the `endpoint` nodes and the parent node (anx7625-dp@58). This is because when the driver core builds the device links, they only add links on nodes with a compatible string for `remote-endpoint` properties[1]. Without a compatible string, the parent node of `typec_port0` (cros-ec-typec in our case) has to be probed before anx7625, but this leads to a deadlock because cros-ec-typec requires anx7625 to register the typec_mux drivers first. I'm not sure if this is cros-ec-typec specific, though. *Any* compatible string fixes this issue, and it doesn't have to be "typec-switch". -- Alternatively, can we split the two muxes into two sub-nodes, like the following snippet? ``` anx_bridge_dp: anx7625-dp@58 { [...] mode-switch; anx_mux0 { compatible = "typec-switch"; reg = <0>; port { anx_typec0: endpoint { remote-endpoint = <&typec_port0>; }; }; }; anx_mux1 { compatible = "typec-switch"; reg = <1>; port { anx_typec1: endpoint { remote-endpoint = <&typec_port1>; }; }; }; ``` This eliminates the additional "switches" node in the devicetree. The sub-nodes also describe our hardware design, which split the DP lanes of anx7625 to two type-c ports. [1]: The `node_not_dev` property searches for a node with a compatible string: https://elixir.bootlin.com/linux/latest/source/drivers/of/property.c#L1390 > > Ok. I'll take a look on v6.