[U-Boot] [PATCH 04/19] arm: socfpga: Add FPGA loadfs command support

Marek Vasut
2017-08-29T11:57:37Z

On 08/29/2017 12:45 PM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> This patch enables FPGA loadfs command support to U-boot console.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>

But you enabled support for this in patch 1/19 ... ordering issue ?
Patch changelog doesn't match what the patch does ...

> ---
> cmd/fpga.c | 2 +-
> drivers/fpga/altera.c | 38 ++++++++++++++++++++++++++++++--------
> drivers/fpga/fpga.c | 8 ++++++++
> include/fpga.h | 2 ++
> 4 files changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/cmd/fpga.c b/cmd/fpga.c
> index 016349f..28e6628 100644
> --- a/cmd/fpga.c
> +++ b/cmd/fpga.c
> @@ -363,7 +363,7 @@ U_BOOT_CMD(fpga, 6, 1, do_fpga,
> "(Xilinx only)\n"
> #endif
> #if defined(CONFIG_CMD_FPGA_LOADFS)
> - "Load device from filesystem (FAT by default) (Xilinx only)\n"
> + "Load device from filesystem (FAT by default)\n"
> " loadfs [dev] [address] [image size] [blocksize] <interface>\n"
> " [<dev[:part]>] <filename>\n"
> #endif
> diff --git a/drivers/fpga/altera.c b/drivers/fpga/altera.c
> index 135a357..8b60bd5 100644
> --- a/drivers/fpga/altera.c
> +++ b/drivers/fpga/altera.c
> @@ -23,25 +23,30 @@ static const struct altera_fpga {
> enum altera_family family;
> const char *name;
> int (*load)(Altera_desc *, const void *, size_t);
> + int (*loadfs)(Altera_desc *, const void *, size_t,
> + fpga_fs_info *);
> int (*dump)(Altera_desc *, const void *, size_t);
> int (*info)(Altera_desc *);
> } altera_fpga[] = {
> #if defined(CONFIG_FPGA_ACEX1K)
> - { Altera_ACEX1K, "ACEX1K", ACEX1K_load, ACEX1K_dump, ACEX1K_info },
> - { Altera_CYC2, "ACEX1K", ACEX1K_load, ACEX1K_dump, ACEX1K_info },
> + { Altera_ACEX1K, "ACEX1K", ACEX1K_load, NULL, ACEX1K_dump, ACEX1K_info },
> + { Altera_CYC2, "ACEX1K", ACEX1K_load, NULL, ACEX1K_dump, ACEX1K_info },
> #elif defined(CONFIG_FPGA_CYCLON2)
> - { Altera_ACEX1K, "CycloneII", CYC2_load, CYC2_dump, CYC2_info },
> - { Altera_CYC2, "CycloneII", CYC2_load, CYC2_dump, CYC2_info },
> + { Altera_ACEX1K, "CycloneII", CYC2_load, NULL, CYC2_dump, CYC2_info },
> + { Altera_CYC2, "CycloneII", CYC2_load, NULL, CYC2_dump, CYC2_info },
> #endif
> #if defined(CONFIG_FPGA_STRATIX_II)
> - { Altera_StratixII, "StratixII", StratixII_load,
> + { Altera_StratixII, "StratixII", StratixII_load, NULL,
> StratixII_dump, StratixII_info },
> #endif
> #if defined(CONFIG_FPGA_STRATIX_V)
> - { Altera_StratixV, "StratixV", stratixv_load, NULL, NULL },
> + { Altera_StratixV, "StratixV", stratixv_load, NULL, NULL, NULL },
> #endif
> -#if defined(CONFIG_FPGA_SOCFPGA)
> - { Altera_SoCFPGA, "SoC FPGA", socfpga_load, NULL, NULL },
> +#if defined(CONFIG_FPGA_SOCFPGA) && !defined(CONFIG_CMD_FPGA_LOADFS)
> + { Altera_SoCFPGA, "SoC FPGA", socfpga_load, NULL, NULL, NULL },
> +#endif
> +#if defined(CONFIG_FPGA_SOCFPGA) && defined(CONFIG_CMD_FPGA_LOADFS)
> + { Altera_SoCFPGA, "SoC FPGA", socfpga_load, socfpga_loadfs, NULL, NULL },
> #endif
> };
>
> @@ -174,3 +179,20 @@ int altera_info(Altera_desc *desc)
>
> return FPGA_SUCCESS;
> }
> +
> +#if defined(CONFIG_CMD_FPGA_LOADFS)
> +int altera_loadfs(Altera_desc *desc, const void *buf, size_t bsize,
> + fpga_fs_info *fpga_fsinfo)
> +{
> + const struct altera_fpga *fpga = altera_desc_to_fpga(desc, __func__);
> +
> + if (!fpga)
> + return FPGA_FAIL;
> +
> + debug_cond(FPGA_DEBUG, "%s: Launching the %s FS Loader...\n",
> + __func__, fpga->name);
> + if (fpga->loadfs)
> + return fpga->loadfs(desc, buf, bsize, fpga_fsinfo);
> + return 0;
> +}
> +#endif
> diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
> index e0fb1b4..42e901e 100644
> --- a/drivers/fpga/fpga.c
> +++ b/drivers/fpga/fpga.c
> @@ -198,6 +198,14 @@ int fpga_fsload(int devnum, const void *buf, size_t size,
> fpga_no_sup((char *)__func__, "Xilinx devices");
> #endif
> break;
> +#if defined(CONFIG_FPGA_ALTERA)
> + case fpga_altera:
> + ret_val = altera_loadfs(desc->devdesc, buf, size,
> + fpga_fsinfo);

Why is this not a generic code ? Load stuff from FS and program it into
the FPGA directly, should be pretty generic operation IMO.

> +#else
> + fpga_no_sup((char *)__func__, "Altera devices");
> +#endif
> + break;
> default:
> printf("%s: Invalid or unsupported device type %d\n",
> __func__, desc->devtype);
> diff --git a/include/fpga.h b/include/fpga.h
> index d768fb1..8920016 100644
> --- a/include/fpga.h
> +++ b/include/fpga.h
> @@ -56,8 +56,10 @@ int fpga_count(void);
> const fpga_desc *const fpga_get_desc(int devnum);
> int fpga_load(int devnum, const void *buf, size_t bsize,
> bitstream_type bstype);
> +#if defined(CONFIG_CMD_FPGA_LOADFS)
> int fpga_fsload(int devnum, const void *buf, size_t size,
> fpga_fs_info *fpga_fsinfo);
> +#endif
> int fpga_loadbitstream(int devnum, char *fpgadata, size_t size,
> bitstream_type bstype);
> int fpga_dump(int devnum, const void *buf, size_t bsize);
>

--
Best regards,
Marek Vasut