* [PATCH v2 0/2] Fix the HyperFlash support in the AMD/Fujitsu/Spansion CFI driver
@ 2019-10-31 20:35 Sergei Shtylyov
2019-10-31 20:37 ` [PATCH v2 1/2] mtd: cfi_cmdset_0002: only check errors when ready in cfi_check_err_status() Sergei Shtylyov
2019-10-31 20:39 ` [PATCH v2 2/2] mtd: cfi_cmdset_0002: fix delayed error detection on HyperFlash Sergei Shtylyov
0 siblings, 2 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2019-10-31 20:35 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Marek Vasut, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, linux-mtd
Hello!
Here's a set of 2 patches against the 'mtd/fixes' branch of the MTD repo.
Linux 5.4-rc1 received initial support for the HyperFlash. However, the patch
adding the status register polling to the AMD/Fujitsu/Spansion CFI driver has
couple issues which I'm trying to solve here...
[1/2] mtd: cfi_cmdset_0002: only check errors when ready in cfi_check_err_status()
[2/2] mtd: cfi_cmdset_0002: fix delayed error detection on HyperFlash
MBR, Sergei
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/2] mtd: cfi_cmdset_0002: only check errors when ready in cfi_check_err_status()
2019-10-31 20:35 [PATCH v2 0/2] Fix the HyperFlash support in the AMD/Fujitsu/Spansion CFI driver Sergei Shtylyov
@ 2019-10-31 20:37 ` Sergei Shtylyov
2019-10-31 20:39 ` [PATCH v2 2/2] mtd: cfi_cmdset_0002: fix delayed error detection on HyperFlash Sergei Shtylyov
1 sibling, 0 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2019-10-31 20:37 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Marek Vasut, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, linux-mtd
Cypress S26K{L|S}P{128|256|512}S datasheet says that the error bits in
the status register are only valid when the "device ready" bit 7 is set.
Add the check for the device ready bit in cfi_check_err_status() as that
function isn't always called with this bit set.
Fixes: 4844ef80305d ("mtd: cfi_cmdset_0002: Add support for polling status register")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
Changes in version 2:
- refreshed the patch.
drivers/mtd/chips/cfi_cmdset_0002.c | 4 ++++
1 file changed, 4 insertions(+)
Index: linux/drivers/mtd/chips/cfi_cmdset_0002.c
===================================================================
--- linux.orig/drivers/mtd/chips/cfi_cmdset_0002.c
+++ linux/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -136,6 +136,10 @@ static void cfi_check_err_status(struct
cfi->device_type, NULL);
status = map_read(map, adr);
+ /* The error bits are invalid while the chip's busy */
+ if (!map_word_bitsset(map, status, CMD(CFI_SR_DRB)))
+ return;
+
if (map_word_bitsset(map, status, CMD(0x3a))) {
unsigned long chipstatus = MERGESTATUS(status);
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] mtd: cfi_cmdset_0002: fix delayed error detection on HyperFlash
2019-10-31 20:35 [PATCH v2 0/2] Fix the HyperFlash support in the AMD/Fujitsu/Spansion CFI driver Sergei Shtylyov
2019-10-31 20:37 ` [PATCH v2 1/2] mtd: cfi_cmdset_0002: only check errors when ready in cfi_check_err_status() Sergei Shtylyov
@ 2019-10-31 20:39 ` Sergei Shtylyov
2019-11-12 16:46 ` Tokunori Ikegami
1 sibling, 1 reply; 4+ messages in thread
From: Sergei Shtylyov @ 2019-10-31 20:39 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Marek Vasut, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, linux-mtd
The commit 4844ef80305d ("mtd: cfi_cmdset_0002: Add support for polling
status register") added checking for the status register error bits into
chip_good() to only return 1 if these bits are 0s. Unfortunately, this
means that polling using chip_good() always reaches a timeout condition
when erase or program failure bits are set. Let's fully delegate the task
of determining the error conditions to cfi_check_err_status() and make
chip_good() only look for the Device Ready/Busy condition.
Fixes: 4844ef80305d ("mtd: cfi_cmdset_0002: Add support for polling status register")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
Changes in version 2:
- added extra check for error bits in cfi_check_err_status();
- reverted from pr_warn() to pr_err() call in do_write_buffer();
- reworded the patch description.
drivers/mtd/chips/cfi_cmdset_0002.c | 58 ++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 25 deletions(-)
Index: linux/drivers/mtd/chips/cfi_cmdset_0002.c
===================================================================
--- linux.orig/drivers/mtd/chips/cfi_cmdset_0002.c
+++ linux/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -123,14 +123,14 @@ static int cfi_use_status_reg(struct cfi
(extp->SoftwareFeatures & poll_mask) == CFI_POLL_STATUS_REG;
}
-static void cfi_check_err_status(struct map_info *map, struct flchip *chip,
- unsigned long adr)
+static int cfi_check_err_status(struct map_info *map, struct flchip *chip,
+ unsigned long adr)
{
struct cfi_private *cfi = map->fldrv_priv;
map_word status;
if (!cfi_use_status_reg(cfi))
- return;
+ return 0;
cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi,
cfi->device_type, NULL);
@@ -138,7 +138,7 @@ static void cfi_check_err_status(struct
/* The error bits are invalid while the chip's busy */
if (!map_word_bitsset(map, status, CMD(CFI_SR_DRB)))
- return;
+ return 0;
if (map_word_bitsset(map, status, CMD(0x3a))) {
unsigned long chipstatus = MERGESTATUS(status);
@@ -155,7 +155,12 @@ static void cfi_check_err_status(struct
if (chipstatus & CFI_SR_SLSB)
pr_err("%s sector write protected, status %lx\n",
map->name, chipstatus);
+
+ /* Erase/Program status bits are set on the operation failure */
+ if (chipstatus & (CFI_SR_ESB | CFI_SR_PSB))
+ return 1;
}
+ return 0;
}
/* #define DEBUG_CFI_FEATURES */
@@ -852,20 +857,16 @@ static int __xipram chip_good(struct map
if (cfi_use_status_reg(cfi)) {
map_word ready = CMD(CFI_SR_DRB);
- map_word err = CMD(CFI_SR_PSB | CFI_SR_ESB);
+
/*
* For chips that support status register, check device
- * ready bit and Erase/Program status bit to know if
- * operation succeeded.
+ * ready bit
*/
cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi,
cfi->device_type, NULL);
curd = map_read(map, addr);
- if (map_word_andequal(map, curd, ready, ready))
- return !map_word_bitsset(map, curd, err);
-
- return 0;
+ return map_word_andequal(map, curd, ready, ready);
}
oldd = map_read(map, addr);
@@ -1703,8 +1704,11 @@ static int __xipram do_write_oneword_onc
break;
}
- if (chip_good(map, chip, adr, datum))
+ if (chip_good(map, chip, adr, datum)) {
+ if (cfi_check_err_status(map, chip, adr))
+ ret = -EIO;
break;
+ }
/* Latency issues. Drop the lock, wait a while and retry */
UDELAY(map, chip, adr, 1);
@@ -1777,7 +1781,6 @@ static int __xipram do_write_oneword_ret
ret = do_write_oneword_once(map, chip, adr, datum, mode, cfi);
if (ret) {
/* reset on all failures. */
- cfi_check_err_status(map, chip, adr);
map_write(map, CMD(0xF0), chip->start);
/* FIXME - should have reset delay before continuing */
@@ -1974,12 +1977,17 @@ static int __xipram do_write_buffer_wait
*/
if (time_after(jiffies, timeo) &&
!chip_good(map, chip, adr, datum)) {
+ pr_err("MTD %s(): software timeout, address:0x%.8lx.\n",
+ __func__, adr);
ret = -EIO;
break;
}
- if (chip_good(map, chip, adr, datum))
+ if (chip_good(map, chip, adr, datum)) {
+ if (cfi_check_err_status(map, chip, adr))
+ ret = -EIO;
break;
+ }
/* Latency issues. Drop the lock, wait a while and retry */
UDELAY(map, chip, adr, 1);
@@ -2075,12 +2083,8 @@ static int __xipram do_write_buffer(stru
chip->word_write_time);
ret = do_write_buffer_wait(map, chip, adr, datum);
- if (ret) {
- cfi_check_err_status(map, chip, adr);
+ if (ret)
do_write_buffer_reset(map, chip, cfi);
- pr_err("MTD %s(): software timeout, address:0x%.8lx.\n",
- __func__, adr);
- }
xip_enable(map, chip, adr);
@@ -2275,9 +2279,9 @@ retry:
udelay(1);
}
- if (!chip_good(map, chip, adr, datum)) {
+ if (!chip_good(map, chip, adr, datum) ||
+ cfi_check_err_status(map, chip, adr)) {
/* reset on all failures. */
- cfi_check_err_status(map, chip, adr);
map_write(map, CMD(0xF0), chip->start);
/* FIXME - should have reset delay before continuing */
@@ -2471,8 +2475,11 @@ static int __xipram do_erase_chip(struct
chip->erase_suspended = 0;
}
- if (chip_good(map, chip, adr, map_word_ff(map)))
+ if (chip_good(map, chip, adr, map_word_ff(map))) {
+ if (cfi_check_err_status(map, chip, adr))
+ ret = -EIO;
break;
+ }
if (time_after(jiffies, timeo)) {
printk(KERN_WARNING "MTD %s(): software timeout\n",
@@ -2487,7 +2494,6 @@ static int __xipram do_erase_chip(struct
/* Did we succeed? */
if (ret) {
/* reset on all failures. */
- cfi_check_err_status(map, chip, adr);
map_write(map, CMD(0xF0), chip->start);
/* FIXME - should have reset delay before continuing */
@@ -2568,8 +2574,11 @@ static int __xipram do_erase_oneblock(st
chip->erase_suspended = 0;
}
- if (chip_good(map, chip, adr, map_word_ff(map)))
+ if (chip_good(map, chip, adr, map_word_ff(map))) {
+ if (cfi_check_err_status(map, chip, adr))
+ ret = -EIO;
break;
+ }
if (time_after(jiffies, timeo)) {
printk(KERN_WARNING "MTD %s(): software timeout\n",
@@ -2584,7 +2593,6 @@ static int __xipram do_erase_oneblock(st
/* Did we succeed? */
if (ret) {
/* reset on all failures. */
- cfi_check_err_status(map, chip, adr);
map_write(map, CMD(0xF0), chip->start);
/* FIXME - should have reset delay before continuing */
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] mtd: cfi_cmdset_0002: fix delayed error detection on HyperFlash
2019-10-31 20:39 ` [PATCH v2 2/2] mtd: cfi_cmdset_0002: fix delayed error detection on HyperFlash Sergei Shtylyov
@ 2019-11-12 16:46 ` Tokunori Ikegami
0 siblings, 0 replies; 4+ messages in thread
From: Tokunori Ikegami @ 2019-11-12 16:46 UTC (permalink / raw)
To: Sergei Shtylyov, David Woodhouse, Brian Norris, Marek Vasut,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-mtd
Hi,
Looks good.
Reviewed-by: Tokunori Ikegami <ikegami.t@gmail.com>
Regards,
Ikegami
On 2019/11/01 5:39, Sergei Shtylyov wrote:
> The commit 4844ef80305d ("mtd: cfi_cmdset_0002: Add support for polling
> status register") added checking for the status register error bits into
> chip_good() to only return 1 if these bits are 0s. Unfortunately, this
> means that polling using chip_good() always reaches a timeout condition
> when erase or program failure bits are set. Let's fully delegate the task
> of determining the error conditions to cfi_check_err_status() and make
> chip_good() only look for the Device Ready/Busy condition.
>
> Fixes: 4844ef80305d ("mtd: cfi_cmdset_0002: Add support for polling status register")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> Changes in version 2:
> - added extra check for error bits in cfi_check_err_status();
> - reverted from pr_warn() to pr_err() call in do_write_buffer();
> - reworded the patch description.
>
> drivers/mtd/chips/cfi_cmdset_0002.c | 58 ++++++++++++++++++++----------------
> 1 file changed, 33 insertions(+), 25 deletions(-)
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-12 16:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-31 20:35 [PATCH v2 0/2] Fix the HyperFlash support in the AMD/Fujitsu/Spansion CFI driver Sergei Shtylyov
2019-10-31 20:37 ` [PATCH v2 1/2] mtd: cfi_cmdset_0002: only check errors when ready in cfi_check_err_status() Sergei Shtylyov
2019-10-31 20:39 ` [PATCH v2 2/2] mtd: cfi_cmdset_0002: fix delayed error detection on HyperFlash Sergei Shtylyov
2019-11-12 16:46 ` Tokunori Ikegami
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).