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

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?

In this way we don't need to export unnecessary firmware functions
from firmware/imx/misc.c, as patch1/3 does.

If you think it can work, I can help to cook a formal patch for the
above helper that you can fold into your series. Let me know.

quoted

With that we can correcly attach to the remote core running, which was
not possible before, where the function returns at "return
imx_rproc_attach_pd(priv);" with the RPROC_OFFLINE state to
rproc_validate().

I see, thanks for clarifying!

Kind regards
Uffe

Thank you!
Hiago.