* [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader
@ 2025-06-17 19:34 Hiago De Franco
2025-06-17 19:34 ` [PATCH v5 1/3] pmdomain: core: introduce dev_pm_genpd_is_on() Hiago De Franco
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Hiago De Franco @ 2025-06-17 19:34 UTC (permalink / raw)
To: Mathieu Poirier, Ulf Hansson, linux-pm, linux-remoteproc
Cc: Shawn Guo, Sascha Hauer, Bjorn Andersson, Hiago De Franco, imx,
linux-arm-kernel, linux-kernel, Peng Fan, daniel.baluta,
iuliana.prodan, Rafael J . Wysocki
From: Hiago De Franco <hiago.franco@toradex.com>
This patch series depends on Ulf's patches that are currently under
review, "pmdomain: Add generic ->sync_state() support to genpd" [1].
Without them, this series is not going to work.
For the i.MX8X and i.MX8 family SoCs, currently when the remotecore is
started by the bootloader and the M core and A core are in the same
partition, the driver is not capable to detect the remote core and
report the correct state of it.
This patch series implement a new function, dev_pm_genpd_is_on(), which
returns the power status of a given power domain (M core power domains
IMX_SC_R_M4_0_PID0 and IMX_SC_R_M4_0_MU_1A in this case). If it is
already powered on, the driver will attach to it.
Finally, the imx_rproc_clk_enable() function was also changed to make it
return before dev_clk_get() is called, as it currently generates an SCU
fault reset if the remote core is already running and the kernel tries
to enable the clock again. These changes are a follow up from a v1 sent
to imx_rproc [2] and from a reported regression [3].
[1] https://lore.kernel.org/all/20250523134025.75130-1-ulf.hansson@linaro.org/
[2] https://lore.kernel.org/lkml/20250423155131.101473-1-hiagofranco@gmail.com/
[3] https://lore.kernel.org/lkml/20250404141713.ac2ntcsjsf7epdfa@hiago-nb/
v5:
- pm_runtime_get_sync() removed in favor of pm_runtime_resume_and_get(),
checking the return value of it.
- Added pm_runtime_disable() and pm_runtime_put() to imx_rproc_remove().
- Fixed missing "()" in dev_pm_genpd_is_on description.
- Updated dev_pm_genpd_is_on() function description to be explicit the
function reflects the current power status of the device and that this
might change after the function returns, especially if the genpd is
shared.
v4:
- https://lore.kernel.org/lkml/20250602131906.25751-1-hiagofranco@gmail.com/
v3:
- https://lore.kernel.org/all/20250519171514.61974-1-hiagofranco@gmail.com/
v2:
- https://lore.kernel.org/lkml/20250507160056.11876-1-hiagofranco@gmail.com/
v1:
- https://lore.kernel.org/lkml/20250505154849.64889-1-hiagofranco@gmail.com/
Hiago De Franco (3):
pmdomain: core: introduce dev_pm_genpd_is_on()
remoteproc: imx_rproc: skip clock enable when M-core is managed by the
SCU
remoteproc: imx_rproc: detect and attach to pre-booted remote cores
drivers/pmdomain/core.c | 33 +++++++++++++++++++++++++++
drivers/remoteproc/imx_rproc.c | 41 ++++++++++++++++++++++++++++------
include/linux/pm_domain.h | 6 +++++
3 files changed, 73 insertions(+), 7 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 1/3] pmdomain: core: introduce dev_pm_genpd_is_on()
2025-06-17 19:34 [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader Hiago De Franco
@ 2025-06-17 19:34 ` Hiago De Franco
2025-06-18 3:01 ` Bjorn Andersson
2025-06-17 19:34 ` [PATCH v5 2/3] remoteproc: imx_rproc: skip clock enable when M-core is managed by the SCU Hiago De Franco
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Hiago De Franco @ 2025-06-17 19:34 UTC (permalink / raw)
To: Mathieu Poirier, Ulf Hansson, linux-pm, linux-remoteproc
Cc: Shawn Guo, Sascha Hauer, Bjorn Andersson, Hiago De Franco, imx,
linux-arm-kernel, linux-kernel, Peng Fan, daniel.baluta,
iuliana.prodan, Rafael J . Wysocki
From: Hiago De Franco <hiago.franco@toradex.com>
This helper function returns the current power status of a given generic
power domain.
As example, remoteproc/imx_rproc.c can now use this function to check
the power status of the remote core to properly set "attached" or
"offline" modes.
Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
---
v4 -> v5:
- s/dev_pm_genpd_is_on/dev_pm_genpd_is_on()/ in function description.
- Updated function description to be explicit the function reflects the
current power status and that this might change after the function
returns, especially if the genpd is shared.
v3 -> v4:
- New patch.
---
drivers/pmdomain/core.c | 33 +++++++++++++++++++++++++++++++++
include/linux/pm_domain.h | 6 ++++++
2 files changed, 39 insertions(+)
diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index ff5c7f2b69ce..2f387e15cb75 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -758,6 +758,39 @@ int dev_pm_genpd_rpm_always_on(struct device *dev, bool on)
}
EXPORT_SYMBOL_GPL(dev_pm_genpd_rpm_always_on);
+/**
+ * dev_pm_genpd_is_on() - Get device's current power domain status
+ *
+ * @dev: Device to get the current power status
+ *
+ * This function checks whether the generic power domain associated with the
+ * given device is on or not by verifying if genpd_status_on equals
+ * GENPD_STATE_ON.
+ *
+ * Note: this function returns the power status of the genpd at the time of the
+ * call. The power status may change after due to activity from other devices
+ * sharing the same genpd. Therefore, this information should not be relied for
+ * long-term decisions about the device power state.
+ *
+ * Return: 'true' if the device's power domain is on, 'false' otherwise.
+ */
+bool dev_pm_genpd_is_on(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ bool is_on;
+
+ genpd = dev_to_genpd_safe(dev);
+ if (!genpd)
+ return false;
+
+ genpd_lock(genpd);
+ is_on = genpd_status_on(genpd);
+ genpd_unlock(genpd);
+
+ return is_on;
+}
+EXPORT_SYMBOL_GPL(dev_pm_genpd_is_on);
+
/**
* pm_genpd_inc_rejected() - Adjust the rejected/usage counts for an idle-state.
*
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 0b18160901a2..c12580b6579b 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -301,6 +301,7 @@ void dev_pm_genpd_synced_poweroff(struct device *dev);
int dev_pm_genpd_set_hwmode(struct device *dev, bool enable);
bool dev_pm_genpd_get_hwmode(struct device *dev);
int dev_pm_genpd_rpm_always_on(struct device *dev, bool on);
+bool dev_pm_genpd_is_on(struct device *dev);
extern struct dev_power_governor simple_qos_governor;
extern struct dev_power_governor pm_domain_always_on_gov;
@@ -393,6 +394,11 @@ static inline int dev_pm_genpd_rpm_always_on(struct device *dev, bool on)
return -EOPNOTSUPP;
}
+static inline bool dev_pm_genpd_is_on(struct device *dev)
+{
+ return false;
+}
+
#define simple_qos_governor (*(struct dev_power_governor *)(NULL))
#define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL))
#endif
--
2.39.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 2/3] remoteproc: imx_rproc: skip clock enable when M-core is managed by the SCU
2025-06-17 19:34 [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader Hiago De Franco
2025-06-17 19:34 ` [PATCH v5 1/3] pmdomain: core: introduce dev_pm_genpd_is_on() Hiago De Franco
@ 2025-06-17 19:34 ` Hiago De Franco
2025-06-23 15:15 ` Mathieu Poirier
2025-06-17 19:34 ` [PATCH v5 3/3] remoteproc: imx_rproc: detect and attach to pre-booted remote cores Hiago De Franco
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Hiago De Franco @ 2025-06-17 19:34 UTC (permalink / raw)
To: Mathieu Poirier, Ulf Hansson, linux-pm, linux-remoteproc
Cc: Shawn Guo, Sascha Hauer, Bjorn Andersson, Hiago De Franco, imx,
linux-arm-kernel, linux-kernel, Peng Fan, daniel.baluta,
iuliana.prodan, Rafael J . Wysocki, Peng Fan
From: Hiago De Franco <hiago.franco@toradex.com>
For the i.MX8X and i.MX8 family SoCs, when the M-core is powered up
by the bootloader, M-core and Linux are in same SCFW (System Controller
Firmware) partition, so linux has permission to control M-core.
But when M-core is started, the SCFW will automatically enable the clock
and configure the rate, and any users that want to enable the clock will
get error 'LOCKED' from SCFW. So current imx_rproc.c probe function
fails because clk_prepare_enable also fails. With that, the M-core power
domain is powered off when it is still running, causing a SCU (System
Controller Unit) fault reset, and the system restarts.
To address the issue, ignore handling the clk for i.MX8X and i.MX8 M-core,
because SCFW will automatically enable and configure the clock.
Suggested-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
---
v4 -> v5:
- Unchanged.
v3 -> v4:
- Unchanged.
v2 -> v3:
- Unchanged.
v1 -> v2:
- Commit description updated, as suggested. Fixed Peng Fan email.
---
drivers/remoteproc/imx_rproc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 74299af1d7f1..627e57a88db2 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -1029,8 +1029,8 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv)
struct device *dev = priv->dev;
int ret;
- /* Remote core is not under control of Linux */
- if (dcfg->method == IMX_RPROC_NONE)
+ /* Remote core is not under control of Linux or it is managed by SCU API */
+ if (dcfg->method == IMX_RPROC_NONE || dcfg->method == IMX_RPROC_SCU_API)
return 0;
priv->clk = devm_clk_get(dev, NULL);
--
2.39.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v5 3/3] remoteproc: imx_rproc: detect and attach to pre-booted remote cores
2025-06-17 19:34 [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader Hiago De Franco
2025-06-17 19:34 ` [PATCH v5 1/3] pmdomain: core: introduce dev_pm_genpd_is_on() Hiago De Franco
2025-06-17 19:34 ` [PATCH v5 2/3] remoteproc: imx_rproc: skip clock enable when M-core is managed by the SCU Hiago De Franco
@ 2025-06-17 19:34 ` Hiago De Franco
2025-06-18 12:12 ` Ulf Hansson
2025-06-23 15:29 ` Mathieu Poirier
2025-06-19 8:44 ` [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader Peng Fan
2025-06-23 15:32 ` Mathieu Poirier
4 siblings, 2 replies; 13+ messages in thread
From: Hiago De Franco @ 2025-06-17 19:34 UTC (permalink / raw)
To: Mathieu Poirier, Ulf Hansson, linux-pm, linux-remoteproc
Cc: Shawn Guo, Sascha Hauer, Bjorn Andersson, Hiago De Franco, imx,
linux-arm-kernel, linux-kernel, Peng Fan, daniel.baluta,
iuliana.prodan, Rafael J . Wysocki
From: Hiago De Franco <hiago.franco@toradex.com>
When the remote core is started before Linux boots (e.g., by the
bootloader), the driver currently is not able to attach because it only
checks for cores running in different partitions. If the core was kicked
by the bootloader, it is in the same partition as Linux and it is
already up and running.
This adds power mode verification through dev_pm_genpd_is_on(), enabling
the driver to detect when the remote core is already running and
properly attach to it if all the power domain devices are on.
To accomplish this, we need to avoid passing any attach_data or flags to
dev_pm_domain_attach_list(), letting the platform device become a
consumer of the power domain provider. With that the current power state
of the genpds will not change, allowing the detection of the remote core
power state.
We enable and sync the device runtime PM during probe to make sure the
power domains are correctly managed when the core is controlled by the
kernel.
Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
---
v4 -> v5:
- pm_runtime_get_sync() removed in favor of
pm_runtime_resume_and_get(). Now it also checks the return value of
this function.
- Added pm_runtime_disable() and pm_runtime_put() to imx_rproc_remove()
function.
v3 -> v4:
- Changed to use the new dev_pm_genpd_is_on() function instead, as
suggested by Ulf. This will now get the power status of the two
remote cores power domains to decided if imx_rpoc needs to attach or
not. In order to do that, pm_runtime_enable() and
pm_runtime_get_sync() were introduced and pd_data was removed.
v2 -> v3:
- Unchanged.
v1 -> v2:
- Dropped unecessary include. Removed the imx_rproc_is_on function, as
suggested.
---
drivers/remoteproc/imx_rproc.c | 37 +++++++++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 627e57a88db2..b53083f2553e 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -18,6 +18,7 @@
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
#include <linux/reboot.h>
#include <linux/regmap.h>
#include <linux/remoteproc.h>
@@ -890,10 +891,8 @@ static int imx_rproc_partition_notify(struct notifier_block *nb,
static int imx_rproc_attach_pd(struct imx_rproc *priv)
{
struct device *dev = priv->dev;
- int ret;
- struct dev_pm_domain_attach_data pd_data = {
- .pd_flags = PD_FLAG_DEV_LINK_ON,
- };
+ int ret, i;
+ bool detached = true;
/*
* If there is only one power-domain entry, the platform driver framework
@@ -902,7 +901,22 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
if (dev->pm_domain)
return 0;
- ret = dev_pm_domain_attach_list(dev, &pd_data, &priv->pd_list);
+ ret = dev_pm_domain_attach_list(dev, NULL, &priv->pd_list);
+ /*
+ * If all the power domain devices are already turned on, the remote
+ * core is already up when the kernel booted (e.g. kicked by the
+ * bootloader). In this case attach to it.
+ */
+ for (i = 0; i < ret; i++) {
+ if (!dev_pm_genpd_is_on(priv->pd_list->pd_devs[i])) {
+ detached = false;
+ break;
+ }
+ }
+
+ if (detached)
+ priv->rproc->state = RPROC_DETACHED;
+
return ret < 0 ? ret : 0;
}
@@ -1146,6 +1160,15 @@ static int imx_rproc_probe(struct platform_device *pdev)
}
}
+ if (dcfg->method == IMX_RPROC_SCU_API) {
+ pm_runtime_enable(dev);
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret) {
+ dev_err(dev, "pm_runtime get failed: %d\n", ret);
+ goto err_put_clk;
+ }
+ }
+
ret = rproc_add(rproc);
if (ret) {
dev_err(dev, "rproc_add failed\n");
@@ -1171,6 +1194,10 @@ static void imx_rproc_remove(struct platform_device *pdev)
struct rproc *rproc = platform_get_drvdata(pdev);
struct imx_rproc *priv = rproc->priv;
+ if (priv->dcfg->method == IMX_RPROC_SCU_API) {
+ pm_runtime_disable(priv->dev);
+ pm_runtime_put(priv->dev);
+ }
clk_disable_unprepare(priv->clk);
rproc_del(rproc);
imx_rproc_put_scu(rproc);
--
2.39.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v5 1/3] pmdomain: core: introduce dev_pm_genpd_is_on()
2025-06-17 19:34 ` [PATCH v5 1/3] pmdomain: core: introduce dev_pm_genpd_is_on() Hiago De Franco
@ 2025-06-18 3:01 ` Bjorn Andersson
0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Andersson @ 2025-06-18 3:01 UTC (permalink / raw)
To: Hiago De Franco
Cc: Mathieu Poirier, Ulf Hansson, linux-pm, linux-remoteproc,
Shawn Guo, Sascha Hauer, Hiago De Franco, imx, linux-arm-kernel,
linux-kernel, Peng Fan, daniel.baluta, iuliana.prodan,
Rafael J . Wysocki
On Tue, Jun 17, 2025 at 04:34:48PM -0300, Hiago De Franco wrote:
> From: Hiago De Franco <hiago.franco@toradex.com>
>
> This helper function returns the current power status of a given generic
> power domain.
>
> As example, remoteproc/imx_rproc.c can now use this function to check
> the power status of the remote core to properly set "attached" or
> "offline" modes.
>
> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Regards,
Bjorn
> ---
> v4 -> v5:
> - s/dev_pm_genpd_is_on/dev_pm_genpd_is_on()/ in function description.
> - Updated function description to be explicit the function reflects the
> current power status and that this might change after the function
> returns, especially if the genpd is shared.
>
> v3 -> v4:
> - New patch.
> ---
> drivers/pmdomain/core.c | 33 +++++++++++++++++++++++++++++++++
> include/linux/pm_domain.h | 6 ++++++
> 2 files changed, 39 insertions(+)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index ff5c7f2b69ce..2f387e15cb75 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -758,6 +758,39 @@ int dev_pm_genpd_rpm_always_on(struct device *dev, bool on)
> }
> EXPORT_SYMBOL_GPL(dev_pm_genpd_rpm_always_on);
>
> +/**
> + * dev_pm_genpd_is_on() - Get device's current power domain status
> + *
> + * @dev: Device to get the current power status
> + *
> + * This function checks whether the generic power domain associated with the
> + * given device is on or not by verifying if genpd_status_on equals
> + * GENPD_STATE_ON.
> + *
> + * Note: this function returns the power status of the genpd at the time of the
> + * call. The power status may change after due to activity from other devices
> + * sharing the same genpd. Therefore, this information should not be relied for
> + * long-term decisions about the device power state.
> + *
> + * Return: 'true' if the device's power domain is on, 'false' otherwise.
> + */
> +bool dev_pm_genpd_is_on(struct device *dev)
> +{
> + struct generic_pm_domain *genpd;
> + bool is_on;
> +
> + genpd = dev_to_genpd_safe(dev);
> + if (!genpd)
> + return false;
> +
> + genpd_lock(genpd);
> + is_on = genpd_status_on(genpd);
> + genpd_unlock(genpd);
> +
> + return is_on;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_genpd_is_on);
> +
> /**
> * pm_genpd_inc_rejected() - Adjust the rejected/usage counts for an idle-state.
> *
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 0b18160901a2..c12580b6579b 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -301,6 +301,7 @@ void dev_pm_genpd_synced_poweroff(struct device *dev);
> int dev_pm_genpd_set_hwmode(struct device *dev, bool enable);
> bool dev_pm_genpd_get_hwmode(struct device *dev);
> int dev_pm_genpd_rpm_always_on(struct device *dev, bool on);
> +bool dev_pm_genpd_is_on(struct device *dev);
>
> extern struct dev_power_governor simple_qos_governor;
> extern struct dev_power_governor pm_domain_always_on_gov;
> @@ -393,6 +394,11 @@ static inline int dev_pm_genpd_rpm_always_on(struct device *dev, bool on)
> return -EOPNOTSUPP;
> }
>
> +static inline bool dev_pm_genpd_is_on(struct device *dev)
> +{
> + return false;
> +}
> +
> #define simple_qos_governor (*(struct dev_power_governor *)(NULL))
> #define pm_domain_always_on_gov (*(struct dev_power_governor *)(NULL))
> #endif
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/3] remoteproc: imx_rproc: detect and attach to pre-booted remote cores
2025-06-17 19:34 ` [PATCH v5 3/3] remoteproc: imx_rproc: detect and attach to pre-booted remote cores Hiago De Franco
@ 2025-06-18 12:12 ` Ulf Hansson
2025-06-23 15:29 ` Mathieu Poirier
1 sibling, 0 replies; 13+ messages in thread
From: Ulf Hansson @ 2025-06-18 12:12 UTC (permalink / raw)
To: Hiago De Franco
Cc: Mathieu Poirier, linux-pm, linux-remoteproc, Shawn Guo,
Sascha Hauer, Bjorn Andersson, Hiago De Franco, imx,
linux-arm-kernel, linux-kernel, Peng Fan, daniel.baluta,
iuliana.prodan, Rafael J . Wysocki
On Tue, 17 Jun 2025 at 21:36, Hiago De Franco <hiagofranco@gmail.com> wrote:
>
> From: Hiago De Franco <hiago.franco@toradex.com>
>
> When the remote core is started before Linux boots (e.g., by the
> bootloader), the driver currently is not able to attach because it only
> checks for cores running in different partitions. If the core was kicked
> by the bootloader, it is in the same partition as Linux and it is
> already up and running.
>
> This adds power mode verification through dev_pm_genpd_is_on(), enabling
> the driver to detect when the remote core is already running and
> properly attach to it if all the power domain devices are on.
>
> To accomplish this, we need to avoid passing any attach_data or flags to
> dev_pm_domain_attach_list(), letting the platform device become a
> consumer of the power domain provider. With that the current power state
> of the genpds will not change, allowing the detection of the remote core
> power state.
>
> We enable and sync the device runtime PM during probe to make sure the
> power domains are correctly managed when the core is controlled by the
> kernel.
>
> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> v4 -> v5:
> - pm_runtime_get_sync() removed in favor of
> pm_runtime_resume_and_get(). Now it also checks the return value of
> this function.
> - Added pm_runtime_disable() and pm_runtime_put() to imx_rproc_remove()
> function.
> v3 -> v4:
> - Changed to use the new dev_pm_genpd_is_on() function instead, as
> suggested by Ulf. This will now get the power status of the two
> remote cores power domains to decided if imx_rpoc needs to attach or
> not. In order to do that, pm_runtime_enable() and
> pm_runtime_get_sync() were introduced and pd_data was removed.
> v2 -> v3:
> - Unchanged.
> v1 -> v2:
> - Dropped unecessary include. Removed the imx_rproc_is_on function, as
> suggested.
> ---
> drivers/remoteproc/imx_rproc.c | 37 +++++++++++++++++++++++++++++-----
> 1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 627e57a88db2..b53083f2553e 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -18,6 +18,7 @@
> #include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> #include <linux/reboot.h>
> #include <linux/regmap.h>
> #include <linux/remoteproc.h>
> @@ -890,10 +891,8 @@ static int imx_rproc_partition_notify(struct notifier_block *nb,
> static int imx_rproc_attach_pd(struct imx_rproc *priv)
> {
> struct device *dev = priv->dev;
> - int ret;
> - struct dev_pm_domain_attach_data pd_data = {
> - .pd_flags = PD_FLAG_DEV_LINK_ON,
> - };
> + int ret, i;
> + bool detached = true;
>
> /*
> * If there is only one power-domain entry, the platform driver framework
> @@ -902,7 +901,22 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
> if (dev->pm_domain)
> return 0;
>
> - ret = dev_pm_domain_attach_list(dev, &pd_data, &priv->pd_list);
> + ret = dev_pm_domain_attach_list(dev, NULL, &priv->pd_list);
> + /*
> + * If all the power domain devices are already turned on, the remote
> + * core is already up when the kernel booted (e.g. kicked by the
> + * bootloader). In this case attach to it.
> + */
> + for (i = 0; i < ret; i++) {
> + if (!dev_pm_genpd_is_on(priv->pd_list->pd_devs[i])) {
> + detached = false;
> + break;
> + }
> + }
> +
> + if (detached)
> + priv->rproc->state = RPROC_DETACHED;
> +
> return ret < 0 ? ret : 0;
> }
>
> @@ -1146,6 +1160,15 @@ static int imx_rproc_probe(struct platform_device *pdev)
> }
> }
>
> + if (dcfg->method == IMX_RPROC_SCU_API) {
> + pm_runtime_enable(dev);
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret) {
> + dev_err(dev, "pm_runtime get failed: %d\n", ret);
> + goto err_put_clk;
> + }
> + }
> +
> ret = rproc_add(rproc);
> if (ret) {
> dev_err(dev, "rproc_add failed\n");
> @@ -1171,6 +1194,10 @@ static void imx_rproc_remove(struct platform_device *pdev)
> struct rproc *rproc = platform_get_drvdata(pdev);
> struct imx_rproc *priv = rproc->priv;
>
> + if (priv->dcfg->method == IMX_RPROC_SCU_API) {
> + pm_runtime_disable(priv->dev);
> + pm_runtime_put(priv->dev);
> + }
> clk_disable_unprepare(priv->clk);
> rproc_del(rproc);
> imx_rproc_put_scu(rproc);
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader
2025-06-17 19:34 [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader Hiago De Franco
` (2 preceding siblings ...)
2025-06-17 19:34 ` [PATCH v5 3/3] remoteproc: imx_rproc: detect and attach to pre-booted remote cores Hiago De Franco
@ 2025-06-19 8:44 ` Peng Fan
2025-06-23 15:32 ` Mathieu Poirier
4 siblings, 0 replies; 13+ messages in thread
From: Peng Fan @ 2025-06-19 8:44 UTC (permalink / raw)
To: Hiago De Franco
Cc: Mathieu Poirier, Ulf Hansson, linux-pm, linux-remoteproc,
Shawn Guo, Sascha Hauer, Bjorn Andersson, Hiago De Franco, imx,
linux-arm-kernel, linux-kernel, daniel.baluta, iuliana.prodan,
Rafael J . Wysocki
On Tue, Jun 17, 2025 at 04:34:47PM -0300, Hiago De Franco wrote:
>From: Hiago De Franco <hiago.franco@toradex.com>
>
>This patch series depends on Ulf's patches that are currently under
>review, "pmdomain: Add generic ->sync_state() support to genpd" [1].
>Without them, this series is not going to work.
>
>For the i.MX8X and i.MX8 family SoCs, currently when the remotecore is
>started by the bootloader and the M core and A core are in the same
>partition, the driver is not capable to detect the remote core and
>report the correct state of it.
>
>This patch series implement a new function, dev_pm_genpd_is_on(), which
>returns the power status of a given power domain (M core power domains
>IMX_SC_R_M4_0_PID0 and IMX_SC_R_M4_0_MU_1A in this case). If it is
>already powered on, the driver will attach to it.
>
>Finally, the imx_rproc_clk_enable() function was also changed to make it
>return before dev_clk_get() is called, as it currently generates an SCU
>fault reset if the remote core is already running and the kernel tries
>to enable the clock again. These changes are a follow up from a v1 sent
>to imx_rproc [2] and from a reported regression [3].
>
>[1] https://lore.kernel.org/all/20250523134025.75130-1-ulf.hansson@linaro.org/
>[2] https://lore.kernel.org/lkml/20250423155131.101473-1-hiagofranco@gmail.com/
>[3] https://lore.kernel.org/lkml/20250404141713.ac2ntcsjsf7epdfa@hiago-nb/
>
>v5:
>- pm_runtime_get_sync() removed in favor of pm_runtime_resume_and_get(),
> checking the return value of it.
>- Added pm_runtime_disable() and pm_runtime_put() to imx_rproc_remove().
>- Fixed missing "()" in dev_pm_genpd_is_on description.
>- Updated dev_pm_genpd_is_on() function description to be explicit the
> function reflects the current power status of the device and that this
> might change after the function returns, especially if the genpd is
> shared.
>
>v4:
>- https://lore.kernel.org/lkml/20250602131906.25751-1-hiagofranco@gmail.com/
>
>v3:
>- https://lore.kernel.org/all/20250519171514.61974-1-hiagofranco@gmail.com/
>
>v2:
>- https://lore.kernel.org/lkml/20250507160056.11876-1-hiagofranco@gmail.com/
>
>v1:
>- https://lore.kernel.org/lkml/20250505154849.64889-1-hiagofranco@gmail.com/
>
>Hiago De Franco (3):
> pmdomain: core: introduce dev_pm_genpd_is_on()
> remoteproc: imx_rproc: skip clock enable when M-core is managed by the
> SCU
> remoteproc: imx_rproc: detect and attach to pre-booted remote cores
Reviewed-by: Peng Fan <peng.fan@nxp.com>
>
> drivers/pmdomain/core.c | 33 +++++++++++++++++++++++++++
> drivers/remoteproc/imx_rproc.c | 41 ++++++++++++++++++++++++++++------
> include/linux/pm_domain.h | 6 +++++
> 3 files changed, 73 insertions(+), 7 deletions(-)
>
>--
>2.39.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/3] remoteproc: imx_rproc: skip clock enable when M-core is managed by the SCU
2025-06-17 19:34 ` [PATCH v5 2/3] remoteproc: imx_rproc: skip clock enable when M-core is managed by the SCU Hiago De Franco
@ 2025-06-23 15:15 ` Mathieu Poirier
2025-06-24 16:15 ` Hiago De Franco
0 siblings, 1 reply; 13+ messages in thread
From: Mathieu Poirier @ 2025-06-23 15:15 UTC (permalink / raw)
To: Hiago De Franco
Cc: Ulf Hansson, linux-pm, linux-remoteproc, Shawn Guo, Sascha Hauer,
Bjorn Andersson, Hiago De Franco, imx, linux-arm-kernel,
linux-kernel, Peng Fan, daniel.baluta, iuliana.prodan,
Rafael J . Wysocki, Peng Fan
Hi Hiago,
On Tue, Jun 17, 2025 at 04:34:49PM -0300, Hiago De Franco wrote:
> From: Hiago De Franco <hiago.franco@toradex.com>
>
> For the i.MX8X and i.MX8 family SoCs, when the M-core is powered up
> by the bootloader, M-core and Linux are in same SCFW (System Controller
> Firmware) partition, so linux has permission to control M-core.
Ok
>
> But when M-core is started, the SCFW will automatically enable the clock
I find the "But when M-core is started" part confusing. Started by who? And
are you making a distinction between "powered up" and "started"? It is not
possible for someone that doesn't have HW documentation to understand what is
going on.
> and configure the rate, and any users that want to enable the clock will
> get error 'LOCKED' from SCFW. So current imx_rproc.c probe function
> fails because clk_prepare_enable also fails. With that, the M-core power
> domain is powered off when it is still running, causing a SCU (System
> Controller Unit) fault reset, and the system restarts.
>
> To address the issue, ignore handling the clk for i.MX8X and i.MX8 M-core,
> because SCFW will automatically enable and configure the clock.
>
> Suggested-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> ---
> v4 -> v5:
> - Unchanged.
> v3 -> v4:
> - Unchanged.
> v2 -> v3:
> - Unchanged.
> v1 -> v2:
> - Commit description updated, as suggested. Fixed Peng Fan email.
> ---
> drivers/remoteproc/imx_rproc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 74299af1d7f1..627e57a88db2 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -1029,8 +1029,8 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv)
> struct device *dev = priv->dev;
> int ret;
>
> - /* Remote core is not under control of Linux */
> - if (dcfg->method == IMX_RPROC_NONE)
> + /* Remote core is not under control of Linux or it is managed by SCU API */
> + if (dcfg->method == IMX_RPROC_NONE || dcfg->method == IMX_RPROC_SCU_API)
> return 0;
>
> priv->clk = devm_clk_get(dev, NULL);
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/3] remoteproc: imx_rproc: detect and attach to pre-booted remote cores
2025-06-17 19:34 ` [PATCH v5 3/3] remoteproc: imx_rproc: detect and attach to pre-booted remote cores Hiago De Franco
2025-06-18 12:12 ` Ulf Hansson
@ 2025-06-23 15:29 ` Mathieu Poirier
2025-06-24 16:16 ` Hiago De Franco
1 sibling, 1 reply; 13+ messages in thread
From: Mathieu Poirier @ 2025-06-23 15:29 UTC (permalink / raw)
To: Hiago De Franco
Cc: Ulf Hansson, linux-pm, linux-remoteproc, Shawn Guo, Sascha Hauer,
Bjorn Andersson, Hiago De Franco, imx, linux-arm-kernel,
linux-kernel, Peng Fan, daniel.baluta, iuliana.prodan,
Rafael J . Wysocki
On Tue, Jun 17, 2025 at 04:34:50PM -0300, Hiago De Franco wrote:
> From: Hiago De Franco <hiago.franco@toradex.com>
>
> When the remote core is started before Linux boots (e.g., by the
> bootloader), the driver currently is not able to attach because it only
> checks for cores running in different partitions. If the core was kicked
Again, we have a nomenclature issue here with "If the core was kicked by the
bootloader". What does "kicked" mean here? Is it just powered and held in
reset or is it executing. And are you referring to the A core or the M core?
> by the bootloader, it is in the same partition as Linux and it is
> already up and running.
>
> This adds power mode verification through dev_pm_genpd_is_on(), enabling
> the driver to detect when the remote core is already running and
> properly attach to it if all the power domain devices are on.
>
> To accomplish this, we need to avoid passing any attach_data or flags to
> dev_pm_domain_attach_list(), letting the platform device become a
> consumer of the power domain provider. With that the current power state
> of the genpds will not change, allowing the detection of the remote core
> power state.
>
> We enable and sync the device runtime PM during probe to make sure the
> power domains are correctly managed when the core is controlled by the
> kernel.
>
> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> ---
> v4 -> v5:
> - pm_runtime_get_sync() removed in favor of
> pm_runtime_resume_and_get(). Now it also checks the return value of
> this function.
> - Added pm_runtime_disable() and pm_runtime_put() to imx_rproc_remove()
> function.
> v3 -> v4:
> - Changed to use the new dev_pm_genpd_is_on() function instead, as
> suggested by Ulf. This will now get the power status of the two
> remote cores power domains to decided if imx_rpoc needs to attach or
> not. In order to do that, pm_runtime_enable() and
> pm_runtime_get_sync() were introduced and pd_data was removed.
> v2 -> v3:
> - Unchanged.
> v1 -> v2:
> - Dropped unecessary include. Removed the imx_rproc_is_on function, as
> suggested.
> ---
> drivers/remoteproc/imx_rproc.c | 37 +++++++++++++++++++++++++++++-----
> 1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 627e57a88db2..b53083f2553e 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -18,6 +18,7 @@
> #include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> #include <linux/reboot.h>
> #include <linux/regmap.h>
> #include <linux/remoteproc.h>
> @@ -890,10 +891,8 @@ static int imx_rproc_partition_notify(struct notifier_block *nb,
> static int imx_rproc_attach_pd(struct imx_rproc *priv)
> {
> struct device *dev = priv->dev;
> - int ret;
> - struct dev_pm_domain_attach_data pd_data = {
> - .pd_flags = PD_FLAG_DEV_LINK_ON,
> - };
> + int ret, i;
> + bool detached = true;
>
> /*
> * If there is only one power-domain entry, the platform driver framework
> @@ -902,7 +901,22 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
> if (dev->pm_domain)
> return 0;
>
> - ret = dev_pm_domain_attach_list(dev, &pd_data, &priv->pd_list);
> + ret = dev_pm_domain_attach_list(dev, NULL, &priv->pd_list);
> + /*
> + * If all the power domain devices are already turned on, the remote
> + * core is already up when the kernel booted (e.g. kicked by the
> + * bootloader). In this case attach to it.
Same comment as above. What got kicked? A core or M core. And what does
"kicked" mean? I can guess what is happening but guessing rarely leads to
anything positive.
In the next revision, please use other words than "kicked".
> + */
> + for (i = 0; i < ret; i++) {
> + if (!dev_pm_genpd_is_on(priv->pd_list->pd_devs[i])) {
> + detached = false;
> + break;
> + }
> + }
> +
> + if (detached)
> + priv->rproc->state = RPROC_DETACHED;
> +
Ok for the above.
> return ret < 0 ? ret : 0;
> }
>
> @@ -1146,6 +1160,15 @@ static int imx_rproc_probe(struct platform_device *pdev)
> }
> }
>
> + if (dcfg->method == IMX_RPROC_SCU_API) {
> + pm_runtime_enable(dev);
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret) {
> + dev_err(dev, "pm_runtime get failed: %d\n", ret);
> + goto err_put_clk;
> + }
> + }
> +
> ret = rproc_add(rproc);
> if (ret) {
> dev_err(dev, "rproc_add failed\n");
> @@ -1171,6 +1194,10 @@ static void imx_rproc_remove(struct platform_device *pdev)
> struct rproc *rproc = platform_get_drvdata(pdev);
> struct imx_rproc *priv = rproc->priv;
>
> + if (priv->dcfg->method == IMX_RPROC_SCU_API) {
> + pm_runtime_disable(priv->dev);
> + pm_runtime_put(priv->dev);
> + }
> clk_disable_unprepare(priv->clk);
> rproc_del(rproc);
> imx_rproc_put_scu(rproc);
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader
2025-06-17 19:34 [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader Hiago De Franco
` (3 preceding siblings ...)
2025-06-19 8:44 ` [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader Peng Fan
@ 2025-06-23 15:32 ` Mathieu Poirier
2025-06-24 16:18 ` Hiago De Franco
4 siblings, 1 reply; 13+ messages in thread
From: Mathieu Poirier @ 2025-06-23 15:32 UTC (permalink / raw)
To: Hiago De Franco
Cc: Ulf Hansson, linux-pm, linux-remoteproc, Shawn Guo, Sascha Hauer,
Bjorn Andersson, Hiago De Franco, imx, linux-arm-kernel,
linux-kernel, Peng Fan, daniel.baluta, iuliana.prodan,
Rafael J . Wysocki
On Tue, Jun 17, 2025 at 04:34:47PM -0300, Hiago De Franco wrote:
> From: Hiago De Franco <hiago.franco@toradex.com>
>
> This patch series depends on Ulf's patches that are currently under
> review, "pmdomain: Add generic ->sync_state() support to genpd" [1].
> Without them, this series is not going to work.
Please resend this patchset when [1] and the work in patch 1/3 have been merged.
Thanks,
Mathieu
>
> For the i.MX8X and i.MX8 family SoCs, currently when the remotecore is
> started by the bootloader and the M core and A core are in the same
> partition, the driver is not capable to detect the remote core and
> report the correct state of it.
>
> This patch series implement a new function, dev_pm_genpd_is_on(), which
> returns the power status of a given power domain (M core power domains
> IMX_SC_R_M4_0_PID0 and IMX_SC_R_M4_0_MU_1A in this case). If it is
> already powered on, the driver will attach to it.
>
> Finally, the imx_rproc_clk_enable() function was also changed to make it
> return before dev_clk_get() is called, as it currently generates an SCU
> fault reset if the remote core is already running and the kernel tries
> to enable the clock again. These changes are a follow up from a v1 sent
> to imx_rproc [2] and from a reported regression [3].
>
> [1] https://lore.kernel.org/all/20250523134025.75130-1-ulf.hansson@linaro.org/
> [2] https://lore.kernel.org/lkml/20250423155131.101473-1-hiagofranco@gmail.com/
> [3] https://lore.kernel.org/lkml/20250404141713.ac2ntcsjsf7epdfa@hiago-nb/
>
> v5:
> - pm_runtime_get_sync() removed in favor of pm_runtime_resume_and_get(),
> checking the return value of it.
> - Added pm_runtime_disable() and pm_runtime_put() to imx_rproc_remove().
> - Fixed missing "()" in dev_pm_genpd_is_on description.
> - Updated dev_pm_genpd_is_on() function description to be explicit the
> function reflects the current power status of the device and that this
> might change after the function returns, especially if the genpd is
> shared.
>
> v4:
> - https://lore.kernel.org/lkml/20250602131906.25751-1-hiagofranco@gmail.com/
>
> v3:
> - https://lore.kernel.org/all/20250519171514.61974-1-hiagofranco@gmail.com/
>
> v2:
> - https://lore.kernel.org/lkml/20250507160056.11876-1-hiagofranco@gmail.com/
>
> v1:
> - https://lore.kernel.org/lkml/20250505154849.64889-1-hiagofranco@gmail.com/
>
> Hiago De Franco (3):
> pmdomain: core: introduce dev_pm_genpd_is_on()
> remoteproc: imx_rproc: skip clock enable when M-core is managed by the
> SCU
> remoteproc: imx_rproc: detect and attach to pre-booted remote cores
>
> drivers/pmdomain/core.c | 33 +++++++++++++++++++++++++++
> drivers/remoteproc/imx_rproc.c | 41 ++++++++++++++++++++++++++++------
> include/linux/pm_domain.h | 6 +++++
> 3 files changed, 73 insertions(+), 7 deletions(-)
>
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/3] remoteproc: imx_rproc: skip clock enable when M-core is managed by the SCU
2025-06-23 15:15 ` Mathieu Poirier
@ 2025-06-24 16:15 ` Hiago De Franco
0 siblings, 0 replies; 13+ messages in thread
From: Hiago De Franco @ 2025-06-24 16:15 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Ulf Hansson, linux-pm, linux-remoteproc, Shawn Guo, Sascha Hauer,
Bjorn Andersson, Hiago De Franco, imx, linux-arm-kernel,
linux-kernel, Peng Fan, daniel.baluta, iuliana.prodan,
Rafael J . Wysocki, Peng Fan
Hi Mathieu,
On Mon, Jun 23, 2025 at 09:15:52AM -0600, Mathieu Poirier wrote:
> Hi Hiago,
>
> On Tue, Jun 17, 2025 at 04:34:49PM -0300, Hiago De Franco wrote:
> > From: Hiago De Franco <hiago.franco@toradex.com>
> >
> > For the i.MX8X and i.MX8 family SoCs, when the M-core is powered up
> > by the bootloader, M-core and Linux are in same SCFW (System Controller
> > Firmware) partition, so linux has permission to control M-core.
>
> Ok
>
> >
> > But when M-core is started, the SCFW will automatically enable the clock
>
> I find the "But when M-core is started" part confusing. Started by who? And
> are you making a distinction between "powered up" and "started"? It is not
> possible for someone that doesn't have HW documentation to understand what is
> going on.
Ok, understood, I will improve this in the next revision. Just to make
clear, I am talking about Cortex-A bootloader starting Cortex-M (U-Boot
bootaux command in this case). This means powered up, started and
running.
>
> > and configure the rate, and any users that want to enable the clock will
> > get error 'LOCKED' from SCFW. So current imx_rproc.c probe function
> > fails because clk_prepare_enable also fails. With that, the M-core power
> > domain is powered off when it is still running, causing a SCU (System
> > Controller Unit) fault reset, and the system restarts.
> >
> > To address the issue, ignore handling the clk for i.MX8X and i.MX8 M-core,
> > because SCFW will automatically enable and configure the clock.
> >
> > Suggested-by: Peng Fan <peng.fan@nxp.com>
> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> > ---
> > v4 -> v5:
> > - Unchanged.
> > v3 -> v4:
> > - Unchanged.
> > v2 -> v3:
> > - Unchanged.
> > v1 -> v2:
> > - Commit description updated, as suggested. Fixed Peng Fan email.
> > ---
> > drivers/remoteproc/imx_rproc.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> > index 74299af1d7f1..627e57a88db2 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -1029,8 +1029,8 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv)
> > struct device *dev = priv->dev;
> > int ret;
> >
> > - /* Remote core is not under control of Linux */
> > - if (dcfg->method == IMX_RPROC_NONE)
> > + /* Remote core is not under control of Linux or it is managed by SCU API */
> > + if (dcfg->method == IMX_RPROC_NONE || dcfg->method == IMX_RPROC_SCU_API)
> > return 0;
> >
> > priv->clk = devm_clk_get(dev, NULL);
> > --
> > 2.39.5
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 3/3] remoteproc: imx_rproc: detect and attach to pre-booted remote cores
2025-06-23 15:29 ` Mathieu Poirier
@ 2025-06-24 16:16 ` Hiago De Franco
0 siblings, 0 replies; 13+ messages in thread
From: Hiago De Franco @ 2025-06-24 16:16 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Ulf Hansson, linux-pm, linux-remoteproc, Shawn Guo, Sascha Hauer,
Bjorn Andersson, Hiago De Franco, imx, linux-arm-kernel,
linux-kernel, Peng Fan, daniel.baluta, iuliana.prodan,
Rafael J . Wysocki
On Mon, Jun 23, 2025 at 09:29:47AM -0600, Mathieu Poirier wrote:
> On Tue, Jun 17, 2025 at 04:34:50PM -0300, Hiago De Franco wrote:
> > From: Hiago De Franco <hiago.franco@toradex.com>
> >
> > When the remote core is started before Linux boots (e.g., by the
> > bootloader), the driver currently is not able to attach because it only
> > checks for cores running in different partitions. If the core was kicked
>
> Again, we have a nomenclature issue here with "If the core was kicked by the
> bootloader". What does "kicked" mean here? Is it just powered and held in
> reset or is it executing. And are you referring to the A core or the M core?
Ok, same here, I will improve this in the next revision. Thanks.
>
>
> > by the bootloader, it is in the same partition as Linux and it is
> > already up and running.
> >
> > This adds power mode verification through dev_pm_genpd_is_on(), enabling
> > the driver to detect when the remote core is already running and
> > properly attach to it if all the power domain devices are on.
> >
> > To accomplish this, we need to avoid passing any attach_data or flags to
> > dev_pm_domain_attach_list(), letting the platform device become a
> > consumer of the power domain provider. With that the current power state
> > of the genpds will not change, allowing the detection of the remote core
> > power state.
> >
> > We enable and sync the device runtime PM during probe to make sure the
> > power domains are correctly managed when the core is controlled by the
> > kernel.
> >
> > Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
> > Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> > ---
> > v4 -> v5:
> > - pm_runtime_get_sync() removed in favor of
> > pm_runtime_resume_and_get(). Now it also checks the return value of
> > this function.
> > - Added pm_runtime_disable() and pm_runtime_put() to imx_rproc_remove()
> > function.
> > v3 -> v4:
> > - Changed to use the new dev_pm_genpd_is_on() function instead, as
> > suggested by Ulf. This will now get the power status of the two
> > remote cores power domains to decided if imx_rpoc needs to attach or
> > not. In order to do that, pm_runtime_enable() and
> > pm_runtime_get_sync() were introduced and pd_data was removed.
> > v2 -> v3:
> > - Unchanged.
> > v1 -> v2:
> > - Dropped unecessary include. Removed the imx_rproc_is_on function, as
> > suggested.
> > ---
> > drivers/remoteproc/imx_rproc.c | 37 +++++++++++++++++++++++++++++-----
> > 1 file changed, 32 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> > index 627e57a88db2..b53083f2553e 100644
> > --- a/drivers/remoteproc/imx_rproc.c
> > +++ b/drivers/remoteproc/imx_rproc.c
> > @@ -18,6 +18,7 @@
> > #include <linux/of_reserved_mem.h>
> > #include <linux/platform_device.h>
> > #include <linux/pm_domain.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/reboot.h>
> > #include <linux/regmap.h>
> > #include <linux/remoteproc.h>
> > @@ -890,10 +891,8 @@ static int imx_rproc_partition_notify(struct notifier_block *nb,
> > static int imx_rproc_attach_pd(struct imx_rproc *priv)
> > {
> > struct device *dev = priv->dev;
> > - int ret;
> > - struct dev_pm_domain_attach_data pd_data = {
> > - .pd_flags = PD_FLAG_DEV_LINK_ON,
> > - };
> > + int ret, i;
> > + bool detached = true;
> >
> > /*
> > * If there is only one power-domain entry, the platform driver framework
> > @@ -902,7 +901,22 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv)
> > if (dev->pm_domain)
> > return 0;
> >
> > - ret = dev_pm_domain_attach_list(dev, &pd_data, &priv->pd_list);
> > + ret = dev_pm_domain_attach_list(dev, NULL, &priv->pd_list);
> > + /*
> > + * If all the power domain devices are already turned on, the remote
> > + * core is already up when the kernel booted (e.g. kicked by the
> > + * bootloader). In this case attach to it.
>
> Same comment as above. What got kicked? A core or M core. And what does
> "kicked" mean? I can guess what is happening but guessing rarely leads to
> anything positive.
>
> In the next revision, please use other words than "kicked".
Noted, thanks.
>
>
> > + */
> > + for (i = 0; i < ret; i++) {
> > + if (!dev_pm_genpd_is_on(priv->pd_list->pd_devs[i])) {
> > + detached = false;
> > + break;
> > + }
> > + }
> > +
> > + if (detached)
> > + priv->rproc->state = RPROC_DETACHED;
> > +
>
> Ok for the above.
>
> > return ret < 0 ? ret : 0;
> > }
> >
> > @@ -1146,6 +1160,15 @@ static int imx_rproc_probe(struct platform_device *pdev)
> > }
> > }
> >
> > + if (dcfg->method == IMX_RPROC_SCU_API) {
> > + pm_runtime_enable(dev);
> > + ret = pm_runtime_resume_and_get(dev);
> > + if (ret) {
> > + dev_err(dev, "pm_runtime get failed: %d\n", ret);
> > + goto err_put_clk;
> > + }
> > + }
> > +
> > ret = rproc_add(rproc);
> > if (ret) {
> > dev_err(dev, "rproc_add failed\n");
> > @@ -1171,6 +1194,10 @@ static void imx_rproc_remove(struct platform_device *pdev)
> > struct rproc *rproc = platform_get_drvdata(pdev);
> > struct imx_rproc *priv = rproc->priv;
> >
> > + if (priv->dcfg->method == IMX_RPROC_SCU_API) {
> > + pm_runtime_disable(priv->dev);
> > + pm_runtime_put(priv->dev);
> > + }
> > clk_disable_unprepare(priv->clk);
> > rproc_del(rproc);
> > imx_rproc_put_scu(rproc);
> > --
> > 2.39.5
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader
2025-06-23 15:32 ` Mathieu Poirier
@ 2025-06-24 16:18 ` Hiago De Franco
0 siblings, 0 replies; 13+ messages in thread
From: Hiago De Franco @ 2025-06-24 16:18 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Ulf Hansson, linux-pm, linux-remoteproc, Shawn Guo, Sascha Hauer,
Bjorn Andersson, Hiago De Franco, imx, linux-arm-kernel,
linux-kernel, Peng Fan, daniel.baluta, iuliana.prodan,
Rafael J . Wysocki
On Mon, Jun 23, 2025 at 09:32:54AM -0600, Mathieu Poirier wrote:
> On Tue, Jun 17, 2025 at 04:34:47PM -0300, Hiago De Franco wrote:
> > From: Hiago De Franco <hiago.franco@toradex.com>
> >
> > This patch series depends on Ulf's patches that are currently under
> > review, "pmdomain: Add generic ->sync_state() support to genpd" [1].
> > Without them, this series is not going to work.
>
> Please resend this patchset when [1] and the work in patch 1/3 have been merged.
All right, I will send the v6 with the corrections you mentioned and
resend it when the other patches have been merged.
Best regards,
Hiago.
>
> Thanks,
> Mathieu
>
> >
> > For the i.MX8X and i.MX8 family SoCs, currently when the remotecore is
> > started by the bootloader and the M core and A core are in the same
> > partition, the driver is not capable to detect the remote core and
> > report the correct state of it.
> >
> > This patch series implement a new function, dev_pm_genpd_is_on(), which
> > returns the power status of a given power domain (M core power domains
> > IMX_SC_R_M4_0_PID0 and IMX_SC_R_M4_0_MU_1A in this case). If it is
> > already powered on, the driver will attach to it.
> >
> > Finally, the imx_rproc_clk_enable() function was also changed to make it
> > return before dev_clk_get() is called, as it currently generates an SCU
> > fault reset if the remote core is already running and the kernel tries
> > to enable the clock again. These changes are a follow up from a v1 sent
> > to imx_rproc [2] and from a reported regression [3].
> >
> > [1] https://lore.kernel.org/all/20250523134025.75130-1-ulf.hansson@linaro.org/
> > [2] https://lore.kernel.org/lkml/20250423155131.101473-1-hiagofranco@gmail.com/
> > [3] https://lore.kernel.org/lkml/20250404141713.ac2ntcsjsf7epdfa@hiago-nb/
> >
> > v5:
> > - pm_runtime_get_sync() removed in favor of pm_runtime_resume_and_get(),
> > checking the return value of it.
> > - Added pm_runtime_disable() and pm_runtime_put() to imx_rproc_remove().
> > - Fixed missing "()" in dev_pm_genpd_is_on description.
> > - Updated dev_pm_genpd_is_on() function description to be explicit the
> > function reflects the current power status of the device and that this
> > might change after the function returns, especially if the genpd is
> > shared.
> >
> > v4:
> > - https://lore.kernel.org/lkml/20250602131906.25751-1-hiagofranco@gmail.com/
> >
> > v3:
> > - https://lore.kernel.org/all/20250519171514.61974-1-hiagofranco@gmail.com/
> >
> > v2:
> > - https://lore.kernel.org/lkml/20250507160056.11876-1-hiagofranco@gmail.com/
> >
> > v1:
> > - https://lore.kernel.org/lkml/20250505154849.64889-1-hiagofranco@gmail.com/
> >
> > Hiago De Franco (3):
> > pmdomain: core: introduce dev_pm_genpd_is_on()
> > remoteproc: imx_rproc: skip clock enable when M-core is managed by the
> > SCU
> > remoteproc: imx_rproc: detect and attach to pre-booted remote cores
> >
> > drivers/pmdomain/core.c | 33 +++++++++++++++++++++++++++
> > drivers/remoteproc/imx_rproc.c | 41 ++++++++++++++++++++++++++++------
> > include/linux/pm_domain.h | 6 +++++
> > 3 files changed, 73 insertions(+), 7 deletions(-)
> >
> > --
> > 2.39.5
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-06-24 19:55 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-17 19:34 [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader Hiago De Franco
2025-06-17 19:34 ` [PATCH v5 1/3] pmdomain: core: introduce dev_pm_genpd_is_on() Hiago De Franco
2025-06-18 3:01 ` Bjorn Andersson
2025-06-17 19:34 ` [PATCH v5 2/3] remoteproc: imx_rproc: skip clock enable when M-core is managed by the SCU Hiago De Franco
2025-06-23 15:15 ` Mathieu Poirier
2025-06-24 16:15 ` Hiago De Franco
2025-06-17 19:34 ` [PATCH v5 3/3] remoteproc: imx_rproc: detect and attach to pre-booted remote cores Hiago De Franco
2025-06-18 12:12 ` Ulf Hansson
2025-06-23 15:29 ` Mathieu Poirier
2025-06-24 16:16 ` Hiago De Franco
2025-06-19 8:44 ` [PATCH v5 0/3] remoteproc: imx_rproc: allow attaching to running core kicked by the bootloader Peng Fan
2025-06-23 15:32 ` Mathieu Poirier
2025-06-24 16:18 ` Hiago De Franco
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).