Skip to contentSkip to conversation

atmel nand patch CE don't care NAND

PATCH In review Michael Trimarchi 4 participants 10 messages Feb 2, 2011 – Feb 8, 2011

lore
Share & tools
lore
author
#1 lore raw

[U-Boot] ATMEL maintainer (WAS Re: [PATCH] atmel nand patch CE don't care NAND)

Dear Wolfgang

I have seen that there is not an atmel maintainer. Is it correct to send the patch to arm one? Who is the right person?

Michael Trimarchi

Quoted 11 lines · Michael Trimarchi

Hi,

this patch fix the support for CE don't care nand

Michael Trimarchi

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#2 lore raw

[U-Boot] ATMEL maintainer (WAS Re: [PATCH] atmel nand patch CE don't care NAND)

Dear Michael Trimarchi,

Quoted 3 lines · In message <[email protected]> you wrote:

I have seen that there is not an atmel maintainer. Is it correct to send the patch to arm one? Who is the right person?

Where have you seen that?

The official source of this information is
http://www.denx.de/wiki/U-Boot/Custodians

There you can see that Reinhard is the custodian for Atmel systems.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Whoever undertakes to set himself up as a judge of Truth  and  Know-
ledge is shipwrecked by the laughter of the gods."  - Albert Einstein
author
#3 lore raw

[U-Boot] ATMEL maintainer (WAS Re: [PATCH] atmel nand patch CE don't care NAND)

Quoted 9 lines · Wolfgang Denk

Dear Michael Trimarchi,

Quoted 2 lines · In message <[email protected]> you wrote:

I have seen that there is not an atmel maintainer. Is it correct to send the patch to arm one? Who is the right person?

Where have you seen that?

The official source of this information is
http://www.denx.de/wiki/U-Boot/Custodians

It was a mistake throw the gitweb. I have seen the at91 tree, sorry

Michael

Quoted 6 lines

There you can see that Reinhard is the custodian for Atmel systems.

Best regards,

Wolfgang Denk

#4 lore raw

PATCH 1/1 atmel nand patch CE don't care NAND

In reply to Michael Trimarchi
Quoted 5 lines · Michael Trimarchi

Hi,

this patch fix the support for CE don't care nand

Michael Trimarchi

Please read http://www.denx.de/wiki/U-Boot/Patches and follow the format therein -- don't use attachments.

Quoted 7 lines

commit 0cb23ef858407a7a9de6e353e08394637c518c89

Author: Michael Trimarchi <[email protected]>
Date:   Wed Feb 2 14:24:21 2011 +0100

Fix the CE for the NAND don't care

Signed-off-by Michael Trimarchi <[email protected]>

The changelog needs to be more descriptive. What is "NAND don't care"? What is broken?

Quoted 5 lines
drivers/mtd/nand/atmel_nand.c +0 -0
index ab8bbb3..bda117a 100644

drivers/mtd/nand/atmel_nand.c static void at91_nand_hwcontrol(struct mtd_info *mtd, · line 258

Quoted 10 lines
drivers/mtd/nand/atmel_nand.c +5 -0
@@ -249,8 +249,13 @@ static void at91_nand_hwcontrol(struct mtd_info *mtd,
 		if (ctrl & NAND_ALE)
 			IO_ADDR_W |= CONFIG_SYS_NAND_MASK_ALE;
 
+		/*
+		 * Nand CS don't care doesn't need the enable pin
+		 */
+#ifdef CONFIG_SYS_NAND_ENABLE_PIN
 		at91_set_gpio_value(CONFIG_SYS_NAND_ENABLE_PIN,
 				    !(ctrl & NAND_NCE));
+#endif

New CONFIG symbols need to be documented, and this particular one should probably be less generic.

-Scott

#5 lore raw

PATCH 1/1 atmel nand patch CE don't care NAND

Quoted 9 lines · On Tue, 8 Feb 2011 14:29:21 -0600 Scott Wood <[email protected]> wrote:
Quoted 8 lines · Michael Trimarchi
drivers/mtd/nand/atmel_nand.c +0 -0
index ab8bbb3..bda117a 100644
@@ -249,8 +249,13 @@ static void at91_nand_hwcontrol(struct mtd_info *mtd,
 		if (ctrl & NAND_ALE)
 			IO_ADDR_W |= CONFIG_SYS_NAND_MASK_ALE;
Quoted 10 lines

drivers/mtd/nand/atmel_nand.c static void at91_nand_hwcontrol(struct mtd_info *mtd, · line 258

Quoted 7 lines
drivers/mtd/nand/atmel_nand.c +5 -0
+		/*
+		 * Nand CS don't care doesn't need the enable pin
+		 */
+#ifdef CONFIG_SYS_NAND_ENABLE_PIN
 		at91_set_gpio_value(CONFIG_SYS_NAND_ENABLE_PIN,
 				    !(ctrl & NAND_NCE));
+#endif

New CONFIG symbols need to be documented, and this particular one should probably be less generic.

Sorry, ignore that -- I see it's not new (it should still be documented, but that's not this patch's problem).

The code change itself looks OK, just needs a better commit message/comment. Some googling indicates that "CE don't care" refers to the ability to deassert the chip enable line once an operation has been initiated. This seems to be different from not having control of CE at all (is it just always asserted on these boards?).

-Scott

author
#6 lore raw

PATCH 1/1 atmel nand patch CE don't care NAND

Hi

Quoted 23 lines · Scott Wood
Quoted 9 lines · On Tue, 8 Feb 2011 14:29:21 -0600 Scott Wood <[email protected]> wrote:
Quoted 8 lines · Michael Trimarchi
drivers/mtd/nand/atmel_nand.c +0 -0
index ab8bbb3..bda117a 100644
@@ -249,8 +249,13 @@ static void at91_nand_hwcontrol(struct mtd_info *mtd,
 		if (ctrl & NAND_ALE)
 			IO_ADDR_W |= CONFIG_SYS_NAND_MASK_ALE;
Quoted 10 lines

drivers/mtd/nand/atmel_nand.c static void at91_nand_hwcontrol(struct mtd_info *mtd, · line 258

Quoted 7 lines
drivers/mtd/nand/atmel_nand.c +5 -0
+		/*
+		 * Nand CS don't care doesn't need the enable pin
+		 */
+#ifdef CONFIG_SYS_NAND_ENABLE_PIN
 		at91_set_gpio_value(CONFIG_SYS_NAND_ENABLE_PIN,
 				    !(ctrl & NAND_NCE));
+#endif

New CONFIG symbols need to be documented, and this particular one should probably be less generic.

Quoted 2 lines

Sorry, ignore that -- I see it's not new (it should still be documented, but that's not this patch's problem).

too late :(

Quoted 6 lines

The code change itself looks OK, just needs a better commit message/comment. Some googling indicates that "CE don't care" refers to the ability to deassert the chip enable line once an operation has been initiated. This seems to be different from not having control of CE at all (is it just always asserted on these boards?).

It is connected to the CS3.

Quoted 3 lines

-Scott

Regards
Michael

Quoted 1 line
#7 lore raw

PATCH 1/1 atmel nand patch CE don't care NAND

In reply to Scott Wood

Dear Scott Wood, Michael Trimarchi,

Quoted 19 lines
Quoted 8 lines
Quoted 8 lines
drivers/mtd/nand/atmel_nand.c +0 -0
index ab8bbb3..bda117a 100644
@@ -249,8 +249,13 @@ static void at91_nand_hwcontrol(struct mtd_info *mtd,
  		if (ctrl&  NAND_ALE)
  			IO_ADDR_W |= CONFIG_SYS_NAND_MASK_ALE;
Quoted 10 lines

drivers/mtd/nand/atmel_nand.c static void at91_nand_hwcontrol(struct mtd_info *mtd, · line 258

Quoted 7 lines
drivers/mtd/nand/atmel_nand.c +5 -0
+		/*
+		 * Nand CS don't care doesn't need the enable pin
+		 */
+#ifdef CONFIG_SYS_NAND_ENABLE_PIN
  		at91_set_gpio_value(CONFIG_SYS_NAND_ENABLE_PIN,
  				    !(ctrl&  NAND_NCE));
+#endif

New CONFIG symbols need to be documented, and this particular one should probably be less generic.

Quoted 8 lines

Sorry, ignore that -- I see it's not new (it should still be documented, but that's not this patch's problem).

The code change itself looks OK, just needs a better commit message/comment. Some googling indicates that "CE don't care" refers to the ability to deassert the chip enable line once an operation has been initiated. This seems to be different from not having control of CE at all (is it just always asserted on these boards?).

I agree that the code looks OK. Apparently it means that CE must be always asserted (wired to GND) and that access to the chip is solely controlled by RE and WE. According to (for example) the Samsung K9F2G08X0B data sheet this is possible.

I also agree that the message/comment should point this out for future understanding.

Best Regards,
Reinhard

author
#8 lore raw

PATCH 1/1 atmel nand patch CE don't care NAND

In reply to Scott Wood

Hi,

Quoted 10 lines · Scott Wood
Quoted 5 lines · Michael Trimarchi

Hi,

this patch fix the support for CE don't care nand

Michael Trimarchi

Please read http://www.denx.de/wiki/U-Boot/Patches and follow the format therein -- don't use attachments.

The thunderbird config was wrong. I will use mutt next time

Quoted 11 lines
Quoted 7 lines

commit 0cb23ef858407a7a9de6e353e08394637c518c89

Author: Michael Trimarchi <[email protected]>
Date:   Wed Feb 2 14:24:21 2011 +0100

Fix the CE for the NAND don't care

Signed-off-by Michael Trimarchi <[email protected]>

The changelog needs to be more descriptive. What is "NAND don't care"? What is broken?

#9 lore raw

PATCH 1/1 atmel nand patch CE don't care NAND

Quoted 22 lines · On Tue, 8 Feb 2011 21:52:10 +0100 Michael Trimarchi…

Hi,

Quoted 10 lines · Scott Wood
Quoted 5 lines · Michael Trimarchi

Hi,

this patch fix the support for CE don't care nand

Michael Trimarchi

Please read http://www.denx.de/wiki/U-Boot/Patches and follow the format therein -- don't use attachments.

The thunderbird config was wrong. I will use mutt next time

Quoted 8 lines
Quoted 7 lines

commit 0cb23ef858407a7a9de6e353e08394637c518c89

Author: Michael Trimarchi <[email protected]>
Date:   Wed Feb 2 14:24:21 2011 +0100

Fix the CE for the NAND don't care

Signed-off-by Michael Trimarchi <[email protected]>
Quoted 7 lines
Quoted 3 lines

The changelog needs to be more descriptive. What is "NAND don't care"? What is broken?

From atmel documentation:

"CE connection depends on the NAND Flash. For standard NAND Flash devices, it must be connected to any free PIO line. For ?CE don?t care? NAND Flash devices, it can be connected to either NCS3/NANDCS or to any free PIO line."

So the absence of CONFIG_SYS_NAND_ENABLE_PIN means it's connected to NCS3/NANDCS (which is not quite the same as having a "CE don't care" NAND chip, since you still have the option of using a PIO line).

I'd just say something like this in the changelog (assuming my assumption about how this works is correct), and wouldn't bother with an in-code comment:

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.

-Scott