Skip to content

SM8750: fix suspend wedge from ucsi USB role on charger unplug#3039

Open
aanze wants to merge 1 commit into
ROCKNIX:nextfrom
aanze:aanze-pr-ucsi-suspend-fix
Open

SM8750: fix suspend wedge from ucsi USB role on charger unplug#3039
aanze wants to merge 1 commit into
ROCKNIX:nextfrom
aanze:aanze-pr-ucsi-suspend-fix

Conversation

@aanze

@aanze aanze commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • What is the goal of this PR? Fix a hard SoC hang on SM8750
    (AYN Odin 3) when the USB cable is unplugged while the device is
    suspended — both for chargers (patch 0509) and for data-capable
    partners such as docks (patch 0510).

Testing

  • How was this tested? Built the SM8750 kernel with these patches
    and flashed it to an AYN Odin 3. Reproduced the original hang on
    stock 7.1.x, then ran the patched kernel through several
    suspend/unplug cycles with a charger and with a dock.
  • Test results: Before — plug charger or dock, suspend, then
    unplug while asleep hard-hangs the SoC (dead screen, runs hot,
    forced reset needed) on every 7.1.x; 7.0.x is fine. After — suspends
    and resumes normally with the cable or dock unplugged during
    suspend, over several cycles each.

Additional Context

  • Root cause: commit 0c8ee850572 ("usb: typec: ucsi: Add
    UCSI_USB4_IMPLIES_USB quirk for X1E80100") gated the "partner has no
    USB data -> USB_ROLE_NONE" decision behind the UCSI_USB4_IMPLIES_USB
    quirk. SM8750's pmic_glink UCSI never sets that quirk, so the USB
    role is no longer cleared for a charger-only partner and stays
    attached across suspend, keeping the USB power island up; the
    firmware power-collapse then hangs on the cable event.
  • Patch 0509 restores the pre-0c8ee850572 behaviour (charger case).
    Patch 0510 extends the same fix to data-capable partners: the role
    is forced to USB_ROLE_NONE at suspend entry and restored at resume,
    so the power island can collapse with a dock attached too.

AI Usage

Did you use AI tools to help write this code? YES

@andrebraga

Copy link
Copy Markdown
Contributor

0509 is confirmed (manual inspection), 0510 is debatable (and has potential bugs such as use-after-free). I'd split the patches into different commits.

@aanze

aanze commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Sure, after all it's a matter of choice, but without 0510, device will absolutely wedge if suspended then uplugged from any usb data capable peripheral

@andrebraga

andrebraga commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

I don't have an issue with 0510's intentions, only with the current implementation, which seems incomplete. Namely:

  • global-static singleton
  • register_pm_notifier with no teardown: reachable use-after-free on ADSP SSR
  • unsynchronized usb_role_switch_set_role at PM_SUSPEND_PREPARE racing the connector workqueue
  • blind stale-role restore at resume

I understand "experiment" is there meaning "trial", but it's an interesting choice of words regardless... Knowing the pattern-matchey nature of transformers I couldn't help it but wonder about a possible "subtext", and no big deal at all, as I see it Fable gave it a measure of effort sufficient for ruling out some things while testing other hypotheses, and now it needs the correction and robustness pass.

@aanze

aanze commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the input, will work on a cleaner 0510

Add a SM8750 kernel patch reverting the u_role gating introduced by
0c8ee850572 ("usb: typec: ucsi: Add UCSI_USB4_IMPLIES_USB quirk for
X1E80100"). Without that quirk the USB data role is never cleared for
a charger-only partner and stays attached across suspend, keeping the
USB power island up; the firmware power-collapse then hard-hangs the
SoC when the cable is unplugged while suspended. Device-tested on the
AYN Odin 3.
@aanze

aanze commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Done — split as suggested: this PR is now 0509 only. 0510 is fully reworked in #3049: per-instance state in struct ucsi, synchronous unregister in ucsi_unregister() (the PDR/SSR path), role writes under con->lock + a suspend flag closing the workqueue race, and no blind restore — the role is re-derived from a fresh GET_CONNECTOR_STATUS at resume. Re-tested on device, including dock kept attached across suspend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants