On Tue, Oct 26, 2021 at 06:00:35PM -0700, Jack Pham wrote:
> On Tue, Oct 26, 2021 at 05:33:51PM +0300, Heikki Krogerus wrote:
> >
> > -static int ucsi_get_src_pdos(struct ucsi_connector *con)
> > +int ucsi_read_pdos(struct ucsi_connector *con, int partner, int source, u32 *pdos)
> > {
> > + u32 pdo[PDO_MAX_OBJECTS];
> > + int num_pdos;
> > int ret;
> >
> > /* UCSI max payload means only getting at most 4 PDOs at a time */
> > - ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
> > + ret = ucsi_get_pdos(con, partner, source, pdo, 0, UCSI_MAX_PDOS);
> > if (ret < 0)
> > return ret;
> >
> > - con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> > - if (con->num_pdos < UCSI_MAX_PDOS)
> > - return 0;
> > + num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> > + if (num_pdos < UCSI_MAX_PDOS)
> > + goto done;
> >
> > /* get the remaining PDOs, if any */
> > - ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
> > + ret = ucsi_get_pdos(con, partner, source, pdo, UCSI_MAX_PDOS,
> > PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
> > if (ret < 0)
> > return ret;
> >
> > - con->num_pdos += ret / sizeof(u32);
> > + num_pdos += ret / sizeof(u32);
> > +done:
> > + memcpy(pdos, pdo, num_pdos * sizeof(pdo));
> > +
> > + return num_pdos;
> > +}
> > +
> > +static int ucsi_get_src_pdos(struct ucsi_connector *con)
> > +{
> > + int ret;
> > +
> > + ret = ucsi_read_pdos(con, 0, 1, con->src_pdos);
>
> Second parameter should be 1 right? Original intent of get_src_pdos()
> is to retrieve the partner's source capabilities in order to populate
> the power_supply. Passing 0 as the partner param here changes the
> behavior to retrieve the source PDOs of the port.
Sounds like there is a bug in the existing code. I'm not changing the
behviour in this patch.
> (BTW I'm going to send a quick patch for this to since this assumes that
> port is sink and partner is source; when it's the other way around we
> end up calling GET_PDOS on the sink partner when it might not even be
> source capable).
I'll check it...
thanks,
--
heikki
