[PATCH] spl: Add function called after fpga image upload

[PATCH] spl: Add function called after fpga image upload

[christian.taedcke-oss]

From: Christian Taedcke <christian.taedcke@weidmueller.com>

This way custom logic can be implemented per board after the fpga
image is uploaded.

Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
---

 common/spl/spl_fit.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 730639f756..3a1e3382ba 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -560,6 +560,16 @@ __weak void *spl_load_simple_fit_fix_load(const void *fit)
 	return (void *)fit;
 }
 
+/*
+ * Weak default function to allow implementing logic after fpga image is
+ * uploaded.
+ */
+__weak void board_spl_fit_post_upload_fpga(const void *fit, int node,
+					   struct spl_image_info *fpga_image,
+					   int ret)
+{
+}
+
 static void warn_deprecated(const char *msg)
 {
 	printf("DEPRECATED: %s\n", msg);
@@ -590,6 +600,9 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
 
 	ret = fpga_load(devnum, (void *)fpga_image->load_addr,
 			fpga_image->size, BIT_FULL, flags);
+
+	board_spl_fit_post_upload_fpga(ctx->fit, node, fpga_image, ret);
+
 	if (ret) {
 		printf("%s: Cannot load the image to the FPGA\n", __func__);
 		return ret;
-- 
2.34.1

Re: [PATCH] spl: Add function called after fpga image upload

[Michal Simek]

On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
> From: Christian Taedcke <christian.taedcke@weidmueller.com>
> 
> This way custom logic can be implemented per board after the fpga
> image is uploaded.

What do you want to do there?

I expect Simon won't like that it is another weak function.

> 
> Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
> ---
> 
>   common/spl/spl_fit.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 730639f756..3a1e3382ba 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -560,6 +560,16 @@ __weak void *spl_load_simple_fit_fix_load(const void *fit)
>   	return (void *)fit;
>   }
>   
> +/*
> + * Weak default function to allow implementing logic after fpga image is
> + * uploaded.
> + */
> +__weak void board_spl_fit_post_upload_fpga(const void *fit, int node,
> +					   struct spl_image_info *fpga_image,
> +					   int ret)

Would be good to have kernel-doc and curious about ret parameter.
Also function like this could fail and you should propagate error.

M

Re: [PATCH] spl: Add function called after fpga image upload

[Marek Vasut]

On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
> From: Christian Taedcke <christian.taedcke@weidmueller.com>
> 
> This way custom logic can be implemented per board after the fpga
> image is uploaded.
> 
> Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
> ---
> 
>   common/spl/spl_fit.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 730639f756..3a1e3382ba 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -560,6 +560,16 @@ __weak void *spl_load_simple_fit_fix_load(const void *fit)
>   	return (void *)fit;
>   }
>   
> +/*
> + * Weak default function to allow implementing logic after fpga image is
> + * uploaded.
> + */
> +__weak void board_spl_fit_post_upload_fpga(const void *fit, int node,
> +					   struct spl_image_info *fpga_image,
> +					   int ret)
> +{

Is there an example use case for this, which cannot be implemented 
elsewhere ?

> +}
> +
>   static void warn_deprecated(const char *msg)
>   {
>   	printf("DEPRECATED: %s\n", msg);
> @@ -590,6 +600,9 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
>   
>   	ret = fpga_load(devnum, (void *)fpga_image->load_addr,
>   			fpga_image->size, BIT_FULL, flags);
> +
> +	board_spl_fit_post_upload_fpga(ctx->fit, node, fpga_image, ret);
> +
>   	if (ret) {

Please implement error handling this way:

ret = fpga_load(...)
if (ret)
   handle_error();

ret = board_spl_...(...)
if (ret)
   handle_error();

Re: [PATCH] spl: Add function called after fpga image upload

[Taedcke, Christian]

Am 10.07.2023 um 13:41 schrieb Michal Simek:
> 
> 
> On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>>
>> This way custom logic can be implemented per board after the fpga
>> image is uploaded.
> 
> What do you want to do there?

I have 2 use-cases for this:
1. Clear the RAM which contained the bitstream (memset to zero). This 
should happen independed of the result of the upload operation.
2. Control a LED based on the upload result. So in case the upload 
failed, i want to enable some error LED.

One issue is that the return values of spl_fit_load_fpga() or 
spl_fit_upload_fpga() are not evaluated in common/spl
/spl_fit.c. So this error is not propagated to higher layers.

I my use-case uploading the bitstream is mandatory before starting u-boot.

> 
> I expect Simon won't like that it is another weak function.

I did not find another way to implement the above use-cases. Maybe i 
missed something.

> 
>>
>> Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
>> ---
>>
>>   common/spl/spl_fit.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index 730639f756..3a1e3382ba 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -560,6 +560,16 @@ __weak void *spl_load_simple_fit_fix_load(const 
>> void *fit)
>>       return (void *)fit;
>>   }
>> +/*
>> + * Weak default function to allow implementing logic after fpga image is
>> + * uploaded.
>> + */
>> +__weak void board_spl_fit_post_upload_fpga(const void *fit, int node,
>> +                       struct spl_image_info *fpga_image,
>> +                       int ret)
> 
> Would be good to have kernel-doc and curious about ret parameter.
> Also function like this could fail and you should propagate error.

I can add some code documentation for this function. These are the 
parametes from spl_fit_upload_fpga() plus the return value of fpga_load().

My intend is that this function should not affect the result of 
spl_fit_upload_fpga(). It should be just used trigger something based on 
the upload result (parameter ret) or every time (e.g. clear the data 
buffer in ram).

> 
> M

Regards,
Christian

Re: [PATCH] spl: Add function called after fpga image upload

[Taedcke, Christian]

Am 10.07.2023 um 13:45 schrieb Marek Vasut:
> On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>>
>> This way custom logic can be implemented per board after the fpga
>> image is uploaded.
>>
>> Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
>> ---
>>
>>   common/spl/spl_fit.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index 730639f756..3a1e3382ba 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -560,6 +560,16 @@ __weak void *spl_load_simple_fit_fix_load(const 
>> void *fit)
>>       return (void *)fit;
>>   }
>> +/*
>> + * Weak default function to allow implementing logic after fpga image is
>> + * uploaded.
>> + */
>> +__weak void board_spl_fit_post_upload_fpga(const void *fit, int node,
>> +                       struct spl_image_info *fpga_image,
>> +                       int ret)
>> +{
> 
> Is there an example use case for this, which cannot be implemented 
> elsewhere ?

See my previous email to Michal, i did not find another way to implemnt 
my use-cases.

> 
>> +}
>> +
>>   static void warn_deprecated(const char *msg)
>>   {
>>       printf("DEPRECATED: %s\n", msg);
>> @@ -590,6 +600,9 @@ static int spl_fit_upload_fpga(struct spl_fit_info 
>> *ctx, int node,
>>       ret = fpga_load(devnum, (void *)fpga_image->load_addr,
>>               fpga_image->size, BIT_FULL, flags);
>> +
>> +    board_spl_fit_post_upload_fpga(ctx->fit, node, fpga_image, ret);
>> +
>>       if (ret) {
> 
> Please implement error handling this way:
> 
> ret = fpga_load(...)
> if (ret)
>    handle_error();
> 
> ret = board_spl_...(...)
> if (ret)
>    handle_error();

I need to call board_spl_fit_post_upload_fpga() independent of the 
return value of fpga_load(). This is not a handle_error() function, more 
like a notify_result() (with can be ok or an error).

Regards,
Christian

Re: [PATCH] spl: Add function called after fpga image upload

[Marek Vasut]

On 7/10/23 15:02, Taedcke, Christian wrote:
> Am 10.07.2023 um 13:41 schrieb Michal Simek:
>>
>>
>> On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
>>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>>>
>>> This way custom logic can be implemented per board after the fpga
>>> image is uploaded.
>>
>> What do you want to do there?
> 
> I have 2 use-cases for this:
> 1. Clear the RAM which contained the bitstream (memset to zero). This 
> should happen independed of the result of the upload operation.

Is this some "secure-boot" related item ?

> 2. Control a LED based on the upload result. So in case the upload 
> failed, i want to enable some error LED.
> 
> One issue is that the return values of spl_fit_load_fpga() or 
> spl_fit_upload_fpga() are not evaluated in common/spl
> /spl_fit.c. So this error is not propagated to higher layers.
> 
> I my use-case uploading the bitstream is mandatory before starting u-boot.
> 
>>
>> I expect Simon won't like that it is another weak function.
> 
> I did not find another way to implement the above use-cases. Maybe i 
> missed something.

Based on the above, probably make a weak wrapper around the fpga_load() 
call, make it call fpga_load() by default and override it in some board 
specific manner if needed.

Re: [PATCH] spl: Add function called after fpga image upload

[Taedcke, Christian]

Am 10.07.2023 um 15:44 schrieb Marek Vasut:
> On 7/10/23 15:02, Taedcke, Christian wrote:
>> Am 10.07.2023 um 13:41 schrieb Michal Simek:
>>>
>>>
>>> On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
>>>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>>>>
>>>> This way custom logic can be implemented per board after the fpga
>>>> image is uploaded.
>>>
>>> What do you want to do there?
>>
>> I have 2 use-cases for this:
>> 1. Clear the RAM which contained the bitstream (memset to zero). This 
>> should happen independed of the result of the upload operation.
> 
> Is this some "secure-boot" related item ?

Not directly. This only helps reducing the time the decrypted bitstream 
is stored in RAM.

> 
>> 2. Control a LED based on the upload result. So in case the upload 
>> failed, i want to enable some error LED.
>>
>> One issue is that the return values of spl_fit_load_fpga() or 
>> spl_fit_upload_fpga() are not evaluated in common/spl
>> /spl_fit.c. So this error is not propagated to higher layers.
>>
>> I my use-case uploading the bitstream is mandatory before starting 
>> u-boot.
>>
>>>
>>> I expect Simon won't like that it is another weak function.
>>
>> I did not find another way to implement the above use-cases. Maybe i 
>> missed something.
> 
> Based on the above, probably make a weak wrapper around the fpga_load() 
> call, make it call fpga_load() by default and override it in some board 
> specific manner if needed.

So instead of the implemented function in this patch, i add

__weak int fpga_load_wrapper(int devnum, const void *buf, size_t bsize,
			     bitstream_type bstype, int flags) {
	return fpga_load(devnum, buf, bsize, bstype, flags);
}

in the file common/spl/spl_fit.c and call this instead of fpga_load()?

This way only the logic for loading the fpga bitstream in the spl from a 
fit image would change, but not anywhere else (which is ok for me).

Regards,
Christian

Re: [PATCH] spl: Add function called after fpga image upload

[Marek Vasut]

On 7/10/23 17:15, Taedcke, Christian wrote:
> 
> 
> Am 10.07.2023 um 15:44 schrieb Marek Vasut:
>> On 7/10/23 15:02, Taedcke, Christian wrote:
>>> Am 10.07.2023 um 13:41 schrieb Michal Simek:
>>>>
>>>>
>>>> On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
>>>>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>>>>>
>>>>> This way custom logic can be implemented per board after the fpga
>>>>> image is uploaded.
>>>>
>>>> What do you want to do there?
>>>
>>> I have 2 use-cases for this:
>>> 1. Clear the RAM which contained the bitstream (memset to zero). This 
>>> should happen independed of the result of the upload operation.
>>
>> Is this some "secure-boot" related item ?
> 
> Not directly. This only helps reducing the time the decrypted bitstream 
> is stored in RAM.
> 
>>
>>> 2. Control a LED based on the upload result. So in case the upload 
>>> failed, i want to enable some error LED.
>>>
>>> One issue is that the return values of spl_fit_load_fpga() or 
>>> spl_fit_upload_fpga() are not evaluated in common/spl
>>> /spl_fit.c. So this error is not propagated to higher layers.
>>>
>>> I my use-case uploading the bitstream is mandatory before starting 
>>> u-boot.
>>>
>>>>
>>>> I expect Simon won't like that it is another weak function.
>>>
>>> I did not find another way to implement the above use-cases. Maybe i 
>>> missed something.
>>
>> Based on the above, probably make a weak wrapper around the 
>> fpga_load() call, make it call fpga_load() by default and override it 
>> in some board specific manner if needed.
> 
> So instead of the implemented function in this patch, i add
> 
> __weak int fpga_load_wrapper(int devnum, const void *buf, size_t bsize,
>                   bitstream_type bstype, int flags) {
>      return fpga_load(devnum, buf, bsize, bstype, flags);
> }
> 
> in the file common/spl/spl_fit.c and call this instead of fpga_load()?
> 
> This way only the logic for loading the fpga bitstream in the spl from a 
> fit image would change, but not anywhere else (which is ok for me).

In fact, the logic would change only for your platform, but other than 
that, yes, looks good.

Re: [PATCH] spl: Add function called after fpga image upload

[Simon Glass]

Hi,

On Mon, 10 Jul 2023 at 09:15, Taedcke, Christian
<christian.taedcke-oss@weidmueller.com> wrote:
>
>
>
> Am 10.07.2023 um 15:44 schrieb Marek Vasut:
> > On 7/10/23 15:02, Taedcke, Christian wrote:
> >> Am 10.07.2023 um 13:41 schrieb Michal Simek:
> >>>
> >>>
> >>> On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
> >>>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
> >>>>
> >>>> This way custom logic can be implemented per board after the fpga
> >>>> image is uploaded.
> >>>
> >>> What do you want to do there?
> >>
> >> I have 2 use-cases for this:
> >> 1. Clear the RAM which contained the bitstream (memset to zero). This
> >> should happen independed of the result of the upload operation.
> >
> > Is this some "secure-boot" related item ?
>
> Not directly. This only helps reducing the time the decrypted bitstream
> is stored in RAM.
>
> >
> >> 2. Control a LED based on the upload result. So in case the upload
> >> failed, i want to enable some error LED.
> >>
> >> One issue is that the return values of spl_fit_load_fpga() or
> >> spl_fit_upload_fpga() are not evaluated in common/spl
> >> /spl_fit.c. So this error is not propagated to higher layers.
> >>
> >> I my use-case uploading the bitstream is mandatory before starting
> >> u-boot.
> >>
> >>>
> >>> I expect Simon won't like that it is another weak function.
> >>
> >> I did not find another way to implement the above use-cases. Maybe i
> >> missed something.
> >
> > Based on the above, probably make a weak wrapper around the fpga_load()
> > call, make it call fpga_load() by default and override it in some board
> > specific manner if needed.
>
> So instead of the implemented function in this patch, i add
>
> __weak int fpga_load_wrapper(int devnum, const void *buf, size_t bsize,
>                              bitstream_type bstype, int flags) {
>         return fpga_load(devnum, buf, bsize, bstype, flags);
> }
>
> in the file common/spl/spl_fit.c and call this instead of fpga_load()?
>
> This way only the logic for loading the fpga bitstream in the spl from a
> fit image would change, but not anywhere else (which is ok for me).

Firstly we should not be using devnum but a struct udevice. Really the
FPGA subsystem needs to be converted to use drivel model properly.

But anyway, could you use an event? [1]

Then you can write:

   event_notify_null(EVT_FPGA_LOAD);

and the event spy (if any) can do what is needed. You can add
parameters to the event too.

Regards,
Simon

[1] https://u-boot.readthedocs.io/en/latest/develop/event.html

Re: [PATCH] spl: Add function called after fpga image upload

[Marek Vasut]

On 7/10/23 17:47, Simon Glass wrote:
> Hi,
> 
> On Mon, 10 Jul 2023 at 09:15, Taedcke, Christian
> <christian.taedcke-oss@weidmueller.com> wrote:
>>
>>
>>
>> Am 10.07.2023 um 15:44 schrieb Marek Vasut:
>>> On 7/10/23 15:02, Taedcke, Christian wrote:
>>>> Am 10.07.2023 um 13:41 schrieb Michal Simek:
>>>>>
>>>>>
>>>>> On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
>>>>>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>>>>>>
>>>>>> This way custom logic can be implemented per board after the fpga
>>>>>> image is uploaded.
>>>>>
>>>>> What do you want to do there?
>>>>
>>>> I have 2 use-cases for this:
>>>> 1. Clear the RAM which contained the bitstream (memset to zero). This
>>>> should happen independed of the result of the upload operation.
>>>
>>> Is this some "secure-boot" related item ?
>>
>> Not directly. This only helps reducing the time the decrypted bitstream
>> is stored in RAM.
>>
>>>
>>>> 2. Control a LED based on the upload result. So in case the upload
>>>> failed, i want to enable some error LED.
>>>>
>>>> One issue is that the return values of spl_fit_load_fpga() or
>>>> spl_fit_upload_fpga() are not evaluated in common/spl
>>>> /spl_fit.c. So this error is not propagated to higher layers.
>>>>
>>>> I my use-case uploading the bitstream is mandatory before starting
>>>> u-boot.
>>>>
>>>>>
>>>>> I expect Simon won't like that it is another weak function.
>>>>
>>>> I did not find another way to implement the above use-cases. Maybe i
>>>> missed something.
>>>
>>> Based on the above, probably make a weak wrapper around the fpga_load()
>>> call, make it call fpga_load() by default and override it in some board
>>> specific manner if needed.
>>
>> So instead of the implemented function in this patch, i add
>>
>> __weak int fpga_load_wrapper(int devnum, const void *buf, size_t bsize,
>>                               bitstream_type bstype, int flags) {
>>          return fpga_load(devnum, buf, bsize, bstype, flags);
>> }
>>
>> in the file common/spl/spl_fit.c and call this instead of fpga_load()?
>>
>> This way only the logic for loading the fpga bitstream in the spl from a
>> fit image would change, but not anywhere else (which is ok for me).
> 
> Firstly we should not be using devnum but a struct udevice. Really the
> FPGA subsystem needs to be converted to use drivel model properly.

Maybe Xilinx can help here as they are the maintainer of the FPGA 
framework ?

Re: [PATCH] spl: Add function called after fpga image upload

[Michal Simek]

On 7/10/23 17:50, Marek Vasut wrote:
> On 7/10/23 17:47, Simon Glass wrote:
>> Hi,
>>
>> On Mon, 10 Jul 2023 at 09:15, Taedcke, Christian
>> <christian.taedcke-oss@weidmueller.com> wrote:
>>>
>>>
>>>
>>> Am 10.07.2023 um 15:44 schrieb Marek Vasut:
>>>> On 7/10/23 15:02, Taedcke, Christian wrote:
>>>>> Am 10.07.2023 um 13:41 schrieb Michal Simek:
>>>>>>
>>>>>>
>>>>>> On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
>>>>>>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>>>>>>>
>>>>>>> This way custom logic can be implemented per board after the fpga
>>>>>>> image is uploaded.
>>>>>>
>>>>>> What do you want to do there?
>>>>>
>>>>> I have 2 use-cases for this:
>>>>> 1. Clear the RAM which contained the bitstream (memset to zero). This
>>>>> should happen independed of the result of the upload operation.
>>>>
>>>> Is this some "secure-boot" related item ?
>>>
>>> Not directly. This only helps reducing the time the decrypted bitstream
>>> is stored in RAM.
>>>
>>>>
>>>>> 2. Control a LED based on the upload result. So in case the upload
>>>>> failed, i want to enable some error LED.
>>>>>
>>>>> One issue is that the return values of spl_fit_load_fpga() or
>>>>> spl_fit_upload_fpga() are not evaluated in common/spl
>>>>> /spl_fit.c. So this error is not propagated to higher layers.
>>>>>
>>>>> I my use-case uploading the bitstream is mandatory before starting
>>>>> u-boot.
>>>>>
>>>>>>
>>>>>> I expect Simon won't like that it is another weak function.
>>>>>
>>>>> I did not find another way to implement the above use-cases. Maybe i
>>>>> missed something.
>>>>
>>>> Based on the above, probably make a weak wrapper around the fpga_load()
>>>> call, make it call fpga_load() by default and override it in some board
>>>> specific manner if needed.
>>>
>>> So instead of the implemented function in this patch, i add
>>>
>>> __weak int fpga_load_wrapper(int devnum, const void *buf, size_t bsize,
>>>                               bitstream_type bstype, int flags) {
>>>          return fpga_load(devnum, buf, bsize, bstype, flags);
>>> }
>>>
>>> in the file common/spl/spl_fit.c and call this instead of fpga_load()?
>>>
>>> This way only the logic for loading the fpga bitstream in the spl from a
>>> fit image would change, but not anywhere else (which is ok for me).
>>
>> Firstly we should not be using devnum but a struct udevice. Really the
>> FPGA subsystem needs to be converted to use drivel model properly.
> 
> Maybe Xilinx can help here as they are the maintainer of the FPGA framework ?

:-) I will add it to our list. And maybe USB maintainer can keep DWC3 and other 
parts taken from Linux kernel aligned with latest version too.

M

Re: [PATCH] spl: Add function called after fpga image upload

[Taedcke, Christian]

Hello,

On 10.07.2023 17:47, Simon Glass wrote:
> Hi,
> 
> On Mon, 10 Jul 2023 at 09:15, Taedcke, Christian
> <christian.taedcke-oss@weidmueller.com> wrote:
>>
>>
>>
>> Am 10.07.2023 um 15:44 schrieb Marek Vasut:
>>> On 7/10/23 15:02, Taedcke, Christian wrote:
>>>> Am 10.07.2023 um 13:41 schrieb Michal Simek:
>>>>>
>>>>>
>>>>> On 6/27/23 11:04, christian.taedcke-oss@weidmueller.com wrote:
>>>>>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>>>>>>
>>>>>> This way custom logic can be implemented per board after the fpga
>>>>>> image is uploaded.
>>>>>
>>>>> What do you want to do there?
>>>>
>>>> I have 2 use-cases for this:
>>>> 1. Clear the RAM which contained the bitstream (memset to zero). This
>>>> should happen independed of the result of the upload operation.
>>>
>>> Is this some "secure-boot" related item ?
>>
>> Not directly. This only helps reducing the time the decrypted bitstream
>> is stored in RAM.
>>
>>>
>>>> 2. Control a LED based on the upload result. So in case the upload
>>>> failed, i want to enable some error LED.
>>>>
>>>> One issue is that the return values of spl_fit_load_fpga() or
>>>> spl_fit_upload_fpga() are not evaluated in common/spl
>>>> /spl_fit.c. So this error is not propagated to higher layers.
>>>>
>>>> I my use-case uploading the bitstream is mandatory before starting
>>>> u-boot.
>>>>
>>>>>
>>>>> I expect Simon won't like that it is another weak function.
>>>>
>>>> I did not find another way to implement the above use-cases. Maybe i
>>>> missed something.
>>>
>>> Based on the above, probably make a weak wrapper around the fpga_load()
>>> call, make it call fpga_load() by default and override it in some board
>>> specific manner if needed.
>>
>> So instead of the implemented function in this patch, i add
>>
>> __weak int fpga_load_wrapper(int devnum, const void *buf, size_t bsize,
>>                               bitstream_type bstype, int flags) {
>>          return fpga_load(devnum, buf, bsize, bstype, flags);
>> }
>>
>> in the file common/spl/spl_fit.c and call this instead of fpga_load()?
>>
>> This way only the logic for loading the fpga bitstream in the spl from a
>> fit image would change, but not anywhere else (which is ok for me).
> 
> Firstly we should not be using devnum but a struct udevice. Really the
> FPGA subsystem needs to be converted to use drivel model properly.
> 
> But anyway, could you use an event? [1]
> 
> Then you can write:
> 
>     event_notify_null(EVT_FPGA_LOAD);
> 
> and the event spy (if any) can do what is needed. You can add
> parameters to the event too.
> 
> Regards,
> Simon
> 
> [1] https://u-boot.readthedocs.io/en/latest/develop/event.html

Thank you for the suggestion. I will prepare a new version of the series 
using an event that is sent from inside fpga_load().

Regards,
Christian