On Thu, Nov 17, 2016 at 10:14:55AM +0000, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:

quoted

On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia -
PL/Wroclaw) wrote:

quoted

quoted

quoted

quoted

On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
[off-list ref]
wrote:

quoted

From: Yunhui Cui <redacted>

With the physical sectors combination, S25FS-S family flash
requires some special operations for read/write functions.

Signed-off-by: Yunhui Cui <redacted>
---
 drivers/mtd/spi-nor/spi-nor.c | 56
+++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c

b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb
100644

--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c

@@ -39,6 +39,10 @@

 #define SPI_NOR_MAX_ID_LEN     6
 #define SPI_NOR_MAX_ADDR_WIDTH 4
+/* Added for S25FS-S family flash */
+#define SPINOR_CONFIG_REG3_OFFSET      0x800004
+#define CR3V_4KB_ERASE_UNABLE  0x8 #define
+SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81

 struct flash_info {
        char            *name;

@@ -78,6 +82,7 @@ struct flash_info {  };

 #define JEDEC_MFR(info)        ((info)->id[0])
+#define EXT_JEDEC(info)        ((info)->id[5])

 static const struct flash_info *spi_nor_match_id(const char

*name);

@@ -899,6 +904,7 @@ static const struct flash_info spi_nor_ids[] = {
         */
        { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,

64,

SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

quoted

        { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024,
128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024,
+ 512, 0)},
        { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
        { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024,
512,

SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

quoted

        { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024,
256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -1036,6

+1042,50

quoted

quoted

@@ static const struct flash_info *spi_nor_read_id(struct

spi_nor
*nor)

quoted

        return ERR_PTR(-ENODEV);  }

+/*
+ * The S25FS-S family physical sectors may be configured as
+a
+ * hybrid combination of eight 4-kB parameter sectors
+ * at the top or bottom of the address space with all
+ * but one of the remaining sectors being uniform size.
+ * The Parameter Sector Erase commands (20h or 21h) must
+ * be used to erase the 4-kB parameter sectors individually.
+ * The Sector (uniform sector) Erase commands (D8h or DCh)
+ * must be used to erase any of the remaining
+ * sectors, including the portion of highest or lowest
+address
+ * sector that is not overlaid by the parameter sectors.
+ * The uniform sector erase command has no effect on
+parameter

sectors.

quoted

+ */
+static int spansion_s25fs_disable_4kb_erase(struct spi_nor *nor) {
+       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
+       u8 cr3v = 0x0;
+       int ret = 0x0;
+
+       nor->cmd_buf[2] = cr3v_addr >> 16;
+       nor->cmd_buf[1] = cr3v_addr >> 8;
+       nor->cmd_buf[0] = cr3v_addr >> 0;
+
+       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,

&cr3v, 1);

quoted

quoted

quoted

quoted

quoted

+       if (ret)
+               return ret;
+       if (cr3v & CR3V_4KB_ERASE_UNABLE)
+               return 0;
+       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
+       if (ret)
+               return ret;
+       cr3v = CR3V_4KB_ERASE_UNABLE;
+       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
+       nor->write(nor, cr3v_addr, 1, &cr3v);
+
+       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,

&cr3v, 1);

quoted

quoted

quoted

quoted

quoted

+       if (ret)
+               return ret;
+       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
+               return -EPERM;
+
+       return 0;
+}
+
 static int spi_nor_read(struct mtd_info *mtd, loff_t from,
size_t

len,

quoted

quoted

quoted

quoted

quoted

                        size_t *retlen, u_char *buf)  { @@
-1361,6
+1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const char
+*name,

enum read_mode mode)

quoted

                spi_nor_wait_till_ready(nor);
        }

+       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC) {
+               ret = spansion_s25fs_disable_4kb_erase(nor);
+               if (ret)
+                       return ret;
+       }
+
        if (!mtd->name)
                mtd->name = dev_name(dev);
        mtd->priv = nor;
--
2.1.0.27.g96db324

Hi Brian, I will ack this change but still feel it's kind of hacking code.

Acked-by: Han xu <han.xu@nxp.com>

I am new on the list so I am not sure if this topic has been discussed.
Generally our product functionality relay on those 4KiB sectors.
I know that this hack is already in u-boot, but if you
mainstream this you will force users of those 4KiB sectors to do hack the

hack...

quoted

quoted

quoted

quoted

I believe the proper solution here is to use erase regions
functionality, I send and RFS about that some time ago.

Do you mind to send me a link for reference?

Han,

Sorry, It seem I have not posted erase region changes (only those
regarding DUAL/QUAD I/O).
Generally, in this flash you need to create 3 erase regions (because
in FS-S family support only  4KiB erase on parameters sector - eg.
1.2.2.4 in

S25FS512S).

quoted

In my case regions are:
1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD

(0xd8/0xdc) 3.

quoted

Rest of the flash SE_CMD (0xd8/0xdc)

To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7)
command, you just need to add one more mtd partition that will cover

whole flash.

quoted

Hi Krzeminski,

Do you think is there any great advantages for enable 4KB?
Because for NXP(Freescale) QSPI controller, there is only support max
to 16 groups command.

So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
So we have to disable the 4kb erase and only use 256kbytes in this patch.

Yes, if you disable parameters sector in spi-nor framework you will disable it for
all spi-nor clients not only for NXP QSPI controller. There are users (at least me)
that relay on parameters sector functionality. This patch will brake it.

Thanks,

Hi Krzeminski,

Get it.
So do you think how about that I add a flag in dts to select it?
The user want's disable 4kb, he can add the flag.

In spi-nor.c:
if (of_property_read_bool(np, "spi-nor, disable-4kb")) {
	spansion_s25fs_disable_4kb_erase();
}
                else
...

In dts:

&qspi {
        num-cs = <2>;
        bus-num = <0>;
        status = "okay";

        qflash0: s25fs512s at 0 {
                compatible = "spansion, s25fs512s";
	 spi-nor, disable-4kb
                #address-cells = <1>;
                #size-cells = <1>;
                spi-max-frequency = <20000000>;
                reg = <0>;
        };

I think it should be a better way.

How about your think?

Thanks.