Re: [PATCH v2] usb: typec: tcpm: improve handling of DISCOVER_MODES failures

[Badhri Jagan Sridharan]

On Tue, Mar 3, 2026 at 8:29 AM Sebastian Reichel
<sebastian.reichel@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> Fixes: 41d9d75344d9 ("usb: typec: tcpm: add discover svids and discover modes support for sop'")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>


Reviewed-by: Badhri Jagan Sridharan <badhri@xxxxxxxxxx>

>
> ---
> Changes in v2:
> - Link to v1: https://lore.kernel.org/r/20260213-tcpm-discover-modes-nak-fix-v1-0-9bcb5adb4ef6@xxxxxxxxxxxxx
> - 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@xxxxxxxxxxxxx>
>