Skip to content

Commit 4c4679b

Browse files
Tzung-Bi ShihSasha Levin
authored andcommitted
remoteproc: mediatek: Break lock dependency to prepare_lock
[ Upstream commit d935187cfb27fc4168f78f3959aef4eafaae76bb ] 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 > -> BlissRoms-x86#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 > -> BlissRoms-x86#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 <tzungbi@kernel.org> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> Tested-by: Chen-Yu Tsai <wenst@chromium.org> Link: https://lore.kernel.org/r/20260112110755.2435899-1-tzungbi@kernel.org Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 332fb84 commit 4c4679b

2 files changed

Lines changed: 30 additions & 13 deletions

File tree

drivers/remoteproc/mtk_scp.c

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -225,15 +225,15 @@ static irqreturn_t scp_irq_handler(int irq, void *priv)
225225
struct mtk_scp *scp = priv;
226226
int ret;
227227

228-
ret = clk_prepare_enable(scp->clk);
228+
ret = clk_enable(scp->clk);
229229
if (ret) {
230230
dev_err(scp->dev, "failed to enable clocks\n");
231231
return IRQ_NONE;
232232
}
233233

234234
scp->data->scp_irq_handler(scp);
235235

236-
clk_disable_unprepare(scp->clk);
236+
clk_disable(scp->clk);
237237

238238
return IRQ_HANDLED;
239239
}
@@ -467,7 +467,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
467467
struct device *dev = scp->dev;
468468
int ret;
469469

470-
ret = clk_prepare_enable(scp->clk);
470+
ret = clk_enable(scp->clk);
471471
if (ret) {
472472
dev_err(dev, "failed to enable clocks\n");
473473
return ret;
@@ -482,7 +482,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
482482

483483
ret = scp_elf_load_segments(rproc, fw);
484484
leave:
485-
clk_disable_unprepare(scp->clk);
485+
clk_disable(scp->clk);
486486

487487
return ret;
488488
}
@@ -493,14 +493,14 @@ static int scp_parse_fw(struct rproc *rproc, const struct firmware *fw)
493493
struct device *dev = scp->dev;
494494
int ret;
495495

496-
ret = clk_prepare_enable(scp->clk);
496+
ret = clk_enable(scp->clk);
497497
if (ret) {
498498
dev_err(dev, "failed to enable clocks\n");
499499
return ret;
500500
}
501501

502502
ret = scp_ipi_init(scp, fw);
503-
clk_disable_unprepare(scp->clk);
503+
clk_disable(scp->clk);
504504
return ret;
505505
}
506506

@@ -511,7 +511,7 @@ static int scp_start(struct rproc *rproc)
511511
struct scp_run *run = &scp->run;
512512
int ret;
513513

514-
ret = clk_prepare_enable(scp->clk);
514+
ret = clk_enable(scp->clk);
515515
if (ret) {
516516
dev_err(dev, "failed to enable clocks\n");
517517
return ret;
@@ -536,14 +536,14 @@ static int scp_start(struct rproc *rproc)
536536
goto stop;
537537
}
538538

539-
clk_disable_unprepare(scp->clk);
539+
clk_disable(scp->clk);
540540
dev_info(dev, "SCP is ready. FW version %s\n", run->fw_ver);
541541

542542
return 0;
543543

544544
stop:
545545
scp->data->scp_reset_assert(scp);
546-
clk_disable_unprepare(scp->clk);
546+
clk_disable(scp->clk);
547547
return ret;
548548
}
549549

@@ -638,20 +638,37 @@ static int scp_stop(struct rproc *rproc)
638638
struct mtk_scp *scp = rproc->priv;
639639
int ret;
640640

641-
ret = clk_prepare_enable(scp->clk);
641+
ret = clk_enable(scp->clk);
642642
if (ret) {
643643
dev_err(scp->dev, "failed to enable clocks\n");
644644
return ret;
645645
}
646646

647647
scp->data->scp_reset_assert(scp);
648648
scp->data->scp_stop(scp);
649-
clk_disable_unprepare(scp->clk);
649+
clk_disable(scp->clk);
650650

651651
return 0;
652652
}
653653

654+
static int scp_prepare(struct rproc *rproc)
655+
{
656+
struct mtk_scp *scp = rproc->priv;
657+
658+
return clk_prepare(scp->clk);
659+
}
660+
661+
static int scp_unprepare(struct rproc *rproc)
662+
{
663+
struct mtk_scp *scp = rproc->priv;
664+
665+
clk_unprepare(scp->clk);
666+
return 0;
667+
}
668+
654669
static const struct rproc_ops scp_ops = {
670+
.prepare = scp_prepare,
671+
.unprepare = scp_unprepare,
655672
.start = scp_start,
656673
.stop = scp_stop,
657674
.load = scp_load,

drivers/remoteproc/mtk_scp_ipi.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
168168
WARN_ON(len > sizeof(send_obj->share_buf)) || WARN_ON(!buf))
169169
return -EINVAL;
170170

171-
ret = clk_prepare_enable(scp->clk);
171+
ret = clk_enable(scp->clk);
172172
if (ret) {
173173
dev_err(scp->dev, "failed to enable clock\n");
174174
return ret;
@@ -208,7 +208,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
208208

209209
unlock_mutex:
210210
mutex_unlock(&scp->send_lock);
211-
clk_disable_unprepare(scp->clk);
211+
clk_disable(scp->clk);
212212

213213
return ret;
214214
}

0 commit comments

Comments
 (0)