[PATCH 0/7] remoteproc: Fixes for memoy carveout management

[PATCH 0/7] remoteproc: Fixes for memoy carveout management

[Loic Pallardy]

These patches fix the comments sent on remoteproc mailing
list after acceptation of memory carveout patch series [1].

In few words, series corrects:
- memory carveout allocation for rproc without iommu
- rproc_da_to_va and trace buffer access to take into account
  late carveout allocation
- resource table cast by adding warning messages

Regards,
Loic

[1] https://lkml.org/lkml/2018/7/27/612

Loic Pallardy (7):
  remoteproc: correct rproc_mem_entry_init() comments
  remoteproc: fix rproc_da_to_va in case of unallocated carveout
  remoteproc: fix rproc_alloc_carveout() bad variable cast
  remoteproc: add warning on resource table cast
  remoteproc: fix rproc_alloc_carveout() for rproc with iommu domain
  remoteproc: fix trace buffer va initialization
  remoteproc: fix rproc_check_carveout_da() returned error and comments

 drivers/remoteproc/remoteproc_core.c     | 91 +++++++++++++++++++-------------
 drivers/remoteproc/remoteproc_debugfs.c  | 21 ++++++--
 drivers/remoteproc/remoteproc_internal.h |  9 +++-
 3 files changed, 78 insertions(+), 43 deletions(-)

-- 
2.7.4

[PATCH 1/7] remoteproc: correct rproc_mem_entry_init() comments

[Loic Pallardy]

Add alloc parameter description and correct comment
about release one.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 54ec38fc5dca..c1a66e25b173 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -900,7 +900,8 @@ EXPORT_SYMBOL(rproc_add_carveout);
  * @dma: dma address
  * @len: memory carveout length
  * @da: device address
- * @release: memory carveout function
+ * @alloc: memory carveout allocation function
+ * @release: memory carveout release function
  * @name: carveout name
  *
  * This function allocates a rproc_mem_entry struct and fill it with parameters
-- 
2.7.4

[PATCH 2/7] remoteproc: fix rproc_da_to_va in case of unallocated carveout

[Loic Pallardy]

With introduction of rproc_alloc_registered_carveouts() which
delays carveout allocation just before the start of the remote
processor, rproc_da_to_va() could be called before all carveouts
are allocated.
This patch adds a check in rproc_da_to_va() to return NULL if
carveout is not allocated.

Fixes: d7c51706d095 ("remoteproc: add alloc ops in rproc_mem_entry struct")

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index c1a66e25b173..28df71cb3fef 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -204,6 +204,10 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
 	list_for_each_entry(carveout, &rproc->carveouts, node) {
 		int offset = da - carveout->da;
 
+		/*  Verify that carveout is allocated */
+		if (!carveout->va)
+			continue;
+
 		/* try next carveout if da is too small */
 		if (offset < 0)
 			continue;
-- 
2.7.4

[PATCH 3/7] remoteproc: fix rproc_alloc_carveout() bad variable cast

[Loic Pallardy]

As dma member of struct rproc_mem_entry is dma_addr_t, no
need to cast in u32.

Fixes: d7c51706d095 ("remoteproc: add alloc ops in rproc_mem_entry struct")

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 28df71cb3fef..18a1bbf820c9 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -775,7 +775,7 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 		mem->da = (u32)dma;
 	}
 
-	mem->dma = (u32)dma;
+	mem->dma = dma;
 	mem->va = va;
 
 	return 0;
-- 
2.7.4

[PATCH 4/7] remoteproc: add warning on resource table cast

[Loic Pallardy]

Today resource table supports only 32bit address fields.
This is not compliant with 64bit platform for which addresses
are cast in 32bit.
This patch adds warn messages when address cast is done.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 18a1bbf820c9..61c954bd695e 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -772,6 +772,10 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 		dev_dbg(dev, "carveout mapped 0x%x to %pad\n",
 			mem->da, &dma);
 	} else {
+		/* Update device address as undefined by requester */
+		if (sizeof(dma_addr_t) > sizeof(u32))
+			dev_warn(dev, "DMA address cast in 32bit to fit resource table format\n");
+
 		mem->da = (u32)dma;
 	}
 
@@ -1150,6 +1154,10 @@ static int rproc_alloc_registered_carveouts(struct rproc *rproc)
 			 */
 
 			/* Use va if defined else dma to generate pa */
+			if (sizeof(dma_addr_t) > sizeof(u32) ||
+			    sizeof(phys_addr_t) > sizeof(u32))
+				dev_warn(dev, "Physical address cast in 32bit to fit resource table format\n");
+
 			if (entry->va)
 				rsc->pa = (u32)rproc_va_to_pa(entry->va);
 			else
-- 
2.7.4

Re: [PATCH 4/7] remoteproc: add warning on resource table cast

[Arnd Bergmann]

On Thu, Nov 29, 2018 at 10:31 PM Loic Pallardy <loic.pallardy@st.com> wrote:
>
> Today resource table supports only 32bit address fields.
> This is not compliant with 64bit platform for which addresses
> are cast in 32bit.
> This patch adds warn messages when address cast is done.
>
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  drivers/remoteproc/remoteproc_core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 18a1bbf820c9..61c954bd695e 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -772,6 +772,10 @@ static int rproc_alloc_carveout(struct rproc *rproc,
>                 dev_dbg(dev, "carveout mapped 0x%x to %pad\n",
>                         mem->da, &dma);
>         } else {
> +               /* Update device address as undefined by requester */
> +               if (sizeof(dma_addr_t) > sizeof(u32))
> +                       dev_warn(dev, "DMA address cast in 32bit to fit resource table format\n");
> +
>                 mem->da = (u32)dma;
>         }

I think this patch is wrong: sizeof(dma_addr_t) is defined to be large enough
to support any machine that the kernel can run on. If you build a kernel that
happens to work on an ARM32 platform with LPAE enabled, then this will
warn every time, even if you are running on a machine that only has a 32-bit
address space.

     Arnd

RE: [PATCH 4/7] remoteproc: add warning on resource table cast

[Loic PALLARDY]

> -----Original Message-----
> From: Arnd Bergmann <arnd@arndb.de>
> Sent: vendredi 30 novembre 2018 15:33
> To: Loic PALLARDY <loic.pallardy@st.com>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>; Ohad Ben-Cohen
> <ohad@wizery.com>; linux-remoteproc@vger.kernel.org; Linux Kernel
> Mailing List <linux-kernel@vger.kernel.org>; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Benjamin Gaignard
> <benjamin.gaignard@linaro.org>; Suman Anna <s-anna@ti.com>
> Subject: Re: [PATCH 4/7] remoteproc: add warning on resource table cast
> 
> On Thu, Nov 29, 2018 at 10:31 PM Loic Pallardy <loic.pallardy@st.com> wrote:
> >
> > Today resource table supports only 32bit address fields.
> > This is not compliant with 64bit platform for which addresses
> > are cast in 32bit.
> > This patch adds warn messages when address cast is done.
> >
> > Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> > ---
> >  drivers/remoteproc/remoteproc_core.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> > index 18a1bbf820c9..61c954bd695e 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -772,6 +772,10 @@ static int rproc_alloc_carveout(struct rproc *rproc,
> >                 dev_dbg(dev, "carveout mapped 0x%x to %pad\n",
> >                         mem->da, &dma);
> >         } else {
> > +               /* Update device address as undefined by requester */
> > +               if (sizeof(dma_addr_t) > sizeof(u32))
> > +                       dev_warn(dev, "DMA address cast in 32bit to fit resource table
> format\n");
> > +
> >                 mem->da = (u32)dma;
> >         }
> 
> I think this patch is wrong: sizeof(dma_addr_t) is defined to be large enough
> to support any machine that the kernel can run on. If you build a kernel that
> happens to work on an ARM32 platform with LPAE enabled, then this will
> warn every time, even if you are running on a machine that only has a 32-bit
> address space.

I suppose if LPAE is enabled, address space is larger than 32bit.
But I agree it is possible to display a warn only when some information will be lost
due to cast.
I'll propose a new version testing dma address itself and displaying warn only if above 4GB.

Regards,
Loic
> 
>      Arnd

[PATCH 5/7] remoteproc: fix rproc_alloc_carveout() for rproc with iommu domain

[Loic Pallardy]

Correct remoteproc core behavior when memory carveout device
address is fixed in resource table and rproc device doesn't have
associated IOMMU.
Current returned error is breaking legacy on TI platforms.
This patch restores previous behavior. It adds a warn message when
allocation doesn't fit carveout request, but doesn't stop rproc_start()
sequence anymore.

Fixes: 3bc8140b157c ("remoteproc: configure IOMMU only if device address requested")

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 61c954bd695e..f19bc6961e40 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -719,6 +719,18 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 	dev_dbg(dev, "carveout va %pK, dma %pad, len 0x%x\n",
 		va, &dma, mem->len);
 
+	if (mem->da != FW_RSC_ADDR_ANY && !rproc->domain) {
+		/*
+		 * Check requested da is equal to dma address
+		 * and print a warn message in case of missalignment.
+		 * Don't stop rproc_start sequence as coprocessor may
+		 * build pa to da translation on its side.
+		 */
+		if (mem->da != (u32)dma)
+			dev_warn(dev->parent,
+				 "Allocated carveout doesn't fit device address request\n");
+	}
+
 	/*
 	 * Ok, this is non-standard.
 	 *
@@ -736,15 +748,7 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 	 * to use the iommu-based DMA API: we expect 'dma' to contain the
 	 * physical address in this case.
 	 */
-
-	if (mem->da != FW_RSC_ADDR_ANY) {
-		if (!rproc->domain) {
-			dev_err(dev->parent,
-				"Bad carveout rsc configuration\n");
-			ret = -ENOMEM;
-			goto dma_free;
-		}
-
+	if (mem->da != FW_RSC_ADDR_ANY && rproc->domain) {
 		mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
 		if (!mapping) {
 			ret = -ENOMEM;
@@ -771,7 +775,9 @@ static int rproc_alloc_carveout(struct rproc *rproc,
 
 		dev_dbg(dev, "carveout mapped 0x%x to %pad\n",
 			mem->da, &dma);
-	} else {
+	}
+
+	if (mem->da == FW_RSC_ADDR_ANY) {
 		/* Update device address as undefined by requester */
 		if (sizeof(dma_addr_t) > sizeof(u32))
 			dev_warn(dev, "DMA address cast in 32bit to fit resource table format\n");
-- 
2.7.4

[PATCH 6/7] remoteproc: fix trace buffer va initialization

[Loic Pallardy]

With rproc_alloc_registered_carveouts() introduction, carveouts are
allocated after resource table parsing.
rproc_da_to_va() may return NULL at trace resource registering.
This patch modifies trace debufs registering to provide device address
(da) instead of va.
da to va translation is done at each trace buffer access
through debugfs interface.

Fixes: d7c51706d095 ("remoteproc: add alloc ops in rproc_mem_entry struct")

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c     | 26 ++++++++++----------------
 drivers/remoteproc/remoteproc_debugfs.c  | 21 +++++++++++++++++----
 drivers/remoteproc/remoteproc_internal.h |  9 ++++++++-
 3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index f19bc6961e40..9dbcc16f8782 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -562,9 +562,8 @@ void rproc_vdev_release(struct kref *ref)
 static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
 			      int offset, int avail)
 {
-	struct rproc_mem_entry *trace;
+	struct rproc_debug_trace *trace;
 	struct device *dev = &rproc->dev;
-	void *ptr;
 	char name[15];
 
 	if (sizeof(*rsc) > avail) {
@@ -578,28 +577,23 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
 		return -EINVAL;
 	}
 
-	/* what's the kernel address of this resource ? */
-	ptr = rproc_da_to_va(rproc, rsc->da, rsc->len);
-	if (!ptr) {
-		dev_err(dev, "erroneous trace resource entry\n");
-		return -EINVAL;
-	}
-
 	trace = kzalloc(sizeof(*trace), GFP_KERNEL);
 	if (!trace)
 		return -ENOMEM;
 
 	/* set the trace buffer dma properties */
-	trace->len = rsc->len;
-	trace->va = ptr;
+	trace->trace_mem.len = rsc->len;
+	trace->trace_mem.da = rsc->da;
+
+	/* set pointer on rproc device */
+	trace->rproc = rproc;
 
 	/* make sure snprintf always null terminates, even if truncating */
 	snprintf(name, sizeof(name), "trace%d", rproc->num_traces);
 
 	/* create the debugfs entry */
-	trace->priv = rproc_create_trace_file(name, rproc, trace);
-	if (!trace->priv) {
-		trace->va = NULL;
+	trace->tfile = rproc_create_trace_file(name, rproc, trace);
+	if (!trace->tfile) {
 		kfree(trace);
 		return -EINVAL;
 	}
@@ -608,8 +602,8 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
 
 	rproc->num_traces++;
 
-	dev_dbg(dev, "%s added: va %pK, da 0x%x, len 0x%x\n",
-		name, ptr, rsc->da, rsc->len);
+	dev_dbg(dev, "%s added: da 0x%x, len 0x%x\n",
+		name, rsc->da, rsc->len);
 
 	return 0;
 }
diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c
index e90135c64af0..11240b4d0a91 100644
--- a/drivers/remoteproc/remoteproc_debugfs.c
+++ b/drivers/remoteproc/remoteproc_debugfs.c
@@ -47,10 +47,23 @@ static struct dentry *rproc_dbg;
 static ssize_t rproc_trace_read(struct file *filp, char __user *userbuf,
 				size_t count, loff_t *ppos)
 {
-	struct rproc_mem_entry *trace = filp->private_data;
-	int len = strnlen(trace->va, trace->len);
+	struct rproc_debug_trace *data = filp->private_data;
+	struct rproc_mem_entry *trace = &data->trace_mem;
+	void *va;
+	char buf[100];
+	int len;
+
+	va = rproc_da_to_va(data->rproc, trace->da, trace->len);
+
+	if (!va) {
+		len = scnprintf(buf, sizeof(buf), "Trace %s not available\n",
+				trace->name);
+		va = buf;
+	} else {
+		len = strnlen(va, trace->len);
+	}
 
-	return simple_read_from_buffer(userbuf, count, ppos, trace->va, len);
+	return simple_read_from_buffer(userbuf, count, ppos, va, len);
 }
 
 static const struct file_operations trace_rproc_ops = {
@@ -288,7 +301,7 @@ void rproc_remove_trace_file(struct dentry *tfile)
 }
 
 struct dentry *rproc_create_trace_file(const char *name, struct rproc *rproc,
-				       struct rproc_mem_entry *trace)
+				       struct rproc_debug_trace *trace)
 {
 	struct dentry *tfile;
 
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index f6cad243d7ca..7d8936688164 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -25,6 +25,13 @@
 
 struct rproc;
 
+struct rproc_debug_trace {
+	struct rproc *rproc;
+	struct dentry *tfile;
+	struct list_head node;
+	struct rproc_mem_entry trace_mem;
+};
+
 /* from remoteproc_core.c */
 void rproc_release(struct kref *kref);
 irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
@@ -37,7 +44,7 @@ void rproc_remove_virtio_dev(struct rproc_vdev *rvdev);
 /* from remoteproc_debugfs.c */
 void rproc_remove_trace_file(struct dentry *tfile);
 struct dentry *rproc_create_trace_file(const char *name, struct rproc *rproc,
-				       struct rproc_mem_entry *trace);
+				       struct rproc_debug_trace *trace);
 void rproc_delete_debug_dir(struct rproc *rproc);
 void rproc_create_debug_dir(struct rproc *rproc);
 void rproc_init_debugfs(void);
-- 
2.7.4

RE: [PATCH 6/7] remoteproc: fix trace buffer va initialization

[Loic PALLARDY]

> -----Original Message-----
> From: Loic PALLARDY
> Sent: jeudi 29 novembre 2018 22:29
> To: bjorn.andersson@linaro.org; ohad@wizery.com
> Cc: linux-remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org;
> Arnaud POULIQUEN <arnaud.pouliquen@st.com>;
> benjamin.gaignard@linaro.org; s-anna@ti.com; Loic PALLARDY
> <loic.pallardy@st.com>
> Subject: [PATCH 6/7] remoteproc: fix trace buffer va initialization
> 
> With rproc_alloc_registered_carveouts() introduction, carveouts are
> allocated after resource table parsing.
> rproc_da_to_va() may return NULL at trace resource registering.
> This patch modifies trace debufs registering to provide device address
> (da) instead of va.
> da to va translation is done at each trace buffer access
> through debugfs interface.
> 
> Fixes: d7c51706d095 ("remoteproc: add alloc ops in rproc_mem_entry
> struct")
> 
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  drivers/remoteproc/remoteproc_core.c     | 26 ++++++++++----------------
>  drivers/remoteproc/remoteproc_debugfs.c  | 21 +++++++++++++++++----
>  drivers/remoteproc/remoteproc_internal.h |  9 ++++++++-
>  3 files changed, 35 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index f19bc6961e40..9dbcc16f8782 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -562,9 +562,8 @@ void rproc_vdev_release(struct kref *ref)
>  static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc,
>  			      int offset, int avail)
>  {
> -	struct rproc_mem_entry *trace;
> +	struct rproc_debug_trace *trace;
>  	struct device *dev = &rproc->dev;
> -	void *ptr;
>  	char name[15];
> 
>  	if (sizeof(*rsc) > avail) {
> @@ -578,28 +577,23 @@ static int rproc_handle_trace(struct rproc *rproc,
> struct fw_rsc_trace *rsc,
>  		return -EINVAL;
>  	}
> 
> -	/* what's the kernel address of this resource ? */
> -	ptr = rproc_da_to_va(rproc, rsc->da, rsc->len);
> -	if (!ptr) {
> -		dev_err(dev, "erroneous trace resource entry\n");
> -		return -EINVAL;
> -	}
> -
>  	trace = kzalloc(sizeof(*trace), GFP_KERNEL);
>  	if (!trace)
>  		return -ENOMEM;
> 
>  	/* set the trace buffer dma properties */
> -	trace->len = rsc->len;
> -	trace->va = ptr;
> +	trace->trace_mem.len = rsc->len;
> +	trace->trace_mem.da = rsc->da;
> +
> +	/* set pointer on rproc device */
> +	trace->rproc = rproc;
> 
>  	/* make sure snprintf always null terminates, even if truncating */
>  	snprintf(name, sizeof(name), "trace%d", rproc->num_traces);
> 
>  	/* create the debugfs entry */
> -	trace->priv = rproc_create_trace_file(name, rproc, trace);
> -	if (!trace->priv) {
> -		trace->va = NULL;
> +	trace->tfile = rproc_create_trace_file(name, rproc, trace);
> +	if (!trace->tfile) {
>  		kfree(trace);
>  		return -EINVAL;
>  	}
> @@ -608,8 +602,8 @@ static int rproc_handle_trace(struct rproc *rproc,
> struct fw_rsc_trace *rsc,
> 
>  	rproc->num_traces++;
> 
> -	dev_dbg(dev, "%s added: va %pK, da 0x%x, len 0x%x\n",
> -		name, ptr, rsc->da, rsc->len);
> +	dev_dbg(dev, "%s added: da 0x%x, len 0x%x\n",
> +		name, rsc->da, rsc->len);
> 
>  	return 0;
>  }

rproc_resource_cleanup() modification not included in this patch. (lost during rebase)
I'll post a v2.

Regards,
Loic

> diff --git a/drivers/remoteproc/remoteproc_debugfs.c
> b/drivers/remoteproc/remoteproc_debugfs.c
> index e90135c64af0..11240b4d0a91 100644
> --- a/drivers/remoteproc/remoteproc_debugfs.c
> +++ b/drivers/remoteproc/remoteproc_debugfs.c
> @@ -47,10 +47,23 @@ static struct dentry *rproc_dbg;
>  static ssize_t rproc_trace_read(struct file *filp, char __user *userbuf,
>  				size_t count, loff_t *ppos)
>  {
> -	struct rproc_mem_entry *trace = filp->private_data;
> -	int len = strnlen(trace->va, trace->len);
> +	struct rproc_debug_trace *data = filp->private_data;
> +	struct rproc_mem_entry *trace = &data->trace_mem;
> +	void *va;
> +	char buf[100];
> +	int len;
> +
> +	va = rproc_da_to_va(data->rproc, trace->da, trace->len);
> +
> +	if (!va) {
> +		len = scnprintf(buf, sizeof(buf), "Trace %s not available\n",
> +				trace->name);
> +		va = buf;
> +	} else {
> +		len = strnlen(va, trace->len);
> +	}
> 
> -	return simple_read_from_buffer(userbuf, count, ppos, trace->va,
> len);
> +	return simple_read_from_buffer(userbuf, count, ppos, va, len);
>  }
> 
>  static const struct file_operations trace_rproc_ops = {
> @@ -288,7 +301,7 @@ void rproc_remove_trace_file(struct dentry *tfile)
>  }
> 
>  struct dentry *rproc_create_trace_file(const char *name, struct rproc
> *rproc,
> -				       struct rproc_mem_entry *trace)
> +				       struct rproc_debug_trace *trace)
>  {
>  	struct dentry *tfile;
> 
> diff --git a/drivers/remoteproc/remoteproc_internal.h
> b/drivers/remoteproc/remoteproc_internal.h
> index f6cad243d7ca..7d8936688164 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -25,6 +25,13 @@
> 
>  struct rproc;
> 
> +struct rproc_debug_trace {
> +	struct rproc *rproc;
> +	struct dentry *tfile;
> +	struct list_head node;
> +	struct rproc_mem_entry trace_mem;
> +};
> +
>  /* from remoteproc_core.c */
>  void rproc_release(struct kref *kref);
>  irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int vq_id);
> @@ -37,7 +44,7 @@ void rproc_remove_virtio_dev(struct rproc_vdev
> *rvdev);
>  /* from remoteproc_debugfs.c */
>  void rproc_remove_trace_file(struct dentry *tfile);
>  struct dentry *rproc_create_trace_file(const char *name, struct rproc
> *rproc,
> -				       struct rproc_mem_entry *trace);
> +				       struct rproc_debug_trace *trace);
>  void rproc_delete_debug_dir(struct rproc *rproc);
>  void rproc_create_debug_dir(struct rproc *rproc);
>  void rproc_init_debugfs(void);
> --
> 2.7.4

[PATCH 7/7] remoteproc: fix rproc_check_carveout_da() returned error and comments

[Loic Pallardy]

Fix typo in comments.
Change returned error from ENOMEM to EINVAL as
not dealing with memory allocation.
Remove carveout forced da update and return an error
when no configuration match

Fixes: c874bf59add0 ("remoteproc: add helper function to check carveout device address")

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/remoteproc_core.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 9dbcc16f8782..b69ce5bff732 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -276,25 +276,27 @@ rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...)
  * @len: associated area size
  *
  * This function is a helper function to verify requested device area (couple
- * da, len) is part of specified carevout.
+ * da, len) is part of specified carveout.
+ * If da is not set (defined as FW_RSC_ADDR_ANY), only requested length is
+ * checked.
  *
- * Return: 0 if carveout match request else -ENOMEM
+ * Return: 0 if carveout matches request else error
  */
-int rproc_check_carveout_da(struct rproc *rproc, struct rproc_mem_entry *mem,
-			    u32 da, u32 len)
+static int rproc_check_carveout_da(struct rproc *rproc,
+				   struct rproc_mem_entry *mem, u32 da, u32 len)
 {
 	struct device *dev = &rproc->dev;
-	int delta = 0;
+	int delta;
 
 	/* Check requested resource length */
 	if (len > mem->len) {
 		dev_err(dev, "Registered carveout doesn't fit len request\n");
-		return -ENOMEM;
+		return -EINVAL;
 	}
 
 	if (da != FW_RSC_ADDR_ANY && mem->da == FW_RSC_ADDR_ANY) {
-		/* Update existing carveout da */
-		mem->da = da;
+		/* Address doesn't match registered carveout configuration */
+		return -EINVAL;
 	} else if (da != FW_RSC_ADDR_ANY && mem->da != FW_RSC_ADDR_ANY) {
 		delta = da - mem->da;
 
@@ -302,13 +304,13 @@ int rproc_check_carveout_da(struct rproc *rproc, struct rproc_mem_entry *mem,
 		if (delta < 0) {
 			dev_err(dev,
 				"Registered carveout doesn't fit da request\n");
-			return -ENOMEM;
+			return -EINVAL;
 		}
 
 		if (delta + len > mem->len) {
 			dev_err(dev,
 				"Registered carveout doesn't fit len request\n");
-			return -ENOMEM;
+			return -EINVAL;
 		}
 	}
 
-- 
2.7.4