All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] Remoteproc cleanups
@ 2018-01-05 23:57 Bjorn Andersson
  2018-01-05 23:57 ` [PATCH v2 1/8] remoteproc: Remove depricated crash completion Bjorn Andersson
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-05 23:57 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, Loic Pallardy

The first patch removes code that became unnecessary when the recovery flow was
redesigned.

The following patches moves the assignment of cached_table to the resource
table loader, rather than core code, which allows this to made optional and
finally drops the various dummy resource tables provided by drivers.

Then finally the last patch ensures that table_ptr isn't left pointing into
memory of a stopped remoteproc.

Bjorn Andersson (8):
  remoteproc: Remove depricated crash completion
  remoteproc: Cache resource table size
  remoteproc: Clone rproc_ops in rproc_alloc()
  remoteproc: Merge rproc_ops and rproc_fw_ops
  remoteproc: Don't handle empty resource table
  remoteproc: Move resource table load logic to find
  remoteproc: Drop dangling find_rsc_table dummies
  remoteproc: Reset table_ptr on stop

 drivers/remoteproc/qcom_adsp_pil.c         |  8 +--
 drivers/remoteproc/qcom_common.c           | 19 -------
 drivers/remoteproc/qcom_common.h           |  4 --
 drivers/remoteproc/qcom_q6v5_pil.c         | 18 +------
 drivers/remoteproc/qcom_wcnss.c            |  8 +--
 drivers/remoteproc/remoteproc_core.c       | 83 ++++++++++++------------------
 drivers/remoteproc/remoteproc_elf_loader.c | 59 ++++++++++-----------
 drivers/remoteproc/remoteproc_internal.h   | 57 +++++++-------------
 drivers/remoteproc/st_slim_rproc.c         | 32 ++----------
 include/linux/remoteproc.h                 | 21 ++++++--
 10 files changed, 106 insertions(+), 203 deletions(-)

-- 
2.15.0

^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH v2 1/8] remoteproc: Remove depricated crash completion
  2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
@ 2018-01-05 23:57 ` Bjorn Andersson
  2018-01-05 23:57 ` [PATCH v2 2/8] remoteproc: Cache resource table size Bjorn Andersson
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-05 23:57 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, Loic Pallardy

The crash handling now happens in a single execution context, so there's
no longer a need for a completion to synchronize this.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- None

 drivers/remoteproc/remoteproc_core.c | 10 ----------
 include/linux/remoteproc.h           |  2 --
 2 files changed, 12 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index eab14b414bf0..758fad3131a3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1028,10 +1028,6 @@ static int rproc_stop(struct rproc *rproc)
 		return ret;
 	}
 
-	/* if in crash state, unlock crash handler */
-	if (rproc->state == RPROC_CRASHED)
-		complete_all(&rproc->crash_comp);
-
 	rproc->state = RPROC_OFFLINE;
 
 	dev_info(dev, "stopped remote processor %s\n", rproc->name);
@@ -1057,8 +1053,6 @@ int rproc_trigger_recovery(struct rproc *rproc)
 
 	dev_err(dev, "recovering %s\n", rproc->name);
 
-	init_completion(&rproc->crash_comp);
-
 	ret = mutex_lock_interruptible(&rproc->lock);
 	if (ret)
 		return ret;
@@ -1067,9 +1061,6 @@ int rproc_trigger_recovery(struct rproc *rproc)
 	if (ret)
 		goto unlock_mutex;
 
-	/* wait until there is no more rproc users */
-	wait_for_completion(&rproc->crash_comp);
-
 	/* load firmware */
 	ret = request_firmware(&firmware_p, rproc->firmware, dev);
 	if (ret < 0) {
@@ -1459,7 +1450,6 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 	INIT_LIST_HEAD(&rproc->subdevs);
 
 	INIT_WORK(&rproc->crash_handler, rproc_crash_handler_work);
-	init_completion(&rproc->crash_comp);
 
 	rproc->state = RPROC_OFFLINE;
 
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 44e630eb3d94..6f1d8e025c81 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -406,7 +406,6 @@ enum rproc_crash_type {
  * @index: index of this rproc device
  * @crash_handler: workqueue for handling a crash
  * @crash_cnt: crash counter
- * @crash_comp: completion used to sync crash handler and the rproc reload
  * @recovery_disabled: flag that state if recovery was disabled
  * @max_notifyid: largest allocated notify id.
  * @table_ptr: pointer to the resource table in effect
@@ -437,7 +436,6 @@ struct rproc {
 	int index;
 	struct work_struct crash_handler;
 	unsigned int crash_cnt;
-	struct completion crash_comp;
 	bool recovery_disabled;
 	int max_notifyid;
 	struct resource_table *table_ptr;
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 2/8] remoteproc: Cache resource table size
  2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
  2018-01-05 23:57 ` [PATCH v2 1/8] remoteproc: Remove depricated crash completion Bjorn Andersson
@ 2018-01-05 23:57 ` Bjorn Andersson
  2018-01-05 23:58 ` [PATCH v2 3/8] remoteproc: Clone rproc_ops in rproc_alloc() Bjorn Andersson
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-05 23:57 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, Loic Pallardy

We don't re-read the resource table during a recovery, so it is possible
in the recovery path that the resource table has a different size than
cached_table. Store the original size of cached_table to avoid these
getting out of sync.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- None

 drivers/remoteproc/remoteproc_core.c | 20 +++++++-------------
 include/linux/remoteproc.h           |  2 ++
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 758fad3131a3..208ccf709cad 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -732,7 +732,7 @@ static rproc_handle_resource_t rproc_loading_handlers[RSC_LAST] = {
 };
 
 /* handle firmware resource entries before booting the remote processor */
-static int rproc_handle_resources(struct rproc *rproc, int len,
+static int rproc_handle_resources(struct rproc *rproc,
 				  rproc_handle_resource_t handlers[RSC_LAST])
 {
 	struct device *dev = &rproc->dev;
@@ -742,7 +742,7 @@ static int rproc_handle_resources(struct rproc *rproc, int len,
 	for (i = 0; i < rproc->table_ptr->num; i++) {
 		int offset = rproc->table_ptr->offset[i];
 		struct fw_rsc_hdr *hdr = (void *)rproc->table_ptr + offset;
-		int avail = len - offset - sizeof(*hdr);
+		int avail = rproc->table_sz - offset - sizeof(*hdr);
 		void *rsc = (void *)hdr + sizeof(*hdr);
 
 		/* make sure table isn't truncated */
@@ -849,16 +849,9 @@ static void rproc_resource_cleanup(struct rproc *rproc)
 
 static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 {
-	struct resource_table *table, *loaded_table;
+	struct resource_table *loaded_table;
 	struct device *dev = &rproc->dev;
-	int ret, tablesz;
-
-	/* look for the resource table */
-	table = rproc_find_rsc_table(rproc, fw, &tablesz);
-	if (!table) {
-		dev_err(dev, "Resource table look up failed\n");
-		return -EINVAL;
-	}
+	int ret;
 
 	/* load the ELF segments to memory */
 	ret = rproc_load_segments(rproc, fw);
@@ -877,7 +870,7 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
 	 */
 	loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
 	if (loaded_table) {
-		memcpy(loaded_table, rproc->cached_table, tablesz);
+		memcpy(loaded_table, rproc->cached_table, rproc->table_sz);
 		rproc->table_ptr = loaded_table;
 	}
 
@@ -951,12 +944,13 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 		goto clean_up;
 
 	rproc->table_ptr = rproc->cached_table;
+	rproc->table_sz = tablesz;
 
 	/* reset max_notifyid */
 	rproc->max_notifyid = -1;
 
 	/* handle fw resources which are required to boot rproc */
-	ret = rproc_handle_resources(rproc, tablesz, rproc_loading_handlers);
+	ret = rproc_handle_resources(rproc, rproc_loading_handlers);
 	if (ret) {
 		dev_err(dev, "Failed to process resources: %d\n", ret);
 		goto clean_up_resources;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 6f1d8e025c81..6fdc62e29d6f 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -410,6 +410,7 @@ enum rproc_crash_type {
  * @max_notifyid: largest allocated notify id.
  * @table_ptr: pointer to the resource table in effect
  * @cached_table: copy of the resource table
+ * @table_sz: size of @cached_table
  * @has_iommu: flag to indicate if remote processor is behind an MMU
  */
 struct rproc {
@@ -440,6 +441,7 @@ struct rproc {
 	int max_notifyid;
 	struct resource_table *table_ptr;
 	struct resource_table *cached_table;
+	size_t table_sz;
 	bool has_iommu;
 	bool auto_boot;
 };
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 3/8] remoteproc: Clone rproc_ops in rproc_alloc()
  2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
  2018-01-05 23:57 ` [PATCH v2 1/8] remoteproc: Remove depricated crash completion Bjorn Andersson
  2018-01-05 23:57 ` [PATCH v2 2/8] remoteproc: Cache resource table size Bjorn Andersson
@ 2018-01-05 23:58 ` Bjorn Andersson
  2018-01-05 23:58 ` [PATCH v2 4/8] remoteproc: Merge rproc_ops and rproc_fw_ops Bjorn Andersson
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-05 23:58 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, Loic Pallardy

In order to allow rproc_alloc() to, in a future patch, update entries in
the "ops" struct we need to make a local copy of it.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- None

 drivers/remoteproc/remoteproc_core.c | 9 ++++++++-
 include/linux/remoteproc.h           | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 208ccf709cad..dbf685dbafcf 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1342,6 +1342,7 @@ static void rproc_type_release(struct device *dev)
 		ida_simple_remove(&rproc_dev_index, rproc->index);
 
 	kfree(rproc->firmware);
+	kfree(rproc->ops);
 	kfree(rproc);
 }
 
@@ -1406,9 +1407,15 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 		return NULL;
 	}
 
+	rproc->ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
+	if (!rproc->ops) {
+		kfree(p);
+		kfree(rproc);
+		return NULL;
+	}
+
 	rproc->firmware = p;
 	rproc->name = name;
-	rproc->ops = ops;
 	rproc->priv = &rproc[1];
 	rproc->auto_boot = true;
 
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 6fdc62e29d6f..cc4d30a790b3 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -419,7 +419,7 @@ struct rproc {
 	const char *name;
 	char *firmware;
 	void *priv;
-	const struct rproc_ops *ops;
+	struct rproc_ops *ops;
 	struct device dev;
 	const struct rproc_fw_ops *fw_ops;
 	atomic_t power;
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 4/8] remoteproc: Merge rproc_ops and rproc_fw_ops
  2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
                   ` (2 preceding siblings ...)
  2018-01-05 23:58 ` [PATCH v2 3/8] remoteproc: Clone rproc_ops in rproc_alloc() Bjorn Andersson
@ 2018-01-05 23:58 ` Bjorn Andersson
  2018-01-08 21:03   ` Loic PALLARDY
  2018-01-05 23:58 ` [PATCH v2 5/8] remoteproc: Don't handle empty resource table Bjorn Andersson
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-05 23:58 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, Loic Pallardy

There are currently a few different schemes used for overriding fw_ops
or parts of fw_ops. Merge fw_ops into rproc_ops and expose the default
ELF-loader symbols so that they can be assigned by the drivers.

To keep backwards compatibility with the "default" case, a driver not
specifying the "load" operation is assumed to want the full ELF-loader
suit of functions.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Fill load, sanity_check and get_boot_addr for st_slim

 drivers/remoteproc/qcom_adsp_pil.c         |  9 ++----
 drivers/remoteproc/qcom_q6v5_pil.c         |  9 ++----
 drivers/remoteproc/qcom_wcnss.c            |  9 ++----
 drivers/remoteproc/remoteproc_core.c       | 10 ++++--
 drivers/remoteproc/remoteproc_elf_loader.c | 30 +++++++-----------
 drivers/remoteproc/remoteproc_internal.h   | 51 +++++++++++-------------------
 drivers/remoteproc/st_slim_rproc.c         | 22 ++++---------
 include/linux/remoteproc.h                 | 17 ++++++++--
 8 files changed, 67 insertions(+), 90 deletions(-)

diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
index 3f6af54dbc96..56156c12bd73 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -85,11 +85,6 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
 			     adsp->mem_region, adsp->mem_phys, adsp->mem_size);
 }
 
-static const struct rproc_fw_ops adsp_fw_ops = {
-	.find_rsc_table = qcom_mdt_find_rsc_table,
-	.load = adsp_load,
-};
-
 static int adsp_start(struct rproc *rproc)
 {
 	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
@@ -182,6 +177,8 @@ 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,
 };
 
 static irqreturn_t adsp_wdog_interrupt(int irq, void *dev)
@@ -344,8 +341,6 @@ static int adsp_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	rproc->fw_ops = &adsp_fw_ops;
-
 	adsp = (struct qcom_adsp *)rproc->priv;
 	adsp->dev = &pdev->dev;
 	adsp->rproc = rproc;
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index 8a3fa2bcc9f6..3ea668d9fd4c 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -342,11 +342,6 @@ static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
 	return 0;
 }
 
-static const struct rproc_fw_ops q6v5_fw_ops = {
-	.find_rsc_table = q6v5_find_rsc_table,
-	.load = q6v5_load,
-};
-
 static int q6v5_rmb_pbl_wait(struct q6v5 *qproc, int ms)
 {
 	unsigned long timeout;
@@ -931,6 +926,8 @@ 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,
 };
 
 static irqreturn_t q6v5_wdog_interrupt(int irq, void *dev)
@@ -1150,8 +1147,6 @@ static int q6v5_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	rproc->fw_ops = &q6v5_fw_ops;
-
 	qproc = (struct q6v5 *)rproc->priv;
 	qproc->dev = &pdev->dev;
 	qproc->rproc = rproc;
diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index c7686393d505..e53fc6f268fc 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -156,11 +156,6 @@ static int wcnss_load(struct rproc *rproc, const struct firmware *fw)
 			     wcnss->mem_region, wcnss->mem_phys, wcnss->mem_size);
 }
 
-static const struct rproc_fw_ops wcnss_fw_ops = {
-	.find_rsc_table = qcom_mdt_find_rsc_table,
-	.load = wcnss_load,
-};
-
 static void wcnss_indicate_nv_download(struct qcom_wcnss *wcnss)
 {
 	u32 val;
@@ -313,6 +308,8 @@ 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,
 };
 
 static irqreturn_t wcnss_wdog_interrupt(int irq, void *dev)
@@ -492,8 +489,6 @@ static int wcnss_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	rproc->fw_ops = &wcnss_fw_ops;
-
 	wcnss = (struct qcom_wcnss *)rproc->priv;
 	wcnss->dev = &pdev->dev;
 	wcnss->rproc = rproc;
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index dbf685dbafcf..2c669a73e77d 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1437,8 +1437,14 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 
 	atomic_set(&rproc->power, 0);
 
-	/* Set ELF as the default fw_ops handler */
-	rproc->fw_ops = &rproc_elf_fw_ops;
+	/* 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->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;
+	}
 
 	mutex_init(&rproc->lock);
 
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index c523983a4aec..822fa1bf893f 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -39,8 +39,7 @@
  *
  * Make sure this fw image is sane.
  */
-static int
-rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
+int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
 {
 	const char *name = rproc->firmware;
 	struct device *dev = &rproc->dev;
@@ -98,6 +97,7 @@ rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
 
 	return 0;
 }
+EXPORT_SYMBOL(rproc_elf_sanity_check);
 
 /**
  * rproc_elf_get_boot_addr() - Get rproc's boot address.
@@ -110,13 +110,13 @@ rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
  * Note that the boot address is not a configurable property of all remote
  * processors. Some will always boot at a specific hard-coded address.
  */
-static
 u32 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
 {
 	struct elf32_hdr *ehdr  = (struct elf32_hdr *)fw->data;
 
 	return ehdr->e_entry;
 }
+EXPORT_SYMBOL(rproc_elf_get_boot_addr);
 
 /**
  * rproc_elf_load_segments() - load firmware segments to memory
@@ -142,8 +142,7 @@ u32 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
  * directly allocate memory for every segment/resource. This is not yet
  * supported, though.
  */
-static int
-rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
+int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 {
 	struct device *dev = &rproc->dev;
 	struct elf32_hdr *ehdr;
@@ -207,6 +206,7 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 
 	return ret;
 }
+EXPORT_SYMBOL(rproc_elf_load_segments);
 
 static struct elf32_shdr *
 find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size)
@@ -282,9 +282,9 @@ find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size)
  * size into @tablesz. If a valid table isn't found, NULL is returned
  * (and @tablesz isn't set).
  */
-static struct resource_table *
-rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
-			 int *tablesz)
+struct resource_table *rproc_elf_find_rsc_table(struct rproc *rproc,
+						const struct firmware *fw,
+						int *tablesz)
 {
 	struct elf32_hdr *ehdr;
 	struct elf32_shdr *shdr;
@@ -303,6 +303,7 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
 
 	return table;
 }
+EXPORT_SYMBOL(rproc_elf_find_rsc_table);
 
 /**
  * rproc_elf_find_loaded_rsc_table() - find the loaded resource table
@@ -315,8 +316,8 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
  * Returns the pointer to the resource table if it is found or NULL otherwise.
  * If the table wasn't loaded yet the result is unspecified.
  */
-static struct resource_table *
-rproc_elf_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
+struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
+						       const struct firmware *fw)
 {
 	struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
 	struct elf32_shdr *shdr;
@@ -327,11 +328,4 @@ rproc_elf_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
 
 	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size);
 }
-
-const struct rproc_fw_ops rproc_elf_fw_ops = {
-	.load = rproc_elf_load_segments,
-	.find_rsc_table = rproc_elf_find_rsc_table,
-	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
-	.sanity_check = rproc_elf_sanity_check,
-	.get_boot_addr = rproc_elf_get_boot_addr
-};
+EXPORT_SYMBOL(rproc_elf_find_loaded_rsc_table);
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index c1077bec5d0b..a42690c514e2 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -25,26 +25,6 @@
 
 struct rproc;
 
-/**
- * struct rproc_fw_ops - firmware format specific operations.
- * @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
- * @sanity_check:	sanity check the fw image
- * @get_boot_addr:	get boot address to entry point specified in firmware
- */
-struct rproc_fw_ops {
-	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);
-	int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
-	u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
-};
-
 /* from remoteproc_core.c */
 void rproc_release(struct kref *kref);
 irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
@@ -74,11 +54,20 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
 void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
 int rproc_trigger_recovery(struct rproc *rproc);
 
+int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw);
+u32 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw);
+int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw);
+struct resource_table *rproc_elf_find_rsc_table(struct rproc *rproc,
+						const struct firmware *fw,
+						int *tablesz);
+struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
+						       const struct firmware *fw);
+
 static inline
 int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
 {
-	if (rproc->fw_ops->sanity_check)
-		return rproc->fw_ops->sanity_check(rproc, fw);
+	if (rproc->ops->sanity_check)
+		return rproc->ops->sanity_check(rproc, fw);
 
 	return 0;
 }
@@ -86,8 +75,8 @@ int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
 static inline
 u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
 {
-	if (rproc->fw_ops->get_boot_addr)
-		return rproc->fw_ops->get_boot_addr(rproc, fw);
+	if (rproc->ops->get_boot_addr)
+		return rproc->ops->get_boot_addr(rproc, fw);
 
 	return 0;
 }
@@ -95,8 +84,8 @@ u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
 static inline
 int rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
 {
-	if (rproc->fw_ops->load)
-		return rproc->fw_ops->load(rproc, fw);
+	if (rproc->ops->load)
+		return rproc->ops->load(rproc, fw);
 
 	return -EINVAL;
 }
@@ -106,8 +95,8 @@ struct resource_table *rproc_find_rsc_table(struct rproc *rproc,
 					    const struct firmware *fw,
 					    int *tablesz)
 {
-	if (rproc->fw_ops->find_rsc_table)
-		return rproc->fw_ops->find_rsc_table(rproc, fw, tablesz);
+	if (rproc->ops->find_rsc_table)
+		return rproc->ops->find_rsc_table(rproc, fw, tablesz);
 
 	return NULL;
 }
@@ -116,12 +105,10 @@ static inline
 struct resource_table *rproc_find_loaded_rsc_table(struct rproc *rproc,
 						   const struct firmware *fw)
 {
-	if (rproc->fw_ops->find_loaded_rsc_table)
-		return rproc->fw_ops->find_loaded_rsc_table(rproc, fw);
+	if (rproc->ops->find_loaded_rsc_table)
+		return rproc->ops->find_loaded_rsc_table(rproc, fw);
 
 	return NULL;
 }
 
-extern const struct rproc_fw_ops rproc_elf_fw_ops;
-
 #endif /* REMOTEPROC_INTERNAL_H */
diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
index 6cfd862f945b..1bce63a06424 100644
--- a/drivers/remoteproc/st_slim_rproc.c
+++ b/drivers/remoteproc/st_slim_rproc.c
@@ -200,12 +200,6 @@ static void *slim_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
 	return va;
 }
 
-static const struct rproc_ops slim_rproc_ops = {
-	.start		= slim_rproc_start,
-	.stop		= slim_rproc_stop,
-	.da_to_va       = slim_rproc_da_to_va,
-};
-
 /*
  * Firmware handler operations: sanity, boot address, load ...
  */
@@ -223,8 +217,14 @@ static struct resource_table *slim_rproc_find_rsc_table(struct rproc *rproc,
 	return &empty_rsc_tbl;
 }
 
-static struct rproc_fw_ops slim_rproc_fw_ops = {
+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,
+	.get_boot_addr	= rproc_elf_get_boot_addr,
+	.load		= rproc_elf_load_segments,
+	.sanity_check	= rproc_elf_sanity_check,
 };
 
 /**
@@ -249,7 +249,6 @@ struct st_slim_rproc *st_slim_rproc_alloc(struct platform_device *pdev,
 	struct rproc *rproc;
 	struct resource *res;
 	int err, i;
-	const struct rproc_fw_ops *elf_ops;
 
 	if (!fw_name)
 		return ERR_PTR(-EINVAL);
@@ -267,13 +266,6 @@ struct st_slim_rproc *st_slim_rproc_alloc(struct platform_device *pdev,
 	slim_rproc = rproc->priv;
 	slim_rproc->rproc = rproc;
 
-	elf_ops = rproc->fw_ops;
-	/* Use some generic elf ops */
-	slim_rproc_fw_ops.load = elf_ops->load;
-	slim_rproc_fw_ops.sanity_check = elf_ops->sanity_check;
-
-	rproc->fw_ops = &slim_rproc_fw_ops;
-
 	/* get imem and dmem */
 	for (i = 0; i < ARRAY_SIZE(mem_names); i++) {
 		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index cc4d30a790b3..ca2021cf7b39 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -324,6 +324,7 @@ struct rproc_mem_entry {
 };
 
 struct rproc;
+struct firmware;
 
 /**
  * struct rproc_ops - platform-specific device handlers
@@ -331,12 +332,26 @@ struct rproc;
  * @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
+ * @sanity_check:	sanity check the fw image
+ * @get_boot_addr:	get boot address to entry point specified in firmware
  */
 struct rproc_ops {
 	int (*start)(struct rproc *rproc);
 	int (*stop)(struct rproc *rproc);
 	void (*kick)(struct rproc *rproc, int vqid);
 	void * (*da_to_va)(struct rproc *rproc, u64 da, int len);
+	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);
+	int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
+	u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
 };
 
 /**
@@ -390,7 +405,6 @@ enum rproc_crash_type {
  * @priv: private data which belongs to the platform-specific rproc module
  * @ops: platform-specific start/stop rproc handlers
  * @dev: virtual device for refcounting and common remoteproc behavior
- * @fw_ops: firmware-specific handlers
  * @power: refcount of users who need this rproc powered up
  * @state: state of the device
  * @lock: lock which protects concurrent manipulations of the rproc
@@ -421,7 +435,6 @@ struct rproc {
 	void *priv;
 	struct rproc_ops *ops;
 	struct device dev;
-	const struct rproc_fw_ops *fw_ops;
 	atomic_t power;
 	unsigned int state;
 	struct mutex lock;
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 5/8] remoteproc: Don't handle empty resource table
  2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
                   ` (3 preceding siblings ...)
  2018-01-05 23:58 ` [PATCH v2 4/8] remoteproc: Merge rproc_ops and rproc_fw_ops Bjorn Andersson
@ 2018-01-05 23:58 ` Bjorn Andersson
  2018-01-05 23:58 ` [PATCH v2 6/8] remoteproc: Move resource table load logic to find Bjorn Andersson
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-05 23:58 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, Loic Pallardy

Allow a NULL table_ptr to have the same meaning as a table with 0
entries, allowing a subsequent patch to skip the assignment step.

A few other places in the implementation does dereference table_ptr, but
they are currently all coming from rproc_handle_resources().

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- None

 drivers/remoteproc/remoteproc_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 2c669a73e77d..3160cfe897da 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -739,6 +739,9 @@ static int rproc_handle_resources(struct rproc *rproc,
 	rproc_handle_resource_t handler;
 	int ret = 0, i;
 
+	if (!rproc->table_ptr)
+		return 0;
+
 	for (i = 0; i < rproc->table_ptr->num; i++) {
 		int offset = rproc->table_ptr->offset[i];
 		struct fw_rsc_hdr *hdr = (void *)rproc->table_ptr + offset;
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 6/8] remoteproc: Move resource table load logic to find
  2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
                   ` (4 preceding siblings ...)
  2018-01-05 23:58 ` [PATCH v2 5/8] remoteproc: Don't handle empty resource table Bjorn Andersson
@ 2018-01-05 23:58 ` Bjorn Andersson
  2018-01-05 23:58 ` [PATCH v2 7/8] remoteproc: Drop dangling find_rsc_table dummies Bjorn Andersson
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-05 23:58 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, Loic Pallardy

Extend the previous operation of finding the resource table in the ELF
with the extra step of populating the rproc struct with a copy and the
size. This allows drivers to override the mechanism used for acquiring
the resource table, or omit it for firmware that is known not to have a
resource table.

This leaves the custom, dummy, find_rsc_table implementations found in
some drivers dangling.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- None

 drivers/remoteproc/remoteproc_core.c       | 32 ++++++--------------------
 drivers/remoteproc/remoteproc_elf_loader.c | 37 ++++++++++++++++++------------
 drivers/remoteproc/remoteproc_internal.h   | 16 +++++--------
 include/linux/remoteproc.h                 |  2 ++
 4 files changed, 37 insertions(+), 50 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 3160cfe897da..84e07d5b7c2c 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -907,8 +907,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 {
 	struct device *dev = &rproc->dev;
 	const char *name = rproc->firmware;
-	struct resource_table *table;
-	int ret, tablesz;
+	int ret;
 
 	ret = rproc_fw_sanity_check(rproc, fw);
 	if (ret)
@@ -927,27 +926,11 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 	}
 
 	rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
-	ret = -EINVAL;
-
-	/* look for the resource table */
-	table = rproc_find_rsc_table(rproc, fw, &tablesz);
-	if (!table) {
-		dev_err(dev, "Failed to find resource table\n");
-		goto clean_up;
-	}
-
-	/*
-	 * Create a copy of the resource table. When a virtio device starts
-	 * and calls vring_new_virtqueue() the address of the allocated vring
-	 * will be stored in the cached_table. Before the device is started,
-	 * cached_table will be copied into device memory.
-	 */
-	rproc->cached_table = kmemdup(table, tablesz, GFP_KERNEL);
-	if (!rproc->cached_table)
-		goto clean_up;
 
-	rproc->table_ptr = rproc->cached_table;
-	rproc->table_sz = tablesz;
+	/* load resource table */
+	ret = rproc_load_rsc_table(rproc, fw);
+	if (ret)
+		goto disable_iommu;
 
 	/* reset max_notifyid */
 	rproc->max_notifyid = -1;
@@ -967,11 +950,10 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 
 clean_up_resources:
 	rproc_resource_cleanup(rproc);
-clean_up:
 	kfree(rproc->cached_table);
 	rproc->cached_table = NULL;
 	rproc->table_ptr = NULL;
-
+disable_iommu:
 	rproc_disable_iommu(rproc);
 	return ret;
 }
@@ -1443,7 +1425,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->find_rsc_table = rproc_elf_find_rsc_table;
+		rproc->ops->load_rsc_table = 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_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index 822fa1bf893f..b17d72ec8603 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -268,42 +268,49 @@ find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size)
 }
 
 /**
- * rproc_elf_find_rsc_table() - find the resource table
+ * rproc_elf_load_rsc_table() - load the resource table
  * @rproc: the rproc handle
  * @fw: the ELF firmware image
- * @tablesz: place holder for providing back the table size
  *
  * This function finds the resource table inside the remote processor's
- * firmware. It is used both upon the registration of @rproc (in order
- * to look for and register the supported virito devices), and when the
- * @rproc is booted.
+ * firmware, load it into the @cached_table and update @table_ptr.
  *
- * Returns the pointer to the resource table if it is found, and write its
- * size into @tablesz. If a valid table isn't found, NULL is returned
- * (and @tablesz isn't set).
+ * Return: 0 on success, negative errno on failure.
  */
-struct resource_table *rproc_elf_find_rsc_table(struct rproc *rproc,
-						const struct firmware *fw,
-						int *tablesz)
+int rproc_elf_load_rsc_table(struct rproc *rproc, const struct firmware *fw)
 {
 	struct elf32_hdr *ehdr;
 	struct elf32_shdr *shdr;
 	struct device *dev = &rproc->dev;
 	struct resource_table *table = NULL;
 	const u8 *elf_data = fw->data;
+	size_t tablesz;
 
 	ehdr = (struct elf32_hdr *)elf_data;
 
 	shdr = find_table(dev, ehdr, fw->size);
 	if (!shdr)
-		return NULL;
+		return -EINVAL;
 
 	table = (struct resource_table *)(elf_data + shdr->sh_offset);
-	*tablesz = shdr->sh_size;
+	tablesz = shdr->sh_size;
+
+	/*
+	 * Create a copy of the resource table. When a virtio device starts
+	 * and calls vring_new_virtqueue() the address of the allocated vring
+	 * will be stored in the cached_table. Before the device is started,
+	 * cached_table will be copied into device memory.
+	 */
+	rproc->cached_table = kmemdup(table, tablesz, GFP_KERNEL);
+	if (!rproc->cached_table)
+		return -ENOMEM;
 
-	return table;
+	rproc->table_ptr = rproc->cached_table;
+	rproc->table_sz = tablesz;
+
+	return 0;
 }
-EXPORT_SYMBOL(rproc_elf_find_rsc_table);
+EXPORT_SYMBOL(rproc_elf_load_rsc_table);
 
 /**
  * rproc_elf_find_loaded_rsc_table() - find the loaded resource table
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index a42690c514e2..55a2950c5cb7 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -57,9 +57,7 @@ int rproc_trigger_recovery(struct rproc *rproc);
 int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw);
 u32 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw);
 int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw);
-struct resource_table *rproc_elf_find_rsc_table(struct rproc *rproc,
-						const struct firmware *fw,
-						int *tablesz);
+int rproc_elf_load_rsc_table(struct rproc *rproc, const struct firmware *fw);
 struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
 						       const struct firmware *fw);
 
@@ -90,15 +88,13 @@ int rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
 	return -EINVAL;
 }
 
-static inline
-struct resource_table *rproc_find_rsc_table(struct rproc *rproc,
-					    const struct firmware *fw,
-					    int *tablesz)
+static inline int rproc_load_rsc_table(struct rproc *rproc,
+				       const struct firmware *fw)
 {
-	if (rproc->ops->find_rsc_table)
-		return rproc->ops->find_rsc_table(rproc, fw, tablesz);
+	if (rproc->ops->load_rsc_table)
+		return rproc->ops->load_rsc_table(rproc, fw);
 
-	return NULL;
+	return 0;
 }
 
 static inline
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index ca2021cf7b39..cc853745e3a1 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -332,6 +332,7 @@ 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
+ * @load_rsc_table:	load resource table from firmware image
  * @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
@@ -344,6 +345,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);
 	struct resource_table *(*find_rsc_table)(struct rproc *rproc,
 						 const struct firmware *fw,
 						 int *tablesz);
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 7/8] remoteproc: Drop dangling find_rsc_table dummies
  2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
                   ` (5 preceding siblings ...)
  2018-01-05 23:58 ` [PATCH v2 6/8] remoteproc: Move resource table load logic to find Bjorn Andersson
@ 2018-01-05 23:58 ` Bjorn Andersson
  2018-01-05 23:58 ` [PATCH v2 8/8] remoteproc: Reset table_ptr on stop Bjorn Andersson
  2018-01-15 10:35 ` [PATCH v2 0/8] Remoteproc cleanups Loic PALLARDY
  8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-05 23:58 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, Loic Pallardy

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>
---

Changes since v1:
- None

 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 56156c12bd73..373c167892d7 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -177,7 +177,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 d487040b528b..6eb9161f80b3 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 832e20271664..728be9834d8b 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -27,10 +27,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 3ea668d9fd4c..b4e5e725848d 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -303,16 +303,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)
@@ -926,7 +916,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 e53fc6f268fc..3f0609236a76 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -308,7 +308,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 1bce63a06424..1ffb1f0c43d6 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,
 	.get_boot_addr	= rproc_elf_get_boot_addr,
 	.load		= rproc_elf_load_segments,
 	.sanity_check	= rproc_elf_sanity_check,
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index cc853745e3a1..728d421fffe9 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -333,7 +333,6 @@ struct firmware;
  * @kick:	kick a virtqueue (virtqueue id given as a parameter)
  * @da_to_va:	optional platform hook to perform address translations
  * @load_rsc_table:	load resource table from firmware image
- * @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
@@ -346,9 +345,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

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v2 8/8] remoteproc: Reset table_ptr on stop
  2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
                   ` (6 preceding siblings ...)
  2018-01-05 23:58 ` [PATCH v2 7/8] remoteproc: Drop dangling find_rsc_table dummies Bjorn Andersson
@ 2018-01-05 23:58 ` Bjorn Andersson
  2018-01-15 10:35 ` [PATCH v2 0/8] Remoteproc cleanups Loic PALLARDY
  8 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-05 23:58 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, Loic Pallardy

The installed resource table is no longer accessible after stopping the
remote, so update table_ptr to point to the local copy.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- None

 drivers/remoteproc/remoteproc_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 84e07d5b7c2c..4170dfbd93bd 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1000,6 +1000,9 @@ static int rproc_stop(struct rproc *rproc)
 	/* remove any subdevices for the remote processor */
 	rproc_remove_subdevices(rproc);
 
+	/* the installed resource table is no longer accessible */
+	rproc->table_ptr = rproc->cached_table;
+
 	/* power off the remote processor */
 	ret = rproc->ops->stop(rproc);
 	if (ret) {
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* RE: [PATCH v2 4/8] remoteproc: Merge rproc_ops and rproc_fw_ops
  2018-01-05 23:58 ` [PATCH v2 4/8] remoteproc: Merge rproc_ops and rproc_fw_ops Bjorn Andersson
@ 2018-01-08 21:03   ` Loic PALLARDY
  0 siblings, 0 replies; 12+ messages in thread
From: Loic PALLARDY @ 2018-01-08 21:03 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen; +Cc: linux-remoteproc, linux-kernel



> -----Original Message-----
> From: Bjorn Andersson [mailto:bjorn.andersson@linaro.org]
> Sent: Saturday, January 06, 2018 12:58 AM
> To: Ohad Ben-Cohen <ohad@wizery.com>; Bjorn Andersson
> <bjorn.andersson@linaro.org>
> Cc: linux-remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org; Loic
> PALLARDY <loic.pallardy@st.com>
> Subject: [PATCH v2 4/8] remoteproc: Merge rproc_ops and rproc_fw_ops
> 
> There are currently a few different schemes used for overriding fw_ops
> or parts of fw_ops. Merge fw_ops into rproc_ops and expose the default
> ELF-loader symbols so that they can be assigned by the drivers.
> 
> To keep backwards compatibility with the "default" case, a driver not
> specifying the "load" operation is assumed to want the full ELF-loader
> suit of functions.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v1:
> - Fill load, sanity_check and get_boot_addr for st_slim
> 
>  drivers/remoteproc/qcom_adsp_pil.c         |  9 ++----
>  drivers/remoteproc/qcom_q6v5_pil.c         |  9 ++----
>  drivers/remoteproc/qcom_wcnss.c            |  9 ++----
>  drivers/remoteproc/remoteproc_core.c       | 10 ++++--
>  drivers/remoteproc/remoteproc_elf_loader.c | 30 +++++++-----------
>  drivers/remoteproc/remoteproc_internal.h   | 51 +++++++++++---------------
> ----
>  drivers/remoteproc/st_slim_rproc.c         | 22 ++++---------
>  include/linux/remoteproc.h                 | 17 ++++++++--
>  8 files changed, 67 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_adsp_pil.c
> b/drivers/remoteproc/qcom_adsp_pil.c
> index 3f6af54dbc96..56156c12bd73 100644
> --- a/drivers/remoteproc/qcom_adsp_pil.c
> +++ b/drivers/remoteproc/qcom_adsp_pil.c
> @@ -85,11 +85,6 @@ static int adsp_load(struct rproc *rproc, const struct
> firmware *fw)
>  			     adsp->mem_region, adsp->mem_phys, adsp-
> >mem_size);
>  }
> 
> -static const struct rproc_fw_ops adsp_fw_ops = {
> -	.find_rsc_table = qcom_mdt_find_rsc_table,
> -	.load = adsp_load,
> -};
> -
>  static int adsp_start(struct rproc *rproc)
>  {
>  	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> @@ -182,6 +177,8 @@ 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,
>  };
> 
>  static irqreturn_t adsp_wdog_interrupt(int irq, void *dev)
> @@ -344,8 +341,6 @@ static int adsp_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
> 
> -	rproc->fw_ops = &adsp_fw_ops;
> -
>  	adsp = (struct qcom_adsp *)rproc->priv;
>  	adsp->dev = &pdev->dev;
>  	adsp->rproc = rproc;
> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c
> b/drivers/remoteproc/qcom_q6v5_pil.c
> index 8a3fa2bcc9f6..3ea668d9fd4c 100644
> --- a/drivers/remoteproc/qcom_q6v5_pil.c
> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
> @@ -342,11 +342,6 @@ static int q6v5_load(struct rproc *rproc, const struct
> firmware *fw)
>  	return 0;
>  }
> 
> -static const struct rproc_fw_ops q6v5_fw_ops = {
> -	.find_rsc_table = q6v5_find_rsc_table,
> -	.load = q6v5_load,
> -};
> -
>  static int q6v5_rmb_pbl_wait(struct q6v5 *qproc, int ms)
>  {
>  	unsigned long timeout;
> @@ -931,6 +926,8 @@ 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,
>  };
> 
>  static irqreturn_t q6v5_wdog_interrupt(int irq, void *dev)
> @@ -1150,8 +1147,6 @@ static int q6v5_probe(struct platform_device
> *pdev)
>  		return -ENOMEM;
>  	}
> 
> -	rproc->fw_ops = &q6v5_fw_ops;
> -
>  	qproc = (struct q6v5 *)rproc->priv;
>  	qproc->dev = &pdev->dev;
>  	qproc->rproc = rproc;
> diff --git a/drivers/remoteproc/qcom_wcnss.c
> b/drivers/remoteproc/qcom_wcnss.c
> index c7686393d505..e53fc6f268fc 100644
> --- a/drivers/remoteproc/qcom_wcnss.c
> +++ b/drivers/remoteproc/qcom_wcnss.c
> @@ -156,11 +156,6 @@ static int wcnss_load(struct rproc *rproc, const struct
> firmware *fw)
>  			     wcnss->mem_region, wcnss->mem_phys, wcnss-
> >mem_size);
>  }
> 
> -static const struct rproc_fw_ops wcnss_fw_ops = {
> -	.find_rsc_table = qcom_mdt_find_rsc_table,
> -	.load = wcnss_load,
> -};
> -
>  static void wcnss_indicate_nv_download(struct qcom_wcnss *wcnss)
>  {
>  	u32 val;
> @@ -313,6 +308,8 @@ 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,
>  };
> 
>  static irqreturn_t wcnss_wdog_interrupt(int irq, void *dev)
> @@ -492,8 +489,6 @@ static int wcnss_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
> 
> -	rproc->fw_ops = &wcnss_fw_ops;
> -
>  	wcnss = (struct qcom_wcnss *)rproc->priv;
>  	wcnss->dev = &pdev->dev;
>  	wcnss->rproc = rproc;
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index dbf685dbafcf..2c669a73e77d 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1437,8 +1437,14 @@ struct rproc *rproc_alloc(struct device *dev, const
> char *name,
> 
>  	atomic_set(&rproc->power, 0);
> 
> -	/* Set ELF as the default fw_ops handler */
> -	rproc->fw_ops = &rproc_elf_fw_ops;
> +	/* 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->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;
> +	}
> 
Hi Bjorn,

Code looks good. I'll tested by end of the week, when I'll be back to the office.

Regards,
Loic
>  	mutex_init(&rproc->lock);
> 
> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c
> b/drivers/remoteproc/remoteproc_elf_loader.c
> index c523983a4aec..822fa1bf893f 100644
> --- a/drivers/remoteproc/remoteproc_elf_loader.c
> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> @@ -39,8 +39,7 @@
>   *
>   * Make sure this fw image is sane.
>   */
> -static int
> -rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
> +int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
>  {
>  	const char *name = rproc->firmware;
>  	struct device *dev = &rproc->dev;
> @@ -98,6 +97,7 @@ rproc_elf_sanity_check(struct rproc *rproc, const struct
> firmware *fw)
> 
>  	return 0;
>  }
> +EXPORT_SYMBOL(rproc_elf_sanity_check);
> 
>  /**
>   * rproc_elf_get_boot_addr() - Get rproc's boot address.
> @@ -110,13 +110,13 @@ rproc_elf_sanity_check(struct rproc *rproc, const
> struct firmware *fw)
>   * Note that the boot address is not a configurable property of all remote
>   * processors. Some will always boot at a specific hard-coded address.
>   */
> -static
>  u32 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware
> *fw)
>  {
>  	struct elf32_hdr *ehdr  = (struct elf32_hdr *)fw->data;
> 
>  	return ehdr->e_entry;
>  }
> +EXPORT_SYMBOL(rproc_elf_get_boot_addr);
> 
>  /**
>   * rproc_elf_load_segments() - load firmware segments to memory
> @@ -142,8 +142,7 @@ u32 rproc_elf_get_boot_addr(struct rproc *rproc,
> const struct firmware *fw)
>   * directly allocate memory for every segment/resource. This is not yet
>   * supported, though.
>   */
> -static int
> -rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
> +int rproc_elf_load_segments(struct rproc *rproc, const struct firmware
> *fw)
>  {
>  	struct device *dev = &rproc->dev;
>  	struct elf32_hdr *ehdr;
> @@ -207,6 +206,7 @@ rproc_elf_load_segments(struct rproc *rproc, const
> struct firmware *fw)
> 
>  	return ret;
>  }
> +EXPORT_SYMBOL(rproc_elf_load_segments);
> 
>  static struct elf32_shdr *
>  find_table(struct device *dev, struct elf32_hdr *ehdr, size_t fw_size)
> @@ -282,9 +282,9 @@ find_table(struct device *dev, struct elf32_hdr *ehdr,
> size_t fw_size)
>   * size into @tablesz. If a valid table isn't found, NULL is returned
>   * (and @tablesz isn't set).
>   */
> -static struct resource_table *
> -rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
> -			 int *tablesz)
> +struct resource_table *rproc_elf_find_rsc_table(struct rproc *rproc,
> +						const struct firmware *fw,
> +						int *tablesz)
>  {
>  	struct elf32_hdr *ehdr;
>  	struct elf32_shdr *shdr;
> @@ -303,6 +303,7 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const
> struct firmware *fw,
> 
>  	return table;
>  }
> +EXPORT_SYMBOL(rproc_elf_find_rsc_table);
> 
>  /**
>   * rproc_elf_find_loaded_rsc_table() - find the loaded resource table
> @@ -315,8 +316,8 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const
> struct firmware *fw,
>   * Returns the pointer to the resource table if it is found or NULL otherwise.
>   * If the table wasn't loaded yet the result is unspecified.
>   */
> -static struct resource_table *
> -rproc_elf_find_loaded_rsc_table(struct rproc *rproc, const struct firmware
> *fw)
> +struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc
> *rproc,
> +						       const struct firmware
> *fw)
>  {
>  	struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
>  	struct elf32_shdr *shdr;
> @@ -327,11 +328,4 @@ rproc_elf_find_loaded_rsc_table(struct rproc
> *rproc, const struct firmware *fw)
> 
>  	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size);
>  }
> -
> -const struct rproc_fw_ops rproc_elf_fw_ops = {
> -	.load = rproc_elf_load_segments,
> -	.find_rsc_table = rproc_elf_find_rsc_table,
> -	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
> -	.sanity_check = rproc_elf_sanity_check,
> -	.get_boot_addr = rproc_elf_get_boot_addr
> -};
> +EXPORT_SYMBOL(rproc_elf_find_loaded_rsc_table);
> diff --git a/drivers/remoteproc/remoteproc_internal.h
> b/drivers/remoteproc/remoteproc_internal.h
> index c1077bec5d0b..a42690c514e2 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -25,26 +25,6 @@
> 
>  struct rproc;
> 
> -/**
> - * struct rproc_fw_ops - firmware format specific operations.
> - * @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
> - * @sanity_check:	sanity check the fw image
> - * @get_boot_addr:	get boot address to entry point specified in firmware
> - */
> -struct rproc_fw_ops {
> -	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);
> -	int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
> -	u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware
> *fw);
> -};
> -
>  /* from remoteproc_core.c */
>  void rproc_release(struct kref *kref);
>  irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
> @@ -74,11 +54,20 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
>  void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
>  int rproc_trigger_recovery(struct rproc *rproc);
> 
> +int rproc_elf_sanity_check(struct rproc *rproc, const struct firmware *fw);
> +u32 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware
> *fw);
> +int rproc_elf_load_segments(struct rproc *rproc, const struct firmware
> *fw);
> +struct resource_table *rproc_elf_find_rsc_table(struct rproc *rproc,
> +						const struct firmware *fw,
> +						int *tablesz);
> +struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc
> *rproc,
> +						       const struct firmware
> *fw);
> +
>  static inline
>  int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
>  {
> -	if (rproc->fw_ops->sanity_check)
> -		return rproc->fw_ops->sanity_check(rproc, fw);
> +	if (rproc->ops->sanity_check)
> +		return rproc->ops->sanity_check(rproc, fw);
> 
>  	return 0;
>  }
> @@ -86,8 +75,8 @@ int rproc_fw_sanity_check(struct rproc *rproc, const
> struct firmware *fw)
>  static inline
>  u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
>  {
> -	if (rproc->fw_ops->get_boot_addr)
> -		return rproc->fw_ops->get_boot_addr(rproc, fw);
> +	if (rproc->ops->get_boot_addr)
> +		return rproc->ops->get_boot_addr(rproc, fw);
> 
>  	return 0;
>  }
> @@ -95,8 +84,8 @@ u32 rproc_get_boot_addr(struct rproc *rproc, const
> struct firmware *fw)
>  static inline
>  int rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
>  {
> -	if (rproc->fw_ops->load)
> -		return rproc->fw_ops->load(rproc, fw);
> +	if (rproc->ops->load)
> +		return rproc->ops->load(rproc, fw);
> 
>  	return -EINVAL;
>  }
> @@ -106,8 +95,8 @@ struct resource_table *rproc_find_rsc_table(struct
> rproc *rproc,
>  					    const struct firmware *fw,
>  					    int *tablesz)
>  {
> -	if (rproc->fw_ops->find_rsc_table)
> -		return rproc->fw_ops->find_rsc_table(rproc, fw, tablesz);
> +	if (rproc->ops->find_rsc_table)
> +		return rproc->ops->find_rsc_table(rproc, fw, tablesz);
> 
>  	return NULL;
>  }
> @@ -116,12 +105,10 @@ static inline
>  struct resource_table *rproc_find_loaded_rsc_table(struct rproc *rproc,
>  						   const struct firmware *fw)
>  {
> -	if (rproc->fw_ops->find_loaded_rsc_table)
> -		return rproc->fw_ops->find_loaded_rsc_table(rproc, fw);
> +	if (rproc->ops->find_loaded_rsc_table)
> +		return rproc->ops->find_loaded_rsc_table(rproc, fw);
> 
>  	return NULL;
>  }
> 
> -extern const struct rproc_fw_ops rproc_elf_fw_ops;
> -
>  #endif /* REMOTEPROC_INTERNAL_H */
> diff --git a/drivers/remoteproc/st_slim_rproc.c
> b/drivers/remoteproc/st_slim_rproc.c
> index 6cfd862f945b..1bce63a06424 100644
> --- a/drivers/remoteproc/st_slim_rproc.c
> +++ b/drivers/remoteproc/st_slim_rproc.c
> @@ -200,12 +200,6 @@ static void *slim_rproc_da_to_va(struct rproc
> *rproc, u64 da, int len)
>  	return va;
>  }
> 
> -static const struct rproc_ops slim_rproc_ops = {
> -	.start		= slim_rproc_start,
> -	.stop		= slim_rproc_stop,
> -	.da_to_va       = slim_rproc_da_to_va,
> -};
> -
>  /*
>   * Firmware handler operations: sanity, boot address, load ...
>   */
> @@ -223,8 +217,14 @@ static struct resource_table
> *slim_rproc_find_rsc_table(struct rproc *rproc,
>  	return &empty_rsc_tbl;
>  }
> 
> -static struct rproc_fw_ops slim_rproc_fw_ops = {
> +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,
> +	.get_boot_addr	= rproc_elf_get_boot_addr,
> +	.load		= rproc_elf_load_segments,
> +	.sanity_check	= rproc_elf_sanity_check,
>  };
> 
>  /**
> @@ -249,7 +249,6 @@ struct st_slim_rproc *st_slim_rproc_alloc(struct
> platform_device *pdev,
>  	struct rproc *rproc;
>  	struct resource *res;
>  	int err, i;
> -	const struct rproc_fw_ops *elf_ops;
> 
>  	if (!fw_name)
>  		return ERR_PTR(-EINVAL);
> @@ -267,13 +266,6 @@ struct st_slim_rproc *st_slim_rproc_alloc(struct
> platform_device *pdev,
>  	slim_rproc = rproc->priv;
>  	slim_rproc->rproc = rproc;
> 
> -	elf_ops = rproc->fw_ops;
> -	/* Use some generic elf ops */
> -	slim_rproc_fw_ops.load = elf_ops->load;
> -	slim_rproc_fw_ops.sanity_check = elf_ops->sanity_check;
> -
> -	rproc->fw_ops = &slim_rproc_fw_ops;
> -
>  	/* get imem and dmem */
>  	for (i = 0; i < ARRAY_SIZE(mem_names); i++) {
>  		res = platform_get_resource_byname(pdev,
> IORESOURCE_MEM,
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index cc4d30a790b3..ca2021cf7b39 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -324,6 +324,7 @@ struct rproc_mem_entry {
>  };
> 
>  struct rproc;
> +struct firmware;
> 
>  /**
>   * struct rproc_ops - platform-specific device handlers
> @@ -331,12 +332,26 @@ struct rproc;
>   * @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
> + * @sanity_check:	sanity check the fw image
> + * @get_boot_addr:	get boot address to entry point specified in firmware
>   */
>  struct rproc_ops {
>  	int (*start)(struct rproc *rproc);
>  	int (*stop)(struct rproc *rproc);
>  	void (*kick)(struct rproc *rproc, int vqid);
>  	void * (*da_to_va)(struct rproc *rproc, u64 da, int len);
> +	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);
> +	int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
> +	u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware
> *fw);
>  };
> 
>  /**
> @@ -390,7 +405,6 @@ enum rproc_crash_type {
>   * @priv: private data which belongs to the platform-specific rproc module
>   * @ops: platform-specific start/stop rproc handlers
>   * @dev: virtual device for refcounting and common remoteproc behavior
> - * @fw_ops: firmware-specific handlers
>   * @power: refcount of users who need this rproc powered up
>   * @state: state of the device
>   * @lock: lock which protects concurrent manipulations of the rproc
> @@ -421,7 +435,6 @@ struct rproc {
>  	void *priv;
>  	struct rproc_ops *ops;
>  	struct device dev;
> -	const struct rproc_fw_ops *fw_ops;
>  	atomic_t power;
>  	unsigned int state;
>  	struct mutex lock;
> --
> 2.15.0

^ permalink raw reply	[flat|nested] 12+ messages in thread
* RE: [PATCH v2 0/8] Remoteproc cleanups
  2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
                   ` (7 preceding siblings ...)
  2018-01-05 23:58 ` [PATCH v2 8/8] remoteproc: Reset table_ptr on stop Bjorn Andersson
@ 2018-01-15 10:35 ` Loic PALLARDY
  2018-01-15 17:30   ` Bjorn Andersson
  8 siblings, 1 reply; 12+ messages in thread
From: Loic PALLARDY @ 2018-01-15 10:35 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen; +Cc: linux-remoteproc, linux-kernel



> -----Original Message-----
> From: Bjorn Andersson [mailto:bjorn.andersson@linaro.org]
> Sent: Saturday, January 06, 2018 12:58 AM
> To: Ohad Ben-Cohen <ohad@wizery.com>; Bjorn Andersson
> <bjorn.andersson@linaro.org>
> Cc: linux-remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org; Loic
> PALLARDY <loic.pallardy@st.com>
> Subject: [PATCH v2 0/8] Remoteproc cleanups
> 
> The first patch removes code that became unnecessary when the recovery
> flow was
> redesigned.
> 
> The following patches moves the assignment of cached_table to the
> resource
> table loader, rather than core code, which allows this to made optional and
> finally drops the various dummy resource tables provided by drivers.
> 
> Then finally the last patch ensures that table_ptr isn't left pointing into
> memory of a stopped remoteproc.

I have tested this series on B2260 96Board for st-slim remote processor support.

Reviewed-By: Loic Pallardy <loic.pallardy@st.com>
Tested-By: Loic Pallardy <loic.pallardy@st.com>

Regards,
Loic
> 
> Bjorn Andersson (8):
>   remoteproc: Remove depricated crash completion
>   remoteproc: Cache resource table size
>   remoteproc: Clone rproc_ops in rproc_alloc()
>   remoteproc: Merge rproc_ops and rproc_fw_ops
>   remoteproc: Don't handle empty resource table
>   remoteproc: Move resource table load logic to find
>   remoteproc: Drop dangling find_rsc_table dummies
>   remoteproc: Reset table_ptr on stop
> 
>  drivers/remoteproc/qcom_adsp_pil.c         |  8 +--
>  drivers/remoteproc/qcom_common.c           | 19 -------
>  drivers/remoteproc/qcom_common.h           |  4 --
>  drivers/remoteproc/qcom_q6v5_pil.c         | 18 +------
>  drivers/remoteproc/qcom_wcnss.c            |  8 +--
>  drivers/remoteproc/remoteproc_core.c       | 83 ++++++++++++---------------
> ---
>  drivers/remoteproc/remoteproc_elf_loader.c | 59 ++++++++++-----------
>  drivers/remoteproc/remoteproc_internal.h   | 57 +++++++-------------
>  drivers/remoteproc/st_slim_rproc.c         | 32 ++----------
>  include/linux/remoteproc.h                 | 21 ++++++--
>  10 files changed, 106 insertions(+), 203 deletions(-)
> 
> --
> 2.15.0

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/8] Remoteproc cleanups
  2018-01-15 10:35 ` [PATCH v2 0/8] Remoteproc cleanups Loic PALLARDY
@ 2018-01-15 17:30   ` Bjorn Andersson
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2018-01-15 17:30 UTC (permalink / raw)
  To: Loic PALLARDY; +Cc: Ohad Ben-Cohen, linux-remoteproc, linux-kernel

On Mon 15 Jan 02:35 PST 2018, Loic PALLARDY wrote:

> 
> 
> > -----Original Message-----
> > From: Bjorn Andersson [mailto:bjorn.andersson@linaro.org]
> > Sent: Saturday, January 06, 2018 12:58 AM
> > To: Ohad Ben-Cohen <ohad@wizery.com>; Bjorn Andersson
> > <bjorn.andersson@linaro.org>
> > Cc: linux-remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org; Loic
> > PALLARDY <loic.pallardy@st.com>
> > Subject: [PATCH v2 0/8] Remoteproc cleanups
> > 
> > The first patch removes code that became unnecessary when the recovery
> > flow was
> > redesigned.
> > 
> > The following patches moves the assignment of cached_table to the
> > resource
> > table loader, rather than core code, which allows this to made optional and
> > finally drops the various dummy resource tables provided by drivers.
> > 
> > Then finally the last patch ensures that table_ptr isn't left pointing into
> > memory of a stopped remoteproc.
> 
> I have tested this series on B2260 96Board for st-slim remote processor support.
> 
> Reviewed-By: Loic Pallardy <loic.pallardy@st.com>
> Tested-By: Loic Pallardy <loic.pallardy@st.com>
> 

Thanks Loic, very much appreciate you looking into this!

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-01-15 17:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-05 23:57 [PATCH v2 0/8] Remoteproc cleanups Bjorn Andersson
2018-01-05 23:57 ` [PATCH v2 1/8] remoteproc: Remove depricated crash completion Bjorn Andersson
2018-01-05 23:57 ` [PATCH v2 2/8] remoteproc: Cache resource table size Bjorn Andersson
2018-01-05 23:58 ` [PATCH v2 3/8] remoteproc: Clone rproc_ops in rproc_alloc() Bjorn Andersson
2018-01-05 23:58 ` [PATCH v2 4/8] remoteproc: Merge rproc_ops and rproc_fw_ops Bjorn Andersson
2018-01-08 21:03   ` Loic PALLARDY
2018-01-05 23:58 ` [PATCH v2 5/8] remoteproc: Don't handle empty resource table Bjorn Andersson
2018-01-05 23:58 ` [PATCH v2 6/8] remoteproc: Move resource table load logic to find Bjorn Andersson
2018-01-05 23:58 ` [PATCH v2 7/8] remoteproc: Drop dangling find_rsc_table dummies Bjorn Andersson
2018-01-05 23:58 ` [PATCH v2 8/8] remoteproc: Reset table_ptr on stop Bjorn Andersson
2018-01-15 10:35 ` [PATCH v2 0/8] Remoteproc cleanups Loic PALLARDY
2018-01-15 17:30   ` Bjorn Andersson

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.