From: Sjur Brændeland <[email protected]>
Hi Ohad,
I think I have addressed most of your review comments, except
dynamic detection of firmware format.
Warning: I have not been able to test this properly,
I don't yet have a proper test environment running.
Regards,
Sjur
Sjur Brændeland (7):
remoteproc: fix print format warnings
remoteproc: Fix missing fault indication in error-path
remoteproc: Pass struct fw to load_segments and find_rsc_table.
remoteproc: Assign boot_addr in a separate function
remoteproc: Move Elf related functions to separate file
remoteproc: Support custom firmware handlers
remoteproc: Add custom STE-modem firmware loader.
drivers/remoteproc/Kconfig | 4 +
drivers/remoteproc/Makefile | 2 +
drivers/remoteproc/omap_remoteproc.c | 1 +
drivers/remoteproc/remoteproc_core.c | 257 +------------------
drivers/remoteproc/remoteproc_elf_loader.c | 287 ++++++++++++++++++++++
drivers/remoteproc/remoteproc_ste_modem_loader.c | 176 +++++++++++++
include/linux/remoteproc.h | 57 +++++
7 files changed, 538 insertions(+), 246 deletions(-)
create mode 100644 drivers/remoteproc/remoteproc_elf_loader.c
create mode 100644 drivers/remoteproc/remoteproc_ste_modem_loader.c
--
1.7.5.4
From: Sjur Brændeland <[email protected]>
Prepare for introduction of custom firmware loaders by
moving the function operating on ELF data-structures into
separate functions. The assignment of boot_addr is moved
to the function rproc_set_boot_addr().
Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 26 +++++++++++++++++---------
1 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 07036e2..1dd7f53 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1003,6 +1003,22 @@ static int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
return 0;
}
+/**
+ * rproc_set_boot_addr() - Set rproc's boot address.
+ * @rproc: remote processor which needs boot address set.
+ * @fw: the the ELF firmware image
+ *
+ * This function reads the ELF entry point address and assign this
+ * to the rproc's boot address.
+ * Note that the boot address is not a configurable property of all remote
+ * processors. Some will always boot at a specific hard-coded address.
+ */
+void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw)
+{
+ struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
+ rproc->bootaddr = ehdr->e_entry;
+}
+
/*
* take a firmware and boot a remote processor with it.
*/
@@ -1010,7 +1026,6 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
{
struct device *dev = rproc->dev;
const char *name = rproc->firmware;
- struct elf32_hdr *ehdr;
struct resource_table *table;
int ret, tablesz;
@@ -1018,8 +1033,6 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
if (ret)
return ret;
- ehdr = (struct elf32_hdr *)fw->data;
-
dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
/*
@@ -1032,12 +1045,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
return ret;
}
- /*
- * The ELF entry point is the rproc's boot addr (though this is not
- * a configurable property of all remote processors: some will always
- * boot at a specific hardcoded address).
- */
- rproc->bootaddr = ehdr->e_entry;
+ rproc_set_boot_addr(rproc, fw);
/* look for the resource table */
table = rproc_find_rsc_table(rproc, fw, &tablesz);
--
1.7.5.4
From: Sjur Brændeland <[email protected]>
Prepare for introduction of custom firmware loaders by changing
the functions rproc_find_rcs_table() and rproc_load_segments()
to use struct firmware as parameter.
When the custom loader framework is introduced all calls into
the firmware specific function must use struct firmware as
parameter.
Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 9d6343d..07036e2 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -192,8 +192,7 @@ static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
/**
* rproc_load_segments() - load firmware segments to memory
* @rproc: remote processor which will be booted using these fw segments
- * @elf_data: the content of the ELF firmware image
- * @len: firmware size (in bytes)
+ * @fw: the the ELF firmware image
*
* This function loads the firmware segments to memory, where the remote
* processor expects them.
@@ -215,12 +214,13 @@ static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
* supported, though.
*/
static int
-rproc_load_segments(struct rproc *rproc, const u8 *elf_data, size_t len)
+rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
{
struct device *dev = rproc->dev;
struct elf32_hdr *ehdr;
struct elf32_phdr *phdr;
int i, ret = 0;
+ const u8 *elf_data = fw->data;
ehdr = (struct elf32_hdr *)elf_data;
phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff);
@@ -246,9 +246,9 @@ rproc_load_segments(struct rproc *rproc, const u8 *elf_data, size_t len)
break;
}
- if (offset + filesz > len) {
+ if (offset + filesz > fw->size) {
dev_err(dev, "truncated fw: need 0x%x avail 0x%zx\n",
- offset + filesz, len);
+ offset + filesz, fw->size);
ret = -EINVAL;
break;
}
@@ -824,8 +824,7 @@ rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int l
/**
* rproc_find_rsc_table() - find the resource table
* @rproc: the rproc handle
- * @elf_data: the content of the ELF firmware image
- * @len: firmware size (in bytes)
+ * @fw: the ELF firmware image
* @tablesz: place holder for providing back the table size
*
* This function finds the resource table inside the remote processor's
@@ -838,7 +837,7 @@ rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int l
* (and @tablesz isn't set).
*/
static struct resource_table *
-rproc_find_rsc_table(struct rproc *rproc, const u8 *elf_data, size_t len,
+rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
int *tablesz)
{
struct elf32_hdr *ehdr;
@@ -847,6 +846,7 @@ rproc_find_rsc_table(struct rproc *rproc, const u8 *elf_data, size_t len,
struct device *dev = rproc->dev;
struct resource_table *table = NULL;
int i;
+ const u8 *elf_data = fw->data;
ehdr = (struct elf32_hdr *)elf_data;
shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff);
@@ -863,7 +863,7 @@ rproc_find_rsc_table(struct rproc *rproc, const u8 *elf_data, size_t len,
table = (struct resource_table *)(elf_data + offset);
/* make sure we have the entire table */
- if (offset + size > len) {
+ if (offset + size > fw->size) {
dev_err(dev, "resource table truncated\n");
return NULL;
}
@@ -1040,7 +1040,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
rproc->bootaddr = ehdr->e_entry;
/* look for the resource table */
- table = rproc_find_rsc_table(rproc, fw->data, fw->size, &tablesz);
+ table = rproc_find_rsc_table(rproc, fw, &tablesz);
if (!table) {
ret = -EINVAL;
goto clean_up;
@@ -1054,7 +1054,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
}
/* load the ELF segments to memory */
- ret = rproc_load_segments(rproc, fw->data, fw->size);
+ ret = rproc_load_segments(rproc, fw);
if (ret) {
dev_err(dev, "Failed to load program segments: %d\n", ret);
goto clean_up;
@@ -1097,7 +1097,7 @@ static void rproc_fw_config_virtio(const struct firmware *fw, void *context)
goto out;
/* look for the resource table */
- table = rproc_find_rsc_table(rproc, fw->data, fw->size, &tablesz);
+ table = rproc_find_rsc_table(rproc, fw, &tablesz);
if (!table)
goto out;
--
1.7.5.4
From: Sjur Brændeland <[email protected]>
Add custom firmware loader for STE firmware. This plugin adds
functions for extracting the resource table and loading the
firmware image into shared memory.
Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/Kconfig | 4 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/remoteproc_ste_modem_loader.c | 176 ++++++++++++++++++++++
include/linux/remoteproc.h | 1 +
4 files changed, 182 insertions(+), 0 deletions(-)
create mode 100644 drivers/remoteproc/remoteproc_ste_modem_loader.c
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 24d880e..fe615d1 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -25,4 +25,8 @@ config OMAP_REMOTEPROC
It's safe to say n here if you're not interested in multimedia
offloading or just want a bare minimum kernel.
+config STE_MODEM_RPROC_FW
+ bool
+ depends on REMOTEPROC
+
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 934ce6e..431b3cf 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -7,4 +7,5 @@ remoteproc-y := remoteproc_core.o
remoteproc-y += remoteproc_debugfs.o
remoteproc-y += remoteproc_virtio.o
remoteproc-y += remoteproc_elf_loader.o
+remoteproc-$(CONFIG_STE_MODEM_RPROC_FW) += remoteproc_ste_modem_loader.o
obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
diff --git a/drivers/remoteproc/remoteproc_ste_modem_loader.c b/drivers/remoteproc/remoteproc_ste_modem_loader.c
new file mode 100644
index 0000000..3ba308f
--- /dev/null
+++ b/drivers/remoteproc/remoteproc_ste_modem_loader.c
@@ -0,0 +1,176 @@
+/*
+ * Copyright (C) ST-Ericsson AB 2012
+ * Author: Sjur Brendeland / [email protected]
+ * License terms: GNU General Public License (GPL) version 2.
+ */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+#include <linux/module.h>
+#include <linux/firmware.h>
+#include <linux/remoteproc.h>
+#include "remoteproc_internal.h"
+
+#define TOC_RSC_TAB_NAME "rsc-table"
+
+/* struct toc_entry - Table of content entry
+ *
+ * @start: Offset to the image data.
+ * @size: Size of the images in bytes.
+ * @flags: Use 0 if no flags are in use.
+ * @entry_point: Modem internal information. Where to jump to start executing.
+ * Only applicable when using SDRAM. Set to 0xffffffff if unused.
+ * @load_addr: Modem internal information. Location in SDRAM to move image.
+ * Set to 0xffffffff if not applicable.
+ * @name: Name of image.
+ */
+struct toc_entry {
+ __le32 start;
+ __le32 size;
+ __le32 flags;
+ __le32 entry_point;
+ __le32 load_addr;
+ char name[12];
+};
+
+/** struct toc - Table of content
+ * @table: Table of toc entries.
+ * The Table Of Content is located at the start of the firmware image and
+ * at offset zero in the shared memory region. The resource table typically
+ * contains the initial boot image (boot strap) and other information elements
+ * such as remoteproc resource table. Each entry is identified by a unique
+ * @name.
+ */
+struct toc {
+ struct toc_entry table[32];
+};
+
+/**
+ * ste_load_segments() - load firmware segments to memory
+ * @rproc: remote processor which will be booted using these fw segments
+ * @fw: the TOC and firmware image to load
+ *
+ * This function loads the firmware segments to memory. STE Modem SHM
+ * does not use an IOMMU, and expects the firmware containing the
+ * "Table Of Content" (TOC) first in the firmware. The TOC specifies the
+ * offset and size of the boot image.
+ */
+static int
+ste_load_segments(struct rproc *rproc, const struct firmware *fw)
+{
+ /*
+ * STE-Modem does not use a IOMMU and the virtual and physical
+ * addresses of the device (modem) is not known. Instead
+ * offsets from the start of the shared memory region is used
+ * as device address (da).
+ *
+ * The STE-Modem must provide a carveout as the first entry with
+ * sufficient space for the firmware image. The device address in
+ * this carveout must be set to zero.
+ *
+ * The firmware must be copied into offset zero, i.e at the start of
+ * the shared memory area.
+ */
+
+ u32 offset = 0;
+ void *ptr = rproc_da_to_va(rproc, offset, fw->size);
+
+ if (!ptr) {
+ dev_err(rproc->dev, "bad offset 0x%x mem 0x%zx\n",
+ offset, fw->size);
+ return -EINVAL;
+ }
+
+ memcpy(ptr, fw->data, fw->size);
+ return 0;
+}
+
+/* Find the entry for resource table in the Table of Content */
+static struct toc_entry *__find_rsc_entry(const struct firmware *fw)
+{
+ int i;
+ struct toc *toc = (void *)fw->data;
+ int entries = ARRAY_SIZE(toc->table);
+
+ /* Search the table for the resource table */
+ for (i = 0; i < entries && toc->table[i].start != 0xffffffff; i++) {
+ if (!strncmp(toc->table[i].name, TOC_RSC_TAB_NAME,
+ sizeof(toc->table[i].name))) {
+ if (toc->table[i].start > fw->size)
+ return NULL;
+ return &toc->table[i];
+ }
+ }
+ return NULL;
+}
+
+/**
+ * ste_find_rsc_table() - find the resource table
+ * @rproc: the rproc handle
+ * @fw: the firmware image
+ * @tablesz: place holder for providing back the table size
+ *
+ * This function finds the resource table inside the remote processor's
+ * firmware. It is used both upon the registration of @rproc (in order
+ * to look for and register the supported virito devices), and when the
+ * @rproc is booted.
+ *
+ * Returns the pointer to the resource table if it is found, and write its
+ * size into @tablesz. If a valid table isn't found, NULL is returned
+ * (and @tablesz isn't set).
+ */
+static struct resource_table *
+ste_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
+ int *tablesz)
+{
+ struct resource_table *table;
+ struct device *dev = rproc->dev;
+ struct toc_entry *entry = __find_rsc_entry(fw);
+
+ if (!entry) {
+ dev_err(dev, "resource table not found in fw\n");
+ return NULL;
+ }
+
+ table = (void *) (fw->data + entry->start);
+
+ /* make sure we have the entire table */
+ if (entry->start + entry->size > fw->size) {
+ dev_err(dev, "resource table truncated\n");
+ return NULL;
+ }
+
+ /* make sure table has at least the header */
+ if (sizeof(struct resource_table) > entry->size) {
+ dev_err(dev, "header-less resource table\n");
+ return NULL;
+ }
+
+ /* we don't support any version beyond the first */
+ if (table->ver != 1) {
+ dev_err(dev, "unsupported fw ver: %d\n", table->ver);
+ return NULL;
+ }
+
+ /* make sure reserved bytes are zeroes */
+ if (table->reserved[0] || table->reserved[1]) {
+ dev_err(dev, "non zero reserved bytes\n");
+ return NULL;
+ }
+
+ /* make sure the offsets array isn't truncated */
+ if (table->num * sizeof(table->offset[0]) +
+ sizeof(struct resource_table) > entry->size) {
+ dev_err(dev, "resource table incomplete\n");
+ return NULL;
+ }
+
+ *tablesz = entry->size;
+
+ return table;
+}
+
+struct rproc_fw_ops rproc_ste_modem_fw_ops = {
+ .load = ste_load_segments,
+ .find_rsc_table = ste_find_rsc_table
+};
+EXPORT_SYMBOL(rproc_ste_modem_fw_ops);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 23c5c254..1cb4f89 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -530,5 +530,6 @@ rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
}
extern struct rproc_fw_ops rproc_elf_fw_ops;
+extern struct rproc_fw_ops rproc_ste_modem_fw_ops;
#endif /* REMOTEPROC_H */
--
1.7.5.4
From: Sjur Brændeland <[email protected]>
Fix compile warnings from GCC 4.6.1 when printing values of type size_t.
drivers/remoteproc/remoteproc_core.c:251:6:
warning: format ‘%x’ expects argument of type ‘unsigned int’,
but argument 4 has type ‘size_t’ [-Wformat]
drivers/remoteproc/remoteproc_core.c:938:9:
warning: format ‘%u’ expects argument of type ‘unsigned int’,
but argument 4 has type ‘size_t’ [-Wformat]
drivers/remoteproc/remoteproc_core.c:1023:2:
warning: format ‘%d’ expects argument of type ‘int’,
but argument 4 has type ‘size_t’ [-Wformat]
Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index ee15c68..1bba479 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -247,7 +247,7 @@ rproc_load_segments(struct rproc *rproc, const u8 *elf_data, size_t len)
}
if (offset + filesz > len) {
- dev_err(dev, "truncated fw: need 0x%x avail 0x%x\n",
+ dev_err(dev, "truncated fw: need 0x%x avail 0x%zx\n",
offset + filesz, len);
ret = -EINVAL;
break;
@@ -934,7 +934,7 @@ static void rproc_resource_cleanup(struct rproc *rproc)
unmapped = iommu_unmap(rproc->domain, entry->da, entry->len);
if (unmapped != entry->len) {
/* nothing much to do besides complaining */
- dev_err(dev, "failed to unmap %u/%u\n", entry->len,
+ dev_err(dev, "failed to unmap %u/%zu\n", entry->len,
unmapped);
}
@@ -1020,7 +1020,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
ehdr = (struct elf32_hdr *)fw->data;
- dev_info(dev, "Booting fw image %s, size %d\n", name, fw->size);
+ dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
/*
* if enabling an IOMMU isn't relevant for this rproc, this is
--
1.7.5.4
From: Sjur Brændeland <[email protected]>
Firmware handling is made customizable.
This is done by creating a separate ops structure for the
firmware functions that depends on a particular firmware
format (such as ELF). The ELF specific functions are exported
by the structure rproc_elf_fw_ops.
Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/omap_remoteproc.c | 1 +
drivers/remoteproc/remoteproc_core.c | 2 +-
drivers/remoteproc/remoteproc_elf_loader.c | 22 +++++++---
include/linux/remoteproc.h | 61 ++++++++++++++++++++++++----
4 files changed, 70 insertions(+), 16 deletions(-)
diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
index 69425c4..53ceaab 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -171,6 +171,7 @@ static struct rproc_ops omap_rproc_ops = {
.start = omap_rproc_start,
.stop = omap_rproc_stop,
.kick = omap_rproc_kick,
+ .fw = &rproc_elf_fw_ops
};
static int __devinit omap_rproc_probe(struct platform_device *pdev)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index db5359a..a68df8c 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1237,7 +1237,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
{
struct rproc *rproc;
- if (!dev || !name || !ops)
+ if (!dev || !name || !ops || !ops->fw_ops)
return NULL;
rproc = kzalloc(sizeof(struct rproc) + len, GFP_KERNEL);
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index 2017ae3..9a53871 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -33,8 +33,8 @@
#include "remoteproc_internal.h"
/* make sure this fw image is sane */
-int
-rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
+static int
+elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
{
const char *name = rproc->firmware;
struct device *dev = rproc->dev;
@@ -103,7 +103,7 @@ rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
* Note that the boot address is not a configurable property of all remote
* processors. Some will always boot at a specific hard-coded address.
*/
-void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw)
+static void elf_set_boot_addr(struct rproc *rproc, const struct firmware *fw)
{
struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
rproc->bootaddr = ehdr->e_entry;
@@ -133,8 +133,8 @@ void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw)
* directly allocate memory for every segment/resource. This is not yet
* supported, though.
*/
-int
-rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
+static int
+elf_load_segments(struct rproc *rproc, const struct firmware *fw)
{
struct device *dev = rproc->dev;
struct elf32_hdr *ehdr;
@@ -214,8 +214,8 @@ rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
* size into @tablesz. If a valid table isn't found, NULL is returned
* (and @tablesz isn't set).
*/
-struct resource_table *
-rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
+static struct resource_table *
+elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
int *tablesz)
{
struct elf32_hdr *ehdr;
@@ -277,3 +277,11 @@ rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
return table;
}
+
+struct rproc_fw_ops rproc_elf_fw_ops = {
+ .load = elf_load_segments,
+ .find_rsc_table = elf_find_rsc_table,
+ .sanity_check = elf_sanity_check,
+ .set_boot_addr = elf_set_boot_addr
+};
+EXPORT_SYMBOL(rproc_elf_fw_ops);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 997e816..23c5c254 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -327,25 +327,37 @@ struct rproc_mem_entry {
struct rproc;
-int
-rproc_load_segments(struct rproc *rproc, const struct firmware *fw);
-struct resource_table *
-rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
- int *tablesz);
-int
-rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw);
-void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw);
+/**
+ * struct rproc_fw_ops - firmware format specific operations.
+ *
+ * @find_rsc_table: finds the resource table inside the firmware image.
+ * @load_fw: load firmeware to memory, where the remote processor
+ * expects to find it.
+ * @sanity_check: sanity check the fw image.
+ * @set_boot_addr: set boot address to entry point as specified in
+ * firmware.
+ */
+struct rproc_fw_ops {
+ struct resource_table *(*find_rsc_table) (struct rproc *rproc,
+ const struct firmware *fw,
+ int *tablesz);
+ int (*load)(struct rproc *rproc, const struct firmware *fw);
+ int (*sanity_check)(struct rproc *rproc, const struct firmware *fw);
+ void (*set_boot_addr)(struct rproc *rproc, const struct firmware *fw);
+};
/**
* struct rproc_ops - platform-specific device handlers
* @start: power on the device and boot it
* @stop: power off the device
* @kick: kick a virtqueue (virtqueue id given as a parameter)
+ * @fw_ops: firmware operations for the remote processor
*/
struct rproc_ops {
int (*start)(struct rproc *rproc);
int (*stop)(struct rproc *rproc);
void (*kick)(struct rproc *rproc, int vqid);
+ struct rproc_fw_ops *fw_ops;
};
/**
@@ -486,4 +498,37 @@ static inline struct rproc *vdev_to_rproc(struct virtio_device *vdev)
return rvdev->rproc;
}
+static inline int
+rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
+{
+ if (rproc->ops->fw_ops->sanity_check)
+ return rproc->ops->fw_ops->sanity_check(rproc, fw);
+ return 0;
+}
+
+static inline
+void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw)
+{
+ if (rproc->ops->fw_ops->set_boot_addr)
+ rproc->ops->fw_ops->set_boot_addr(rproc, fw);
+}
+
+static inline int
+rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
+{
+ if (rproc->ops->fw_ops->load)
+ return rproc->ops->fw_ops->load(rproc, fw);
+ return -EINVAL;
+}
+static inline struct resource_table *
+rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
+ int *tablesz)
+{
+ if (rproc->ops->fw_ops->find_rsc_table)
+ return rproc->ops->fw_ops->find_rsc_table(rproc, fw, tablesz);
+ return NULL;
+}
+
+extern struct rproc_fw_ops rproc_elf_fw_ops;
+
#endif /* REMOTEPROC_H */
--
1.7.5.4
From: Sjur Brændeland <[email protected]>
If rproc_find_rsc_table() fails, rproc_fw_boot() must set
return-value before jumping to clean_up label. Otherwise no
error value is returned.
Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 1bba479..9d6343d 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1041,8 +1041,10 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
/* look for the resource table */
table = rproc_find_rsc_table(rproc, fw->data, fw->size, &tablesz);
- if (!table)
+ if (!table) {
+ ret = -EINVAL;
goto clean_up;
+ }
/* handle fw resources which are required to boot rproc */
ret = rproc_handle_boot_rsc(rproc, table, tablesz);
--
1.7.5.4
From: Sjur Brændeland <[email protected]>
Prepare for introduction of custom firmware loaders by
moving all ELF related handling into a separate file.
The functions: rproc_find_rsc_table(), rproc_fw_sanity_check(),
rproc_find_rsc_table() and rproc_set_boot_addr() are moved
to the new file remoteproc_elf_loader.c. No functional
changes are introduced in this patch.
Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/remoteproc_core.c | 247 +------------------------
drivers/remoteproc/remoteproc_elf_loader.c | 279 ++++++++++++++++++++++++++++
include/linux/remoteproc.h | 11 +
4 files changed, 292 insertions(+), 246 deletions(-)
create mode 100644 drivers/remoteproc/remoteproc_elf_loader.c
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 5445d9b..934ce6e 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -6,4 +6,5 @@ obj-$(CONFIG_REMOTEPROC) += remoteproc.o
remoteproc-y := remoteproc_core.o
remoteproc-y += remoteproc_debugfs.o
remoteproc-y += remoteproc_virtio.o
+remoteproc-y += remoteproc_elf_loader.o
obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 1dd7f53..db5359a 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -165,7 +165,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.
*/
-static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
{
struct rproc_mem_entry *carveout;
void *ptr = NULL;
@@ -189,96 +189,6 @@ static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
return ptr;
}
-/**
- * rproc_load_segments() - load firmware segments to memory
- * @rproc: remote processor which will be booted using these fw segments
- * @fw: the the ELF firmware image
- *
- * This function loads the firmware segments to memory, where the remote
- * processor expects them.
- *
- * Some remote processors will expect their code and data to be placed
- * in specific device addresses, and can't have them dynamically assigned.
- *
- * We currently support only those kind of remote processors, and expect
- * the program header's paddr member to contain those addresses. We then go
- * through the physically contiguous "carveout" memory regions which we
- * allocated (and mapped) earlier on behalf of the remote processor,
- * and "translate" device address to kernel addresses, so we can copy the
- * segments where they are expected.
- *
- * Currently we only support remote processors that required carveout
- * allocations and got them mapped onto their iommus. Some processors
- * might be different: they might not have iommus, and would prefer to
- * directly allocate memory for every segment/resource. This is not yet
- * supported, though.
- */
-static int
-rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
-{
- struct device *dev = rproc->dev;
- struct elf32_hdr *ehdr;
- struct elf32_phdr *phdr;
- int i, ret = 0;
- const u8 *elf_data = fw->data;
-
- ehdr = (struct elf32_hdr *)elf_data;
- phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff);
-
- /* go through the available ELF segments */
- for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
- u32 da = phdr->p_paddr;
- u32 memsz = phdr->p_memsz;
- u32 filesz = phdr->p_filesz;
- u32 offset = phdr->p_offset;
- void *ptr;
-
- if (phdr->p_type != PT_LOAD)
- continue;
-
- dev_dbg(dev, "phdr: type %d da 0x%x memsz 0x%x filesz 0x%x\n",
- phdr->p_type, da, memsz, filesz);
-
- if (filesz > memsz) {
- dev_err(dev, "bad phdr filesz 0x%x memsz 0x%x\n",
- filesz, memsz);
- ret = -EINVAL;
- break;
- }
-
- if (offset + filesz > fw->size) {
- dev_err(dev, "truncated fw: need 0x%x avail 0x%zx\n",
- offset + filesz, fw->size);
- ret = -EINVAL;
- break;
- }
-
- /* grab the kernel address for this device address */
- ptr = rproc_da_to_va(rproc, da, memsz);
- if (!ptr) {
- dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
- ret = -EINVAL;
- break;
- }
-
- /* put the segment where the remote processor expects it */
- if (phdr->p_filesz)
- memcpy(ptr, elf_data + phdr->p_offset, filesz);
-
- /*
- * Zero out remaining memory for this segment.
- *
- * This isn't strictly required since dma_alloc_coherent already
- * did this for us. albeit harmless, we may consider removing
- * this.
- */
- if (memsz > filesz)
- memset(ptr + filesz, 0, memsz - filesz);
- }
-
- return ret;
-}
-
static int
__rproc_handle_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)
{
@@ -822,85 +732,6 @@ rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int l
}
/**
- * rproc_find_rsc_table() - find the resource table
- * @rproc: the rproc handle
- * @fw: the ELF firmware image
- * @tablesz: place holder for providing back the table size
- *
- * This function finds the resource table inside the remote processor's
- * firmware. It is used both upon the registration of @rproc (in order
- * to look for and register the supported virito devices), and when the
- * @rproc is booted.
- *
- * Returns the pointer to the resource table if it is found, and write its
- * size into @tablesz. If a valid table isn't found, NULL is returned
- * (and @tablesz isn't set).
- */
-static struct resource_table *
-rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
- int *tablesz)
-{
- struct elf32_hdr *ehdr;
- struct elf32_shdr *shdr;
- const char *name_table;
- struct device *dev = rproc->dev;
- struct resource_table *table = NULL;
- int i;
- const u8 *elf_data = fw->data;
-
- ehdr = (struct elf32_hdr *)elf_data;
- shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff);
- name_table = elf_data + shdr[ehdr->e_shstrndx].sh_offset;
-
- /* look for the resource table and handle it */
- for (i = 0; i < ehdr->e_shnum; i++, shdr++) {
- int size = shdr->sh_size;
- int offset = shdr->sh_offset;
-
- if (strcmp(name_table + shdr->sh_name, ".resource_table"))
- continue;
-
- table = (struct resource_table *)(elf_data + offset);
-
- /* make sure we have the entire table */
- if (offset + size > fw->size) {
- dev_err(dev, "resource table truncated\n");
- return NULL;
- }
-
- /* make sure table has at least the header */
- if (sizeof(struct resource_table) > size) {
- dev_err(dev, "header-less resource table\n");
- return NULL;
- }
-
- /* we don't support any version beyond the first */
- if (table->ver != 1) {
- dev_err(dev, "unsupported fw ver: %d\n", table->ver);
- return NULL;
- }
-
- /* make sure reserved bytes are zeroes */
- if (table->reserved[0] || table->reserved[1]) {
- dev_err(dev, "non zero reserved bytes\n");
- return NULL;
- }
-
- /* make sure the offsets array isn't truncated */
- if (table->num * sizeof(table->offset[0]) +
- sizeof(struct resource_table) > size) {
- dev_err(dev, "resource table incomplete\n");
- return NULL;
- }
-
- *tablesz = shdr->sh_size;
- break;
- }
-
- return table;
-}
-
-/**
* rproc_resource_cleanup() - clean up and free all acquired resources
* @rproc: rproc handle
*
@@ -943,82 +774,6 @@ static void rproc_resource_cleanup(struct rproc *rproc)
}
}
-/* make sure this fw image is sane */
-static int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
-{
- const char *name = rproc->firmware;
- struct device *dev = rproc->dev;
- struct elf32_hdr *ehdr;
- char class;
-
- if (!fw) {
- dev_err(dev, "failed to load %s\n", name);
- return -EINVAL;
- }
-
- if (fw->size < sizeof(struct elf32_hdr)) {
- dev_err(dev, "Image is too small\n");
- return -EINVAL;
- }
-
- ehdr = (struct elf32_hdr *)fw->data;
-
- /* We only support ELF32 at this point */
- class = ehdr->e_ident[EI_CLASS];
- if (class != ELFCLASS32) {
- dev_err(dev, "Unsupported class: %d\n", class);
- return -EINVAL;
- }
-
- /* We assume the firmware has the same endianess as the host */
-# ifdef __LITTLE_ENDIAN
- if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) {
-# else /* BIG ENDIAN */
- if (ehdr->e_ident[EI_DATA] != ELFDATA2MSB) {
-# endif
- dev_err(dev, "Unsupported firmware endianess\n");
- return -EINVAL;
- }
-
- if (fw->size < ehdr->e_shoff + sizeof(struct elf32_shdr)) {
- dev_err(dev, "Image is too small\n");
- return -EINVAL;
- }
-
- if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG)) {
- dev_err(dev, "Image is corrupted (bad magic)\n");
- return -EINVAL;
- }
-
- if (ehdr->e_phnum == 0) {
- dev_err(dev, "No loadable segments\n");
- return -EINVAL;
- }
-
- if (ehdr->e_phoff > fw->size) {
- dev_err(dev, "Firmware size is too small\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
-/**
- * rproc_set_boot_addr() - Set rproc's boot address.
- * @rproc: remote processor which needs boot address set.
- * @fw: the the ELF firmware image
- *
- * This function reads the ELF entry point address and assign this
- * to the rproc's boot address.
- * Note that the boot address is not a configurable property of all remote
- * processors. Some will always boot at a specific hard-coded address.
- */
-void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw)
-{
- struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
- rproc->bootaddr = ehdr->e_entry;
-}
-
/*
* take a firmware and boot a remote processor with it.
*/
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
new file mode 100644
index 0000000..2017ae3
--- /dev/null
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -0,0 +1,279 @@
+/*
+ * Remote Processor Framework Elf loader
+ *
+ * Copyright (C) 2011 Texas Instruments, Inc.
+ * Copyright (C) 2011 Google, Inc.
+ *
+ * Ohad Ben-Cohen <[email protected]>
+ * Brian Swetland <[email protected]>
+ * Mark Grosen <[email protected]>
+ * Fernando Guzman Lugo <[email protected]>
+ * Suman Anna <[email protected]>
+ * Robert Tivy <[email protected]>
+ * Armando Uribe De Leon <[email protected]>
+ * Sjur Brændeland <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#include <linux/module.h>
+#include <linux/firmware.h>
+#include <linux/remoteproc.h>
+#include <linux/elf.h>
+
+#include "remoteproc_internal.h"
+
+/* make sure this fw image is sane */
+int
+rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
+{
+ const char *name = rproc->firmware;
+ struct device *dev = rproc->dev;
+ struct elf32_hdr *ehdr;
+ char class;
+
+ if (!fw) {
+ dev_err(dev, "failed to load %s\n", name);
+ return -EINVAL;
+ }
+
+ if (fw->size < sizeof(struct elf32_hdr)) {
+ dev_err(dev, "Image is too small\n");
+ return -EINVAL;
+ }
+
+ ehdr = (struct elf32_hdr *)fw->data;
+
+ /* We only support ELF32 at this point */
+ class = ehdr->e_ident[EI_CLASS];
+ if (class != ELFCLASS32) {
+ dev_err(dev, "Unsupported class: %d\n", class);
+ return -EINVAL;
+ }
+
+ /* We assume the firmware has the same endianess as the host */
+# ifdef __LITTLE_ENDIAN
+ if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) {
+# else /* BIG ENDIAN */
+ if (ehdr->e_ident[EI_DATA] != ELFDATA2MSB) {
+# endif
+ dev_err(dev, "Unsupported firmware endianess\n");
+ return -EINVAL;
+ }
+
+ if (fw->size < ehdr->e_shoff + sizeof(struct elf32_shdr)) {
+ dev_err(dev, "Image is too small\n");
+ return -EINVAL;
+ }
+
+ if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG)) {
+ dev_err(dev, "Image is corrupted (bad magic)\n");
+ return -EINVAL;
+ }
+
+ if (ehdr->e_phnum == 0) {
+ dev_err(dev, "No loadable segments\n");
+ return -EINVAL;
+ }
+
+ if (ehdr->e_phoff > fw->size) {
+ dev_err(dev, "Firmware size is too small\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/**
+ * rproc_set_boot_addr() - Set rproc's boot address.
+ * @rproc: remote processor which needs boot address set.
+ * @fw: the the ELF firmware image
+ *
+ * This function reads the ELF entry point address and assign this
+ * to the rproc's boot address.
+ * Note that the boot address is not a configurable property of all remote
+ * processors. Some will always boot at a specific hard-coded address.
+ */
+void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw)
+{
+ struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
+ rproc->bootaddr = ehdr->e_entry;
+}
+
+/**
+ * rproc_load_segments() - load firmware segments to memory
+ * @rproc: remote processor which will be booted using these fw segments
+ * @fw: the the ELF firmware image
+ *
+ * This function loads the firmware segments to memory, where the remote
+ * processor expects them.
+ *
+ * Some remote processors will expect their code and data to be placed
+ * in specific device addresses, and can't have them dynamically assigned.
+ *
+ * We currently support only those kind of remote processors, and expect
+ * the program header's paddr member to contain those addresses. We then go
+ * through the physically contiguous "carveout" memory regions which we
+ * allocated (and mapped) earlier on behalf of the remote processor,
+ * and "translate" device address to kernel addresses, so we can copy the
+ * segments where they are expected.
+ *
+ * Currently we only support remote processors that required carveout
+ * allocations and got them mapped onto their iommus. Some processors
+ * might be different: they might not have iommus, and would prefer to
+ * directly allocate memory for every segment/resource. This is not yet
+ * supported, though.
+ */
+int
+rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
+{
+ struct device *dev = rproc->dev;
+ struct elf32_hdr *ehdr;
+ struct elf32_phdr *phdr;
+ int i, ret = 0;
+ const u8 *elf_data = fw->data;
+
+ ehdr = (struct elf32_hdr *)elf_data;
+ phdr = (struct elf32_phdr *)(elf_data + ehdr->e_phoff);
+
+ /* go through the available ELF segments */
+ for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
+ u32 da = phdr->p_paddr;
+ u32 memsz = phdr->p_memsz;
+ u32 filesz = phdr->p_filesz;
+ u32 offset = phdr->p_offset;
+ void *ptr;
+
+ if (phdr->p_type != PT_LOAD)
+ continue;
+
+ dev_dbg(dev, "phdr: type %d da 0x%x memsz 0x%x filesz 0x%x\n",
+ phdr->p_type, da, memsz, filesz);
+
+ if (filesz > memsz) {
+ dev_err(dev, "bad phdr filesz 0x%x memsz 0x%x\n",
+ filesz, memsz);
+ ret = -EINVAL;
+ break;
+ }
+
+ if (offset + filesz > fw->size) {
+ dev_err(dev, "truncated fw: need 0x%x avail 0x%zx\n",
+ offset + filesz, fw->size);
+ ret = -EINVAL;
+ break;
+ }
+
+ /* grab the kernel address for this device address */
+ ptr = rproc_da_to_va(rproc, da, memsz);
+ if (!ptr) {
+ dev_err(dev, "bad phdr da 0x%x mem 0x%x\n", da, memsz);
+ ret = -EINVAL;
+ break;
+ }
+
+ /* put the segment where the remote processor expects it */
+ if (phdr->p_filesz)
+ memcpy(ptr, elf_data + phdr->p_offset, filesz);
+
+ /*
+ * Zero out remaining memory for this segment.
+ *
+ * This isn't strictly required since dma_alloc_coherent already
+ * did this for us. albeit harmless, we may consider removing
+ * this.
+ */
+ if (memsz > filesz)
+ memset(ptr + filesz, 0, memsz - filesz);
+ }
+
+ return ret;
+}
+
+/**
+ * rproc_find_rsc_table() - find the resource table
+ * @rproc: the rproc handle
+ * @fw: the ELF firmware image
+ * @tablesz: place holder for providing back the table size
+ *
+ * This function finds the resource table inside the remote processor's
+ * firmware. It is used both upon the registration of @rproc (in order
+ * to look for and register the supported virito devices), and when the
+ * @rproc is booted.
+ *
+ * Returns the pointer to the resource table if it is found, and write its
+ * size into @tablesz. If a valid table isn't found, NULL is returned
+ * (and @tablesz isn't set).
+ */
+struct resource_table *
+rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
+ int *tablesz)
+{
+ struct elf32_hdr *ehdr;
+ struct elf32_shdr *shdr;
+ const char *name_table;
+ struct device *dev = rproc->dev;
+ struct resource_table *table = NULL;
+ int i;
+ const u8 *elf_data = fw->data;
+
+ ehdr = (struct elf32_hdr *)elf_data;
+ shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff);
+ name_table = elf_data + shdr[ehdr->e_shstrndx].sh_offset;
+
+ /* look for the resource table and handle it */
+ for (i = 0; i < ehdr->e_shnum; i++, shdr++) {
+ int size = shdr->sh_size;
+ int offset = shdr->sh_offset;
+
+ if (strcmp(name_table + shdr->sh_name, ".resource_table"))
+ continue;
+
+ table = (struct resource_table *)(elf_data + offset);
+
+ /* make sure we have the entire table */
+ if (offset + size > fw->size) {
+ dev_err(dev, "resource table truncated\n");
+ return NULL;
+ }
+
+ /* make sure table has at least the header */
+ if (sizeof(struct resource_table) > size) {
+ dev_err(dev, "header-less resource table\n");
+ return NULL;
+ }
+
+ /* we don't support any version beyond the first */
+ if (table->ver != 1) {
+ dev_err(dev, "unsupported fw ver: %d\n", table->ver);
+ return NULL;
+ }
+
+ /* make sure reserved bytes are zeroes */
+ if (table->reserved[0] || table->reserved[1]) {
+ dev_err(dev, "non zero reserved bytes\n");
+ return NULL;
+ }
+
+ /* make sure the offsets array isn't truncated */
+ if (table->num * sizeof(table->offset[0]) +
+ sizeof(struct resource_table) > size) {
+ dev_err(dev, "resource table incomplete\n");
+ return NULL;
+ }
+
+ *tablesz = shdr->sh_size;
+ break;
+ }
+
+ return table;
+}
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index f1ffabb..997e816 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -42,6 +42,7 @@
#include <linux/virtio.h>
#include <linux/completion.h>
#include <linux/idr.h>
+#include <linux/firmware.h>
/**
* struct resource_table - firmware resource table header
@@ -326,6 +327,15 @@ struct rproc_mem_entry {
struct rproc;
+int
+rproc_load_segments(struct rproc *rproc, const struct firmware *fw);
+struct resource_table *
+rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
+ int *tablesz);
+int
+rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw);
+void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw);
+
/**
* struct rproc_ops - platform-specific device handlers
* @start: power on the device and boot it
@@ -462,6 +472,7 @@ int rproc_unregister(struct rproc *rproc);
int rproc_boot(struct rproc *rproc);
void rproc_shutdown(struct rproc *rproc);
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
static inline struct rproc_vdev *vdev_to_rvdev(struct virtio_device *vdev)
{
--
1.7.5.4
On Wed, Jun 6, 2012 at 4:38 PM, <[email protected]> wrote:
> From: Sjur Br?ndeland <[email protected]>
>
> Fix compile warnings from GCC 4.6.1 when printing values of type size_t.
>
> drivers/remoteproc/remoteproc_core.c:251:6:
> warning: format ?%x? expects argument of type ?unsigned int?,
> but argument 4 has type ?size_t? [-Wformat]
> drivers/remoteproc/remoteproc_core.c:938:9:
> warning: format ?%u? expects argument of type ?unsigned int?,
> but argument 4 has type ?size_t? [-Wformat]
> drivers/remoteproc/remoteproc_core.c:1023:2:
> warning: format ?%d? expects argument of type ?int?,
> but argument 4 has type ?size_t? [-Wformat]
>
> Signed-off-by: Sjur Br?ndeland <[email protected]>
Cc'ed stable, applied and pushed to remoteproc's fixes branch.
Thanks,
Ohad.
On Wed, Jun 6, 2012 at 4:38 PM, <[email protected]> wrote:
> From: Sjur Br?ndeland <[email protected]>
>
> If rproc_find_rsc_table() fails, rproc_fw_boot() must set
> return-value before jumping to clean_up label. Otherwise no
> error value is returned.
>
> Signed-off-by: Sjur Br?ndeland <[email protected]>
Cc'ed stable, applied and pushed to remoteproc's fixes branch.
Thanks,
Ohad.
Hi Sjur,
On Wed, Jun 6, 2012 at 4:38 PM, <[email protected]> wrote:
> I think I have addressed most of your review comments
Thanks! The patches look fairly good, I'm pretty sure we could merge
them very soon.
> except dynamic detection of firmware format.
It could be really nice to have that so we don't have to couple
platforms with specific binary formats.
Is it possible to dynamically detect the STE binary format ? Do you
have some magic string in the header of the file or something ?
> Warning: I have not been able to test this properly,
> I don't yet have a proper test environment running.
Ok, please let me know when you're satisfied with the testing and
would like me to go ahead and do my own pre-merging tests.
Thanks,
Ohad.
On Wed, Jun 6, 2012 at 4:38 PM, <[email protected]> wrote:
> From: Sjur Br?ndeland <[email protected]>
>
> Prepare for introduction of custom firmware loaders by changing
> the functions rproc_find_rcs_table() and rproc_load_segments()
> to use struct firmware as parameter.
>
> When the custom loader framework is introduced all calls into
> the firmware specific function must use struct firmware as
> parameter.
>
> Signed-off-by: Sjur Br?ndeland <[email protected]>
Looks good, just a small typo below:
> + * @fw: the ?the ELF firmware image
On Wed, Jun 6, 2012 at 4:38 PM, <[email protected]> wrote:
> +/**
> + * rproc_set_boot_addr() - Set rproc's boot address.
> + * @rproc: remote processor which needs boot address set.
> + * @fw: the ?the ELF firmware image
typo: s/the the/the/
> - ? ? ? rproc->bootaddr = ehdr->e_entry;
> + ? ? ? rproc_set_boot_addr(rproc, fw);
We might want to do this a little bit differently, something like:
rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
This way the binary-format-specific handler really only retrieves the
boot address, without manipulating the framework's rproc structure.
If such handler doesn't exist, a default zero can be returned.
Thanks,
Ohad.
On Wed, Jun 6, 2012 at 4:38 PM, <[email protected]> wrote:
> From: Sjur Br?ndeland <[email protected]>
>
> Prepare for introduction of custom firmware loaders by
> moving all ELF related handling into a separate file.
> The functions: rproc_find_rsc_table(), rproc_fw_sanity_check(),
> rproc_find_rsc_table() and rproc_set_boot_addr() are moved
> to the new file remoteproc_elf_loader.c. No functional
> changes are introduced in this patch.
>
> Signed-off-by: Sjur Br?ndeland <[email protected]>
Very nice!
> @@ -165,7 +165,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.
> ?*/
> -static void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
> +void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
Can be nice to shortly mention this change in the commit log too.
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> +int
> +rproc_load_segments(struct rproc *rproc, const struct firmware *fw);
> +struct resource_table *
> +rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int *tablesz);
> +int
> +rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw);
> +void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw);
...
> +void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
remoteproc_internal.h might be a better place for these, I think,
because it's not exposed to the users.
Thanks,
Ohad.
Hi Sjur,
On Wed, Jun 6, 2012 at 4:38 PM, <[email protected]> wrote:
> From: Sjur Br?ndeland <[email protected]>
>
> Firmware handling is made customizable.
> This is done by creating a separate ops structure for the
> firmware functions that depends on a particular firmware
> format (such as ELF). The ELF specific functions are exported
> by the structure rproc_elf_fw_ops.
>
> Signed-off-by: Sjur Br?ndeland <[email protected]>
> ---
> @@ -171,6 +171,7 @@ static struct rproc_ops omap_rproc_ops = {
> ? ? ? ?.start ? ? ? ? ?= omap_rproc_start,
> ? ? ? ?.stop ? ? ? ? ? = omap_rproc_stop,
> ? ? ? ?.kick ? ? ? ? ? = omap_rproc_kick,
> + ? ? ? .fw ? ? ? ? ? ? = &rproc_elf_fw_ops
It might be more appropriate to provide the binary format handlers as
a separate argument to rproc_alloc, instead of adding it to the
rproc_ops struct (rproc_ops is dedicated to platform-specific handlers
that are implemented by the low-level driver).
>
> ? ? ? ?rproc = kzalloc(sizeof(struct rproc) + len, GFP_KERNEL);
> diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
> index 2017ae3..9a53871 100644
> --- a/drivers/remoteproc/remoteproc_elf_loader.c
> +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> @@ -33,8 +33,8 @@
> ?#include "remoteproc_internal.h"
>
> ?/* make sure this fw image is sane */
> -int
> -rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
> +static int
> +elf_sanity_check(struct rproc *rproc, const struct firmware *fw)
Do we want to keep the "rproc" prefix for the function names (e.g.
rproc_elf_sanity_check) ? we'll be better namespace citizens that way.
> -int
> -rproc_load_segments(struct rproc *rproc, const struct firmware *fw);
> -struct resource_table *
> -rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int *tablesz);
> -int
> -rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw);
> -void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw);
Oh, great, you removed those. In that case we don't really care they'd
stay in remoteproc.h for the transition.. (but that probably doesn't
apply to rproc_da_to_va).
> +static inline int
> +rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
> +{
> + ? ? ? if (rproc->ops->fw_ops->sanity_check)
> + ? ? ? ? ? ? ? return rproc->ops->fw_ops->sanity_check(rproc, fw);
> + ? ? ? return 0;
> +}
> +
> +static inline
> +void rproc_set_boot_addr(struct rproc *rproc, const struct firmware *fw)
> +{
> + ? ? ? if (rproc->ops->fw_ops->set_boot_addr)
> + ? ? ? ? ? ? ? rproc->ops->fw_ops->set_boot_addr(rproc, fw);
> +}
> +
> +static inline int
> +rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
> +{
> + ? ? ? if (rproc->ops->fw_ops->load)
> + ? ? ? ? ? ? ? return rproc->ops->fw_ops->load(rproc, fw);
> + ? ? ? return -EINVAL;
> +}
> +static inline struct resource_table *
> +rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int *tablesz)
> +{
> + ? ? ? if (rproc->ops->fw_ops->find_rsc_table)
> + ? ? ? ? ? ? ? return rproc->ops->fw_ops->find_rsc_table(rproc, fw, tablesz);
> + ? ? ? return NULL;
> +}
> +
> +extern struct rproc_fw_ops rproc_elf_fw_ops;
I'd keep those too in remoteproc_internal.h, so they won't be
accessible to code outside of drivers/remoteproc.
Thanks!
Ohad.
Hi Sjur,
On Wed, Jun 6, 2012 at 4:38 PM, <[email protected]> wrote:
> From: Sjur Br?ndeland <[email protected]>
>
> Add custom firmware loader for STE firmware. This plugin adds
> functions for extracting the resource table and loading the
> firmware image into shared memory.
>
> Signed-off-by: Sjur Br?ndeland <[email protected]>
Nice patch!
> +struct rproc_fw_ops rproc_ste_modem_fw_ops = {
> + ? ? ? .load = ste_load_segments,
> + ? ? ? .find_rsc_table = ste_find_rsc_table
> +};
> +EXPORT_SYMBOL(rproc_ste_modem_fw_ops);
After you told me there's no simple & robust way to dynamically probe
the STE binary format, I agree it'd make sense to allow low-lever
drivers to statically impose it.
I'm just still not sure we want to publicly export those fw_ops
symbols throughout the kernel.
Alternatively, we can do something like how i2c algos are determined:
expose a rproc_set_ste_modem_fw() function, which takes an rproc
struct, and then sets the fw_ops to rproc_ste_modem_fw_ops. Then call
it from your driver after rproc_alloc() but before rproc_register().
This way only statically-format-imposing drivers will call those
functions, and all others, who use generic and detectable formats,
will just follow the current registration scheme (so no extra param
needed to rproc_alloc either). In the long term those generic formats
will be dynamically detected, but meanwhile, we can just set ELF as
the default.
What do you think?
Thanks,
Ohad.
Hi Ohad,
>> +struct rproc_fw_ops rproc_ste_modem_fw_ops = {
>> + .load = ste_load_segments,
>> + .find_rsc_table = ste_find_rsc_table
>> +};
>> +EXPORT_SYMBOL(rproc_ste_modem_fw_ops);
...
> I'm just still not sure we want to publicly export those fw_ops
> symbols throughout the kernel.
>
>
> Alternatively, we can do something like how i2c algos are determined:
> expose a rproc_set_ste_modem_fw() function, which takes an rproc
> struct, and then sets the fw_ops to rproc_ste_modem_fw_ops. Then call
> it from your driver after rproc_alloc() but before rproc_register().
>
> This way only statically-format-imposing drivers will call those
> functions, and all others, who use generic and detectable formats,
> will just follow the current registration scheme (so no extra param
> needed to rproc_alloc either). In the long term those generic formats
> will be dynamically detected, but meanwhile, we can just set ELF as
> the default.
>
> What do you think?
How about this:
In rproc_alloc, we set the rproc_elf_fw_ops as default, and we add a function:
void rproc_set_fw_ops(struct rproc *rproc, const struct rproc_fw_ops *fw_ops)
{
rproc->fw_ops = fw_ops;
}
EXPORT_SYMBOL(rproc_set_fw_ops);
allowing the ste_modem_driver to override the default ELF-firmware handler.
Then the file remoteproc_elf_loader.c containing rproc_elf_fw_ops is
default fw_ops
and is contained remoteproc.ko.
The ste_modem_fw_ops will be linked together with the ste_modem_rproc module,
and none of the fw_ops needs to be exported as symbols globally.
Regards,
Sjur
Hi Sjur,
On Mon, Jun 11, 2012 at 12:55 PM, Sjur Br?ndeland
<[email protected]> wrote:
> How about this:
> In rproc_alloc, we set the rproc_elf_fw_ops as default
...
> Then the file remoteproc_elf_loader.c containing rproc_elf_fw_ops is
> default fw_ops
> and is contained remoteproc.ko.
>
> The ste_modem_fw_ops will be linked together with the ste_modem_rproc module,
> and none of the fw_ops needs to be exported as symbols globally.
All of the above sounds very good to me.
>, and we add a function:
>
> void rproc_set_fw_ops(struct rproc *rproc, const struct rproc_fw_ops *fw_ops)
> {
> ? ? ? ?rproc->fw_ops = fw_ops;
> }
> EXPORT_SYMBOL(rproc_set_fw_ops);
I'm just not sure about this part.
If we're going to have the STE fw ops linked together with the STE
module (which sounds really good btw), can the STE module just
directly do
rproc->fw_ops = ste_modem_fw_ops;
before calling rproc_register() ?
This way we're not exposing any other symbol, and things look pretty
clean and simple.
Thanks,
Ohad.