Skip to content

Commit d935187

Browse files
Tzung-Bi Shihmathieupoirier
authored andcommitted
remoteproc: mediatek: Break lock dependency to prepare_lock
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>
1 parent 1aab44c commit d935187

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
@@ -283,15 +283,15 @@ static irqreturn_t scp_irq_handler(int irq, void *priv)
283283
struct mtk_scp *scp = priv;
284284
int ret;
285285

286-
ret = clk_prepare_enable(scp->clk);
286+
ret = clk_enable(scp->clk);
287287
if (ret) {
288288
dev_err(scp->dev, "failed to enable clocks\n");
289289
return IRQ_NONE;
290290
}
291291

292292
scp->data->scp_irq_handler(scp);
293293

294-
clk_disable_unprepare(scp->clk);
294+
clk_disable(scp->clk);
295295

296296
return IRQ_HANDLED;
297297
}
@@ -665,7 +665,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
665665
struct device *dev = scp->dev;
666666
int ret;
667667

668-
ret = clk_prepare_enable(scp->clk);
668+
ret = clk_enable(scp->clk);
669669
if (ret) {
670670
dev_err(dev, "failed to enable clocks\n");
671671
return ret;
@@ -680,7 +680,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
680680

681681
ret = scp_elf_load_segments(rproc, fw);
682682
leave:
683-
clk_disable_unprepare(scp->clk);
683+
clk_disable(scp->clk);
684684

685685
return ret;
686686
}
@@ -691,14 +691,14 @@ static int scp_parse_fw(struct rproc *rproc, const struct firmware *fw)
691691
struct device *dev = scp->dev;
692692
int ret;
693693

694-
ret = clk_prepare_enable(scp->clk);
694+
ret = clk_enable(scp->clk);
695695
if (ret) {
696696
dev_err(dev, "failed to enable clocks\n");
697697
return ret;
698698
}
699699

700700
ret = scp_ipi_init(scp, fw);
701-
clk_disable_unprepare(scp->clk);
701+
clk_disable(scp->clk);
702702
return ret;
703703
}
704704

@@ -709,7 +709,7 @@ static int scp_start(struct rproc *rproc)
709709
struct scp_run *run = &scp->run;
710710
int ret;
711711

712-
ret = clk_prepare_enable(scp->clk);
712+
ret = clk_enable(scp->clk);
713713
if (ret) {
714714
dev_err(dev, "failed to enable clocks\n");
715715
return ret;
@@ -734,14 +734,14 @@ static int scp_start(struct rproc *rproc)
734734
goto stop;
735735
}
736736

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

740740
return 0;
741741

742742
stop:
743743
scp->data->scp_reset_assert(scp);
744-
clk_disable_unprepare(scp->clk);
744+
clk_disable(scp->clk);
745745
return ret;
746746
}
747747

@@ -909,20 +909,37 @@ static int scp_stop(struct rproc *rproc)
909909
struct mtk_scp *scp = rproc->priv;
910910
int ret;
911911

912-
ret = clk_prepare_enable(scp->clk);
912+
ret = clk_enable(scp->clk);
913913
if (ret) {
914914
dev_err(scp->dev, "failed to enable clocks\n");
915915
return ret;
916916
}
917917

918918
scp->data->scp_reset_assert(scp);
919919
scp->data->scp_stop(scp);
920-
clk_disable_unprepare(scp->clk);
920+
clk_disable(scp->clk);
921921

922922
return 0;
923923
}
924924

925+
static int scp_prepare(struct rproc *rproc)
926+
{
927+
struct mtk_scp *scp = rproc->priv;
928+
929+
return clk_prepare(scp->clk);
930+
}
931+
932+
static int scp_unprepare(struct rproc *rproc)
933+
{
934+
struct mtk_scp *scp = rproc->priv;
935+
936+
clk_unprepare(scp->clk);
937+
return 0;
938+
}
939+
925940
static const struct rproc_ops scp_ops = {
941+
.prepare = scp_prepare,
942+
.unprepare = scp_unprepare,
926943
.start = scp_start,
927944
.stop = scp_stop,
928945
.load = scp_load,

drivers/remoteproc/mtk_scp_ipi.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
171171
WARN_ON(len > scp_sizes->ipi_share_buffer_size) || WARN_ON(!buf))
172172
return -EINVAL;
173173

174-
ret = clk_prepare_enable(scp->clk);
174+
ret = clk_enable(scp->clk);
175175
if (ret) {
176176
dev_err(scp->dev, "failed to enable clock\n");
177177
return ret;
@@ -211,7 +211,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
211211

212212
unlock_mutex:
213213
mutex_unlock(&scp->send_lock);
214-
clk_disable_unprepare(scp->clk);
214+
clk_disable(scp->clk);
215215

216216
return ret;
217217
}

0 commit comments

Comments
 (0)