2021-09-03 00:05:08

by Prashant Malani

[permalink] [raw]
Subject: [RFC PATCH 0/3] Type C partner power supply and PDO support.

Hi,

We'd like to expose the USB PD Power Source/Sink capabilities provided
by a partner (SOP) to userspace. Towards that end,
here is a short series which adds the capabilities to the power supply
class. We also add a function to the Type C connector class
to register a power supply device for a connected partner.

I'm sending this out as an RFC to get some initial feedback from the
relevant groups. Once we can settle on an approach, I can resend the
series with a sample implementation in the Chrome OS Type C port driver
(cros-ec-typec).

Thanks!

Prashant Malani (3):
usb: pd: Increase max PDO objects to 13
power: supply: Add support for PDOs props
usb: typec: Add partner power registration call

Documentation/ABI/testing/sysfs-class-power | 30 +++++++++++++++++++++
drivers/power/supply/power_supply_sysfs.c | 18 ++++++++++++-
drivers/usb/typec/class.c | 18 ++++++++++++-
drivers/usb/typec/class.h | 2 ++
include/linux/power_supply.h | 5 ++++
include/linux/usb/pd.h | 8 +++++-
include/linux/usb/typec.h | 5 ++++
7 files changed, 83 insertions(+), 3 deletions(-)

--
2.33.0.153.gba50c8fa24-goog


2021-09-03 00:05:08

by Prashant Malani

[permalink] [raw]
Subject: [RFC PATCH 1/3] usb: pd: Increase max PDO objects to 13

Increase the max number of PDO objects to 13, to accommodate the extra
PDOs added as a part of EPR (Extended Power Range) operation introduced
in the USB PD Spec Rev 3.1, v 1.0. See Figure 6-54 for details.

Signed-off-by: Prashant Malani <[email protected]>
---
include/linux/usb/pd.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 96b7ff66f074..7e8bdca1ce6e 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -201,7 +201,13 @@ struct pd_message {
} __packed;

/* PDO: Power Data Object */
-#define PDO_MAX_OBJECTS 7
+
+/*
+ * The EPR (Extended Power Range) structure is a superset of the SPR (Standard Power Range)
+ * capabilities structure, so set the max number of PDOs to 13 instead of 7. On SPR-only systems,
+ * objects 8 through 13 will just be empty.
+ */
+#define PDO_MAX_OBJECTS 13

enum pd_pdo_type {
PDO_TYPE_FIXED = 0,
--
2.33.0.153.gba50c8fa24-goog

2021-09-03 07:51:09

by Jack Pham

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] usb: pd: Increase max PDO objects to 13

Hi Prashant,

On Thu, Sep 02, 2021 at 02:34:58PM -0700, Prashant Malani wrote:
> Increase the max number of PDO objects to 13, to accommodate the extra
> PDOs added as a part of EPR (Extended Power Range) operation introduced
> in the USB PD Spec Rev 3.1, v 1.0. See Figure 6-54 for details.
>
> Signed-off-by: Prashant Malani <[email protected]>
> ---
> include/linux/usb/pd.h | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index 96b7ff66f074..7e8bdca1ce6e 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -201,7 +201,13 @@ struct pd_message {
> } __packed;
>
> /* PDO: Power Data Object */
> -#define PDO_MAX_OBJECTS 7
> +
> +/*
> + * The EPR (Extended Power Range) structure is a superset of the SPR (Standard Power Range)
> + * capabilities structure, so set the max number of PDOs to 13 instead of 7. On SPR-only systems,
> + * objects 8 through 13 will just be empty.
> + */
> +#define PDO_MAX_OBJECTS 13

Hmm this might break the recent change I made to UCSI in commit
1f4642b72be7 ("usb: typec: ucsi: Retrieve all the PDOs instead of just
the first 4").

520 static void ucsi_get_src_pdos(struct ucsi_connector *con, int is_partner)
521 {
522 int ret;
523
524 /* UCSI max payload means only getting at most 4 PDOs at a time */
525 ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
526 if (ret < 0)
527 return;
528
529 con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
530 if (con->num_pdos < UCSI_MAX_PDOS)
531 return;
532
533 /* get the remaining PDOs, if any */
534 ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
535 PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
^^^^^^^^^^^^^^^
This routine calls the UCSI GET_PDOS command for up to 4 PDOs at a time
since that's the most the return payload can carry. Currently this
assumes that we'd only need to request the PPM at most twice to retrieve
all the PDOs for up to a maximum of 7 (first request for 4 then again if
needed for the remaining 3). I'm not sure if any existing UCSI FW would
be updatable to support more than 7 PDOs in the future, much less
support EPR. In fact, current UCSI 1.2 spec [1] Table 4-34 mentions PDO
offset valid values are 0-7 and anything else "shall not be used", so I
don't know how UCSI will eventually cope with EPR without a spec update.

So if this macro changes to 13 then this call would result in a call to
the UCSI GET_PDOS command passing num_pdos == 13-4 = 9 which would
probably result in an error from the PPM FW. So we might need to retain
the maximum value of 7 PDOs at least for UCSI here. Maybe that means
this UCSI driver needs to carry its own definition of
UCSI_MAX_TOTAL_PDOS=7 instead of using PDO_MAX_OBJECTS?

Jack
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2021-09-03 19:17:25

by Jack Pham

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] usb: pd: Increase max PDO objects to 13

On Thu, Sep 02, 2021 at 11:47:01PM -0700, Jack Pham wrote:
> Hi Prashant,
>
> On Thu, Sep 02, 2021 at 02:34:58PM -0700, Prashant Malani wrote:
> > Increase the max number of PDO objects to 13, to accommodate the extra
> > PDOs added as a part of EPR (Extended Power Range) operation introduced
> > in the USB PD Spec Rev 3.1, v 1.0. See Figure 6-54 for details.
> >
> > Signed-off-by: Prashant Malani <[email protected]>
> > ---
> > include/linux/usb/pd.h | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> > index 96b7ff66f074..7e8bdca1ce6e 100644
> > --- a/include/linux/usb/pd.h
> > +++ b/include/linux/usb/pd.h
> > @@ -201,7 +201,13 @@ struct pd_message {
> > } __packed;
> >
> > /* PDO: Power Data Object */
> > -#define PDO_MAX_OBJECTS 7
> > +
> > +/*
> > + * The EPR (Extended Power Range) structure is a superset of the SPR (Standard Power Range)
> > + * capabilities structure, so set the max number of PDOs to 13 instead of 7. On SPR-only systems,
> > + * objects 8 through 13 will just be empty.
> > + */
> > +#define PDO_MAX_OBJECTS 13
>
> Hmm this might break the recent change I made to UCSI in commit
> 1f4642b72be7 ("usb: typec: ucsi: Retrieve all the PDOs instead of just
> the first 4").
>
> 520 static void ucsi_get_src_pdos(struct ucsi_connector *con, int is_partner)
> 521 {
> 522 int ret;
> 523
> 524 /* UCSI max payload means only getting at most 4 PDOs at a time */
> 525 ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
> 526 if (ret < 0)
> 527 return;
> 528
> 529 con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> 530 if (con->num_pdos < UCSI_MAX_PDOS)
> 531 return;
> 532
> 533 /* get the remaining PDOs, if any */
> 534 ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
> 535 PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
> ^^^^^^^^^^^^^^^
> This routine calls the UCSI GET_PDOS command for up to 4 PDOs at a time
> since that's the most the return payload can carry. Currently this
> assumes that we'd only need to request the PPM at most twice to retrieve
> all the PDOs for up to a maximum of 7 (first request for 4 then again if
> needed for the remaining 3). I'm not sure if any existing UCSI FW would
> be updatable to support more than 7 PDOs in the future, much less
> support EPR. In fact, current UCSI 1.2 spec [1] Table 4-34 mentions PDO

Sorry, forgot the footnote with the link to the spec:
[1] https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/usb-type-c-ucsi-spec.pdf

> offset valid values are 0-7 and anything else "shall not be used", so I
> don't know how UCSI will eventually cope with EPR without a spec update.
>
> So if this macro changes to 13 then this call would result in a call to
> the UCSI GET_PDOS command passing num_pdos == 13-4 = 9 which would
> probably result in an error from the PPM FW. So we might need to retain
> the maximum value of 7 PDOs at least for UCSI here. Maybe that means
> this UCSI driver needs to carry its own definition of
> UCSI_MAX_TOTAL_PDOS=7 instead of using PDO_MAX_OBJECTS?
>
> Jack
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2021-09-07 22:25:58

by Prashant Malani

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] usb: pd: Increase max PDO objects to 13

Hi Jack,

Thanks for taking a look at the patch.

On Fri, Sep 3, 2021 at 11:05 AM Jack Pham <[email protected]> wrote:
>
> On Thu, Sep 02, 2021 at 11:47:01PM -0700, Jack Pham wrote:
> > Hi Prashant,
> >
> > On Thu, Sep 02, 2021 at 02:34:58PM -0700, Prashant Malani wrote:
> > > Increase the max number of PDO objects to 13, to accommodate the extra
> > > PDOs added as a part of EPR (Extended Power Range) operation introduced
> > > in the USB PD Spec Rev 3.1, v 1.0. See Figure 6-54 for details.
> > >
> > > Signed-off-by: Prashant Malani <[email protected]>
> > > ---
> > > include/linux/usb/pd.h | 8 +++++++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> > > index 96b7ff66f074..7e8bdca1ce6e 100644
> > > --- a/include/linux/usb/pd.h
> > > +++ b/include/linux/usb/pd.h
> > > @@ -201,7 +201,13 @@ struct pd_message {
> > > } __packed;
> > >
> > > /* PDO: Power Data Object */
> > > -#define PDO_MAX_OBJECTS 7
> > > +
> > > +/*
> > > + * The EPR (Extended Power Range) structure is a superset of the SPR (Standard Power Range)
> > > + * capabilities structure, so set the max number of PDOs to 13 instead of 7. On SPR-only systems,
> > > + * objects 8 through 13 will just be empty.
> > > + */
> > > +#define PDO_MAX_OBJECTS 13
> >
> > Hmm this might break the recent change I made to UCSI in commit
> > 1f4642b72be7 ("usb: typec: ucsi: Retrieve all the PDOs instead of just
> > the first 4").
> >
> > 520 static void ucsi_get_src_pdos(struct ucsi_connector *con, int is_partner)
> > 521 {
> > 522 int ret;
> > 523
> > 524 /* UCSI max payload means only getting at most 4 PDOs at a time */
> > 525 ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
> > 526 if (ret < 0)
> > 527 return;
> > 528
> > 529 con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> > 530 if (con->num_pdos < UCSI_MAX_PDOS)
> > 531 return;
> > 532
> > 533 /* get the remaining PDOs, if any */
> > 534 ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
> > 535 PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
> > ^^^^^^^^^^^^^^^
> > This routine calls the UCSI GET_PDOS command for up to 4 PDOs at a time
> > since that's the most the return payload can carry. Currently this
> > assumes that we'd only need to request the PPM at most twice to retrieve
> > all the PDOs for up to a maximum of 7 (first request for 4 then again if
> > needed for the remaining 3). I'm not sure if any existing UCSI FW would
> > be updatable to support more than 7 PDOs in the future, much less
> > support EPR. In fact, current UCSI 1.2 spec [1] Table 4-34 mentions PDO
>
> Sorry, forgot the footnote with the link to the spec:
> [1] https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/usb-type-c-ucsi-spec.pdf
>
> > offset valid values are 0-7 and anything else "shall not be used", so I
> > don't know how UCSI will eventually cope with EPR without a spec update.
> >
> > So if this macro changes to 13 then this call would result in a call to
> > the UCSI GET_PDOS command passing num_pdos == 13-4 = 9 which would
> > probably result in an error from the PPM FW. So we might need to retain
> > the maximum value of 7 PDOs at least for UCSI here. Maybe that means
> > this UCSI driver needs to carry its own definition of
> > UCSI_MAX_TOTAL_PDOS=7 instead of using PDO_MAX_OBJECTS?

Thanks for pointing this out. We can perhaps just add another macro
for EPR_PDO_MAX_OBJECTS, and leave the current macro as is for now.

Best regards,

2021-09-07 23:33:06

by Benson Leung

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] usb: pd: Increase max PDO objects to 13

Hi Jack,

On Thu, Sep 02, 2021 at 11:47:01PM -0700, Jack Pham wrote:
> Hi Prashant,
>
> On Thu, Sep 02, 2021 at 02:34:58PM -0700, Prashant Malani wrote:
> > Increase the max number of PDO objects to 13, to accommodate the extra
> > PDOs added as a part of EPR (Extended Power Range) operation introduced
> > in the USB PD Spec Rev 3.1, v 1.0. See Figure 6-54 for details.
> >
> > Signed-off-by: Prashant Malani <[email protected]>
> > ---
> > include/linux/usb/pd.h | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> > index 96b7ff66f074..7e8bdca1ce6e 100644
> > --- a/include/linux/usb/pd.h
> > +++ b/include/linux/usb/pd.h
> > @@ -201,7 +201,13 @@ struct pd_message {
> > } __packed;
> >
> > /* PDO: Power Data Object */
> > -#define PDO_MAX_OBJECTS 7
> > +
> > +/*
> > + * The EPR (Extended Power Range) structure is a superset of the SPR (Standard Power Range)
> > + * capabilities structure, so set the max number of PDOs to 13 instead of 7. On SPR-only systems,
> > + * objects 8 through 13 will just be empty.
> > + */
> > +#define PDO_MAX_OBJECTS 13
>
> Hmm this might break the recent change I made to UCSI in commit
> 1f4642b72be7 ("usb: typec: ucsi: Retrieve all the PDOs instead of just
> the first 4").
>
> 520 static void ucsi_get_src_pdos(struct ucsi_connector *con, int is_partner)
> 521 {
> 522 int ret;
> 523
> 524 /* UCSI max payload means only getting at most 4 PDOs at a time */
> 525 ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
> 526 if (ret < 0)
> 527 return;
> 528
> 529 con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> 530 if (con->num_pdos < UCSI_MAX_PDOS)
> 531 return;
> 532
> 533 /* get the remaining PDOs, if any */
> 534 ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
> 535 PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
> ^^^^^^^^^^^^^^^
> This routine calls the UCSI GET_PDOS command for up to 4 PDOs at a time
> since that's the most the return payload can carry. Currently this
> assumes that we'd only need to request the PPM at most twice to retrieve
> all the PDOs for up to a maximum of 7 (first request for 4 then again if
> needed for the remaining 3). I'm not sure if any existing UCSI FW would
> be updatable to support more than 7 PDOs in the future, much less
> support EPR. In fact, current UCSI 1.2 spec [1] Table 4-34 mentions PDO
> offset valid values are 0-7 and anything else "shall not be used", so I
> don't know how UCSI will eventually cope with EPR without a spec update.
>

I've had a conversation with Dmitriy Berchanskiy at Intel (the UCSI WG Chair)
about this, and it sounds like the UCSI spec is planned on being revved
(post R2.0) in order to support the additional messages and expanded structures
of USB PD R3.1 around EPR.

> So if this macro changes to 13 then this call would result in a call to
> the UCSI GET_PDOS command passing num_pdos == 13-4 = 9 which would
> probably result in an error from the PPM FW. So we might need to retain
> the maximum value of 7 PDOs at least for UCSI here. Maybe that means
> this UCSI driver needs to carry its own definition of
> UCSI_MAX_TOTAL_PDOS=7 instead of using PDO_MAX_OBJECTS?
>

Prashant mentioned this as well, but maybe it makes sense to define a separate
EPR_PDO_MAX_OBJECTS to handle the EPR case, as there are completely separate
underlying PD messages (EPR_Source_Capabilities) where we expect up to 13
objects, and the classic SPR Source and Sink capabilities will still have the
7 object limit.

Thanks,
Benson

> Jack
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (3.99 kB)
signature.asc (235.00 B)
Download all attachments

2021-09-09 17:38:34

by Jack Pham

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] usb: pd: Increase max PDO objects to 13

Hi Benson,

On Tue, Sep 07, 2021 at 04:28:53PM -0700, Benson Leung wrote:
> Hi Jack,
>
> On Thu, Sep 02, 2021 at 11:47:01PM -0700, Jack Pham wrote:
> > Hi Prashant,
> >
> > On Thu, Sep 02, 2021 at 02:34:58PM -0700, Prashant Malani wrote:
> > > Increase the max number of PDO objects to 13, to accommodate the extra
> > > PDOs added as a part of EPR (Extended Power Range) operation introduced
> > > in the USB PD Spec Rev 3.1, v 1.0. See Figure 6-54 for details.
> > >
> > > Signed-off-by: Prashant Malani <[email protected]>
> > > ---
> > > include/linux/usb/pd.h | 8 +++++++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> > > index 96b7ff66f074..7e8bdca1ce6e 100644
> > > --- a/include/linux/usb/pd.h
> > > +++ b/include/linux/usb/pd.h
> > > @@ -201,7 +201,13 @@ struct pd_message {
> > > } __packed;
> > >
> > > /* PDO: Power Data Object */
> > > -#define PDO_MAX_OBJECTS 7
> > > +
> > > +/*
> > > + * The EPR (Extended Power Range) structure is a superset of the SPR (Standard Power Range)
> > > + * capabilities structure, so set the max number of PDOs to 13 instead of 7. On SPR-only systems,
> > > + * objects 8 through 13 will just be empty.
> > > + */
> > > +#define PDO_MAX_OBJECTS 13
> >
> > Hmm this might break the recent change I made to UCSI in commit
> > 1f4642b72be7 ("usb: typec: ucsi: Retrieve all the PDOs instead of just
> > the first 4").
> >
> > 520 static void ucsi_get_src_pdos(struct ucsi_connector *con, int is_partner)
> > 521 {
> > 522 int ret;
> > 523
> > 524 /* UCSI max payload means only getting at most 4 PDOs at a time */
> > 525 ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
> > 526 if (ret < 0)
> > 527 return;
> > 528
> > 529 con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> > 530 if (con->num_pdos < UCSI_MAX_PDOS)
> > 531 return;
> > 532
> > 533 /* get the remaining PDOs, if any */
> > 534 ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
> > 535 PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
> > ^^^^^^^^^^^^^^^
> > This routine calls the UCSI GET_PDOS command for up to 4 PDOs at a time
> > since that's the most the return payload can carry. Currently this
> > assumes that we'd only need to request the PPM at most twice to retrieve
> > all the PDOs for up to a maximum of 7 (first request for 4 then again if
> > needed for the remaining 3). I'm not sure if any existing UCSI FW would
> > be updatable to support more than 7 PDOs in the future, much less
> > support EPR. In fact, current UCSI 1.2 spec [1] Table 4-34 mentions PDO
> > offset valid values are 0-7 and anything else "shall not be used", so I
> > don't know how UCSI will eventually cope with EPR without a spec update.
> >
>
> I've had a conversation with Dmitriy Berchanskiy at Intel (the UCSI WG Chair)
> about this, and it sounds like the UCSI spec is planned on being revved
> (post R2.0) in order to support the additional messages and expanded structures
> of USB PD R3.1 around EPR.

Good to know! Look forward to seeing it once it's ready.

I've access to the current R2.0 draft as well, and it looks like there's
gonna be a bit of work to update this driver to support it. The big
standout is the data structure format change to accommodate much larger
payloads. So I guess that bridge will be constructed when we get there,
both for 2.0 and later for EPR.

> > So if this macro changes to 13 then this call would result in a call to
> > the UCSI GET_PDOS command passing num_pdos == 13-4 = 9 which would
> > probably result in an error from the PPM FW. So we might need to retain
> > the maximum value of 7 PDOs at least for UCSI here. Maybe that means
> > this UCSI driver needs to carry its own definition of
> > UCSI_MAX_TOTAL_PDOS=7 instead of using PDO_MAX_OBJECTS?
> >
>
> Prashant mentioned this as well, but maybe it makes sense to define a separate
> EPR_PDO_MAX_OBJECTS to handle the EPR case, as there are completely separate
> underlying PD messages (EPR_Source_Capabilities) where we expect up to 13
> objects, and the classic SPR Source and Sink capabilities will still have the
> 7 object limit.

Sounds good to me FWIW. Plus this will even avoid unnecessarily
bloating TCPM's internal {source,sink}_caps and {src,snk}_pdo arrays
(an additional 96 bytes) prematurely before that driver is ready to be
updated to handle EPR with all the new messages and states needed.

Thanks,
Jack
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project