From: Loic PALLARDY <loic.pallardy@st.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
Ohad Ben-Cohen <ohad@wizery.com>
Cc: "linux-remoteproc@vger.kernel.org"
<linux-remoteproc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 7/8] remoteproc: Drop dangling find_rsc_table dummies
Date: Fri, 5 Jan 2018 16:53:18 +0000 [thread overview]
Message-ID: <c3e80239c8d84645919713897fcf9e46@SFHDAG7NODE2.st.com> (raw)
In-Reply-To: <20171213224111.17864-8-bjorn.andersson@linaro.org>
> -----Original Message-----
> From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc-
> owner@vger.kernel.org] On Behalf Of Bjorn Andersson
> Sent: Wednesday, December 13, 2017 11:41 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
> Subject: [PATCH 7/8] remoteproc: Drop dangling find_rsc_table dummies
>
> As the core now deals with the lack of a resource table, remove the
> dangling custom dummy implementations of find_rsc_table from drivers.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> drivers/remoteproc/qcom_adsp_pil.c | 1 -
> drivers/remoteproc/qcom_common.c | 19 -------------------
> drivers/remoteproc/qcom_common.h | 4 ----
> drivers/remoteproc/qcom_q6v5_pil.c | 11 -----------
> drivers/remoteproc/qcom_wcnss.c | 1 -
> drivers/remoteproc/st_slim_rproc.c | 18 ------------------
> include/linux/remoteproc.h | 4 ----
> 7 files changed, 58 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_adsp_pil.c
> b/drivers/remoteproc/qcom_adsp_pil.c
> index 7b9d810b23f1..b0b0d5ca1ca0 100644
> --- a/drivers/remoteproc/qcom_adsp_pil.c
> +++ b/drivers/remoteproc/qcom_adsp_pil.c
> @@ -181,7 +181,6 @@ static const struct rproc_ops adsp_ops = {
> .start = adsp_start,
> .stop = adsp_stop,
> .da_to_va = adsp_da_to_va,
> - .find_rsc_table = qcom_mdt_find_rsc_table,
> .load = adsp_load,
> };
>
> diff --git a/drivers/remoteproc/qcom_common.c
> b/drivers/remoteproc/qcom_common.c
> index 818ee3657043..ce2dcc4f7de7 100644
> --- a/drivers/remoteproc/qcom_common.c
> +++ b/drivers/remoteproc/qcom_common.c
> @@ -32,25 +32,6 @@
>
> static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
>
> -/**
> - * qcom_mdt_find_rsc_table() - provide dummy resource table for
> remoteproc
> - * @rproc: remoteproc handle
> - * @fw: firmware header
> - * @tablesz: outgoing size of the table
> - *
> - * Returns a dummy table.
> - */
> -struct resource_table *qcom_mdt_find_rsc_table(struct rproc *rproc,
> - const struct firmware *fw,
> - int *tablesz)
> -{
> - static struct resource_table table = { .ver = 1, };
> -
> - *tablesz = sizeof(table);
> - return &table;
> -}
> -EXPORT_SYMBOL_GPL(qcom_mdt_find_rsc_table);
> -
> static int glink_subdev_probe(struct rproc_subdev *subdev)
> {
> struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
> diff --git a/drivers/remoteproc/qcom_common.h
> b/drivers/remoteproc/qcom_common.h
> index 541586e528b3..73efed969bfd 100644
> --- a/drivers/remoteproc/qcom_common.h
> +++ b/drivers/remoteproc/qcom_common.h
> @@ -30,10 +30,6 @@ struct qcom_rproc_ssr {
> const char *name;
> };
>
> -struct resource_table *qcom_mdt_find_rsc_table(struct rproc *rproc,
> - const struct firmware *fw,
> - int *tablesz);
> -
> void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink
> *glink);
> void qcom_remove_glink_subdev(struct rproc *rproc, struct
> qcom_rproc_glink *glink);
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c
> b/drivers/remoteproc/qcom_q6v5_pil.c
> index fbff5d842581..6f6ea0414366 100644
> --- a/drivers/remoteproc/qcom_q6v5_pil.c
> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
> @@ -304,16 +304,6 @@ static void q6v5_clk_disable(struct device *dev,
> clk_disable_unprepare(clks[i]);
> }
>
> -static struct resource_table *q6v5_find_rsc_table(struct rproc *rproc,
> - const struct firmware *fw,
> - int *tablesz)
> -{
> - static struct resource_table table = { .ver = 1, };
> -
> - *tablesz = sizeof(table);
> - return &table;
> -}
> -
> static int q6v5_xfer_mem_ownership(struct q6v5 *qproc, int
> *current_perm,
> bool remote_owner, phys_addr_t addr,
> size_t size)
> @@ -927,7 +917,6 @@ static const struct rproc_ops q6v5_ops = {
> .start = q6v5_start,
> .stop = q6v5_stop,
> .da_to_va = q6v5_da_to_va,
> - .find_rsc_table = q6v5_find_rsc_table,
> .load = q6v5_load,
> };
>
> diff --git a/drivers/remoteproc/qcom_wcnss.c
> b/drivers/remoteproc/qcom_wcnss.c
> index cc44ec598522..1fa5253020dd 100644
> --- a/drivers/remoteproc/qcom_wcnss.c
> +++ b/drivers/remoteproc/qcom_wcnss.c
> @@ -310,7 +310,6 @@ static const struct rproc_ops wcnss_ops = {
> .start = wcnss_start,
> .stop = wcnss_stop,
> .da_to_va = wcnss_da_to_va,
> - .find_rsc_table = qcom_mdt_find_rsc_table,
> .load = wcnss_load,
> };
>
> diff --git a/drivers/remoteproc/st_slim_rproc.c
> b/drivers/remoteproc/st_slim_rproc.c
> index 1538ea915c49..c6a2a8b68c7a 100644
> --- a/drivers/remoteproc/st_slim_rproc.c
> +++ b/drivers/remoteproc/st_slim_rproc.c
> @@ -200,28 +200,10 @@ static void *slim_rproc_da_to_va(struct rproc
> *rproc, u64 da, int len)
> return va;
> }
>
> -/*
> - * Firmware handler operations: sanity, boot address, load ...
> - */
> -
> -static struct resource_table empty_rsc_tbl = {
> - .ver = 1,
> - .num = 0,
> -};
> -
> -static struct resource_table *slim_rproc_find_rsc_table(struct rproc *rproc,
> - const struct firmware *fw,
> - int *tablesz)
> -{
> - *tablesz = sizeof(empty_rsc_tbl);
> - return &empty_rsc_tbl;
> -}
> -
> static const struct rproc_ops slim_rproc_ops = {
> .start = slim_rproc_start,
> .stop = slim_rproc_stop,
> .da_to_va = slim_rproc_da_to_va,
> - .find_rsc_table = slim_rproc_find_rsc_table,
Hi Bjorn,
Your patch is not complete for st_slim_rproc and so not working.
In your patch 6/8, .load_rsc_table is define to default rproc_elf_load_rsc_table if no load ops defined.
/* Default to ELF loader if no load function is specified */
if (!rproc->ops->load) {
rproc->ops->load = rproc_elf_load_segments;
- rproc->ops->find_rsc_table = rproc_elf_find_rsc_table;
+ rproc->ops->load_rsc_table = rproc_elf_load_rsc_table;
As st_slim_rproc has no load ops, it will inherit from all default ops including rproc_elf_load_rsc_table.
As no resource table present in firmware, an error will be returned and st_slim_rproc will failed.
In case of Qualcom, load ops is defined...
See below B2260 log (with additional error message in rproc_load_rsc_table function.
[ 10.201079] remoteproc remoteproc2: st231-delta is available
[ 10.258121] remoteproc remoteproc2: powering up st231-delta
[ 10.258143] remoteproc remoteproc2: Booting fw image rproc-st231-delta-fw, size 44416
[ 10.258151] rproc_load_rsc_table: error -22
Moreover with your proposal, as the choice to support or not a resource table is based on the fact rproc->ops->load_rsc_table is set or not, it is not possible with one unique driver to support firmware with resource table and firmware without resource table.
Will be better from my pov to consider that no resource table found in rproc_elf_load_rsc_table function as a normal case and not an error.
Regards,
Loic
> };
>
> /**
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index ec1ada7cc6d7..51ddde7535b9 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -332,7 +332,6 @@ struct firmware;
> * @stop: power off the device
> * @kick: kick a virtqueue (virtqueue id given as a parameter)
> * @da_to_va: optional platform hook to perform address translations
> - * @find_rsc_table: find the resource table inside the firmware image
> * @find_loaded_rsc_table: find the loaded resouce table
> * @load: load firmeware to memory, where the remote
> processor
> * expects to find it
> @@ -345,9 +344,6 @@ struct rproc_ops {
> 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);
> - struct resource_table *(*find_rsc_table)(struct rproc *rproc,
> - const struct firmware *fw,
> - int *tablesz);
> 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
next prev parent reply other threads:[~2018-01-05 16:53 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-13 22:41 [PATCH 0/8] Remoteproc cleanups Bjorn Andersson
2017-12-13 22:41 ` [PATCH 1/8] remoteproc: Remove depricated crash completion Bjorn Andersson
2017-12-13 22:41 ` [PATCH 2/8] remoteproc: Cache resource table size Bjorn Andersson
2017-12-13 22:41 ` [PATCH 3/8] remoteproc: Clone rproc_ops in rproc_alloc() Bjorn Andersson
2017-12-13 22:41 ` [PATCH 4/8] remoteproc: Merge rproc_ops and rproc_fw_ops Bjorn Andersson
2017-12-13 22:41 ` [PATCH 5/8] remoteproc: Don't handle empty resource table Bjorn Andersson
2017-12-13 22:41 ` [PATCH 6/8] remoteproc: Move resource table load logic to find Bjorn Andersson
2017-12-14 11:25 ` Loic PALLARDY
2017-12-14 19:25 ` Bjorn Andersson
2017-12-14 12:00 ` Loic PALLARDY
2017-12-14 19:47 ` Bjorn Andersson
2017-12-14 20:12 ` Loic PALLARDY
2017-12-13 22:41 ` [PATCH 7/8] remoteproc: Drop dangling find_rsc_table dummies Bjorn Andersson
2018-01-05 16:53 ` Loic PALLARDY [this message]
2018-01-05 18:50 ` Bjorn Andersson
2018-01-08 8:14 ` Loic PALLARDY
2017-12-13 22:41 ` [PATCH 8/8] remoteproc: Reset table_ptr on stop Bjorn Andersson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c3e80239c8d84645919713897fcf9e46@SFHDAG7NODE2.st.com \
--to=loic.pallardy@st.com \
--cc=bjorn.andersson@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=ohad@wizery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.