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

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

[Chia-Lin Kao (AceLan)]

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@vger.kernel.org
Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
---
v4. rebase
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 f181afca2bb28..eebbb80fae566 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -498,6 +498,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)
 {
@@ -622,6 +689,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.53.0

[PATCH v4 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path

[Chia-Lin Kao (AceLan)]

Extend the duplicate altmode detection to ucsi_register_altmodes_nvidia()
which is used when a driver provides the update_altmodes() callback.

This ensures all drivers benefit from duplicate detection, whether they
use the standard registration path or the nvidia path with update_altmodes
callback.

Without this fix, drivers using the nvidia path (like yoga_c630) would
still encounter duplicate altmode registration errors from buggy firmware.

Fixes: a79f16efcd00 ("usb: typec: ucsi: Add support for the partner USB Modes")
Cc: stable@vger.kernel.org
Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
--
v4. rebase
---
 drivers/usb/typec/ucsi/ucsi.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index eebbb80fae566..43389fc7e13f2 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -619,19 +619,25 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
 
 	/* now register altmodes */
 	for (i = 0; i < max_altmodes; i++) {
-		memset(&desc, 0, sizeof(desc));
-		if (multi_dp) {
-			desc.svid = updated[i].svid;
-			desc.vdo = updated[i].mid;
-		} else {
-			desc.svid = orig[i].svid;
-			desc.vdo = orig[i].mid;
-		}
-		desc.roles = TYPEC_PORT_DRD;
+		struct ucsi_altmode *altmode_array = multi_dp ? updated : orig;
 
-		if (!desc.svid)
+		if (!altmode_array[i].svid)
 			return 0;
 
+		/*
+		 * Check for duplicates in current array and already
+		 * registered altmodes. Skip if duplicate found.
+		 */
+		if (ucsi_altmode_is_duplicate(con, recipient, altmode_array, i,
+					      altmode_array[i].svid,
+					      altmode_array[i].mid, i))
+			continue;
+
+		memset(&desc, 0, sizeof(desc));
+		desc.svid = altmode_array[i].svid;
+		desc.vdo = altmode_array[i].mid;
+		desc.roles = TYPEC_PORT_DRD;
+
 		ret = ucsi_register_altmode(con, &desc, recipient);
 		if (ret)
 			return ret;
-- 
2.53.0

[PATCH v4 3/3] usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode handling

[Chia-Lin Kao (AceLan)]

This reverts commit e0c48e42d818 ("usb: typec: ucsi: yoga-c630: remove
duplicate AltModes").

The yoga_c630 driver previously implemented its own duplicate altmode
detection in yoga_c630_ucsi_update_altmodes() to work around buggy EC
firmware that returns duplicate AltModes instead of empty ones.

With the introduction of the common ucsi_altmode_is_duplicate() helper
in both the standard and nvidia registration paths, duplicate detection
is now handled automatically in the core UCSI code. This makes the
yoga_c630-specific implementation added in e0c48e42d818 redundant.

Remove yoga_c630_ucsi_update_altmodes() and its callback to eliminate
code duplication and simplify the driver. Note that this causes the
driver to switch back from the nvidia registration path to the standard
path, which is the original behavior before e0c48e42d818. Both paths
now include duplicate detection, ensuring the firmware bug is still
properly handled.

Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
--
v4. rebase
---
 drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
index 0187c1c4b21ab..dae2f41f8d823 100644
--- a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
+++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
@@ -137,28 +137,6 @@ static int yoga_c630_ucsi_sync_control(struct ucsi *ucsi,
 	return ret;
 }
 
-static bool yoga_c630_ucsi_update_altmodes(struct ucsi *ucsi,
-					   u8 recipient,
-					   struct ucsi_altmode *orig,
-					   struct ucsi_altmode *updated)
-{
-	int i;
-
-	if (orig[0].svid == 0 || recipient != UCSI_RECIPIENT_SOP)
-		return false;
-
-	/* EC is nice and repeats altmodes again and again. Ignore copies. */
-	for (i = 1; i < UCSI_MAX_ALTMODES; i++) {
-		if (orig[i].svid == orig[0].svid) {
-			dev_dbg(ucsi->dev, "Found duplicate altmodes, starting from %d\n", i);
-			memset(&orig[i], 0, (UCSI_MAX_ALTMODES - i) * sizeof(*orig));
-			break;
-		}
-	}
-
-	return false;
-}
-
 static void yoga_c630_ucsi_update_connector(struct ucsi_connector *con)
 {
 	if (con->num == 1)
@@ -172,7 +150,6 @@ static const struct ucsi_operations yoga_c630_ucsi_ops = {
 	.read_message_in = yoga_c630_ucsi_read_message_in,
 	.sync_control = yoga_c630_ucsi_sync_control,
 	.async_control = yoga_c630_ucsi_async_control,
-	.update_altmodes = yoga_c630_ucsi_update_altmodes,
 	.update_connector = yoga_c630_ucsi_update_connector,
 };
 
-- 
2.53.0

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

[Heikki Krogerus]

On Mon, Apr 13, 2026 at 03:35:49PM +0800, Chia-Lin Kao (AceLan) wrote:
> 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@vger.kernel.org
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> ---
> v4. rebase
> 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 f181afca2bb28..eebbb80fae566 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -498,6 +498,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;
> +		}
> +	}

What is this loop meant to do? It will now always return true because
the svid is always from one of the altmodes in the alt_batch, no?

The "batch" here means what the PPM returns to the GET_ALTERNATE_MODES
command (right?), so you can have maximum of two entries in it. So
wouldn't it be simpler to just check if there is two (instead of only
the one that was requested) altmodes returned, and then just directly
compare the two (alt[0].* == alt[1].*)?

> +	/* 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);

You have to clean up these prints. Provide a helper function for them.
You can split the print in two. A generic part, and separate for
details if necessary.

With the "recipient_name" I would use a lookup table.

> +		}
> +		return true;
> +	}
> +
> +	return false;
> +}

thanks,

-- 
heikki