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@xxxxxxxxxxxxxxx
> Signed-off-by: RD Babiera <rdbabiera@xxxxxxxxxx>
Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> 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
--
heikki
