MPC5200 + LocalPlus Bus + memcpy

MPC5200 + LocalPlus Bus + memcpy

[Sascha Hauer]

Hi all,

I try to use jffs2 on a flash device connected to the mpc5200
LocalPlus Bus. This bus does not allow misaligned accesses.
The jffs2 code uses memcpy to copy from a word aligned address to an
odd address. The ppc memcpy implementation first copies three bytes to get
the target address word aligned, but then the source address is on an
odd address. The following word accesses on this unaligned address fail
badly.

I have fixed my problem by modifying the physmap mtd driver, but some
day someone wants to connect SRAM to the LocalPlus Bus and I guess he
will expect memcpy to work.

(BTW the arm implementation of memcpy seems to work around this problem)

Sascha

Re: MPC5200 + LocalPlus Bus + memcpy

[Andrey Volkov]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

Hello, Sascha.

On Wednesday, April 5, 2006, Sascha Hauer wrote:

> Hi all,

> I try to use jffs2 on a flash device connected to the mpc5200
> LocalPlus Bus. This bus does not allow misaligned accesses.
> The jffs2 code uses memcpy to copy from a word aligned address to an
> odd address. The ppc memcpy implementation first copies three bytes to get
> the target address word aligned, but then the source address is on an
> odd address. The following word accesses on this unaligned address fail
> badly.
Invalid crc on 'name' field ;)?

> I have fixed my problem by modifying the physmap mtd driver, but some
> day someone wants to connect SRAM to the LocalPlus Bus and I guess he
> will expect memcpy to work.
Heh, I'll have same problem. Patch (dirty hack)
attached (vs head of vanilla 2.6. tree)

> (BTW the arm implementation of memcpy seems to work around this problem)
Wrong, memcpy to/from SDRAM _may_ be unaligned, only
memcpy_fromio/memcpy_toio _must_ be aligned to even addresses.

P.S.
Paul, Sylvain, any suggestions to fix it?
AFAIK, memcpy_to/from_io must be it string.s, isn't it?

P.P.S Sacha if you start to write mscan driver, then please wait a
week, I planned to send my work to can-socket at next Friday.

-- 
Regards,
Andrey Volkov

[-- Attachment #2: mpc52xx_LPB_jffs2_hack.diff --]
[-- Type: application/octet-stream, Size: 1493 bytes --]

diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index cf55b22..3bf61d6 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -831,7 +831,7 @@ static int jffs2_scan_dirent_node(struct
 	if (!fd) {
 		return -ENOMEM;
 	}
-	memcpy(&fd->name, rd->name, rd->nsize);
+	memcpy_fromio(&fd->name, rd->name, rd->nsize);
 	fd->name[rd->nsize] = 0;
 
 	crc = crc32(0, fd->name, rd->nsize);
diff --git a/include/asm-ppc/io.h b/include/asm-ppc/io.h
index b919d8f..c2f173a 100644
--- a/include/asm-ppc/io.h
+++ b/include/asm-ppc/io.h
@@ -354,6 +354,29 @@ extern void _outsl_ns(volatile u32 __iom
 #define memset_io(a,b,c)       memset((void *)(a),(b),(c))
 #define memcpy_fromio(a,b,c)   memcpy((a),(void *)(b),(c))
 #define memcpy_toio(a,b,c)     memcpy((void *)(a),(b),(c))
+#elif defined (CONFIG_PPC_MPC52xx)
+static inline void memset_io(volatile void __iomem *addr, unsigned char val, int count)
+{
+	volatile u8 __iomem *addr_u8 = addr;
+	for(;count>0;count--)
+		*addr_u8++ = val;
+}
+
+static inline void memcpy_fromio(void *dst,const volatile void __iomem *src, int count)
+{
+	const volatile u8 __iomem *src_u8 = src;
+	u8 *dst_u8 = dst;
+	for(;count>0;count--)
+		*dst_u8++ = *src_u8++;
+}
+
+static inline void memcpy_toio(volatile void __iomem *dst, const void *src, int count)
+{
+	const u8 *src_u8 = src;
+	volatile u8 __iomem *dst_u8 = dst;
+	for(;count>0;count--)
+		*dst_u8++ = *src_u8++;
+}
 #else
 static inline void memset_io(volatile void __iomem *addr, unsigned char val, int count)
 {

Re: MPC5200 + LocalPlus Bus + memcpy

[Sascha Hauer]

Hello,

On Thu, Apr 06, 2006 at 06:21:44PM +0400, Andrey Volkov wrote:
> Hello, Sascha.
> 
> On Wednesday, April 5, 2006, Sascha Hauer wrote:
> 
> > Hi all,
> 
> > I try to use jffs2 on a flash device connected to the mpc5200
> > LocalPlus Bus. This bus does not allow misaligned accesses.
> > The jffs2 code uses memcpy to copy from a word aligned address to an
> > odd address. The ppc memcpy implementation first copies three bytes to get
> > the target address word aligned, but then the source address is on an
> > odd address. The following word accesses on this unaligned address fail
> > badly.
> Invalid crc on 'name' field ;)?

Yes, exactly ;)

> 
> > I have fixed my problem by modifying the physmap mtd driver, but some
> > day someone wants to connect SRAM to the LocalPlus Bus and I guess he
> > will expect memcpy to work.
> Heh, I'll have same problem. Patch (dirty hack)
> attached (vs head of vanilla 2.6. tree)
> 
> > (BTW the arm implementation of memcpy seems to work around this problem)
> Wrong, memcpy to/from SDRAM _may_ be unaligned, only
> memcpy_fromio/memcpy_toio _must_ be aligned to even addresses.

Hm, then a proper fix would be:
- implement an optimized version of memcpy_fromio/memcpy_toio, this
  could be a version of memcpy which only alignes on the io side
- use memcpy_fromio/memcpy_toio in jffs2 code

Do I see this right?

Is SRAM considered io? I know (Arm-)Boards which do not have SDRAM, they
run completely from SRAM.

> 
> P.S.
> Paul, Sylvain, any suggestions to fix it?
> AFAIK, memcpy_to/from_io must be it string.s, isn't it?
> 
> P.P.S Sacha if you start to write mscan driver, then please wait a
> week, I planned to send my work to can-socket at next Friday.

No I haven't started yet, and I will happily wait another week ;)

Sascha

Re[2]: MPC5200 + LocalPlus Bus + memcpy

[Andrey Volkov]

Hello, Sascha.

On Friday, April 7, 2006, Sascha Hauer wrote:

> Hello,

> On Thu, Apr 06, 2006 at 06:21:44PM +0400, Andrey Volkov wrote:
>> Hello, Sascha.
>> 
>> On Wednesday, April 5, 2006, Sascha Hauer wrote:
>> 
>> > Hi all,
>> 
>> > I try to use jffs2 on a flash device connected to the mpc5200
>> > LocalPlus Bus. This bus does not allow misaligned accesses.
>> > The jffs2 code uses memcpy to copy from a word aligned address to an
>> > odd address. The ppc memcpy implementation first copies three bytes to get
>> > the target address word aligned, but then the source address is on an
>> > odd address. The following word accesses on this unaligned address fail
>> > badly.
>> Invalid crc on 'name' field ;)?

> Yes, exactly ;)
:)
>> 
>> > I have fixed my problem by modifying the physmap mtd driver, but some
>> > day someone wants to connect SRAM to the LocalPlus Bus and I guess he
>> > will expect memcpy to work.
>> Heh, I'll have same problem. Patch (dirty hack)
>> attached (vs head of vanilla 2.6. tree)
>> 
>> > (BTW the arm implementation of memcpy seems to work around this problem)
>> Wrong, memcpy to/from SDRAM _may_ be unaligned, only
>> memcpy_fromio/memcpy_toio _must_ be aligned to even addresses.

> Hm, then a proper fix would be:
> - implement an optimized version of memcpy_fromio/memcpy_toio, this
>   could be a version of memcpy which only alignes on the io side
Yes.

> - use memcpy_fromio/memcpy_toio in jffs2 code
They already use it (from jffs2_read/write), only scan.c doesn't
calling this fns.

> Do I see this right?
Yes.

> Is SRAM considered io? I know (Arm-)Boards which do not have SDRAM, they
> run completely from SRAM.
Only LBP demand alignment, SDRAM/DDR and internal SRAM didn't.



-- 
Regards,
Andrey Volkov