cfi_cmdset_0001 unaligned write problems.
Either this code is suspect or I need more caffeine. Probably both.
Scenario: Buswidth is 2. CPU is big-endian. Writing '5a' to an odd address.
/* If it's not bus-aligned, do the first byte write */
if (ofs & (CFIDEV_BUSWIDTH-1)) {
unsigned long bus_ofs = ofs & ~(CFIDEV_BUSWIDTH-1);
int i = 0, n = 0;
u_char tmp_buf[4];
__u32 datum;
while (bus_ofs++ < ofs)
tmp_buf[i++] = 0xff;
while (len && i < CFIDEV_BUSWIDTH)
tmp_buf[i++] = buf[n++], len--;
while (i < CFIDEV_BUSWIDTH)
tmp_buf[i++] = 0xff;// now tmp_buf is 0xff,0x5a,xx,yy
if (cfi_buswidth_is_2()) {
datum = *(__u16*)tmp_buf;// now datum is 0xff5a (stored as 0,0,ff,5a)
} else if (cfi_buswidth_is_4()) {
datum = *(__u32*)tmp_buf;
} else {
return -EINVAL; /* should never happen, but be safe */
}
ret = do_write_oneword(map, &cfi->chips[chipnum],
ofs, datum);// do_write_oneword writes 2 bytes at &datum, which is 0,0.
// What's more, it writes it to 'ofs' (and faults on the unaligned access) // instead of to buf_ofs, which was presumably the intention.
if (ret)
return ret;
ofs += n;
buf += n;
(*retlen) += n;
if (ofs >> cfi->chipshift) {
chipnum ++;
ofs = 0;
if (chipnum == cfi->numchips)
return 0;
}
}--
dwmw2
To unsubscribe, send "unsubscribe mtd" to [email protected]
Quoted 32 lines · David Woodhouse
Either this code is suspect or I need more caffeine. Probably both.
Scenario: Buswidth is 2. CPU is big-endian. Writing '5a' to an odd address./* If it's not bus-aligned, do the first byte write */ if (ofs & (CFIDEV_BUSWIDTH-1)) { unsigned long bus_ofs = ofs & ~(CFIDEV_BUSWIDTH-1); int i = 0, n = 0; u_char tmp_buf[4]; __u32 datum; while (bus_ofs++ < ofs) tmp_buf[i++] = 0xff; while (len && i < CFIDEV_BUSWIDTH) tmp_buf[i++] = buf[n++], len--; while (i < CFIDEV_BUSWIDTH) tmp_buf[i++] = 0xff;// now
tmp_bufis 0xff,0x5a,xx,yyif (cfi_buswidth_is_2()) { datum = *(__u16*)tmp_buf;// now datum is
0xff5a(stored as 0,0,ff,5a)} else if (cfi_buswidth_is_4()) { datum = *(__u32*)tmp_buf; } else { return -EINVAL; /* should never happen, but be safe */ } ret = do_write_oneword(map, &cfi->chips[chipnum], ofs, datum);
Quoted 3 lines
//
do_write_onewordwrites 2 bytes at &datum, which is 0,0.
// What's more, it writes it to 'ofs' (and faults on the unaligned access) // instead of tobuf_ofs, which was presumably the intention.
Yeah... I just fixed that one... Wonder why it worked for me till now...
However I suspect that there might be an issue with __u16 promoted to __u32 then back to __u16 on big endian CPUs. What is the expected behavior?
On LE, 0x1234 becomes 0x00001234 the back to 0x1234 which is fine...
Nicolas
To unsubscribe, send "unsubscribe mtd" to [email protected]
Quick response :)
Quoted 2 lines · [email protected] said:
Yeah... I just fixed that one... Wonder why it worked for me till
now...
If you're testing with JFFS you won't have exercised that code.
Quoted 5 lines
However I suspect that there might be an issue with
__u16promoted to__u32then back to__u16on big endian CPUs. What is the expected behavior?On LE,
0x1234becomes0x00001234the back to0x1234which is fine...
On BE, I think it'll match the comments I put in.
(writing 0x5a to an odd address)tmp_buf holds {0xff, 0x5a, xx, yy}
*(__u16)tmp_buf is 0xff5a
datum is 0x0000ff5a, which is stored as 0x00,0x00,0xff,0x5a do_write_oneword uses the two bytes at &datum, which are 0x00 and 0x00.
I have no BE machine to test on ATM, though.
I was also seeing problems with JFFS - whatever I was writing to the flash wasn't 'taking'. Immediately reading files back would give corrupted data, and the filesystem was entirely untouched on rebooting - it didn't even complain of invalid nodes.
I very much doubt that was the same problem - I'm running on SH3 where the unaligned trap would kill it first time. The unaligned problems showed up when I was using 'cat' to write to the flash, to check it was working.
I've reverted to v1.25 for now.
--
dwmw2
To unsubscribe, send "unsubscribe mtd" to [email protected]
[...]
Quoted 5 lines
However I suspect that there might be an issue with
__u16promoted to__u32then back to__u16on big endian CPUs. What is the expected behavior?On LE,
0x1234becomes0x00001234the back to0x1234which is fine...
Ditto on BE. Endianness only matters when you are reading and writing non-quad values to memory. The internal representations are identical.
Signature · 7 lines
David Given [email protected] To unsubscribe, send "unsubscribe mtd" to [email protected]
Quoted 3 lines · [email protected] said:
Ditto on BE. Endianness only matters when you are reading and writing
non-quad values to memory. The internal representations are identical.
Yep, I was misunderstanding what do_write_oneword() does. In fact, it passes datum intact to cfi_write() which does the correct thing, rather than passing it by reference. That code does look OK.
--
dwmw2
To unsubscribe, send "unsubscribe mtd" to [email protected]