On Sel, 2017-09-05 at 11:36 +0200, Marek Vasut wrote: > On 09/05/2017 11:23 AM, Chee, Tien Fong wrote: > > > > On Sel, 2017-09-05 at 11:04 +0200, Marek Vasut wrote: > > > > > > On 09/05/2017 07:53 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > On Isn, 2017-09-04 at 11:39 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > On 09/04/2017 09:08 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Rab, 2017-08-30 at 10:52 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 08/30/2017 10:05 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sel, 2017-08-29 at 13:55 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 08/29/2017 12:45 PM, [email protected] > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: Tien Fong Chee <[email protected]> > > > > > > > > > > > > > > > > > > > > This driver handles FPGA program operation from > > > > > > > > > > flash > > > > > > > > > > loading > > > > > > > > > > RBF to memory and then to program FPGA. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel > > > > > > > > > > .com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > .../include/mach/fpga_manager_arria10.h > > > > > > > > > > | > > > > > > > > > > 27 > > > > > > > > > > ++ > > > > > > > > > > drivers/fpga/socfpga_arria10.c > > > > > > > > > > | > > > > > > > > > > 386 > > > > > > > > > > +++++++++++++++++++- > > > > > > > > > > include/altera.h > > > > > > > > > > | > > > > > > > > > > 6 > > > > > > > > > > + > > > > > > > > > > include/configs/socfpga_common.h > > > > > > > > > > | > > > > > > > > > > 4 > > > > > > > > > > + > > > > > > > > > > 4 files changed, 422 insertions(+), 1 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/arch/arm/mach- > > > > > > > > > > socfpga/include/mach/fpga_manager_arria10.h > > > > > > > > > > b/arch/arm/mach- > > > > > > > > > > socfpga/include/mach/fpga_manager_arria10.h > > > > > > > > > > index 9cbf696..93a9122 100644 > > > > > > > > > > --- a/arch/arm/mach- > > > > > > > > > > socfpga/include/mach/fpga_manager_arria10.h > > > > > > > > > > +++ b/arch/arm/mach- > > > > > > > > > > socfpga/include/mach/fpga_manager_arria10.h > > > > > > > > > > @@ -8,6 +8,8 @@ > > > > > > > > > > #ifndef _FPGA_MANAGER_ARRIA10_H_ > > > > > > > > > > #define _FPGA_MANAGER_ARRIA10_H_ > > > > > > > > > > > > > > > > > > > > +#include <asm/cache.h> > > > > > > > > > > + > > > > > > > > > > #define > > > > > > > > > > ALT_FPGAMGR_IMGCFG_STAT_F2S_CRC_ERROR_SET_MSK > > > > > > > > > > > > > > > > > > > > BIT(0) > > > > > > > > > > #define > > > > > > > > > > ALT_FPGAMGR_IMGCFG_STAT_F2S_EARLY_USERMODE_SET_MSK > > > > > > > > > > > > > > > > > > > > BIT(1) > > > > > > > > > > #define > > > > > > > > > > ALT_FPGAMGR_IMGCFG_STAT_F2S_USERMODE_SET_MSK > > > > > > > > > > > > > > > > > > > > BIT(2) > > > > > > > > > > @@ -89,11 +91,36 @@ struct socfpga_fpga_manager { > > > > > > > > > > u32 imgcfg_fifo_status; > > > > > > > > > > }; > > > > > > > > > > > > > > > > > > > > +#if defined(CONFIG_CMD_FPGA_LOADFS) > > > > > > > > > > +enum rbf_type {unknown, periph_section, > > > > > > > > > > core_section}; > > > > > > > > > > +enum rbf_security {invalid, unencrypted, > > > > > > > > > > encrypted}; > > > > > > > > > > + > > > > > > > > > > +struct rbf_info { > > > > > > > > > > + enum rbf_type section; > > > > > > > > > > + enum rbf_security security; > > > > > > > > > > +}; > > > > > > > > > > + > > > > > > > > > > +struct flash_info { > > > > > > > > > > + char *interface; > > > > > > > > > > + char *dev_part; > > > > > > > > > > + char *filename; > > > > > > > > > > + int fstype; > > > > > > > > > > + u32 remaining; > > > > > > > > > > + u32 flash_offset; > > > > > > > > > > + struct rbf_info rbfinfo; > > > > > > > > > > + struct image_header header; > > > > > > > > > > +}; > > > > > > > > > > +#endif > > > > > > > > > > + > > > > > > > > > > /* Functions */ > > > > > > > > > > int fpgamgr_program_init(u32 * rbf_data, size_t > > > > > > > > > > rbf_size); > > > > > > > > > > int fpgamgr_program_finish(void); > > > > > > > > > > int is_fpgamgr_user_mode(void); > > > > > > > > > > int fpgamgr_wait_early_user_mode(void); > > > > > > > > > > +#if defined(CONFIG_CMD_FPGA_LOADFS) > > > > > > > > > > +const char *get_cff_filename(const void *fdt, int > > > > > > > > > > *len, > > > > > > > > > > u32 > > > > > > > > > > core); > > > > > > > > > > +const char *get_cff_devpart(const void *fdt, int > > > > > > > > > > *len); > > > > > > > > > > +#endif > > > > > > > > > > > > > > > > > > > > #endif /* __ASSEMBLY__ */ > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/fpga/socfpga_arria10.c > > > > > > > > > > b/drivers/fpga/socfpga_arria10.c > > > > > > > > > > index 5c1a68a..90c55e5 100644 > > > > > > > > > > --- a/drivers/fpga/socfpga_arria10.c > > > > > > > > > > +++ b/drivers/fpga/socfpga_arria10.c > > > > > > > > > > @@ -13,6 +13,12 @@ > > > > > > > > > > #include <altera.h> > > > > > > > > > > #include <common.h> > > > > > > > > > > #include <errno.h> > > > > > > > > > > +#include <fat.h> > > > > > > > > > > +#include <fs.h> > > > > > > > > > > +#include <fdtdec.h> > > > > > > > > > > +#include <malloc.h> > > > > > > > > > > +#include <part.h> > > > > > > > > > > +#include <spl.h> > > > > > > > > > > #include <wait_bit.h> > > > > > > > > > > #include <watchdog.h> > > > > > > > > > > > > > > > > > > > > @@ -22,6 +28,10 @@ > > > > > > > > > > #define COMPRESSION_OFFSET 229 > > > > > > > > > > #define FPGA_TIMEOUT_MSEC 1000 /* timeout > > > > > > > > > > in > > > > > > > > > > ms */ > > > > > > > > > > #define FPGA_TIMEOUT_CNT 0x1000000 > > > > > > > > > > +#define RBF_UNENCRYPTED 0xa65c > > > > > > > > > > +#define RBF_ENCRYPTED 0xa65d > > > > > > > > > > +#define ARRIA10RBF_PERIPH 0x0001 > > > > > > > > > > +#define ARRIA10RBF_CORE 0x8001 > > > > > > > > > > > > > > > > > > > > DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > > > > > > > > > @@ -118,7 +128,7 @@ static int > > > > > > > > > > wait_for_nconfig_pin_and_nstatus_pin(void) > > > > > > > > > > return wait_for_bit(__func__, > > > > > > > > > > &fpga_manager_base- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > imgcfg_stat, > > > > > > > > > > mask, > > > > > > > > > > - false, > > > > > > > > > > FPGA_TIMEOUT_MSEC, > > > > > > > > > > false); > > > > > > > > > > + true, > > > > > > > > > > FPGA_TIMEOUT_MSEC, > > > > > > > > > > false); > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > static int wait_for_f2s_nstatus_pin(unsigned long > > > > > > > > > > value) > > > > > > > > > > @@ -453,6 +463,281 @@ int > > > > > > > > > > fpgamgr_program_finish(void) > > > > > > > > > > return 0; > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > +#if defined(CONFIG_CMD_FPGA_LOADFS) > > > > > > > > > > +const char *get_cff_filename(const void *fdt, int > > > > > > > > > > *len, > > > > > > > > > > u32 > > > > > > > > > > core) > > > > > > > > > > +{ > > > > > > > > > > + const char *cff_filename = NULL; > > > > > > > > > > + const char *cell; > > > > > > > > > > + int nodeoffset; > > > > > > > > > > + nodeoffset = fdt_subnode_offset(fdt, 0, > > > > > > > > > > "chosen"); > > > > > > > > > > + > > > > > > > > > > + if (nodeoffset >= 0) { > > > > > > > > > > + if (core) > > > > > > > > > > + cell = fdt_getprop(fdt, > > > > > > > > > > + nodeoffset > > > > > > > > > > , > > > > > > > > > > + "cffcore- > > > > > > > > > > file", > > > > > > > > > > + len); > > > > > > > > > > + else > > > > > > > > > > + cell = fdt_getprop(fdt, > > > > > > > > > > nodeoffset, > > > > > > > > > > "cff- > > > > > > > > > > file", len); > > > > > > > > > This should be a property of the FPGA , not the > > > > > > > > > system . > > > > > > > > > You > > > > > > > > > can > > > > > > > > > have > > > > > > > > > multiple FPGAs and then this would become a problem. > > > > > > > > > > > > > > > > > This setting is for the only one FPGA inside our > > > > > > > > SoCFPGA. > > > > > > > You just said it yourself, it is for the only FPGA in > > > > > > > your > > > > > > > SOCFPGA , > > > > > > > thus it is a property of the FPGA , not a chosen . > > > > > > > > > > > > > Okay, what i trying to tell is that there is no multiple > > > > > > FPGAs > > > > > > in > > > > > > our > > > > > > SOCFPGA. The filename is not any hardware properties, it is > > > > > > just a > > > > > > info > > > > > > to tell SPL and U-boot which file to look for programming > > > > > > FPGA. > > > > > What would happen if you attached an FPGA over ie. SPI or > > > > > PCIe ? > > > > > Then you have two FPGAs in the system and you need to > > > > > describe > > > > > them > > > > > in > > > > > the DT and your "chosen" approach breaks down. > > > > > > > > > We only need to decribe the internal FPGA of SoCFPGA in DT > > > This is incorrect. You describe the hardware in DT, if you have > > > multiple > > > FPGAs, then your approach breaks down. > > > > > Let me clarify, we don't have FPGA HW properties described in DT, > > only > > the RBF files are defined under chosen node. The files are defined > > only > > apply for FPGA inside SOCFPGA. > > Multiple external FPGA are configured through U-boot. > > > > > > > > > > > > > > > such as > > > > "chosen" to specify which file should SPL looking for getting > > > > SDRAM > > > > up > > > > with programming correct periph rbf into FPGA. When the SDRAM > > > > is up > > > > running, then only SPL can load U-boot and booting from there. > > > > The rest of external multiple FPGAs can be configured through > > > > U- > > > > boot, > > > > such as boot script, interactive in console or from PC host. > > > > Other > > > > than > > > > that, external FPGA can also be configured through Linux. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > According to chosen node document, chosen node doesn't > > > > > > represent a > > > > > > real > > > > > > HW, but serves as place for passing data. This is why our > > > > > > BSP > > > > > > tool > > > > > > put > > > > > > the filename info here, the file is named by user in our > > > > > > tool, > > > > > > and > > > > > > this > > > > > > info would be consumed by SPL to program FPGA. > > > > > > What do you think? > > > > > Your BSP tool is broken. > > > > > > > > > The BSP tool is used to describe internal FPGA in SOCFPGA. > > > > Other > > > > external FPGAs other than SOCFPGA itself, it can be programmed > > > > through > > > > U-boot. > > > The BSP tool is broken if it generates broken DT, do I have to > > > repeat > > > myself ? > > > > > BSP tool is only generate the RBF filename for FPGA inside SOCFPGA. > > Multiple external FPGA are configured through U-boot. > What happens if you have FPGA connector over SPI ? > I assume you are saying FPGA connected to EPCQ, and this is one of the external FPGA configuration, like PCIE. For any external FPGA configuration, FPGA itself/external HOST would get the FPGA data from storage such as EPCQ and configuring the FPGA without HPS/Bootloader intervine. SPL/U-boot would skip the FPGA configuration process when they see the mode is set to external FPGA configuration.
I know you want a DT to describe all FPGAs with FPGA node and their own data filename in every node. But, at this moment, all external FPGAs chip other than SOCFPGA itself are configured through U-boot(script & env variable), or external FPGA configuration method. How about i just create a FPGA node for SOCFPGA, with FPGA data filename within the node? > > > > > > > > [...] > _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot

