[U-Boot] [PATCH 03/19] arm: socfpga: Add driver for flash to program FPGA

Chee, Tien Fong
2017-09-06T05:06:25Z

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, tien.fong.chee at intel.com
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > > > > >
> > > > > > > > > > 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?
> >
> > >
> > > [...]
>