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 <[email protected]> > --- > 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; >
