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

[Artem Bityutskiy <dedekind1@gmail.com>]

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