public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* power management routines for NAND driver
@ 2005-08-11 14:19 Vitaly Wool
  2005-08-11 15:09 ` Thomas Gleixner
  2005-08-11 20:23 ` Todd Poynor
  0 siblings, 2 replies; 17+ messages in thread
From: Vitaly Wool @ 2005-08-11 14:19 UTC (permalink / raw)
  To: linux-mtd

Hello all,

I'd like to implement power management for NAND flash on ARMv5 platform. 
However, I haven't seen any NAND driver implementing suspend/resume 
routines so I'm not sure how to do it in an appropriate way.

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.

Another way could be define the platform_device and provide its 
suspend/resume functions, that would be called during the power state 
transition. This is basically how SA1100 NOR flash mapping driver works, 
but for Intel CFI-compliant  NOR flash there're appropriate 
suspend/resume functions implemented, and NAND base driver is not the case.

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.

Could you point me how can I synchronize access requests to the flash 
with suspend/resume calls (e.g., fail requests when flash is in suspend 
state) ? Isn't it appropriate to implement a state machine here similar 
to NOR CFI's one?

TIA!

Vitaly

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-11 14:19 power management routines for NAND driver Vitaly Wool
@ 2005-08-11 15:09 ` Thomas Gleixner
  2005-08-11 15:25   ` Vitaly Wool
  2005-08-12 15:11   ` Vitaly Wool
  2005-08-11 20:23 ` Todd Poynor
  1 sibling, 2 replies; 17+ messages in thread
From: Thomas Gleixner @ 2005-08-11 15:09 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mtd

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.

> 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.

Of course we need an additional board function pointer in struct
nand_chip to have a board specific callback for suspend/resume
operations.

> Could you point me how can I synchronize access requests to the flash 
> with suspend/resume calls (e.g., fail requests when flash is in suspend 
> state) ? 

Fail requests in suspend state needs more modifications. 

The current serialization in nand_get_device would stall the calling
task and add it to the wait_queue and woken up again in the resume /
nand_release_device() path.

>From my POV this is not a bad way to go.

> Isn't it appropriate to implement a state machine here similar 
> to NOR CFI's one?

See above. :)

tglx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-11 15:09 ` Thomas Gleixner
@ 2005-08-11 15:25   ` Vitaly Wool
  2005-08-11 16:25     ` Thomas Gleixner
  2005-08-12 15:11   ` Vitaly Wool
  1 sibling, 1 reply; 17+ messages in thread
From: Vitaly Wool @ 2005-08-11 15:25 UTC (permalink / raw)
  To: tglx; +Cc: linux-mtd

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-11 15:25   ` Vitaly Wool
@ 2005-08-11 16:25     ` Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2005-08-11 16:25 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mtd

On Thu, 2005-08-11 at 19:25 +0400, Vitaly Wool wrote:
> >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.

Well, those functions are marked deprecated. The device model is the
right way to do it. the SA1100 driver got it right apparently.

> >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')?

If its necessary and worth the work :)

> 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?

Sounds like a plan.

tglx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-11 14:19 power management routines for NAND driver Vitaly Wool
  2005-08-11 15:09 ` Thomas Gleixner
@ 2005-08-11 20:23 ` Todd Poynor
  2005-08-11 20:31   ` Thomas Gleixner
  1 sibling, 1 reply; 17+ messages in thread
From: Todd Poynor @ 2005-08-11 20:23 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mtd

Vitaly Wool wrote:

> 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.
> 
> Another way could be define the platform_device and provide its 
> suspend/resume functions, that would be called during the power state 
> transition. 

Yes, the pm_send etc. APM-style calls are deprecated.  The Linux Driver 
Model PM hooks (incl. struct device_driver suspend/resume callbacks) are 
the way to do it now.  I floated a patch for another board in thread 
"MTD PM Resume" in July that does approximately what you're talking 
about, although calling existing Intel/Sharp CFI NOR flash routines.  If 
there's no objections to that style of PM support then I'll check it in.

-- 
Todd

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-11 20:23 ` Todd Poynor
@ 2005-08-11 20:31   ` Thomas Gleixner
  2005-08-11 20:49     ` Vitaly Wool
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Gleixner @ 2005-08-11 20:31 UTC (permalink / raw)
  To: Todd Poynor; +Cc: linux-mtd, Vitaly Wool

On Thu, 2005-08-11 at 13:23 -0700, Todd Poynor wrote:

> Yes, the pm_send etc. APM-style calls are deprecated.  The Linux Driver 
> Model PM hooks (incl. struct device_driver suspend/resume callbacks) are 
> the way to do it now.  I floated a patch for another board in thread 
> "MTD PM Resume" in July that does approximately what you're talking 
> about, although calling existing Intel/Sharp CFI NOR flash routines.  If 
> there's no objections to that style of PM support then I'll check it in.

I removed the pm_oldstyle stuff in mtdcore.c anyway. 

Look at the SA1100 driver as reference. Thats the way to go.

tglx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-11 20:31   ` Thomas Gleixner
@ 2005-08-11 20:49     ` Vitaly Wool
  2005-08-11 20:53       ` Thomas Gleixner
  0 siblings, 1 reply; 17+ messages in thread
From: Vitaly Wool @ 2005-08-11 20:49 UTC (permalink / raw)
  To: tglx; +Cc: linux-mtd



Thomas Gleixner wrote:

>On Thu, 2005-08-11 at 13:23 -0700, Todd Poynor wrote:
>
>  
>
>>Yes, the pm_send etc. APM-style calls are deprecated.  The Linux Driver 
>>Model PM hooks (incl. struct device_driver suspend/resume callbacks) are 
>>the way to do it now.  I floated a patch for another board in thread 
>>"MTD PM Resume" in July that does approximately what you're talking 
>>about, although calling existing Intel/Sharp CFI NOR flash routines.  If 
>>there's no objections to that style of PM support then I'll check it in.
>>    
>>
>
>I removed the pm_oldstyle stuff in mtdcore.c anyway. 
>
>Look at the SA1100 driver as reference. Thats the way to go.
>
>tglx
>
>  
>
That will require some extensions to nand_base.c. I'm planning to 
provide the corresponding patch shortly, if you don't mind.

Best regards,
   Vitaly

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-11 20:49     ` Vitaly Wool
@ 2005-08-11 20:53       ` Thomas Gleixner
  2005-08-11 20:55         ` Josh Boyer
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Gleixner @ 2005-08-11 20:53 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mtd

On Fri, 2005-08-12 at 00:49 +0400, Vitaly Wool wrote:
> That will require some extensions to nand_base.c. I'm planning to 
> provide the corresponding patch shortly, if you don't mind.

I never have a problem with work which I dont have to do :)

tglx

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-11 20:53       ` Thomas Gleixner
@ 2005-08-11 20:55         ` Josh Boyer
  0 siblings, 0 replies; 17+ messages in thread
From: Josh Boyer @ 2005-08-11 20:55 UTC (permalink / raw)
  To: tglx; +Cc: linux-mtd, Vitaly Wool

On Thu, 2005-08-11 at 20:53 +0000, Thomas Gleixner wrote:
> On Fri, 2005-08-12 at 00:49 +0400, Vitaly Wool wrote:
> > That will require some extensions to nand_base.c. I'm planning to 
> > provide the corresponding patch shortly, if you don't mind.
> 
> I never have a problem with work which I dont have to do :)

Unless it's broken and you wind up having to support it ;)

josh

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-11 15:09 ` Thomas Gleixner
  2005-08-11 15:25   ` Vitaly Wool
@ 2005-08-12 15:11   ` Vitaly Wool
  2005-08-12 15:36     ` Thomas Gleixner
  1 sibling, 1 reply; 17+ messages in thread
From: Vitaly Wool @ 2005-08-12 15:11 UTC (permalink / raw)
  To: tglx; +Cc: linux-mtd

Hi Thomas,
please find the patch below.

Best regards,
   Vitaly

P. S. Is it reasonable to send the use case (i. e. the NAND driver I'm working at that uses the simple framework introduced)?

File: nand-pm.patch
Type: Enhancement
Signed-off-by: Vitaly Wool <vwool@ru.mvista.com>
Description:

	Add LDM-compliant power management support for NAND flashes.

Index: linux-2.6.10/include/linux/mtd/nand.h
===================================================================
--- linux-2.6.10.orig/include/linux/mtd/nand.h
+++ linux-2.6.10/include/linux/mtd/nand.h
@@ -244,6 +244,7 @@
 	FL_ERASING,
 	FL_SYNCING,
 	FL_CACHEDPRG,
+	FL_PM_SUSPENDED,
 } nand_state_t;
 
 /* Keep gcc happy */
Index: linux-2.6.10/drivers/mtd/nand/nand_base.c
===================================================================
--- linux-2.6.10.orig/drivers/mtd/nand/nand_base.c
+++ linux-2.6.10/drivers/mtd/nand/nand_base.c
@@ -46,6 +46,8 @@
  *		perform extra error status checks on erase and write failures.  This required
  *		adding a wrapper function for nand_read_ecc.
  *
+ * 08-20-2005	vwool: suspend/resume added
+ *
  * Credits:
  *	David Woodhouse for adding multichip support  
  *	
@@ -2284,6 +2286,45 @@
 }
 
 /**
+ * nand_suspend - [MTD Interface] Suspend the NAND flash
+ * @mtd:	MTD device structure
+ */
+static int nand_suspend(struct mtd_info *mtd)
+{
+	struct nand_chip *this = mtd->priv;
+	int ret = 0;
+
+	switch(this->state) {
+		case FL_READY:
+			nand_get_device (this, mtd, FL_PM_SUSPENDED);
+		case FL_PM_SUSPENDED:
+			break;
+
+		default:
+			ret = -EAGAIN;
+			break;
+	}
+	return ret;
+}
+
+/**
+ * nand_resume - [MTD Interface] Resume the NAND flash
+ * @mtd:	MTD device structure
+ */
+static void nand_resume(struct mtd_info *mtd)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (this->state == FL_PM_SUSPENDED)
+		nand_release_device(mtd);
+	else
+		printk(KERN_ERR "resume() called for the chip which is not "
+				"in suspended state\n");
+
+}
+
+
+/**
  * nand_scan - [NAND Interface] Scan for the NAND device
  * @mtd:	MTD device structure
  * @maxchips:	Number of chips to scan for
@@ -2642,8 +2683,8 @@
 	mtd->sync = nand_sync;
 	mtd->lock = NULL;
 	mtd->unlock = NULL;
-	mtd->suspend = NULL;
-	mtd->resume = NULL;
+	mtd->suspend = nand_suspend;
+	mtd->resume = nand_resume;
 	mtd->block_isbad = nand_block_isbad;
 	mtd->block_markbad = nand_block_markbad;
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-12 15:11   ` Vitaly Wool
@ 2005-08-12 15:36     ` Thomas Gleixner
  2005-08-12 15:41       ` Thomas Gleixner
                         ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Thomas Gleixner @ 2005-08-12 15:36 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mtd

On Fri, 2005-08-12 at 19:11 +0400, Vitaly Wool wrote:
> Hi Thomas,
> please find the patch below.
> 
> Best regards,
>    Vitaly
> 
> P. S. Is it reasonable to send the use case (i. e. the NAND driver I'm
> working at that uses the simple framework introduced)?

Sure

>  /**
> + * nand_suspend - [MTD Interface] Suspend the NAND flash
> + * @mtd:	MTD device structure
> + */
> +static int nand_suspend(struct mtd_info *mtd)
> +{
> +	struct nand_chip *this = mtd->priv;
> +	int ret = 0;
> +
> +	switch(this->state) {
> +		case FL_READY:
> +			nand_get_device (this, mtd, FL_PM_SUSPENDED);
> +		case FL_PM_SUSPENDED:
> +			break;
> +
> +		default:
> +			ret = -EAGAIN;
> +			break;
> +	}
> +	return ret;
> +}

This solution is racy. 

The correct solution is to do the check in nand_get_device with the lock
held. 

Index: nand_base.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/nand/nand_base.c,v
retrieving revision 1.148
diff -u -p -r1.148 nand_base.c
--- nand_base.c	4 Aug 2005 17:14:48 -0000	1.148
+++ nand_base.c	12 Aug 2005 15:28:14 -0000
@@ -755,7 +755,7 @@ static void nand_command_lp (struct mtd_
  *
  * Get the device and lock it for exclusive access
  */
-static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
+static int nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
 {
 	struct nand_chip *active;
 	spinlock_t *lock;
@@ -778,8 +778,11 @@ retry:
 	if (active == this && this->state == FL_READY) {
 		this->state = new_state;
 		spin_unlock(lock);
-		return;
+		return 0;
 	}
+	if (new_state == FL_PM_SUSPENDED)
+		return -EAGAIN;
+
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	add_wait_queue(wq, &wait);
 	spin_unlock(lock);

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-12 15:36     ` Thomas Gleixner
@ 2005-08-12 15:41       ` Thomas Gleixner
  2005-08-12 15:55       ` Vitaly Wool
  2005-08-12 15:57       ` Vitaly Wool
  2 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2005-08-12 15:41 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mtd

On Fri, 2005-08-12 at 15:36 +0000, Thomas Gleixner wrote:

> The correct solution is to do the check in nand_get_device with the lock
> held. 

Of course it works better when the lock is unlocked before returning :)

tglx


Index: nand_base.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/nand/nand_base.c,v
retrieving revision 1.148
diff -u -p -r1.148 nand_base.c
--- nand_base.c	4 Aug 2005 17:14:48 -0000	1.148
+++ nand_base.c	12 Aug 2005 15:39:55 -0000
@@ -755,7 +755,7 @@ static void nand_command_lp (struct mtd_
  *
  * Get the device and lock it for exclusive access
  */
-static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
+static int nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
 {
 	struct nand_chip *active;
 	spinlock_t *lock;
@@ -778,7 +778,11 @@ retry:
 	if (active == this && this->state == FL_READY) {
 		this->state = new_state;
 		spin_unlock(lock);
-		return;
+		return 0;
+	}
+	if (newstate == FL_PM_SUSPENDED) {
+		spin_unlock(lock);
+		return -EAGAIN;
 	}
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	add_wait_queue(wq, &wait);

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-12 15:36     ` Thomas Gleixner
  2005-08-12 15:41       ` Thomas Gleixner
@ 2005-08-12 15:55       ` Vitaly Wool
  2005-08-12 16:16         ` Thomas Gleixner
  2005-08-12 15:57       ` Vitaly Wool
  2 siblings, 1 reply; 17+ messages in thread
From: Vitaly Wool @ 2005-08-12 15:55 UTC (permalink / raw)
  To: tglx; +Cc: linux-mtd

Ok, here's the new one :) :

File: nand-pm.patch
Type: Enhancement
Signed-off-by: Vitaly Wool <vwool@ru.mvista.com>
Description:

	Add LDM-compliant power management support for NAND flashes.

Index: linux-2.6.10/include/linux/mtd/nand.h
===================================================================
--- linux-2.6.10.orig/include/linux/mtd/nand.h
+++ linux-2.6.10/include/linux/mtd/nand.h
@@ -244,6 +244,7 @@
 	FL_ERASING,
 	FL_SYNCING,
 	FL_CACHEDPRG,
+	FL_PM_SUSPENDED,
 } nand_state_t;
 
 /* Keep gcc happy */
Index: linux-2.6.10/drivers/mtd/nand/nand_base.c
===================================================================
--- linux-2.6.10.orig/drivers/mtd/nand/nand_base.c
+++ linux-2.6.10/drivers/mtd/nand/nand_base.c
@@ -46,6 +46,8 @@
  *		perform extra error status checks on erase and write failures.  This required
  *		adding a wrapper function for nand_read_ecc.
  *
+ * 08-20-2005	vwool: suspend/resume added
+ *
  * Credits:
  *	David Woodhouse for adding multichip support  
  *	
@@ -768,6 +770,11 @@
 	active = this;
 	spin_lock(lock);
 
+	if (this->state == new_state) {
+		spin_unlock(lock);
+		return;
+	}
+
 	/* Hardware controller shared among independend devices */
 	if (this->controller) {
 		if (this->controller->active)
@@ -2284,6 +2291,36 @@
 }
 
 /**
+ * nand_suspend - [MTD Interface] Suspend the NAND flash
+ * @mtd:	MTD device structure
+ */
+static int nand_suspend(struct mtd_info *mtd)
+{
+	struct nand_chip *this = mtd->priv;
+
+	nand_get_device (this, mtd, FL_PM_SUSPENDED);
+	
+	return 0;
+}
+
+/**
+ * nand_resume - [MTD Interface] Resume the NAND flash
+ * @mtd:	MTD device structure
+ */
+static void nand_resume(struct mtd_info *mtd)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (this->state == FL_PM_SUSPENDED)
+		nand_release_device(mtd);
+	else
+		printk(KERN_ERR "resume() called for the chip which is not "
+				"in suspended state\n");
+
+}
+
+
+/**
  * nand_scan - [NAND Interface] Scan for the NAND device
  * @mtd:	MTD device structure
  * @maxchips:	Number of chips to scan for
@@ -2642,8 +2679,8 @@
 	mtd->sync = nand_sync;
 	mtd->lock = NULL;
 	mtd->unlock = NULL;
-	mtd->suspend = NULL;
-	mtd->resume = NULL;
+	mtd->suspend = nand_suspend;
+	mtd->resume = nand_resume;
 	mtd->block_isbad = nand_block_isbad;
 	mtd->block_markbad = nand_block_markbad;
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-12 15:36     ` Thomas Gleixner
  2005-08-12 15:41       ` Thomas Gleixner
  2005-08-12 15:55       ` Vitaly Wool
@ 2005-08-12 15:57       ` Vitaly Wool
  2 siblings, 0 replies; 17+ messages in thread
From: Vitaly Wool @ 2005-08-12 15:57 UTC (permalink / raw)
  To: tglx; +Cc: linux-mtd


> > P. S. Is it reasonable to send the use case (i. e. the NAND driver I'm
> > working at that uses the simple framework introduced)?
> 
> Sure

Here it is (no cleanup, work in progress... ;) :

/*
 *  drivers/mtd/nand/pnx4008.c
 *
 *  Copyright (c) 2005 MontaVista <sources@mvista.com> 
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License version 2 as
 * published by the Free Software Foundation.
 *
 *  Overview:
 *   This is a device driver for the NAND flash device found on the
 *   PNX4008 board. It supports 16-bit 32MiB Samsung k9f5616 chip.
 *
 */

#include <linux/slab.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/pm.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/nand.h>
#include <linux/mtd/partitions.h>
#include <linux/dma-mapping.h>

#include <asm/io.h>
#include <asm/sizes.h>
#include <asm/dma.h>

#include <asm/arch/hardware.h>
#include <asm/arch/dma.h>
#include <asm/arch/gpio_new.h>

#include "pnx4008.h"

#define MAX_DMA_TRANSFER_SIZE 0x7FF

struct pnx4008_mtd_s {
    struct mtd_info 	mtd;
    struct nand_chip 	nand;
    int 		dma;
    struct completion 	done;
    u32			flashclk_reg;
    struct device *	dev;
};

static struct pnx4008_mtd_s *pnx4008_nand_data = NULL;
static int mlc_flash_dma = 1;

static int pnx4008_nand_ready(struct mtd_info *mtd);
static void pnx4008_nand_command ( struct mtd_info *mtd, unsigned command,
		                   int column, int page_addr);
static void pnx4008_nand_hwcontrol( struct mtd_info* mtd, int hwc );

#define MLC_WRITE( nand, offset, what ) __raw_writel( what, (u8*)( ((struct nand_chip*)nand)->IO_ADDR_R) + offset )
#define MLC_READ( nand, offset ) __raw_readl( (u8*)( ((struct nand_chip*)nand)->IO_ADDR_R) + offset )
#define MLC_READ2( nand, offset ) __raw_readb( (u8*)( ((struct nand_chip*)nand)->IO_ADDR_W) + offset ) 
#define MLC_WRITE2( nand, offset, what ) __raw_writeb( what, (u8*)( ((struct nand_chip*)nand)->IO_ADDR_W) + offset )

static inline u8 pnx4008_read_data( struct nand_chip* this )
{
	while( ! (MLC_READ( this, MLC_ISR ) & MLC_ISR_READY ) ) {
		continue;
	}
	return MLC_READ2( this, 0 );
}

static u8 pnx4008_nand_read_byte( struct mtd_info *mtd ) 
{
	return pnx4008_read_data( mtd->priv );
}

static void pnx4008_nand_write_byte( struct mtd_info* mtd, u8 byte )
{
	MLC_WRITE2( mtd->priv, 0, byte );
}

static void pnx4008_nand_dma_handler( int irq, void* context, struct pt_regs* regs)
{
	struct pnx4008_mtd_s* data = context;
	complete( &data->done );
}

static inline int  
pnx4008_nand_config_dma( int channel, u32 dma_buf, int len, int mode )
{
        pnx4008_dma_config_t cfg;
        pnx4008_dma_ch_config_t ch_cfg;
        pnx4008_dma_ch_ctrl_t ch_ctrl;
	int peripheral_id = 12;
	int err;
		
	if( mode == DMA_MODE_WRITE ) {
		ch_cfg.flow_cntrl = FC_MEM2PER_DMA;
                cfg.src_addr = dma_buf;
                ch_cfg.src_per = 0;
                cfg.dest_addr = PNX4008_FLASH_DATA;
                ch_cfg.dest_per = peripheral_id;
                ch_ctrl.di = 0;
                ch_ctrl.si = 1;
	} else if ( mode == DMA_MODE_READ ) {
		ch_cfg.flow_cntrl = FC_PER2MEM_DMA;
		cfg.dest_addr = dma_buf;
		ch_cfg.dest_per = 0;
		cfg.src_addr = PNX4008_FLASH_DATA;
		ch_cfg.src_per = peripheral_id;
		ch_ctrl.si = 0;
		ch_ctrl.di = 1;
	} else {
		return -EFAULT;
	}
	ch_cfg.halt = 0;
	ch_cfg.active = 1;
	ch_cfg.lock = 0;
	ch_cfg.itc = 1;
	ch_cfg.ie = 1;
	
	ch_ctrl.tc_mask = 1;
	ch_ctrl.cacheable = 0;
	ch_ctrl.bufferable = 0;
	ch_ctrl.priv_mode = 1;
	ch_ctrl.dest_ahb1 = 0;
	ch_ctrl.src_ahb1 = 0;
	ch_ctrl.dwidth = ch_ctrl.swidth = WIDTH_WORD;
	ch_ctrl.dbsize = 1;
	ch_ctrl.sbsize = 1;
	ch_ctrl.tr_size = len;

	if( 0 > ( err = pnx4008_dma_pack_config( &ch_cfg, &cfg.ch_cfg ))) {
		goto out;
	}
	if( 0 > (err = pnx4008_dma_pack_control(&ch_ctrl, &cfg.ch_ctrl))) {
		goto out;
	}
	err = pnx4008_config_channel( channel, &cfg ); 
out:	
	return err;
}

static void pnx4008_nand_write_buf( struct mtd_info *mtd, const u8 *buf, int len)
{
	struct nand_chip* this = mtd->priv;
	struct pnx4008_mtd_s* ctx = this->priv;
	dma_addr_t dma_addr;
	void* vaddr;
	int dma_len;

	if( mlc_flash_dma ) {
		do {
			dma_len = min( MAX_DMA_TRANSFER_SIZE, len );
			vaddr = dma_alloc_coherent( ctx->dev, dma_len, &dma_addr, GFP_KERNEL );
			memcpy( vaddr, buf, dma_len );
			pnx4008_nand_config_dma( ctx->dma, dma_addr, dma_len, DMA_MODE_WRITE );
			init_completion( &ctx->done );
			pnx4008_dma_ch_enable( ctx->dma );
			wait_for_completion( &ctx->done );		
			pnx4008_dma_ch_disable( ctx->dma );
			dma_free_coherent( ctx->dev, dma_len, vaddr, dma_addr );
			len -= dma_len; buf += dma_len;
		} while( len > 0 );
	} else {
    		while( len -- ) 
			pnx4008_nand_write_byte( mtd, *buf++ );
		while( !this->dev_ready( mtd ) ) 
			continue;
	}
}

static void pnx4008_nand_read_buf(struct mtd_info *mtd, u8 * buf, int len)
{
	struct nand_chip* this = mtd->priv;
	struct pnx4008_mtd_s* ctx = this->priv;
	dma_addr_t dma_addr;
	void* vaddr;
	int dma_len;

	if( mlc_flash_dma ) {	
		do {
			dma_len = min( MAX_DMA_TRANSFER_SIZE, len );
			vaddr = dma_alloc_coherent( ctx->dev, dma_len, &dma_addr, GFP_KERNEL );
			pnx4008_nand_config_dma( ctx->dma, dma_addr, dma_len, DMA_MODE_READ );
			init_completion( &ctx->done );
			pnx4008_dma_ch_enable( ctx->dma );
			wait_for_completion( &ctx->done );		
			pnx4008_dma_ch_disable( ctx->dma );
			memcpy( buf, vaddr, dma_len );
 			dma_free_coherent( ctx->dev, dma_len, vaddr, dma_addr );
			len -= dma_len; buf += dma_len;
		} while( len > 0 );
	} else {
		while( len -- ) 
			*buf ++ = pnx4008_nand_read_byte( mtd );
	}
}

static int pnx4008_nand_verify_buf( struct mtd_info *mtd, const u8 *buf, int len)
{
	int err = 0;

	while( len -- ) {
		if( *buf ++ != pnx4008_nand_read_byte( mtd ) ) {
			err = -EFAULT;
			break;
		}
	}
	return err;
}

static int pnx4008_nand_suspend(struct device *dev, u32 state, u32 level)
{
	int retval = 0;

#ifdef CONFIG_PM
	struct mtd_info *mtd = dev_get_drvdata(dev);
	if (mtd && mtd->suspend && level == SUSPEND_SAVE_STATE) {
		retval = mtd->suspend(mtd);
		if (retval == 0) {
			pnx4008_nand_data->flashclk_reg = __raw_readl(IO_ADDRESS(PNX4008_FLASHCLK_CTRL_BASE));
			__raw_writel(0, IO_ADDRESS(PNX4008_FLASHCLK_CTRL_BASE));
			udelay(1);
		}
	}
#endif
	return retval;
}

static int pnx4008_nand_resume(struct device *dev, u32 level)
{
#ifdef CONFIG_PM
	struct mtd_info *mtd = dev_get_drvdata(dev);

	if (mtd && mtd->resume && level == RESUME_RESTORE_STATE) {
		mtd->resume(mtd);
		__raw_writel(pnx4008_nand_data->flashclk_reg, IO_ADDRESS(PNX4008_FLASHCLK_CTRL_BASE));	
		udelay(1);
	}
#endif
	return 0;
}

#ifdef CONFIG_MTD_PARTITIONS
static struct mtd_partition static_partition[] = {
	{ .name = "Booting Image",
	  .offset =	0,
	  .size = 64 * 1024,
	  .mask_flags =	MTD_WRITEABLE  /* force read-only */
 	},
	{ .name = "U-Boot",
	  .offset =	MTDPART_OFS_APPEND,
	  .size = 256 * 1024,
          .mask_flags = MTD_WRITEABLE  /* force read-only */
 	},
	{ .name = "U-Boot Environment",
	  .offset =	MTDPART_OFS_APPEND,
	  .size = 192 * 1024
	},
	{ .name = "Kernel",
	  .offset =	MTDPART_OFS_APPEND,
	  .size = 2 * SZ_1M
	},
	{ .name = "File System",
	  .size = MTDPART_SIZ_FULL,
	  .offset =	MTDPART_OFS_APPEND,
	},
};

const char *part_probes[] = { 
	"cmdlinepart", 
	NULL,  
};

#endif

static int __init pnx4008_nand_probe (struct device *dev)
{
	struct nand_chip *this;
	struct mtd_info *mtd;
#ifdef CONFIG_MTD_PARTITIONS
	struct mtd_partition *dynamic_partition = 0;
#endif
	int err = 0;
	u32 flashclk_init = FLASHCLK_MLC_CLOCKS | FLASHCLK_MLC_SELECT;

	pnx4008_nand_data = kmalloc( sizeof( struct pnx4008_mtd_s ), GFP_KERNEL );
	if (!pnx4008_nand_data) {
		printk (KERN_ERR "%s: failed to allocate mtd_info\n", __FUNCTION__ );
		err = -ENOMEM;
		goto out;
	}
	memset( pnx4008_nand_data, 0, sizeof( struct pnx4008_mtd_s ) );
	pnx4008_nand_data->dma = -1;

	pnx4008_nand_data->dev = dev;

	/* structures must be linked */
	this = &pnx4008_nand_data->nand;
	mtd = &pnx4008_nand_data->mtd;
	mtd->priv = this;
	this->priv = pnx4008_nand_data;

	this->chip_delay = 30;
	this->IO_ADDR_R = (void __iomem*)IO_ADDRESS( PNX4008_MLC_FLASH_BASE );
        this->IO_ADDR_W = (void __iomem*)IO_ADDRESS( PNX4008_FLASH_DATA );
        this->options = NAND_SAMSUNG_LP_OPTIONS;
        this->hwcontrol = pnx4008_nand_hwcontrol;
	this->dev_ready = pnx4008_nand_ready;
        this->eccmode = NAND_ECC_SOFT;
	this->cmdfunc = pnx4008_nand_command;
	this->read_byte = pnx4008_nand_read_byte;
	this->write_byte = pnx4008_nand_write_byte;
	this->write_buf = pnx4008_nand_write_buf;
	this->read_buf = pnx4008_nand_read_buf;
	this->verify_buf = pnx4008_nand_verify_buf;
	
	if( mlc_flash_dma ) {
		pnx4008_nand_data->dma = pnx4008_request_channel( 
				"MLC Flash", -1, 
				pnx4008_nand_dma_handler, pnx4008_nand_data );
		if( pnx4008_nand_data->dma <= 0 ) {
			printk( KERN_ERR"%s: cannot request DMA channel (%d), falling back to non-DMA mode.\n", __FUNCTION__, pnx4008_nand_data->dma );
			mlc_flash_dma = 0;
		} else {
			flashclk_init |= FLASHCLK_NAND_RnB_REQ_E;
		}
	}	
	__raw_writel( flashclk_init, IO_ADDRESS( PNX4008_FLASHCLK_CTRL_BASE ) );
	pnx4008_nand_data->flashclk_reg = flashclk_init;	
	
	MLC_WRITE( this, MLC_LOCK_PR, MLC_UNLOCK_MAGIC );
	/* Setting Buswidth for controller */
	MLC_WRITE( this, MLC_ICR, 
	  	MLC_ICR_WIDTH8 | MLC_ICR_AWC3 | 
	  	MLC_ICR_SMALLBLK | MLC_ICR_WP_DISABLED ); 

    	/* remove the write protection (/WP), by setting GPO_01 to high */
    	__raw_writel( GPIO_FLASH_WP, 
			IO_ADDRESS( PNX4008_PIO_BASE ) + GPIO_OUTP_SET);

	MLC_WRITE( this, MLC_LOCK_PR, MLC_UNLOCK_MAGIC );
	MLC_WRITE( this, MLC_TIME, 
			FLASH_MLC_TIME_tWP( 4 )  |
			FLASH_MLC_TIME_tWH( 7 )  |
			FLASH_MLC_TIME_tRP( 6 )  |
			FLASH_MLC_TIME_tREH( 2 ) |
			FLASH_MLC_TIME_tRHZ( 3 ) |
			FLASH_MLC_TIME_tRWB( 3 ) |
			FLASH_MLC_TIME_tCEA( 0 ) );  /* 104 Hz */
	/* wait 6 uSeconds for device to become ready */
	udelay(6);

	if( nand_scan( mtd, 1 ) ) {
		goto out_1;
	}

#ifdef CONFIG_MTD_PARTITIONS
        err = parse_mtd_partitions( mtd, part_probes,
                                    &dynamic_partition, 0);
        if (err > 0)
             err = add_mtd_partitions( mtd,
				       dynamic_partition, err);
        else 
             err = add_mtd_partitions( mtd,
				       static_partition, 
				       ARRAY_SIZE( static_partition ) );
	if( err < 0 ) {
		goto out_2;
	}

#endif
	dev_set_drvdata(dev, mtd);
	return 0;

#ifdef CONFIG_MTD_PARTITIONS
out_2:
	nand_release( mtd );
#endif

out_1:	
	kfree ( pnx4008_nand_data );
out:
	return err;
}

static void __exit pnx4008_nand_remove (struct device *dev)
{
	dev_set_drvdata(dev, NULL);

	nand_release ( &pnx4008_nand_data->mtd);
	if( pnx4008_nand_data->dma >=0 ) {
		pnx4008_free_channel( pnx4008_nand_data->dma );
	}
	kfree ( pnx4008_nand_data );
	
	__raw_writel( 0, IO_ADDRESS( PNX4008_FLASHCLK_CTRL_BASE ) );
}


static struct device_driver pnx4008_nand_driver = {
	.name		= "pnx4008-flash",
	.bus		= &platform_bus_type,
	.probe		= pnx4008_nand_probe,
	.remove		= __exit_p(pnx4008_nand_remove),
	.suspend	= pnx4008_nand_suspend,
	.resume		= pnx4008_nand_resume,
};

static int __init pnx4008_nand_init(void)
{
	return driver_register(&pnx4008_nand_driver);
}

static void __exit pnx4008_nand_exit(void)
{
	driver_unregister(&pnx4008_nand_driver);
}

module_exit(pnx4008_nand_exit);
module_init(pnx4008_nand_init);

static int pnx4008_nand_ready(struct mtd_info *mtd)
{	
    const u32 mask = MLC_ISR_READY | MLC_ISR_IFREADY | MLC_ISR_ECCREADY;
    return ( ( MLC_READ( mtd->priv, MLC_ISR ) & mask ) == mask ) ? 1 : 0;
}

static void pnx4008_nand_hwcontrol( struct mtd_info* mtd, int hwc )
{
}

static void pnx4008_nand_command ( struct mtd_info *mtd, unsigned command,
		                   int column, int page_addr)
{
    struct nand_chip *this = mtd->priv;

    /* Emulate NAND_CMD_READOOB */
    if (command == NAND_CMD_READOOB) {
        column += mtd->oobblock;
        command = NAND_CMD_READ0;
    }
    
    /* Write out the command to the device. */
    MLC_WRITE( this, MLC_CMD, command );

    if (column != -1 || page_addr != -1) {
        /* Serially input address */
        if (column != -1) {
            /* Adjust columns for 16 bit buswidth */
            if (this->options & NAND_BUSWIDTH_16)
                column >>= 1;
            MLC_WRITE( this, MLC_ADDR, column & 0xff);
            MLC_WRITE( this, MLC_ADDR, (column >> 8) & 0xff);
        }
        if (page_addr != -1) {
            MLC_WRITE( this, MLC_ADDR, page_addr & 0xff );
            MLC_WRITE( this, MLC_ADDR, (page_addr >> 8) & 0xff);

            /* One more address cycle for devices > 128MiB */
            if (this->chipsize > (128 << 20)) {
		MLC_WRITE( this, MLC_ADDR, (page_addr >> 16) & 0xff);
	    }
        }
    }
    /*
     * program and erase have their own busy handlers
     * status and sequential in needs no delay
    */
    switch (command) 
    {
    case NAND_CMD_CACHEDPROG:
    case NAND_CMD_PAGEPROG:
    case NAND_CMD_ERASE1:
    case NAND_CMD_ERASE2:
    case NAND_CMD_SEQIN:
    case NAND_CMD_STATUS:
        return;
    case NAND_CMD_RESET:
        if (this->dev_ready)
            break;
        udelay(this->chip_delay);
        MLC_WRITE( this, MLC_CMD, NAND_CMD_STATUS );
	while(( pnx4008_read_data( this ) & 0x40 ) == 0 ) 
	    continue;
        return;
    case NAND_CMD_READ0:
        /* Write out the start read command */
        MLC_WRITE( this, MLC_CMD, NAND_CMD_READSTART);
	/* fall down */
    default:
        /*
         * If we don't have access to the busy pin, we apply the given
         * command delay
        */
        if (!this->dev_ready) {
            udelay (this->chip_delay);
            return;
        }
    }
    /* Apply this short delay always to ensure that we do wait tWB in
     * any case on any machine. */
    ndelay (100);
    /* wait until command is processed */
    while (!this->dev_ready(mtd)) {
	continue;
    }
}

MODULE_LICENSE("GPL");
MODULE_AUTHOR("dmitry pervushin <dpervushin@ru.mvista.com>");
MODULE_PARM( mlc_flash_dma, "i" );

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-12 15:55       ` Vitaly Wool
@ 2005-08-12 16:16         ` Thomas Gleixner
  2005-08-12 16:46           ` Vitaly Wool
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Gleixner @ 2005-08-12 16:16 UTC (permalink / raw)
  To: Vitaly Wool; +Cc: linux-mtd

On Fri, 2005-08-12 at 19:55 +0400, Vitaly Wool wrote:
> @@ -768,6 +770,11 @@
>  	active = this;
>  	spin_lock(lock);
>  
> +	if (this->state == new_state) {
> +		spin_unlock(lock);
> +		return;
> +	}
> +
>  	/* Hardware controller shared among independend devices */

Urggh. That breaks everything except FL_PM_SUSPENDED as you return for
all cases, where this->state and new_state are equal. 

You want to return 0, when the FLASH is already in suspend state ?

tglx

Index: nand_base.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/nand/nand_base.c,v
retrieving revision 1.148
diff -u -p -r1.148 nand_base.c
--- nand_base.c	4 Aug 2005 17:14:48 -0000	1.148
+++ nand_base.c	12 Aug 2005 16:14:47 -0000
@@ -755,7 +755,7 @@ static void nand_command_lp (struct mtd_
  *
  * Get the device and lock it for exclusive access
  */
-static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
+static int nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
 {
 	struct nand_chip *active;
 	spinlock_t *lock;
@@ -778,7 +778,11 @@ retry:
 	if (active == this && this->state == FL_READY) {
 		this->state = new_state;
 		spin_unlock(lock);
-		return;
+		return 0;
+	}
+	if (new_state == FL_PM_SUSPENDED) {
+		spin_unlock(lock);
+		return (this->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
 	}
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	add_wait_queue(wq, &wait);

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-12 16:16         ` Thomas Gleixner
@ 2005-08-12 16:46           ` Vitaly Wool
  2005-08-22 12:54             ` Vitaly Wool
  0 siblings, 1 reply; 17+ messages in thread
From: Vitaly Wool @ 2005-08-12 16:46 UTC (permalink / raw)
  To: tglx; +Cc: linux-mtd


> Index: nand_base.c
> ===================================================================
> RCS file: /home/cvs/mtd/drivers/mtd/nand/nand_base.c,v
> retrieving revision 1.148
> diff -u -p -r1.148 nand_base.c
> --- nand_base.c	4 Aug 2005 17:14:48 -0000	1.148
> +++ nand_base.c	12 Aug 2005 16:14:47 -0000
> @@ -755,7 +755,7 @@ static void nand_command_lp (struct mtd_
>   *
>   * Get the device and lock it for exclusive access
>   */
> -static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
> +static int nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
>  {
>  	struct nand_chip *active;
>  	spinlock_t *lock;
> @@ -778,7 +778,11 @@ retry:
>  	if (active == this && this->state == FL_READY) {
>  		this->state = new_state;
>  		spin_unlock(lock);
> -		return;
> +		return 0;
> +	}
> +	if (new_state == FL_PM_SUSPENDED) {
> +		spin_unlock(lock);
> +		return (this->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
>  	}
>  	set_current_state(TASK_UNINTERRUPTIBLE);
>  	add_wait_queue(wq, &wait);
> 

10x, that's definitely better... 
Lemme insert the complete patch then:

Index: linux-2.6.10/include/linux/mtd/nand.h
===================================================================
--- linux-2.6.10.orig/include/linux/mtd/nand.h
+++ linux-2.6.10/include/linux/mtd/nand.h
@@ -244,6 +244,7 @@
 	FL_ERASING,
 	FL_SYNCING,
 	FL_CACHEDPRG,
+	FL_PM_SUSPENDED,
 } nand_state_t;
 
 /* Keep gcc happy */
Index: linux-2.6.10/drivers/mtd/nand/nand_base.c
===================================================================
--- linux-2.6.10.orig/drivers/mtd/nand/nand_base.c
+++ linux-2.6.10/drivers/mtd/nand/nand_base.c
@@ -46,6 +46,8 @@
  *		perform extra error status checks on erase and write failures.  This required
  *		adding a wrapper function for nand_read_ecc.
  *
+ * 08-20-2005	vwool: suspend/resume added
+ *
  * Credits:
  *	David Woodhouse for adding multichip support  
  *	
@@ -153,7 +155,7 @@
 #define nand_verify_pages(...) (0)
 #endif
 		
-static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state);
+static int nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state);
 
 /**
  * nand_release_device - [GENERIC] release chip
@@ -755,7 +757,7 @@
  *
  * Get the device and lock it for exclusive access
  */
-static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
+static int nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
 {
 	struct nand_chip *active;
 	spinlock_t *lock;
@@ -778,7 +780,11 @@
 	if (active == this && this->state == FL_READY) {
 		this->state = new_state;
 		spin_unlock(lock);
-		return;
+		return 0;
+	}
+	if (new_state == FL_PM_SUSPENDED) {
+		spin_unlock(lock);
+		return (this->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
 	}
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	add_wait_queue(wq, &wait);
@@ -2284,6 +2290,34 @@
 }
 
 /**
+ * nand_suspend - [MTD Interface] Suspend the NAND flash
+ * @mtd:	MTD device structure
+ */
+static int nand_suspend(struct mtd_info *mtd)
+{
+	struct nand_chip *this = mtd->priv;
+
+	return nand_get_device (this, mtd, FL_PM_SUSPENDED);
+}
+
+/**
+ * nand_resume - [MTD Interface] Resume the NAND flash
+ * @mtd:	MTD device structure
+ */
+static void nand_resume(struct mtd_info *mtd)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (this->state == FL_PM_SUSPENDED)
+		nand_release_device(mtd);
+	else
+		printk(KERN_ERR "resume() called for the chip which is not "
+				"in suspended state\n");
+
+}
+
+
+/**
  * nand_scan - [NAND Interface] Scan for the NAND device
  * @mtd:	MTD device structure
  * @maxchips:	Number of chips to scan for
@@ -2642,8 +2676,8 @@
 	mtd->sync = nand_sync;
 	mtd->lock = NULL;
 	mtd->unlock = NULL;
-	mtd->suspend = NULL;
-	mtd->resume = NULL;
+	mtd->suspend = nand_suspend;
+	mtd->resume = nand_resume;
 	mtd->block_isbad = nand_block_isbad;
 	mtd->block_markbad = nand_block_markbad;
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: power management routines for NAND driver
  2005-08-12 16:46           ` Vitaly Wool
@ 2005-08-22 12:54             ` Vitaly Wool
  0 siblings, 0 replies; 17+ messages in thread
From: Vitaly Wool @ 2005-08-22 12:54 UTC (permalink / raw)
  To: tglx; +Cc: linux-mtd

Hi Thomas,
does that look nice enough?

Index: linux-2.6.10/include/linux/mtd/nand.h
===================================================================
--- linux-2.6.10.orig/include/linux/mtd/nand.h
+++ linux-2.6.10/include/linux/mtd/nand.h
@@ -244,6 +244,7 @@
 	FL_ERASING,
 	FL_SYNCING,
 	FL_CACHEDPRG,
+	FL_PM_SUSPENDED,
 } nand_state_t;
 
 /* Keep gcc happy */
Index: linux-2.6.10/drivers/mtd/nand/nand_base.c
===================================================================
--- linux-2.6.10.orig/drivers/mtd/nand/nand_base.c
+++ linux-2.6.10/drivers/mtd/nand/nand_base.c
@@ -46,6 +46,8 @@
  *		perform extra error status checks on erase and write failures.  This required
  *		adding a wrapper function for nand_read_ecc.
  *
+ * 08-20-2005	vwool: suspend/resume added
+ *
  * Credits:
  *	David Woodhouse for adding multichip support  
  *	
@@ -153,7 +155,7 @@
 #define nand_verify_pages(...) (0)
 #endif
 		
-static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state);
+static int nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state);
 
 /**
  * nand_release_device - [GENERIC] release chip
@@ -755,7 +757,7 @@
  *
  * Get the device and lock it for exclusive access
  */
-static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
+static int nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
 {
 	struct nand_chip *active;
 	spinlock_t *lock;
@@ -778,7 +780,11 @@
 	if (active == this && this->state == FL_READY) {
 		this->state = new_state;
 		spin_unlock(lock);
-		return;
+		return 0;
+	}
+	if (new_state == FL_PM_SUSPENDED) {
+		spin_unlock(lock);
+		return (this->state == FL_PM_SUSPENDED) ? 0 : -EAGAIN;
 	}
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	add_wait_queue(wq, &wait);
@@ -2284,6 +2290,34 @@
 }
 
 /**
+ * nand_suspend - [MTD Interface] Suspend the NAND flash
+ * @mtd:	MTD device structure
+ */
+static int nand_suspend(struct mtd_info *mtd)
+{
+	struct nand_chip *this = mtd->priv;
+
+	return nand_get_device (this, mtd, FL_PM_SUSPENDED);
+}
+
+/**
+ * nand_resume - [MTD Interface] Resume the NAND flash
+ * @mtd:	MTD device structure
+ */
+static void nand_resume(struct mtd_info *mtd)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (this->state == FL_PM_SUSPENDED)
+		nand_release_device(mtd);
+	else
+		printk(KERN_ERR "resume() called for the chip which is not "
+				"in suspended state\n");
+
+}
+
+
+/**
  * nand_scan - [NAND Interface] Scan for the NAND device
  * @mtd:	MTD device structure
  * @maxchips:	Number of chips to scan for
@@ -2642,8 +2676,8 @@
 	mtd->sync = nand_sync;
 	mtd->lock = NULL;
 	mtd->unlock = NULL;
-	mtd->suspend = NULL;
-	mtd->resume = NULL;
+	mtd->suspend = nand_suspend;
+	mtd->resume = nand_resume;
 	mtd->block_isbad = nand_block_isbad;
 	mtd->block_markbad = nand_block_markbad;
 

Best regards,
   Vitaly

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2005-08-22 12:54 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-11 14:19 power management routines for NAND driver Vitaly Wool
2005-08-11 15:09 ` Thomas Gleixner
2005-08-11 15:25   ` Vitaly Wool
2005-08-11 16:25     ` Thomas Gleixner
2005-08-12 15:11   ` Vitaly Wool
2005-08-12 15:36     ` Thomas Gleixner
2005-08-12 15:41       ` Thomas Gleixner
2005-08-12 15:55       ` Vitaly Wool
2005-08-12 16:16         ` Thomas Gleixner
2005-08-12 16:46           ` Vitaly Wool
2005-08-22 12:54             ` Vitaly Wool
2005-08-12 15:57       ` Vitaly Wool
2005-08-11 20:23 ` Todd Poynor
2005-08-11 20:31   ` Thomas Gleixner
2005-08-11 20:49     ` Vitaly Wool
2005-08-11 20:53       ` Thomas Gleixner
2005-08-11 20:55         ` Josh Boyer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox