>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 <b48...@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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to