lkml.org 
[lkml]   [2023]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor
From
On 1/18/2023 7:24 PM, Mathieu Poirier wrote:
> Hi Iuliana,
>
> On Tue, Jan 17, 2023 at 01:03:57PM +0200, Iuliana Prodan (OSS) wrote:
>> From: Iuliana Prodan <iuliana.prodan@nxp.com>
>>
>> There are cases when we want to test a simple "hello world"
>> application on the DSP and we don't have IPC between the cores.
>> Therefore, skip the wait for remote processor to start.
>>
>> Added "ignore_dsp_ready" flag while inserting the module to ignore
>> remote processor reply after start.
>> By default, this is off - do not ignore reply from rproc.
>>
>> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
>>
>> ---
>> Changes since v2
>> - s/ignoreready/ignore_dsp_ready
>>
>> Changes since v1
>> - change BIT(31) to BIT(1) for REMOTE_SKIP_WAIT
>>
>> ---
>> drivers/remoteproc/imx_dsp_rproc.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
>> index 95da1cbefacf..22e2ef068c67 100644
>> --- a/drivers/remoteproc/imx_dsp_rproc.c
>> +++ b/drivers/remoteproc/imx_dsp_rproc.c
>> @@ -26,9 +26,20 @@
>> #include "remoteproc_elf_helpers.h"
>> #include "remoteproc_internal.h"
>>
>> +#define IMX_DSP_IGNORE_REMOTE_READY 0
>> +
>> +/*
>> + * Module parameters
>> + */
>> +static unsigned int imx_dsp_rproc_ignore_ready = IMX_DSP_IGNORE_REMOTE_READY;
> Static variables are initialised to '0' and as such this is not needed.
>
>> +module_param_named(ignore_dsp_ready, imx_dsp_rproc_ignore_ready, int, 0644);
>> +MODULE_PARM_DESC(ignore_dsp_ready,
>> + "Ignore remote proc reply after start, default is 0 (off).");
>> +
>> #define DSP_RPROC_CLK_MAX 5
>>
>> #define REMOTE_IS_READY BIT(0)
>> +#define REMOTE_SKIP_WAIT BIT(1)
>> #define REMOTE_READY_WAIT_MAX_RETRIES 500
>>
>> /* att flags */
>> @@ -285,6 +296,9 @@ static int imx_dsp_rproc_ready(struct rproc *rproc)
>> if (!priv->rxdb_ch)
>> return 0;
>>
>> + if (priv->flags & REMOTE_SKIP_WAIT)
>> + return 0;
>> +
> This looks very hackish to me...
>
> Here priv->rxdb_ch is valid and as such the DB mailbox has been setup, which
> contradicts the commit log where it is stated that "we don't have IPC between
> cores". Moreover, the commit log mentions to "skip the wait for remote
> processor to start". How can the remote processor executed an sample
> application if it is not ready?
>
> Lastly, is there even a need to call imx_dsp_rproc_mbox_init() if an IPC is not
> needed?
>
> I'm fine with the module parameter but would much rather see a solution that
> does not configure any kind of IPC related mechanic when it is not needed.
>
> Thanks,
> Mathieu
Hi Mathieu,

I've tested this with hello_world sample from Zephyr.
This was loaded on a hifi4 core from Linux using remoteproc. And, with
this patch is working, otherwise I get "can't start rproc imx-dsp-rproc:
-110:" because the ARM core is waiting for a reply from the hifi4.

I agree, I shouldn't initialize mbox if there is no IPC between the cores.
I'll fix this in a v4.

Thanks,
Iulia

\
 
 \ /
  Last update: 2023-03-26 23:46    [W:0.088 / U:1.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site