Skip to contentSkip to conversation

Add memory ownership switch support and enable mss rproc on msm8996

PATCH Superseded Avaneesh Kumar Dwivedi 3 participants 14 messages Jul 21, 2017 – Oct 19, 2017

Subsystems (3)

v7 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v7 cover Add memory ownership switch support and enable mss rproc on msm8996

This patch set does following
1- Adds new scm call which helps in stage two translation of a memory region
so that memory ownership sharing and switching can be achieved on armv8 and later.
2- Enable mss remoteproc on msm8996

Changes w.r.t. last patchset
1- Uses dma_alloc_coherent() that dma_alloc_attrs() 2- Fixes warning from checkpatch with --strict option 3- Fixes other minor comments

Avaneesh Kumar Dwivedi (4):
firmware: scm: Add new SCM call API for switching memory ownership
remoteproc: qcom: refactor mss fw image loading sequence
remoteproc: qcom: Make secure world call for mem ownership switch
remoteproc: qcom: Add support for mss remoteproc on msm8996

 .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |   1 +
 drivers/firmware/qcom_scm-32.c                     |   7 +
 drivers/firmware/qcom_scm-64.c                     |  27 ++
 drivers/firmware/qcom_scm.c                        |  94 +++++++
 drivers/firmware/qcom_scm.h                        |   5 +
 drivers/remoteproc/qcom_q6v5_pil.c                 | 273 ++++++++++++++++++---
 include/linux/qcom_scm.h                           |  16 ++
 7 files changed, 385 insertions(+), 38 deletions(-)
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#1 lore raw

PATCH v7 1/4 firmware: scm: Add new SCM call API for switching memory ownership

Two different processors on a SOC need to switch memory ownership during load/unload. To enable this, second level memory map table need to be updated, which is done by secure layer.
This patch adds the interface for making secure monitor call for memory ownership switching request.

---
 drivers/firmware/qcom_scm-32.c |  7 ++++
 drivers/firmware/qcom_scm-64.c | 27 ++++++++++++
 drivers/firmware/qcom_scm.c    | 94 ++++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h    |  5 +++
 include/linux/qcom_scm.h       | 16 +++++++
 5 files changed, 149 insertions(+)
drivers/firmware/qcom_scm-32.c +7 -0
index 93e3b96..758b910 100644
@@ -596,3 +596,10 @@ int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size,
 {
 	return -ENODEV;
 }
+
+int __qcom_scm_assign_mem(struct device *dev, phys_addr_t mem_region,
+			  size_t mem_sz, phys_addr_t src, size_t src_sz,
+			  phys_addr_t dest, size_t dest_sz)
+{
+	return -ENODEV;
+}
drivers/firmware/qcom_scm-64.c +27 -0
index 6e6d561..6bdbeae 100644
@@ -439,3 +439,30 @@ int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size,
 
 	return ret;
 }
+
+int __qcom_scm_assign_mem(struct device *dev, phys_addr_t mem_region,
+			  size_t mem_sz, phys_addr_t src, size_t src_sz,
+			  phys_addr_t dest, size_t dest_sz)
+{
+	int ret;
+	struct qcom_scm_desc desc = {0};
+	struct arm_smccc_res res;
+
+	desc.args[0] = mem_region;
+	desc.args[1] = mem_sz;
+	desc.args[2] = src;
+	desc.args[3] = src_sz;
+	desc.args[4] = dest;
+	desc.args[5] = dest_sz;
+	desc.args[6] = 0;
+
+	desc.arginfo = QCOM_SCM_ARGS(7, QCOM_SCM_RO, QCOM_SCM_VAL,
+				     QCOM_SCM_RO, QCOM_SCM_VAL, QCOM_SCM_RO,
+				     QCOM_SCM_VAL, QCOM_SCM_VAL);
+
+	ret = qcom_scm_call(dev, QCOM_SCM_SVC_MP,
+			    QCOM_MEM_PROT_ASSIGN_ID,
+			    &desc, &res);
+
+	return ret ? : res.a1;
+}
drivers/firmware/qcom_scm.c +94 -0

106 lines not on this page · raw message

drivers/firmware/qcom_scm.h +5 -0
index 9bea691..fe54b7b 100644
@@ -95,5 +95,10 @@ extern int __qcom_scm_iommu_secure_ptbl_size(struct device *dev, u32 spare,
 					     size_t *size);
 extern int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr,
 					     u32 size, u32 spare);
+#define QCOM_MEM_PROT_ASSIGN_ID	0x16
+extern int  __qcom_scm_assign_mem(struct device *dev,
+				  phys_addr_t mem_region, size_t mem_sz,
+				  phys_addr_t src, size_t src_sz,
+				  phys_addr_t dest, size_t dest_sz);
 
 #endif
include/linux/qcom_scm.h +16 -0
index e538047..a9c284d 100644
@@ -23,6 +23,19 @@ struct qcom_scm_hdcp_req {
 	u32 val;
 };
 
+struct qcom_scm_vmperm {
+	int vmid;
+	int perm;
+};
+
+#define QCOM_SCM_VMID_HLOS       0x3
+#define QCOM_SCM_VMID_MSS_MSA    0xF
+#define QCOM_SCM_PERM_READ       0x4
+#define QCOM_SCM_PERM_WRITE      0x2
+#define QCOM_SCM_PERM_EXEC       0x1
+#define QCOM_SCM_PERM_RW (QCOM_SCM_PERM_READ | QCOM_SCM_PERM_WRITE)
+#define QCOM_SCM_PERM_RWX (QCOM_SCM_PERM_RW | QCOM_SCM_PERM_EXEC)
+
 #if IS_ENABLED(CONFIG_QCOM_SCM)
 extern int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus);
 extern int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus);
@@ -37,6 +50,9 @@ extern int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr,
 				  phys_addr_t size);
 extern int qcom_scm_pas_auth_and_reset(u32 peripheral);
 extern int qcom_scm_pas_shutdown(u32 peripheral);
+extern int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
+			       int src, struct qcom_scm_vmperm *newvm,
+			       int dest_cnt);
 extern void qcom_scm_cpu_power_down(u32 flags);
 extern u32 qcom_scm_get_version(void);
 extern int qcom_scm_set_remote_state(u32 state, u32 id);
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#2 lore raw

Re: [PATCH v7 1/4] firmware: scm: Add new SCM call API for switching memory ownership

Quoted 6 lines · Avaneesh Kumar Dwivedi

Two different processors on a SOC need to switch memory ownership during load/unload. To enable this, second level memory map table need to be updated, which is done by secure layer.
This patch adds the interface for making secure monitor call for memory ownership switching request.

As I reported to you a while back I finally managed to use this to get the modem on db820c up and running (with "all" IPCROUTER services showing up). So let's try to resurrect and get this merged!

In addition I've successfully used this patch in extending the rmtfs shared memory driver to allow setting up the ownership of that memory.

[..]

Quoted 5 lines
drivers/firmware/qcom_scm.c +0 -0
index bb16510..009a42d 100644

drivers/firmware/qcom_scm.c struct qcom_scm { · line 48

Quoted 9 lines
drivers/firmware/qcom_scm.c +6 -0
@@ -40,6 +40,18 @@ struct qcom_scm {
 	struct reset_controller_dev reset;
 };
 
+struct qcom_scm_current_perm_info {
+	__le32 vmid;
+	__le32 perm;
+	__le64 ctx;
+	__le32 ctx_size;
+};

I learned the hard way that this struct is supposed to be 24 bytes, so please add a __le32 padding; at the end of this.

[..]

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 321

Quoted 3 lines
drivers/firmware/qcom_scm.c +3 -0
+int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, int srcvm,
+			struct qcom_scm_vmperm *newvm, int dest_cnt)
+{

It turns out that the standard way of calling this (shown by the remoteproc and rmtfs-memory driver implementations) is:

	ret = qcom_scm_assign_mem(mem, len, curr_vm, perms, sizeof(perms));
	if (ret < 0)
		fail();
	curr_vm = ret;

I therefor suggest that we make one more adjustment to the prototype in the form of taking srcvm as a pointer. And as this is now a bitmask it should be made an unsigned int. I.e.

int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, unsigned int srcvm,
struct qcom_scm_vmperm *newvm, int dest_cnt)

Quoted 10 lines
drivers/firmware/qcom_scm.c +10 -0
+	struct qcom_scm_current_perm_info *destvm;
+	struct qcom_scm_mem_map_info *mem;
+	phys_addr_t memory_phys;
+	phys_addr_t dest_phys;
+	phys_addr_t src_phys;
+	size_t mem_all_sz;
+	size_t memory_sz;
+	size_t dest_sz;
+	size_t src_sz;
+	int next_vm;

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 341

Quoted 10 lines
drivers/firmware/qcom_scm.c +10 -0
+	__le32 *src;
+	void *ptr;
+	int ret;
+	int len;
+	int i;
+
+	src_sz = hweight_long(srcvm) * sizeof(*src);
+	memory_sz = sizeof(*mem);
+	dest_sz = dest_cnt * sizeof(*destvm);
+	mem_all_sz = src_sz + memory_sz + dest_sz;

ALIGN(x + y + z, 64) <= ALIGN(x, 64) + ALIGN(y, 64) + ALIGN(z, 64)

So please replace this with:

	ptr_sz = ALIGN(src_sz, SZ_64) + ALIGN(memory_sz, SZ_64) +
		 ALIGN(dest_sz, SZ_64);

(renaming the variable to ptr_sz saves you some line wraps as well)

Quoted 15 lines
drivers/firmware/qcom_scm.c +15 -0
+
+	ptr = dma_alloc_coherent(__scm->dev, ALIGN(mem_all_sz, SZ_64),
+				 &src_phys, GFP_KERNEL);
+
+	if (!ptr)
+		return -ENOMEM;
+
+	/* Fill source vmid detail */
+	src = ptr;
+	len = hweight_long(srcvm);
+	for (i = 0; i < len; i++) {
+		src[i] = cpu_to_le32(ffs(srcvm) - 1);
+		srcvm ^= 1 << (ffs(srcvm) - 1);
+	}
+

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 366

Quoted 10 lines
drivers/firmware/qcom_scm.c +10 -0
+	/* Fill details of mem buff to map */
+	mem = ptr + ALIGN(src_sz, SZ_64);
+	memory_phys = src_phys + ALIGN(src_sz, SZ_64);
+	mem[0].mem_addr = cpu_to_le64(mem_addr);
+	mem[0].mem_size = cpu_to_le64(mem_sz);
+
+	next_vm = 0;
+	/* Fill details of next vmid detail */
+	destvm = ptr + ALIGN(memory_sz, SZ_64) + ALIGN(src_sz, SZ_64);
+	dest_phys = memory_phys + ALIGN(memory_sz, SZ_64);

For clarity it would be nice if you keep the math for virtual and physical addresses the same; so add another variable "ptr_phys" and replace this with:

	dest_phys = ptr_phys + ALIGN(src_sz, 64) + ALIGN(memory_sz, 64);
Quoted 9 lines
drivers/firmware/qcom_scm.c +9 -0
+	for (i = 0; i < dest_cnt; i++) {
+		destvm[i].vmid = cpu_to_le32(newvm[i].vmid);
+		destvm[i].perm = cpu_to_le32(newvm[i].perm);
+		destvm[i].ctx = 0;
+		destvm[i].ctx_size = 0;
+		next_vm |= BIT(newvm[i].vmid);
+	}
+
+	ret = __qcom_scm_assign_mem(__scm->dev, memory_phys, memory_sz,

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 385

Quoted 10 lines
drivers/firmware/qcom_scm.c +10 -0
+				    src_phys, src_sz, dest_phys, dest_sz);
+	dma_free_coherent(__scm->dev, ALIGN(mem_all_sz, SZ_64),
+			  ptr, src_phys);
+	if (ret != 0) {
+		dev_err(__scm->dev,
+			"Assign memory protection call failed %d.\n", ret);
+		return -EINVAL;
+	} else {
+		return next_vm;
+	}

Replace this with:

	if (ret) {
		dev_err(__scm->dev,
			"Assign memory protection call failed %d.\n", ret);
		return -EINVAL;
	}

	*srcvm = next_vm;
	return 0;

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 388

Quoted 3 lines
drivers/firmware/qcom_scm.c +3 -0
+}
+EXPORT_SYMBOL(qcom_scm_assign_mem);
+

Regards,
Bjorn

#3 lore raw

Re: [PATCH v7 1/4] firmware: scm: Add new SCM call API for switching memory ownership

Quoted 13 lines · Bjorn Andersson
Quoted 6 lines · Avaneesh Kumar Dwivedi

Two different processors on a SOC need to switch memory ownership during load/unload. To enable this, second level memory map table need to be updated, which is done by secure layer.
This patch adds the interface for making secure monitor call for memory ownership switching request.

As I reported to you a while back I finally managed to use this to get the modem on db820c up and running (with "all" IPCROUTER services showing up). So let's try to resurrect and get this merged!

Quoted 4 lines

In addition I've successfully used this patch in extending the rmtfs shared memory driver to allow setting up the ownership of that memory.

[..]

Thanks for reviving this patch series Bjorn!

Will try to take it closure this time.

Quoted 17 lines
Quoted 5 lines
drivers/firmware/qcom_scm.c +0 -0
index bb16510..009a42d 100644

drivers/firmware/qcom_scm.c struct qcom_scm { · line 48

Quoted 9 lines
drivers/firmware/qcom_scm.c +6 -0
@@ -40,6 +40,18 @@ struct qcom_scm {
  	struct reset_controller_dev reset;
  };
 
+struct qcom_scm_current_perm_info {
+	__le32 vmid;
+	__le32 perm;
+	__le64 ctx;
+	__le32 ctx_size;
+};

I learned the hard way that this struct is supposed to be 24 bytes, so please add a __le32 padding; at the end of this.

Quoted 1 line

[..]

OK.

Quoted 40 lines

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 321

Quoted 3 lines
drivers/firmware/qcom_scm.c +3 -0
+int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, int srcvm,
+			struct qcom_scm_vmperm *newvm, int dest_cnt)
+{

It turns out that the standard way of calling this (shown by the remoteproc and rmtfs-memory driver implementations) is:

	ret = qcom_scm_assign_mem(mem, len, curr_vm, perms, sizeof(perms));
	if (ret < 0)
		fail();
	curr_vm = ret;

I therefor suggest that we make one more adjustment to the prototype in the form of taking srcvm as a pointer. And as this is now a bitmask it should be made an unsigned int. I.e.

int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, unsigned int srcvm,
struct qcom_scm_vmperm *newvm, int dest_cnt)

Quoted 10 lines
drivers/firmware/qcom_scm.c +10 -0
+	struct qcom_scm_current_perm_info *destvm;
+	struct qcom_scm_mem_map_info *mem;
+	phys_addr_t memory_phys;
+	phys_addr_t dest_phys;
+	phys_addr_t src_phys;
+	size_t mem_all_sz;
+	size_t memory_sz;
+	size_t dest_sz;
+	size_t src_sz;
+	int next_vm;

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 341

Quoted 10 lines
drivers/firmware/qcom_scm.c +10 -0
+	__le32 *src;
+	void *ptr;
+	int ret;
+	int len;
+	int i;
+
+	src_sz = hweight_long(srcvm) * sizeof(*src);
+	memory_sz = sizeof(*mem);
+	dest_sz = dest_cnt * sizeof(*destvm);
+	mem_all_sz = src_sz + memory_sz + dest_sz;

ALIGN(x + y + z, 64) <= ALIGN(x, 64) + ALIGN(y, 64) + ALIGN(z, 64)

Quoted 6 lines

So please replace this with:

	ptr_sz = ALIGN(src_sz, SZ_64) + ALIGN(memory_sz, SZ_64) +
		 ALIGN(dest_sz, SZ_64);

(renaming the variable to ptr_sz saves you some line wraps as well)

Ok, Will try to incorporate your idea, and if any concern will revert back soon.

Quoted 28 lines
Quoted 15 lines
drivers/firmware/qcom_scm.c +15 -0
+
+	ptr = dma_alloc_coherent(__scm->dev, ALIGN(mem_all_sz, SZ_64),
+				 &src_phys, GFP_KERNEL);
+
+	if (!ptr)
+		return -ENOMEM;
+
+	/* Fill source vmid detail */
+	src = ptr;
+	len = hweight_long(srcvm);
+	for (i = 0; i < len; i++) {
+		src[i] = cpu_to_le32(ffs(srcvm) - 1);
+		srcvm ^= 1 << (ffs(srcvm) - 1);
+	}
+

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 366

Quoted 10 lines
drivers/firmware/qcom_scm.c +10 -0
+	/* Fill details of mem buff to map */
+	mem = ptr + ALIGN(src_sz, SZ_64);
+	memory_phys = src_phys + ALIGN(src_sz, SZ_64);
+	mem[0].mem_addr = cpu_to_le64(mem_addr);
+	mem[0].mem_size = cpu_to_le64(mem_sz);
+
+	next_vm = 0;
+	/* Fill details of next vmid detail */
+	destvm = ptr + ALIGN(memory_sz, SZ_64) + ALIGN(src_sz, SZ_64);
+	dest_phys = memory_phys + ALIGN(memory_sz, SZ_64);

For clarity it would be nice if you keep the math for virtual and physical addresses the same; so add another variable "ptr_phys" and replace this with:

Quoted 1 line
	dest_phys = ptr_phys + ALIGN(src_sz, 64) + ALIGN(memory_sz, 64);

OK.

Quoted 22 lines
Quoted 10 lines
drivers/firmware/qcom_scm.c +9 -0
+	for (i = 0; i < dest_cnt; i++) {
+		destvm[i].vmid = cpu_to_le32(newvm[i].vmid);
+		destvm[i].perm = cpu_to_le32(newvm[i].perm);
+		destvm[i].ctx = 0;
+		destvm[i].ctx_size = 0;
+		next_vm |= BIT(newvm[i].vmid);
+	}
+
+	ret = __qcom_scm_assign_mem(__scm->dev, memory_phys, memory_sz,

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 385

Quoted 10 lines
drivers/firmware/qcom_scm.c +10 -0
+				    src_phys, src_sz, dest_phys, dest_sz);
+	dma_free_coherent(__scm->dev, ALIGN(mem_all_sz, SZ_64),
+			  ptr, src_phys);
+	if (ret != 0) {
+		dev_err(__scm->dev,
+			"Assign memory protection call failed %d.\n", ret);
+		return -EINVAL;
+	} else {
+		return next_vm;
+	}

Replace this with:

Quoted 8 lines
	if (ret) {
		dev_err(__scm->dev,
			"Assign memory protection call failed %d.\n", ret);
		return -EINVAL;
	}

	*srcvm = next_vm;
	return 0;

OK.

Quoted 10 lines

drivers/firmware/qcom_scm.c int qcom_scm_pas_shutdown(u32 peripheral) · line 388

Quoted 4 lines
drivers/firmware/qcom_scm.c +3 -0
+}
+EXPORT_SYMBOL(qcom_scm_assign_mem);
+

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#4 lore raw

PATCH v7 2/4 remoteproc: qcom: refactor mss fw image loading sequence

In reply to Avaneesh Kumar Dwivedi

This patch refactor code to first load all firmware blobs and then update modem proc to authenticate and boot fw.

---
 drivers/remoteproc/qcom_q6v5_pil.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)
drivers/remoteproc/qcom_q6v5_pil.c +8 -11
index 8fd697a..f5f8850 100644
@@ -503,7 +503,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 	bool relocate = false;
 	char seg_name[10];
 	ssize_t offset;
-	size_t size;
+	size_t size = 0;
 	void *ptr;
 	int ret;
 	int i;
@@ -541,7 +541,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 	}
 
 	mpss_reloc = relocate ? min_addr : qproc->mpss_phys;
-
+	/* Load firmware segments */
 	for (i = 0; i < ehdr->e_phnum; i++) {
 		phdr = &phdrs[i];
 
@@ -574,18 +574,15 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 			memset(ptr + phdr->p_filesz, 0,
 			       phdr->p_memsz - phdr->p_filesz);
 		}
-
-		size = readl(qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
-		if (!size) {
-			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);
-		}
-
 		size += phdr->p_memsz;
-		writel(size, qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
 	}
 
+	/* Transfer ownership of modem ddr region with q6*/
+	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);
+	writel(size, qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
+
 	ret = q6v5_rmb_mba_wait(qproc, RMB_MBA_AUTH_COMPLETE, 10000);
 	if (ret == -ETIMEDOUT)
 		dev_err(qproc->dev, "MPSS authentication timed out\n");
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
Acked-by on v7 2/4
#5 lore raw

Re: [PATCH v7 2/4] remoteproc: qcom: refactor mss fw image loading sequence

Quoted 3 lines · Avaneesh Kumar Dwivedi

This patch refactor code to first load all firmware blobs and then update modem proc to authenticate and boot fw.

Regards,
Bjorn

Quoted 56 lines
---
 drivers/remoteproc/qcom_q6v5_pil.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)
drivers/remoteproc/qcom_q6v5_pil.c +8 -11
index 8fd697a..f5f8850 100644
@@ -503,7 +503,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 	bool relocate = false;
 	char seg_name[10];
 	ssize_t offset;
-	size_t size;
+	size_t size = 0;
 	void *ptr;
 	int ret;
 	int i;
@@ -541,7 +541,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 	}
 
 	mpss_reloc = relocate ? min_addr : qproc->mpss_phys;
-
+	/* Load firmware segments */
 	for (i = 0; i < ehdr->e_phnum; i++) {
 		phdr = &phdrs[i];
 
@@ -574,18 +574,15 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 			memset(ptr + phdr->p_filesz, 0,
 			       phdr->p_memsz - phdr->p_filesz);
 		}
-
-		size = readl(qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
-		if (!size) {
-			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);
-		}
-
 		size += phdr->p_memsz;
-		writel(size, qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
 	}
 
+	/* Transfer ownership of modem ddr region with q6*/
+	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);
+	writel(size, qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
+
 	ret = q6v5_rmb_mba_wait(qproc, RMB_MBA_AUTH_COMPLETE, 10000);
 	if (ret == -ETIMEDOUT)
 		dev_err(qproc->dev, "MPSS authentication timed out\n");
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#6 lore raw

PATCH v7 3/4 remoteproc: qcom: Make secure world call for mem ownership switch

In reply to Avaneesh Kumar Dwivedi

MSS proc on msm8996 can not access fw loaded region without stage second translation of memory pages where mpss image are loaded. This patch in order to enable mss boot on msm8996 invoke scm call to switch or share ownership between apps and modem.

---
 drivers/remoteproc/qcom_q6v5_pil.c | 90 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 87 insertions(+), 3 deletions(-)
drivers/remoteproc/qcom_q6v5_pil.c +87 -3
index f5f8850..a82b62a 100644
@@ -110,6 +110,7 @@ struct rproc_hexagon_res {
 	struct qcom_mss_reg_res *active_supply;
 	char **proxy_clk_names;
 	char **active_clk_names;
+	bool need_mem_protection;
 };
 
 struct q6v5 {
@@ -151,8 +152,11 @@ struct q6v5 {
 	phys_addr_t mpss_reloc;
 	void *mpss_region;
 	size_t mpss_size;
+	bool need_mem_protection;
 
 	struct qcom_rproc_subdev smd_subdev;
+	int mpss_perm;
+	int mba_perm;
 };
 
 static int q6v5_regulator_init(struct device *dev, struct reg_info *regs,
@@ -288,6 +292,36 @@ static struct resource_table *q6v5_find_rsc_table(struct rproc *rproc,
 	return &table;
 }
 
+static int q6v5_xfer_mem_ownership(struct q6v5 *qproc, int *current_perm,
+				   bool remote_owner, phys_addr_t addr,
+				   size_t size)
+{
+	struct qcom_scm_vmperm next;
+	int ret;
+
+	if (!qproc->need_mem_protection)
+		return 0;
+	if (remote_owner && *current_perm == BIT(QCOM_SCM_VMID_MSS_MSA))
+		return 0;
+	if (!remote_owner && *current_perm == BIT(QCOM_SCM_VMID_HLOS))
+		return 0;
+
+	next.vmid = remote_owner ? QCOM_SCM_VMID_MSS_MSA : QCOM_SCM_VMID_HLOS;
+	next.perm = remote_owner ? QCOM_SCM_PERM_RW : QCOM_SCM_PERM_RWX;
+
+	ret = qcom_scm_assign_mem(addr, ALIGN(size, SZ_4K),
+				  *current_perm, &next, 1);
+	if (ret < 0) {
+		pr_err("Failed to assign memory access in range %p to %p to %s ret = %d\n",
+		       (void *)addr, (void *)(addr + size),
+		       remote_owner ? "mss" : "hlos", ret);
+		return ret;
+	}
+
+	*current_perm = ret;
+	return 0;
+}
+
 static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
 {
 	struct q6v5 *qproc = rproc->priv;
@@ -450,6 +484,8 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
 {
 	unsigned long dma_attrs = DMA_ATTR_FORCE_CONTIGUOUS;
 	dma_addr_t phys;
+	int mdata_perm;
+	int xferop_ret;
 	void *ptr;
 	int ret;
 
@@ -461,6 +497,12 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
 
 	memcpy(ptr, fw->data, fw->size);
 
+	/* Hypervisor mapping to access metadata by modem */
+	mdata_perm = BIT(QCOM_SCM_VMID_HLOS);
+	ret = q6v5_xfer_mem_ownership(qproc, &mdata_perm,
+				      1, phys, fw->size);
+	if (ret)
+		return -EAGAIN;
 	writel(phys, qproc->rmb_base + RMB_PMI_META_DATA_REG);
 	writel(RMB_CMD_META_DATA_READY, qproc->rmb_base + RMB_MBA_COMMAND_REG);
 
@@ -470,6 +512,12 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
 	else if (ret < 0)
 		dev_err(qproc->dev, "MPSS header authentication failed: %d\n", ret);
 
+	/* Metadata authentication done, remove modem access */
+	xferop_ret = q6v5_xfer_mem_ownership(qproc, &mdata_perm,
+					     0, phys, fw->size);
+	if (xferop_ret)
+		dev_warn(qproc->dev,
+			 "mdt buffer not reclaimed system may become unstable\n");
 	dma_free_attrs(qproc->dev, fw->size, ptr, phys, dma_attrs);
 
 	return ret < 0 ? ret : 0;
@@ -579,6 +627,10 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 
 	/* Transfer ownership of modem ddr region with q6*/
 	boot_addr = relocate ? qproc->mpss_phys : min_addr;
+	ret = q6v5_xfer_mem_ownership(qproc, &qproc->mpss_perm, 1,
+				      qproc->mpss_phys, qproc->mpss_size);
+	if (ret)
+		return -EAGAIN;
 	writel(boot_addr, qproc->rmb_base + RMB_PMI_CODE_START_REG);
 	writel(RMB_CMD_LOAD_READY, qproc->rmb_base + RMB_MBA_COMMAND_REG);
 	writel(size, qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
@@ -598,6 +650,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 static int q6v5_start(struct rproc *rproc)
 {
 	struct q6v5 *qproc = (struct q6v5 *)rproc->priv;
+	int xfermemop_ret;
 	int ret;
 
 	ret = q6v5_regulator_enable(qproc, qproc->proxy_regs,
@@ -633,6 +686,12 @@ static int q6v5_start(struct rproc *rproc)
 		goto assert_reset;
 	}
 
+	/* Assign MBA image access in DDR to q6 */
+	xfermemop_ret = q6v5_xfer_mem_ownership(qproc, &qproc->mba_perm, 1,
+						qproc->mba_phys,
+						qproc->mba_size);
+	if (xfermemop_ret)
+		goto assert_reset;
 	writel(qproc->mba_phys, qproc->rmb_base + RMB_MBA_IMAGE_REG);
 
 	ret = q6v5proc_reset(qproc);
@@ -654,16 +713,22 @@ static int q6v5_start(struct rproc *rproc)
 
 	ret = q6v5_mpss_load(qproc);
 	if (ret)
-		goto halt_axi_ports;
+		goto reclaim_mem;
 
 	ret = wait_for_completion_timeout(&qproc->start_done,
 					  msecs_to_jiffies(5000));
 	if (ret == 0) {
 		dev_err(qproc->dev, "start timed out\n");
 		ret = -ETIMEDOUT;
-		goto halt_axi_ports;
+		goto reclaim_mem;
 	}
 
+	xfermemop_ret = q6v5_xfer_mem_ownership(qproc, &qproc->mba_perm, 0,
+						qproc->mba_phys,
+						qproc->mba_size);
+	if (xfermemop_ret)
+		dev_err(qproc->dev,
+			"Failed to reclaim mba buffer system may become unstable\n");
 	qproc->running = true;
 
 	q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
@@ -673,12 +738,23 @@ static int q6v5_start(struct rproc *rproc)
 
 	return 0;
 
+reclaim_mem:
+	xfermemop_ret = q6v5_xfer_mem_ownership(qproc, &qproc->mpss_perm,
+						0, qproc->mpss_phys,
+						qproc->mpss_size);
+	WARN_ON(xfermemop_ret);
 halt_axi_ports:
 	q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_q6);
 	q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem);
 	q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc);
 	q6v5_clk_disable(qproc->dev, qproc->active_clks,
 			 qproc->active_clk_count);
+	xfermemop_ret = q6v5_xfer_mem_ownership(qproc, &qproc->mba_perm, 0,
+						qproc->mba_phys,
+						qproc->mba_size);
+	if (xfermemop_ret)
+		dev_err(qproc->dev, "Failed to reclaim mba buffer, system may become unstable\n");
+
 assert_reset:
 	reset_control_assert(qproc->mss_restart);
 disable_vdd:
@@ -698,6 +774,7 @@ static int q6v5_stop(struct rproc *rproc)
 {
 	struct q6v5 *qproc = (struct q6v5 *)rproc->priv;
 	int ret;
+	u32 val;
 
 	qproc->running = false;
 
@@ -715,6 +792,9 @@ static int q6v5_stop(struct rproc *rproc)
 	q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem);
 	q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc);
 
+	ret = q6v5_xfer_mem_ownership(qproc, &qproc->mpss_perm, 0,
+				      qproc->mpss_phys, qproc->mpss_size);
+	WARN_ON(ret);
 	reset_control_assert(qproc->mss_restart);
 	q6v5_clk_disable(qproc->dev, qproc->active_clks,
 			 qproc->active_clk_count);
@@ -1012,6 +1092,7 @@ static int q6v5_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_rproc;
 
+	qproc->need_mem_protection = desc->need_mem_protection;
 	ret = q6v5_request_irq(qproc, pdev, "wdog", q6v5_wdog_interrupt);
 	if (ret < 0)
 		goto free_rproc;
@@ -1033,7 +1114,8 @@ static int q6v5_probe(struct platform_device *pdev)
 		ret = PTR_ERR(qproc->state);
 		goto free_rproc;
 	}
-
+	qproc->mpss_perm = BIT(QCOM_SCM_VMID_HLOS);
+	qproc->mba_perm = BIT(QCOM_SCM_VMID_HLOS);
 	qcom_add_smd_subdev(rproc, &qproc->smd_subdev);
 
 	ret = rproc_add(rproc);
@@ -1087,6 +1169,7 @@ static int q6v5_remove(struct platform_device *pdev)
 		"mem",
 		NULL
 	},
+	.need_mem_protection = false,
 };
 
 static const struct rproc_hexagon_res msm8974_mss = {
@@ -1124,6 +1207,7 @@ static int q6v5_remove(struct platform_device *pdev)
 		"mem",
 		NULL
 	},
+	.need_mem_protection = false,
 };
 
 static const struct of_device_id q6v5_of_match[] = {
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#7 lore raw

Re: [PATCH v7 3/4] remoteproc: qcom: Make secure world call for mem ownership switch

Quoted 11 lines · Avaneesh Kumar Dwivedi

MSS proc on msm8996 can not access fw loaded region without stage second translation of memory pages where mpss image are loaded. This patch in order to enable mss boot on msm8996 invoke scm call to switch or share ownership between apps and modem.

---
 drivers/remoteproc/qcom_q6v5_pil.c | 90 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 87 insertions(+), 3 deletions(-)
drivers/remoteproc/qcom_q6v5_pil.c +0 -0

[..]

Quoted 13 lines
drivers/remoteproc/qcom_q6v5_pil.c +13 -0
+static int q6v5_xfer_mem_ownership(struct q6v5 *qproc, int *current_perm,
+				   bool remote_owner, phys_addr_t addr,
+				   size_t size)
+{
+	struct qcom_scm_vmperm next;
+	int ret;
+
+	if (!qproc->need_mem_protection)
+		return 0;
+	if (remote_owner && *current_perm == BIT(QCOM_SCM_VMID_MSS_MSA))
+		return 0;
+	if (!remote_owner && *current_perm == BIT(QCOM_SCM_VMID_HLOS))
+		return 0;

drivers/remoteproc/qcom_q6v5_pil.c static struct resource_table *q6v5_find_rsc_table(struct rproc *rproc, · line 317

Quoted 10 lines
drivers/remoteproc/qcom_q6v5_pil.c +10 -0
+
+	next.vmid = remote_owner ? QCOM_SCM_VMID_MSS_MSA : QCOM_SCM_VMID_HLOS;
+	next.perm = remote_owner ? QCOM_SCM_PERM_RW : QCOM_SCM_PERM_RWX;
+
+	ret = qcom_scm_assign_mem(addr, ALIGN(size, SZ_4K),
+				  *current_perm, &next, 1);
+	if (ret < 0) {
+		pr_err("Failed to assign memory access in range %p to %p to %s ret = %d\n",
+		       (void *)addr, (void *)(addr + size),
+		       remote_owner ? "mss" : "hlos", ret);

qcom_scm_assign_mem() also prints an error when this happens, there's no need for that. I'm happy with you dropping the print from qcom_scm_assign_mem() and keeping this one.

Quoted 20 lines
drivers/remoteproc/qcom_q6v5_pil.c +9 -0
+		return ret;
+	}
+
+	*current_perm = ret;
+	return 0;
+}
+
 static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
 {
 	struct q6v5 *qproc = rproc->priv;
@@ -450,6 +484,8 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
 {
 	unsigned long dma_attrs = DMA_ATTR_FORCE_CONTIGUOUS;
 	dma_addr_t phys;
+	int mdata_perm;
+	int xferop_ret;
 	void *ptr;
 	int ret;
 

drivers/remoteproc/qcom_q6v5_pil.c static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw) · line 503

Quoted 7 lines
drivers/remoteproc/qcom_q6v5_pil.c +4 -0
@@ -461,6 +497,12 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
 
 	memcpy(ptr, fw->data, fw->size);
 
+	/* Hypervisor mapping to access metadata by modem */
+	mdata_perm = BIT(QCOM_SCM_VMID_HLOS);
+	ret = q6v5_xfer_mem_ownership(qproc, &mdata_perm,
+				      1, phys, fw->size);

Please use "true" instead of "1".

drivers/remoteproc/qcom_q6v5_pil.c static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw) · line 505

Quoted 2 lines
drivers/remoteproc/qcom_q6v5_pil.c +2 -0
+	if (ret)
+		return -EAGAIN;

Please keep a empty line here, to keep the "paragraphs" or "chunks" of code split up.

drivers/remoteproc/qcom_q6v5_pil.c static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw) · line 508

Quoted 3 lines
drivers/remoteproc/qcom_q6v5_pil.c +0 -0
 	writel(phys, qproc->rmb_base + RMB_PMI_META_DATA_REG);
 	writel(RMB_CMD_META_DATA_READY, qproc->rmb_base + RMB_MBA_COMMAND_REG);
 

Regards,
Bjorn

#8 lore raw

Re: [PATCH v7 3/4] remoteproc: qcom: Make secure world call for mem ownership switch

Quoted 62 lines · Bjorn Andersson
Quoted 11 lines · Avaneesh Kumar Dwivedi

MSS proc on msm8996 can not access fw loaded region without stage second translation of memory pages where mpss image are loaded. This patch in order to enable mss boot on msm8996 invoke scm call to switch or share ownership between apps and modem.

---
  drivers/remoteproc/qcom_q6v5_pil.c | 90 ++++++++++++++++++++++++++++++++++++--
  1 file changed, 87 insertions(+), 3 deletions(-)
drivers/remoteproc/qcom_q6v5_pil.c +0 -0

[..]

Quoted 13 lines
drivers/remoteproc/qcom_q6v5_pil.c +13 -0
+static int q6v5_xfer_mem_ownership(struct q6v5 *qproc, int *current_perm,
+				   bool remote_owner, phys_addr_t addr,
+				   size_t size)
+{
+	struct qcom_scm_vmperm next;
+	int ret;
+
+	if (!qproc->need_mem_protection)
+		return 0;
+	if (remote_owner && *current_perm == BIT(QCOM_SCM_VMID_MSS_MSA))
+		return 0;
+	if (!remote_owner && *current_perm == BIT(QCOM_SCM_VMID_HLOS))
+		return 0;

drivers/remoteproc/qcom_q6v5_pil.c static struct resource_table *q6v5_find_rsc_table(struct rproc *rproc, · line 317

Quoted 10 lines
drivers/remoteproc/qcom_q6v5_pil.c +10 -0
+
+	next.vmid = remote_owner ? QCOM_SCM_VMID_MSS_MSA : QCOM_SCM_VMID_HLOS;
+	next.perm = remote_owner ? QCOM_SCM_PERM_RW : QCOM_SCM_PERM_RWX;
+
+	ret = qcom_scm_assign_mem(addr, ALIGN(size, SZ_4K),
+				  *current_perm, &next, 1);
+	if (ret < 0) {
+		pr_err("Failed to assign memory access in range %p to %p to %s ret = %d\n",
+		       (void *)addr, (void *)(addr + size),
+		       remote_owner ? "mss" : "hlos", ret);

qcom_scm_assign_mem() also prints an error when this happens, there's no need for that. I'm happy with you dropping the print from qcom_scm_assign_mem() and keeping this one.

Quoted 21 lines
drivers/remoteproc/qcom_q6v5_pil.c +9 -0
+		return ret;
+	}
+
+	*current_perm = ret;
+	return 0;
+}
+
  static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
  {
  	struct q6v5 *qproc = rproc->priv;
@@ -450,6 +484,8 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
  {
  	unsigned long dma_attrs = DMA_ATTR_FORCE_CONTIGUOUS;
  	dma_addr_t phys;
+	int mdata_perm;
+	int xferop_ret;
  	void *ptr;
  	int ret;
 
@@ -461,6 +497,12 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
 
Quoted 7 lines

drivers/remoteproc/qcom_q6v5_pil.c static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw) · line 503

Quoted 6 lines
drivers/remoteproc/qcom_q6v5_pil.c +4 -0
  	memcpy(ptr, fw->data, fw->size);
 
+	/* Hypervisor mapping to access metadata by modem */
+	mdata_perm = BIT(QCOM_SCM_VMID_HLOS);
+	ret = q6v5_xfer_mem_ownership(qproc, &mdata_perm,
+				      1, phys, fw->size);

Please use "true" instead of "1".

OK Sure, will do it.

Quoted 4 lines

drivers/remoteproc/qcom_q6v5_pil.c static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw) · line 505

Quoted 2 lines
drivers/remoteproc/qcom_q6v5_pil.c +2 -0
+	if (ret)
+		return -EAGAIN;

Please keep a empty line here, to keep the "paragraphs" or "chunks" of code split up.

OK.

Quoted 10 lines

drivers/remoteproc/qcom_q6v5_pil.c static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw) · line 508

Quoted 4 lines
drivers/remoteproc/qcom_q6v5_pil.c +0 -0
  	writel(phys, qproc->rmb_base + RMB_PMI_META_DATA_REG);
  	writel(RMB_CMD_META_DATA_READY, qproc->rmb_base + RMB_MBA_COMMAND_REG);
 

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#9 lore raw

PATCH v7 4/4 remoteproc: qcom: Add support for mss remoteproc on msm8996

In reply to Avaneesh Kumar Dwivedi

This patch add support for mss boot on msm8996. Major changes include initializing mss rproc for msm8996, making appropriate change for executing mss reset sequence etc.

---
 .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |   1 +
 drivers/remoteproc/qcom_q6v5_pil.c                 | 164 ++++++++++++++++++---
 2 files changed, 141 insertions(+), 24 deletions(-)
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +1 -0
index 92347fe..87a8e51 100644
@@ -11,6 +11,7 @@ on the Qualcomm Hexagon core.
 		    "qcom,msm8916-mss-pil",
 		    "qcom,msm8974-mss-pil"
 
+		    "qcom,msm8996-mss-pil"
 - reg:
 	Usage: required
 	Value type: <prop-encoded-array>
drivers/remoteproc/qcom_q6v5_pil.c +140 -24

239 lines not on this page · raw message

Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#10 lore raw

Re: [PATCH v7 4/4] remoteproc: qcom: Add support for mss remoteproc on msm8996

Quoted 15 lines · Avaneesh Kumar Dwivedi

This patch add support for mss boot on msm8996. Major changes include initializing mss rproc for msm8996, making appropriate change for executing mss reset sequence etc.

---
 .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |   1 +
 drivers/remoteproc/qcom_q6v5_pil.c                 | 164 ++++++++++++++++++---
 2 files changed, 141 insertions(+), 24 deletions(-)
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +0 -0
index 92347fe..87a8e51 100644

Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt on the Qualcomm Hexagon core. · line 14

Quoted 4 lines
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +1 -0
@@ -11,6 +11,7 @@ on the Qualcomm Hexagon core.
 		    "qcom,msm8916-mss-pil",
 		    "qcom,msm8974-mss-pil"
 
+		    "qcom,msm8996-mss-pil"

I like the empty line, but it should be after the list, not in the middle.

Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt on the Qualcomm Hexagon core. · line 15

Quoted 1 line
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +0 -0
 - reg:

Please fix this and add my Acked-by

Regards,
Bjorn

#11 lore raw

Re: [PATCH v7 4/4] remoteproc: qcom: Add support for mss remoteproc on msm8996

Quoted 20 lines · Bjorn Andersson
Quoted 18 lines · Avaneesh Kumar Dwivedi

This patch add support for mss boot on msm8996. Major changes include initializing mss rproc for msm8996, making appropriate change for executing mss reset sequence etc.

---
  .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |   1 +
  drivers/remoteproc/qcom_q6v5_pil.c                 | 164 ++++++++++++++++++---
  2 files changed, 141 insertions(+), 24 deletions(-)
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +0 -0
index 92347fe..87a8e51 100644
@@ -11,6 +11,7 @@ on the Qualcomm Hexagon core.
  		    "qcom,msm8916-mss-pil",
  		    "qcom,msm8974-mss-pil"
 
Quoted 6 lines

Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt on the Qualcomm Hexagon core. · line 14

Quoted 1 line
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +1 -0
+		    "qcom,msm8996-mss-pil"

I like the empty line, but it should be after the list, not in the middle.

Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt on the Qualcomm Hexagon core. · line 15

Quoted 1 line
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +0 -0
  - reg:

Please fix this and add my Acked-by

Sure will do, just want to ask, when i am sending updated patches, should i sent all the 4 patches in series or should skip those which are acked by you.

Quoted 7 lines

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#12 lore raw

Re: [PATCH v7 4/4] remoteproc: qcom: Add support for mss remoteproc on msm8996

Quoted 1 line · Avaneesh Kumar (avani)

On 10/12/2017 11:50 PM, Bjorn Andersson wrote:

[..]

Quoted 4 lines
Quoted 1 line

Please fix this and add my Acked-by

Sure will do, just want to ask, when i am sending updated patches, should i sent all the 4 patches in series or should skip those which are acked by you.

Please include all the patches in each submission (unless the maintainer tells you that he/she applied some of them already).

Make sure to add any "Acked-by", "Reviewed-by" and "Tested-by" on your patches, so that people know that they are already reviewed/tested.

And finally, when resending patches, add a section between "---" and the diff-stat describing changes since the last version - that makes it easier to see what changed since last time and will help the reviewer look at the "new" things.

Thanks,
Bjorn

#13 lore raw

Re: [PATCH v7 4/4] remoteproc: qcom: Add support for mss remoteproc on msm8996

Hi Bjorn,

Quoted 11 lines · Bjorn Andersson
Quoted 1 line · Avaneesh Kumar (avani)

On 10/12/2017 11:50 PM, Bjorn Andersson wrote:

[..]

Quoted 4 lines
Quoted 1 line

Please fix this and add my Acked-by

Sure will do, just want to ask, when i am sending updated patches, should i sent all the 4 patches in series or should skip those which are acked by you.

Please include all the patches in each submission (unless the maintainer tells you that he/she applied some of them already).

Quoted 8 lines

Make sure to add any "Acked-by", "Reviewed-by" and "Tested-by" on your patches, so that people know that they are already reviewed/tested.

And finally, when resending patches, add a section between "---" and the diff-stat describing changes since the last version - that makes it easier to see what changed since last time and will help the reviewer look at the "new" things.

So i was having the ipq8074 q6 remoteproc support based on this series last time. With the msm8996 now getting cleared, i will rebase my patches and repost. Otherwise, just hope that you were ok with the approach [1].

[1] https://www.mail-archive.com/[email protected]/msg1481961.html

Regards,
Sricharan

Signature · 5 lines
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus