On Wed 10 Mar 15:10 CST 2021, Mathieu Poirier wrote:
> When a remote processor that was attached to is stopped, special care
> must be taken to make sure the shutdown process is similar to what
> it would be had it been started by the remoteproc core.
>
> This patch takes care of that by making a copy of the resource
> table currently used by the remote processor. From that point on
> the copy is used, as if the remote processor had been started by
> the remoteproc core.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> ---
> New for V7:
> New Patch, used to be part of 11/16 in V6.
> ---
> drivers/remoteproc/remoteproc_core.c | 53 +++++++++++++++++++++++++++-
> 1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index e9ea2558432d..c488b1aa6119 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1634,6 +1634,52 @@ static int rproc_reset_rsc_table_on_detach(struct rproc *rproc)
> return 0;
> }
>
> +static int rproc_reset_rsc_table_on_stop(struct rproc *rproc)
> +{
> + struct resource_table *table_ptr;
> +
> + /* A resource table was never retrieved, nothing to do here */
> + if (!rproc->table_ptr)
> + return 0;
> +
> + /*
> + * If a cache table exists the remote processor was started by
> + * the remoteproc core. That cache table should be used for
> + * the rest of the shutdown process.
> + */
> + if (rproc->cached_table)
> + goto out;
> +
> + /* Remember where the external entity installed the resource table */
> + table_ptr = rproc->table_ptr;
> +
Afaict this is just a remnant from the detach case.
I think the series looks really good now, please let me know and I can
drop the local "table_ptr" as I apply the patches.
Regards,
Bjorn
> + /*
> + * If we made it here the remote processor was started by another
> + * entity and a cache table doesn't exist. As such make a copy of
> + * the resource table currently used by the remote processor and
> + * use that for the rest of the shutdown process. The memory
> + * allocated here is free'd in rproc_shutdown().
> + */
> + rproc->cached_table = kmemdup(rproc->table_ptr,
> + rproc->table_sz, GFP_KERNEL);
> + if (!rproc->cached_table)
> + return -ENOMEM;
> +
> + /*
> + * Since the remote processor is being switched off the clean table
> + * won't be needed. Allocated in rproc_set_rsc_table().
> + */
> + kfree(rproc->clean_table);
> +
> +out:
> + /*
> + * Use a copy of the resource table for the remainder of the
> + * shutdown process.
> + */
> + rproc->table_ptr = rproc->cached_table;
> + return 0;
> +}
> +
> /*
> * Attach to remote processor - similar to rproc_fw_boot() but without
> * the steps that deal with the firmware image.
> @@ -1759,7 +1805,12 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
> rproc_stop_subdevices(rproc, crashed);
>
> /* the installed resource table is no longer accessible */
> - rproc->table_ptr = rproc->cached_table;
> + ret = rproc_reset_rsc_table_on_stop(rproc);
> + if (ret) {
> + dev_err(dev, "can't reset resource table: %d\n", ret);
> + return ret;
> + }
> +
>
> /* power off the remote processor */
> ret = rproc->ops->stop(rproc);
> --
> 2.25.1
>
