cfi_cmdset_001.c problem

cfi_cmdset_001.c problem

[Aleksander Sanochkin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 514 bytes --]

Hi!

I see that after write and erase operations,
the current version of cfi_cmdset_0001.c driver
can left a Flash device not in the READY state.
If soft reboot is performed in such a situation,
applications that use flash will behave erroneously
since they expect that the flash is in the READY state.
For instance, if the system is supposed to boot from flash,
it will hang.

I've made a patch that fixes the problem for me - it is attached.
I was hoping it might be helpful for other people too.

Best regards.

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2442 bytes --]

diff -ur cvs/drivers/mtd/chips/cfi_cmdset_0001.c ready_state/drivers/mtd/chips/cfi_cmdset_0001.c
--- cvs/drivers/mtd/chips/cfi_cmdset_0001.c	Tue Oct  2 19:05:11 2001
+++ ready_state/drivers/mtd/chips/cfi_cmdset_0001.c	Wed Nov 21 12:57:27 2001
@@ -170,8 +170,10 @@
 	map->fldrv = &cfi_intelext_chipdrv;
 	MOD_INC_USE_COUNT;
 	
+	/* Clear Status Register. */
+	cfi_send_gen_cmd(0x50, 0x55, base, map, cfi, cfi->device_type, NULL);
+
 	/* Make sure it's in read mode */
-	cfi_send_gen_cmd(0xff, 0x55, base, map, cfi, cfi->device_type, NULL);
 	return cfi_intelext_setup(map);
 }
 
@@ -551,12 +553,16 @@
 	if (status & CMD(0x02)) {
 		/* clear status */
 		cfi_write(map, CMD(0x50), adr);
-		/* put back into read status register mode */
-		cfi_write(map, CMD(0x70), adr);
+		/* Return to the read array mode. */
+		cfi_write(map, CMD(0xff), adr);
+		chip->state = FL_READY;
 		wake_up(&chip->wq);
 		spin_unlock_bh(chip->mutex);
 		return -EROFS;
 	}
+	/* Return to the read array mode. */
+	cfi_write(map, CMD(0xff), adr);
+	chip->state = FL_READY;
 	wake_up(&chip->wq);
 	spin_unlock_bh(chip->mutex);
 	return 0;
@@ -839,12 +845,16 @@
 	if (status & CMD(0x02)) {
 		/* clear status */
 		cfi_write(map, CMD(0x50), cmd_adr);
-		/* put back into read status register mode */
-		cfi_write(map, CMD(0x70), adr);
+		/* Return to the read array mode. */
+		cfi_write(map, CMD(0xff), adr);
+		chip->state = FL_READY;
 		wake_up(&chip->wq);
 		spin_unlock_bh(chip->mutex);
 		return -EROFS;
 	}
+	/* Return to the read array mode. */
+	cfi_write(map, CMD(0xff), adr);
+	chip->state = FL_READY;
 	wake_up(&chip->wq);
 	spin_unlock_bh(chip->mutex);
 	return 0;
@@ -1077,6 +1087,10 @@
 		}
 	}
 
+	/* Return to the read array mode. */
+	cfi_write(map, CMD(0xff), adr);
+	chip->state = FL_READY;
+
 	wake_up(&chip->wq);
 	spin_unlock_bh(chip->mutex);
 	return ret;
@@ -1319,7 +1333,9 @@
 	}
 	
 	/* Done and happy. */
-	chip->state = FL_STATUS;
+	/* Return to the read array mode. */
+	cfi_write(map, CMD(0xff), adr);
+	chip->state = FL_READY;
 	DISABLE_VPP(map);
 	wake_up(&chip->wq);
 	spin_unlock_bh(chip->mutex);
@@ -1468,7 +1484,9 @@
 	}
 	
 	/* Done and happy. */
-	chip->state = FL_STATUS;
+	/* Return to the read array mode. */
+	cfi_write(map, CMD(0xff), adr);
+	chip->state = FL_READY;
 	DISABLE_VPP(map);
 	wake_up(&chip->wq);
 	spin_unlock_bh(chip->mutex);

Re: cfi_cmdset_001.c problem

[David Woodhouse]

asanochkin@Lnxw.COM said:
>  I see that after write and erase operations, the current version of
> cfi_cmdset_0001.c driver can left a Flash device not in the READY
> state. If soft reboot is performed in such a situation, applications
> that use flash will behave erroneously since they expect that the
> flash is in the READY state. For instance, if the system is supposed
> to boot from flash, it will hang.

Thanks for this. One thing that concerns me is that I have a vague 
recollection that Nico once drastically improved the performance of the 
same driver by ripping out all the gratuitous state changes. I wonder if it 
would be better to do this with a reboot notifier than by changing back to 
read mode after every other operation?

Also, we need to make sure we do the right thing if the JFFS or JFFS2 GC
thread is actually in the process of writing to the flash while
machine_restart() happens on another CPU.

--
dwmw2

Re: cfi_cmdset_001.c problem

[Aleksander Sanochkin]

> > I see that after write and erase operations, the current version of
> > cfi_cmdset_0001.c driver can left a Flash device not in the READY
> > state. If soft reboot is performed in such a situation, applications
> > that use flash will behave erroneously since they expect that the
> > flash is in the READY state. For instance, if the system is supposed
> > to boot from flash, it will hang.
>
> Thanks for this. One thing that concerns me is that I have a vague
> recollection that Nico once drastically improved the performance of the
> same driver by ripping out all the gratuitous state changes.

I seem to remember this, too. But it was in the read() routine. It used to
switch flash to the status mode and then back to ready even if it had been
ready right away. Our modification affects only write and erase routines.

> I wonder if it would be better to do this with a reboot notifier
> than by changing back to read mode after every other operation?

Probably you are right. The thing is that our modification was originally
made in the context of 2.2.x kernel, which didn't have any reboot
notifiers.

> Also, we need to make sure we do the right thing if the JFFS or JFFS2 GC
> thread is actually in the process of writing to the flash while
> machine_restart() happens on another CPU.

In this case, our patch won't help. A reboot notifier probably would.

Re: cfi_cmdset_001.c problem

[David Woodhouse]

asanochkin@Lnxw.COM said:
>  I seem to remember this, too. But it was in the read() routine. It
> used to switch flash to the status mode and then back to ready even if
> it had been ready right away. Our modification affects only write and
> erase routines.

OK, if you're sure it doesn't adversely affect performance, then please go 
ahead and commit it.

--
dwmw2