Unlocking with Intel J3 series flash

Unlocking with Intel J3 series flash

[Ralph Siemsen]

Greetings,

When using Intel J3 flash, unlocking operations are horribly slow.
The reason for this is that the J3 has "Flexible Block-locking" feature
(to quote that data sheet) which "allows blocks to be individually
locked and simultaneously unlocked".

Unlike other flash chips which power up either locked or unlocked,
the J3 series stores lock status in the flash itself (non-volatile).
Locking is reasonably fast (~64us) and can be done per-block, but
unlocking is very slow (~0.5s) and does the whole chip at once.

Current MTD code makes the situation much worse, because the
unlock() operation goes via cfi_varsize_frob() and iterates over
the block lists.  This is of course correct behaviour when the
cells are indivudually un-lockable, however it results in minute-long
delays and many needeless erase cycles for the J3 series.

I am looking for ideas on how to deal with this.  One idea would be
to introduce a cousin to the existing MTD_STUPID_LOCK, perhaps one
called MTD_STUPID_UNLOCK, which would then be used to ensure that
unlocking does not iterate over all flash blocks.

-R

Re: Unlocking with Intel J3 series flash

[Ben Dooks]

On Tue, Jul 31, 2007 at 11:35:47AM -0400, Ralph Siemsen wrote:
> Greetings,
> 
> When using Intel J3 flash, unlocking operations are horribly slow.
> The reason for this is that the J3 has "Flexible Block-locking" feature
> (to quote that data sheet) which "allows blocks to be individually
> locked and simultaneously unlocked".
> 
> Unlike other flash chips which power up either locked or unlocked,
> the J3 series stores lock status in the flash itself (non-volatile).
> Locking is reasonably fast (~64us) and can be done per-block, but
> unlocking is very slow (~0.5s) and does the whole chip at once.
> 
> Current MTD code makes the situation much worse, because the
> unlock() operation goes via cfi_varsize_frob() and iterates over
> the block lists.  This is of course correct behaviour when the
> cells are indivudually un-lockable, however it results in minute-long
> delays and many needeless erase cycles for the J3 series.
> 
> I am looking for ideas on how to deal with this.  One idea would be
> to introduce a cousin to the existing MTD_STUPID_LOCK, perhaps one
> called MTD_STUPID_UNLOCK, which would then be used to ensure that
> unlocking does not iterate over all flash blocks.

My solution, which I can post the patch for, is to check the lock
status of the page before trying to unlock the device.

| 2621-rc6-trizeps2-mtd-chip-unlock-check2.patch
|
| When migrating from Intel 28F128K3C to 28F128J3D parts,
| it was found that the device unlock command unlocks the
| entire flash.
|
| The 28F128J3D also takes time to unlock, even if the state
| is unlocked, thus an unlock of an partition (which sends the
| unlock command to all sectors) can take 10-15 seconds to
| complete.
|
| To work around this problem, this patch places a check in the
| unlock/lock code to check the status of the sector before issuing
| the command.
|
| Note, we make the assumption that all flashes return the correct
| sector status, and that there is no need to force an lock or
| unlock if the status is the same as the chip is reporting.
|
| Files affected:
|   drivers/mtd/chips/cfi_cmdset_0001.c |   25 	25 +	0 -	0 !
|   1 file changed, 25 insertions(+)
|
| Ben Dooks, Mon, 23 Apr 2007 21:46:45 +0100
|
| Signed-off-by: Ben Dooks <ben-linux@fluff.org> 

diff -urpN -X linux-2.6.21-rc6-mtdfix1/Documentation/dontdiff linux-2.6.21-rc6-teradyne-mtdfix1/drivers/mtd/chips/cfi_cmdset_0001.c linux-2.6.21-rc6/drivers/mtd/chips/cfi_cmdset_0001.c
--- linux-2.6.21-rc6-mtdfix1/drivers/mtd/chips/cfi_cmdset_0001.c	2007-04-10 11:37:40.000000000 +0100
+++ linux-2.6.21-rc6/drivers/mtd/chips/cfi_cmdset_0001.c	2007-04-23 21:44:23.000000000 +0100
@@ -1854,6 +1854,9 @@ static int __xipram do_xxlock_oneblock(s
 {
 	struct cfi_private *cfi = map->fldrv_priv;
 	struct cfi_pri_intelext *extp = cfi->cmdset_priv;
+	unsigned int ofs_factor = cfi->interleave * cfi->device_type;
+	map_word locked = CMD(0x01);
+	map_word status;
 	int udelay;
 	int ret;
 
@@ -1869,6 +1872,27 @@ static int __xipram do_xxlock_oneblock(s
 	ENABLE_VPP(map);
 	xip_disable(map, chip, adr);
 
+	/* check to see if the block is actually in the right state */
+
+	map_write(map, CMD(0x90), adr + (2 * ofs_factor));
+	chip->state = FL_JEDEC_QUERY;
+
+	status = map_read(map, adr + (2 * ofs_factor));
+
+	/* return the chip to a state */
+	map_write(map, CMD(0xf0), adr);
+	map_write(map, CMD(0xff), adr);
+
+	chip->state = FL_READY;
+
+	if (map_word_andequal(map, status, locked, locked)) {
+		if (thunk == DO_XXLOCK_ONEBLOCK_LOCK)
+			goto already;
+	} else {
+		if (thunk == DO_XXLOCK_ONEBLOCK_UNLOCK)
+			goto already;
+	}
+
 	map_write(map, CMD(0x60), adr);
 	if (thunk == DO_XXLOCK_ONEBLOCK_LOCK) {
 		map_write(map, CMD(0x01), adr);
@@ -1894,6 +1918,7 @@ static int __xipram do_xxlock_oneblock(s
 		goto out;
 	}
 
+already:
 	xip_enable(map, chip, adr);
 out:	put_chip(map, chip, adr);
 	spin_unlock(chip->mutex);

Re: Unlocking with Intel J3 series flash

[Ralph Siemsen]

On Tue, Jul 31, 2007 at 05:23:31PM +0100, Ben Dooks wrote:
 
> My solution, which I can post the patch for, is to check the lock
> status of the page before trying to unlock the device.

In principle this sounds like a good idea, but we then trust the
device to give us status... wherease prevously the unlock command
would be issued regardless.  I wonder if there are any errata
regarding device status... probably... here is one:
http://download.intel.com/design/flcomp/applnots/30855503.pdf
although your code looks like it will deal correctly with this.

> +	/* check to see if the block is actually in the right state */
> +
> +	map_write(map, CMD(0x90), adr + (2 * ofs_factor));
> +	chip->state = FL_JEDEC_QUERY;

Ok.

> +	status = map_read(map, adr + (2 * ofs_factor));

Any reason you're not doing:
	 status = cfi_read_query(map, adr+(2*ofs_factor));
as is done in the do_printlockstatus_oneblock() function?

> +	/* return the chip to a state */
> +	map_write(map, CMD(0xf0), adr);
> +	map_write(map, CMD(0xff), adr);
> +
> +	chip->state = FL_READY;

That sequence worries me... I don't know enough about the state
machines in the MTD code to know if it safe to do this here...

The rest looks ok.

I do wonder if this should be done in cfi_cmdset_0001, or should 
it happen more globally?  Are there AMD chips with the same "feature"?

-R

Re: Unlocking with Intel J3 series flash

[Ralph Siemsen]

On Tue, Jul 31, 2007 at 05:23:31PM +0100, Ben Dooks wrote:
> 
> My solution, which I can post the patch for, is to check the lock
> status of the page before trying to unlock the device.
> 
> | 2621-rc6-trizeps2-mtd-chip-unlock-check2.patch

I have tested this and it "works for me", thanks!

> | Note, we make the assumption that all flashes return the correct
> | sector status, and that there is no need to force an lock or
> | unlock if the status is the same as the chip is reporting.

That is indeed a bit of departure from previous way that unlock/lock
commands worked.  And I suspect its enough reason for this patch to
not be accepted into mainline.  Then again, there are bound to be a
whole lot of J3 flash parts out there.

So, can this patch be merged, and if not, what needs to be done to
make it acceptible?

-R

Re: Unlocking with Intel J3 series flash

[MikeW]

Ralph Siemsen <ralphs <at> netwinder.org> writes:

> 
> Greetings,
> 
> When using Intel J3 flash, unlocking operations are horribly slow.
> The reason for this is that the J3 has "Flexible Block-locking" feature
> (to quote that data sheet) which "allows blocks to be individually
> locked and simultaneously unlocked".
> ...snip...
> 
> I am looking for ideas on how to deal with this.  One idea would be
> to introduce a cousin to the existing MTD_STUPID_LOCK, perhaps one
> called MTD_STUPID_UNLOCK, which would then be used to ensure that
> unlocking does not iterate over all flash blocks.
> 
> -R
> 
> ____________________________

It would be nice to keep pejorative and uninformative naming
out of the codebase - may I suggest MTD_POWERON_LOCK and MTD_CHIP_UNLOCK.

But you get the idea - STUPID tells you nothing except the lack of
patience of the originator ;)

MikeW

Re: Unlocking with Intel J3 series flash

[David Woodhouse]

On Tue, 2007-07-31 at 16:23 +0000, MikeW wrote:
> It would be nice to keep pejorative and uninformative naming
> out of the codebase - may I suggest MTD_POWERON_LOCK and
> MTD_CHIP_UNLOCK.
> 
> But you get the idea - STUPID tells you nothing except the lack of
> patience of the originator ;) 

It's true. Who knew Intel would come up with _more_ ways of breaking
locking, thus making MTD_STUPID_LOCK ambiguous? :)

-- 
dwmw2