[PATCH] mtd: cfi_cmdset_0002: Change erase functions to retry for error
Boris Brezillon
boris.brezillon at bootlin.com
Tue May 8 04:20:37 PDT 2018
Hello,
On Tue, 8 May 2018 10:20:29 +0000
IKEGAMI Tokunori <ikegami at allied-telesis.co.jp> wrote:
> From: Tokunori Ikegami <ikegami at allied-telesis.co.jp>
>
> For the word write functions it is retried for error.
> But it is not implemented to retry for the erase functions.
> To make sure for the erase functions change to retry as same.
Can you give a bit more context, like why this is needed, where did
you see the problem (which chip/controller), ...
>
> Signed-off-by: Tokunori Ikegami <ikegami at allied-telesis.co.jp>
> Cc: Chris Packham <chris.packham at alliedtelesis.co.nz>
> Cc: David Woodhouse <dwmw2 at infradead.org>
> Cc: Boris Brezillon <boris.brezillon at free-electrons.com>
> Cc: Marek Vasut <marek.vasut at gmail.com>
> Cc: Richard Weinberger <richard at nod.at>
> Cc: Cyrille Pitchen <cyrille.pitchen at wedev4u.fr>
> Cc: linux-mtd at lists.infradead.org
> ---
> drivers/mtd/chips/cfi_cmdset_0002.c | 36 +++++++++++++++++++++++-------------
> 1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 56aa6b75213d..e2dc020cd102 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -42,7 +42,7 @@
> #define AMD_BOOTLOC_BUG
> #define FORCE_WORD_WRITE 0
>
> -#define MAX_WORD_RETRIES 3
> +#define MAX_WRITE_RETRIES 3
Not sure why you rename this macro, but if it's needed can you do that
in a separate patch?
>
> #define SST49LF004B 0x0060
> #define SST49LF040B 0x0050
> @@ -207,7 +207,7 @@ static void fixup_use_write_buffers(struct mtd_info *mtd)
> struct map_info *map = mtd->priv;
> struct cfi_private *cfi = map->fldrv_priv;
> if (cfi->cfiq->BufWriteTimeoutTyp) {
> - pr_debug("Using buffer write method\n" );
> + pr_debug("Using buffer write method\n");
Same for coding style issues, they should be fixed in a separate commit.
> mtd->_write = cfi_amdstd_write_buffers;
> }
> }
> @@ -1577,7 +1577,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
> }
>
> pr_debug("MTD %s(): WRITE 0x%.8lx(0x%.8lx)\n",
> - __func__, adr, datum.x[0] );
> + __func__, adr, datum.x[0]);
>
> if (mode == FL_OTP_WRITE)
> otp_enter(map, chip, adr, map_bankwidth(map));
> @@ -1643,10 +1643,10 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
> /* Did we succeed? */
> if (!chip_good(map, adr, datum)) {
> /* reset on all failures. */
> - map_write( map, CMD(0xF0), chip->start );
> + map_write(map, CMD(0xF0), chip->start);
> /* FIXME - should have reset delay before continuing */
>
> - if (++retry_cnt <= MAX_WORD_RETRIES)
> + if (++retry_cnt <= MAX_WRITE_RETRIES)
> goto retry;
>
> ret = -EIO;
> @@ -1821,7 +1821,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> datum = map_word_load(map, buf);
>
> pr_debug("MTD %s(): WRITE 0x%.8lx(0x%.8lx)\n",
> - __func__, adr, datum.x[0] );
> + __func__, adr, datum.x[0]);
>
> XIP_INVAL_CACHED_RANGE(map, adr, len);
> ENABLE_VPP(map);
> @@ -2105,7 +2105,7 @@ static int do_panic_write_oneword(struct map_info *map, struct flchip *chip,
> map_write(map, CMD(0xF0), chip->start);
> /* FIXME - should have reset delay before continuing */
>
> - if (++retry_cnt <= MAX_WORD_RETRIES)
> + if (++retry_cnt <= MAX_WRITE_RETRIES)
> goto retry;
>
> ret = -EIO;
> @@ -2240,6 +2240,7 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip)
> unsigned long int adr;
> DECLARE_WAITQUEUE(wait, current);
> int ret = 0;
> + int retry_cnt = 0;
>
> adr = cfi->addr_unlock1;
>
> @@ -2251,12 +2252,13 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip)
> }
>
> pr_debug("MTD %s(): ERASE 0x%.8lx\n",
> - __func__, chip->start );
> + __func__, chip->start);
>
> XIP_INVAL_CACHED_RANGE(map, adr, map->size);
> ENABLE_VPP(map);
> xip_disable(map, chip, adr);
>
> + retry:
> 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(0x80, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
> @@ -2297,7 +2299,7 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip)
>
> if (time_after(jiffies, timeo)) {
> printk(KERN_WARNING "MTD %s(): software timeout\n",
> - __func__ );
> + __func__);
> break;
> }
>
> @@ -2307,9 +2309,12 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip)
> /* Did we succeed? */
> if (!chip_good(map, adr, map_word_ff(map))) {
> /* reset on all failures. */
> - map_write( map, CMD(0xF0), chip->start );
> + map_write(map, CMD(0xF0), chip->start);
> /* FIXME - should have reset delay before continuing */
>
> + if (++retry_cnt <= MAX_WRITE_RETRIES)
> + goto retry;
Shouldn't we have a MAX_ERASE_RETRY for that? Also, what makes you
think the following erase operations will work?
Thanks,
Boris
More information about the linux-mtd
mailing list