Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH] usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported
@ 2026-09-14  9:38 Xu Yang
  2026-09-14  9:51 ` sashiko-bot
  2026-09-18  7:53 ` Heikki Krogerus
  0 siblings, 2 replies; 4+ messages in thread
From: Xu Yang @ 2026-09-14  9:38 UTC (permalink / raw)
  To: badhri, heikki.krogerus, gregkh; +Cc: linux-usb, linux-kernel, imx, jun.li

From: Xu Yang <xu.yang_2@nxp.com>

Commit fa4d0a7cd036 ("usb: typec: tcpm: fix Discover Identity retry loop
on Control Message reject") advances vdm_discovery_state to
VDM_DISCOVERY_COMPLETE when a CMDT_RSP_NAK is received, but it does not
advance the state when a non-modal port returns PD_CTRL_NOT_SUPP.

This can re-arm another vdm_discovery_work if the port is going to
respond with PD_CTRL_NOT_SUPP to the partner.

  tcpm_pd_handle_msg(port, PD_MSG_CTRL_NOT_SUPP, NONE_AMS);
    tcpm_queue_message();
      mod_tcpm_delayed_work();
        tcpm_state_machine_work()
          tcpm_send_queued_message()  -> send PD_MSG_CTRL_NOT_SUPP
          run_state_machine()
            case SRC_READY:
            case SNK_READY:
              mod_vdm_discovery_delayed_work(port, 0);

If both the port and its partner do not support modal operation, they
will send DISCOVER_IDENTITY to each other infinitely.

Advance vdm_discovery_state to VDM_DISCOVERY_COMPLETE in this path as
well, mirroring the existing handling for the SVDM CMDT_RSP_NAK case.

Fixes: fa4d0a7cd036 ("usb: typec: tcpm: fix Discover Identity retry loop on Control Message reject")
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 2d6b14aa2085..cc9ea5d5e78a 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -3891,6 +3891,9 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
 		case SRC_READY:
 		case SNK_READY:
 			if (port->vdm_state > VDM_STATE_READY) {
+				if (port->ams == DISCOVER_IDENTITY)
+					tcpm_update_vdm_discovery_state(port,
+									VDM_DISCOVERY_COMPLETE);
 				port->vdm_state = VDM_STATE_DONE;
 				if (tcpm_vdm_ams(port))
 					tcpm_ams_finish(port);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported
  2026-09-14  9:38 [PATCH] usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported Xu Yang
@ 2026-09-14  9:51 ` sashiko-bot
  2026-09-18  7:53 ` Heikki Krogerus
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-09-14  9:51 UTC (permalink / raw)
  To: Xu Yang; +Cc: imx, Frank.Li

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unconditionally aborting VDM discovery when an active cable (SOP') rejects DISCOVER_IDENTITY, breaking partner alternate modes.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch provides an incomplete fix that leaves DISCOVER_SVIDS and DISCOVER_MODES vulnerable to the same infinite loop it attempts to fix for DISCOVER_IDENTITY.
--

commit 03a3e7fd132280443ce55a6167c00de5bda37072
Author: Xu Yang <xu.yang_2@nxp.com>

usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported

This patch advances vdm_discovery_state to VDM_DISCOVERY_COMPLETE when a
non-modal port returns PD_CTRL_NOT_SUPP to a Discover Identity request.
This prevents an infinite loop of DISCOVER_IDENTITY messages when both the
port and partner do not support modal operation.

> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 2d6b14aa20856..cc9ea5d5e78a1 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
[ ... ]
> @@ -3891,6 +3891,9 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>  		case SRC_READY:
>  		case SNK_READY:
>  			if (port->vdm_state > VDM_STATE_READY) {
> +				if (port->ams == DISCOVER_IDENTITY)
> +					tcpm_update_vdm_discovery_state(port,
> +									VDM_DISCOVERY_COMPLETE);

[Severity: High]
This is a pre-existing issue, but the fix seems incomplete because it
leaves DISCOVER_SVIDS and DISCOVER_MODES vulnerable to the same infinite
loop.

If a partner or cable rejects either of those with PD_CTRL_NOT_SUPP,
tcpm_pd_ctrl_request() finishes the AMS but vdm_discovery_state won't
advance since it is strictly limited to DISCOVER_IDENTITY here.

When the port idles in SRC_READY or SNK_READY, run_state_machine() will
re-arm mod_vdm_discovery_delayed_work(), which will evaluate the
unmodified vdm_discovery_state and resend the rejected command.

Can this create a persistent infinite loop of VDM transmissions triggered by
an external non-compliant device?

[Severity: High]
Does this logic incorrectly assume a rejected DISCOVER_IDENTITY always means
the partner lacks modal support?

When a DISCOVER_IDENTITY VDM is sent to an active cable (TCPC_TX_SOP_PRIME)
and the cable rejects it with PD_CTRL_NOT_SUPP, the rejection is handled
here. By unconditionally setting vdm_discovery_state to
VDM_DISCOVERY_COMPLETE and ignoring the rx_sop_type parameter, does this
abort the entire VDM discovery process, skipping the subsequent discovery of
the partner's SVIDs and alternate modes?

The timeout error path for passive cables transitions to
VDM_DISCOVERY_CABLE_IDENT instead so partner discovery can proceed. Could
this break alternate modes (like DisplayPort) when connected via active
cables lacking modal support?

>  				port->vdm_state = VDM_STATE_DONE;
>  				if (tcpm_vdm_ams(port))
>  					tcpm_ams_finish(port);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914093828.1910201-1-xu.yang_2@oss.nxp.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported
  2026-09-14  9:38 [PATCH] usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported Xu Yang
  2026-09-14  9:51 ` sashiko-bot
@ 2026-09-18  7:53 ` Heikki Krogerus
  2026-09-21  2:11   ` Xu Yang
  1 sibling, 1 reply; 4+ messages in thread
From: Heikki Krogerus @ 2026-09-18  7:53 UTC (permalink / raw)
  To: Xu Yang; +Cc: badhri, gregkh, linux-usb, linux-kernel, imx, jun.li

On Mon, Sep 14, 2026 at 05:38:28PM +0800, Xu Yang wrote:
> From: Xu Yang <xu.yang_2@nxp.com>
> 
> Commit fa4d0a7cd036 ("usb: typec: tcpm: fix Discover Identity retry loop
> on Control Message reject") advances vdm_discovery_state to
> VDM_DISCOVERY_COMPLETE when a CMDT_RSP_NAK is received, but it does not
> advance the state when a non-modal port returns PD_CTRL_NOT_SUPP.
> 
> This can re-arm another vdm_discovery_work if the port is going to
> respond with PD_CTRL_NOT_SUPP to the partner.
> 
>   tcpm_pd_handle_msg(port, PD_MSG_CTRL_NOT_SUPP, NONE_AMS);
>     tcpm_queue_message();
>       mod_tcpm_delayed_work();
>         tcpm_state_machine_work()
>           tcpm_send_queued_message()  -> send PD_MSG_CTRL_NOT_SUPP
>           run_state_machine()
>             case SRC_READY:
>             case SNK_READY:
>               mod_vdm_discovery_delayed_work(port, 0);
> 
> If both the port and its partner do not support modal operation, they
> will send DISCOVER_IDENTITY to each other infinitely.
> 
> Advance vdm_discovery_state to VDM_DISCOVERY_COMPLETE in this path as
> well, mirroring the existing handling for the SVDM CMDT_RSP_NAK case.
> 
> Fixes: fa4d0a7cd036 ("usb: typec: tcpm: fix Discover Identity retry loop on Control Message reject")

I can't find that commit from anywhere?

Thanks,

> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 2d6b14aa2085..cc9ea5d5e78a 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -3891,6 +3891,9 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
>  		case SRC_READY:
>  		case SNK_READY:
>  			if (port->vdm_state > VDM_STATE_READY) {
> +				if (port->ams == DISCOVER_IDENTITY)
> +					tcpm_update_vdm_discovery_state(port,
> +									VDM_DISCOVERY_COMPLETE);
>  				port->vdm_state = VDM_STATE_DONE;
>  				if (tcpm_vdm_ams(port))
>  					tcpm_ams_finish(port);
> -- 
> 2.34.1

-- 
heikki

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported
  2026-09-18  7:53 ` Heikki Krogerus
@ 2026-09-21  2:11   ` Xu Yang
  0 siblings, 0 replies; 4+ messages in thread
From: Xu Yang @ 2026-09-21  2:11 UTC (permalink / raw)
  To: Heikki Krogerus; +Cc: badhri, gregkh, linux-usb, linux-kernel, imx, jun.li

Hi Heikki,

On Fri, Sep 18, 2026 at 09:53:17AM +0200, Heikki Krogerus wrote:
> On Mon, Sep 14, 2026 at 05:38:28PM +0800, Xu Yang wrote:
> > From: Xu Yang <xu.yang_2@nxp.com>
> > 
> > Commit fa4d0a7cd036 ("usb: typec: tcpm: fix Discover Identity retry loop
> > on Control Message reject") advances vdm_discovery_state to
> > VDM_DISCOVERY_COMPLETE when a CMDT_RSP_NAK is received, but it does not
> > advance the state when a non-modal port returns PD_CTRL_NOT_SUPP.
> > 
> > This can re-arm another vdm_discovery_work if the port is going to
> > respond with PD_CTRL_NOT_SUPP to the partner.
> > 
> >   tcpm_pd_handle_msg(port, PD_MSG_CTRL_NOT_SUPP, NONE_AMS);
> >     tcpm_queue_message();
> >       mod_tcpm_delayed_work();
> >         tcpm_state_machine_work()
> >           tcpm_send_queued_message()  -> send PD_MSG_CTRL_NOT_SUPP
> >           run_state_machine()
> >             case SRC_READY:
> >             case SNK_READY:
> >               mod_vdm_discovery_delayed_work(port, 0);
> > 
> > If both the port and its partner do not support modal operation, they
> > will send DISCOVER_IDENTITY to each other infinitely.
> > 
> > Advance vdm_discovery_state to VDM_DISCOVERY_COMPLETE in this path as
> > well, mirroring the existing handling for the SVDM CMDT_RSP_NAK case.
> > 
> > Fixes: fa4d0a7cd036 ("usb: typec: tcpm: fix Discover Identity retry loop on Control Message reject")
> 
> I can't find that commit from anywhere?

It's a mistake, and I sent v2 to correct it:
https://lore.kernel.org/linux-usb/20260916043957.3362995-1-xu.yang_2@oss.nxp.com/

Thanks,
Xu Yang

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-21  2:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14  9:38 [PATCH] usb: typec: tcpm: advance vdm_discovery_state on Discover Identity Not_Supported Xu Yang
2026-09-14  9:51 ` sashiko-bot
2026-09-18  7:53 ` Heikki Krogerus
2026-09-21  2:11   ` Xu Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox