On 4/24/20 10:01 PM, Mathieu Poirier wrote:
> The remoteproc core must not allow function rproc_shutdown() to
> proceed if currently synchronising with a remote processor and
> the synchronisation operations of that remote processor does not
> support it. Also part of the process is to set the synchronisation
> flag so that the remoteproc core can make the right decisions when
> restarting the system.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> ---
> drivers/remoteproc/remoteproc_core.c | 32 ++++++++++++++++++++++++
> drivers/remoteproc/remoteproc_internal.h | 7 ++++++
> 2 files changed, 39 insertions(+)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 3a84a38ba37b..48afa1f80a8f 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1849,6 +1849,27 @@ int rproc_boot(struct rproc *rproc)
> }
> EXPORT_SYMBOL(rproc_boot);
>
> +static bool rproc_can_shutdown(struct rproc *rproc)
> +{
> + /*
> + * The remoteproc core is the lifecycle manager, no problem
> + * calling for a shutdown.
> + */
> + if (!rproc_needs_syncing(rproc))
> + return true;
> +
> + /*
> + * The remoteproc has been loaded by another entity (as per above
> + * condition) and the platform code has given us the capability
> + * of stopping it.
> + */
> + if (rproc->sync_ops->stop)
> + return true;
This means that if rproc->sync_ops->stop is null rproc_stop_subdevices will not
be called? seems not symmetric with the start sequence.
Probably not useful to test it here as condition is already handled in rproc_stop_device...
Regards
Arnaud
> +
> + /* Any other condition should not be allowed */
> + return false;
> +}
> +
> /**
> * rproc_shutdown() - power off the remote processor
> * @rproc: the remote processor
> @@ -1879,6 +1900,9 @@ void rproc_shutdown(struct rproc *rproc)
> return;
> }
>
> + if (!rproc_can_shutdown(rproc))
> + goto out;
> +
> /* if the remote proc is still needed, bail out */
> if (!atomic_dec_and_test(&rproc->power))
> goto out;
> @@ -1898,6 +1922,14 @@ void rproc_shutdown(struct rproc *rproc)
> kfree(rproc->cached_table);
> rproc->cached_table = NULL;
> rproc->table_ptr = NULL;
> +
> + /*
> + * The remote processor has been switched off - tell the core what
> + * operation to use from hereon, i.e whether an external entity will
> + * reboot the remote processor or it is now the remoteproc core's
> + * responsability.
> + */
> + rproc_set_sync_flag(rproc, RPROC_SYNC_STATE_SHUTDOWN);
> out:
> mutex_unlock(&rproc->lock);
> }
> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
> index 61500981155c..7dcc0a26892b 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -27,6 +27,9 @@ struct rproc_debug_trace {
> /*
> * enum rproc_sync_states - remote processsor sync states
> *
> + * @RPROC_SYNC_STATE_SHUTDOWN state to use after the remoteproc core
> + * has shutdown (rproc_shutdown()) the
> + * remote processor.
> * @RPROC_SYNC_STATE_CRASHED state to use after the remote processor
> * has crashed but has not been recovered by
> * the remoteproc core yet.
> @@ -36,6 +39,7 @@ struct rproc_debug_trace {
> * operation to use.
> */
> enum rproc_sync_states {
> + RPROC_SYNC_STATE_SHUTDOWN,
> RPROC_SYNC_STATE_CRASHED,
> };
>
> @@ -43,6 +47,9 @@ static inline void rproc_set_sync_flag(struct rproc *rproc,
> enum rproc_sync_states state)
> {
> switch (state) {
> + case RPROC_SYNC_STATE_SHUTDOWN:
> + rproc->sync_with_rproc = rproc->sync_flags.after_stop;
> + break;
> case RPROC_SYNC_STATE_CRASHED:
> rproc->sync_with_rproc = rproc->sync_flags.after_crash;
> break;
>
