[PATCH v2 1/2] mtd: spi-nor: Add SPI NOR layer PM support

[PATCH v2 1/2] mtd: spi-nor: Add SPI NOR layer PM support

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@freescale.com>

Add the Power Management API in SPI NOR framework.
The Power Management system will turn off power supply to SPI flash
when system suspending, and then the SPI flash will be in the reset
state after system resuming. As a result, the status&configurations
of SPI flash driver will mismatch with its current hardware state.
So reinitialize SPI flash to make sure it is resumed to the correct
state.
And the SPI NOR layer just do common configuration depending on the
records in structure spi_nor.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@freescale.com>
---
V2:
 - New patch

 drivers/mtd/spi-nor/spi-nor.c | 44 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spi-nor.h   |  9 +++++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 4988390..4030d37c 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1365,6 +1365,50 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 }
 EXPORT_SYMBOL_GPL(spi_nor_scan);
 
+#ifdef CONFIG_PM_SLEEP
+static int spi_nor_hw_reinit(struct spi_nor *nor)
+{
+	const struct flash_info *info = NULL;
+	struct device *dev = nor->dev;
+	int ret;
+
+	info = spi_nor_read_id(nor);
+	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
+	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
+	    JEDEC_MFR(info) == SNOR_MFR_SST ||
+	    JEDEC_MFR(info) == SNOR_MFR_WINBOND) {
+		write_enable(nor);
+		write_sr(nor, 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;
+		}
+	}
+
+	if (nor->addr_width == 4 && !info->addr_width &&
+			JEDEC_MFR(info) != SNOR_MFR_SPANSION)
+		set_4byte(nor, info, 1);
+
+	return 0;
+}
+
+int spi_nor_suspend(struct spi_nor *nor)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_nor_suspend);
+
+int spi_nor_resume(struct spi_nor *nor)
+{
+	return spi_nor_hw_reinit(nor);
+}
+EXPORT_SYMBOL_GPL(spi_nor_resume);
+#endif /* CONFIG_PM_SLEEP */
+
 static const struct flash_info *spi_nor_match_id(const char *name)
 {
 	const struct flash_info *id = spi_nor_ids;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index c8723b6..d3a7888 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -201,4 +201,13 @@ struct spi_nor {
  */
 int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode);
 
+/**
+ * spi_nor_suspend/resume() - the SPI NOR layer PM API
+ * @nor:	the spi_nor structure
+ *
+ * Return: 0 for success, others for failure.
+ */
+int spi_nor_suspend(struct spi_nor *nor);
+int spi_nor_resume(struct spi_nor *nor);
+
 #endif
-- 
2.1.0.27.g96db324

[PATCH v2 2/2] mtd: m25p80: Add Power Management support

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@freescale.com>

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@freescale.com>
---
V2:
 - Call spi_nor_suspend/resume instead of spi_nor_scan.

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

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index fe9ceb7..9f07f94 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -243,6 +243,26 @@ static int m25p_remove(struct spi_device *spi)
 	return mtd_device_unregister(&flash->spi_nor.mtd);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int m25p_suspend(struct device *dev)
+{
+	struct m25p *flash = dev_get_drvdata(dev);
+	struct spi_nor *nor = &flash->spi_nor;
+
+	return spi_nor_suspend(nor);
+}
+
+static int m25p_resume(struct device *dev)
+{
+	struct m25p *flash = dev_get_drvdata(dev);
+	struct spi_nor *nor = &flash->spi_nor;
+
+	return spi_nor_resume(nor);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(m25p_pm_ops, m25p_suspend, m25p_resume);
+
 /*
  * Do NOT add to this array without reading the following:
  *
@@ -302,6 +322,7 @@ 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,
-- 
2.1.0.27.g96db324

Re: [PATCH v2 1/2] mtd: spi-nor: Add SPI NOR layer PM support

[Brian Norris]

On Fri, Dec 25, 2015 at 04:55:06PM +0800, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@freescale.com>
> 
> Add the Power Management API in SPI NOR framework.
> The Power Management system will turn off power supply to SPI flash
> when system suspending, and then the SPI flash will be in the reset
> state after system resuming. As a result, the status&configurations
> of SPI flash driver will mismatch with its current hardware state.
> So reinitialize SPI flash to make sure it is resumed to the correct
> state.
> And the SPI NOR layer just do common configuration depending on the
> records in structure spi_nor.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@freescale.com>
> ---
> V2:
>  - New patch
> 
>  drivers/mtd/spi-nor/spi-nor.c | 44 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spi-nor.h   |  9 +++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 4988390..4030d37c 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1365,6 +1365,50 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  }
>  EXPORT_SYMBOL_GPL(spi_nor_scan);
>  
> +#ifdef CONFIG_PM_SLEEP
> +static int spi_nor_hw_reinit(struct spi_nor *nor)
> +{
> +	const struct flash_info *info = NULL;
> +	struct device *dev = nor->dev;
> +	int ret;
> +
> +	info = spi_nor_read_id(nor);
> +	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
> +	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
> +	    JEDEC_MFR(info) == SNOR_MFR_SST ||
> +	    JEDEC_MFR(info) == SNOR_MFR_WINBOND) {
> +		write_enable(nor);
> +		write_sr(nor, 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;
> +		}
> +	}
> +
> +	if (nor->addr_width == 4 && !info->addr_width &&
> +			JEDEC_MFR(info) != SNOR_MFR_SPANSION)
> +		set_4byte(nor, info, 1);

You're duplicating a bunch of code from spi_nor_scan(). Please don't do
that. Find a good way to share it.

> +	return 0;
> +}
> +
> +int spi_nor_suspend(struct spi_nor *nor)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(spi_nor_suspend);
> +
> +int spi_nor_resume(struct spi_nor *nor)
> +{
> +	return spi_nor_hw_reinit(nor);
> +}
> +EXPORT_SYMBOL_GPL(spi_nor_resume);
> +#endif /* CONFIG_PM_SLEEP */

What happens if a driver wants this with !CONFIG_PM_SLEEP? I think it's
best if you just unconditionally add spi_nor_{suspend,resume}().

Brian

> +
>  static const struct flash_info *spi_nor_match_id(const char *name)
>  {
>  	const struct flash_info *id = spi_nor_ids;
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index c8723b6..d3a7888 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -201,4 +201,13 @@ struct spi_nor {
>   */
>  int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode);
>  
> +/**
> + * spi_nor_suspend/resume() - the SPI NOR layer PM API
> + * @nor:	the spi_nor structure
> + *
> + * Return: 0 for success, others for failure.
> + */
> +int spi_nor_suspend(struct spi_nor *nor);
> +int spi_nor_resume(struct spi_nor *nor);
> +
>  #endif
> -- 
> 2.1.0.27.g96db324
> 

Re: [PATCH v2 1/2] mtd: spi-nor: Add SPI NOR layer PM support

[Hou Zhiqiang]

Hi Brian,

Thanks for your comments!

>On Fri, Dec 25, 2015 at 04:55:06PM +0800, Zhiqiang Hou wrote:
>> From: Hou Zhiqiang <Zhiqiang.Hou@freescale.com>
>> 
>> Add the Power Management API in SPI NOR framework.
>> The Power Management system will turn off power supply to SPI flash
>> when system suspending, and then the SPI flash will be in the reset
>> state after system resuming. As a result, the status&configurations
>> of SPI flash driver will mismatch with its current hardware state.
>> So reinitialize SPI flash to make sure it is resumed to the correct
>> state.
>> And the SPI NOR layer just do common configuration depending on the 
>> records in structure spi_nor.
>> 
>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@freescale.com>
>> --- 
>> V2: 
>>  - New patch
>> 
>>  drivers/mtd/spi-nor/spi-nor.c | 44 +++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/mtd/spi-nor.h   |  9 +++++++++
>>  2 files changed, 53 insertions(+)
>> 
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 4988390..4030d37c 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -1365,6 +1365,50 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>  }
>>  EXPORT_SYMBOL_GPL(spi_nor_scan);
>>  
>> +#ifdef CONFIG_PM_SLEEP
>> +static int spi_nor_hw_reinit(struct spi_nor *nor)
>> +{
>> +     const struct flash_info *info = NULL;
>> +     struct device *dev = nor->dev;
>> +     int ret;
>> +
>> +     info = spi_nor_read_id(nor);
>> +     if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>> +         JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>> +         JEDEC_MFR(info) == SNOR_MFR_SST ||
>> +         JEDEC_MFR(info) == SNOR_MFR_WINBOND) {
>> +             write_enable(nor);
>> +             write_sr(nor, 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;
>> +             }
>> +     }
>> +
>> +     if (nor->addr_width == 4 && !info->addr_width &&
>> +                     JEDEC_MFR(info) != SNOR_MFR_SPANSION)
>> +             set_4byte(nor, info, 1);
>
>You're duplicating a bunch of code from spi_nor_scan(). Please don't do
>that. Find a good way to share it.
>

This will be arranged in next version.

>> +     return 0;
>> +}
>> +
>> +int spi_nor_suspend(struct spi_nor *nor)
>> +{
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(spi_nor_suspend);
>> +
>> +int spi_nor_resume(struct spi_nor *nor)
>> +{
>> +     return spi_nor_hw_reinit(nor);
>> +}
>> +EXPORT_SYMBOL_GPL(spi_nor_resume);
>> +#endif /* CONFIG_PM_SLEEP */
>
>What happens if a driver wants this with !CONFIG_PM_SLEEP? I think it's
>best if you just unconditionally add spi_nor_{suspend,resume}().
>

Ok, will remove the condition.

Thanks,
Zhiqiang