On Fri 24 Apr 13:25 PDT 2020, Mathieu Poirier wrote: > Introduce new parse firmware rproc_ops functions to be used when > synchonising with an MCU. > > Mainly based on the work published by Arnaud Pouliquen [1]. > > [1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877 > > Signed-off-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx> > Reviewed-by: Loic Pallardy <loic.pallardy@xxxxxx> > --- > drivers/remoteproc/stm32_rproc.c | 51 +++++++++++++++++++++++++++++++- > 1 file changed, 50 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c > index 86d23c35d805..b8ae8aed5585 100644 > --- a/drivers/remoteproc/stm32_rproc.c > +++ b/drivers/remoteproc/stm32_rproc.c > @@ -215,7 +215,34 @@ static int stm32_rproc_elf_load_rsc_table(struct rproc *rproc, > return 0; > } > > -static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw) > +static int stm32_rproc_sync_elf_load_rsc_table(struct rproc *rproc, > + const struct firmware *fw) > +{ > + struct resource_table *table = NULL; > + struct stm32_rproc *ddata = rproc->priv; > + > + if (ddata->rsc_va) { Does it really make sense to try to sync with a remote that doesn't have a resource table? > + table = (struct resource_table *)ddata->rsc_va; > + /* Assuming that the resource table fits in 1kB is fair */ > + rproc->cached_table = kmemdup(table, RSC_TBL_SIZE, GFP_KERNEL); It's unfortunate that we need to create a clone of the resource table that we found in ram, and then return the original memory when the core ask for the loaded table... I wonder if we somehow can avoid this in the core (i.e. skip overwriting table_ptr with the cached_table during stop) > + if (!rproc->cached_table) > + return -ENOMEM; > + > + rproc->table_ptr = rproc->cached_table; > + rproc->table_sz = RSC_TBL_SIZE; > + return 0; > + } > + > + rproc->cached_table = NULL; > + rproc->table_ptr = NULL; > + rproc->table_sz = 0; > + > + dev_warn(&rproc->dev, "no resource table found for this firmware\n"); > + return 0; > +} > + > +static int stm32_rproc_parse_memory_regions(struct rproc *rproc, > + const struct firmware *fw) > { > struct device *dev = rproc->dev.parent; > struct device_node *np = dev->of_node; > @@ -268,9 +295,30 @@ static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw) > index++; > } > > + return 0; > +} > + > +static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw) > +{ > + int ret = stm32_rproc_parse_memory_regions(rproc, fw); > + > + if (ret) > + return ret; > + > return stm32_rproc_elf_load_rsc_table(rproc, fw); > } > > +static int stm32_rproc_sync_parse_fw(struct rproc *rproc, > + const struct firmware *fw) Rather than having a function parse_fw that is passed no fw and return some state that was setup in probe, how about just do this during probe? Regards, Bjorn > +{ > + int ret = stm32_rproc_parse_memory_regions(rproc, fw); > + > + if (ret) > + return ret; > + > + return stm32_rproc_sync_elf_load_rsc_table(rproc, fw); > +} > + > static irqreturn_t stm32_rproc_wdg(int irq, void *data) > { > struct platform_device *pdev = data; > @@ -544,6 +592,7 @@ static struct rproc_ops st_rproc_ops = { > static __maybe_unused struct rproc_ops st_rproc_sync_ops = { > .start = stm32_rproc_sync_start, > .stop = stm32_rproc_stop, > + .parse_fw = stm32_rproc_sync_parse_fw, > }; > > static const struct of_device_id stm32_rproc_match[] = { > -- > 2.20.1 >
