Hi Bjorn, Thanks for the review > -----Original Message----- > From: Bjorn Andersson [mailto:bjorn.andersson@xxxxxxxxxx] > Sent: Thursday, May 10, 2018 2:19 AM > To: Loic PALLARDY <loic.pallardy@xxxxxx> > Cc: ohad@xxxxxxxxxx; linux-remoteproc@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; Arnaud POULIQUEN <arnaud.pouliquen@xxxxxx>; > benjamin.gaignard@xxxxxxxxxx > Subject: Re: [PATCH v3 07/13] remoteproc: introduce > rproc_find_carveout_by_name function > > On Thu 01 Mar 08:23 PST 2018, Loic Pallardy wrote: > > > This patch provides a new function to find a carveout according > > to a name. > > If match found, this function returns a pointer on the corresponding > > carveout (rproc_mem_entry structure). > > > > Signed-off-by: Loic Pallardy <loic.pallardy@xxxxxx> > > --- > > drivers/remoteproc/remoteproc_core.c | 43 > ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 43 insertions(+) > > > > diff --git a/drivers/remoteproc/remoteproc_core.c > b/drivers/remoteproc/remoteproc_core.c > > index 91aa22b..7a500cb 100644 > > --- a/drivers/remoteproc/remoteproc_core.c > > +++ b/drivers/remoteproc/remoteproc_core.c > > @@ -216,6 +216,49 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, > int len) > > } > > EXPORT_SYMBOL(rproc_da_to_va); > > > > +/** > > + * rproc_find_carveout_by_name() - lookup the carveout region by a > name > > + * @rproc: handle of a remote processor > > + * @name,..: carveout name to find (standard printf format) > > + * > > + * Platform driver has the capability to register some pre-allacoted > carveout > > + * (physically contiguous memory regions) before rproc firmware loading > and > > + * associated resource table analysis. These regions may be dedicated > memory > > + * regions internal to the coprocessor or specified DDR region with specific > > + * attributes > > + * > > + * This function is a helper function with which we can go over the > > + * allocated carveouts and return associated region characteristics like > > + * coprocessor address, length or processor virtual address. > > + * > > + * The function returns a valid pointer on carveout entry on success > > + * or NULL on failure. > > The kerneldoc format for describing the return value is > > * Return: description Yes I'll update > > > + */ > > +struct rproc_mem_entry * > > +rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...) > > +{ > > + va_list args; > > + char _name[32]; > > + struct rproc_mem_entry *carveout, *mem = NULL; > > + > > + va_start(args, name); > > + snprintf(_name, sizeof(_name), name, args); > > + va_end(args); > > + > > + if (!name) > > + return NULL; > > If you would like for it to actually be valid to pass NULL for name, > then I think you should move this check to the top of the function. > > But I suspect that you're looking for (name[0] == '\0'), to check if we > where passed an empty string, e.g. from a resource table without > resource names. Good catch, I updated function prototype without changing associated test > > > + > > + list_for_each_entry(carveout, &rproc->carveouts, node) { > > + /* Compare carveout and requested names */ > > + if (!strcmp(carveout->name, name)) { > > + mem = carveout; > > + break; > > Just return carveout when you find it. Sure > > > + } > > + } > > + > > + return mem; > > +} > > + > > Regards, > Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
