MPC5200: workaround data corruption for unaligned local bus accesses

The MPC5200 has a nasty problem that will cause silent data corruption when performing unaligned 16 or 32 byte accesses when reading from the local bus - typically this affects reading from flash. The problem can be easily shown:

=> md fc0c0000 10
fc0c0000: 323e4337 01626f6f 74636d64 3d72756e 2>C7.bootcmd=run
fc0c0010: 206e6574 5f6e6673 00626f6f 7464656c net_nfs.bootdel
fc0c0020: 61793d35 00626175 64726174 653d3131 ay=5.baudrate=11
fc0c0030: 35323030 00707265 626f6f74 3d656368 5200.preboot=ech
=> md fc0c0001 10
fc0c0001: 65636801 00000074 0000003d 00000020 ech....t...=...
fc0c0011: 0000005f 00000000 00000074 00000061 ..._.......t...a
fc0c0021: 00000000 00000064 00000065 00000035 .......d...e...5
fc0c0031: 00000000 00000062 0000003d 0000006f .......b...=...o
=> md.w fc0c0001 10
fc0c0001: 0000 3701 0000 6f74 0000 643d 0000 6e20 ..7...ot..d=..n
fc0c0011: 0000 745f 0000 7300 0000 6f74 0000 6c61 ..t_..s...ot..la

This commit implements a workaround at least for the most blatant
problem: using memcpy() from NOR flash. We rename the assembler routine into __memcpy() and provide a wrapper, which will use a byte-wise copy loop for source addresses in NOR flash, and branch to the optimized __memcpy() otherwise.

---
 arch/powerpc/cpu/mpc5xxx/Makefile         |    5 ++
 arch/powerpc/cpu/mpc5xxx/memcpy_mpc5200.c |   62 +++++++++++++++++++++++++++++
 arch/powerpc/lib/Makefile                 |    5 ++
 3 files changed, 72 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/cpu/mpc5xxx/memcpy_mpc5200.c

index 0ee0611..4ab2b7b 100644
@@ -30,6 +30,11 @@ SOBJS	= io.o firmware_sc_task_bestcomm.impl.o
 COBJS	= i2c.o traps.o cpu.o cpu_init.o ide.o interrupts.o \
 	  loadtask.o pci_mpc5200.o serial.o speed.o usb_ohci.o usb.o
 
+# Workaround for local bus unaligned access problem on MPC5200
+#ifdef CONFIG_MPC5200
+COBJS	+= memcpy_mpc5200.o
+#endif
+
 SRCS	:= $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
 START	:= $(addprefix $(obj),$(START))

index 5f85502..4ba51b3 100644
@@ -43,6 +43,11 @@ COBJS-y	+= time.o
 SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
 
+# Workaround for local bus unaligned access problem on MPC5200
+#ifdef CONFIG_MPC5200
+$(obj)ppcstring.o: AFLAGS += -Dmemcpy=__memcpy
+#endif
+
 $(LIB):	$(obj).depend $(OBJS)
 	@if ! $(CROSS_COMPILE)readelf -S $(OBJS) | grep -q '\.fixup.*PROGBITS';\
 	then \

1.7.0.1

The MPC5200 has a nasty problem that will cause silent data corruption when performing unaligned 16 or 32 byte accesses when reading from the local bus - typically this affects reading from flash. The problem can be easily shown:

=> md fc0c0000 10
fc0c0000: 323e4337 01626f6f 74636d64 3d72756e 2>C7.bootcmd=run
fc0c0010: 206e6574 5f6e6673 00626f6f 7464656c net_nfs.bootdel
fc0c0020: 61793d35 00626175 64726174 653d3131 ay=5.baudrate=11
fc0c0030: 35323030 00707265 626f6f74 3d656368 5200.preboot=ech
=> md fc0c0001 10
fc0c0001: 65636801 00000074 0000003d 00000020 ech....t...=...
fc0c0011: 0000005f 00000000 00000074 00000061 ..._.......t...a
fc0c0021: 00000000 00000064 00000065 00000035 .......d...e...5
fc0c0031: 00000000 00000062 0000003d 0000006f .......b...=...o
=> md.w fc0c0001 10
fc0c0001: 0000 3701 0000 6f74 0000 643d 0000 6e20 ..7...ot..d=..n
fc0c0011: 0000 745f 0000 7300 0000 6f74 0000 6c61 ..t_..s...ot..la

This commit implements a workaround at least for the most blatant
problem: using memcpy() from NOR flash. We rename the assembler routine into __memcpy() and provide a wrapper, which will use a byte-wise copy loop for unaligned source or target addresses when reading from NOR flash, and branch to the optimized __memcpy() in all other cases, thus minimizing the performance impact.

Tested on lite5200b and TQM5200S.

---
 arch/powerpc/cpu/mpc5xxx/Makefile         |    5 ++
 arch/powerpc/cpu/mpc5xxx/memcpy_mpc5200.c |   71 +++++++++++++++++++++++++++++
 arch/powerpc/lib/Makefile                 |    5 ++
 3 files changed, 81 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/cpu/mpc5xxx/memcpy_mpc5200.c

index 0ee0611..4ab2b7b 100644
@@ -30,6 +30,11 @@ SOBJS	= io.o firmware_sc_task_bestcomm.impl.o
 COBJS	= i2c.o traps.o cpu.o cpu_init.o ide.o interrupts.o \
 	  loadtask.o pci_mpc5200.o serial.o speed.o usb_ohci.o usb.o
 
+# Workaround for local bus unaligned access problem on MPC5200
+#ifdef CONFIG_MPC5200
+COBJS	+= memcpy_mpc5200.o
+#endif
+
 SRCS	:= $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
 START	:= $(addprefix $(obj),$(START))

index 5f85502..4ba51b3 100644
@@ -43,6 +43,11 @@ COBJS-y	+= time.o
 SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
 
+# Workaround for local bus unaligned access problem on MPC5200
+#ifdef CONFIG_MPC5200
+$(obj)ppcstring.o: AFLAGS += -Dmemcpy=__memcpy
+#endif
+
 $(LIB):	$(obj).depend $(OBJS)
 	@if ! $(CROSS_COMPILE)readelf -S $(OBJS) | grep -q '\.fixup.*PROGBITS';\
 	then \

1.7.0.1

The MPC5200 has a nasty problem that will cause silent data corruption when performing unaligned 16 or 32 byte accesses when reading from the local bus - typically this affects reading from flash. The problem can be easily shown:

=> md fc0c0000 10
fc0c0000: 323e4337 01626f6f 74636d64 3d72756e 2>C7.bootcmd=run
fc0c0010: 206e6574 5f6e6673 00626f6f 7464656c net_nfs.bootdel
fc0c0020: 61793d35 00626175 64726174 653d3131 ay=5.baudrate=11
fc0c0030: 35323030 00707265 626f6f74 3d656368 5200.preboot=ech
=> md fc0c0001 10
fc0c0001: 65636801 00000074 0000003d 00000020 ech....t...=...
fc0c0011: 0000005f 00000000 00000074 00000061 ..._.......t...a
fc0c0021: 00000000 00000064 00000065 00000035 .......d...e...5
fc0c0031: 00000000 00000062 0000003d 0000006f .......b...=...o
=> md.w fc0c0001 10
fc0c0001: 0000 3701 0000 6f74 0000 643d 0000 6e20 ..7...ot..d=..n
fc0c0011: 0000 745f 0000 7300 0000 6f74 0000 6c61 ..t_..s...ot..la

This commit implements a workaround at least for the most blatant
problem: using memcpy() from NOR flash. We rename the assembler routine into __memcpy() and provide a wrapper, which will use a byte-wise copy loop for unaligned source or target addresses when reading from NOR flash, and branch to the optimized __memcpy() in all other cases, thus minimizing the performance impact.

Tested on lite5200b and TQM5200S.

---
v2:
Check alignment of source and target addresses, use byte-wise copy only when needed (i. e. unaligned, and reading from flash)
v3:
Fix Makefile typo which will break all non-MPC5200 boards

 arch/powerpc/cpu/mpc5xxx/Makefile         |    5 ++
 arch/powerpc/cpu/mpc5xxx/memcpy_mpc5200.c |   71 +++++++++++++++++++++++++++++
 arch/powerpc/lib/Makefile                 |    5 ++
 3 files changed, 81 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/cpu/mpc5xxx/memcpy_mpc5200.c

index 0ee0611..4ab2b7b 100644
@@ -30,6 +30,11 @@ SOBJS	= io.o firmware_sc_task_bestcomm.impl.o
 COBJS	= i2c.o traps.o cpu.o cpu_init.o ide.o interrupts.o \
 	  loadtask.o pci_mpc5200.o serial.o speed.o usb_ohci.o usb.o
 
+# Workaround for local bus unaligned access problem on MPC5200
+ifdef CONFIG_MPC5200
+COBJS	+= memcpy_mpc5200.o
+endif
+
 SRCS	:= $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
 START	:= $(addprefix $(obj),$(START))

index 5f85502..4ba51b3 100644
@@ -43,6 +43,11 @@ COBJS-y	+= time.o
 SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
 
+# Workaround for local bus unaligned access problem on MPC5200
+#ifdef CONFIG_MPC5200
+$(obj)ppcstring.o: AFLAGS += -Dmemcpy=__memcpy
+#endif
+
 $(LIB):	$(obj).depend $(OBJS)
 	@if ! $(CROSS_COMPILE)readelf -S $(OBJS) | grep -q '\.fixup.*PROGBITS';\
 	then \

1.7.0.1

Hi Wolfgang,

Now that we keep the old speed on aligned access, I'm ok with the change and only see one typo in the comment.

[...]

                                                           ^
                                                           +- copy

WARNING: The external boundaries of India as depicted in map(s) are neither
correct nor authentic.  Other external boundaries as depicted in the map(s)
may neither be correct nor authentic.
                                                 -- Garmin MapSource manual
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Commit 460c2ce3 "MPC5200: workaround data corruption for unaligned local bus accesses" fixed the problem for MPC5200 only, but MPC512x is affected as well, so apply the same fix here, too.

---
 arch/powerpc/cpu/mpc5xxx/Makefile                  |    5 -----
 arch/powerpc/lib/Makefile                          |   16 ++++++++++++----
 arch/powerpc/{cpu/mpc5xxx => lib}/memcpy_mpc5200.c |    0
 3 files changed, 12 insertions(+), 9 deletions(-)
 rename arch/powerpc/{cpu/mpc5xxx => lib}/memcpy_mpc5200.c (100%)

index 4ab2b7b..0ee0611 100644
@@ -30,11 +30,6 @@ SOBJS	= io.o firmware_sc_task_bestcomm.impl.o
 COBJS	= i2c.o traps.o cpu.o cpu_init.o ide.o interrupts.o \
 	  loadtask.o pci_mpc5200.o serial.o speed.o usb_ohci.o usb.o
 
-# Workaround for local bus unaligned access problem on MPC5200
-#ifdef CONFIG_MPC5200
-COBJS	+= memcpy_mpc5200.o
-#endif
-
 SRCS	:= $(START:.o=.S) $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
 START	:= $(addprefix $(obj),$(START))

index bf23790..2065b6d 100644
@@ -40,14 +40,22 @@ COBJS-y	+= interrupts.o
 COBJS-$(CONFIG_CMD_KGDB) += kgdb.o
 COBJS-y	+= time.o
 
-SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
-OBJS	:= $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
-
-# Workaround for local bus unaligned access problem on MPC5200
+# Workaround for local bus unaligned access problems
+# on MPC512x and MPC5200
+ifdef CONFIG_MPC512X
+$(obj)ppcstring.o: AFLAGS += -Dmemcpy=__memcpy
+COBJS-y += memcpy_mpc5200.o
+endif
 ifdef CONFIG_MPC5200
 $(obj)ppcstring.o: AFLAGS += -Dmemcpy=__memcpy
+COBJS-y += memcpy_mpc5200.o
 endif
 
+COBJS	+= $(sort $(COBJS-y))
+
+SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
+OBJS	:= $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
+
 $(LIB):	$(obj).depend $(OBJS)
 	@if ! $(CROSS_COMPILE)readelf -S $(OBJS) | grep -q '\.fixup.*PROGBITS';\
 	then \

similarity index 100%
rename from arch/powerpc/cpu/mpc5xxx/memcpy_mpc5200.c
rename to arch/powerpc/lib/memcpy_mpc5200.c

1.7.0.1

Hi Wolfgang,

The only thing I wonder is the filename 'memcpy_mpc5200.c' as the code doesn't really have any 5200 specifics in it. What about 'memcpy_align32wrap' or something likew that to express the more general nature of the code?

If one thinks further along this line, why not move the wrapper to lib/?

Cheers
Detlev

LISP is the most powerful programming language, and if you want an inter-
preter, LISP is the best.  None of the other languages come anywhere near
LISP in their power.  The most exciting things about LISP are read, eval,
and print.  If you look at other languages,  they have no equivalent for
any of those.                             -- Richard Stallman
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Dear Detlev Zundel,

thanks.

As far as I understand this behaviour is specific to the MPC512x and MPC5200 processors; I did not notice it on other cores yet. Unfortunately we do not have a generic name that includes these processors (5xxx is taken by something else).

I could not come up with a better name... What is "align32wrap" supposed to mean?

Please re-check what my patch does - exactly that:

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
"The value of marriage is not that adults produce children, but  that
children produce adults."                            - Peter De Vries

Hi Wolfgang,

I believe that other architectures may also suffer from such unaligned byte copies, but maybe in those cases 'memcpy' already takes car of that, I don't know. The point I wanted to make is that the file in question has no powerpc specific code in it at all.

It was meant to mean somehow to express the fact that for some condition the original memcpy was called and for some conditions not - hence "wrap". The condition which was passed on to the original memcpy is connected to correct 32 bit alignment, hence the proposal.

Actually I was aiming for "lib" outside of any architecure for the reason noted above.

But as I said, this is only place for improvement, I surely will not nak the patch ;)

Cheers
Detlev

System going down at 1:45 this afternoon for disk crashing.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

mpc5200_memcpy_fromio() resp. mpc5200_memcpy_toio()?

Dear Joakim Tjernlund,

No. It's not only MPC5200, but also MPC521x. It's not I/O in general, but only I/O from the Local Bus. And even then only unaliged read access.

But memcpy_for_unaligned_read_from_local_bus() was too long for me, and memcpy_furflb() too cryptic ;-)

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
"There is nothing new under the sun, but there are lots of old things
we don't know yet."                                  - Ambrose Bierce

Wolfgang Denk <[email protected]> wrote on 2010/06/29 14:55:44:

memcpy_align_src()?

Jocke

BTW, perhaps this memcpy I wrote for uClibc long time ago is useful? You can easily change dst or src alignment:

void *memcpy(void *to, const void *from, size_t len)

{
	unsigned long rem, chunks, tmp1, tmp2;
	unsigned char *tmp_to;
	unsigned char *tmp_from = (unsigned char *)from;

	chunks = len / 8;
	tmp_from -= 4;
	tmp_to = to - 4;
	if (!chunks)
		goto lessthan8;
	rem = (unsigned long )tmp_to % 4;
	if (rem)
		goto align;
 copy_chunks:
	do {
		/* make gcc to load all data, then store it */
		tmp1 = *(unsigned long *)(tmp_from+4);
		tmp_from += 8;
		tmp2 = *(unsigned long *)tmp_from;
		*(unsigned long *)(tmp_to+4) = tmp1;
		tmp_to += 8;
		*(unsigned long *)tmp_to = tmp2;
	} while (--chunks);
 lessthan8:
	len = len % 8;
	if (len >= 4) {
		tmp_from += 4;
		tmp_to += 4;
		*(unsigned long *)(tmp_to) = *(unsigned long *)(tmp_from);
		len -= 4;
	}
	if (!len)
		return to;
	tmp_from += 3;
	tmp_to += 3;
	do {
		*++tmp_to = *++tmp_from;
	} while (--len);

	return to;
 align:
	/* ???: Do we really need to generate the carry flag here? If not, then:
	rem -= 4; */
	rem = 4 - rem;
	len -= rem;
	do {
		*(tmp_to+4) = *(tmp_from+4);
		++tmp_from;
		++tmp_to;
	} while (--rem);
	chunks = len / 8;
	if (chunks)
		goto copy_chunks;
	goto lessthan8;
}

I just posted a patch on the linuxppc-dev list that simply uses a slightly modified version of memcpy to always keep the source address aligned. I had conditionals in that one so it only used it for MPC512x or MPC52xx but you should be able to replace the regular memcpy with this version. This way you can avoid the wrappers and extra checks. It is a simple enough change in that case:

index 97023a0..4e17265 100644
@@ -114,7 +114,7 @@ memcpy:
 	addi	r6,r3,-4
 	addi	r4,r4,-4
 	beq	2f			/* if less than 8 bytes to do */
-	andi.	r0,r6,3			/* get dest word aligned */
+	andi.	r0,r4,3			/* get src word aligned */
 	mtctr	r7
 	bne	5f
 1:	lwz	r7,4(r4)

@@ -125,6 +125,8 @@ memcpy:
 	andi.	r5,r5,7
 2:	cmplwi	0,r5,4
 	blt	3f
+	andi.	r0,r4,3
+	bne	3f
 	lwzu	r0,4(r4)
 	addi	r5,r5,-4
 	stwu	r0,4(r6)

Dear "Steve Deiters",

Thanks. I'll keep this queued and try to run some tests with it on different architectures - I don't know which side effects might be caused by changing the alignment from target to source address, so I will not introduce such a change at this point in the release cycle, literally minutes before a release.

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
If you hear an onion ring, answer it.

Wolfgang Denk <[email protected]> wrote on 2010/06/29 21:24:03:

Try the C memcpy I sent. It will/should compile into the same assembler(it did when I wrote it). It is far more generic and is easier to understand.

Jocke

Hi Detlev, Hi Wolfgang,

'memcpy_align32wrap' isn't really good since the fixed memcpy also fixes 16-bit accesses, too.

BTW, shouldn't we fix print_buffer() also? do_mem_md() doesn't use memcpy() and the issue with corrupted dumps still remains here. I'm testing a patch to fix it. Will submit it soon.

Best regards,
Anatolij

Dear Anatolij,

we discussed this a bit more internally, and could not find a good name yet, either. Guess I'll stick with the current one until someone comes up with a really good idea.

I though about this, too. But then I decided against it. "md" is intended to to exactly what the user requests - as is, we can easily demonstrate the issue. I consider this an extremely useful debug feature. If I command the memory to be read in units of 32 bits I really mean that. A "fix" ins print_buffer() whould hush up the symptoms, so the problem would be much harder to detect.

Let's keep this as is, please: if you need a memory dump, you can either align your start address, or use "md.b".

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
"How to make a million dollars:  First, get a million dollars."
- Steve Martin

Dear Wolfgang,

On Tue, 29 Jun 2010 14:47:14 +0200
Wolfgang Denk <[email protected]> wrote:

...

Thanks for the explanation, I didn't thought about it. This is indeed a reasonable debug feature.

Best regards,
Anatolij

Unaligned 32-/16-bit accesses to local bus on MPC5200 and MPC512x deliver corrupted data:

=> md F0000000 10
f0000000: 27051956 552d426f 6f742032 3031302e '..VU-Boot 2010.
f0000010: 30362d72 63332d30 38303336 2d676665 06-rc3-08036-gfe
f0000020: 38663238 362d6469 72747920 284a756e 8f286-dirty (Jun
f0000030: 20323920 32303130 202d2031 343a3235 29 2010 - 14:25
=> md F0000001 10
f0000001: 00005655 00006f6f 00003230 00002e30 ..VU..oo..20...0
f0000011: 00007263 00003038 0000362d 00006538 ..rc..08..6-..e8
f0000021: 00003836 00006972 00002028 00006e20 ..86..ir.. (..n
f0000031: 00002032 00003020 00003134 0000353a .. 2..0 ..14..5:
=> md.w F0000001 20
f0000001: 0000 5655 0000 6f6f 0000 3230 0000 2e30 ..VU..oo..20...0
f0000011: 0000 7263 0000 3038 0000 362d 0000 6538 ..rc..08..6-..e8
f0000021: 0000 3836 0000 6972 0000 2028 0000 6e20 ..86..ir.. (..n
f0000031: 0000 2032 0000 3020 0000 3134 0000 353a .. 2..0 ..14..5:

Use memcpy in print_buffer() to fix the problem.

---
 lib/display_options.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

index a711425..c0b89b2 100644
@@ -122,10 +122,24 @@ int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen)
 		/* Copy from memory into linebuf and print hex values */
 		for (i = 0; i < linelen; i++) {
 			if (width == 4) {
+#if defined(CONFIG_MPC5200) || defined(CONFIG_MPC512X)
+				/*
+				 * workaround for issues on the MPC5200/MPC512X,
+				 * where unaligned 32-/16-bit-accesses to the
+				 * local bus will deliver corrupted data. Just
+				 * use fixed memcpy here.
+				 */
+				memcpy(&uip[i], data, 4);
+#else
 				uip[i] = *(volatile uint32_t *)data;
+#endif
 				printf(" %08x", uip[i]);
 			} else if (width == 2) {
+#if defined(CONFIG_MPC5200) || defined(CONFIG_MPC512X)
+				memcpy(&usp[i], data, 2);
+#else
 				usp[i] = *(volatile uint16_t *)data;
+#endif
 				printf(" %04x", usp[i]);
 			} else {
 				ucp[i] = *(volatile uint8_t *)data;

1.7.0.4

Dear Anatolij Gustschin,

Right, and the current version of print_buffer() shopws the real data, i. e. you can see that such a corruption happens.

NAK. This violates the design of the command.

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
Be kind to unkind people - they need it the most.

Dear Anatolij,

Also note that the fix would be incomplete - the same issue happens with unaligned 16 bit accesses:

=> md.w FC0C0000 10
fc0c0000: 7012 ab65 0161 6464 636f 6e73 3d73 6574 p..e.addcons=set
fc0c0010: 656e 7620 626f 6f74 6172 6773 2024 7b62 env bootargs ${b
=> md.w FC0C0001 10
fc0c0001: 007b 6501 0000 6463 0000 733d 0000 7465 .{e...dc..s=..te
fc0c0011: 0000 2062 0000 7461 0000 7320 0000 626f .. b..ta..s ..bo

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
Politics:  A  strife  of  interests  masquerading  as  a  contest  of
principles. The conduct of public affairs for private advantage.
- Ambrose Bierce

i also prefer the current behavior. on Blackfin systems, you get unaligned exceptions when you try to do 'md.l 1' and it's a nice way to quickly exercise those code paths ;).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100629/2260d730/attachment.pgp

Applied

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
Men of peace usually are [brave].
	-- Spock, "The Savage Curtain", stardate 5906.5