Messages in this thread | | | Date | Thu, 19 Jan 2023 01:55:14 +0200 | Subject | Re: [PATCH v3] remoteproc: imx_dsp_rproc: add module parameter to ignore ready flag from remote processor | From | Iuliana Prodan <> |
| |
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
| |