| Message ID | 1519921440-21356-9-git-send-email-loic.pallardy@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu 01 Mar 08:23 PST 2018, Loic Pallardy wrote: > On some SoC architecture, it is needed to enable HW like > clock, bus, regulator, memory region... before loading > co-processor firmware. > > This patch introduces prepare and unprepare ops to execute > platform specific function before firmware loading and after > stop execution. > > Signed-off-by: Loic Pallardy <loic.pallardy@st.com> > --- > drivers/remoteproc/remoteproc_core.c | 20 +++++++++++++++++++- > include/linux/remoteproc.h | 4 ++++ > 2 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index 7a500cb..0ebbc4f 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -1058,12 +1058,22 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) > return ret; > } > > + /* Prepare rproc for firmware loading if needed */ > + if (rproc->ops->prepare) { > + ret = rproc->ops->prepare(rproc); > + if (ret) { > + dev_err(dev, "can't prepare rproc %s: %d\n", > + rproc->name, ret); > + goto disable_iommu; > + } > + } We do allow drivers to implement custom versions of parse_fw() - and they can call the resource-table-parse-fw from the custom function. So with the proposed refactoring in patch 9 I would like for parse_fw() to call back into the core to fill out the resource lists and then before jumping to rproc_start() we loop over the allocator functions. > + > rproc->bootaddr = rproc_get_boot_addr(rproc, fw); > > /* load resource table */ > ret = rproc_load_rsc_table(rproc, fw); > if (ret) > - goto disable_iommu; > + goto unprepare_rproc; > > /* reset max_notifyid */ > rproc->max_notifyid = -1; > @@ -1086,6 +1096,10 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) > kfree(rproc->cached_table); > rproc->cached_table = NULL; > rproc->table_ptr = NULL; > +unprepare_rproc: > + /* release HW resources if needed */ > + if (rproc->ops->unprepare) > + rproc->ops->unprepare(rproc); > disable_iommu: > rproc_disable_iommu(rproc); > return ret; > @@ -1331,6 +1345,10 @@ void rproc_shutdown(struct rproc *rproc) > /* clean up all acquired resources */ > rproc_resource_cleanup(rproc); > > + /* release HW resources if needed */ > + if (rproc->ops->unprepare) > + rproc->ops->unprepare(rproc); And this would then be handled by the rproc_resource_cleanup() function, looping over all resources and calling release(). Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Bjorn Andersson [mailto:bjorn.andersson@linaro.org] > Sent: Thursday, May 10, 2018 2:53 AM > To: Loic PALLARDY <loic.pallardy@st.com> > Cc: ohad@wizery.com; linux-remoteproc@vger.kernel.org; linux- > kernel@vger.kernel.org; Arnaud POULIQUEN <arnaud.pouliquen@st.com>; > benjamin.gaignard@linaro.org > Subject: Re: [PATCH v3 08/13] remoteproc: add prepare and unprepare ops > > On Thu 01 Mar 08:23 PST 2018, Loic Pallardy wrote: > > > On some SoC architecture, it is needed to enable HW like > > clock, bus, regulator, memory region... before loading > > co-processor firmware. > > > > This patch introduces prepare and unprepare ops to execute > > platform specific function before firmware loading and after > > stop execution. > > > > Signed-off-by: Loic Pallardy <loic.pallardy@st.com> > > --- > > drivers/remoteproc/remoteproc_core.c | 20 +++++++++++++++++++- > > include/linux/remoteproc.h | 4 ++++ > > 2 files changed, 23 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/remoteproc/remoteproc_core.c > b/drivers/remoteproc/remoteproc_core.c > > index 7a500cb..0ebbc4f 100644 > > --- a/drivers/remoteproc/remoteproc_core.c > > +++ b/drivers/remoteproc/remoteproc_core.c > > @@ -1058,12 +1058,22 @@ static int rproc_fw_boot(struct rproc *rproc, > const struct firmware *fw) > > return ret; > > } > > > > + /* Prepare rproc for firmware loading if needed */ > > + if (rproc->ops->prepare) { > > + ret = rproc->ops->prepare(rproc); > > + if (ret) { > > + dev_err(dev, "can't prepare rproc %s: %d\n", > > + rproc->name, ret); > > + goto disable_iommu; > > + } > > + } > > We do allow drivers to implement custom versions of parse_fw() - and > they can call the resource-table-parse-fw from the custom function. > > So with the proposed refactoring in patch 9 I would like for parse_fw() > to call back into the core to fill out the resource lists and then > before jumping to rproc_start() we loop over the allocator functions. Agree Regards, Loic > > > + > > rproc->bootaddr = rproc_get_boot_addr(rproc, fw); > > > > /* load resource table */ > > ret = rproc_load_rsc_table(rproc, fw); > > if (ret) > > - goto disable_iommu; > > + goto unprepare_rproc; > > > > /* reset max_notifyid */ > > rproc->max_notifyid = -1; > > @@ -1086,6 +1096,10 @@ static int rproc_fw_boot(struct rproc *rproc, > const struct firmware *fw) > > kfree(rproc->cached_table); > > rproc->cached_table = NULL; > > rproc->table_ptr = NULL; > > +unprepare_rproc: > > + /* release HW resources if needed */ > > + if (rproc->ops->unprepare) > > + rproc->ops->unprepare(rproc); > > disable_iommu: > > rproc_disable_iommu(rproc); > > return ret; > > @@ -1331,6 +1345,10 @@ void rproc_shutdown(struct rproc *rproc) > > /* clean up all acquired resources */ > > rproc_resource_cleanup(rproc); > > > > + /* release HW resources if needed */ > > + if (rproc->ops->unprepare) > > + rproc->ops->unprepare(rproc); > > And this would then be handled by the rproc_resource_cleanup() function, > looping over all resources and calling release(). > > Regards, > Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, > >> -----Original Message----- >> From: Bjorn Andersson [mailto:bjorn.andersson@linaro.org] >> Sent: Thursday, May 10, 2018 2:53 AM >> To: Loic PALLARDY <loic.pallardy@st.com> >> Cc: ohad@wizery.com; linux-remoteproc@vger.kernel.org; linux- >> kernel@vger.kernel.org; Arnaud POULIQUEN <arnaud.pouliquen@st.com>; >> benjamin.gaignard@linaro.org >> Subject: Re: [PATCH v3 08/13] remoteproc: add prepare and unprepare ops >> >> On Thu 01 Mar 08:23 PST 2018, Loic Pallardy wrote: >> >>> On some SoC architecture, it is needed to enable HW like >>> clock, bus, regulator, memory region... before loading >>> co-processor firmware. >>> >>> This patch introduces prepare and unprepare ops to execute >>> platform specific function before firmware loading and after >>> stop execution. >>> >>> Signed-off-by: Loic Pallardy <loic.pallardy@st.com> >>> --- >>> drivers/remoteproc/remoteproc_core.c | 20 +++++++++++++++++++- >>> include/linux/remoteproc.h | 4 ++++ >>> 2 files changed, 23 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/remoteproc/remoteproc_core.c >> b/drivers/remoteproc/remoteproc_core.c >>> index 7a500cb..0ebbc4f 100644 >>> --- a/drivers/remoteproc/remoteproc_core.c >>> +++ b/drivers/remoteproc/remoteproc_core.c >>> @@ -1058,12 +1058,22 @@ static int rproc_fw_boot(struct rproc *rproc, >> const struct firmware *fw) >>> return ret; >>> } >>> >>> + /* Prepare rproc for firmware loading if needed */ >>> + if (rproc->ops->prepare) { >>> + ret = rproc->ops->prepare(rproc); >>> + if (ret) { >>> + dev_err(dev, "can't prepare rproc %s: %d\n", >>> + rproc->name, ret); >>> + goto disable_iommu; >>> + } >>> + } >> >> We do allow drivers to implement custom versions of parse_fw() - and >> they can call the resource-table-parse-fw from the custom function. >> >> So with the proposed refactoring in patch 9 I would like for parse_fw() >> to call back into the core to fill out the resource lists and then >> before jumping to rproc_start() we loop over the allocator functions. I do like this patch and actually have a need for something similar for supporting loading into internal memories for some R5F remote processors on the latest TI SoCs. R5Fs have both resets and halt signals, and the reset needs to be released to allow loading into TCMs, with the start performing the halt. I am forced to do this between rproc_alloc() and rproc_start() at the moment, but this really creates a mismatch between my probe, start, stop and remove. That said, I do agree with you that the way this was used with carveouts should not have been used. Overloading the parse_fw to achieve above is not right either. Please see my comments on Loic's v4 ST remoteproc patch [1]. [1] https://patchwork.kernel.org/patch/10547153/ > > Agree > Regards, > Loic >> >>> + >>> rproc->bootaddr = rproc_get_boot_addr(rproc, fw); >>> >>> /* load resource table */ >>> ret = rproc_load_rsc_table(rproc, fw); >>> if (ret) >>> - goto disable_iommu; >>> + goto unprepare_rproc; >>> >>> /* reset max_notifyid */ >>> rproc->max_notifyid = -1; >>> @@ -1086,6 +1096,10 @@ static int rproc_fw_boot(struct rproc *rproc, >> const struct firmware *fw) >>> kfree(rproc->cached_table); >>> rproc->cached_table = NULL; >>> rproc->table_ptr = NULL; >>> +unprepare_rproc: >>> + /* release HW resources if needed */ >>> + if (rproc->ops->unprepare) >>> + rproc->ops->unprepare(rproc); >>> disable_iommu: >>> rproc_disable_iommu(rproc); >>> return ret; >>> @@ -1331,6 +1345,10 @@ void rproc_shutdown(struct rproc *rproc) >>> /* clean up all acquired resources */ >>> rproc_resource_cleanup(rproc); >>> >>> + /* release HW resources if needed */ >>> + if (rproc->ops->unprepare) >>> + rproc->ops->unprepare(rproc); >> >> And this would then be handled by the rproc_resource_cleanup() function, >> looping over all resources and calling release(). >> >> Regards, >> Bjorn > -- > To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 7a500cb..0ebbc4f 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -1058,12 +1058,22 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) return ret; } + /* Prepare rproc for firmware loading if needed */ + if (rproc->ops->prepare) { + ret = rproc->ops->prepare(rproc); + if (ret) { + dev_err(dev, "can't prepare rproc %s: %d\n", + rproc->name, ret); + goto disable_iommu; + } + } + rproc->bootaddr = rproc_get_boot_addr(rproc, fw); /* load resource table */ ret = rproc_load_rsc_table(rproc, fw); if (ret) - goto disable_iommu; + goto unprepare_rproc; /* reset max_notifyid */ rproc->max_notifyid = -1; @@ -1086,6 +1096,10 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) kfree(rproc->cached_table); rproc->cached_table = NULL; rproc->table_ptr = NULL; +unprepare_rproc: + /* release HW resources if needed */ + if (rproc->ops->unprepare) + rproc->ops->unprepare(rproc); disable_iommu: rproc_disable_iommu(rproc); return ret; @@ -1331,6 +1345,10 @@ void rproc_shutdown(struct rproc *rproc) /* clean up all acquired resources */ rproc_resource_cleanup(rproc); + /* release HW resources if needed */ + if (rproc->ops->unprepare) + rproc->ops->unprepare(rproc); + rproc_disable_iommu(rproc); /* Free the copy of the resource table */ diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 4aa30bd..dcfa601 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -333,6 +333,8 @@ struct rproc_mem_entry { /** * struct rproc_ops - platform-specific device handlers + * @prepare: prepare device for code loading + * @unprepare: unprepare device after stop * @start: power on the device and boot it * @stop: power off the device * @kick: kick a virtqueue (virtqueue id given as a parameter) @@ -345,6 +347,8 @@ struct rproc_mem_entry { * @get_boot_addr: get boot address to entry point specified in firmware */ struct rproc_ops { + int (*prepare)(struct rproc *rproc); + int (*unprepare)(struct rproc *rproc); int (*start)(struct rproc *rproc); int (*stop)(struct rproc *rproc); void (*kick)(struct rproc *rproc, int vqid);
On some SoC architecture, it is needed to enable HW like clock, bus, regulator, memory region... before loading co-processor firmware. This patch introduces prepare and unprepare ops to execute platform specific function before firmware loading and after stop execution. Signed-off-by: Loic Pallardy <loic.pallardy@st.com> --- drivers/remoteproc/remoteproc_core.c | 20 +++++++++++++++++++- include/linux/remoteproc.h | 4 ++++ 2 files changed, 23 insertions(+), 1 deletion(-)