[PATCH v4 0/2] Added support for spi-nor device pm in m25p80

[PATCH v4 0/2] Added support for spi-nor device pm in m25p80

[Kamal Dasu]

Changes since v3 based on review comments:
spi-nor.h
- Moved flash_info field to end of structure
- Added flash_info entry to the spi_nor structure description
- Reworded the function description
- Reworded the commit message

m25p80.c:
- Only implement and initialize the resume() pm_op

The V4 changes below implements power management support in m25p80 driver.
m25p80 pm resume() calls newly a implemented spi_nor_init() function
that sets up the spi-nor flash to its probed state. This is needed on
platfroms that turn off power to the spi-nor flash on pm suspend.
The code has been rebased to git://github.com/spi-nor/linux.git next
branch.


Kamal Dasu (2):
  mtd: spi-nor: Added spi-nor init function
  mtd: m25p80: Added pm ops support

 drivers/mtd/devices/m25p80.c  | 11 +++++++
 drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   | 18 ++++++++++-
 3 files changed, 73 insertions(+), 28 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 0/2] Added support for spi-nor device pm in m25p80

[Kamal Dasu]

Changes since v3 based on review comments:
spi-nor.h
- Moved flash_info field to end of structure
- Added flash_info entry to the spi_nor structure description
- Reworded the function description
- Reworded the commit message

m25p80.c:
- Only implement and initialize the resume() pm_op

The V4 changes below implements power management support in m25p80 driver.
m25p80 pm resume() calls newly a implemented spi_nor_init() function
that sets up the spi-nor flash to its probed state. This is needed on
platfroms that turn off power to the spi-nor flash on pm suspend.
The code has been rebased to git://github.com/spi-nor/linux.git next
branch.


Kamal Dasu (2):
  mtd: spi-nor: Added spi-nor init function
  mtd: m25p80: Added pm ops support

 drivers/mtd/devices/m25p80.c  | 11 +++++++
 drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   | 18 ++++++++++-
 3 files changed, 73 insertions(+), 28 deletions(-)

-- 
1.9.1

[PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[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

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

Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   | 18 ++++++++++-
 2 files changed, 62 insertions(+), 28 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 70e52ff..2bf7f4f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -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;
@@ -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;
 	} 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);
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index f2a7180..29a8283 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -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{}.
  *
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[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

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

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   | 18 ++++++++++-
 2 files changed, 62 insertions(+), 28 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 70e52ff..2bf7f4f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -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;
@@ -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;
 	} 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);
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index f2a7180..29a8283 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -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{}.
  *
-- 
1.9.1

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Marek Vasut]

On 02/14/2017 04:32 PM, Kamal Dasu wrote:
> 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 <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Changelog missing ?

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>  2 files changed, 62 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 70e52ff..2bf7f4f 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -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;
> @@ -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 ?

>  	} 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);
>  
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index f2a7180..29a8283 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -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{}.
>   *
> 


-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Marek Vasut]

On 02/14/2017 04:32 PM, Kamal Dasu wrote:
> 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 <kdasu.kdev@gmail.com>

Changelog missing ?

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>  2 files changed, 62 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 70e52ff..2bf7f4f 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -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;
> @@ -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 ?

>  	} 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);
>  
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index f2a7180..29a8283 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -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{}.
>   *
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Kamal Dasu]

On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>> 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 <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Changelog missing ?

Yes will add it and resend.

>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>>  2 files changed, 62 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 70e52ff..2bf7f4f 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -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;
>> +             }
>> +     }

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

>> +
>> +     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)
>>
>>       /* 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;

>
>>       } 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);
>>
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index f2a7180..29a8283 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -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{}.
>>   *
>>
>
>
> --
> Best regards,
> Marek Vasut

Thanks
Kamal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Kamal Dasu]

On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex@denx.de> wrote:
> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>> 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 <kdasu.kdev@gmail.com>
>
> Changelog missing ?

Yes will add it and resend.

>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>>  2 files changed, 62 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 70e52ff..2bf7f4f 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -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;
>> +             }
>> +     }

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

>> +
>> +     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)
>>
>>       /* 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;

>
>>       } 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);
>>
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index f2a7180..29a8283 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -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{}.
>>   *
>>
>
>
> --
> Best regards,
> Marek Vasut

Thanks
Kamal

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Marek Vasut]

On 02/19/2017 10:47 AM, Kamal Dasu wrote:
> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>> 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 <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> Changelog missing ?
> 
> Yes will add it and resend.
> 
>>
>>> ---
>>>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>>>  2 files changed, 62 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 70e52ff..2bf7f4f 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -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;
>>> +             }
>>> +     }
> 
> quad mode is being set in the above block of code.
> 
>>> +
>>> +     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)
>>>
>>>       /* 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;
> 

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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Marek Vasut]

On 02/19/2017 10:47 AM, Kamal Dasu wrote:
> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex@denx.de> wrote:
>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>> 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 <kdasu.kdev@gmail.com>
>>
>> Changelog missing ?
> 
> Yes will add it and resend.
> 
>>
>>> ---
>>>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>>>  2 files changed, 62 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 70e52ff..2bf7f4f 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -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;
>>> +             }
>>> +     }
> 
> quad mode is being set in the above block of code.
> 
>>> +
>>> +     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)
>>>
>>>       /* 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;
> 

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

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Kamal Dasu]

Marek,

On Sun, Feb 19, 2017 at 5:37 AM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
> On 02/19/2017 10:47 AM, Kamal Dasu wrote:
>> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
>>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>>> 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 <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>
>>> Changelog missing ?
>>
>> Yes will add it and resend.
>>
>>>
>>>> ---
>>>>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>>>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>>>>  2 files changed, 62 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>> index 70e52ff..2bf7f4f 100644
>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>> @@ -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;
>>>> +             }
>>>> +     }
>>
>> quad mode is being set in the above block of code.
>>
>>>> +
>>>> +     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)
>>>>
>>>>       /* 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;
>>
>
> 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.


> [...]
>
> --
> Best regards,
> Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Kamal Dasu]

Marek,

On Sun, Feb 19, 2017 at 5:37 AM, Marek Vasut <marex@denx.de> wrote:
> On 02/19/2017 10:47 AM, Kamal Dasu wrote:
>> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex@denx.de> wrote:
>>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>>> 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 <kdasu.kdev@gmail.com>
>>>
>>> Changelog missing ?
>>
>> Yes will add it and resend.
>>
>>>
>>>> ---
>>>>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>>>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>>>>  2 files changed, 62 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>> index 70e52ff..2bf7f4f 100644
>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>> @@ -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;
>>>> +             }
>>>> +     }
>>
>> quad mode is being set in the above block of code.
>>
>>>> +
>>>> +     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)
>>>>
>>>>       /* 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;
>>
>
> 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.


> [...]
>
> --
> Best regards,
> Marek Vasut

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Marek Vasut]

On 02/19/2017 11:38 PM, Kamal Dasu wrote:
> Marek,
> 
> On Sun, Feb 19, 2017 at 5:37 AM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
>> On 02/19/2017 10:47 AM, Kamal Dasu wrote:
>>> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
>>>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>>>> 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 <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>
>>>> Changelog missing ?
>>>
>>> Yes will add it and resend.
>>>
>>>>
>>>>> ---
>>>>>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>>>>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>>>>>  2 files changed, 62 insertions(+), 28 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>>> index 70e52ff..2bf7f4f 100644
>>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>>> @@ -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;
>>>>> +             }
>>>>> +     }
>>>
>>> quad mode is being set in the above block of code.
>>>
>>>>> +
>>>>> +     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)
>>>>>
>>>>>       /* 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;
>>>
>>
>> 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.

Hmmmm, I wonder if nor->flash_read = SPI_NOR_QUAD; shouldn't be set in
set_quad_mode() ?

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Marek Vasut]

On 02/19/2017 11:38 PM, Kamal Dasu wrote:
> Marek,
> 
> On Sun, Feb 19, 2017 at 5:37 AM, Marek Vasut <marex@denx.de> wrote:
>> On 02/19/2017 10:47 AM, Kamal Dasu wrote:
>>> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex@denx.de> wrote:
>>>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>>>> 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 <kdasu.kdev@gmail.com>
>>>>
>>>> Changelog missing ?
>>>
>>> Yes will add it and resend.
>>>
>>>>
>>>>> ---
>>>>>  drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>>>>  include/linux/mtd/spi-nor.h   | 18 ++++++++++-
>>>>>  2 files changed, 62 insertions(+), 28 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>>> index 70e52ff..2bf7f4f 100644
>>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>>> @@ -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;
>>>>> +             }
>>>>> +     }
>>>
>>> quad mode is being set in the above block of code.
>>>
>>>>> +
>>>>> +     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)
>>>>>
>>>>>       /* 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;
>>>
>>
>> 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.

Hmmmm, I wonder if nor->flash_read = SPI_NOR_QUAD; shouldn't be set in
set_quad_mode() ?

-- 
Best regards,
Marek Vasut

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Rafał Miłecki]

On 14 February 2017 at 16:32, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> 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 <kdasu.kdev@gmail.com>

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

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

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Rafał Miłecki]

On 14 February 2017 at 16:32, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> 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 <kdasu.kdev@gmail.com>

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

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Marek Vasut]

On 02/20/2017 09:12 AM, Rafał Miłecki wrote:
> On 14 February 2017 at 16:32, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>> 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 <kdasu.kdev@gmail.com>
> 
> Ideally a short reason info should go there.
> "This allows reusing spi_nor_init during resume"
> or whatever :)
> 
It reinits the SPI NOR (if I understand it correctly). Being able to use
it on some boards after resume is a side-effect, so it should be listed
as such.

-- 
Best regards,
Marek Vasut

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

Re: [PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function

[Marek Vasut]

On 02/20/2017 09:12 AM, Rafał Miłecki wrote:
> On 14 February 2017 at 16:32, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>> 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 <kdasu.kdev@gmail.com>
> 
> Ideally a short reason info should go there.
> "This allows reusing spi_nor_init during resume"
> or whatever :)
> 
It reinits the SPI NOR (if I understand it correctly). Being able to use
it on some boards after resume is a side-effect, so it should be listed
as such.

-- 
Best regards,
Marek Vasut

[PATCH v4 2/2] mtd: m25p80: Added pm ops support

[Kamal Dasu]

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 <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/mtd/devices/m25p80.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index c4df3b1..3ab30b2 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -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,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 2/2] mtd: m25p80: Added pm ops support

[Kamal Dasu]

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 <kdasu.kdev@gmail.com>
---
 drivers/mtd/devices/m25p80.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index c4df3b1..3ab30b2 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -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,
-- 
1.9.1

Re: [PATCH v4 2/2] mtd: m25p80: Added pm ops support

[Marek Vasut]

On 02/14/2017 04:32 PM, Kamal Dasu wrote:
> 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 .

> 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 ...

> part to its probed state.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/mtd/devices/m25p80.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index c4df3b1..3ab30b2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -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,
> 


-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/2] mtd: m25p80: Added pm ops support

[Marek Vasut]

On 02/14/2017 04:32 PM, Kamal Dasu wrote:
> 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 .

> 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 ...

> part to its probed state.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  drivers/mtd/devices/m25p80.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index c4df3b1..3ab30b2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -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,
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v4 2/2] mtd: m25p80: Added pm ops support

[Kamal Dasu]

On Sat, Feb 18, 2017 at 5:31 PM, Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> wrote:
> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>> 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 .
>
>> 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 ...
>
>> part to its probed state.
>>

Will fix typos.

>> Signed-off-by: Kamal Dasu <kdasu.kdev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/mtd/devices/m25p80.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index c4df3b1..3ab30b2 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -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,
>>
>
>
> --
> Best regards,
> Marek Vasut

Thanks
Kamal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/2] mtd: m25p80: Added pm ops support

[Kamal Dasu]

On Sat, Feb 18, 2017 at 5:31 PM, Marek Vasut <marex@denx.de> wrote:
> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>> 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 .
>
>> 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 ...
>
>> part to its probed state.
>>

Will fix typos.

>> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
>> ---
>>  drivers/mtd/devices/m25p80.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index c4df3b1..3ab30b2 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -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,
>>
>
>
> --
> Best regards,
> Marek Vasut

Thanks
Kamal