Skip to content
/ linux Public

Commit 7af7ebe

Browse files
Tzung-Bi ShihSasha Levin
authored andcommitted
remoteproc: mediatek: Break lock dependency to prepare_lock
[ Upstream commit d935187 ] 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 <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 7839bdf commit 7af7ebe

File tree

2 files changed

+30
-13
lines changed

2 files changed

+30
-13
lines changed

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 = (struct mtk_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
@@ -164,7 +164,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
164164
WARN_ON(len > sizeof(send_obj->share_buf)) || WARN_ON(!buf))
165165
return -EINVAL;
166166

167-
ret = clk_prepare_enable(scp->clk);
167+
ret = clk_enable(scp->clk);
168168
if (ret) {
169169
dev_err(scp->dev, "failed to enable clock\n");
170170
return ret;
@@ -207,7 +207,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
207207

208208
unlock_mutex:
209209
mutex_unlock(&scp->send_lock);
210-
clk_disable_unprepare(scp->clk);
210+
clk_disable(scp->clk);
211211

212212
return ret;
213213
}

0 commit comments

Comments
 (0)