| Message ID | 20230731165926.1815338-1-rdbabiera@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c97cd0b4b54eb42aed7f6c3c295a2d137f6d2416 |
| Headers | show |
| Series | [v2] usb: typec: tcpm: set initial svdm version based on pd revision | expand |
Hello, Just wanted to ping this patch for review in case it got lost/forgotten about. --- Thanks in advance, rd
On Mon, Aug 21, 2023 at 10:09:14AM -0700, RD Babiera wrote: > Hello, > > Just wanted to ping this patch for review in case it got lost/forgotten about. What patch? And note, people were on vacation. If can, to help your patch get to the front of the review queue, please review other typec patches on the mailing list. thanks, greg k-h
On Tue, Aug 22, 2023 at 5:38 AM Greg KH <gregkh@linuxfoundation.org> wrote: > What patch? My previous email was a reply-to for the email containing the patch, at least when I look at the kernel lore it's accounted for with Message-Id <20230731165926.1815338-1-rdbabiera@google.com>. > And note, people were on vacation. If can, to help your patch get to > the front of the review queue, please review other typec patches on the > mailing list. I'll start looking more actively and helping where I can, thanks for the advice. --- thanks, rd
On Mon, Jul 31, 2023 at 04:59:23PM +0000, RD Babiera wrote: > When sending Discover Identity messages to a Port Partner that uses Power > Delivery v2 and SVDM v1, we currently send PD v2 messages with SVDM v2.0, > expecting the port partner to respond with its highest supported SVDM > version as stated in Section 6.4.4.2.3 in the Power Delivery v3 > specification. However, sending SVDM v2 to some Power Delivery v2 port > partners results in a NAK whereas sending SVDM v1 does not. > > NAK messages can be handled by the initiator (PD v3 section 6.4.4.2.5.1), > and one solution could be to resend Discover Identity on a lower SVDM > version if possible. But, Section 6.4.4.3 of PD v2 states that "A NAK > response Should be taken as an indication not to retry that particular > Command." > > Instead, we can set the SVDM version to the maximum one supported by the > negotiated PD revision. When operating in PD v2, this obeys Section > 6.4.4.2.3, which states the SVDM field "Shall be set to zero to indicate > Version 1.0." In PD v3, the SVDM field "Shall be set to 01b to indicate > Version 2.0." > > Fixes: c34e85fa69b9 ("usb: typec: tcpm: Send DISCOVER_IDENTITY from dedicated work") > Cc: stable@vger.kernel.org > Signed-off-by: RD Babiera <rdbabiera@google.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > Changes since v1: > * Fixed styling errors. > --- > drivers/usb/typec/tcpm/tcpm.c | 35 +++++++++++++++++++++++++++++++---- > 1 file changed, 31 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index 829d75ebab42..5024354a0fe0 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -3928,6 +3928,29 @@ static enum typec_cc_status tcpm_pwr_opmode_to_rp(enum typec_pwr_opmode opmode) > } > } > > +static void tcpm_set_initial_svdm_version(struct tcpm_port *port) > +{ > + switch (port->negotiated_rev) { > + case PD_REV30: > + break; > + /* > + * 6.4.4.2.3 Structured VDM Version > + * 2.0 states "At this time, there is only one version (1.0) defined. > + * This field Shall be set to zero to indicate Version 1.0." > + * 3.0 states "This field Shall be set to 01b to indicate Version 2.0." > + * To ensure that we follow the Power Delivery revision we are currently > + * operating on, downgrade the SVDM version to the highest one supported > + * by the Power Delivery revision. > + */ > + case PD_REV20: > + typec_partner_set_svdm_version(port->partner, SVDM_VER_1_0); > + break; > + default: > + typec_partner_set_svdm_version(port->partner, SVDM_VER_1_0); > + break; > + } > +} > + > static void run_state_machine(struct tcpm_port *port) > { > int ret; > @@ -4165,10 +4188,12 @@ static void run_state_machine(struct tcpm_port *port) > * For now, this driver only supports SOP for DISCOVER_IDENTITY, thus using > * port->explicit_contract to decide whether to send the command. > */ > - if (port->explicit_contract) > + if (port->explicit_contract) { > + tcpm_set_initial_svdm_version(port); > mod_send_discover_delayed_work(port, 0); > - else > + } else { > port->send_discover = false; > + } > > /* > * 6.3.5 > @@ -4455,10 +4480,12 @@ static void run_state_machine(struct tcpm_port *port) > * For now, this driver only supports SOP for DISCOVER_IDENTITY, thus using > * port->explicit_contract. > */ > - if (port->explicit_contract) > + if (port->explicit_contract) { > + tcpm_set_initial_svdm_version(port); > mod_send_discover_delayed_work(port, 0); > - else > + } else { > port->send_discover = false; > + } > > power_supply_changed(port->psy); > break; > > base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c > -- > 2.41.0.585.gd2178a4bd4-goog
On Mon, Jul 31, 2023 at 04:59:23PM +0000, RD Babiera wrote: > When sending Discover Identity messages to a Port Partner that uses Power > Delivery v2 and SVDM v1, we currently send PD v2 messages with SVDM v2.0, > expecting the port partner to respond with its highest supported SVDM > version as stated in Section 6.4.4.2.3 in the Power Delivery v3 > specification. However, sending SVDM v2 to some Power Delivery v2 port > partners results in a NAK whereas sending SVDM v1 does not. > > NAK messages can be handled by the initiator (PD v3 section 6.4.4.2.5.1), > and one solution could be to resend Discover Identity on a lower SVDM > version if possible. But, Section 6.4.4.3 of PD v2 states that "A NAK > response Should be taken as an indication not to retry that particular > Command." > > Instead, we can set the SVDM version to the maximum one supported by the > negotiated PD revision. When operating in PD v2, this obeys Section > 6.4.4.2.3, which states the SVDM field "Shall be set to zero to indicate > Version 1.0." In PD v3, the SVDM field "Shall be set to 01b to indicate > Version 2.0." > > Fixes: c34e85fa69b9 ("usb: typec: tcpm: Send DISCOVER_IDENTITY from dedicated work") > Cc: stable@vger.kernel.org > Signed-off-by: RD Babiera <rdbabiera@google.com> > --- > Changes since v1: > * Fixed styling errors. I'll take this now, but again, your email is failing to be authenticated. Perhaps install patatt and sign them that way if google's email infrastructure isn't going to play nice with patches? Grabbing thread from lore.kernel.org/all/20230731165926.1815338-1-rdbabiera@google.com/t.mbox.gz Analyzing 5 messages in the thread Checking attestation on all messages, may take a moment... --- ✗ [PATCH v2] usb: typec: tcpm: set initial svdm version based on pd revision + Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> (✓ DKIM/intel.com) --- ✗ BADSIG: DKIM/google.com --- Total patches: 1 --- Hey, if Intel can do it, surely Google can :) thanks, greg k-h
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 829d75ebab42..5024354a0fe0 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -3928,6 +3928,29 @@ static enum typec_cc_status tcpm_pwr_opmode_to_rp(enum typec_pwr_opmode opmode) } } +static void tcpm_set_initial_svdm_version(struct tcpm_port *port) +{ + switch (port->negotiated_rev) { + case PD_REV30: + break; + /* + * 6.4.4.2.3 Structured VDM Version + * 2.0 states "At this time, there is only one version (1.0) defined. + * This field Shall be set to zero to indicate Version 1.0." + * 3.0 states "This field Shall be set to 01b to indicate Version 2.0." + * To ensure that we follow the Power Delivery revision we are currently + * operating on, downgrade the SVDM version to the highest one supported + * by the Power Delivery revision. + */ + case PD_REV20: + typec_partner_set_svdm_version(port->partner, SVDM_VER_1_0); + break; + default: + typec_partner_set_svdm_version(port->partner, SVDM_VER_1_0); + break; + } +} + static void run_state_machine(struct tcpm_port *port) { int ret; @@ -4165,10 +4188,12 @@ static void run_state_machine(struct tcpm_port *port) * For now, this driver only supports SOP for DISCOVER_IDENTITY, thus using * port->explicit_contract to decide whether to send the command. */ - if (port->explicit_contract) + if (port->explicit_contract) { + tcpm_set_initial_svdm_version(port); mod_send_discover_delayed_work(port, 0); - else + } else { port->send_discover = false; + } /* * 6.3.5 @@ -4455,10 +4480,12 @@ static void run_state_machine(struct tcpm_port *port) * For now, this driver only supports SOP for DISCOVER_IDENTITY, thus using * port->explicit_contract. */ - if (port->explicit_contract) + if (port->explicit_contract) { + tcpm_set_initial_svdm_version(port); mod_send_discover_delayed_work(port, 0); - else + } else { port->send_discover = false; + } power_supply_changed(port->psy); break;
When sending Discover Identity messages to a Port Partner that uses Power Delivery v2 and SVDM v1, we currently send PD v2 messages with SVDM v2.0, expecting the port partner to respond with its highest supported SVDM version as stated in Section 6.4.4.2.3 in the Power Delivery v3 specification. However, sending SVDM v2 to some Power Delivery v2 port partners results in a NAK whereas sending SVDM v1 does not. NAK messages can be handled by the initiator (PD v3 section 6.4.4.2.5.1), and one solution could be to resend Discover Identity on a lower SVDM version if possible. But, Section 6.4.4.3 of PD v2 states that "A NAK response Should be taken as an indication not to retry that particular Command." Instead, we can set the SVDM version to the maximum one supported by the negotiated PD revision. When operating in PD v2, this obeys Section 6.4.4.2.3, which states the SVDM field "Shall be set to zero to indicate Version 1.0." In PD v3, the SVDM field "Shall be set to 01b to indicate Version 2.0." Fixes: c34e85fa69b9 ("usb: typec: tcpm: Send DISCOVER_IDENTITY from dedicated work") Cc: stable@vger.kernel.org Signed-off-by: RD Babiera <rdbabiera@google.com> --- Changes since v1: * Fixed styling errors. --- drivers/usb/typec/tcpm/tcpm.c | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c