Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
Hmm, unfortunately, it's usage is not clearly documented in mtd- physmap.txt, It's pretty clear I think. Patches for making it better are welcome of course. so I never thought of this parameter. And IMHO the problem goes further - basically *any* chip which is attached to the LPB can be affected by this problem, so it might be better to have a more general approach like a chip select property. You cannot treat devices on the LPB as random access, that's all. Drivers that assume they can, cannot be used for devices on the LPB. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
On Fri, 9 Jul 2010 14:59:09 +0200 Segher Boessenkool seg...@kernel.crashing.org wrote: Actually, this is something which might need closer attention - and maybe some support in the device tree indicating which read or write width a device can accept? There already is device-width; the drivers never should use any other access width unless they *know* that will work. Wouldn't you want to use bank-width instead? We were talking about single devices. But, sure, when you have multiple devices in parallel the driver needs to know about that. It would be nice to have a device tree property that can specify that all access widths supported by the CPU will work, though. Oh please no. A device binding should not depend on what CPU there is in the system. There could be multiple CPUs of different architectures, even. What I meant by that was that the flash interface was claiming that it is not the limiting factor in which access widths are useable -- it would be a way to claim that it is as flexible as ordinary memory in that regard. If there is a transaction size that is capable of being presented to this component that it cannot handle, it would not present this property. To figure out how to access a device, the driver looks at the device's node, and all its parent nodes (or asks generic code to do that, or platform code). looks or should look? :-) If there are transaction sizes supported by the CPU that won't work with a given device through no fault of that device (or the interface to that device for which we don't have a separate node), then in theory, yes, it should be described at a higher level. In reality, device tree parsing code is not AI, so rather than say the driver looks at this and figures it out it would be better to provide a more specific proposal of how a device tree might express this and what the driver would look for, if you think the simple solution is not expressive enough. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
-Original Message- From: glik...@secretlab.ca [mailto:glik...@secretlab.ca] On Behalf Of Grant Likely Sent: Thursday, July 08, 2010 12:38 AM To: Benjamin Herrenschmidt Cc: Steve Deiters; linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx On Wed, Jul 7, 2010 at 11:10 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Tue, 2010-06-29 at 11:04 -0500, Steve Deiters wrote: These processors will corrupt data if accessing the local bus with unaligned addresses. This version fixes the typical case of copying from Flash on the local bus by keeping the source address always aligned. Shouldn't this be solved by using memcpy_to/fromio ? Maybe. plain memcpy() access to anything localbus-attached on the mpc5xxx is the wrong thing to do. memcpy_to/fromio might do the right thing; but the caller must understand the limitations of the localplus bus. The byte-wise alignment that memcpy_to/fromio() does may not work (depending on configuration). Steve, what code is doing a memcpy from flash? g. This was done in the JFFS2 code, in fs/jffs2/scan.c. At least in one function, in jffs2_scan_dirent_node it was using memcpy on a localbus mapped structure. I was getting JFFS2 filesystem corruption because of this. In fact I first tried changing this to memcpy_fromio and it fixed that particular problem. I was concerned though that other places I was not aware of might be using memcpy from the localbus in a similar manner so I decided to just tweak the memcpy routine. Just out of curiousity, what configuration might cause a byte-wise alignment not to work? ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
On Thu, Jul 8, 2010 at 8:38 AM, Steve Deiters stevedeit...@basler.com wrote: -Original Message- From: glik...@secretlab.ca [mailto:glik...@secretlab.ca] On Behalf Of Grant Likely Sent: Thursday, July 08, 2010 12:38 AM To: Benjamin Herrenschmidt Cc: Steve Deiters; linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx On Wed, Jul 7, 2010 at 11:10 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Tue, 2010-06-29 at 11:04 -0500, Steve Deiters wrote: These processors will corrupt data if accessing the local bus with unaligned addresses. This version fixes the typical case of copying from Flash on the local bus by keeping the source address always aligned. Shouldn't this be solved by using memcpy_to/fromio ? Maybe. plain memcpy() access to anything localbus-attached on the mpc5xxx is the wrong thing to do. memcpy_to/fromio might do the right thing; but the caller must understand the limitations of the localplus bus. The byte-wise alignment that memcpy_to/fromio() does may not work (depending on configuration). Steve, what code is doing a memcpy from flash? g. This was done in the JFFS2 code, in fs/jffs2/scan.c. At least in one function, in jffs2_scan_dirent_node it was using memcpy on a localbus mapped structure. I was getting JFFS2 filesystem corruption because of this. In fact I first tried changing this to memcpy_fromio and it fixed that particular problem. I was concerned though that other places I was not aware of might be using memcpy from the localbus in a similar manner so I decided to just tweak the memcpy routine. [cc'ing David Woodhouse] Sounds to me like the right thing to do is to fix the jffs2 code. Just out of curiousity, what configuration might cause a byte-wise alignment not to work? Can't remember the register configuration, but I worked on one project where this was the case. In hindsight, it was probably a mis-configuration of the localbus CS for the particular device. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
Am 08.07.10 17:22 schrieb(en) Grant Likely: Just out of curiousity, what configuration might cause a byte-wise alignment not to work? Can't remember the register configuration, but I worked on one project where this was the case. In hindsight, it was probably a mis-configuration of the localbus CS for the particular device. Not sure if you're thinking of this configuration, but if you attach a device in 16-bit mode (i.e. 16 data lines) to the LPB, byte writes simply don't work. I ran into that problem as I have a nvram attached this way to a 5200b. Using the device as mtd-ram with a jffs2 file system on it I also sometimes saw corruption after a write. I had a patch for that last year, but it was actually badly crafted (see http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-June/072903.html). I still use the mpc52xx_memcpy2lpb16() function somewhere in my current code which is actually an ugly hack (but it works...). Actually, this is something which might need closer attention - and maybe some support in the device tree indicating which read or write width a device can accept? Best, Albrecht. pgpQDQ2The7jf.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
Actually, this is something which might need closer attention - and maybe some support in the device tree indicating which read or write width a device can accept? There already is device-width; the drivers never should use any other access width unless they *know* that will work. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
On Thu, 8 Jul 2010 21:30:33 +0200 Segher Boessenkool seg...@kernel.crashing.org wrote: Actually, this is something which might need closer attention - and maybe some support in the device tree indicating which read or write width a device can accept? There already is device-width; the drivers never should use any other access width unless they *know* that will work. Wouldn't you want to use bank-width instead? device-width might not even work if, say, you've got two 8-bit chips interleaved, feeding into a bus controller in 16-bit mode that only accepts 16-bit accesses. It would be nice to have a device tree property that can specify that all access widths supported by the CPU will work, though. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
Am 08.07.10 21:30 schrieb(en) Segher Boessenkool: Actually, this is something which might need closer attention - and maybe some support in the device tree indicating which read or write width a device can accept? There already is device-width; the drivers never should use any other access width unless they *know* that will work. Hmm, unfortunately, it's usage is not clearly documented in mtd-physmap.txt, so I never thought of this parameter. And IMHO the problem goes further - basically *any* chip which is attached to the LPB can be affected by this problem, so it might be better to have a more general approach like a chip select property. Cheers, Albrecht. pgpyhtduU1Hps.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
On Tue, 2010-06-29 at 11:04 -0500, Steve Deiters wrote: These processors will corrupt data if accessing the local bus with unaligned addresses. This version fixes the typical case of copying from Flash on the local bus by keeping the source address always aligned. Shouldn't this be solved by using memcpy_to/fromio ? Cheers, Ben. Signed-off-by: Steve Deiters stevedeit...@basler.com --- arch/powerpc/lib/copy_32.S | 56 1 files changed, 56 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S index 74a7f41..42e7df5 100644 --- a/arch/powerpc/lib/copy_32.S +++ b/arch/powerpc/lib/copy_32.S @@ -226,6 +226,60 @@ _GLOBAL(memmove) bgt backwards_memcpy /* fall through */ +#if defined(CONFIG_PPC_MPC512x) || defined(CONFIG_PPC_MPC52xx) + +/* + * Alternate memcpy for MPC512x and MPC52xx to guarantee source + * address is always aligned to prevent corruption issues when + * copying unaligned from the local bus. This only fixes the usage + * when copying from the local bus (e.g. Flash) and will not fix + * issues copying to the local bus + */ +_GLOBAL(memcpy) + srwi. r7,r5,3 + addir6,r3,-4 + addir4,r4,-4 + beq 2f /* if less than 8 bytes to do */ + andi. r0,r4,3 /* get src word aligned */ + mtctr r7 + bne 5f +1: lwz r7,4(r4) + lwzur8,8(r4) + stw r7,4(r6) + stwur8,8(r6) + bdnz1b + andi. r5,r5,7 +2: cmplwi 0,r5,4 + blt 3f + andi. r0,r4,3 + bne 3f + lwzur0,4(r4) + addir5,r5,-4 + stwur0,4(r6) +3: cmpwi 0,r5,0 + beqlr + mtctr r5 + addir4,r4,3 + addir6,r6,3 +4: lbzur0,1(r4) + stbur0,1(r6) + bdnz4b + blr +5: subfic r0,r0,4 + mtctr r0 +6: lbz r7,4(r4) + addir4,r4,1 + stb r7,4(r6) + addir6,r6,1 + bdnz6b + subfr5,r0,r5 + rlwinm. r7,r5,32-3,3,31 + beq 2b + mtctr r7 + b 1b + +#else + _GLOBAL(memcpy) srwi. r7,r5,3 addir6,r3,-4 @@ -267,6 +321,8 @@ _GLOBAL(memcpy) mtctr r7 b 1b +#endif + _GLOBAL(backwards_memcpy) rlwinm. r7,r5,32-3,3,31 /* r0 = r5 3 */ add r6,r3,r5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
On Wed, Jul 7, 2010 at 11:10 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Tue, 2010-06-29 at 11:04 -0500, Steve Deiters wrote: These processors will corrupt data if accessing the local bus with unaligned addresses. This version fixes the typical case of copying from Flash on the local bus by keeping the source address always aligned. Shouldn't this be solved by using memcpy_to/fromio ? Maybe. plain memcpy() access to anything localbus-attached on the mpc5xxx is the wrong thing to do. memcpy_to/fromio might do the right thing; but the caller must understand the limitations of the localplus bus. The byte-wise alignment that memcpy_to/fromio() does may not work (depending on configuration). Steve, what code is doing a memcpy from flash? g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
These processors will corrupt data if accessing the local bus with unaligned addresses. This version fixes the typical case of copying from Flash on the local bus by keeping the source address always aligned. Signed-off-by: Steve Deiters stevedeit...@basler.com --- arch/powerpc/lib/copy_32.S | 56 1 files changed, 56 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S index 74a7f41..42e7df5 100644 --- a/arch/powerpc/lib/copy_32.S +++ b/arch/powerpc/lib/copy_32.S @@ -226,6 +226,60 @@ _GLOBAL(memmove) bgt backwards_memcpy /* fall through */ +#if defined(CONFIG_PPC_MPC512x) || defined(CONFIG_PPC_MPC52xx) + +/* + * Alternate memcpy for MPC512x and MPC52xx to guarantee source + * address is always aligned to prevent corruption issues when + * copying unaligned from the local bus. This only fixes the usage + * when copying from the local bus (e.g. Flash) and will not fix + * issues copying to the local bus + */ +_GLOBAL(memcpy) + srwi. r7,r5,3 + addir6,r3,-4 + addir4,r4,-4 + beq 2f /* if less than 8 bytes to do */ + andi. r0,r4,3 /* get src word aligned */ + mtctr r7 + bne 5f +1: lwz r7,4(r4) + lwzur8,8(r4) + stw r7,4(r6) + stwur8,8(r6) + bdnz1b + andi. r5,r5,7 +2: cmplwi 0,r5,4 + blt 3f + andi. r0,r4,3 + bne 3f + lwzur0,4(r4) + addir5,r5,-4 + stwur0,4(r6) +3: cmpwi 0,r5,0 + beqlr + mtctr r5 + addir4,r4,3 + addir6,r6,3 +4: lbzur0,1(r4) + stbur0,1(r6) + bdnz4b + blr +5: subfic r0,r0,4 + mtctr r0 +6: lbz r7,4(r4) + addir4,r4,1 + stb r7,4(r6) + addir6,r6,1 + bdnz6b + subfr5,r0,r5 + rlwinm. r7,r5,32-3,3,31 + beq 2b + mtctr r7 + b 1b + +#else + _GLOBAL(memcpy) srwi. r7,r5,3 addir6,r3,-4 @@ -267,6 +321,8 @@ _GLOBAL(memcpy) mtctr r7 b 1b +#endif + _GLOBAL(backwards_memcpy) rlwinm. r7,r5,32-3,3,31 /* r0 = r5 3 */ add r6,r3,r5 -- 1.5.4.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] arch/powerpc/lib/copy_32.S: Use alternate memcpy for MPC512x and MPC52xx
These processors will corrupt data if accessing the local bus with unaligned addresses. This version fixes the typical case of copying from Flash on the local bus by keeping the source address always aligned. On many platforms accessing ROM as RAM simply doesn't work(*). You shouldn't map ROM as if it is RAM, and shouldn't use the same access functions on it. Segher (*) Example: any existing 970 system will checkstop as soon as you try to do any cacheable access to some ROM. Another example of course is unaligned accesses on pretty much any system, no matter whether it's called a bug or a feature on that system :-P ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
