The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware
@ 2026-06-12  5:21 Chia-Lin Kao (AceLan)
  2026-06-12  5:21 ` [PATCH v6 1/3] usb: typec: ucsi: Detect and skip " Chia-Lin Kao (AceLan)
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2026-06-12  5:21 UTC (permalink / raw)
  To: heikki.krogerus, gregkh
  Cc: bleung, jthies, myrrhperiwinkle, pooja.katiyar, abelvesa,
	yuanhsinte, johan, dmitry.baryshkov, linux-usb, linux-kernel

Some UCSI firmware implementations return the same altmode multiple
times when queried with UCSI_GET_ALTERNATE_MODES.  When the resulting
duplicate partner altmodes reach the typec class, two altmodes with the
same SVID end up matched to the same port-side altmode by
altmode_match()/device_find_child(), and the second registration
collides on /sys/.../portN.M/partner with a sysfs duplicate-filename
error and a probe failure (visible as a kernel call trace from
typec_altmode_create_links()).

  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

This series teaches ucsi_register_altmodes() and
ucsi_register_altmodes_nvidia() to filter these duplicates before they
hit the typec class, replacing the call trace with a single dev_warn().
It also retires the open-coded workaround that ucsi_yoga_c630.c was
using for the same firmware bug now that the core handles it.

The matching rules differ by recipient, because the pairing semantics
in the typec class differ:

  - UCSI_RECIPIENT_CON  (port) and
    UCSI_RECIPIENT_SOP_P (plug):
    Drop only byte-for-byte duplicates (same SVID and same VDO);
    a second copy adds nothing.

  - UCSI_RECIPIENT_SOP  (partner):
    A partner altmode is paired with a port altmode of the same SVID
    via altmode_match()/device_find_child(), which returns the first
    port altmode with the matching SVID.  The number of partner
    altmodes that can coexist for a given SVID is therefore bounded
    by the number of port altmodes the firmware advertises for that
    SVID.  Use that port-side count as the cap and reject any partner
    altmode that would exceed it.

Why cardinality and not just SVID+VDO?  A plain SVID+VDO comparison is
too lax for the partner case: real firmware in the wild reports two
"partner" Thunderbolt altmodes (SVID 0x8087) with different VDOs while
the port side only advertises one, and both would still collapse onto
the same port altmode and trip the sysfs collision.  A plain SVID
comparison is the other extreme: it would falsely drop the second mode
of vendor SVIDs that the port really does advertise twice (for example,
Dell's 0x413c which legitimately registers as port1.1 and port1.2 with
two distinct VDOs).  The cardinality cap captures both cases correctly
and is consistent with how the typec class actually pairs altmodes.

Tested on two Dell platforms:

  * Dell platform A: firmware reports two partner altmodes for SVID
    0x8087 (Thunderbolt) against a single port-side 0x8087, and the
    same port also advertises two distinct 0x413c altmodes
    (port1.1 and port1.2) paired with two 0x413c partner altmodes.
    Before: sysfs duplicate-filename error and probe failure on
    portN-partner.1 / typec-thunderbolt.
    After: the surplus 0x8087 partner altmode is dropped with a
    clean dev_warn(); typec-thunderbolt probes once and the dock
    works, and both legitimate 0x413c partner altmodes still
    register as portN-partner.1 and portN-partner.2.  DisplayPort
    (0xff01) is unaffected.

  * Dell platform B: firmware reports a single 0x8087 partner
    altmode matching its single port-side 0x8087 (no firmware bug
    present).  Used as a regression-check baseline.
    After: behaviour unchanged; no altmodes are spuriously dropped.

v5 -> v6:
  - Replace the partner-side SVID+VDO duplicate check with a
    cardinality cap: skip a partner altmode for SVID X once the
    number of already-registered partner altmodes for X has reached
    the number of port altmodes for X.  This is the only rule that
    is consistent with how the typec class pairs partner-to-port
    altmodes, and is the change that actually fixes the Thunderbolt
    case while keeping the Dell case working.
  - Add a small ucsi_altmode_count_svid() helper used by the
    cardinality check.
  - Move the recipient-name table into ucsi_dump_duplicate_altmode()
    as a function-local static const; it has no other users.
  - Expand the kernel-doc block on ucsi_altmode_is_duplicate() to
    document the per-recipient rules and the rationale.
  - Reword patch 3/3 to use the canonical 'commit <sha12> ("<title>")'
    style for the reference to the original yoga_c630 workaround.
  - Tested on the two platforms described above (TBT firmware bug
    machine and Dell multi-Mode 0x413c machine); no regression on
    either, call trace gone on the TBT machine.

v4 -> v5:
  - Add ucsi_dump_duplicate_altmode() helper to consolidate the
    duplicate-detected dev_warn() output; all three previous
    open-coded warning callsites (batch-internal, partner already
    registered, port/plug already registered) route through it.
  - Add ucsi_recipient_names[] lookup table and use it in the new
    helper instead of repeating the switch over recipient values
    inside ucsi_altmode_is_duplicate() and the warning strings.
    (v6 later moves this table into the helper itself as a
    function-local static const; see "Changes since v5".)
  - Split the user-visible warning into two shorter lines:
      "Firmware bug: duplicate <recipient> altmode SVID 0xNNNN at
       offset N, ignoring."
      "VDO mismatch: 0xNNNNNNNN vs 0xNNNNNNNN" (only emitted when
      the two VDOs actually differ).
    The previous single line ran past 75 chars and embedded the
    VDOs and a "Please update your system firmware." trailer.

v3 -> v4:
  - Rebased onto the latest tree.

v2 -> v3:
  - Move ucsi_altmode_is_duplicate() above
    ucsi_register_altmodes_nvidia() so the nvidia path can call it
    too (groundwork for the nvidia-path fix that later lands as
    patch 2/3).
  - Use struct typec_altmode ** to walk the already-registered
    altmode arrays (port_altmode / partner_altmode / plug_altmode)
    instead of three separate branches; simplifies the dedup logic.

v1 -> v2:
  - Split the single patch into a 3-patch series:
      1/3 core helper + standard registration path,
      2/3 same dedup applied in the nvidia registration path so
          all drivers benefit, not just those on the standard path,
      3/3 retire the open-coded duplicate filter in ucsi_yoga_c630.c
          now that the core covers it.
  - Extend ucsi_altmode_is_duplicate() to cover all three recipient
    types (CON / SOP / SOP_P), not just SOP.
  - Improve the warning message to also include the differing VDOs
    when a partner altmode collides on SVID with a different VDO,
    so the firmware bug is actually diagnosable from dmesg.
  - Print a single dev_warn() per skipped altmode instead of letting
    the registration path generate a sysfs duplicate-filename call
    trace.

Previous revisions:
  [v1] https://lore.kernel.org/lkml/20251016055332.914106-1-acelan.kao@canonical.com/
  [v2] https://lore.kernel.org/lkml/20251111010541.145421-1-acelan.kao@canonical.com/
  [v3] https://lore.kernel.org/lkml/20251224070022.4082182-1-acelan.kao@canonical.com/
  [v4] https://lore.kernel.org/lkml/20260413073552.894395-1-acelan.kao@canonical.com/
  [v5] https://lore.kernel.org/lkml/20260504115927.48925-1-acelan.kao@canonical.com/

Some UCSI firmware implementations return the same altmode multiple
times when queried with UCSI_GET_ALTERNATE_MODES.  When the resulting
duplicate partner altmodes reach the typec class, two altmodes with the
same SVID end up matched to the same port-side altmode by
altmode_match()/device_find_child(), and the second registration
collides on /sys/.../portN.M/partner with a sysfs duplicate-filename
error and a probe failure (visible as a kernel call trace from
typec_altmode_create_links()).

  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

This series teaches ucsi_register_altmodes() and
ucsi_register_altmodes_nvidia() to filter these duplicates before they
hit the typec class, replacing the call trace with a single dev_warn().
It also retires the open-coded workaround that ucsi_yoga_c630.c was
using for the same firmware bug now that the core handles it.

The matching rules differ by recipient, because the pairing semantics
in the typec class differ:

  - UCSI_RECIPIENT_CON  (port) and
    UCSI_RECIPIENT_SOP_P (plug):
    Drop only byte-for-byte duplicates (same SVID and same VDO);
    a second copy adds nothing.

  - UCSI_RECIPIENT_SOP  (partner):
    A partner altmode is paired with a port altmode of the same SVID
    via altmode_match()/device_find_child(), which returns the first
    port altmode with the matching SVID.  The number of partner
    altmodes that can coexist for a given SVID is therefore bounded
    by the number of port altmodes the firmware advertises for that
    SVID.  Use that port-side count as the cap and reject any partner
    altmode that would exceed it.

Why cardinality and not just SVID+VDO?  A plain SVID+VDO comparison is
too lax for the partner case: real firmware in the wild reports two
"partner" Thunderbolt altmodes (SVID 0x8087) with different VDOs while
the port side only advertises one, and both would still collapse onto
the same port altmode and trip the sysfs collision.  A plain SVID
comparison is the other extreme: it would falsely drop the second mode
of vendor SVIDs that the port really does advertise twice (for example,
Dell's 0x413c which legitimately registers as port1.1 and port1.2 with
two distinct VDOs).  The cardinality cap captures both cases correctly
and is consistent with how the typec class actually pairs altmodes.

Tested on two Dell platforms:

  * Dell platform A: firmware reports two partner altmodes for SVID
    0x8087 (Thunderbolt) against a single port-side 0x8087, and the
    same port also advertises two distinct 0x413c altmodes
    (port1.1 and port1.2) paired with two 0x413c partner altmodes.
    Before: sysfs duplicate-filename error and probe failure on
    portN-partner.1 / typec-thunderbolt.
    After: the surplus 0x8087 partner altmode is dropped with a
    clean dev_warn(); typec-thunderbolt probes once and the dock
    works, and both legitimate 0x413c partner altmodes still
    register as portN-partner.1 and portN-partner.2.  DisplayPort
    (0xff01) is unaffected.

  * Dell platform B: firmware reports a single 0x8087 partner
    altmode matching its single port-side 0x8087 (no firmware bug
    present).  Used as a regression-check baseline.
    After: behaviour unchanged; no altmodes are spuriously dropped.

v5 -> v6:
  - Replace the partner-side SVID+VDO duplicate check with a
    cardinality cap: skip a partner altmode for SVID X once the
    number of already-registered partner altmodes for X has reached
    the number of port altmodes for X.  This is the only rule that
    is consistent with how the typec class pairs partner-to-port
    altmodes, and is the change that actually fixes the Thunderbolt
    case while keeping the Dell case working.
  - Add a small ucsi_altmode_count_svid() helper used by the
    cardinality check.
  - Move the recipient-name table into ucsi_dump_duplicate_altmode()
    as a function-local static const; it has no other users.
  - Expand the kernel-doc block on ucsi_altmode_is_duplicate() to
    document the per-recipient rules and the rationale.
  - Reword patch 3/3 to use the canonical 'commit <sha12> ("<title>")'
    style for the reference to the original yoga_c630 workaround.
  - Tested on the two platforms described above (TBT firmware bug
    machine and Dell multi-Mode 0x413c machine); no regression on
    either, call trace gone on the TBT machine.

v4 -> v5:
  - Add ucsi_dump_duplicate_altmode() helper to consolidate the
    duplicate-detected dev_warn() output; all three previous
    open-coded warning callsites (batch-internal, partner already
    registered, port/plug already registered) route through it.
  - Add ucsi_recipient_names[] lookup table and use it in the new
    helper instead of repeating the switch over recipient values
    inside ucsi_altmode_is_duplicate() and the warning strings.
    (v6 later moves this table into the helper itself as a
    function-local static const; see "Changes since v5".)
  - Split the user-visible warning into two shorter lines:
      "Firmware bug: duplicate <recipient> altmode SVID 0xNNNN at
       offset N, ignoring."
      "VDO mismatch: 0xNNNNNNNN vs 0xNNNNNNNN" (only emitted when
      the two VDOs actually differ).
    The previous single line ran past 75 chars and embedded the
    VDOs and a "Please update your system firmware." trailer.

v3 -> v4:
  - Rebased onto the latest tree.

v2 -> v3:
  - Move ucsi_altmode_is_duplicate() above
    ucsi_register_altmodes_nvidia() so the nvidia path can call it
    too (groundwork for the nvidia-path fix that later lands as
    patch 2/3).
  - Use struct typec_altmode ** to walk the already-registered
    altmode arrays (port_altmode / partner_altmode / plug_altmode)
    instead of three separate branches; simplifies the dedup logic.

v1 -> v2:
  - Split the single patch into a 3-patch series:
      1/3 core helper + standard registration path,
      2/3 same dedup applied in the nvidia registration path so
          all drivers benefit, not just those on the standard path,
      3/3 retire the open-coded duplicate filter in ucsi_yoga_c630.c
          now that the core covers it.
  - Extend ucsi_altmode_is_duplicate() to cover all three recipient
    types (CON / SOP / SOP_P), not just SOP.
  - Improve the warning message to also include the differing VDOs
    when a partner altmode collides on SVID with a different VDO,
    so the firmware bug is actually diagnosable from dmesg.
  - Print a single dev_warn() per skipped altmode instead of letting
    the registration path generate a sysfs duplicate-filename call
    trace.

Previous revisions:
  [v1] https://lore.kernel.org/lkml/20251016055332.914106-1-acelan.kao@canonical.com/
  [v2] https://lore.kernel.org/lkml/20251111010541.145421-1-acelan.kao@canonical.com/
  [v3] https://lore.kernel.org/lkml/20251224070022.4082182-1-acelan.kao@canonical.com/
  [v4] https://lore.kernel.org/lkml/20260413073552.894395-1-acelan.kao@canonical.com/
  [v5] https://lore.kernel.org/lkml/20260504115927.48925-1-acelan.kao@canonical.com/

Chia-Lin Kao (AceLan) (3):
  usb: typec: ucsi: Detect and skip duplicate altmodes from buggy
    firmware
  usb: typec: ucsi: Add duplicate detection to nvidia registration path
  usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode
    handling

 drivers/usb/typec/ucsi/ucsi.c           | 158 ++++++++++++++++++++++--
 drivers/usb/typec/ucsi/ucsi_yoga_c630.c |  23 ----
 2 files changed, 148 insertions(+), 33 deletions(-)


base-commit: ec039126b7fac4e3af35ebccaa7c6f9b6875ba81
--
2.53.0


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

* [PATCH v6 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware
  2026-06-12  5:21 [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware Chia-Lin Kao (AceLan)
@ 2026-06-12  5:21 ` Chia-Lin Kao (AceLan)
  2026-07-08 11:45   ` Greg KH
  2026-06-12  5:21 ` [PATCH v6 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path Chia-Lin Kao (AceLan)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2026-06-12  5:21 UTC (permalink / raw)
  To: heikki.krogerus, gregkh
  Cc: bleung, jthies, myrrhperiwinkle, pooja.katiyar, abelvesa,
	yuanhsinte, johan, dmitry.baryshkov, 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

The matching rules differ by recipient:

  - UCSI_RECIPIENT_CON (port) and UCSI_RECIPIENT_SOP_P (plug):
    Two altmodes with identical SVID and VDO are byte-for-byte
    duplicates and the second has no observable function, so drop it.

  - UCSI_RECIPIENT_SOP (partner):
    The typec class binds each partner altmode to a port altmode of
    the same SVID via altmode_match()/device_find_child(), which
    returns the first port altmode with a matching SVID.  If the
    partner advertises more altmodes for SVID X than the port
    advertises, the surplus partner altmode(s) collapse onto an
    already-paired port altmode and trigger the
    "duplicate filename .../partner" sysfs error during
    typec_altmode_create_links().  Use the port-side altmode count for
    SVID X as the authoritative cap and reject any partner altmode
    that would exceed it.  This preserves legitimate multi-Mode
    partner altmodes (vendor SVIDs that the port really does
    advertise more than once) while filtering the firmware-generated
    duplicates that have no port counterpart, and is therefore
    stricter than a plain SVID+VDO comparison (which still admits the
    Thunderbolt case where firmware reports the same SVID twice with
    different VDOs) without being over-broad like a plain SVID match
    (which would falsely drop legitimate vendor multi-Mode entries).

If a duplicate is detected, skip it and emit a clean warning instead
of generating a kernel call trace:

  ucsi_acpi USBC000:00: con2: Firmware bug: duplicate partner altmode SVID 0x8087 at offset 1, ignoring.
  ucsi_acpi USBC000:00: con2: VDO mismatch: 0x8087a043 vs 0x00000001

The duplicate detection logic lives in a reusable helper
ucsi_altmode_is_duplicate() and is invoked from
ucsi_register_altmodes().  It 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>
--
v6. - New helper ucsi_altmode_count_svid() counts altmodes with a given
      SVID in any of the per-connector altmode arrays.
    - ucsi_altmode_is_duplicate() for UCSI_RECIPIENT_SOP no longer asks
      "have I seen this SVID before". It now checks whether
      partner_count_for(svid) >= port_count_for(svid) and rejects only the
      surplus.
    - For UCSI_RECIPIENT_CON and UCSI_RECIPIENT_SOP_P, behavior is
      unchanged: still SVID+VDO exact-dup match.
    - Commit message rewritten to spell out the per-recipient rules and
      why partner needs the cardinality cap (avoids both the SVID-only
      false positives that hurt the legitimate Dell 0x413c case and the
      SVID+VDO false negatives that let the Thunderbolt 0x8087 firmware
      bug through).
---
 drivers/usb/typec/ucsi/ucsi.c | 132 ++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 92166a3725b16..9cd7fd7a38521 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -529,6 +529,129 @@ static int ucsi_register_altmode(struct ucsi_connector *con,
 	return ret;
 }
 
+static void ucsi_dump_duplicate_altmode(struct ucsi_connector *con,
+					u8 recipient, u16 svid,
+					u32 existing_vdo, u32 new_vdo,
+					int offset)
+{
+	static const char * const recipient_names[] = {
+		[UCSI_RECIPIENT_CON]    = "port",
+		[UCSI_RECIPIENT_SOP]    = "partner",
+		[UCSI_RECIPIENT_SOP_P]  = "plug",
+		[UCSI_RECIPIENT_SOP_PP] = "cable plug prime",
+	};
+
+	dev_warn(con->ucsi->dev,
+		 "con%d: Firmware bug: duplicate %s altmode SVID 0x%04x at offset %d, ignoring.\n",
+		 con->num, recipient_names[recipient], svid, offset);
+
+	if (existing_vdo != new_vdo)
+		dev_warn(con->ucsi->dev,
+			 "con%d: VDO mismatch: 0x%08x vs 0x%08x\n",
+			 con->num, existing_vdo, new_vdo);
+}
+
+/* Count altmodes in @altmodes that advertise @svid. */
+static int ucsi_altmode_count_svid(struct typec_altmode **altmodes, u16 svid)
+{
+	int count = 0;
+	int k;
+
+	for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
+		if (!altmodes[k])
+			break;
+		if (altmodes[k]->svid == svid)
+			count++;
+	}
+
+	return count;
+}
+
+/*
+ * 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.
+ *
+ * The matching rules differ by recipient:
+ *
+ *   - UCSI_RECIPIENT_CON (port) and UCSI_RECIPIENT_SOP_P (plug):
+ *     Two altmodes with identical SVID and VDO are byte-for-byte duplicates
+ *     and the second has no observable function. Drop them.
+ *
+ *   - UCSI_RECIPIENT_SOP (partner):
+ *     The typec class binds each partner altmode to a port altmode of the
+ *     same SVID via altmode_match()/device_find_child(), which returns the
+ *     first port altmode with a matching SVID. If the partner advertises
+ *     more altmodes for SVID X than the port advertises, the surplus
+ *     partner altmode(s) collapse onto an already-paired port altmode and
+ *     trigger a "duplicate filename .../partner" sysfs error during
+ *     typec_altmode_create_links(). Use the port-side altmode count for
+ *     SVID X as the authoritative cap and reject any partner altmode that
+ *     would exceed it. This preserves legitimate multi-Mode partner
+ *     altmodes (e.g. vendor SVIDs that the port really does advertise
+ *     twice) while filtering the firmware-generated duplicates that have
+ *     no port counterpart.
+ */
+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;
+	int port_count, partner_count;
+	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) {
+			ucsi_dump_duplicate_altmode(con, recipient, svid,
+						    vdo, vdo, offset);
+			return true;
+		}
+	}
+
+	switch (recipient) {
+	case UCSI_RECIPIENT_SOP:
+		/*
+		 * Cap partner altmodes per SVID by the port-side count:
+		 * any further partner altmode for that SVID would alias an
+		 * already-paired port altmode and break typec sysfs.
+		 */
+		port_count = ucsi_altmode_count_svid(con->port_altmode, svid);
+		partner_count = ucsi_altmode_count_svid(con->partner_altmode,
+							svid);
+		if (port_count && partner_count >= port_count) {
+			ucsi_dump_duplicate_altmode(con, recipient, svid,
+						    con->partner_altmode[partner_count - 1]->vdo,
+						    vdo, offset);
+			return true;
+		}
+		return false;
+	case UCSI_RECIPIENT_CON:
+		altmodes = con->port_altmode;
+		break;
+	case UCSI_RECIPIENT_SOP_P:
+		altmodes = con->plug_altmode;
+		break;
+	default:
+		return false;
+	}
+
+	/* CON and SOP_P: drop only exact SVID+VDO duplicates. */
+	for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
+		if (!altmodes[k])
+			break;
+
+		if (altmodes[k]->svid != svid || altmodes[k]->vdo != vdo)
+			continue;
+
+		ucsi_dump_duplicate_altmode(con, recipient, svid,
+					    altmodes[k]->vdo, vdo, offset);
+		return true;
+	}
+
+	return false;
+}
+
 static int
 ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
 {
@@ -653,6 +776,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


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

* [PATCH v6 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path
  2026-06-12  5:21 [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware Chia-Lin Kao (AceLan)
  2026-06-12  5:21 ` [PATCH v6 1/3] usb: typec: ucsi: Detect and skip " Chia-Lin Kao (AceLan)
@ 2026-06-12  5:21 ` Chia-Lin Kao (AceLan)
  2026-06-12  5:21 ` [PATCH v6 3/3] usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode handling Chia-Lin Kao (AceLan)
  2026-06-12  5:32 ` [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware AceLan Kao
  3 siblings, 0 replies; 7+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2026-06-12  5:21 UTC (permalink / raw)
  To: heikki.krogerus, gregkh
  Cc: bleung, jthies, myrrhperiwinkle, pooja.katiyar, abelvesa,
	yuanhsinte, johan, dmitry.baryshkov, 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 9cd7fd7a38521..9eeb38e7472c0 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -706,19 +706,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


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

* [PATCH v6 3/3] usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode handling
  2026-06-12  5:21 [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware Chia-Lin Kao (AceLan)
  2026-06-12  5:21 ` [PATCH v6 1/3] usb: typec: ucsi: Detect and skip " Chia-Lin Kao (AceLan)
  2026-06-12  5:21 ` [PATCH v6 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path Chia-Lin Kao (AceLan)
@ 2026-06-12  5:21 ` Chia-Lin Kao (AceLan)
  2026-06-12  5:32 ` [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware AceLan Kao
  3 siblings, 0 replies; 7+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2026-06-12  5:21 UTC (permalink / raw)
  To: heikki.krogerus, gregkh
  Cc: bleung, jthies, myrrhperiwinkle, pooja.katiyar, abelvesa,
	yuanhsinte, johan, dmitry.baryshkov, 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 commit e0c48e42d818 ("usb:
typec: ucsi: yoga-c630: remove duplicate AltModes") 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 commit e0c48e42d818 ("usb:
typec: ucsi: yoga-c630: remove duplicate AltModes"). 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 1be18d1018426..bb7b09bc7c1c1 100644
--- a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
+++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
@@ -139,28 +139,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)
@@ -174,7 +152,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


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

* [PATCH v6 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware
  2026-06-12  5:28 Chia-Lin Kao (AceLan)
@ 2026-06-12  5:28 ` Chia-Lin Kao (AceLan)
  0 siblings, 0 replies; 7+ messages in thread
From: Chia-Lin Kao (AceLan) @ 2026-06-12  5:28 UTC (permalink / raw)
  To: acelan; +Cc: 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

The matching rules differ by recipient:

  - UCSI_RECIPIENT_CON (port) and UCSI_RECIPIENT_SOP_P (plug):
    Two altmodes with identical SVID and VDO are byte-for-byte
    duplicates and the second has no observable function, so drop it.

  - UCSI_RECIPIENT_SOP (partner):
    The typec class binds each partner altmode to a port altmode of
    the same SVID via altmode_match()/device_find_child(), which
    returns the first port altmode with a matching SVID.  If the
    partner advertises more altmodes for SVID X than the port
    advertises, the surplus partner altmode(s) collapse onto an
    already-paired port altmode and trigger the
    "duplicate filename .../partner" sysfs error during
    typec_altmode_create_links().  Use the port-side altmode count for
    SVID X as the authoritative cap and reject any partner altmode
    that would exceed it.  This preserves legitimate multi-Mode
    partner altmodes (vendor SVIDs that the port really does
    advertise more than once) while filtering the firmware-generated
    duplicates that have no port counterpart, and is therefore
    stricter than a plain SVID+VDO comparison (which still admits the
    Thunderbolt case where firmware reports the same SVID twice with
    different VDOs) without being over-broad like a plain SVID match
    (which would falsely drop legitimate vendor multi-Mode entries).

If a duplicate is detected, skip it and emit a clean warning instead
of generating a kernel call trace:

  ucsi_acpi USBC000:00: con2: Firmware bug: duplicate partner altmode SVID 0x8087 at offset 1, ignoring.
  ucsi_acpi USBC000:00: con2: VDO mismatch: 0x8087a043 vs 0x00000001

The duplicate detection logic lives in a reusable helper
ucsi_altmode_is_duplicate() and is invoked from
ucsi_register_altmodes().  It 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>
--
v6. - New helper ucsi_altmode_count_svid() counts altmodes with a given
      SVID in any of the per-connector altmode arrays.
    - ucsi_altmode_is_duplicate() for UCSI_RECIPIENT_SOP no longer asks
      "have I seen this SVID before". It now checks whether
      partner_count_for(svid) >= port_count_for(svid) and rejects only the
      surplus.
    - For UCSI_RECIPIENT_CON and UCSI_RECIPIENT_SOP_P, behavior is
      unchanged: still SVID+VDO exact-dup match.
    - Commit message rewritten to spell out the per-recipient rules and
      why partner needs the cardinality cap (avoids both the SVID-only
      false positives that hurt the legitimate Dell 0x413c case and the
      SVID+VDO false negatives that let the Thunderbolt 0x8087 firmware
      bug through).
---
 drivers/usb/typec/ucsi/ucsi.c | 132 ++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 92166a3725b16..9cd7fd7a38521 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -529,6 +529,129 @@ static int ucsi_register_altmode(struct ucsi_connector *con,
 	return ret;
 }
 
+static void ucsi_dump_duplicate_altmode(struct ucsi_connector *con,
+					u8 recipient, u16 svid,
+					u32 existing_vdo, u32 new_vdo,
+					int offset)
+{
+	static const char * const recipient_names[] = {
+		[UCSI_RECIPIENT_CON]    = "port",
+		[UCSI_RECIPIENT_SOP]    = "partner",
+		[UCSI_RECIPIENT_SOP_P]  = "plug",
+		[UCSI_RECIPIENT_SOP_PP] = "cable plug prime",
+	};
+
+	dev_warn(con->ucsi->dev,
+		 "con%d: Firmware bug: duplicate %s altmode SVID 0x%04x at offset %d, ignoring.\n",
+		 con->num, recipient_names[recipient], svid, offset);
+
+	if (existing_vdo != new_vdo)
+		dev_warn(con->ucsi->dev,
+			 "con%d: VDO mismatch: 0x%08x vs 0x%08x\n",
+			 con->num, existing_vdo, new_vdo);
+}
+
+/* Count altmodes in @altmodes that advertise @svid. */
+static int ucsi_altmode_count_svid(struct typec_altmode **altmodes, u16 svid)
+{
+	int count = 0;
+	int k;
+
+	for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
+		if (!altmodes[k])
+			break;
+		if (altmodes[k]->svid == svid)
+			count++;
+	}
+
+	return count;
+}
+
+/*
+ * 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.
+ *
+ * The matching rules differ by recipient:
+ *
+ *   - UCSI_RECIPIENT_CON (port) and UCSI_RECIPIENT_SOP_P (plug):
+ *     Two altmodes with identical SVID and VDO are byte-for-byte duplicates
+ *     and the second has no observable function. Drop them.
+ *
+ *   - UCSI_RECIPIENT_SOP (partner):
+ *     The typec class binds each partner altmode to a port altmode of the
+ *     same SVID via altmode_match()/device_find_child(), which returns the
+ *     first port altmode with a matching SVID. If the partner advertises
+ *     more altmodes for SVID X than the port advertises, the surplus
+ *     partner altmode(s) collapse onto an already-paired port altmode and
+ *     trigger a "duplicate filename .../partner" sysfs error during
+ *     typec_altmode_create_links(). Use the port-side altmode count for
+ *     SVID X as the authoritative cap and reject any partner altmode that
+ *     would exceed it. This preserves legitimate multi-Mode partner
+ *     altmodes (e.g. vendor SVIDs that the port really does advertise
+ *     twice) while filtering the firmware-generated duplicates that have
+ *     no port counterpart.
+ */
+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;
+	int port_count, partner_count;
+	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) {
+			ucsi_dump_duplicate_altmode(con, recipient, svid,
+						    vdo, vdo, offset);
+			return true;
+		}
+	}
+
+	switch (recipient) {
+	case UCSI_RECIPIENT_SOP:
+		/*
+		 * Cap partner altmodes per SVID by the port-side count:
+		 * any further partner altmode for that SVID would alias an
+		 * already-paired port altmode and break typec sysfs.
+		 */
+		port_count = ucsi_altmode_count_svid(con->port_altmode, svid);
+		partner_count = ucsi_altmode_count_svid(con->partner_altmode,
+							svid);
+		if (port_count && partner_count >= port_count) {
+			ucsi_dump_duplicate_altmode(con, recipient, svid,
+						    con->partner_altmode[partner_count - 1]->vdo,
+						    vdo, offset);
+			return true;
+		}
+		return false;
+	case UCSI_RECIPIENT_CON:
+		altmodes = con->port_altmode;
+		break;
+	case UCSI_RECIPIENT_SOP_P:
+		altmodes = con->plug_altmode;
+		break;
+	default:
+		return false;
+	}
+
+	/* CON and SOP_P: drop only exact SVID+VDO duplicates. */
+	for (k = 0; k < UCSI_MAX_ALTMODES; k++) {
+		if (!altmodes[k])
+			break;
+
+		if (altmodes[k]->svid != svid || altmodes[k]->vdo != vdo)
+			continue;
+
+		ucsi_dump_duplicate_altmode(con, recipient, svid,
+					    altmodes[k]->vdo, vdo, offset);
+		return true;
+	}
+
+	return false;
+}
+
 static int
 ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
 {
@@ -653,6 +776,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


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

* Re: [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware
  2026-06-12  5:21 [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware Chia-Lin Kao (AceLan)
                   ` (2 preceding siblings ...)
  2026-06-12  5:21 ` [PATCH v6 3/3] usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode handling Chia-Lin Kao (AceLan)
@ 2026-06-12  5:32 ` AceLan Kao
  3 siblings, 0 replies; 7+ messages in thread
From: AceLan Kao @ 2026-06-12  5:32 UTC (permalink / raw)
  To: heikki.krogerus, gregkh
  Cc: bleung, jthies, myrrhperiwinkle, pooja.katiyar, abelvesa,
	yuanhsinte, johan, dmitry.baryshkov, linux-usb, linux-kernel

My email got rejected by some developers, so I manually reply to see
if it works.

Chia-Lin Kao (AceLan) <acelan.kao@canonical.com> 於 2026年6月12日週五 下午1:21寫道:
>
> Some UCSI firmware implementations return the same altmode multiple
> times when queried with UCSI_GET_ALTERNATE_MODES.  When the resulting
> duplicate partner altmodes reach the typec class, two altmodes with the
> same SVID end up matched to the same port-side altmode by
> altmode_match()/device_find_child(), and the second registration
> collides on /sys/.../portN.M/partner with a sysfs duplicate-filename
> error and a probe failure (visible as a kernel call trace from
> typec_altmode_create_links()).
>
>   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
>
> This series teaches ucsi_register_altmodes() and
> ucsi_register_altmodes_nvidia() to filter these duplicates before they
> hit the typec class, replacing the call trace with a single dev_warn().
> It also retires the open-coded workaround that ucsi_yoga_c630.c was
> using for the same firmware bug now that the core handles it.
>
> The matching rules differ by recipient, because the pairing semantics
> in the typec class differ:
>
>   - UCSI_RECIPIENT_CON  (port) and
>     UCSI_RECIPIENT_SOP_P (plug):
>     Drop only byte-for-byte duplicates (same SVID and same VDO);
>     a second copy adds nothing.
>
>   - UCSI_RECIPIENT_SOP  (partner):
>     A partner altmode is paired with a port altmode of the same SVID
>     via altmode_match()/device_find_child(), which returns the first
>     port altmode with the matching SVID.  The number of partner
>     altmodes that can coexist for a given SVID is therefore bounded
>     by the number of port altmodes the firmware advertises for that
>     SVID.  Use that port-side count as the cap and reject any partner
>     altmode that would exceed it.
>
> Why cardinality and not just SVID+VDO?  A plain SVID+VDO comparison is
> too lax for the partner case: real firmware in the wild reports two
> "partner" Thunderbolt altmodes (SVID 0x8087) with different VDOs while
> the port side only advertises one, and both would still collapse onto
> the same port altmode and trip the sysfs collision.  A plain SVID
> comparison is the other extreme: it would falsely drop the second mode
> of vendor SVIDs that the port really does advertise twice (for example,
> Dell's 0x413c which legitimately registers as port1.1 and port1.2 with
> two distinct VDOs).  The cardinality cap captures both cases correctly
> and is consistent with how the typec class actually pairs altmodes.
>
> Tested on two Dell platforms:
>
>   * Dell platform A: firmware reports two partner altmodes for SVID
>     0x8087 (Thunderbolt) against a single port-side 0x8087, and the
>     same port also advertises two distinct 0x413c altmodes
>     (port1.1 and port1.2) paired with two 0x413c partner altmodes.
>     Before: sysfs duplicate-filename error and probe failure on
>     portN-partner.1 / typec-thunderbolt.
>     After: the surplus 0x8087 partner altmode is dropped with a
>     clean dev_warn(); typec-thunderbolt probes once and the dock
>     works, and both legitimate 0x413c partner altmodes still
>     register as portN-partner.1 and portN-partner.2.  DisplayPort
>     (0xff01) is unaffected.
>
>   * Dell platform B: firmware reports a single 0x8087 partner
>     altmode matching its single port-side 0x8087 (no firmware bug
>     present).  Used as a regression-check baseline.
>     After: behaviour unchanged; no altmodes are spuriously dropped.
>
> v5 -> v6:
>   - Replace the partner-side SVID+VDO duplicate check with a
>     cardinality cap: skip a partner altmode for SVID X once the
>     number of already-registered partner altmodes for X has reached
>     the number of port altmodes for X.  This is the only rule that
>     is consistent with how the typec class pairs partner-to-port
>     altmodes, and is the change that actually fixes the Thunderbolt
>     case while keeping the Dell case working.
>   - Add a small ucsi_altmode_count_svid() helper used by the
>     cardinality check.
>   - Move the recipient-name table into ucsi_dump_duplicate_altmode()
>     as a function-local static const; it has no other users.
>   - Expand the kernel-doc block on ucsi_altmode_is_duplicate() to
>     document the per-recipient rules and the rationale.
>   - Reword patch 3/3 to use the canonical 'commit <sha12> ("<title>")'
>     style for the reference to the original yoga_c630 workaround.
>   - Tested on the two platforms described above (TBT firmware bug
>     machine and Dell multi-Mode 0x413c machine); no regression on
>     either, call trace gone on the TBT machine.
>
> v4 -> v5:
>   - Add ucsi_dump_duplicate_altmode() helper to consolidate the
>     duplicate-detected dev_warn() output; all three previous
>     open-coded warning callsites (batch-internal, partner already
>     registered, port/plug already registered) route through it.
>   - Add ucsi_recipient_names[] lookup table and use it in the new
>     helper instead of repeating the switch over recipient values
>     inside ucsi_altmode_is_duplicate() and the warning strings.
>     (v6 later moves this table into the helper itself as a
>     function-local static const; see "Changes since v5".)
>   - Split the user-visible warning into two shorter lines:
>       "Firmware bug: duplicate <recipient> altmode SVID 0xNNNN at
>        offset N, ignoring."
>       "VDO mismatch: 0xNNNNNNNN vs 0xNNNNNNNN" (only emitted when
>       the two VDOs actually differ).
>     The previous single line ran past 75 chars and embedded the
>     VDOs and a "Please update your system firmware." trailer.
>
> v3 -> v4:
>   - Rebased onto the latest tree.
>
> v2 -> v3:
>   - Move ucsi_altmode_is_duplicate() above
>     ucsi_register_altmodes_nvidia() so the nvidia path can call it
>     too (groundwork for the nvidia-path fix that later lands as
>     patch 2/3).
>   - Use struct typec_altmode ** to walk the already-registered
>     altmode arrays (port_altmode / partner_altmode / plug_altmode)
>     instead of three separate branches; simplifies the dedup logic.
>
> v1 -> v2:
>   - Split the single patch into a 3-patch series:
>       1/3 core helper + standard registration path,
>       2/3 same dedup applied in the nvidia registration path so
>           all drivers benefit, not just those on the standard path,
>       3/3 retire the open-coded duplicate filter in ucsi_yoga_c630.c
>           now that the core covers it.
>   - Extend ucsi_altmode_is_duplicate() to cover all three recipient
>     types (CON / SOP / SOP_P), not just SOP.
>   - Improve the warning message to also include the differing VDOs
>     when a partner altmode collides on SVID with a different VDO,
>     so the firmware bug is actually diagnosable from dmesg.
>   - Print a single dev_warn() per skipped altmode instead of letting
>     the registration path generate a sysfs duplicate-filename call
>     trace.
>
> Previous revisions:
>   [v1] https://lore.kernel.org/lkml/20251016055332.914106-1-acelan.kao@canonical.com/
>   [v2] https://lore.kernel.org/lkml/20251111010541.145421-1-acelan.kao@canonical.com/
>   [v3] https://lore.kernel.org/lkml/20251224070022.4082182-1-acelan.kao@canonical.com/
>   [v4] https://lore.kernel.org/lkml/20260413073552.894395-1-acelan.kao@canonical.com/
>   [v5] https://lore.kernel.org/lkml/20260504115927.48925-1-acelan.kao@canonical.com/
>
> Some UCSI firmware implementations return the same altmode multiple
> times when queried with UCSI_GET_ALTERNATE_MODES.  When the resulting
> duplicate partner altmodes reach the typec class, two altmodes with the
> same SVID end up matched to the same port-side altmode by
> altmode_match()/device_find_child(), and the second registration
> collides on /sys/.../portN.M/partner with a sysfs duplicate-filename
> error and a probe failure (visible as a kernel call trace from
> typec_altmode_create_links()).
>
>   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
>
> This series teaches ucsi_register_altmodes() and
> ucsi_register_altmodes_nvidia() to filter these duplicates before they
> hit the typec class, replacing the call trace with a single dev_warn().
> It also retires the open-coded workaround that ucsi_yoga_c630.c was
> using for the same firmware bug now that the core handles it.
>
> The matching rules differ by recipient, because the pairing semantics
> in the typec class differ:
>
>   - UCSI_RECIPIENT_CON  (port) and
>     UCSI_RECIPIENT_SOP_P (plug):
>     Drop only byte-for-byte duplicates (same SVID and same VDO);
>     a second copy adds nothing.
>
>   - UCSI_RECIPIENT_SOP  (partner):
>     A partner altmode is paired with a port altmode of the same SVID
>     via altmode_match()/device_find_child(), which returns the first
>     port altmode with the matching SVID.  The number of partner
>     altmodes that can coexist for a given SVID is therefore bounded
>     by the number of port altmodes the firmware advertises for that
>     SVID.  Use that port-side count as the cap and reject any partner
>     altmode that would exceed it.
>
> Why cardinality and not just SVID+VDO?  A plain SVID+VDO comparison is
> too lax for the partner case: real firmware in the wild reports two
> "partner" Thunderbolt altmodes (SVID 0x8087) with different VDOs while
> the port side only advertises one, and both would still collapse onto
> the same port altmode and trip the sysfs collision.  A plain SVID
> comparison is the other extreme: it would falsely drop the second mode
> of vendor SVIDs that the port really does advertise twice (for example,
> Dell's 0x413c which legitimately registers as port1.1 and port1.2 with
> two distinct VDOs).  The cardinality cap captures both cases correctly
> and is consistent with how the typec class actually pairs altmodes.
>
> Tested on two Dell platforms:
>
>   * Dell platform A: firmware reports two partner altmodes for SVID
>     0x8087 (Thunderbolt) against a single port-side 0x8087, and the
>     same port also advertises two distinct 0x413c altmodes
>     (port1.1 and port1.2) paired with two 0x413c partner altmodes.
>     Before: sysfs duplicate-filename error and probe failure on
>     portN-partner.1 / typec-thunderbolt.
>     After: the surplus 0x8087 partner altmode is dropped with a
>     clean dev_warn(); typec-thunderbolt probes once and the dock
>     works, and both legitimate 0x413c partner altmodes still
>     register as portN-partner.1 and portN-partner.2.  DisplayPort
>     (0xff01) is unaffected.
>
>   * Dell platform B: firmware reports a single 0x8087 partner
>     altmode matching its single port-side 0x8087 (no firmware bug
>     present).  Used as a regression-check baseline.
>     After: behaviour unchanged; no altmodes are spuriously dropped.
>
> v5 -> v6:
>   - Replace the partner-side SVID+VDO duplicate check with a
>     cardinality cap: skip a partner altmode for SVID X once the
>     number of already-registered partner altmodes for X has reached
>     the number of port altmodes for X.  This is the only rule that
>     is consistent with how the typec class pairs partner-to-port
>     altmodes, and is the change that actually fixes the Thunderbolt
>     case while keeping the Dell case working.
>   - Add a small ucsi_altmode_count_svid() helper used by the
>     cardinality check.
>   - Move the recipient-name table into ucsi_dump_duplicate_altmode()
>     as a function-local static const; it has no other users.
>   - Expand the kernel-doc block on ucsi_altmode_is_duplicate() to
>     document the per-recipient rules and the rationale.
>   - Reword patch 3/3 to use the canonical 'commit <sha12> ("<title>")'
>     style for the reference to the original yoga_c630 workaround.
>   - Tested on the two platforms described above (TBT firmware bug
>     machine and Dell multi-Mode 0x413c machine); no regression on
>     either, call trace gone on the TBT machine.
>
> v4 -> v5:
>   - Add ucsi_dump_duplicate_altmode() helper to consolidate the
>     duplicate-detected dev_warn() output; all three previous
>     open-coded warning callsites (batch-internal, partner already
>     registered, port/plug already registered) route through it.
>   - Add ucsi_recipient_names[] lookup table and use it in the new
>     helper instead of repeating the switch over recipient values
>     inside ucsi_altmode_is_duplicate() and the warning strings.
>     (v6 later moves this table into the helper itself as a
>     function-local static const; see "Changes since v5".)
>   - Split the user-visible warning into two shorter lines:
>       "Firmware bug: duplicate <recipient> altmode SVID 0xNNNN at
>        offset N, ignoring."
>       "VDO mismatch: 0xNNNNNNNN vs 0xNNNNNNNN" (only emitted when
>       the two VDOs actually differ).
>     The previous single line ran past 75 chars and embedded the
>     VDOs and a "Please update your system firmware." trailer.
>
> v3 -> v4:
>   - Rebased onto the latest tree.
>
> v2 -> v3:
>   - Move ucsi_altmode_is_duplicate() above
>     ucsi_register_altmodes_nvidia() so the nvidia path can call it
>     too (groundwork for the nvidia-path fix that later lands as
>     patch 2/3).
>   - Use struct typec_altmode ** to walk the already-registered
>     altmode arrays (port_altmode / partner_altmode / plug_altmode)
>     instead of three separate branches; simplifies the dedup logic.
>
> v1 -> v2:
>   - Split the single patch into a 3-patch series:
>       1/3 core helper + standard registration path,
>       2/3 same dedup applied in the nvidia registration path so
>           all drivers benefit, not just those on the standard path,
>       3/3 retire the open-coded duplicate filter in ucsi_yoga_c630.c
>           now that the core covers it.
>   - Extend ucsi_altmode_is_duplicate() to cover all three recipient
>     types (CON / SOP / SOP_P), not just SOP.
>   - Improve the warning message to also include the differing VDOs
>     when a partner altmode collides on SVID with a different VDO,
>     so the firmware bug is actually diagnosable from dmesg.
>   - Print a single dev_warn() per skipped altmode instead of letting
>     the registration path generate a sysfs duplicate-filename call
>     trace.
>
> Previous revisions:
>   [v1] https://lore.kernel.org/lkml/20251016055332.914106-1-acelan.kao@canonical.com/
>   [v2] https://lore.kernel.org/lkml/20251111010541.145421-1-acelan.kao@canonical.com/
>   [v3] https://lore.kernel.org/lkml/20251224070022.4082182-1-acelan.kao@canonical.com/
>   [v4] https://lore.kernel.org/lkml/20260413073552.894395-1-acelan.kao@canonical.com/
>   [v5] https://lore.kernel.org/lkml/20260504115927.48925-1-acelan.kao@canonical.com/
>
> Chia-Lin Kao (AceLan) (3):
>   usb: typec: ucsi: Detect and skip duplicate altmodes from buggy
>     firmware
>   usb: typec: ucsi: Add duplicate detection to nvidia registration path
>   usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode
>     handling
>
>  drivers/usb/typec/ucsi/ucsi.c           | 158 ++++++++++++++++++++++--
>  drivers/usb/typec/ucsi/ucsi_yoga_c630.c |  23 ----
>  2 files changed, 148 insertions(+), 33 deletions(-)
>
>
> base-commit: ec039126b7fac4e3af35ebccaa7c6f9b6875ba81
> --
> 2.53.0
>


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

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

* Re: [PATCH v6 1/3] usb: typec: ucsi: Detect and skip duplicate altmodes from buggy firmware
  2026-06-12  5:21 ` [PATCH v6 1/3] usb: typec: ucsi: Detect and skip " Chia-Lin Kao (AceLan)
@ 2026-07-08 11:45   ` Greg KH
  0 siblings, 0 replies; 7+ messages in thread
From: Greg KH @ 2026-07-08 11:45 UTC (permalink / raw)
  To: Chia-Lin Kao (AceLan)
  Cc: heikki.krogerus, bleung, jthies, myrrhperiwinkle, pooja.katiyar,
	abelvesa, yuanhsinte, johan, dmitry.baryshkov, linux-usb,
	linux-kernel

On Fri, Jun 12, 2026 at 01:21:10PM +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
> 
> The matching rules differ by recipient:
> 
>   - UCSI_RECIPIENT_CON (port) and UCSI_RECIPIENT_SOP_P (plug):
>     Two altmodes with identical SVID and VDO are byte-for-byte
>     duplicates and the second has no observable function, so drop it.
> 
>   - UCSI_RECIPIENT_SOP (partner):
>     The typec class binds each partner altmode to a port altmode of
>     the same SVID via altmode_match()/device_find_child(), which
>     returns the first port altmode with a matching SVID.  If the
>     partner advertises more altmodes for SVID X than the port
>     advertises, the surplus partner altmode(s) collapse onto an
>     already-paired port altmode and trigger the
>     "duplicate filename .../partner" sysfs error during
>     typec_altmode_create_links().  Use the port-side altmode count for
>     SVID X as the authoritative cap and reject any partner altmode
>     that would exceed it.  This preserves legitimate multi-Mode
>     partner altmodes (vendor SVIDs that the port really does
>     advertise more than once) while filtering the firmware-generated
>     duplicates that have no port counterpart, and is therefore
>     stricter than a plain SVID+VDO comparison (which still admits the
>     Thunderbolt case where firmware reports the same SVID twice with
>     different VDOs) without being over-broad like a plain SVID match
>     (which would falsely drop legitimate vendor multi-Mode entries).
> 
> If a duplicate is detected, skip it and emit a clean warning instead
> of generating a kernel call trace:
> 
>   ucsi_acpi USBC000:00: con2: Firmware bug: duplicate partner altmode SVID 0x8087 at offset 1, ignoring.
>   ucsi_acpi USBC000:00: con2: VDO mismatch: 0x8087a043 vs 0x00000001

Can you change the ", ignoring." to ", ignoring but please contact the
BIOS vendor to fix this issue." as they need to get told this, otherwise
this looks like it is a kernel issue.

thanks,

greg k-h

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

end of thread, other threads:[~2026-07-08 11:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12  5:21 [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware Chia-Lin Kao (AceLan)
2026-06-12  5:21 ` [PATCH v6 1/3] usb: typec: ucsi: Detect and skip " Chia-Lin Kao (AceLan)
2026-07-08 11:45   ` Greg KH
2026-06-12  5:21 ` [PATCH v6 2/3] usb: typec: ucsi: Add duplicate detection to nvidia registration path Chia-Lin Kao (AceLan)
2026-06-12  5:21 ` [PATCH v6 3/3] usb: typec: ucsi: yoga_c630: Remove redundant duplicate altmode handling Chia-Lin Kao (AceLan)
2026-06-12  5:32 ` [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware AceLan Kao
  -- strict thread matches above, loose matches on Subject: below --
2026-06-12  5:28 Chia-Lin Kao (AceLan)
2026-06-12  5:28 ` [PATCH v6 1/3] usb: typec: ucsi: Detect and skip " Chia-Lin Kao (AceLan)

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