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 <pmalani@xxxxxxxxxxxx>
> > ---
> > 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
