From cda0923e3d56e0e14b2e2b6852bd0012d3038de4 Mon Sep 17 00:00:00 2001 From: Erik Bourget Date: Wed, 28 Oct 2026 18:26:57 -0400 Subject: [PATCH 1/8] phy: apple: atc: Let an active T8132 USB3 link through suspend-to-idle On the J713 the SuperSpeed port refuses system sleep for as long as any USB device is attached: atcphy_suspend() returns -EBUSY while the PHY owns its common block, so a laptop with a USB keyboard on that port never suspends: PM: dpm_run_callback(): atcphy_suspend returns -16 PM: Some devices failed to suspend, or early wake event detected Suspend-to-idle does not take the link down. While its core runs, DWC3 is a syscore device and keeps the shared power domain on across system sleep, and xHCI keeps running so that a USB device can wake the system. The PHY keeps its state with the domain, and resume restores it only if the domain went off. Allow suspend-to-idle with an active common epoch while a host runs on it. Keep refusing one without a host, as in a USB3 device role, where nothing holds the domain; an incomplete one; and any deeper sleep state, which would lose the link. Warn on resume if the domain went off under an active host link after all. A KUnit case covers these outcomes. Signed-off-by: Erik Bourget --- drivers/phy/apple/atc-test.c | 38 ++++++++++++++++++++++++++++++++++++ drivers/phy/apple/atc.c | 16 +++++++++++++-- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/drivers/phy/apple/atc-test.c b/drivers/phy/apple/atc-test.c index b64bcdbee1bb6..682e5a3809aa8 100644 --- a/drivers/phy/apple/atc-test.c +++ b/drivers/phy/apple/atc-test.c @@ -1158,6 +1158,41 @@ static void atcphy_test_t8132_rpm_busy(struct kunit *test) KUNIT_EXPECT_TRUE(test, atcphy_rpm_busy(atcphy)); } +#ifdef CONFIG_SUSPEND +/* An epoch with a host survives suspend-to-idle only; without one, or incomplete, never. */ +static void atcphy_test_t8132_system_suspend(struct kunit *test) +{ + struct apple_atcphy *atcphy = atcphy_test_alloc_powered_off(test); + suspend_state_t target = pm_suspend_target_state; + + atcphy->hw = &atcphy_hw_t8132; + atcphy->t8132_ss = true; + dev_set_drvdata(atcphy->dev, atcphy); + + pm_suspend_target_state = PM_SUSPEND_TO_IDLE; + atcphy->mode_dirty = true; + KUNIT_EXPECT_EQ(test, atcphy_suspend(atcphy->dev), -EBUSY); + atcphy->mode_dirty = false; + + /* Without a host, as in a USB3 device role, nothing holds the domain */ + atcphy->t8132_common_owned = true; + KUNIT_EXPECT_EQ(test, atcphy_suspend(atcphy->dev), -EBUSY); + KUNIT_EXPECT_FALSE(test, atcphy->t8132_suspended); + + atcphy->host_active = true; + KUNIT_EXPECT_EQ(test, atcphy_suspend(atcphy->dev), 0); + KUNIT_EXPECT_TRUE(test, atcphy->t8132_suspended); + KUNIT_EXPECT_EQ(test, atcphy_resume(atcphy->dev), 0); + KUNIT_EXPECT_FALSE(test, atcphy->t8132_suspended); + + pm_suspend_target_state = PM_SUSPEND_MEM; + KUNIT_EXPECT_EQ(test, atcphy_suspend(atcphy->dev), -EBUSY); + KUNIT_EXPECT_FALSE(test, atcphy->t8132_suspended); + + pm_suspend_target_state = target; +} +#endif + /* The hold follows the busy state, with one reference at most. */ static void atcphy_test_t8132_rpm_hold(struct kunit *test) { @@ -1356,6 +1391,9 @@ static struct kunit_case atcphy_t8132_pm_test_cases[] = { KUNIT_CASE(atcphy_test_t8132_rpm_hold), KUNIT_CASE(atcphy_test_t8132_restore_idle), KUNIT_CASE(atcphy_test_t8132_restore_mode), +#ifdef CONFIG_SUSPEND + KUNIT_CASE(atcphy_test_t8132_system_suspend), +#endif {} }; diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c index b4c91cf4d373c..9520c96b7e136 100644 --- a/drivers/phy/apple/atc.c +++ b/drivers/phy/apple/atc.c @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -6152,8 +6153,16 @@ static int atcphy_suspend(struct device *dev) return 0; guard(mutex)(&atcphy->lock); - /* Active and incomplete common epochs have no qualified sleep handoff. */ - if (atcphy->t8132_common_owned || atcphy->mode_dirty) + /* An incomplete common epoch has no qualified sleep handoff. */ + if (atcphy->mode_dirty) + return -EBUSY; + /* + * An active one survives suspend-to-idle while a host runs on it: DWC3 + * then keeps the power domain on across system sleep, so the link stays + * as it is. Without a host, or in a deeper sleep state, it is lost. + */ + if (atcphy->t8132_common_owned && + (!atcphy->host_active || pm_suspend_target_state != PM_SUSPEND_TO_IDLE)) return -EBUSY; /* Do not admit a new epoch after this check and before resume. */ atcphy->t8132_suspended = true; @@ -6172,6 +6181,9 @@ static int atcphy_resume(struct device *dev) guard(mutex)(&atcphy->lock); atcphy->t8132_suspended = false; + if (atcphy->t8132_common_owned && atcphy->host_active && + READ_ONCE(atcphy->pd_was_off)) + dev_warn(dev, "power domain went off under an active USB3 link\n"); /* * A suspended PHY holds nothing, and its domain may drop again once system * resume completes: leave the restore to its next operation. From 9ec757d1fbe6bddafbd0ef69b9fa318bd5da7377 Mon Sep 17 00:00:00 2001 From: Erik Bourget Date: Wed, 28 Oct 2026 19:04:44 -0400 Subject: [PATCH 2/8] usb: dwc3: apple: Keep the T8132 USB3 PHY out of P3 in host mode On the J713 a SuperSpeed link does not come back from U3 once the core has suspended the USB3 PHY with it (GUSB3PIPECTL.SUSPHY): resuming the port drops the link, and the whole tree behind it disconnects. This happens when a USB3 hub autosuspends, and on every resume from suspend-to-idle, where the USB core suspends all ports. A mounted disk behind a USB3 hub is removed under its filesystem. Clear SUSPHY for the USB3 PHY once the T8132 PIPE switch to USB3 is done, by setting dis_u3_susphy_quirk, which dwc3_enable_susphy() then honours when the xHCI host starts. The switch itself still runs with SUSPHY set, as it requires. Restore the device tree's setting before the host is stopped, which sets SUSPHY again: the PHY hands the PIPE back in P3, and fails to lock the PIPE handler otherwise. The core stop does the same for the error paths. USB2 PHY suspend is unchanged. The port routes the USB3 PIPE for any USB-C device in USB mode, so the USB3 PHY stays out of P3 while any USB device is attached, also a USB 2.0 one. The idle USB-C ports are powered down, so it costs nothing with nothing attached. The host test stubs of struct dwc3 and struct dwc3_apple gain dis_u3_susphy_quirk and dt_dis_u3_susphy, and the teardown test checks that the device tree's setting is back before the host exits. Signed-off-by: Erik Bourget --- drivers/usb/dwc3/dwc3-apple.c | 16 ++++++++++++++++ tools/asahi/neo-stability-tests.py | 4 ++-- .../selftests/drivers/gpu/apple-dcp/dwc3/mocks.c | 2 ++ .../drivers/gpu/apple-dcp/teardown_pm.py | 8 ++++---- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/drivers/usb/dwc3/dwc3-apple.c b/drivers/usb/dwc3/dwc3-apple.c index c11d299840d80..b45776269a087 100644 --- a/drivers/usb/dwc3/dwc3-apple.c +++ b/drivers/usb/dwc3/dwc3-apple.c @@ -125,6 +125,7 @@ * @idle_rpm: Let the controller runtime-suspend while the core is down * @rpm_held: The glue holds a runtime PM reference for the running core * @pre_core_rpm: Runtime PM is the glue's own until the first core bring-up + * @dt_dis_u3_susphy: The device tree keeps USB3 PHY suspend off itself * @drd_cfg: The controller's configuration block (optional, for its tunable) * @tunables: Controller tunables from the device tree, see dwc3_apple_tunable_list */ @@ -154,6 +155,7 @@ struct dwc3_apple { bool idle_rpm; bool rpm_held; bool pre_core_rpm; + bool dt_dis_u3_susphy; void __iomem *drd_cfg; struct apple_tunable *tunables[DWC3_APPLE_NR_TUNABLES]; @@ -435,6 +437,8 @@ static int dwc3_apple_core_stop(struct dwc3_apple *appledwc) lockdep_assert_held(&appledwc->lock); if (appledwc->core_active) { if (!appledwc->core_exit_pending) { + /* The PHY hands the PIPE back in P3, as it took it over */ + dwc->dis_u3_susphy_quirk = appledwc->dt_dis_u3_susphy; dwc3_enable_susphy(dwc, true); dwc3_event_buffers_cleanup(dwc); appledwc->core_exit_pending = true; @@ -619,6 +623,7 @@ static int dwc3_apple_set_role(struct dwc3_apple *appledwc, enum dwc3_apple_stat * device mode and the T8132 host role were selected in dwc3_apple_core_start(); * other host roles are selected by the xHCI root hub as it starts. */ + appledwc->dwc.dis_u3_susphy_quirk = appledwc->dt_dis_u3_susphy; dwc3_enable_susphy(&appledwc->dwc, true); ret = phy_set_mode(appledwc->usb3_phy, host ? PHY_MODE_USB_HOST : PHY_MODE_USB_DEVICE); if (ret) { @@ -626,6 +631,14 @@ static int dwc3_apple_set_role(struct dwc3_apple *appledwc, enum dwc3_apple_stat if (of_device_is_compatible(appledwc->dev->of_node, "apple,t8132-dwc3")) return ret; dev_warn(appledwc->dev, "USB3 PHY setup failed, USB2 only, err=%d\n", ret); + } else if (host && appledwc->usb3_phy && + of_device_is_compatible(appledwc->dev->of_node, "apple,t8132-dwc3")) { + /* + * The T8132 PHY does not bring a SuperSpeed link back from U3 once + * the core has suspended it: keep it out of P3 from here on. + */ + appledwc->dwc.dis_u3_susphy_quirk = true; + dwc3_enable_susphy(&appledwc->dwc, true); } else if (appledwc->usb3_phy && of_device_is_compatible(appledwc->dev->of_node, "apple,t6040-dwc3")) { dwc3_apple_usb3_pipe_reset(appledwc); @@ -728,6 +741,8 @@ static int dwc3_apple_exit(struct dwc3_apple *appledwc) appledwc->state = DWC3_APPLE_NO_CABLE; break; case DWC3_APPLE_HOST: + /* dwc3_host_exit() lets the USB3 PHY into P3 for the PIPE handback */ + appledwc->dwc.dis_u3_susphy_quirk = appledwc->dt_dis_u3_susphy; dwc3_host_exit(&appledwc->dwc); appledwc->state = DWC3_APPLE_NO_CABLE; break; @@ -855,6 +870,7 @@ static int dwc3_apple_probe(struct platform_device *pdev) appledwc->dev = &pdev->dev; appledwc->fixed_hub = dwc3_apple_has_fixed_hub(dev); appledwc->host_only = dwc3_apple_host_only(dev); + appledwc->dt_dis_u3_susphy = device_property_read_bool(dev, "snps,dis_u3_susphy_quirk"); /* The PHY driver and the DARTs follow runtime PM on these SoCs. */ appledwc->idle_rpm = of_device_is_compatible(dev->of_node, "apple,t6040-dwc3") || of_device_is_compatible(dev->of_node, "apple,t8132-dwc3"); diff --git a/tools/asahi/neo-stability-tests.py b/tools/asahi/neo-stability-tests.py index a8319e71b33d5..067a9d502801c 100644 --- a/tools/asahi/neo-stability-tests.py +++ b/tools/asahi/neo-stability-tests.py @@ -79,8 +79,8 @@ def function(source, name): assert(h==dockchannel_rx_irq&&t==dockchannel_rx_irq_thread&&!strcmp(n,"apple-dockchannel-rx"));rx_irq_requested=irq;rx_flags=f;return 0;} enum dwc3_apple_state {DWC3_APPLE_PROBE_PENDING,DWC3_APPLE_NO_CABLE,DWC3_APPLE_HOST,DWC3_APPLE_DEVICE,DWC3_APPLE_SUSPENDED}; #define DWC3_GCTL_PRTCAP_HOST 1 -struct dwc3 {int num_usb3_ports,num_usb2_ports;int usb3_generic_phy[2],usb2_generic_phy[2],usb3_phy,usb2_phy;int current_dr_role;}; -struct dwc3_apple {struct device*dev;struct dwc3 dwc;int lock,reset;enum dwc3_apple_state state;bool fixed_hub,host_only,rpm_held,core_active,core_exit_pending,reset_pending;unsigned usb3_power_off_pending,usb2_power_off_pending;}; +struct dwc3 {int num_usb3_ports,num_usb2_ports;int usb3_generic_phy[2],usb2_generic_phy[2],usb3_phy,usb2_phy;int current_dr_role;bool dis_u3_susphy_quirk;}; +struct dwc3_apple {struct device*dev;struct dwc3 dwc;int lock,reset;enum dwc3_apple_state state;bool fixed_hub,host_only,rpm_held,core_active,core_exit_pending,reset_pending,dt_dis_u3_susphy;unsigned usb3_power_off_pending,usb2_power_off_pending;}; static int core_error,host_error,gadget_error,reset_error,role_error,start_role; static unsigned rpm_puts; /* A started core holds the glue runtime PM reference until core_stop drops it. */ diff --git a/tools/testing/selftests/drivers/gpu/apple-dcp/dwc3/mocks.c b/tools/testing/selftests/drivers/gpu/apple-dcp/dwc3/mocks.c index 22e01ce7be38d..7d643a83eedd8 100644 --- a/tools/testing/selftests/drivers/gpu/apple-dcp/dwc3/mocks.c +++ b/tools/testing/selftests/drivers/gpu/apple-dcp/dwc3/mocks.c @@ -74,6 +74,7 @@ struct dwc3 { int num_usb2_ports, num_usb3_ports, usb2_phy, usb3_phy; struct reset_controller_dev *reset; int dev, usb_psy, vbus_draw_work, dr_mode, current_dr_role; + bool dis_u3_susphy_quirk; struct { void *notifier_call; } psy_nb; }; struct dwc3_apple { @@ -87,6 +88,7 @@ struct dwc3_apple { bool rpm_held, host_only; /* T8132 only: runtime PM before the first core probe; never set here. */ bool pre_core_rpm; + bool dt_dis_u3_susphy; unsigned long usb2_power_off_pending, usb3_power_off_pending; }; static unsigned int polls, fail_poll, reset_calls, reset_writes, core_writes; diff --git a/tools/testing/selftests/drivers/gpu/apple-dcp/teardown_pm.py b/tools/testing/selftests/drivers/gpu/apple-dcp/teardown_pm.py index 7381092399252..b293087d4917c 100644 --- a/tools/testing/selftests/drivers/gpu/apple-dcp/teardown_pm.py +++ b/tools/testing/selftests/drivers/gpu/apple-dcp/teardown_pm.py @@ -92,15 +92,15 @@ def unit_for(source): static struct xhci_hcd xhci_context; #define XHCI_STATE_REMOVING 1 #define hcd_to_xhci(h) (&xhci_context) -struct dwc3 { bool sys_wakeup; struct platform_device *xhci; }; +struct dwc3 { bool sys_wakeup, dis_u3_susphy_quirk; struct platform_device *xhci; }; enum { DWC3_APPLE_PROBE_PENDING, DWC3_APPLE_NO_CABLE, DWC3_APPLE_HOST, DWC3_APPLE_DEVICE }; /* pre_core_rpm is T8132 only: runtime PM before the first core probe; never set here. */ -struct dwc3_apple { struct dwc3 dwc; struct device *dev; int lock,state; void *role_sw; bool pre_core_rpm; }; +struct dwc3_apple { struct dwc3 dwc; struct device *dev; int lock,state; void *role_sw; bool pre_core_rpm, dt_dis_u3_susphy; }; static struct dwc3_apple apple_context; #define to_dwc3_apple(d) container_of(d,struct dwc3_apple,dwc) static bool inside_host_remove; static void *platform_get_drvdata(struct platform_device *d) { return d==&controller?(void *)&apple_context.dwc:(void *)&hcd; } -static void dwc3_enable_susphy(struct dwc3 *d,bool v) { (void)d;assert(v);susphy_calls++;observed_controller_write(); } +static void dwc3_enable_susphy(struct dwc3 *d,bool v) { assert(v&&d->dis_u3_susphy_quirk==apple_context.dt_dis_u3_susphy);susphy_calls++;observed_controller_write(); } static void device_init_wakeup(struct device *d,bool b) { (void)d;(void)b; } static void platform_device_unregister(struct platform_device *d); static void dwc3_gadget_exit(struct dwc3 *d) { (void)d;assert(0); } @@ -158,7 +158,7 @@ def unit_for(source): struct apple_atcphy p;prepare(&p,true,true,false); struct device_private private={0};controller.dev.p=&private;host_device.p=&private; controller_driver.remove=controller_remove;host_driver.remove=host_remove; - apple_context=(struct dwc3_apple){.dwc={.xhci=&host_platform_device},.dev=&controller.dev,.state=DWC3_APPLE_HOST}; + apple_context=(struct dwc3_apple){.dwc={.xhci=&host_platform_device,.dis_u3_susphy_quirk=true},.dev=&controller.dev,.state=DWC3_APPLE_HOST}; xhci_context=(struct xhci_hcd){0}; inject_remove=true;removal_started=blocked_attempt=release_remove=false; atomic_store(&early_write,false);atomic_store(&handoff_active,true); From 3578bc3697370e7e2ddc0d6555a536fc29decf94 Mon Sep 17 00:00:00 2001 From: Erik Bourget Date: Wed, 28 Oct 2026 19:19:03 -0400 Subject: [PATCH 3/8] usb: core: quirks: Keep the VIA VL822 USB2 hub awake for wakeups The USB 2.0 hub of the VIA Labs VL822 (2109:2822), as in Anker USB-C hubs, does not pass on a remote wakeup from a device below it while it is suspended itself, like the VL122. A USB keyboard behind it cannot wake the system from suspend-to-idle. Give it USB_QUIRK_HUB_NO_SUSPENDED_WAKEUP, so that it is kept awake in suspend-to-idle while a device below it may wake the system. Its autosuspend is unchanged. A KUnit case checks the entry. Signed-off-by: Erik Bourget --- drivers/usb/core/quirks.c | 4 ++++ drivers/usb/core/usb-quirks-test.c | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 50c564c46b48a..a374ff625a24c 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -592,6 +592,10 @@ static const struct usb_device_id usb_quirk_list[] = { /* VIA Labs, Inc. USB2.0 Hub */ { USB_DEVICE(0x2109, 0x2817), .driver_info = USB_QUIRK_NO_LPM }, + /* VIA Labs, Inc. VL822 USB2.0 Hub */ + { USB_DEVICE(0x2109, 0x2822), .driver_info = + USB_QUIRK_HUB_NO_SUSPENDED_WAKEUP }, + /* Raydium Touchscreen */ { USB_DEVICE(0x2386, 0x3114), .driver_info = USB_QUIRK_NO_LPM }, diff --git a/drivers/usb/core/usb-quirks-test.c b/drivers/usb/core/usb-quirks-test.c index 5587e1d4cfa0c..9073f6bd3149e 100644 --- a/drivers/usb/core/usb-quirks-test.c +++ b/drivers/usb/core/usb-quirks-test.c @@ -53,6 +53,19 @@ static void usb_quirks_vl122_test(struct kunit *test) KUNIT_EXPECT_TRUE(test, usb_hub_id_quirks(&t->intf) & HUB_QUIRK_DISABLE_AUTOSUSPEND); } +/* + * The VIA Labs VL822 USB 2.0 hub passes on no remote wakeup from below while it + * is suspended. It is kept awake in suspend-to-idle and keeps its autosuspend. + */ +static void usb_quirks_vl822_test(struct kunit *test) +{ + struct usb_quirks_test *t = usb_quirks_test_hub(test, 0x2109, 0x2822); + + KUNIT_EXPECT_EQ(test, usb_detect_device_list_quirks(&t->udev), + USB_QUIRK_HUB_NO_SUSPENDED_WAKEUP); + KUNIT_EXPECT_EQ(test, usb_hub_id_quirks(&t->intf), 0UL); +} + /* Another VIA Labs hub keeps its own device quirk and no hub quirk. */ static void usb_quirks_other_via_hub_test(struct kunit *test) { @@ -73,6 +86,7 @@ static void usb_quirks_generic_hub_test(struct kunit *test) static struct kunit_case usb_quirks_test_cases[] = { KUNIT_CASE(usb_quirks_vl122_test), + KUNIT_CASE(usb_quirks_vl822_test), KUNIT_CASE(usb_quirks_other_via_hub_test), KUNIT_CASE(usb_quirks_generic_hub_test), {} From df65571d77c09e2fcba7431f9bb15507df6dcbb5 Mon Sep 17 00:00:00 2001 From: Erik Bourget Date: Thu, 29 Oct 2026 00:51:39 -0400 Subject: [PATCH 4/8] usb: typec: tipd: cd321x: Refresh the power status on resume without a plug The resume revalidates the port and reports a cable removed in system sleep, but reads the power status only while a plug is present. A power source removed in sleep then leaves the cached power status, and with it the port's USB power supply, online until the next power event, and upower reports AC power while on battery. Read the power status unconditionally. Without a plug it reads as no connection. This matters once the events of a sleeping controller are acknowledged in sleep, before resume could handle them, which the following changes do. Signed-off-by: Erik Bourget --- drivers/usb/typec/tipd/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c index defc67404b6ad..30547a110ebef 100644 --- a/drivers/usb/typec/tipd/core.c +++ b/drivers/usb/typec/tipd/core.c @@ -1532,10 +1532,11 @@ static void cd321x_resume_work(struct work_struct *work) if (!tps6598x_read_status(tps, &status)) goto retry; + /* Without a plug too, or a power supply removed in sleep stays online */ + if (!tps6598x_read_power_status(tps)) + goto retry; if (status & TPS_STATUS_PLUG_PRESENT) { - if (!tps6598x_read_power_status(tps)) - goto retry; if (TPS_POWER_STATUS_PWROPMODE(tps->pwr_status) == TYPEC_PWR_MODE_PD && tps6598x_read_partner_identity(tps)) goto retry; From c556b0049594207a953474b8c4810da520496239 Mon Sep 17 00:00:00 2001 From: Erik Bourget Date: Thu, 29 Oct 2026 01:05:07 -0400 Subject: [PATCH 5/8] dt-bindings: usb: apple,sn201202x: Allow wakeup-source As on ti,tps6598x, the controller may wake the system from sleep for a change on its port. Signed-off-by: Erik Bourget --- Documentation/devicetree/bindings/usb/apple,sn201202x.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/apple,sn201202x.yaml b/Documentation/devicetree/bindings/usb/apple,sn201202x.yaml index 88ee208e102ef..c2d2eb1068ea8 100644 --- a/Documentation/devicetree/bindings/usb/apple,sn201202x.yaml +++ b/Documentation/devicetree/bindings/usb/apple,sn201202x.yaml @@ -39,6 +39,8 @@ properties: minItems: 3 maxItems: 4 + wakeup-source: true + connector: $ref: /schemas/connector/usb-connector.yaml# From f738b993b715e1d964f88474244fee2569769206 Mon Sep 17 00:00:00 2001 From: Erik Bourget Date: Thu, 29 Oct 2026 01:05:08 -0400 Subject: [PATCH 6/8] usb: typec: tipd: sn201202x: Wake from suspend-to-idle for a new USB link A USB device plugged into a J713 in suspend-to-idle cannot wake it, and cannot wake it later either: an idle USB-C port is powered down, so the device is never enumerated. Only the Type-C controller notices it: its event interrupt fires for the attach even while it sleeps. As a plain wakeup source that interrupt would also wake the system for a charger, on every plug, unplug and power negotiation. Filter the events instead. With wakeup-source set, the event interrupt stays live in system sleep (IRQF_NO_SUSPEND) and sn201202x_sleep_event() handles it there: it wakes the controller, acknowledges its events, reads its data status and status, and wakes the system with pm_wakeup_dev_event() only for a link that was not there before: - a USB2 or USB3 link with the port as the host: a port attaches as a USB device, as to a charger, before any data role swap; - from a power source, only one capable of USB communication, as its first source capability PDO says: a charger may still report a USB link as host for a moment after the swap; - DisplayPort, Thunderbolt or USB4. A charger plugged in or unplugged leaves the system asleep. A read that fails wakes it. The controller goes back to sleep either way, and the resume revalidates the whole port as before. A controller reset seen in sleep is set up again on resume, as cd321x_check_setup() latches it. The interrupt is not armed for wakeup, so IRQF_NO_SUSPEND is used as Documentation/power/suspend-and-interrupts.rst describes for a handler that decides itself whether to wake the system, with pm_system_wakeup() underneath. The watch is set up for suspend-to-idle only. In suspend-to-idle the tick is frozen, so a sleeping wait does not end until something else wakes the system: the controller is polled with busy waits there, within a fixed budget, and WAKEUP is repeated in case it reached a controller still going to sleep. A resume whose WAKEUP is not confirmed now falls back to polling as well. Its other interrupts are suspended, so this needs a controller without select notification, as on T8132; others keep their behaviour. A transfer the SPMI controller never answers still waits on its timer, so it ends only once the system is awake again. The port's power/wakeup setting turns the wakeup off. KUnit cases cover the link decision with the data status and status as read on a J713 for a charger as it attaches, swaps and settles, and for a USB keyboard. The tipd_init() host test stubs gain sleep_event and IRQF_NO_SUSPEND, and the SN201202x transport test covers the atomic selection too. Signed-off-by: Erik Bourget --- drivers/usb/typec/tipd/Kconfig | 3 +- drivers/usb/typec/tipd/core.c | 71 ++++++++- drivers/usb/typec/tipd/sn201202x-pm-test.c | 43 ++++++ drivers/usb/typec/tipd/spmi.c | 141 ++++++++++++++++-- drivers/usb/typec/tipd/tps6598x.h | 17 +++ .../drivers/apple/sn201202x-transport.py | 10 +- .../drivers/gpu/apple-dcp/transaction.py | 3 +- 7 files changed, 267 insertions(+), 21 deletions(-) diff --git a/drivers/usb/typec/tipd/Kconfig b/drivers/usb/typec/tipd/Kconfig index 6cc0e1d860704..83cd877951886 100644 --- a/drivers/usb/typec/tipd/Kconfig +++ b/drivers/usb/typec/tipd/Kconfig @@ -75,5 +75,6 @@ config TYPEC_SN201202X_KUNIT_TEST help Test that the SN201202x events are not handled while the controller sleeps, and that a system suspend that fails leaves the controller - awake. These tests run on a simulated SPMI bus, without accessing a + awake. Also test which data status and status count as a link + that wakes the system from suspend-to-idle. These tests run on a simulated SPMI bus, without accessing a physical Type-C controller. diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c index 30547a110ebef..2f1b3f4d4ff31 100644 --- a/drivers/usb/typec/tipd/core.c +++ b/drivers/usb/typec/tipd/core.c @@ -14,6 +14,8 @@ #include #include #include +#include +#include #include #include #include @@ -46,6 +48,7 @@ #define TPS_REG_SYSTEM_CONF 0x28 #define TPS_REG_CTRL_CONF 0x29 #define TPS_REG_BOOT_STATUS 0x2D +#define TPS_REG_RX_SOURCE_CAPS 0x30 #define TPS_REG_POWER_STATUS 0x3f #define TPS_REG_PD_STATUS 0x40 #define TPS_REG_RX_IDENTITY_SOP 0x48 @@ -1646,6 +1649,46 @@ static int cd321x_check_setup(struct cd321x *cd321x, u64 event) return 0; } +/** + * cd321x_sleep_events - Acknowledge the events of a controller in system sleep + * @tps: The controller, woken for this + * @st: Its data status and status, and for a power sink whether its source is + * capable of USB communication + * + * The resume revalidates the whole port state, so nothing else is read. + * + * Context: tps->lock held. + * Return: 0, or a negative error code. + */ +int cd321x_sleep_events(struct tps6598x *tps, struct cd321x_sleep_state *st) +{ + /* The PDO count, then the first PDO, vSafe5V, which carries the flags */ + u8 caps[1 + sizeof(u32)]; + u64 event; + int ret; + + lockdep_assert_held(&tps->lock); + st->usb_comm = false; + ret = tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event); + if (!ret && event) + ret = tps6598x_write64(tps, TPS_REG_INT_CLEAR1, event); + if (ret) + return ret; + /* A controller that was reset is set up again on resume */ + cd321x_check_setup(container_of(tps, struct cd321x, tps), event); + + ret = tps6598x_read32(tps, TPS_REG_DATA_STATUS, &st->data_status); + if (!ret) + ret = tps6598x_read32(tps, TPS_REG_STATUS, &st->status); + if (!ret && !(st->status & TPS_STATUS_PORTROLE)) { + ret = tps6598x_block_read(tps, TPS_REG_RX_SOURCE_CAPS, caps, sizeof(caps)); + if (!ret && (caps[0] & GENMASK(2, 0))) + st->usb_comm = get_unaligned_le32(&caps[1]) & PDO_FIXED_USB_COMM; + } + return ret; +} +EXPORT_SYMBOL_GPL(cd321x_sleep_events); + static irqreturn_t cd321x_interrupt(int irq, void *data) { struct tps6598x *tps = data; @@ -1654,6 +1697,9 @@ static irqreturn_t cd321x_interrupt(int irq, void *data) u32 status; int ret; + if (tps->sleep_event && tps->sleep_event(tps)) + return IRQ_HANDLED; + mutex_lock(&tps->lock); ret = tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event); @@ -2952,10 +2998,17 @@ int tipd_init(struct tps6598x *tps) } } + tps->wakeup = device_property_read_bool(tps->dev, "wakeup-source"); if (tps->irq) { + /* + * A variant that filters its events in system sleep keeps the + * interrupt live and decides itself whether to wake the system. + */ ret = devm_request_threaded_irq(tps->dev, tps->irq, NULL, tps->data->irq_handler, - IRQF_SHARED | IRQF_ONESHOT, + IRQF_SHARED | IRQF_ONESHOT | + (tps->wakeup && tps->sleep_event ? + IRQF_NO_SUSPEND : 0), dev_name(tps->dev), tps); } else { dev_dbg(tps->dev, "no IRQ specified, using polling mode\n"); @@ -2969,10 +3022,10 @@ int tipd_init(struct tps6598x *tps) fwnode_handle_put(fwnode); - tps->wakeup = device_property_read_bool(tps->dev, "wakeup-source"); if (tps->wakeup && tps->irq) { devm_device_init_wakeup(tps->dev); - enable_irq_wake(tps->irq); + if (!tps->sleep_event) + enable_irq_wake(tps->irq); } tipd_debugfs_register(tps); @@ -3041,8 +3094,10 @@ EXPORT_SYMBOL_GPL(tipd_remove); int tipd_suspend(struct tps6598x *tps) { if (tps->wakeup) { - disable_irq(tps->irq); - enable_irq_wake(tps->irq); + if (!tps->sleep_event) { + disable_irq(tps->irq); + enable_irq_wake(tps->irq); + } } else if (tps->reset) { gpiod_set_value_cansleep(tps->reset, 1); } @@ -3072,8 +3127,10 @@ int tipd_resume(struct tps6598x *tps) } if (tps->wakeup) { - disable_irq_wake(tps->irq); - enable_irq(tps->irq); + if (!tps->sleep_event) { + disable_irq_wake(tps->irq); + enable_irq(tps->irq); + } } else if (tps->reset) { gpiod_set_value_cansleep(tps->reset, 0); msleep(TPS_SETUP_MS); diff --git a/drivers/usb/typec/tipd/sn201202x-pm-test.c b/drivers/usb/typec/tipd/sn201202x-pm-test.c index 1082774dba56f..1d0efec7fa9a3 100644 --- a/drivers/usb/typec/tipd/sn201202x-pm-test.c +++ b/drivers/usb/typec/tipd/sn201202x-pm-test.c @@ -220,12 +220,55 @@ static void sn201202x_unconfirmed_sleep_failed_wakeup_test(struct kunit *test) KUNIT_EXPECT_TRUE(test, sn201202x_test_irq_enabled(t)); } +/* As read on a J713, the dock aside */ +static const struct cd321x_sleep_state sn201202x_test_charger_attach = { + /* Attached as a USB3 device, before the data role swap */ + .data_status = 0x800000e1, .status = 0x1000600d, +}; + +static const struct cd321x_sleep_state sn201202x_test_charger_swapped = { + /* A USB3 link as host for a moment after it, then none */ + .data_status = 0x80000061, .status = 0x1000904d, +}; + +static const struct cd321x_sleep_state sn201202x_test_charger = { + .data_status = 0x80000001, .status = 0x100c90cd, +}; + +static const struct cd321x_sleep_state sn201202x_test_keyboard = { + /* Powered by this port, as are hubs */ + .data_status = 0x00000073, .status = 0x100000fd, +}; + +static const struct cd321x_sleep_state sn201202x_test_dock = { + /* Powers this port and passes USB: a USB capable source */ + .data_status = 0x80000073, .status = 0x100c90cd, .usb_comm = true, +}; + +/* In system sleep a charger is no link; a USB device, dock or display is. */ +static void sn201202x_sleep_links_test(struct kunit *test) +{ + struct cd321x_sleep_state dp = { + .data_status = TPS_DATA_STATUS_DATA_CONNECTION | + TPS_DATA_STATUS_DP_CONNECTION, + .status = 0x100c90cd, + }; + + KUNIT_EXPECT_EQ(test, sn201202x_sleep_links(&sn201202x_test_charger_attach), 0); + KUNIT_EXPECT_EQ(test, sn201202x_sleep_links(&sn201202x_test_charger_swapped), 0); + KUNIT_EXPECT_EQ(test, sn201202x_sleep_links(&sn201202x_test_charger), 0); + KUNIT_EXPECT_NE(test, sn201202x_sleep_links(&sn201202x_test_keyboard), 0); + KUNIT_EXPECT_NE(test, sn201202x_sleep_links(&sn201202x_test_dock), 0); + KUNIT_EXPECT_EQ(test, sn201202x_sleep_links(&dp), TPS_DATA_STATUS_DP_CONNECTION); +} + static struct kunit_case sn201202x_pm_cases[] = { KUNIT_CASE(sn201202x_sleep_cycle_test), KUNIT_CASE(sn201202x_failed_wakeup_test), KUNIT_CASE(sn201202x_rejected_sleep_test), KUNIT_CASE(sn201202x_unconfirmed_sleep_test), KUNIT_CASE(sn201202x_unconfirmed_sleep_failed_wakeup_test), + KUNIT_CASE(sn201202x_sleep_links_test), {} }; diff --git a/drivers/usb/typec/tipd/spmi.c b/drivers/usb/typec/tipd/spmi.c index 4f0ceca25ee7f..65b630d76371a 100644 --- a/drivers/usb/typec/tipd/spmi.c +++ b/drivers/usb/typec/tipd/spmi.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include +#include #include #include #include @@ -8,12 +9,16 @@ #include #include #include +#include #include #include "tps6598x.h" #define tps_to_sn(tps) container_of_const((tps), struct sn201202x, cd.tps) +/* A sleeping controller does not answer a select; give up on it sooner. */ +#define SN201202X_ATOMIC_SELECT_MS 10 + static int regmap_sn201202x_select_reg(struct spmi_device *sdev, u8 reg) { int err; @@ -35,9 +40,15 @@ static int regmap_sn201202x_select_reg(struct spmi_device *sdev, u8 reg) if (sn->polled || sn->select_polled) { int poll_err; - poll_err = read_poll_timeout(spmi_register_read, err, - err || val == reg, 100, 100000, - false, sdev, 0, &val); + if (sn->atomic) + poll_err = read_poll_timeout_atomic(spmi_register_read, err, + err || val == reg, 100, + SN201202X_ATOMIC_SELECT_MS * 1000, + false, sdev, 0, &val); + else + poll_err = read_poll_timeout(spmi_register_read, err, + err || val == reg, 100, 100000, + false, sdev, 0, &val); return err ?: poll_err; } @@ -190,8 +201,15 @@ static const struct of_device_id sn201202x_of_match[] = { {} }; -static int sn201202x_wake(struct sn201202x *sn) +/* + * In system sleep the wake interrupt is suspended, so the controller is + * polled as without interrupts. + */ +static int sn201202x_wake_poll(struct sn201202x *sn, bool poll) { + /* Busy-waiting has no clock to go by: each step is charged instead */ + int budget_ms = 500; + unsigned int tries = 0; unsigned long deadline; u8 size; int ret; @@ -200,16 +218,24 @@ static int sn201202x_wake(struct sn201202x *sn) ret = spmi_command_wakeup(sn->sdev); if (ret) return ret; - if (!sn->polled) - return wait_for_completion_timeout(&sn->wake_completion, - msecs_to_jiffies(100)) ? 0 : -ETIMEDOUT; + /* A WAKEUP sent while it was still going to sleep may go unanswered */ + if (!poll && wait_for_completion_timeout(&sn->wake_completion, + msecs_to_jiffies(100))) + return 0; /* A sleeping ACE3 ACKs commands but returns zeros until it is awake. */ deadline = jiffies + msecs_to_jiffies(500); do { + if (++tries % 20 == 0) { + ret = spmi_command_wakeup(sn->sdev); + if (ret) + return ret; + } ret = regmap_sn201202x_select_reg(sn->sdev, TPS_REG_MODE); - if (ret == -ETIMEDOUT) + if (ret == -ETIMEDOUT) { + budget_ms -= SN201202X_ATOMIC_SELECT_MS; continue; + } if (ret) return ret; ret = spmi_register_read(sn->sdev, 0x1f, &size); @@ -217,12 +243,77 @@ static int sn201202x_wake(struct sn201202x *sn) return ret; if (size) return 0; - usleep_range(1000, 2000); - } while (time_before(jiffies, deadline)); + if (sn->atomic) + mdelay(1); + else + usleep_range(1000, 2000); + budget_ms--; + } while (sn->atomic ? budget_ms > 0 : time_before(jiffies, deadline)); return -ETIMEDOUT; } +static int sn201202x_wake(struct sn201202x *sn) +{ + return sn201202x_wake_poll(sn, sn->polled); +} + +/* + * The links that wake the system when they appear in system sleep. A port + * attaches as a USB device before any data role swap, as it does to a + * charger, and a charger may still show a USB link for a moment after one: + * a USB link counts only with this port as the host, and from a power source + * only if the source is capable of USB communication. + */ +VISIBLE_IF_KUNIT u32 sn201202x_sleep_links(const struct cd321x_sleep_state *st) +{ + u32 links = st->data_status & (TPS_DATA_STATUS_DP_CONNECTION | + TPS_DATA_STATUS_TBT_CONNECTION | + CD321X_DATA_STATUS_USB4_CONNECTION); + + if (st->data_status & TPS_DATA_STATUS_USB_DATA_ROLE) + return links; + if (!(st->status & TPS_STATUS_PORTROLE) && !st->usb_comm) + return links; + return links | (st->data_status & (TPS_DATA_STATUS_USB2_CONNECTION | + TPS_DATA_STATUS_USB3_CONNECTION)); +} +EXPORT_SYMBOL_IF_KUNIT(sn201202x_sleep_links); + +/* + * The event interrupt stays live in system sleep (IRQF_NO_SUSPEND). Wake the + * controller to acknowledge the event, and wake the system only for a new + * data connection, or if the controller cannot be read. It goes back to sleep + * either way, as the resume expects. + */ +static bool sn201202x_sleep_event(struct tps6598x *tps) +{ + struct sn201202x *sn = tps_to_sn(tps); + struct cd321x_sleep_state st; + u32 links = 0; + bool wake; + int ret; + + guard(mutex)(&tps->lock); + if (!READ_ONCE(sn->sleep_watch)) + return false; + + sn->atomic = true; + ret = sn201202x_wake_poll(sn, true); + if (!ret) + ret = cd321x_sleep_events(tps, &st); + if (!ret) + links = sn201202x_sleep_links(&st); + wake = ret || (links & ~sn->sleep_links); + if (!ret) + sn->sleep_links = links; + spmi_command_sleep(sn->sdev); + sn->atomic = false; + if (wake) + pm_wakeup_dev_event(tps->dev, 0, true); + return true; +} + static int sn201202x_probe(struct spmi_device *device) { const struct of_device_id *match; @@ -308,6 +399,13 @@ static int sn201202x_probe(struct spmi_device *device) return ret; } + /* + * In system sleep only the event interrupt stays live. Register access + * then has to poll, which a select notification would not. + */ + if (!sn->polled && sn->select_polled) + tps->sleep_event = sn201202x_sleep_event; + spmi_device_set_drvdata(device, tps); tps->regmap = devm_regmap_init_sn201202x(device, &tps6598x_regmap_config); if (IS_ERR(tps->regmap)) @@ -343,6 +441,11 @@ VISIBLE_IF_KUNIT int __maybe_unused sn201202x_resume(struct device *dev) struct sn201202x *sn = tps_to_sn(tps); int err; + if (READ_ONCE(sn->sleep_watch)) { + /* Waits for a running sleep_event() */ + disable_irq(tps->irq); + WRITE_ONCE(sn->sleep_watch, false); + } err = sn201202x_wake(sn); if (!err) err = tipd_resume(tps); @@ -357,6 +460,7 @@ VISIBLE_IF_KUNIT int __maybe_unused sn201202x_suspend(struct device *dev) { struct tps6598x *tps = dev_get_drvdata(dev); struct sn201202x *sn = tps_to_sn(tps); + bool watch; int err, ret; /* @@ -369,6 +473,19 @@ VISIBLE_IF_KUNIT int __maybe_unused sn201202x_suspend(struct device *dev) err = tipd_suspend(tps); if (err) goto out_enable; + watch = tps->sleep_event && device_may_wakeup(dev) && + pm_suspend_target_state == PM_SUSPEND_TO_IDLE; + if (watch) { + struct cd321x_sleep_state st; + + guard(mutex)(&tps->lock); + /* Without a baseline, a plug-in in sleep cannot be told from a charger */ + watch = !cd321x_sleep_events(tps, &st); + if (watch) + sn->sleep_links = sn201202x_sleep_links(&st); + else + dev_warn(dev, "port state unreadable, no wakeup for an attach\n"); + } reinit_completion(&sn->sleep_completion); err = spmi_command_sleep(sn->sdev); if (err) @@ -378,6 +495,10 @@ VISIBLE_IF_KUNIT int __maybe_unused sn201202x_suspend(struct device *dev) err = -ETIMEDOUT; goto out_wake; } + if (watch) { + WRITE_ONCE(sn->sleep_watch, true); + enable_irq(tps->irq); + } return 0; out_wake: diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h index 5a1b73a6d4601..1fa0a4f67258f 100644 --- a/drivers/usb/typec/tipd/tps6598x.h +++ b/drivers/usb/typec/tipd/tps6598x.h @@ -363,6 +363,8 @@ struct tps6598x { enum power_supply_usb_type usb_type; int wakeup; + /* Handles events while the system sleeps, if the variant can filter them */ + bool (*sleep_event)(struct tps6598x *tps); u32 status; /* status reg */ u32 data_status; u16 pwr_status; @@ -464,6 +466,11 @@ struct sn201202x { struct completion sleep_completion; struct completion wake_completion; struct spmi_device *sdev; + /* Links that wake the system, while the controller's events are filtered */ + u32 sleep_links; + bool sleep_watch; + /* Busy-wait: in suspend-to-idle a sleeping wait outlasts the timers */ + bool atomic; }; extern const struct tipd_data *tipd_hw_variants[]; @@ -478,6 +485,15 @@ int tipd_resume(struct tps6598x *tps); /* Internal to tps6598x-core */ int tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len); +/* What the system-sleep event filter of a port reads */ +struct cd321x_sleep_state { + u32 data_status; + u32 status; + /* A power source this port sinks from is capable of USB communication */ + bool usb_comm; +}; + +int cd321x_sleep_events(struct tps6598x *tps, struct cd321x_sleep_state *st); int tps6598x_exec_cmd_tmo(struct tps6598x *tps, const char *cmd, size_t in_len, const u8 *in_data, size_t out_len, u8 *out_data, @@ -504,6 +520,7 @@ cd321x_tbt_switch_get(struct device *dev, struct fwnode_handle *fwnode, int cd321x_tbt_watch_late(struct cd321x *cd321x, struct fwnode_handle *fwnode); int sn201202x_suspend(struct device *dev); int sn201202x_resume(struct device *dev); +u32 sn201202x_sleep_links(const struct cd321x_sleep_state *st); #endif #endif /* __TPS6598X_H__ */ diff --git a/tools/testing/selftests/drivers/apple/sn201202x-transport.py b/tools/testing/selftests/drivers/apple/sn201202x-transport.py index 8f80c82ead71c..1197e8ab80de2 100755 --- a/tools/testing/selftests/drivers/apple/sn201202x-transport.py +++ b/tools/testing/selftests/drivers/apple/sn201202x-transport.py @@ -43,7 +43,7 @@ def main(): struct completion { int unused; }; struct device { void *of_node; }; struct tps6598x { struct device *dev; }; -struct sn201202x { bool polled, select_polled; struct completion select_completion; }; +struct sn201202x { bool polled, select_polled, atomic; struct completion select_completion; }; struct spmi_device { struct device dev; struct tps6598x tps; }; static struct sn201202x sn; static u8 selected, advertised; @@ -90,6 +90,8 @@ def main(): for(unsigned attempt=0;attempt<8;attempt++) { \ (ret)=op(__VA_ARGS__); if(condition) { poll_ret=0; break; } \ } poll_ret; }) +#define SN201202X_ATOMIC_SELECT_MS 10 +#define read_poll_timeout_atomic read_poll_timeout ''' + function(source, "regmap_sn201202x_select_reg") + function(source, "regmap_sn201202x_read") + r''' static void clean(struct spmi_device *s) { @@ -108,6 +110,10 @@ def main(): assert(!regmap_sn201202x_read(&s,®,1,out,4)); assert(writes==1 && polls==3 && completed && length_reads==1 && payload_reads==1); assert(bytes==4 && out[0]==0 && out[3]==3 && out[4]==0xa5); + clean(&s); sn.atomic=true; memset(out,0xa5,sizeof(out)); + assert(!regmap_sn201202x_read(&s,®,1,out,4)); + assert(writes==1 && polls==3 && completed && length_reads==1 && payload_reads==1); + assert(bytes==4 && out[0]==0 && out[3]==3 && out[4]==0xa5); puts("ok 1 - exact selector completion precedes length and payload"); for(unsigned length=0;length<=255;length++) for(unsigned request=0;request<=64;request++) { clean(&s); advertised=length; memset(out,0xa5,sizeof(out)); @@ -147,7 +153,7 @@ def main(): return 0; } ''' - busy = program.replace("err || val == reg", "err || (val & 0x7f) == reg", 1) + busy = program.replace("err || val == reg", "err || (val & 0x7f) == reg") unchecked = program.replace( 'if (of_device_is_compatible(sdev->dev.of_node, "apple,t8132-sn201202x"))', 'if (false)', 1) diff --git a/tools/testing/selftests/drivers/gpu/apple-dcp/transaction.py b/tools/testing/selftests/drivers/gpu/apple-dcp/transaction.py index 6c40adf5d3a85..f3890d43e1b07 100644 --- a/tools/testing/selftests/drivers/gpu/apple-dcp/transaction.py +++ b/tools/testing/selftests/drivers/gpu/apple-dcp/transaction.py @@ -341,7 +341,7 @@ def probe(root): unit = quiesce.make_unit(root) unit = unit.replace(block(unit, 'int main('), block(unit, 'int main(').replace('int main(', 'int __attribute__((unused)) prior_main(', 1), 1) unit = unit.replace('struct tipd_data {', 'struct fwnode_handle;\nstruct tipd_data { unsigned irq_mask1; int (*init)(struct tps6598x *); int (*switch_power_state)(struct tps6598x *, int); int (*register_port)(struct tps6598x *, struct fwnode_handle *); bool (*read_data_status)(struct tps6598x *); bool (*read_power_status)(struct tps6598x *); int (*connect)(struct tps6598x *,u32); void (*ready)(struct tps6598x *); void *irq_handler;') - unit = unit.replace('struct tps6598x {', 'struct tps6598x { bool dbma_at_probe,wakeup; u32 status;') + unit = unit.replace('struct tps6598x {', 'struct tps6598x { bool dbma_at_probe,wakeup; u32 status; bool (*sleep_event)(struct tps6598x *);') helpers = r""" #undef dev_err #define dev_err(...) ((void)0) @@ -359,6 +359,7 @@ def probe(root): #define PTR_ERR(p) ((int)(intptr_t)(p)) #define IRQF_SHARED 0 #define IRQF_ONESHOT 0 +#define IRQF_NO_SUSPEND 0 #define POLL_INTERVAL 1000 #define INIT_DELAYED_WORK(...) ((void)0) #define tps6598x_poll_work 0 From 496efe65f45830099280b9eb1833742b381adc63 Mon Sep 17 00:00:00 2001 From: Erik Bourget Date: Thu, 29 Oct 2026 01:05:08 -0400 Subject: [PATCH 7/8] arm64: dts: apple: t8132-j713: Wake from suspend-to-idle on a USB-C attach Mark both USB-C port controllers as wakeup sources. A USB device, dock or display plugged in while the J713 is in suspend-to-idle wakes it; a charger does not. Signed-off-by: Erik Bourget --- arch/arm64/boot/dts/apple/t8132-j713.dts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/boot/dts/apple/t8132-j713.dts b/arch/arm64/boot/dts/apple/t8132-j713.dts index 2bf6dbe86337a..896d677323103 100644 --- a/arch/arm64/boot/dts/apple/t8132-j713.dts +++ b/arch/arm64/boot/dts/apple/t8132-j713.dts @@ -311,6 +311,7 @@ <43 IRQ_TYPE_EDGE_RISING>, <45 IRQ_TYPE_EDGE_RISING>; interrupt-names = "irq", "sleep", "wake"; + wakeup-source; typec1: connector { compatible = "usb-c-connector"; @@ -345,6 +346,7 @@ <17 IRQ_TYPE_EDGE_RISING>, <19 IRQ_TYPE_EDGE_RISING>; interrupt-names = "irq", "sleep", "wake"; + wakeup-source; typec0: connector { compatible = "usb-c-connector"; From 8e656b0b6979dd4ba074438a6a318dd88d0d3f57 Mon Sep 17 00:00:00 2001 From: Erik Bourget Date: Fri, 30 Oct 2026 01:31:05 -0400 Subject: [PATCH 8/8] usb: typec: tipd: sn201202x: Sleep while idle An SN201202x stays awake for as long as the system is, even with nothing attached and nothing to do. On a MacBook Air (13-inch, M4) the SMC reads about 190 mW on the 3.3 V rail that feeds the two controllers while they are awake, and about 1 mW once they sleep. Let an idle controller sleep, with runtime PM: a register access wakes it, and it sleeps again a second after the last one. Its event interrupt stays live while it sleeps, so an attach or a power change still reaches the handler, whose first read wakes it. Nothing else reads the controller on its own, so it sleeps through charging and with a USB device attached too. A display, Thunderbolt or USB4 link keeps it awake, as nothing was tested with one while it sleeps, and so does Apple's DBMa debug mode. After a few failed tries to sleep, it stays awake until the next access. System sleep wakes an idle controller before its own SLEEP handshake, and resume hands it back to runtime PM; one that does not wake sleeps on through system sleep, without the wakeup watch, and resume wakes it. The SPMI bus holds a runtime PM reference while the driver is bound. Probe gives it to idle sleep only on the T8132 controllers, where the event interrupt is known to stay live while they sleep, as the system sleep event filter relies on. Elsewhere it is kept and the controller stays awake as before. power/autosuspend_delay_ms and power/control change the delay or turn idle sleep off. The SN201202x transport test now takes __regmap_sn201202x_read(), the transfer itself, rather than its new wrapper. Signed-off-by: Erik Bourget --- drivers/usb/typec/tipd/Kconfig | 11 +- drivers/usb/typec/tipd/debugfs.c | 18 +- drivers/usb/typec/tipd/sn201202x-pm-test.c | 445 +++++++++++++++++- drivers/usb/typec/tipd/spmi.c | 198 +++++++- drivers/usb/typec/tipd/tps6598x.h | 18 + .../drivers/apple/sn201202x-transport.py | 3 +- 6 files changed, 647 insertions(+), 46 deletions(-) diff --git a/drivers/usb/typec/tipd/Kconfig b/drivers/usb/typec/tipd/Kconfig index 83cd877951886..ccaa72f8bbee4 100644 --- a/drivers/usb/typec/tipd/Kconfig +++ b/drivers/usb/typec/tipd/Kconfig @@ -69,12 +69,13 @@ config TYPEC_TIPD_KUNIT_TEST config TYPEC_SN201202X_KUNIT_TEST tristate "KUnit tests for SN201202x" if !KUNIT_ALL_TESTS - depends on KUNIT && TYPEC_SN201202X + depends on KUNIT && TYPEC_SN201202X && SUSPEND select REGMAP default KUNIT_ALL_TESTS help Test that the SN201202x events are not handled while the controller - sleeps, and that a system suspend that fails leaves the controller - awake. Also test which data status and status count as a link - that wakes the system from suspend-to-idle. These tests run on a simulated SPMI bus, without accessing a - physical Type-C controller. + sleeps, that a system suspend that fails leaves the controller awake, + and that an idle controller sleeps and wakes for register access and + for system suspend. Also test which data status and status count as + a link that wakes the system from suspend-to-idle. These tests run on + a simulated SPMI bus, without accessing a physical Type-C controller. diff --git a/drivers/usb/typec/tipd/debugfs.c b/drivers/usb/typec/tipd/debugfs.c index 77252b3ed3c90..b5cb4b2b1a618 100644 --- a/drivers/usb/typec/tipd/debugfs.c +++ b/drivers/usb/typec/tipd/debugfs.c @@ -22,6 +22,7 @@ */ #include +#include #include #include #include @@ -33,11 +34,18 @@ #define TPS_REG_MAX 0x7f -struct tipd_debugfs { - struct dentry *dentry; - /* Open "command" files that entered "DBMa"; protected by tps->lock */ - unsigned int dbma_holders; -}; +/* + * Whether an open "command" file holds the controller in "DBMa". Read + * without tps->lock, by runtime suspend: remove unregisters only once the + * bus has woken the controller, so no runtime suspend runs then. + */ +bool tipd_debugfs_dbma_held(struct tps6598x *tps) +{ + struct tipd_debugfs *debugfs = READ_ONCE(tps->debugfs); + + return debugfs && READ_ONCE(debugfs->dbma_holders); +} +EXPORT_SYMBOL_GPL(tipd_debugfs_dbma_held); struct tipd_debugfs_cmd { char cmd[4]; diff --git a/drivers/usb/typec/tipd/sn201202x-pm-test.c b/drivers/usb/typec/tipd/sn201202x-pm-test.c index 1d0efec7fa9a3..a71fee6b7a9ee 100644 --- a/drivers/usb/typec/tipd/sn201202x-pm-test.c +++ b/drivers/usb/typec/tipd/sn201202x-pm-test.c @@ -1,29 +1,40 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * KUnit tests for SN201202x system sleep + * KUnit tests for SN201202x system sleep and idle sleep */ #include #include +#include #include #include #include +#include +#include #include #include +#include #include "tps6598x.h" /* * A controller on a simulated SPMI bus. It confirms SLEEP and WAKEUP with the - * interrupts the driver waits for, unless the SLEEP confirmation gets lost, - * and it does not answer register accesses while it is asleep. + * interrupts the driver waits for, unless the SLEEP confirmation gets lost + * (every time, or once). Asleep, it does not confirm a register selection, + * and it reads as zeros. */ struct sn201202x_test { + struct kunit *test; struct sn201202x sn; struct device *dev; int irq; + /* Bound as the SPMI bus binds it, with its runtime PM reference */ + bool bound; bool asleep; + u8 selected; bool sleep_unconfirmed; + bool sleep_unconfirmed_once; + unsigned int sleeps; int sleep_error; int wakeup_error; unsigned int wakeups; @@ -41,10 +52,13 @@ static int sn201202x_test_cmd(struct spmi_controller *ctrl, u8 opcode, u8 sid) switch (opcode) { case SPMI_CMD_SLEEP: + t->sleeps++; if (t->sleep_error) return t->sleep_error; t->asleep = true; - if (!t->sleep_unconfirmed) + if (t->sleep_unconfirmed_once) + t->sleep_unconfirmed_once = false; + else if (!t->sleep_unconfirmed) complete(&t->sn.sleep_completion); return 0; case SPMI_CMD_WAKEUP: @@ -60,40 +74,102 @@ static int sn201202x_test_cmd(struct spmi_controller *ctrl, u8 opcode, u8 sid) return -EOPNOTSUPP; } -static int sn201202x_test_read(void *context, const void *reg, size_t reg_size, - void *val, size_t val_size) +/* The selected register reads back at 0, its size at 0x1f, its payload from 0x20 */ +static int sn201202x_test_read_cmd(struct spmi_controller *ctrl, u8 opcode, u8 sid, + u16 addr, u8 *buf, size_t len) { - struct sn201202x_test *t = context; + struct sn201202x_test *t = spmi_controller_get_drvdata(ctrl); - /* A sleeping controller never confirms the register selection. */ + memset(buf, 0, len); if (t->asleep) - return -ETIMEDOUT; - memset(val, 0, val_size); - if (*(const u8 *)reg == TPS_REG_MODE) - memcpy(val, "APP ", min(val_size, 4)); + return 0; + if (opcode == SPMI_CMD_READ && addr == 0) + buf[0] = t->selected; + else if (opcode == SPMI_CMD_READ && addr == 0x1f) + buf[0] = TPS_MAX_LEN; + else if (opcode == SPMI_CMD_EXT_READ && addr == 0x20 && + t->selected == TPS_REG_MODE) + memcpy(buf, "APP ", min(len, 4)); return 0; } -static int sn201202x_test_write(void *context, const void *data, size_t count) +static int sn201202x_test_write_cmd(struct spmi_controller *ctrl, u8 opcode, u8 sid, + u16 addr, const u8 *buf, size_t len) { - struct sn201202x_test *t = context; + struct sn201202x_test *t = spmi_controller_get_drvdata(ctrl); - return t->asleep ? -ETIMEDOUT : 0; + if (opcode == SPMI_CMD_ZERO_WRITE && !t->asleep) + t->selected = buf[0]; + + return 0; } -static const struct regmap_bus sn201202x_test_bus = { - .read = sn201202x_test_read, - .write = sn201202x_test_write, - .reg_format_endian_default = REGMAP_ENDIAN_NATIVE, - .val_format_endian_default = REGMAP_ENDIAN_NATIVE, -}; +/* A register access, through the driver's regmap bus */ +static int sn201202x_test_access(struct sn201202x_test *t) +{ + u8 mode[4]; + int err; + + err = regmap_raw_read(t->sn.cd.tps.regmap, TPS_REG_MODE, mode, sizeof(mode)); + if (!err && memcmp(mode, "APP ", 4)) + err = -EIO; + return err; +} static irqreturn_t sn201202x_test_irq(int irq, void *data) { return IRQ_HANDLED; } +/* The driver's runtime PM callbacks, for the test device */ +static const struct dev_pm_ops sn201202x_test_pm_ops = { + SET_RUNTIME_PM_OPS(sn201202x_runtime_suspend, sn201202x_runtime_resume, NULL) +}; + +static const struct device_type sn201202x_test_type = { + .name = "sn201202x-test", + .pm = &sn201202x_test_pm_ops, +}; + +static bool sn201202x_test_sleep_event(struct tps6598x *tps) +{ + return true; +} + +/* As the SPMI bus probes the driver, from its runtime PM setup */ +static void sn201202x_test_bind(struct sn201202x_test *t) +{ + pm_runtime_get_noresume(t->dev); + pm_runtime_set_active(t->dev); + pm_runtime_enable(t->dev); + sn201202x_idle_sleep_start(&t->sn.cd.tps); + t->bound = true; +} + +/* As the SPMI bus removes the driver, without the driver's own teardown */ +static void sn201202x_test_unbind(struct sn201202x_test *t) +{ + pm_runtime_get_sync(t->dev); + sn201202x_idle_sleep_stop(&t->sn.cd.tps); + pm_runtime_put_noidle(t->dev); + /* Held until runtime PM is off: no idle sleep after the driver's remove */ + KUNIT_EXPECT_EQ(t->test, atomic_read(&t->dev->power.usage_count), 1); + + pm_runtime_disable(t->dev); + pm_runtime_set_suspended(t->dev); + pm_runtime_put_noidle(t->dev); + t->bound = false; +} + +static void sn201202x_test_unbind_action(void *data) +{ + struct sn201202x_test *t = data; + + if (t->bound) + sn201202x_test_unbind(t); +} + static void sn201202x_test_put_device(void *data) { spmi_device_put(data); @@ -111,7 +187,8 @@ static void sn201202x_test_free_irq(void *data) free_irq(t->irq, t); } -static struct sn201202x_test *sn201202x_test_port(struct kunit *test) +/* A T8132 controller, or with @idle_sleep false, one without idle sleep */ +static struct sn201202x_test *__sn201202x_test_port(struct kunit *test, bool idle_sleep) { struct spmi_controller *ctrl; struct sn201202x_test *t; @@ -119,6 +196,7 @@ static struct sn201202x_test *sn201202x_test_port(struct kunit *test) t = kunit_kzalloc(test, sizeof(*t), GFP_KERNEL); KUNIT_ASSERT_NOT_NULL(test, t); + t->test = test; t->dev = kunit_device_register(test, "sn201202x-test"); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, t->dev); @@ -126,6 +204,8 @@ static struct sn201202x_test *sn201202x_test_port(struct kunit *test) KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctrl); spmi_controller_set_drvdata(ctrl, t); ctrl->cmd = sn201202x_test_cmd; + ctrl->read_cmd = sn201202x_test_read_cmd; + ctrl->write_cmd = sn201202x_test_write_cmd; t->sn.sdev = spmi_device_alloc(ctrl); KUNIT_ASSERT_NOT_NULL(test, t->sn.sdev); KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, sn201202x_test_put_device, @@ -133,14 +213,20 @@ static struct sn201202x_test *sn201202x_test_port(struct kunit *test) init_completion(&t->sn.select_completion); init_completion(&t->sn.sleep_completion); init_completion(&t->sn.wake_completion); + /* T8132 has no select notification */ + t->sn.select_polled = true; tps = &t->sn.cd.tps; tps->dev = t->dev; tps->data = tipd_hw_variants[TIPD_VARIANT_SN201202X]; - tps->regmap = devm_regmap_init(t->dev, &sn201202x_test_bus, t, + if (idle_sleep) + tps->sleep_event = sn201202x_test_sleep_event; + spmi_device_set_drvdata(t->sn.sdev, tps); + tps->regmap = devm_regmap_init(t->dev, ®map_sn201202x, t->sn.sdev, &tps6598x_regmap_config); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, tps->regmap); dev_set_drvdata(t->dev, tps); + mutex_init(&tps->lock); /* The event interrupt, which is never raised. */ t->irq = irq_alloc_desc(NUMA_NO_NODE); @@ -154,9 +240,25 @@ static struct sn201202x_test *sn201202x_test_port(struct kunit *test) KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, sn201202x_test_free_irq, t), 0); tps->irq = t->irq; + /* + * Awake as probe leaves it; no case waits out the autosuspend delay. + * Bound last, so it is unbound first: no runtime PM callback can + * reach the interrupt or the SPMI device once they are gone. + */ + t->dev->type = &sn201202x_test_type; + sn201202x_test_bind(t); + KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, sn201202x_test_unbind_action, + t), 0); + pm_runtime_set_autosuspend_delay(t->dev, 60 * MSEC_PER_SEC); + return t; } +static struct sn201202x_test *sn201202x_test_port(struct kunit *test) +{ + return __sn201202x_test_port(test, true); +} + static void sn201202x_sleep_cycle_test(struct kunit *test) { struct sn201202x_test *t = sn201202x_test_port(test); @@ -170,6 +272,7 @@ static void sn201202x_sleep_cycle_test(struct kunit *test) KUNIT_EXPECT_EQ(test, t->wakeups, 1U); KUNIT_EXPECT_FALSE(test, t->irq_enabled_at_wakeup); KUNIT_EXPECT_TRUE(test, sn201202x_test_irq_enabled(t)); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); } static void sn201202x_failed_wakeup_test(struct kunit *test) @@ -181,6 +284,7 @@ static void sn201202x_failed_wakeup_test(struct kunit *test) KUNIT_EXPECT_EQ(test, sn201202x_resume(t->dev), -EIO); /* Events are handled again once the controller answers. */ KUNIT_EXPECT_TRUE(test, sn201202x_test_irq_enabled(t)); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); } static void sn201202x_rejected_sleep_test(struct kunit *test) @@ -192,6 +296,7 @@ static void sn201202x_rejected_sleep_test(struct kunit *test) KUNIT_EXPECT_FALSE(test, t->asleep); KUNIT_EXPECT_EQ(test, t->wakeups, 0U); KUNIT_EXPECT_TRUE(test, sn201202x_test_irq_enabled(t)); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); } static void sn201202x_unconfirmed_sleep_test(struct kunit *test) @@ -206,6 +311,7 @@ static void sn201202x_unconfirmed_sleep_test(struct kunit *test) /* ...before its events are handled again. */ KUNIT_EXPECT_FALSE(test, t->irq_enabled_at_wakeup); KUNIT_EXPECT_TRUE(test, sn201202x_test_irq_enabled(t)); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); } static void sn201202x_unconfirmed_sleep_failed_wakeup_test(struct kunit *test) @@ -218,6 +324,282 @@ static void sn201202x_unconfirmed_sleep_failed_wakeup_test(struct kunit *test) KUNIT_EXPECT_EQ(test, sn201202x_suspend(t->dev), -ETIMEDOUT); KUNIT_EXPECT_EQ(test, t->wakeups, 1U); KUNIT_EXPECT_TRUE(test, sn201202x_test_irq_enabled(t)); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); +} + +/* An idle controller sleeps, and the next register access wakes it. */ +static void sn201202x_idle_sleep_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + KUNIT_ASSERT_EQ(test, pm_runtime_suspend(t->dev), 0); + KUNIT_EXPECT_TRUE(test, t->asleep); + + KUNIT_EXPECT_EQ(test, sn201202x_test_access(t), 0); + KUNIT_EXPECT_EQ(test, t->wakeups, 1U); + KUNIT_EXPECT_FALSE(test, t->asleep); + KUNIT_EXPECT_TRUE(test, pm_runtime_active(t->dev)); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); +} + +/* The system sleep event path wakes it itself; its accesses leave runtime PM alone. */ +static void sn201202x_atomic_access_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + KUNIT_ASSERT_EQ(test, pm_runtime_suspend(t->dev), 0); + t->sn.atomic = true; + KUNIT_EXPECT_EQ(test, sn201202x_test_access(t), -ETIMEDOUT); + t->sn.atomic = false; + KUNIT_EXPECT_EQ(test, t->wakeups, 0U); + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(t->dev)); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); +} + +/* A lost SLEEP confirmation leaves it awake, and runtime PM tries again. */ +static void sn201202x_idle_sleep_retry_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + int i; + + /* Busy from now, a short delay schedules the first attempt, which loses its SLEEP. */ + t->sleep_unconfirmed_once = true; + pm_runtime_mark_last_busy(t->dev); + pm_runtime_set_autosuspend_delay(t->dev, 10); + for (i = 0; i < 100 && !pm_runtime_suspended(t->dev); i++) + msleep(20); + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(t->dev)); + KUNIT_EXPECT_EQ(test, t->sleeps, 2U); + KUNIT_EXPECT_EQ(test, t->wakeups, 1U); + KUNIT_EXPECT_TRUE(test, t->asleep); +} + +/* Without SLEEP confirmations it gives up after a few tries, until the next access. */ +static void sn201202x_idle_sleep_gives_up_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + int i; + + t->sleep_unconfirmed = true; + pm_runtime_mark_last_busy(t->dev); + pm_runtime_set_autosuspend_delay(t->dev, 10); + for (i = 0; i < 100 && t->sleeps < 3; i++) + msleep(20); + /* Long enough for a fourth try, which must not come */ + msleep(200); + KUNIT_EXPECT_EQ(test, t->sleeps, 3U); + KUNIT_EXPECT_TRUE(test, pm_runtime_active(t->dev)); + KUNIT_EXPECT_FALSE(test, t->asleep); + + /* The next access lets it try again */ + KUNIT_ASSERT_EQ(test, sn201202x_test_access(t), 0); + for (i = 0; i < 100 && t->sleeps < 6; i++) + msleep(20); + msleep(200); + KUNIT_EXPECT_EQ(test, t->sleeps, 6U); +} + +/* An access between failed tries counts them afresh. */ +static void sn201202x_idle_sleep_tries_restart_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + t->sleep_unconfirmed = true; + /* Not autosuspend, so runtime PM does not try again by itself */ + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), -EAGAIN); + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), -EAGAIN); + KUNIT_ASSERT_EQ(test, sn201202x_test_access(t), 0); + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), -EAGAIN); + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), -EAGAIN); + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), -EBUSY); + KUNIT_EXPECT_EQ(test, t->sleeps, 5U); +} + +/* Not woken after an unconfirmed SLEEP, it is taken as asleep; the port works on. */ +static void sn201202x_unconfirmed_idle_sleep_failed_wakeup_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + t->sleep_unconfirmed = true; + t->wakeup_error = -EIO; + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), 0); + KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(t->dev)); + + t->wakeup_error = 0; + KUNIT_EXPECT_EQ(test, sn201202x_test_access(t), 0); + KUNIT_EXPECT_FALSE(test, t->asleep); +} + +/* A display link keeps it awake. */ +static void sn201202x_display_link_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + t->sn.cd.tps.status = TPS_STATUS_PLUG_PRESENT; + t->sn.cd.tps.data_status = TPS_DATA_STATUS_DATA_CONNECTION | + TPS_DATA_STATUS_DP_CONNECTION; + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), -EBUSY); + KUNIT_EXPECT_EQ(test, t->sleeps, 0U); + KUNIT_EXPECT_FALSE(test, t->asleep); +} + +/* So does Apple's DBMa debug mode, found at probe... */ +static void sn201202x_dbma_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + t->sn.cd.tps.dbma_at_probe = 1; + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), -EBUSY); + KUNIT_EXPECT_EQ(test, t->sleeps, 0U); + KUNIT_EXPECT_FALSE(test, t->asleep); +} + +static void sn201202x_test_debugfs_off(void *data) +{ + struct tps6598x *tps = data; + + tps->debugfs = NULL; +} + +/* ...or entered through debugfs, for as long as a file holds it. */ +static void sn201202x_dbma_held_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + struct tipd_debugfs *debugfs; + + if (!IS_ENABLED(CONFIG_DEBUG_FS)) + kunit_skip(test, "needs CONFIG_DEBUG_FS"); + debugfs = kunit_kzalloc(test, sizeof(*debugfs), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, debugfs); + debugfs->dbma_holders = 1; + t->sn.cd.tps.debugfs = debugfs; + KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, sn201202x_test_debugfs_off, + &t->sn.cd.tps), 0); + + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), -EBUSY); + KUNIT_EXPECT_EQ(test, t->sleeps, 0U); + + debugfs->dbma_holders = 0; + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), 0); + KUNIT_EXPECT_TRUE(test, t->asleep); +} + +/* Unbound from idle sleep, it is woken and the bus gets its reference back. */ +static void sn201202x_unbind_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + KUNIT_ASSERT_EQ(test, pm_runtime_suspend(t->dev), 0); + sn201202x_test_unbind(t); + KUNIT_EXPECT_EQ(test, t->wakeups, 1U); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); + + /* Bound again, it sleeps idle again */ + sn201202x_test_bind(t); + pm_runtime_set_autosuspend_delay(t->dev, 60 * MSEC_PER_SEC); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), 0); + KUNIT_EXPECT_TRUE(test, t->asleep); +} + +/* Without the system sleep event path there is no idle sleep. */ +static void sn201202x_no_idle_sleep_test(struct kunit *test) +{ + struct sn201202x_test *t = __sn201202x_test_port(test, false); + + KUNIT_EXPECT_LT(test, pm_runtime_suspend(t->dev), 0); + KUNIT_EXPECT_EQ(test, sn201202x_test_access(t), 0); + KUNIT_EXPECT_EQ(test, t->sleeps, 0U); + KUNIT_EXPECT_TRUE(test, pm_runtime_active(t->dev)); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 1); + + sn201202x_test_unbind(t); + KUNIT_EXPECT_EQ(test, t->sleeps, 0U); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); +} + +/* Unplugged in system sleep, the cached data status stays, but not the plug. */ +static void sn201202x_display_unplugged_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + t->sn.cd.tps.data_status = TPS_DATA_STATUS_DATA_CONNECTION | + TPS_DATA_STATUS_DP_CONNECTION; + KUNIT_EXPECT_EQ(test, pm_runtime_suspend(t->dev), 0); + KUNIT_EXPECT_TRUE(test, t->asleep); +} + +/* System sleep wakes an idle controller before its own SLEEP handshake. */ +static void sn201202x_sleep_from_idle_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + KUNIT_ASSERT_EQ(test, pm_runtime_suspend(t->dev), 0); + KUNIT_ASSERT_EQ(test, sn201202x_suspend(t->dev), 0); + KUNIT_EXPECT_EQ(test, t->wakeups, 1U); + KUNIT_EXPECT_TRUE(test, t->asleep); + KUNIT_EXPECT_TRUE(test, pm_runtime_active(t->dev)); + + KUNIT_EXPECT_EQ(test, sn201202x_resume(t->dev), 0); + KUNIT_EXPECT_EQ(test, t->wakeups, 2U); + KUNIT_EXPECT_FALSE(test, t->asleep); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); +} + +/* An idle controller that does not wake sleeps on through system sleep. */ +static void sn201202x_sleep_from_idle_failed_wakeup_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + + KUNIT_ASSERT_EQ(test, pm_runtime_suspend(t->dev), 0); + t->wakeup_error = -EIO; + KUNIT_EXPECT_EQ(test, sn201202x_suspend(t->dev), 0); + KUNIT_EXPECT_TRUE(test, t->asleep); + KUNIT_EXPECT_FALSE(test, t->sn.sleep_watch); + KUNIT_EXPECT_FALSE(test, sn201202x_test_irq_enabled(t)); + + t->wakeup_error = 0; + KUNIT_EXPECT_EQ(test, sn201202x_resume(t->dev), 0); + KUNIT_EXPECT_FALSE(test, t->asleep); + /* Woken through runtime PM, so the next access sends no second WAKEUP */ + KUNIT_EXPECT_TRUE(test, pm_runtime_active(t->dev)); + KUNIT_EXPECT_TRUE(test, sn201202x_test_irq_enabled(t)); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); +} + +static void sn201202x_test_restore_target(void *data) +{ + pm_suspend_target_state = (suspend_state_t)(unsigned long)data; +} + +static void sn201202x_test_wakeup_off(void *data) +{ + device_init_wakeup(data, false); +} + +/* So does suspend-to-idle with the wakeup watch, whose baseline read needs it awake. */ +static void sn201202x_watched_sleep_from_idle_test(struct kunit *test) +{ + struct sn201202x_test *t = sn201202x_test_port(test); + suspend_state_t target; + + KUNIT_ASSERT_EQ(test, device_init_wakeup(t->dev, true), 0); + KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, sn201202x_test_wakeup_off, + t->dev), 0); + target = pm_suspend_target_state; + KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test, sn201202x_test_restore_target, + (void *)(unsigned long)target), 0); + pm_suspend_target_state = PM_SUSPEND_TO_IDLE; + + KUNIT_ASSERT_EQ(test, pm_runtime_suspend(t->dev), 0); + KUNIT_ASSERT_EQ(test, sn201202x_suspend(t->dev), 0); + KUNIT_EXPECT_TRUE(test, t->sn.sleep_watch); + KUNIT_EXPECT_TRUE(test, t->asleep); + + KUNIT_EXPECT_EQ(test, sn201202x_resume(t->dev), 0); + KUNIT_EXPECT_FALSE(test, t->sn.sleep_watch); + KUNIT_EXPECT_FALSE(test, t->asleep); + KUNIT_EXPECT_EQ(test, atomic_read(&t->dev->power.usage_count), 0); } /* As read on a J713, the dock aside */ @@ -269,6 +651,21 @@ static struct kunit_case sn201202x_pm_cases[] = { KUNIT_CASE(sn201202x_unconfirmed_sleep_test), KUNIT_CASE(sn201202x_unconfirmed_sleep_failed_wakeup_test), KUNIT_CASE(sn201202x_sleep_links_test), + KUNIT_CASE(sn201202x_idle_sleep_test), + KUNIT_CASE(sn201202x_atomic_access_test), + KUNIT_CASE(sn201202x_idle_sleep_retry_test), + KUNIT_CASE(sn201202x_idle_sleep_gives_up_test), + KUNIT_CASE(sn201202x_idle_sleep_tries_restart_test), + KUNIT_CASE(sn201202x_unconfirmed_idle_sleep_failed_wakeup_test), + KUNIT_CASE(sn201202x_display_link_test), + KUNIT_CASE(sn201202x_display_unplugged_test), + KUNIT_CASE(sn201202x_dbma_test), + KUNIT_CASE(sn201202x_dbma_held_test), + KUNIT_CASE(sn201202x_unbind_test), + KUNIT_CASE(sn201202x_no_idle_sleep_test), + KUNIT_CASE(sn201202x_sleep_from_idle_test), + KUNIT_CASE(sn201202x_sleep_from_idle_failed_wakeup_test), + KUNIT_CASE(sn201202x_watched_sleep_from_idle_test), {} }; @@ -278,6 +675,6 @@ static struct kunit_suite sn201202x_pm_suite = { }; kunit_test_suite(sn201202x_pm_suite); -MODULE_DESCRIPTION("KUnit tests for SN201202x system sleep"); +MODULE_DESCRIPTION("KUnit tests for SN201202x system sleep and idle sleep"); MODULE_LICENSE("GPL"); MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING"); diff --git a/drivers/usb/typec/tipd/spmi.c b/drivers/usb/typec/tipd/spmi.c index 65b630d76371a..b2b55b6625207 100644 --- a/drivers/usb/typec/tipd/spmi.c +++ b/drivers/usb/typec/tipd/spmi.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -19,6 +20,11 @@ /* A sleeping controller does not answer a select; give up on it sooner. */ #define SN201202X_ATOMIC_SELECT_MS 10 +/* An idle controller sleeps after this long without register access. */ +#define SN201202X_AUTOSUSPEND_MS 1000 +/* Failed tries to sleep before it waits for the next access */ +#define SN201202X_SLEEP_TRIES 3 + static int regmap_sn201202x_select_reg(struct spmi_device *sdev, u8 reg) { int err; @@ -76,9 +82,9 @@ static int regmap_sn201202x_select_reg(struct spmi_device *sdev, u8 reg) return -EIO; } -static int regmap_sn201202x_read(void *context, - const void *reg, size_t reg_size, - void *val, size_t val_size) +static int __regmap_sn201202x_read(void *context, + const void *reg, size_t reg_size, + void *val, size_t val_size) { struct spmi_device *sdev = context; int err; @@ -129,8 +135,8 @@ static int regmap_sn201202x_read(void *context, return 0; } -static int regmap_sn201202x_write(void *context, const void *data, - size_t count) +static int __regmap_sn201202x_write(void *context, const void *data, + size_t count) { int err = 0; unsigned int offset = 0xa0; @@ -168,6 +174,64 @@ static int regmap_sn201202x_write(void *context, const void *data, return err; } +/* + * A register access wakes a controller that sleeps idle, and lets it sleep + * again SN201202X_AUTOSUSPEND_MS after the last one, with its tries to sleep + * counted afresh. In system sleep the event path wakes it and puts it back + * to sleep itself. + */ +static int sn201202x_access_get(struct tps6598x *tps, bool *rpm) +{ + int err; + + *rpm = !READ_ONCE(tps_to_sn(tps)->atomic); + if (!*rpm) + return 0; + err = pm_runtime_resume_and_get(tps->dev); + if (err) + return err; + /* Held awake, so no runtime suspend runs meanwhile */ + tps_to_sn(tps)->sleep_tries = 0; + return 0; +} + +static void sn201202x_access_put(struct tps6598x *tps, bool rpm) +{ + if (rpm) + pm_runtime_put_autosuspend(tps->dev); +} + +static int regmap_sn201202x_read(void *context, + const void *reg, size_t reg_size, + void *val, size_t val_size) +{ + struct tps6598x *tps = spmi_device_get_drvdata(context); + bool rpm; + int err; + + err = sn201202x_access_get(tps, &rpm); + if (err) + return err; + err = __regmap_sn201202x_read(context, reg, reg_size, val, val_size); + sn201202x_access_put(tps, rpm); + return err; +} + +static int regmap_sn201202x_write(void *context, const void *data, + size_t count) +{ + struct tps6598x *tps = spmi_device_get_drvdata(context); + bool rpm; + int err; + + err = sn201202x_access_get(tps, &rpm); + if (err) + return err; + err = __regmap_sn201202x_write(context, data, count); + sn201202x_access_put(tps, rpm); + return err; +} + static irqreturn_t sn201202x_irq(int irq, void *data) { struct completion *c = data; @@ -176,12 +240,13 @@ static irqreturn_t sn201202x_irq(int irq, void *data) return IRQ_HANDLED; } -static const struct regmap_bus regmap_sn201202x = { +VISIBLE_IF_KUNIT const struct regmap_bus regmap_sn201202x = { .read = regmap_sn201202x_read, .write = regmap_sn201202x_write, .reg_format_endian_default = REGMAP_ENDIAN_NATIVE, .val_format_endian_default = REGMAP_ENDIAN_NATIVE, }; +EXPORT_SYMBOL_IF_KUNIT(regmap_sn201202x); static struct regmap *__devm_regmap_init_sn201202x(struct spmi_device *sdev, const struct regmap_config *config, @@ -258,6 +323,93 @@ static int sn201202x_wake(struct sn201202x *sn) return sn201202x_wake_poll(sn, sn->polled); } +VISIBLE_IF_KUNIT int __maybe_unused sn201202x_runtime_suspend(struct device *dev) +{ + struct tps6598x *tps = dev_get_drvdata(dev); + struct sn201202x *sn = tps_to_sn(tps); + + /* + * A display, Thunderbolt or USB4 link keeps it awake: its events are + * untested with the controller asleep. The next event, such as the + * unplug, lets it sleep again. The data status is not read again + * without a plug, so the plug status says when the link is gone. + */ + if ((READ_ONCE(tps->status) & TPS_STATUS_PLUG_PRESENT) && + (READ_ONCE(tps->data_status) & (TPS_DATA_STATUS_DP_CONNECTION | + TPS_DATA_STATUS_TBT_CONNECTION | + CD321X_DATA_STATUS_USB4_CONNECTION))) + return -EBUSY; + /* So does Apple's "DBMa" debug mode, for the routes it carries */ + if (tps->dbma_at_probe || tipd_debugfs_dbma_held(tps)) + return -EBUSY; + + reinit_completion(&sn->sleep_completion); + if (spmi_command_sleep(sn->sdev)) + goto retry; + if (!sn->polled && + !wait_for_completion_timeout(&sn->sleep_completion, msecs_to_jiffies(100))) { + /* + * It accepted SLEEP, so it may be asleep without having + * confirmed it. Woken, it tries again later; if it does not + * wake, it is taken as asleep, and the next access wakes it. + */ + if (sn201202x_wake(sn)) + goto asleep; + goto retry; + } +asleep: + sn->sleep_tries = 0; + return 0; + +retry: + /* + * With a new expiry, runtime PM tries again after the delay. Without + * a delay it does not, and the controller waits for the next access. + */ + if (++sn->sleep_tries < SN201202X_SLEEP_TRIES) { + pm_runtime_mark_last_busy(dev); + return -EAGAIN; + } + sn->sleep_tries = 0; + dev_warn_once(dev, "cannot sleep while idle, awake until the next access\n"); + return -EBUSY; +} +EXPORT_SYMBOL_IF_KUNIT(sn201202x_runtime_suspend); + +VISIBLE_IF_KUNIT int __maybe_unused sn201202x_runtime_resume(struct device *dev) +{ + struct tps6598x *tps = dev_get_drvdata(dev); + + return sn201202x_wake(tps_to_sn(tps)); +} +EXPORT_SYMBOL_IF_KUNIT(sn201202x_runtime_resume); + +/* + * The SPMI bus holds a runtime PM reference while the driver is bound. + * Probe gives it to idle sleep only where the event interrupt stays live + * while the controller sleeps, as the system sleep event filter has it; + * elsewhere the controller stays awake. + */ +VISIBLE_IF_KUNIT void sn201202x_idle_sleep_start(struct tps6598x *tps) +{ + if (!tps->sleep_event) + return; + pm_runtime_set_autosuspend_delay(tps->dev, SN201202X_AUTOSUSPEND_MS); + pm_runtime_use_autosuspend(tps->dev); + pm_runtime_put_autosuspend(tps->dev); +} +EXPORT_SYMBOL_IF_KUNIT(sn201202x_idle_sleep_start); + +/* On remove, once the bus has woken it: take back the reference */ +VISIBLE_IF_KUNIT void sn201202x_idle_sleep_stop(struct tps6598x *tps) +{ + if (!tps->sleep_event) + return; + pm_runtime_get_noresume(tps->dev); + pm_runtime_dont_use_autosuspend(tps->dev); +} +EXPORT_SYMBOL_IF_KUNIT(sn201202x_idle_sleep_stop); + /* * The links that wake the system when they appear in system sleep. A port * attaches as a USB device before any data role swap, as it does to a @@ -298,7 +450,7 @@ static bool sn201202x_sleep_event(struct tps6598x *tps) if (!READ_ONCE(sn->sleep_watch)) return false; - sn->atomic = true; + WRITE_ONCE(sn->atomic, true); ret = sn201202x_wake_poll(sn, true); if (!ret) ret = cd321x_sleep_events(tps, &st); @@ -308,7 +460,7 @@ static bool sn201202x_sleep_event(struct tps6598x *tps) if (!ret) sn->sleep_links = links; spmi_command_sleep(sn->sdev); - sn->atomic = false; + WRITE_ONCE(sn->atomic, false); if (wake) pm_wakeup_dev_event(tps->dev, 0, true); return true; @@ -423,6 +575,8 @@ static int sn201202x_probe(struct spmi_device *device) /* Events can latch before the IRQ is registered. Consume that state now. */ tps->data->irq_handler(tps->irq, tps); + + sn201202x_idle_sleep_start(tps); return 0; } @@ -431,6 +585,7 @@ static void sn201202x_remove(struct spmi_device *device) struct tps6598x *tps = spmi_device_get_drvdata(device); struct sn201202x *sn = tps_to_sn(tps); + sn201202x_idle_sleep_stop(tps); tipd_remove(tps); spmi_command_sleep(sn->sdev); } @@ -446,12 +601,20 @@ VISIBLE_IF_KUNIT int __maybe_unused sn201202x_resume(struct device *dev) disable_irq(tps->irq); WRITE_ONCE(sn->sleep_watch, false); } - err = sn201202x_wake(sn); + /* One that slept on from idle sleep wakes through runtime PM */ + if (pm_runtime_status_suspended(dev)) { + err = pm_runtime_resume(dev); + if (err > 0) + err = 0; + } else { + err = sn201202x_wake(sn); + } if (!err) err = tipd_resume(tps); /* The resume revalidation acknowledges events latched while asleep. */ if (tps->irq) enable_irq(tps->irq); + pm_runtime_put_autosuspend(dev); return err; } EXPORT_SYMBOL_IF_KUNIT(sn201202x_resume); @@ -470,9 +633,20 @@ VISIBLE_IF_KUNIT int __maybe_unused sn201202x_suspend(struct device *dev) */ if (tps->irq) disable_irq(tps->irq); + /* Awake, also from idle sleep, until resume */ + err = pm_runtime_resume_and_get(dev); + if (err) { + /* + * It sleeps idle and does not wake: let it sleep on without + * the wakeup watch, and let resume wake it. + */ + dev_warn(dev, "asleep and not waking: %d, no wakeup for an attach\n", err); + pm_runtime_get_noresume(dev); + return 0; + } err = tipd_suspend(tps); if (err) - goto out_enable; + goto out_put; watch = tps->sleep_event && device_may_wakeup(dev) && pm_suspend_target_state == PM_SUSPEND_TO_IDLE; if (watch) { @@ -508,7 +682,8 @@ VISIBLE_IF_KUNIT int __maybe_unused sn201202x_suspend(struct device *dev) dev_err(dev, "failed to wake up after failed suspend: %d\n", ret); out_resume: tipd_resume(tps); -out_enable: +out_put: + pm_runtime_put_autosuspend(dev); if (tps->irq) enable_irq(tps->irq); return err; @@ -521,6 +696,7 @@ static const struct dev_pm_ops sn201202x_pm_ops = { .prepare = pm_sleep_ptr(tipd_prepare), .complete = pm_sleep_ptr(tipd_complete), SET_SYSTEM_SLEEP_PM_OPS(sn201202x_suspend, sn201202x_resume) + SET_RUNTIME_PM_OPS(sn201202x_runtime_suspend, sn201202x_runtime_resume, NULL) }; static struct spmi_driver sn201202x_driver = { diff --git a/drivers/usb/typec/tipd/tps6598x.h b/drivers/usb/typec/tipd/tps6598x.h index 1fa0a4f67258f..2cbcf3f91c8fc 100644 --- a/drivers/usb/typec/tipd/tps6598x.h +++ b/drivers/usb/typec/tipd/tps6598x.h @@ -471,6 +471,8 @@ struct sn201202x { bool sleep_watch; /* Busy-wait: in suspend-to-idle a sleeping wait outlasts the timers */ bool atomic; + /* Failed tries to sleep while idle since it last slept, gave up or was accessed */ + unsigned int sleep_tries; }; extern const struct tipd_data *tipd_hw_variants[]; @@ -499,11 +501,22 @@ int tps6598x_exec_cmd_tmo(struct tps6598x *tps, const char *cmd, size_t out_len, u8 *out_data, u32 cmd_timeout_ms, u32 res_delay_ms); +struct tipd_debugfs { + struct dentry *dentry; + /* + * Open "command" files that entered "DBMa"; written under tps->lock, + * read without it by tipd_debugfs_dbma_held() + */ + unsigned int dbma_holders; +}; + #ifdef CONFIG_DEBUG_FS +bool tipd_debugfs_dbma_held(struct tps6598x *tps); void tipd_debugfs_register(struct tps6598x *tps); void tipd_debugfs_unregister(struct tps6598x *tps); void tipd_debugfs_exit(void); #else +static inline bool tipd_debugfs_dbma_held(struct tps6598x *tps) { return false; } static inline void tipd_debugfs_register(struct tps6598x *tps) { } static inline void tipd_debugfs_unregister(struct tps6598x *tps) { } static inline void tipd_debugfs_exit(void) { } @@ -520,6 +533,11 @@ cd321x_tbt_switch_get(struct device *dev, struct fwnode_handle *fwnode, int cd321x_tbt_watch_late(struct cd321x *cd321x, struct fwnode_handle *fwnode); int sn201202x_suspend(struct device *dev); int sn201202x_resume(struct device *dev); +int sn201202x_runtime_suspend(struct device *dev); +int sn201202x_runtime_resume(struct device *dev); +void sn201202x_idle_sleep_start(struct tps6598x *tps); +void sn201202x_idle_sleep_stop(struct tps6598x *tps); +extern const struct regmap_bus regmap_sn201202x; u32 sn201202x_sleep_links(const struct cd321x_sleep_state *st); #endif diff --git a/tools/testing/selftests/drivers/apple/sn201202x-transport.py b/tools/testing/selftests/drivers/apple/sn201202x-transport.py index 1197e8ab80de2..1c37169ba45b5 100755 --- a/tools/testing/selftests/drivers/apple/sn201202x-transport.py +++ b/tools/testing/selftests/drivers/apple/sn201202x-transport.py @@ -92,7 +92,8 @@ def main(): } poll_ret; }) #define SN201202X_ATOMIC_SELECT_MS 10 #define read_poll_timeout_atomic read_poll_timeout -''' + function(source, "regmap_sn201202x_select_reg") + function(source, "regmap_sn201202x_read") + r''' +''' + function(source, "regmap_sn201202x_select_reg") + function(source, "__regmap_sn201202x_read") + r''' +#define regmap_sn201202x_read __regmap_sn201202x_read static void clean(struct spmi_device *s) { memset(s,0,sizeof(*s)); s->dev.of_node=s; s->tps.dev=&s->dev;