Added support for spi-nor device pm in m25p80

Refactored spi_nor_scan() code to add spi_nor_init() function that programs the spi-nor flash to:
1) remove flash protection if applicable
2) set read mode to quad mode if configured such
3) set the address width based on the flash size and vendor

spi_nor_scan() now calls spi_nor_init() to setup nor flash.

Signed-off-by Kamal Dasu <[email protected]>

---
 drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   | 18 ++++++++++-
 2 files changed, 62 insertions(+), 28 deletions(-)

drivers/mtd/spi-nor/spi-nor.c +39 -14

index 70e52ff..2bf7f4f 100644
@@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
 	return 0;
 }
 
+int spi_nor_init(struct spi_nor *nor)
+{
+	int ret = 0;
+	const struct flash_info *info = nor->info;
+	struct device *dev = nor->dev;
+
+	/*
+	 * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
+	 * with the software protection bits set
+	 */
+
+	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
+	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
+	    JEDEC_MFR(info) == SNOR_MFR_SST ||
+	    info->flags & SPI_NOR_HAS_LOCK) {
+		write_enable(nor);
+		write_sr(nor, 0);
+		spi_nor_wait_till_ready(nor);
+	}
+
+	if (nor->flash_read == SPI_NOR_QUAD) {
+		ret = set_quad_mode(nor, info);
+		if (ret) {
+			dev_err(dev, "quad mode not supported\n");
+			return ret;
+		}
+	}
+
+	if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
+	    info->flags & SPI_NOR_4B_OPCODES)
+		spi_nor_set_4byte_opcodes(nor, info);
+	else
+		set_4byte(nor, info, 1);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(spi_nor_init);
+
 int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 {
 	const struct flash_info *info = NULL;

@@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 		}
 	}
 
+	nor->info = info;
 	mutex_init(&nor->lock);
 
 	/*

@@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	if (info->flags & SPI_S3AN)
 		nor->flags |=  SNOR_F_READY_XSR_RDY;
 
-	/*
-	 * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
-	 * with the software protection bits set
-	 */
-
-	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
-	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
-	    JEDEC_MFR(info) == SNOR_MFR_SST ||
-	    info->flags & SPI_NOR_HAS_LOCK) {
-		write_enable(nor);
-		write_sr(nor, 0);
-		spi_nor_wait_till_ready(nor);
-	}
-
 	if (!mtd->name)
 		mtd->name = dev_name(dev);
 	mtd->priv = nor;

drivers/mtd/spi-nor/spi-nor.c +6 -8

 	} else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
 		nor->flash_read = SPI_NOR_DUAL;

@@ -1702,17 +1722,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
 	if (info->addr_width)
 		nor->addr_width = info->addr_width;
-	else if (mtd->size > 0x1000000) {
+	else if (mtd->size > 0x1000000)
 		/* enable 4-byte addressing if the device exceeds 16MiB */
 		nor->addr_width = 4;
-		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
-		    info->flags & SPI_NOR_4B_OPCODES)
-			spi_nor_set_4byte_opcodes(nor, info);
-		else
-			set_4byte(nor, info, 1);
-	} else {
+	else
 		nor->addr_width = 3;
-	}
 
 	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
 		dev_err(dev, "address width is too large: %u\n",

@@ -1728,6 +1742,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 			return ret;
 	}
 
+	ret = spi_nor_init(nor);
+	if (ret)
+		return ret;
+
 	dev_info(dev, "%s (%lld Kbytes)\n", info->name,
 			(long long)mtd->size >> 10);
 

include/linux/mtd/spi-nor.h +17 -1

index f2a7180..29a8283 100644
@@ -143,6 +143,8 @@ enum spi_nor_option_flags {
 	SNOR_F_READY_XSR_RDY	= BIT(4),
 };
 
+struct flash_info;
+
 /**
  * struct spi_nor - Structure for defining a the SPI NOR layer
  * @mtd:		point to a mtd_info structure

@@ -174,6 +176,7 @@ enum spi_nor_option_flags {
  * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
  *			completely locked
  * @priv:		the private data
+ * @info:		points to the flash_info structure
  */
 struct spi_nor {
 	struct mtd_info		mtd;

@@ -206,6 +209,7 @@ struct spi_nor {
 	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
 
 	void *priv;
+	const struct flash_info *info;
 };
 
 static inline void spi_nor_set_flash_node(struct spi_nor *nor,

@@ -220,12 +224,24 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
 }
 
 /**
+ * spi_nor_init() - initialize SPI NOR
+ * @nor:	the spi_nor structure
+ *
+ * The drivers uses this function to initialize the SPI NOR flash device to
+ * settings in spi_nor structure. The functions sets read mode, address width
+ * and removes protection on the flash device based on those settings.
+ *
+ * Return: 0 for success, others for failure.
+ */
+int spi_nor_init(struct spi_nor *nor);
+
+/**
  * spi_nor_scan() - scan the SPI NOR
  * @nor:	the spi_nor structure
  * @name:	the chip type name
  * @mode:	the read mode supported by the driver
  *
- * The drivers can use this fuction to scan the SPI NOR.
+ * The drivers can use this function to scan the SPI NOR.
  * In the scanning, it will try to get all the necessary information to
  * fill the mtd_info{} and the spi_nor{}.
  *

Quoted 6 lines · Kamal Dasu

Refactored spi_nor_scan() code to add spi_nor_init() function that programs the spi-nor flash to:
1) remove flash protection if applicable
2) set read mode to quad mode if configured such
3) set the address width based on the flash size and vendor

Quoted 3 lines

spi_nor_scan() now calls spi_nor_init() to setup nor flash.

Signed-off-by Kamal Dasu <[email protected]>

Changelog missing ?

Quoted 33 lines

---
 drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   | 18 ++++++++++-
 2 files changed, 62 insertions(+), 28 deletions(-)

drivers/mtd/spi-nor/spi-nor.c +19 -0

index 70e52ff..2bf7f4f 100644
@@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
      return 0;
 }
 
+int spi_nor_init(struct spi_nor *nor)
+{
+     int ret = 0;
+     const struct flash_info *info = nor->info;
+     struct device *dev = nor->dev;
+
+     /*
+      * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
+      * with the software protection bits set
+      */
+
+     if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
+         JEDEC_MFR(info) == SNOR_MFR_INTEL ||
+         JEDEC_MFR(info) == SNOR_MFR_SST ||
+         info->flags & SPI_NOR_HAS_LOCK) {
+             write_enable(nor);
+             write_sr(nor, 0);
+             spi_nor_wait_till_ready(nor);
+     }

drivers/mtd/spi-nor/spi-nor.c static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor) · line 1555

Quoted 8 lines

drivers/mtd/spi-nor/spi-nor.c +8 -0

+
+     if (nor->flash_read == SPI_NOR_QUAD) {
+             ret = set_quad_mode(nor, info);
+             if (ret) {
+                     dev_err(dev, "quad mode not supported\n");
+                     return ret;
+             }
+     }

Quoted 45 lines

drivers/mtd/spi-nor/spi-nor.c +12 -14

+
+     if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
+         info->flags & SPI_NOR_4B_OPCODES)
+             spi_nor_set_4byte_opcodes(nor, info);
+     else
+             set_4byte(nor, info, 1);
+
+     return ret;
+}
+EXPORT_SYMBOL_GPL(spi_nor_init);
+
 int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 {
      const struct flash_info *info = NULL;

@@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
              }
      }
 
+     nor->info = info;
      mutex_init(&nor->lock);
 
      /*

@@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
      if (info->flags & SPI_S3AN)
              nor->flags |=  SNOR_F_READY_XSR_RDY;
 
-     /*
-      * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
-      * with the software protection bits set
-      */
-
-     if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
-         JEDEC_MFR(info) == SNOR_MFR_INTEL ||
-         JEDEC_MFR(info) == SNOR_MFR_SST ||
-         info->flags & SPI_NOR_HAS_LOCK) {
-             write_enable(nor);
-             write_sr(nor, 0);
-             spi_nor_wait_till_ready(nor);
-     }
-
      if (!mtd->name)
              mtd->name = dev_name(dev);
      mtd->priv = nor;

@@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 

drivers/mtd/spi-nor/spi-nor.c int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode) · line 1697

Quoted 8 lines

drivers/mtd/spi-nor/spi-nor.c +0 -5

      /* Quad/Dual-read mode takes precedence over fast/normal */
      if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
-             ret = set_quad_mode(nor, info);
-             if (ret) {
-                     dev_err(dev, "quad mode not supported\n");
-                     return ret;
-             }
              nor->flash_read = SPI_NOR_QUAD;

So from here on, any user will expect quad mode to be correctly set, but it really isn't. Is that OK ?

Quoted 87 lines

drivers/mtd/spi-nor/spi-nor.c +6 -8

      } else if (mode == SPI_NOR_DUAL && info->flags & SPI_NOR_DUAL_READ) {
              nor->flash_read = SPI_NOR_DUAL;

@@ -1702,17 +1722,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
      if (info->addr_width)
              nor->addr_width = info->addr_width;
-     else if (mtd->size > 0x1000000) {
+     else if (mtd->size > 0x1000000)
              /* enable 4-byte addressing if the device exceeds 16MiB */
              nor->addr_width = 4;
-             if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
-                 info->flags & SPI_NOR_4B_OPCODES)
-                     spi_nor_set_4byte_opcodes(nor, info);
-             else
-                     set_4byte(nor, info, 1);
-     } else {
+     else
              nor->addr_width = 3;
-     }
 
      if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
              dev_err(dev, "address width is too large: %u\n",

@@ -1728,6 +1742,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
                      return ret;
      }
 
+     ret = spi_nor_init(nor);
+     if (ret)
+             return ret;
+
      dev_info(dev, "%s (%lld Kbytes)\n", info->name,
                      (long long)mtd->size >> 10);
 

include/linux/mtd/spi-nor.h +17 -1

index f2a7180..29a8283 100644
@@ -143,6 +143,8 @@ enum spi_nor_option_flags {
      SNOR_F_READY_XSR_RDY    = BIT(4),
 };
 
+struct flash_info;
+
 /**
  * struct spi_nor - Structure for defining a the SPI NOR layer
  * @mtd:             point to a mtd_info structure

@@ -174,6 +176,7 @@ enum spi_nor_option_flags {
  * @flash_is_locked: [FLASH-SPECIFIC] check if a region of the SPI NOR is
  *                   completely locked
  * @priv:            the private data
+ * @info:            points to the flash_info structure
  */
 struct spi_nor {
      struct mtd_info         mtd;

@@ -206,6 +209,7 @@ struct spi_nor {
      int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
 
      void *priv;
+     const struct flash_info *info;
 };
 
 static inline void spi_nor_set_flash_node(struct spi_nor *nor,

@@ -220,12 +224,24 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
 }
 
 /**
+ * spi_nor_init() - initialize SPI NOR
+ * @nor:     the spi_nor structure
+ *
+ * The drivers uses this function to initialize the SPI NOR flash device to
+ * settings in spi_nor structure. The functions sets read mode, address width
+ * and removes protection on the flash device based on those settings.
+ *
+ * Return: 0 for success, others for failure.
+ */
+int spi_nor_init(struct spi_nor *nor);
+
+/**
  * spi_nor_scan() - scan the SPI NOR
  * @nor:     the spi_nor structure
  * @name:    the chip type name
  * @mode:    the read mode supported by the driver
  *
- * The drivers can use this fuction to scan the SPI NOR.
+ * The drivers can use this function to scan the SPI NOR.
  * In the scanning, it will try to get all the necessary information to

include/linux/mtd/spi-nor.h static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor) · line 247

Quoted 3 lines

include/linux/mtd/spi-nor.h +0 -0

  * fill the mtd_info{} and the spi_nor{}.
  *

--
Best regards,
Marek Vasut

Quoted 7 lines · Marek Vasut

Quoted 6 lines · Kamal Dasu

Refactored spi_nor_scan() code to add spi_nor_init() function that programs the spi-nor flash to:
1) remove flash protection if applicable
2) set read mode to quad mode if configured such
3) set the address width based on the flash size and vendor

Quoted 5 lines

Quoted 3 lines

spi_nor_scan() now calls spi_nor_init() to setup nor flash.

Signed-off-by Kamal Dasu <[email protected]>

Changelog missing ?

Yes will add it and resend.

Quoted 33 lines

Quoted 33 lines

---
 drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   | 18 ++++++++++-
 2 files changed, 62 insertions(+), 28 deletions(-)

drivers/mtd/spi-nor/spi-nor.c +19 -0

index 70e52ff..2bf7f4f 100644
@@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
      return 0;
 }
 
+int spi_nor_init(struct spi_nor *nor)
+{
+     int ret = 0;
+     const struct flash_info *info = nor->info;
+     struct device *dev = nor->dev;
+
+     /*
+      * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
+      * with the software protection bits set
+      */
+
+     if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
+         JEDEC_MFR(info) == SNOR_MFR_INTEL ||
+         JEDEC_MFR(info) == SNOR_MFR_SST ||
+         info->flags & SPI_NOR_HAS_LOCK) {
+             write_enable(nor);
+             write_sr(nor, 0);
+             spi_nor_wait_till_ready(nor);
+     }

Quoted 8 lines

drivers/mtd/spi-nor/spi-nor.c static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor) · line 1555

Quoted 8 lines

drivers/mtd/spi-nor/spi-nor.c +8 -0

+
+     if (nor->flash_read == SPI_NOR_QUAD) {
+             ret = set_quad_mode(nor, info);
+             if (ret) {
+                     dev_err(dev, "quad mode not supported\n");
+                     return ret;
+             }
+     }

quad mode is being set in the above block of code.

Quoted 45 lines

Quoted 45 lines

drivers/mtd/spi-nor/spi-nor.c +12 -14

+
+     if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
+         info->flags & SPI_NOR_4B_OPCODES)
+             spi_nor_set_4byte_opcodes(nor, info);
+     else
+             set_4byte(nor, info, 1);
+
+     return ret;
+}
+EXPORT_SYMBOL_GPL(spi_nor_init);
+
 int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 {
      const struct flash_info *info = NULL;

@@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
              }
      }
 
+     nor->info = info;
      mutex_init(&nor->lock);
 
      /*

@@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
      if (info->flags & SPI_S3AN)
              nor->flags |=  SNOR_F_READY_XSR_RDY;
 
-     /*
-      * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
-      * with the software protection bits set
-      */
-
-     if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
-         JEDEC_MFR(info) == SNOR_MFR_INTEL ||
-         JEDEC_MFR(info) == SNOR_MFR_SST ||
-         info->flags & SPI_NOR_HAS_LOCK) {
-             write_enable(nor);
-             write_sr(nor, 0);
-             spi_nor_wait_till_ready(nor);
-     }
-
      if (!mtd->name)
              mtd->name = dev_name(dev);
      mtd->priv = nor;

@@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 

Quoted 11 lines

drivers/mtd/spi-nor/spi-nor.c int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode) · line 1697

Quoted 8 lines

drivers/mtd/spi-nor/spi-nor.c +0 -5

      /* Quad/Dual-read mode takes precedence over fast/normal */
      if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
-             ret = set_quad_mode(nor, info);
-             if (ret) {
-                     dev_err(dev, "quad mode not supported\n");
-                     return ret;
-             }
              nor->flash_read = SPI_NOR_QUAD;

So from here on, any user will expect quad mode to be correctly set, but it really isn't. Is that OK ?

It is being set in spi_nor_init() based on the nor->flash_read == SPI_NOR_QUAD;

Quoted 117 lines · Kamal Dasu

Quoted 7 lines · Marek Vasut

Quoted 6 lines · Kamal Dasu

Refactored spi_nor_scan() code to add spi_nor_init() function that programs the spi-nor flash to:
1) remove flash protection if applicable
2) set read mode to quad mode if configured such
3) set the address width based on the flash size and vendor

Quoted 5 lines

Quoted 3 lines

spi_nor_scan() now calls spi_nor_init() to setup nor flash.

Signed-off-by Kamal Dasu <[email protected]>

Changelog missing ?

Yes will add it and resend.

Quoted 33 lines

Quoted 33 lines

---
 drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   | 18 ++++++++++-
 2 files changed, 62 insertions(+), 28 deletions(-)

drivers/mtd/spi-nor/spi-nor.c +19 -0

index 70e52ff..2bf7f4f 100644
@@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
      return 0;
 }
 
+int spi_nor_init(struct spi_nor *nor)
+{
+     int ret = 0;
+     const struct flash_info *info = nor->info;
+     struct device *dev = nor->dev;
+
+     /*
+      * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
+      * with the software protection bits set
+      */
+
+     if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
+         JEDEC_MFR(info) == SNOR_MFR_INTEL ||
+         JEDEC_MFR(info) == SNOR_MFR_SST ||
+         info->flags & SPI_NOR_HAS_LOCK) {
+             write_enable(nor);
+             write_sr(nor, 0);
+             spi_nor_wait_till_ready(nor);
+     }

Quoted 8 lines

drivers/mtd/spi-nor/spi-nor.c static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor) · line 1555

Quoted 8 lines

drivers/mtd/spi-nor/spi-nor.c +8 -0

+
+     if (nor->flash_read == SPI_NOR_QUAD) {
+             ret = set_quad_mode(nor, info);
+             if (ret) {
+                     dev_err(dev, "quad mode not supported\n");
+                     return ret;
+             }
+     }

quad mode is being set in the above block of code.

Quoted 56 lines

Quoted 44 lines

drivers/mtd/spi-nor/spi-nor.c +12 -14

+
+     if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
+         info->flags & SPI_NOR_4B_OPCODES)
+             spi_nor_set_4byte_opcodes(nor, info);
+     else
+             set_4byte(nor, info, 1);
+
+     return ret;
+}
+EXPORT_SYMBOL_GPL(spi_nor_init);
+
 int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 {
      const struct flash_info *info = NULL;

@@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
              }
      }
 
+     nor->info = info;
      mutex_init(&nor->lock);
 
      /*

@@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
      if (info->flags & SPI_S3AN)
              nor->flags |=  SNOR_F_READY_XSR_RDY;
 
-     /*
-      * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
-      * with the software protection bits set
-      */
-
-     if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
-         JEDEC_MFR(info) == SNOR_MFR_INTEL ||
-         JEDEC_MFR(info) == SNOR_MFR_SST ||
-         info->flags & SPI_NOR_HAS_LOCK) {
-             write_enable(nor);
-             write_sr(nor, 0);
-             spi_nor_wait_till_ready(nor);
-     }
-
      if (!mtd->name)
              mtd->name = dev_name(dev);
      mtd->priv = nor;

drivers/mtd/spi-nor/spi-nor.c int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode) · line 1697

Quoted 9 lines

drivers/mtd/spi-nor/spi-nor.c +0 -5

@@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
      /* Quad/Dual-read mode takes precedence over fast/normal */
      if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
-             ret = set_quad_mode(nor, info);
-             if (ret) {
-                     dev_err(dev, "quad mode not supported\n");
-                     return ret;
-             }
              nor->flash_read = SPI_NOR_QUAD;

So from here on, any user will expect quad mode to be correctly set, but it really isn't. Is that OK ?

Quoted 2 lines

It is being set in spi_nor_init() based on the nor->flash_read == SPI_NOR_QUAD;

But that's invoked later, so whoever adds code between this place and the invocation of spi_nor_init() will be misled into believing the SPI NOR is set up in quad-mode, while it would not be, right ?

[...]

--
Best regards,
Marek Vasut

Marek,

Quoted 118 lines · Marek Vasut

Quoted 117 lines · Kamal Dasu

Quoted 7 lines · Marek Vasut

Quoted 6 lines · Kamal Dasu

Refactored spi_nor_scan() code to add spi_nor_init() function that programs the spi-nor flash to:
1) remove flash protection if applicable
2) set read mode to quad mode if configured such
3) set the address width based on the flash size and vendor

Quoted 5 lines

Quoted 3 lines

spi_nor_scan() now calls spi_nor_init() to setup nor flash.

Signed-off-by Kamal Dasu <[email protected]>

Changelog missing ?

Yes will add it and resend.

Quoted 33 lines

Quoted 33 lines

---
 drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   | 18 ++++++++++-
 2 files changed, 62 insertions(+), 28 deletions(-)

drivers/mtd/spi-nor/spi-nor.c +19 -0

index 70e52ff..2bf7f4f 100644
@@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
      return 0;
 }
 
+int spi_nor_init(struct spi_nor *nor)
+{
+     int ret = 0;
+     const struct flash_info *info = nor->info;
+     struct device *dev = nor->dev;
+
+     /*
+      * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
+      * with the software protection bits set
+      */
+
+     if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
+         JEDEC_MFR(info) == SNOR_MFR_INTEL ||
+         JEDEC_MFR(info) == SNOR_MFR_SST ||
+         info->flags & SPI_NOR_HAS_LOCK) {
+             write_enable(nor);
+             write_sr(nor, 0);
+             spi_nor_wait_till_ready(nor);
+     }

Quoted 8 lines

drivers/mtd/spi-nor/spi-nor.c static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor) · line 1555

Quoted 8 lines

drivers/mtd/spi-nor/spi-nor.c +8 -0

+
+     if (nor->flash_read == SPI_NOR_QUAD) {
+             ret = set_quad_mode(nor, info);
+             if (ret) {
+                     dev_err(dev, "quad mode not supported\n");
+                     return ret;
+             }
+     }

quad mode is being set in the above block of code.

Quoted 56 lines

Quoted 44 lines

drivers/mtd/spi-nor/spi-nor.c +12 -14

+
+     if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
+         info->flags & SPI_NOR_4B_OPCODES)
+             spi_nor_set_4byte_opcodes(nor, info);
+     else
+             set_4byte(nor, info, 1);
+
+     return ret;
+}
+EXPORT_SYMBOL_GPL(spi_nor_init);
+
 int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 {
      const struct flash_info *info = NULL;

@@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
              }
      }
 
+     nor->info = info;
      mutex_init(&nor->lock);
 
      /*

@@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
      if (info->flags & SPI_S3AN)
              nor->flags |=  SNOR_F_READY_XSR_RDY;
 
-     /*
-      * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
-      * with the software protection bits set
-      */
-
-     if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
-         JEDEC_MFR(info) == SNOR_MFR_INTEL ||
-         JEDEC_MFR(info) == SNOR_MFR_SST ||
-         info->flags & SPI_NOR_HAS_LOCK) {
-             write_enable(nor);
-             write_sr(nor, 0);
-             spi_nor_wait_till_ready(nor);
-     }
-
      if (!mtd->name)
              mtd->name = dev_name(dev);
      mtd->priv = nor;

drivers/mtd/spi-nor/spi-nor.c int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode) · line 1697

Quoted 9 lines

drivers/mtd/spi-nor/spi-nor.c +0 -5

@@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
      /* Quad/Dual-read mode takes precedence over fast/normal */
      if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
-             ret = set_quad_mode(nor, info);
-             if (ret) {
-                     dev_err(dev, "quad mode not supported\n");
-                     return ret;
-             }
              nor->flash_read = SPI_NOR_QUAD;

So from here on, any user will expect quad mode to be correctly set, but it really isn't. Is that OK ?

Quoted 7 lines

Quoted 2 lines

It is being set in spi_nor_init() based on the nor->flash_read == SPI_NOR_QUAD;

But that's invoked later, so whoever adds code between this place and the invocation of spi_nor_init() will be misled into believing the SPI NOR is set up in quad-mode, while it would not be, right ?

I made this change based on previous comments from Cyrlle. All spi-nor based settings that need commands to the flash happens in one function now. The code before calling spi_nor_init is setting the configuration in the nor structure. I don't see how its very different from before. I did separate each setting in functions without changing the flow in spi_nor_scan() before and was to told to change it this way to be more optimal. I don't see how anyone will have any confusion. So please let me know how exactly you want it.

Refactored spi_nor_scan() code to add spi_nor_init() function that programs the spi-nor flash to:
1) remove flash protection if applicable
2) set read mode to quad mode if configured such
3) set the address width based on the flash size and vendor

spi_nor_scan() now calls spi_nor_init() to setup nor flash.

Signed-off-by Kamal Dasu <[email protected]>

Quoted 6 lines · Kamal Dasu

Refactored spi_nor_scan() code to add spi_nor_init() function that programs the spi-nor flash to:
1) remove flash protection if applicable
2) set read mode to quad mode if configured such
3) set the address width based on the flash size and vendor

Quoted 3 lines

spi_nor_scan() now calls spi_nor_init() to setup nor flash.

Signed-off-by Kamal Dasu <[email protected]>

Ideally a short reason info should go there.
"This allows reusing spi_nor_init during resume"
or whatever :)

Added power management ops for resume to be able to resan spi-nor

device and set it to right transfer modes in its probed state after poweron. Some SoC implementations might power down the spi-nor flash and loose its initial settings on suspend. A resume should retore the

part to its probed state.

Signed-off-by Kamal Dasu <[email protected]>

---
 drivers/mtd/devices/m25p80.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/mtd/devices/m25p80.c +11 -0

index c4df3b1..3ab30b2 100644
@@ -324,10 +324,21 @@ static int m25p_remove(struct spi_device *spi)
 };
 MODULE_DEVICE_TABLE(of, m25p_of_table);
 
+#ifdef CONFIG_PM_SLEEP
+static int m25p_resume(struct device *dev)
+{
+	struct m25p *flash = dev_get_drvdata(dev);
+
+	return spi_nor_init(&flash->spi_nor);
+}
+#endif
+static SIMPLE_DEV_PM_OPS(m25p_pm_ops, NULL, m25p_resume);
+
 static struct spi_driver m25p80_driver = {
 	.driver = {
 		.name	= "m25p80",
 		.of_match_table = m25p_of_table,
+		.pm     = &m25p_pm_ops,
 	},
 	.id_table	= m25p_ids,
 	.probe	= m25p_probe,

Quoted 1 line · Kamal Dasu

Added power management ops for resume to be able to resan spi-nor

rescan ... but you're not really rescanning it, are you ? You're just reconfiguring the pre-suspend parameters .

Quoted 3 lines

device and set it to right transfer modes in its probed state after poweron. Some SoC implementations might power down the spi-nor flash and loose its initial settings on suspend. A resume should retore the

restore ...

Quoted 2 lines

part to its probed state.

Quoted 29 lines

Signed-off-by Kamal Dasu <[email protected]>

---
 drivers/mtd/devices/m25p80.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/mtd/devices/m25p80.c +11 -0

index c4df3b1..3ab30b2 100644
@@ -324,10 +324,21 @@ static int m25p_remove(struct spi_device *spi)
 };
 MODULE_DEVICE_TABLE(of, m25p_of_table);
 
+#ifdef CONFIG_PM_SLEEP
+static int m25p_resume(struct device *dev)
+{
+     struct m25p *flash = dev_get_drvdata(dev);
+
+     return spi_nor_init(&flash->spi_nor);
+}
+#endif
+static SIMPLE_DEV_PM_OPS(m25p_pm_ops, NULL, m25p_resume);
+
 static struct spi_driver m25p80_driver = {
      .driver = {
              .name   = "m25p80",
              .of_match_table = m25p_of_table,
+             .pm     = &m25p_pm_ops,
      },

drivers/mtd/devices/m25p80.c static int m25p_remove(struct spi_device *spi) · line 344

Quoted 3 lines

drivers/mtd/devices/m25p80.c +0 -0

      .id_table       = m25p_ids,
      .probe  = m25p_probe,

--
Best regards,
Marek Vasut