Mon, Sep 13, 2021 at 12:30:58PM -0700, Benson Leung kirjoitti:

Hi Adam and Heikki,

On Mon, Sep 13, 2021 at 03:15:46PM +0000, Adam Thomson wrote:

quoted

On 13 September 2021 14:30, Heikki Krogerus wrote:

quoted

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.

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.

Hi Heikki,

Thanks for CCing me. My two pence worth is that I always envisaged the PSY
representation as being 1 PSY for 1 power source. I consider this in a
similar manner to the Regulator framework, where 1 regulator can support a range
of voltages and currents, but this is covered by 1 regulator instance as it's
just a single output. For USB-PD we have a number of options for voltage/current
combos, including PPS which is even lower granularity, but it's still only one
port. I get the feeling that having PSY instances for each and every PDO might
be a little confusing and these will never be concurrent.

However, I'd be keen to understand further and see what restrictions/issues are
currently present as I probably don't have a complete view of this right now. I
wouldn't want to dismiss something out of turn, especially when you obviously
have good reason to suggest such an approach.

I thought of one more potential downside to one-psy-per-pdo:

Remember that a source or sink's Capabilities may dynamically change without
a port disconnect, and this could make one-psy-per-pdo much more chatty with
power supply deletions and re-registrations on load balancing events.

At basically any time, a power source may send a new SRC_CAP to the sink which
adjusts, deletes, or adds to the list of PDOs, without the connection state
machine registering a disconnect.

In a real world case, I have a charger in my kitchen that has 2 USB-C ports
and supports a total of 30W output. When one device is plugged in:
5V 3A, 9V 3A, 15V 2A
However, when two devices are plugged in, each sees:
5V 3A

The load balancing event would result in two power supply deletions, whereas
if it were a single psy per power supply (incorporating the list of PDO choices)
it would just be a single PROP_CHANGED event.

It seems cleaner to me to have deletions and additions only possible when the
thing is unplugged or plugged.

I just argued to Adam that because the capabilities can change in
reality at any time, just like you pointed out here, using a psy
hierarchy instead of trying to handle everything with a single psy is
not only more clear, it's actually safer, and definitely less "hacky"
approach.

I don't really see why would it be a problem to unregister and
register the psys in the hierarchy be a problem?

thanks,

-- 
heikki