Re: [PATCH 2/2] mtd: spi-nor: Disable the flash quad mode in spi_nor_restore()

[Yicong Yang <yangyicong@hisilicon.com>]

Hi Mathhias and Pratyush,

I've tested the following patch with s25fs128s1.
I left the flash quad enabled before managed by spi-nor driver,
and it'll stay QE after removed. So I think it'll also address the issue
mentioned. Please have a test.

Regards,
Yicong


From 43aa1afa12a10036f722b272a9a39b8c83218f33 Mon Sep 17 00:00:00 2001
From: Yicong Yang <yangyicong@hisilicon.com>
Date: Tue, 1 Sep 2020 22:09:46 +0800
Subject: [PATCH] mtd: spi-nor: don't disable quad mode of flash whose is
 originally enabled

Currently we'll disable the flash's Quad mode when remove/shutdown in
spi_nor_restore(), no matter whether it's Quad enabled or not.
For flashes originally in Quad mode, it'll clear the flash's QE bit
and restore an incorrect state.

Record the flash's original QE state, and don't disable the Quad mode
of these originally Quad enabled flash.

Reported-by: Matthias Weisser <m.weisser.m@gmail.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/mtd/spi-nor/core.c  | 30 +++++++++++++++++++++++-------
 include/linux/mtd/spi-nor.h |  3 +++
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 65eff4c..b13b3b3 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1924,8 +1924,11 @@ int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor, bool enable)
     if (ret)
         return ret;
 
-    if ((enable && (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)) ||
-        (!enable && !(nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)))
+    if (enable && (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)) {
+        nor->orig_qe_state = true;
+        return 0;
+    }
+    if (!enable && !(nor->bouncebuf[0] & SR1_QUAD_EN_BIT6))
         return 0;
 
     if (enable)
@@ -1958,8 +1961,12 @@ int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor, bool enable)
     if (ret)
         return ret;
 
-    if ((enable && (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)) ||
-        (!enable && !(nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)))
+    if (enable && (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)) {
+        nor->orig_qe_state = true;
+        return 0;
+    }
+
+    if (!enable && !(nor->bouncebuf[0] & SR2_QUAD_EN_BIT1))
         return 0;
 
     if (enable)
@@ -1993,8 +2000,12 @@ int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor, bool enable)
     ret = spi_nor_read_sr2(nor, sr2);
     if (ret)
         return ret;
-    if ((enable && (*sr2 & SR2_QUAD_EN_BIT7)) ||
-        (!enable && !(*sr2 & SR2_QUAD_EN_BIT7)))
+    if (enable && (*sr2 & SR2_QUAD_EN_BIT7)) {
+        nor->orig_qe_state = true;
+        return 0;
+    }
+
+    if (!enable && !(*sr2 & SR2_QUAD_EN_BIT7))
         return 0;
 
     /* Update the Quad Enable bit. */
@@ -3001,7 +3012,12 @@ void spi_nor_restore(struct spi_nor *nor)
         nor->flags & SNOR_F_BROKEN_RESET)
         nor->params->set_4byte_addr_mode(nor, false);
 
-    spi_nor_quad_enable(nor, false);
+    /*
+     * restore the flash's quad mode. if the flash's quad mode is
+     * enabled originally, we'll not disable it.
+     */
+    if (!nor->orig_qe_state)
+        spi_nor_quad_enable(nor, false);
 }
 EXPORT_SYMBOL_GPL(spi_nor_restore);
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 60bac2c..f343416 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -344,6 +344,8 @@ struct spi_nor_flash_parameter;
  * @read_dummy:        the dummy needed by the read operation
  * @program_opcode:    the program opcode
  * @sst_write_second:    used by the SST write operation
+ * @orig_qe_state:    used to indicate the flash's original Quad mode enable
+ *                      state. True for enabled and false for disabled.
  * @flags:        flag options for the current SPI NOR (SNOR_F_*)
  * @read_proto:        the SPI protocol for read operations
  * @write_proto:    the SPI protocol for write operations
@@ -375,6 +377,7 @@ struct spi_nor {
     enum spi_nor_protocol    write_proto;
     enum spi_nor_protocol    reg_proto;
     bool            sst_write_second;
+    bool            orig_qe_state;
     u32            flags;
 
     const struct spi_nor_controller_ops *controller_ops;
-- 
2.8.1



On 2020/9/1 14:16, Matthias Weißer wrote:
> Am Di., 16. Juni 2020 um 15:03 Uhr schrieb Yicong Yang
> <yangyicong@hisilicon.com>:
>> If the flash's quad mode is enabled, it'll remain in the quad mode when
>> it's removed. If we drive the flash next time in SPI/Dual mode, then
>> problem occurs as the flash's quad enable bit is not cleared.
> On flash devices with a non-volatile quad enable bit (we use S25FL512S)
> this will wear out the quad enable bit as on every boot the bit is
> set and reset on shutdown. Or do I miss something here?
>
>> Disable the quad mode in spi_nor_restore(), the flash will leave
>> quad mode when remove. This will make sure the flash always enter the
>> correct mode when loaded.
> We have a system which relies on an enabled quad mode on boot
> (bootloader uses quad mode without enabling it) so using this patch
> will prevent our device from booting.
>
> Regards,
> Matthias
> .
>


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/