cfi_cmdset_0001 unaligned write problems.

cfi_cmdset_0001 unaligned write problems.

[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_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 majordomo@infradead.org

Re: cfi_cmdset_0001 unaligned write problems.

[Nicolas Pitre]

On Thu, 19 Oct 2000, David Woodhouse wrote:

> 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.

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 majordomo@infradead.org

Re: cfi_cmdset_0001 unaligned write problems.

[David Woodhouse]

Quick response :)

nico@cam.org 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. 

> 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... 

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 majordomo@infradead.org

Re: cfi_cmdset_0001 unaligned write problems.

[David Given]

[...]
>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...

Ditto on BE. Endianness only matters when you are reading and writing non-quad 
values to memory. The internal representations are identical.


-- 
David Given
dg@tao-group.com




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: cfi_cmdset_0001 unaligned write problems.

[David Woodhouse]

dg@tao-group.com 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 majordomo@infradead.org