On Mon, Jun 23, 2025 at 11:11 PM Mathieu Poirier
<mathieu.poir...@linaro.org> wrote:
>
> Good day,
>
> On Wed, Jun 18, 2025 at 02:26:43PM +0800, Shengjiu Wang wrote:
> > when recovery is triggered, rproc_stop() is called first then
> > rproc_start(), but there is no rproc_unprepare_device() and
> > rproc_prepare_device() in the flow.
> >
> > So power enablement needs to be moved from prepare callback to start
> > callback, power disablement needs to be moved from unprepare callback
> > to stop callback, loading elf function also needs to be moved to start
> > callback, the load callback only store the firmware handler.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.w...@nxp.com>
> > ---
> > drivers/remoteproc/imx_dsp_rproc.c | 58 ++++++++++++++++++------------
> > 1 file changed, 36 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/remoteproc/imx_dsp_rproc.c
> > b/drivers/remoteproc/imx_dsp_rproc.c
> > index 5ee622bf5352..9b9cddb224b0 100644
> > --- a/drivers/remoteproc/imx_dsp_rproc.c
> > +++ b/drivers/remoteproc/imx_dsp_rproc.c
> > @@ -122,6 +122,7 @@ enum imx_dsp_rp_mbox_messages {
> > * @ipc_handle: System Control Unit ipc handle
> > * @rproc_work: work for processing virtio interrupts
> > * @pm_comp: completion primitive to sync for suspend response
> > + * @firmware: firmware handler
> > * @flags: control flags
> > */
> > struct imx_dsp_rproc {
> > @@ -139,6 +140,7 @@ struct imx_dsp_rproc {
> > struct imx_sc_ipc *ipc_handle;
> > struct work_struct rproc_work;
> > struct completion pm_comp;
> > + const struct firmware *firmware;
> > u32 flags;
> > };
> >
> > @@ -211,6 +213,7 @@ static const struct imx_rproc_att
> > imx_dsp_rproc_att_imx8ulp[] = {
> >
> > /* Initialize the mailboxes between cores, if exists */
> > static int (*imx_dsp_rproc_mbox_init)(struct imx_dsp_rproc *priv);
> > +static int imx_dsp_rproc_elf_load_segments(struct rproc *rproc, const
> > struct firmware *fw);
> >
> > /* Reset function for DSP on i.MX8MP */
> > static int imx8mp_dsp_reset(struct imx_dsp_rproc *priv)
> > @@ -402,8 +405,24 @@ static int imx_dsp_rproc_start(struct rproc *rproc)
> > const struct imx_dsp_rproc_dcfg *dsp_dcfg = priv->dsp_dcfg;
> > const struct imx_rproc_dcfg *dcfg = dsp_dcfg->dcfg;
> > struct device *dev = rproc->dev.parent;
> > + struct rproc_mem_entry *carveout;
> > int ret;
> >
> > + pm_runtime_get_sync(dev);
> > +
> > + /*
> > + * Clear buffers after pm rumtime for internal ocram is not
> > + * accessible if power and clock are not enabled.
> > + */
> > + list_for_each_entry(carveout, &rproc->carveouts, node) {
> > + if (carveout->va)
> > + memset(carveout->va, 0, carveout->len);
> > + }
> > +
> > + ret = imx_dsp_rproc_elf_load_segments(rproc, priv->firmware);
> > + if (ret)
> > + return ret;
> > +
> > switch (dcfg->method) {
> > case IMX_RPROC_MMIO:
> > ret = regmap_update_bits(priv->regmap,
> > @@ -446,6 +465,7 @@ static int imx_dsp_rproc_stop(struct rproc *rproc)
> >
> > if (rproc->state == RPROC_CRASHED) {
> > priv->flags &= ~REMOTE_IS_READY;
> > + pm_runtime_put_sync(dev);
>
> From this patch I understand that for a recovery to be successful, the
> remote processor _has_ to go through a hard reset. But here the PM runtime
> API
> is used, meaning the remote processor won't be switched off if another device
> in
> the same power domain still neeeds power. If that is the case, the solution
> in
> tihs patch won't work.
Thanks for reviewing.
With the case you mentioned, there is software reset to make the
recovery process work.
best regards
Shengjiu Wang
>
> Thanks,
> Mathieu
>
> > return 0;
> > }
> >
> > @@ -472,6 +492,8 @@ static int imx_dsp_rproc_stop(struct rproc *rproc)
> > else
> > priv->flags &= ~REMOTE_IS_READY;
> >
> > + pm_runtime_put_sync(dev);
> > +
> > return ret;
> > }
> >
> > @@ -774,7 +796,6 @@ static int imx_dsp_rproc_prepare(struct rproc *rproc)
> > {
> > struct imx_dsp_rproc *priv = rproc->priv;
> > struct device *dev = rproc->dev.parent;
> > - struct rproc_mem_entry *carveout;
> > int ret;
> >
> > ret = imx_dsp_rproc_add_carveout(priv);
> > @@ -783,25 +804,6 @@ static int imx_dsp_rproc_prepare(struct rproc *rproc)
> > return ret;
> > }
> >
> > - pm_runtime_get_sync(dev);
> > -
> > - /*
> > - * Clear buffers after pm rumtime for internal ocram is not
> > - * accessible if power and clock are not enabled.
> > - */
> > - list_for_each_entry(carveout, &rproc->carveouts, node) {
> > - if (carveout->va)
> > - memset(carveout->va, 0, carveout->len);
> > - }
> > -
> > - return 0;
> > -}
> > -
> > -/* Unprepare function for rproc_ops */
> > -static int imx_dsp_rproc_unprepare(struct rproc *rproc)
> > -{
> > - pm_runtime_put_sync(rproc->dev.parent);
> > -
> > return 0;
> > }
> >
> > @@ -1022,13 +1024,25 @@ static int imx_dsp_rproc_parse_fw(struct rproc
> > *rproc, const struct firmware *fw
> > return 0;
> > }
> >
> > +static int imx_dsp_rproc_load(struct rproc *rproc, const struct firmware
> > *fw)
> > +{
> > + struct imx_dsp_rproc *priv = rproc->priv;
> > +
> > + /*
> > + * Just save the fw handler, the firmware loading will be after
> > + * power enabled
> > + */
> > + priv->firmware = fw;
> > +
> > + return 0;
> > +}
> > +
> > static const struct rproc_ops imx_dsp_rproc_ops = {
> > .prepare = imx_dsp_rproc_prepare,
> > - .unprepare = imx_dsp_rproc_unprepare,
> > .start = imx_dsp_rproc_start,
> > .stop = imx_dsp_rproc_stop,
> > .kick = imx_dsp_rproc_kick,
> > - .load = imx_dsp_rproc_elf_load_segments,
> > + .load = imx_dsp_rproc_load,
> > .parse_fw = imx_dsp_rproc_parse_fw,
> > .handle_rsc = imx_dsp_rproc_handle_rsc,
> > .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> > --
> > 2.34.1
> >
>
