Hi Heikki,

On Mon, Sep 13, 2021 at 04:30:08PM +0300, Heikki Krogerus wrote:

My plan is to register a separate power supply for each PDO. So for
every source PDO and every sink PDO a port has in its capabilities,
you'll have a separate power supply registered, and the same for the
partner when it's connected. With every connection there should always
be one active (online) source psy and active sink psy (if the port is
source, one of the port's source psys will be active and the partner
will have the active sink psy, or vise versa - depending on the role).

Each PDO represents a "Power Supply" so to me that approach just
makes the most sense. It will require a bit of work in kernel, however
in user space it should mean that we only have a single new attribute
file for the power supplies named "pdo" that returns a single PDO.

There's a few downsides to this approach (one psy for each pdo).

The PDOs returned by Source Capabilities and Sink Capabilities do not just
contain possible Voltage, Current, and Power combinations, but they also contain
additional information in the form of properties.

In the Fixed Supply PDO, the following bits convey information about the supply
or sink (See USB PD Spec R3.1 V1.0 Table 6-9):

* B29 - Dual-Role Power
* B28 - USB Suspend Supported
* B27 - Unconstrained Power
* B26 - USB Communications Capable
* B25 - Dual-Role Data
* B24 - Unchunked Extended Messages Supported
* B23 - EPR Mode Capable

These bits exist in every single 32-bit Fixed Supply PDO, however, 
Section 6.4.1.2.2 requires that they be appropriately set in the vSafe5V Fixed
PDO (which is required for all sources and sinks), and set to 0 in all other
PDOs in the caps.

Since all USB Providers support vSafe5V, the required vSafe5V Fixed Supply
Power Data Object is also used to convey additional information that is
returned in bits 29 through 25. All other Fixed Supply Power Data Objects
Shall set bits 29…22 to zero.

So, splitting out a particular port partner or port's PDOs into individual
power supplies runs the risk of the information above not properly being
attributed to the actual power supply.

For example, if you're connected to a 18W power supply that has a vSafe5V PDO,
and a 9V Fixed PDO, and you're operating at 18W, the 9V Fixed PDO will be Active
but the inactive vSafe5V PDO has information in those higher order bits that
remain relevant.

Just something to keep in mind.

Let me know if you guys see any obvious problems with the idea.
Otherwise, that is how we really need to do this. That will make
things much more clear in user space. I have a feeling it will make
things easier in kernel as well in the long run.

Adding Adam and Guenter. It would be good if you guys could comment
the idea as well.

I'm supportive of using a separate PSY to represent the different power roles
of a particular port and port-partner, however. If you're connected to a USB-C
power bank, you should see two objects for that partner, one for when the
power bank is in source role, and one when the power bank is in sink role.

Even when you're in one role or the other, you should still be able to read
information from the other role in order to make an informed decision whether
you want to power role swap.

Thanks so much!
Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org