Re: [PATCH v2 1/1] dt-bindings: connector: Add role‑switch provider phandle

[Dmitry Baryshkov]

On Thu, Mar 26, 2026 at 11:50:26AM -0700, Elson Serrao wrote:
> 
> 
> On 3/25/2026 6:57 PM, Dmitry Baryshkov wrote:
> > On Thu, 26 Mar 2026 at 03:49, Elson Serrao
> > <elson.serrao@xxxxxxxxxxxxxxxx> wrote:
> >>
> 
> [...]
> 
> >>>
> >>> Previously we didn't have such an issue for the usb-role-switch,
> >>> because there always have been a direct link between the USB connector
> >>> (be it gpio-usb-b-connector or usb-c-connector) and the USB controller
> >>> (implementing usb-role-switch). As with the EUD this is no longer a
> >>> case, my suggestion would be to follow prior art and let EUD receive,
> >>> interpret and resend usb-role-switch events.
> >>>
> >>
> >> In this topology, the EUD hardware spans more than one independent
> >> role-switch relationship, as a single EUD node is the direct neighbor of
> >> multiple connectors. This introduces additional considerations around
> >> role-switch discovery.
> >>
> >> One practical consideration if the EUD registers multiple role-switch
> >> instances is that fwnode_usb_role_switch_get() ( which relies on
> >> class_find_device_by_fwnode API), assumes a unique firmware node per
> >> role-switch instance. If multiple role-switch instances are registered
> >> against the same firmware node (the EUD fwnode), the lookup will return
> >> only the first registered instance, making it difficult for a connector to
> >> reliably bind to its intended role-switch provider.
> >>
> >> Supporting multiple role-switch instances in this model would therefore
> >> require extending the lookup mechanism to allow additional disambiguation
> >> (for example, associating role-switch instances with connector context).
> >>
> >> I want to make sure I clearly understand the intended modeling and whether
> >> these USB role-switch framework implications are considered acceptable.
> > 
> > As far as I can see, you can register two usb-role-switches, one per
> > the EUD path. then the connector will still be able to discover
> > correct switch by following the chain from the connector. On the other
> > hand, the EUD driver can use fwnode_usb_role_switch_get() passing the
> > path's fwnode and find the next role-switch connected to the each of
> > the EUD ports / paths.
> > 
> 
> My earlier questions were primarily around a flattened ports representation.
> I agree that modeling each EUD path as a distinct child node with its own
> firmware node addresses those concerns cleanly.
> 
> For context, the existing EUD binding [1] models a single controller ↔
> connector relationship using a flat top-level ports block. An earlier
> attempt [2] to reinterpret that top-level structure to represent
> multiple paths ran into DT ABI concerns, as it changed the meaning of
> existing bindings.
> 
> Based on your example, my understanding is that the intended direction is
> to keep the existing top-level `ports` semantics unchanged for backward
> compatibility, and model multi-path hardware using explicit child nodes,
> each representing one controller ↔ connector relationship and registering
> a separate usb-role-switch instance.

I'd say, it is a good idea. This matches the port / ports case, where we
don't create ports node if we know that there will be only ports@0 node.
Likewise you can have only one OF graph if there is only one path (for
backwards compatibility) and add subnodes if there is more than one.

> 
> Please let me know if this matches the intended direction.
> 
> For the purposes of the usb‑role‑switch discussion, the other feedback in
> [2] around PHY handling is orthogonal and will be addressed in a follow‑up
> revision.
> 
> Thanks,
> Elson
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/Documentation/devicetree/bindings/soc/qcom/qcom,eud.yaml?h=v7.0-rc5
> [2] https://lore.kernel.org/all/20260126233830.2193816-2-elson.serrao@xxxxxxxxxxxxxxxx/
> 
> > Here I am assuming that EUD device structured in a way like:
> > 
> > eud {
> >   compatible = "qcom,eud";
> > 
> >   path@0 {
> >     ports {
> >       port@0 {
> >         endpoint {
> >           remote-endpoint = <&usb_con_0_hs>;
> >         };
> >       };
> >       port@1 {
> >         endpoint {
> >           remote-endpoint = <&usb0_hs>;
> >         };
> >       };
> >     };
> >   };
> > 
> >   path@1 {
> >     ports {
> >       port@0 {
> >         endpoint {
> >           remote-endpoint = <&usb_con_1_hs>;
> >         };
> >       };
> >       port@1 {
> >         endpoint {
> >           remote-endpoint = <&usb1_hs>;
> >         };
> >       };
> >     };
> >   };
> > 
> > };
> > 

-- 
With best wishes
Dmitry