On Thu, Jun 23, 2022 at 5:35 PM Prashant Malani pmalani@chromium.org wrote:
On Thu, Jun 23, 2022 at 4:14 PM Stephen Boyd swboyd@chromium.org wrote:
Quoting Prashant Malani (2022-06-23 12:08:21)
On Thu, Jun 23, 2022 at 11:30 AM Stephen Boyd swboyd@chromium.org wrote:
Quoting Prashant Malani (2022-06-22 10:34:30)
diff --git a/Documentation/devicetree/bindings/usb/typec-switch.yaml b/Documentation/devicetree/bindings/usb/typec-switch.yaml new file mode 100644 index 000000000000..78b0190c8543 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/typec-switch.yaml @@ -0,0 +1,74 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2
[...]
- ports:
- $ref: /schemas/graph.yaml#/properties/ports
- description: OF graph binding modelling data lines to the Type-C switch.
- properties:
port@0:$ref: /schemas/graph.yaml#/properties/portdescription: Link between the switch and a Type-C connector.Is there an update to the usb-c-connector binding to accept this port connection?
Not at this time. I don't think we should enforce that either. (Type-C data-lines could theoretically be routed through intermediate hardware like retimers/repeaters)
I'm mostly wondering if having such a connection to the usb-c-connector, or even through some retimer/repeater, would be sufficient to detect how many type-c ports are connected to the device. If the type-c pin assignments only support two or four lanes for DP then it seems like we should describe the two lanes or four lanes as one graph endpoint "output" and then have some 'data-lanes' property in case the DP lanes are flipped while being sent to the retimer or usb-c-connector. This would of course depend on the capability of the device, i.e. if it can remap DP lanes or only has 2 lanes of DP, etc.
- |
- drm-bridge {
usb-switch {compatible = "typec-switch";I still don't understand the subnode design here. usb-switch as a container node indicates to me that this is a bus, but in earlier rounds of this series it was stated this isn't a bus.
I am not aware of this as a requirement. Can you please point me to the documentation that states this needs to be the case?
I'm not aware of any documentation for the dos and don'ts here. Are there any examples in the bindings directory that split up a device into subnodes that isn't in bindings/mfd?
usb-c-connector [3] and its users is an example.
I just know from experience that any time I try to make a child node of an existing node that I'm supposed to be describing a bus, unless I'm adding some sort of exception node like a graph binding or an opp table. Typically a node corresponds 1:1 with a device in the kernel. I'll defer to Rob for any citations.
Why doesn't it work to merge everything inside usb-switch directly into the drm-bridge node?
I attempted to explain the rationale in the previous version [1], but using a dedicated sub-node means the driver doesn't haven't to inspect individual ports to determine which of them need switches registered for them. If it sees a `typec-switch`, it registers a mode-switch and/or orientation-switch. IMO it simplifies the hardware device binding too.
How is that any harder than hard-coding that detail into the driver about which port and endpoint is possibly connected to the usb-c-connector (or retimer)? All of that logic could be behind some API that registers a typec-switch based on a graph port number that's passed in, ala drm_of_find_panel_or_bridge()'s design.
If each driver has to do it (and the port specifics vary for each driver), it becomes an avoidable overhead for each of them. I prefer hard-coding such details if avoidable. I suppose both approaches
Sorry, I meant "I prefer not hard-coding such details..."
require modifications to the binding and the driver code.
Coming from a DT writer's perspective, I just want to go through the list of output pins in the datasheet and match them up to the ports binding for this device. If it's a pure DP bridge, where USB hardware isn't an input or an output like the ITE chip, then I don't want to have to describe a port graph binding for the case when it's connected to a dp-connector (see dp-connector.yaml) in the top-level node and then have to make an entirely different subnode for the usb-c-connector case with a whole other set of graph ports.
This approach still allows for that, if the driver has any use for it (AFAICT these drivers don't). Iff that driver uses it, one can (optionally) route their output (top-level) ports through the "typec-switch" sub-node (and onwards as required). If it's being used in a "pure-DP" configuration, the "typec-switch" just goes away (the top level ports can be routed as desired by the driver). (Again, I must reiterate that neither this driver or the anx driver utilizes this)
How would I even know which two differential pairs correspond to port0 or port1 in this binding in the ITE case?
Why do we need to know that? It doesn't affect this or the other driver or hardware's functioning in a perceivable way.
Ideally we make the graph binding more strict for devices by enforcing that their graph ports exist. Otherwise we're not fully describing the connections between devices and our dtb checkers are going to let things through where the driver most likely will fail because it can't figure out what to do, e.g. display DP on 4 lanes or play some DP lane rerouting games to act as a mux.
How is the current binding enforcing this? The typec-switch binding as a first step ensures that the DT is connecting the hardware(anx,ite etc) to something that at least "claims" to be a Type-C switch.
It also maps with the internal block diagram for these hardware components (for ex. the anx7625 crosspoint switch is a separate sub-block within anx7625).
We don't make DT bindings for sub-components like this very often. It would make more sense to me to have a subnode if a typec switch was some sort of off the shelf hard macro that the hardware engineer placed down inside the IC that they delivered. Then we could have a completely generic driver that binds to the generic binding that knows how to drive the hardware, because it's an unchangeable hard macro with a well defined programming interface.
[1] https://lore.kernel.org/linux-usb/CACeCKaeH6qTTdG_huC4yw0xxG8TYEOtfPW3tiVNwY...
I looked at the fsa4480 driver and the device has a publicly available datasheet[2]. That device is designed for "audio accessory mode" but I guess it's being used to simply mux SBU lines? There isn't an upstream user of the binding so far, but it also doesn't look like a complete binding. I'd expect to see DN_L/R as a graph output connected to the usb-c-connector and probably have a usb2.0 input port and a 'sound-dai' property to represent the input audio path.
Finally, simply connecting to the typec controller node isn't sufficient because a typec controller can be controlling many usb-c-connectors so I don't see how the graph binding would be able to figure out how many usb-c-connectors are connected to a mux like device, unless we took the approach of this patch.
It can follow the endpoint of the typec-switch port (the port parent of the remote end-point would be a 'usb-c-connector'). That is if the graph binding (I'm assuming you mean the switch device here) wants to figure this out in the first place.
Is that why you're proposing this binding? To avoid describing a graph binding in the usb-c-connector and effectively "pushing" the port count up to the mux?
No, that is not the intention behind this series. The 'usb-c-connector' still needs the graph binding to the `typec-switch`. SBU, HS and SS lanes might have different muxes altogether (usb-c-connect has separate ports for SBU, HS and SS lanes)
[3] https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bind...