[U-Boot] [PATCH 1/3][v3] Pointers in ESBC header made 32 bit

[U-Boot] [PATCH 1/3][v3] Pointers in ESBC header made 32 bit

[Aneesh Bansal]

For the Chain of Trust, the esbc_validate command supports
32 bit fields for location of the image. In the header structure
definition, these were declared as pointers which made them
64 bit on a 64 bit core.

Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
---
Changes in v3:
Patch Rebased and removed compile time warnings

 board/freescale/common/fsl_validate.c | 20 ++++++++++----------
 include/fsl_validate.h                | 14 +++++++-------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/board/freescale/common/fsl_validate.c b/board/freescale/common/fsl_validate.c
index 5283648..465676f 100644
--- a/board/freescale/common/fsl_validate.c
+++ b/board/freescale/common/fsl_validate.c
@@ -63,12 +63,12 @@ static u32 check_ie(struct fsl_secboot_img_priv *img)
  * address
  */
 #if defined(CONFIG_MPC85xx)
-int get_csf_base_addr(ulong *csf_addr, ulong *flash_base_addr)
+int get_csf_base_addr(u32 *csf_addr, u32 *flash_base_addr)
 {
 	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
 	u32 csf_hdr_addr = in_be32(&gur->scratchrw[0]);
 	u32 csf_flash_offset = csf_hdr_addr & ~(CONFIG_SYS_PBI_FLASH_BASE);
-	ulong flash_addr, addr;
+	u32 flash_addr, addr;
 	int found = 0;
 	int i = 0;
 
@@ -76,7 +76,7 @@ int get_csf_base_addr(ulong *csf_addr, ulong *flash_base_addr)
 		flash_addr = flash_info[i].start[0];
 		addr = flash_info[i].start[0] + csf_flash_offset;
 		if (memcmp((u8 *)addr, barker_code, ESBC_BARKER_LEN) == 0) {
-			debug("Barker found on addr %lx\n", addr);
+			debug("Barker found on addr %x\n", addr);
 			found = 1;
 			break;
 		}
@@ -94,7 +94,7 @@ int get_csf_base_addr(ulong *csf_addr, ulong *flash_base_addr)
 /* For platforms like LS1020, correct flash address is present in
  * the header. So the function reqturns flash base address as 0
  */
-int get_csf_base_addr(ulong *csf_addr, ulong *flash_base_addr)
+int get_csf_base_addr(u32 *csf_addr, u32 *flash_base_addr)
 {
 	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
 	u32 csf_hdr_addr = in_be32(&gur->scratchrw[0]);
@@ -108,11 +108,11 @@ int get_csf_base_addr(ulong *csf_addr, ulong *flash_base_addr)
 }
 #endif
 
-static int get_ie_info_addr(ulong *ie_addr)
+static int get_ie_info_addr(u32 *ie_addr)
 {
 	struct fsl_secboot_img_hdr *hdr;
 	struct fsl_secboot_sg_table *sg_tbl;
-	ulong flash_base_addr, csf_addr;
+	u32 flash_base_addr, csf_addr;
 
 	if (get_csf_base_addr(&csf_addr, &flash_base_addr))
 		return -1;
@@ -127,11 +127,11 @@ static int get_ie_info_addr(ulong *ie_addr)
 	 */
 #if defined(CONFIG_FSL_TRUST_ARCH_v1) && defined(CONFIG_FSL_CORENET)
 	sg_tbl = (struct fsl_secboot_sg_table *)
-		 (((ulong)hdr->psgtable & ~(CONFIG_SYS_PBI_FLASH_BASE)) +
+		 (((u32)hdr->psgtable & ~(CONFIG_SYS_PBI_FLASH_BASE)) +
 		  flash_base_addr);
 #else
 	sg_tbl = (struct fsl_secboot_sg_table *)(csf_addr +
-						 (ulong)hdr->psgtable);
+						 (u32)hdr->psgtable);
 #endif
 
 	/* IE Key Table is the first entry in the SG Table */
@@ -142,7 +142,7 @@ static int get_ie_info_addr(ulong *ie_addr)
 	*ie_addr = sg_tbl->src_addr;
 #endif
 
-	debug("IE Table address is %lx\n", *ie_addr);
+	debug("IE Table address is %x\n", *ie_addr);
 	return 0;
 }
 
@@ -549,7 +549,7 @@ static int read_validate_esbc_client_header(struct fsl_secboot_img_priv *img)
 	if (memcmp(hdr->barker, barker_code, ESBC_BARKER_LEN))
 		return ERROR_ESBC_CLIENT_HEADER_BARKER;
 
-	sprintf(buf, "%p", hdr->pimg);
+	sprintf(buf, "%x", hdr->pimg);
 	setenv("img_addr", buf);
 
 	if (!hdr->img_size)
diff --git a/include/fsl_validate.h b/include/fsl_validate.h
index c460534..92dd98b 100644
--- a/include/fsl_validate.h
+++ b/include/fsl_validate.h
@@ -82,14 +82,14 @@ struct fsl_secboot_img_hdr {
 	u32 psign;		/* signature offset */
 	u32 sign_len;		/* length of the signature in bytes */
 	union {
-		struct fsl_secboot_sg_table *psgtable;	/* ptr to SG table */
-		u8 *pimg;	/* ptr to ESBC client image */
+		u32 psgtable;	/* ptr to SG table */
+		u32 pimg;	/* ptr to ESBC client image */
 	};
 	union {
 		u32 sg_entries;	/* no of entries in SG table */
 		u32 img_size;	/* ESBC client image size in bytes */
 	};
-	ulong img_start;		/* ESBC client entry point */
+	u32 img_start;		/* ESBC client entry point */
 	u32 sg_flag;		/* Scatter gather flag */
 	u32 uid_flag;
 	u32 fsl_uid_0;
@@ -133,7 +133,7 @@ struct srk_table {
  */
 struct fsl_secboot_sg_table {
 	u32 len;		/* length of the segment in bytes */
-	ulong src_addr;		/* ptr to the data segment */
+	u32 src_addr;		/* ptr to the data segment */
 };
 #else
 /*
@@ -146,8 +146,8 @@ struct fsl_secboot_sg_table {
 struct fsl_secboot_sg_table {
 	u32 len;
 	u32 trgt_id;
-	ulong src_addr;
-	ulong dst_addr;
+	u32 src_addr;
+	u32 dst_addr;
 };
 #endif
 
@@ -162,7 +162,7 @@ struct fsl_secboot_sg_table {
  */
 struct fsl_secboot_img_priv {
 	uint32_t hdr_location;
-	ulong ie_addr;
+	u32 ie_addr;
 	u32 key_len;
 	struct fsl_secboot_img_hdr hdr;
 
-- 
1.8.1.4

[U-Boot] [PATCH 2/3][v3] Data types defined for 64 bit physical address

[Aneesh Bansal]

Data types and I/O functions have been defined for
64 bit physical addresses in arm.

Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
---
Changes in v3:
Corrected the definition of virt_to_phys() and definition of phys_addr_t.

 arch/arm/include/asm/io.h    |  4 ++--
 arch/arm/include/asm/types.h | 10 +++++++---
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index bfbe0a0..75773bd 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -46,7 +46,7 @@ static inline void sync(void)
 static inline void *
 map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
 {
-	return (void *)paddr;
+	return (void *)((unsigned long)paddr);
 }
 
 /*
@@ -59,7 +59,7 @@ static inline void unmap_physmem(void *vaddr, unsigned long flags)
 
 static inline phys_addr_t virt_to_phys(void * vaddr)
 {
-	return (phys_addr_t)(vaddr);
+	return (phys_addr_t)((unsigned long)vaddr);
 }
 
 /*
diff --git a/arch/arm/include/asm/types.h b/arch/arm/include/asm/types.h
index ee77c41..388058e 100644
--- a/arch/arm/include/asm/types.h
+++ b/arch/arm/include/asm/types.h
@@ -45,12 +45,16 @@ typedef unsigned long long u64;
 #define BITS_PER_LONG 32
 #endif	/* CONFIG_ARM64 */
 
-/* Dma addresses are 32-bits wide.  */
-
+#ifdef CONFIG_PHYS_64BIT
+typedef unsigned long long dma_addr_t;
+typedef unsigned long long phys_addr_t;
+typedef unsigned long long phys_size_t;
+#else
+/* DMA addresses are 32-bits wide */
 typedef u32 dma_addr_t;
-
 typedef unsigned long phys_addr_t;
 typedef unsigned long phys_size_t;
+#endif
 
 #endif /* __KERNEL__ */
 
-- 
1.8.1.4

[U-Boot] [PATCH 3/3][v3] crypto/fsl: SEC driver cleanup for 64 bit and endianness

[Aneesh Bansal]

The SEC driver code has been cleaned up to work for 64 bit
physical addresses and systems where endianess of SEC block
is different from the Core.
Changes:
1. Descriptor created on Core is modified as per SEC block
   endianness before the job is submitted.
2. The read/write of physical addresses to Job Rings will
   be depend on endianness of SEC block as 32 bit low and
   high part of the 64 bit address will vary.
3. The 32 bit low and high part of the 64 bit address in
   descriptor will vary depending on endianness of SEC.

Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
---
Changes in v3:sec_out_phys and sec_in_phys 
The rwad/write for 64 bit address is done using 32 bit
aadr_lo and addr_hi. There is no need to define wrapper
functions sec_out_phys and sec_in_phys.

 drivers/crypto/fsl/desc_constr.h | 26 +++++++++++++
 drivers/crypto/fsl/fsl_hash.c    |  8 ++--
 drivers/crypto/fsl/jr.c          | 83 ++++++++++++++++++++++++++++++++++------
 drivers/crypto/fsl/jr.h          |  7 ++--
 include/fsl_sec.h                |  6 +--
 5 files changed, 106 insertions(+), 24 deletions(-)

diff --git a/drivers/crypto/fsl/desc_constr.h b/drivers/crypto/fsl/desc_constr.h
index f9cae91..2559ccd 100644
--- a/drivers/crypto/fsl/desc_constr.h
+++ b/drivers/crypto/fsl/desc_constr.h
@@ -36,6 +36,23 @@
 			       LDST_SRCDST_WORD_DECOCTRL | \
 			       (LDOFF_ENABLE_AUTO_NFIFO << LDST_OFFSET_SHIFT))
 
+#ifdef CONFIG_PHYS_64BIT
+union ptr_addr_t {
+	u64 m_whole;
+	struct {
+#ifdef CONFIG_SYS_FSL_SEC_LE
+		u32 low;
+		u32 high;
+#elif defined(CONFIG_SYS_FSL_SEC_BE)
+		u32 high;
+		u32 low;
+#else
+#error Neither CONFIG_SYS_FSL_SEC_LE nor CONFIG_SYS_FSL_SEC_BE is defined
+#endif
+	} m_halfs;
+};
+#endif
+
 static inline int desc_len(u32 *desc)
 {
 	return *desc & HDR_DESCLEN_MASK;
@@ -65,7 +82,16 @@ static inline void append_ptr(u32 *desc, dma_addr_t ptr)
 {
 	dma_addr_t *offset = (dma_addr_t *)desc_end(desc);
 
+#ifdef CONFIG_PHYS_64BIT
+	/* The Position of low and high part of 64 bit address
+	 * will depend on the endianness of CAAM Block */
+	union ptr_addr_t ptr_addr;
+	ptr_addr.m_halfs.high = (u32)(ptr >> 32);
+	ptr_addr.m_halfs.low = (u32)ptr;
+	*offset = ptr_addr.m_whole;
+#else
 	*offset = ptr;
+#endif
 
 	(*desc) += CAAM_PTR_SZ / CAAM_CMD_SZ;
 }
diff --git a/drivers/crypto/fsl/fsl_hash.c b/drivers/crypto/fsl/fsl_hash.c
index c298404..887e88c 100644
--- a/drivers/crypto/fsl/fsl_hash.c
+++ b/drivers/crypto/fsl/fsl_hash.c
@@ -84,7 +84,7 @@ static int caam_hash_update(void *hash_ctx, const void *buf,
 			    enum caam_hash_algos caam_algo)
 {
 	uint32_t final = 0;
-	dma_addr_t addr = virt_to_phys((void *)buf);
+	phys_addr_t addr = virt_to_phys((void *)buf);
 	struct sha_ctx *ctx = hash_ctx;
 
 	if (ctx->sg_num >= MAX_SG_32) {
@@ -93,11 +93,11 @@ static int caam_hash_update(void *hash_ctx, const void *buf,
 	}
 
 #ifdef CONFIG_PHYS_64BIT
-	ctx->sg_tbl[ctx->sg_num].addr_hi = addr >> 32;
+	sec_out32(&ctx->sg_tbl[ctx->sg_num].addr_hi, (uint32_t)(addr >> 32));
 #else
-	ctx->sg_tbl[ctx->sg_num].addr_hi = 0x0;
+	sec_out32(&ctx->sg_tbl[ctx->sg_num].addr_hi, 0x0);
 #endif
-	ctx->sg_tbl[ctx->sg_num].addr_lo = addr;
+	sec_out32(&ctx->sg_tbl[ctx->sg_num].addr_lo, (uint32_t)addr);
 
 	sec_out32(&ctx->sg_tbl[ctx->sg_num].len_flag,
 		  (size & SG_ENTRY_LENGTH_MASK));
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index 17392c9..c88c727 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -11,6 +11,7 @@
 #include "fsl_sec.h"
 #include "jr.h"
 #include "jobdesc.h"
+#include "desc_constr.h"
 
 #define CIRC_CNT(head, tail, size)	(((head) - (tail)) & (size - 1))
 #define CIRC_SPACE(head, tail, size)	CIRC_CNT((tail), (head) + 1, (size))
@@ -154,19 +155,35 @@ static int jr_hw_reset(void)
 
 /* -1 --- error, can't enqueue -- no space available */
 static int jr_enqueue(uint32_t *desc_addr,
-	       void (*callback)(uint32_t desc, uint32_t status, void *arg),
+	       void (*callback)(uint32_t status, void *arg),
 	       void *arg)
 {
 	struct jr_regs *regs = (struct jr_regs *)CONFIG_SYS_FSL_JR0_ADDR;
 	int head = jr.head;
-	dma_addr_t desc_phys_addr = virt_to_phys(desc_addr);
+	uint32_t desc_word;
+	int length = desc_len(desc_addr);
+	int i;
+#ifdef CONFIG_PHYS_64BIT
+	uint32_t *addr_hi, *addr_lo;
+#endif
+
+	/* The descriptor must be submitted to SEC block as per endianness
+	 * of the SEC Block.
+	 * So, if the endianness of Core and SEC block is different, each word
+	 * of the descriptor will be byte-swapped.
+	 */
+	for (i = 0; i < length; i++) {
+		desc_word = desc_addr[i];
+		sec_out32((uint32_t *)&desc_addr[i], desc_word);
+	}
+
+	phys_addr_t desc_phys_addr = virt_to_phys(desc_addr);
 
 	if (sec_in32(&regs->irsa) == 0 ||
 	    CIRC_SPACE(jr.head, jr.tail, jr.size) <= 0)
 		return -1;
 
 	jr.info[head].desc_phys_addr = desc_phys_addr;
-	jr.info[head].desc_addr = (uint32_t)desc_addr;
 	jr.info[head].callback = (void *)callback;
 	jr.info[head].arg = arg;
 	jr.info[head].op_done = 0;
@@ -177,9 +194,29 @@ static int jr_enqueue(uint32_t *desc_addr,
 					ARCH_DMA_MINALIGN);
 	flush_dcache_range(start, end);
 
-	jr.input_ring[head] = desc_phys_addr;
+#ifdef CONFIG_PHYS_64BIT
+	/* Write the 64 bit Descriptor address on Input Ring.
+	 * The 32 bit hign and low part of the address will
+	 * depend on endianness of SEC block.
+	 */
+#ifdef CONFIG_SYS_FSL_SEC_LE
+	addr_lo = (uint32_t *)(&jr.input_ring[head]);
+	addr_hi = (uint32_t *)(&jr.input_ring[head]) + 1;
+#elif defined(CONFIG_SYS_FSL_SEC_BE)
+	addr_hi = (uint32_t *)(&jr.input_ring[head]);
+	addr_lo = (uint32_t *)(&jr.input_ring[head]) + 1;
+#endif /* ifdef CONFIG_SYS_FSL_SEC_LE */
+
+	sec_out32(addr_hi, (uint32_t)(desc_phys_addr >> 32));
+	sec_out32(addr_lo, (uint32_t)(desc_phys_addr));
+
+#else
+	/* Write the 32 bit Descriptor address on Input Ring. */
+	sec_out32(&jr.input_ring[head], desc_phys_addr);
+#endif /* ifdef CONFIG_PHYS_64BIT */
+
 	start = (unsigned long)&jr.input_ring[head] & ~(ARCH_DMA_MINALIGN - 1);
-	end = ALIGN(start + sizeof(dma_addr_t), ARCH_DMA_MINALIGN);
+	end = ALIGN(start + sizeof(phys_addr_t), ARCH_DMA_MINALIGN);
 	flush_dcache_range(start, end);
 
 	jr.head = (head + 1) & (jr.size - 1);
@@ -195,8 +232,11 @@ static int jr_dequeue(void)
 	int head = jr.head;
 	int tail = jr.tail;
 	int idx, i, found;
-	void (*callback)(uint32_t desc, uint32_t status, void *arg);
+	void (*callback)(uint32_t status, void *arg);
 	void *arg = NULL;
+#ifdef CONFIG_PHYS_64BIT
+	uint32_t *addr_hi, *addr_lo;
+#endif
 
 	while (sec_in32(&regs->orsf) && CIRC_CNT(jr.head, jr.tail, jr.size)) {
 		unsigned long start = (unsigned long)jr.output_ring &
@@ -208,14 +248,33 @@ static int jr_dequeue(void)
 
 		found = 0;
 
-		dma_addr_t op_desc = jr.output_ring[jr.tail].desc;
-		uint32_t status = jr.output_ring[jr.tail].status;
-		uint32_t desc_virt;
+		phys_addr_t op_desc;
+	#ifdef CONFIG_PHYS_64BIT
+		/* Read the 64 bit Descriptor address from Output Ring.
+		 * The 32 bit hign and low part of the address will
+		 * depend on endianness of SEC block.
+		 */
+	#ifdef CONFIG_SYS_FSL_SEC_LE
+		addr_lo = (uint32_t *)(&jr.output_ring[jr.tail].desc);
+		addr_hi = (uint32_t *)(&jr.output_ring[jr.tail].desc) + 1;
+	#elif defined(CONFIG_SYS_FSL_SEC_BE)
+		addr_hi = (uint32_t *)(&jr.output_ring[jr.tail].desc);
+		addr_lo = (uint32_t *)(&jr.output_ring[jr.tail].desc) + 1;
+	#endif /* ifdef CONFIG_SYS_FSL_SEC_LE */
+
+		op_desc = ((u64)sec_in32(addr_hi) << 32) |
+			  ((u64)sec_in32(addr_lo));
+
+	#else
+		/* Read the 32 bit Descriptor address from Output Ring. */
+		op_desc = sec_in32(&jr.output_ring[jr.tail].desc);
+	#endif /* ifdef CONFIG_PHYS_64BIT */
+
+		uint32_t status = sec_in32(&jr.output_ring[jr.tail].status);
 
 		for (i = 0; CIRC_CNT(head, tail + i, jr.size) >= 1; i++) {
 			idx = (tail + i) & (jr.size - 1);
 			if (op_desc == jr.info[idx].desc_phys_addr) {
-				desc_virt = jr.info[idx].desc_addr;
 				found = 1;
 				break;
 			}
@@ -244,13 +303,13 @@ static int jr_dequeue(void)
 		sec_out32(&regs->orjr, 1);
 		jr.info[idx].op_done = 0;
 
-		callback(desc_virt, status, arg);
+		callback(status, arg);
 	}
 
 	return 0;
 }
 
-static void desc_done(uint32_t desc, uint32_t status, void *arg)
+static void desc_done(uint32_t status, void *arg)
 {
 	struct result *x = arg;
 	x->status = status;
diff --git a/drivers/crypto/fsl/jr.h b/drivers/crypto/fsl/jr.h
index 1526060..5899696 100644
--- a/drivers/crypto/fsl/jr.h
+++ b/drivers/crypto/fsl/jr.h
@@ -37,14 +37,13 @@
 #define JQ_ENQ_ERR		-3
 
 struct op_ring {
-	dma_addr_t desc;
+	phys_addr_t desc;
 	uint32_t status;
 } __packed;
 
 struct jr_info {
-	void (*callback)(dma_addr_t desc, uint32_t status, void *arg);
-	dma_addr_t desc_phys_addr;
-	uint32_t desc_addr;
+	void (*callback)(uint32_t status, void *arg);
+	phys_addr_t desc_phys_addr;
 	uint32_t desc_len;
 	uint32_t op_done;
 	void *arg;
diff --git a/include/fsl_sec.h b/include/fsl_sec.h
index abc62da..2ddced3 100644
--- a/include/fsl_sec.h
+++ b/include/fsl_sec.h
@@ -194,11 +194,9 @@ struct jr_regs {
 struct sg_entry {
 #if defined(CONFIG_SYS_FSL_SEC_LE) && !defined(CONFIG_MX6)
 	uint32_t addr_lo;	/* Memory Address - lo */
-	uint16_t addr_hi;	/* Memory Address of start of buffer - hi */
-	uint16_t reserved_zero;
+	uint32_t addr_hi;	/* Memory Address of start of buffer - hi */
 #else
-	uint16_t reserved_zero;
-	uint16_t addr_hi;	/* Memory Address of start of buffer - hi */
+	uint32_t addr_hi;	/* Memory Address of start of buffer - hi */
 	uint32_t addr_lo;	/* Memory Address - lo */
 #endif
 
-- 
1.8.1.4

[U-Boot] [PATCH 3/3][v3] crypto/fsl: SEC driver cleanup for 64 bit and endianness

[York Sun]

On 09/17/2015 03:46 AM, Aneesh Bansal wrote:
> The SEC driver code has been cleaned up to work for 64 bit
> physical addresses and systems where endianess of SEC block
> is different from the Core.
> Changes:
> 1. Descriptor created on Core is modified as per SEC block
>    endianness before the job is submitted.
> 2. The read/write of physical addresses to Job Rings will
>    be depend on endianness of SEC block as 32 bit low and
>    high part of the 64 bit address will vary.
> 3. The 32 bit low and high part of the 64 bit address in
>    descriptor will vary depending on endianness of SEC.
> 
> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> ---
> Changes in v3:sec_out_phys and sec_in_phys 
> The rwad/write for 64 bit address is done using 32 bit
> aadr_lo and addr_hi. There is no need to define wrapper
> functions sec_out_phys and sec_in_phys.
> 
>  drivers/crypto/fsl/desc_constr.h | 26 +++++++++++++
>  drivers/crypto/fsl/fsl_hash.c    |  8 ++--
>  drivers/crypto/fsl/jr.c          | 83 ++++++++++++++++++++++++++++++++++------
>  drivers/crypto/fsl/jr.h          |  7 ++--
>  include/fsl_sec.h                |  6 +--
>  5 files changed, 106 insertions(+), 24 deletions(-)
> 

<snip>

> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
> index 17392c9..c88c727 100644
> --- a/drivers/crypto/fsl/jr.c
> +++ b/drivers/crypto/fsl/jr.c
> @@ -11,6 +11,7 @@
>  #include "fsl_sec.h"
>  #include "jr.h"
>  #include "jobdesc.h"
> +#include "desc_constr.h"
>  
>  #define CIRC_CNT(head, tail, size)	(((head) - (tail)) & (size - 1))
>  #define CIRC_SPACE(head, tail, size)	CIRC_CNT((tail), (head) + 1, (size))
> @@ -154,19 +155,35 @@ static int jr_hw_reset(void)
>  
>  /* -1 --- error, can't enqueue -- no space available */
>  static int jr_enqueue(uint32_t *desc_addr,
> -	       void (*callback)(uint32_t desc, uint32_t status, void *arg),
> +	       void (*callback)(uint32_t status, void *arg),
>  	       void *arg)
>  {
>  	struct jr_regs *regs = (struct jr_regs *)CONFIG_SYS_FSL_JR0_ADDR;
>  	int head = jr.head;
> -	dma_addr_t desc_phys_addr = virt_to_phys(desc_addr);
> +	uint32_t desc_word;
> +	int length = desc_len(desc_addr);
> +	int i;
> +#ifdef CONFIG_PHYS_64BIT
> +	uint32_t *addr_hi, *addr_lo;
> +#endif
> +
> +	/* The descriptor must be submitted to SEC block as per endianness
> +	 * of the SEC Block.
> +	 * So, if the endianness of Core and SEC block is different, each word
> +	 * of the descriptor will be byte-swapped.
> +	 */
> +	for (i = 0; i < length; i++) {
> +		desc_word = desc_addr[i];
> +		sec_out32((uint32_t *)&desc_addr[i], desc_word);
> +	}
> +
> +	phys_addr_t desc_phys_addr = virt_to_phys(desc_addr);
>  
>  	if (sec_in32(&regs->irsa) == 0 ||
>  	    CIRC_SPACE(jr.head, jr.tail, jr.size) <= 0)
>  		return -1;
>  
>  	jr.info[head].desc_phys_addr = desc_phys_addr;
> -	jr.info[head].desc_addr = (uint32_t)desc_addr;
>  	jr.info[head].callback = (void *)callback;
>  	jr.info[head].arg = arg;
>  	jr.info[head].op_done = 0;
> @@ -177,9 +194,29 @@ static int jr_enqueue(uint32_t *desc_addr,
>  					ARCH_DMA_MINALIGN);
>  	flush_dcache_range(start, end);
>  
> -	jr.input_ring[head] = desc_phys_addr;
> +#ifdef CONFIG_PHYS_64BIT
> +	/* Write the 64 bit Descriptor address on Input Ring.
> +	 * The 32 bit hign and low part of the address will
> +	 * depend on endianness of SEC block.
> +	 */
> +#ifdef CONFIG_SYS_FSL_SEC_LE
> +	addr_lo = (uint32_t *)(&jr.input_ring[head]);
> +	addr_hi = (uint32_t *)(&jr.input_ring[head]) + 1;
> +#elif defined(CONFIG_SYS_FSL_SEC_BE)
> +	addr_hi = (uint32_t *)(&jr.input_ring[head]);
> +	addr_lo = (uint32_t *)(&jr.input_ring[head]) + 1;
> +#endif /* ifdef CONFIG_SYS_FSL_SEC_LE */
> +
> +	sec_out32(addr_hi, (uint32_t)(desc_phys_addr >> 32));
> +	sec_out32(addr_lo, (uint32_t)(desc_phys_addr));
> +
> +#else
> +	/* Write the 32 bit Descriptor address on Input Ring. */
> +	sec_out32(&jr.input_ring[head], desc_phys_addr);
> +#endif /* ifdef CONFIG_PHYS_64BIT */
> +
>  	start = (unsigned long)&jr.input_ring[head] & ~(ARCH_DMA_MINALIGN - 1);
> -	end = ALIGN(start + sizeof(dma_addr_t), ARCH_DMA_MINALIGN);
> +	end = ALIGN(start + sizeof(phys_addr_t), ARCH_DMA_MINALIGN);
>  	flush_dcache_range(start, end);
>  
>  	jr.head = (head + 1) & (jr.size - 1);
> @@ -195,8 +232,11 @@ static int jr_dequeue(void)
>  	int head = jr.head;
>  	int tail = jr.tail;
>  	int idx, i, found;
> -	void (*callback)(uint32_t desc, uint32_t status, void *arg);
> +	void (*callback)(uint32_t status, void *arg);
>  	void *arg = NULL;
> +#ifdef CONFIG_PHYS_64BIT
> +	uint32_t *addr_hi, *addr_lo;
> +#endif
>  
>  	while (sec_in32(&regs->orsf) && CIRC_CNT(jr.head, jr.tail, jr.size)) {
>  		unsigned long start = (unsigned long)jr.output_ring &
> @@ -208,14 +248,33 @@ static int jr_dequeue(void)
>  
>  		found = 0;
>  
> -		dma_addr_t op_desc = jr.output_ring[jr.tail].desc;
> -		uint32_t status = jr.output_ring[jr.tail].status;
> -		uint32_t desc_virt;
> +		phys_addr_t op_desc;
> +	#ifdef CONFIG_PHYS_64BIT
> +		/* Read the 64 bit Descriptor address from Output Ring.
> +		 * The 32 bit hign and low part of the address will
> +		 * depend on endianness of SEC block.
> +		 */
> +	#ifdef CONFIG_SYS_FSL_SEC_LE
> +		addr_lo = (uint32_t *)(&jr.output_ring[jr.tail].desc);
> +		addr_hi = (uint32_t *)(&jr.output_ring[jr.tail].desc) + 1;
> +	#elif defined(CONFIG_SYS_FSL_SEC_BE)
> +		addr_hi = (uint32_t *)(&jr.output_ring[jr.tail].desc);
> +		addr_lo = (uint32_t *)(&jr.output_ring[jr.tail].desc) + 1;
> +	#endif /* ifdef CONFIG_SYS_FSL_SEC_LE */
> +
> +		op_desc = ((u64)sec_in32(addr_hi) << 32) |
> +			  ((u64)sec_in32(addr_lo));
> +
> +	#else
> +		/* Read the 32 bit Descriptor address from Output Ring. */
> +		op_desc = sec_in32(&jr.output_ring[jr.tail].desc);

I got a warning when compiling for ls1021a targets

../drivers/crypto/fsl/jr.c: In function ?jr_dequeue?:
../drivers/crypto/fsl/jr.c:270:3: warning: dereferencing type-punned pointer
will break strict-aliasing rules [-Wstrict-aliasing]

York

[U-Boot] [PATCH 3/3][v3] crypto/fsl: SEC driver cleanup for 64 bit and endianness

[Bansal Aneesh]

> -----Original Message-----
> From: York Sun [mailto:yorksun at freescale.com]
> Sent: Thursday, October 15, 2015 10:20 PM
> To: Bansal Aneesh-B39320 <aneesh.bansal@freescale.com>; u-boot at lists.denx.de
> Cc: Wood Scott-B07421 <scottwood@freescale.com>; Gupta Ruchika-R66431
> <ruchika.gupta@freescale.com>; Kushwaha Prabhakar-B32579
> <prabhakar@freescale.com>
> Subject: Re: [PATCH 3/3][v3] crypto/fsl: SEC driver cleanup for 64 bit and
> endianness
> 
> 
> 
> On 09/17/2015 03:46 AM, Aneesh Bansal wrote:
> > The SEC driver code has been cleaned up to work for 64 bit physical
> > addresses and systems where endianess of SEC block is different from
> > the Core.
> > Changes:
> > 1. Descriptor created on Core is modified as per SEC block
> >    endianness before the job is submitted.
> > 2. The read/write of physical addresses to Job Rings will
> >    be depend on endianness of SEC block as 32 bit low and
> >    high part of the 64 bit address will vary.
> > 3. The 32 bit low and high part of the 64 bit address in
> >    descriptor will vary depending on endianness of SEC.
> >
> > Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> > ---
> > Changes in v3:sec_out_phys and sec_in_phys The rwad/write for 64 bit
> > address is done using 32 bit aadr_lo and addr_hi. There is no need to
> > define wrapper functions sec_out_phys and sec_in_phys.
> >
> >  drivers/crypto/fsl/desc_constr.h | 26 +++++++++++++
> >  drivers/crypto/fsl/fsl_hash.c    |  8 ++--
> >  drivers/crypto/fsl/jr.c          | 83 ++++++++++++++++++++++++++++++++++------
> >  drivers/crypto/fsl/jr.h          |  7 ++--
> >  include/fsl_sec.h                |  6 +--
> >  5 files changed, 106 insertions(+), 24 deletions(-)
> >
> 
> <snip>
> 
> > diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index
> > 17392c9..c88c727 100644
> > --- a/drivers/crypto/fsl/jr.c
> > +++ b/drivers/crypto/fsl/jr.c
> > @@ -11,6 +11,7 @@
> >  #include "fsl_sec.h"
> >  #include "jr.h"
> >  #include "jobdesc.h"
> > +#include "desc_constr.h"
> >
> >  #define CIRC_CNT(head, tail, size)	(((head) - (tail)) & (size - 1))
> >  #define CIRC_SPACE(head, tail, size)	CIRC_CNT((tail), (head) + 1, (size))
> > @@ -154,19 +155,35 @@ static int jr_hw_reset(void)
> >
> >  /* -1 --- error, can't enqueue -- no space available */  static int
> > jr_enqueue(uint32_t *desc_addr,
> > -	       void (*callback)(uint32_t desc, uint32_t status, void *arg),
> > +	       void (*callback)(uint32_t status, void *arg),
> >  	       void *arg)
> >  {
> >  	struct jr_regs *regs = (struct jr_regs *)CONFIG_SYS_FSL_JR0_ADDR;
> >  	int head = jr.head;
> > -	dma_addr_t desc_phys_addr = virt_to_phys(desc_addr);
> > +	uint32_t desc_word;
> > +	int length = desc_len(desc_addr);
> > +	int i;
> > +#ifdef CONFIG_PHYS_64BIT
> > +	uint32_t *addr_hi, *addr_lo;
> > +#endif
> > +
> > +	/* The descriptor must be submitted to SEC block as per endianness
> > +	 * of the SEC Block.
> > +	 * So, if the endianness of Core and SEC block is different, each word
> > +	 * of the descriptor will be byte-swapped.
> > +	 */
> > +	for (i = 0; i < length; i++) {
> > +		desc_word = desc_addr[i];
> > +		sec_out32((uint32_t *)&desc_addr[i], desc_word);
> > +	}
> > +
> > +	phys_addr_t desc_phys_addr = virt_to_phys(desc_addr);
> >
> >  	if (sec_in32(&regs->irsa) == 0 ||
> >  	    CIRC_SPACE(jr.head, jr.tail, jr.size) <= 0)
> >  		return -1;
> >
> >  	jr.info[head].desc_phys_addr = desc_phys_addr;
> > -	jr.info[head].desc_addr = (uint32_t)desc_addr;
> >  	jr.info[head].callback = (void *)callback;
> >  	jr.info[head].arg = arg;
> >  	jr.info[head].op_done = 0;
> > @@ -177,9 +194,29 @@ static int jr_enqueue(uint32_t *desc_addr,
> >  					ARCH_DMA_MINALIGN);
> >  	flush_dcache_range(start, end);
> >
> > -	jr.input_ring[head] = desc_phys_addr;
> > +#ifdef CONFIG_PHYS_64BIT
> > +	/* Write the 64 bit Descriptor address on Input Ring.
> > +	 * The 32 bit hign and low part of the address will
> > +	 * depend on endianness of SEC block.
> > +	 */
> > +#ifdef CONFIG_SYS_FSL_SEC_LE
> > +	addr_lo = (uint32_t *)(&jr.input_ring[head]);
> > +	addr_hi = (uint32_t *)(&jr.input_ring[head]) + 1; #elif
> > +defined(CONFIG_SYS_FSL_SEC_BE)
> > +	addr_hi = (uint32_t *)(&jr.input_ring[head]);
> > +	addr_lo = (uint32_t *)(&jr.input_ring[head]) + 1; #endif /* ifdef
> > +CONFIG_SYS_FSL_SEC_LE */
> > +
> > +	sec_out32(addr_hi, (uint32_t)(desc_phys_addr >> 32));
> > +	sec_out32(addr_lo, (uint32_t)(desc_phys_addr));
> > +
> > +#else
> > +	/* Write the 32 bit Descriptor address on Input Ring. */
> > +	sec_out32(&jr.input_ring[head], desc_phys_addr); #endif /* ifdef
> > +CONFIG_PHYS_64BIT */
> > +
> >  	start = (unsigned long)&jr.input_ring[head] & ~(ARCH_DMA_MINALIGN -
> 1);
> > -	end = ALIGN(start + sizeof(dma_addr_t), ARCH_DMA_MINALIGN);
> > +	end = ALIGN(start + sizeof(phys_addr_t), ARCH_DMA_MINALIGN);
> >  	flush_dcache_range(start, end);
> >
> >  	jr.head = (head + 1) & (jr.size - 1); @@ -195,8 +232,11 @@ static
> > int jr_dequeue(void)
> >  	int head = jr.head;
> >  	int tail = jr.tail;
> >  	int idx, i, found;
> > -	void (*callback)(uint32_t desc, uint32_t status, void *arg);
> > +	void (*callback)(uint32_t status, void *arg);
> >  	void *arg = NULL;
> > +#ifdef CONFIG_PHYS_64BIT
> > +	uint32_t *addr_hi, *addr_lo;
> > +#endif
> >
> >  	while (sec_in32(&regs->orsf) && CIRC_CNT(jr.head, jr.tail, jr.size)) {
> >  		unsigned long start = (unsigned long)jr.output_ring & @@ -208,14
> > +248,33 @@ static int jr_dequeue(void)
> >
> >  		found = 0;
> >
> > -		dma_addr_t op_desc = jr.output_ring[jr.tail].desc;
> > -		uint32_t status = jr.output_ring[jr.tail].status;
> > -		uint32_t desc_virt;
> > +		phys_addr_t op_desc;
> > +	#ifdef CONFIG_PHYS_64BIT
> > +		/* Read the 64 bit Descriptor address from Output Ring.
> > +		 * The 32 bit hign and low part of the address will
> > +		 * depend on endianness of SEC block.
> > +		 */
> > +	#ifdef CONFIG_SYS_FSL_SEC_LE
> > +		addr_lo = (uint32_t *)(&jr.output_ring[jr.tail].desc);
> > +		addr_hi = (uint32_t *)(&jr.output_ring[jr.tail].desc) + 1;
> > +	#elif defined(CONFIG_SYS_FSL_SEC_BE)
> > +		addr_hi = (uint32_t *)(&jr.output_ring[jr.tail].desc);
> > +		addr_lo = (uint32_t *)(&jr.output_ring[jr.tail].desc) + 1;
> > +	#endif /* ifdef CONFIG_SYS_FSL_SEC_LE */
> > +
> > +		op_desc = ((u64)sec_in32(addr_hi) << 32) |
> > +			  ((u64)sec_in32(addr_lo));
> > +
> > +	#else
> > +		/* Read the 32 bit Descriptor address from Output Ring. */
> > +		op_desc = sec_in32(&jr.output_ring[jr.tail].desc);
> 
> I got a warning when compiling for ls1021a targets
> 
> ../drivers/crypto/fsl/jr.c: In function 'jr_dequeue':
> ../drivers/crypto/fsl/jr.c:270:3: warning: dereferencing type-punned pointer will break
> strict-aliasing rules [-Wstrict-aliasing]
> 
> York
Warning has been removed and a new version of patch has been sent.

-Aneesh

[U-Boot] [PATCH 1/3][v3] Pointers in ESBC header made 32 bit

[York Sun]

On 09/17/2015 03:46 AM, Aneesh Bansal wrote:
> For the Chain of Trust, the esbc_validate command supports
> 32 bit fields for location of the image. In the header structure
> definition, these were declared as pointers which made them
> 64 bit on a 64 bit core.
> 
> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> ---
> Changes in v3:
> Patch Rebased and removed compile time warnings


Applied to u-boot-fsl-qoriq. Awaiting upstream. Thanks.

York

[U-Boot] [PATCH 2/3][v3] Data types defined for 64 bit physical address

[York Sun]

On 09/17/2015 03:46 AM, Aneesh Bansal wrote:
> Data types and I/O functions have been defined for
> 64 bit physical addresses in arm.
> 
> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> ---
> Changes in v3:
> Corrected the definition of virt_to_phys() and definition of phys_addr_t.


Applied to u-boot-fsl-qoriq. Awaiting upstream. Thanks.

York

[U-Boot] [PATCH 2/3][v3] Data types defined for 64 bit physical address

[york sun]

On 09/17/2015 03:48 AM, Aneesh Bansal wrote:
> Data types and I/O functions have been defined for
> 64 bit physical addresses in arm.
> 
> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> ---
> Changes in v3:
> Corrected the definition of virt_to_phys() and definition of phys_addr_t.
> 
>  arch/arm/include/asm/io.h    |  4 ++--
>  arch/arm/include/asm/types.h | 10 +++++++---
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> index bfbe0a0..75773bd 100644
> --- a/arch/arm/include/asm/io.h
> +++ b/arch/arm/include/asm/io.h
> @@ -46,7 +46,7 @@ static inline void sync(void)
>  static inline void *
>  map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
>  {
> -	return (void *)paddr;
> +	return (void *)((unsigned long)paddr);
>  }
>  
>  /*
> @@ -59,7 +59,7 @@ static inline void unmap_physmem(void *vaddr, unsigned long flags)
>  
>  static inline phys_addr_t virt_to_phys(void * vaddr)
>  {
> -	return (phys_addr_t)(vaddr);
> +	return (phys_addr_t)((unsigned long)vaddr);
>  }
>  
>  /*
> diff --git a/arch/arm/include/asm/types.h b/arch/arm/include/asm/types.h
> index ee77c41..388058e 100644
> --- a/arch/arm/include/asm/types.h
> +++ b/arch/arm/include/asm/types.h
> @@ -45,12 +45,16 @@ typedef unsigned long long u64;
>  #define BITS_PER_LONG 32
>  #endif	/* CONFIG_ARM64 */
>  
> -/* Dma addresses are 32-bits wide.  */
> -
> +#ifdef CONFIG_PHYS_64BIT
> +typedef unsigned long long dma_addr_t;
> +typedef unsigned long long phys_addr_t;
> +typedef unsigned long long phys_size_t;
> +#else
> +/* DMA addresses are 32-bits wide */
>  typedef u32 dma_addr_t;
> -
>  typedef unsigned long phys_addr_t;
>  typedef unsigned long phys_size_t;
> +#endif
>  
>  #endif /* __KERNEL__ */
>  
> 

Aneesh and Scott,

I need to revisit this patch. Would it be better to change it as below?

+#if defined(CONFIG_PHYS_64BIT) && !defined(CONFIG_ARM64)
+typedef unsigned long long dma_addr_t;
+typedef unsigned long long phys_addr_t;
+typedef unsigned long long phys_size_t;
+#else
+/* DMA addresses are 32-bits wide */
 typedef u32 dma_addr_t;
-
 typedef unsigned long phys_addr_t;
 typedef unsigned long phys_size_t;
+#endif

I am debugging another patch and found changing phys_addr_t makes some trouble
for ARM64, especially to mix with ulong.

York

[U-Boot] [PATCH 2/3][v3] Data types defined for 64 bit physical address

[Scott Wood]

On Wed, 2016-02-10 at 02:30 +0000, york sun wrote:
> On 09/17/2015 03:48 AM, Aneesh Bansal wrote:
> > Data types and I/O functions have been defined for
> > 64 bit physical addresses in arm.
> > 
> > Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> > ---
> > Changes in v3:
> > Corrected the definition of virt_to_phys() and definition of phys_addr_t.
> > 
> >  arch/arm/include/asm/io.h    |  4 ++--
> >  arch/arm/include/asm/types.h | 10 +++++++---
> >  2 files changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
> > index bfbe0a0..75773bd 100644
> > --- a/arch/arm/include/asm/io.h
> > +++ b/arch/arm/include/asm/io.h
> > @@ -46,7 +46,7 @@ static inline void sync(void)
> >  static inline void *
> >  map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
> >  {
> > -	return (void *)paddr;
> > +	return (void *)((unsigned long)paddr);
> >  }
> >  
> >  /*
> > @@ -59,7 +59,7 @@ static inline void unmap_physmem(void *vaddr, unsigned
> > long flags)
> >  
> >  static inline phys_addr_t virt_to_phys(void * vaddr)
> >  {
> > -	return (phys_addr_t)(vaddr);
> > +	return (phys_addr_t)((unsigned long)vaddr);
> >  }
> >  
> >  /*
> > diff --git a/arch/arm/include/asm/types.h b/arch/arm/include/asm/types.h
> > index ee77c41..388058e 100644
> > --- a/arch/arm/include/asm/types.h
> > +++ b/arch/arm/include/asm/types.h
> > @@ -45,12 +45,16 @@ typedef unsigned long long u64;
> >  #define BITS_PER_LONG 32
> >  #endif	/* CONFIG_ARM64 */
> >  
> > -/* Dma addresses are 32-bits wide.  */
> > -
> > +#ifdef CONFIG_PHYS_64BIT
> > +typedef unsigned long long dma_addr_t;
> > +typedef unsigned long long phys_addr_t;
> > +typedef unsigned long long phys_size_t;
> > +#else
> > +/* DMA addresses are 32-bits wide */
> >  typedef u32 dma_addr_t;
> > -
> >  typedef unsigned long phys_addr_t;
> >  typedef unsigned long phys_size_t;
> > +#endif
> >  
> >  #endif /* __KERNEL__ */
> >  
> > 
> 
> Aneesh and Scott,
> 
> I need to revisit this patch. Would it be better to change it as below?
> 
> +#if defined(CONFIG_PHYS_64BIT) && !defined(CONFIG_ARM64)
> +typedef unsigned long long dma_addr_t;
> +typedef unsigned long long phys_addr_t;
> +typedef unsigned long long phys_size_t;
> +#else
> +/* DMA addresses are 32-bits wide */
>  typedef u32 dma_addr_t;
> -
>  typedef unsigned long phys_addr_t;
>  typedef unsigned long phys_size_t;
> +#endif
> 
> I am debugging another patch and found changing phys_addr_t makes some
> trouble
> for ARM64, especially to mix with ulong.

What sort of trouble is it causing?  And why would you mix it with ulong?

-Scott

[U-Boot] [PATCH 2/3][v3] Data types defined for 64 bit physical address

[york sun]

On 02/09/2016 09:10 PM, Scott Wood wrote:
> On Wed, 2016-02-10 at 02:30 +0000, york sun wrote:

<snip>

>>
>> Aneesh and Scott,
>>
>> I need to revisit this patch. Would it be better to change it as below?
>>
>> +#if defined(CONFIG_PHYS_64BIT) && !defined(CONFIG_ARM64)
>> +typedef unsigned long long dma_addr_t;
>> +typedef unsigned long long phys_addr_t;
>> +typedef unsigned long long phys_size_t;
>> +#else
>> +/* DMA addresses are 32-bits wide */
>>  typedef u32 dma_addr_t;
>> -
>>  typedef unsigned long phys_addr_t;
>>  typedef unsigned long phys_size_t;
>> +#endif
>>
>> I am debugging another patch and found changing phys_addr_t makes some
>> trouble
>> for ARM64, especially to mix with ulong.
> 
> What sort of trouble is it causing?  And why would you mix it with ulong?
> 

I am debugging this patch http://patchwork.ozlabs.org/patch/514590/.
ulong is used a lot for image related calls. I tried to change to phys_addr_t,
but only buried myself even deeper. Basically I am battling on three sides

1. All 32-bit SoCs should continue to work without using 64-bit variables for
addresses.
2. 64-bit SoCs such as ARMv8 will support FIT with addresses beyond 32 bits.
3. Host tool such as mkimage should work on both 32- and 64-bit host OS.

Any suggestion is welcomed.

York

[U-Boot] [PATCH 2/3][v3] Data types defined for 64 bit physical address

[Aneesh Bansal]

> -----Original Message-----
> From: york sun
> Sent: Wednesday, February 10, 2016 10:51 AM
> To: Scott Wood <oss@buserror.net>; Aneesh Bansal <aneesh.bansal@nxp.com>
> Cc: u-boot at lists.denx.de; Ruchika Gupta <ruchika.gupta@freescale.com>; Prabhakar
> Kushwaha <prabhakar@freescale.com>
> Subject: Re: [PATCH 2/3][v3] Data types defined for 64 bit physical address
> 
> On 02/09/2016 09:10 PM, Scott Wood wrote:
> > On Wed, 2016-02-10 at 02:30 +0000, york sun wrote:
> 
> <snip>
> 
> >>
> >> Aneesh and Scott,
> >>
> >> I need to revisit this patch. Would it be better to change it as below?
> >>
> >> +#if defined(CONFIG_PHYS_64BIT) && !defined(CONFIG_ARM64) typedef
> >> +unsigned long long dma_addr_t; typedef unsigned long long
> >> +phys_addr_t; typedef unsigned long long phys_size_t; #else
> >> +/* DMA addresses are 32-bits wide */
> >>  typedef u32 dma_addr_t;
> >> -
> >>  typedef unsigned long phys_addr_t;
> >>  typedef unsigned long phys_size_t;
> >> +#endif
> >>
> >> I am debugging another patch and found changing phys_addr_t makes
> >> some trouble for ARM64, especially to mix with ulong.
> >
> > What sort of trouble is it causing?  And why would you mix it with ulong?
> >
> 
> I am debugging this patch http://patchwork.ozlabs.org/patch/514590/.
> ulong is used a lot for image related calls. I tried to change to phys_addr_t, but only
> buried myself even deeper. Basically I am battling on three sides
> 
> 1. All 32-bit SoCs should continue to work without using 64-bit variables for
> addresses.
If CONFIG_PHYS_64BIT is not defined, phys_addr_t is defined to ulong. So it will
be 32 bit for 32 bit cores (Armv7) and 64 bit for 64 bit cores (Armv8).
So as long as CONFIG_PHYS_64BIT is not defined, using phys_addr_t will use 32 bit variable.
> 2. 64-bit SoCs such as ARMv8 will support FIT with addresses beyond 32 bits.
> 3. Host tool such as mkimage should work on both 32- and 64-bit host OS.
> 
> Any suggestion is welcomed.
> 
> York

[U-Boot] [PATCH 2/3][v3] Data types defined for 64 bit physical address

[Scott Wood]

On Wed, 2016-02-10 at 05:20 +0000, york sun wrote:
> On 02/09/2016 09:10 PM, Scott Wood wrote:
> > On Wed, 2016-02-10 at 02:30 +0000, york sun wrote:
> 
> <snip>
> 
> > > 
> > > Aneesh and Scott,
> > > 
> > > I need to revisit this patch. Would it be better to change it as below?
> > > 
> > > +#if defined(CONFIG_PHYS_64BIT) && !defined(CONFIG_ARM64)
> > > +typedef unsigned long long dma_addr_t;
> > > +typedef unsigned long long phys_addr_t;
> > > +typedef unsigned long long phys_size_t;
> > > +#else
> > > +/* DMA addresses are 32-bits wide */
> > >  typedef u32 dma_addr_t;
> > > -
> > >  typedef unsigned long phys_addr_t;
> > >  typedef unsigned long phys_size_t;
> > > +#endif
> > > 
> > > I am debugging another patch and found changing phys_addr_t makes some
> > > trouble
> > > for ARM64, especially to mix with ulong.
> > 
> > What sort of trouble is it causing?  And why would you mix it with ulong?
> > 
> 
> I am debugging this patch http://patchwork.ozlabs.org/patch/514590/.
> ulong is used a lot for image related calls. I tried to change to
> phys_addr_t,
> but only buried myself even deeper. Basically I am battling on three sides
> 
> 1. All 32-bit SoCs should continue to work without using 64-bit variables
> for
> addresses.
> 2. 64-bit SoCs such as ARMv8 will support FIT with addresses beyond 32 bits.
> 3. Host tool such as mkimage should work on both 32- and 64-bit host OS.
> 
> Any suggestion is welcomed.

Is there any situation where we'd support loading images to addresses that
don't fit in ulong?  Why do you need to switch it to phys_addr_t?

mkimage is another matter -- since it could be generating images for any
target, it should be using a fixed 64-bit type for internally representing
target addresses.  Not ulong and not phys_addr_t (which shouldn't exist in
userspace).

In any case, the answer isn't to undo this patch or make an exception for
ARM64.

-Scott

[U-Boot] [PATCH 2/3][v3] Data types defined for 64 bit physical address

[york sun]

On 02/22/2016 05:38 PM, Scott Wood wrote:
> On Wed, 2016-02-10 at 05:20 +0000, york sun wrote:
>> On 02/09/2016 09:10 PM, Scott Wood wrote:
>>> On Wed, 2016-02-10 at 02:30 +0000, york sun wrote:
>>
>> <snip>
>>
>>>>
>>>> Aneesh and Scott,
>>>>
>>>> I need to revisit this patch. Would it be better to change it as below?
>>>>
>>>> +#if defined(CONFIG_PHYS_64BIT) && !defined(CONFIG_ARM64)
>>>> +typedef unsigned long long dma_addr_t;
>>>> +typedef unsigned long long phys_addr_t;
>>>> +typedef unsigned long long phys_size_t;
>>>> +#else
>>>> +/* DMA addresses are 32-bits wide */
>>>>  typedef u32 dma_addr_t;
>>>> -
>>>>  typedef unsigned long phys_addr_t;
>>>>  typedef unsigned long phys_size_t;
>>>> +#endif
>>>>
>>>> I am debugging another patch and found changing phys_addr_t makes some
>>>> trouble
>>>> for ARM64, especially to mix with ulong.
>>>
>>> What sort of trouble is it causing?  And why would you mix it with ulong?
>>>
>>
>> I am debugging this patch http://patchwork.ozlabs.org/patch/514590/.
>> ulong is used a lot for image related calls. I tried to change to
>> phys_addr_t,
>> but only buried myself even deeper. Basically I am battling on three sides
>>
>> 1. All 32-bit SoCs should continue to work without using 64-bit variables
>> for
>> addresses.
>> 2. 64-bit SoCs such as ARMv8 will support FIT with addresses beyond 32 bits.
>> 3. Host tool such as mkimage should work on both 32- and 64-bit host OS.
>>
>> Any suggestion is welcomed.
> 
> Is there any situation where we'd support loading images to addresses that
> don't fit in ulong?  Why do you need to switch it to phys_addr_t?
> 
> mkimage is another matter -- since it could be generating images for any
> target, it should be using a fixed 64-bit type for internally representing
> target addresses.  Not ulong and not phys_addr_t (which shouldn't exist in
> userspace).
> 
> In any case, the answer isn't to undo this patch or make an exception for
> ARM64.
> 

Scott,

I moved away from this thread. I am not going to revert/undo this patch. I have
sent a RFC set "Enable FIT image to be loaded beyond 32-bit space".

York