Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1570216 > unrolled thread

[PATCH 1/5] remoteproc: qcom: q6v5: Decouple driver from MDT loader

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/5] remoteproc: qcom: q6v5: Decouple driver from MDT loader Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-01-31 02:00 +0100
    [PATCH 4/5] remoteproc: qcom-common: Extract non-mdt related helper Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-01-31 02:00 +0100
    [PATCH 2/5] remoteproc: qcom: mdt_loader: Don't overwrite firmware object Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-01-31 02:00 +0100
    [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-01-31 02:00 +0100
      Re: [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc Andy Gross <andy.gross@linaro.org> - 2017-02-02 00:30 +0100
    [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-01-31 02:00 +0100
      Re: [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-02-02 16:30 +0100
      Re: [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-02-08 11:40 +0100

#1570216 — [PATCH 1/5] remoteproc: qcom: q6v5: Decouple driver from MDT loader

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

[toc] | [next] | [standalone]

#1570217 — [PATCH 4/5] remoteproc: qcom-common: Extract non-mdt related helper

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

[toc] | [prev] | [next] | [standalone]

#1570218 — [PATCH 2/5] remoteproc: qcom: mdt_loader: Don't overwrite firmware object

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

[toc] | [prev] | [next] | [standalone]

#1570219 — [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc

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

[toc] | [prev] | [next] | [standalone]

#1572052 — Re: [PATCH 5/5] soc/qcom: Move qcom_mdt_loader from remoteproc

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>

[toc] | [prev] | [next] | [standalone]

#1570221 — [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader

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

[toc] | [prev] | [next] | [standalone]

#1572463 — Re: [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader

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

[toc] | [prev] | [next] | [standalone]

#1576427 — Re: [PATCH 3/5] remoteproc: qcom: mdt_loader: Refactor MDT loader

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web