| Message ID | 1386749970-22021-1-git-send-email-b48286@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Dec 11, 2013 at 04:19:30PM +0800, Hou Zhiqiang wrote: > Add PM support using callback function suspend and resume in .driver of > spi_driver. > > Signed-off-by: Hou Zhiqiang <b48286@freescale.com> > --- > drivers/mtd/devices/m25p80.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 7eda71d..b0c2b8c 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > @@ -66,6 +66,8 @@ > > /* Used for Spansion flashes only. */ > #define OPCODE_BRWR 0x17 /* Bank register write */ > +#define OPCODE_DP 0xb9 /* Enter deep power down mode */ > +#define OPCODE_RES 0xab /* Exit deep power down mode */ Where did you get these opcodes from? They are not in the Spansion datasheets I'm reading. And in fact, they are overloaded as RES (Read Electronic Signature, 0xab) and Bank Register Access (0xb9) in the datasheet I'm looking at. So this patch is wrong. Also, can you describe the purpose of these "deep power down" modes? I've never seen PM states where the *flash* needs to be put into a lower power mode. Typically the flash is pretty low-power when idle, and it may even be powered off completely when the system enters a lower-power state. Anyway, please describe why this patch is needed. > > /* Status Register bits. */ > #define SR_WIP 1 /* Write in progress */ > @@ -1128,11 +1130,46 @@ static int m25p_remove(struct spi_device *spi) > return mtd_device_unregister(&flash->mtd); > } > > +#ifdef CONFIG_PM > +static int m25p_suspend(struct device *dev, pm_message_t mesg) > +{ > + struct m25p *flash = dev_get_drvdata(dev); > + int ret; > + > + flash->command[0] = OPCODE_DP; As mentioned above, this opcode is not recognized by many flash supported in this driver. So we might want one or more of the following: (1) to assign different suspend/resume opcodes for use in m25p_suspend/resume (2) to provide over-loadable callbacks so that different flash could use different suspend/resume routines And of course, we need to avoid sending these commands at all to unsupported flash. > + mutex_lock(&flash->lock); > + /* Wait until finished previous write/erase command. */ > + ret = wait_till_ready(flash); > + if (ret) { > + mutex_unlock(&flash->lock); > + return ret; > + } > + ret = spi_write(flash->spi, flash->command, 1); > + mutex_unlock(&flash->lock); > + > + return ret; > +} > + > +static int m25p_resume(struct device *dev) > +{ > + struct m25p *flash = dev_get_drvdata(dev); > + int ret; > + > + flash->command[0] = OPCODE_RES; > + ret = spi_write(flash->spi, flash->command, 1); > + > + return ret; > +} > +#endif /* CONFIG_PM */ > > static struct spi_driver m25p80_driver = { > .driver = { > .name = "m25p80", > .owner = THIS_MODULE, > +#ifdef CONFIG_PM > + .suspend = m25p_suspend, > + .resume = m25p_resume, > +#endif > }, > .id_table = m25p_ids, > .probe = m25p_probe, Brian
>On Wed, Dec 11, 2013 at 04:19:30PM +0800, Hou Zhiqiang wrote: >> Add PM support using callback function suspend and resume in .driver >> of spi_driver. >> >> Signed-off-by: Hou Zhiqiang <b48286@freescale.com> >> --- >> drivers/mtd/devices/m25p80.c | 37 >> +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 37 insertions(+) >> >> diff --git a/drivers/mtd/devices/m25p80.c >> b/drivers/mtd/devices/m25p80.c index 7eda71d..b0c2b8c 100644 >> --- a/drivers/mtd/devices/m25p80.c >> +++ b/drivers/mtd/devices/m25p80.c >> @@ -66,6 +66,8 @@ >> >> /* Used for Spansion flashes only. */ >> #define OPCODE_BRWR 0x17 /* Bank register write */ >> +#define OPCODE_DP 0xb9 /* Enter deep power down mode */ >> +#define OPCODE_RES 0xab /* Exit deep power down mode */ > >Where did you get these opcodes from? They are not in the Spansion datasheets I'm reading. And in fact, they are overloaded as RES (Read Electronic Signature, 0xab) and Bank Register Access (0xb9) in the datasheet I'm looking at. So this patch is wrong. > In datasheet S25FL128P, Deep Power Down command is b9h and Release from Deep Power Down command is abh. In S25FL-A to S25FL-P Migration Guide those commands are the same. >Also, can you describe the purpose of these "deep power down" modes? >I've never seen PM states where the *flash* needs to be put into a lower power mode. Typically the flash is pretty low-power when idle, and it may even be powered off completely when the system enters a lower-power state. Anyway, please describe why this patch is needed. > In standby mode, the MAX currunt consumption is 200mA, and in Deep Power Down mode, the MAX is 20mA. In actually the typically value of currunt consumption is 3mA, so it save power consumption significantly I think. >> >> /* Status Register bits. */ >> #define SR_WIP 1 /* Write in progress */ >> @@ -1128,11 +1130,46 @@ static int m25p_remove(struct spi_device *spi) >> return mtd_device_unregister(&flash->mtd); >> } >> >> +#ifdef CONFIG_PM >> +static int m25p_suspend(struct device *dev, pm_message_t mesg) { >> + struct m25p *flash = dev_get_drvdata(dev); >> + int ret; >> + >> + flash->command[0] = OPCODE_DP; > >As mentioned above, this opcode is not recognized by many flash supported in this driver. So we might want one or more of the following: > > (1) to assign different suspend/resume opcodes for use in > m25p_suspend/resume > (2) to provide over-loadable callbacks so that different flash could > use different suspend/resume routines > >And of course, we need to avoid sending these commands at all to unsupported flash. > Yeah, in m25p_probe we can get spi flash specified PM commands from somewhere, but where can we set the PM commands, in struct spi_device_id? Do you have some good suggestion? >> + mutex_lock(&flash->lock); >> + /* Wait until finished previous write/erase command. */ >> + ret = wait_till_ready(flash); >> + if (ret) { >> + mutex_unlock(&flash->lock); >> + return ret; >> + } >> + ret = spi_write(flash->spi, flash->command, 1); >> + mutex_unlock(&flash->lock); >> + >> + return ret; >> +} >> + >> +static int m25p_resume(struct device *dev) { >> + struct m25p *flash = dev_get_drvdata(dev); >> + int ret; >> + >> + flash->command[0] = OPCODE_RES; >> + ret = spi_write(flash->spi, flash->command, 1); >> + >> + return ret; >> +} >> +#endif /* CONFIG_PM */ >> >> static struct spi_driver m25p80_driver = { >> .driver = { >> .name = "m25p80", >> .owner = THIS_MODULE, >> +#ifdef CONFIG_PM >> + .suspend = m25p_suspend, >> + .resume = m25p_resume, >> +#endif >> }, >> .id_table = m25p_ids, >> .probe = m25p_probe, > >Brian Zhiqiang Hou
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 7eda71d..b0c2b8c 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -66,6 +66,8 @@ /* Used for Spansion flashes only. */ #define OPCODE_BRWR 0x17 /* Bank register write */ +#define OPCODE_DP 0xb9 /* Enter deep power down mode */ +#define OPCODE_RES 0xab /* Exit deep power down mode */ /* Status Register bits. */ #define SR_WIP 1 /* Write in progress */ @@ -1128,11 +1130,46 @@ static int m25p_remove(struct spi_device *spi) return mtd_device_unregister(&flash->mtd); } +#ifdef CONFIG_PM +static int m25p_suspend(struct device *dev, pm_message_t mesg) +{ + struct m25p *flash = dev_get_drvdata(dev); + int ret; + + flash->command[0] = OPCODE_DP; + mutex_lock(&flash->lock); + /* Wait until finished previous write/erase command. */ + ret = wait_till_ready(flash); + if (ret) { + mutex_unlock(&flash->lock); + return ret; + } + ret = spi_write(flash->spi, flash->command, 1); + mutex_unlock(&flash->lock); + + return ret; +} + +static int m25p_resume(struct device *dev) +{ + struct m25p *flash = dev_get_drvdata(dev); + int ret; + + flash->command[0] = OPCODE_RES; + ret = spi_write(flash->spi, flash->command, 1); + + return ret; +} +#endif /* CONFIG_PM */ static struct spi_driver m25p80_driver = { .driver = { .name = "m25p80", .owner = THIS_MODULE, +#ifdef CONFIG_PM + .suspend = m25p_suspend, + .resume = m25p_resume, +#endif }, .id_table = m25p_ids, .probe = m25p_probe,
Add PM support using callback function suspend and resume in .driver of spi_driver. Signed-off-by: Hou Zhiqiang <b48286@freescale.com> --- drivers/mtd/devices/m25p80.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)