Re: [PATCH v6 0/3] usb: typec: ucsi: Filter duplicate altmodes from buggy firmware

[AceLan Kao]

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

Chia-Lin Kao (AceLan) <acelan.kao@xxxxxxxxxxxxx> 於 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@xxxxxxxxxxxxx/
>   [v2] https://lore.kernel.org/lkml/20251111010541.145421-1-acelan.kao@xxxxxxxxxxxxx/
>   [v3] https://lore.kernel.org/lkml/20251224070022.4082182-1-acelan.kao@xxxxxxxxxxxxx/
>   [v4] https://lore.kernel.org/lkml/20260413073552.894395-1-acelan.kao@xxxxxxxxxxxxx/
>   [v5] https://lore.kernel.org/lkml/20260504115927.48925-1-acelan.kao@xxxxxxxxxxxxx/
>
> 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@xxxxxxxxxxxxx/
>   [v2] https://lore.kernel.org/lkml/20251111010541.145421-1-acelan.kao@xxxxxxxxxxxxx/
>   [v3] https://lore.kernel.org/lkml/20251224070022.4082182-1-acelan.kao@xxxxxxxxxxxxx/
>   [v4] https://lore.kernel.org/lkml/20260413073552.894395-1-acelan.kao@xxxxxxxxxxxxx/
>   [v5] https://lore.kernel.org/lkml/20260504115927.48925-1-acelan.kao@xxxxxxxxxxxxx/
>
> 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/@/)