public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] usb: typec: tcpm: improve handling of DISCOVER_MODES failures
@ 2026-03-03 16:29 Sebastian Reichel
  2026-03-05 22:11 ` Badhri Jagan Sridharan
  2026-03-09 12:34 ` Heikki Krogerus
  0 siblings, 2 replies; 3+ messages in thread
From: Sebastian Reichel @ 2026-03-03 16:29 UTC (permalink / raw)
  To: Badhri Jagan Sridharan, Heikki Krogerus, Greg Kroah-Hartman,
	RD Babiera
  Cc: linux-usb, linux-kernel, kernel, stable, Sebastian Reichel

UGREEN USB-C Multifunction Adapter Model CM512 (AKA "Revodok 107")
exposes two SVIDs: 0xff01 (DP Alt Mode) and 0x1d5c. The DISCOVER_MODES
step succeeds for 0xff01 and gets a NAK for 0x1d5c. Currently this
results in DP Alt Mode not being registered either, since the modes
are only registered once all of them have been discovered. The NAK
results in the processing being stopped and thus no Alt modes being
registered.

Improve the situation by handling the NAK gracefully and continue
processing the other modes.

Before this change, the TCPM log ends like this:

(more log entries before this)
[    5.028287] AMS DISCOVER_SVIDS finished
[    5.028291] cc:=4
[    5.040040] SVID 1: 0xff01
[    5.040054] SVID 2: 0x1d5c
[    5.040082] AMS DISCOVER_MODES start
[    5.040096] PD TX, header: 0x1b6f
[    5.050946] PD TX complete, status: 0
[    5.059609] PD RX, header: 0x264f [1]
[    5.059626] Rx VDM cmd 0xff018043 type 1 cmd 3 len 2
[    5.059640] AMS DISCOVER_MODES finished
[    5.059644] cc:=4
[    5.069994]  Alternate mode 0: SVID 0xff01, VDO 1: 0x000c0045
[    5.070029] AMS DISCOVER_MODES start
[    5.070043] PD TX, header: 0x1d6f
[    5.081139] PD TX complete, status: 0
[    5.087498] PD RX, header: 0x184f [1]
[    5.087515] Rx VDM cmd 0x1d5c8083 type 2 cmd 3 len 1
[    5.087529] AMS DISCOVER_MODES finished
[    5.087534] cc:=4
(no further log entries after this point)

After this patch the TCPM log looks exactly the same, but then
continues like this:

[    5.100222] Skip SVID 0x1d5c (failed to discover mode)
[    5.101699] AMS DFP_TO_UFP_ENTER_MODE start
(log goes on as the system initializes DP AltMode)

Cc: stable@vger.kernel.org
Fixes: 41d9d75344d9 ("usb: typec: tcpm: add discover svids and discover modes support for sop'")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
Changes in v2:
- Link to v1: https://lore.kernel.org/r/20260213-tcpm-discover-modes-nak-fix-v1-0-9bcb5adb4ef6@collabora.com
- Squash patches (Badhri Jagan Sridharan)
- Add Fixes tag (Badhri Jagan Sridharan)
- Move common svdm_consume_modes out of conditional statement (Badhri Jagan Sridharan)
- Add TCPM log to commit message (Badhri Jagan Sridharan)
---
 drivers/usb/typec/tcpm/tcpm.c | 97 +++++++++++++++++++++++++++----------------
 1 file changed, 61 insertions(+), 36 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 1d2f3af034c5..cd5260f408fb 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1995,6 +1995,55 @@ static bool tcpm_cable_vdm_supported(struct tcpm_port *port)
 	       tcpm_can_communicate_sop_prime(port);
 }
 
+static void tcpm_handle_discover_mode(struct tcpm_port *port,
+				      const u32 *p, int cnt, u32 *response,
+				      enum tcpm_transmit_type rx_sop_type,
+				      enum tcpm_transmit_type *response_tx_sop_type,
+				      struct pd_mode_data *modep,
+				      struct pd_mode_data *modep_prime,
+				      int svdm_version, int *rlen,
+				      bool success)
+
+{
+	struct typec_port *typec = port->typec_port;
+
+	/* 6.4.4.3.3 */
+	if (success)
+		svdm_consume_modes(port, p, cnt, rx_sop_type);
+
+	if (rx_sop_type == TCPC_TX_SOP) {
+		modep->svid_index++;
+		if (modep->svid_index < modep->nsvids) {
+			u16 svid = modep->svids[modep->svid_index];
+			*response_tx_sop_type = TCPC_TX_SOP;
+			response[0] = VDO(svid, 1, svdm_version,
+					  CMD_DISCOVER_MODES);
+			*rlen = 1;
+		} else if (tcpm_cable_vdm_supported(port)) {
+			*response_tx_sop_type = TCPC_TX_SOP_PRIME;
+			response[0] = VDO(USB_SID_PD, 1,
+					  typec_get_cable_svdm_version(typec),
+					  CMD_DISCOVER_SVID);
+			*rlen = 1;
+		} else {
+			tcpm_register_partner_altmodes(port);
+		}
+	} else if (rx_sop_type == TCPC_TX_SOP_PRIME) {
+		modep_prime->svid_index++;
+		if (modep_prime->svid_index < modep_prime->nsvids) {
+			u16 svid = modep_prime->svids[modep_prime->svid_index];
+			*response_tx_sop_type = TCPC_TX_SOP_PRIME;
+			response[0] = VDO(svid, 1,
+					  typec_get_cable_svdm_version(typec),
+					  CMD_DISCOVER_MODES);
+			*rlen = 1;
+		} else {
+			tcpm_register_plug_altmodes(port);
+			tcpm_register_partner_altmodes(port);
+		}
+	}
+}
+
 static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			const u32 *p, int cnt, u32 *response,
 			enum adev_actions *adev_action,
@@ -2252,41 +2301,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			}
 			break;
 		case CMD_DISCOVER_MODES:
-			if (rx_sop_type == TCPC_TX_SOP) {
-				/* 6.4.4.3.3 */
-				svdm_consume_modes(port, p, cnt, rx_sop_type);
-				modep->svid_index++;
-				if (modep->svid_index < modep->nsvids) {
-					u16 svid = modep->svids[modep->svid_index];
-					*response_tx_sop_type = TCPC_TX_SOP;
-					response[0] = VDO(svid, 1, svdm_version,
-							  CMD_DISCOVER_MODES);
-					rlen = 1;
-				} else if (tcpm_cable_vdm_supported(port)) {
-					*response_tx_sop_type = TCPC_TX_SOP_PRIME;
-					response[0] = VDO(USB_SID_PD, 1,
-							  typec_get_cable_svdm_version(typec),
-							  CMD_DISCOVER_SVID);
-					rlen = 1;
-				} else {
-					tcpm_register_partner_altmodes(port);
-				}
-			} else if (rx_sop_type == TCPC_TX_SOP_PRIME) {
-				/* 6.4.4.3.3 */
-				svdm_consume_modes(port, p, cnt, rx_sop_type);
-				modep_prime->svid_index++;
-				if (modep_prime->svid_index < modep_prime->nsvids) {
-					u16 svid = modep_prime->svids[modep_prime->svid_index];
-					*response_tx_sop_type = TCPC_TX_SOP_PRIME;
-					response[0] = VDO(svid, 1,
-							  typec_get_cable_svdm_version(typec),
-							  CMD_DISCOVER_MODES);
-					rlen = 1;
-				} else {
-					tcpm_register_plug_altmodes(port);
-					tcpm_register_partner_altmodes(port);
-				}
-			}
+			tcpm_handle_discover_mode(port, p, cnt, response,
+						  rx_sop_type, response_tx_sop_type,
+						  modep, modep_prime, svdm_version,
+						  &rlen, true);
 			break;
 		case CMD_ENTER_MODE:
 			*response_tx_sop_type = rx_sop_type;
@@ -2329,9 +2347,16 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 		switch (cmd) {
 		case CMD_DISCOVER_IDENT:
 		case CMD_DISCOVER_SVID:
-		case CMD_DISCOVER_MODES:
 		case VDO_CMD_VENDOR(0) ... VDO_CMD_VENDOR(15):
 			break;
+		case CMD_DISCOVER_MODES:
+			tcpm_log(port, "Skip SVID 0x%04x (failed to discover mode)",
+				 PD_VDO_SVID_SVID0(p[0]));
+			tcpm_handle_discover_mode(port, p, cnt, response,
+						  rx_sop_type, response_tx_sop_type,
+						  modep, modep_prime, svdm_version,
+						  &rlen, false);
+			break;
 		case CMD_ENTER_MODE:
 			/* Back to USB Operation */
 			*adev_action = ADEV_NOTIFY_USB_AND_QUEUE_VDM;

---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260213-tcpm-discover-modes-nak-fix-09070bb529c5

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>


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

* Re: [PATCH v2] usb: typec: tcpm: improve handling of DISCOVER_MODES failures
  2026-03-03 16:29 [PATCH v2] usb: typec: tcpm: improve handling of DISCOVER_MODES failures Sebastian Reichel
@ 2026-03-05 22:11 ` Badhri Jagan Sridharan
  2026-03-09 12:34 ` Heikki Krogerus
  1 sibling, 0 replies; 3+ messages in thread
From: Badhri Jagan Sridharan @ 2026-03-05 22:11 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Heikki Krogerus, Greg Kroah-Hartman, RD Babiera, linux-usb,
	linux-kernel, kernel, stable

On Tue, Mar 3, 2026 at 8:29 AM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> UGREEN USB-C Multifunction Adapter Model CM512 (AKA "Revodok 107")
> exposes two SVIDs: 0xff01 (DP Alt Mode) and 0x1d5c. The DISCOVER_MODES
> step succeeds for 0xff01 and gets a NAK for 0x1d5c. Currently this
> results in DP Alt Mode not being registered either, since the modes
> are only registered once all of them have been discovered. The NAK
> results in the processing being stopped and thus no Alt modes being
> registered.
>
> Improve the situation by handling the NAK gracefully and continue
> processing the other modes.
>
> Before this change, the TCPM log ends like this:
>
> (more log entries before this)
> [    5.028287] AMS DISCOVER_SVIDS finished
> [    5.028291] cc:=4
> [    5.040040] SVID 1: 0xff01
> [    5.040054] SVID 2: 0x1d5c
> [    5.040082] AMS DISCOVER_MODES start
> [    5.040096] PD TX, header: 0x1b6f
> [    5.050946] PD TX complete, status: 0
> [    5.059609] PD RX, header: 0x264f [1]
> [    5.059626] Rx VDM cmd 0xff018043 type 1 cmd 3 len 2
> [    5.059640] AMS DISCOVER_MODES finished
> [    5.059644] cc:=4
> [    5.069994]  Alternate mode 0: SVID 0xff01, VDO 1: 0x000c0045
> [    5.070029] AMS DISCOVER_MODES start
> [    5.070043] PD TX, header: 0x1d6f
> [    5.081139] PD TX complete, status: 0
> [    5.087498] PD RX, header: 0x184f [1]
> [    5.087515] Rx VDM cmd 0x1d5c8083 type 2 cmd 3 len 1
> [    5.087529] AMS DISCOVER_MODES finished
> [    5.087534] cc:=4
> (no further log entries after this point)
>
> After this patch the TCPM log looks exactly the same, but then
> continues like this:
>
> [    5.100222] Skip SVID 0x1d5c (failed to discover mode)
> [    5.101699] AMS DFP_TO_UFP_ENTER_MODE start
> (log goes on as the system initializes DP AltMode)
>
> Cc: stable@vger.kernel.org
> Fixes: 41d9d75344d9 ("usb: typec: tcpm: add discover svids and discover modes support for sop'")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>


Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>

>
> ---
> Changes in v2:
> - Link to v1: https://lore.kernel.org/r/20260213-tcpm-discover-modes-nak-fix-v1-0-9bcb5adb4ef6@collabora.com
> - Squash patches (Badhri Jagan Sridharan)
> - Add Fixes tag (Badhri Jagan Sridharan)
> - Move common svdm_consume_modes out of conditional statement (Badhri Jagan Sridharan)
> - Add TCPM log to commit message (Badhri Jagan Sridharan)
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 97 +++++++++++++++++++++++++++----------------
>  1 file changed, 61 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 1d2f3af034c5..cd5260f408fb 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -1995,6 +1995,55 @@ static bool tcpm_cable_vdm_supported(struct tcpm_port *port)
>                tcpm_can_communicate_sop_prime(port);
>  }
>
> +static void tcpm_handle_discover_mode(struct tcpm_port *port,
> +                                     const u32 *p, int cnt, u32 *response,
> +                                     enum tcpm_transmit_type rx_sop_type,
> +                                     enum tcpm_transmit_type *response_tx_sop_type,
> +                                     struct pd_mode_data *modep,
> +                                     struct pd_mode_data *modep_prime,
> +                                     int svdm_version, int *rlen,
> +                                     bool success)
> +
> +{
> +       struct typec_port *typec = port->typec_port;
> +
> +       /* 6.4.4.3.3 */
> +       if (success)
> +               svdm_consume_modes(port, p, cnt, rx_sop_type);
> +
> +       if (rx_sop_type == TCPC_TX_SOP) {
> +               modep->svid_index++;
> +               if (modep->svid_index < modep->nsvids) {
> +                       u16 svid = modep->svids[modep->svid_index];
> +                       *response_tx_sop_type = TCPC_TX_SOP;
> +                       response[0] = VDO(svid, 1, svdm_version,
> +                                         CMD_DISCOVER_MODES);
> +                       *rlen = 1;
> +               } else if (tcpm_cable_vdm_supported(port)) {
> +                       *response_tx_sop_type = TCPC_TX_SOP_PRIME;
> +                       response[0] = VDO(USB_SID_PD, 1,
> +                                         typec_get_cable_svdm_version(typec),
> +                                         CMD_DISCOVER_SVID);
> +                       *rlen = 1;
> +               } else {
> +                       tcpm_register_partner_altmodes(port);
> +               }
> +       } else if (rx_sop_type == TCPC_TX_SOP_PRIME) {
> +               modep_prime->svid_index++;
> +               if (modep_prime->svid_index < modep_prime->nsvids) {
> +                       u16 svid = modep_prime->svids[modep_prime->svid_index];
> +                       *response_tx_sop_type = TCPC_TX_SOP_PRIME;
> +                       response[0] = VDO(svid, 1,
> +                                         typec_get_cable_svdm_version(typec),
> +                                         CMD_DISCOVER_MODES);
> +                       *rlen = 1;
> +               } else {
> +                       tcpm_register_plug_altmodes(port);
> +                       tcpm_register_partner_altmodes(port);
> +               }
> +       }
> +}
> +
>  static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>                         const u32 *p, int cnt, u32 *response,
>                         enum adev_actions *adev_action,
> @@ -2252,41 +2301,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>                         }
>                         break;
>                 case CMD_DISCOVER_MODES:
> -                       if (rx_sop_type == TCPC_TX_SOP) {
> -                               /* 6.4.4.3.3 */
> -                               svdm_consume_modes(port, p, cnt, rx_sop_type);
> -                               modep->svid_index++;
> -                               if (modep->svid_index < modep->nsvids) {
> -                                       u16 svid = modep->svids[modep->svid_index];
> -                                       *response_tx_sop_type = TCPC_TX_SOP;
> -                                       response[0] = VDO(svid, 1, svdm_version,
> -                                                         CMD_DISCOVER_MODES);
> -                                       rlen = 1;
> -                               } else if (tcpm_cable_vdm_supported(port)) {
> -                                       *response_tx_sop_type = TCPC_TX_SOP_PRIME;
> -                                       response[0] = VDO(USB_SID_PD, 1,
> -                                                         typec_get_cable_svdm_version(typec),
> -                                                         CMD_DISCOVER_SVID);
> -                                       rlen = 1;
> -                               } else {
> -                                       tcpm_register_partner_altmodes(port);
> -                               }
> -                       } else if (rx_sop_type == TCPC_TX_SOP_PRIME) {
> -                               /* 6.4.4.3.3 */
> -                               svdm_consume_modes(port, p, cnt, rx_sop_type);
> -                               modep_prime->svid_index++;
> -                               if (modep_prime->svid_index < modep_prime->nsvids) {
> -                                       u16 svid = modep_prime->svids[modep_prime->svid_index];
> -                                       *response_tx_sop_type = TCPC_TX_SOP_PRIME;
> -                                       response[0] = VDO(svid, 1,
> -                                                         typec_get_cable_svdm_version(typec),
> -                                                         CMD_DISCOVER_MODES);
> -                                       rlen = 1;
> -                               } else {
> -                                       tcpm_register_plug_altmodes(port);
> -                                       tcpm_register_partner_altmodes(port);
> -                               }
> -                       }
> +                       tcpm_handle_discover_mode(port, p, cnt, response,
> +                                                 rx_sop_type, response_tx_sop_type,
> +                                                 modep, modep_prime, svdm_version,
> +                                                 &rlen, true);
>                         break;
>                 case CMD_ENTER_MODE:
>                         *response_tx_sop_type = rx_sop_type;
> @@ -2329,9 +2347,16 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>                 switch (cmd) {
>                 case CMD_DISCOVER_IDENT:
>                 case CMD_DISCOVER_SVID:
> -               case CMD_DISCOVER_MODES:
>                 case VDO_CMD_VENDOR(0) ... VDO_CMD_VENDOR(15):
>                         break;
> +               case CMD_DISCOVER_MODES:
> +                       tcpm_log(port, "Skip SVID 0x%04x (failed to discover mode)",
> +                                PD_VDO_SVID_SVID0(p[0]));
> +                       tcpm_handle_discover_mode(port, p, cnt, response,
> +                                                 rx_sop_type, response_tx_sop_type,
> +                                                 modep, modep_prime, svdm_version,
> +                                                 &rlen, false);
> +                       break;
>                 case CMD_ENTER_MODE:
>                         /* Back to USB Operation */
>                         *adev_action = ADEV_NOTIFY_USB_AND_QUEUE_VDM;
>
> ---
> base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
> change-id: 20260213-tcpm-discover-modes-nak-fix-09070bb529c5
>
> Best regards,
> --
> Sebastian Reichel <sebastian.reichel@collabora.com>
>

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

* Re: [PATCH v2] usb: typec: tcpm: improve handling of DISCOVER_MODES failures
  2026-03-03 16:29 [PATCH v2] usb: typec: tcpm: improve handling of DISCOVER_MODES failures Sebastian Reichel
  2026-03-05 22:11 ` Badhri Jagan Sridharan
@ 2026-03-09 12:34 ` Heikki Krogerus
  1 sibling, 0 replies; 3+ messages in thread
From: Heikki Krogerus @ 2026-03-09 12:34 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Badhri Jagan Sridharan, Greg Kroah-Hartman, RD Babiera, linux-usb,
	linux-kernel, kernel, stable

Tue, Mar 03, 2026 at 05:29:10PM +0100, Sebastian Reichel kirjoitti:
> UGREEN USB-C Multifunction Adapter Model CM512 (AKA "Revodok 107")
> exposes two SVIDs: 0xff01 (DP Alt Mode) and 0x1d5c. The DISCOVER_MODES
> step succeeds for 0xff01 and gets a NAK for 0x1d5c. Currently this
> results in DP Alt Mode not being registered either, since the modes
> are only registered once all of them have been discovered. The NAK
> results in the processing being stopped and thus no Alt modes being
> registered.
> 
> Improve the situation by handling the NAK gracefully and continue
> processing the other modes.
> 
> Before this change, the TCPM log ends like this:
> 
> (more log entries before this)
> [    5.028287] AMS DISCOVER_SVIDS finished
> [    5.028291] cc:=4
> [    5.040040] SVID 1: 0xff01
> [    5.040054] SVID 2: 0x1d5c
> [    5.040082] AMS DISCOVER_MODES start
> [    5.040096] PD TX, header: 0x1b6f
> [    5.050946] PD TX complete, status: 0
> [    5.059609] PD RX, header: 0x264f [1]
> [    5.059626] Rx VDM cmd 0xff018043 type 1 cmd 3 len 2
> [    5.059640] AMS DISCOVER_MODES finished
> [    5.059644] cc:=4
> [    5.069994]  Alternate mode 0: SVID 0xff01, VDO 1: 0x000c0045
> [    5.070029] AMS DISCOVER_MODES start
> [    5.070043] PD TX, header: 0x1d6f
> [    5.081139] PD TX complete, status: 0
> [    5.087498] PD RX, header: 0x184f [1]
> [    5.087515] Rx VDM cmd 0x1d5c8083 type 2 cmd 3 len 1
> [    5.087529] AMS DISCOVER_MODES finished
> [    5.087534] cc:=4
> (no further log entries after this point)
> 
> After this patch the TCPM log looks exactly the same, but then
> continues like this:
> 
> [    5.100222] Skip SVID 0x1d5c (failed to discover mode)
> [    5.101699] AMS DFP_TO_UFP_ENTER_MODE start
> (log goes on as the system initializes DP AltMode)
> 
> Cc: stable@vger.kernel.org
> Fixes: 41d9d75344d9 ("usb: typec: tcpm: add discover svids and discover modes support for sop'")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> Changes in v2:
> - Link to v1: https://lore.kernel.org/r/20260213-tcpm-discover-modes-nak-fix-v1-0-9bcb5adb4ef6@collabora.com
> - Squash patches (Badhri Jagan Sridharan)
> - Add Fixes tag (Badhri Jagan Sridharan)
> - Move common svdm_consume_modes out of conditional statement (Badhri Jagan Sridharan)
> - Add TCPM log to commit message (Badhri Jagan Sridharan)
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 97 +++++++++++++++++++++++++++----------------
>  1 file changed, 61 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 1d2f3af034c5..cd5260f408fb 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -1995,6 +1995,55 @@ static bool tcpm_cable_vdm_supported(struct tcpm_port *port)
>  	       tcpm_can_communicate_sop_prime(port);
>  }
>  
> +static void tcpm_handle_discover_mode(struct tcpm_port *port,
> +				      const u32 *p, int cnt, u32 *response,
> +				      enum tcpm_transmit_type rx_sop_type,
> +				      enum tcpm_transmit_type *response_tx_sop_type,
> +				      struct pd_mode_data *modep,
> +				      struct pd_mode_data *modep_prime,
> +				      int svdm_version, int *rlen,
> +				      bool success)
> +

Extra newline.

Why not just make this function return rlen?

> +{
> +	struct typec_port *typec = port->typec_port;
> +
> +	/* 6.4.4.3.3 */
> +	if (success)
> +		svdm_consume_modes(port, p, cnt, rx_sop_type);

This condition does not look very useful.

> +	if (rx_sop_type == TCPC_TX_SOP) {
> +		modep->svid_index++;
> +		if (modep->svid_index < modep->nsvids) {
> +			u16 svid = modep->svids[modep->svid_index];
> +			*response_tx_sop_type = TCPC_TX_SOP;
> +			response[0] = VDO(svid, 1, svdm_version,
> +					  CMD_DISCOVER_MODES);
> +			*rlen = 1;
> +		} else if (tcpm_cable_vdm_supported(port)) {
> +			*response_tx_sop_type = TCPC_TX_SOP_PRIME;
> +			response[0] = VDO(USB_SID_PD, 1,
> +					  typec_get_cable_svdm_version(typec),
> +					  CMD_DISCOVER_SVID);
> +			*rlen = 1;
> +		} else {
> +			tcpm_register_partner_altmodes(port);
> +		}
> +	} else if (rx_sop_type == TCPC_TX_SOP_PRIME) {
> +		modep_prime->svid_index++;
> +		if (modep_prime->svid_index < modep_prime->nsvids) {
> +			u16 svid = modep_prime->svids[modep_prime->svid_index];
> +			*response_tx_sop_type = TCPC_TX_SOP_PRIME;
> +			response[0] = VDO(svid, 1,
> +					  typec_get_cable_svdm_version(typec),

You could use svdm_version in this case too, no?

If you read it separately like that, then you might as well do the
same with typec_get_negotiated_svdm_version() in the other condition.

> +					  CMD_DISCOVER_MODES);
> +			*rlen = 1;
> +		} else {
> +			tcpm_register_plug_altmodes(port);
> +			tcpm_register_partner_altmodes(port);
> +		}
> +	}
> +}
> +
>  static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			const u32 *p, int cnt, u32 *response,
>  			enum adev_actions *adev_action,
> @@ -2252,41 +2301,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			}
>  			break;
>  		case CMD_DISCOVER_MODES:
> -			if (rx_sop_type == TCPC_TX_SOP) {
> -				/* 6.4.4.3.3 */
> -				svdm_consume_modes(port, p, cnt, rx_sop_type);
> -				modep->svid_index++;
> -				if (modep->svid_index < modep->nsvids) {
> -					u16 svid = modep->svids[modep->svid_index];
> -					*response_tx_sop_type = TCPC_TX_SOP;
> -					response[0] = VDO(svid, 1, svdm_version,
> -							  CMD_DISCOVER_MODES);
> -					rlen = 1;
> -				} else if (tcpm_cable_vdm_supported(port)) {
> -					*response_tx_sop_type = TCPC_TX_SOP_PRIME;
> -					response[0] = VDO(USB_SID_PD, 1,
> -							  typec_get_cable_svdm_version(typec),
> -							  CMD_DISCOVER_SVID);
> -					rlen = 1;
> -				} else {
> -					tcpm_register_partner_altmodes(port);
> -				}
> -			} else if (rx_sop_type == TCPC_TX_SOP_PRIME) {
> -				/* 6.4.4.3.3 */
> -				svdm_consume_modes(port, p, cnt, rx_sop_type);
> -				modep_prime->svid_index++;
> -				if (modep_prime->svid_index < modep_prime->nsvids) {
> -					u16 svid = modep_prime->svids[modep_prime->svid_index];
> -					*response_tx_sop_type = TCPC_TX_SOP_PRIME;
> -					response[0] = VDO(svid, 1,
> -							  typec_get_cable_svdm_version(typec),
> -							  CMD_DISCOVER_MODES);
> -					rlen = 1;
> -				} else {
> -					tcpm_register_plug_altmodes(port);
> -					tcpm_register_partner_altmodes(port);
> -				}
> -			}
> +			tcpm_handle_discover_mode(port, p, cnt, response,
> +						  rx_sop_type, response_tx_sop_type,
> +						  modep, modep_prime, svdm_version,
> +						  &rlen, true);

Just call svdm_consume_modes() directly from here and drop a few
parameters from that tcpm_handle_discover_mode() function:

        		svdm_consume_modes(port, p, cnt, rx_sop_type);
			tcpm_handle_discover_mode(port, response,
						  rx_sop_type, response_tx_sop_type,
						  modep, modep_prime, svdm_version,
						  &rlen);

>  			break;
>  		case CMD_ENTER_MODE:
>  			*response_tx_sop_type = rx_sop_type;
> @@ -2329,9 +2347,16 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  		switch (cmd) {
>  		case CMD_DISCOVER_IDENT:
>  		case CMD_DISCOVER_SVID:
> -		case CMD_DISCOVER_MODES:
>  		case VDO_CMD_VENDOR(0) ... VDO_CMD_VENDOR(15):
>  			break;
> +		case CMD_DISCOVER_MODES:
> +			tcpm_log(port, "Skip SVID 0x%04x (failed to discover mode)",
> +				 PD_VDO_SVID_SVID0(p[0]));
> +			tcpm_handle_discover_mode(port, p, cnt, response,
> +						  rx_sop_type, response_tx_sop_type,
> +						  modep, modep_prime, svdm_version,
> +						  &rlen, false);
> +			break;
>  		case CMD_ENTER_MODE:
>  			/* Back to USB Operation */
>  			*adev_action = ADEV_NOTIFY_USB_AND_QUEUE_VDM;


I would also use local variable for modep (modep_prime is modep in
case of TCPC_TX_SOP_PRIME) instead of passing both modep and
modep_prime to the function.

With all these changes, the prototype would look like this:

static int tcpm_handle_discover_mode(struct tcpm_port *port, u32 *response,
				     enum tcpm_transmit_type rx_sop_type,
				     enum tcpm_transmit_type *response_tx_sop_type);

thanks,

-- 
heikki

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

end of thread, other threads:[~2026-03-09 12:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03 16:29 [PATCH v2] usb: typec: tcpm: improve handling of DISCOVER_MODES failures Sebastian Reichel
2026-03-05 22:11 ` Badhri Jagan Sridharan
2026-03-09 12:34 ` Heikki Krogerus

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