Re: PATCH 0/4] usbd: typec: fusb302: Add support for specifying supported alternate-modes through devicetree/fwnodes

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 12/7/21 10:56, Heikki Krogerus wrote:
> Hi,
> 
> On Fri, Dec 03, 2021 at 12:22:35PM -0800, Prashant Malani wrote:
>> Hi Hans,
>>
>> On Fri, Dec 3, 2021 at 2:14 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi Prashant,
>>>
>>> On 12/2/21 20:29, Prashant Malani wrote:
>>>> Hi Hans,
>>>>
>>>> Sorry for posting on an old thread, but I was wondering if there was
>>>> still a plan to submit this? This is something we'd like to use on
>>>> Chrome OS too.
>>>>
>>>> It sounded like the primary discussion was whether to have an "altmodes"
>>>> property encaspulating the various alt modes, but not sure what the
>>>> final consensus on that was (sounded to me like your current
>>>> implementation was fine for now, and ACPI use cases would be handled
>>>> later?).
>>>
>>> Support for this has already landed, but so far has only been tested
>>> on a x86/ACPI device, where the firmware-nodes parsed by the new
>>> typec_port_register_altmodes() helper are setup through software-nodes,
>>> see:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7b458a4c5d7302947556e12c83cfe4da769665d0
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=55d8b34772e0728a224198ba605eed8cfc570aa0
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3d28466e5f4f8
>>>
>>> In theory this should be usable for devicetree as is. But that would
>>> require documenting the current in kernel swnode bindings as
>>> official devicetree bindings and getting that through the devicetree
>>> bindings review process.
>>
>> That's good to hear :)
>>
>>>
>>> I have deliberately not done this because the devicetree maintainers
>>> have asked for properties / swnode "bindings" used only inside
>>> the kernel (1) to NOT be documented as official devicetree bindings,
>>> they (the dt bindings maintainers) want to first see at least one
>>> real devicetree users before adding things like this to the
>>> official devicetree bindings docs.
>>>
>>> Note if the way typec_port_register_altmodes() parses the fwnode
>>> properties needs to change as result of the devicetree bindings review
>>> process, please let me know, because then the swnode-s created in
>>> drivers/platform/x86/intel/int33fe/intel_cht_int33fe_typec.c
>>> need to change to match so as to not regress things on those devices.
>>
>> Heikki, can we reconcile this with the format you had in mind for ACPI
>> devices which specify this in ASL files?
> 
> I don't know. I'm not sure what are the changes that need to be made
> in order to fit this thing into the devicetree bindings (or are there
> any)?
> 
> Assuming that the proposal is still that each connector device node
> would have a sub-node "altmodes" which then has a child node for each
> supported alt mode,

Right, this is the format that the current implementation of
typec_port_register_altmodes() expects.

Regards,

Hans


> then the ASL for the first USB Type-C port (as an
> example) should look roughly like this (this is prepared on top the
> ACPI tables from Intel Tigerlake based Chromebook system):
> 
>         Scope (\_SB.H_EC.USBC.CON0)
>         {
>                 Name (_DSD, Package () {
>                         ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>                         Package () {
>                                 Package () { "altmodes", "ALT0" },
>                         }
>                 })
> 
>                 /* The "altmodes" sub-node */
>                 Name (ALT0, Package () {
>                         ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
>                         Package () {
>                                 Package () { "tbt", "ALT1" },
>                                 Package () { "dp", "ALT2" },
>                         }
>                 })
> 
>                 /* Thunderbolt 3 Alternate Mode */
>                 Name (ALT1, Package() {
>                         ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                         Package () {
>                                 Package () { "svid", 0x8087 },
>                                 Package () { "vdo", 0x00000001 },
>                         },
>                 })
> 
>                 /* DisplayPort Alternate Mode */
>                 Name (ALT2, Package() {
>                         ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                         Package () {
>                                 Package () { "svid", 0xff01 },
>                                 Package () { "vdo", 0x001c1c47 },
>                         },
>                 })
>         }
> 
> So with that, this series should work as is. Let me know if you need
> me to explain that in more detail. The Hierarchical Data Extension
> _DSD UUID is documented here:
> https://uefi.org/sites/default/files/resources/_DSD-hierarchical-data-extension-UUID-v1.1.pdf
> 
> But as said, I'm now not sure what the final design should look like?
> 
> The ACPI format we can in any case quite likely make work with what
> ever requirements/limitation the devicetree has. We just need to
> understand what those are.
> 
> After that I would really like to see the format documented for
> ACPI. Though, I'm not sure where should it be documented. I think we
> are talking about some kind of BIOS writing guide or similar.
> 
> thanks,
>