On Tue, May 13, 2025 at 04:39:09PM +0300, Heikki Krogerus wrote:
> On Tue, May 13, 2025 at 09:08:34PM +0800, Cosmo Chou wrote:
> > Initialize negotiated_rev and negotiated_rev_prime based on the port's
> > configured PD revision (rev_major) rather than always defaulting to
> > PD_MAX_REV. This ensures ports start PD communication using their
> > appropriate revision level.
> >
> > This allows proper communication with devices that require specific
> > PD revision levels, especially for the hardware designed for PD 1.0
> > or 2.0 specifications.
> >
> > Signed-off-by: Cosmo Chou <chou.cosmo@xxxxxxxxx>
> > ---
> > Change log:
> >
> > v2:
> > - Add PD_CAP_REVXX macros and use switch-case for better readability.
> >
> > ---
> > drivers/usb/typec/tcpm/tcpm.c | 29 +++++++++++++++++++++++++----
> > 1 file changed, 25 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 8adf6f954633..48e9cfc2b49a 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -313,6 +313,10 @@ struct pd_data {
> > unsigned int operating_snk_mw;
> > };
> >
> > +#define PD_CAP_REV10 0x1
> > +#define PD_CAP_REV20 0x2
> > +#define PD_CAP_REV30 0x3
> > +
> > struct pd_revision_info {
> > u8 rev_major;
> > u8 rev_minor;
> > @@ -4665,6 +4669,25 @@ static void tcpm_set_initial_svdm_version(struct tcpm_port *port)
> > }
> > }
> >
> > +static void tcpm_set_initial_negotiated_rev(struct tcpm_port *port)
> > +{
> > + switch (port->pd_rev.rev_major) {
> > + case PD_CAP_REV10:
> > + port->negotiated_rev = PD_REV10;
> > + break;
> > + case PD_CAP_REV20:
> > + port->negotiated_rev = PD_REV20;
> > + break;
> > + case PD_CAP_REV30:
> > + port->negotiated_rev = PD_REV30;
> > + break;
> > + default:
> > + port->negotiated_rev = PD_MAX_REV;
> > + break;
> > + }
> > + port->negotiated_rev_prime = port->negotiated_rev;
> > +}
>
> Do we need this? Couldn't you just add one to rev_major?
>
> port->negotiated_rev = port->pd_rev.rev_major + 1;
> port->negotiated_rev_prime = port->pd_rev.rev_major + 1;
>
> Or am I missing something?
Sorry, I mean minus one :-)
port->negotiated_rev = port->pd_rev.rev_major - 1;
port->negotiated_rev_prime = port->pd_rev.rev_major - 1;
--
heikki
