From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.bootlin.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1fG69m-0007FP-NL for linux-mtd@lists.infradead.org; Tue, 08 May 2018 17:12:14 +0000 Date: Tue, 8 May 2018 19:11:44 +0200 From: Boris Brezillon To: IKEGAMI Tokunori Cc: Brian Norris , Boris Brezillon , Richard Weinberger , Marek Vasut , PACKHAM Chris , "linux-mtd@lists.infradead.org" , Cyrille Pitchen , David Woodhouse , Joakim Tjernlund Subject: Re: [PATCH] mtd: cfi_cmdset_0002: Change erase functions to retry for error Message-ID: <20180508191144.5d8412b6@bbrezillon> In-Reply-To: References: <20180508132037.152cc581@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , +Joakim On Tue, 8 May 2018 16:52:55 +0000 IKEGAMI Tokunori wrote: > From: Tokunori Ikegami > > 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. > > This is needed to prevent the flash erase error caused only once. > It was caused by the error case of chip_good() in the do_erase_oneblock(). > Also it was confirmed on the MACRONIX flash device MX29GL512FHT2I-11G. > But the error issue behavior is not able to reproduce at this moment. Hm, that's a problem. How can you be sure the retry approach fixes the issue if you can't reproduce the it? > The flash controller is parallel Flash interface integrated on BCM53003. > > Also the change is possible to resolve other erase error. > For example the Erase suspend issue was caused on Cypress AMD NOR flash. > S29GL01GS / S29GL512S / S29GL256S / S29GL128S Is the Erase suspend issue related to this problem? > > Signed-off-by: Tokunori Ikegami > Cc: Chris Packham > Cc: Brian Norris > Cc: David Woodhouse > Cc: Boris Brezillon > Cc: Marek Vasut > Cc: Richard Weinberger > Cc: Cyrille Pitchen > Cc: linux-mtd@lists.infradead.org > --- > drivers/mtd/chips/cfi_cmdset_0002.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c > index 56aa6b75213d..f522a856526a 100644 > --- a/drivers/mtd/chips/cfi_cmdset_0002.c > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c > @@ -43,6 +43,7 @@ > #define FORCE_WORD_WRITE 0 > > #define MAX_WORD_RETRIES 3 > +#define MAX_ERASE_RETRY 3 > > #define SST49LF004B 0x0060 > #define SST49LF040B 0x0050 > @@ -2240,6 +2241,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; > > @@ -2257,6 +2259,7 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip) > 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); > @@ -2310,6 +2313,9 @@ static int __xipram do_erase_chip(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_ERASE_RETRY) > + goto retry; > + > ret = -EIO; > } > > @@ -2329,6 +2335,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip, > unsigned long timeo = jiffies + HZ; > DECLARE_WAITQUEUE(wait, current); > int ret = 0; > + int retry_cnt = 0; > > adr += chip->start; > > @@ -2346,6 +2353,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip, > 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); > @@ -2402,6 +2410,9 @@ static int __xipram do_erase_oneblock(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_ERASE_RETRY) > + goto retry; > + > ret = -EIO; > } >