Re: [PATCH v2 3/3] remoteproc: imx_rproc: add power mode check for remote core attachment

[Peng Fan <peng.fan@oss.nxp.com>]

Hi Ulf,

On Thu, May 08, 2025 at 12:03:33PM +0200, Ulf Hansson wrote:
>On Wed, 7 May 2025 at 18:02, 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 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 <hiago.franco@toradex.com>
>> 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?

If M4 is not powered up by bootloader, attach_pd is to power
up the related power domains. And the rproc->state should be OFFLINE.

If M4 is powered up by bootloader, the rproc->state should be set as
DETACHED, then attach_pd here will not touch the real pd, because
scu-pd driver set is_off as false when doing pm_genpd_init.
In this case, we still need attach_pd to avoid power shutdown when
pd_ignore_unused and also need to support linux stop/start m4 even
it is started by bootloader.


So we could not reverse the logic.

Regards,
Peng

>
>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.
>
>>                 }
>>
>> --
>> 2.39.5
>>
>
>Kind regards
>Uffe