public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware
@ 2025-11-11  1:05 Chia-Lin Kao (AceLan)
  2025-11-11  1:05 ` [PATCH v2 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path Chia-Lin Kao (AceLan)
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2025-11-11  1:05 UTC (permalink / raw)
  To: Heikki Krogerus, Greg Kroah-Hartman, Dmitry Baryshkov,
	Fedor Pchelkin, Andrei Kuchynski, Venkat Jayaraman,
	Myrrh Periwinkle, Chia-Lin Kao (AceLan), linux-usb, linux-kernel

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>
---
 drivers/usb/typec/ucsi/ucsi.c | 77 +++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 3f568f790f39..7b79e7491094 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -556,6 +556,74 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
 	return 0;
 }
 
+/*
+ * 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)
+{
+	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 */
+	if (recipient == UCSI_RECIPIENT_SOP) {
+		for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
+			if (!con->partner_altmode[k])
+				break;
+			/*
+			 * Some buggy firmware returns the same SVID multiple times
+			 * with different VDOs. This causes duplicate device registration
+			 * and sysfs errors. Check SVID only for partner altmodes.
+			 */
+			if (con->partner_altmode[k]->svid == svid) {
+				dev_warn(con->ucsi->dev,
+					 "con%d: Firmware bug: duplicate partner altmode SVID 0x%04x (VDO 0x%08x vs 0x%08x) at offset %d, ignoring. Please update your system firmware.\n",
+					 con->num, svid, con->partner_altmode[k]->vdo,
+					 vdo, offset);
+				return true;
+			}
+		}
+	} else if (recipient == UCSI_RECIPIENT_CON) {
+		for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
+			if (!con->port_altmode[k])
+				break;
+			if (con->port_altmode[k]->svid == svid &&
+			    con->port_altmode[k]->vdo == vdo) {
+				dev_warn_once(con->ucsi->dev,
+					      "con%d: Firmware bug: duplicate port altmode SVID 0x%04x at offset %d, ignoring. Please update your system firmware.\n",
+					      con->num, svid, offset);
+				return true;
+			}
+		}
+	} else if (recipient == UCSI_RECIPIENT_SOP_P) {
+		for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
+			if (!con->plug_altmode[k])
+				break;
+			if (con->plug_altmode[k]->svid == svid &&
+			    con->plug_altmode[k]->vdo == vdo) {
+				dev_warn_once(con->ucsi->dev,
+					      "con%d: Firmware bug: duplicate plug altmode SVID 0x%04x at offset %d, ignoring. Please update your system firmware.\n",
+					      con->num, svid, offset);
+				return true;
+			}
+		}
+	}
+
+	return false;
+}
+
 static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient)
 {
 	int max_altmodes = UCSI_MAX_ALTMODES;
@@ -605,6 +673,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


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

* [PATCH v2 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path
  2025-11-11  1:05 [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware Chia-Lin Kao (AceLan)
@ 2025-11-11  1:05 ` Chia-Lin Kao (AceLan)
  2025-11-11 16:54   ` kernel test robot
  2025-11-11  1:05 ` [PATCH v2 3/3] usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode handling Chia-Lin Kao (AceLan)
  2025-11-17  9:00 ` [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware Heikki Krogerus
  2 siblings, 1 reply; 7+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2025-11-11  1:05 UTC (permalink / raw)
  To: Heikki Krogerus, Greg Kroah-Hartman, Dmitry Baryshkov,
	Fedor Pchelkin, Andrei Kuchynski, Venkat Jayaraman,
	Myrrh Periwinkle, Chia-Lin Kao (AceLan), linux-usb, linux-kernel

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>
---
 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 7b79e7491094..923a7bd30936 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -535,19 +535,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.43.0


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

* [PATCH v2 3/3] usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode handling
  2025-11-11  1:05 [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware Chia-Lin Kao (AceLan)
  2025-11-11  1:05 ` [PATCH v2 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path Chia-Lin Kao (AceLan)
@ 2025-11-11  1:05 ` Chia-Lin Kao (AceLan)
  2025-11-17  9:00 ` [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware Heikki Krogerus
  2 siblings, 0 replies; 7+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2025-11-11  1:05 UTC (permalink / raw)
  To: Heikki Krogerus, Greg Kroah-Hartman, Dmitry Baryshkov,
	Fedor Pchelkin, Andrei Kuchynski, Venkat Jayaraman,
	Myrrh Periwinkle, Chia-Lin Kao (AceLan), linux-usb, linux-kernel

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>
---
 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 0187c1c4b21a..dae2f41f8d82 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.43.0


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

* Re: [PATCH v2 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path
  2025-11-11  1:05 ` [PATCH v2 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path Chia-Lin Kao (AceLan)
@ 2025-11-11 16:54   ` kernel test robot
  0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-11-11 16:54 UTC (permalink / raw)
  To: Chia-Lin Kao (AceLan), Heikki Krogerus, Greg Kroah-Hartman,
	Dmitry Baryshkov, Fedor Pchelkin, Andrei Kuchynski,
	Venkat Jayaraman, Myrrh Periwinkle, linux-usb, linux-kernel
  Cc: llvm, oe-kbuild-all

Hi Chia-Lin,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.18-rc5 next-20251111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chia-Lin-Kao-AceLan/usb-typec-ucsi-Add-duplicate-detection-to-nvidia-registration-path/20251111-092153
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20251111010541.145421-2-acelan.kao%40canonical.com
patch subject: [PATCH v2 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path
config: x86_64-randconfig-072-20251111 (https://download.01.org/0day-ci/archive/20251112/202511120044.i2blPN85-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251112/202511120044.i2blPN85-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511120044.i2blPN85-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/typec/ucsi/ucsi.c:547:7: error: call to undeclared function 'ucsi_altmode_is_duplicate'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     547 |                 if (ucsi_altmode_is_duplicate(con, recipient, altmode_array, i,
         |                     ^
   drivers/usb/typec/ucsi/ucsi.c:570:13: error: static declaration of 'ucsi_altmode_is_duplicate' follows non-static declaration
     570 | static bool ucsi_altmode_is_duplicate(struct ucsi_connector *con, u8 recipient,
         |             ^
   drivers/usb/typec/ucsi/ucsi.c:547:7: note: previous implicit declaration is here
     547 |                 if (ucsi_altmode_is_duplicate(con, recipient, altmode_array, i,
         |                     ^
   2 errors generated.


vim +/ucsi_altmode_is_duplicate +547 drivers/usb/typec/ucsi/ucsi.c

   483	
   484	static int
   485	ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
   486	{
   487		int max_altmodes = UCSI_MAX_ALTMODES;
   488		struct typec_altmode_desc desc;
   489		struct ucsi_altmode alt;
   490		struct ucsi_altmode orig[UCSI_MAX_ALTMODES];
   491		struct ucsi_altmode updated[UCSI_MAX_ALTMODES];
   492		struct ucsi *ucsi = con->ucsi;
   493		bool multi_dp = false;
   494		u64 command;
   495		int ret;
   496		int len;
   497		int i;
   498		int k = 0;
   499	
   500		if (recipient == UCSI_RECIPIENT_CON)
   501			max_altmodes = con->ucsi->cap.num_alt_modes;
   502	
   503		memset(orig, 0, sizeof(orig));
   504		memset(updated, 0, sizeof(updated));
   505	
   506		/* First get all the alternate modes */
   507		for (i = 0; i < max_altmodes; i++) {
   508			memset(&alt, 0, sizeof(alt));
   509			command = UCSI_GET_ALTERNATE_MODES;
   510			command |= UCSI_GET_ALTMODE_RECIPIENT(recipient);
   511			command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
   512			command |= UCSI_GET_ALTMODE_OFFSET(i);
   513			len = ucsi_send_command(con->ucsi, command, &alt, sizeof(alt));
   514			/*
   515			 * We are collecting all altmodes first and then registering.
   516			 * Some type-C device will return zero length data beyond last
   517			 * alternate modes. We should not return if length is zero.
   518			 */
   519			if (len < 0)
   520				return len;
   521	
   522			/* We got all altmodes, now break out and register them */
   523			if (!len || !alt.svid)
   524				break;
   525	
   526			orig[k].mid = alt.mid;
   527			orig[k].svid = alt.svid;
   528			k++;
   529		}
   530		/*
   531		 * Update the original altmode table as some ppms may report
   532		 * multiple DP altmodes.
   533		 */
   534		multi_dp = ucsi->ops->update_altmodes(ucsi, recipient, orig, updated);
   535	
   536		/* now register altmodes */
   537		for (i = 0; i < max_altmodes; i++) {
   538			struct ucsi_altmode *altmode_array = multi_dp ? updated : orig;
   539	
   540			if (!altmode_array[i].svid)
   541				return 0;
   542	
   543			/*
   544			 * Check for duplicates in current array and already
   545			 * registered altmodes. Skip if duplicate found.
   546			 */
 > 547			if (ucsi_altmode_is_duplicate(con, recipient, altmode_array, i,
   548						      altmode_array[i].svid,
   549						      altmode_array[i].mid, i))
   550				continue;
   551	
   552			memset(&desc, 0, sizeof(desc));
   553			desc.svid = altmode_array[i].svid;
   554			desc.vdo = altmode_array[i].mid;
   555			desc.roles = TYPEC_PORT_DRD;
   556	
   557			ret = ucsi_register_altmode(con, &desc, recipient);
   558			if (ret)
   559				return ret;
   560		}
   561	
   562		return 0;
   563	}
   564	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware
  2025-11-11  1:05 [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware Chia-Lin Kao (AceLan)
  2025-11-11  1:05 ` [PATCH v2 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path Chia-Lin Kao (AceLan)
  2025-11-11  1:05 ` [PATCH v2 3/3] usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode handling Chia-Lin Kao (AceLan)
@ 2025-11-17  9:00 ` Heikki Krogerus
  2025-12-04  3:08   ` Chia-Lin Kao (AceLan)
  2025-12-23  9:43   ` Heikki Krogerus
  2 siblings, 2 replies; 7+ messages in thread
From: Heikki Krogerus @ 2025-11-17  9:00 UTC (permalink / raw)
  To: Chia-Lin Kao (AceLan)
  Cc: Greg Kroah-Hartman, Dmitry Baryshkov, Fedor Pchelkin,
	Andrei Kuchynski, Venkat Jayaraman, Myrrh Periwinkle, linux-usb,
	linux-kernel

Hi,

Tue, Nov 11, 2025 at 09:05:39AM +0800, Chia-Lin Kao (AceLan) kirjoitti:
> 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>
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 77 +++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 3f568f790f39..7b79e7491094 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -556,6 +556,74 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
>  	return 0;
>  }
>  
> +/*
> + * 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)
> +{
> +	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 */
> +	if (recipient == UCSI_RECIPIENT_SOP) {
> +		for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
> +			if (!con->partner_altmode[k])
> +				break;
> +			/*
> +			 * Some buggy firmware returns the same SVID multiple times
> +			 * with different VDOs. This causes duplicate device registration
> +			 * and sysfs errors. Check SVID only for partner altmodes.
> +			 */
> +			if (con->partner_altmode[k]->svid == svid) {

I'm not sure this works. Some vendor specific modes always come in
pairs. Check Apple for example. I think you always need to check the
VID on top of the SVID.

> +				dev_warn(con->ucsi->dev,
> +					 "con%d: Firmware bug: duplicate partner altmode SVID 0x%04x (VDO 0x%08x vs 0x%08x) at offset %d, ignoring. Please update your system firmware.\n",
> +					 con->num, svid, con->partner_altmode[k]->vdo,
> +					 vdo, offset);
> +				return true;
> +			}
> +		}
> +	} else if (recipient == UCSI_RECIPIENT_CON) {
> +		for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
> +			if (!con->port_altmode[k])
> +				break;
> +			if (con->port_altmode[k]->svid == svid &&
> +			    con->port_altmode[k]->vdo == vdo) {
> +				dev_warn_once(con->ucsi->dev,
> +					      "con%d: Firmware bug: duplicate port altmode SVID 0x%04x at offset %d, ignoring. Please update your system firmware.\n",
> +					      con->num, svid, offset);
> +				return true;
> +			}
> +		}
> +	} else if (recipient == UCSI_RECIPIENT_SOP_P) {
> +		for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
> +			if (!con->plug_altmode[k])
> +				break;
> +			if (con->plug_altmode[k]->svid == svid &&
> +			    con->plug_altmode[k]->vdo == vdo) {
> +				dev_warn_once(con->ucsi->dev,
> +					      "con%d: Firmware bug: duplicate plug altmode SVID 0x%04x at offset %d, ignoring. Please update your system firmware.\n",
> +					      con->num, svid, offset);
> +				return true;
> +			}
> +		}
> +	}

        struct typec_altmode *altmodes;

        switch (recipient) {
        case UCSI_RECIPIENT_CON:
                altmodes = con->port->altmode;
                break;
        case UCSI_RECIPIENT_SOP_P:
                altmodes = con->plug_altmode;
                break;
        ...

> +
> +	return false;
> +}
> +
>  static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient)
>  {
>  	int max_altmodes = UCSI_MAX_ALTMODES;
> @@ -605,6 +673,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

thanks,

-- 
heikki

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

* Re: [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware
  2025-11-17  9:00 ` [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware Heikki Krogerus
@ 2025-12-04  3:08   ` Chia-Lin Kao (AceLan)
  2025-12-23  9:43   ` Heikki Krogerus
  1 sibling, 0 replies; 7+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2025-12-04  3:08 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Dmitry Baryshkov, Fedor Pchelkin,
	Andrei Kuchynski, Venkat Jayaraman, Myrrh Periwinkle, linux-usb,
	linux-kernel

On Mon, Nov 17, 2025 at 11:00:29AM +0200, Heikki Krogerus wrote:
> Hi,
> 
> Tue, Nov 11, 2025 at 09:05:39AM +0800, Chia-Lin Kao (AceLan) kirjoitti:
> > 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>
> > ---
> >  drivers/usb/typec/ucsi/ucsi.c | 77 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 77 insertions(+)
> > 
> > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> > index 3f568f790f39..7b79e7491094 100644
> > --- a/drivers/usb/typec/ucsi/ucsi.c
> > +++ b/drivers/usb/typec/ucsi/ucsi.c
> > @@ -556,6 +556,74 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * 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)
> > +{
> > +	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 */
> > +	if (recipient == UCSI_RECIPIENT_SOP) {
> > +		for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
> > +			if (!con->partner_altmode[k])
> > +				break;
> > +			/*
> > +			 * Some buggy firmware returns the same SVID multiple times
> > +			 * with different VDOs. This causes duplicate device registration
> > +			 * and sysfs errors. Check SVID only for partner altmodes.
> > +			 */
> > +			if (con->partner_altmode[k]->svid == svid) {
> 
> I'm not sure this works. Some vendor specific modes always come in
> pairs. Check Apple for example. I think you always need to check the
> VID on top of the SVID.
Sorry, just found that my emails were queued/deferred locally.

What does VID means here? USB VID?
I'm not quite understanding what you're trying to do.
Could you elaborate?

> 
> > +				dev_warn(con->ucsi->dev,
> > +					 "con%d: Firmware bug: duplicate partner altmode SVID 0x%04x (VDO 0x%08x vs 0x%08x) at offset %d, ignoring. Please update your system firmware.\n",
> > +					 con->num, svid, con->partner_altmode[k]->vdo,
> > +					 vdo, offset);
> > +				return true;
> > +			}
> > +		}
> > +	} else if (recipient == UCSI_RECIPIENT_CON) {
> > +		for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
> > +			if (!con->port_altmode[k])
> > +				break;
> > +			if (con->port_altmode[k]->svid == svid &&
> > +			    con->port_altmode[k]->vdo == vdo) {
> > +				dev_warn_once(con->ucsi->dev,
> > +					      "con%d: Firmware bug: duplicate port altmode SVID 0x%04x at offset %d, ignoring. Please update your system firmware.\n",
> > +					      con->num, svid, offset);
> > +				return true;
> > +			}
> > +		}
> > +	} else if (recipient == UCSI_RECIPIENT_SOP_P) {
> > +		for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
> > +			if (!con->plug_altmode[k])
> > +				break;
> > +			if (con->plug_altmode[k]->svid == svid &&
> > +			    con->plug_altmode[k]->vdo == vdo) {
> > +				dev_warn_once(con->ucsi->dev,
> > +					      "con%d: Firmware bug: duplicate plug altmode SVID 0x%04x at offset %d, ignoring. Please update your system firmware.\n",
> > +					      con->num, svid, offset);
> > +				return true;
> > +			}
> > +		}
> > +	}
> 
>         struct typec_altmode *altmodes;
> 
>         switch (recipient) {
>         case UCSI_RECIPIENT_CON:
>                 altmodes = con->port->altmode;
>                 break;
>         case UCSI_RECIPIENT_SOP_P:
>                 altmodes = con->plug_altmode;
>                 break;
>         ...
Cool, this make the code cleaner, I'll submit v3 later.

> 
> > +
> > +	return false;
> > +}
> > +
> >  static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient)
> >  {
> >  	int max_altmodes = UCSI_MAX_ALTMODES;
> > @@ -605,6 +673,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
> 
> thanks,
> 
> -- 
> heikki

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

* Re: [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware
  2025-11-17  9:00 ` [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware Heikki Krogerus
  2025-12-04  3:08   ` Chia-Lin Kao (AceLan)
@ 2025-12-23  9:43   ` Heikki Krogerus
  1 sibling, 0 replies; 7+ messages in thread
From: Heikki Krogerus @ 2025-12-23  9:43 UTC (permalink / raw)
  To: Chia-Lin Kao (AceLan)
  Cc: Greg Kroah-Hartman, Dmitry Baryshkov, Fedor Pchelkin,
	Andrei Kuchynski, Venkat Jayaraman, Myrrh Periwinkle, linux-usb,
	linux-kernel

Hi,

Mon, Nov 17, 2025 at 11:00:34AM +0200, Heikki Krogerus kirjoitti:
> Hi,
> 
> Tue, Nov 11, 2025 at 09:05:39AM +0800, Chia-Lin Kao (AceLan) kirjoitti:
> > 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.

Will you be sending v3? This does need a fix.

thanks,

-- 
heikki

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

end of thread, other threads:[~2025-12-23  9:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-11  1:05 [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware Chia-Lin Kao (AceLan)
2025-11-11  1:05 ` [PATCH v2 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path Chia-Lin Kao (AceLan)
2025-11-11 16:54   ` kernel test robot
2025-11-11  1:05 ` [PATCH v2 3/3] usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode handling Chia-Lin Kao (AceLan)
2025-11-17  9:00 ` [PATCH v2 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware Heikki Krogerus
2025-12-04  3:08   ` Chia-Lin Kao (AceLan)
2025-12-23  9:43   ` Heikki Krogerus

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