| Message ID | 20200601175139.22097-6-mathieu.poirier@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | remoteproc: Add support for attaching with rproc | expand |
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 >
On Mon, Jun 22, 2020 at 12:25:02AM -0700, Bjorn Andersson wrote: > 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. > Sure thing. > > 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 > >
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; + + 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 */
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(+)