Re: power management routines for NAND driver

[Vitaly Wool <vwool@ru.mvista.com>]

Hi Thomas,

Thomas Gleixner wrote:

>On Thu, 2005-08-11 at 18:19 +0400, Vitaly Wool wrote:
>  
>
>>Hello all,
>>    
>>
>
>  
>
>>I've looked through mtd code and found mtd_pm_callback that should be 
>>called to handle PM events. This callback should in turn call 
>>mtd->suspend/mtd->resume functions, if any. Therefore one evident way of 
>>PM stuff implementation for this NAND flash is to provide suspend/resume 
>>functions. However, pm_send (that calls mtd_pm_callback) is never called
>>on ARM targets. So I doubt if it's appropriate to implement PM for the 
>>driver this way since it's looking somehow obsolete.
>>    
>>
>
>I have no idea whats the state of those PM functions and why ARM is not
>using them.
>  
>
Well, they are not at all widely used, only one MIPS board and x86 APM 
code use them.

>  
>
>>So, unfortunately, both methods don't provide the level of integrity I'd 
>>like to have. The problem is that nand core has no state machine in its 
>>internals so the driver can't track the flash being in suspended state.
>>    
>>
>
>As far as I know the nand driver code, there is a member "state" in the
>nand_chip private data structure, which is exactly tracking the state of
>the device. 
>
>FL_READY, FL_READING, FL_WRITING, FL_ERASING, FL_SYNCING, FL_CACHEDPRG,
>are the currently implemented states. Adding FL_SUSPEND is not a big
>problem.
>
>All functions which access the FLASH device are serialized by
>nand_get_device(), where the state variable is modified. So adding a
>suspend/resume function is simple enough.
>
>suspend justs sets the state to FL_SUSPEND and resume releases the
>device by calling nand_release_device() which sets the state back to
>FL_READY and wakes up the tasks on the wait_queue.
>
>It's discussable, whether suspend is supposed to fail, when a flash
>operation is in progress or just waits until the operation is finished,
>which is usually only a matter of a couple of ms.
>  
>
What if introduce an additional parameter ('nowait')?

>Of course we need an additional board function pointer in struct
>nand_chip to have a board specific callback for suspend/resume
>operations.
>
>  
>
I'd prefer probably to use platform_device and device_driver structures 
and put suspend/resume functions into device_driver. Those functions 
will first call mtd->suspend/mtd->resume and then shut down/re-enable 
the clocks.
(that's more or less how it's done in drivers/mtd/maps/sa1100-flash.c)
mtd->suspend and mtd->resume need to be implemented in nand_base then.
Does that sound reasonable?

Best regards,
   Vitaly