Re: [PATCH v5 1/9] dt-bindings: usb: Add Type-C switch binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Prashant Malani (2022-06-24 14:41:36)
> On Fri, Jun 24, 2022 at 12:50 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> >
> > Quoting Prashant Malani (2022-06-23 19:48:04)
> > > On Thu, Jun 23, 2022 at 7:13 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> > > >
> > > > Quoting Prashant Malani (2022-06-23 17:35:38)
> > > > > On Thu, Jun 23, 2022 at 4:14 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> > > > > >
> > > > > > 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.
> > > >
> > > > What are the subnodes? The graph ports? That is not what I meant.
> > >
> > > cros-ec-typec [4] uses subnodes of usb-c-connector. Chrome OS DTs
> > > use the ports from the included usb-c-connector to switching hardware.
> >
> > Ok, got it. usb-c-connector nodes are children of the typec controller
> > (in this case cros-ec-typec) because otherwise we would need to make a
> > phandle link from the usb-c-connector node(s) under the root node / to
> > the typec controller. The phandle link may need to be done in both
> > directions, so it makes more sense to put the usb-c-connector nodes
> > underneath the typec controller to express the direct relationship
> > between the typec controller and the usb-c-connectors.
> >
> > Furthermore, the usb-c-connector is not integrated as part of the EC in
> > the same package. There is a discrete part placed on the board that
> > corresponds to the usb-c-connector and that is separate from the EC. The
> > connectors are in essence only controllable through the EC because
> > that's the typec controller.
>
> From the perspective of the AP, the `usb-c-connector` *is* an integrated part of
> the Chrome EC; there is no alternative way to control it except
> through the Chrome EC.
> So the above example reinforces the usage model for typec-switch (which is
> also an "integrated" component).

No the usb-c-connector is not an integrated part of the EC. It is a
discrete part with a part number that gets placed on the PCB. From the
perspective of the AP it is controlled via the EC, but it is not
integrated into the same package that the EC is packaged in to be
soldered down to the PCB.

So the example of usb-c-connector as a subnode doesn't reinforce the
argument for a typec-switch binding. It highlights the difference that
I've been trying to explain. The difference between internal vs.
external components of a device. In the EC case the usb-c-connector is
an external component from the EC, hence the two nodes. In the anx or
ite case the typec switch is an internal component, hence the single
node for the anx or ite part.

>
> This really is a limited binding change that helps describe connections
> between Type-C components, helps these components integrate with
> the kernel Type-C framework, and consolidates the associated properties.
> I believe it works for most current use cases in the upstream kernel.
>
> I'm happy to discuss more theoretical use cases further, but
> respectfully, I prefer to do
> so off-list.

I'm happy to move the discussion to the anx or ite bindings if you'd
prefer to discuss more concrete bindings.

>
> If the maintainer is OK with it (Krzysztof has reviewed it, but I
> don't want to presume
> what the protocol is for patches in this subsystem), and we've
> provided 2 users as asked for
> in v4 [5], then I request its consideration for submission.
> If the maintainers have further concerns, we'd be happy to address them.
>

Rob?



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux