[PATCH] usb: typec: tcpm: Complete discovery when the partner lacks SVDM support

[PATCH] usb: typec: tcpm: Complete discovery when the partner lacks SVDM support

[Alexey Charkov]

A PD 3.0 port that does not support Structured VDMs answers Discover
Identity with Not_Supported, as TCPM itself does when no sink-vdos are
configured. TCPM treats a NAK to Discover Identity or Discover SVIDs as
the end of discovery, but on Not_Supported it only ends the VDM exchange
and leaves discovery pending. Every pass through the ready state then
restarts it, and the state machine passes through it after each reply
it sends.

Two such ports therefore keep each other busy: each answers the other's
Discover Identity with Not_Supported, which restarts its own discovery,
which draws a Not_Supported in turn. Two RK3576 boards connected to each
other exchange about 25 Discover Identity requests per second each way,
indefinitely, crowding out other PD traffic.

Mark discovery complete when the partner answers Discover Identity or
Discover SVIDs with Not_Supported, as on a NAK. Leave a Not_Supported
from the cable plug alone, so that it cannot cut short discovery of the
partner.

Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 drivers/usb/typec/tcpm/tcpm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index d721baef5210..e21fe544b2d4 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -3908,6 +3908,17 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
 		case SNK_READY:
 			if (port->vdm_state > VDM_STATE_READY) {
 				port->vdm_state = VDM_STATE_DONE;
+				/*
+				 * A partner without Structured VDM support answers
+				 * discovery with Not_Supported: there is nothing left
+				 * to discover, just as after a NAK.
+				 */
+				if (type == PD_CTRL_NOT_SUPP &&
+				    rx_sop_type == TCPC_TX_SOP &&
+				    (port->ams == DISCOVER_IDENTITY ||
+				     port->ams == DISCOVER_SVIDS))
+					tcpm_update_vdm_discovery_state(port,
+									VDM_DISCOVERY_COMPLETE);
 				if (tcpm_vdm_ams(port))
 					tcpm_ams_finish(port);
 				mod_vdm_delayed_work(port, 0);

---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
change-id: 20261102-tcpm-discovery-no-svdm-b8aa1f5768dd

Best regards,
--  
Alexey Charkov <alchark@flipper.net>

Re: [PATCH] usb: typec: tcpm: Complete discovery when the partner lacks SVDM support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: typec: tcpm: SOP' Not_Supported in tcpm_pd_ctrl_request() halts discovery
--

commit f5763c417e2367d11a45635b9af7ac9ca073d24a
Author: Alexey Charkov <alchark@flipper.net>

usb: typec: tcpm: Complete discovery when the partner lacks SVDM support

This commit marks VDM discovery complete when a port partner responds to
Discover Identity or Discover SVIDs over SOP with PD_CTRL_NOT_SUPP,
avoiding an infinite loop of discovery requests.

> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index d721baef52104..e21fe544b2d4b 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -3908,6 +3908,17 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>  		case SNK_READY:
>  			if (port->vdm_state > VDM_STATE_READY) {
>  				port->vdm_state = VDM_STATE_DONE;
> +				/*
> +				 * A partner without Structured VDM support answers
> +				 * discovery with Not_Supported: there is nothing left
> +				 * to discover, just as after a NAK.
> +				 */
> +				if (type == PD_CTRL_NOT_SUPP &&
> +				    rx_sop_type == TCPC_TX_SOP &&
> +				    (port->ams == DISCOVER_IDENTITY ||
> +				     port->ams == DISCOVER_SVIDS))
> +					tcpm_update_vdm_discovery_state(port,
> +									VDM_DISCOVERY_COMPLETE);

[Severity: Medium]
The commit message notes that a Not_Supported reply from the cable plug is
left alone so that it cannot cut short discovery of the partner. However, if a
cable plug replies with PD_CTRL_NOT_SUPP (rx_sop_type == TCPC_TX_SOP_PRIME) in
tcpm_pd_ctrl_request(), doesn't leaving port->vdm_discovery_state unchanged
still stall partner discovery and repeatedly re-send the SOP' request?

Specifically, after the partner ACKs SOP CMD_DISCOVER_IDENT, tcpm_pd_svdm()
sets port->vdm_discovery_state to VDM_DISCOVERY_PARTNER_IDENT and probes SOP'
before discovering the partner's SVIDs:

drivers/usb/typec/tcpm/tcpm.c:tcpm_pd_svdm() {
    ...
	tcpm_update_vdm_discovery_state(port, VDM_DISCOVERY_PARTNER_IDENT);
    ...
	if (IS_ERR_OR_NULL(port->cable) &&
	    tcpm_can_communicate_sop_prime(port)) {
		*response_tx_sop_type = TCPC_TX_SOP_PRIME;
		response[0] = VDO(USB_SID_PD, 1,
				  typec_get_negotiated_svdm_version(typec),
				  CMD_DISCOVER_IDENT);
		rlen = 1;
	} else {
		*response_tx_sop_type = TCPC_TX_SOP;
		response[0] = VDO(USB_SID_PD, 1,
				  typec_get_negotiated_svdm_version(typec),
				  CMD_DISCOVER_SVID);
		rlen = 1;
	}
}

If the cable plug replies to SOP' CMD_DISCOVER_IDENT with PD_CTRL_NOT_SUPP,
tcpm_pd_ctrl_request() finishes the AMS without advancing
port->vdm_discovery_state or queueing SOP CMD_DISCOVER_SVID. Because
port->cable is still NULL, every subsequent run of tcpm_vdm_discovery_work()
will re-send SOP' CMD_DISCOVER_IDENT instead of moving on to partner SVID
discovery:

drivers/usb/typec/tcpm/tcpm.c:tcpm_vdm_discovery_work() {
    ...
	case VDM_DISCOVERY_PARTNER_IDENT:
		if (tcpm_can_communicate_sop_prime(port) && !port->cable) {
			tx_sop_type = TCPC_TX_SOP_PRIME;
			msg[0] = VDO(USB_SID_PD, 1, svdm_version, CMD_DISCOVER_IDENT);
		} else {
			if (tcpm_can_communicate_sop_prime(port))
				tcpm_update_vdm_discovery_state(port, VDM_DISCOVERY_CABLE_IDENT);
			msg[0] = VDO(USB_SID_PD, 1, svdm_version, CMD_DISCOVER_SVID);
		}
		break;
}

Similarly, when tcpm_handle_discover_mode() finishes discovering partner modes
and sends SOP' CMD_DISCOVER_SVID in VDM_DISCOVERY_PARTNER_MODES, a
PD_CTRL_NOT_SUPP on SOP' leaves port->vdm_discovery_state in
VDM_DISCOVERY_PARTNER_MODES without calling tcpm_register_partner_altmodes(),
causing tcpm_vdm_discovery_work() to repeatedly re-send SOP'
CMD_DISCOVER_SVID.

Should a PD_CTRL_NOT_SUPP on TCPC_TX_SOP_PRIME fall back to SOP discovery in
the same way vdm_run_state_machine() handles SOP' transmission failures in
VDM_STATE_ERR_SEND (advancing to VDM_DISCOVERY_CABLE_IDENT and queueing SOP
CMD_DISCOVER_SVID on DISCOVER_IDENTITY, or calling
tcpm_register_partner_altmodes() and marking VDM_DISCOVERY_COMPLETE on
DISCOVER_SVIDS and DISCOVER_MODES)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-tcpm-discovery-no-svdm-v1-1-54bb9b613065@flipper.net?part=1

Re: [PATCH] usb: typec: tcpm: Complete discovery when the partner lacks SVDM support

[Alexey Charkov]

On Mon, Nov 2, 2026 at 6:18 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] usb: typec: tcpm: SOP' Not_Supported in tcpm_pd_ctrl_request() halts discovery
> --
>
> commit f5763c417e2367d11a45635b9af7ac9ca073d24a
> Author: Alexey Charkov <alchark@flipper.net>
>
> usb: typec: tcpm: Complete discovery when the partner lacks SVDM support
>
> This commit marks VDM discovery complete when a port partner responds to
> Discover Identity or Discover SVIDs over SOP with PD_CTRL_NOT_SUPP,
> avoiding an infinite loop of discovery requests.
>
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index d721baef52104..e21fe544b2d4b 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -3908,6 +3908,17 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
> >               case SNK_READY:
> >                       if (port->vdm_state > VDM_STATE_READY) {
> >                               port->vdm_state = VDM_STATE_DONE;
> > +                             /*
> > +                              * A partner without Structured VDM support answers
> > +                              * discovery with Not_Supported: there is nothing left
> > +                              * to discover, just as after a NAK.
> > +                              */
> > +                             if (type == PD_CTRL_NOT_SUPP &&
> > +                                 rx_sop_type == TCPC_TX_SOP &&
> > +                                 (port->ams == DISCOVER_IDENTITY ||
> > +                                  port->ams == DISCOVER_SVIDS))
> > +                                     tcpm_update_vdm_discovery_state(port,
> > +                                                                     VDM_DISCOVERY_COMPLETE);
>
> [Severity: Medium]
> The commit message notes that a Not_Supported reply from the cable plug is
> left alone so that it cannot cut short discovery of the partner. However, if a
> cable plug replies with PD_CTRL_NOT_SUPP (rx_sop_type == TCPC_TX_SOP_PRIME) in
> tcpm_pd_ctrl_request(), doesn't leaving port->vdm_discovery_state unchanged
> still stall partner discovery and repeatedly re-send the SOP' request?
>
> Specifically, after the partner ACKs SOP CMD_DISCOVER_IDENT, tcpm_pd_svdm()
> sets port->vdm_discovery_state to VDM_DISCOVERY_PARTNER_IDENT and probes SOP'
> before discovering the partner's SVIDs:
>
> drivers/usb/typec/tcpm/tcpm.c:tcpm_pd_svdm() {
>     ...
>         tcpm_update_vdm_discovery_state(port, VDM_DISCOVERY_PARTNER_IDENT);
>     ...
>         if (IS_ERR_OR_NULL(port->cable) &&
>             tcpm_can_communicate_sop_prime(port)) {
>                 *response_tx_sop_type = TCPC_TX_SOP_PRIME;
>                 response[0] = VDO(USB_SID_PD, 1,
>                                   typec_get_negotiated_svdm_version(typec),
>                                   CMD_DISCOVER_IDENT);
>                 rlen = 1;
>         } else {
>                 *response_tx_sop_type = TCPC_TX_SOP;
>                 response[0] = VDO(USB_SID_PD, 1,
>                                   typec_get_negotiated_svdm_version(typec),
>                                   CMD_DISCOVER_SVID);
>                 rlen = 1;
>         }
> }
>
> If the cable plug replies to SOP' CMD_DISCOVER_IDENT with PD_CTRL_NOT_SUPP,
> tcpm_pd_ctrl_request() finishes the AMS without advancing
> port->vdm_discovery_state or queueing SOP CMD_DISCOVER_SVID. Because
> port->cable is still NULL, every subsequent run of tcpm_vdm_discovery_work()
> will re-send SOP' CMD_DISCOVER_IDENT instead of moving on to partner SVID
> discovery:
>
> drivers/usb/typec/tcpm/tcpm.c:tcpm_vdm_discovery_work() {
>     ...
>         case VDM_DISCOVERY_PARTNER_IDENT:
>                 if (tcpm_can_communicate_sop_prime(port) && !port->cable) {
>                         tx_sop_type = TCPC_TX_SOP_PRIME;
>                         msg[0] = VDO(USB_SID_PD, 1, svdm_version, CMD_DISCOVER_IDENT);
>                 } else {
>                         if (tcpm_can_communicate_sop_prime(port))
>                                 tcpm_update_vdm_discovery_state(port, VDM_DISCOVERY_CABLE_IDENT);
>                         msg[0] = VDO(USB_SID_PD, 1, svdm_version, CMD_DISCOVER_SVID);
>                 }
>                 break;
> }
>
> Similarly, when tcpm_handle_discover_mode() finishes discovering partner modes
> and sends SOP' CMD_DISCOVER_SVID in VDM_DISCOVERY_PARTNER_MODES, a
> PD_CTRL_NOT_SUPP on SOP' leaves port->vdm_discovery_state in
> VDM_DISCOVERY_PARTNER_MODES without calling tcpm_register_partner_altmodes(),
> causing tcpm_vdm_discovery_work() to repeatedly re-send SOP'
> CMD_DISCOVER_SVID.
>
> Should a PD_CTRL_NOT_SUPP on TCPC_TX_SOP_PRIME fall back to SOP discovery in
> the same way vdm_run_state_machine() handles SOP' transmission failures in
> VDM_STATE_ERR_SEND (advancing to VDM_DISCOVERY_CABLE_IDENT and queueing SOP
> CMD_DISCOVER_SVID on DISCOVER_IDENTITY, or calling
> tcpm_register_partner_altmodes() and marking VDM_DISCOVERY_COMPLETE on
> DISCOVER_SVIDS and DISCOVER_MODES)?

True. I've simulated that with two TCPM-managed devices on two ends of
a dumb Type-C cable, with one pretending to be an e-marker in addition
to a partner, and this patch does indeed put them in a SOP'
CMD_DISCOVER_SVID loop. This takes a non-compliant e-marker though.

I'll add a precursor patch to address that at the class level, and
send it as a mini-series after waiting for any other feedback on this
patch.

Best regards,
Alexey