[PATCH v2] event: Add fpga load event

[Simon Glass]

Hi  Christian,

On Tue, 11 Jul 2023 at 05:44, <christian.taedcke-oss at weidmueller.com> wrote:
>
> From: Christian Taedcke <christian.taedcke at weidmueller.com>
>
> This enables implementing custom logic after a bitstream was loaded
> into the fpga.
>
> Signed-off-by: Christian Taedcke <christian.taedcke at weidmueller.com>
> ---
>
> Changes in v2:
> - replace __weak function with a new event
>
>  drivers/fpga/fpga.c | 18 ++++++++++++++++++
>  include/event.h     | 16 ++++++++++++++++
>  2 files changed, 34 insertions(+)

Looks good with a few nits

>
> diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
> index 7f6b6bc73a..7b00bf4d55 100644
> --- a/drivers/fpga/fpga.c
> +++ b/drivers/fpga/fpga.c
> @@ -244,6 +244,22 @@ int fpga_loads(int devnum, const void *buf, size_t size,
>  }
>  #endif
>
> +static void fpga_load_event_notify(const void *buf, size_t bsize, int result)
> +{
> +#if CONFIG_IS_ENABLED(EVENT)

Please use if (CONFIG_IS_ENABLED()) to reduce the number of build paths.

> +       int ret;
> +       struct event_fpga_load load = {
> +               .buf = buf,
> +               .bsize = bsize,
> +               .result = result
> +       };
> +
> +       ret = event_notify(EVT_FPGA_LOAD, &load, sizeof(load));
> +       if (ret)
> +               printf("%s: fpga load event failed: %d\n", __func__, ret);

That adds a string to U-Boot. I suppose that is OK...I'm just pointing it out.

I think it is better to pass the error number back to the caller...see below.

> +#endif
> +}
> +
>  /*
>   * Generic multiplexing code
>   */
> @@ -284,6 +300,8 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype,
>                 }
>         }
>
> +       fpga_load_event_notify(buf, bsize, ret_val);

... and check the error here, since presumably we should return it and
fail at this point.

> +
>         return ret_val;
>  }
>
> diff --git a/include/event.h b/include/event.h
> index fe41080fa6..77124c2e73 100644
> --- a/include/event.h
> +++ b/include/event.h
> @@ -31,6 +31,9 @@ enum event_t {
>         /* Init hooks */
>         EVT_MISC_INIT_F,
>
> +       /* Fpga load hook */
> +       EVT_FPGA_LOAD,
> +
>         /* Device tree fixups before booting */
>         EVT_FT_FIXUP,
>
> @@ -59,6 +62,19 @@ union event_data {
>                 struct udevice *dev;
>         } dm;
>
> +       /**
> +        * struct event_fpga_load - fpga load event
> +        *
> +        * @buf: The buffer that was loaded into the fpga
> +        * @bsize: The size of the buffer that was loaded into the fpga
> +        * @result: Result of the load operation
> +        */
> +       struct event_fpga_load {
> +               const void *buf;
> +               size_t bsize;
> +               int result;
> +       } fpga_load;
> +
>         /**
>          * struct event_ft_fixup - FDT fixup before booting
>          *
> --
> 2.34.1
>

Regards,
Simon