[PATCH] remoteproc: report error if resource table doesn't exist

[PATCH] remoteproc: report error if resource table doesn't exist

[Stefan Agner]

Currently, if the resource table is completely missing in the
firmware, powering up the remoteproc fails silently. Add a message
indicating that the resource table is missing in the firmware.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Hi Ohad,

I am currently working on remoteproc support for Freescale Vybrid's
secondary Cortex-M4 core. I stumbled upon this rough spot since the
little test firmware I am using now does not have a resource table
yet.

This also opens up a more general question: Is it mandatory to have
a resource table in the firmware? Theoretically a remoteproc could
also work completely independent, all what would be used from the
remoteproc framework is the loading and starting capabilities...

--
Stefan

 drivers/remoteproc/remoteproc_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 8b3130f..29db8b3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -823,8 +823,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, &tablesz);
-	if (!table)
+	if (!table) {
+		dev_err(dev, "Failed to find resource table\n");
 		goto clean_up;
+	}
 
 	/* Verify that resource table in loaded fw is unchanged */
 	if (rproc->table_csum != crc32(0, table, tablesz)) {
-- 
2.5.0

Re: [PATCH] remoteproc: report error if resource table doesn't exist

[Suman Anna]

On 08/28/2015 08:08 PM, Stefan Agner wrote:
> Currently, if the resource table is completely missing in the
> firmware, powering up the remoteproc fails silently. Add a message
> indicating that the resource table is missing in the firmware.

Yeah, pretty useful to have a trace there..

Acked-by: Suman Anna <s-anna@ti.com>

> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> Hi Ohad,
> 
> I am currently working on remoteproc support for Freescale Vybrid's
> secondary Cortex-M4 core. I stumbled upon this rough spot since the
> little test firmware I am using now does not have a resource table
> yet.
> 
> This also opens up a more general question: Is it mandatory to have
> a resource table in the firmware? Theoretically a remoteproc could
> also work completely independent, all what would be used from the
> remoteproc framework is the loading and starting capabilities...

Hi Ohad,

We will probably be seeing more of such scenarios for very simplistic
devices (like the ones that load into their internal memories), it looks
like the framework needs some kind of support for booting such devices,
whether auto-boot, or give some kind of sysfs control for userspace. We
do have the rproc_boot() and rproc_shutdown() API, but that almost
always requires some other entity in the kernel to be able to invoke
those API. Any suggestions here?

regards
Suman

> 
> --
> Stefan
> 
>  drivers/remoteproc/remoteproc_core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 8b3130f..29db8b3 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -823,8 +823,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, &tablesz);
> -	if (!table)
> +	if (!table) {
> +		dev_err(dev, "Failed to find resource table\n");
>  		goto clean_up;
> +	}
>  
>  	/* Verify that resource table in loaded fw is unchanged */
>  	if (rproc->table_csum != crc32(0, table, tablesz)) {
> 

Re: [PATCH] remoteproc: report error if resource table doesn't exist

[Bjorn Andersson]

On Thu, Sep 3, 2015 at 12:32 PM, Suman Anna <s-anna@ti.com> wrote:
> On 08/28/2015 08:08 PM, Stefan Agner wrote:
>> Currently, if the resource table is completely missing in the
>> firmware, powering up the remoteproc fails silently. Add a message
>> indicating that the resource table is missing in the firmware.
>
> Yeah, pretty useful to have a trace there..
>
> Acked-by: Suman Anna <s-anna@ti.com>
>
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>> Hi Ohad,
>>
>> I am currently working on remoteproc support for Freescale Vybrid's
>> secondary Cortex-M4 core. I stumbled upon this rough spot since the
>> little test firmware I am using now does not have a resource table
>> yet.
>>
>> This also opens up a more general question: Is it mandatory to have
>> a resource table in the firmware? Theoretically a remoteproc could
>> also work completely independent, all what would be used from the
>> remoteproc framework is the loading and starting capabilities...

In the Qualcomm case we do not have a resource table and adding one
does not add any value. My current approach is to fake an empty table
to satisfy the framework, but I will have a look at trying to make it
optional.

>
> Hi Ohad,
>
> We will probably be seeing more of such scenarios for very simplistic
> devices (like the ones that load into their internal memories), it looks
> like the framework needs some kind of support for booting such devices,

They don't have to be simple, in the Qualcomm case we have two solutions;
1) Statically allocated memory chunks, through memory-reserve
2) Relocatable firmware, that can be loaded wherever and will fix up
its own mmu tables upon booting the remote.

Further more mode modem firmware is loaded as a two step process,
where one load and boot the boot loader (fits nicely into remoteproc)
and then one loads and feeds the actual firmware to the boot loader.
I.e. we have to load firmware in more than one step. So we need these
decisions to be more flexible.

> whether auto-boot, or give some kind of sysfs control for userspace. We
> do have the rproc_boot() and rproc_shutdown() API, but that almost
> always requires some other entity in the kernel to be able to invoke
> those API. Any suggestions here?
>

This is something we need to address.

I've been looking at adding this request to the module_init()/exit()
of the drivers that later will be communicating with the remote
firmware. E.g. the wifi driver upon loading would resolve the rproc
for the wifi/bt chip and call rproc_boot() on this upon loading.

This would solve the problem of figuring out whom should be requesting
the firmware to be loaded.

However, it would require either userspace to be fully aware of the
dependency towards remoteproc or us having a mechanism for marking a
future available remoteproc for loading - either because the
remoteproc isn't probed yet or simply because the firmware isn't
available yet.

Regards,
Bjorn

Re: [PATCH] remoteproc: report error if resource table doesn't exist

[Ohad Ben-Cohen]

Hi Stefan,

On Sat, Aug 29, 2015 at 4:08 AM, Stefan Agner <stefan@agner.ch> wrote:
> Currently, if the resource table is completely missing in the
> firmware, powering up the remoteproc fails silently. Add a message
> indicating that the resource table is missing in the firmware.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Applied to remoteproc-next, thanks.

> This also opens up a more general question: Is it mandatory to have
> a resource table in the firmware?

The implementation we have today does require it, but that's just
because this is what we had to support so far.

> Theoretically a remoteproc could
> also work completely independent, all what would be used from the
> remoteproc framework is the loading and starting capabilities...

Sure. Feel free to add support for your hardware as needed.

Thanks,
Ohad.