lkml.org 
[lkml]   [2020]   [Jun]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 5/9] remoteproc: Introducing function rproc_validate()
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 <mathieu.poirier@linaro.org>
> ---
> 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
>

\
 
 \ /
  Last update: 2020-06-22 09:28    [W:0.243 / U:1.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site