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]
Message-ID: <DB9PR10MB4652B4A6A2A2157018307AE380D99@DB9PR10MB4652.EURPRD10.PROD.OUTLOOK.COM>
Date:   Mon, 13 Sep 2021 15:15:46 +0000
From:   Adam Thomson <Adam.Thomson.Opensource@...semi.com>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Prashant Malani <pmalani@...omium.org>
CC:     "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>,
        Adam Thomson <Adam.Thomson.Opensource@...semi.com>,
        Guenter Roeck <linux@...ck-us.net>
Subject: RE: [RFC PATCH 2/3] power: supply: Add support for PDOs props

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

> > Add support for reporting Source and Sink Capabilities
> > Power Data Object (PDO) property. These are reported by USB
> > Power Delivery (PD) capable power sources.
> >
> > Signed-off-by: Prashant Malani <pmalani@...omium.org>
> > ---
> >  Documentation/ABI/testing/sysfs-class-power | 30 +++++++++++++++++++++
> >  drivers/power/supply/power_supply_sysfs.c   | 18 ++++++++++++-
> >  include/linux/power_supply.h                |  5 ++++
> >  3 files changed, 52 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-power
> b/Documentation/ABI/testing/sysfs-class-power
> > index ca830c6cd809..90d4198e6dfb 100644
> > --- a/Documentation/ABI/testing/sysfs-class-power
> > +++ b/Documentation/ABI/testing/sysfs-class-power
> > @@ -562,6 +562,36 @@ Description:
> >  			      "Unknown", "SDP", "DCP", "CDP", "ACA", "C", "PD",
> >  			      "PD_DRP", "PD_PPS", "BrickID"
> >
> > +What:
> 	/sys/class/power_supply/<supply_name>/source_cap_pdos
> > +Date:		September 2021
> > +Contact:	linux-pm@...r.kernel.org
> > +Description:
> > +		Reports the Source Capabilities Power Data Objects (PDO)
> reported by the USB
> > +		PD-capable power source. 13 PDOs are listed. PDOs 1-7 represent
> the Source Caps
> > +		for devices which only support Standard Power Range (SPR),
> whereas PDOs 8-13
> > +		are for Extended Power Range (EPR) capable sources.
> > +		NOTE: The EPR Source Caps message is a superset of the Source
> Cap message, so on
> > +		SPR-only sources, PDOs 8-13 will be 0.
> > +
> > +		Access: Read-Only
> > +
> > +		Valid values: Represented as a list of 13 32-bit PDO objects in
> hexadecimal format.
> > +
> > +What:
> 	/sys/class/power_supply/<supply_name>/sink_cap_pdos
> > +Date:		September 2021
> > +Contact:	linux-pm@...r.kernel.org
> > +Description:
> > +		Reports the Sink Capabilities Power Data Objects (PDO) reported
> by the USB
> > +		PD-capable power source. 13 PDOs are listed. PDOs 1-7 represent
> the Sink Caps
> > +		for devices which only support Standard Power Range (SPR),
> whereas PDOs 8-13
> > +		are for Extended Power Range (EPR) capable sinks.
> > +		NOTE: The EPR Sink Caps message is a superset of the Sink Cap
> message, so on
> > +		SPR-only sinks, PDOs 8-13 will be 0.
> > +
> > +		Access: Read-Only
> > +
> > +		Valid values: Represented as a list of 13 32-bit PDO objects in
> hexadecimal format.
>
> 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.