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

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

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@redhat.com>
> ---
>  .../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.


thanks,

-- 
heikki