From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Wed, 30 Aug 2017 10:52:31 +0200 Subject: [U-Boot] [PATCH 03/19] arm: socfpga: Add driver for flash to program FPGA In-Reply-To: <1504080328.7727.37.camel@intel.com> References: <1504003561-6290-1-git-send-email-tien.fong.chee@intel.com> <1504003561-6290-4-git-send-email-tien.fong.chee@intel.com> <04c8e56d-860b-98c9-2893-2692c64197ee@denx.de> <1504080328.7727.37.camel@intel.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>> >>> This driver handles FPGA program operation from flash loading >>> RBF to memory and then to program FPGA. >>> >>> Signed-off-by: Tien Fong Chee >>> --- >>> .../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 >>> + >>> #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 >>> #include >>> #include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> #include >>> #include >>> >>> @@ -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 . > For external > multiple FPGAs programming, user is adviced to store the FPGA filename > in environment variable and programming FPGA with fpga loadfs command. > > Please note that, peripheral rbf and partition are required in SPL to > set up DDR before booting to U-boot. > >>> >>> + >>> + if (cell) >>> + cff_filename = cell; >>> + } >>> + >>> + return cff_filename; >>> +} >>> + >>> +const char *get_cff_devpart(const void *fdt, int *len) >>> +{ >>> + const char *cff_devpart = NULL; >>> + const char *cell; >>> + int nodeoffset; >>> + nodeoffset = fdt_subnode_offset(fdt, 0, "chosen"); >>> + >>> + cell = fdt_getprop(fdt, nodeoffset, "cff_devpart", >>> len); >> Indent ? What is this new undocumented DT node about ? >> > You can look the dtbinding doc on patch 12. Ugh, the patch should be in the series before this one. >>> >>> + if (cell) >>> + cff_devpart = cell; >>> + >>> + return cff_devpart; >>> +} >>> + >>> +void get_rbf_image_info(struct rbf_info *rbf, u16 *buffer) >>> +{ >>> + /* >>> + Magic ID starting at: >>> + -> 1st dword in periph.rbf >>> + -> 2nd dword in core.rbf >>> + */ >> Checkpatch should complain about incorrect multiline comment style >> here ... >> > Okay. I will fix that. Can you please run checkpatch before submitting your patches ? >>> >>> + u32 word_reading_max = 2; >>> + u32 i; >>> + >>> + for(i = 0; i < word_reading_max; i++) >>> + { >>> + if(RBF_UNENCRYPTED == *(buffer + i)) /* PERIPH RBF >>> */ >>> + rbf->security = unencrypted; >>> + else if (RBF_ENCRYPTED == *(buffer + i)) >>> + rbf->security = encrypted; >>> + else if (RBF_UNENCRYPTED == *(buffer + i + 1)) /* >>> CORE RBF */ >>> + rbf->security = >>> unencrypted; >>> + else if (RBF_ENCRYPTED == *(buffer + i + 1)) >>> + rbf->security = encrypted; >>> + else { >>> + rbf->security = invalid; >>> + continue; >>> + } >>> + >>> + /* PERIPH RBF */ >>> + if (ARRIA10RBF_PERIPH == *(buffer + i + 1)) { >>> + rbf->section = periph_section; >>> + break; >>> + } >>> + else if (ARRIA10RBF_CORE == *(buffer + i + 1)) { >>> + rbf->section = core_section; >>> + break; >>> + } /* CORE RBF */ >>> + else if (ARRIA10RBF_PERIPH == *(buffer + i + 2)) { >>> + rbf->section = periph_section; >>> + break; >>> + } >>> + else if (ARRIA10RBF_CORE == *(buffer + i + 2)) { >>> + rbf->section = core_section; >>> + break; >>> + } >>> + else { >> } else { ... coding style ... >> > Okay, i will fix that. This apply to else if? >>> >>> + rbf->section = unknown; >>> + break; >>> + } >>> + } >>> + >>> + return; >>> +} >>> + >>> +static int flash_read(struct flash_info *flashinfo, >>> + u32 size_read, >>> + u32 *buffer_ptr) >>> +{ >>> + size_t ret = EEXIST; >>> + loff_t actread = 0; >>> + >>> +#ifdef CONFIG_FS_FAT >>> + ret = fat_read_file(flashinfo->filename, >>> + buffer_ptr, flashinfo- >>>> flash_offset, >>> + size_read, &actread); >>> +#endif >> How can a generic FPGA driver depend on random FS functionality ? >> This is broken ... >> > random FS? There would having FAT FS for SDMMC, and UBI FS for QSPI and > NAND(implement later). Driver should not depend on specific FS. > May be i can replace #ifdef CONFIG_FS_FAT witht > the codes below, what do you think? > bootdev.boot_device = spl_boot_device(); > > if(BOOT_DEVICE_MMC1 == bootdev.boot_device) > { ... } Nor should it depend on random boot device. >>> >>> + if (ret || actread != size_read) { >>> + printf("Failed to read %s from flash %d ", >>> + flashinfo->filename, >>> + ret); >>> + printf("!= %d.\n", size_read); >>> + return -EPERM; >>> + } else >>> + ret = actread; >>> + >>> + return ret; >>> +} >>> + >>> +static int fs_flash_preinit(struct flash_info *flashinfo, >>> + u32 *buffer, u32 *buffer_sizebytes) >> Is this an FPGA driver or MTD driver ? >> > This is FPGA driver. Reading header of rbf, getting filesize and > adjusting the beginning offset of RBF. Why do you pass around the struct flashinfo then ? [...] >>> diff --git a/include/configs/socfpga_common.h >>> b/include/configs/socfpga_common.h >>> index 9be9e79..c15d244 100644 >>> --- a/include/configs/socfpga_common.h >>> +++ b/include/configs/socfpga_common.h >>> @@ -27,7 +27,11 @@ >>> */ >>> #define CONFIG_NR_DRAM_BANKS 1 >>> #define PHYS_SDRAM_1 0x0 >>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>> #define CONFIG_SYS_MALLOC_LEN (64 * 1024 * 1024) >>> +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10) >>> +#define CONFIG_SYS_MALLOC_LEN (128 * 1024 * 1024) >>> +#endif >> 128 MiB malloc area is nonsense, even those 64 MiB are iffy. Why >> would >> you ever need that in a bootloader ? >> > This is min require to malloc the buffer in SDRAM for core rbf. Less > than this value, something would going wrong, i'm not recall what issue > because i already tested this quite long time ago, problem related to > FAT and malloc. Isn't the loadfs supposed to read the data from the FS and program them into the FPGA without having to read the whole bitstream into the RAM ? >>> >>> #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM_1 >>> #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZE >>> #if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>> -- Best regards, Marek Vasut