From: Sjur Brændeland <[email protected]>
Hi Ohad,
Here is a new patch series. I dropped the two bug-fix patches
since last time, as you already included these. I've tried to
address all of your review comments since last time, hope I
didn't miss any.
Status:
I have done a simple simulated test now with the STE firmware
loader and main-flow seems ok, but I haven't done any negative
tests.
Changes since V1:
- Changed rproc_set_boot_addr to rproc_get_boot_addr avoiding
firmware handlers to manipulate the rproc structure.
- Functions in remoteproc_elf_loader are now static and prefixed
with _rproc to avoid any namespace issues.
- Added commit note about rproc_da_to_va() becoming non-static
and moved it to remoteproc_internal.h
- Made the elf handler default.
- Moved pointer to rproc_fw_ops from the rproc_ops struct to
the rproc struct.
- Fixed typos
- The fw_ops are no longer exported as symbols.
- The struct rproc_fw_ops is made const.
- The inline helper functions are moved from remoteproc.h to
remoteproc_internal.h
- The latest patch is a bit odd. The ste_modem_remoteproc module
contains only the firmware handler, and nothing else.
Regards,
Sjur
Sjur Brændeland (5):
remoteproc: Pass struct fw to load_segments and find_rsc_table.
remoteproc: Add function rproc_get_boot_addr
remoteproc: Move Elf related functions to separate file
remoteproc: Support custom firmware handlers
remoteproc: Add custom STE-modem firmware loader.
drivers/remoteproc/Kconfig | 9 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/remoteproc_core.c | 251 +------------------
drivers/remoteproc/remoteproc_elf_loader.c | 289 ++++++++++++++++++++++
drivers/remoteproc/remoteproc_internal.h | 37 +++
drivers/remoteproc/remoteproc_ste_modem_loader.c | 179 +++++++++++++
include/linux/remoteproc.h | 23 ++-
7 files changed, 546 insertions(+), 243 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. Move lookup of the boot_addr in the
ELF binary to the function rproc_get_boot_addr().
Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 3e02f7f..895674d 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1003,6 +1003,20 @@ static int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
return 0;
}
+/**
+ * rproc_get_boot_addr() - Get rproc's boot address.
+ * @fw: the ELF firmware image
+ *
+ * This function reads the ELF entry point 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.
+ */
+u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
+{
+ struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
+ return ehdr->e_entry;
+}
+
/*
* take a firmware and boot a remote processor with it.
*/
@@ -1010,7 +1024,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 +1031,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 +1043,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->bootaddr = rproc_get_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]>
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 | 9 +
drivers/remoteproc/Makefile | 2 +
drivers/remoteproc/remoteproc_internal.h | 1 +
drivers/remoteproc/remoteproc_ste_modem_loader.c | 179 ++++++++++++++++++++++
4 files changed, 191 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..7a6fc23 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -25,4 +25,13 @@ 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
+ tristate "STE-Modem remoteproc support"
+ select REMOTEPROC
+ default n
+ help
+ Say y or m here to support STE-Modem shared memory driver.
+ This can be either built-in or a loadable module.
+ If unsure say N.
+
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 934ce6e..107586b 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -8,3 +8,5 @@ remoteproc-y += remoteproc_debugfs.o
remoteproc-y += remoteproc_virtio.o
remoteproc-y += remoteproc_elf_loader.o
obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
+obj-$(CONFIG_STE_MODEM_RPROC) += ste_modem_remoteproc.o
+ste_modem_remoteproc-y := remoteproc_ste_modem_loader.o
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index ddb45de..2564288 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -76,5 +76,6 @@ rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
}
extern const struct rproc_fw_ops rproc_elf_fw_ops;
+extern const struct rproc_fw_ops rproc_ste_modem_fw_ops;
#endif /* REMOTEPROC_INTERNAL_H */
diff --git a/drivers/remoteproc/remoteproc_ste_modem_loader.c b/drivers/remoteproc/remoteproc_ste_modem_loader.c
new file mode 100644
index 0000000..139222d
--- /dev/null
+++ b/drivers/remoteproc/remoteproc_ste_modem_loader.c
@@ -0,0 +1,179 @@
+/*
+ * 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;
+ int entries = ARRAY_SIZE(toc->table);
+
+ if (!fw)
+ return NULL;
+
+ toc = (void *)fw->data;
+
+ /* 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;
+}
+
+const struct rproc_fw_ops rproc_ste_modem_fw_ops = {
+ .load = ste_load_segments,
+ .find_rsc_table = ste_find_rsc_table
+};
--
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..3e02f7f 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 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]>
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 functions are default used
unless the HW driver explicitly injects another firmware
handler by calling the function rproc_set_fw_ops().
The function rproc_da_to_va() is exported, as custom
firmware handlers may need to use this function.
Signed-off-by: Sjur Brændeland <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 4 +++
drivers/remoteproc/remoteproc_elf_loader.c | 27 +++++++++++++--------
drivers/remoteproc/remoteproc_internal.h | 35 ++++++++++++++++++++++++++++
include/linux/remoteproc.h | 29 ++++++++++++++++------
4 files changed, 77 insertions(+), 18 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 682273f..bee4644 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -188,6 +188,7 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
return ptr;
}
+EXPORT_SYMBOL(rproc_da_to_va);
static int
__rproc_handle_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc, int i)
@@ -1254,6 +1255,9 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
atomic_set(&rproc->power, 0);
+ /* Set ELF as the default fw_ops handler */
+ rproc->fw_ops = &rproc_elf_fw_ops;
+
kref_init(&rproc->refcount);
mutex_init(&rproc->lock);
diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index 06e35dc..5e8ee06 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -33,13 +33,13 @@
#include "remoteproc_internal.h"
/**
- * rproc_fw_sanity_check() - Sanity Check ELF firmware image
+ * _rproc_sanity_check() - Sanity Check ELF firmware image
* @fw: the ELF firmware image
*
* Make sure this fw image is sane.
*/
-int
-rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
+static int
+_rproc_sanity_check(struct rproc *rproc, const struct firmware *fw)
{
const char *name = rproc->firmware;
struct device *dev = rproc->dev;
@@ -99,14 +99,14 @@ rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
}
/**
- * rproc_get_boot_addr() - Get rproc's boot address.
+ * _rproc_get_boot_addr() - Get rproc's boot address.
* @fw: the ELF firmware image
*
* This function reads the ELF entry point 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.
*/
-u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
+static u32 _rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
{
struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
return ehdr->e_entry;
@@ -136,8 +136,8 @@ u32 rproc_get_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
+_rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
{
struct device *dev = rproc->dev;
struct elf32_hdr *ehdr;
@@ -203,7 +203,7 @@ rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
}
/**
- * rproc_find_rsc_table() - find the resource table
+ * _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
@@ -217,8 +217,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 *
+_rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
int *tablesz)
{
struct elf32_hdr *ehdr;
@@ -280,3 +280,10 @@ rproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw,
return table;
}
+
+const struct rproc_fw_ops rproc_elf_fw_ops = {
+ .load = _rproc_load_segments,
+ .find_rsc_table = _rproc_find_rsc_table,
+ .sanity_check = _rproc_sanity_check,
+ .get_boot_addr = _rproc_get_boot_addr
+};
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 25e0b91..ddb45de 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -42,4 +42,39 @@ void rproc_init_debugfs(void);
void rproc_exit_debugfs(void);
void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
+static inline int
+rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
+{
+ if (rproc->fw_ops->sanity_check)
+ return rproc->fw_ops->sanity_check(rproc, fw);
+ return 0;
+}
+
+static inline
+u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
+{
+ if (rproc->fw_ops->get_boot_addr)
+ return rproc->fw_ops->get_boot_addr(rproc, fw);
+ return 0;
+}
+
+static inline int
+rproc_load_segments(struct rproc *rproc, const struct firmware *fw)
+{
+ if (rproc->fw_ops->load)
+ return rproc->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->fw_ops->find_rsc_table)
+ return rproc->fw_ops->find_rsc_table(rproc, fw, tablesz);
+ return NULL;
+}
+
+extern const struct rproc_fw_ops rproc_elf_fw_ops;
+
#endif /* REMOTEPROC_INTERNAL_H */
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index d6853da..7790fdd 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -327,14 +327,6 @@ struct rproc_mem_entry {
struct rproc;
-struct resource_table *rproc_find_rsc_table(struct rproc *rproc,
- const struct firmware *fw,
- int *tablesz);
-int rproc_load_segments(struct rproc *rproc, const struct firmware *fw);
-int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw);
-u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw);
-
-
/**
* struct rproc_ops - platform-specific device handlers
* @start: power on the device and boot it
@@ -347,6 +339,26 @@ struct rproc_ops {
void (*kick)(struct rproc *rproc, int vqid);
};
+
+/**
+ * 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);
+ u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw);
+};
+
/**
* enum rproc_state - remote processor states
* @RPROC_OFFLINE: device is powered off
@@ -400,6 +412,7 @@ struct rproc {
const char *firmware;
void *priv;
const struct rproc_ops *ops;
+ const struct rproc_fw_ops *fw_ops;
struct device *dev;
struct kref refcount;
atomic_t power;
--
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. The function
rproc_da_to_va() is made non-static and is declared in
remoteproc_internal.h
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 | 245 +------------------------
drivers/remoteproc/remoteproc_elf_loader.c | 282 ++++++++++++++++++++++++++++
drivers/remoteproc/remoteproc_internal.h | 1 +
include/linux/remoteproc.h | 9 +
5 files changed, 294 insertions(+), 244 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 895674d..682273f 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 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,80 +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_get_boot_addr() - Get rproc's boot address.
- * @fw: the ELF firmware image
- *
- * This function reads the ELF entry point 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.
- */
-u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
-{
- struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
- return 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..06e35dc
--- /dev/null
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -0,0 +1,282 @@
+/*
+ * 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"
+
+/**
+ * rproc_fw_sanity_check() - Sanity Check ELF firmware image
+ * @fw: the ELF firmware image
+ *
+ * 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_get_boot_addr() - Get rproc's boot address.
+ * @fw: the ELF firmware image
+ *
+ * This function reads the ELF entry point 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.
+ */
+u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
+{
+ struct elf32_hdr *ehdr = (struct elf32_hdr *)fw->data;
+ return 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/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 9f336d6..25e0b91 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -40,5 +40,6 @@ void rproc_delete_debug_dir(struct rproc *rproc);
void rproc_create_debug_dir(struct rproc *rproc);
void rproc_init_debugfs(void);
void rproc_exit_debugfs(void);
+void *rproc_da_to_va(struct rproc *rproc, u64 da, int len);
#endif /* REMOTEPROC_INTERNAL_H */
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index f1ffabb..d6853da 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,14 @@ struct rproc_mem_entry {
struct rproc;
+struct resource_table *rproc_find_rsc_table(struct rproc *rproc,
+ const struct firmware *fw,
+ int *tablesz);
+int rproc_load_segments(struct rproc *rproc, const struct firmware *fw);
+int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw);
+u32 rproc_get_boot_addr(struct rproc *rproc, const struct firmware *fw);
+
+
/**
* struct rproc_ops - platform-specific device handlers
* @start: power on the device and boot it
--
1.7.5.4
Hi Sjur,
On Mon, Jun 11, 2012 at 8:29 PM, <[email protected]> wrote:
> Here is a new patch series. I dropped the two bug-fix patches
> since last time, as you already included these. I've tried to
> address all of your review comments since last time, hope I
> didn't miss any.
Thanks, this looks good.
I have a few small mostly-style comments, but I could just do them
while I apply the patches.
I've tried applying the (first four) patches, but since they are still
based on the two bug-fixes patches, they don't apply.
So we can either wait until the bug fixes are merged by Linus (I'm
going to send them soon) or you can rebase the patches on a current
mainline tree (which I could then push to linux-next) - however you
prefer.
> Status:
> I have done a simple simulated test now with the STE firmware
> loader and main-flow seems ok, but I haven't done any negative
> tests.
Ok, thanks. The first four patches seem trivial enough for me to merge
even without any extensive testing.
> - The latest patch is a bit odd. The ste_modem_remoteproc module
> ?contains only the firmware handler, and nothing else.
What are your plans regarding the rest of that module? we may want to
wait with the fifth patch until we have some basic functionality.
Thanks,
Ohad.
Hi Ohad,
> I have a few small mostly-style comments, but I could just do them
> while I apply the patches.
Yes, please feel free to fix things when needed.
> I've tried applying the (first four) patches, but since they are still
> based on the two bug-fixes patches, they don't apply.
>
> So we can either wait until the bug fixes are merged by Linus (I'm
> going to send them soon) or you can rebase the patches on a current
> mainline tree (which I could then push to linux-next) - however you
> prefer.
Either way works for me. I'm not in particular rush to get things in, but
I'll send you a rebased version tomorrow anyway.
>> - The latest patch is a bit odd. The ste_modem_remoteproc module
>> contains only the firmware handler, and nothing else.
>
> What are your plans regarding the rest of that module? we may want to
> wait with the fifth patch until we have some basic functionality.
I am brewing a module and I hope to send it out for review
as RFC within a week or two. It will probably make most sense to
merge the module as a whole and not just the firmware handler.
So I suggest we wait with the firmware handler until you have reviewed the
patches on the ste-modem-rproc and we can take it from there.
Regards,
Sjur
On Sun, Jun 17, 2012 at 2:57 PM, Sjur Br?ndeland
<[email protected]> wrote:
> Yes, please feel free to fix things when needed.
..
> Either way works for me. I'm not in particular rush to get things in, but
> I'll send you a rebased version tomorrow anyway.
..
> I am brewing a module and I hope to send it out for review
> as RFC within a week or two. It will probably make most sense to
> merge the module as a whole and not just the firmware handler.
> So I suggest we wait with the firmware handler until you have reviewed the
> patches on the ste-modem-rproc and we can take it from there.
Sounds good.
Thanks!
Ohad.