Patchwork [U-Boot,V2] This patch fix the usage of the "CE don't care"-type NAND chips

login
register
mail settings
Submitter michael
Date Feb. 23, 2011, 11:28 a.m.
Message ID <20110223112834.GB985@gandalf.sssup.it>
Download mbox | patch
Permalink /patch/84128/
State Changes Requested
Headers show

Comments

michael - Feb. 23, 2011, 11:28 a.m.
Change since V1:

   - add a better description

Signed-off-by: Michael Trimarchi <michael@evidence.eu.com>
Cc: Scott Wood <scottwood@freescale.com>
---
 drivers/mtd/nand/atmel_nand.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
Albert ARIBAUD - Feb. 23, 2011, 11:42 a.m.
Hi Michael,

Le 23/02/2011 12:28, Michael Trimarchi a écrit :
> Change since V1:
>
>     - add a better description

History should not be put above the '---' cut line; it should be below.

> Signed-off-by: Michael Trimarchi<michael@evidence.eu.com>
> Cc: Scott Wood<scottwood@freescale.com>
> ---
>   drivers/mtd/nand/atmel_nand.c |   10 ++++++++++
>   1 files changed, 10 insertions(+), 0 deletions(-)
>[...]

Amicalement,
michael - Feb. 23, 2011, 12:10 p.m.
Hi,

On 02/23/2011 12:42 PM, Albert ARIBAUD wrote:
> Hi Michael,
> 
> Le 23/02/2011 12:28, Michael Trimarchi a écrit :
>> Change since V1:
>>
>>     - add a better description
> 
> History should not be put above the '---' cut line; it should be below.
> 
>> Signed-off-by: Michael Trimarchi<michael@evidence.eu.com>
>> Cc: Scott Wood<scottwood@freescale.com>
>> ---
>>   drivers/mtd/nand/atmel_nand.c |   10 ++++++++++
>>   1 files changed, 10 insertions(+), 0 deletions(-)
>> [...]
> 

I will edit the text and resend with mutt. do I need to change again revision?

Michael Trimarchi

> Amicalement,
Wolfgang Denk - Feb. 23, 2011, 12:46 p.m.
Dear Michael Trimarchi,

In message <4D64F927.1000200@gandalf.sssup.it> you wrote:
> 
> I will edit the text and resend with mutt. do I need to change again revision?

Yes, please, as it's a new commit (commit message has changed).

Best regards,

Wolfgang Denk
Scott Wood - Feb. 23, 2011, 4:32 p.m.
On Wed, 23 Feb 2011 12:28:34 +0100
Michael Trimarchi <trimarchi@gandalf.sssup.it> wrote:

> Change since V1:
> 
>    - add a better description

This part does not go in the changelog -- it should be below the --- line.

> Signed-off-by: Michael Trimarchi <michael@evidence.eu.com>
> Cc: Scott Wood <scottwood@freescale.com>
> ---
>  drivers/mtd/nand/atmel_nand.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
> index ab8bbb3..c167f77 100644
> --- a/drivers/mtd/nand/atmel_nand.c
> +++ b/drivers/mtd/nand/atmel_nand.c
> @@ -249,8 +249,18 @@ static void at91_nand_hwcontrol(struct mtd_info *mtd,
>  		if (ctrl & NAND_ALE)
>  			IO_ADDR_W |= CONFIG_SYS_NAND_MASK_ALE;
>  
> +		/*
> +		 * atmel_nand: don't require CONFIG_SYS_NAND_ENABLE_PIN
> +		 * If NCE is hooked up to NCS3, we don't need to (and can't)
> +		 * explicitly set the state of the NCE pin. Instead, the
> +		 * controller asserts it automatically as part of a
> +		 * command/data access. Only "CE don't care"-type NAND chips
> +		 * can be used in this manner.
> +		 */

This was meant for the changelog, not the code.  For the code, it seems
reasonably self-explanatory that if you don't have
CONFIG_SYS_NAND_ENABLE_PIN, you don't use it.

If you want to put a short comment in the code about this situation, fine
(though it really belongs in a README that documents
CONFIG_SYS_NAND_ENABLE_PIN instead), but the text above should go in the git
changelog.

-Scott

Patch

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index ab8bbb3..c167f77 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -249,8 +249,18 @@  static void at91_nand_hwcontrol(struct mtd_info *mtd,
 		if (ctrl & NAND_ALE)
 			IO_ADDR_W |= CONFIG_SYS_NAND_MASK_ALE;
 
+		/*
+		 * atmel_nand: don't require CONFIG_SYS_NAND_ENABLE_PIN
+		 * If NCE is hooked up to NCS3, we don't need to (and can't)
+		 * explicitly set the state of the NCE pin. Instead, the
+		 * controller asserts it automatically as part of a
+		 * command/data access. Only "CE don't care"-type NAND chips
+		 * can be used in this manner.
+		 */
+#ifdef CONFIG_SYS_NAND_ENABLE_PIN
 		at91_set_gpio_value(CONFIG_SYS_NAND_ENABLE_PIN,
 				    !(ctrl & NAND_NCE));
+#endif
 		this->IO_ADDR_W = (void *) IO_ADDR_W;
 	}