[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[Marek Vasut]

The Arria10 uses slightly different boot image header than the Gen5 SoCs,
in particular the header itself contains an offset from the start of the
header to which the Arria10 jumps. This offset must not be negative, yet
the header is placed at offset 0x40 of the bootable binary. Therefore, to
jump into U-Boot, add a trampoline just past the Arria10 boot header and
point to this trampoline at fixed offset from the header generated using
the mkimage -T socfpgaimage_v1 . Note that it is not needed to jump back
to offset 0x0 of the image, it is possible to jump directly at the reset
label and save processing two instructions.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Chin Liang See <chin.liang.see@intel.com>
---
 arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h b/arch/arm/mach-socfpga/include/mach/boot0.h
index d6b9435d33..06bbe27d2c 100644
--- a/arch/arm/mach-socfpga/include/mach/boot0.h
+++ b/arch/arm/mach-socfpga/include/mach/boot0.h
@@ -18,10 +18,10 @@ _start:
 	.word	0xcafec0d3;	/* Checksum, zero-pad */
 	nop;
 
-	b reset;		/* SoCFPGA jumps here */
-	nop;
+	b reset;		/* SoCFPGA Gen5 jumps here */
 	nop;
 	nop;
+	b reset;		/* SoCFPGA Gen10 trampoline */
 #endif
 
 #endif /* __BOOT0_H */
-- 
2.16.2

[U-Boot] [PATCH 2/5] image: socfpga: Add SFP image version 1 definition

[Marek Vasut]

Add support for the SoCFPGA header v1, which is used on Arria 10.
The layout of the v0 and v1 header is similar, yet there are a few
differences which make it incompatible with previous v0 header, so
add a new entry.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Chin Liang See <chin.liang.see@intel.com>
---
 common/image.c  | 3 ++-
 include/image.h | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/image.c b/common/image.c
index e1c50eb25d..5024da3b17 100644
--- a/common/image.c
+++ b/common/image.c
@@ -146,7 +146,8 @@ static const table_entry_t uimage_type[] = {
 	{	IH_TYPE_PBLIMAGE,   "pblimage",   "Freescale PBL Boot Image",},
 	{	IH_TYPE_RAMDISK,    "ramdisk",	  "RAMDisk Image",	},
 	{	IH_TYPE_SCRIPT,     "script",	  "Script",		},
-	{	IH_TYPE_SOCFPGAIMAGE, "socfpgaimage", "Altera SOCFPGA preloader",},
+	{	IH_TYPE_SOCFPGAIMAGE, "socfpgaimage", "Altera SoCFPGA CV/AV preloader",},
+	{	IH_TYPE_SOCFPGAIMAGE_V1, "socfpgaimage_v1", "Altera SoCFPGA A10 preloader",},
 	{	IH_TYPE_STANDALONE, "standalone", "Standalone Program", },
 	{	IH_TYPE_UBLIMAGE,   "ublimage",   "Davinci UBL image",},
 	{	IH_TYPE_MXSIMAGE,   "mxsimage",   "Freescale MXS Boot Image",},
diff --git a/include/image.h b/include/image.h
index a579c5f509..8ce722866b 100644
--- a/include/image.h
+++ b/include/image.h
@@ -260,7 +260,7 @@ enum {
 	IH_TYPE_MXSIMAGE,		/* Freescale MXSBoot Image	*/
 	IH_TYPE_GPIMAGE,		/* TI Keystone GPHeader Image	*/
 	IH_TYPE_ATMELIMAGE,		/* ATMEL ROM bootable Image	*/
-	IH_TYPE_SOCFPGAIMAGE,		/* Altera SOCFPGA Preloader	*/
+	IH_TYPE_SOCFPGAIMAGE,		/* Altera SOCFPGA CV/AV Preloader */
 	IH_TYPE_X86_SETUP,		/* x86 setup.bin Image		*/
 	IH_TYPE_LPC32XXIMAGE,		/* x86 setup.bin Image		*/
 	IH_TYPE_LOADABLE,		/* A list of typeless images	*/
@@ -275,6 +275,7 @@ enum {
 	IH_TYPE_FIRMWARE_IVT,		/* Firmware Image with HABv4 IVT */
 	IH_TYPE_PMMC,            /* TI Power Management Micro-Controller Firmware */
 	IH_TYPE_STM32IMAGE,		/* STMicroelectronics STM32 Image */
+	IH_TYPE_SOCFPGAIMAGE_V1,	/* Altera SOCFPGA A10 Preloader	*/
 
 	IH_TYPE_COUNT,			/* Number of image types */
 };
-- 
2.16.2

[U-Boot] [PATCH 3/5] tools: socfpga: Stop using global struct socfpga_image

[Marek Vasut]

The structure is passed around correctly, create local instances
where necessary and zap the global struct socfpga_image instance.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Chin Liang See <chin.liang.see@intel.com>
---
 tools/socfpgaimage.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/socfpgaimage.c b/tools/socfpgaimage.c
index 8fe91fe80e..d77459cfed 100644
--- a/tools/socfpgaimage.c
+++ b/tools/socfpgaimage.c
@@ -46,14 +46,14 @@
 
 static uint8_t buffer[PADDED_SIZE];
 
-static struct socfpga_header {
+struct socfpga_header {
 	uint32_t validation;
 	uint8_t  version;
 	uint8_t  flags;
 	uint16_t length_u32;
 	uint16_t zero;
 	uint16_t checksum;
-} header;
+};
 
 /*
  * The header checksum is just a very simple checksum over
@@ -76,6 +76,8 @@ static uint16_t hdr_checksum(struct socfpga_header *header)
 static void build_header(uint8_t *buf, uint8_t version, uint8_t flags,
 			 uint16_t length_bytes)
 {
+	struct socfpga_header header;
+
 	header.validation = cpu_to_le32(VALIDATION_WORD);
 	header.version = version;
 	header.flags = flags;
@@ -92,6 +94,8 @@ static void build_header(uint8_t *buf, uint8_t version, uint8_t flags,
  */
 static int verify_header(const uint8_t *buf)
 {
+	struct socfpga_header header;
+
 	memcpy(&header, buf, sizeof(header));
 
 	if (le32_to_cpu(header.validation) != VALIDATION_WORD)
-- 
2.16.2

[U-Boot] [PATCH 4/5] tools: socfpga: Add SFP image V1 support

[Marek Vasut]

Add support for the SoCFPGA header v1 , which is used on Arria 10.
Thus far the mkimage-socfpga image only supported header format v0
used on Cyclone V and Arria V, but is not supported on Arria 10.
The layout of the v0 and v1 header is similar, yet there are a few
differences, see the patch body for details.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Chin Liang See <chin.liang.see@intel.com>
---
 tools/socfpgaimage.c | 293 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 216 insertions(+), 77 deletions(-)

diff --git a/tools/socfpgaimage.c b/tools/socfpgaimage.c
index d77459cfed..19dcca9ed5 100644
--- a/tools/socfpgaimage.c
+++ b/tools/socfpgaimage.c
@@ -3,30 +3,52 @@
  *
  * SPDX-License-Identifier:	GPL-2.0+
  *
- * Reference doc http://www.altera.com.cn/literature/hb/cyclone-v/cv_5400A.pdf
- * Note this doc is not entirely accurate. Of particular interest to us is the
- * "header" length field being in U32s and not bytes.
+ * Reference documents:
+ *   Cyclone V SoC: https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/cyclone-v/cv_5400a.pdf
+ *   Arria V SoC:   https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/arria-v/av_5400a.pdf
+ *   Arria 10 SoC:  https://www.altera.com/content/dam/altera-www/global/en_US/pdfs/literature/hb/arria-10/a10_5400a.pdf
  *
- * "Header" is a structure of the following format.
- * this is positioned at 0x40.
+ * Bootable SoCFPGA image requires a structure of the following format
+ * positioned at offset 0x40 of the bootable image. Endian is LSB.
  *
- * Endian is LSB.
+ * There are two versions of the SoCFPGA header format, v0 and v1.
+ * The version 0 is used by Cyclone V SoC and Arria V SoC, while
+ * the version 1 is used by the Arria 10 SoC.
  *
+ * Version 0:
  * Offset   Length   Usage
  * -----------------------
- *   0x40        4   Validation word 0x31305341
- *   0x44        1   Version (whatever, zero is fine)
- *   0x45        1   Flags   (unused, zero is fine)
- *   0x46        2   Length  (in units of u32, including the end checksum).
- *   0x48        2   Zero
+ *   0x40        4   Validation word (0x31305341)
+ *   0x44        1   Version (0x0)
+ *   0x45        1   Flags (unused, zero is fine)
+ *   0x46        2   Length (in units of u32, including the end checksum).
+ *   0x48        2   Zero (0x0)
  *   0x4A        2   Checksum over the header. NB Not CRC32
  *
+ * Version 1:
+ * Offset   Length   Usage
+ * -----------------------
+ *   0x40        4   Validation word (0x31305341)
+ *   0x44        1   Version (0x1)
+ *   0x45        1   Flags (unused, zero is fine)
+ *   0x46        2   Header length (in units of u8).
+ *   0x48        4   Length (in units of u8).
+ *   0x4C        4   Image entry offset from standard of header
+ *   0x50        2   Zero (0x0)
+ *   0x52        2   Checksum over the header. NB Not CRC32
+ *
  * At the end of the code we have a 32-bit CRC checksum over whole binary
  * excluding the CRC.
  *
  * Note that the CRC used here is **not** the zlib/Adler crc32. It is the
  * CRC-32 used in bzip2, ethernet and elsewhere.
  *
+ * The Image entry offset in version 1 image is relative the the start of
+ * the header, 0x40, and must not be a negative number. Therefore, it is
+ * only possible to make the SoCFPGA jump forward. The U-Boot bootloader
+ * places a trampoline instruction at offset 0x5c, 0x1c bytes from the
+ * start of the SoCFPGA header, which jumps to the reset vector.
+ *
  * The image is padded out to 64k, because that is what is
  * typically used to write the image to the boot medium.
  */
@@ -39,32 +61,57 @@
 
 #define HEADER_OFFSET	0x40
 #define VALIDATION_WORD	0x31305341
-#define PADDED_SIZE	0x10000
 
-/* To allow for adding CRC, the max input size is a bit smaller. */
-#define MAX_INPUT_SIZE	(PADDED_SIZE - sizeof(uint32_t))
+static uint8_t buffer_v0[0x10000];
+static uint8_t buffer_v1[0x40000];
 
-static uint8_t buffer[PADDED_SIZE];
+struct socfpga_header_v0 {
+	uint32_t	validation;
+	uint8_t		version;
+	uint8_t		flags;
+	uint16_t	length_u32;
+	uint16_t	zero;
+	uint16_t	checksum;
+};
 
-struct socfpga_header {
-	uint32_t validation;
-	uint8_t  version;
-	uint8_t  flags;
-	uint16_t length_u32;
-	uint16_t zero;
-	uint16_t checksum;
+struct socfpga_header_v1 {
+	uint32_t	validation;
+	uint8_t		version;
+	uint8_t		flags;
+	uint16_t	header_u8;
+	uint32_t	length_u8;
+	uint32_t	entry_offset;
+	uint16_t	zero;
+	uint16_t	checksum;
 };
 
+static unsigned int sfp_hdr_size(uint8_t ver)
+{
+	if (ver == 0)
+		return sizeof(struct socfpga_header_v0);
+	if (ver == 1)
+		return sizeof(struct socfpga_header_v1);
+	return 0;
+}
+
+static unsigned int sfp_pad_size(uint8_t ver)
+{
+	if (ver == 0)
+		return sizeof(buffer_v0);
+	if (ver == 1)
+		return sizeof(buffer_v1);
+	return 0;
+}
+
 /*
  * The header checksum is just a very simple checksum over
  * the header area.
  * There is still a crc32 over the whole lot.
  */
-static uint16_t hdr_checksum(struct socfpga_header *header)
+static uint16_t sfp_hdr_checksum(uint8_t *buf, unsigned char ver)
 {
-	int len = sizeof(*header) - sizeof(header->checksum);
-	uint8_t *buf = (uint8_t *)header;
 	uint16_t ret = 0;
+	int len = sfp_hdr_size(ver) - sizeof(ret);
 
 	while (--len)
 		ret += *buf++;
@@ -72,52 +119,93 @@ static uint16_t hdr_checksum(struct socfpga_header *header)
 	return ret;
 }
 
-
-static void build_header(uint8_t *buf, uint8_t version, uint8_t flags,
-			 uint16_t length_bytes)
+static void sfp_build_header(uint8_t *buf, uint8_t ver, uint8_t flags,
+			     uint32_t length_bytes)
 {
-	struct socfpga_header header;
-
-	header.validation = cpu_to_le32(VALIDATION_WORD);
-	header.version = version;
-	header.flags = flags;
-	header.length_u32 = cpu_to_le16(length_bytes/4);
-	header.zero = 0;
-	header.checksum = cpu_to_le16(hdr_checksum(&header));
-
-	memcpy(buf, &header, sizeof(header));
+	struct socfpga_header_v0 header_v0 = {
+		.validation	= cpu_to_le32(VALIDATION_WORD),
+		.version	= 0,
+		.flags		= flags,
+		.length_u32	= cpu_to_le16(length_bytes / 4),
+		.zero		= 0,
+	};
+
+	struct socfpga_header_v1 header_v1 = {
+		.validation	= cpu_to_le32(VALIDATION_WORD),
+		.version	= 1,
+		.flags		= flags,
+		.header_u8	= cpu_to_le16(sizeof(header_v1)),
+		.length_u8	= cpu_to_le32(length_bytes),
+		.entry_offset	= cpu_to_le32(0x1c),	/* Trampoline offset */
+		.zero		= 0,
+	};
+
+	uint16_t csum;
+
+	if (ver == 0) {
+		csum = sfp_hdr_checksum((uint8_t *)&header_v0, 0);
+		header_v0.checksum = cpu_to_le16(csum);
+		memcpy(buf, &header_v0, sizeof(header_v0));
+	} else {
+		csum = sfp_hdr_checksum((uint8_t *)&header_v1, 1);
+		header_v1.checksum = cpu_to_le16(csum);
+		memcpy(buf, &header_v1, sizeof(header_v1));
+	}
 }
 
 /*
  * Perform a rudimentary verification of header and return
  * size of image.
  */
-static int verify_header(const uint8_t *buf)
+static int sfp_verify_header(const uint8_t *buf, uint8_t *ver)
 {
-	struct socfpga_header header;
+	struct socfpga_header_v0 header_v0;
+	struct socfpga_header_v1 header_v1;
+	uint16_t hdr_csum, sfp_csum;
+	uint32_t img_len;
 
-	memcpy(&header, buf, sizeof(header));
+	/*
+	 * Header v0 is always smaller than Header v1 and the validation
+	 * word and version field is at the same place, so use Header v0
+	 * to check for version during verifiction and upgrade to Header
+	 * v1 if needed.
+	 */
+	memcpy(&header_v0, buf, sizeof(header_v0));
 
-	if (le32_to_cpu(header.validation) != VALIDATION_WORD)
-		return -1;
-	if (le16_to_cpu(header.checksum) != hdr_checksum(&header))
+	if (le32_to_cpu(header_v0.validation) != VALIDATION_WORD)
 		return -1;
 
-	return le16_to_cpu(header.length_u32) * 4;
+	if (header_v0.version == 0) {
+		hdr_csum = le16_to_cpu(header_v0.checksum);
+		sfp_csum = sfp_hdr_checksum((uint8_t *)&header_v0, 0);
+		img_len = le16_to_cpu(header_v0.length_u32) * 4;
+	} else if (header_v0.version == 1) {
+		memcpy(&header_v1, buf, sizeof(header_v1));
+		hdr_csum = le16_to_cpu(header_v1.checksum);
+		sfp_csum = sfp_hdr_checksum((uint8_t *)&header_v1, 1);
+		img_len = le32_to_cpu(header_v1.length_u8);
+	} else {	/* Invalid version */
+		return -EINVAL;
+	}
+
+	/* Verify checksum */
+	if (hdr_csum != sfp_csum)
+		return -EINVAL;
+
+	return img_len;
 }
 
 /* Sign the buffer and return the signed buffer size */
-static int sign_buffer(uint8_t *buf,
-			uint8_t version, uint8_t flags,
-			int len, int pad_64k)
+static int sfp_sign_buffer(uint8_t *buf, uint8_t ver, uint8_t flags,
+			   int len, int pad_64k)
 {
 	uint32_t calc_crc;
 
 	/* Align the length up */
-	len = (len + 3) & (~3);
+	len = (len + 3) & ~3;
 
 	/* Build header, adding 4 bytes to length to hold the CRC32. */
-	build_header(buf + HEADER_OFFSET,  version, flags, len + 4);
+	sfp_build_header(buf + HEADER_OFFSET, ver, flags, len + 4);
 
 	/* Calculate and apply the CRC */
 	calc_crc = ~pbl_crc32(0, (char *)buf, len);
@@ -127,23 +215,24 @@ static int sign_buffer(uint8_t *buf,
 	if (!pad_64k)
 		return len + 4;
 
-	return PADDED_SIZE;
+	return sfp_pad_size(ver);
 }
 
 /* Verify that the buffer looks sane */
-static int verify_buffer(const uint8_t *buf)
+static int sfp_verify_buffer(const uint8_t *buf)
 {
 	int len; /* Including 32bit CRC */
 	uint32_t calc_crc;
 	uint32_t buf_crc;
+	uint8_t ver = 0;
 
-	len = verify_header(buf + HEADER_OFFSET);
+	len = sfp_verify_header(buf + HEADER_OFFSET, &ver);
 	if (len < 0) {
 		debug("Invalid header\n");
 		return -1;
 	}
 
-	if (len < HEADER_OFFSET || len > PADDED_SIZE) {
+	if (len < HEADER_OFFSET || len > sfp_pad_size(ver)) {
 		debug("Invalid header length (%i)\n", len);
 		return -1;
 	}
@@ -169,17 +258,17 @@ static int verify_buffer(const uint8_t *buf)
 
 /* mkimage glue functions */
 static int socfpgaimage_verify_header(unsigned char *ptr, int image_size,
-			struct image_tool_params *params)
+				      struct image_tool_params *params)
 {
-	if (image_size != PADDED_SIZE)
+	if (image_size < 0x80)
 		return -1;
 
-	return verify_buffer(ptr);
+	return sfp_verify_buffer(ptr);
 }
 
 static void socfpgaimage_print_header(const void *ptr)
 {
-	if (verify_buffer(ptr) == 0)
+	if (sfp_verify_buffer(ptr) == 0)
 		printf("Looks like a sane SOCFPGA preloader\n");
 	else
 		printf("Not a sane SOCFPGA preloader\n");
@@ -193,18 +282,25 @@ static int socfpgaimage_check_params(struct image_tool_params *params)
 		(params->lflag && (params->dflag || params->fflag));
 }
 
-static int socfpgaimage_check_image_types(uint8_t type)
+static int socfpgaimage_check_image_types_v0(uint8_t type)
 {
 	if (type == IH_TYPE_SOCFPGAIMAGE)
 		return EXIT_SUCCESS;
 	return EXIT_FAILURE;
 }
 
+static int socfpgaimage_check_image_types_v1(uint8_t type)
+{
+	if (type == IH_TYPE_SOCFPGAIMAGE_V1)
+		return EXIT_SUCCESS;
+	return EXIT_FAILURE;
+}
+
 /*
  * To work in with the mkimage framework, we do some ugly stuff...
  *
  * First, socfpgaimage_vrec_header() is called.
- * We prepend a fake header big enough to make the file PADDED_SIZE.
+ * We prepend a fake header big enough to make the file sfp_pad_size().
  * This gives us enough space to do what we want later.
  *
  * Next, socfpgaimage_set_header() is called.
@@ -213,51 +309,94 @@ static int socfpgaimage_check_image_types(uint8_t type)
  */
 
 static int data_size;
-#define FAKE_HEADER_SIZE (PADDED_SIZE - data_size)
 
-static int socfpgaimage_vrec_header(struct image_tool_params *params,
-				struct image_type_params *tparams)
+static int sfp_fake_header_size(unsigned int size, uint8_t ver)
+{
+	return sfp_pad_size(ver) - size;
+}
+
+static int sfp_vrec_header(struct image_tool_params *params,
+			   struct image_type_params *tparams, uint8_t ver)
 {
 	struct stat sbuf;
 
 	if (params->datafile &&
 	    stat(params->datafile, &sbuf) == 0 &&
-	    sbuf.st_size <= MAX_INPUT_SIZE) {
+	    sbuf.st_size <= (sfp_pad_size(ver) - sizeof(uint32_t))) {
 		data_size = sbuf.st_size;
-		tparams->header_size = FAKE_HEADER_SIZE;
+		tparams->header_size = sfp_fake_header_size(data_size, ver);
 	}
 	return 0;
+
 }
 
-static void socfpgaimage_set_header(void *ptr, struct stat *sbuf, int ifd,
-				struct image_tool_params *params)
+static int socfpgaimage_vrec_header_v0(struct image_tool_params *params,
+				       struct image_type_params *tparams)
+{
+	return sfp_vrec_header(params, tparams, 0);
+}
+
+static int socfpgaimage_vrec_header_v1(struct image_tool_params *params,
+				       struct image_type_params *tparams)
+{
+	return sfp_vrec_header(params, tparams, 1);
+}
+
+static void sfp_set_header(void *ptr, unsigned char ver)
 {
 	uint8_t *buf = (uint8_t *)ptr;
 
 	/*
 	 * This function is called after vrec_header() has been called.
-	 * At this stage we have the FAKE_HEADER_SIZE dummy bytes followed by
-	 * data_size image bytes. Total = PADDED_SIZE.
+	 * At this stage we have the sfp_fake_header_size() dummy bytes
+	 * followed by data_size image bytes. Total = sfp_pad_size().
 	 * We need to fix the buffer by moving the image bytes back to
 	 * the beginning of the buffer, then actually do the signing stuff...
 	 */
-	memmove(buf, buf + FAKE_HEADER_SIZE, data_size);
-	memset(buf + data_size, 0, FAKE_HEADER_SIZE);
+	memmove(buf, buf + sfp_fake_header_size(data_size, ver), data_size);
+	memset(buf + data_size, 0, sfp_fake_header_size(data_size, ver));
+
+	sfp_sign_buffer(buf, ver, 0, data_size, 0);
+}
 
-	sign_buffer(buf, 0, 0, data_size, 0);
+static void socfpgaimage_set_header_v0(void *ptr, struct stat *sbuf, int ifd,
+				       struct image_tool_params *params)
+{
+	sfp_set_header(ptr, 0);
+}
+
+static void socfpgaimage_set_header_v1(void *ptr, struct stat *sbuf, int ifd,
+				       struct image_tool_params *params)
+{
+	sfp_set_header(ptr, 1);
 }
 
 U_BOOT_IMAGE_TYPE(
 	socfpgaimage,
-	"Altera SOCFPGA preloader support",
+	"Altera SoCFPGA Cyclone V / Arria V image support",
+	0, /* This will be modified by vrec_header() */
+	(void *)buffer_v0,
+	socfpgaimage_check_params,
+	socfpgaimage_verify_header,
+	socfpgaimage_print_header,
+	socfpgaimage_set_header_v0,
+	NULL,
+	socfpgaimage_check_image_types_v0,
+	NULL,
+	socfpgaimage_vrec_header_v0
+);
+
+U_BOOT_IMAGE_TYPE(
+	socfpgaimage_v1,
+	"Altera SoCFPGA Arria10 image support",
 	0, /* This will be modified by vrec_header() */
-	(void *)buffer,
+	(void *)buffer_v1,
 	socfpgaimage_check_params,
 	socfpgaimage_verify_header,
 	socfpgaimage_print_header,
-	socfpgaimage_set_header,
+	socfpgaimage_set_header_v1,
 	NULL,
-	socfpgaimage_check_image_types,
+	socfpgaimage_check_image_types_v1,
 	NULL,
-	socfpgaimage_vrec_header
+	socfpgaimage_vrec_header_v1
 );
-- 
2.16.2

[U-Boot] [PATCH 5/5] spl: socfpga: Generate Arria10 SFP header V1

[Marek Vasut]

Generate SoCFPGA boot header version 1 instead of version 0 for Arria10.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Chin Liang See <chin.liang.see@intel.com>
---
 scripts/Makefile.spl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 2993ade41e..5d9adda996 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -309,7 +309,11 @@ LDFLAGS_$(SPL_BIN) += -Ttext $(CONFIG_SPL_TEXT_BASE)
 endif
 endif
 
+ifdef CONFIG_TARGET_SOCFPGA_ARRIA10
+MKIMAGEFLAGS_$(SPL_BIN).sfp = -T socfpgaimage_v1
+else
 MKIMAGEFLAGS_$(SPL_BIN).sfp = -T socfpgaimage
+endif
 $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
 	$(call if_changed,mkimage)
 
-- 
2.16.2

[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[See, Chin Liang]

Hi Marek,

On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> The Arria10 uses slightly different boot image header than the Gen5
> SoCs,
> in particular the header itself contains an offset from the start of
> the
> header to which the Arria10 jumps. This offset must not be negative,
> yet
> the header is placed at offset 0x40 of the bootable binary.
> Therefore, to
> jump into U-Boot, add a trampoline just past the Arria10 boot header
> and
> point to this trampoline at fixed offset from the header generated
> using
> the mkimage -T socfpgaimage_v1 . Note that it is not needed to jump
> back
> to offset 0x0 of the image, it is possible to jump directly at the
> reset
> label and save processing two instructions.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> ---
>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
> b/arch/arm/mach-socfpga/include/mach/boot0.h
> index d6b9435d33..06bbe27d2c 100644
> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
> @@ -18,10 +18,10 @@ _start:
>  	.word	0xcafec0d3;	/* Checksum, zero-pad */
>  	nop;
>  
> -	b reset;		/* SoCFPGA jumps here */
> -	nop;
> +	b reset;		/* SoCFPGA Gen5 jumps here */
>  	nop;
>  	nop;
> +	b reset;		/* SoCFPGA Gen10 trampoline */

Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder can we
standardize that by using 0x14 instead of proposed 0x18 in this patch?

Thanks
Chin Liang

>  #endif
>  
>  #endif /* __BOOT0_H */

[U-Boot] [PATCH 2/5] image: socfpga: Add SFP image version 1 definition

[See, Chin Liang]

On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> Add support for the SoCFPGA header v1, which is used on Arria 10.
> The layout of the v0 and v1 header is similar, yet there are a few
> differences which make it incompatible with previous v0 header, so
> add a new entry.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> ---
>  common/image.c  | 3 ++-
>  include/image.h | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 

Acked-by: Chin Liang See <chin.liang.see@intel.com>

[U-Boot] [PATCH 3/5] tools: socfpga: Stop using global struct socfpga_image

[See, Chin Liang]

On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> The structure is passed around correctly, create local instances
> where necessary and zap the global struct socfpga_image instance.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> ---
>  tools/socfpgaimage.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 

Acked-by: Chin Liang See <chin.liang.see@intel.com>

[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[Marek Vasut]

On 04/17/2018 10:40 AM, See, Chin Liang wrote:
> Hi Marek,
> 
> On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
>> The Arria10 uses slightly different boot image header than the Gen5
>> SoCs,
>> in particular the header itself contains an offset from the start of
>> the
>> header to which the Arria10 jumps. This offset must not be negative,
>> yet
>> the header is placed at offset 0x40 of the bootable binary.
>> Therefore, to
>> jump into U-Boot, add a trampoline just past the Arria10 boot header
>> and
>> point to this trampoline at fixed offset from the header generated
>> using
>> the mkimage -T socfpgaimage_v1 . Note that it is not needed to jump
>> back
>> to offset 0x0 of the image, it is possible to jump directly at the
>> reset
>> label and save processing two instructions.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> Cc: Chin Liang See <chin.liang.see@intel.com>
>> ---
>>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
>> b/arch/arm/mach-socfpga/include/mach/boot0.h
>> index d6b9435d33..06bbe27d2c 100644
>> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
>> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
>> @@ -18,10 +18,10 @@ _start:
>>  	.word	0xcafec0d3;	/* Checksum, zero-pad */
>>  	nop;
>>  
>> -	b reset;		/* SoCFPGA jumps here */
>> -	nop;
>> +	b reset;		/* SoCFPGA Gen5 jumps here */
>>  	nop;
>>  	nop;
>> +	b reset;		/* SoCFPGA Gen10 trampoline */
> 
> Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder can we
> standardize that by using 0x14 instead of proposed 0x18 in this patch?

What difference does it make, the entire image is generated during the
build anyway ? This patch uses offset 0x1c, but what is the reason for
address 0x14 in your proprietary tool, is there one ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[See, Chin Liang]

On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
> On 04/17/2018 10:40 AM, See, Chin Liang wrote:
> > 
> > Hi Marek,
> > 
> > On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> > > 
> > > The Arria10 uses slightly different boot image header than the
> > > Gen5
> > > SoCs,
> > > in particular the header itself contains an offset from the start
> > > of
> > > the
> > > header to which the Arria10 jumps. This offset must not be
> > > negative,
> > > yet
> > > the header is placed at offset 0x40 of the bootable binary.
> > > Therefore, to
> > > jump into U-Boot, add a trampoline just past the Arria10 boot
> > > header
> > > and
> > > point to this trampoline at fixed offset from the header
> > > generated
> > > using
> > > the mkimage -T socfpgaimage_v1 . Note that it is not needed to
> > > jump
> > > back
> > > to offset 0x0 of the image, it is possible to jump directly at
> > > the
> > > reset
> > > label and save processing two instructions.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > > Cc: Chin Liang See <chin.liang.see@intel.com>
> > > ---
> > >  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > index d6b9435d33..06bbe27d2c 100644
> > > --- a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > @@ -18,10 +18,10 @@ _start:
> > >  	.word	0xcafec0d3;	/* Checksum, zero-pad */
> > >  	nop;
> > >  
> > > -	b reset;		/* SoCFPGA jumps here */
> > > -	nop;
> > > +	b reset;		/* SoCFPGA Gen5 jumps here */
> > >  	nop;
> > >  	nop;
> > > +	b reset;		/* SoCFPGA Gen10 trampoline */
> > Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder can
> > we
> > standardize that by using 0x14 instead of proposed 0x18 in this
> > patch?
> What difference does it make, the entire image is generated during
> the
> build anyway ? This patch uses offset 0x1c, but what is the reason
> for
> address 0x14 in your proprietary tool, is there one ?

Our A10 header ended at 0x13 today. Hence we are continuing the code at
0x14 without any spacing.

While for 0x1c, should it be 3 nop?

Thanks
Chin Liang

> 

[U-Boot] [PATCH 5/5] spl: socfpga: Generate Arria10 SFP header V1

[See, Chin Liang]

On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> Generate SoCFPGA boot header version 1 instead of version 0 for
> Arria10.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> ---
>  scripts/Makefile.spl | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Acked-by: Chin Liang See <chin.liang.see@intel.com>

[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[Marek Vasut]

On 04/17/2018 10:52 AM, See, Chin Liang wrote:
> On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
>> On 04/17/2018 10:40 AM, See, Chin Liang wrote:
>>>
>>> Hi Marek,
>>>
>>> On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
>>>>
>>>> The Arria10 uses slightly different boot image header than the
>>>> Gen5
>>>> SoCs,
>>>> in particular the header itself contains an offset from the start
>>>> of
>>>> the
>>>> header to which the Arria10 jumps. This offset must not be
>>>> negative,
>>>> yet
>>>> the header is placed at offset 0x40 of the bootable binary.
>>>> Therefore, to
>>>> jump into U-Boot, add a trampoline just past the Arria10 boot
>>>> header
>>>> and
>>>> point to this trampoline at fixed offset from the header
>>>> generated
>>>> using
>>>> the mkimage -T socfpgaimage_v1 . Note that it is not needed to
>>>> jump
>>>> back
>>>> to offset 0x0 of the image, it is possible to jump directly at
>>>> the
>>>> reset
>>>> label and save processing two instructions.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>> ---
>>>>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>> b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>> index d6b9435d33..06bbe27d2c 100644
>>>> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>> @@ -18,10 +18,10 @@ _start:
>>>>  	.word	0xcafec0d3;	/* Checksum, zero-pad */
>>>>  	nop;
>>>>  
>>>> -	b reset;		/* SoCFPGA jumps here */
>>>> -	nop;
>>>> +	b reset;		/* SoCFPGA Gen5 jumps here */
>>>>  	nop;
>>>>  	nop;
>>>> +	b reset;		/* SoCFPGA Gen10 trampoline */
>>> Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder can
>>> we
>>> standardize that by using 0x14 instead of proposed 0x18 in this
>>> patch?
>> What difference does it make, the entire image is generated during
>> the
>> build anyway ? This patch uses offset 0x1c, but what is the reason
>> for
>> address 0x14 in your proprietary tool, is there one ?
> 
> Our A10 header ended at 0x13 today. Hence we are continuing the code at
> 0x14 without any spacing.
> 
> While for 0x1c, should it be 3 nop?

Yes, gives enough space were the header grow for whatever reason. Mind
you, the NOPs are not executed, the socfpga jumps to 0x1c directly via
0x0c -- Image entry offset

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[See, Chin Liang]

On Tue, 2018-04-17 at 11:01 +0200, Marek Vasut wrote:
> On 04/17/2018 10:52 AM, See, Chin Liang wrote:
> > 
> > On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
> > > 
> > > On 04/17/2018 10:40 AM, See, Chin Liang wrote:
> > > > 
> > > > 
> > > > Hi Marek,
> > > > 
> > > > On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > The Arria10 uses slightly different boot image header than
> > > > > the
> > > > > Gen5
> > > > > SoCs,
> > > > > in particular the header itself contains an offset from the
> > > > > start
> > > > > of
> > > > > the
> > > > > header to which the Arria10 jumps. This offset must not be
> > > > > negative,
> > > > > yet
> > > > > the header is placed at offset 0x40 of the bootable binary.
> > > > > Therefore, to
> > > > > jump into U-Boot, add a trampoline just past the Arria10 boot
> > > > > header
> > > > > and
> > > > > point to this trampoline at fixed offset from the header
> > > > > generated
> > > > > using
> > > > > the mkimage -T socfpgaimage_v1 . Note that it is not needed
> > > > > to
> > > > > jump
> > > > > back
> > > > > to offset 0x0 of the image, it is possible to jump directly
> > > > > at
> > > > > the
> > > > > reset
> > > > > label and save processing two instructions.
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > > > > Cc: Chin Liang See <chin.liang.see@intel.com>
> > > > > ---
> > > > >  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > index d6b9435d33..06bbe27d2c 100644
> > > > > --- a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > @@ -18,10 +18,10 @@ _start:
> > > > >  	.word	0xcafec0d3;	/* Checksum, zero-
> > > > > pad */
> > > > >  	nop;
> > > > >  
> > > > > -	b reset;		/* SoCFPGA jumps here */
> > > > > -	nop;
> > > > > +	b reset;		/* SoCFPGA Gen5 jumps here
> > > > > */
> > > > >  	nop;
> > > > >  	nop;
> > > > > +	b reset;		/* SoCFPGA Gen10 trampoline
> > > > > */
> > > > Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder
> > > > can
> > > > we
> > > > standardize that by using 0x14 instead of proposed 0x18 in this
> > > > patch?
> > > What difference does it make, the entire image is generated
> > > during
> > > the
> > > build anyway ? This patch uses offset 0x1c, but what is the
> > > reason
> > > for
> > > address 0x14 in your proprietary tool, is there one ?
> > Our A10 header ended at 0x13 today. Hence we are continuing the
> > code at
> > 0x14 without any spacing.
> > 
> > While for 0x1c, should it be 3 nop?
> Yes, gives enough space were the header grow for whatever reason.
> Mind
> you, the NOPs are not executed, the socfpga jumps to 0x1c directly
> via
> 0x0c -- Image entry offset

Ok, I don't have strong objection on this. We can claim that we don't
support use case where we use mkpimage tools from SCOEDS to sign SPL
binary from mainstream.

Acked-By: Chin Liang See <chin.liang.see@intel.com>


Thanks
Chin Liang

[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[Marek Vasut]

On 04/17/2018 11:11 AM, See, Chin Liang wrote:
> On Tue, 2018-04-17 at 11:01 +0200, Marek Vasut wrote:
>> On 04/17/2018 10:52 AM, See, Chin Liang wrote:
>>>
>>> On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
>>>>
>>>> On 04/17/2018 10:40 AM, See, Chin Liang wrote:
>>>>>
>>>>>
>>>>> Hi Marek,
>>>>>
>>>>> On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> The Arria10 uses slightly different boot image header than
>>>>>> the
>>>>>> Gen5
>>>>>> SoCs,
>>>>>> in particular the header itself contains an offset from the
>>>>>> start
>>>>>> of
>>>>>> the
>>>>>> header to which the Arria10 jumps. This offset must not be
>>>>>> negative,
>>>>>> yet
>>>>>> the header is placed at offset 0x40 of the bootable binary.
>>>>>> Therefore, to
>>>>>> jump into U-Boot, add a trampoline just past the Arria10 boot
>>>>>> header
>>>>>> and
>>>>>> point to this trampoline at fixed offset from the header
>>>>>> generated
>>>>>> using
>>>>>> the mkimage -T socfpgaimage_v1 . Note that it is not needed
>>>>>> to
>>>>>> jump
>>>>>> back
>>>>>> to offset 0x0 of the image, it is possible to jump directly
>>>>>> at
>>>>>> the
>>>>>> reset
>>>>>> label and save processing two instructions.
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>>>> ---
>>>>>>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>> b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>> index d6b9435d33..06bbe27d2c 100644
>>>>>> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>> @@ -18,10 +18,10 @@ _start:
>>>>>>  	.word	0xcafec0d3;	/* Checksum, zero-
>>>>>> pad */
>>>>>>  	nop;
>>>>>>  
>>>>>> -	b reset;		/* SoCFPGA jumps here */
>>>>>> -	nop;
>>>>>> +	b reset;		/* SoCFPGA Gen5 jumps here
>>>>>> */
>>>>>>  	nop;
>>>>>>  	nop;
>>>>>> +	b reset;		/* SoCFPGA Gen10 trampoline
>>>>>> */
>>>>> Our mkpimage tools from SOCEDS is using 0x14 as offset. Wonder
>>>>> can
>>>>> we
>>>>> standardize that by using 0x14 instead of proposed 0x18 in this
>>>>> patch?
>>>> What difference does it make, the entire image is generated
>>>> during
>>>> the
>>>> build anyway ? This patch uses offset 0x1c, but what is the
>>>> reason
>>>> for
>>>> address 0x14 in your proprietary tool, is there one ?
>>> Our A10 header ended at 0x13 today. Hence we are continuing the
>>> code at
>>> 0x14 without any spacing.
>>>
>>> While for 0x1c, should it be 3 nop?
>> Yes, gives enough space were the header grow for whatever reason.
>> Mind
>> you, the NOPs are not executed, the socfpga jumps to 0x1c directly
>> via
>> 0x0c -- Image entry offset
> 
> Ok, I don't have strong objection on this. We can claim that we don't
> support use case where we use mkpimage tools from SCOEDS to sign SPL
> binary from mainstream.

Which you can, why wouldn't it work ?

What is the benefit of using mkpimage if mkimage does the same thing
though ?

And what do you mean by "signing" ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[See, Chin Liang]

On Tue, 2018-04-17 at 11:28 +0200, Marek Vasut wrote:
> On 04/17/2018 11:11 AM, See, Chin Liang wrote:
> > 
> > On Tue, 2018-04-17 at 11:01 +0200, Marek Vasut wrote:
> > > 
> > > On 04/17/2018 10:52 AM, See, Chin Liang wrote:
> > > > 
> > > > 
> > > > On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 04/17/2018 10:40 AM, See, Chin Liang wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Hi Marek,
> > > > > > 
> > > > > > On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > The Arria10 uses slightly different boot image header
> > > > > > > than
> > > > > > > the
> > > > > > > Gen5
> > > > > > > SoCs,
> > > > > > > in particular the header itself contains an offset from
> > > > > > > the
> > > > > > > start
> > > > > > > of
> > > > > > > the
> > > > > > > header to which the Arria10 jumps. This offset must not
> > > > > > > be
> > > > > > > negative,
> > > > > > > yet
> > > > > > > the header is placed at offset 0x40 of the bootable
> > > > > > > binary.
> > > > > > > Therefore, to
> > > > > > > jump into U-Boot, add a trampoline just past the Arria10
> > > > > > > boot
> > > > > > > header
> > > > > > > and
> > > > > > > point to this trampoline at fixed offset from the header
> > > > > > > generated
> > > > > > > using
> > > > > > > the mkimage -T socfpgaimage_v1 . Note that it is not
> > > > > > > needed
> > > > > > > to
> > > > > > > jump
> > > > > > > back
> > > > > > > to offset 0x0 of the image, it is possible to jump
> > > > > > > directly
> > > > > > > at
> > > > > > > the
> > > > > > > reset
> > > > > > > label and save processing two instructions.
> > > > > > > 
> > > > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > > > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > > > > > > Cc: Chin Liang See <chin.liang.see@intel.com>
> > > > > > > ---
> > > > > > >  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
> > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > > > b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > > > index d6b9435d33..06bbe27d2c 100644
> > > > > > > --- a/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > > > +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
> > > > > > > @@ -18,10 +18,10 @@ _start:
> > > > > > >  	.word	0xcafec0d3;	/* Checksum,
> > > > > > > zero-
> > > > > > > pad */
> > > > > > >  	nop;
> > > > > > >  
> > > > > > > -	b reset;		/* SoCFPGA jumps here */
> > > > > > > -	nop;
> > > > > > > +	b reset;		/* SoCFPGA Gen5 jumps
> > > > > > > here
> > > > > > > */
> > > > > > >  	nop;
> > > > > > >  	nop;
> > > > > > > +	b reset;		/* SoCFPGA Gen10
> > > > > > > trampoline
> > > > > > > */
> > > > > > Our mkpimage tools from SOCEDS is using 0x14 as offset.
> > > > > > Wonder
> > > > > > can
> > > > > > we
> > > > > > standardize that by using 0x14 instead of proposed 0x18 in
> > > > > > this
> > > > > > patch?
> > > > > What difference does it make, the entire image is generated
> > > > > during
> > > > > the
> > > > > build anyway ? This patch uses offset 0x1c, but what is the
> > > > > reason
> > > > > for
> > > > > address 0x14 in your proprietary tool, is there one ?
> > > > Our A10 header ended at 0x13 today. Hence we are continuing the
> > > > code at
> > > > 0x14 without any spacing.
> > > > 
> > > > While for 0x1c, should it be 3 nop?
> > > Yes, gives enough space were the header grow for whatever reason.
> > > Mind
> > > you, the NOPs are not executed, the socfpga jumps to 0x1c
> > > directly
> > > via
> > > 0x0c -- Image entry offset
> > Ok, I don't have strong objection on this. We can claim that we
> > don't
> > support use case where we use mkpimage tools from SCOEDS to sign
> > SPL
> > binary from mainstream.
> Which you can, why wouldn't it work ?
> 

Rethinking on this, yes as the nop shall able to handle this. Just the
vice versa won't work since downstream U-Boot already have the entry
fixed at 0x14.


> What is the benefit of using mkpimage if mkimage does the same thing
> though ?

Initial thought is for having both tools compatible. But rethinking the
makefile will handle this, we just advise user directly use sfp file
directly.

Thanks
Chin Liang

[U-Boot] [PATCH 1/5] ARM: socfpga: Add boot trampoline for Arria10

[Marek Vasut]

On 04/19/2018 07:51 AM, See, Chin Liang wrote:
> On Tue, 2018-04-17 at 11:28 +0200, Marek Vasut wrote:
>> On 04/17/2018 11:11 AM, See, Chin Liang wrote:
>>>
>>> On Tue, 2018-04-17 at 11:01 +0200, Marek Vasut wrote:
>>>>
>>>> On 04/17/2018 10:52 AM, See, Chin Liang wrote:
>>>>>
>>>>>
>>>>> On Tue, 2018-04-17 at 10:46 +0200, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 04/17/2018 10:40 AM, See, Chin Liang wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hi Marek,
>>>>>>>
>>>>>>> On Sun, 2018-04-15 at 15:37 +0200, Marek Vasut wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The Arria10 uses slightly different boot image header
>>>>>>>> than
>>>>>>>> the
>>>>>>>> Gen5
>>>>>>>> SoCs,
>>>>>>>> in particular the header itself contains an offset from
>>>>>>>> the
>>>>>>>> start
>>>>>>>> of
>>>>>>>> the
>>>>>>>> header to which the Arria10 jumps. This offset must not
>>>>>>>> be
>>>>>>>> negative,
>>>>>>>> yet
>>>>>>>> the header is placed at offset 0x40 of the bootable
>>>>>>>> binary.
>>>>>>>> Therefore, to
>>>>>>>> jump into U-Boot, add a trampoline just past the Arria10
>>>>>>>> boot
>>>>>>>> header
>>>>>>>> and
>>>>>>>> point to this trampoline at fixed offset from the header
>>>>>>>> generated
>>>>>>>> using
>>>>>>>> the mkimage -T socfpgaimage_v1 . Note that it is not
>>>>>>>> needed
>>>>>>>> to
>>>>>>>> jump
>>>>>>>> back
>>>>>>>> to offset 0x0 of the image, it is possible to jump
>>>>>>>> directly
>>>>>>>> at
>>>>>>>> the
>>>>>>>> reset
>>>>>>>> label and save processing two instructions.
>>>>>>>>
>>>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>>>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>>>>>>>> Cc: Chin Liang See <chin.liang.see@intel.com>
>>>>>>>> ---
>>>>>>>>  arch/arm/mach-socfpga/include/mach/boot0.h | 4 ++--
>>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>>>> b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>>>> index d6b9435d33..06bbe27d2c 100644
>>>>>>>> --- a/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>>>> +++ b/arch/arm/mach-socfpga/include/mach/boot0.h
>>>>>>>> @@ -18,10 +18,10 @@ _start:
>>>>>>>>  	.word	0xcafec0d3;	/* Checksum,
>>>>>>>> zero-
>>>>>>>> pad */
>>>>>>>>  	nop;
>>>>>>>>  
>>>>>>>> -	b reset;		/* SoCFPGA jumps here */
>>>>>>>> -	nop;
>>>>>>>> +	b reset;		/* SoCFPGA Gen5 jumps
>>>>>>>> here
>>>>>>>> */
>>>>>>>>  	nop;
>>>>>>>>  	nop;
>>>>>>>> +	b reset;		/* SoCFPGA Gen10
>>>>>>>> trampoline
>>>>>>>> */
>>>>>>> Our mkpimage tools from SOCEDS is using 0x14 as offset.
>>>>>>> Wonder
>>>>>>> can
>>>>>>> we
>>>>>>> standardize that by using 0x14 instead of proposed 0x18 in
>>>>>>> this
>>>>>>> patch?
>>>>>> What difference does it make, the entire image is generated
>>>>>> during
>>>>>> the
>>>>>> build anyway ? This patch uses offset 0x1c, but what is the
>>>>>> reason
>>>>>> for
>>>>>> address 0x14 in your proprietary tool, is there one ?
>>>>> Our A10 header ended at 0x13 today. Hence we are continuing the
>>>>> code at
>>>>> 0x14 without any spacing.
>>>>>
>>>>> While for 0x1c, should it be 3 nop?
>>>> Yes, gives enough space were the header grow for whatever reason.
>>>> Mind
>>>> you, the NOPs are not executed, the socfpga jumps to 0x1c
>>>> directly
>>>> via
>>>> 0x0c -- Image entry offset
>>> Ok, I don't have strong objection on this. We can claim that we
>>> don't
>>> support use case where we use mkpimage tools from SCOEDS to sign
>>> SPL
>>> binary from mainstream.
>> Which you can, why wouldn't it work ?
>>
> 
> Rethinking on this, yes as the nop shall able to handle this. Just the
> vice versa won't work since downstream U-Boot already have the entry
> fixed at 0x14.
> 
> 
>> What is the benefit of using mkpimage if mkimage does the same thing
>> though ?
> 
> Initial thought is for having both tools compatible. But rethinking the
> makefile will handle this, we just advise user directly use sfp file
> directly.

I mean, I can place the trampoline to 0x14 , it doesn't really matter.
But is there some hidden gem in the SoCFPGA bootrom which might bite us
later ?

-- 
Best regards,
Marek Vasut