Re: [RFC PATCH 0/2] usb: Link USB devices with their USB Type-C partner counterparts

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

Hi Douglas,

On Tue, Aug 22, 2023 at 10:52:12AM -0400, Douglas Gilbert wrote:
> On 2023-08-22 09:32, Heikki Krogerus wrote:
> On a related matter, I wonder why there aren't symlinks between typec ports
> (under /sys/class/typec ) and/or the corresponding pd objects (under
> /sys/class/usb_power_delivery ) to the related power_supply objects under
> /sys/class/power_supply . For example under the latter directory I see:
>     $ ls | more
>     AC
>     BAT0
>     hidpp_battery_1
>     ucsi-source-psy-USBC000:001
>     ucsi-source-psy-USBC000:002
> 
> Those last two power supplies are obviously connected to typec port0 and port1
> (but offset by 1). Those power_supply objects hold inaccurate data which I hope
> will improve in time. Significantly power_supply objects don't seem to report
> the direction of the power. Here is a little utility I have been working on
> to report the USB Type-C port/pd disposition on my machine:
>     $ lsucpd
>     port0 [pd0]  > {5V, 0.9A}
>     port1 [pd1]  <<===  partner: [pd8]
> 
> My laptop (Thinkpad X13 G3) has two type-C ports and port1 is a sink with a
> PD contract. I would like that second line to have 20V, 3.25A appended to it
> but there are several issues:
>   - no typec or pd symlink to ucsi-source-psy-USBC000:002
>   - that power supply_object says it is online (correct) with a voltage_now:
>     5000000 uV (incorrect) and current_now: 3000000 uA (incorrect). See below.
> 
>   ucsi-source-psy-USBC000:002 $ ls_name_value
>     current_max : 3250000
>     current_now : 3000000
>     online : 1
>     scope : Unknown
>     type : USB
>     uevent : <removed>
>     usb_type : C [PD] PD_PPS
>     voltage_max : 20000000
>     voltage_min : 5000000
>     voltage_now : 5000000

I'm glad you brought that up. The major problem with the Type-C power
supplies is that the Type-C connector class does not actually take
care of them. They are all registered by the device drivers, and all
of them seem to expose different kind of information. In your case the
power supplies are registered by the UCSI driver, and the above may
indicate a bug in that driver.

To improve the situation, I originally proposed that instead of
adding a separate device class for USB Power Delivery objects, we
would utilise the already existing power supply class. That proposal
was not seen acceptable by many (including Benson if I recall), and I
now tend to agree with that because of several reasons, starting from
the fact that USB PD objects supply other informations on top of power
delivery details (so completely unrelated to PM).

Even before that I had proposed that the Type-C connector class could
supply API for the drivers to take care of the registration of the
power supplies. I proposed that not only the Type-C ports should
register the power supplies but also the partners should represent
their own power supplies. That would make things much more clear for
the user space IMO. The port and partner would always create a "chain"
of supplies where the other is the supply the other the sink of power.
That is already supported by the power supply class. For some reason
this proposal was also not seen as a good idea at the time, but it may
be that I just failed to explain it properly.

Nevertheless, I still think that that is exactly how the Type-C power
supplies should be always presented - separate supplies for both ports
and partners - and that obviously the Type-C connector class should
take care of those supplies so that they always supply the same
information. Unfortunately I do not have any time at the moment to
work on this right now.

Br,

-- 
heikki