linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] remoteproc: qcom: q6v5: Decouple driver from MDT loader
@ 2017-01-30 16:55 Bjorn Andersson
  2017-01-30 16:55 ` [PATCH 2/5] remoteproc: qcom: mdt_loader: Don't overwrite firmware object Bjorn Andersson
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Bjorn Andersson @ 2017-01-30 16:55 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, linux-arm-msm, Avaneesh Kumar Dwivedi

Rather than duplicating half of the MDT loader in the validation step
move the entire MDT parser into the q6v5 driver. This allows us to make
the shared MDT-loader call the SCM PAS operations directly which
simplifies the client code and allows for better reuse of the code.

Cc: Avaneesh Kumar Dwivedi <akdwived@codeaurora.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/remoteproc/Kconfig         |   1 -
 drivers/remoteproc/qcom_q6v5_pil.c | 149 +++++++++++++++++++++++--------------
 2 files changed, 92 insertions(+), 58 deletions(-)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index a5e888043f1f..454fd9a4dd96 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -92,7 +92,6 @@ config QCOM_Q6V5_PIL
 	depends on QCOM_SMEM
 	depends on REMOTEPROC
 	select MFD_SYSCON
-	select QCOM_MDT_LOADER
 	select QCOM_SCM
 	help
 	  Say y here to support the Qualcomm Peripherial Image Loader for the
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index 79a8a371a2fb..db341028f2d1 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -37,7 +37,7 @@
 
 #include <linux/qcom_scm.h>
 
-#define MPSS_FIRMWARE_NAME		"modem.mdt"
+#define MPSS_FIRMWARE_NAME		"modem"
 
 #define MPSS_CRASH_REASON_SMEM		421
 
@@ -277,6 +277,16 @@ 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_load(struct rproc *rproc, const struct firmware *fw)
 {
 	struct q6v5 *qproc = rproc->priv;
@@ -287,7 +297,7 @@ static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
 }
 
 static const struct rproc_fw_ops q6v5_fw_ops = {
-	.find_rsc_table = qcom_mdt_find_rsc_table,
+	.find_rsc_table = q6v5_find_rsc_table,
 	.load = q6v5_load,
 };
 
@@ -464,46 +474,109 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
 	return ret < 0 ? ret : 0;
 }
 
-static int q6v5_mpss_validate(struct q6v5 *qproc, const struct firmware *fw)
+static bool q6v5_phdr_valid(const struct elf32_phdr *phdr)
+{
+	if (phdr->p_type != PT_LOAD)
+		return false;
+
+	if ((phdr->p_flags & QCOM_MDT_TYPE_MASK) == QCOM_MDT_TYPE_HASH)
+		return false;
+
+	if (!phdr->p_memsz)
+		return false;
+
+	return true;
+}
+
+static int q6v5_mpss_load(struct q6v5 *qproc)
 {
 	const struct elf32_phdr *phdrs;
 	const struct elf32_phdr *phdr;
+	const struct firmware *seg_fw;
+	const struct firmware *fw;
 	struct elf32_hdr *ehdr;
-	phys_addr_t boot_addr;
-	phys_addr_t fw_addr;
-	bool relocate;
+	phys_addr_t mpss_reloc;
+	phys_addr_t min_addr = (phys_addr_t)ULLONG_MAX;
+	phys_addr_t max_addr = 0;
+	bool relocate = false;
+	char seg_name[10];
+	size_t offset;
 	size_t size;
+	void *ptr;
 	int ret;
 	int i;
 
-	ret = qcom_mdt_parse(fw, &fw_addr, NULL, &relocate);
-	if (ret) {
-		dev_err(qproc->dev, "failed to parse mdt header\n");
+	ret = request_firmware(&fw, MPSS_FIRMWARE_NAME ".mdt", qproc->dev);
+	if (ret < 0) {
+		dev_err(qproc->dev, "unable to load " MPSS_FIRMWARE_NAME ".mdt\n");
 		return ret;
 	}
 
-	if (relocate)
-		boot_addr = qproc->mpss_phys;
-	else
-		boot_addr = fw_addr;
+	/* Initialize the RMB validator */
+	writel(0, qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
+
+	ret = q6v5_mpss_init_image(qproc, fw);
+	if (ret)
+		goto release_firmware;
 
 	ehdr = (struct elf32_hdr *)fw->data;
 	phdrs = (struct elf32_phdr *)(ehdr + 1);
-	for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
+
+	for (i = 0; i < ehdr->e_phnum; i++) {
 		phdr = &phdrs[i];
 
-		if (phdr->p_type != PT_LOAD)
+		if (!q6v5_phdr_valid(phdr))
 			continue;
 
-		if ((phdr->p_flags & QCOM_MDT_TYPE_MASK) == QCOM_MDT_TYPE_HASH)
-			continue;
+		if (phdr->p_flags & QCOM_MDT_RELOCATABLE)
+			relocate = true;
 
-		if (!phdr->p_memsz)
+		if (phdr->p_paddr < min_addr)
+			min_addr = phdr->p_paddr;
+
+		if (phdr->p_paddr + phdr->p_memsz > max_addr)
+			max_addr = ALIGN(phdr->p_paddr + phdr->p_memsz, SZ_4K);
+	}
+
+	mpss_reloc = relocate ? min_addr : qproc->mpss_phys;
+
+	for (i = 0; i < ehdr->e_phnum; i++) {
+		phdr = &phdrs[i];
+
+		if (!q6v5_phdr_valid(phdr))
 			continue;
 
+		offset = phdr->p_paddr - mpss_reloc;
+		if (offset < 0 || offset + phdr->p_memsz > qproc->mpss_size) {
+			dev_err(qproc->dev, "segment outside memory range\n");
+			ret = -EINVAL;
+			goto release_firmware;
+		}
+
+		ptr = qproc->mpss_region + offset;
+
+		if (phdr->p_filesz) {
+			snprintf(seg_name, sizeof(seg_name),
+				 MPSS_FIRMWARE_NAME ".b%02d", i);
+			ret = request_firmware(&seg_fw, seg_name, qproc->dev);
+			if (ret) {
+				dev_err(qproc->dev, "failed to load %s\n", seg_name);
+				goto release_firmware;
+			}
+
+			memcpy(ptr, seg_fw->data, seg_fw->size);
+
+			release_firmware(seg_fw);
+		}
+
+		if (phdr->p_memsz > phdr->p_filesz) {
+			memset(ptr + phdr->p_filesz, 0,
+			       phdr->p_memsz - phdr->p_filesz);
+		}
+
 		size = readl(qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
 		if (!size) {
-			writel(boot_addr, qproc->rmb_base + RMB_PMI_CODE_START_REG);
+			writel(mpss_reloc, qproc->rmb_base + RMB_PMI_CODE_START_REG);
 			writel(RMB_CMD_LOAD_READY, qproc->rmb_base + RMB_MBA_COMMAND_REG);
 		}
 
@@ -517,44 +590,6 @@ static int q6v5_mpss_validate(struct q6v5 *qproc, const struct firmware *fw)
 	else if (ret < 0)
 		dev_err(qproc->dev, "MPSS authentication failed: %d\n", ret);
 
-	return ret < 0 ? ret : 0;
-}
-
-static int q6v5_mpss_load(struct q6v5 *qproc)
-{
-	const struct firmware *fw;
-	phys_addr_t fw_addr;
-	bool relocate;
-	int ret;
-
-	ret = request_firmware(&fw, MPSS_FIRMWARE_NAME, qproc->dev);
-	if (ret < 0) {
-		dev_err(qproc->dev, "unable to load " MPSS_FIRMWARE_NAME "\n");
-		return ret;
-	}
-
-	ret = qcom_mdt_parse(fw, &fw_addr, NULL, &relocate);
-	if (ret) {
-		dev_err(qproc->dev, "failed to parse mdt header\n");
-		goto release_firmware;
-	}
-
-	if (relocate)
-		qproc->mpss_reloc = fw_addr;
-
-	/* Initialize the RMB validator */
-	writel(0, qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
-
-	ret = q6v5_mpss_init_image(qproc, fw);
-	if (ret)
-		goto release_firmware;
-
-	ret = qcom_mdt_load(qproc->rproc, fw, MPSS_FIRMWARE_NAME);
-	if (ret)
-		goto release_firmware;
-
-	ret = q6v5_mpss_validate(qproc, fw);
-
 release_firmware:
 	release_firmware(fw);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] remoteproc: qcom: mdt_loader: Don't overwrite firmware object
  2017-01-30 16:55 [PATCH 1/5] remoteproc: qcom: q6v5: Decouple driver from MDT loader Bjorn Andersson
@ 2017-01-30 16:55 ` Bjorn Andersson
  2017-01-30 16:55 ` [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader Bjorn Andersson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Bjorn Andersson @ 2017-01-30 16:55 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, linux-arm-msm, Andy Gross, stable

The "fw" firmware object is passed from the remoteproc core and should
not be overwritten, as that results in leaked buffers and a double free
of the the last firmware object.

Fixes: 051fb70fd4ea ("remoteproc: qcom: Driver for the self-authenticating Hexagon v5")
Cc: stable@vger.kernel.org
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/remoteproc/qcom_mdt_loader.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/qcom_mdt_loader.c b/drivers/remoteproc/qcom_mdt_loader.c
index 2ff18cd6c096..2393398f63ea 100644
--- a/drivers/remoteproc/qcom_mdt_loader.c
+++ b/drivers/remoteproc/qcom_mdt_loader.c
@@ -116,6 +116,7 @@ int qcom_mdt_load(struct rproc *rproc,
 	const struct elf32_phdr *phdrs;
 	const struct elf32_phdr *phdr;
 	const struct elf32_hdr *ehdr;
+	const struct firmware *seg_fw;
 	size_t fw_name_len;
 	char *fw_name;
 	void *ptr;
@@ -154,16 +155,16 @@ int qcom_mdt_load(struct rproc *rproc,
 
 		if (phdr->p_filesz) {
 			sprintf(fw_name + fw_name_len - 3, "b%02d", i);
-			ret = request_firmware(&fw, fw_name, &rproc->dev);
+			ret = request_firmware(&seg_fw, fw_name, &rproc->dev);
 			if (ret) {
 				dev_err(&rproc->dev, "failed to load %s\n",
 					fw_name);
 				break;
 			}
 
-			memcpy(ptr, fw->data, fw->size);
+			memcpy(ptr, seg_fw->data, seg_fw->size);
 
-			release_firmware(fw);
+			release_firmware(seg_fw);
 		}
 
 		if (phdr->p_memsz > phdr->p_filesz)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader
  2017-01-30 16:55 [PATCH 1/5] remoteproc: qcom: q6v5: Decouple driver from MDT loader Bjorn Andersson
  2017-01-30 16:55 ` [PATCH 2/5] remoteproc: qcom: mdt_loader: Don't overwrite firmware object Bjorn Andersson
@ 2017-01-30 16:55 ` Bjorn Andersson
  2017-02-02 15:21   ` Stanimir Varbanov
  2017-02-08 10:33   ` Stanimir Varbanov
  2017-01-30 16:55 ` [PATCH 4/5] remoteproc: qcom-common: Extract non-mdt related helper Bjorn Andersson
  2017-01-30 16:55 ` [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc Bjorn Andersson
  3 siblings, 2 replies; 9+ messages in thread
From: Bjorn Andersson @ 2017-01-30 16:55 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, linux-arm-msm, Andy Gross

Pushing the SCM calls into the MDT loader reduces duplication in the
callers and allows for non-remoteproc clients to use the helper for
parsing and loading MDT files.

Cc: Andy Gross <andy.gross@linaro.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/remoteproc/qcom_adsp_pil.c   |  29 +-------
 drivers/remoteproc/qcom_mdt_loader.c | 134 +++++++++++++++++++++++------------
 drivers/remoteproc/qcom_mdt_loader.h |   6 +-
 drivers/remoteproc/qcom_q6v5_pil.c   |   4 +-
 drivers/remoteproc/qcom_wcnss.c      |  29 +-------
 5 files changed, 100 insertions(+), 102 deletions(-)

diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
index 43a4ed2f346c..e300b8c7e5aa 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -65,34 +65,9 @@ struct qcom_adsp {
 static int adsp_load(struct rproc *rproc, const struct firmware *fw)
 {
 	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
-	phys_addr_t fw_addr;
-	size_t fw_size;
-	bool relocate;
-	int ret;
-
-	ret = qcom_scm_pas_init_image(ADSP_PAS_ID, fw->data, fw->size);
-	if (ret) {
-		dev_err(&rproc->dev, "invalid firmware metadata\n");
-		return ret;
-	}
-
-	ret = qcom_mdt_parse(fw, &fw_addr, &fw_size, &relocate);
-	if (ret) {
-		dev_err(&rproc->dev, "failed to parse mdt header\n");
-		return ret;
-	}
-
-	if (relocate) {
-		adsp->mem_reloc = fw_addr;
-
-		ret = qcom_scm_pas_mem_setup(ADSP_PAS_ID, adsp->mem_phys, fw_size);
-		if (ret) {
-			dev_err(&rproc->dev, "unable to setup memory for image\n");
-			return ret;
-		}
-	}
 
-	return qcom_mdt_load(rproc, fw, rproc->firmware);
+	return qcom_mdt_load(adsp->dev, fw, rproc->firmware, ADSP_PAS_ID,
+			     adsp->mem_region, adsp->mem_phys, adsp->mem_size);
 }
 
 static const struct rproc_fw_ops adsp_fw_ops = {
diff --git a/drivers/remoteproc/qcom_mdt_loader.c b/drivers/remoteproc/qcom_mdt_loader.c
index 2393398f63ea..f239f6fddbb7 100644
--- a/drivers/remoteproc/qcom_mdt_loader.c
+++ b/drivers/remoteproc/qcom_mdt_loader.c
@@ -19,6 +19,7 @@
 #include <linux/firmware.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/qcom_scm.h>
 #include <linux/remoteproc.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
@@ -45,24 +46,33 @@ struct resource_table *qcom_mdt_find_rsc_table(struct rproc *rproc,
 }
 EXPORT_SYMBOL_GPL(qcom_mdt_find_rsc_table);
 
+static bool mdt_phdr_valid(const struct elf32_phdr *phdr)
+{
+	if (phdr->p_type != PT_LOAD)
+		return false;
+
+	if ((phdr->p_flags & QCOM_MDT_TYPE_MASK) == QCOM_MDT_TYPE_HASH)
+		return false;
+
+	if (!phdr->p_memsz)
+		return false;
+
+	return true;
+}
+
 /**
- * qcom_mdt_parse() - extract useful parameters from the mdt header
- * @fw:		firmware handle
- * @fw_addr:	optional reference for base of the firmware's memory region
- * @fw_size:	optional reference for size of the firmware's memory region
- * @fw_relocate: optional reference for flagging if the firmware is relocatable
+ * qcom_mdt_get_size() - acquire size of the memory region needed to load mdt
+ * @fw:		firmware object for the mdt file
  *
- * Returns 0 on success, negative errno otherwise.
+ * Returns size of the loaded firmware blob, or -EINVAL on failure.
  */
-int qcom_mdt_parse(const struct firmware *fw, phys_addr_t *fw_addr,
-		   size_t *fw_size, bool *fw_relocate)
+ssize_t qcom_mdt_get_size(const struct firmware *fw)
 {
 	const struct elf32_phdr *phdrs;
 	const struct elf32_phdr *phdr;
 	const struct elf32_hdr *ehdr;
 	phys_addr_t min_addr = (phys_addr_t)ULLONG_MAX;
 	phys_addr_t max_addr = 0;
-	bool relocate = false;
 	int i;
 
 	ehdr = (struct elf32_hdr *)fw->data;
@@ -71,18 +81,9 @@ int qcom_mdt_parse(const struct firmware *fw, phys_addr_t *fw_addr,
 	for (i = 0; i < ehdr->e_phnum; i++) {
 		phdr = &phdrs[i];
 
-		if (phdr->p_type != PT_LOAD)
+		if (!mdt_phdr_valid(phdr))
 			continue;
 
-		if ((phdr->p_flags & QCOM_MDT_TYPE_MASK) == QCOM_MDT_TYPE_HASH)
-			continue;
-
-		if (!phdr->p_memsz)
-			continue;
-
-		if (phdr->p_flags & QCOM_MDT_RELOCATABLE)
-			relocate = true;
-
 		if (phdr->p_paddr < min_addr)
 			min_addr = phdr->p_paddr;
 
@@ -90,39 +91,44 @@ int qcom_mdt_parse(const struct firmware *fw, phys_addr_t *fw_addr,
 			max_addr = ALIGN(phdr->p_paddr + phdr->p_memsz, SZ_4K);
 	}
 
-	if (fw_addr)
-		*fw_addr = min_addr;
-	if (fw_size)
-		*fw_size = max_addr - min_addr;
-	if (fw_relocate)
-		*fw_relocate = relocate;
-
-	return 0;
+	return min_addr < max_addr ? max_addr - min_addr : -EINVAL;
 }
-EXPORT_SYMBOL_GPL(qcom_mdt_parse);
+EXPORT_SYMBOL_GPL(qcom_mdt_get_size);
 
 /**
- * qcom_mdt_load() - load the firmware which header is defined in fw
- * @rproc:	rproc handle
- * @fw:		frimware object for the header
- * @firmware:	filename of the firmware, for building .bXX names
+ * qcom_mdt_load() - load the firmware which header is loaded as fw
+ * @dev:	device handle to associate resources with
+ * @fw:		firmware object for the mdt file
+ * @firmware:	name of the firmware, for construction of segment file names
+ * @pas_id:	PAS identifier
+ * @mem_region:	allocated memory region to load firmware into
+ * @mem_phys:	physical address of allocated memory region
+ * @mem_size:	size of the allocated memory region
  *
  * Returns 0 on success, negative errno otherwise.
  */
-int qcom_mdt_load(struct rproc *rproc,
-		  const struct firmware *fw,
-		  const char *firmware)
+int qcom_mdt_load(struct device *dev, const struct firmware *fw,
+		  const char *firmware, int pas_id, void *mem_region,
+		  phys_addr_t mem_phys, size_t mem_size)
 {
 	const struct elf32_phdr *phdrs;
 	const struct elf32_phdr *phdr;
 	const struct elf32_hdr *ehdr;
 	const struct firmware *seg_fw;
+	phys_addr_t mem_reloc;
+	phys_addr_t min_addr = (phys_addr_t)ULLONG_MAX;
+	phys_addr_t max_addr = 0;
 	size_t fw_name_len;
+	size_t offset;
 	char *fw_name;
+	bool relocate = false;
 	void *ptr;
 	int ret;
 	int i;
 
+	if (!fw || !mem_region || !mem_phys || !mem_size)
+		return -EINVAL;
+
 	ehdr = (struct elf32_hdr *)fw->data;
 	phdrs = (struct elf32_phdr *)(ehdr + 1);
 
@@ -134,31 +140,68 @@ int qcom_mdt_load(struct rproc *rproc,
 	if (!fw_name)
 		return -ENOMEM;
 
+	ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size);
+	if (ret) {
+		dev_err(dev, "invalid firmware metadata\n");
+		goto out;
+	}
+
 	for (i = 0; i < ehdr->e_phnum; i++) {
 		phdr = &phdrs[i];
 
-		if (phdr->p_type != PT_LOAD)
+		if (!mdt_phdr_valid(phdr))
 			continue;
 
-		if ((phdr->p_flags & QCOM_MDT_TYPE_MASK) == QCOM_MDT_TYPE_HASH)
-			continue;
+		if (phdr->p_flags & QCOM_MDT_RELOCATABLE)
+			relocate = true;
+
+		if (phdr->p_paddr < min_addr)
+			min_addr = phdr->p_paddr;
+
+		if (phdr->p_paddr + phdr->p_memsz > max_addr)
+			max_addr = ALIGN(phdr->p_paddr + phdr->p_memsz, SZ_4K);
+	}
+
+	if (relocate) {
+		ret = qcom_scm_pas_mem_setup(pas_id, mem_phys, max_addr - min_addr);
+		if (ret) {
+			dev_err(dev, "unable to setup relocation\n");
+			goto out;
+		}
+
+		/*
+		 * The image is relocatable, so offset each segment based on
+		 * the lowest segment address.
+		 */
+		mem_reloc = min_addr;
+	} else {
+		/*
+		 * Image is not relocatable, so offset each segment based on
+		 * the allocated physical chunk of memory.
+		 */
+		mem_reloc = mem_phys;
+	}
 
-		if (!phdr->p_memsz)
+	for (i = 0; i < ehdr->e_phnum; i++) {
+		phdr = &phdrs[i];
+
+		if (!mdt_phdr_valid(phdr))
 			continue;
 
-		ptr = rproc_da_to_va(rproc, phdr->p_paddr, phdr->p_memsz);
-		if (!ptr) {
-			dev_err(&rproc->dev, "segment outside memory range\n");
+		offset = phdr->p_paddr - mem_reloc;
+		if (offset < 0 || offset + phdr->p_memsz > mem_size) {
+			dev_err(dev, "segment outside memory range\n");
 			ret = -EINVAL;
 			break;
 		}
 
+		ptr = mem_region + offset;
+
 		if (phdr->p_filesz) {
 			sprintf(fw_name + fw_name_len - 3, "b%02d", i);
-			ret = request_firmware(&seg_fw, fw_name, &rproc->dev);
+			ret = request_firmware(&seg_fw, fw_name, dev);
 			if (ret) {
-				dev_err(&rproc->dev, "failed to load %s\n",
-					fw_name);
+				dev_err(dev, "failed to load %s\n", fw_name);
 				break;
 			}
 
@@ -171,6 +214,7 @@ int qcom_mdt_load(struct rproc *rproc,
 			memset(ptr + phdr->p_filesz, 0, phdr->p_memsz - phdr->p_filesz);
 	}
 
+out:
 	kfree(fw_name);
 
 	return ret;
diff --git a/drivers/remoteproc/qcom_mdt_loader.h b/drivers/remoteproc/qcom_mdt_loader.h
index c5d7122755b6..261088c8da18 100644
--- a/drivers/remoteproc/qcom_mdt_loader.h
+++ b/drivers/remoteproc/qcom_mdt_loader.h
@@ -6,8 +6,10 @@
 #define QCOM_MDT_RELOCATABLE	BIT(27)
 
 struct resource_table * qcom_mdt_find_rsc_table(struct rproc *rproc, const struct firmware *fw, int *tablesz);
-int qcom_mdt_load(struct rproc *rproc, const struct firmware *fw, const char *fw_name);
 
-int qcom_mdt_parse(const struct firmware *fw, phys_addr_t *fw_addr, size_t *fw_size, bool *fw_relocate);
+ssize_t qcom_mdt_get_size(const struct firmware *fw);
+int qcom_mdt_load(struct device *dev, const struct firmware *fw,
+		  const char *fw_name, int pas_id, void *mem_region,
+		  phys_addr_t mem_phys, size_t mem_size);
 
 #endif
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index db341028f2d1..1afadab4edfd 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -496,6 +496,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 	const struct firmware *fw;
 	struct elf32_hdr *ehdr;
 	phys_addr_t mpss_reloc;
+	phys_addr_t boot_addr;
 	phys_addr_t min_addr = (phys_addr_t)ULLONG_MAX;
 	phys_addr_t max_addr = 0;
 	bool relocate = false;
@@ -576,7 +577,8 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 
 		size = readl(qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
 		if (!size) {
-			writel(mpss_reloc, qproc->rmb_base + RMB_PMI_CODE_START_REG);
+			boot_addr = relocate ? qproc->mpss_phys : min_addr;
+			writel(boot_addr, qproc->rmb_base + RMB_PMI_CODE_START_REG);
 			writel(RMB_CMD_LOAD_READY, qproc->rmb_base + RMB_MBA_COMMAND_REG);
 		}
 
diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index ebd61f5d18bb..aa97a61e6a69 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -152,34 +152,9 @@ void qcom_wcnss_assign_iris(struct qcom_wcnss *wcnss,
 static int wcnss_load(struct rproc *rproc, const struct firmware *fw)
 {
 	struct qcom_wcnss *wcnss = (struct qcom_wcnss *)rproc->priv;
-	phys_addr_t fw_addr;
-	size_t fw_size;
-	bool relocate;
-	int ret;
-
-	ret = qcom_scm_pas_init_image(WCNSS_PAS_ID, fw->data, fw->size);
-	if (ret) {
-		dev_err(&rproc->dev, "invalid firmware metadata\n");
-		return ret;
-	}
-
-	ret = qcom_mdt_parse(fw, &fw_addr, &fw_size, &relocate);
-	if (ret) {
-		dev_err(&rproc->dev, "failed to parse mdt header\n");
-		return ret;
-	}
-
-	if (relocate) {
-		wcnss->mem_reloc = fw_addr;
-
-		ret = qcom_scm_pas_mem_setup(WCNSS_PAS_ID, wcnss->mem_phys, fw_size);
-		if (ret) {
-			dev_err(&rproc->dev, "unable to setup memory for image\n");
-			return ret;
-		}
-	}
 
-	return qcom_mdt_load(rproc, fw, rproc->firmware);
+	return qcom_mdt_load(wcnss->dev, fw, rproc->firmware, WCNSS_PAS_ID,
+			     wcnss->mem_region, wcnss->mem_phys, wcnss->mem_size);
 }
 
 static const struct rproc_fw_ops wcnss_fw_ops = {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] remoteproc: qcom-common: Extract non-mdt related helper
  2017-01-30 16:55 [PATCH 1/5] remoteproc: qcom: q6v5: Decouple driver from MDT loader Bjorn Andersson
  2017-01-30 16:55 ` [PATCH 2/5] remoteproc: qcom: mdt_loader: Don't overwrite firmware object Bjorn Andersson
  2017-01-30 16:55 ` [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader Bjorn Andersson
@ 2017-01-30 16:55 ` Bjorn Andersson
  2017-01-30 16:55 ` [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc Bjorn Andersson
  3 siblings, 0 replies; 9+ messages in thread
From: Bjorn Andersson @ 2017-01-30 16:55 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, linux-arm-msm, Andy Gross

In preparation for moving the mdt loader out of remoteproc let's move
the somewhat unrelated resource table dummy helper to a Qualcomm
"common" file.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/remoteproc/Kconfig           |  6 +++++
 drivers/remoteproc/Makefile          |  1 +
 drivers/remoteproc/qcom_adsp_pil.c   |  1 +
 drivers/remoteproc/qcom_common.c     | 46 ++++++++++++++++++++++++++++++++++++
 drivers/remoteproc/qcom_common.h     | 11 +++++++++
 drivers/remoteproc/qcom_mdt_loader.c | 19 ---------------
 drivers/remoteproc/qcom_mdt_loader.h |  1 -
 drivers/remoteproc/qcom_q6v5_pil.c   |  1 +
 drivers/remoteproc/qcom_wcnss.c      |  1 +
 9 files changed, 67 insertions(+), 20 deletions(-)
 create mode 100644 drivers/remoteproc/qcom_common.c
 create mode 100644 drivers/remoteproc/qcom_common.h

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 454fd9a4dd96..71ea703190c6 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -78,11 +78,15 @@ config QCOM_ADSP_PIL
 	depends on QCOM_SMEM
 	select MFD_SYSCON
 	select QCOM_MDT_LOADER
+	select QCOM_RPROC_COMMON
 	select QCOM_SCM
 	help
 	  Say y here to support the TrustZone based Peripherial Image Loader
 	  for the Qualcomm ADSP remote processors.
 
+config QCOM_RPROC_COMMON
+	tristate
+
 config QCOM_MDT_LOADER
 	tristate
 
@@ -92,6 +96,7 @@ config QCOM_Q6V5_PIL
 	depends on QCOM_SMEM
 	depends on REMOTEPROC
 	select MFD_SYSCON
+	select QCOM_RPROC_COMMON
 	select QCOM_SCM
 	help
 	  Say y here to support the Qualcomm Peripherial Image Loader for the
@@ -104,6 +109,7 @@ config QCOM_WCNSS_PIL
 	depends on QCOM_SMEM
 	depends on REMOTEPROC
 	select QCOM_MDT_LOADER
+	select QCOM_RPROC_COMMON
 	select QCOM_SCM
 	help
 	  Say y here to support the Peripheral Image Loader for the Qualcomm
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 0938ea3c41ba..d4f9525a226d 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
 obj-$(CONFIG_QCOM_ADSP_PIL)		+= qcom_adsp_pil.o
 obj-$(CONFIG_QCOM_MDT_LOADER)		+= qcom_mdt_loader.o
+obj-$(CONFIG_QCOM_RPROC_COMMON)		+= qcom_common.o
 obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
 obj-$(CONFIG_QCOM_WCNSS_PIL)		+= qcom_wcnss_pil.o
 qcom_wcnss_pil-y			+= qcom_wcnss.o
diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
index e300b8c7e5aa..e2151b017616 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -29,6 +29,7 @@
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/smem_state.h>
 
+#include "qcom_common.h"
 #include "qcom_mdt_loader.h"
 #include "remoteproc_internal.h"
 
diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
new file mode 100644
index 000000000000..bd400336e209
--- /dev/null
+++ b/drivers/remoteproc/qcom_common.c
@@ -0,0 +1,46 @@
+/*
+ * Qualcomm Peripheral Image Loader helpers
+ *
+ * Copyright (C) 2016 Linaro Ltd
+ * Copyright (C) 2015 Sony Mobile Communications Inc
+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/firmware.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+#include "qcom_common.h"
+
+/**
+ * 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);
+
+MODULE_DESCRIPTION("Qualcomm Remoteproc helper driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
new file mode 100644
index 000000000000..caecf27c4ffa
--- /dev/null
+++ b/drivers/remoteproc/qcom_common.h
@@ -0,0 +1,11 @@
+#ifndef __RPROC_QCOM_COMMON_H__
+#define __RPROC_QCOM_COMMON_H__
+
+struct resource_table;
+struct rproc;
+
+struct resource_table *qcom_mdt_find_rsc_table(struct rproc *rproc,
+					       const struct firmware *fw,
+					       int *tablesz);
+
+#endif
diff --git a/drivers/remoteproc/qcom_mdt_loader.c b/drivers/remoteproc/qcom_mdt_loader.c
index f239f6fddbb7..b36a47bef056 100644
--- a/drivers/remoteproc/qcom_mdt_loader.c
+++ b/drivers/remoteproc/qcom_mdt_loader.c
@@ -27,25 +27,6 @@
 #include "remoteproc_internal.h"
 #include "qcom_mdt_loader.h"
 
-/**
- * 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 bool mdt_phdr_valid(const struct elf32_phdr *phdr)
 {
 	if (phdr->p_type != PT_LOAD)
diff --git a/drivers/remoteproc/qcom_mdt_loader.h b/drivers/remoteproc/qcom_mdt_loader.h
index 261088c8da18..ca9a91709810 100644
--- a/drivers/remoteproc/qcom_mdt_loader.h
+++ b/drivers/remoteproc/qcom_mdt_loader.h
@@ -5,7 +5,6 @@
 #define QCOM_MDT_TYPE_HASH	(2 << 24)
 #define QCOM_MDT_RELOCATABLE	BIT(27)
 
-struct resource_table * qcom_mdt_find_rsc_table(struct rproc *rproc, const struct firmware *fw, int *tablesz);
 
 ssize_t qcom_mdt_get_size(const struct firmware *fw);
 int qcom_mdt_load(struct device *dev, const struct firmware *fw,
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index 1afadab4edfd..d37f7d206b10 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -33,6 +33,7 @@
 #include <linux/soc/qcom/smem_state.h>
 
 #include "remoteproc_internal.h"
+#include "qcom_common.h"
 #include "qcom_mdt_loader.h"
 
 #include <linux/qcom_scm.h>
diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index aa97a61e6a69..fbb25ea4ae8a 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -32,6 +32,7 @@
 #include <linux/soc/qcom/smem_state.h>
 #include <linux/rpmsg/qcom_smd.h>
 
+#include "qcom_common.h"
 #include "qcom_mdt_loader.h"
 #include "remoteproc_internal.h"
 #include "qcom_wcnss.h"
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc
  2017-01-30 16:55 [PATCH 1/5] remoteproc: qcom: q6v5: Decouple driver from MDT loader Bjorn Andersson
                   ` (2 preceding siblings ...)
  2017-01-30 16:55 ` [PATCH 4/5] remoteproc: qcom-common: Extract non-mdt related helper Bjorn Andersson
@ 2017-01-30 16:55 ` Bjorn Andersson
  2017-02-01 23:24   ` Andy Gross
  3 siblings, 1 reply; 9+ messages in thread
From: Bjorn Andersson @ 2017-01-30 16:55 UTC (permalink / raw)
  To: Ohad Ben-Cohen, Bjorn Andersson
  Cc: linux-remoteproc, linux-kernel, linux-arm-msm, Andy Gross,
	Georgi Djakov, Jordan Crouse

With the remoteproc parts cleaned out of the MDT loader we can move it
to drivers/soc/qcom.

Cc: Andy Gross <andy.gross@linaro.com>
Cc: Georgi Djakov <georgi.djakov@linaro.org>
Cc: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

For testing purposes a temporary branch containing these patches can be found
here:

https://github.com/andersson/kernel/commits/linux-next/rproc-q6v5

 drivers/remoteproc/Kconfig                                            | 3 ---
 drivers/remoteproc/Makefile                                           | 1 -
 drivers/remoteproc/qcom_adsp_pil.c                                    | 2 +-
 drivers/remoteproc/qcom_q6v5_pil.c                                    | 2 +-
 drivers/remoteproc/qcom_wcnss.c                                       | 2 +-
 drivers/soc/qcom/Kconfig                                              | 4 ++++
 drivers/soc/qcom/Makefile                                             | 1 +
 drivers/{remoteproc/qcom_mdt_loader.c => soc/qcom/mdt_loader.c}       | 4 +---
 .../qcom_mdt_loader.h => include/linux/soc/qcom/mdt_loader.h          | 4 ++++
 9 files changed, 13 insertions(+), 10 deletions(-)
 rename drivers/{remoteproc/qcom_mdt_loader.c => soc/qcom/mdt_loader.c} (98%)
 rename drivers/remoteproc/qcom_mdt_loader.h => include/linux/soc/qcom/mdt_loader.h (87%)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 71ea703190c6..555dba04b5ae 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -87,9 +87,6 @@ config QCOM_ADSP_PIL
 config QCOM_RPROC_COMMON
 	tristate
 
-config QCOM_MDT_LOADER
-	tristate
-
 config QCOM_Q6V5_PIL
 	tristate "Qualcomm Hexagon V5 Peripherial Image Loader"
 	depends on OF && ARCH_QCOM
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index d4f9525a226d..ffc5e430df27 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -12,7 +12,6 @@ obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
 obj-$(CONFIG_QCOM_ADSP_PIL)		+= qcom_adsp_pil.o
-obj-$(CONFIG_QCOM_MDT_LOADER)		+= qcom_mdt_loader.o
 obj-$(CONFIG_QCOM_RPROC_COMMON)		+= qcom_common.o
 obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
 obj-$(CONFIG_QCOM_WCNSS_PIL)		+= qcom_wcnss_pil.o
diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
index e2151b017616..ce98818d3526 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -26,11 +26,11 @@
 #include <linux/qcom_scm.h>
 #include <linux/regulator/consumer.h>
 #include <linux/remoteproc.h>
+#include <linux/soc/qcom/mdt_loader.h>
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/smem_state.h>
 
 #include "qcom_common.h"
-#include "qcom_mdt_loader.h"
 #include "remoteproc_internal.h"
 
 #define ADSP_CRASH_REASON_SMEM		423
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index d37f7d206b10..a19e7ae5009c 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -29,12 +29,12 @@
 #include <linux/regulator/consumer.h>
 #include <linux/remoteproc.h>
 #include <linux/reset.h>
+#include <linux/soc/qcom/mdt_loader.h>
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/smem_state.h>
 
 #include "remoteproc_internal.h"
 #include "qcom_common.h"
-#include "qcom_mdt_loader.h"
 
 #include <linux/qcom_scm.h>
 
diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index fbb25ea4ae8a..781211c144c6 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -28,12 +28,12 @@
 #include <linux/qcom_scm.h>
 #include <linux/regulator/consumer.h>
 #include <linux/remoteproc.h>
+#include <linux/soc/qcom/mdt_loader.h>
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/smem_state.h>
 #include <linux/rpmsg/qcom_smd.h>
 
 #include "qcom_common.h"
-#include "qcom_mdt_loader.h"
 #include "remoteproc_internal.h"
 #include "qcom_wcnss.h"
 
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 461b387d03cc..78b1bb7bcf20 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -10,6 +10,10 @@ config QCOM_GSBI
           functions for connecting the underlying serial UART, SPI, and I2C
           devices to the output pins.
 
+config QCOM_MDT_LOADER
+	tristate
+	select QCOM_SCM
+
 config QCOM_PM
 	bool "Qualcomm Power Management"
 	depends on ARCH_QCOM && !ARM64
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index fdd664edf0bd..1f30260b06b8 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
+obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
 obj-$(CONFIG_QCOM_PM)	+=	spm.o
 obj-$(CONFIG_QCOM_SMD) +=	smd.o
 obj-$(CONFIG_QCOM_SMD_RPM)	+= smd-rpm.o
diff --git a/drivers/remoteproc/qcom_mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
similarity index 98%
rename from drivers/remoteproc/qcom_mdt_loader.c
rename to drivers/soc/qcom/mdt_loader.c
index b36a47bef056..15cb6e089c42 100644
--- a/drivers/remoteproc/qcom_mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -23,9 +23,7 @@
 #include <linux/remoteproc.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
-
-#include "remoteproc_internal.h"
-#include "qcom_mdt_loader.h"
+#include <linux/soc/qcom/mdt_loader.h>
 
 static bool mdt_phdr_valid(const struct elf32_phdr *phdr)
 {
diff --git a/drivers/remoteproc/qcom_mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
similarity index 87%
rename from drivers/remoteproc/qcom_mdt_loader.h
rename to include/linux/soc/qcom/mdt_loader.h
index ca9a91709810..f423001db3a9 100644
--- a/drivers/remoteproc/qcom_mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -1,10 +1,14 @@
 #ifndef __QCOM_MDT_LOADER_H__
 #define __QCOM_MDT_LOADER_H__
 
+#include <linux/types.h>
+
 #define QCOM_MDT_TYPE_MASK	(7 << 24)
 #define QCOM_MDT_TYPE_HASH	(2 << 24)
 #define QCOM_MDT_RELOCATABLE	BIT(27)
 
+struct device;
+struct firmware;
 
 ssize_t qcom_mdt_get_size(const struct firmware *fw);
 int qcom_mdt_load(struct device *dev, const struct firmware *fw,
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc
  2017-01-30 16:55 ` [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc Bjorn Andersson
@ 2017-02-01 23:24   ` Andy Gross
  0 siblings, 0 replies; 9+ messages in thread
From: Andy Gross @ 2017-02-01 23:24 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Ohad Ben-Cohen, linux-remoteproc, linux-kernel, linux-arm-msm,
	Andy Gross, Georgi Djakov, Jordan Crouse

On Mon, Jan 30, 2017 at 08:55:47AM -0800, Bjorn Andersson wrote:
> With the remoteproc parts cleaned out of the MDT loader we can move it
> to drivers/soc/qcom.

<snip>

> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 461b387d03cc..78b1bb7bcf20 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -10,6 +10,10 @@ config QCOM_GSBI
>            functions for connecting the underlying serial UART, SPI, and I2C
>            devices to the output pins.
>  
> +config QCOM_MDT_LOADER
> +	tristate
> +	select QCOM_SCM

Do we need a help description here?

> +
>  config QCOM_PM
>  	bool "Qualcomm Power Management"
>  	depends on ARCH_QCOM && !ARM64

Add the description on the next set and add my ack as well.

Acked-by: Andy Gross <andy.gross@linaro.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader
  2017-01-30 16:55 ` [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader Bjorn Andersson
@ 2017-02-02 15:21   ` Stanimir Varbanov
  2017-02-08 10:33   ` Stanimir Varbanov
  1 sibling, 0 replies; 9+ messages in thread
From: Stanimir Varbanov @ 2017-02-02 15:21 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen
  Cc: linux-remoteproc, linux-kernel, linux-arm-msm, Andy Gross

Hi Bjorn,

Thanks for the patch!

On 01/30/2017 06:55 PM, Bjorn Andersson wrote:
> Pushing the SCM calls into the MDT loader reduces duplication in the
> callers and allows for non-remoteproc clients to use the helper for
> parsing and loading MDT files.
> 
> Cc: Andy Gross <andy.gross@linaro.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  drivers/remoteproc/qcom_adsp_pil.c   |  29 +-------
>  drivers/remoteproc/qcom_mdt_loader.c | 134 +++++++++++++++++++++++------------
>  drivers/remoteproc/qcom_mdt_loader.h |   6 +-
>  drivers/remoteproc/qcom_q6v5_pil.c   |   4 +-
>  drivers/remoteproc/qcom_wcnss.c      |  29 +-------
>  5 files changed, 100 insertions(+), 102 deletions(-)
> 

<snip>

> diff --git a/drivers/remoteproc/qcom_mdt_loader.c b/drivers/remoteproc/qcom_mdt_loader.c
> index 2393398f63ea..f239f6fddbb7 100644
> --- a/drivers/remoteproc/qcom_mdt_loader.c
> +++ b/drivers/remoteproc/qcom_mdt_loader.c
> @@ -19,6 +19,7 @@
>  #include <linux/firmware.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/qcom_scm.h>
>  #include <linux/remoteproc.h>

Please delete remoteproc.h it is not needed anymore.

>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> @@ -45,24 +46,33 @@ struct resource_table *qcom_mdt_find_rsc_table(struct rproc *rproc,
>  }
>  EXPORT_SYMBOL_GPL(qcom_mdt_find_rsc_table);
>  
> +static bool mdt_phdr_valid(const struct elf32_phdr *phdr)
> +{
> +	if (phdr->p_type != PT_LOAD)
> +		return false;
> +
> +	if ((phdr->p_flags & QCOM_MDT_TYPE_MASK) == QCOM_MDT_TYPE_HASH)
> +		return false;
> +
> +	if (!phdr->p_memsz)
> +		return false;
> +
> +	return true;
> +}
> +

<snip>

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader
  2017-01-30 16:55 ` [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader Bjorn Andersson
  2017-02-02 15:21   ` Stanimir Varbanov
@ 2017-02-08 10:33   ` Stanimir Varbanov
  2017-02-16  6:01     ` Bjorn Andersson
  1 sibling, 1 reply; 9+ messages in thread
From: Stanimir Varbanov @ 2017-02-08 10:33 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen
  Cc: linux-remoteproc, linux-kernel, linux-arm-msm, Andy Gross

Hi Bjorn,

On 01/30/2017 06:55 PM, Bjorn Andersson wrote:
> Pushing the SCM calls into the MDT loader reduces duplication in the
> callers and allows for non-remoteproc clients to use the helper for
> parsing and loading MDT files.
> 
> Cc: Andy Gross <andy.gross@linaro.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  drivers/remoteproc/qcom_adsp_pil.c   |  29 +-------
>  drivers/remoteproc/qcom_mdt_loader.c | 134 +++++++++++++++++++++++------------
>  drivers/remoteproc/qcom_mdt_loader.h |   6 +-
>  drivers/remoteproc/qcom_q6v5_pil.c   |   4 +-
>  drivers/remoteproc/qcom_wcnss.c      |  29 +-------
>  5 files changed, 100 insertions(+), 102 deletions(-)
> 

>  /**
> - * qcom_mdt_load() - load the firmware which header is defined in fw
> - * @rproc:	rproc handle
> - * @fw:		frimware object for the header
> - * @firmware:	filename of the firmware, for building .bXX names
> + * qcom_mdt_load() - load the firmware which header is loaded as fw
> + * @dev:	device handle to associate resources with
> + * @fw:		firmware object for the mdt file
> + * @firmware:	name of the firmware, for construction of segment file names
> + * @pas_id:	PAS identifier
> + * @mem_region:	allocated memory region to load firmware into
> + * @mem_phys:	physical address of allocated memory region
> + * @mem_size:	size of the allocated memory region
>   *
>   * Returns 0 on success, negative errno otherwise.
>   */
> -int qcom_mdt_load(struct rproc *rproc,
> -		  const struct firmware *fw,
> -		  const char *firmware)
> +int qcom_mdt_load(struct device *dev, const struct firmware *fw,
> +		  const char *firmware, int pas_id, void *mem_region,
> +		  phys_addr_t mem_phys, size_t mem_size)
>  {
>  	const struct elf32_phdr *phdrs;
>  	const struct elf32_phdr *phdr;
>  	const struct elf32_hdr *ehdr;
>  	const struct firmware *seg_fw;
> +	phys_addr_t mem_reloc;
> +	phys_addr_t min_addr = (phys_addr_t)ULLONG_MAX;
> +	phys_addr_t max_addr = 0;
>  	size_t fw_name_len;
> +	size_t offset;
>  	char *fw_name;
> +	bool relocate = false;
>  	void *ptr;
>  	int ret;
>  	int i;
>  
> +	if (!fw || !mem_region || !mem_phys || !mem_size)
> +		return -EINVAL;
> +
>  	ehdr = (struct elf32_hdr *)fw->data;
>  	phdrs = (struct elf32_phdr *)(ehdr + 1);
>  
> @@ -134,31 +140,68 @@ int qcom_mdt_load(struct rproc *rproc,
>  	if (!fw_name)
>  		return -ENOMEM;
>  
> +	ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size);
> +	if (ret) {
> +		dev_err(dev, "invalid firmware metadata\n");
> +		goto out;
> +	}
> +
>  	for (i = 0; i < ehdr->e_phnum; i++) {
>  		phdr = &phdrs[i];
>  
> -		if (phdr->p_type != PT_LOAD)
> +		if (!mdt_phdr_valid(phdr))
>  			continue;
>  
> -		if ((phdr->p_flags & QCOM_MDT_TYPE_MASK) == QCOM_MDT_TYPE_HASH)
> -			continue;
> +		if (phdr->p_flags & QCOM_MDT_RELOCATABLE)
> +			relocate = true;
> +
> +		if (phdr->p_paddr < min_addr)
> +			min_addr = phdr->p_paddr;
> +
> +		if (phdr->p_paddr + phdr->p_memsz > max_addr)
> +			max_addr = ALIGN(phdr->p_paddr + phdr->p_memsz, SZ_4K);
> +	}
> +
> +	if (relocate) {
> +		ret = qcom_scm_pas_mem_setup(pas_id, mem_phys, max_addr - min_addr);
> +		if (ret) {
> +			dev_err(dev, "unable to setup relocation\n");
> +			goto out;
> +		}
> +
> +		/*
> +		 * The image is relocatable, so offset each segment based on
> +		 * the lowest segment address.
> +		 */
> +		mem_reloc = min_addr;
> +	} else {
> +		/*
> +		 * Image is not relocatable, so offset each segment based on
> +		 * the allocated physical chunk of memory.
> +		 */
> +		mem_reloc = mem_phys;
> +	}
>  
> -		if (!phdr->p_memsz)
> +	for (i = 0; i < ehdr->e_phnum; i++) {
> +		phdr = &phdrs[i];
> +
> +		if (!mdt_phdr_valid(phdr))
>  			continue;
>  
> -		ptr = rproc_da_to_va(rproc, phdr->p_paddr, phdr->p_memsz);
> -		if (!ptr) {
> -			dev_err(&rproc->dev, "segment outside memory range\n");
> +		offset = phdr->p_paddr - mem_reloc;

Shouldn't 'offset' variable be of signed type i.e. ssize_t? Also p_paddr
is of type Elf32_Addr and mem_reloc is of type phys_addr_t which on
64bit systems is 64bit long, I think it should be better to make
mem_reloc of the same type as p_paddr.

> +		if (offset < 0 || offset + phdr->p_memsz > mem_size) {
> +			dev_err(dev, "segment outside memory range\n");
>  			ret = -EINVAL;
>  			break;
>  		}
>  
> +		ptr = mem_region + offset;
> +
>  		if (phdr->p_filesz) {
>  			sprintf(fw_name + fw_name_len - 3, "b%02d", i);
> -			ret = request_firmware(&seg_fw, fw_name, &rproc->dev);
> +			ret = request_firmware(&seg_fw, fw_name, dev);
>  			if (ret) {
> -				dev_err(&rproc->dev, "failed to load %s\n",
> -					fw_name);
> +				dev_err(dev, "failed to load %s\n", fw_name);
>  				break;
>  			}
>  


-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader
  2017-02-08 10:33   ` Stanimir Varbanov
@ 2017-02-16  6:01     ` Bjorn Andersson
  0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Andersson @ 2017-02-16  6:01 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Ohad Ben-Cohen, linux-remoteproc, linux-kernel, linux-arm-msm,
	Andy Gross

On Wed 08 Feb 02:33 PST 2017, Stanimir Varbanov wrote:

> Hi Bjorn,
> 
> On 01/30/2017 06:55 PM, Bjorn Andersson wrote:
[..]
> > +int qcom_mdt_load(struct device *dev, const struct firmware *fw,
[..]
> > +	size_t offset;
[..]
> > +	for (i = 0; i < ehdr->e_phnum; i++) {
> > +		phdr = &phdrs[i];
> > +
> > +		if (!mdt_phdr_valid(phdr))
> >  			continue;
> >  
> > -		ptr = rproc_da_to_va(rproc, phdr->p_paddr, phdr->p_memsz);
> > -		if (!ptr) {
> > -			dev_err(&rproc->dev, "segment outside memory range\n");
> > +		offset = phdr->p_paddr - mem_reloc;
> 
> Shouldn't 'offset' variable be of signed type i.e. ssize_t?

It should, as a small "negative" value will wrap back into the
acceptable range of the second part of the check below. I got another
report of this as well yesterday, so I will prepare a patch for this.

> Also p_paddr is of type Elf32_Addr and mem_reloc is of type
> phys_addr_t which on 64bit systems is 64bit long, I think it should be
> better to make mem_reloc of the same type as p_paddr.
> 

If I remember what the C standard says, mem_reloc would in the 64-bit
case have higher "rank" and the type of p_paddr would therefor be
converted to the type of mem_reloc (i.e.  u64) and the result would be
stored in a 64 bit type. But it's been a while and I can't find my C
reference manual right now...

Casting mem_reloc to the type of p_paddr would cause invalid results in
the event that the system is configured to actually load a relocatable
blob into memory above 4GB.

> > +		if (offset < 0 || offset + phdr->p_memsz > mem_size) {
> > +			dev_err(dev, "segment outside memory range\n");
> >  			ret = -EINVAL;
> >  			break;
> >  		}
> >  
> > +		ptr = mem_region + offset;
> > +
> >  		if (phdr->p_filesz) {
> >  			sprintf(fw_name + fw_name_len - 3, "b%02d", i);
> > -			ret = request_firmware(&seg_fw, fw_name, &rproc->dev);
> > +			ret = request_firmware(&seg_fw, fw_name, dev);
> >  			if (ret) {
> > -				dev_err(&rproc->dev, "failed to load %s\n",
> > -					fw_name);
> > +				dev_err(dev, "failed to load %s\n", fw_name);
> >  				break;
> >  			}
> >  
> 

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-02-16  6:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-30 16:55 [PATCH 1/5] remoteproc: qcom: q6v5: Decouple driver from MDT loader Bjorn Andersson
2017-01-30 16:55 ` [PATCH 2/5] remoteproc: qcom: mdt_loader: Don't overwrite firmware object Bjorn Andersson
2017-01-30 16:55 ` [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader Bjorn Andersson
2017-02-02 15:21   ` Stanimir Varbanov
2017-02-08 10:33   ` Stanimir Varbanov
2017-02-16  6:01     ` Bjorn Andersson
2017-01-30 16:55 ` [PATCH 4/5] remoteproc: qcom-common: Extract non-mdt related helper Bjorn Andersson
2017-01-30 16:55 ` [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc Bjorn Andersson
2017-02-01 23:24   ` Andy Gross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).