From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chee, Tien Fong Date: Wed, 30 Aug 2017 08:18:48 +0000 Subject: [U-Boot] [PATCH 04/19] arm: socfpga: Add FPGA loadfs command support In-Reply-To: <36f9aa80-801a-0e75-5480-dbe745dd3c83@denx.de> References: <1504003561-6290-1-git-send-email-tien.fong.chee@intel.com> <1504003561-6290-5-git-send-email-tien.fong.chee@intel.com> <36f9aa80-801a-0e75-5480-dbe745dd3c83@denx.de> Message-ID: <1504081127.7727.45.camel@intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Sel, 2017-08-29 at 13:57 +0200, Marek Vasut wrote: > On 08/29/2017 12:45 PM, tien.fong.chee at intel.com wrote: > > > > From: Tien Fong Chee > > > > This patch enables FPGA loadfs command support to U-boot console. > > > > Signed-off-by: Tien Fong Chee > But you enabled support for this in patch 1/19 ... ordering issue ? > Patch changelog doesn't match what the patch does ... > This patch is adding the FPGA loadfs command support for Arria 10. I enabled FPGA loadfs in patch 1/19, so i can compile the changes in this patch immediately. So, you want me to improve the commit message and reordering? > > > > --- > > 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] > > \n" > > " [] \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. > This is generic multiplexing code, calling generic function name fpga_fsload will lead to repective FPGA driver. > > > > +#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); > > >