[PATCH v3 0/2] couple of generic remoteproc enhancements

[PATCH v3 0/2] couple of generic remoteproc enhancements

[Suman Anna]

Hi Ohad,

The following is an updated patchset addressing the previous pending comments
from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
actually). 

The patches are mainly developed to support the WkupM3 remote processor driver
on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
version of Dave's WkupM3 remoteproc work [1]

The only change in v3 is on the second patch, it mainly leverages the
memcpy_toio and memset_io functions for copying/loading code into the
internal memory sections. An additional argument has to be added to the
rproc_da_to_va function to make this distinction.

regards
Suman

[1] http://marc.info/?l=linux-omap&m=142022842323885&w=2

v2:
http://marc.info/?l=linux-omap&m=141089879412807&w=2
- Add explicit setting of the .has_iommu field in each of the existing
  remoteproc platform drivers
- Update patch description to add the usecase details for the change
  summary
- Fixed a minor checkpatch warning.

v1:
http://marc.info/?l=linux-omap&m=140483657604924&w=2

Suman Anna (2):
  remoteproc: use a flag to detect the presence of IOMMU
  remoteproc: add support to handle internal memories

 drivers/remoteproc/da8xx_remoteproc.c      |   1 +
 drivers/remoteproc/omap_remoteproc.c       |   5 ++
 drivers/remoteproc/remoteproc_core.c       | 104 ++++++++++++++++++++++++-----
 drivers/remoteproc/remoteproc_elf_loader.c |  23 +++++--
 drivers/remoteproc/remoteproc_internal.h   |   6 +-
 drivers/remoteproc/ste_modem_rproc.c       |   1 +
 include/linux/remoteproc.h                 |  45 ++++++++++++-
 7 files changed, 161 insertions(+), 24 deletions(-)

-- 
2.2.1

[PATCH v3 0/2] couple of generic remoteproc enhancements

[Suman Anna]

Hi Ohad,

The following is an updated patchset addressing the previous pending comments
from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
actually). 

The patches are mainly developed to support the WkupM3 remote processor driver
on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
version of Dave's WkupM3 remoteproc work [1]

The only change in v3 is on the second patch, it mainly leverages the
memcpy_toio and memset_io functions for copying/loading code into the
internal memory sections. An additional argument has to be added to the
rproc_da_to_va function to make this distinction.

regards
Suman

[1] http://marc.info/?l=linux-omap&m=142022842323885&w=2

v2:
http://marc.info/?l=linux-omap&m=141089879412807&w=2
- Add explicit setting of the .has_iommu field in each of the existing
  remoteproc platform drivers
- Update patch description to add the usecase details for the change
  summary
- Fixed a minor checkpatch warning.

v1:
http://marc.info/?l=linux-omap&m=140483657604924&w=2

Suman Anna (2):
  remoteproc: use a flag to detect the presence of IOMMU
  remoteproc: add support to handle internal memories

 drivers/remoteproc/da8xx_remoteproc.c      |   1 +
 drivers/remoteproc/omap_remoteproc.c       |   5 ++
 drivers/remoteproc/remoteproc_core.c       | 104 ++++++++++++++++++++++++-----
 drivers/remoteproc/remoteproc_elf_loader.c |  23 +++++--
 drivers/remoteproc/remoteproc_internal.h   |   6 +-
 drivers/remoteproc/ste_modem_rproc.c       |   1 +
 include/linux/remoteproc.h                 |  45 ++++++++++++-
 7 files changed, 161 insertions(+), 24 deletions(-)

-- 
2.2.1

[PATCH v3 0/2] couple of generic remoteproc enhancements

[Suman Anna]

Hi Ohad,

The following is an updated patchset addressing the previous pending comments
from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
actually). 

The patches are mainly developed to support the WkupM3 remote processor driver
on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
version of Dave's WkupM3 remoteproc work [1]

The only change in v3 is on the second patch, it mainly leverages the
memcpy_toio and memset_io functions for copying/loading code into the
internal memory sections. An additional argument has to be added to the
rproc_da_to_va function to make this distinction.

regards
Suman

[1] http://marc.info/?l=linux-omap&m=142022842323885&w=2

v2:
http://marc.info/?l=linux-omap&m=141089879412807&w=2
- Add explicit setting of the .has_iommu field in each of the existing
  remoteproc platform drivers
- Update patch description to add the usecase details for the change
  summary
- Fixed a minor checkpatch warning.

v1:
http://marc.info/?l=linux-omap&m=140483657604924&w=2

Suman Anna (2):
  remoteproc: use a flag to detect the presence of IOMMU
  remoteproc: add support to handle internal memories

 drivers/remoteproc/da8xx_remoteproc.c      |   1 +
 drivers/remoteproc/omap_remoteproc.c       |   5 ++
 drivers/remoteproc/remoteproc_core.c       | 104 ++++++++++++++++++++++++-----
 drivers/remoteproc/remoteproc_elf_loader.c |  23 +++++--
 drivers/remoteproc/remoteproc_internal.h   |   6 +-
 drivers/remoteproc/ste_modem_rproc.c       |   1 +
 include/linux/remoteproc.h                 |  45 ++++++++++++-
 7 files changed, 161 insertions(+), 24 deletions(-)

-- 
2.2.1

[PATCH v3 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Suman Anna]

The remoteproc driver core currently relies on iommu_present() on
the bus the device is on, to perform MMU management. However, this
logic doesn't scale for multi-arch, especially for processors that
do not have an IOMMU. Replace this logic instead by using a h/w
capability flag for the presence of IOMMU in the rproc structure.

This issue is seen on OMAP platforms when trying to add a remoteproc
driver for a small Cortex M3 called the WkupM3 used for suspend /
resume management on TI AM335/AM437x SoCs. This processor does not
have an MMU. Same is the case with another processor subsystem
PRU-ICSS on AM335/AM437x. All these are platform devices, and the
current iommu_present check will not scale for the same kernel image
to support OMAP4/OMAP5 and AM335/AM437x.

The existing platform implementation drivers - OMAP remoteproc, STE
Modem remoteproc and DA8xx remoteproc, are updated as well to properly
configure the newly added rproc field.

Cc: Robert Tivy <rtivy@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v3:
- no changes w.r.t v2

 drivers/remoteproc/da8xx_remoteproc.c |  1 +
 drivers/remoteproc/omap_remoteproc.c  |  5 +++++
 drivers/remoteproc/remoteproc_core.c  | 15 ++-------------
 drivers/remoteproc/ste_modem_rproc.c  |  1 +
 include/linux/remoteproc.h            |  2 ++
 5 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index 89fd057e5f1d..f8d6a0661c14 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -224,6 +224,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 
 	drproc = rproc->priv;
 	drproc->rproc = rproc;
+	rproc->has_iommu = false;
 
 	platform_set_drvdata(pdev, rproc);
 
diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
index e85f30370760..4be005b272a7 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -202,6 +202,11 @@ static int omap_rproc_probe(struct platform_device *pdev)
 
 	oproc = rproc->priv;
 	oproc->rproc = rproc;
+	/*
+	 * All existing OMAP IPU and DSP processors do have an MMU, and
+	 * are expected to use MMU, so this statement suffices.
+	 */
+	rproc->has_iommu = true;
 
 	platform_set_drvdata(pdev, rproc);
 
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 3cd85a638afa..11cdb119e4f3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -94,19 +94,8 @@ static int rproc_enable_iommu(struct rproc *rproc)
 	struct device *dev = rproc->dev.parent;
 	int ret;
 
-	/*
-	 * We currently use iommu_present() to decide if an IOMMU
-	 * setup is needed.
-	 *
-	 * This works for simple cases, but will easily fail with
-	 * platforms that do have an IOMMU, but not for this specific
-	 * rproc.
-	 *
-	 * This will be easily solved by introducing hw capabilities
-	 * that will be set by the remoteproc driver.
-	 */
-	if (!iommu_present(dev->bus)) {
-		dev_dbg(dev, "iommu not found\n");
+	if (!rproc->has_iommu) {
+		dev_dbg(dev, "iommu not present\n");
 		return 0;
 	}
 
diff --git a/drivers/remoteproc/ste_modem_rproc.c b/drivers/remoteproc/ste_modem_rproc.c
index 16b7b7bd805b..dd193f35a1ff 100644
--- a/drivers/remoteproc/ste_modem_rproc.c
+++ b/drivers/remoteproc/ste_modem_rproc.c
@@ -289,6 +289,7 @@ static int sproc_probe(struct platform_device *pdev)
 	sproc = rproc->priv;
 	sproc->mdev = mdev;
 	sproc->rproc = rproc;
+	rproc->has_iommu = false;
 	mdev->drv_data = sproc;
 
 	/* Provide callback functions to modem device */
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 9e7e745dac55..78b8a9b9d40a 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -404,6 +404,7 @@ enum rproc_crash_type {
  * @table_ptr: pointer to the resource table in effect
  * @cached_table: copy of the resource table
  * @table_csum: checksum of the resource table
+ * @has_iommu: flag to indicate if remote processor is behind an MMU
  */
 struct rproc {
 	struct klist_node node;
@@ -435,6 +436,7 @@ struct rproc {
 	struct resource_table *table_ptr;
 	struct resource_table *cached_table;
 	u32 table_csum;
+	bool has_iommu;
 };
 
 /* we currently support only two vrings per rvdev */
-- 
2.2.1

[PATCH v3 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Suman Anna]

The remoteproc driver core currently relies on iommu_present() on
the bus the device is on, to perform MMU management. However, this
logic doesn't scale for multi-arch, especially for processors that
do not have an IOMMU. Replace this logic instead by using a h/w
capability flag for the presence of IOMMU in the rproc structure.

This issue is seen on OMAP platforms when trying to add a remoteproc
driver for a small Cortex M3 called the WkupM3 used for suspend /
resume management on TI AM335/AM437x SoCs. This processor does not
have an MMU. Same is the case with another processor subsystem
PRU-ICSS on AM335/AM437x. All these are platform devices, and the
current iommu_present check will not scale for the same kernel image
to support OMAP4/OMAP5 and AM335/AM437x.

The existing platform implementation drivers - OMAP remoteproc, STE
Modem remoteproc and DA8xx remoteproc, are updated as well to properly
configure the newly added rproc field.

Cc: Robert Tivy <rtivy@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v3:
- no changes w.r.t v2

 drivers/remoteproc/da8xx_remoteproc.c |  1 +
 drivers/remoteproc/omap_remoteproc.c  |  5 +++++
 drivers/remoteproc/remoteproc_core.c  | 15 ++-------------
 drivers/remoteproc/ste_modem_rproc.c  |  1 +
 include/linux/remoteproc.h            |  2 ++
 5 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index 89fd057e5f1d..f8d6a0661c14 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -224,6 +224,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 
 	drproc = rproc->priv;
 	drproc->rproc = rproc;
+	rproc->has_iommu = false;
 
 	platform_set_drvdata(pdev, rproc);
 
diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
index e85f30370760..4be005b272a7 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -202,6 +202,11 @@ static int omap_rproc_probe(struct platform_device *pdev)
 
 	oproc = rproc->priv;
 	oproc->rproc = rproc;
+	/*
+	 * All existing OMAP IPU and DSP processors do have an MMU, and
+	 * are expected to use MMU, so this statement suffices.
+	 */
+	rproc->has_iommu = true;
 
 	platform_set_drvdata(pdev, rproc);
 
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 3cd85a638afa..11cdb119e4f3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -94,19 +94,8 @@ static int rproc_enable_iommu(struct rproc *rproc)
 	struct device *dev = rproc->dev.parent;
 	int ret;
 
-	/*
-	 * We currently use iommu_present() to decide if an IOMMU
-	 * setup is needed.
-	 *
-	 * This works for simple cases, but will easily fail with
-	 * platforms that do have an IOMMU, but not for this specific
-	 * rproc.
-	 *
-	 * This will be easily solved by introducing hw capabilities
-	 * that will be set by the remoteproc driver.
-	 */
-	if (!iommu_present(dev->bus)) {
-		dev_dbg(dev, "iommu not found\n");
+	if (!rproc->has_iommu) {
+		dev_dbg(dev, "iommu not present\n");
 		return 0;
 	}
 
diff --git a/drivers/remoteproc/ste_modem_rproc.c b/drivers/remoteproc/ste_modem_rproc.c
index 16b7b7bd805b..dd193f35a1ff 100644
--- a/drivers/remoteproc/ste_modem_rproc.c
+++ b/drivers/remoteproc/ste_modem_rproc.c
@@ -289,6 +289,7 @@ static int sproc_probe(struct platform_device *pdev)
 	sproc = rproc->priv;
 	sproc->mdev = mdev;
 	sproc->rproc = rproc;
+	rproc->has_iommu = false;
 	mdev->drv_data = sproc;
 
 	/* Provide callback functions to modem device */
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 9e7e745dac55..78b8a9b9d40a 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -404,6 +404,7 @@ enum rproc_crash_type {
  * @table_ptr: pointer to the resource table in effect
  * @cached_table: copy of the resource table
  * @table_csum: checksum of the resource table
+ * @has_iommu: flag to indicate if remote processor is behind an MMU
  */
 struct rproc {
 	struct klist_node node;
@@ -435,6 +436,7 @@ struct rproc {
 	struct resource_table *table_ptr;
 	struct resource_table *cached_table;
 	u32 table_csum;
+	bool has_iommu;
 };
 
 /* we currently support only two vrings per rvdev */
-- 
2.2.1

[PATCH v3 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Suman Anna]

The remoteproc driver core currently relies on iommu_present() on
the bus the device is on, to perform MMU management. However, this
logic doesn't scale for multi-arch, especially for processors that
do not have an IOMMU. Replace this logic instead by using a h/w
capability flag for the presence of IOMMU in the rproc structure.

This issue is seen on OMAP platforms when trying to add a remoteproc
driver for a small Cortex M3 called the WkupM3 used for suspend /
resume management on TI AM335/AM437x SoCs. This processor does not
have an MMU. Same is the case with another processor subsystem
PRU-ICSS on AM335/AM437x. All these are platform devices, and the
current iommu_present check will not scale for the same kernel image
to support OMAP4/OMAP5 and AM335/AM437x.

The existing platform implementation drivers - OMAP remoteproc, STE
Modem remoteproc and DA8xx remoteproc, are updated as well to properly
configure the newly added rproc field.

Cc: Robert Tivy <rtivy@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v3:
- no changes w.r.t v2

 drivers/remoteproc/da8xx_remoteproc.c |  1 +
 drivers/remoteproc/omap_remoteproc.c  |  5 +++++
 drivers/remoteproc/remoteproc_core.c  | 15 ++-------------
 drivers/remoteproc/ste_modem_rproc.c  |  1 +
 include/linux/remoteproc.h            |  2 ++
 5 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index 89fd057e5f1d..f8d6a0661c14 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -224,6 +224,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 
 	drproc = rproc->priv;
 	drproc->rproc = rproc;
+	rproc->has_iommu = false;
 
 	platform_set_drvdata(pdev, rproc);
 
diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
index e85f30370760..4be005b272a7 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -202,6 +202,11 @@ static int omap_rproc_probe(struct platform_device *pdev)
 
 	oproc = rproc->priv;
 	oproc->rproc = rproc;
+	/*
+	 * All existing OMAP IPU and DSP processors do have an MMU, and
+	 * are expected to use MMU, so this statement suffices.
+	 */
+	rproc->has_iommu = true;
 
 	platform_set_drvdata(pdev, rproc);
 
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 3cd85a638afa..11cdb119e4f3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -94,19 +94,8 @@ static int rproc_enable_iommu(struct rproc *rproc)
 	struct device *dev = rproc->dev.parent;
 	int ret;
 
-	/*
-	 * We currently use iommu_present() to decide if an IOMMU
-	 * setup is needed.
-	 *
-	 * This works for simple cases, but will easily fail with
-	 * platforms that do have an IOMMU, but not for this specific
-	 * rproc.
-	 *
-	 * This will be easily solved by introducing hw capabilities
-	 * that will be set by the remoteproc driver.
-	 */
-	if (!iommu_present(dev->bus)) {
-		dev_dbg(dev, "iommu not found\n");
+	if (!rproc->has_iommu) {
+		dev_dbg(dev, "iommu not present\n");
 		return 0;
 	}
 
diff --git a/drivers/remoteproc/ste_modem_rproc.c b/drivers/remoteproc/ste_modem_rproc.c
index 16b7b7bd805b..dd193f35a1ff 100644
--- a/drivers/remoteproc/ste_modem_rproc.c
+++ b/drivers/remoteproc/ste_modem_rproc.c
@@ -289,6 +289,7 @@ static int sproc_probe(struct platform_device *pdev)
 	sproc = rproc->priv;
 	sproc->mdev = mdev;
 	sproc->rproc = rproc;
+	rproc->has_iommu = false;
 	mdev->drv_data = sproc;
 
 	/* Provide callback functions to modem device */
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 9e7e745dac55..78b8a9b9d40a 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -404,6 +404,7 @@ enum rproc_crash_type {
  * @table_ptr: pointer to the resource table in effect
  * @cached_table: copy of the resource table
  * @table_csum: checksum of the resource table
+ * @has_iommu: flag to indicate if remote processor is behind an MMU
  */
 struct rproc {
 	struct klist_node node;
@@ -435,6 +436,7 @@ struct rproc {
 	struct resource_table *table_ptr;
 	struct resource_table *cached_table;
 	u32 table_csum;
+	bool has_iommu;
 };
 
 /* we currently support only two vrings per rvdev */
-- 
2.2.1

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

A remote processor may need to load certain firmware sections into
internal memories (eg: RAM at L1 or L2 levels) for performance or
other reasons. Introduce a new resource type (RSC_INTMEM) and add
an associated handler function to handle such memories. The handler
creates a kernel mapping for the resource's 'pa' (physical address).

Note that no iommu mapping is performed for this resource, as the
resource is primarily used to represent physical internal memories.
If the internal memory region can only be accessed through an iommu,
a devmem resource entry should be used instead.

Signed-off-by: Robert Tivy <rtivy@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v3:
- leverage memcpy_toio and memset_io for loading into internal memory
- rproc_da_to_va takes an additional argument to allow this distinction

 drivers/remoteproc/remoteproc_core.c       | 89 +++++++++++++++++++++++++++++-
 drivers/remoteproc/remoteproc_elf_loader.c | 23 ++++++--
 drivers/remoteproc/remoteproc_internal.h   |  6 +-
 include/linux/remoteproc.h                 | 43 ++++++++++++++-
 4 files changed, 150 insertions(+), 11 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 11cdb119e4f3..e0ecc0f802c1 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -153,7 +153,7 @@ static void rproc_disable_iommu(struct rproc *rproc)
  * but only on kernel direct mapped RAM memory. Instead, we're just using
  * here the output of the DMA API, which should be more correct.
  */
-void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 *flags)
 {
 	struct rproc_mem_entry *carveout;
 	void *ptr = NULL;
@@ -170,6 +170,8 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
 			continue;
 
 		ptr = carveout->va + offset;
+		if (flags && carveout->priv)
+			*flags = RPROC_INTMEM;
 
 		break;
 	}
@@ -404,7 +406,7 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
 	}
 
 	/* what's the kernel address of this resource ? */
-	ptr = rproc_da_to_va(rproc, rsc->da, rsc->len);
+	ptr = rproc_da_to_va(rproc, rsc->da, rsc->len, NULL);
 	if (!ptr) {
 		dev_err(dev, "erroneous trace resource entry\n");
 		return -EINVAL;
@@ -664,6 +666,82 @@ free_carv:
 	return ret;
 }
 
+/**
+ * rproc_handle_intmem() - handle internal memory resource entry
+ * @rproc: rproc handle
+ * @rsc: the intmem resource entry
+ * @offset: offset of the resource data in resource table
+ * @avail: size of available data (for image validation)
+ *
+ * This function will handle firmware requests for mapping a memory region
+ * internal to a remote processor into kernel. It neither allocates any
+ * physical pages, nor performs any iommu mapping, as this resource entry
+ * is primarily used for representing physical internal memories. If the
+ * internal memory region can only be accessed through an iommu, please
+ * use a devmem resource entry.
+ *
+ * These resource entries should be grouped near the carveout entries in
+ * the firmware's resource table, as other firmware entries might request
+ * placing other data objects inside these memory regions (e.g. data/code
+ * segments, trace resource entries, ...).
+ */
+static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
+			       int offset, int avail)
+{
+	struct rproc_mem_entry *intmem;
+	struct device *dev = &rproc->dev;
+	void *va;
+	int ret;
+
+	if (sizeof(*rsc) > avail) {
+		dev_err(dev, "intmem rsc is truncated\n");
+		return -EINVAL;
+	}
+
+	if (rsc->version != 1) {
+		dev_err(dev, "intmem rsc version %d is not supported\n",
+			rsc->version);
+		return -EINVAL;
+	}
+
+	if (rsc->reserved) {
+		dev_err(dev, "intmem rsc has non zero reserved bytes\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "intmem rsc: da 0x%x, pa 0x%x, len 0x%x\n",
+		rsc->da, rsc->pa, rsc->len);
+
+	intmem = kzalloc(sizeof(*intmem), GFP_KERNEL);
+	if (!intmem)
+		return -ENOMEM;
+
+	va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
+	if (!va) {
+		dev_err(dev, "ioremap_nocache err: %d\n", rsc->len);
+		ret = -ENOMEM;
+		goto free_intmem;
+	}
+
+	dev_dbg(dev, "intmem mapped pa 0x%x of len 0x%x into kernel va %p\n",
+		rsc->pa, rsc->len, va);
+
+	intmem->va = va;
+	intmem->len = rsc->len;
+	intmem->dma = rsc->pa;
+	intmem->da = rsc->da;
+	intmem->priv = (void *)RPROC_INTMEM;    /* prevents freeing */
+
+	/* reuse the rproc->carveouts list, so that loading is automatic */
+	list_add_tail(&intmem->node, &rproc->carveouts);
+
+	return 0;
+
+free_intmem:
+	kfree(intmem);
+	return ret;
+}
+
 static int rproc_count_vrings(struct rproc *rproc, struct fw_rsc_vdev *rsc,
 			      int offset, int avail)
 {
@@ -681,6 +759,7 @@ static rproc_handle_resource_t rproc_loading_handlers[RSC_LAST] = {
 	[RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout,
 	[RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem,
 	[RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace,
+	[RSC_INTMEM] = (rproc_handle_resource_t)rproc_handle_intmem,
 	[RSC_VDEV] = NULL, /* VDEVs were handled upon registrarion */
 };
 
@@ -768,7 +847,11 @@ static void rproc_resource_cleanup(struct rproc *rproc)
 
 	/* clean up carveout allocations */
 	list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
-		dma_free_coherent(dev->parent, entry->len, entry->va, entry->dma);
+		if (!entry->priv)
+			dma_free_coherent(dev->parent, entry->len, entry->va,
+					  entry->dma);
+		else
+			iounmap((__force void __iomem *)entry->va);
 		list_del(&entry->node);
 		kfree(entry);
 	}
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index ce283a5b42a1..cdd7b622cee3 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -150,6 +150,7 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 	struct elf32_phdr *phdr;
 	int i, ret = 0;
 	const u8 *elf_data = fw->data;
+	u32 flags = 0;
 
 	ehdr = (struct elf32_hdr *)elf_data;
 	phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff);
@@ -183,7 +184,7 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 		}
 
 		/* grab the kernel address for this device address */
-		ptr = rproc_da_to_va(rproc, da, memsz);
+		ptr = rproc_da_to_va(rproc, da, memsz, &flags);
 		if (!ptr) {
 			dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
 			ret = -EINVAL;
@@ -191,8 +192,13 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 		}
 
 		/* put the segment where the remote processor expects it */
-		if (phdr->p_filesz)
-			memcpy(ptr, elf_data + phdr->p_offset, filesz);
+		if (phdr->p_filesz) {
+			if (flags & RPROC_INTMEM)
+				memcpy_toio((void __iomem *)ptr,
+					    elf_data + phdr->p_offset, filesz);
+			else
+				memcpy(ptr, elf_data + phdr->p_offset, filesz);
+		}
 
 		/*
 		 * Zero out remaining memory for this segment.
@@ -201,8 +207,13 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 		 * did this for us. albeit harmless, we may consider removing
 		 * this.
 		 */
-		if (memsz > filesz)
-			memset(ptr + filesz, 0, memsz - filesz);
+		if (memsz > filesz) {
+			if (flags & RPROC_INTMEM)
+				memset_io((void __iomem *)ptr + filesz,
+					  0, memsz - filesz);
+			else
+				memset(ptr + filesz, 0, memsz - filesz);
+		}
 	}
 
 	return ret;
@@ -325,7 +336,7 @@ rproc_elf_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
 	if (!shdr)
 		return NULL;
 
-	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size);
+	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size, NULL);
 }
 
 const struct rproc_fw_ops rproc_elf_fw_ops = {
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 70701a50ddfa..8af4a8188488 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -23,6 +23,10 @@
 #include <linux/irqreturn.h>
 #include <linux/firmware.h>
 
+enum rproc_mem_type {
+	RPROC_INTMEM = 1,
+};
+
 struct rproc;
 
 /**
@@ -65,7 +69,7 @@ void rproc_exit_debugfs(void);
 void rproc_free_vring(struct rproc_vring *rvring);
 int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
 
-void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 *flags);
 int rproc_trigger_recovery(struct rproc *rproc);
 
 static inline
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 78b8a9b9d40a..2a25ee8a34dd 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -100,6 +100,7 @@ struct fw_rsc_hdr {
  *		    the remote processor will be writing logs.
  * @RSC_VDEV:       declare support for a virtio device, and serve as its
  *		    virtio header.
+ * @RSC_INTMEM:     request to map into kernel an internal memory region.
  * @RSC_LAST:       just keep this one at the end
  *
  * For more details regarding a specific resource type, please see its
@@ -115,7 +116,8 @@ enum fw_resource_type {
 	RSC_DEVMEM	= 1,
 	RSC_TRACE	= 2,
 	RSC_VDEV	= 3,
-	RSC_LAST	= 4,
+	RSC_INTMEM	= 4,
+	RSC_LAST	= 5,
 };
 
 #define FW_RSC_ADDR_ANY (0xFFFFFFFFFFFFFFFF)
@@ -306,6 +308,45 @@ struct fw_rsc_vdev {
 } __packed;
 
 /**
+ * struct fw_rsc_intmem - internal memory publishing request
+ * @version: version for this resource type (must be one)
+ * @da: device address
+ * @pa: physical address
+ * @len: length (in bytes)
+ * @reserved: reserved (must be zero)
+ * @name: human-readable name of the region being published
+ *
+ * This resource entry allows a remote processor to publish an internal
+ * memory region to the host. This resource type allows a remote processor
+ * to publish the whole or just a portion of certain internal memories,
+ * while it owns and manages any unpublished portion (eg: a shared L1
+ * memory that can be split configured as RAM and/or cache). This is
+ * primarily provided to allow a host to load code/data into internal
+ * memories, the memory for which is neither allocated nor required to
+ * be mapped into an iommu.
+ *
+ * @da should specify the required address as accessible by the device
+ * without going through an iommu, @pa should specify the physical address
+ * for the region as seen on the bus, @len should specify the size of the
+ * memory region. As always, @name may (optionally) contain a human readable
+ * name of this mapping (mainly for debugging purposes). The @version field
+ * is added for future scalability, and should be 1 for now.
+ *
+ * Note: at this point we just "trust" these intmem entries to contain valid
+ * physical bus addresses. these are not currently intended to be managed
+ * as host-controlled heaps, as it is much better to do that from the remote
+ * processor side.
+ */
+struct fw_rsc_intmem {
+	u32 version;
+	u32 da;
+	u32 pa;
+	u32 len;
+	u32 reserved;
+	u8 name[32];
+} __packed;
+
+/**
  * struct rproc_mem_entry - memory entry descriptor
  * @va:	virtual address
  * @dma: dma address
-- 
2.2.1

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

A remote processor may need to load certain firmware sections into
internal memories (eg: RAM at L1 or L2 levels) for performance or
other reasons. Introduce a new resource type (RSC_INTMEM) and add
an associated handler function to handle such memories. The handler
creates a kernel mapping for the resource's 'pa' (physical address).

Note that no iommu mapping is performed for this resource, as the
resource is primarily used to represent physical internal memories.
If the internal memory region can only be accessed through an iommu,
a devmem resource entry should be used instead.

Signed-off-by: Robert Tivy <rtivy@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v3:
- leverage memcpy_toio and memset_io for loading into internal memory
- rproc_da_to_va takes an additional argument to allow this distinction

 drivers/remoteproc/remoteproc_core.c       | 89 +++++++++++++++++++++++++++++-
 drivers/remoteproc/remoteproc_elf_loader.c | 23 ++++++--
 drivers/remoteproc/remoteproc_internal.h   |  6 +-
 include/linux/remoteproc.h                 | 43 ++++++++++++++-
 4 files changed, 150 insertions(+), 11 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 11cdb119e4f3..e0ecc0f802c1 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -153,7 +153,7 @@ static void rproc_disable_iommu(struct rproc *rproc)
  * but only on kernel direct mapped RAM memory. Instead, we're just using
  * here the output of the DMA API, which should be more correct.
  */
-void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 *flags)
 {
 	struct rproc_mem_entry *carveout;
 	void *ptr = NULL;
@@ -170,6 +170,8 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
 			continue;
 
 		ptr = carveout->va + offset;
+		if (flags && carveout->priv)
+			*flags = RPROC_INTMEM;
 
 		break;
 	}
@@ -404,7 +406,7 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
 	}
 
 	/* what's the kernel address of this resource ? */
-	ptr = rproc_da_to_va(rproc, rsc->da, rsc->len);
+	ptr = rproc_da_to_va(rproc, rsc->da, rsc->len, NULL);
 	if (!ptr) {
 		dev_err(dev, "erroneous trace resource entry\n");
 		return -EINVAL;
@@ -664,6 +666,82 @@ free_carv:
 	return ret;
 }
 
+/**
+ * rproc_handle_intmem() - handle internal memory resource entry
+ * @rproc: rproc handle
+ * @rsc: the intmem resource entry
+ * @offset: offset of the resource data in resource table
+ * @avail: size of available data (for image validation)
+ *
+ * This function will handle firmware requests for mapping a memory region
+ * internal to a remote processor into kernel. It neither allocates any
+ * physical pages, nor performs any iommu mapping, as this resource entry
+ * is primarily used for representing physical internal memories. If the
+ * internal memory region can only be accessed through an iommu, please
+ * use a devmem resource entry.
+ *
+ * These resource entries should be grouped near the carveout entries in
+ * the firmware's resource table, as other firmware entries might request
+ * placing other data objects inside these memory regions (e.g. data/code
+ * segments, trace resource entries, ...).
+ */
+static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
+			       int offset, int avail)
+{
+	struct rproc_mem_entry *intmem;
+	struct device *dev = &rproc->dev;
+	void *va;
+	int ret;
+
+	if (sizeof(*rsc) > avail) {
+		dev_err(dev, "intmem rsc is truncated\n");
+		return -EINVAL;
+	}
+
+	if (rsc->version != 1) {
+		dev_err(dev, "intmem rsc version %d is not supported\n",
+			rsc->version);
+		return -EINVAL;
+	}
+
+	if (rsc->reserved) {
+		dev_err(dev, "intmem rsc has non zero reserved bytes\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "intmem rsc: da 0x%x, pa 0x%x, len 0x%x\n",
+		rsc->da, rsc->pa, rsc->len);
+
+	intmem = kzalloc(sizeof(*intmem), GFP_KERNEL);
+	if (!intmem)
+		return -ENOMEM;
+
+	va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
+	if (!va) {
+		dev_err(dev, "ioremap_nocache err: %d\n", rsc->len);
+		ret = -ENOMEM;
+		goto free_intmem;
+	}
+
+	dev_dbg(dev, "intmem mapped pa 0x%x of len 0x%x into kernel va %p\n",
+		rsc->pa, rsc->len, va);
+
+	intmem->va = va;
+	intmem->len = rsc->len;
+	intmem->dma = rsc->pa;
+	intmem->da = rsc->da;
+	intmem->priv = (void *)RPROC_INTMEM;    /* prevents freeing */
+
+	/* reuse the rproc->carveouts list, so that loading is automatic */
+	list_add_tail(&intmem->node, &rproc->carveouts);
+
+	return 0;
+
+free_intmem:
+	kfree(intmem);
+	return ret;
+}
+
 static int rproc_count_vrings(struct rproc *rproc, struct fw_rsc_vdev *rsc,
 			      int offset, int avail)
 {
@@ -681,6 +759,7 @@ static rproc_handle_resource_t rproc_loading_handlers[RSC_LAST] = {
 	[RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout,
 	[RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem,
 	[RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace,
+	[RSC_INTMEM] = (rproc_handle_resource_t)rproc_handle_intmem,
 	[RSC_VDEV] = NULL, /* VDEVs were handled upon registrarion */
 };
 
@@ -768,7 +847,11 @@ static void rproc_resource_cleanup(struct rproc *rproc)
 
 	/* clean up carveout allocations */
 	list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
-		dma_free_coherent(dev->parent, entry->len, entry->va, entry->dma);
+		if (!entry->priv)
+			dma_free_coherent(dev->parent, entry->len, entry->va,
+					  entry->dma);
+		else
+			iounmap((__force void __iomem *)entry->va);
 		list_del(&entry->node);
 		kfree(entry);
 	}
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index ce283a5b42a1..cdd7b622cee3 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -150,6 +150,7 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 	struct elf32_phdr *phdr;
 	int i, ret = 0;
 	const u8 *elf_data = fw->data;
+	u32 flags = 0;
 
 	ehdr = (struct elf32_hdr *)elf_data;
 	phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff);
@@ -183,7 +184,7 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 		}
 
 		/* grab the kernel address for this device address */
-		ptr = rproc_da_to_va(rproc, da, memsz);
+		ptr = rproc_da_to_va(rproc, da, memsz, &flags);
 		if (!ptr) {
 			dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
 			ret = -EINVAL;
@@ -191,8 +192,13 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 		}
 
 		/* put the segment where the remote processor expects it */
-		if (phdr->p_filesz)
-			memcpy(ptr, elf_data + phdr->p_offset, filesz);
+		if (phdr->p_filesz) {
+			if (flags & RPROC_INTMEM)
+				memcpy_toio((void __iomem *)ptr,
+					    elf_data + phdr->p_offset, filesz);
+			else
+				memcpy(ptr, elf_data + phdr->p_offset, filesz);
+		}
 
 		/*
 		 * Zero out remaining memory for this segment.
@@ -201,8 +207,13 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 		 * did this for us. albeit harmless, we may consider removing
 		 * this.
 		 */
-		if (memsz > filesz)
-			memset(ptr + filesz, 0, memsz - filesz);
+		if (memsz > filesz) {
+			if (flags & RPROC_INTMEM)
+				memset_io((void __iomem *)ptr + filesz,
+					  0, memsz - filesz);
+			else
+				memset(ptr + filesz, 0, memsz - filesz);
+		}
 	}
 
 	return ret;
@@ -325,7 +336,7 @@ rproc_elf_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
 	if (!shdr)
 		return NULL;
 
-	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size);
+	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size, NULL);
 }
 
 const struct rproc_fw_ops rproc_elf_fw_ops = {
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 70701a50ddfa..8af4a8188488 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -23,6 +23,10 @@
 #include <linux/irqreturn.h>
 #include <linux/firmware.h>
 
+enum rproc_mem_type {
+	RPROC_INTMEM = 1,
+};
+
 struct rproc;
 
 /**
@@ -65,7 +69,7 @@ void rproc_exit_debugfs(void);
 void rproc_free_vring(struct rproc_vring *rvring);
 int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
 
-void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 *flags);
 int rproc_trigger_recovery(struct rproc *rproc);
 
 static inline
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 78b8a9b9d40a..2a25ee8a34dd 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -100,6 +100,7 @@ struct fw_rsc_hdr {
  *		    the remote processor will be writing logs.
  * @RSC_VDEV:       declare support for a virtio device, and serve as its
  *		    virtio header.
+ * @RSC_INTMEM:     request to map into kernel an internal memory region.
  * @RSC_LAST:       just keep this one at the end
  *
  * For more details regarding a specific resource type, please see its
@@ -115,7 +116,8 @@ enum fw_resource_type {
 	RSC_DEVMEM	= 1,
 	RSC_TRACE	= 2,
 	RSC_VDEV	= 3,
-	RSC_LAST	= 4,
+	RSC_INTMEM	= 4,
+	RSC_LAST	= 5,
 };
 
 #define FW_RSC_ADDR_ANY (0xFFFFFFFFFFFFFFFF)
@@ -306,6 +308,45 @@ struct fw_rsc_vdev {
 } __packed;
 
 /**
+ * struct fw_rsc_intmem - internal memory publishing request
+ * @version: version for this resource type (must be one)
+ * @da: device address
+ * @pa: physical address
+ * @len: length (in bytes)
+ * @reserved: reserved (must be zero)
+ * @name: human-readable name of the region being published
+ *
+ * This resource entry allows a remote processor to publish an internal
+ * memory region to the host. This resource type allows a remote processor
+ * to publish the whole or just a portion of certain internal memories,
+ * while it owns and manages any unpublished portion (eg: a shared L1
+ * memory that can be split configured as RAM and/or cache). This is
+ * primarily provided to allow a host to load code/data into internal
+ * memories, the memory for which is neither allocated nor required to
+ * be mapped into an iommu.
+ *
+ * @da should specify the required address as accessible by the device
+ * without going through an iommu, @pa should specify the physical address
+ * for the region as seen on the bus, @len should specify the size of the
+ * memory region. As always, @name may (optionally) contain a human readable
+ * name of this mapping (mainly for debugging purposes). The @version field
+ * is added for future scalability, and should be 1 for now.
+ *
+ * Note: at this point we just "trust" these intmem entries to contain valid
+ * physical bus addresses. these are not currently intended to be managed
+ * as host-controlled heaps, as it is much better to do that from the remote
+ * processor side.
+ */
+struct fw_rsc_intmem {
+	u32 version;
+	u32 da;
+	u32 pa;
+	u32 len;
+	u32 reserved;
+	u8 name[32];
+} __packed;
+
+/**
  * struct rproc_mem_entry - memory entry descriptor
  * @va:	virtual address
  * @dma: dma address
-- 
2.2.1

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

A remote processor may need to load certain firmware sections into
internal memories (eg: RAM at L1 or L2 levels) for performance or
other reasons. Introduce a new resource type (RSC_INTMEM) and add
an associated handler function to handle such memories. The handler
creates a kernel mapping for the resource's 'pa' (physical address).

Note that no iommu mapping is performed for this resource, as the
resource is primarily used to represent physical internal memories.
If the internal memory region can only be accessed through an iommu,
a devmem resource entry should be used instead.

Signed-off-by: Robert Tivy <rtivy@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v3:
- leverage memcpy_toio and memset_io for loading into internal memory
- rproc_da_to_va takes an additional argument to allow this distinction

 drivers/remoteproc/remoteproc_core.c       | 89 +++++++++++++++++++++++++++++-
 drivers/remoteproc/remoteproc_elf_loader.c | 23 ++++++--
 drivers/remoteproc/remoteproc_internal.h   |  6 +-
 include/linux/remoteproc.h                 | 43 ++++++++++++++-
 4 files changed, 150 insertions(+), 11 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 11cdb119e4f3..e0ecc0f802c1 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -153,7 +153,7 @@ static void rproc_disable_iommu(struct rproc *rproc)
  * but only on kernel direct mapped RAM memory. Instead, we're just using
  * here the output of the DMA API, which should be more correct.
  */
-void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 *flags)
 {
 	struct rproc_mem_entry *carveout;
 	void *ptr = NULL;
@@ -170,6 +170,8 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
 			continue;
 
 		ptr = carveout->va + offset;
+		if (flags && carveout->priv)
+			*flags = RPROC_INTMEM;
 
 		break;
 	}
@@ -404,7 +406,7 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
 	}
 
 	/* what's the kernel address of this resource ? */
-	ptr = rproc_da_to_va(rproc, rsc->da, rsc->len);
+	ptr = rproc_da_to_va(rproc, rsc->da, rsc->len, NULL);
 	if (!ptr) {
 		dev_err(dev, "erroneous trace resource entry\n");
 		return -EINVAL;
@@ -664,6 +666,82 @@ free_carv:
 	return ret;
 }
 
+/**
+ * rproc_handle_intmem() - handle internal memory resource entry
+ * @rproc: rproc handle
+ * @rsc: the intmem resource entry
+ * @offset: offset of the resource data in resource table
+ * @avail: size of available data (for image validation)
+ *
+ * This function will handle firmware requests for mapping a memory region
+ * internal to a remote processor into kernel. It neither allocates any
+ * physical pages, nor performs any iommu mapping, as this resource entry
+ * is primarily used for representing physical internal memories. If the
+ * internal memory region can only be accessed through an iommu, please
+ * use a devmem resource entry.
+ *
+ * These resource entries should be grouped near the carveout entries in
+ * the firmware's resource table, as other firmware entries might request
+ * placing other data objects inside these memory regions (e.g. data/code
+ * segments, trace resource entries, ...).
+ */
+static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
+			       int offset, int avail)
+{
+	struct rproc_mem_entry *intmem;
+	struct device *dev = &rproc->dev;
+	void *va;
+	int ret;
+
+	if (sizeof(*rsc) > avail) {
+		dev_err(dev, "intmem rsc is truncated\n");
+		return -EINVAL;
+	}
+
+	if (rsc->version != 1) {
+		dev_err(dev, "intmem rsc version %d is not supported\n",
+			rsc->version);
+		return -EINVAL;
+	}
+
+	if (rsc->reserved) {
+		dev_err(dev, "intmem rsc has non zero reserved bytes\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(dev, "intmem rsc: da 0x%x, pa 0x%x, len 0x%x\n",
+		rsc->da, rsc->pa, rsc->len);
+
+	intmem = kzalloc(sizeof(*intmem), GFP_KERNEL);
+	if (!intmem)
+		return -ENOMEM;
+
+	va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
+	if (!va) {
+		dev_err(dev, "ioremap_nocache err: %d\n", rsc->len);
+		ret = -ENOMEM;
+		goto free_intmem;
+	}
+
+	dev_dbg(dev, "intmem mapped pa 0x%x of len 0x%x into kernel va %p\n",
+		rsc->pa, rsc->len, va);
+
+	intmem->va = va;
+	intmem->len = rsc->len;
+	intmem->dma = rsc->pa;
+	intmem->da = rsc->da;
+	intmem->priv = (void *)RPROC_INTMEM;    /* prevents freeing */
+
+	/* reuse the rproc->carveouts list, so that loading is automatic */
+	list_add_tail(&intmem->node, &rproc->carveouts);
+
+	return 0;
+
+free_intmem:
+	kfree(intmem);
+	return ret;
+}
+
 static int rproc_count_vrings(struct rproc *rproc, struct fw_rsc_vdev *rsc,
 			      int offset, int avail)
 {
@@ -681,6 +759,7 @@ static rproc_handle_resource_t rproc_loading_handlers[RSC_LAST] = {
 	[RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout,
 	[RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem,
 	[RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace,
+	[RSC_INTMEM] = (rproc_handle_resource_t)rproc_handle_intmem,
 	[RSC_VDEV] = NULL, /* VDEVs were handled upon registrarion */
 };
 
@@ -768,7 +847,11 @@ static void rproc_resource_cleanup(struct rproc *rproc)
 
 	/* clean up carveout allocations */
 	list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) {
-		dma_free_coherent(dev->parent, entry->len, entry->va, entry->dma);
+		if (!entry->priv)
+			dma_free_coherent(dev->parent, entry->len, entry->va,
+					  entry->dma);
+		else
+			iounmap((__force void __iomem *)entry->va);
 		list_del(&entry->node);
 		kfree(entry);
 	}
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index ce283a5b42a1..cdd7b622cee3 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -150,6 +150,7 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 	struct elf32_phdr *phdr;
 	int i, ret = 0;
 	const u8 *elf_data = fw->data;
+	u32 flags = 0;
 
 	ehdr = (struct elf32_hdr *)elf_data;
 	phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff);
@@ -183,7 +184,7 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 		}
 
 		/* grab the kernel address for this device address */
-		ptr = rproc_da_to_va(rproc, da, memsz);
+		ptr = rproc_da_to_va(rproc, da, memsz, &flags);
 		if (!ptr) {
 			dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
 			ret = -EINVAL;
@@ -191,8 +192,13 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 		}
 
 		/* put the segment where the remote processor expects it */
-		if (phdr->p_filesz)
-			memcpy(ptr, elf_data + phdr->p_offset, filesz);
+		if (phdr->p_filesz) {
+			if (flags & RPROC_INTMEM)
+				memcpy_toio((void __iomem *)ptr,
+					    elf_data + phdr->p_offset, filesz);
+			else
+				memcpy(ptr, elf_data + phdr->p_offset, filesz);
+		}
 
 		/*
 		 * Zero out remaining memory for this segment.
@@ -201,8 +207,13 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
 		 * did this for us. albeit harmless, we may consider removing
 		 * this.
 		 */
-		if (memsz > filesz)
-			memset(ptr + filesz, 0, memsz - filesz);
+		if (memsz > filesz) {
+			if (flags & RPROC_INTMEM)
+				memset_io((void __iomem *)ptr + filesz,
+					  0, memsz - filesz);
+			else
+				memset(ptr + filesz, 0, memsz - filesz);
+		}
 	}
 
 	return ret;
@@ -325,7 +336,7 @@ rproc_elf_find_loaded_rsc_table(struct rproc *rproc, const struct firmware *fw)
 	if (!shdr)
 		return NULL;
 
-	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size);
+	return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size, NULL);
 }
 
 const struct rproc_fw_ops rproc_elf_fw_ops = {
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 70701a50ddfa..8af4a8188488 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -23,6 +23,10 @@
 #include <linux/irqreturn.h>
 #include <linux/firmware.h>
 
+enum rproc_mem_type {
+	RPROC_INTMEM = 1,
+};
+
 struct rproc;
 
 /**
@@ -65,7 +69,7 @@ void rproc_exit_debugfs(void);
 void rproc_free_vring(struct rproc_vring *rvring);
 int rproc_alloc_vring(struct rproc_vdev *rvdev, int i);
 
-void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len, u32 *flags);
 int rproc_trigger_recovery(struct rproc *rproc);
 
 static inline
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 78b8a9b9d40a..2a25ee8a34dd 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -100,6 +100,7 @@ struct fw_rsc_hdr {
  *		    the remote processor will be writing logs.
  * @RSC_VDEV:       declare support for a virtio device, and serve as its
  *		    virtio header.
+ * @RSC_INTMEM:     request to map into kernel an internal memory region.
  * @RSC_LAST:       just keep this one at the end
  *
  * For more details regarding a specific resource type, please see its
@@ -115,7 +116,8 @@ enum fw_resource_type {
 	RSC_DEVMEM	= 1,
 	RSC_TRACE	= 2,
 	RSC_VDEV	= 3,
-	RSC_LAST	= 4,
+	RSC_INTMEM	= 4,
+	RSC_LAST	= 5,
 };
 
 #define FW_RSC_ADDR_ANY (0xFFFFFFFFFFFFFFFF)
@@ -306,6 +308,45 @@ struct fw_rsc_vdev {
 } __packed;
 
 /**
+ * struct fw_rsc_intmem - internal memory publishing request
+ * @version: version for this resource type (must be one)
+ * @da: device address
+ * @pa: physical address
+ * @len: length (in bytes)
+ * @reserved: reserved (must be zero)
+ * @name: human-readable name of the region being published
+ *
+ * This resource entry allows a remote processor to publish an internal
+ * memory region to the host. This resource type allows a remote processor
+ * to publish the whole or just a portion of certain internal memories,
+ * while it owns and manages any unpublished portion (eg: a shared L1
+ * memory that can be split configured as RAM and/or cache). This is
+ * primarily provided to allow a host to load code/data into internal
+ * memories, the memory for which is neither allocated nor required to
+ * be mapped into an iommu.
+ *
+ * @da should specify the required address as accessible by the device
+ * without going through an iommu, @pa should specify the physical address
+ * for the region as seen on the bus, @len should specify the size of the
+ * memory region. As always, @name may (optionally) contain a human readable
+ * name of this mapping (mainly for debugging purposes). The @version field
+ * is added for future scalability, and should be 1 for now.
+ *
+ * Note: at this point we just "trust" these intmem entries to contain valid
+ * physical bus addresses. these are not currently intended to be managed
+ * as host-controlled heaps, as it is much better to do that from the remote
+ * processor side.
+ */
+struct fw_rsc_intmem {
+	u32 version;
+	u32 da;
+	u32 pa;
+	u32 len;
+	u32 reserved;
+	u8 name[32];
+} __packed;
+
+/**
  * struct rproc_mem_entry - memory entry descriptor
  * @va:	virtual address
  * @dma: dma address
-- 
2.2.1

Re: [PATCH v3 0/2] couple of generic remoteproc enhancements

[Suman Anna]

Hi Ohad,

On 01/09/2015 03:21 PM, Suman Anna wrote:
> Hi Ohad,
> 
> The following is an updated patchset addressing the previous pending comments
> from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
> actually). 
> 
> The patches are mainly developed to support the WkupM3 remote processor driver
> on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
> version of Dave's WkupM3 remoteproc work [1]
> 
> The only change in v3 is on the second patch, it mainly leverages the
> memcpy_toio and memset_io functions for copying/loading code into the
> internal memory sections. An additional argument has to be added to the
> rproc_da_to_va function to make this distinction.

Any comments on this series, or can I assume that this will make it to
v3.20?

regards
Suman

> 
> [1] http://marc.info/?l=linux-omap&m=142022842323885&w=2
> 
> v2:
> http://marc.info/?l=linux-omap&m=141089879412807&w=2
> - Add explicit setting of the .has_iommu field in each of the existing
>   remoteproc platform drivers
> - Update patch description to add the usecase details for the change
>   summary
> - Fixed a minor checkpatch warning.
> 
> v1:
> http://marc.info/?l=linux-omap&m=140483657604924&w=2
> 
> Suman Anna (2):
>   remoteproc: use a flag to detect the presence of IOMMU
>   remoteproc: add support to handle internal memories
> 
>  drivers/remoteproc/da8xx_remoteproc.c      |   1 +
>  drivers/remoteproc/omap_remoteproc.c       |   5 ++
>  drivers/remoteproc/remoteproc_core.c       | 104 ++++++++++++++++++++++++-----
>  drivers/remoteproc/remoteproc_elf_loader.c |  23 +++++--
>  drivers/remoteproc/remoteproc_internal.h   |   6 +-
>  drivers/remoteproc/ste_modem_rproc.c       |   1 +
>  include/linux/remoteproc.h                 |  45 ++++++++++++-
>  7 files changed, 161 insertions(+), 24 deletions(-)
> 

Re: [PATCH v3 0/2] couple of generic remoteproc enhancements

[Suman Anna]

Hi Ohad,

On 01/09/2015 03:21 PM, Suman Anna wrote:
> Hi Ohad,
> 
> The following is an updated patchset addressing the previous pending comments
> from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
> actually). 
> 
> The patches are mainly developed to support the WkupM3 remote processor driver
> on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
> version of Dave's WkupM3 remoteproc work [1]
> 
> The only change in v3 is on the second patch, it mainly leverages the
> memcpy_toio and memset_io functions for copying/loading code into the
> internal memory sections. An additional argument has to be added to the
> rproc_da_to_va function to make this distinction.

Any comments on this series, or can I assume that this will make it to
v3.20?

regards
Suman

> 
> [1] http://marc.info/?l=linux-omap&m=142022842323885&w=2
> 
> v2:
> http://marc.info/?l=linux-omap&m=141089879412807&w=2
> - Add explicit setting of the .has_iommu field in each of the existing
>   remoteproc platform drivers
> - Update patch description to add the usecase details for the change
>   summary
> - Fixed a minor checkpatch warning.
> 
> v1:
> http://marc.info/?l=linux-omap&m=140483657604924&w=2
> 
> Suman Anna (2):
>   remoteproc: use a flag to detect the presence of IOMMU
>   remoteproc: add support to handle internal memories
> 
>  drivers/remoteproc/da8xx_remoteproc.c      |   1 +
>  drivers/remoteproc/omap_remoteproc.c       |   5 ++
>  drivers/remoteproc/remoteproc_core.c       | 104 ++++++++++++++++++++++++-----
>  drivers/remoteproc/remoteproc_elf_loader.c |  23 +++++--
>  drivers/remoteproc/remoteproc_internal.h   |   6 +-
>  drivers/remoteproc/ste_modem_rproc.c       |   1 +
>  include/linux/remoteproc.h                 |  45 ++++++++++++-
>  7 files changed, 161 insertions(+), 24 deletions(-)
> 

[PATCH v3 0/2] couple of generic remoteproc enhancements

[Suman Anna]

Hi Ohad,

On 01/09/2015 03:21 PM, Suman Anna wrote:
> Hi Ohad,
> 
> The following is an updated patchset addressing the previous pending comments
> from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
> actually). 
> 
> The patches are mainly developed to support the WkupM3 remote processor driver
> on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
> version of Dave's WkupM3 remoteproc work [1]
> 
> The only change in v3 is on the second patch, it mainly leverages the
> memcpy_toio and memset_io functions for copying/loading code into the
> internal memory sections. An additional argument has to be added to the
> rproc_da_to_va function to make this distinction.

Any comments on this series, or can I assume that this will make it to
v3.20?

regards
Suman

> 
> [1] http://marc.info/?l=linux-omap&m=142022842323885&w=2
> 
> v2:
> http://marc.info/?l=linux-omap&m=141089879412807&w=2
> - Add explicit setting of the .has_iommu field in each of the existing
>   remoteproc platform drivers
> - Update patch description to add the usecase details for the change
>   summary
> - Fixed a minor checkpatch warning.
> 
> v1:
> http://marc.info/?l=linux-omap&m=140483657604924&w=2
> 
> Suman Anna (2):
>   remoteproc: use a flag to detect the presence of IOMMU
>   remoteproc: add support to handle internal memories
> 
>  drivers/remoteproc/da8xx_remoteproc.c      |   1 +
>  drivers/remoteproc/omap_remoteproc.c       |   5 ++
>  drivers/remoteproc/remoteproc_core.c       | 104 ++++++++++++++++++++++++-----
>  drivers/remoteproc/remoteproc_elf_loader.c |  23 +++++--
>  drivers/remoteproc/remoteproc_internal.h   |   6 +-
>  drivers/remoteproc/ste_modem_rproc.c       |   1 +
>  include/linux/remoteproc.h                 |  45 ++++++++++++-
>  7 files changed, 161 insertions(+), 24 deletions(-)
> 

Re: [PATCH v3 0/2] couple of generic remoteproc enhancements

[Suman Anna]

Ohad,

> On 01/09/2015 03:21 PM, Suman Anna wrote:
>> Hi Ohad,
>>
>> The following is an updated patchset addressing the previous pending comments
>> from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
>> actually). 
>>
>> The patches are mainly developed to support the WkupM3 remote processor driver
>> on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
>> version of Dave's WkupM3 remoteproc work [1]
>>
>> The only change in v3 is on the second patch, it mainly leverages the
>> memcpy_toio and memset_io functions for copying/loading code into the
>> internal memory sections. An additional argument has to be added to the
>> rproc_da_to_va function to make this distinction.
> 
> Any comments on this series, or can I assume that this will make it to
> v3.20?

ping, want to make sure this has not fallen off your radar..

> 
> regards
> Suman
> 
>>
>> [1] http://marc.info/?l=linux-omap&m=142022842323885&w=2
>>
>> v2:
>> http://marc.info/?l=linux-omap&m=141089879412807&w=2
>> - Add explicit setting of the .has_iommu field in each of the existing
>>   remoteproc platform drivers
>> - Update patch description to add the usecase details for the change
>>   summary
>> - Fixed a minor checkpatch warning.
>>
>> v1:
>> http://marc.info/?l=linux-omap&m=140483657604924&w=2
>>
>> Suman Anna (2):
>>   remoteproc: use a flag to detect the presence of IOMMU
>>   remoteproc: add support to handle internal memories
>>
>>  drivers/remoteproc/da8xx_remoteproc.c      |   1 +
>>  drivers/remoteproc/omap_remoteproc.c       |   5 ++
>>  drivers/remoteproc/remoteproc_core.c       | 104 ++++++++++++++++++++++++-----
>>  drivers/remoteproc/remoteproc_elf_loader.c |  23 +++++--
>>  drivers/remoteproc/remoteproc_internal.h   |   6 +-
>>  drivers/remoteproc/ste_modem_rproc.c       |   1 +
>>  include/linux/remoteproc.h                 |  45 ++++++++++++-
>>  7 files changed, 161 insertions(+), 24 deletions(-)
>>
> 

Re: [PATCH v3 0/2] couple of generic remoteproc enhancements

[Suman Anna]

Ohad,

> On 01/09/2015 03:21 PM, Suman Anna wrote:
>> Hi Ohad,
>>
>> The following is an updated patchset addressing the previous pending comments
>> from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
>> actually). 
>>
>> The patches are mainly developed to support the WkupM3 remote processor driver
>> on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
>> version of Dave's WkupM3 remoteproc work [1]
>>
>> The only change in v3 is on the second patch, it mainly leverages the
>> memcpy_toio and memset_io functions for copying/loading code into the
>> internal memory sections. An additional argument has to be added to the
>> rproc_da_to_va function to make this distinction.
> 
> Any comments on this series, or can I assume that this will make it to
> v3.20?

ping, want to make sure this has not fallen off your radar..

> 
> regards
> Suman
> 
>>
>> [1] http://marc.info/?l=linux-omap&m=142022842323885&w=2
>>
>> v2:
>> http://marc.info/?l=linux-omap&m=141089879412807&w=2
>> - Add explicit setting of the .has_iommu field in each of the existing
>>   remoteproc platform drivers
>> - Update patch description to add the usecase details for the change
>>   summary
>> - Fixed a minor checkpatch warning.
>>
>> v1:
>> http://marc.info/?l=linux-omap&m=140483657604924&w=2
>>
>> Suman Anna (2):
>>   remoteproc: use a flag to detect the presence of IOMMU
>>   remoteproc: add support to handle internal memories
>>
>>  drivers/remoteproc/da8xx_remoteproc.c      |   1 +
>>  drivers/remoteproc/omap_remoteproc.c       |   5 ++
>>  drivers/remoteproc/remoteproc_core.c       | 104 ++++++++++++++++++++++++-----
>>  drivers/remoteproc/remoteproc_elf_loader.c |  23 +++++--
>>  drivers/remoteproc/remoteproc_internal.h   |   6 +-
>>  drivers/remoteproc/ste_modem_rproc.c       |   1 +
>>  include/linux/remoteproc.h                 |  45 ++++++++++++-
>>  7 files changed, 161 insertions(+), 24 deletions(-)
>>
> 

[PATCH v3 0/2] couple of generic remoteproc enhancements

[Suman Anna]

Ohad,

> On 01/09/2015 03:21 PM, Suman Anna wrote:
>> Hi Ohad,
>>
>> The following is an updated patchset addressing the previous pending comments
>> from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
>> actually). 
>>
>> The patches are mainly developed to support the WkupM3 remote processor driver
>> on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
>> version of Dave's WkupM3 remoteproc work [1]
>>
>> The only change in v3 is on the second patch, it mainly leverages the
>> memcpy_toio and memset_io functions for copying/loading code into the
>> internal memory sections. An additional argument has to be added to the
>> rproc_da_to_va function to make this distinction.
> 
> Any comments on this series, or can I assume that this will make it to
> v3.20?

ping, want to make sure this has not fallen off your radar..

> 
> regards
> Suman
> 
>>
>> [1] http://marc.info/?l=linux-omap&m=142022842323885&w=2
>>
>> v2:
>> http://marc.info/?l=linux-omap&m=141089879412807&w=2
>> - Add explicit setting of the .has_iommu field in each of the existing
>>   remoteproc platform drivers
>> - Update patch description to add the usecase details for the change
>>   summary
>> - Fixed a minor checkpatch warning.
>>
>> v1:
>> http://marc.info/?l=linux-omap&m=140483657604924&w=2
>>
>> Suman Anna (2):
>>   remoteproc: use a flag to detect the presence of IOMMU
>>   remoteproc: add support to handle internal memories
>>
>>  drivers/remoteproc/da8xx_remoteproc.c      |   1 +
>>  drivers/remoteproc/omap_remoteproc.c       |   5 ++
>>  drivers/remoteproc/remoteproc_core.c       | 104 ++++++++++++++++++++++++-----
>>  drivers/remoteproc/remoteproc_elf_loader.c |  23 +++++--
>>  drivers/remoteproc/remoteproc_internal.h   |   6 +-
>>  drivers/remoteproc/ste_modem_rproc.c       |   1 +
>>  include/linux/remoteproc.h                 |  45 ++++++++++++-
>>  7 files changed, 161 insertions(+), 24 deletions(-)
>>
> 

Re: [PATCH v3 0/2] couple of generic remoteproc enhancements

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Feb 3, 2015 at 10:55 PM, Suman Anna <s-anna@ti.com> wrote:
> > On 01/09/2015 03:21 PM, Suman Anna wrote:
> >> Hi Ohad,
> >>
> >> The following is an updated patchset addressing the previous pending comments
> >> from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
> >> actually).
> >>
> >> The patches are mainly developed to support the WkupM3 remote processor driver
> >> on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
> >> version of Dave's WkupM3 remoteproc work [1]
> >>
> >> The only change in v3 is on the second patch, it mainly leverages the
> >> memcpy_toio and memset_io functions for copying/loading code into the
> >> internal memory sections. An additional argument has to be added to the
> >> rproc_da_to_va function to make this distinction.
> >
> > Any comments on this series, or can I assume that this will make it to
> > v3.20?
>
> ping, want to make sure this has not fallen off your radar..

It has not. I'm a bit swamped so apologies for not taking care of this
faster. I believe I'll get to it next week.

Thanks,
Ohad.

[PATCH v3 0/2] couple of generic remoteproc enhancements

[Ohad Ben-Cohen]

Hi Suman,

On Tue, Feb 3, 2015 at 10:55 PM, Suman Anna <s-anna@ti.com> wrote:
> > On 01/09/2015 03:21 PM, Suman Anna wrote:
> >> Hi Ohad,
> >>
> >> The following is an updated patchset addressing the previous pending comments
> >> from v1 & v2, and are rebased onto the latest 3.19-rc3 (are rc independent
> >> actually).
> >>
> >> The patches are mainly developed to support the WkupM3 remote processor driver
> >> on TI AM335x/AM437x SoCs, and I have verified the loading using the latest
> >> version of Dave's WkupM3 remoteproc work [1]
> >>
> >> The only change in v3 is on the second patch, it mainly leverages the
> >> memcpy_toio and memset_io functions for copying/loading code into the
> >> internal memory sections. An additional argument has to be added to the
> >> rproc_da_to_va function to make this distinction.
> >
> > Any comments on this series, or can I assume that this will make it to
> > v3.20?
>
> ping, want to make sure this has not fallen off your radar..

It has not. I'm a bit swamped so apologies for not taking care of this
faster. I believe I'll get to it next week.

Thanks,
Ohad.

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

Hi Suman,

On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> A remote processor may need to load certain firmware sections into
> internal memories (eg: RAM at L1 or L2 levels) for performance or
> other reasons. Introduce a new resource type (RSC_INTMEM) and add
> an associated handler function to handle such memories. The handler
> creates a kernel mapping for the resource's 'pa' (physical address).
...
> + * rproc_handle_intmem() - handle internal memory resource entry
> + * @rproc: rproc handle
> + * @rsc: the intmem resource entry
> + * @offset: offset of the resource data in resource table
> + * @avail: size of available data (for image validation)
> + *
> + * This function will handle firmware requests for mapping a memory region
> + * internal to a remote processor into kernel. It neither allocates any
> + * physical pages, nor performs any iommu mapping, as this resource entry
> + * is primarily used for representing physical internal memories. If the
> + * internal memory region can only be accessed through an iommu, please
> + * use a devmem resource entry.
> + *
> + * These resource entries should be grouped near the carveout entries in
> + * the firmware's resource table, as other firmware entries might request
> + * placing other data objects inside these memory regions (e.g. data/code
> + * segments, trace resource entries, ...).
> + */
> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
> +                              int offset, int avail)
> +{
...
> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);

Back in the days when we developed remoteproc there was a tremendous
effort to move away from ioremap when not strictly needed.

I'd be happy if someone intimate with the related hardware could ack
that in this specific case ioremap is indeed needed. No need to review
the entire patch, or anything remoteproc, just make sure that
generally ioremap is how we want to access this internal memory.

Tony or Kevin any chance you could take a look and ack?

If ioremap is indeed the way to go, I'd also expect that we wouldn't
have to use __force here, but that's probably a minor patch cleanup.

Thanks,
Ohad.

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

Hi Suman,

On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> A remote processor may need to load certain firmware sections into
> internal memories (eg: RAM at L1 or L2 levels) for performance or
> other reasons. Introduce a new resource type (RSC_INTMEM) and add
> an associated handler function to handle such memories. The handler
> creates a kernel mapping for the resource's 'pa' (physical address).
...
> + * rproc_handle_intmem() - handle internal memory resource entry
> + * @rproc: rproc handle
> + * @rsc: the intmem resource entry
> + * @offset: offset of the resource data in resource table
> + * @avail: size of available data (for image validation)
> + *
> + * This function will handle firmware requests for mapping a memory region
> + * internal to a remote processor into kernel. It neither allocates any
> + * physical pages, nor performs any iommu mapping, as this resource entry
> + * is primarily used for representing physical internal memories. If the
> + * internal memory region can only be accessed through an iommu, please
> + * use a devmem resource entry.
> + *
> + * These resource entries should be grouped near the carveout entries in
> + * the firmware's resource table, as other firmware entries might request
> + * placing other data objects inside these memory regions (e.g. data/code
> + * segments, trace resource entries, ...).
> + */
> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
> +                              int offset, int avail)
> +{
...
> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);

Back in the days when we developed remoteproc there was a tremendous
effort to move away from ioremap when not strictly needed.

I'd be happy if someone intimate with the related hardware could ack
that in this specific case ioremap is indeed needed. No need to review
the entire patch, or anything remoteproc, just make sure that
generally ioremap is how we want to access this internal memory.

Tony or Kevin any chance you could take a look and ack?

If ioremap is indeed the way to go, I'd also expect that we wouldn't
have to use __force here, but that's probably a minor patch cleanup.

Thanks,
Ohad.

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Tony Lindgren]

* Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
> Hi Suman,
> 
> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> > A remote processor may need to load certain firmware sections into
> > internal memories (eg: RAM at L1 or L2 levels) for performance or
> > other reasons. Introduce a new resource type (RSC_INTMEM) and add
> > an associated handler function to handle such memories. The handler
> > creates a kernel mapping for the resource's 'pa' (physical address).
> ...
> > + * rproc_handle_intmem() - handle internal memory resource entry
> > + * @rproc: rproc handle
> > + * @rsc: the intmem resource entry
> > + * @offset: offset of the resource data in resource table
> > + * @avail: size of available data (for image validation)
> > + *
> > + * This function will handle firmware requests for mapping a memory region
> > + * internal to a remote processor into kernel. It neither allocates any
> > + * physical pages, nor performs any iommu mapping, as this resource entry
> > + * is primarily used for representing physical internal memories. If the
> > + * internal memory region can only be accessed through an iommu, please
> > + * use a devmem resource entry.
> > + *
> > + * These resource entries should be grouped near the carveout entries in
> > + * the firmware's resource table, as other firmware entries might request
> > + * placing other data objects inside these memory regions (e.g. data/code
> > + * segments, trace resource entries, ...).
> > + */
> > +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
> > +                              int offset, int avail)
> > +{
> ...
> > +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
> 
> Back in the days when we developed remoteproc there was a tremendous
> effort to move away from ioremap when not strictly needed.

The use of ioremap in general is just fine for drivers as long
as they access a dedicated area to the specific device. Accessing
random registers and memory in the SoC is what I'm worried about.
 
> I'd be happy if someone intimate with the related hardware could ack
> that in this specific case ioremap is indeed needed. No need to review
> the entire patch, or anything remoteproc, just make sure that
> generally ioremap is how we want to access this internal memory.
> 
> Tony or Kevin any chance you could take a look and ack?
> 
> If ioremap is indeed the way to go, I'd also expect that we wouldn't
> have to use __force here, but that's probably a minor patch cleanup.

Hmm sounds like this memory should be dedicated to the accelerator?

In that case it should use memblock to reserve that area early so
the kernel won't be accessing it at all.

If it needs to be shared between the kernel and the accelerator,
then is the remoteproc or mailbox somehow needs to coordinating the
shared access to this memory.. I think those cases should be handled
separately and not with a single interface.

Regards,

Tony

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Tony Lindgren]

* Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
> Hi Suman,
> 
> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> > A remote processor may need to load certain firmware sections into
> > internal memories (eg: RAM at L1 or L2 levels) for performance or
> > other reasons. Introduce a new resource type (RSC_INTMEM) and add
> > an associated handler function to handle such memories. The handler
> > creates a kernel mapping for the resource's 'pa' (physical address).
> ...
> > + * rproc_handle_intmem() - handle internal memory resource entry
> > + * @rproc: rproc handle
> > + * @rsc: the intmem resource entry
> > + * @offset: offset of the resource data in resource table
> > + * @avail: size of available data (for image validation)
> > + *
> > + * This function will handle firmware requests for mapping a memory region
> > + * internal to a remote processor into kernel. It neither allocates any
> > + * physical pages, nor performs any iommu mapping, as this resource entry
> > + * is primarily used for representing physical internal memories. If the
> > + * internal memory region can only be accessed through an iommu, please
> > + * use a devmem resource entry.
> > + *
> > + * These resource entries should be grouped near the carveout entries in
> > + * the firmware's resource table, as other firmware entries might request
> > + * placing other data objects inside these memory regions (e.g. data/code
> > + * segments, trace resource entries, ...).
> > + */
> > +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
> > +                              int offset, int avail)
> > +{
> ...
> > +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
> 
> Back in the days when we developed remoteproc there was a tremendous
> effort to move away from ioremap when not strictly needed.

The use of ioremap in general is just fine for drivers as long
as they access a dedicated area to the specific device. Accessing
random registers and memory in the SoC is what I'm worried about.
 
> I'd be happy if someone intimate with the related hardware could ack
> that in this specific case ioremap is indeed needed. No need to review
> the entire patch, or anything remoteproc, just make sure that
> generally ioremap is how we want to access this internal memory.
> 
> Tony or Kevin any chance you could take a look and ack?
> 
> If ioremap is indeed the way to go, I'd also expect that we wouldn't
> have to use __force here, but that's probably a minor patch cleanup.

Hmm sounds like this memory should be dedicated to the accelerator?

In that case it should use memblock to reserve that area early so
the kernel won't be accessing it at all.

If it needs to be shared between the kernel and the accelerator,
then is the remoteproc or mailbox somehow needs to coordinating the
shared access to this memory.. I think those cases should be handled
separately and not with a single interface.

Regards,

Tony

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Tony Lindgren]

* Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
> Hi Suman,
> 
> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> > A remote processor may need to load certain firmware sections into
> > internal memories (eg: RAM at L1 or L2 levels) for performance or
> > other reasons. Introduce a new resource type (RSC_INTMEM) and add
> > an associated handler function to handle such memories. The handler
> > creates a kernel mapping for the resource's 'pa' (physical address).
> ...
> > + * rproc_handle_intmem() - handle internal memory resource entry
> > + * @rproc: rproc handle
> > + * @rsc: the intmem resource entry
> > + * @offset: offset of the resource data in resource table
> > + * @avail: size of available data (for image validation)
> > + *
> > + * This function will handle firmware requests for mapping a memory region
> > + * internal to a remote processor into kernel. It neither allocates any
> > + * physical pages, nor performs any iommu mapping, as this resource entry
> > + * is primarily used for representing physical internal memories. If the
> > + * internal memory region can only be accessed through an iommu, please
> > + * use a devmem resource entry.
> > + *
> > + * These resource entries should be grouped near the carveout entries in
> > + * the firmware's resource table, as other firmware entries might request
> > + * placing other data objects inside these memory regions (e.g. data/code
> > + * segments, trace resource entries, ...).
> > + */
> > +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
> > +                              int offset, int avail)
> > +{
> ...
> > +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
> 
> Back in the days when we developed remoteproc there was a tremendous
> effort to move away from ioremap when not strictly needed.

The use of ioremap in general is just fine for drivers as long
as they access a dedicated area to the specific device. Accessing
random registers and memory in the SoC is what I'm worried about.
 
> I'd be happy if someone intimate with the related hardware could ack
> that in this specific case ioremap is indeed needed. No need to review
> the entire patch, or anything remoteproc, just make sure that
> generally ioremap is how we want to access this internal memory.
> 
> Tony or Kevin any chance you could take a look and ack?
> 
> If ioremap is indeed the way to go, I'd also expect that we wouldn't
> have to use __force here, but that's probably a minor patch cleanup.

Hmm sounds like this memory should be dedicated to the accelerator?

In that case it should use memblock to reserve that area early so
the kernel won't be accessing it at all.

If it needs to be shared between the kernel and the accelerator,
then is the remoteproc or mailbox somehow needs to coordinating the
shared access to this memory.. I think those cases should be handled
separately and not with a single interface.

Regards,

Tony

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

On 02/11/2015 02:57 PM, Tony Lindgren wrote:
> * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
>> Hi Suman,
>>
>> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
>>> A remote processor may need to load certain firmware sections into
>>> internal memories (eg: RAM at L1 or L2 levels) for performance or
>>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
>>> an associated handler function to handle such memories. The handler
>>> creates a kernel mapping for the resource's 'pa' (physical address).
>> ...
>>> + * rproc_handle_intmem() - handle internal memory resource entry
>>> + * @rproc: rproc handle
>>> + * @rsc: the intmem resource entry
>>> + * @offset: offset of the resource data in resource table
>>> + * @avail: size of available data (for image validation)
>>> + *
>>> + * This function will handle firmware requests for mapping a memory region
>>> + * internal to a remote processor into kernel. It neither allocates any
>>> + * physical pages, nor performs any iommu mapping, as this resource entry
>>> + * is primarily used for representing physical internal memories. If the
>>> + * internal memory region can only be accessed through an iommu, please
>>> + * use a devmem resource entry.
>>> + *
>>> + * These resource entries should be grouped near the carveout entries in
>>> + * the firmware's resource table, as other firmware entries might request
>>> + * placing other data objects inside these memory regions (e.g. data/code
>>> + * segments, trace resource entries, ...).
>>> + */
>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>>> +                              int offset, int avail)
>>> +{
>> ...
>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>
>> Back in the days when we developed remoteproc there was a tremendous
>> effort to move away from ioremap when not strictly needed.
> 
> The use of ioremap in general is just fine for drivers as long
> as they access a dedicated area to the specific device. Accessing
> random registers and memory in the SoC is what I'm worried about.
>  
>> I'd be happy if someone intimate with the related hardware could ack
>> that in this specific case ioremap is indeed needed. No need to review
>> the entire patch, or anything remoteproc, just make sure that
>> generally ioremap is how we want to access this internal memory.
>>
>> Tony or Kevin any chance you could take a look and ack?
>>
>> If ioremap is indeed the way to go, I'd also expect that we wouldn't
>> have to use __force here, but that's probably a minor patch cleanup.
> 
> Hmm sounds like this memory should be dedicated to the accelerator?
> 
> In that case it should use memblock to reserve that area early so
> the kernel won't be accessing it at all.

The usage here is not really on regular memory, but on internal device
memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
For the regular shared memory for vrings and vring buffers, the
remoteproc core does rely on CMA pools.

regards
Suman

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

On 02/11/2015 02:57 PM, Tony Lindgren wrote:
> * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
>> Hi Suman,
>>
>> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
>>> A remote processor may need to load certain firmware sections into
>>> internal memories (eg: RAM at L1 or L2 levels) for performance or
>>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
>>> an associated handler function to handle such memories. The handler
>>> creates a kernel mapping for the resource's 'pa' (physical address).
>> ...
>>> + * rproc_handle_intmem() - handle internal memory resource entry
>>> + * @rproc: rproc handle
>>> + * @rsc: the intmem resource entry
>>> + * @offset: offset of the resource data in resource table
>>> + * @avail: size of available data (for image validation)
>>> + *
>>> + * This function will handle firmware requests for mapping a memory region
>>> + * internal to a remote processor into kernel. It neither allocates any
>>> + * physical pages, nor performs any iommu mapping, as this resource entry
>>> + * is primarily used for representing physical internal memories. If the
>>> + * internal memory region can only be accessed through an iommu, please
>>> + * use a devmem resource entry.
>>> + *
>>> + * These resource entries should be grouped near the carveout entries in
>>> + * the firmware's resource table, as other firmware entries might request
>>> + * placing other data objects inside these memory regions (e.g. data/code
>>> + * segments, trace resource entries, ...).
>>> + */
>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>>> +                              int offset, int avail)
>>> +{
>> ...
>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>
>> Back in the days when we developed remoteproc there was a tremendous
>> effort to move away from ioremap when not strictly needed.
> 
> The use of ioremap in general is just fine for drivers as long
> as they access a dedicated area to the specific device. Accessing
> random registers and memory in the SoC is what I'm worried about.
>  
>> I'd be happy if someone intimate with the related hardware could ack
>> that in this specific case ioremap is indeed needed. No need to review
>> the entire patch, or anything remoteproc, just make sure that
>> generally ioremap is how we want to access this internal memory.
>>
>> Tony or Kevin any chance you could take a look and ack?
>>
>> If ioremap is indeed the way to go, I'd also expect that we wouldn't
>> have to use __force here, but that's probably a minor patch cleanup.
> 
> Hmm sounds like this memory should be dedicated to the accelerator?
> 
> In that case it should use memblock to reserve that area early so
> the kernel won't be accessing it at all.

The usage here is not really on regular memory, but on internal device
memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
For the regular shared memory for vrings and vring buffers, the
remoteproc core does rely on CMA pools.

regards
Suman

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [150211 14:32]:
> On 02/11/2015 02:57 PM, Tony Lindgren wrote:
> > * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
> >> Hi Suman,
> >>
> >> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> >>> A remote processor may need to load certain firmware sections into
> >>> internal memories (eg: RAM at L1 or L2 levels) for performance or
> >>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
> >>> an associated handler function to handle such memories. The handler
> >>> creates a kernel mapping for the resource's 'pa' (physical address).
> >> ...
> >>> + * rproc_handle_intmem() - handle internal memory resource entry
> >>> + * @rproc: rproc handle
> >>> + * @rsc: the intmem resource entry
> >>> + * @offset: offset of the resource data in resource table
> >>> + * @avail: size of available data (for image validation)
> >>> + *
> >>> + * This function will handle firmware requests for mapping a memory region
> >>> + * internal to a remote processor into kernel. It neither allocates any
> >>> + * physical pages, nor performs any iommu mapping, as this resource entry
> >>> + * is primarily used for representing physical internal memories. If the
> >>> + * internal memory region can only be accessed through an iommu, please
> >>> + * use a devmem resource entry.
> >>> + *
> >>> + * These resource entries should be grouped near the carveout entries in
> >>> + * the firmware's resource table, as other firmware entries might request
> >>> + * placing other data objects inside these memory regions (e.g. data/code
> >>> + * segments, trace resource entries, ...).
> >>> + */
> >>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
> >>> +                              int offset, int avail)
> >>> +{
> >> ...
> >>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
> >>
> >> Back in the days when we developed remoteproc there was a tremendous
> >> effort to move away from ioremap when not strictly needed.
> > 
> > The use of ioremap in general is just fine for drivers as long
> > as they access a dedicated area to the specific device. Accessing
> > random registers and memory in the SoC is what I'm worried about.
> >  
> >> I'd be happy if someone intimate with the related hardware could ack
> >> that in this specific case ioremap is indeed needed. No need to review
> >> the entire patch, or anything remoteproc, just make sure that
> >> generally ioremap is how we want to access this internal memory.
> >>
> >> Tony or Kevin any chance you could take a look and ack?
> >>
> >> If ioremap is indeed the way to go, I'd also expect that we wouldn't
> >> have to use __force here, but that's probably a minor patch cleanup.
> > 
> > Hmm sounds like this memory should be dedicated to the accelerator?
> > 
> > In that case it should use memblock to reserve that area early so
> > the kernel won't be accessing it at all.
> 
> The usage here is not really on regular memory, but on internal device
> memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
> For the regular shared memory for vrings and vring buffers, the
> remoteproc core does rely on CMA pools.

OK sounds like Linux needs to access it initially to load the DSP boot
code to L2RAM to get the DSP booted.

Maybe it can be done with the API provided by drivers/misc/sram.c?

You could set up the L2RAM as compatible = "mmio-sram" and then
parse the optional phandle for that in the remoteproc code, then
allocate some memory from it to load the DSP boot code and free
it.

Regards,

Tony

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [150211 14:32]:
> On 02/11/2015 02:57 PM, Tony Lindgren wrote:
> > * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
> >> Hi Suman,
> >>
> >> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> >>> A remote processor may need to load certain firmware sections into
> >>> internal memories (eg: RAM at L1 or L2 levels) for performance or
> >>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
> >>> an associated handler function to handle such memories. The handler
> >>> creates a kernel mapping for the resource's 'pa' (physical address).
> >> ...
> >>> + * rproc_handle_intmem() - handle internal memory resource entry
> >>> + * @rproc: rproc handle
> >>> + * @rsc: the intmem resource entry
> >>> + * @offset: offset of the resource data in resource table
> >>> + * @avail: size of available data (for image validation)
> >>> + *
> >>> + * This function will handle firmware requests for mapping a memory region
> >>> + * internal to a remote processor into kernel. It neither allocates any
> >>> + * physical pages, nor performs any iommu mapping, as this resource entry
> >>> + * is primarily used for representing physical internal memories. If the
> >>> + * internal memory region can only be accessed through an iommu, please
> >>> + * use a devmem resource entry.
> >>> + *
> >>> + * These resource entries should be grouped near the carveout entries in
> >>> + * the firmware's resource table, as other firmware entries might request
> >>> + * placing other data objects inside these memory regions (e.g. data/code
> >>> + * segments, trace resource entries, ...).
> >>> + */
> >>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
> >>> +                              int offset, int avail)
> >>> +{
> >> ...
> >>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
> >>
> >> Back in the days when we developed remoteproc there was a tremendous
> >> effort to move away from ioremap when not strictly needed.
> > 
> > The use of ioremap in general is just fine for drivers as long
> > as they access a dedicated area to the specific device. Accessing
> > random registers and memory in the SoC is what I'm worried about.
> >  
> >> I'd be happy if someone intimate with the related hardware could ack
> >> that in this specific case ioremap is indeed needed. No need to review
> >> the entire patch, or anything remoteproc, just make sure that
> >> generally ioremap is how we want to access this internal memory.
> >>
> >> Tony or Kevin any chance you could take a look and ack?
> >>
> >> If ioremap is indeed the way to go, I'd also expect that we wouldn't
> >> have to use __force here, but that's probably a minor patch cleanup.
> > 
> > Hmm sounds like this memory should be dedicated to the accelerator?
> > 
> > In that case it should use memblock to reserve that area early so
> > the kernel won't be accessing it at all.
> 
> The usage here is not really on regular memory, but on internal device
> memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
> For the regular shared memory for vrings and vring buffers, the
> remoteproc core does rely on CMA pools.

OK sounds like Linux needs to access it initially to load the DSP boot
code to L2RAM to get the DSP booted.

Maybe it can be done with the API provided by drivers/misc/sram.c?

You could set up the L2RAM as compatible = "mmio-sram" and then
parse the optional phandle for that in the remoteproc code, then
allocate some memory from it to load the DSP boot code and free
it.

Regards,

Tony

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

On 02/11/2015 04:48 PM, Tony Lindgren wrote:
> * Suman Anna <s-anna@ti.com> [150211 14:32]:
>> On 02/11/2015 02:57 PM, Tony Lindgren wrote:
>>> * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
>>>> Hi Suman,
>>>>
>>>> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
>>>>> A remote processor may need to load certain firmware sections into
>>>>> internal memories (eg: RAM at L1 or L2 levels) for performance or
>>>>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
>>>>> an associated handler function to handle such memories. The handler
>>>>> creates a kernel mapping for the resource's 'pa' (physical address).
>>>> ...
>>>>> + * rproc_handle_intmem() - handle internal memory resource entry
>>>>> + * @rproc: rproc handle
>>>>> + * @rsc: the intmem resource entry
>>>>> + * @offset: offset of the resource data in resource table
>>>>> + * @avail: size of available data (for image validation)
>>>>> + *
>>>>> + * This function will handle firmware requests for mapping a memory region
>>>>> + * internal to a remote processor into kernel. It neither allocates any
>>>>> + * physical pages, nor performs any iommu mapping, as this resource entry
>>>>> + * is primarily used for representing physical internal memories. If the
>>>>> + * internal memory region can only be accessed through an iommu, please
>>>>> + * use a devmem resource entry.
>>>>> + *
>>>>> + * These resource entries should be grouped near the carveout entries in
>>>>> + * the firmware's resource table, as other firmware entries might request
>>>>> + * placing other data objects inside these memory regions (e.g. data/code
>>>>> + * segments, trace resource entries, ...).
>>>>> + */
>>>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>>>>> +                              int offset, int avail)
>>>>> +{
>>>> ...
>>>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>>>
>>>> Back in the days when we developed remoteproc there was a tremendous
>>>> effort to move away from ioremap when not strictly needed.
>>>
>>> The use of ioremap in general is just fine for drivers as long
>>> as they access a dedicated area to the specific device. Accessing
>>> random registers and memory in the SoC is what I'm worried about.
>>>  
>>>> I'd be happy if someone intimate with the related hardware could ack
>>>> that in this specific case ioremap is indeed needed. No need to review
>>>> the entire patch, or anything remoteproc, just make sure that
>>>> generally ioremap is how we want to access this internal memory.
>>>>
>>>> Tony or Kevin any chance you could take a look and ack?
>>>>
>>>> If ioremap is indeed the way to go, I'd also expect that we wouldn't
>>>> have to use __force here, but that's probably a minor patch cleanup.
>>>
>>> Hmm sounds like this memory should be dedicated to the accelerator?
>>>
>>> In that case it should use memblock to reserve that area early so
>>> the kernel won't be accessing it at all.
>>
>> The usage here is not really on regular memory, but on internal device
>> memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
>> For the regular shared memory for vrings and vring buffers, the
>> remoteproc core does rely on CMA pools.
> 
> OK sounds like Linux needs to access it initially to load the DSP boot
> code to L2RAM to get the DSP booted.
> 
> Maybe it can be done with the API provided by drivers/misc/sram.c?
> 
> You could set up the L2RAM as compatible = "mmio-sram" and then
> parse the optional phandle for that in the remoteproc code, then
> allocate some memory from it to load the DSP boot code and free
> it.

Not quite the same usage, there are no implicit assumptions on managing
this memory. Isn't the SRAM driver better suited for allocating memory
using the gen_pool API. It is just regular code that is being placed
into RAM, and the linker file on the remoteproc side dictates which
portion we are using. So, the section can be anywhere based on the ELF
parsing. Further, the same RAM space can be partitioned into Cache
and/or RAM, which is usually controlled from internal processor
subsystem register programming.

regards
Suman

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

On 02/11/2015 04:48 PM, Tony Lindgren wrote:
> * Suman Anna <s-anna@ti.com> [150211 14:32]:
>> On 02/11/2015 02:57 PM, Tony Lindgren wrote:
>>> * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
>>>> Hi Suman,
>>>>
>>>> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
>>>>> A remote processor may need to load certain firmware sections into
>>>>> internal memories (eg: RAM at L1 or L2 levels) for performance or
>>>>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
>>>>> an associated handler function to handle such memories. The handler
>>>>> creates a kernel mapping for the resource's 'pa' (physical address).
>>>> ...
>>>>> + * rproc_handle_intmem() - handle internal memory resource entry
>>>>> + * @rproc: rproc handle
>>>>> + * @rsc: the intmem resource entry
>>>>> + * @offset: offset of the resource data in resource table
>>>>> + * @avail: size of available data (for image validation)
>>>>> + *
>>>>> + * This function will handle firmware requests for mapping a memory region
>>>>> + * internal to a remote processor into kernel. It neither allocates any
>>>>> + * physical pages, nor performs any iommu mapping, as this resource entry
>>>>> + * is primarily used for representing physical internal memories. If the
>>>>> + * internal memory region can only be accessed through an iommu, please
>>>>> + * use a devmem resource entry.
>>>>> + *
>>>>> + * These resource entries should be grouped near the carveout entries in
>>>>> + * the firmware's resource table, as other firmware entries might request
>>>>> + * placing other data objects inside these memory regions (e.g. data/code
>>>>> + * segments, trace resource entries, ...).
>>>>> + */
>>>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>>>>> +                              int offset, int avail)
>>>>> +{
>>>> ...
>>>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>>>
>>>> Back in the days when we developed remoteproc there was a tremendous
>>>> effort to move away from ioremap when not strictly needed.
>>>
>>> The use of ioremap in general is just fine for drivers as long
>>> as they access a dedicated area to the specific device. Accessing
>>> random registers and memory in the SoC is what I'm worried about.
>>>  
>>>> I'd be happy if someone intimate with the related hardware could ack
>>>> that in this specific case ioremap is indeed needed. No need to review
>>>> the entire patch, or anything remoteproc, just make sure that
>>>> generally ioremap is how we want to access this internal memory.
>>>>
>>>> Tony or Kevin any chance you could take a look and ack?
>>>>
>>>> If ioremap is indeed the way to go, I'd also expect that we wouldn't
>>>> have to use __force here, but that's probably a minor patch cleanup.
>>>
>>> Hmm sounds like this memory should be dedicated to the accelerator?
>>>
>>> In that case it should use memblock to reserve that area early so
>>> the kernel won't be accessing it at all.
>>
>> The usage here is not really on regular memory, but on internal device
>> memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
>> For the regular shared memory for vrings and vring buffers, the
>> remoteproc core does rely on CMA pools.
> 
> OK sounds like Linux needs to access it initially to load the DSP boot
> code to L2RAM to get the DSP booted.
> 
> Maybe it can be done with the API provided by drivers/misc/sram.c?
> 
> You could set up the L2RAM as compatible = "mmio-sram" and then
> parse the optional phandle for that in the remoteproc code, then
> allocate some memory from it to load the DSP boot code and free
> it.

Not quite the same usage, there are no implicit assumptions on managing
this memory. Isn't the SRAM driver better suited for allocating memory
using the gen_pool API. It is just regular code that is being placed
into RAM, and the linker file on the remoteproc side dictates which
portion we are using. So, the section can be anywhere based on the ELF
parsing. Further, the same RAM space can be partitioned into Cache
and/or RAM, which is usually controlled from internal processor
subsystem register programming.

regards
Suman

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [150211 16:05]:
> On 02/11/2015 04:48 PM, Tony Lindgren wrote:
> > * Suman Anna <s-anna@ti.com> [150211 14:32]:
> >> On 02/11/2015 02:57 PM, Tony Lindgren wrote:
> >>> * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
> >>>> Hi Suman,
> >>>>
> >>>> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> >>>>> A remote processor may need to load certain firmware sections into
> >>>>> internal memories (eg: RAM at L1 or L2 levels) for performance or
> >>>>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
> >>>>> an associated handler function to handle such memories. The handler
> >>>>> creates a kernel mapping for the resource's 'pa' (physical address).
> >>>> ...
> >>>>> + * rproc_handle_intmem() - handle internal memory resource entry
> >>>>> + * @rproc: rproc handle
> >>>>> + * @rsc: the intmem resource entry
> >>>>> + * @offset: offset of the resource data in resource table
> >>>>> + * @avail: size of available data (for image validation)
> >>>>> + *
> >>>>> + * This function will handle firmware requests for mapping a memory region
> >>>>> + * internal to a remote processor into kernel. It neither allocates any
> >>>>> + * physical pages, nor performs any iommu mapping, as this resource entry
> >>>>> + * is primarily used for representing physical internal memories. If the
> >>>>> + * internal memory region can only be accessed through an iommu, please
> >>>>> + * use a devmem resource entry.
> >>>>> + *
> >>>>> + * These resource entries should be grouped near the carveout entries in
> >>>>> + * the firmware's resource table, as other firmware entries might request
> >>>>> + * placing other data objects inside these memory regions (e.g. data/code
> >>>>> + * segments, trace resource entries, ...).
> >>>>> + */
> >>>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
> >>>>> +                              int offset, int avail)
> >>>>> +{
> >>>> ...
> >>>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
> >>>>
> >>>> Back in the days when we developed remoteproc there was a tremendous
> >>>> effort to move away from ioremap when not strictly needed.
> >>>
> >>> The use of ioremap in general is just fine for drivers as long
> >>> as they access a dedicated area to the specific device. Accessing
> >>> random registers and memory in the SoC is what I'm worried about.
> >>>  
> >>>> I'd be happy if someone intimate with the related hardware could ack
> >>>> that in this specific case ioremap is indeed needed. No need to review
> >>>> the entire patch, or anything remoteproc, just make sure that
> >>>> generally ioremap is how we want to access this internal memory.
> >>>>
> >>>> Tony or Kevin any chance you could take a look and ack?
> >>>>
> >>>> If ioremap is indeed the way to go, I'd also expect that we wouldn't
> >>>> have to use __force here, but that's probably a minor patch cleanup.
> >>>
> >>> Hmm sounds like this memory should be dedicated to the accelerator?
> >>>
> >>> In that case it should use memblock to reserve that area early so
> >>> the kernel won't be accessing it at all.
> >>
> >> The usage here is not really on regular memory, but on internal device
> >> memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
> >> For the regular shared memory for vrings and vring buffers, the
> >> remoteproc core does rely on CMA pools.
> > 
> > OK sounds like Linux needs to access it initially to load the DSP boot
> > code to L2RAM to get the DSP booted.
> > 
> > Maybe it can be done with the API provided by drivers/misc/sram.c?
> > 
> > You could set up the L2RAM as compatible = "mmio-sram" and then
> > parse the optional phandle for that in the remoteproc code, then
> > allocate some memory from it to load the DSP boot code and free
> > it.
> 
> Not quite the same usage, there are no implicit assumptions on managing
> this memory. Isn't the SRAM driver better suited for allocating memory
> using the gen_pool API. It is just regular code that is being placed
> into RAM, and the linker file on the remoteproc side dictates which
> portion we are using. So, the section can be anywhere based on the ELF
> parsing. Further, the same RAM space can be partitioned into Cache
> and/or RAM, which is usually controlled from internal processor
> subsystem register programming.

It still sounds like you need an API like gen_pool to allocate and
load the DSP code though? So from that point of view it's best to
use some Linux generic API.

Just guessing, but the process here is probably something like
request_firmware, configure hardware, allocate memory area,
copy firmware to memory, unallocate memory, boot m3 :)

Regards,

Tony

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [150211 16:05]:
> On 02/11/2015 04:48 PM, Tony Lindgren wrote:
> > * Suman Anna <s-anna@ti.com> [150211 14:32]:
> >> On 02/11/2015 02:57 PM, Tony Lindgren wrote:
> >>> * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
> >>>> Hi Suman,
> >>>>
> >>>> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> >>>>> A remote processor may need to load certain firmware sections into
> >>>>> internal memories (eg: RAM at L1 or L2 levels) for performance or
> >>>>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
> >>>>> an associated handler function to handle such memories. The handler
> >>>>> creates a kernel mapping for the resource's 'pa' (physical address).
> >>>> ...
> >>>>> + * rproc_handle_intmem() - handle internal memory resource entry
> >>>>> + * @rproc: rproc handle
> >>>>> + * @rsc: the intmem resource entry
> >>>>> + * @offset: offset of the resource data in resource table
> >>>>> + * @avail: size of available data (for image validation)
> >>>>> + *
> >>>>> + * This function will handle firmware requests for mapping a memory region
> >>>>> + * internal to a remote processor into kernel. It neither allocates any
> >>>>> + * physical pages, nor performs any iommu mapping, as this resource entry
> >>>>> + * is primarily used for representing physical internal memories. If the
> >>>>> + * internal memory region can only be accessed through an iommu, please
> >>>>> + * use a devmem resource entry.
> >>>>> + *
> >>>>> + * These resource entries should be grouped near the carveout entries in
> >>>>> + * the firmware's resource table, as other firmware entries might request
> >>>>> + * placing other data objects inside these memory regions (e.g. data/code
> >>>>> + * segments, trace resource entries, ...).
> >>>>> + */
> >>>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
> >>>>> +                              int offset, int avail)
> >>>>> +{
> >>>> ...
> >>>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
> >>>>
> >>>> Back in the days when we developed remoteproc there was a tremendous
> >>>> effort to move away from ioremap when not strictly needed.
> >>>
> >>> The use of ioremap in general is just fine for drivers as long
> >>> as they access a dedicated area to the specific device. Accessing
> >>> random registers and memory in the SoC is what I'm worried about.
> >>>  
> >>>> I'd be happy if someone intimate with the related hardware could ack
> >>>> that in this specific case ioremap is indeed needed. No need to review
> >>>> the entire patch, or anything remoteproc, just make sure that
> >>>> generally ioremap is how we want to access this internal memory.
> >>>>
> >>>> Tony or Kevin any chance you could take a look and ack?
> >>>>
> >>>> If ioremap is indeed the way to go, I'd also expect that we wouldn't
> >>>> have to use __force here, but that's probably a minor patch cleanup.
> >>>
> >>> Hmm sounds like this memory should be dedicated to the accelerator?
> >>>
> >>> In that case it should use memblock to reserve that area early so
> >>> the kernel won't be accessing it at all.
> >>
> >> The usage here is not really on regular memory, but on internal device
> >> memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
> >> For the regular shared memory for vrings and vring buffers, the
> >> remoteproc core does rely on CMA pools.
> > 
> > OK sounds like Linux needs to access it initially to load the DSP boot
> > code to L2RAM to get the DSP booted.
> > 
> > Maybe it can be done with the API provided by drivers/misc/sram.c?
> > 
> > You could set up the L2RAM as compatible = "mmio-sram" and then
> > parse the optional phandle for that in the remoteproc code, then
> > allocate some memory from it to load the DSP boot code and free
> > it.
> 
> Not quite the same usage, there are no implicit assumptions on managing
> this memory. Isn't the SRAM driver better suited for allocating memory
> using the gen_pool API. It is just regular code that is being placed
> into RAM, and the linker file on the remoteproc side dictates which
> portion we are using. So, the section can be anywhere based on the ELF
> parsing. Further, the same RAM space can be partitioned into Cache
> and/or RAM, which is usually controlled from internal processor
> subsystem register programming.

It still sounds like you need an API like gen_pool to allocate and
load the DSP code though? So from that point of view it's best to
use some Linux generic API.

Just guessing, but the process here is probably something like
request_firmware, configure hardware, allocate memory area,
copy firmware to memory, unallocate memory, boot m3 :)

Regards,

Tony

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

On 02/11/2015 06:18 PM, Tony Lindgren wrote:
> * Suman Anna <s-anna@ti.com> [150211 16:05]:
>> On 02/11/2015 04:48 PM, Tony Lindgren wrote:
>>> * Suman Anna <s-anna@ti.com> [150211 14:32]:
>>>> On 02/11/2015 02:57 PM, Tony Lindgren wrote:
>>>>> * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
>>>>>> Hi Suman,
>>>>>>
>>>>>> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
>>>>>>> A remote processor may need to load certain firmware sections into
>>>>>>> internal memories (eg: RAM at L1 or L2 levels) for performance or
>>>>>>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
>>>>>>> an associated handler function to handle such memories. The handler
>>>>>>> creates a kernel mapping for the resource's 'pa' (physical address).
>>>>>> ...
>>>>>>> + * rproc_handle_intmem() - handle internal memory resource entry
>>>>>>> + * @rproc: rproc handle
>>>>>>> + * @rsc: the intmem resource entry
>>>>>>> + * @offset: offset of the resource data in resource table
>>>>>>> + * @avail: size of available data (for image validation)
>>>>>>> + *
>>>>>>> + * This function will handle firmware requests for mapping a memory region
>>>>>>> + * internal to a remote processor into kernel. It neither allocates any
>>>>>>> + * physical pages, nor performs any iommu mapping, as this resource entry
>>>>>>> + * is primarily used for representing physical internal memories. If the
>>>>>>> + * internal memory region can only be accessed through an iommu, please
>>>>>>> + * use a devmem resource entry.
>>>>>>> + *
>>>>>>> + * These resource entries should be grouped near the carveout entries in
>>>>>>> + * the firmware's resource table, as other firmware entries might request
>>>>>>> + * placing other data objects inside these memory regions (e.g. data/code
>>>>>>> + * segments, trace resource entries, ...).
>>>>>>> + */
>>>>>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>>>>>>> +                              int offset, int avail)
>>>>>>> +{
>>>>>> ...
>>>>>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>>>>>
>>>>>> Back in the days when we developed remoteproc there was a tremendous
>>>>>> effort to move away from ioremap when not strictly needed.
>>>>>
>>>>> The use of ioremap in general is just fine for drivers as long
>>>>> as they access a dedicated area to the specific device. Accessing
>>>>> random registers and memory in the SoC is what I'm worried about.
>>>>>  
>>>>>> I'd be happy if someone intimate with the related hardware could ack
>>>>>> that in this specific case ioremap is indeed needed. No need to review
>>>>>> the entire patch, or anything remoteproc, just make sure that
>>>>>> generally ioremap is how we want to access this internal memory.
>>>>>>
>>>>>> Tony or Kevin any chance you could take a look and ack?
>>>>>>
>>>>>> If ioremap is indeed the way to go, I'd also expect that we wouldn't
>>>>>> have to use __force here, but that's probably a minor patch cleanup.
>>>>>
>>>>> Hmm sounds like this memory should be dedicated to the accelerator?
>>>>>
>>>>> In that case it should use memblock to reserve that area early so
>>>>> the kernel won't be accessing it at all.
>>>>
>>>> The usage here is not really on regular memory, but on internal device
>>>> memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
>>>> For the regular shared memory for vrings and vring buffers, the
>>>> remoteproc core does rely on CMA pools.
>>>
>>> OK sounds like Linux needs to access it initially to load the DSP boot
>>> code to L2RAM to get the DSP booted.
>>>
>>> Maybe it can be done with the API provided by drivers/misc/sram.c?
>>>
>>> You could set up the L2RAM as compatible = "mmio-sram" and then
>>> parse the optional phandle for that in the remoteproc code, then
>>> allocate some memory from it to load the DSP boot code and free
>>> it.
>>
>> Not quite the same usage, there are no implicit assumptions on managing
>> this memory. Isn't the SRAM driver better suited for allocating memory
>> using the gen_pool API. It is just regular code that is being placed
>> into RAM, and the linker file on the remoteproc side dictates which
>> portion we are using. So, the section can be anywhere based on the ELF
>> parsing. Further, the same RAM space can be partitioned into Cache
>> and/or RAM, which is usually controlled from internal processor
>> subsystem register programming.
> 
> It still sounds like you need an API like gen_pool to allocate and
> load the DSP code though? So from that point of view it's best to
> use some Linux generic API.
> 
> Just guessing, but the process here is probably something like
> request_firmware, configure hardware, allocate memory area,
> copy firmware to memory, unallocate memory, boot m3 :)

Yeah, atleast for the processors with MMUs, it's usually allocate
memory, program IOMMU, copy firmware, boot rproc. Memory is freed when
unloading the processor and loading a different firmware. For the cases
with internal memory, either I need an ioremap of the region for copying
the firmware sections, or as you said, allocate, copy and unallocate.
That almost always means, I have to allocate the entire region, since I
would need to usually copy the data to a specific location based on the
ELF pheader data. The sram driver also does an ioremap internally, so I
guess it can be done, and probably a bit more code for management within
the rproc core.

regards
Suman

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

On 02/11/2015 06:18 PM, Tony Lindgren wrote:
> * Suman Anna <s-anna@ti.com> [150211 16:05]:
>> On 02/11/2015 04:48 PM, Tony Lindgren wrote:
>>> * Suman Anna <s-anna@ti.com> [150211 14:32]:
>>>> On 02/11/2015 02:57 PM, Tony Lindgren wrote:
>>>>> * Ohad Ben-Cohen <ohad@wizery.com> [150210 02:14]:
>>>>>> Hi Suman,
>>>>>>
>>>>>> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
>>>>>>> A remote processor may need to load certain firmware sections into
>>>>>>> internal memories (eg: RAM at L1 or L2 levels) for performance or
>>>>>>> other reasons. Introduce a new resource type (RSC_INTMEM) and add
>>>>>>> an associated handler function to handle such memories. The handler
>>>>>>> creates a kernel mapping for the resource's 'pa' (physical address).
>>>>>> ...
>>>>>>> + * rproc_handle_intmem() - handle internal memory resource entry
>>>>>>> + * @rproc: rproc handle
>>>>>>> + * @rsc: the intmem resource entry
>>>>>>> + * @offset: offset of the resource data in resource table
>>>>>>> + * @avail: size of available data (for image validation)
>>>>>>> + *
>>>>>>> + * This function will handle firmware requests for mapping a memory region
>>>>>>> + * internal to a remote processor into kernel. It neither allocates any
>>>>>>> + * physical pages, nor performs any iommu mapping, as this resource entry
>>>>>>> + * is primarily used for representing physical internal memories. If the
>>>>>>> + * internal memory region can only be accessed through an iommu, please
>>>>>>> + * use a devmem resource entry.
>>>>>>> + *
>>>>>>> + * These resource entries should be grouped near the carveout entries in
>>>>>>> + * the firmware's resource table, as other firmware entries might request
>>>>>>> + * placing other data objects inside these memory regions (e.g. data/code
>>>>>>> + * segments, trace resource entries, ...).
>>>>>>> + */
>>>>>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>>>>>>> +                              int offset, int avail)
>>>>>>> +{
>>>>>> ...
>>>>>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>>>>>
>>>>>> Back in the days when we developed remoteproc there was a tremendous
>>>>>> effort to move away from ioremap when not strictly needed.
>>>>>
>>>>> The use of ioremap in general is just fine for drivers as long
>>>>> as they access a dedicated area to the specific device. Accessing
>>>>> random registers and memory in the SoC is what I'm worried about.
>>>>>  
>>>>>> I'd be happy if someone intimate with the related hardware could ack
>>>>>> that in this specific case ioremap is indeed needed. No need to review
>>>>>> the entire patch, or anything remoteproc, just make sure that
>>>>>> generally ioremap is how we want to access this internal memory.
>>>>>>
>>>>>> Tony or Kevin any chance you could take a look and ack?
>>>>>>
>>>>>> If ioremap is indeed the way to go, I'd also expect that we wouldn't
>>>>>> have to use __force here, but that's probably a minor patch cleanup.
>>>>>
>>>>> Hmm sounds like this memory should be dedicated to the accelerator?
>>>>>
>>>>> In that case it should use memblock to reserve that area early so
>>>>> the kernel won't be accessing it at all.
>>>>
>>>> The usage here is not really on regular memory, but on internal device
>>>> memory (eg: L2RAM within DSP which is accessible by MPU through L3 bus).
>>>> For the regular shared memory for vrings and vring buffers, the
>>>> remoteproc core does rely on CMA pools.
>>>
>>> OK sounds like Linux needs to access it initially to load the DSP boot
>>> code to L2RAM to get the DSP booted.
>>>
>>> Maybe it can be done with the API provided by drivers/misc/sram.c?
>>>
>>> You could set up the L2RAM as compatible = "mmio-sram" and then
>>> parse the optional phandle for that in the remoteproc code, then
>>> allocate some memory from it to load the DSP boot code and free
>>> it.
>>
>> Not quite the same usage, there are no implicit assumptions on managing
>> this memory. Isn't the SRAM driver better suited for allocating memory
>> using the gen_pool API. It is just regular code that is being placed
>> into RAM, and the linker file on the remoteproc side dictates which
>> portion we are using. So, the section can be anywhere based on the ELF
>> parsing. Further, the same RAM space can be partitioned into Cache
>> and/or RAM, which is usually controlled from internal processor
>> subsystem register programming.
> 
> It still sounds like you need an API like gen_pool to allocate and
> load the DSP code though? So from that point of view it's best to
> use some Linux generic API.
> 
> Just guessing, but the process here is probably something like
> request_firmware, configure hardware, allocate memory area,
> copy firmware to memory, unallocate memory, boot m3 :)

Yeah, atleast for the processors with MMUs, it's usually allocate
memory, program IOMMU, copy firmware, boot rproc. Memory is freed when
unloading the processor and loading a different firmware. For the cases
with internal memory, either I need an ioremap of the region for copying
the firmware sections, or as you said, allocate, copy and unallocate.
That almost always means, I have to allocate the entire region, since I
would need to usually copy the data to a specific location based on the
ELF pheader data. The sram driver also does an ioremap internally, so I
guess it can be done, and probably a bit more code for management within
the rproc core.

regards
Suman

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

On Wed, Feb 11, 2015 at 10:57 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>> > +                              int offset, int avail)
>> > +{
>> ...
>> > +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>
>> Back in the days when we developed remoteproc there was a tremendous
>> effort to move away from ioremap when not strictly needed.
>
> The use of ioremap in general is just fine for drivers as long
> as they access a dedicated area to the specific device. Accessing
> random registers and memory in the SoC is what I'm worried about.

Yes, the proposed interface essentially allows exactly this random
access, since the parameters to ioremap would be provided from the
user space (specifically from the resource table contained within the
firmware of the remote processor).

Thanks,
Ohad.

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

On Wed, Feb 11, 2015 at 10:57 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>> > +                              int offset, int avail)
>> > +{
>> ...
>> > +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>
>> Back in the days when we developed remoteproc there was a tremendous
>> effort to move away from ioremap when not strictly needed.
>
> The use of ioremap in general is just fine for drivers as long
> as they access a dedicated area to the specific device. Accessing
> random registers and memory in the SoC is what I'm worried about.

Yes, the proposed interface essentially allows exactly this random
access, since the parameters to ioremap would be provided from the
user space (specifically from the resource table contained within the
firmware of the remote processor).

Thanks,
Ohad.

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

Hi Ohad,

On 02/12/2015 03:09 AM, Ohad Ben-Cohen wrote:
> On Wed, Feb 11, 2015 at 10:57 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>>>> +                              int offset, int avail)
>>>> +{
>>> ...
>>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>>
>>> Back in the days when we developed remoteproc there was a tremendous
>>> effort to move away from ioremap when not strictly needed.
>>
>> The use of ioremap in general is just fine for drivers as long
>> as they access a dedicated area to the specific device. Accessing
>> random registers and memory in the SoC is what I'm worried about.
> 
> Yes, the proposed interface essentially allows exactly this random
> access, since the parameters to ioremap would be provided from the
> user space (specifically from the resource table contained within the
> firmware of the remote processor).

My original motivation was that it would only need to be added on
firmwares requiring support for loading into internal memories,
otherwise, these are something left to be managed by the software
running on the remote processor completely, and MPU will not even touch
them.

So, let me know if this is a NAK. If so, we have two options - one to go
the sram node model where each of them have to be defined separately,
and have a specific property in the rproc nodes to be able to get the
gen_pool handles. The other one is simply to define these as <reg> and
use devm_ioremap_resource() (so use DT for defining the regions instead
of a resource table entry).

regards
Suman

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

Hi Ohad,

On 02/12/2015 03:09 AM, Ohad Ben-Cohen wrote:
> On Wed, Feb 11, 2015 at 10:57 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>> +static int rproc_handle_intmem(struct rproc *rproc, struct fw_rsc_intmem *rsc,
>>>> +                              int offset, int avail)
>>>> +{
>>> ...
>>>> +       va = (__force void *)ioremap_nocache(rsc->pa, rsc->len);
>>>
>>> Back in the days when we developed remoteproc there was a tremendous
>>> effort to move away from ioremap when not strictly needed.
>>
>> The use of ioremap in general is just fine for drivers as long
>> as they access a dedicated area to the specific device. Accessing
>> random registers and memory in the SoC is what I'm worried about.
> 
> Yes, the proposed interface essentially allows exactly this random
> access, since the parameters to ioremap would be provided from the
> user space (specifically from the resource table contained within the
> firmware of the remote processor).

My original motivation was that it would only need to be added on
firmwares requiring support for loading into internal memories,
otherwise, these are something left to be managed by the software
running on the remote processor completely, and MPU will not even touch
them.

So, let me know if this is a NAK. If so, we have two options - one to go
the sram node model where each of them have to be defined separately,
and have a specific property in the rproc nodes to be able to get the
gen_pool handles. The other one is simply to define these as <reg> and
use devm_ioremap_resource() (so use DT for defining the regions instead
of a resource table entry).

regards
Suman

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

On Thu, Feb 12, 2015 at 10:54 PM, Suman Anna <s-anna@ti.com> wrote:
> My original motivation was that it would only need to be added on
> firmwares requiring support for loading into internal memories,
> otherwise, these are something left to be managed by the software
> running on the remote processor completely, and MPU will not even touch
> them.

Sure. But even if you guys will use this interface correctly, this
patch essentially exposes ioremap to user space, which is something we
generally want to avoid.

> So, let me know if this is a NAK. If so, we have two options - one to go
> the sram node model where each of them have to be defined separately,
> and have a specific property in the rproc nodes to be able to get the
> gen_pool handles. The other one is simply to define these as <reg> and
> use devm_ioremap_resource() (so use DT for defining the regions instead
> of a resource table entry).

Any approach where these regions are defined explicitly really sounds
better. If you could look into these two alternatives that would be
great.

Thanks,
Ohad.

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Ohad Ben-Cohen]

On Thu, Feb 12, 2015 at 10:54 PM, Suman Anna <s-anna@ti.com> wrote:
> My original motivation was that it would only need to be added on
> firmwares requiring support for loading into internal memories,
> otherwise, these are something left to be managed by the software
> running on the remote processor completely, and MPU will not even touch
> them.

Sure. But even if you guys will use this interface correctly, this
patch essentially exposes ioremap to user space, which is something we
generally want to avoid.

> So, let me know if this is a NAK. If so, we have two options - one to go
> the sram node model where each of them have to be defined separately,
> and have a specific property in the rproc nodes to be able to get the
> gen_pool handles. The other one is simply to define these as <reg> and
> use devm_ioremap_resource() (so use DT for defining the regions instead
> of a resource table entry).

Any approach where these regions are defined explicitly really sounds
better. If you could look into these two alternatives that would be
great.

Thanks,
Ohad.

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

Ohad,

On 02/12/2015 11:20 PM, Ohad Ben-Cohen wrote:
> On Thu, Feb 12, 2015 at 10:54 PM, Suman Anna <s-anna@ti.com> wrote:
>> My original motivation was that it would only need to be added on
>> firmwares requiring support for loading into internal memories,
>> otherwise, these are something left to be managed by the software
>> running on the remote processor completely, and MPU will not even touch
>> them.
> 
> Sure. But even if you guys will use this interface correctly, this
> patch essentially exposes ioremap to user space, which is something we
> generally want to avoid.
> 
>> So, let me know if this is a NAK. If so, we have two options - one to go
>> the sram node model where each of them have to be defined separately,
>> and have a specific property in the rproc nodes to be able to get the
>> gen_pool handles. The other one is simply to define these as <reg> and
>> use devm_ioremap_resource() (so use DT for defining the regions instead
>> of a resource table entry).
> 
> Any approach where these regions are defined explicitly really sounds
> better. If you could look into these two alternatives that would be
> great.

OK, will do. Meanwhile, can you pick up Patch 1, that is independent of
this patch.

regards
Suman

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Suman Anna]

Ohad,

On 02/12/2015 11:20 PM, Ohad Ben-Cohen wrote:
> On Thu, Feb 12, 2015 at 10:54 PM, Suman Anna <s-anna@ti.com> wrote:
>> My original motivation was that it would only need to be added on
>> firmwares requiring support for loading into internal memories,
>> otherwise, these are something left to be managed by the software
>> running on the remote processor completely, and MPU will not even touch
>> them.
> 
> Sure. But even if you guys will use this interface correctly, this
> patch essentially exposes ioremap to user space, which is something we
> generally want to avoid.
> 
>> So, let me know if this is a NAK. If so, we have two options - one to go
>> the sram node model where each of them have to be defined separately,
>> and have a specific property in the rproc nodes to be able to get the
>> gen_pool handles. The other one is simply to define these as <reg> and
>> use devm_ioremap_resource() (so use DT for defining the regions instead
>> of a resource table entry).
> 
> Any approach where these regions are defined explicitly really sounds
> better. If you could look into these two alternatives that would be
> great.

OK, will do. Meanwhile, can you pick up Patch 1, that is independent of
this patch.

regards
Suman

Re: [PATCH v3 2/2] remoteproc: add support to handle internal memories

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [150213 08:17]:
> Ohad,
> 
> On 02/12/2015 11:20 PM, Ohad Ben-Cohen wrote:
> > On Thu, Feb 12, 2015 at 10:54 PM, Suman Anna <s-anna@ti.com> wrote:
> >> My original motivation was that it would only need to be added on
> >> firmwares requiring support for loading into internal memories,
> >> otherwise, these are something left to be managed by the software
> >> running on the remote processor completely, and MPU will not even touch
> >> them.
> > 
> > Sure. But even if you guys will use this interface correctly, this
> > patch essentially exposes ioremap to user space, which is something we
> > generally want to avoid.
> > 
> >> So, let me know if this is a NAK. If so, we have two options - one to go
> >> the sram node model where each of them have to be defined separately,
> >> and have a specific property in the rproc nodes to be able to get the
> >> gen_pool handles. The other one is simply to define these as <reg> and
> >> use devm_ioremap_resource() (so use DT for defining the regions instead
> >> of a resource table entry).
> > 
> > Any approach where these regions are defined explicitly really sounds
> > better. If you could look into these two alternatives that would be
> > great.
> 
> OK, will do. Meanwhile, can you pick up Patch 1, that is independent of
> this patch.

If the memory are is hardware specific, then it should be specified in
the dts file. If some further configuration depending on the firmware
version is needed, then you can parse that from the firmware and make
sure it's contained within the hardware specific memory area defined
in the dts file. I guess in some cases module options may be also
needed.

Regards,

Tony

[PATCH v3 2/2] remoteproc: add support to handle internal memories

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [150213 08:17]:
> Ohad,
> 
> On 02/12/2015 11:20 PM, Ohad Ben-Cohen wrote:
> > On Thu, Feb 12, 2015 at 10:54 PM, Suman Anna <s-anna@ti.com> wrote:
> >> My original motivation was that it would only need to be added on
> >> firmwares requiring support for loading into internal memories,
> >> otherwise, these are something left to be managed by the software
> >> running on the remote processor completely, and MPU will not even touch
> >> them.
> > 
> > Sure. But even if you guys will use this interface correctly, this
> > patch essentially exposes ioremap to user space, which is something we
> > generally want to avoid.
> > 
> >> So, let me know if this is a NAK. If so, we have two options - one to go
> >> the sram node model where each of them have to be defined separately,
> >> and have a specific property in the rproc nodes to be able to get the
> >> gen_pool handles. The other one is simply to define these as <reg> and
> >> use devm_ioremap_resource() (so use DT for defining the regions instead
> >> of a resource table entry).
> > 
> > Any approach where these regions are defined explicitly really sounds
> > better. If you could look into these two alternatives that would be
> > great.
> 
> OK, will do. Meanwhile, can you pick up Patch 1, that is independent of
> this patch.

If the memory are is hardware specific, then it should be specified in
the dts file. If some further configuration depending on the firmware
version is needed, then you can parse that from the firmware and make
sure it's contained within the hardware specific memory area defined
in the dts file. I guess in some cases module options may be also
needed.

Regards,

Tony

Re: [PATCH v3 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Ohad Ben-Cohen]

On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> The remoteproc driver core currently relies on iommu_present() on
> the bus the device is on, to perform MMU management. However, this
> logic doesn't scale for multi-arch, especially for processors that
> do not have an IOMMU. Replace this logic instead by using a h/w
> capability flag for the presence of IOMMU in the rproc structure.
>
> This issue is seen on OMAP platforms when trying to add a remoteproc
> driver for a small Cortex M3 called the WkupM3 used for suspend /
> resume management on TI AM335/AM437x SoCs. This processor does not
> have an MMU. Same is the case with another processor subsystem
> PRU-ICSS on AM335/AM437x. All these are platform devices, and the
> current iommu_present check will not scale for the same kernel image
> to support OMAP4/OMAP5 and AM335/AM437x.
>
> The existing platform implementation drivers - OMAP remoteproc, STE
> Modem remoteproc and DA8xx remoteproc, are updated as well to properly
> configure the newly added rproc field.
>
> Cc: Robert Tivy <rtivy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Suman Anna <s-anna@ti.com>

Applied to remoteproc's for-next branch.

Thanks,
Ohad.

[PATCH v3 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Ohad Ben-Cohen]

On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
> The remoteproc driver core currently relies on iommu_present() on
> the bus the device is on, to perform MMU management. However, this
> logic doesn't scale for multi-arch, especially for processors that
> do not have an IOMMU. Replace this logic instead by using a h/w
> capability flag for the presence of IOMMU in the rproc structure.
>
> This issue is seen on OMAP platforms when trying to add a remoteproc
> driver for a small Cortex M3 called the WkupM3 used for suspend /
> resume management on TI AM335/AM437x SoCs. This processor does not
> have an MMU. Same is the case with another processor subsystem
> PRU-ICSS on AM335/AM437x. All these are platform devices, and the
> current iommu_present check will not scale for the same kernel image
> to support OMAP4/OMAP5 and AM335/AM437x.
>
> The existing platform implementation drivers - OMAP remoteproc, STE
> Modem remoteproc and DA8xx remoteproc, are updated as well to properly
> configure the newly added rproc field.
>
> Cc: Robert Tivy <rtivy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Suman Anna <s-anna@ti.com>

Applied to remoteproc's for-next branch.

Thanks,
Ohad.

Re: [PATCH v3 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Suman Anna]

On 03/12/2015 04:04 AM, Ohad Ben-Cohen wrote:
> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
>> The remoteproc driver core currently relies on iommu_present() on
>> the bus the device is on, to perform MMU management. However, this
>> logic doesn't scale for multi-arch, especially for processors that
>> do not have an IOMMU. Replace this logic instead by using a h/w
>> capability flag for the presence of IOMMU in the rproc structure.
>>
>> This issue is seen on OMAP platforms when trying to add a remoteproc
>> driver for a small Cortex M3 called the WkupM3 used for suspend /
>> resume management on TI AM335/AM437x SoCs. This processor does not
>> have an MMU. Same is the case with another processor subsystem
>> PRU-ICSS on AM335/AM437x. All these are platform devices, and the
>> current iommu_present check will not scale for the same kernel image
>> to support OMAP4/OMAP5 and AM335/AM437x.
>>
>> The existing platform implementation drivers - OMAP remoteproc, STE
>> Modem remoteproc and DA8xx remoteproc, are updated as well to properly
>> configure the newly added rproc field.
>>
>> Cc: Robert Tivy <rtivy@ti.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
> 
> Applied to remoteproc's for-next branch.
> 

Thanks Ohad. Can you pick up the minor checkpatch fixes I posted as well
for 4.1?

regards
Suman

[PATCH v3 1/2] remoteproc: use a flag to detect the presence of IOMMU

[Suman Anna]

On 03/12/2015 04:04 AM, Ohad Ben-Cohen wrote:
> On Fri, Jan 9, 2015 at 11:21 PM, Suman Anna <s-anna@ti.com> wrote:
>> The remoteproc driver core currently relies on iommu_present() on
>> the bus the device is on, to perform MMU management. However, this
>> logic doesn't scale for multi-arch, especially for processors that
>> do not have an IOMMU. Replace this logic instead by using a h/w
>> capability flag for the presence of IOMMU in the rproc structure.
>>
>> This issue is seen on OMAP platforms when trying to add a remoteproc
>> driver for a small Cortex M3 called the WkupM3 used for suspend /
>> resume management on TI AM335/AM437x SoCs. This processor does not
>> have an MMU. Same is the case with another processor subsystem
>> PRU-ICSS on AM335/AM437x. All these are platform devices, and the
>> current iommu_present check will not scale for the same kernel image
>> to support OMAP4/OMAP5 and AM335/AM437x.
>>
>> The existing platform implementation drivers - OMAP remoteproc, STE
>> Modem remoteproc and DA8xx remoteproc, are updated as well to properly
>> configure the newly added rproc field.
>>
>> Cc: Robert Tivy <rtivy@ti.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
> 
> Applied to remoteproc's for-next branch.
> 

Thanks Ohad. Can you pick up the minor checkpatch fixes I posted as well
for 4.1?

regards
Suman