Skip to contentSkip to conversation

remoteproc: mediatek: Break lock dependency to `prepare_lock`

PATCH Applied Tzung-Bi Shih 3 participants 4 messages Jan 12 – Jan 26

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 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:

Quoted 43 lines

-> #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

Quoted 8 lines

clk_prepare
-> #0 (prepare_lock)
clk_prepare
scp_ipi_send
scp_send_ipi
mtk_rpmsg_send
rpmsg_send
cros_ec_pkt_xfer_rpmsg

---
v2:
- Re-organized commit message.
- Rebased to next-20260109.

v1: https://lore.kernel.org/r/[email protected]

 drivers/remoteproc/mtk_scp.c     | 39 +++++++++++++++++++++++---------
 drivers/remoteproc/mtk_scp_ipi.c |  4 ++--
 2 files changed, 30 insertions(+), 13 deletions(-)
drivers/remoteproc/mtk_scp.c +28 -11
index 328541e62158..4651311aeb07 100644
@@ -283,7 +283,7 @@ static irqreturn_t scp_irq_handler(int irq, void *priv)
 	struct mtk_scp *scp = priv;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(scp->dev, "failed to enable clocks\n");
 		return IRQ_NONE;
@@ -291,7 +291,7 @@ static irqreturn_t scp_irq_handler(int irq, void *priv)
 
 	scp->data->scp_irq_handler(scp);
 
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 
 	return IRQ_HANDLED;
 }
@@ -665,7 +665,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
 	struct device *dev = scp->dev;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(dev, "failed to enable clocks\n");
 		return ret;
@@ -680,7 +680,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
 
 	ret = scp_elf_load_segments(rproc, fw);
 leave:
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 
 	return ret;
 }
@@ -691,14 +691,14 @@ static int scp_parse_fw(struct rproc *rproc, const struct firmware *fw)
 	struct device *dev = scp->dev;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(dev, "failed to enable clocks\n");
 		return ret;
 	}
 
 	ret = scp_ipi_init(scp, fw);
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 	return ret;
 }
 
@@ -709,7 +709,7 @@ static int scp_start(struct rproc *rproc)
 	struct scp_run *run = &scp->run;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(dev, "failed to enable clocks\n");
 		return ret;
@@ -734,14 +734,14 @@ static int scp_start(struct rproc *rproc)
 		goto stop;
 	}
 
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 	dev_info(dev, "SCP is ready. FW version %s\n", run->fw_ver);
 
 	return 0;
 
 stop:
 	scp->data->scp_reset_assert(scp);
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 	return ret;
 }
 
@@ -909,7 +909,7 @@ static int scp_stop(struct rproc *rproc)
 	struct mtk_scp *scp = rproc->priv;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(scp->dev, "failed to enable clocks\n");
 		return ret;
@@ -917,12 +917,29 @@ static int scp_stop(struct rproc *rproc)
 
 	scp->data->scp_reset_assert(scp);
 	scp->data->scp_stop(scp);
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 
 	return 0;
 }
 
+static int scp_prepare(struct rproc *rproc)
+{
+	struct mtk_scp *scp = rproc->priv;
+
+	return clk_prepare(scp->clk);
+}
+
+static int scp_unprepare(struct rproc *rproc)
+{
+	struct mtk_scp *scp = rproc->priv;
+
+	clk_unprepare(scp->clk);
+	return 0;
+}
+
 static const struct rproc_ops scp_ops = {
+	.prepare	= scp_prepare,
+	.unprepare	= scp_unprepare,
 	.start		= scp_start,
 	.stop		= scp_stop,
 	.load		= scp_load,
drivers/remoteproc/mtk_scp_ipi.c +2 -2
index c068227e251e..7a37e273b3af 100644
@@ -171,7 +171,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
 	    WARN_ON(len > scp_sizes->ipi_share_buffer_size) || WARN_ON(!buf))
 		return -EINVAL;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(scp->dev, "failed to enable clock\n");
 		return ret;
@@ -211,7 +211,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
 
 unlock_mutex:
 	mutex_unlock(&scp->send_lock);
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 
 	return ret;
 }
Signature · 1 line
2.52.0.457.g6b5491de43-goog
maintainer
#1 lore raw
Quoted 36 lines · Tzung-Bi Shih

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.

Quoted 5 lines

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.

I certainly would have appreciated this kind of information upon first looking at this patch. It would have saved me valuable time, time that could have been spent on reviewing other people's work.

Quoted 214 lines

A simplified calling trace reported by lockdep:

Quoted 43 lines

-> #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

Quoted 8 lines

clk_prepare
-> #0 (prepare_lock)
clk_prepare
scp_ipi_send
scp_send_ipi
mtk_rpmsg_send
rpmsg_send
cros_ec_pkt_xfer_rpmsg

---
v2:
- Re-organized commit message.
- Rebased to next-20260109.

v1: https://lore.kernel.org/r/[email protected]

 drivers/remoteproc/mtk_scp.c     | 39 +++++++++++++++++++++++---------
 drivers/remoteproc/mtk_scp_ipi.c |  4 ++--
 2 files changed, 30 insertions(+), 13 deletions(-)
drivers/remoteproc/mtk_scp.c +28 -11
index 328541e62158..4651311aeb07 100644
@@ -283,7 +283,7 @@ static irqreturn_t scp_irq_handler(int irq, void *priv)
        struct mtk_scp *scp = priv;
        int ret;
 
-       ret = clk_prepare_enable(scp->clk);
+       ret = clk_enable(scp->clk);
        if (ret) {
                dev_err(scp->dev, "failed to enable clocks\n");
                return IRQ_NONE;
@@ -291,7 +291,7 @@ static irqreturn_t scp_irq_handler(int irq, void *priv)
 
        scp->data->scp_irq_handler(scp);
 
-       clk_disable_unprepare(scp->clk);
+       clk_disable(scp->clk);
 
        return IRQ_HANDLED;
 }
@@ -665,7 +665,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
        struct device *dev = scp->dev;
        int ret;
 
-       ret = clk_prepare_enable(scp->clk);
+       ret = clk_enable(scp->clk);
        if (ret) {
                dev_err(dev, "failed to enable clocks\n");
                return ret;
@@ -680,7 +680,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
 
        ret = scp_elf_load_segments(rproc, fw);
 leave:
-       clk_disable_unprepare(scp->clk);
+       clk_disable(scp->clk);
 
        return ret;
 }
@@ -691,14 +691,14 @@ static int scp_parse_fw(struct rproc *rproc, const struct firmware *fw)
        struct device *dev = scp->dev;
        int ret;
 
-       ret = clk_prepare_enable(scp->clk);
+       ret = clk_enable(scp->clk);
        if (ret) {
                dev_err(dev, "failed to enable clocks\n");
                return ret;
        }
 
        ret = scp_ipi_init(scp, fw);
-       clk_disable_unprepare(scp->clk);
+       clk_disable(scp->clk);
        return ret;
 }
 
@@ -709,7 +709,7 @@ static int scp_start(struct rproc *rproc)
        struct scp_run *run = &scp->run;
        int ret;
 
-       ret = clk_prepare_enable(scp->clk);
+       ret = clk_enable(scp->clk);
        if (ret) {
                dev_err(dev, "failed to enable clocks\n");
                return ret;
@@ -734,14 +734,14 @@ static int scp_start(struct rproc *rproc)
                goto stop;
        }
 
-       clk_disable_unprepare(scp->clk);
+       clk_disable(scp->clk);
        dev_info(dev, "SCP is ready. FW version %s\n", run->fw_ver);
 
        return 0;
 
 stop:
        scp->data->scp_reset_assert(scp);
-       clk_disable_unprepare(scp->clk);
+       clk_disable(scp->clk);
        return ret;
 }
 
@@ -909,7 +909,7 @@ static int scp_stop(struct rproc *rproc)
        struct mtk_scp *scp = rproc->priv;
        int ret;
 
-       ret = clk_prepare_enable(scp->clk);
+       ret = clk_enable(scp->clk);
        if (ret) {
                dev_err(scp->dev, "failed to enable clocks\n");
                return ret;
@@ -917,12 +917,29 @@ static int scp_stop(struct rproc *rproc)
 
        scp->data->scp_reset_assert(scp);
        scp->data->scp_stop(scp);
-       clk_disable_unprepare(scp->clk);
+       clk_disable(scp->clk);
 
        return 0;
 }
 
+static int scp_prepare(struct rproc *rproc)
+{
+       struct mtk_scp *scp = rproc->priv;
+
+       return clk_prepare(scp->clk);
+}
+
+static int scp_unprepare(struct rproc *rproc)
+{
+       struct mtk_scp *scp = rproc->priv;
+
+       clk_unprepare(scp->clk);
+       return 0;
+}
+
 static const struct rproc_ops scp_ops = {
+       .prepare        = scp_prepare,
+       .unprepare      = scp_unprepare,
        .start          = scp_start,
        .stop           = scp_stop,
        .load           = scp_load,
drivers/remoteproc/mtk_scp_ipi.c +2 -3
index c068227e251e..7a37e273b3af 100644
@@ -171,7 +171,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
            WARN_ON(len > scp_sizes->ipi_share_buffer_size) || WARN_ON(!buf))
                return -EINVAL;
 
-       ret = clk_prepare_enable(scp->clk);
+       ret = clk_enable(scp->clk);
        if (ret) {
                dev_err(scp->dev, "failed to enable clock\n");
                return ret;
@@ -211,7 +211,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
 
 unlock_mutex:
        mutex_unlock(&scp->send_lock);
-       clk_disable_unprepare(scp->clk);
+       clk_disable(scp->clk);
 
        return ret;
 }
--

2.52.0.457.g6b5491de43-goog

Reviewed-by Tested-by on v2 1/1
#2 lore raw
In reply to Tzung-Bi Shih
Quoted 94 lines · Tzung-Bi Shih

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:

Quoted 43 lines

-> #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

Quoted 8 lines

clk_prepare
-> #0 (prepare_lock)
clk_prepare
scp_ipi_send
scp_send_ipi
mtk_rpmsg_send
rpmsg_send
cros_ec_pkt_xfer_rpmsg

Quoted 1 line

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.

maintainer Applied
#3 lore raw
In reply to Tzung-Bi Shih
Quoted 99 lines · Tzung-Bi Shih

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:

Quoted 43 lines

-> #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

Quoted 8 lines

clk_prepare
-> #0 (prepare_lock)
clk_prepare
scp_ipi_send
scp_send_ipi
mtk_rpmsg_send
rpmsg_send
cros_ec_pkt_xfer_rpmsg

---
v2:
- Re-organized commit message.
- Rebased to next-20260109.

Quoted 6 lines

v1: https://lore.kernel.org/r/[email protected]

 drivers/remoteproc/mtk_scp.c     | 39 +++++++++++++++++++++++---------
 drivers/remoteproc/mtk_scp_ipi.c |  4 ++--
 2 files changed, 30 insertions(+), 13 deletions(-)

Applied.

Quoted 149 lines
drivers/remoteproc/mtk_scp.c +28 -11
index 328541e62158..4651311aeb07 100644
@@ -283,7 +283,7 @@ static irqreturn_t scp_irq_handler(int irq, void *priv)
 	struct mtk_scp *scp = priv;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(scp->dev, "failed to enable clocks\n");
 		return IRQ_NONE;
@@ -291,7 +291,7 @@ static irqreturn_t scp_irq_handler(int irq, void *priv)
 
 	scp->data->scp_irq_handler(scp);
 
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 
 	return IRQ_HANDLED;
 }
@@ -665,7 +665,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
 	struct device *dev = scp->dev;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(dev, "failed to enable clocks\n");
 		return ret;
@@ -680,7 +680,7 @@ static int scp_load(struct rproc *rproc, const struct firmware *fw)
 
 	ret = scp_elf_load_segments(rproc, fw);
 leave:
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 
 	return ret;
 }
@@ -691,14 +691,14 @@ static int scp_parse_fw(struct rproc *rproc, const struct firmware *fw)
 	struct device *dev = scp->dev;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(dev, "failed to enable clocks\n");
 		return ret;
 	}
 
 	ret = scp_ipi_init(scp, fw);
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 	return ret;
 }
 
@@ -709,7 +709,7 @@ static int scp_start(struct rproc *rproc)
 	struct scp_run *run = &scp->run;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(dev, "failed to enable clocks\n");
 		return ret;
@@ -734,14 +734,14 @@ static int scp_start(struct rproc *rproc)
 		goto stop;
 	}
 
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 	dev_info(dev, "SCP is ready. FW version %s\n", run->fw_ver);
 
 	return 0;
 
 stop:
 	scp->data->scp_reset_assert(scp);
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 	return ret;
 }
 
@@ -909,7 +909,7 @@ static int scp_stop(struct rproc *rproc)
 	struct mtk_scp *scp = rproc->priv;
 	int ret;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(scp->dev, "failed to enable clocks\n");
 		return ret;
@@ -917,12 +917,29 @@ static int scp_stop(struct rproc *rproc)
 
 	scp->data->scp_reset_assert(scp);
 	scp->data->scp_stop(scp);
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 
 	return 0;
 }
 
+static int scp_prepare(struct rproc *rproc)
+{
+	struct mtk_scp *scp = rproc->priv;
+
+	return clk_prepare(scp->clk);
+}
+
+static int scp_unprepare(struct rproc *rproc)
+{
+	struct mtk_scp *scp = rproc->priv;
+
+	clk_unprepare(scp->clk);
+	return 0;
+}
+
 static const struct rproc_ops scp_ops = {
+	.prepare	= scp_prepare,
+	.unprepare	= scp_unprepare,
 	.start		= scp_start,
 	.stop		= scp_stop,
 	.load		= scp_load,
drivers/remoteproc/mtk_scp_ipi.c +2 -2
index c068227e251e..7a37e273b3af 100644
@@ -171,7 +171,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
 	    WARN_ON(len > scp_sizes->ipi_share_buffer_size) || WARN_ON(!buf))
 		return -EINVAL;
 
-	ret = clk_prepare_enable(scp->clk);
+	ret = clk_enable(scp->clk);
 	if (ret) {
 		dev_err(scp->dev, "failed to enable clock\n");
 		return ret;
@@ -211,7 +211,7 @@ int scp_ipi_send(struct mtk_scp *scp, u32 id, void *buf, unsigned int len,
 
 unlock_mutex:
 	mutex_unlock(&scp->send_lock);
-	clk_disable_unprepare(scp->clk);
+	clk_disable(scp->clk);
 
 	return ret;
 }
Signature · 1 line
2.52.0.457.g6b5491de43-goog