Hi Ulf,

On Tue, May 20, 2025 at 02:21:49PM +0200, Ulf Hansson wrote:

On Mon, 19 May 2025 at 19:24, Hiago De Franco [off-list ref] wrote:

quoted

Hi Ulf,

On Mon, May 19, 2025 at 04:33:30PM +0200, Ulf Hansson wrote:

quoted

On Fri, 9 May 2025 at 21:13, Hiago De Franco [off-list ref] wrote:

quoted

On Fri, May 09, 2025 at 12:37:02PM +0200, Ulf Hansson wrote:

quoted

On Thu, 8 May 2025 at 22:28, Hiago De Franco [off-list ref] wrote:

quoted

Hello,

On Thu, May 08, 2025 at 12:03:33PM +0200, Ulf Hansson wrote:

quoted

On Wed, 7 May 2025 at 18:02, Hiago De Franco [off-list ref] wrote:

quoted

From: Hiago De Franco <redacted>

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 the SCU interface, enabling
the driver to detect when the remote core is already running and
properly attach to it.

Signed-off-by: Hiago De Franco <redacted>
Suggested-by: Peng Fan <peng.fan@nxp.com>
---
v2: Dropped unecessary include. Removed the imx_rproc_is_on function, as
suggested.
---
 drivers/remoteproc/imx_rproc.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 627e57a88db2..9b6e9e41b7fc 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c

@@ -949,6 +949,19 @@ static int imx_rproc_detect_mode(struct imx_rproc *priv)
                        if (of_property_read_u32(dev->of_node, "fsl,entry-address", &priv->entry))
                                return -EINVAL;

+                       /*
+                        * If remote core is already running (e.g. kicked by
+                        * the bootloader), attach to it.
+                        */
+                       ret = imx_sc_pm_get_resource_power_mode(priv->ipc_handle,
+                                                               priv->rsrc_id);
+                       if (ret < 0)
+                               dev_err(dev, "failed to get power resource %d mode, ret %d\n",
+                                       priv->rsrc_id, ret);
+
+                       if (ret == IMX_SC_PM_PW_MODE_ON)
+                               priv->rproc->state = RPROC_DETACHED;
+
                        return imx_rproc_attach_pd(priv);

Why is it important to potentially set "priv->rproc->state =
RPROC_DETACHED" before calling imx_rproc_attach_pd()?

Would it be possible to do it the other way around? First calling
imx_rproc_attach_pd() then get the power-mode to know if
RPROC_DETACHED should be set or not?

The main reason why I ask, is because of how we handle the single PM
domain case. In that case, the PM domain has already been attached
(and powered-on) before we reach this point.

I am not sure if I understood correcly, let me know if I missed
something. From my understanding in this case it does not matter, since
the RPROC_DETACHED will only be a flag to trigger the attach callback
from rproc_validate(), when rproc_add() is called inside
remoteproc_core.c.

Okay, I see.

To me, it sounds like we should introduce a new genpd helper function
instead. Something along the lines of this (drivers/pmdomain/core.c)

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;
}

After imx_rproc_attach_pd() has run, we have the devices that
correspond to the genpd(s). Those can then be passed as in-parameters
to the above function to get the power-state of their PM domains
(genpds). Based on that, we can decide if priv->rproc->state should be
to RPROC_DETACHED or not. Right?

Got your idea, I think it should work yes, I am not so sure how. From
what I can see these power domains are managed by
drivers/pmdomain/imx/scu-pd.c and by enabling the debug messages I can
see the power mode is correct when the remote core is powered on:

[    0.317369] imx-scu-pd system-controller:power-controller: cm40-pid0 : IMX_SC_PM_PW_MODE_ON

and powered off:

[    0.314953] imx-scu-pd system-controller:power-controller: cm40-pid0 : IMX_SC_PM_PW_MODE_OFF

But I cannot see how to integrate this into the dev_pm_genpd_is_on() you
proposed. For a quick check, I added this function and it always return
NULL at dev_to_genpd_safe(). Can you help me to understand this part?

As your device has multiple PM domains and those gets attached with
dev_pm_domain_attach_list(), the device(s) that you should use with
dev_pm_genpd_is_on() are in imx_rproc->pd_list->pd_devs[n].

Ok got it, thanks for sharing.

I just send the v3 with the changes Peng proposed (here
https://lore.kernel.org/lkml/20250519171514.61974-1-hiagofranco@gmail.com/T/#t (local)),
but I am a bit confused which path we should take, the initial approach
proposed or using these PD functions. Maybe we can discuss this in the
new v3 patch series?

I think it would be better if we can avoid sharing low-level firmware
functions for PM domains. I am worried that they may become abused for
other future use-cases.

So, if possible, I would rather make us try to use
dev_pm_genpd_is_on() (or something along those lines), but let's see
what Peng thinks about it before we make the decision.

There are two power domains for this m4:
power-domains = <&pd IMX_SC_R_M4_0_PID0>, <&pd IMX_SC_R_M4_0_MU_1A>;

So before attach the pd, dev_pm_genpd_is_on should also return false
per my understanding. If run dev_pm_genpd_is_on after attaching the pd,
the pd will be powered on. So we are not able to know whether M4 is started
by bootloader or not.

Hiago's case needs the real power status before attaching the
pd to set remoteproc as DETACHED(M4 kicked by bootloader) or OFFLINE(
M4 not kicked by bootloader) state.

Seems there is no other SCFW API to check whether M4 is started by
bootloader.

I not have good idea as of now except directly checking the real
power status to indicate M4 started by bootloader or not. Or using a
device tree property runtime added by bootloader,
saying "fsl,rproc-started"?


Thanks,
Peng

[...]

Kind regards
Uffe