Re: [PATCH v2] ARM: arria10: xload: retry FPGA configuration

[Sascha Hauer <sha@pengutronix.de>]

On Wed, Nov 15, 2023 at 02:11:22PM +0100, Steffen Trumtrar wrote:
> From: Bruno Knittel <bruno.knittel@bruker.com>
> 
> The Intel documentation states that the FPGA configuration might fail.
> This has been observed on the Enclustra AA1+ board where up to 4 retries
> where required to configure the FPGA.
> 
> Debugging session showed that the data where correctly read from the
> eMMC but yet the configuration failed.
> 
> This commit introduces a retry loop on the FPGA configuration.
> Up to 10 retries (arbitrary) are attempted.
> As the hardware can't be used anyway without the FPGA loaded, this
> doesn't introduce any boot time problems. Taking longer is better than
> just hang()ing.
> 
> Signed-off-by: Bruno Knittel <bruno.knittel@bruker.com>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
> Changes since v1:
>     - split loop into inline function
> ---
>  arch/arm/mach-socfpga/arria10-xload.c | 49 ++++++++++++++++++++++++++---------
>  1 file changed, 37 insertions(+), 12 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/arch/arm/mach-socfpga/arria10-xload.c b/arch/arm/mach-socfpga/arria10-xload.c
> index 9d54a1de58..db3cc209ee 100644
> --- a/arch/arm/mach-socfpga/arria10-xload.c
> +++ b/arch/arm/mach-socfpga/arria10-xload.c
> @@ -353,17 +353,9 @@ int arria10_prepare_mmc(int barebox_part, int rbf_part)
>  	return 0;
>  }
>  
> -int arria10_load_fpga(int offset, int bitstream_size)
> +static inline int __arria10_load_fpga(void *buf, uint32_t count, uint32_t size)
>  {
> -	void *buf = (void *)0xffe00000 + SZ_256K - 256 - SZ_16K;
>  	int ret;
> -	uint32_t count;
> -	uint32_t size = bitstream_size / SECTOR_SIZE;
> -
> -	if (offset)
> -		offset = offset / SECTOR_SIZE;
> -
> -	count = offset;
>  
>  	arria10_read_blocks(buf, count + bitstream.first_sec, SZ_16K);
>  
> @@ -371,23 +363,56 @@ int arria10_load_fpga(int offset, int bitstream_size)
>  
>  	ret = a10_fpga_init(buf);
>  	if (ret)
> -		hang();
> +		return -EAGAIN;
>  
>  	while (count <= size) {
>  		ret = a10_fpga_write(buf, SZ_16K);
>  		if (ret == -ENOSPC)
> -			break;
> +			return -EAGAIN;
> +
>  		count += SZ_16K / SECTOR_SIZE;
>  		ret = arria10_read_blocks(buf, count, SZ_16K);
> +		// Reading failed, consider this a failed attempt to configure the FPGA and retry
> +		if (ret)
> +			return -EAGAIN;
>  	}
>  
>  	ret = a10_fpga_write_complete();
>  	if (ret)
> -		hang();
> +		return -EAGAIN;
>  
>  	return 0;
>  }
>  
> +int arria10_load_fpga(int offset, int bitstream_size)
> +{
> +	int ret;
> +	void *buf = (void *)0xffe00000 + SZ_256K - 256 - SZ_16K;
> +	uint32_t count;
> +	uint32_t size = bitstream_size / SECTOR_SIZE;
> +	uint32_t retryCount;
> +
> +	if (offset)
> +		offset = offset / SECTOR_SIZE;
> +
> +	/* Up to 4 retries have been seen on the Enclustra Mercury AA1+ board, as
> +	 * FPGA configuration is mandatory to be able to continue the boot, take
> +	 * some margin and try up to 10 times
> +	 */
> +	for (retryCount = 0; retryCount < 10; ++retryCount) {
> +		count = offset;
> +
> +		ret = __arria10_load_fpga(buf, count, size);
> +		if (!ret)
> +			return 0;
> +		else if (ret == -EAGAIN)
> +			continue;
> +	}
> +
> +	hang();
> +	return -EIO;
> +}
> +
>  void arria10_start_image(int offset)
>  {
>  	void *buf = (void *)0x0;
> 
> ---
> base-commit: bc5e49335067d0c6c6e97c082a0368fe9d86b725
> change-id: 20231115-arria10-fpga-reload-on-failure-ba64deafcc52
> 
> Best regards,
> -- 
> Steffen Trumtrar <s.trumtrar@pengutronix.de>
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |