Re: [PATCH 2/3] usb: typec: tcpm: Add support for configuring DP altmode through device-properties

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

 



On Thu, Nov 14, 2019 at 12:16:09PM +0100, Hans de Goede wrote:
> 
> Hi,
> 
> On 21-10-2019 08:55, Heikki Krogerus wrote:
> > Hi Hans,
> > 
> > On Fri, Oct 18, 2019 at 09:57:18PM +0200, Hans de Goede wrote:
> > > Add support for configuring display-port altmode through device-properties.
> > > 
> > > We could try to add a generic mechanism for describing altmodes in
> > > device-properties, but various altmodes will likely need altmode specific
> > > configuration. E.g. the display-port altmode needs some way to describe
> > > which set of DP pins on the GPU is connected to the USB Type-C connector.
> > > 
> > > As such it is better to have a separate set of altmode specific properties
> > > per altmode and this commit adds a property for basic display-port altmode
> > > support.
> > > 
> > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > > ---
> > >   .../bindings/connector/usb-connector.txt      |  3 ++
> > >   drivers/usb/typec/tcpm/tcpm.c                 | 33 +++++++++++++++++++
> > >   2 files changed, 36 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > index d357987181ee..7bae3cc9c76a 100644
> > > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > @@ -38,6 +38,9 @@ Optional properties for usb-c-connector:
> > >     or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
> > >   - data-role: should be one of "host", "device", "dual"(DRD) if typec
> > >     connector supports USB data.
> > > +- displayport-vdo: The presenence of this property indicates that the
> > > +  usb-connector supports displayport-altmode (svid 0xff01), the value of
> > > +  this property is an u32 with the vdo value for the displayport-altmode,
> > 
> > No, let's not take this approach.
> > 
> > Every alternate mode a connector supports will need to have its own
> > "sub-fwnode" under the connector fwnode. I thought we agreed this
> > earlier?
> > 
> > In any case, those sub-nodes will have default device properties named
> > "svid" and "vdo". If the alternate mode still needs some other
> > details, it can have other device properties that are specific to it,
> > but note that displayport alt mode does not need anything extra. The
> > "vdo" will already tells which pin configurations the connector
> > supports and that is all that the driver needs to know.
> > 
> > After we have the sub-nodes, it's not a big deal to walk through the
> > child-nodes the port has during port registration and register the
> > port alternate modes at the same time. That we can do in
> > typec_register_port(), so we do not need to do it in every driver
> > separately.
> 
> Yes we did agree to do the sub-fwnode thingie. But since this is a hobby
> project I do not have a whole lot of time to work on this.
> 
> So when I started working on this, I though that the approach from this
> patch-set would be more KISS and IMHO it works out well. But the sub-fwnode
> approach is probably more future proof.
> 
> Anyways as said I do not have a whole lot of time to work on this,
> if you want to go the sub-fwnode route, perhaps you can do a PoC
> patch series for this? I would be happy to test this and if necessary
> work it into something which works for the DP case.

Sure, I'll prepare something for that once I have some spare time.

> Doing the port alternate modes registration from typec_register_port()
> does sound like a good idea.
> 
> The first patch in this series is independent of this and IMHO it
> would be good to get that upstream regardless of this alt-mode
> registration stuff, so I will resend that as a standalone patch.

OK,

thanks,

-- 
heikki



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux