The changes are to make sure to check the operation status. Actually the flash write and erase error behavior is caused on our products. The flash is Macronix flash device MX29GL512FHT2I-11G used by our products. The patch series was separated for changes of flash write and erase. Since those were not depended each other at the time. But by additional changes the changes are related more as same way currently. So combine patch series for the flash write and erase changes as v6.
Signed-off-by: Tokunori Ikegami ikegami@allied-telesis.co.jp Reviewed-by: Joakim Tjernlund Joakim.Tjernlund@infinera.com Cc: Chris Packham chris.packham@alliedtelesis.co.nz Cc: Brian Norris computersforpeace@gmail.com Cc: David Woodhouse dwmw2@infradead.org Cc: Boris Brezillon boris.brezillon@free-electrons.com Cc: Marek Vasut marek.vasut@gmail.com Cc: Richard Weinberger richard@nod.at Cc: Cyrille Pitchen cyrille.pitchen@wedev4u.fr Cc: linux-mtd@lists.infradead.org Cc: stable@vger.kernel.org
Tokunori Ikegami (5): mtd: cfi_cmdset_0002: Change write buffer to check correct value mtd: cfi_cmdset_0002: Change definition naming to retry write operation mtd: cfi_cmdset_0002: Change erase functions to retry for error mtd: cfi_cmdset_0002: Change erase functions to check chip good only mtd: cfi_cmdset_0002: Change erase one block to enable XIP once
drivers/mtd/chips/cfi_cmdset_0002.c | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-)
For the word write it is checked if the chip has the correct value. But it is not checked for the write buffer as only checked if ready. To make sure for the write buffer change to check the value.
It is enough as this patch is only checking the last written word. Since it is described by data sheets to check the operation status.
Signed-off-by: Tokunori Ikegami ikegami@allied-telesis.co.jp Reviewed-by: Joakim Tjernlund Joakim.Tjernlund@infinera.com Cc: Chris Packham chris.packham@alliedtelesis.co.nz Cc: Brian Norris computersforpeace@gmail.com Cc: David Woodhouse dwmw2@infradead.org Cc: Boris Brezillon boris.brezillon@free-electrons.com Cc: Marek Vasut marek.vasut@gmail.com Cc: Richard Weinberger richard@nod.at Cc: Cyrille Pitchen cyrille.pitchen@wedev4u.fr Cc: linux-mtd@lists.infradead.org Cc: stable@vger.kernel.org --- Changes since v7: - None.
Changes since v6: - Change Cc: Joakim Tjernlund tag as Reviewed-by: tag. - Add Cc: stable tag.
Changes since v5: - None since the v4 change is just combined as same series by v6.
Changes since v3: - Removed to check if chip ready.
Changes since v2: - Just updated the patch to add v3 version into subject and desribe a changelog.
Changes since v1: - Just changed to add Brian as Cc and the patch is sent to Boris-san as To
Changes since v0: - Changed Split the part to change write buffer from the v1 patch
drivers/mtd/chips/cfi_cmdset_0002.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index e397b80e40cc..1d6be8c63d39 100644 --- a/drivers/mtd/chips/cfi_cmdset_0002.c +++ b/drivers/mtd/chips/cfi_cmdset_0002.c @@ -1879,7 +1879,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip, if (time_after(jiffies, timeo) && !chip_ready(map, adr)) break;
- if (chip_ready(map, adr)) { + if (chip_good(map, adr, datum)) { xip_enable(map, chip, adr); goto op_done; }
The definition can be used for other program and erase operations also. So change the naming to MAX_RETRIES from MAX_WORD_RETRIES.
Signed-off-by: Tokunori Ikegami ikegami@allied-telesis.co.jp Reviewed-by: Joakim Tjernlund Joakim.Tjernlund@infinera.com Cc: Chris Packham chris.packham@alliedtelesis.co.nz Cc: Brian Norris computersforpeace@gmail.com Cc: David Woodhouse dwmw2@infradead.org Cc: Boris Brezillon boris.brezillon@free-electrons.com Cc: Marek Vasut marek.vasut@gmail.com Cc: Richard Weinberger richard@nod.at Cc: Cyrille Pitchen cyrille.pitchen@wedev4u.fr Cc: linux-mtd@lists.infradead.org Cc: stable@vger.kernel.org --- Changes since v7: - None.
Changes since v6: - Change Cc: Joakim Tjernlund tag as Reviewed-by: tag. - Add Cc: stable tag.
Changes since v5: - No change for this patch.
Changes since v4: - No change for this patch.
Changes since v3: - The patch series are just rebased since the coding style fix has been applied.
Changes since v2: - Change the definition naming to MAX_RETRIES from MAX_WRITE_RETRIES.
Changes since v1: - Just split the change from the v1 patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index 1d6be8c63d39..0b67f90415fd 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_RETRIES 3
#define SST49LF004B 0x0060 #define SST49LF040B 0x0050 @@ -1646,7 +1646,7 @@ static int __xipram do_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_RETRIES) goto retry;
ret = -EIO; @@ -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_RETRIES) goto retry;
ret = -EIO;
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. The flash controller is parallel Flash interface integrated on BCM53003.
Signed-off-by: Tokunori Ikegami ikegami@allied-telesis.co.jp Reviewed-by: Joakim Tjernlund Joakim.Tjernlund@infinera.com Cc: Chris Packham chris.packham@alliedtelesis.co.nz Cc: Brian Norris computersforpeace@gmail.com Cc: David Woodhouse dwmw2@infradead.org Cc: Boris Brezillon boris.brezillon@free-electrons.com Cc: Marek Vasut marek.vasut@gmail.com Cc: Richard Weinberger richard@nod.at Cc: Cyrille Pitchen cyrille.pitchen@wedev4u.fr Cc: linux-mtd@lists.infradead.org Cc: stable@vger.kernel.org --- Changes since v7: - None.
Changes since v6: - Change Cc: Joakim Tjernlund tag as Reviewed-by: tag. - Add Cc: stable tag.
Changes since v5: - No change for this patch.
Changes since v4: - Just fixed the commit message.
Changes since v3: - The patch series are just rebased since the coding style fix has been applied.
Changes since v2: - Just fix the commit message of patch to retry the chip erase error.
Changes since v1: - Just split the change from the v1 patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index 0b67f90415fd..e703900975d4 100644 --- a/drivers/mtd/chips/cfi_cmdset_0002.c +++ b/drivers/mtd/chips/cfi_cmdset_0002.c @@ -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;
@@ -2257,6 +2258,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 +2312,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_RETRIES) + goto retry; + ret = -EIO; }
@@ -2329,6 +2334,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 +2352,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 +2409,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_RETRIES) + goto retry; + ret = -EIO; }
Currently the functions use to check both chip ready and good. But the chip ready is not enough to check the operation status. So change this to check the chip good instead of this. About the retry functions to make sure the error handling remain it.
Signed-off-by: Tokunori Ikegami ikegami@allied-telesis.co.jp Reviewed-by: Joakim Tjernlund Joakim.Tjernlund@infinera.com Cc: Chris Packham chris.packham@alliedtelesis.co.nz Cc: Brian Norris computersforpeace@gmail.com Cc: David Woodhouse dwmw2@infradead.org Cc: Boris Brezillon boris.brezillon@free-electrons.com Cc: Marek Vasut marek.vasut@gmail.com Cc: Richard Weinberger richard@nod.at Cc: Cyrille Pitchen cyrille.pitchen@wedev4u.fr Cc: linux-mtd@lists.infradead.org Cc: stable@vger.kernel.org --- Changes since v7: - None.
Changes since v6: - Change Cc: Joakim Tjernlund tag as Reviewed-by: tag. - Add Cc: stable tag.
Changes since v5: - None since this change is implemented by v6 patch series.
drivers/mtd/chips/cfi_cmdset_0002.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index e703900975d4..6adda4dc2007 100644 --- a/drivers/mtd/chips/cfi_cmdset_0002.c +++ b/drivers/mtd/chips/cfi_cmdset_0002.c @@ -2294,12 +2294,13 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip) chip->erase_suspended = 0; }
- if (chip_ready(map, adr)) + if (chip_good(map, adr, map_word_ff(map))) break;
if (time_after(jiffies, timeo)) { printk(KERN_WARNING "MTD %s(): software timeout\n", __func__); + ret = -EIO; break; }
@@ -2307,15 +2308,15 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip) UDELAY(map, chip, adr, 1000000/HZ); } /* Did we succeed? */ - if (!chip_good(map, adr, map_word_ff(map))) { + if (ret) { /* reset on all failures. */ map_write(map, CMD(0xF0), chip->start); /* FIXME - should have reset delay before continuing */
- if (++retry_cnt <= MAX_RETRIES) + if (++retry_cnt <= MAX_RETRIES) { + ret = 0; goto retry; - - ret = -EIO; + } }
chip->state = FL_READY; @@ -2388,7 +2389,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip, chip->erase_suspended = 0; }
- if (chip_ready(map, adr)) { + if (chip_good(map, adr, map_word_ff(map))) { xip_enable(map, chip, adr); break; } @@ -2397,6 +2398,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip, xip_enable(map, chip, adr); printk(KERN_WARNING "MTD %s(): software timeout\n", __func__); + ret = -EIO; break; }
@@ -2404,15 +2406,15 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip, UDELAY(map, chip, adr, 1000000/HZ); } /* Did we succeed? */ - if (!chip_good(map, adr, map_word_ff(map))) { + if (ret) { /* reset on all failures. */ map_write(map, CMD(0xF0), chip->start); /* FIXME - should have reset delay before continuing */
- if (++retry_cnt <= MAX_RETRIES) + if (++retry_cnt <= MAX_RETRIES) { + ret = 0; goto retry; - - ret = -EIO; + } }
chip->state = FL_READY;
On Wed, 30 May 2018 18:32:25 +0900 Tokunori Ikegami ikegami@allied-telesis.co.jp wrote:
The changes are to make sure to check the operation status. Actually the flash write and erase error behavior is caused on our products. The flash is Macronix flash device MX29GL512FHT2I-11G used by our products. The patch series was separated for changes of flash write and erase. Since those were not depended each other at the time. But by additional changes the changes are related more as same way currently. So combine patch series for the flash write and erase changes as v6.
Applied to mtd/next.
Thanks,
Boris
Signed-off-by: Tokunori Ikegami ikegami@allied-telesis.co.jp Reviewed-by: Joakim Tjernlund Joakim.Tjernlund@infinera.com Cc: Chris Packham chris.packham@alliedtelesis.co.nz Cc: Brian Norris computersforpeace@gmail.com Cc: David Woodhouse dwmw2@infradead.org Cc: Boris Brezillon boris.brezillon@free-electrons.com Cc: Marek Vasut marek.vasut@gmail.com Cc: Richard Weinberger richard@nod.at Cc: Cyrille Pitchen cyrille.pitchen@wedev4u.fr Cc: linux-mtd@lists.infradead.org Cc: stable@vger.kernel.org
Tokunori Ikegami (5): mtd: cfi_cmdset_0002: Change write buffer to check correct value mtd: cfi_cmdset_0002: Change definition naming to retry write operation mtd: cfi_cmdset_0002: Change erase functions to retry for error mtd: cfi_cmdset_0002: Change erase functions to check chip good only mtd: cfi_cmdset_0002: Change erase one block to enable XIP once
drivers/mtd/chips/cfi_cmdset_0002.c | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-)
linux-stable-mirror@lists.linaro.org