[U-Boot] [PATCH v4 3/4] arm: socfpga: stratix10: Add Stratix10 FPGA into FPGA device table

Simon Goldschmidt simon.k.r.goldschmidt at gmail.com
Mon Nov 19 09:57:21 UTC 2018


On Mon, Nov 19, 2018 at 10:46 AM <chee.hong.ang at intel.com> wrote:
>
> From: "Ang, Chee Hong" <chee.hong.ang at intel.com>
>
> Enable 'fpga' command in u-boot. User will be able to use the FPGA
> command to program the FPGA on Stratix10 SoC.
>
> Signed-off-by: Ang, Chee Hong <chee.hong.ang at intel.com>
> ---
>  arch/arm/mach-socfpga/Makefile            |  4 +++
>  arch/arm/mach-socfpga/fpga_device.c       | 59 +++++++++++++++++++++++++++++++
>  arch/arm/mach-socfpga/include/mach/misc.h |  4 ---
>  arch/arm/mach-socfpga/misc.c              | 31 +---------------
>  arch/arm/mach-socfpga/misc_s10.c          |  2 ++
>  drivers/fpga/altera.c                     |  6 ++++
>  include/altera.h                          |  4 +++
>  7 files changed, 76 insertions(+), 34 deletions(-)
>  create mode 100644 arch/arm/mach-socfpga/fpga_device.c
>
> diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
> index e667204..2ff1b3f 100644
> --- a/arch/arm/mach-socfpga/Makefile
> +++ b/arch/arm/mach-socfpga/Makefile
> @@ -10,6 +10,10 @@ obj-y        += clock_manager.o
>  obj-y  += misc.o
>  obj-y  += reset_manager.o
>
> +ifdef CONFIG_FPGA
> +obj-y  += fpga_device.o
> +endif
> +
>  ifdef CONFIG_TARGET_SOCFPGA_GEN5
>  obj-y  += clock_manager_gen5.o
>  obj-y  += misc_gen5.o
> diff --git a/arch/arm/mach-socfpga/fpga_device.c b/arch/arm/mach-socfpga/fpga_device.c
> new file mode 100644
> index 0000000..97b27eb
> --- /dev/null
> +++ b/arch/arm/mach-socfpga/fpga_device.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Intel Corporation <www.intel.com>
> + */
> +
> +#include <common.h>
> +#include <altera.h>
> +
> +#ifdef CONFIG_FPGA_STRATIX10
> +/*
> + * FPGA programming support for SoC FPGA Stratix 10
> + */
> +static Altera_desc altera_fpga[] = {
> +       {
> +               /* Family */
> +               Intel_FPGA_Stratix10,
> +               /* Interface type */
> +               secure_device_manager_mailbox,
> +               /* No limitation as additional data will be ignored */
> +               -1,
> +               /* No device function table */
> +               NULL,
> +               /* Base interface address specified in driver */
> +               NULL,
> +               /* No cookie implementation */
> +               0
> +       },
> +};
> +#else
> +/*
> + * FPGA programming support for SoC FPGA Cyclone V
> + */
> +static Altera_desc altera_fpga[] = {
> +       {
> +               /* Family */
> +               Altera_SoCFPGA,
> +               /* Interface type */
> +               fast_passive_parallel,
> +               /* No limitation as additional data will be ignored */
> +               -1,
> +               /* No device function table */
> +               NULL,
> +               /* Base interface address specified in driver */
> +               NULL,
> +               /* No cookie implementation */
> +               0
> +       },
> +};
> +#endif
> +
> +/* add device descriptor to FPGA device table */
> +void socfpga_fpga_add(void)
> +{
> +       int i;
> +
> +       fpga_init();
> +       for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
> +               fpga_add(fpga_altera, &altera_fpga[i]);
> +}
> diff --git a/arch/arm/mach-socfpga/include/mach/misc.h b/arch/arm/mach-socfpga/include/mach/misc.h
> index 4fc9570..6fa9495 100644
> --- a/arch/arm/mach-socfpga/include/mach/misc.h
> +++ b/arch/arm/mach-socfpga/include/mach/misc.h
> @@ -15,11 +15,7 @@ struct bsel {
>
>  extern struct bsel bsel_str[];
>
> -#ifdef CONFIG_FPGA
>  void socfpga_fpga_add(void);
> -#else
> -static inline void socfpga_fpga_add(void) {}
> -#endif
>
>  #ifdef CONFIG_TARGET_SOCFPGA_GEN5
>  void socfpga_sdram_remap_zero(void);
> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
> index a4f6d5c..55f846a 100644
> --- a/arch/arm/mach-socfpga/misc.c
> +++ b/arch/arm/mach-socfpga/misc.c
> @@ -87,36 +87,7 @@ int overwrite_console(void)
>  }
>  #endif
>
> -#ifdef CONFIG_FPGA
> -/*
> - * FPGA programming support for SoC FPGA Cyclone V
> - */
> -static Altera_desc altera_fpga[] = {
> -       {
> -               /* Family */
> -               Altera_SoCFPGA,
> -               /* Interface type */
> -               fast_passive_parallel,
> -               /* No limitation as additional data will be ignored */
> -               -1,
> -               /* No device function table */
> -               NULL,
> -               /* Base interface address specified in driver */
> -               NULL,
> -               /* No cookie implementation */
> -               0
> -       },
> -};
> -
> -/* add device descriptor to FPGA device table */
> -void socfpga_fpga_add(void)
> -{
> -       int i;
> -       fpga_init();
> -       for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
> -               fpga_add(fpga_altera, &altera_fpga[i]);
> -}
> -#endif
> +__weak void socfpga_fpga_add(void) {}

I'm not sure I completely followed the discussion of previous versions
of this series, but is this really the coding style U-Boot wants?
I would have thought weak functions are used for unknown extension
points (multiple architectures or boards), but this is a config option
in a defined set of files. In my opinion, using weak here is not the
right thing to do.

I'd rather add a header file fpga_device.h that declares this function
depending on CONFIG_FPGA.

Simon

>
>  int arch_cpu_init(void)
>  {
> diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-socfpga/misc_s10.c
> index e599362..e611d2c 100644
> --- a/arch/arm/mach-socfpga/misc_s10.c
> +++ b/arch/arm/mach-socfpga/misc_s10.c
> @@ -125,6 +125,8 @@ int arch_misc_init(void)
>
>  int arch_early_init_r(void)
>  {
> +       socfpga_fpga_add();
> +
>         return 0;
>  }
>
> diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
> index 9605554..7c8f518 100644
> --- a/drivers/fpga/altera.c
> +++ b/drivers/fpga/altera.c
> @@ -39,6 +39,9 @@ static const struct altera_fpga {
>  #if defined(CONFIG_FPGA_STRATIX_V)
>         { Altera_StratixV, "StratixV", stratixv_load, NULL, NULL },
>  #endif
> +#if defined(CONFIG_FPGA_STRATIX10)
> +       { Intel_FPGA_Stratix10, "Stratix10", stratix10_load, NULL, NULL },
> +#endif
>  #if defined(CONFIG_FPGA_SOCFPGA)
>         { Altera_SoCFPGA, "SoC FPGA", socfpga_load, NULL, NULL },
>  #endif
> @@ -154,6 +157,9 @@ int altera_info(Altera_desc *desc)
>         case fast_passive_parallel_security:
>                 printf("Fast Passive Parallel with Security (FPPS)\n");
>                 break;
> +       case secure_device_manager_mailbox:
> +               puts("Secure Device Manager (SDM) Mailbox\n");
> +               break;
>                 /* Add new interface types here */
>         default:
>                 printf("Unsupported interface type, %d\n", desc->iface);
> diff --git a/include/altera.h b/include/altera.h
> index 233b467..22d55cf 100644
> --- a/include/altera.h
> +++ b/include/altera.h
> @@ -39,6 +39,8 @@ enum altera_iface {
>         fast_passive_parallel,
>         /* fast passive parallel with security (FPPS) */
>         fast_passive_parallel_security,
> +       /* secure device manager (SDM) mailbox */
> +       secure_device_manager_mailbox,
>         /* insert all new types before this */
>         max_altera_iface_type,
>  };
> @@ -54,6 +56,8 @@ enum altera_family {
>         Altera_StratixII,
>         /* StratixV Family */
>         Altera_StratixV,
> +       /* Stratix10 Family */
> +       Intel_FPGA_Stratix10,
>         /* SoCFPGA Family */
>         Altera_SoCFPGA,
>
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot


More information about the U-Boot mailing list