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

Groups > linux.kernel > #1456249 > unrolled thread

[PATCH 0/9] remoteproc: Allow platform-specific drivers to request resources

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/9] remoteproc: Allow platform-specific drivers to request resources Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
    [PATCH 4/9] remoteproc: core: New API to add new resources to the resource table Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
      Re: [PATCH 4/9] remoteproc: core: New API to add new resources to  the resource table Lee Jones <lee.jones@linaro.org> - 2016-08-04 16:10 +0200
      Re: [PATCH 4/9] remoteproc: core: New API to add new resources to the  resource table loic pallardy <loic.pallardy@st.com> - 2016-08-08 15:50 +0200
        Re: [PATCH 4/9] remoteproc: core: New API to add new resources to  the resource table Lee Jones <lee.jones@linaro.org> - 2016-08-09 14:50 +0200
    [PATCH 9/9] remoteproc: core: Support empty resource tables Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
      Re: [PATCH 9/9] remoteproc: core: Support empty resource tables Lee Jones <lee.jones@linaro.org> - 2016-08-05 09:40 +0200
    [PATCH 6/9] remoteproc: core: Add function to append a new resource table entry Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
    [PATCH 7/9] remoteproc: core: Add function to over-ride current resource table Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
      Re: [PATCH 7/9] remoteproc: core: Add function to over-ride current  resource table loic pallardy <loic.pallardy@st.com> - 2016-08-08 15:50 +0200
        Re: [PATCH 7/9] remoteproc: core: Add function to over-ride current  resource table Lee Jones <lee.jones@linaro.org> - 2016-08-09 14:50 +0200
    [PATCH 3/9] remoteproc: core: Remove pointless OOM print Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
      Re: [PATCH 3/9] remoteproc: core: Remove pointless OOM print Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-08-09 19:40 +0200
        Re: [PATCH 3/9] remoteproc: core: Remove pointless OOM print Lee Jones <lee.jones@linaro.org> - 2016-08-09 20:10 +0200
          Re: [PATCH 3/9] remoteproc: core: Remove pointless OOM print Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-08-10 20:20 +0200
    [PATCH 5/9] remoteproc: core: Add function to amend an existing resource table entry Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
    [PATCH 1/9] remoteproc: core: Ensure error message is clear Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
      Re: [PATCH 1/9] remoteproc: core: Ensure error message is clear Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-08-09 19:30 +0200
        Re: [PATCH 1/9] remoteproc: core: Ensure error message is clear Lee Jones <lee.jones@linaro.org> - 2016-08-09 20:20 +0200
          Re: [PATCH 1/9] remoteproc: core: Ensure error message is clear Suman Anna <s-anna@ti.com> - 2016-08-10 22:20 +0200
    [PATCH 8/9] remoteproc: core: Skip resource table integrity checks if there are amendments Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
      Re: [PATCH 8/9] remoteproc: core: Skip resource table integrity  checks if there are amendments Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-08-09 19:50 +0200
    [PATCH 2/9] remoteproc: core: Trivial: Improve error checking, spelling and debug prints Lee Jones <lee.jones@linaro.org> - 2016-08-04 11:30 +0200
      Re: [PATCH 2/9] remoteproc: core: Trivial: Improve error checking,  spelling and debug prints Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-08-09 19:40 +0200
        Re: [PATCH 2/9] remoteproc: core: Trivial: Improve error checking,  spelling and debug prints Lee Jones <lee.jones@linaro.org> - 2016-08-09 20:20 +0200

Page 1 of 2  [1] 2  Next page →

#1456249 — [PATCH 0/9] remoteproc: Allow platform-specific drivers to request resources

Once this patch-set has been applied; platform-specific remoteproc
drivers will be able to amend existing resource table entries,
provide new entries to be appended to an existing resource table
(if one already exists), start a new resource table (if one does
not already exist), and dump out resource table contents during
development/debugging.  It will also be possible to start remote
processors which do not require a resource table without being
forced to provide a fake one.

We're also taking the liberty to provide some simple fix-ups.

Lee Jones (9):
  remoteproc: core: Ensure error message is clear
  remoteproc: core: Trivial: Improve error checking, spelling and debug
    prints
  remoteproc: core: Remove pointless OOM print
  remoteproc: core: New API to add new resources to the resource table
  remoteproc: core: Add function to amend an existing resource table
    entry
  remoteproc: core: Add function to append a new resource table entry
  remoteproc: core: Add function to over-ride current resource table
  remoteproc: core: Skip resource table integrity checks if there are
    amendments
  remoteproc: core: Support empty resource tables

 drivers/remoteproc/remoteproc_core.c | 354 +++++++++++++++++++++++++++++++++--
 include/linux/remoteproc.h           |  21 +++
 2 files changed, 358 insertions(+), 17 deletions(-)

-- 
2.9.0

[toc] | [next] | [standalone]

#1456252 — [PATCH 4/9] remoteproc: core: New API to add new resources to the resource table

In order to amend or add a new resource table entry we need a method
for a platform-specific to submit them. rproc_request_resource() is a
new public API which provides this functionality.

It is to be called between rproc_alloc() and rproc_add().

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 125 +++++++++++++++++++++++++++++++++++
 include/linux/remoteproc.h           |  21 ++++++
 2 files changed, 146 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 4914482..0abfa2b 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -793,6 +793,130 @@ static void rproc_resource_cleanup(struct rproc *rproc)
 	}
 }
 
+static void rproc_dump_resource_table(struct rproc *rproc,
+				      struct resource_table *table, int size)
+{
+	const char *types[] = {"carveout", "devmem", "trace", "vdev"};
+	struct device *dev = &rproc->dev;
+	struct fw_rsc_carveout *c;
+	struct fw_rsc_devmem *d;
+	struct fw_rsc_trace *t;
+	struct fw_rsc_vdev *v;
+	int i, j;
+
+	if (!table) {
+		dev_dbg(dev, "No resource table found\n");
+		return;
+	}
+
+	dev_dbg(dev, "Resource Table: Version %d with %d entries [size: %x]\n",
+		table->ver, table->num, size);
+
+	for (i = 0; i < table->num; i++) {
+		int offset = table->offset[i];
+		struct fw_rsc_hdr *hdr = (void *)table + offset;
+		void *rsc = (void *)hdr + sizeof(*hdr);
+
+		switch (hdr->type) {
+		case RSC_CARVEOUT:
+			c = rsc;
+			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
+			dev_dbg(dev, "  Device Address 0x%x\n", c->da);
+			dev_dbg(dev, "  Physical Address 0x%x\n", c->pa);
+			dev_dbg(dev, "  Length 0x%x Bytes\n", c->len);
+			dev_dbg(dev, "  Flags 0x%x\n", c->flags);
+			dev_dbg(dev, "  Reserved (should be zero) [%d]\n", c->reserved);
+			dev_dbg(dev, "  Name %s\n\n", c->name);
+			break;
+		case RSC_DEVMEM:
+			d = rsc;
+			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
+			dev_dbg(dev, "  Device Address 0x%x\n", d->da);
+			dev_dbg(dev, "  Physical Address 0x%x\n", d->pa);
+			dev_dbg(dev, "  Length 0x%x Bytes\n", d->len);
+			dev_dbg(dev, "  Flags 0x%x\n", d->flags);
+			dev_dbg(dev, "  Reserved (should be zero) [%d]\n", d->reserved);
+			dev_dbg(dev, "  Name %s\n\n", d->name);
+			break;
+		case RSC_TRACE:
+			t = rsc;
+			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
+			dev_dbg(dev, "  Device Address 0x%x\n", t->da);
+			dev_dbg(dev, "  Length 0x%x Bytes\n", t->len);
+			dev_dbg(dev, "  Reserved (should be zero) [%d]\n", t->reserved);
+			dev_dbg(dev, "  Name %s\n\n", t->name);
+			break;
+		case RSC_VDEV:
+			v = rsc;
+			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
+
+			dev_dbg(dev, "  ID %d\n", v->id);
+			dev_dbg(dev, "  Notify ID %d\n", v->notifyid);
+			dev_dbg(dev, "  Device features 0x%x\n", v->dfeatures);
+			dev_dbg(dev, "  Guest features 0x%x\n", v->gfeatures);
+			dev_dbg(dev, "  Config length 0x%x\n", v->config_len);
+			dev_dbg(dev, "  Status 0x%x\n", v->status);
+			dev_dbg(dev, "  Number of vrings %d\n", v->num_of_vrings);
+			dev_dbg(dev, "  Reserved (should be zero) [%d][%d]\n\n",
+				v->reserved[0], v->reserved[1]);
+
+			for (j = 0; j < v->num_of_vrings; j++) {
+				dev_dbg(dev, "  Vring %d\n", j);
+				dev_dbg(dev, "    Device Address 0x%x\n", v->vring[j].da);
+				dev_dbg(dev, "    Alignment %d\n", v->vring[j].align);
+				dev_dbg(dev, "    Number of buffers %d\n", v->vring[j].num);
+				dev_dbg(dev, "    Notify ID %d\n", v->vring[j].notifyid);
+				dev_dbg(dev, "    Reserved (should be zero) [%d]\n\n",
+					v->vring[j].reserved);
+			}
+			break;
+		default:
+			dev_dbg(dev, "Invalid resource type found: %d [hdr: %p]\n",
+				hdr->type, hdr);
+			return;
+		}
+	}
+}
+
+int rproc_request_resource(struct rproc *rproc, u32 type, void *resource)
+{
+	struct device *dev = &rproc->dev;
+	struct rproc_request_resource *request;
+	int size;
+
+	request = devm_kzalloc(dev, sizeof(*request), GFP_KERNEL);
+	if (!request)
+		return -ENOMEM;
+
+	switch (type) {
+	case RSC_CARVEOUT:
+		size = sizeof(struct fw_rsc_carveout);
+		break;
+	case RSC_DEVMEM:
+		size = sizeof(struct fw_rsc_devmem);
+		break;
+	case RSC_TRACE:
+		size = sizeof(struct fw_rsc_trace);
+		break;
+	default:
+		dev_err(dev, "Unsupported resource type: %d\n", type);
+		return -EINVAL;
+	}
+
+	request->resource = devm_kzalloc(dev, size, GFP_KERNEL);
+	if (!request->resource)
+		return -ENOMEM;
+
+	memcpy(request->resource, resource, size);
+	request->type = type;
+	request->size = size;
+
+	list_add_tail(&request->node, &rproc->override_resources);
+
+	return 0;
+}
+EXPORT_SYMBOL(rproc_request_resource);
+
 /*
  * take a firmware and boot a remote processor with it.
  */
@@ -1452,6 +1576,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 	INIT_LIST_HEAD(&rproc->mappings);
 	INIT_LIST_HEAD(&rproc->traces);
 	INIT_LIST_HEAD(&rproc->rvdevs);
+	INIT_LIST_HEAD(&rproc->override_resources);
 
 	INIT_WORK(&rproc->crash_handler, rproc_crash_handler_work);
 	init_completion(&rproc->crash_comp);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 3608d20..c620177 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -323,6 +323,25 @@ struct rproc_mem_entry {
 	struct list_head node;
 };
 
+/**
+ * struct rproc_requested_resources - add a resource to the resource table
+ *
+ * @resource:	pointer to a 'struct fw_rsc_*' resource
+ * @type:	'fw_resource_type' resource type
+ * @size:	size of resource
+ * @node:	list node
+ *
+ * Resources can be added by platform-specific rproc drivers calling
+ * rproc_request_resource()
+ *
+ */
+struct rproc_request_resource {
+	void *resource;
+	u32 type;
+	u32 size;
+	struct list_head node;
+};
+
 struct rproc;
 
 /**
@@ -429,6 +448,7 @@ struct rproc {
 	int num_traces;
 	struct list_head carveouts;
 	struct list_head mappings;
+	struct list_head override_resources;
 	struct completion firmware_loading_complete;
 	u32 bootaddr;
 	struct list_head rvdevs;
@@ -487,6 +507,7 @@ struct rproc_vdev {
 	u32 rsc_offset;
 };
 
+int rproc_request_resource(struct rproc *rproc, u32 type, void *res);
 struct rproc *rproc_alloc(struct device *dev, const char *name,
 				const struct rproc_ops *ops,
 				const char *firmware, int len);
-- 
2.9.0

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

#1456416 — Re: [PATCH 4/9] remoteproc: core: New API to add new resources to the resource table

On Thu, 04 Aug 2016, Lee Jones wrote:

> In order to amend or add a new resource table entry we need a method
> for a platform-specific to submit them. rproc_request_resource() is a
> new public API which provides this functionality.
> 
> It is to be called between rproc_alloc() and rproc_add().
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/remoteproc/remoteproc_core.c | 125 +++++++++++++++++++++++++++++++++++
>  include/linux/remoteproc.h           |  21 ++++++
>  2 files changed, 146 insertions(+)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 4914482..0abfa2b 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -793,6 +793,130 @@ static void rproc_resource_cleanup(struct rproc *rproc)
>  	}
>  }
>  
> +static void rproc_dump_resource_table(struct rproc *rproc,
> +				      struct resource_table *table, int size)
> +{

Ah!  Looks like I mistakenly squashed two patches together.  I won't
re-send, but please treat these two functions as separate patches.  I
will fix this during v2, once I've received some feedback.

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

#1457800 — Re: [PATCH 4/9] remoteproc: core: New API to add new resources to the resource table

Hi Lee,

After splitting this patch in 2, it may be good to add a debugfs entry 
to display current rsc associated to one rproc.

Regards,
Loic

On 08/04/2016 11:21 AM, Lee Jones wrote:
> In order to amend or add a new resource table entry we need a method
> for a platform-specific to submit them. rproc_request_resource() is a
> new public API which provides this functionality.
>
> It is to be called between rproc_alloc() and rproc_add().
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>   drivers/remoteproc/remoteproc_core.c | 125 +++++++++++++++++++++++++++++++++++
>   include/linux/remoteproc.h           |  21 ++++++
>   2 files changed, 146 insertions(+)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 4914482..0abfa2b 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -793,6 +793,130 @@ static void rproc_resource_cleanup(struct rproc *rproc)
>   	}
>   }
>
> +static void rproc_dump_resource_table(struct rproc *rproc,
> +				      struct resource_table *table, int size)
> +{
> +	const char *types[] = {"carveout", "devmem", "trace", "vdev"};
> +	struct device *dev = &rproc->dev;
> +	struct fw_rsc_carveout *c;
> +	struct fw_rsc_devmem *d;
> +	struct fw_rsc_trace *t;
> +	struct fw_rsc_vdev *v;
> +	int i, j;
> +
> +	if (!table) {
> +		dev_dbg(dev, "No resource table found\n");
> +		return;
> +	}
> +
> +	dev_dbg(dev, "Resource Table: Version %d with %d entries [size: %x]\n",
> +		table->ver, table->num, size);
> +
> +	for (i = 0; i < table->num; i++) {
> +		int offset = table->offset[i];
> +		struct fw_rsc_hdr *hdr = (void *)table + offset;
> +		void *rsc = (void *)hdr + sizeof(*hdr);
> +
> +		switch (hdr->type) {
> +		case RSC_CARVEOUT:
> +			c = rsc;
> +			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
> +			dev_dbg(dev, "  Device Address 0x%x\n", c->da);
> +			dev_dbg(dev, "  Physical Address 0x%x\n", c->pa);
> +			dev_dbg(dev, "  Length 0x%x Bytes\n", c->len);
> +			dev_dbg(dev, "  Flags 0x%x\n", c->flags);
> +			dev_dbg(dev, "  Reserved (should be zero) [%d]\n", c->reserved);
> +			dev_dbg(dev, "  Name %s\n\n", c->name);
> +			break;
> +		case RSC_DEVMEM:
> +			d = rsc;
> +			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
> +			dev_dbg(dev, "  Device Address 0x%x\n", d->da);
> +			dev_dbg(dev, "  Physical Address 0x%x\n", d->pa);
> +			dev_dbg(dev, "  Length 0x%x Bytes\n", d->len);
> +			dev_dbg(dev, "  Flags 0x%x\n", d->flags);
> +			dev_dbg(dev, "  Reserved (should be zero) [%d]\n", d->reserved);
> +			dev_dbg(dev, "  Name %s\n\n", d->name);
> +			break;
> +		case RSC_TRACE:
> +			t = rsc;
> +			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
> +			dev_dbg(dev, "  Device Address 0x%x\n", t->da);
> +			dev_dbg(dev, "  Length 0x%x Bytes\n", t->len);
> +			dev_dbg(dev, "  Reserved (should be zero) [%d]\n", t->reserved);
> +			dev_dbg(dev, "  Name %s\n\n", t->name);
> +			break;
> +		case RSC_VDEV:
> +			v = rsc;
> +			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
> +
> +			dev_dbg(dev, "  ID %d\n", v->id);
> +			dev_dbg(dev, "  Notify ID %d\n", v->notifyid);
> +			dev_dbg(dev, "  Device features 0x%x\n", v->dfeatures);
> +			dev_dbg(dev, "  Guest features 0x%x\n", v->gfeatures);
> +			dev_dbg(dev, "  Config length 0x%x\n", v->config_len);
> +			dev_dbg(dev, "  Status 0x%x\n", v->status);
> +			dev_dbg(dev, "  Number of vrings %d\n", v->num_of_vrings);
> +			dev_dbg(dev, "  Reserved (should be zero) [%d][%d]\n\n",
> +				v->reserved[0], v->reserved[1]);
> +
> +			for (j = 0; j < v->num_of_vrings; j++) {
> +				dev_dbg(dev, "  Vring %d\n", j);
> +				dev_dbg(dev, "    Device Address 0x%x\n", v->vring[j].da);
> +				dev_dbg(dev, "    Alignment %d\n", v->vring[j].align);
> +				dev_dbg(dev, "    Number of buffers %d\n", v->vring[j].num);
> +				dev_dbg(dev, "    Notify ID %d\n", v->vring[j].notifyid);
> +				dev_dbg(dev, "    Reserved (should be zero) [%d]\n\n",
> +					v->vring[j].reserved);
> +			}
> +			break;
> +		default:
> +			dev_dbg(dev, "Invalid resource type found: %d [hdr: %p]\n",
> +				hdr->type, hdr);
> +			return;
> +		}
> +	}
> +}
> +
> +int rproc_request_resource(struct rproc *rproc, u32 type, void *resource)
> +{
> +	struct device *dev = &rproc->dev;
> +	struct rproc_request_resource *request;
> +	int size;
> +
> +	request = devm_kzalloc(dev, sizeof(*request), GFP_KERNEL);
> +	if (!request)
> +		return -ENOMEM;
> +
> +	switch (type) {
> +	case RSC_CARVEOUT:
> +		size = sizeof(struct fw_rsc_carveout);
> +		break;
> +	case RSC_DEVMEM:
> +		size = sizeof(struct fw_rsc_devmem);
> +		break;
> +	case RSC_TRACE:
> +		size = sizeof(struct fw_rsc_trace);
> +		break;
> +	default:
> +		dev_err(dev, "Unsupported resource type: %d\n", type);
> +		return -EINVAL;
> +	}
> +
> +	request->resource = devm_kzalloc(dev, size, GFP_KERNEL);
> +	if (!request->resource)
> +		return -ENOMEM;
> +
> +	memcpy(request->resource, resource, size);
> +	request->type = type;
> +	request->size = size;
> +
> +	list_add_tail(&request->node, &rproc->override_resources);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(rproc_request_resource);
> +
>   /*
>    * take a firmware and boot a remote processor with it.
>    */
> @@ -1452,6 +1576,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
>   	INIT_LIST_HEAD(&rproc->mappings);
>   	INIT_LIST_HEAD(&rproc->traces);
>   	INIT_LIST_HEAD(&rproc->rvdevs);
> +	INIT_LIST_HEAD(&rproc->override_resources);
>
>   	INIT_WORK(&rproc->crash_handler, rproc_crash_handler_work);
>   	init_completion(&rproc->crash_comp);
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 3608d20..c620177 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -323,6 +323,25 @@ struct rproc_mem_entry {
>   	struct list_head node;
>   };
>
> +/**
> + * struct rproc_requested_resources - add a resource to the resource table
> + *
> + * @resource:	pointer to a 'struct fw_rsc_*' resource
> + * @type:	'fw_resource_type' resource type
> + * @size:	size of resource
> + * @node:	list node
> + *
> + * Resources can be added by platform-specific rproc drivers calling
> + * rproc_request_resource()
> + *
> + */
> +struct rproc_request_resource {
> +	void *resource;
> +	u32 type;
> +	u32 size;
> +	struct list_head node;
> +};
> +
>   struct rproc;
>
>   /**
> @@ -429,6 +448,7 @@ struct rproc {
>   	int num_traces;
>   	struct list_head carveouts;
>   	struct list_head mappings;
> +	struct list_head override_resources;
>   	struct completion firmware_loading_complete;
>   	u32 bootaddr;
>   	struct list_head rvdevs;
> @@ -487,6 +507,7 @@ struct rproc_vdev {
>   	u32 rsc_offset;
>   };
>
> +int rproc_request_resource(struct rproc *rproc, u32 type, void *res);
>   struct rproc *rproc_alloc(struct device *dev, const char *name,
>   				const struct rproc_ops *ops,
>   				const char *firmware, int len);
>

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

#1458690 — Re: [PATCH 4/9] remoteproc: core: New API to add new resources to the resource table

On Mon, 08 Aug 2016, loic pallardy wrote:

> Hi Lee,
> 
> After splitting this patch in 2, it may be good to add a debugfs entry to
> display current rsc associated to one rproc.

Happy to take suggestions to add functionality, but I would like to
prevent *this* set from suffering from feature creep.  Thus, I would
like to code-up your suggestion in a subsequent submission if it's all
the same to you.

> On 08/04/2016 11:21 AM, Lee Jones wrote:
> > In order to amend or add a new resource table entry we need a method
> > for a platform-specific to submit them. rproc_request_resource() is a
> > new public API which provides this functionality.
> > 
> > It is to be called between rproc_alloc() and rproc_add().
> > 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >   drivers/remoteproc/remoteproc_core.c | 125 +++++++++++++++++++++++++++++++++++
> >   include/linux/remoteproc.h           |  21 ++++++
> >   2 files changed, 146 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> > index 4914482..0abfa2b 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -793,6 +793,130 @@ static void rproc_resource_cleanup(struct rproc *rproc)
> >   	}
> >   }
> > 
> > +static void rproc_dump_resource_table(struct rproc *rproc,
> > +				      struct resource_table *table, int size)
> > +{
> > +	const char *types[] = {"carveout", "devmem", "trace", "vdev"};
> > +	struct device *dev = &rproc->dev;
> > +	struct fw_rsc_carveout *c;
> > +	struct fw_rsc_devmem *d;
> > +	struct fw_rsc_trace *t;
> > +	struct fw_rsc_vdev *v;
> > +	int i, j;
> > +
> > +	if (!table) {
> > +		dev_dbg(dev, "No resource table found\n");
> > +		return;
> > +	}
> > +
> > +	dev_dbg(dev, "Resource Table: Version %d with %d entries [size: %x]\n",
> > +		table->ver, table->num, size);
> > +
> > +	for (i = 0; i < table->num; i++) {
> > +		int offset = table->offset[i];
> > +		struct fw_rsc_hdr *hdr = (void *)table + offset;
> > +		void *rsc = (void *)hdr + sizeof(*hdr);
> > +
> > +		switch (hdr->type) {
> > +		case RSC_CARVEOUT:
> > +			c = rsc;
> > +			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
> > +			dev_dbg(dev, "  Device Address 0x%x\n", c->da);
> > +			dev_dbg(dev, "  Physical Address 0x%x\n", c->pa);
> > +			dev_dbg(dev, "  Length 0x%x Bytes\n", c->len);
> > +			dev_dbg(dev, "  Flags 0x%x\n", c->flags);
> > +			dev_dbg(dev, "  Reserved (should be zero) [%d]\n", c->reserved);
> > +			dev_dbg(dev, "  Name %s\n\n", c->name);
> > +			break;
> > +		case RSC_DEVMEM:
> > +			d = rsc;
> > +			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
> > +			dev_dbg(dev, "  Device Address 0x%x\n", d->da);
> > +			dev_dbg(dev, "  Physical Address 0x%x\n", d->pa);
> > +			dev_dbg(dev, "  Length 0x%x Bytes\n", d->len);
> > +			dev_dbg(dev, "  Flags 0x%x\n", d->flags);
> > +			dev_dbg(dev, "  Reserved (should be zero) [%d]\n", d->reserved);
> > +			dev_dbg(dev, "  Name %s\n\n", d->name);
> > +			break;
> > +		case RSC_TRACE:
> > +			t = rsc;
> > +			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
> > +			dev_dbg(dev, "  Device Address 0x%x\n", t->da);
> > +			dev_dbg(dev, "  Length 0x%x Bytes\n", t->len);
> > +			dev_dbg(dev, "  Reserved (should be zero) [%d]\n", t->reserved);
> > +			dev_dbg(dev, "  Name %s\n\n", t->name);
> > +			break;
> > +		case RSC_VDEV:
> > +			v = rsc;
> > +			dev_dbg(dev, "Entry %d is of type %s\n", i, types[hdr->type]);
> > +
> > +			dev_dbg(dev, "  ID %d\n", v->id);
> > +			dev_dbg(dev, "  Notify ID %d\n", v->notifyid);
> > +			dev_dbg(dev, "  Device features 0x%x\n", v->dfeatures);
> > +			dev_dbg(dev, "  Guest features 0x%x\n", v->gfeatures);
> > +			dev_dbg(dev, "  Config length 0x%x\n", v->config_len);
> > +			dev_dbg(dev, "  Status 0x%x\n", v->status);
> > +			dev_dbg(dev, "  Number of vrings %d\n", v->num_of_vrings);
> > +			dev_dbg(dev, "  Reserved (should be zero) [%d][%d]\n\n",
> > +				v->reserved[0], v->reserved[1]);
> > +
> > +			for (j = 0; j < v->num_of_vrings; j++) {
> > +				dev_dbg(dev, "  Vring %d\n", j);
> > +				dev_dbg(dev, "    Device Address 0x%x\n", v->vring[j].da);
> > +				dev_dbg(dev, "    Alignment %d\n", v->vring[j].align);
> > +				dev_dbg(dev, "    Number of buffers %d\n", v->vring[j].num);
> > +				dev_dbg(dev, "    Notify ID %d\n", v->vring[j].notifyid);
> > +				dev_dbg(dev, "    Reserved (should be zero) [%d]\n\n",
> > +					v->vring[j].reserved);
> > +			}
> > +			break;
> > +		default:
> > +			dev_dbg(dev, "Invalid resource type found: %d [hdr: %p]\n",
> > +				hdr->type, hdr);
> > +			return;
> > +		}
> > +	}
> > +}
> > +
> > +int rproc_request_resource(struct rproc *rproc, u32 type, void *resource)
> > +{
> > +	struct device *dev = &rproc->dev;
> > +	struct rproc_request_resource *request;
> > +	int size;
> > +
> > +	request = devm_kzalloc(dev, sizeof(*request), GFP_KERNEL);
> > +	if (!request)
> > +		return -ENOMEM;
> > +
> > +	switch (type) {
> > +	case RSC_CARVEOUT:
> > +		size = sizeof(struct fw_rsc_carveout);
> > +		break;
> > +	case RSC_DEVMEM:
> > +		size = sizeof(struct fw_rsc_devmem);
> > +		break;
> > +	case RSC_TRACE:
> > +		size = sizeof(struct fw_rsc_trace);
> > +		break;
> > +	default:
> > +		dev_err(dev, "Unsupported resource type: %d\n", type);
> > +		return -EINVAL;
> > +	}
> > +
> > +	request->resource = devm_kzalloc(dev, size, GFP_KERNEL);
> > +	if (!request->resource)
> > +		return -ENOMEM;
> > +
> > +	memcpy(request->resource, resource, size);
> > +	request->type = type;
> > +	request->size = size;
> > +
> > +	list_add_tail(&request->node, &rproc->override_resources);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(rproc_request_resource);
> > +
> >   /*
> >    * take a firmware and boot a remote processor with it.
> >    */
> > @@ -1452,6 +1576,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
> >   	INIT_LIST_HEAD(&rproc->mappings);
> >   	INIT_LIST_HEAD(&rproc->traces);
> >   	INIT_LIST_HEAD(&rproc->rvdevs);
> > +	INIT_LIST_HEAD(&rproc->override_resources);
> > 
> >   	INIT_WORK(&rproc->crash_handler, rproc_crash_handler_work);
> >   	init_completion(&rproc->crash_comp);
> > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> > index 3608d20..c620177 100644
> > --- a/include/linux/remoteproc.h
> > +++ b/include/linux/remoteproc.h
> > @@ -323,6 +323,25 @@ struct rproc_mem_entry {
> >   	struct list_head node;
> >   };
> > 
> > +/**
> > + * struct rproc_requested_resources - add a resource to the resource table
> > + *
> > + * @resource:	pointer to a 'struct fw_rsc_*' resource
> > + * @type:	'fw_resource_type' resource type
> > + * @size:	size of resource
> > + * @node:	list node
> > + *
> > + * Resources can be added by platform-specific rproc drivers calling
> > + * rproc_request_resource()
> > + *
> > + */
> > +struct rproc_request_resource {
> > +	void *resource;
> > +	u32 type;
> > +	u32 size;
> > +	struct list_head node;
> > +};
> > +
> >   struct rproc;
> > 
> >   /**
> > @@ -429,6 +448,7 @@ struct rproc {
> >   	int num_traces;
> >   	struct list_head carveouts;
> >   	struct list_head mappings;
> > +	struct list_head override_resources;
> >   	struct completion firmware_loading_complete;
> >   	u32 bootaddr;
> >   	struct list_head rvdevs;
> > @@ -487,6 +507,7 @@ struct rproc_vdev {
> >   	u32 rsc_offset;
> >   };
> > 
> > +int rproc_request_resource(struct rproc *rproc, u32 type, void *res);
> >   struct rproc *rproc_alloc(struct device *dev, const char *name,
> >   				const struct rproc_ops *ops,
> >   				const char *firmware, int len);
> > 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

#1456255 — [PATCH 9/9] remoteproc: core: Support empty resource tables

Currently, when a remote processor does not require resources, the
platform-specific remoteproc driver has to create a fake resource
table in order to by-pass the strict checking.  But there is no hard
requirement for a remote processor so require or support shared
resources.  This patch removes the strict checking and skips
resource table related operations if one is not provided.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 9a077e4..366e197 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1104,9 +1104,6 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 	struct resource_table *table, *loaded_table;
 	int ret, tablesz;
 
-	if (!rproc->table_ptr)
-		return -ENOMEM;
-
 	ret = rproc_fw_sanity_check(rproc, fw);
 	if (ret)
 		return ret;
@@ -1130,12 +1127,10 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 	if (!list_empty(&rproc->override_resources))
 		goto skip_table_check;
 
-	/* look for the resource table */
+	/* Resource tables aren't compulsory. */
 	table = rproc_find_rsc_table(rproc, fw, &tablesz);
-	if (!table) {
-		dev_err(dev, "Failed to find resource table\n");
-		goto clean_up;
-	}
+	if (!table)
+		goto skip_resources;
 
 	/* Verify that resource table in loaded fw is unchanged */
 	if (rproc->table_csum != crc32(0, table, tablesz)) {
@@ -1152,6 +1147,8 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 		goto clean_up;
 	}
 
+ skip_resources:
+
 	/* load the ELF segments to memory */
 	ret = rproc_load_segments(rproc, fw);
 	if (ret) {
@@ -1213,8 +1210,12 @@ static void rproc_fw_config_virtio(const struct firmware *fw, void *context)
 	if (rproc_fw_sanity_check(rproc, fw) < 0)
 		goto out;
 
-	/* look for the resource table */
 	table = rproc_find_rsc_table(rproc, fw,  &tablesz);
+	if (IS_ERR(table)) {
+		dev_err(&rproc->dev, "Resource table expected, but failed\n");
+		goto out;
+	}
+
 	if (!table)
 		goto out;
 
-- 
2.9.0

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

#1456917 — Re: [PATCH 9/9] remoteproc: core: Support empty resource tables

On Thu, 04 Aug 2016, Lee Jones wrote:

> Currently, when a remote processor does not require resources, the
> platform-specific remoteproc driver has to create a fake resource
> table in order to by-pass the strict checking.  But there is no hard
> requirement for a remote processor so require or support shared
> resources.  This patch removes the strict checking and skips
> resource table related operations if one is not provided.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/remoteproc/remoteproc_core.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)

Due to the fact that a) this patch conflicts heavily with Bjorn's set
and b) it isn't fully tested with real resourceless firmware, I'm
pulling it from the set.  I will submit it as part of a subsequent set
-- please ignore.

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

#1456256 — [PATCH 6/9] remoteproc: core: Add function to append a new resource table entry

A new function now exists to pull in and amend and existing resource
table entry.  But what if we wish to provide a new resource?  This
function provides functionality to append a brand new resource entry
onto the resource table.  All complexity related to shuffling parts
of the table around, providing new offsets and incriminating number
of entries in the resource table's top-level header is taken care of
here.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 55 ++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 3318ebd..111350e 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -980,6 +980,61 @@ static int rproc_update_resource_table_entry(struct rproc *rproc,
 	return !updated;
 }
 
+static struct resource_table*
+rproc_add_resource_table_entry(struct rproc *rproc,
+			       struct rproc_request_resource *request,
+			       struct resource_table *old_table, int *tablesz)
+{
+	struct resource_table *table;
+	struct fw_rsc_hdr h;
+	void *new_rsc_loc;
+	void *fw_header_loc;
+	void *start_of_rscs;
+	int new_rsc_offset;
+	int size = *tablesz;
+	int i;
+
+	h.type = request->type;
+
+	new_rsc_offset = size;
+
+	/*
+	 * Allocate another contiguous chunk of memory, large enough to
+	 * contain the new, expanded resource table.
+	 *
+	 * The +4 is for the extra offset[] element in the top level header
+	 */
+	size += sizeof(struct fw_rsc_hdr) + request->size + 4;
+	table = devm_kmemdup(&rproc->dev, old_table, size, GFP_KERNEL);
+	if (!table)
+		return ERR_PTR(-ENOMEM);
+
+	/* Shunt table by 4 Bytes to account for the extra offset[] element */
+	start_of_rscs = (void *)table + table->offset[0];
+	memmove(start_of_rscs + 4,
+		start_of_rscs, new_rsc_offset - table->offset[0]);
+	new_rsc_offset += 4;
+
+	/* Update existing resource entry's offsets */
+	for (i = 0; i < table->num; i++)
+		table->offset[i] += 4;
+
+	/* Update the top level 'resource table' header */
+	table->offset[table->num] = new_rsc_offset;
+	table->num++;
+
+	/* Copy new firmware header into table */
+	fw_header_loc = (void *)table + new_rsc_offset;
+	memcpy(fw_header_loc, &h, sizeof(h));
+
+	/* Copy new resource entry into table */
+	new_rsc_loc = (void *)fw_header_loc + sizeof(h);
+	memcpy(new_rsc_loc, request->resource, request->size);
+
+	*tablesz = size;
+	return table;
+}
+
 /*
  * take a firmware and boot a remote processor with it.
  */
-- 
2.9.0

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

#1456257 — [PATCH 7/9] remoteproc: core: Add function to over-ride current resource table

Most of the new resource table handling function are now in place, so
it's time to put it all together.  Once new resource table information
has been requested, the structures will be held in a holding pen until
boot-time.  During boot-time rproc_apply_resource_overrides() will be
invoked which in turn will pull the new information out of the holding
pen and edit the table accordingly.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 65 ++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 111350e..6b4e29a 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1035,6 +1035,65 @@ rproc_add_resource_table_entry(struct rproc *rproc,
 	return table;
 }
 
+static struct resource_table*
+rproc_apply_resource_overrides(struct rproc *rproc,
+			       struct resource_table **orig_table,
+			       int *tablesz)
+{
+	struct rproc_request_resource *resource;
+	struct resource_table *table = *orig_table;
+	int size = *tablesz;
+
+	if (!table && size != 0) {
+		dev_err(&rproc->dev, "No table present but table size is set\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	mutex_lock(&rproc->lock);
+
+	rproc_dump_resource_table(rproc, table, size);
+
+	if (!table) {
+		size = sizeof(*table);
+		table = devm_kzalloc(&rproc->dev, size, GFP_KERNEL);
+		if (!table) {
+			table = ERR_PTR(-ENOMEM);
+			goto out;
+		}
+		table->ver = 1;
+	}
+
+	list_for_each_entry(resource, &rproc->override_resources, node) {
+		int updated = 0;
+
+		/* If we already have a table, update it with the new values. */
+		updated = rproc_update_resource_table_entry(rproc, resource,
+							    table, size);
+		if (updated < 0) {
+			table = ERR_PTR(updated);
+			goto out;
+		}
+		if (updated)
+			continue;
+
+		/* Didn't find matching resource entry -- creating a new one. */
+		table = rproc_add_resource_table_entry(rproc, resource,
+						       table, &size);
+		if (IS_ERR(table))
+			goto out;
+
+		*orig_table = table;
+	}
+
+	rproc_dump_resource_table(rproc, table, size);
+
+	*tablesz = size;
+
+ out:
+	mutex_unlock(&rproc->lock);
+	return table;
+}
+
 /*
  * take a firmware and boot a remote processor with it.
  */
@@ -1153,6 +1212,12 @@ static void rproc_fw_config_virtio(const struct firmware *fw, void *context)
 	if (!table)
 		goto out;
 
+	if (!list_empty(&rproc->override_resources)) {
+		table = rproc_apply_resource_overrides(rproc, &table, &tablesz);
+		if (IS_ERR(table))
+			goto out;
+	}
+
 	rproc->table_csum = crc32(0, table, tablesz);
 
 	/*
-- 
2.9.0

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

#1457805 — Re: [PATCH 7/9] remoteproc: core: Add function to over-ride current resource table

Hi Lee

On 08/04/2016 11:21 AM, Lee Jones wrote:
> Most of the new resource table handling function are now in place, so
> it's time to put it all together.  Once new resource table information
> has been requested, the structures will be held in a holding pen until
> boot-time.  During boot-time rproc_apply_resource_overrides() will be
> invoked which in turn will pull the new information out of the holding
> pen and edit the table accordingly.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>   drivers/remoteproc/remoteproc_core.c | 65 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 65 insertions(+)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 111350e..6b4e29a 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1035,6 +1035,65 @@ rproc_add_resource_table_entry(struct rproc *rproc,
>   	return table;
>   }
>
> +static struct resource_table*
> +rproc_apply_resource_overrides(struct rproc *rproc,
> +			       struct resource_table **orig_table,
> +			       int *tablesz)
> +{
> +	struct rproc_request_resource *resource;
> +	struct resource_table *table = *orig_table;
> +	int size = *tablesz;
> +
> +	if (!table && size != 0) {
> +		dev_err(&rproc->dev, "No table present but table size is set\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	mutex_lock(&rproc->lock);
> +
> +	rproc_dump_resource_table(rproc, table, size);
It will be good to find a way to not dump resource table if debug is not 
activated. For the same reason as you mentioned in your patch 8, 
resource table parsing will waste CPUS cycles.

> +
> +	if (!table) {
> +		size = sizeof(*table);
> +		table = devm_kzalloc(&rproc->dev, size, GFP_KERNEL);
> +		if (!table) {
> +			table = ERR_PTR(-ENOMEM);
> +			goto out;
> +		}
> +		table->ver = 1;
> +	}
> +
> +	list_for_each_entry(resource, &rproc->override_resources, node) {
> +		int updated = 0;
> +
> +		/* If we already have a table, update it with the new values. */
> +		updated = rproc_update_resource_table_entry(rproc, resource,
> +							    table, size);
> +		if (updated < 0) {
> +			table = ERR_PTR(updated);
> +			goto out;
> +		}
> +		if (updated)
> +			continue;
> +
> +		/* Didn't find matching resource entry -- creating a new one. */
> +		table = rproc_add_resource_table_entry(rproc, resource,
> +						       table, &size);
> +		if (IS_ERR(table))
> +			goto out;
> +
> +		*orig_table = table;
> +	}
> +
> +	rproc_dump_resource_table(rproc, table, size);
ditto

Regards,
Loic
> +
> +	*tablesz = size;
> +
> + out:
> +	mutex_unlock(&rproc->lock);
> +	return table;
> +}
> +
>   /*
>    * take a firmware and boot a remote processor with it.
>    */
> @@ -1153,6 +1212,12 @@ static void rproc_fw_config_virtio(const struct firmware *fw, void *context)
>   	if (!table)
>   		goto out;
>
> +	if (!list_empty(&rproc->override_resources)) {
> +		table = rproc_apply_resource_overrides(rproc, &table, &tablesz);
> +		if (IS_ERR(table))
> +			goto out;
> +	}
> +
>   	rproc->table_csum = crc32(0, table, tablesz);
>
>   	/*
>

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

#1458708 — Re: [PATCH 7/9] remoteproc: core: Add function to over-ride current resource table

On Mon, 08 Aug 2016, loic pallardy wrote:

> Hi Lee
> 
> On 08/04/2016 11:21 AM, Lee Jones wrote:
> > Most of the new resource table handling function are now in place, so
> > it's time to put it all together.  Once new resource table information
> > has been requested, the structures will be held in a holding pen until
> > boot-time.  During boot-time rproc_apply_resource_overrides() will be
> > invoked which in turn will pull the new information out of the holding
> > pen and edit the table accordingly.
> > 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >   drivers/remoteproc/remoteproc_core.c | 65 ++++++++++++++++++++++++++++++++++++
> >   1 file changed, 65 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> > index 111350e..6b4e29a 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -1035,6 +1035,65 @@ rproc_add_resource_table_entry(struct rproc *rproc,
> >   	return table;
> >   }
> > 
> > +static struct resource_table*
> > +rproc_apply_resource_overrides(struct rproc *rproc,
> > +			       struct resource_table **orig_table,
> > +			       int *tablesz)
> > +{
> > +	struct rproc_request_resource *resource;
> > +	struct resource_table *table = *orig_table;
> > +	int size = *tablesz;
> > +
> > +	if (!table && size != 0) {
> > +		dev_err(&rproc->dev, "No table present but table size is set\n");
> > +		return ERR_PTR(-EINVAL);
> > +	}
> > +
> > +	mutex_lock(&rproc->lock);
> > +
> > +	rproc_dump_resource_table(rproc, table, size);
> It will be good to find a way to not dump resource table if debug is not
> activated. For the same reason as you mentioned in your patch 8, resource
> table parsing will waste CPUS cycles.

+1

I've been meaning to fix this, thanks.

> > +
> > +	if (!table) {
> > +		size = sizeof(*table);
> > +		table = devm_kzalloc(&rproc->dev, size, GFP_KERNEL);
> > +		if (!table) {
> > +			table = ERR_PTR(-ENOMEM);
> > +			goto out;
> > +		}
> > +		table->ver = 1;
> > +	}
> > +
> > +	list_for_each_entry(resource, &rproc->override_resources, node) {
> > +		int updated = 0;
> > +
> > +		/* If we already have a table, update it with the new values. */
> > +		updated = rproc_update_resource_table_entry(rproc, resource,
> > +							    table, size);
> > +		if (updated < 0) {
> > +			table = ERR_PTR(updated);
> > +			goto out;
> > +		}
> > +		if (updated)
> > +			continue;
> > +
> > +		/* Didn't find matching resource entry -- creating a new one. */
> > +		table = rproc_add_resource_table_entry(rproc, resource,
> > +						       table, &size);
> > +		if (IS_ERR(table))
> > +			goto out;
> > +
> > +		*orig_table = table;
> > +	}
> > +
> > +	rproc_dump_resource_table(rproc, table, size);
> ditto

Ditto

> > +
> > +	*tablesz = size;
> > +
> > + out:
> > +	mutex_unlock(&rproc->lock);
> > +	return table;
> > +}
> > +
> >   /*
> >    * take a firmware and boot a remote processor with it.
> >    */
> > @@ -1153,6 +1212,12 @@ static void rproc_fw_config_virtio(const struct firmware *fw, void *context)
> >   	if (!table)
> >   		goto out;
> > 
> > +	if (!list_empty(&rproc->override_resources)) {
> > +		table = rproc_apply_resource_overrides(rproc, &table, &tablesz);
> > +		if (IS_ERR(table))
> > +			goto out;
> > +	}
> > +
> >   	rproc->table_csum = crc32(0, table, tablesz);
> > 
> >   	/*
> > 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

#1456259 — [PATCH 3/9] remoteproc: core: Remove pointless OOM print

These types of error prints are superfluous.  The system will
pick up on OOM issues and let the user know.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 5654a81..4914482 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -619,7 +619,6 @@ static int rproc_handle_carveout(struct rproc *rproc,
 	if (rproc->domain) {
 		mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
 		if (!mapping) {
-			dev_err(dev, "kzalloc mapping failed\n");
 			ret = -ENOMEM;
 			goto dma_free;
 		}
-- 
2.9.0

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

#1459032 — Re: [PATCH 3/9] remoteproc: core: Remove pointless OOM print

On Thu 04 Aug 02:21 PDT 2016, Lee Jones wrote:

> These types of error prints are superfluous.  The system will
> pick up on OOM issues and let the user know.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Although, I think we should drop the "mapping" object. We have all the
information in the carveout object to reverse the mapping at cleanup.

Regards,
Bjorn

> ---
>  drivers/remoteproc/remoteproc_core.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 5654a81..4914482 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -619,7 +619,6 @@ static int rproc_handle_carveout(struct rproc *rproc,
>  	if (rproc->domain) {
>  		mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
>  		if (!mapping) {
> -			dev_err(dev, "kzalloc mapping failed\n");
>  			ret = -ENOMEM;
>  			goto dma_free;
>  		}
> -- 
> 2.9.0
> 

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

#1459041 — Re: [PATCH 3/9] remoteproc: core: Remove pointless OOM print

On Tue, 09 Aug 2016, Bjorn Andersson wrote:

> On Thu 04 Aug 02:21 PDT 2016, Lee Jones wrote:
> 
> > These types of error prints are superfluous.  The system will
> > pick up on OOM issues and let the user know.
> > 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

This doesn't rely on anything else, so you can just take it.

> Although, I think we should drop the "mapping" object. We have all the
> information in the carveout object to reverse the mapping at cleanup.

Let's fix it for now, then work on 'nice to have's in a subsequent set.
> > ---
> >  drivers/remoteproc/remoteproc_core.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> > index 5654a81..4914482 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -619,7 +619,6 @@ static int rproc_handle_carveout(struct rproc *rproc,
> >  	if (rproc->domain) {
> >  		mapping = kzalloc(sizeof(*mapping), GFP_KERNEL);
> >  		if (!mapping) {
> > -			dev_err(dev, "kzalloc mapping failed\n");
> >  			ret = -ENOMEM;
> >  			goto dma_free;
> >  		}

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

#1459376 — Re: [PATCH 3/9] remoteproc: core: Remove pointless OOM print

On Tue 09 Aug 11:10 PDT 2016, Lee Jones wrote:

> On Tue, 09 Aug 2016, Bjorn Andersson wrote:
> 
> > On Thu 04 Aug 02:21 PDT 2016, Lee Jones wrote:
> > 
> > > These types of error prints are superfluous.  The system will
> > > pick up on OOM issues and let the user know.
> > > 
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> This doesn't rely on anything else, so you can just take it.
> 

Of course...

Applied patch 1 through 3.

Regards,
Bjorn

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

#1456260 — [PATCH 5/9] remoteproc: core: Add function to amend an existing resource table entry

Sometimes the firmware does not know best.

When a firmware is built, it can be loaded with a resource table, usually
detailing shared; memory, virtual device, trace log information etc.
However, some vendors require this hard-coded information to be amended
with new/improved information obtained from Device Tree for instance.

Until now, no method exists which allows the resource table to be amended.
The addition of this function changes that.  It is now possible to pull in
a resource table and amend it before it is finally shared with the remote
device.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 63 ++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 0abfa2b..3318ebd 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -917,6 +917,69 @@ int rproc_request_resource(struct rproc *rproc, u32 type, void *resource)
 }
 EXPORT_SYMBOL(rproc_request_resource);
 
+static int rproc_update_resource_table_entry(struct rproc *rproc,
+				struct rproc_request_resource *request,
+				struct resource_table *table, int size)
+{
+	struct fw_rsc_carveout *tblc, *newc;
+	struct fw_rsc_devmem *tbld, *newd;
+	struct fw_rsc_trace *tblt, *newt;
+	int updated = true;
+	int i;
+
+	for (i = 0; i < table->num; i++) {
+		int offset = table->offset[i];
+		struct fw_rsc_hdr *hdr = (void *)table + offset;
+		void *rsc = (void *)hdr + sizeof(*hdr);
+
+		if (request->type != hdr->type)
+			continue;
+
+		switch (hdr->type) {
+		case RSC_CARVEOUT:
+			tblc = rsc;
+			newc = request->resource;
+
+			if (strncmp(newc->name, tblc->name,
+				    sizeof(*tblc->name)))
+				break;
+
+			memcpy(tblc, newc, request->size);
+
+			return updated;
+		case RSC_DEVMEM:
+			tbld = rsc;
+			newd = request->resource;
+
+			if (strncmp(newd->name, tbld->name,
+				    sizeof(*tbld->name)))
+				break;
+
+			memcpy(tbld, newd, request->size);
+
+			return updated;
+		case RSC_TRACE:
+			tblt = rsc;
+			newt = request->resource;
+
+			if (strncmp(newt->name, tblt->name,
+				    sizeof(*tblt->name)))
+				break;
+
+			memcpy(tblt, newt, request->size);
+
+			return updated;
+		default:
+			dev_err(&rproc->dev,
+				"Unsupported resource type: %d\n",
+				hdr->type);
+			return -EINVAL;
+		}
+	}
+
+	return !updated;
+}
+
 /*
  * take a firmware and boot a remote processor with it.
  */
-- 
2.9.0

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

#1456261 — [PATCH 1/9] remoteproc: core: Ensure error message is clear

Before this patch, the dma_alloc_coherent() failure path printed out:

  "dma_alloc_coherent err: 16760832"

... alluding to the Linux error code being 16760832, but seeing as
Linux error codes are all negative, this looks like a signed/unsigned
issue.  In fact, the message is trying to print the length of the
requested memory region.  Let's clear that up.

While we're at it, let's standardise the way 'len' is printed.  In
all other locations 'len' is in hex prefixed by a '0x' for clarity.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/remoteproc/remoteproc_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index aea29a75c..3566dc9 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -581,7 +581,7 @@ static int rproc_handle_carveout(struct rproc *rproc,
 		return -EINVAL;
 	}
 
-	dev_dbg(dev, "carveout rsc: da %x, pa %x, len %x, flags %x\n",
+	dev_dbg(dev, "carveout rsc: da %x, pa %x, len 0x%x, flags %x\n",
 			rsc->da, rsc->pa, rsc->len, rsc->flags);
 
 	carveout = kzalloc(sizeof(*carveout), GFP_KERNEL);
@@ -590,7 +590,8 @@ static int rproc_handle_carveout(struct rproc *rproc,
 
 	va = dma_alloc_coherent(dev->parent, rsc->len, &dma, GFP_KERNEL);
 	if (!va) {
-		dev_err(dev->parent, "dma_alloc_coherent err: %d\n", rsc->len);
+		dev_err(dev->parent,
+			"failed to allocate dma memory: len 0x%x\n", rsc->len);
 		ret = -ENOMEM;
 		goto free_carv;
 	}
-- 
2.9.0

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

#1459026 — Re: [PATCH 1/9] remoteproc: core: Ensure error message is clear

On Thu 04 Aug 02:21 PDT 2016, Lee Jones wrote:

> Before this patch, the dma_alloc_coherent() failure path printed out:
> 
>   "dma_alloc_coherent err: 16760832"
> 
> ... alluding to the Linux error code being 16760832, but seeing as
> Linux error codes are all negative, this looks like a signed/unsigned
> issue.  In fact, the message is trying to print the length of the
> requested memory region.  Let's clear that up.
> 
> While we're at it, let's standardise the way 'len' is printed.  In
> all other locations 'len' is in hex prefixed by a '0x' for clarity.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/remoteproc/remoteproc_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index aea29a75c..3566dc9 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -581,7 +581,7 @@ static int rproc_handle_carveout(struct rproc *rproc,
>  		return -EINVAL;
>  	}
>  
> -	dev_dbg(dev, "carveout rsc: da %x, pa %x, len %x, flags %x\n",
> +	dev_dbg(dev, "carveout rsc: da %x, pa %x, len 0x%x, flags %x\n",
>  			rsc->da, rsc->pa, rsc->len, rsc->flags);
>  
>  	carveout = kzalloc(sizeof(*carveout), GFP_KERNEL);
> @@ -590,7 +590,8 @@ static int rproc_handle_carveout(struct rproc *rproc,
>  
>  	va = dma_alloc_coherent(dev->parent, rsc->len, &dma, GFP_KERNEL);
>  	if (!va) {
> -		dev_err(dev->parent, "dma_alloc_coherent err: %d\n", rsc->len);
> +		dev_err(dev->parent,
> +			"failed to allocate dma memory: len 0x%x\n", rsc->len);
>  		ret = -ENOMEM;
>  		goto free_carv;
>  	}
> -- 
> 2.9.0
> 

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

#1459044 — Re: [PATCH 1/9] remoteproc: core: Ensure error message is clear

On Tue, 09 Aug 2016, Bjorn Andersson wrote:

> On Thu 04 Aug 02:21 PDT 2016, Lee Jones wrote:
> 
> > Before this patch, the dma_alloc_coherent() failure path printed out:
> > 
> >   "dma_alloc_coherent err: 16760832"
> > 
> > ... alluding to the Linux error code being 16760832, but seeing as
> > Linux error codes are all negative, this looks like a signed/unsigned
> > issue.  In fact, the message is trying to print the length of the
> > requested memory region.  Let's clear that up.
> > 
> > While we're at it, let's standardise the way 'len' is printed.  In
> > all other locations 'len' is in hex prefixed by a '0x' for clarity.
> > 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Again, this can just be applied.

> > ---
> >  drivers/remoteproc/remoteproc_core.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> > index aea29a75c..3566dc9 100644
> > --- a/drivers/remoteproc/remoteproc_core.c
> > +++ b/drivers/remoteproc/remoteproc_core.c
> > @@ -581,7 +581,7 @@ static int rproc_handle_carveout(struct rproc *rproc,
> >  		return -EINVAL;
> >  	}
> >  
> > -	dev_dbg(dev, "carveout rsc: da %x, pa %x, len %x, flags %x\n",
> > +	dev_dbg(dev, "carveout rsc: da %x, pa %x, len 0x%x, flags %x\n",
> >  			rsc->da, rsc->pa, rsc->len, rsc->flags);
> >  
> >  	carveout = kzalloc(sizeof(*carveout), GFP_KERNEL);
> > @@ -590,7 +590,8 @@ static int rproc_handle_carveout(struct rproc *rproc,
> >  
> >  	va = dma_alloc_coherent(dev->parent, rsc->len, &dma, GFP_KERNEL);
> >  	if (!va) {
> > -		dev_err(dev->parent, "dma_alloc_coherent err: %d\n", rsc->len);
> > +		dev_err(dev->parent,
> > +			"failed to allocate dma memory: len 0x%x\n", rsc->len);
> >  		ret = -ENOMEM;
> >  		goto free_carv;
> >  	}

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

#1459822 — Re: [PATCH 1/9] remoteproc: core: Ensure error message is clear

On 08/09/2016 01:12 PM, Lee Jones wrote:
> On Tue, 09 Aug 2016, Bjorn Andersson wrote:
> 
>> On Thu 04 Aug 02:21 PDT 2016, Lee Jones wrote:
>>
>>> Before this patch, the dma_alloc_coherent() failure path printed out:
>>>
>>>   "dma_alloc_coherent err: 16760832"
>>>
>>> ... alluding to the Linux error code being 16760832, but seeing as
>>> Linux error codes are all negative, this looks like a signed/unsigned
>>> issue.  In fact, the message is trying to print the length of the
>>> requested memory region.  Let's clear that up.
>>>
>>> While we're at it, let's standardise the way 'len' is printed.  In
>>> all other locations 'len' is in hex prefixed by a '0x' for clarity.
>>>
>>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>
>> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> Again, this can just be applied.
> 
>>> ---
>>>  drivers/remoteproc/remoteproc_core.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>>> index aea29a75c..3566dc9 100644
>>> --- a/drivers/remoteproc/remoteproc_core.c
>>> +++ b/drivers/remoteproc/remoteproc_core.c
>>> @@ -581,7 +581,7 @@ static int rproc_handle_carveout(struct rproc *rproc,
>>>  		return -EINVAL;
>>>  	}
>>>  
>>> -	dev_dbg(dev, "carveout rsc: da %x, pa %x, len %x, flags %x\n",
>>> +	dev_dbg(dev, "carveout rsc: da %x, pa %x, len 0x%x, flags %x\n",
>>>  			rsc->da, rsc->pa, rsc->len, rsc->flags);

If you are modifying this trace, it's better to following the leading 0x
convention on all arguments rather than just the length.

regards
Suman

>>>  
>>>  	carveout = kzalloc(sizeof(*carveout), GFP_KERNEL);
>>> @@ -590,7 +590,8 @@ static int rproc_handle_carveout(struct rproc *rproc,
>>>  
>>>  	va = dma_alloc_coherent(dev->parent, rsc->len, &dma, GFP_KERNEL);
>>>  	if (!va) {
>>> -		dev_err(dev->parent, "dma_alloc_coherent err: %d\n", rsc->len);
>>> +		dev_err(dev->parent,
>>> +			"failed to allocate dma memory: len 0x%x\n", rsc->len);
>>>  		ret = -ENOMEM;
>>>  		goto free_carv;
>>>  	}
> 

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web