| Message ID | 20171226203832.14928-1-bjorn.andersson@linaro.org |
|---|---|
| Headers | show |
| Series | Remoteproc core dump support | expand |
> -----Original Message----- > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc- > owner@vger.kernel.org] On Behalf Of Bjorn Andersson > Sent: Tuesday, December 26, 2017 9:39 PM > To: Ohad Ben-Cohen <ohad@wizery.com>; Bjorn Andersson > <bjorn.andersson@linaro.org> > Cc: linux-remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > arm-msm@vger.kernel.org; linux-soc@vger.kernel.org; Suman Anna <s- > anna@ti.com>; Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> > Subject: [PATCH v2 2/4] remoteproc: Rename "load_rsc_table" to > "parse_fw" > > The resource table is just one possible source of information that can > be extracted from the firmware file. Generalize this interface to allow > drivers to override this with parsers of other types of information. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v1: > - New patch > > drivers/remoteproc/remoteproc_core.c | 6 +++--- > drivers/remoteproc/remoteproc_internal.h | 7 +++---- > include/linux/remoteproc.h | 2 +- > 3 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/remoteproc/remoteproc_core.c > b/drivers/remoteproc/remoteproc_core.c > index 5af7547b9d8d..6a72daa94673 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -944,8 +944,8 @@ static int rproc_fw_boot(struct rproc *rproc, const > struct firmware *fw) > > rproc->bootaddr = rproc_get_boot_addr(rproc, fw); > > - /* load resource table */ > - ret = rproc_load_rsc_table(rproc, fw); > + /* parse firmware resources */ > + ret = rproc_parse_fw(rproc, fw); Hi Bjorn, I think it will be good to keep resource (aka rsc) in function name. only "parse_fw" is not enough explicit and we don't know why rproc should parse firmware. Regards, Loic > if (ret) > goto disable_iommu; > > @@ -1555,7 +1555,7 @@ struct rproc *rproc_alloc(struct device *dev, const > char *name, > /* Default to ELF loader if no load function is specified */ > if (!rproc->ops->load) { > rproc->ops->load = rproc_elf_load_segments; > - rproc->ops->load_rsc_table = rproc_elf_load_rsc_table; > + rproc->ops->parse_fw = rproc_elf_load_rsc_table; > rproc->ops->find_loaded_rsc_table = > rproc_elf_find_loaded_rsc_table; > rproc->ops->sanity_check = rproc_elf_sanity_check; > rproc->ops->get_boot_addr = rproc_elf_get_boot_addr; > diff --git a/drivers/remoteproc/remoteproc_internal.h > b/drivers/remoteproc/remoteproc_internal.h > index 55a2950c5cb7..7570beb035b5 100644 > --- a/drivers/remoteproc/remoteproc_internal.h > +++ b/drivers/remoteproc/remoteproc_internal.h > @@ -88,11 +88,10 @@ int rproc_load_segments(struct rproc *rproc, const > struct firmware *fw) > return -EINVAL; > } > > -static inline int rproc_load_rsc_table(struct rproc *rproc, > - const struct firmware *fw) > +static inline int rproc_parse_fw(struct rproc *rproc, const struct firmware > *fw) > { > - if (rproc->ops->load_rsc_table) > - return rproc->ops->load_rsc_table(rproc, fw); > + if (rproc->ops->parse_fw) > + return rproc->ops->parse_fw(rproc, fw); > > return 0; > } > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h > index de6e20a3f061..dc93ac3d1692 100644 > --- a/include/linux/remoteproc.h > +++ b/include/linux/remoteproc.h > @@ -343,7 +343,7 @@ struct rproc_ops { > int (*stop)(struct rproc *rproc); > void (*kick)(struct rproc *rproc, int vqid); > void * (*da_to_va)(struct rproc *rproc, u64 da, int len); > - int (*load_rsc_table)(struct rproc *rproc, const struct firmware *fw); > + int (*parse_fw)(struct rproc *rproc, const struct firmware *fw); > struct resource_table *(*find_loaded_rsc_table)( > struct rproc *rproc, const struct firmware > *fw); > int (*load)(struct rproc *rproc, const struct firmware *fw); > -- > 2.15.0 > > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc- > owner@vger.kernel.org] On Behalf Of Loic PALLARDY > Sent: Wednesday, January 03, 2018 11:27 AM > To: Bjorn Andersson <bjorn.andersson@linaro.org>; Ohad Ben-Cohen > <ohad@wizery.com> > Cc: linux-remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > arm-msm@vger.kernel.org; linux-soc@vger.kernel.org; Suman Anna <s- > anna@ti.com>; Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> > Subject: RE: [PATCH v2 2/4] remoteproc: Rename "load_rsc_table" to > "parse_fw" > > > > > -----Original Message----- > > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux- > remoteproc- > > owner@vger.kernel.org] On Behalf Of Bjorn Andersson > > Sent: Tuesday, December 26, 2017 9:39 PM > > To: Ohad Ben-Cohen <ohad@wizery.com>; Bjorn Andersson > > <bjorn.andersson@linaro.org> > > Cc: linux-remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org; > linux- > > arm-msm@vger.kernel.org; linux-soc@vger.kernel.org; Suman Anna <s- > > anna@ti.com>; Avaneesh Kumar Dwivedi <akdwived@codeaurora.org> > > Subject: [PATCH v2 2/4] remoteproc: Rename "load_rsc_table" to > > "parse_fw" > > > > The resource table is just one possible source of information that can > > be extracted from the firmware file. Generalize this interface to allow > > drivers to override this with parsers of other types of information. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > > > Changes since v1: > > - New patch > > > > drivers/remoteproc/remoteproc_core.c | 6 +++--- > > drivers/remoteproc/remoteproc_internal.h | 7 +++---- > > include/linux/remoteproc.h | 2 +- > > 3 files changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/remoteproc/remoteproc_core.c > > b/drivers/remoteproc/remoteproc_core.c > > index 5af7547b9d8d..6a72daa94673 100644 > > --- a/drivers/remoteproc/remoteproc_core.c > > +++ b/drivers/remoteproc/remoteproc_core.c > > @@ -944,8 +944,8 @@ static int rproc_fw_boot(struct rproc *rproc, const > > struct firmware *fw) > > > > rproc->bootaddr = rproc_get_boot_addr(rproc, fw); > > > > - /* load resource table */ > > - ret = rproc_load_rsc_table(rproc, fw); > > + /* parse firmware resources */ > > + ret = rproc_parse_fw(rproc, fw); > Hi Bjorn, > > I think it will be good to keep resource (aka rsc) in function name. only > "parse_fw" is not enough explicit and we don't know why rproc should parse > firmware. > > Regards, > Loic Forgot my previous remark, better understanding thanks to the rest of the series. Anyway, will be nice to have a comment here as it is not only parsing the firmware, you collect some information like copy of the resource table, list of elf segment to dump... I think it is important to be clear about resource table management as it is a key element of the remoteproc core, where it is loaded, where it is copied back in memory... Regards, Loic > > > if (ret) > > goto disable_iommu; > > > > @@ -1555,7 +1555,7 @@ struct rproc *rproc_alloc(struct device *dev, > const > > char *name, > > /* Default to ELF loader if no load function is specified */ > > if (!rproc->ops->load) { > > rproc->ops->load = rproc_elf_load_segments; > > - rproc->ops->load_rsc_table = rproc_elf_load_rsc_table; > > + rproc->ops->parse_fw = rproc_elf_load_rsc_table; > > rproc->ops->find_loaded_rsc_table = > > rproc_elf_find_loaded_rsc_table; > > rproc->ops->sanity_check = rproc_elf_sanity_check; > > rproc->ops->get_boot_addr = rproc_elf_get_boot_addr; > > diff --git a/drivers/remoteproc/remoteproc_internal.h > > b/drivers/remoteproc/remoteproc_internal.h > > index 55a2950c5cb7..7570beb035b5 100644 > > --- a/drivers/remoteproc/remoteproc_internal.h > > +++ b/drivers/remoteproc/remoteproc_internal.h > > @@ -88,11 +88,10 @@ int rproc_load_segments(struct rproc *rproc, const > > struct firmware *fw) > > return -EINVAL; > > } > > > > -static inline int rproc_load_rsc_table(struct rproc *rproc, > > - const struct firmware *fw) > > +static inline int rproc_parse_fw(struct rproc *rproc, const struct firmware > > *fw) > > { > > - if (rproc->ops->load_rsc_table) > > - return rproc->ops->load_rsc_table(rproc, fw); > > + if (rproc->ops->parse_fw) > > + return rproc->ops->parse_fw(rproc, fw); > > > > return 0; > > } > > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h > > index de6e20a3f061..dc93ac3d1692 100644 > > --- a/include/linux/remoteproc.h > > +++ b/include/linux/remoteproc.h > > @@ -343,7 +343,7 @@ struct rproc_ops { > > int (*stop)(struct rproc *rproc); > > void (*kick)(struct rproc *rproc, int vqid); > > void * (*da_to_va)(struct rproc *rproc, u64 da, int len); > > - int (*load_rsc_table)(struct rproc *rproc, const struct firmware *fw); > > + int (*parse_fw)(struct rproc *rproc, const struct firmware *fw); > > struct resource_table *(*find_loaded_rsc_table)( > > struct rproc *rproc, const struct firmware > > *fw); > > int (*load)(struct rproc *rproc, const struct firmware *fw); > > -- > > 2.15.0 > > > > -- > > 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 > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html