[U-Boot] [RFC] Add 64-bit data support for memory commands

[U-Boot] [RFC] Add 64-bit data support for memory commands

[York Sun]

For aarch64, unsigned long is 64-bit data. Memory commands should be fixed
with u32 for 32-bit address access. A double word data size is added to
support 64-bit data.

Signed-off-by: York Sun <yorksun@freescale.com>
---
This patch fix this problem on aarch64

cp 30000004 80000000 1

This command causes alighment exception becuase "unsigned long" is 64-bit.

 common/cmd_mem.c      |  137 +++++++++++++++++++++++++++++++------------------
 common/command.c      |    2 +
 lib/display_options.c |    9 ++--
 3 files changed, 96 insertions(+), 52 deletions(-)

diff --git a/common/cmd_mem.c b/common/cmd_mem.c
index c3aab3d..24658ca 100644
--- a/common/cmd_mem.c
+++ b/common/cmd_mem.c
@@ -187,12 +187,14 @@ static int do_mem_mw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	bytes = size * count;
 	buf = map_sysmem(addr, bytes);
 	while (count-- > 0) {
-		if (size == 4)
-			*((ulong *)buf) = (ulong)writeval;
+		if (size == 8)
+			*((u64 *)buf) = (u64)writeval;
+		else if (size == 4)
+			*((u32 *)buf) = (u32)writeval;
 		else if (size == 2)
-			*((ushort *)buf) = (ushort)writeval;
+			*((u16 *)buf) = (u16)writeval;
 		else
-			*((u_char *)buf) = (u_char)writeval;
+			*((u8 *)buf) = (u8)writeval;
 		buf += size;
 	}
 	unmap_sysmem(buf);
@@ -270,7 +272,9 @@ static int do_mem_cmp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	*/
 	if ((size = cmd_get_data_size(argv[0], 4)) < 0)
 		return 1;
-	type = size == 4 ? "word" : size == 2 ? "halfword" : "byte";
+	type = size == 8 ? "double word" :
+	       size == 4 ? "word" :
+	       size == 2 ? "halfword" : "byte";
 
 	addr1 = simple_strtoul(argv[1], NULL, 16);
 	addr1 += base_address;
@@ -298,23 +302,26 @@ static int do_mem_cmp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	base = buf1 = map_sysmem(addr1, bytes);
 	buf2 = map_sysmem(addr2, bytes);
 	for (ngood = 0; ngood < count; ++ngood) {
-		ulong word1, word2;
-		if (size == 4) {
-			word1 = *(ulong *)buf1;
-			word2 = *(ulong *)buf2;
+		u64 word1, word2;
+		if (size == 8) {
+			word1 = *(u64 *)buf1;
+			word2 = *(u64 *)buf2;
+		} else if (size == 4) {
+			word1 = *(u32 *)buf1;
+			word2 = *(u32 *)buf2;
 		} else if (size == 2) {
-			word1 = *(ushort *)buf1;
-			word2 = *(ushort *)buf2;
+			word1 = *(u16 *)buf1;
+			word2 = *(u16 *)buf2;
 		} else {
-			word1 = *(u_char *)buf1;
-			word2 = *(u_char *)buf2;
+			word1 = *(u8 *)buf1;
+			word2 = *(u8 *)buf2;
 		}
 		if (word1 != word2) {
 			ulong offset = buf1 - base;
 
-			printf("%s at 0x%08lx (%#0*lx) != %s at 0x%08lx (%#0*lx)\n",
-				type, (ulong)(addr1 + offset), size, word1,
-				type, (ulong)(addr2 + offset), size, word2);
+			printf("%s at 0x%p (%#0*llx) != %s@0x%p (%#0*llx)\n",
+			       type, (void *)(addr1 + offset), size, word1,
+			       type, (void *)(addr2 + offset), size, word2);
 			rcode = 1;
 			break;
 		}
@@ -432,12 +439,14 @@ static int do_mem_cp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	buf = map_sysmem(dest, bytes);
 	src = map_sysmem(addr, bytes);
 	while (count-- > 0) {
-		if (size == 4)
-			*((ulong *)buf) = *((ulong  *)src);
+		if (size == 8)
+			*((u64 *)buf) = *((u64 *)src);
+		else if (size == 4)
+			*((u32 *)buf) = *((u32 *)src);
 		else if (size == 2)
-			*((ushort *)buf) = *((ushort *)src);
+			*((u16 *)buf) = *((u16 *)src);
 		else
-			*((u_char *)buf) = *((u_char *)src);
+			*((u8 *)buf) = *((u8 *)src);
 		src += size;
 		buf += size;
 
@@ -465,11 +474,12 @@ static int do_mem_base(cmd_tbl_t *cmdtp, int flag, int argc,
 static int do_mem_loop(cmd_tbl_t *cmdtp, int flag, int argc,
 		       char * const argv[])
 {
-	ulong	addr, length, i, bytes;
+	u64	addr, length, i, bytes;
 	int	size;
-	volatile uint	*longp;
-	volatile ushort *shortp;
-	volatile u_char	*cp;
+	volatile u64 *llp;
+	volatile u32 *longp;
+	volatile u16 *shortp;
+	volatile u8 *cp;
 	const void *buf;
 
 	if (argc < 3)
@@ -497,24 +507,36 @@ static int do_mem_loop(cmd_tbl_t *cmdtp, int flag, int argc,
 	 * If we have only one object, just run infinite loops.
 	 */
 	if (length == 1) {
-		if (size == 4) {
-			longp = (uint *)buf;
+		if (size == 8) {
+			llp = (u64 *)buf;
+			for (;;)
+				i = *llp;
+		} else if (size == 4) {
+			longp = (u32 *)buf;
 			for (;;)
 				i = *longp;
 		}
 		if (size == 2) {
-			shortp = (ushort *)buf;
+			shortp = (u16 *)buf;
 			for (;;)
 				i = *shortp;
 		}
-		cp = (u_char *)buf;
+		cp = (u8 *)buf;
 		for (;;)
 			i = *cp;
 	}
 
+	if (size == 8) {
+		for (;;) {
+			llp = (u64 *)buf;
+			i = length;
+			while (i-- > 0)
+				*llp++;
+		}
+	}
 	if (size == 4) {
 		for (;;) {
-			longp = (uint *)buf;
+			longp = (u32 *)buf;
 			i = length;
 			while (i-- > 0)
 				*longp++;
@@ -542,11 +564,12 @@ static int do_mem_loop(cmd_tbl_t *cmdtp, int flag, int argc,
 #ifdef CONFIG_LOOPW
 int do_mem_loopw (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	ulong	addr, length, i, data, bytes;
+	u64	addr, length, i, data, bytes;
 	int	size;
-	volatile uint	*longp;
-	volatile ushort *shortp;
-	volatile u_char	*cp;
+	volatile u64 *llp;
+	volatile u32 *longp;
+	volatile u16 *shortp;
+	volatile u8 *cp;
 	void *buf;
 
 	if (argc < 4)
@@ -577,24 +600,36 @@ int do_mem_loopw (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	 * If we have only one object, just run infinite loops.
 	 */
 	if (length == 1) {
+		if (size == 8) {
+			llp = (u64 *)buf;
+			for (;;)
+				*llp = data;
 		if (size == 4) {
-			longp = (uint *)buf;
+			longp = (u32 *)buf;
 			for (;;)
 				*longp = data;
 					}
 		if (size == 2) {
-			shortp = (ushort *)buf;
+			shortp = (u16 *)buf;
 			for (;;)
 				*shortp = data;
 		}
-		cp = (u_char *)buf;
+		cp = (u8 *)buf;
 		for (;;)
 			*cp = data;
 	}
 
+	if (size == 8) {
+		for (;;) {
+			llp = (u64 *)buf;
+			i = length;
+			while (i-- > 0)
+				*llp++ = data;
+		}
+	}
 	if (size == 4) {
 		for (;;) {
-			longp = (uint *)buf;
+			longp = (u32 *)buf;
 			i = length;
 			while (i-- > 0)
 				*longp++ = data;
@@ -602,14 +637,14 @@ int do_mem_loopw (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	}
 	if (size == 2) {
 		for (;;) {
-			shortp = (ushort *)buf;
+			shortp = (u16 *)buf;
 			i = length;
 			while (i-- > 0)
 				*shortp++ = data;
 		}
 	}
 	for (;;) {
-		cp = (u_char *)buf;
+		cp = (u8 *)buf;
 		i = length;
 		while (i-- > 0)
 			*cp++ = data;
@@ -1000,7 +1035,7 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int argc,
 static int
 mod_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const argv[])
 {
-	ulong	addr, i;
+	u64	addr, i;
 	int	nbytes, size;
 	void *ptr = NULL;
 
@@ -1048,13 +1083,15 @@ mod_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const argv[])
 	 */
 	do {
 		ptr = map_sysmem(addr, size);
-		printf("%08lx:", addr);
-		if (size == 4)
-			printf(" %08x", *((uint *)ptr));
+		printf("%p:", (void *)addr);
+		if (size == 8)
+			printf(" %08llx", *((u64 *)ptr));
+		else if (size == 4)
+			printf(" %08x", *((u32 *)ptr));
 		else if (size == 2)
-			printf(" %04x", *((ushort *)ptr));
+			printf(" %04x", *((u16 *)ptr));
 		else
-			printf(" %02x", *((u_char *)ptr));
+			printf(" %02x", *((u8 *)ptr));
 
 		nbytes = readline (" ? ");
 		if (nbytes == 0 || (nbytes == 1 && console_buffer[0] == '-')) {
@@ -1083,12 +1120,14 @@ mod_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char * const argv[])
 				 */
 				reset_cmd_timeout();
 #endif
-				if (size == 4)
-					*((uint *)ptr) = i;
+				if (size == 8)
+					*((u64 *)ptr) = i;
+				else if (size == 4)
+					*((u32 *)ptr) = i;
 				else if (size == 2)
-					*((ushort *)ptr) = i;
+					*((u16 *)ptr) = i;
 				else
-					*((u_char *)ptr) = i;
+					*((u8 *)ptr) = i;
 				if (incrflag)
 					addr += size;
 			}
diff --git a/common/command.c b/common/command.c
index 597ab4c..bdf5c2f 100644
--- a/common/command.c
+++ b/common/command.c
@@ -421,6 +421,8 @@ int cmd_get_data_size(char* arg, int default_size)
 			return 2;
 		case 'l':
 			return 4;
+		case 'q':
+			return 8;
 		case 's':
 			return -2;
 		default:
diff --git a/lib/display_options.c b/lib/display_options.c
index 4a972b0..7731cf0 100644
--- a/lib/display_options.c
+++ b/lib/display_options.c
@@ -87,6 +87,7 @@ int print_buffer(ulong addr, const void *data, uint width, uint count,
 {
 	/* linebuf as a union causes proper alignment */
 	union linebuf {
+		uint64_t uq[MAX_LINE_LENGTH_BYTES/sizeof(uint64_t) + 1];
 		uint32_t ui[MAX_LINE_LENGTH_BYTES/sizeof(uint32_t) + 1];
 		uint16_t us[MAX_LINE_LENGTH_BYTES/sizeof(uint16_t) + 1];
 		uint8_t  uc[MAX_LINE_LENGTH_BYTES/sizeof(uint8_t) + 1];
@@ -108,14 +109,16 @@ int print_buffer(ulong addr, const void *data, uint width, uint count,
 
 		/* Copy from memory into linebuf and print hex values */
 		for (i = 0; i < thislinelen; i++) {
-			uint32_t x;
-			if (width == 4)
+			uint64_t x;
+			if (width == 8)
+				x = lb.uq[i] = *(volatile uint64_t *)data;
+			else if (width == 4)
 				x = lb.ui[i] = *(volatile uint32_t *)data;
 			else if (width == 2)
 				x = lb.us[i] = *(volatile uint16_t *)data;
 			else
 				x = lb.uc[i] = *(volatile uint8_t *)data;
-			printf(" %0*x", width * 2, x);
+			printf(" %0*llx", width * 2, x);
 			data += width;
 		}
 
-- 
1.7.9.5

[U-Boot] [RFC] Add 64-bit data support for memory commands

[Wolfgang Denk]

Dear York Sun,

In message <1392240023-25205-1-git-send-email-yorksun@freescale.com> you wrote:
> For aarch64, unsigned long is 64-bit data. Memory commands should be fixed
> with u32 for 32-bit address access. A double word data size is added to
> support 64-bit data.
> 
> Signed-off-by: York Sun <yorksun@freescale.com>

Can you please make all this code conditional for 64 bit architectures
only, so that 32 bit systems do not suffer from the increased code
size?

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Our universe is a fragile house of atoms, held together by the mortar
of cause-and-effect. One magician would be two too many.
                        - Terry Pratchett, _The Dark Side of the Sun_

[U-Boot] [RFC] Add 64-bit data support for memory commands

[York Sun]

On 02/12/2014 02:11 PM, Wolfgang Denk wrote:
> Dear York Sun,
> 
> In message <1392240023-25205-1-git-send-email-yorksun@freescale.com> you wrote:
>> For aarch64, unsigned long is 64-bit data. Memory commands should be fixed
>> with u32 for 32-bit address access. A double word data size is added to
>> support 64-bit data.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
> 
> Can you please make all this code conditional for 64 bit architectures
> only, so that 32 bit systems do not suffer from the increased code
> size?
> 

Will do.

York

[U-Boot] [RFC] Add 64-bit data support for memory commands

[York Sun]

On 02/12/2014 02:11 PM, Wolfgang Denk wrote:
> Dear York Sun,
> 
> In message <1392240023-25205-1-git-send-email-yorksun@freescale.com> you wrote:
>> For aarch64, unsigned long is 64-bit data. Memory commands should be fixed
>> with u32 for 32-bit address access. A double word data size is added to
>> support 64-bit data.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
> 
> Can you please make all this code conditional for 64 bit architectures
> only, so that 32 bit systems do not suffer from the increased code
> size?
> 

I should point out the main reason of this patch is to fix the wrong size of
ulong, used in many places of cmd_mem.c. I can leave out the support of 64-bit
data. I don't see how useful it is.

York

[U-Boot] [RFC] Add 64-bit data support for memory commands

[Albert ARIBAUD]

Hi York,

On Wed, 12 Feb 2014 14:42:30 -0800, York Sun <yorksun@freescale.com>
wrote:

> On 02/12/2014 02:11 PM, Wolfgang Denk wrote:
> > Dear York Sun,
> > 
> > In message <1392240023-25205-1-git-send-email-yorksun@freescale.com> you wrote:
> >> For aarch64, unsigned long is 64-bit data. Memory commands should be fixed
> >> with u32 for 32-bit address access. A double word data size is added to
> >> support 64-bit data.
> >>
> >> Signed-off-by: York Sun <yorksun@freescale.com>
> > 
> > Can you please make all this code conditional for 64 bit architectures
> > only, so that 32 bit systems do not suffer from the increased code
> > size?
> > 
> 
> I should point out the main reason of this patch is to fix the wrong size of
> ulong, used in many places of cmd_mem.c. I can leave out the support of 64-bit
> data. I don't see how useful it is.

Support for the architecture's native size seems sensible to me.

Mind you, I would personally be ok if it was put under its own option
(something like CONFIG_SYS_SUPPORT_64BIT_DATA) which would be active by
default for 64-bit arches and inactive for others, but would allow any
non-64-bit target to enable double-word values if they so want.

> York

Amicalement,
-- 
Albert.

[U-Boot] [RFC] Add 64-bit data support for memory commands

[FengHua]

> -----Original Messages-----
> From: "Wolfgang Denk" <wd@denx.de>
> Sent Time: 2014-02-13 06:11:01 (Thursday)
> To: "York Sun" <yorksun@freescale.com>
> Cc: scottwood at freescale.com, u-boot at lists.denx.de
> Subject: Re: [U-Boot] [RFC] Add 64-bit data support for memory commands
> 
> Dear York Sun,
> 
> In message <1392240023-25205-1-git-send-email-yorksun@freescale.com> you wrote:
> > For aarch64, unsigned long is 64-bit data. Memory commands should be fixed
> > with u32 for 32-bit address access. A double word data size is added to
> > support 64-bit data.
> > 
> > Signed-off-by: York Sun <yorksun@freescale.com>
> 
> Can you please make all this code conditional for 64 bit architectures
> only, so that 32 bit systems do not suffer from the increased code
> size?
> 
> Thanks.
> 
> Best regards,
> 
> Wolfgang Denk
How about use some macro from compiler of sizeof(long)? 
Maybe this could avoid conditional switch, and cmd_mem work with 32 and 64 bit platform.

Best Wishes

[U-Boot] [RFC] Add 64-bit data support for memory commands

[Albert ARIBAUD]

Hi FengHua,

On Thu, 13 Feb 2014 10:41:07 +0800 (GMT+08:00), FengHua
<fenghua@phytium.com.cn> wrote:

> 
> 
> 
> > -----Original Messages-----
> > From: "Wolfgang Denk" <wd@denx.de>
> > Sent Time: 2014-02-13 06:11:01 (Thursday)
> > To: "York Sun" <yorksun@freescale.com>
> > Cc: scottwood at freescale.com, u-boot at lists.denx.de
> > Subject: Re: [U-Boot] [RFC] Add 64-bit data support for memory commands
> > 
> > Dear York Sun,
> > 
> > In message <1392240023-25205-1-git-send-email-yorksun@freescale.com> you wrote:
> > > For aarch64, unsigned long is 64-bit data. Memory commands should be fixed
> > > with u32 for 32-bit address access. A double word data size is added to
> > > support 64-bit data.
> > > 
> > > Signed-off-by: York Sun <yorksun@freescale.com>
> > 
> > Can you please make all this code conditional for 64 bit architectures
> > only, so that 32 bit systems do not suffer from the increased code
> > size?
> > 
> > Thanks.
> > 
> > Best regards,
> > 
> > Wolfgang Denk
> How about use some macro from compiler of sizeof(long)? 
> Maybe this could avoid conditional switch, and cmd_mem work with 32 and 64 bit platform.

That would make "md.l" be 32 or 64 bits depending on the target's
natural int size, wouldn't it? But then, "md.w" and "md.b" would remain
fixed-size, respectively 16 and 8 bits. That would not be consistent
IMO.

I think "md.l" should clearly remain 32 bits, and if 64-bit is
introduced in "md" and others it should be clearly recognizable e.g. as
".d" whatever the target's natural int size.

> Best Wishes

Amicalement,
-- 
Albert.

[U-Boot] [RFC] Add 64-bit data support for memory commands

[York Sun]

On Feb 12, 2014, at 6:41 PM, FengHua wrote:

> 
> 
> 
>> -----Original Messages-----
>> From: "Wolfgang Denk" <wd@denx.de>
>> Sent Time: 2014-02-13 06:11:01 (Thursday)
>> To: "York Sun" <yorksun@freescale.com>
>> Cc: scottwood at freescale.com, u-boot at lists.denx.de
>> Subject: Re: [U-Boot] [RFC] Add 64-bit data support for memory commands
>> 
>> Dear York Sun,
>> 
>> In message <1392240023-25205-1-git-send-email-yorksun@freescale.com> you wrote:
>>> For aarch64, unsigned long is 64-bit data. Memory commands should be fixed
>>> with u32 for 32-bit address access. A double word data size is added to
>>> support 64-bit data.
>>> 
>>> Signed-off-by: York Sun <yorksun@freescale.com>
>> 
>> Can you please make all this code conditional for 64 bit architectures
>> only, so that 32 bit systems do not suffer from the increased code
>> size?
>> 
>> Thanks.
>> 
>> Best regards,
>> 
>> Wolfgang Denk
> How about use some macro from compiler of sizeof(long)? 
> Maybe this could avoid conditional switch, and cmd_mem work with 32 and 64 bit platform.
> 

I dropped the 64-bit support in v2 RFC patch, focusing on fixing things. We can add 64-bit data is needed. I don't see the need yet.

York

[U-Boot] [RFC] Add 64-bit data support for memory commands

[Wolfgang Denk]

Dear York,

In message <52FBF8D6.1050806@freescale.com> you wrote:
>
> > Can you please make all this code conditional for 64 bit architectures
> > only, so that 32 bit systems do not suffer from the increased code
> > size?
> 
> I should point out the main reason of this patch is to fix the wrong size of
> ulong, used in many places of cmd_mem.c. I can leave out the support of 64-bit
> data. I don't see how useful it is.

I think it is very useful.  On such systems you may want to read and
manipulate 64 bit device registers and such, so we need tools to
atomically access these.

Ideally your patch should be split in two: the first would fix ulong,
and the second would add the 64 bit support.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In general, they do what you want, unless you want consistency.
                                    - Larry Wall in the perl man page