[U-Boot] [RFC PATCH 05/11] tools: mksunxiboot: allow larger SPL binaries

[André Przywara]

On 21/01/17 04:24, Siarhei Siamashka wrote:

Hi Siarhei,

> On Fri, 20 Jan 2017 01:53:25 +0000
> Andre Przywara <andre.przywara at arm.com> wrote:
> 
>> mksunxiboot limits the size of the resulting SPL binaries to pretty
>> conservative values to cover all SoCs and all boot media (NAND).
>> In preparation for supporting modern SoCs without NAND, which may
>> require a really large SPL, introduce comamnd line parameters to
>> push the possible SPL size to the limits.
>>
>> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> 
> Thanks for trying to improve mksunxiboot.
> 
> The proposed "--mmc", "--nand" and "--max" options seem to be completely
> redundant and serve no real purpose. The 8K alignment is perfectly
> fine for any needs and allows to increase the SPL size to 32K (which
> is presumably your intention).

Mmmh, interesting, I think my first observation was that adjusting the
limit wasn't good enough, but there was some side effect of the
alignment connected to the limit not being exactly 32K, but slightly
below. So the easiest solution was to just follow the comment, but make
this configurable to not break NAND flash.

But as I can't reproduce this anymore, I am all too happy to drop this part.

> The only Allwinner SoC that may potentially benefit from the 512 bytes
> alignment is A13:
>     https://linux-sunxi.org/BROM#U-Boot_SPL_limitations
> But for the sake of simplicity, we can pretend that A13 has the
> same 24K SPL size limit as A10 and A20.

Fair enough.

> That said, I agree that we should fix the mksunxiboot tool and allow
> it to create 32K SPL files. Exceeding the SPL size limit is already
> checked elsewhere (via the linker script). So mksunxiboot does not
> need to enforce the 24K limit even for A10/A20 boards.

But what about the 0x7600 limit for the older SoCs?:
#define SUN4I_SRAM_SIZE 0x7600  /* 0x7748+ is used by BROM */

Or is this snake oil as we limit earlier to 24K already anyway?

So do you hint that this whole patch can eventually be reduced to:

-#define SUN4I_SRAM_SIZE 0x7600  /* 0x7748+ is used by BROM */
+#define SUN4I_SRAM_SIZE 0x8000

Cheers,
Andre.


>> ---
>>  tools/mksunxiboot.c | 51 ++++++++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 38 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c
>> index 0f0b003..fa54bce 100644
>> --- a/tools/mksunxiboot.c
>> +++ b/tools/mksunxiboot.c
>> @@ -48,37 +48,58 @@ int gen_check_sum(struct boot_file_head *head_p)
>>  #define ALIGN(x, a) __ALIGN_MASK((x), (typeof(x))(a)-1)
>>  #define __ALIGN_MASK(x, mask) (((x)+(mask))&~(mask))
>>  
>> -#define SUN4I_SRAM_SIZE 0x7600	/* 0x7748+ is used by BROM */
>> -#define SRAM_LOAD_MAX_SIZE (SUN4I_SRAM_SIZE - sizeof(struct boot_file_head))
>> +#define SUN4I_SRAM_SIZE		0x7600	/* 0x7748+ is used by BROM */
>> +#define SUNXI_MAX_SRAM_SIZE	0x8000
>> +#define SRAM_LOAD_MAX_SIZE (SUNXI_MAX_SRAM_SIZE - sizeof(struct boot_file_head))
>>  
>>  /*
>>   * BROM (at least on A10 and A20) requires NAND-images to be explicitly aligned
>>   * to a multiple of 8K, and rejects the image otherwise. MMC-images are fine
>> - * with 512B blocks. To cater for both, align to the largest of the two.
>> + * with 512B blocks.
>>   */
>> -#define BLOCK_SIZE 0x2000
>> +#define BLOCK_SIZE_NAND 0x2000
>> +#define BLOCK_SIZE_MMC  0x0200
>> +#define MAX_BLOCK_SIZE	BLOCK_SIZE_NAND
>>  
>>  struct boot_img {
>>  	struct boot_file_head header;
>>  	char code[SRAM_LOAD_MAX_SIZE];
>> -	char pad[BLOCK_SIZE];
>> +	char pad[MAX_BLOCK_SIZE];
>>  };
>>  
>>  int main(int argc, char *argv[])
>>  {
>>  	int fd_in, fd_out;
>> +	int ac = 1;
>>  	struct boot_img img;
>>  	unsigned file_size;
>> +	unsigned size_limit = SUN4I_SRAM_SIZE - sizeof(struct boot_file_head);
>> +	unsigned block_size = BLOCK_SIZE_NAND;
>>  	int count;
>>  
>>  	if (argc < 2) {
>>  		printf("\tThis program makes an input bin file to sun4i " \
>>  		       "bootable image.\n" \
>> -		       "\tUsage: %s input_file out_putfile\n", argv[0]);
>> +		       "\tUsage: %s input_file [out_putfile]\n", argv[0]);
>>  		return EXIT_FAILURE;
>>  	}
>>  
>> -	fd_in = open(argv[1], O_RDONLY);
>> +	if (!strcmp(argv[ac], "--mmc")) {
>> +		block_size = BLOCK_SIZE_MMC;
>> +		ac++;
>> +	}
>> +	if (!strcmp(argv[ac], "--nand")) {
>> +		block_size = BLOCK_SIZE_NAND;
>> +		ac++;
>> +	}
>> +	if (!strcmp(argv[ac], "--max")) {
>> +		size_limit = SUNXI_MAX_SRAM_SIZE - sizeof(struct boot_file_head);
>> +		ac++;
>> +	}
>> +	if (ac >= argc)
>> +		return EXIT_FAILURE;
>> +
>> +	fd_in = open(argv[ac++], O_RDONLY);
>>  	if (fd_in < 0) {
>>  		perror("Open input file");
>>  		return EXIT_FAILURE;
>> @@ -89,15 +110,19 @@ int main(int argc, char *argv[])
>>  	/* get input file size */
>>  	file_size = lseek(fd_in, 0, SEEK_END);
>>  
>> -	if (file_size > SRAM_LOAD_MAX_SIZE) {
>> +	if (file_size > size_limit) {
>>  		fprintf(stderr, "ERROR: File too large!\n");
>>  		return EXIT_FAILURE;
>>  	}
>>  
>> -	fd_out = open(argv[2], O_WRONLY | O_CREAT, 0666);
>> -	if (fd_out < 0) {
>> -		perror("Open output file");
>> -		return EXIT_FAILURE;
>> +	if (ac >= argc) {
>> +		fd_out = STDOUT_FILENO;
>> +	} else {
>> +		fd_out = open(argv[ac], O_WRONLY | O_CREAT, 0666);
>> +		if (fd_out < 0) {
>> +			perror("Open output file");
>> +			return EXIT_FAILURE;
>> +		}
>>  	}
>>  
>>  	/* read file to buffer to calculate checksum */
>> @@ -115,7 +140,7 @@ int main(int argc, char *argv[])
>>  		 & 0x00FFFFFF);
>>  	memcpy(img.header.magic, BOOT0_MAGIC, 8);	/* no '0' termination */
>>  	img.header.length =
>> -		ALIGN(file_size + sizeof(struct boot_file_head), BLOCK_SIZE);
>> +		ALIGN(file_size + sizeof(struct boot_file_head), block_size);
>>  	img.header.b_instruction = cpu_to_le32(img.header.b_instruction);
>>  	img.header.length = cpu_to_le32(img.header.length);
>>  
> 
> 
>