On Mon 01 Jun 10:51 PDT 2020, Mathieu Poirier wrote:
> Add a new function to assert the general health of the remote
> processor before handing it to the remoteproc core.
>
> Signed-off-by: Mathieu Poirier <[email protected]>
> ---
> drivers/remoteproc/remoteproc_core.c | 45 ++++++++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index c70fa0372d07..0be8343dd851 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2060,6 +2060,47 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
> #endif
> EXPORT_SYMBOL(rproc_get_by_phandle);
>
> +static int rproc_validate(struct rproc *rproc)
> +{
> + /*
> + * When adding a remote processor, the state of the device
> + * can be offline or detached, nothing else.
> + */
> + if (rproc->state != RPROC_OFFLINE &&
> + rproc->state != RPROC_DETACHED)
> + goto inval;
I would prefer that you just return -EINVAL; directly.
Overall I think this would be better represented as a switch on
rproc->state though.
I think the logic is sound though.
Regards,
Bjorn
> +
> + if (rproc->state == RPROC_OFFLINE) {
> + /*
> + * An offline processor without a start()
> + * function makes no sense.
> + */
> + if (!rproc->ops->start)
> + goto inval;
> + }
> +
> + if (rproc->state == RPROC_DETACHED) {
> + /*
> + * A remote processor in a detached state without an
> + * attach() function makes not sense.
> + */
> + if (!rproc->ops->attach)
> + goto inval;
> + /*
> + * When attaching to a remote processor the device memory
> + * is already available and as such there is no need to have a
> + * cached table.
> + */
> + if (rproc->cached_table)
> + goto inval;
> + }
> +
> + return 0;
> +
> +inval:
> + return -EINVAL;
> +}
> +
> /**
> * rproc_add() - register a remote processor
> * @rproc: the remote processor handle to register
> @@ -2089,6 +2130,10 @@ int rproc_add(struct rproc *rproc)
> if (ret < 0)
> return ret;
>
> + ret = rproc_validate(rproc);
> + if (ret < 0)
> + return ret;
> +
> dev_info(dev, "%s is available\n", rproc->name);
>
> /* create debugfs entries */
> --
> 2.20.1
>
