2019-08-07 09:42:25

by Loic Pallardy

[permalink] [raw]
Subject: [RESEND 0/2] remoteproc: add support for preloaded firmware

This patch series introduces a new flag in remoteproc core to add
support of remote processor having their firmware loading by another
way than standard remoteproc core sequence.

Firmware could be ROMed, loaded by security or bootloader before kernel
boot or loaded by a special rproc platform driver interface.

When "preloaded" flag is set by rproc platform driver, remoteproc core
doesn't request firmware and execute rproc_start sequence as usual allocating
associated rproc resources.
It is rproc platform driver responsibility to implement the right firmware
load operations according to HW specificities like resource table location
or firmware definition if needed.

Regards,
Loic

Loic Pallardy (2):
remoteproc: replace bool from struct rproc by u8
remoteproc: add support for co-processor booted before kernel

drivers/remoteproc/remoteproc_core.c | 37 +++++++++++++++++++++++++++---------
include/linux/remoteproc.h | 14 ++++++++------
2 files changed, 36 insertions(+), 15 deletions(-)

--
2.7.4


2019-08-07 09:42:29

by Loic Pallardy

[permalink] [raw]
Subject: [RESEND 1/2] remoteproc: replace bool from struct rproc by u8

Post [1] and checkpatch tool indicate that usage of bool type
in structure is now no more allowed/advised.
This patch replaces bool by unsigned char (u8) and reorders
struct rproc fields to avoid padding.

[1] https://lkml.org/lkml/2017/11/21/384

Signed-off-by: Loic Pallardy <[email protected]>
---
include/linux/remoteproc.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 16ad66683ad0..8cd22fecea61 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -472,15 +472,15 @@ struct rproc_dump_segment {
* @index: index of this rproc device
* @crash_handler: workqueue for handling a crash
* @crash_cnt: crash counter
- * @recovery_disabled: flag that state if recovery was disabled
* @max_notifyid: largest allocated notify id.
* @table_ptr: pointer to the resource table in effect
* @cached_table: copy of the resource table
* @table_sz: size of @cached_table
- * @has_iommu: flag to indicate if remote processor is behind an MMU
- * @auto_boot: flag to indicate if remote processor should be auto-started
* @dump_segments: list of segments in the firmware
* @nb_vdev: number of vdev currently handled by rproc
+ * @recovery_disabled: flag that state if recovery was disabled
+ * @has_iommu: flag to indicate if remote processor is behind an MMU
+ * @auto_boot: flag to indicate if remote processor should be auto-started
*/
struct rproc {
struct list_head node;
@@ -505,15 +505,15 @@ struct rproc {
int index;
struct work_struct crash_handler;
unsigned int crash_cnt;
- bool recovery_disabled;
int max_notifyid;
struct resource_table *table_ptr;
struct resource_table *cached_table;
size_t table_sz;
- bool has_iommu;
- bool auto_boot;
struct list_head dump_segments;
int nb_vdev;
+ u8 recovery_disabled;
+ u8 has_iommu;
+ u8 auto_boot;
};

/**
--
2.7.4

2019-08-07 09:43:05

by Loic Pallardy

[permalink] [raw]
Subject: [RESEND 2/2] remoteproc: add support for co-processor booted before kernel

Remote processor could boot independently or be started before Linux
kernel by bootloader or any firmware.
This patch introduces a new property in rproc core, named preloaded,
to be able to allocate resources and sub-devices like vdev and to
synchronize with current state without loading firmware from file system.
It is platform driver responsibility to implement the right firmware
load ops according to HW specificities.

Signed-off-by: Loic Pallardy <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 37 +++++++++++++++++++++++++++---------
include/linux/remoteproc.h | 2 ++
2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 3c5fbbbfb0f1..7eaf0f949afa 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1372,7 +1372,11 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
if (ret)
return ret;

- dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
+ if (fw)
+ dev_info(dev, "Booting fw image %s, size %zd\n", name,
+ fw->size);
+ else
+ dev_info(dev, "Synchronizing with preloaded co-processor\n");

/*
* if enabling an IOMMU isn't relevant for this rproc, this is
@@ -1728,7 +1732,7 @@ static void rproc_crash_handler_work(struct work_struct *work)
*/
int rproc_boot(struct rproc *rproc)
{
- const struct firmware *firmware_p;
+ const struct firmware *firmware_p = NULL;
struct device *dev;
int ret;

@@ -1759,11 +1763,17 @@ int rproc_boot(struct rproc *rproc)

dev_info(dev, "powering up %s\n", rproc->name);

- /* load firmware */
- ret = request_firmware(&firmware_p, rproc->firmware, dev);
- if (ret < 0) {
- dev_err(dev, "request_firmware failed: %d\n", ret);
- goto downref_rproc;
+ if (!rproc->preloaded) {
+ /* load firmware */
+ ret = request_firmware(&firmware_p, rproc->firmware, dev);
+ if (ret < 0) {
+ dev_err(dev, "request_firmware failed: %d\n", ret);
+ goto downref_rproc;
+ }
+ } else {
+ /* set firmware name to null as unknown */
+ kfree(rproc->firmware);
+ rproc->firmware = NULL;
}

ret = rproc_fw_boot(rproc, firmware_p);
@@ -1917,8 +1927,17 @@ int rproc_add(struct rproc *rproc)
/* create debugfs entries */
rproc_create_debug_dir(rproc);

- /* if rproc is marked always-on, request it to boot */
- if (rproc->auto_boot) {
+ if (rproc->preloaded) {
+ /*
+ * If rproc is marked already booted, no need to wait
+ * for firmware.
+ * Just handle associated resources and start sub devices
+ */
+ ret = rproc_boot(rproc);
+ if (ret < 0)
+ return ret;
+ } else if (rproc->auto_boot) {
+ /* if rproc is marked always-on, request it to boot */
ret = rproc_trigger_auto_boot(rproc);
if (ret < 0)
return ret;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 8cd22fecea61..27f0dfdd3837 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -481,6 +481,7 @@ struct rproc_dump_segment {
* @recovery_disabled: flag that state if recovery was disabled
* @has_iommu: flag to indicate if remote processor is behind an MMU
* @auto_boot: flag to indicate if remote processor should be auto-started
+ * @preloaded: remote processor has been preloaded before start sequence
*/
struct rproc {
struct list_head node;
@@ -514,6 +515,7 @@ struct rproc {
u8 recovery_disabled;
u8 has_iommu;
u8 auto_boot;
+ u8 preloaded;
};

/**
--
2.7.4

2019-08-07 17:06:48

by Suman Anna

[permalink] [raw]
Subject: Re: [RESEND 1/2] remoteproc: replace bool from struct rproc by u8

Hi Loic,

On 8/7/19 4:41 AM, Loic Pallardy wrote:
> Post [1] and checkpatch tool indicate that usage of bool type
> in structure is now no more allowed/advised.
> This patch replaces bool by unsigned char (u8) and reorders
> struct rproc fields to avoid padding.
>
> [1] https://lkml.org/lkml/2017/11/21/384

Btw, that checkpatch warning has been removed and documentation
clarified in commit 7967656ffbfa ("coding-style: Clarify the
expectations around bool") added in 5.1 kernel.

I have actually switched to using bitfields on some of my patches
downstream because of the same checkpatch message, since we seem to have
a number of these fields, but that also requires updating all the field
values in various drivers to use 0 or 1 instead of the boolean values.

regards
Suman

>
> Signed-off-by: Loic Pallardy <[email protected]>
> ---
> include/linux/remoteproc.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 16ad66683ad0..8cd22fecea61 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -472,15 +472,15 @@ struct rproc_dump_segment {
> * @index: index of this rproc device
> * @crash_handler: workqueue for handling a crash
> * @crash_cnt: crash counter
> - * @recovery_disabled: flag that state if recovery was disabled
> * @max_notifyid: largest allocated notify id.
> * @table_ptr: pointer to the resource table in effect
> * @cached_table: copy of the resource table
> * @table_sz: size of @cached_table
> - * @has_iommu: flag to indicate if remote processor is behind an MMU
> - * @auto_boot: flag to indicate if remote processor should be auto-started
> * @dump_segments: list of segments in the firmware
> * @nb_vdev: number of vdev currently handled by rproc
> + * @recovery_disabled: flag that state if recovery was disabled
> + * @has_iommu: flag to indicate if remote processor is behind an MMU
> + * @auto_boot: flag to indicate if remote processor should be auto-started
> */
> struct rproc {
> struct list_head node;
> @@ -505,15 +505,15 @@ struct rproc {
> int index;
> struct work_struct crash_handler;
> unsigned int crash_cnt;
> - bool recovery_disabled;
> int max_notifyid;
> struct resource_table *table_ptr;
> struct resource_table *cached_table;
> size_t table_sz;
> - bool has_iommu;
> - bool auto_boot;
> struct list_head dump_segments;
> int nb_vdev;
> + u8 recovery_disabled;
> + u8 has_iommu;
> + u8 auto_boot;
> };
>
> /**
>