[PATCH RFC] usb: typec: ucsi: Schedule connector worker on freezable workqueue

[PATCH RFC] usb: typec: ucsi: Schedule connector worker on freezable workqueue

[Abel Vesa]

Currently, the UCSI connector worker is scheduled on the non-freezable
system workqueue. During system suspend, on a plug/unplug event, the
worker can run before the devices have actually resumed. The UCSI
instances can implement operations that might need to do some HW accesses
while the devices are still suspended.

Scheduling the USCI connector worker on the freezable system workqueue
instead will ensure the devices are resumed by the time the worker is
scheduled to run.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
Sending this as an RFC since I'm not really sure if this should be done
in the ucsi generic implementation or in the pmic glink UCSI instance.

For context, on some Qualcomm Snapdragon X Elite laptops, there are some
i2c interfaced USB Type-C retimers (ParadeTech PS8830) that need to be
configured on each plug/unplug event. Since the i2c controller is
suspended when the UCSI connector worker gets scheduled, it results in
the following:

[   70.036669] i2c i2c-4: Transfer while suspended
[   70.036802] WARNING: CPU: 0 PID: 819 at drivers/i2c/i2c-core.h:56 __i2c_transfer+0xb4/0x57c [i2c_core]
[   70.036945] CPU: 0 UID: 0 PID: 819 Comm: kworker/0:4 Tainted: G        W          6.13.0+ #84
[   70.036949] Tainted: [W]=WARN
[   70.036950] Hardware name: LENOVO 21N10007UK/21N10007UK, BIOS N42ET85W (2.15 ) 11/22/2024
[   70.036952] Workqueue: events ucsi_handle_connector_change [typec_ucsi]
[   70.036959] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[   70.036961] pc : __i2c_transfer+0xb4/0x57c [i2c_core]
[   70.036963] lr : __i2c_transfer+0xb0/0x57c [i2c_core]
[   70.036964] sp : ffff800082ffba90
[   70.036966] x29: ffff800082ffba90 x28: 0000000000000000 x27: ffff1cc400dba0a0
[   70.036969] x26: 0000000000000000 x25: ffff1cc4034bd500 x24: ffff1cc400010005
[   70.036970] x23: 0000000000000000 x22: ffff1cc400dba0a1 x21: 0000000000000001
[   70.036972] x20: ffff1cc4011ab0f0 x19: ffff1cc4011ab160 x18: 000000000009eb8a
[   70.036974] x17: 00000005bf44b304 x16: 00000000000000cc x15: 0000000000000004
[   70.036976] x14: ffffde65ecd46798 x13: 0000000000000fff x12: 0000000000000003
[   70.036978] x11: ffff3e658f1f7000 x10: 00000000ffffffff x9 : 340eced73efb4000
[   70.036980] x8 : 340eced73efb4000 x7 : 656c696877207265 x6 : 66736e617254203a
[   70.036982] x5 : ffffde65ece89084 x4 : ffffde65c9120093 x3 : 0000000000000000
[   70.036984] x2 : ffff800082ffb854 x1 : 00000000000000c0 x0 : 00000000ffffff94
[   70.036987] Call trace:
[   70.036989]  __i2c_transfer+0xb4/0x57c [i2c_core] (P)
[   70.036994]  i2c_transfer+0x98/0xf0 [i2c_core]
[   70.036995]  i2c_transfer_buffer_flags+0x54/0x88 [i2c_core]
[   70.036997]  regmap_i2c_write+0x20/0x48 [regmap_i2c]
[   70.037001]  _regmap_raw_write_impl+0x780/0x944
[   70.037012]  _regmap_bus_raw_write+0x60/0x7c
[   70.037014]  _regmap_write+0x134/0x184
[   70.037016]  regmap_write+0x54/0x78
[   70.037018]  ps883x_set+0x58/0xec [ps883x]
[   70.037021]  ps883x_sw_set+0x60/0x84 [ps883x]
[   70.037022]  typec_switch_set+0x48/0x74 [typec]
[   70.037026]  typec_set_orientation+0x24/0x6c [typec]
[   70.037027]  pmic_glink_ucsi_connector_status+0x30/0x7c [ucsi_glink]
[   70.037032]  ucsi_handle_connector_change+0x98/0x614 [typec_ucsi]
[   70.037034]  process_scheduled_works+0x1a0/0x2d0
[   70.037045]  worker_thread+0x2a8/0x3c8
[   70.037046]  kthread+0xfc/0x184
[   70.037048]  ret_from_fork+0x10/0x20
---
 drivers/usb/typec/ucsi/ucsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index fcf499cc9458c0d12015a7e36e5f1ac448c3a431..8c6081e0cd6155a59ca733070cd93e6b79398b3e 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1307,7 +1307,7 @@ void ucsi_connector_change(struct ucsi *ucsi, u8 num)
 	}
 
 	if (!test_and_set_bit(EVENT_PENDING, &ucsi->flags))
-		schedule_work(&con->work);
+		queue_work(system_freezable_wq, &con->work);
 }
 EXPORT_SYMBOL_GPL(ucsi_connector_change);
 

---
base-commit: 00f3246adeeacbda0bd0b303604e46eb59c32e6e
change-id: 20250205-ucsi-schedule-conn-worker-on-freezable-wq-cff0a880b08b

Best regards,
-- 
Abel Vesa <abel.vesa@linaro.org>

Re: [PATCH RFC] usb: typec: ucsi: Schedule connector worker on freezable workqueue

[Dmitry Baryshkov]

On Wed, Feb 05, 2025 at 01:45:05PM +0200, Abel Vesa wrote:
> Currently, the UCSI connector worker is scheduled on the non-freezable
> system workqueue. During system suspend, on a plug/unplug event, the
> worker can run before the devices have actually resumed. The UCSI
> instances can implement operations that might need to do some HW accesses
> while the devices are still suspended.
> 
> Scheduling the USCI connector worker on the freezable system workqueue
> instead will ensure the devices are resumed by the time the worker is
> scheduled to run.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> Sending this as an RFC since I'm not really sure if this should be done
> in the ucsi generic implementation or in the pmic glink UCSI instance.

Can we really fix PMIC GLINK instead, please? Previously you were saying
that it for some reason has to be woken up earlier.

> 
> For context, on some Qualcomm Snapdragon X Elite laptops, there are some
> i2c interfaced USB Type-C retimers (ParadeTech PS8830) that need to be
> configured on each plug/unplug event. Since the i2c controller is
> suspended when the UCSI connector worker gets scheduled, it results in
> the following:
> 
> [   70.036669] i2c i2c-4: Transfer while suspended
> [   70.036802] WARNING: CPU: 0 PID: 819 at drivers/i2c/i2c-core.h:56 __i2c_transfer+0xb4/0x57c [i2c_core]
> [   70.036945] CPU: 0 UID: 0 PID: 819 Comm: kworker/0:4 Tainted: G        W          6.13.0+ #84
> [   70.036949] Tainted: [W]=WARN
> [   70.036950] Hardware name: LENOVO 21N10007UK/21N10007UK, BIOS N42ET85W (2.15 ) 11/22/2024
> [   70.036952] Workqueue: events ucsi_handle_connector_change [typec_ucsi]
> [   70.036959] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [   70.036961] pc : __i2c_transfer+0xb4/0x57c [i2c_core]
> [   70.036963] lr : __i2c_transfer+0xb0/0x57c [i2c_core]
> [   70.036964] sp : ffff800082ffba90
> [   70.036966] x29: ffff800082ffba90 x28: 0000000000000000 x27: ffff1cc400dba0a0
> [   70.036969] x26: 0000000000000000 x25: ffff1cc4034bd500 x24: ffff1cc400010005
> [   70.036970] x23: 0000000000000000 x22: ffff1cc400dba0a1 x21: 0000000000000001
> [   70.036972] x20: ffff1cc4011ab0f0 x19: ffff1cc4011ab160 x18: 000000000009eb8a
> [   70.036974] x17: 00000005bf44b304 x16: 00000000000000cc x15: 0000000000000004
> [   70.036976] x14: ffffde65ecd46798 x13: 0000000000000fff x12: 0000000000000003
> [   70.036978] x11: ffff3e658f1f7000 x10: 00000000ffffffff x9 : 340eced73efb4000
> [   70.036980] x8 : 340eced73efb4000 x7 : 656c696877207265 x6 : 66736e617254203a
> [   70.036982] x5 : ffffde65ece89084 x4 : ffffde65c9120093 x3 : 0000000000000000
> [   70.036984] x2 : ffff800082ffb854 x1 : 00000000000000c0 x0 : 00000000ffffff94
> [   70.036987] Call trace:
> [   70.036989]  __i2c_transfer+0xb4/0x57c [i2c_core] (P)
> [   70.036994]  i2c_transfer+0x98/0xf0 [i2c_core]
> [   70.036995]  i2c_transfer_buffer_flags+0x54/0x88 [i2c_core]
> [   70.036997]  regmap_i2c_write+0x20/0x48 [regmap_i2c]
> [   70.037001]  _regmap_raw_write_impl+0x780/0x944
> [   70.037012]  _regmap_bus_raw_write+0x60/0x7c
> [   70.037014]  _regmap_write+0x134/0x184
> [   70.037016]  regmap_write+0x54/0x78
> [   70.037018]  ps883x_set+0x58/0xec [ps883x]
> [   70.037021]  ps883x_sw_set+0x60/0x84 [ps883x]
> [   70.037022]  typec_switch_set+0x48/0x74 [typec]
> [   70.037026]  typec_set_orientation+0x24/0x6c [typec]
> [   70.037027]  pmic_glink_ucsi_connector_status+0x30/0x7c [ucsi_glink]
> [   70.037032]  ucsi_handle_connector_change+0x98/0x614 [typec_ucsi]
> [   70.037034]  process_scheduled_works+0x1a0/0x2d0
> [   70.037045]  worker_thread+0x2a8/0x3c8
> [   70.037046]  kthread+0xfc/0x184
> [   70.037048]  ret_from_fork+0x10/0x20
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index fcf499cc9458c0d12015a7e36e5f1ac448c3a431..8c6081e0cd6155a59ca733070cd93e6b79398b3e 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1307,7 +1307,7 @@ void ucsi_connector_change(struct ucsi *ucsi, u8 num)
>  	}
>  
>  	if (!test_and_set_bit(EVENT_PENDING, &ucsi->flags))
> -		schedule_work(&con->work);
> +		queue_work(system_freezable_wq, &con->work);
>  }
>  EXPORT_SYMBOL_GPL(ucsi_connector_change);
>  
> 
> ---
> base-commit: 00f3246adeeacbda0bd0b303604e46eb59c32e6e
> change-id: 20250205-ucsi-schedule-conn-worker-on-freezable-wq-cff0a880b08b
> 
> Best regards,
> -- 
> Abel Vesa <abel.vesa@linaro.org>
> 

-- 
With best wishes
Dmitry

Re: [PATCH RFC] usb: typec: ucsi: Schedule connector worker on freezable workqueue

[Joonhoe Kim]

(Resending with the lists on Cc; my earlier reply only went to Abel.)

On Wed, Feb 05, 2025 at 01:45:05PM +0200, Abel Vesa wrote:
> Scheduling the USCI connector worker on the freezable system workqueue
> instead will ensure the devices are resumed by the time the worker is
> scheduled to run.

Hi Abel,

You mentioned respinning this together with the altmode change (now
7d0767c5cd87). Are you still planning to? I hit the same problem on a
Kaanapali tablet (Lenovo Legion Tab Y700 Gen 5) with an FSA4480
orientation switch. Unplugging a USB-C to HDMI adapter during s2idle,
with the altmode change applied:

  i2c i2c-2: Transfer while suspended
  Workqueue: events ucsi_handle_connector_change
   fsa4480_set
   typec_switch_set
   typec_set_orientation
   pmic_glink_ucsi_connector_status
   ucsi_handle_connector_change

and the system never resumes. With this patch on v7.3-rc4 it resumes
normally (2 of 2) and the change is handled after thaw.

On Dmitry's question: freezing the ucsi_glink notify_work instead would
also hold back command completions (ucsi_notify_common() ->
complete()), so a command in flight at freeze time would wait for
UCSI_TIMEOUT_MS. That is from reading the code, not tested.

If you respin it, I can reply with a Tested-by. If you'd rather I send
it, I have it rebased on next-20260928; just tell me how to credit you.

Thanks,
Joonhoe Kim

Re: [PATCH RFC] usb: typec: ucsi: Schedule connector worker on freezable workqueue

[Vladyslav Tymoshchuk]

Hi Abel, Joonhoe,

On Fri, 16 Oct 2026 21:50:44 +0900, Joonhoe Kim wrote:
> and the system never resumes. With this patch on v7.3-rc4 it resumes
> normally (2 of 2) and the change is handled after thaw.

I see the same on SM8750 (OnePlus Pad 3; the board DT is still local),
v7.3-rc5 plus board patches, but here it is the USB role switch that
breaks, not an i2c retimer or switch. The port is in device role,
connected to a PC. Unplugging the cable during s2idle resets the
board, every time I tried it without this change. My tree also has
local ucsi_glink and qcom_battmgr changes that make a cable plug or
unplug wake the system, similar to Jishnu Prakash's ucsi_glink patch.

A failing run, with times from "PM: resume from suspend-to-idle":

  +12.5 ms  ucsi_handle_connector_change() starts (noirq phase)
  +16.0 ms  GET_CONNECTOR_STATUS and ->connector_status() are done
  +16.8 ms  ucsi_partner_change() calls
            usb_role_switch_set_role(USB_ROLE_NONE)

Nothing is logged after that. The role switch is dwc3 with the
dwc3-qcom glue, and the glue's role notifier writes to QSCRATCH while
the glue clocks are still off. They only come back in
dwc3_qcom_pm_resume(), about 1 s after the resume message. The
pm_runtime_resume_and_get() in the notifier succeeds without doing
anything, as runtime PM is disabled at that point. In a debug build
that skips only the usb_role_switch_set_role() call, the same unplug
resumes fine. The GLINK side works in that window, as the UCSI
commands complete; what breaks is what the connector work calls into.

With the connector work on system_freezable_wq, both unplug runs
resumed (2 of 2). The work started 1.50 and 1.54 s after the resume
message, right after "Restarting tasks", and the partner was removed
without errors. I tested the same queue_work() change behind a module
parameter in my test build, not the patch as posted. The
pmic_glink_altmode worker was on the freezable workqueue too, as with
7d0767c5cd87, but it never runs here (no DP partner).

Plugging in during s2idle survives only because the PPM reports the
partner type in a later event, about 1.4 s after the first one, by
which time dwc3_qcom_pm_resume() has run. The first event still
registers the partner in the noirq phase, and the kernel warns:

  typec port0-partner: PM: parent port0 should not be sleeping

> On Dmitry's question: freezing the ucsi_glink notify_work instead would
> also hold back command completions (ucsi_notify_common() ->
> complete()), so a command in flight at freeze time would wait for
> UCSI_TIMEOUT_MS. That is from reading the code, not tested.

One more data point on that question. Here the unplug notification has
to get through during s2idle, as it is what wakes the system, and
GLINK itself works in that window. Only the connector work has to
wait. So a GLINK rework would need both parts that 7d0767c5cd87
describes: letting the notification wake the system, and queueing it
until the system has resumed. From reading the code, dropping
IRQF_NO_SUSPEND would not be enough on its own: the interrupt would
then be re-enabled at the end of the noirq phase, before
dwc3_qcom_pm_resume() runs in the normal resume phase. I can test such
a rework on this board. Until then, this change does for the connector
work what 7d0767c5cd87 does for the altmode worker.

Tested-by: Vladyslav Tymoshchuk <tvlad744@gmail.com> # OnePlus Pad 3 (SM8750)

Separately, I've sent a patch so that the dwc3-qcom role notifier does
not touch the hardware while the glue is suspended [1]; with it alone,
the unplug resumes here too (2 of 2). That does not help the PS8830
and FSA4480 cases, so this change is still needed. I can test a
respin on this board.

[1] https://lore.kernel.org/linux-usb/20261018143126.3978-1-tvlad744@gmail.com/

Thanks,
Vladyslav

(I used an AI assistant, Claude, for the analysis and to draft this
mail; the tests ran on my device.)