On Fri, Mar 27, 2020 at 11:10:20AM +0000, Loic PALLARDY wrote:
> Hi Mathieu,
>
> > -----Original Message-----
> > From: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> > Sent: mardi 24 mars 2020 22:46
> > To: bjorn.andersson@xxxxxxxxxx
> > Cc: ohad@xxxxxxxxxx; Loic PALLARDY <loic.pallardy@xxxxxx>; s-
> > anna@xxxxxx; peng.fan@xxxxxxx; Arnaud POULIQUEN
> > <arnaud.pouliquen@xxxxxx>; Fabien DESSENNE
> > <fabien.dessenne@xxxxxx>; linux-remoteproc@xxxxxxxxxxxxxxx
> > Subject: [PATCH v2 06/17] remoteproc: Introduce function
> > rproc_alloc_internals()
> >
> > In preparation to allocate the synchronisation operation and state
> > machine, spin off a new function in order to keep rproc_alloc() as
> > clean as possible.
> >
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> > ---
> > drivers/remoteproc/remoteproc_core.c | 26 ++++++++++++++++++++++---
> > -
> > 1 file changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c
> > b/drivers/remoteproc/remoteproc_core.c
> > index ee277bc5556c..9da245734db6 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -2018,6 +2018,26 @@ static int rproc_alloc_ops(struct rproc *rproc,
> > const struct rproc_ops *ops)
> > return 0;
> > }
> >
> > +static int rproc_alloc_internals(struct rproc *rproc, const char *name,
> > + const struct rproc_ops *boot_ops,
> > + const char *firmware, int len)
>
> len argument is not used in the patch nor in the following, maybe removed from my pov.
I debated over this one... It is either introduce the function signature as a
whole or incrementally as parameters are needed. I'm fine with both and will
adopt the latter on the next revision.
>
> Regards,
> Loic
> > +{
> > + int ret;
> > +
> > + /* We have a boot_ops so allocate firmware name and operations */
> > + if (boot_ops) {
> > + ret = rproc_alloc_firmware(rproc, name, firmware);
> > + if (ret)
> > + return ret;
> > +
> > + ret = rproc_alloc_ops(rproc, boot_ops);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * rproc_alloc() - allocate a remote processor handle
> > * @dev: the underlying device
> > @@ -2064,10 +2084,8 @@ struct rproc *rproc_alloc(struct device *dev, const
> > char *name,
> > rproc->dev.class = &rproc_class;
> > rproc->dev.driver_data = rproc;
> >
> > - if (rproc_alloc_firmware(rproc, name, firmware))
> > - goto out;
> > -
> > - if (rproc_alloc_ops(rproc, ops))
> > + if (rproc_alloc_internals(rproc, name, ops,
> > + firmware, len))
> > goto out;
> >
> > /* Assign a unique device index and name */
> > --
> > 2.20.1
>
