Re: do_erase_oneblock failing to detect lock-bit failure

[Dan Eisenhut <deisenhut@wi.rr.com>]

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

On Thu, 2004-03-25 at 07:40, David Woodhouse wrote:
> On Thu, 2004-03-25 at 07:30 -0600, Dan Eisenhut wrote:
> > Is byte-lane swapping common?  
> 
> Not really. It's generally a symptom of hardware engineers who really
> don't understand the problem, and think it might help.

I looked into why they did this and apparently it was a recommendation
from Motorola.  We are using a MPC8270 processor.

> > Wouldn't this code fail for someone
> > without byte-lane swapping but requiring little endian enabled?  By
> > changing the if statements with (chipstatus & 0xNN) with (status &
> > CMD(0xNN)) appears to correct my problem, but I sure this is not the
> > best solution.
> 
> You made me think about it again and now my head hurts. I suspect that
> we should be using cfi_read_query() to read the status bits, not
> cfi_read(). 

I tried changing cfi_read to cfi_read_query, which sets status to
0x00a2.  But the immediate next check of "if (status & CMD(0x3a))" fails
since CMD(0x3a) resolves to 0x3a00.

Looking at it a little more, I made a couple changes which corrects the
problem for me.  (See attached diff)  How would this effect other boards
that are currently working on the existing code?  I'm not sure how
boards that do any kind of swapping were working in the first place. 
Maybe just never tested the return value on an erase to a locked block?

Dan

[-- Attachment #2: mtd_cfi_erase.diff --]
[-- Type: text/x-patch, Size: 1272 bytes --]

diff -uNr clean/drivers/mtd/chips/cfi_cmdset_0001.c mtd/drivers/mtd/chips/cfi_cmdset_0001.c
--- clean/drivers/mtd/chips/cfi_cmdset_0001.c	2004-03-26 10:33:48.000000000 -0600
+++ mtd/drivers/mtd/chips/cfi_cmdset_0001.c	2004-03-26 10:34:26.000000000 -0600
@@ -1373,8 +1373,8 @@
 
 	/* check for lock bit */
 	if (status & CMD(0x3a)) {
-		unsigned char chipstatus = status;
-		if (status != CMD(status & 0xff)) {
+		unsigned char chipstatus = cfi_to_cpu(status);
+		if (status != CMD(cfi_to_cpu(status) & 0xff)) {
 			int i;
 			for (i = 1; i<CFIDEV_INTERLEAVE; i++) {
 				      chipstatus |= status >> (cfi->device_type * 8);
diff -uNr clean/include/linux/mtd/cfi.h mtd/include/linux/mtd/cfi.h
--- clean/include/linux/mtd/cfi.h	2004-03-26 10:33:50.000000000 -0600
+++ mtd/include/linux/mtd/cfi.h	2004-03-26 10:35:42.000000000 -0600
@@ -473,6 +473,21 @@
 	}
 }
 
+static inline __u8 cfi_to_cpu(cfi_word val)
+{
+	if (cfi_buswidth_is_1()) {
+		return val;
+	} else if (cfi_buswidth_is_2()) {
+		return cfi16_to_cpu(val);
+	} else if (cfi_buswidth_is_4()) {
+		return cfi32_to_cpu(val);
+	} else if (cfi_buswidth_is_8()) {
+		return cfi64_to_cpu(val);
+	} else {
+		return 0;
+	}
+}
+
 static inline void cfi_udelay(int us)
 {
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,2,0)