linux-kernel - Re: [RFC PATCH 2/3] power: supply: Add support for PDOs props

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 13 Sep 2021 12:30:58 -0700
From:   Benson Leung <bleung@...gle.com>
To:     Adam Thomson <Adam.Thomson.Opensource@...semi.com>
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Prashant Malani <pmalani@...omium.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "bleung@...omium.org" <bleung@...omium.org>,
        "badhri@...gle.com" <badhri@...gle.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Sebastian Reichel <sre@...nel.org>,
        Guenter Roeck <linux@...ck-us.net>
Subject: Re: [RFC PATCH 2/3] power: supply: Add support for PDOs props

Hi Adam and Heikki,

On Mon, Sep 13, 2021 at 03:15:46PM +0000, Adam Thomson wrote:
> On 13 September 2021 14:30, 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.
> >
> > 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.

Thanks,
Benson

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

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)