| Message ID | 1392240023-25205-1-git-send-email-yorksun@freescale.com |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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
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,
> -----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@freescale.com, u-boot@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
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@freescale.com, u-boot@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,
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@freescale.com, u-boot@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
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
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 at 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; }
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(-)