| Message ID | 1451033707-44170-1-git-send-email-Zhiqiang.Hou@freescale.com |
|---|---|
| State | Changes Requested |
| Headers | show |
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 >
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
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