On Mon, Jan 12, 2026 at 7:08 PM Tzung-Bi Shih <[email protected]> wrote: > > A potential circular locking dependency (ABBA deadlock) exists between > `ec_dev->lock` and the clock framework's `prepare_lock`. > > The first order (A -> B) occurs when scp_ipi_send() is called while > `ec_dev->lock` is held (e.g., within cros_ec_cmd_xfer()): > 1. cros_ec_cmd_xfer() acquires `ec_dev->lock` and calls scp_ipi_send(). > 2. scp_ipi_send() calls clk_prepare_enable(), which acquires > `prepare_lock`. > See #0 in the following example calling trace. > (Lock Order: `ec_dev->lock` -> `prepare_lock`) > > The reverse order (B -> A) is more complex and has been observed > (learned) by lockdep. It involves the clock prepare operation > triggering power domain changes, which then propagates through sysfs > and power supply uevents, eventually calling back into the ChromeOS EC > driver and attempting to acquire `ec_dev->lock`: > 1. Something calls clk_prepare(), which acquires `prepare_lock`. It > then triggers genpd operations like genpd_runtime_resume(), which > takes `&genpd->mlock`. > 2. Power domain changes can trigger regulator changes; regulator > changes can then trigger device link changes; device link changes > can then trigger sysfs changes. Eventually, power_supply_uevent() > is called. > 3. This leads to calls like cros_usbpd_charger_get_prop(), which calls > cros_ec_cmd_xfer_status(), which then attempts to acquire > `ec_dev->lock`. > See #1 ~ #6 in the following example calling trace. > (Lock Order: `prepare_lock` -> `&genpd->mlock` -> ... -> `&ec_dev->lock`) > > Move the clk_prepare()/clk_unprepare() operations for `scp->clk` to the > remoteproc prepare()/unprepare() callbacks. This ensures `prepare_lock` > is only acquired in prepare()/unprepare() callbacks. Since > `ec_dev->lock` is not involved in the callbacks, the dependency loop is > broken. > > This means the clock is always "prepared" when the SCP is running. The > prolonged "prepared time" for the clock should be acceptable as SCP is > designed to be a very power efficient processor. The power consumption > impact can be negligible. > > A simplified calling trace reported by lockdep: > > -> #6 (&ec_dev->lock) > > cros_ec_cmd_xfer > > cros_ec_cmd_xfer_status > > cros_usbpd_charger_get_port_status > > cros_usbpd_charger_get_prop > > power_supply_get_property > > power_supply_show_property > > power_supply_uevent > > dev_uevent > > uevent_show > > dev_attr_show > > sysfs_kf_seq_show > > kernfs_seq_show > > -> #5 (kn->active#2) > > kernfs_drain > > __kernfs_remove > > kernfs_remove_by_name_ns > > sysfs_remove_file_ns > > device_del > > __device_link_del > > device_links_driver_bound > > -> #4 (device_links_lock) > > device_link_remove > > _regulator_put > > regulator_put > > -> #3 (regulator_list_mutex) > > regulator_lock_dependent > > regulator_disable > > scpsys_power_off > > _genpd_power_off > > genpd_power_off > > -> #2 (&genpd->mlock/1) > > genpd_add_subdomain > > pm_genpd_add_subdomain > > scpsys_add_subdomain > > scpsys_probe > > -> #1 (&genpd->mlock) > > genpd_runtime_resume > > __rpm_callback > > rpm_callback > > rpm_resume > > __pm_runtime_resume > > clk_core_prepare > > clk_prepare > > -> #0 (prepare_lock) > > clk_prepare > > scp_ipi_send > > scp_send_ipi > > mtk_rpmsg_send > > rpmsg_send > > cros_ec_pkt_xfer_rpmsg > > Signed-off-by: Tzung-Bi Shih <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]> Tested-by: Chen-Yu Tsai <[email protected]> # MT8183 & MT8188 no regressions AFAIU this works because the SCP clock is internal MMIO based, and the prepare op is a no-op; so splitting the clk_prepare() call out helps to break the dependency while not incurring additional cost.
