[U-Boot] [PATCH v2 09/19] arm: socfpga: Add drivers for programing FPGA from flash

[Marek Vasut]

On 09/25/2017 10:40 AM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee at 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 at 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)
> 


-- 
Best regards,
Marek Vasut