Re: Fix recovery after failed write-buffer operation in cfi_cmdset_0002.c

Artem Bityutskiy
2012-11-21T07:47:59Z

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

Hi,

On Fri, 2012-11-09 at 14:23 +0100, Harald Nordgard-Hansen wrote:
> When working on a problem with some flash chips that lock up during
> write-buffer operations, I think there may be a bug in the linux
> handling of chips using cfi_cmdset_0002.c.
>
> The datasheets I have found for a number of these chips all specify that
> when aborting a write-buffer command, it is not enough to use the
> standard reset. Rather a "write-to-buffer-reset command" is needed.
> This command is quite similar for all chips, the main variance seem to
> be if the final 0xF0 can go to any address or must go to addr_unlock1.
>
> The bug is then in the recovery handling when timing out at the end of
> do_write_buffer, where using the normal reset command is not sufficient.
>
> Without this change, if the write-buffer command fails then any
> following operations on the flash also fail.
>
> The small patch here should apply against just about all kernels I've
> seen over the last 5 years, the code has not changed in this area for a
> long time...
>
> -Harald Nordgård-Hansen

Hi, your patch is not applicable, it is line-wrapped. Can you please
send a patch I can easily apply?
>
> --------
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c 2012-05-21
> 13:46:28.679794861 +0200
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c 2012-10-30
> 18:27:49.939109556 +0100
> @@ -1536,8 +1536,10 @@
> UDELAY(map, chip, adr, 1);
> }
>
> - /* reset on all failures. */
> - map_write( map, CMD(0xF0), chip->start );

Would be nice to put a short comment about what you do and why, may be
some reference as well. Just to make sure if someone reads the code,
he/she has some clue what are these about.

Thanks!

> + /* write-to-buffer-reset on all failures. */
> + cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
> cfi->device_type, NULL);
> + cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
> cfi->device_type, NULL);
> + cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, chip->start, map, cfi,
> cfi->device_type, NULL);
> xip_enable(map, chip, adr);
> /* FIXME - should have reset delay before continuing */

--
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]