| Message ID | 1506328815-23733-10-git-send-email-tien.fong.chee@intel.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | Add FPGA, SDRAM, SPL loads U-boot & booting to console | expand |
On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: > From: Tien Fong Chee <tien.fong.chee@intel.com> > > These drivers handle 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> Did you run checkpatch on this before submitting ? I presume no ... > --- > .../include/mach/fpga_manager_arria10.h | 27 ++ > drivers/fpga/socfpga_arria10.c | 391 ++++++++++++++++++++- > include/altera.h | 6 + > include/configs/socfpga_common.h | 4 + > 4 files changed, 425 insertions(+), 3 deletions(-) [...] > @@ -112,13 +122,14 @@ static int wait_for_nconfig_pin_and_nstatus_pin(void) > unsigned long mask = ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | > ALT_FPGAMGR_IMGCFG_STAT_F2S_NSTATUS_PIN_SET_MSK; > > - /* Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop until de-asserted, > - * timeout at 1000ms > + /* > + * Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop until > + * de-asserted, timeout at 1000ms > */ > return wait_for_bit(__func__, > &fpga_manager_base->imgcfg_stat, > mask, > - false, FPGA_TIMEOUT_MSEC, false); > + true, FPGA_TIMEOUT_MSEC, false); > } Seems more like a fix, split this out. > static int wait_for_f2s_nstatus_pin(unsigned long value) > @@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) > > /* Initialize the FPGA Manager */ > status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); > + > if (status) > return status; > > @@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) > > return fpgamgr_program_finish(); > } > + > +#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 = fdtdec_next_compatible(fdt, 0, > + COMPAT_ALTERA_SOCFPGA_FPGA0); > + > + if (nodeoffset >= 0) { > + if (core) > + cell = fdt_getprop(fdt, > + nodeoffset, > + "bitstream_core", > + len); > + else > + cell = fdt_getprop(fdt, nodeoffset, "bitstream_periph", > + len); > + > + 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 = fdtdec_next_compatible(fdt, 0, > + COMPAT_ALTERA_SOCFPGA_FPGA0); > + > + cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart", len); > + > + if (cell) > + cff_devpart = cell; > + > + return cff_devpart; > +} Take a look at splash*.c , I believe that can be reworked into generic firmware loader , which you could then use here. [...] > diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h > index 9897e11..eadce2d 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 You definitely don't need 128 MiB of malloc area. > #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM_1 > #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZE > #if defined(CONFIG_TARGET_SOCFPGA_GEN5) >
On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: > On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: > > > > From: Tien Fong Chee <tien.fong.chee@intel.com> > > > > These drivers handle 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> > Did you run checkpatch on this before submitting ? I presume no ... > Yeah, i run checkpatch for all patches. What's the issue here? > > > > --- > > .../include/mach/fpga_manager_arria10.h | 27 ++ > > drivers/fpga/socfpga_arria10.c | 391 > > ++++++++++++++++++++- > > include/altera.h | 6 + > > include/configs/socfpga_common.h | 4 + > > 4 files changed, 425 insertions(+), 3 deletions(-) > [...] > > > > > @@ -112,13 +122,14 @@ static int > > wait_for_nconfig_pin_and_nstatus_pin(void) > > unsigned long mask = > > ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | > > ALT_FPGAMGR_IMGCFG_STAT_F2S_NSTATU > > S_PIN_SET_MSK; > > > > - /* Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop > > until de-asserted, > > - * timeout at 1000ms > > + /* > > + * Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop > > until > > + * de-asserted, timeout at 1000ms > > */ > > return wait_for_bit(__func__, > > &fpga_manager_base->imgcfg_stat, > > mask, > > - false, FPGA_TIMEOUT_MSEC, false); > > + true, FPGA_TIMEOUT_MSEC, false); > > } > Seems more like a fix, split this out. > Okay. > > > > static int wait_for_f2s_nstatus_pin(unsigned long value) > > @@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const void > > *rbf_data, size_t rbf_size) > > > > /* Initialize the FPGA Manager */ > > status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); > > + > > if (status) > > return status; > > > > @@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const > > void *rbf_data, size_t rbf_size) > > > > return fpgamgr_program_finish(); > > } > > + > > +#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 = fdtdec_next_compatible(fdt, 0, > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > + > > + if (nodeoffset >= 0) { > > + if (core) > > + cell = fdt_getprop(fdt, > > + nodeoffset, > > + "bitstream_core", > > + len); > > + else > > + cell = fdt_getprop(fdt, nodeoffset, > > "bitstream_periph", > > + len); > > + > > + 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 = fdtdec_next_compatible(fdt, 0, > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > + > > + cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart", > > len); > > + > > + if (cell) > > + cff_devpart = cell; > > + > > + return cff_devpart; > > +} > Take a look at splash*.c , I believe that can be reworked into > generic > firmware loader , which you could then use here. > > [...] > > > > > diff --git a/include/configs/socfpga_common.h > > b/include/configs/socfpga_common.h > > index 9897e11..eadce2d 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 > You definitely don't need 128 MiB of malloc area. > Okay, i will try out with smaller size. > > > > #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM_1 > > #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZE > > #if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > >
On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: > On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: > > > > From: Tien Fong Chee <tien.fong.chee@intel.com> > > > > These drivers handle 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> > Did you run checkpatch on this before submitting ? I presume no ... > > > > > --- > > .../include/mach/fpga_manager_arria10.h | 27 ++ > > drivers/fpga/socfpga_arria10.c | 391 > > ++++++++++++++++++++- > > include/altera.h | 6 + > > include/configs/socfpga_common.h | 4 + > > 4 files changed, 425 insertions(+), 3 deletions(-) > [...] > > > > > @@ -112,13 +122,14 @@ static int > > wait_for_nconfig_pin_and_nstatus_pin(void) > > unsigned long mask = > > ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | > > ALT_FPGAMGR_IMGCFG_STAT_F2S_NSTATU > > S_PIN_SET_MSK; > > > > - /* Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop > > until de-asserted, > > - * timeout at 1000ms > > + /* > > + * Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop > > until > > + * de-asserted, timeout at 1000ms > > */ > > return wait_for_bit(__func__, > > &fpga_manager_base->imgcfg_stat, > > mask, > > - false, FPGA_TIMEOUT_MSEC, false); > > + true, FPGA_TIMEOUT_MSEC, false); > > } > Seems more like a fix, split this out. > > > > > static int wait_for_f2s_nstatus_pin(unsigned long value) > > @@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const void > > *rbf_data, size_t rbf_size) > > > > /* Initialize the FPGA Manager */ > > status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); > > + > > if (status) > > return status; > > > > @@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const > > void *rbf_data, size_t rbf_size) > > > > return fpgamgr_program_finish(); > > } > > + > > +#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 = fdtdec_next_compatible(fdt, 0, > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > + > > + if (nodeoffset >= 0) { > > + if (core) > > + cell = fdt_getprop(fdt, > > + nodeoffset, > > + "bitstream_core", > > + len); > > + else > > + cell = fdt_getprop(fdt, nodeoffset, > > "bitstream_periph", > > + len); > > + > > + 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 = fdtdec_next_compatible(fdt, 0, > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > + > > + cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart", > > len); > > + > > + if (cell) > > + cff_devpart = cell; > > + > > + return cff_devpart; > > +} > Take a look at splash*.c , I believe that can be reworked into > generic > firmware loader , which you could then use here. > the devpart is hard coded in splash*.c. The function here is getting devpart info from DTS. So, is there any similar function in splash*.c? May be you can share more about your idea. > [...] > > > > > diff --git a/include/configs/socfpga_common.h > > b/include/configs/socfpga_common.h > > index 9897e11..eadce2d 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 > You definitely don't need 128 MiB of malloc area. > > > > > #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM_1 > > #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZE > > #if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > >
On 09/26/2017 10:30 AM, Chee, Tien Fong wrote: > On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: >> On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: >>> >>> From: Tien Fong Chee <tien.fong.chee@intel.com> >>> >>> These drivers handle 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> >> Did you run checkpatch on this before submitting ? I presume no ... >> > Yeah, i run checkpatch for all patches. What's the issue here? It should definitely indicate problem with ie. yoda-notation +if (0 == flashinfo->remaining) { and indent ... >>> >>> --- >>> .../include/mach/fpga_manager_arria10.h | 27 ++ >>> drivers/fpga/socfpga_arria10.c | 391 >>> ++++++++++++++++++++- >>> include/altera.h | 6 + >>> include/configs/socfpga_common.h | 4 + >>> 4 files changed, 425 insertions(+), 3 deletions(-) >> [...] >> >>> >>> @@ -112,13 +122,14 @@ static int >>> wait_for_nconfig_pin_and_nstatus_pin(void) >>> unsigned long mask = >>> ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | >>> ALT_FPGAMGR_IMGCFG_STAT_F2S_NSTATU >>> S_PIN_SET_MSK; >>> >>> - /* Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop >>> until de-asserted, >>> - * timeout at 1000ms >>> + /* >>> + * Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop >>> until >>> + * de-asserted, timeout at 1000ms >>> */ >>> return wait_for_bit(__func__, >>> &fpga_manager_base->imgcfg_stat, >>> mask, >>> - false, FPGA_TIMEOUT_MSEC, false); >>> + true, FPGA_TIMEOUT_MSEC, false); >>> } >> Seems more like a fix, split this out. >> > Okay. >>> >>> static int wait_for_f2s_nstatus_pin(unsigned long value) >>> @@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const void >>> *rbf_data, size_t rbf_size) >>> >>> /* Initialize the FPGA Manager */ >>> status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); >>> + >>> if (status) >>> return status; >>> >>> @@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const >>> void *rbf_data, size_t rbf_size) >>> >>> return fpgamgr_program_finish(); >>> } >>> + >>> +#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 = fdtdec_next_compatible(fdt, 0, >>> + COMPAT_ALTERA_SOCFPGA_FPGA0); >>> + >>> + if (nodeoffset >= 0) { >>> + if (core) >>> + cell = fdt_getprop(fdt, >>> + nodeoffset, >>> + "bitstream_core", >>> + len); >>> + else >>> + cell = fdt_getprop(fdt, nodeoffset, >>> "bitstream_periph", >>> + len); >>> + >>> + 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 = fdtdec_next_compatible(fdt, 0, >>> + COMPAT_ALTERA_SOCFPGA_FPGA0); >>> + >>> + cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart", >>> len); >>> + >>> + if (cell) >>> + cff_devpart = cell; >>> + >>> + return cff_devpart; >>> +} >> Take a look at splash*.c , I believe that can be reworked into >> generic >> firmware loader , which you could then use here. This is important here, I don't want yet another ad-hoc loader ... >> [...] >> >>> >>> diff --git a/include/configs/socfpga_common.h >>> b/include/configs/socfpga_common.h >>> index 9897e11..eadce2d 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 >> You definitely don't need 128 MiB of malloc area. >> > Okay, i will try out with smaller size. Why do you need such massive area ? It's not a matter of "try out", you should know why this change was needed for your use-case. >>> >>> #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM_1 >>> #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZE >>> #if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>
On 09/26/2017 11:52 AM, Chee, Tien Fong wrote: > On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: >> On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: >>> >>> From: Tien Fong Chee <tien.fong.chee@intel.com> >>> >>> These drivers handle 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> [...] >>> +const char *get_cff_devpart(const void *fdt, int *len) >>> +{ >>> + const char *cff_devpart = NULL; >>> + const char *cell; >>> + int nodeoffset; >>> + nodeoffset = fdtdec_next_compatible(fdt, 0, >>> + COMPAT_ALTERA_SOCFPGA_FPGA0); >>> + >>> + cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart", >>> len); >>> + >>> + if (cell) >>> + cff_devpart = cell; >>> + >>> + return cff_devpart; >>> +} >> Take a look at splash*.c , I believe that can be reworked into >> generic >> firmware loader , which you could then use here. >> > the devpart is hard coded in splash*.c. The function here is getting > devpart info from DTS. So, is there any similar function in splash*.c? > May be you can share more about your idea. The generic loader could use some work of course ... [...]
On Sel, 2017-09-26 at 12:32 +0200, Marek Vasut wrote: > On 09/26/2017 10:30 AM, Chee, Tien Fong wrote: > > > > On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: > > > > > > On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com> > > > > > > > > These drivers handle 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> > > > Did you run checkpatch on this before submitting ? I presume no > > > ... > > > > > Yeah, i run checkpatch for all patches. What's the issue here? > It should definitely indicate problem with ie. yoda-notation > +if (0 == flashinfo->remaining) { > and indent ... > No complaint from checkpath. I know someone saying bad for readbility, but yoda-notation at this simple implementation doesn't impact the readbility, and having benefit to leverage detection of compiler on missing "=". Overall, this can help to improve coding quality. I can remove it if this doesn't favored in U-boot. > > > > > > > > > > > > > > > > > --- > > > > .../include/mach/fpga_manager_arria10.h | 27 ++ > > > > drivers/fpga/socfpga_arria10.c | 391 > > > > ++++++++++++++++++++- > > > > include/altera.h | 6 + > > > > include/configs/socfpga_common.h | 4 + > > > > 4 files changed, 425 insertions(+), 3 deletions(-) > > > [...] > > > > > > > > > > > > > > > @@ -112,13 +122,14 @@ static int > > > > wait_for_nconfig_pin_and_nstatus_pin(void) > > > > unsigned long mask = > > > > ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | > > > > ALT_FPGAMGR_IMGCFG_STAT_F2S_NS > > > > TATU > > > > S_PIN_SET_MSK; > > > > > > > > - /* Poll until f2s_nconfig_pin and f2s_nstatus_pin; > > > > loop > > > > until de-asserted, > > > > - * timeout at 1000ms > > > > + /* > > > > + * Poll until f2s_nconfig_pin and f2s_nstatus_pin; > > > > loop > > > > until > > > > + * de-asserted, timeout at 1000ms > > > > */ > > > > return wait_for_bit(__func__, > > > > &fpga_manager_base->imgcfg_stat, > > > > mask, > > > > - false, FPGA_TIMEOUT_MSEC, false); > > > > + true, FPGA_TIMEOUT_MSEC, false); > > > > } > > > Seems more like a fix, split this out. > > > > > Okay. > > > > > > > > > > > > > > > static int wait_for_f2s_nstatus_pin(unsigned long value) > > > > @@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const > > > > void > > > > *rbf_data, size_t rbf_size) > > > > > > > > /* Initialize the FPGA Manager */ > > > > status = fpgamgr_program_init((u32 *)rbf_data, > > > > rbf_size); > > > > + > > > > if (status) > > > > return status; > > > > > > > > @@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const > > > > void *rbf_data, size_t rbf_size) > > > > > > > > return fpgamgr_program_finish(); > > > > } > > > > + > > > > +#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 = fdtdec_next_compatible(fdt, 0, > > > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > > > + > > > > + if (nodeoffset >= 0) { > > > > + if (core) > > > > + cell = fdt_getprop(fdt, > > > > + nodeoffset, > > > > + "bitstream_core", > > > > + len); > > > > + else > > > > + cell = fdt_getprop(fdt, nodeoffset, > > > > "bitstream_periph", > > > > + len); > > > > + > > > > + 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 = fdtdec_next_compatible(fdt, 0, > > > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > > > + > > > > + cell = fdt_getprop(fdt, nodeoffset, > > > > "bitstream_devpart", > > > > len); > > > > + > > > > + if (cell) > > > > + cff_devpart = cell; > > > > + > > > > + return cff_devpart; > > > > +} > > > Take a look at splash*.c , I believe that can be reworked into > > > generic > > > firmware loader , which you could then use here. > This is important here, I don't want yet another ad-hoc loader ... > Disucss in another email reply separately. I need time to think. > > > > > > > > [...] > > > > > > > > > > > > > > > diff --git a/include/configs/socfpga_common.h > > > > b/include/configs/socfpga_common.h > > > > index 9897e11..eadce2d 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 > > > You definitely don't need 128 MiB of malloc area. > > > > > Okay, i will try out with smaller size. > Why do you need such massive area ? It's not a matter of "try out", > you > should know why this change was needed for your use-case. > I forgot what reason i put this value. I need to find out. > > > > > > > > > > > > > > > > > #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM_1 > > > > #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZ > > > > E > > > > #if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > >
On 09/27/2017 08:05 AM, Chee, Tien Fong wrote: > On Sel, 2017-09-26 at 12:32 +0200, Marek Vasut wrote: >> On 09/26/2017 10:30 AM, Chee, Tien Fong wrote: >>> >>> On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: >>>> >>>> On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: >>>>> >>>>> >>>>> From: Tien Fong Chee <tien.fong.chee@intel.com> >>>>> >>>>> These drivers handle 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> >>>> Did you run checkpatch on this before submitting ? I presume no >>>> ... >>>> >>> Yeah, i run checkpatch for all patches. What's the issue here? >> It should definitely indicate problem with ie. yoda-notation >> +if (0 == flashinfo->remaining) { >> and indent ... >> > No complaint from checkpath. I know someone saying bad for readbility, > but yoda-notation at this simple implementation doesn't impact the > readbility, and having benefit to leverage detection of compiler on > missing "=". Overall, this can help to improve coding quality. I can > remove it if this doesn't favored in U-boot. It is not welcome and modern gcc warns you about such things . >>> >>>> >>>>> >>>>> >>>>> --- >>>>> .../include/mach/fpga_manager_arria10.h | 27 ++ >>>>> drivers/fpga/socfpga_arria10.c | 391 >>>>> ++++++++++++++++++++- >>>>> include/altera.h | 6 + >>>>> include/configs/socfpga_common.h | 4 + >>>>> 4 files changed, 425 insertions(+), 3 deletions(-) >>>> [...] >>>> >>>>> >>>>> >>>>> @@ -112,13 +122,14 @@ static int >>>>> wait_for_nconfig_pin_and_nstatus_pin(void) >>>>> unsigned long mask = >>>>> ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | >>>>> ALT_FPGAMGR_IMGCFG_STAT_F2S_NS >>>>> TATU >>>>> S_PIN_SET_MSK; >>>>> >>>>> - /* Poll until f2s_nconfig_pin and f2s_nstatus_pin; >>>>> loop >>>>> until de-asserted, >>>>> - * timeout at 1000ms >>>>> + /* >>>>> + * Poll until f2s_nconfig_pin and f2s_nstatus_pin; >>>>> loop >>>>> until >>>>> + * de-asserted, timeout at 1000ms >>>>> */ >>>>> return wait_for_bit(__func__, >>>>> &fpga_manager_base->imgcfg_stat, >>>>> mask, >>>>> - false, FPGA_TIMEOUT_MSEC, false); >>>>> + true, FPGA_TIMEOUT_MSEC, false); >>>>> } >>>> Seems more like a fix, split this out. >>>> >>> Okay. >>>> >>>>> >>>>> >>>>> static int wait_for_f2s_nstatus_pin(unsigned long value) >>>>> @@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const >>>>> void >>>>> *rbf_data, size_t rbf_size) >>>>> >>>>> /* Initialize the FPGA Manager */ >>>>> status = fpgamgr_program_init((u32 *)rbf_data, >>>>> rbf_size); >>>>> + >>>>> if (status) >>>>> return status; >>>>> >>>>> @@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const >>>>> void *rbf_data, size_t rbf_size) >>>>> >>>>> return fpgamgr_program_finish(); >>>>> } >>>>> + >>>>> +#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 = fdtdec_next_compatible(fdt, 0, >>>>> + COMPAT_ALTERA_SOCFPGA_FPGA0); >>>>> + >>>>> + if (nodeoffset >= 0) { >>>>> + if (core) >>>>> + cell = fdt_getprop(fdt, >>>>> + nodeoffset, >>>>> + "bitstream_core", >>>>> + len); >>>>> + else >>>>> + cell = fdt_getprop(fdt, nodeoffset, >>>>> "bitstream_periph", >>>>> + len); >>>>> + >>>>> + 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 = fdtdec_next_compatible(fdt, 0, >>>>> + COMPAT_ALTERA_SOCFPGA_FPGA0); >>>>> + >>>>> + cell = fdt_getprop(fdt, nodeoffset, >>>>> "bitstream_devpart", >>>>> len); >>>>> + >>>>> + if (cell) >>>>> + cff_devpart = cell; >>>>> + >>>>> + return cff_devpart; >>>>> +} >>>> Take a look at splash*.c , I believe that can be reworked into >>>> generic >>>> firmware loader , which you could then use here. >> This is important here, I don't want yet another ad-hoc loader ... >> > Disucss in another email reply separately. I need time to think. >>> >>>> >>>> [...] >>>> >>>>> >>>>> >>>>> diff --git a/include/configs/socfpga_common.h >>>>> b/include/configs/socfpga_common.h >>>>> index 9897e11..eadce2d 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 >>>> You definitely don't need 128 MiB of malloc area. >>>> >>> Okay, i will try out with smaller size. >> Why do you need such massive area ? It's not a matter of "try out", >> you >> should know why this change was needed for your use-case. >> > I forgot what reason i put this value. I need to find out. If this was properly documented in the commit message, you would not have to ... >>> >>>> >>>>> >>>>> >>>>> #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM_1 >>>>> #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZ >>>>> E >>>>> #if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>
On Sel, 2017-09-26 at 12:39 +0200, Marek Vasut wrote: > On 09/26/2017 11:52 AM, Chee, Tien Fong wrote: > > > > On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: > > > > > > On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com> > > > > > > > > These drivers handle 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> > [...] > > > > > > > > > > > > > > +const char *get_cff_devpart(const void *fdt, int *len) > > > > +{ > > > > + const char *cff_devpart = NULL; > > > > + const char *cell; > > > > + int nodeoffset; > > > > + nodeoffset = fdtdec_next_compatible(fdt, 0, > > > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > > > + > > > > + cell = fdt_getprop(fdt, nodeoffset, > > > > "bitstream_devpart", > > > > len); > > > > + > > > > + if (cell) > > > > + cff_devpart = cell; > > > > + > > > > + return cff_devpart; > > > > +} > > > Take a look at splash*.c , I believe that can be reworked into > > > generic > > > firmware loader , which you could then use here. > > > > > the devpart is hard coded in splash*.c. The function here is > > getting > > devpart info from DTS. So, is there any similar function in > > splash*.c? > > May be you can share more about your idea. > The generic loader could use some work of course ... > Sorry, i am still confusing. Allow me to ask you more: 1. Is the generic firmware loader already exists in splash*.c? 2. Are you talking about get_cff_devpart or whole fpga laodfs? 3. You want me integrate get_cff_devpart function into splash*.c? 4. Are you means to hard code the devpart instead providing dynamic devpart described in DTS? Current implementation are located in spl_board_init func (arcg/arm/mach-socfpga/spl.c). Based on boot device such as mmc, nand and QSPI, then reading some info from DTS, setting dev and partition with generic fs functions, and reading with generic fs function before programming RBF into FPGA. All these are in patch 19. Thanks. > [...] >
On 09/27/2017 11:13 AM, Chee, Tien Fong wrote: > On Sel, 2017-09-26 at 12:39 +0200, Marek Vasut wrote: >> On 09/26/2017 11:52 AM, Chee, Tien Fong wrote: >>> >>> On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: >>>> >>>> On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: >>>>> >>>>> >>>>> From: Tien Fong Chee <tien.fong.chee@intel.com> >>>>> >>>>> These drivers handle 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> >> [...] >> >>> >>>> >>>>> >>>>> +const char *get_cff_devpart(const void *fdt, int *len) >>>>> +{ >>>>> + const char *cff_devpart = NULL; >>>>> + const char *cell; >>>>> + int nodeoffset; >>>>> + nodeoffset = fdtdec_next_compatible(fdt, 0, >>>>> + COMPAT_ALTERA_SOCFPGA_FPGA0); >>>>> + >>>>> + cell = fdt_getprop(fdt, nodeoffset, >>>>> "bitstream_devpart", >>>>> len); >>>>> + >>>>> + if (cell) >>>>> + cff_devpart = cell; >>>>> + >>>>> + return cff_devpart; >>>>> +} >>>> Take a look at splash*.c , I believe that can be reworked into >>>> generic >>>> firmware loader , which you could then use here. >>>> >>> the devpart is hard coded in splash*.c. The function here is >>> getting >>> devpart info from DTS. So, is there any similar function in >>> splash*.c? >>> May be you can share more about your idea. >> The generic loader could use some work of course ... >> > Sorry, i am still confusing. Allow me to ask you more: > 1. Is the generic firmware loader already exists in splash*.c? No > 2. Are you talking about get_cff_devpart or whole fpga laodfs? > 3. You want me integrate get_cff_devpart function into splash*.c? > 4. Are you means to hard code the devpart instead providing dynamic > devpart described in DTS? I am talking about factoring out generic firmware loader from splash*c , since it already contains most of the parts for such a thing. > Current implementation are located in spl_board_init func > (arcg/arm/mach-socfpga/spl.c). Based on boot device such as mmc, nand > and QSPI, then reading some info from DTS, setting dev and partition > with generic fs functions, and reading with generic fs function before > programming RBF into FPGA. All these are in patch 19. That's what splash*c also does, so adding separate parallel implementation of the same functionality is a no-no. > Thanks. >> [...]
On Rab, 2017-09-27 at 10:30 +0200, Marek Vasut wrote: > On 09/27/2017 08:05 AM, Chee, Tien Fong wrote: > > > > On Sel, 2017-09-26 at 12:32 +0200, Marek Vasut wrote: > > > > > > On 09/26/2017 10:30 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: > > > > > > > > > > > > > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com> > > > > > > > > > > > > These drivers handle 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> > > > > > Did you run checkpatch on this before submitting ? I presume > > > > > no > > > > > ... > > > > > > > > > Yeah, i run checkpatch for all patches. What's the issue here? > > > It should definitely indicate problem with ie. yoda-notation > > > +if (0 == flashinfo->remaining) { > > > and indent ... > > > > > No complaint from checkpath. I know someone saying bad for > > readbility, > > but yoda-notation at this simple implementation doesn't impact the > > readbility, and having benefit to leverage detection of compiler on > > missing "=". Overall, this can help to improve coding quality. I > > can > > remove it if this doesn't favored in U-boot. > It is not welcome and modern gcc warns you about such things . > Okay. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > .../include/mach/fpga_manager_arria10.h | 27 > > > > > > ++ > > > > > > drivers/fpga/socfpga_arria10.c | 391 > > > > > > ++++++++++++++++++++- > > > > > > include/altera.h | 6 + > > > > > > include/configs/socfpga_common.h | 4 + > > > > > > 4 files changed, 425 insertions(+), 3 deletions(-) > > > > > [...] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > @@ -112,13 +122,14 @@ static int > > > > > > wait_for_nconfig_pin_and_nstatus_pin(void) > > > > > > unsigned long mask = > > > > > > ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | > > > > > > ALT_FPGAMGR_IMGCFG_STAT_F2 > > > > > > S_NS > > > > > > TATU > > > > > > S_PIN_SET_MSK; > > > > > > > > > > > > - /* Poll until f2s_nconfig_pin and f2s_nstatus_pin; > > > > > > loop > > > > > > until de-asserted, > > > > > > - * timeout at 1000ms > > > > > > + /* > > > > > > + * Poll until f2s_nconfig_pin and f2s_nstatus_pin; > > > > > > loop > > > > > > until > > > > > > + * de-asserted, timeout at 1000ms > > > > > > */ > > > > > > return wait_for_bit(__func__, > > > > > > &fpga_manager_base- > > > > > > >imgcfg_stat, > > > > > > mask, > > > > > > - false, FPGA_TIMEOUT_MSEC, > > > > > > false); > > > > > > + true, FPGA_TIMEOUT_MSEC, > > > > > > false); > > > > > > } > > > > > Seems more like a fix, split this out. > > > > > > > > > Okay. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > static int wait_for_f2s_nstatus_pin(unsigned long value) > > > > > > @@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, > > > > > > const > > > > > > void > > > > > > *rbf_data, size_t rbf_size) > > > > > > > > > > > > /* Initialize the FPGA Manager */ > > > > > > status = fpgamgr_program_init((u32 *)rbf_data, > > > > > > rbf_size); > > > > > > + > > > > > > if (status) > > > > > > return status; > > > > > > > > > > > > @@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, > > > > > > const > > > > > > void *rbf_data, size_t rbf_size) > > > > > > > > > > > > return fpgamgr_program_finish(); > > > > > > } > > > > > > + > > > > > > +#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 = fdtdec_next_compatible(fdt, 0, > > > > > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > > > > > + > > > > > > + if (nodeoffset >= 0) { > > > > > > + if (core) > > > > > > + cell = fdt_getprop(fdt, > > > > > > + nodeoffset, > > > > > > + "bitstream_core", > > > > > > + len); > > > > > > + else > > > > > > + cell = fdt_getprop(fdt, > > > > > > nodeoffset, > > > > > > "bitstream_periph", > > > > > > + len); > > > > > > + > > > > > > + 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 = fdtdec_next_compatible(fdt, 0, > > > > > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > > > > > + > > > > > > + cell = fdt_getprop(fdt, nodeoffset, > > > > > > "bitstream_devpart", > > > > > > len); > > > > > > + > > > > > > + if (cell) > > > > > > + cff_devpart = cell; > > > > > > + > > > > > > + return cff_devpart; > > > > > > +} > > > > > Take a look at splash*.c , I believe that can be reworked > > > > > into > > > > > generic > > > > > firmware loader , which you could then use here. > > > This is important here, I don't want yet another ad-hoc loader > > > ... > > > > > Disucss in another email reply separately. I need time to think. > > > > > > > > > > > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/include/configs/socfpga_common.h > > > > > > b/include/configs/socfpga_common.h > > > > > > index 9897e11..eadce2d 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 > > > > > You definitely don't need 128 MiB of malloc area. > > > > > > > > > Okay, i will try out with smaller size. > > > Why do you need such massive area ? It's not a matter of "try > > > out", > > > you > > > should know why this change was needed for your use-case. > > > > > I forgot what reason i put this value. I need to find out. > If this was properly documented in the commit message, you would not > have to ... > Okay. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM_1 > > > > > > #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1 > > > > > > _SIZ > > > > > > E > > > > > > #if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > >
On Rab, 2017-09-27 at 11:23 +0200, Marek Vasut wrote: > On 09/27/2017 11:13 AM, Chee, Tien Fong wrote: > > > > On Sel, 2017-09-26 at 12:39 +0200, Marek Vasut wrote: > > > > > > On 09/26/2017 11:52 AM, Chee, Tien Fong wrote: > > > > > > > > > > > > On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: > > > > > > > > > > > > > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com> > > > > > > > > > > > > These drivers handle 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> > > > [...] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +const char *get_cff_devpart(const void *fdt, int *len) > > > > > > +{ > > > > > > + const char *cff_devpart = NULL; > > > > > > + const char *cell; > > > > > > + int nodeoffset; > > > > > > + nodeoffset = fdtdec_next_compatible(fdt, 0, > > > > > > + COMPAT_ALTERA_SOCFPGA_FPGA0); > > > > > > + > > > > > > + cell = fdt_getprop(fdt, nodeoffset, > > > > > > "bitstream_devpart", > > > > > > len); > > > > > > + > > > > > > + if (cell) > > > > > > + cff_devpart = cell; > > > > > > + > > > > > > + return cff_devpart; > > > > > > +} > > > > > Take a look at splash*.c , I believe that can be reworked > > > > > into > > > > > generic > > > > > firmware loader , which you could then use here. > > > > > > > > > the devpart is hard coded in splash*.c. The function here is > > > > getting > > > > devpart info from DTS. So, is there any similar function in > > > > splash*.c? > > > > May be you can share more about your idea. > > > The generic loader could use some work of course ... > > > > > Sorry, i am still confusing. Allow me to ask you more: > > 1. Is the generic firmware loader already exists in splash*.c? > No > > > > > 2. Are you talking about get_cff_devpart or whole fpga laodfs? > > 3. You want me integrate get_cff_devpart function into splash*.c? > > 4. Are you means to hard code the devpart instead providing dynamic > > devpart described in DTS? > I am talking about factoring out generic firmware loader from > splash*c , > since it already contains most of the parts for such a thing. > > > > > Current implementation are located in spl_board_init func > > (arcg/arm/mach-socfpga/spl.c). Based on boot device such as mmc, > > nand > > and QSPI, then reading some info from DTS, setting dev and > > partition > > with generic fs functions, and reading with generic fs function > > before > > programming RBF into FPGA. All these are in patch 19. > That's what splash*c also does, so adding separate parallel > implementation of the same functionality is a no-no. > After reading through splash*c, i found there are two functions bear a close similarity to. 1st function --> In /common/splash.c : static struct splash_location default_splash_locations[] = { { .name = "sf", .storage = SPLASH_STORAGE_SF, .flags = SPLASH_STORAGE_RAW, .offset = 0x0, }, { .name = "mmc_fs", .storage = SPLASH_STORAGE_MMC, .flags = SPLASH_STORAGE_FS, .devpart = "0:1", }, { .name = "usb_fs", .storage = SPLASH_STORAGE_USB, .flags = SPLASH_STORAGE_FS, .devpart = "0:1", }, { .name = "sata_fs", .storage = SPLASH_STORAGE_SATA, .flags = SPLASH_STORAGE_FS, .devpart = "0:1", }, }; In my /arch/arm/mach-socfpga/spl.c (spl_board_init(void)) bootdev.boot_device = spl_boot_device(); if (BOOT_DEVICE_MMC1 == bootdev.boot_device) { struct mmc *mmc = NULL; int err = 0; spl_mmc_find_device(&mmc, bootdev.boot_device); err = mmc_init(mmc); if (err) { #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: mmc init failed with error: %d\n", err); #endif } fpga_fsinfo.dev_part = (char *)get_cff_devpart(gd- >fdt_blob, &len); fpga_fsinfo.filename = (char *)get_cff_filename(gd- >fdt_blob, &len, PERIPH_ RBF); fpga_fsinfo.interface = "mmc"; fpga_fsinfo.fstype = FS_TYPE_FAT; } else { printf("Invalid boot device!\n"); return; } /* Program peripheral RBF */ if (fpga_fsinfo.filename && fpga_fsinfo.dev_part && (len > 0)) rval = fpga_fsload(0, buffer, BSIZE, &fpga_fsinfo); In /common/splash.c, dev_Part and flash type everything are hard coded in struct splash_location. In my spl.c, flash type are determined on run time and dev_part are retrived from DTS, and then assigned to struct fpga_fsinfo. Please note that this is in SPL, mmc need to be initialized 1st before loading raw file into memory. In SPL, raw file are coppied to OCRAM chunk by chunk, but In u-boot it would normally done in one big chunk to DRAM. This would be handled by fpga loadfs. So, you want me hard code everthing like in splash.c? 2nd function --> In /common/splash_source.c static int splash_select_fs_dev(struct splash_location *location) { int res; switch (location->storage) { case SPLASH_STORAGE_MMC: res = fs_set_blk_dev("mmc", location->devpart, FS_TYPE_ANY); break; case SPLASH_STORAGE_USB: res = fs_set_blk_dev("usb", location->devpart, FS_TYPE_ANY); break; case SPLASH_STORAGE_SATA: res = fs_set_blk_dev("sata", location->devpart, FS_TYPE_ANY); break; case SPLASH_STORAGE_NAND: if (location->ubivol != NULL) res = fs_set_blk_dev("ubi", NULL, FS_TYPE_UBIFS); else res = -ENODEV; break; default: printf("Error: unsupported location storage.\n"); return -ENODEV; } if (res) printf("Error: could not access storage.\n"); return res; } In my /drivers/fpga/socfpga_arria10.c static int flash_read(struct flash_info *flashinfo, u32 size_read, u32 *buffer_ptr) { size_t ret = EEXIST; loff_t actread = 0; if (fs_set_blk_dev(flashinfo->interface, flashinfo->dev_part, flashinfo->fstype)) return FPGA_FAIL; ret = fs_read(flashinfo->filename, (u32) buffer_ptr, flashinfo->flash_offset, size_read, &actread); 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; } Some attributes like flash type is determined on run time and and dev_part is retrieved from DTS, so every infos driver need to know are assinged into struct flashinfo and passed to fs_set_blk_dev as arguments. I found that function in splash_source.c some like flash type are getting from env variable, but we are still in SPL phase, those env variable is not set up yet. So, i think that is very ineffcient to factor out them as common. If you want me create a generic firmware loader which is generic enough loading content for all components like flashes, FPGA, splash ....etc, i don't think that is effient enough, as fpga loadfs has different handling in both SPL and U-boot like copying raw into memory. It would be good you can direct point me out which functions have similirity and how to factor them out as common. Thanks a lot. > > > > Thanks. > > > > > > [...] >
On 09/28/2017 05:14 PM, Chee, Tien Fong wrote: > On Rab, 2017-09-27 at 11:23 +0200, Marek Vasut wrote: >> On 09/27/2017 11:13 AM, Chee, Tien Fong wrote: >>> >>> On Sel, 2017-09-26 at 12:39 +0200, Marek Vasut wrote: >>>> >>>> On 09/26/2017 11:52 AM, Chee, Tien Fong wrote: >>>>> >>>>> >>>>> On Isn, 2017-09-25 at 11:14 +0200, Marek Vasut wrote: >>>>>> >>>>>> >>>>>> On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> From: Tien Fong Chee <tien.fong.chee@intel.com> >>>>>>> >>>>>>> These drivers handle 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> >>>> [...] >>>> >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> +const char *get_cff_devpart(const void *fdt, int *len) >>>>>>> +{ >>>>>>> + const char *cff_devpart = NULL; >>>>>>> + const char *cell; >>>>>>> + int nodeoffset; >>>>>>> + nodeoffset = fdtdec_next_compatible(fdt, 0, >>>>>>> + COMPAT_ALTERA_SOCFPGA_FPGA0); >>>>>>> + >>>>>>> + cell = fdt_getprop(fdt, nodeoffset, >>>>>>> "bitstream_devpart", >>>>>>> len); >>>>>>> + >>>>>>> + if (cell) >>>>>>> + cff_devpart = cell; >>>>>>> + >>>>>>> + return cff_devpart; >>>>>>> +} >>>>>> Take a look at splash*.c , I believe that can be reworked >>>>>> into >>>>>> generic >>>>>> firmware loader , which you could then use here. >>>>>> >>>>> the devpart is hard coded in splash*.c. The function here is >>>>> getting >>>>> devpart info from DTS. So, is there any similar function in >>>>> splash*.c? >>>>> May be you can share more about your idea. >>>> The generic loader could use some work of course ... >>>> >>> Sorry, i am still confusing. Allow me to ask you more: >>> 1. Is the generic firmware loader already exists in splash*.c? >> No >> >>> >>> 2. Are you talking about get_cff_devpart or whole fpga laodfs? >>> 3. You want me integrate get_cff_devpart function into splash*.c? >>> 4. Are you means to hard code the devpart instead providing dynamic >>> devpart described in DTS? >> I am talking about factoring out generic firmware loader from >> splash*c , >> since it already contains most of the parts for such a thing. >> >>> >>> Current implementation are located in spl_board_init func >>> (arcg/arm/mach-socfpga/spl.c). Based on boot device such as mmc, >>> nand >>> and QSPI, then reading some info from DTS, setting dev and >>> partition >>> with generic fs functions, and reading with generic fs function >>> before >>> programming RBF into FPGA. All these are in patch 19. >> That's what splash*c also does, so adding separate parallel >> implementation of the same functionality is a no-no. >> > After reading through splash*c, i found there are two functions bear a > close similarity to. > 1st function --> > In /common/splash.c : > static struct splash_location default_splash_locations[] = { > { > .name = "sf", > .storage = SPLASH_STORAGE_SF, > .flags = SPLASH_STORAGE_RAW, > .offset = 0x0, > }, > { > .name = "mmc_fs", > .storage = SPLASH_STORAGE_MMC, > .flags = SPLASH_STORAGE_FS, > .devpart = "0:1", > }, > { > .name = "usb_fs", > .storage = SPLASH_STORAGE_USB, > .flags = SPLASH_STORAGE_FS, > .devpart = "0:1", > }, > { > .name = "sata_fs", > .storage = SPLASH_STORAGE_SATA, > .flags = SPLASH_STORAGE_FS, > .devpart = "0:1", > }, > }; > > In my /arch/arm/mach-socfpga/spl.c (spl_board_init(void)) > bootdev.boot_device = spl_boot_device(); > > if (BOOT_DEVICE_MMC1 == bootdev.boot_device) { > struct mmc *mmc = NULL; > int err = 0; > > spl_mmc_find_device(&mmc, bootdev.boot_device); > > err = mmc_init(mmc); > > if (err) { > #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT > printf("spl: mmc init failed with error: %d\n", > err); > #endif > } > > fpga_fsinfo.dev_part = (char *)get_cff_devpart(gd- >> fdt_blob, > &len); > > fpga_fsinfo.filename = (char *)get_cff_filename(gd- >> fdt_blob, > &len, > PERIPH_ > RBF); > > fpga_fsinfo.interface = "mmc"; > > fpga_fsinfo.fstype = FS_TYPE_FAT; > } else { > printf("Invalid boot device!\n"); > return; > } > > /* Program peripheral RBF */ > if (fpga_fsinfo.filename && fpga_fsinfo.dev_part && (len > 0)) > rval = fpga_fsload(0, buffer, BSIZE, &fpga_fsinfo); > > In /common/splash.c, dev_Part and flash type everything are hard coded > in struct splash_location. In my spl.c, flash type are determined on > run time and dev_part are retrived from DTS, and then assigned to > struct fpga_fsinfo. Please note that this is in SPL, mmc need to be > initialized 1st before loading raw file into memory. In SPL, raw file > are coppied to OCRAM chunk by chunk, but In u-boot it would normally > done in one big chunk to DRAM. This would be handled by fpga loadfs. > > So, you want me hard code everthing like in splash.c? No, I need you to play around with this and come up with generic firmware loader that's flexible enough. > 2nd function --> > In /common/splash_source.c > static int splash_select_fs_dev(struct splash_location *location) > { > int res; > > switch (location->storage) { > case SPLASH_STORAGE_MMC: > res = fs_set_blk_dev("mmc", location->devpart, > FS_TYPE_ANY); > break; > case SPLASH_STORAGE_USB: > res = fs_set_blk_dev("usb", location->devpart, > FS_TYPE_ANY); > break; > case SPLASH_STORAGE_SATA: > res = fs_set_blk_dev("sata", location->devpart, > FS_TYPE_ANY); > break; > case SPLASH_STORAGE_NAND: > if (location->ubivol != NULL) > res = fs_set_blk_dev("ubi", NULL, > FS_TYPE_UBIFS); > else > res = -ENODEV; > break; > default: > printf("Error: unsupported location storage.\n"); > return -ENODEV; > } > > if (res) > printf("Error: could not access storage.\n"); > > return res; > } > > In my /drivers/fpga/socfpga_arria10.c > static int flash_read(struct flash_info *flashinfo, > u32 size_read, > u32 *buffer_ptr) > { > size_t ret = EEXIST; > loff_t actread = 0; > > if (fs_set_blk_dev(flashinfo->interface, flashinfo->dev_part, > flashinfo->fstype)) > return FPGA_FAIL; > > ret = fs_read(flashinfo->filename, > (u32) buffer_ptr, flashinfo->flash_offset, > size_read, &actread); > > 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; > } > > Some attributes like flash type is determined on run time and and > dev_part is retrieved from DTS, so every infos driver need to know are > assinged into struct flashinfo and passed to fs_set_blk_dev as > arguments. I found that function in splash_source.c some like flash > type are getting from env variable, but we are still in SPL phase, > those env variable is not set up yet. So, i think that is very > ineffcient to factor out them as common. > > If you want me create a generic firmware loader which is generic enough > loading content for all components like flashes, FPGA, splash ....etc, > i don't think that is effient enough, as fpga loadfs has different > handling in both SPL and U-boot like copying raw into memory. Duplicating code is nonsense and I believe generic firmware loader would be the way to go here. > It would be good you can direct point me out which functions have > similirity and how to factor them out as common. > > Thanks a lot. >>> >>> Thanks. >>>> >>>> [...]
Hi, On 25 September 2017 at 02:40, <tien.fong.chee@intel.com> wrote: > From: Tien Fong Chee <tien.fong.chee@intel.com> > > These drivers handle 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 | 391 ++++++++++++++++++++- > include/altera.h | 6 + > include/configs/socfpga_common.h | 4 + > 4 files changed, 425 insertions(+), 3 deletions(-) Agreed on the generic loader, but also, can you please use driver model? Am I missing something? Regards, Simon
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..5fe57ef 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; @@ -112,13 +122,14 @@ static int wait_for_nconfig_pin_and_nstatus_pin(void) unsigned long mask = ALT_FPGAMGR_IMGCFG_STAT_F2S_NCONFIG_PIN_SET_MSK | ALT_FPGAMGR_IMGCFG_STAT_F2S_NSTATUS_PIN_SET_MSK; - /* Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop until de-asserted, - * timeout at 1000ms + /* + * Poll until f2s_nconfig_pin and f2s_nstatus_pin; loop until + * de-asserted, timeout at 1000ms */ 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) @@ -469,6 +480,7 @@ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) /* Initialize the FPGA Manager */ status = fpgamgr_program_init((u32 *)rbf_data, rbf_size); + if (status) return status; @@ -477,3 +489,376 @@ int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size) return fpgamgr_program_finish(); } + +#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 = fdtdec_next_compatible(fdt, 0, + COMPAT_ALTERA_SOCFPGA_FPGA0); + + if (nodeoffset >= 0) { + if (core) + cell = fdt_getprop(fdt, + nodeoffset, + "bitstream_core", + len); + else + cell = fdt_getprop(fdt, nodeoffset, "bitstream_periph", + len); + + 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 = fdtdec_next_compatible(fdt, 0, + COMPAT_ALTERA_SOCFPGA_FPGA0); + + cell = fdt_getprop(fdt, nodeoffset, "bitstream_devpart", len); + + 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 + */ + 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(buffer + i + 1), CORE RBF(buffer + i + 2) */ + if (ARRIA10RBF_PERIPH == *(buffer + i + 1)) { + rbf->section = periph_section; + break; + } else if (ARRIA10RBF_CORE == *(buffer + i + 1)) { + rbf->section = core_section; + break; + } 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 { + 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; + + if (fs_set_blk_dev(flashinfo->interface, flashinfo->dev_part, + flashinfo->fstype)) + return FPGA_FAIL; + + ret = fs_read(flashinfo->filename, + (u32) buffer_ptr, flashinfo->flash_offset, + size_read, &actread); + + 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) +{ + u32 *bufferptr_after_header = NULL; + u32 buffersize_after_header = 0; + u32 rbf_header_data_size = 0; + int ret = 0; + + flashinfo->flash_offset = 0; + + /* To avoid from keeping re-read the contents */ + struct image_header *header = &(flashinfo->header); + size_t buffer_size = *buffer_sizebytes; + u32 *buffer_ptr = (u32 *)*buffer; + + /* Load mkimage header into buffer */ + ret = flash_read(flashinfo, + sizeof(struct image_header), buffer_ptr); + + if (0 >= ret) { + printf(" Failed to read mkimage header from flash.\n"); + return -ENOENT; + } + + WATCHDOG_RESET(); + + memcpy(header, (u_char *)buffer_ptr, sizeof(*header)); + + if (!image_check_magic(header)) { + printf("FPGA: Bad Magic Number.\n"); + return -EBADF; + } + + if (!image_check_hcrc(header)) { + printf("FPGA: Bad Header Checksum.\n"); + return -EPERM; + } + + /* Getting rbf data size */ + flashinfo->remaining = + image_get_data_size(header); + + /* Calculate total size of both rbf data with mkimage header */ + rbf_header_data_size = flashinfo->remaining + + sizeof(struct image_header); + + /* Loading to buffer chunk by chunk, normally for OCRAM buffer */ + if (rbf_header_data_size > buffer_size) { + /* Calculate size of rbf data in the buffer */ + buffersize_after_header = + buffer_size - sizeof(struct image_header); + flashinfo->remaining -= buffersize_after_header; + } else { + /* Loading whole rbf image into buffer, normally for DDR buffer */ + buffer_size = rbf_header_data_size; + /* Calculate size of rbf data in the buffer */ + buffersize_after_header = + buffer_size - sizeof(struct image_header); + flashinfo->remaining = 0; + } + + /* Loading mkimage header and rbf data into buffer */ + ret = flash_read(flashinfo, buffer_size, buffer_ptr); + + if (0 >= ret) { + printf(" Failed to read mkimage header and rbf data "); + printf("from flash.\n"); + return -ENOENT; + } + + /* + * Getting pointer of rbf data starting address where is it + * right after mkimage header + */ + bufferptr_after_header = + (u32 *)((u_char *)buffer_ptr + sizeof(struct image_header)); + + /* Update next reading rbf data flash offset */ + flashinfo->flash_offset += buffer_size; + + /* + * Update the starting addr of rbf data to init FPGA & programming + * into FPGA + */ + *buffer = (u32)bufferptr_after_header; + + get_rbf_image_info(&flashinfo->rbfinfo, (u16 *)bufferptr_after_header); + + /* Update the size of rbf data to be programmed into FPGA */ + *buffer_sizebytes = buffersize_after_header; + +#ifdef CONFIG_CHECK_FPGA_DATA_CRC + flashinfo->datacrc = + crc32(flashinfo->datacrc, + (u_char *)bufferptr_after_header, + buffersize_after_header); +#endif + +if (0 == flashinfo->remaining) { +#ifdef CONFIG_CHECK_FPGA_DATA_CRC + if (flashinfo->datacrc != + image_get_dcrc(&(flashinfo->header))) { + printf("FPGA: Bad Data Checksum.\n"); + return -EPERM; + } +#endif +} + return 0; +} + +static int fs_flash_read(struct flash_info *flashinfo, u32 *buffer, + u32 *buffer_sizebytes) +{ + int ret = 0; + /* To avoid from keeping re-read the contents */ + size_t buffer_size = *buffer_sizebytes; + u32 *buffer_ptr = (u32 *)*buffer; + u32 flash_addr = flashinfo->flash_offset; + + /* Buffer allocated in OCRAM */ + /* Read the data by small chunk by chunk. */ + if (flashinfo->remaining > buffer_size) + flashinfo->remaining -= buffer_size; + else { + /* + * Buffer allocated in DDR, larger than rbf data most + * of the time + */ + buffer_size = flashinfo->remaining; + flashinfo->remaining = 0; + } + + ret = flash_read(flashinfo, buffer_size, buffer_ptr); + + if (0 >= ret) { + printf(" Failed to read rbf data from flash.\n"); + return -ENOENT; + } + +#ifdef CONFIG_CHECK_FPGA_DATA_CRC + flashinfo->datacrc = + crc32(flashinfo->datacrc, + (unsigned char *)buffer_ptr, buffer_size); +#endif + +if (0 == flashinfo->remaining) { +#ifdef CONFIG_CHECK_FPGA_DATA_CRC + if (flashinfo->datacrc != + image_get_dcrc(&(flashinfo->header))) { + printf("FPGA: Bad Data Checksum.\n"); + return -EPERM; + } +#endif +} + /* Update next reading rbf data flash offset */ + flash_addr += buffer_size; + + flashinfo->flash_offset = flash_addr; + + /* Update the size of rbf data to be programmed into FPGA */ + *buffer_sizebytes = buffer_size; + + return 0; +} + +int socfpga_loadfs(Altera_desc *desc, const void *buf, size_t bsize, + fpga_fs_info *fpga_fsinfo) +{ + u32 buffer = 0; + u32 buffer_ori = 0; + size_t buffer_sizebytes = 0; + size_t buffer_sizebytes_ori = 0; + struct flash_info flashinfo; + u32 status = 0; + int ret = 0; + + memset(&flashinfo, 0, sizeof(flashinfo)); + + if (fpga_fsinfo->filename == NULL) { + printf("no peripheral RBF filename specified.\n"); + return -EINVAL; + } + + WATCHDOG_RESET(); + + buffer_sizebytes = buffer_sizebytes_ori = bsize; + buffer = buffer_ori = (u32) buf; + flashinfo.interface = fpga_fsinfo->interface; + flashinfo.dev_part = fpga_fsinfo->dev_part; + flashinfo.filename = fpga_fsinfo->filename; + flashinfo.fstype = fpga_fsinfo->fstype; + + /* + * Note: Both buffer and buffer_sizebytes values can be altered by + * function below. + */ + ret = fs_flash_preinit(&flashinfo, &buffer, &buffer_sizebytes); + + if (ret) + return ret; + + if (periph_section == flashinfo.rbfinfo.section) { + /* Initialize the FPGA Manager */ + status = fpgamgr_program_init((u32 *)buffer, buffer_sizebytes); + if (status) { + printf("FPGA: Init with periph rbf failed with error. "); + printf("code %d\n", status); + return -EPERM; + } + } + + WATCHDOG_RESET(); + + /* Transfer data to FPGA Manager */ + fpgamgr_program_write((void *)buffer, + buffer_sizebytes); + + WATCHDOG_RESET(); + + while (flashinfo.remaining) { + ret = fs_flash_read(&flashinfo, &buffer_ori, + &buffer_sizebytes_ori); + + if (ret) + return ret; + + /* transfer data to FPGA Manager */ + fpgamgr_program_write((void *)buffer_ori, + buffer_sizebytes_ori); + + WATCHDOG_RESET(); + } + + if (periph_section == flashinfo.rbfinfo.section) { + if (-ETIMEDOUT != fpgamgr_wait_early_user_mode()) + printf("FPGA: Early Release Succeeded.\n"); + else { + printf("FPGA: Failed to see Early Release.\n"); + return -EIO; + } + } else if (core_section == flashinfo.rbfinfo.section) { + /* Ensure the FPGA entering config done */ + status = fpgamgr_program_finish(); + if (status) + return status; + else + printf("FPGA: Enter user mode.\n"); + + } else { + printf("Config Error: Unsupported FGPA raw binary type.\n"); + return -ENOEXEC; + } + + WATCHDOG_RESET(); + return 1; +} +#endif diff --git a/include/altera.h b/include/altera.h index 48d3eb7..0597e8a 100644 --- a/include/altera.h +++ b/include/altera.h @@ -84,6 +84,10 @@ typedef struct { extern int altera_load(Altera_desc *desc, const void *image, size_t size); extern int altera_dump(Altera_desc *desc, const void *buf, size_t bsize); extern int altera_info(Altera_desc *desc); +#if defined(CONFIG_CMD_FPGA_LOADFS) +int altera_loadfs(Altera_desc *desc, const void *buf, size_t bsize, + fpga_fs_info *fpga_fsinfo); +#endif /* Board specific implementation specific function types *********************************************************************/ @@ -111,6 +115,8 @@ typedef struct { #ifdef CONFIG_FPGA_SOCFPGA int socfpga_load(Altera_desc *desc, const void *rbf_data, size_t rbf_size); +int socfpga_loadfs(Altera_desc *desc, const void *buf, size_t bsize, + fpga_fs_info *fpga_fsinfo); #endif #ifdef CONFIG_FPGA_STRATIX_V diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index 9897e11..eadce2d 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 #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM_1 #define CONFIG_SYS_MEMTEST_END PHYS_SDRAM_1_SIZE #if defined(CONFIG_TARGET_SOCFPGA_GEN5)