Re: [PATCH v3 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware

[AceLan Kao]

A gentle ping.
Please help to review these patches, thanks.

Chia-Lin Kao (AceLan) <acelan.kao@xxxxxxxxxxxxx> 於 2025年12月24日週三 下午3:00寫道：
>
> Some firmware implementations incorrectly return the same altmode
> multiple times at different offsets when queried via UCSI_GET_ALTERNATE_MODES.
> This causes sysfs duplicate filename errors and kernel call traces when
> the driver attempts to register the same altmode twice:
>
>   sysfs: cannot create duplicate filename '/devices/.../typec/port0/port0.0/partner'
>   typec-thunderbolt port0-partner.1: failed to create symlinks
>   typec-thunderbolt port0-partner.1: probe with driver typec-thunderbolt failed with error -17
>
> Detect duplicate altmodes by comparing SVID and VDO before registration.
> If a duplicate is detected, skip it and print a single clean warning
> message instead of generating a kernel call trace:
>
>   ucsi_acpi USBC000:00: con2: Firmware bug: duplicate partner altmode SVID 0x8087 (VDO 0x8087a043 vs 0x00000001) at offset 1, ignoring. Please update your system firmware.
>
> This makes the error handling more user-friendly while still alerting
> users to the firmware bug.
>
> The duplicate detection logic is implemented in a reusable helper
> function ucsi_altmode_is_duplicate() and used in ucsi_register_altmodes().
> The fix applies to all three recipient types: partner (SOP), port (CON),
> and plug (SOP_P) altmodes.
>
> Fixes: a79f16efcd00 ("usb: typec: ucsi: Add support for the partner USB Modes")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@xxxxxxxxxxxxx>
> ---
> v3. 1. move ucsi_altmode_is_duplicate() before ucsi_register_altmodes_nvidia()
>        for later modification on ucsi_register_altmodes_nvidia()
>     2. use struct typec_altmode **altmodes to simplify the logic
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 76 +++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 9b3df776137a1..b99c86e9f31cb 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -501,6 +501,73 @@ static int ucsi_register_altmode(struct ucsi_connector *con,
>         return ret;
>  }
>
> +/*
> + * Check if an altmode is a duplicate. Some firmware implementations
> + * incorrectly return the same altmode multiple times, causing sysfs errors.
> + * Returns true if the altmode should be skipped.
> + */
> +static bool ucsi_altmode_is_duplicate(struct ucsi_connector *con, u8 recipient,
> +                                     const struct ucsi_altmode *alt_batch, int batch_idx,
> +                                     u16 svid, u32 vdo, int offset)
> +{
> +       struct typec_altmode **altmodes;
> +       const char *recipient_name;
> +       int k;
> +
> +       /* Check for duplicates within the current batch first */
> +       for (k = 0; k < batch_idx; k++) {
> +               if (alt_batch[k].svid == svid && alt_batch[k].mid == vdo) {
> +                       dev_warn_once(con->ucsi->dev,
> +                                     "con%d: Firmware bug: duplicate altmode SVID 0x%04x in same response at offset %d, ignoring. Please update your system firmware.\n",
> +                                     con->num, svid, offset);
> +                       return true;
> +               }
> +       }
> +
> +       /* Check for duplicates in already registered altmodes */
> +
> +       switch (recipient) {
> +       case UCSI_RECIPIENT_CON:
> +               altmodes = con->port_altmode;
> +               recipient_name = "port";
> +               break;
> +       case UCSI_RECIPIENT_SOP:
> +               altmodes = con->partner_altmode;
> +               recipient_name = "partner";
> +               break;
> +       case UCSI_RECIPIENT_SOP_P:
> +               altmodes = con->plug_altmode;
> +               recipient_name = "plug";
> +               break;
> +       default:
> +               return false;
> +       }
> +
> +       for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
> +               if (!altmodes[k])
> +                       break;
> +
> +               /* Check SVID for all, VDO only for non-SOP */
> +               if (altmodes[k]->svid != svid)
> +                       continue;
> +               if (recipient != UCSI_RECIPIENT_SOP && altmodes[k]->vdo != vdo)
> +                       continue;
> +
> +               if (recipient == UCSI_RECIPIENT_SOP) {
> +                       dev_warn(con->ucsi->dev,
> +                                "con%d: Firmware bug: duplicate %s altmode SVID 0x%04x (VDO 0x%08x vs 0x%08x) at offset %d, ignoring. Please update your system firmware.\n",
> +                                con->num, recipient_name, svid, altmodes[k]->vdo, vdo, offset);
> +               } else {
> +                       dev_warn_once(con->ucsi->dev,
> +                                     "con%d: Firmware bug: duplicate %s altmode SVID 0x%04x at offset %d, ignoring. Please update your system firmware.\n",
> +                                     con->num, recipient_name, svid, offset);
> +               }
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
>  static int
>  ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
>  {
> @@ -631,6 +698,15 @@ static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient)
>                         if (!alt[j].svid)
>                                 return 0;
>
> +                       /*
> +                        * Check for duplicates in current batch and already
> +                        * registered altmodes. Skip if duplicate found.
> +                        */
> +                       if (ucsi_altmode_is_duplicate(con, recipient, alt, j,
> +                                                     alt[j].svid, alt[j].mid,
> +                                                     i - num + j))
> +                               continue;
> +
>                         memset(&desc, 0, sizeof(desc));
>                         desc.vdo = alt[j].mid;
>                         desc.svid = alt[j].svid;
> --
> 2.43.0
>


-- 
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)