MTD for Taco

PATCH In review Sean MacLennan 6 participants 23 messages Jan 5, 2008 – Jan 16, 2008

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Sean MacLennan

author

2008-01-05 05:17 UTC #0 lore raw

PATCH 1/1 MTD for Taco

This patch adds the maps for the taco. It also gets the ndfc.c NAND driver in a compilable state. The map is guaranteed to change since the exact NOR/NAND flash configuration is in flux right now when we found the 256M NAND flash won't boot properly.

Currently it configures the NOR in a reasonable fashion and leaves the NAND as one honkin' parition.

Cheers,
Sean

drivers/mtd/maps/Kconfig +9 -0

index a592fc0..24cbafa 100644
@@ -588,6 +588,15 @@ config MTD_INTEL_VR_NOR
 	  Map driver for a NOR flash bank located on the Expansion Bus of the
 	  Intel Vermilion Range chipset.
 
+config MTD_TACO
+	tristate "Flash devices mapped on Taco boards."
+	depends on MTD_CFI && PPC32 && 44x && TACO
+	help
+	  This enables access routined for the flash chips on the PIKA
+	  Taco board.
+	  If you have this board and would like to use the flash
+	  chips on it, say 'Y'.
+
 config MTD_PLATRAM
 	tristate "Map driver for platform device RAM (mtd-ram)"
 	select MTD_RAM

drivers/mtd/maps/Makefile +1 -0

index 316382a..4de7ef9 100644
@@ -69,3 +69,4 @@ obj-$(CONFIG_MTD_PLATRAM)	+= plat-ram.o
 obj-$(CONFIG_MTD_OMAP_NOR)	+= omap_nor.o
 obj-$(CONFIG_MTD_MTX1)		+= mtx-1_flash.o
 obj-$(CONFIG_MTD_INTEL_VR_NOR)	+= intel_vr_nor.o
+obj-$(CONFIG_MTD_TACO)	        += taco.o

drivers/mtd/nand/Kconfig +1 -1

index 246d451..ca9f35a 100644
@@ -153,7 +153,7 @@ config MTD_NAND_S3C2410_HWECC
 
 config MTD_NAND_NDFC
 	tristate "NDFC NanD Flash Controller"
-	depends on 4xx && !PPC_MERGE
+	depends on 4xx
 	select MTD_NAND_ECC_SMC
 	help
 	 NDFC Nand Flash Controllers are integrated in IBM/AMCC's 4xx SoCs

drivers/mtd/nand/ndfc.c +36 -6

66 lines not on this page · raw message

mtd/maps/taco.c new file +140 -0

140 lines not on this page · raw message

Stefan Roese

2008-01-05 09:47 UTC #1 lore raw

Quoted 7 lines · Sean MacLennan

This patch adds the maps for the taco. It also gets the ndfc.c NAND driver in a compilable state. The map is guaranteed to change since the exact NOR/NAND flash configuration is in flux right now when we found the 256M NAND flash won't boot properly.
Currently it configures the NOR in a reasonable fashion and leaves the NAND as one honkin' parition.

<snip>

drivers/mtd/nand/ndfc.c line 31

Quoted 13 lines

drivers/mtd/nand/ndfc.c +0 -0
index 1c0e89f..f5e93cf 100644
@@ -24,11 +24,6 @@
=A0#include <linux/platform_device.h>
=A0
=A0#include <asm/io.h>
drivers/mtd/nand/ndfc.c +0 -5
-#ifdef CONFIG_40x
-#include <asm/ibm405.h>
-#else
-#include <asm/ibm44x.h>
-#endif

You do break arch/ppc support with this patch. We have to still support arch/ppc a few month, so please don't break this support for now.
=A0

Quoted 2 lines

=A0struct ndfc_nand_mtd {
=A0=A0=A0=A0=A0=A0=A0=A0struct mtd_info=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=

=A0=A0=A0=A0=A0=A0mtd;

drivers/mtd/nand/ndfc.c static int ndfc_calculate_ecc(struct mtd_info *mtd, · line 111

Quoted 8 lines

drivers/mtd/nand/ndfc.c +0 -0

@@ -110,6 +105,40 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
 =A0=A0=A0=A0=A0=A0=A0=A0return 0;
 =A0}

=A0

drivers/mtd/nand/ndfc.c +4 -0

+#ifdef CONFIG_TACO
+/* The NDFC may allow 32bit read/writes, but it sure doesn't work on
+ * the taco!
+ */

We definitely don't want to see such board specific stuff in the common NDFC driver. And I really doubt that you need this change for your board. We are using this ndfc driver on multiple boards, and all have no problems accessing the controller with 32bit read/writes. So you most likely have a problem with your board port. Perhaps something with with the EBC setup. Please re-check and compare with boards that are know to work, like Sequoia.

drivers/mtd/nand/ndfc.c

Quoted 7 lines

drivers/mtd/nand/ndfc.c +7 -0

+static void ndfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+=A0=A0=A0=A0=A0=A0=A0struct ndfc_controller *ndfc =3D &ndfc_ctrl;
+=A0=A0=A0=A0=A0=A0=A0uint8_t *p =3D (uint8_t *) buf;
+
+=A0=A0=A0=A0=A0=A0=A0for(;len > 0; len -=3D 1)
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0*p++ =3D __raw_readb(ndfc->=

ndfcbase + NDFC_DATA);

Quoted 9 lines

drivers/mtd/nand/ndfc.c +3 -0

+}
+
+static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int

len) +{

+=A0=A0=A0=A0=A0=A0=A0struct ndfc_controller *ndfc =3D &ndfc_ctrl;
+=A0=A0=A0=A0=A0=A0=A0uint8_t *p =3D (uint8_t *) buf;
+
+=A0=A0=A0=A0=A0=A0=A0for(;len > 0; len -=3D 1)
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0__raw_writeb(*p++, ndfc->nd=

fcbase + NDFC_DATA);

Quoted 9 lines

drivers/mtd/nand/ndfc.c +3 -0

+}
+
+static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int

len) +{

+=A0=A0=A0=A0=A0=A0=A0struct ndfc_controller *ndfc =3D &ndfc_ctrl;
+=A0=A0=A0=A0=A0=A0=A0uint8_t *p =3D (uint8_t *) buf;
+
+=A0=A0=A0=A0=A0=A0=A0for(;len > 0; len -=3D 1)
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (*p++ !=3D __raw_readb(n=

dfc->ndfcbase + NDFC_DATA))

Quoted 1 line

+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0ret=

urn -EFAULT;

Quoted 4 lines

drivers/mtd/nand/ndfc.c +4 -0
+
+=A0=A0=A0=A0=A0=A0=A0return 0;
+}
+#else

<snip>

Quoted 1 line

+++ drivers/mtd/maps/taco.c=A0=A0=A0=A0=A02008-01-02 13:07:43.000000000 -=

0500

drivers/mtd/nand/ndfc.c line 6

Quoted 7 lines

drivers/mtd/nand/ndfc.c +6 -0

@@ -0,0 +1,140 @@
+/*
+ * $Id: $
+ *
+ * drivers/mtd/maps/taco.c
+ *
+ * Mapping for PIKA Taco flash

I'm pretty sure that you don't need a board specific mapping driver for NOR flash. physmap_of should be exactly what you need. You just need to fill the device tree properties correctly.

BTW: I noticed you are using the boot wrapper approach. This is not necessary anymore, since the latest U-Boot version has flattened device tree support included for 4xx too. Let me know if you have any questions about this.

Best regards,
Stefan

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: [email protected] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

David Gibson

2008-01-05 12:25 UTC #2 lore raw

Quoted 13 lines · Stefan Roese

mtd/maps/taco.c line 6
Quoted 8 lines · Sean MacLennan
+++ drivers/mtd/maps/taco.c 2008-01-02 13:07:43.000000000 -0500
mtd/maps/taco.c new file +6 -0
@@ -0,0 +1,140 @@
+/*
+ * $Id: $
+ *
+ * drivers/mtd/maps/taco.c
+ *
+ * Mapping for PIKA Taco flash
I'm pretty sure that you don't need a board specific mapping driver for NOR flash. physmap_of should be exactly what you need. You just need to fill the device tree properties correctly.

Absolutely. We should not be using C-coded maps in arch/powerpc

Signature · 4 lines

David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Sean MacLennan

author

2008-01-06 03:21 UTC #3 lore raw

Quoted 8 lines · David Gibson

Quoted 4 lines
I'm pretty sure that you don't need a board specific mapping driver for NOR flash. physmap_of should be exactly what you need. You just need to fill the device tree properties correctly.
Absolutely. We should not be using C-coded maps in arch/powerpc

Ok a couple of questions. How do I find out the size of the NOR flash? It was in a global in the ppc arch.

Also, where is the best place to fixup the partitions? We have paritions that must be at the top and ones that must be at the bottom, so we dynamically setup the partitions based on the NOR size. The NOR size is variable.

And is it ok the name the paritions in the dts based on an index? I notice others use an address, but since I don't know the address up front I just used index numbers.

Cheers,
Sean

David Gibson

2008-01-06 03:44 UTC #4 lore raw

Quoted 10 lines · Sean MacLennan

Quoted 6 lines · David Gibson
Quoted 4 lines
I'm pretty sure that you don't need a board specific mapping driver for NOR flash. physmap_of should be exactly what you need. You just need to fill the device tree properties correctly.
Absolutely. We should not be using C-coded maps in arch/powerpc
Ok a couple of questions. How do I find out the size of the NOR flash? It was in a global in the ppc arch.

The flash size goes in the "reg" property of the flash node.

Quoted 4 lines

Also, where is the best place to fixup the partitions? We have paritions that must be at the top and ones that must be at the bottom, so we dynamically setup the partitions based on the NOR size. The NOR size is variable.

Ok, you can fix this up in the bootwrapper.

Quoted 3 lines

And is it ok the name the paritions in the dts based on an index? I notice others use an address, but since I don't know the address up front I just used index numbers.

No, the node name must match the node's "reg" property, which is the partition's offset. If you're creating the partition nodess on the fly in the bootwrapper you can get that right there. I'm also hoping soon to add a function to fixup node names to libfdt, which will then be usable in the wrapper.

Signature · 4 lines

David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Sean MacLennan

author

2008-01-05 18:20 UTC #5 lore raw

In reply to Stefan Roese

Quoted 21 lines · Stefan Roese

Quoted 8 lines · Sean MacLennan
This patch adds the maps for the taco. It also gets the ndfc.c NAND driver in a compilable state. The map is guaranteed to change since the exact NOR/NAND flash configuration is in flux right now when we found the 256M NAND flash won't boot properly.
Currently it configures the NOR in a reasonable fashion and leaves the NAND as one honkin' parition.
<snip>
Quoted 7 lines
drivers/mtd/nand/ndfc.c +0 -0
index 1c0e89f..f5e93cf 100644
@@ -24,11 +24,6 @@
 #include <linux/platform_device.h>
 

Quoted 11 lines

drivers/mtd/nand/ndfc.c line 27
Quoted 7 lines
drivers/mtd/nand/ndfc.c +0 -5
 #include <asm/io.h>
-#ifdef CONFIG_40x
-#include <asm/ibm405.h>
-#else
-#include <asm/ibm44x.h>
-#endif
 
You do break arch/ppc support with this patch. We have to still support arch/ppc a few month, so please don't break this support for now.

Gotcha. Is CONFIG_PPC_MERGED the right flag for things like this?

Quoted 8 lines

drivers/mtd/nand/ndfc.c +0 -0

 struct ndfc_nand_mtd {
        struct mtd_info                 mtd;

@@ -110,6 +105,40 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
        return 0;
 }

Quoted 15 lines

drivers/mtd/nand/ndfc.c static int ndfc_calculate_ecc(struct mtd_info *mtd,
Quoted 5 lines
drivers/mtd/nand/ndfc.c +4 -0
+#ifdef CONFIG_TACO
+/* The NDFC may allow 32bit read/writes, but it sure doesn't work on
+ * the taco!
+ */
 
We definitely don't want to see such board specific stuff in the common NDFC driver. And I really doubt that you need this change for your board. We are using this ndfc driver on multiple boards, and all have no problems accessing the controller with 32bit read/writes. So you most likely have a problem with your board port. Perhaps something with with the EBC setup. Please re-check and compare with boards that are know to work, like Sequoia.

Does the Sequoia use a Rev C chip? We had to modify u-boot the same way. As soon as we do an 8-bit access we get a machine check exception.

To be honest, I don't know what more to check.

Quoted 35 lines

Quoted 31 lines

drivers/mtd/nand/ndfc.c +28 -0

+static void ndfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+       struct ndfc_controller *ndfc = &ndfc_ctrl;
+       uint8_t *p = (uint8_t *) buf;
+
+       for(;len > 0; len -= 1)
+               *p++ = __raw_readb(ndfc->ndfcbase + NDFC_DATA);
+}
+
+static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int
 len) +{
+       struct ndfc_controller *ndfc = &ndfc_ctrl;
+       uint8_t *p = (uint8_t *) buf;
+
+       for(;len > 0; len -= 1)
+               __raw_writeb(*p++, ndfc->ndfcbase + NDFC_DATA);
+}
+
+static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int
 len) +{
+       struct ndfc_controller *ndfc = &ndfc_ctrl;
+       uint8_t *p = (uint8_t *) buf;
+
+       for(;len > 0; len -= 1)
+               if (*p++ != __raw_readb(ndfc->ndfcbase + NDFC_DATA))
+                       return -EFAULT;
+
+       return 0;
+}
+#else

<snip>

Quoted 14 lines

drivers/mtd/nand/ndfc.c line 6
Quoted 9 lines
+++ drivers/mtd/maps/taco.c 2008-01-02 13:07:43.000000000 -0500
drivers/mtd/nand/ndfc.c +6 -0
@@ -0,0 +1,140 @@
+/*
+ * $Id: $
+ *
+ * drivers/mtd/maps/taco.c
+ *
+ * Mapping for PIKA Taco flash
I'm pretty sure that you don't need a board specific mapping driver for NOR flash. physmap_of should be exactly what you need. You just need to fill the device tree properties correctly.

Yes, this was copied straight from the PPC port. The whole NOR/NAND flash is in flux. Marketing really really wants to drop NOR for price reasons. So I will probably leave this until a final decision is made.

Quoted 5 lines

BTW: I noticed you are using the boot wrapper approach. This is not necessary anymore, since the latest U-Boot version has flattened device tree support included for 4xx too. Let me know if you have any questions about this.

I did this port over Christmas and had no access to the hardware guys so I stayed away from u-boot since I had no way to recover from mistakes. I hope to make towards the tree image approach but for now the in-kernel version is much easier for me to work with.

Cheers,
Sean

Arnd Bergmann

2008-01-05 19:20 UTC #6 lore raw

Quoted 4 lines · Sean MacLennan

Quoted 3 lines
You do break arch/ppc support with this patch. We have to still support arch/ppc a few month, so please don't break this support for now. =A0=20
Gotcha. Is CONFIG_PPC_MERGED the right flag for things like this?

Yes, but it it's spelled CONFIG_PPC_MERGE, not CONFIG_PPC_MERGED.

	Arnd=20

Sean MacLennan

author

2008-01-09 18:05 UTC #7 lore raw

In reply to Stefan Roese

Quoted 21 lines · Stefan Roese

Quoted 8 lines · Sean MacLennan
This patch adds the maps for the taco. It also gets the ndfc.c NAND driver in a compilable state. The map is guaranteed to change since the exact NOR/NAND flash configuration is in flux right now when we found the 256M NAND flash won't boot properly.
Currently it configures the NOR in a reasonable fashion and leaves the NAND as one honkin' parition.
<snip>
Quoted 7 lines
drivers/mtd/nand/ndfc.c +0 -0
index 1c0e89f..f5e93cf 100644
@@ -24,11 +24,6 @@
 #include <linux/platform_device.h>
 

Quoted 11 lines

drivers/mtd/nand/ndfc.c line 27
Quoted 7 lines
drivers/mtd/nand/ndfc.c +0 -5
 #include <asm/io.h>
-#ifdef CONFIG_40x
-#include <asm/ibm405.h>
-#else
-#include <asm/ibm44x.h>
-#endif
 
You do break arch/ppc support with this patch. We have to still support arch/ppc a few month, so please don't break this support for now.

Here is an updated patch that does not break the ppc arch support. Unfortunately, it still has the taco specific patch. I can't find anything different between the taco and the sequoia and since this patch fixes the problem, the hardware guys refuse to look into it. I can't blame them as we are overloaded as it is.

But I will send out the patch anyway in case anybody needs the ndfc driver working under the powerpc arch.

Cheers,
Sean

Signed-off-by Sean MacLennan <[email protected]>

---

drivers/mtd/nand/Kconfig +0 -0

index 246d451..ca9f35a 100644
@@ -153,7 +153,7 @@ config MTD_NAND_S3C2410_HWECC

config MTD_NAND_NDFC
tristate "NDFC NanD Flash Controller"
- depends on 4xx && !PPC_MERGE
+ depends on 4xx
select MTD_NAND_ECC_SMC
help
NDFC Nand Flash Controllers are integrated in IBM/AMCC's 4xx SoCs

drivers/mtd/nand/ndfc.c +0 -0

index 1c0e89f..fe19177 100644
@@ -24,11 +24,13 @@

#include <linux/platform_device.h>

#include <asm/io.h>
+#ifndef CONFIG_PPC_MERGE
#ifdef CONFIG_40x
#include <asm/ibm405.h>
#else
#include <asm/ibm44x.h>
#endif
+#endif

struct ndfc_nand_mtd {
	struct mtd_info			mtd;
@@ -110,6 +112,40 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
	return 0;
}

+#ifdef CONFIG_WARP
+/* The NDFC may allow 32bit read/writes, but it sure doesn't work on
+ * the Warp!
+ */
+static void ndfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+ struct ndfc_controller *ndfc = &ndfc_ctrl;
+ uint8_t *p = (uint8_t *) buf;
+
+ for(;len > 0; len -= 1)
+ *p++ = __raw_readb(ndfc->ndfcbase + NDFC_DATA);
+}
+
+static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+ struct ndfc_controller *ndfc = &ndfc_ctrl;
+ uint8_t *p = (uint8_t *) buf;
+
+ for(;len > 0; len -= 1)
+ __raw_writeb(*p++, ndfc->ndfcbase + NDFC_DATA);
+}
+
+static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+ struct ndfc_controller *ndfc = &ndfc_ctrl;
+ uint8_t *p = (uint8_t *) buf;
+
+ for(;len > 0; len -= 1)
+ if (*p++ != __raw_readb(ndfc->ndfcbase + NDFC_DATA))
+ return -EFAULT;
+
+ return 0;
+}
+#else
/*
* Speedups for buffer read/write/verify
*
@@ -145,6 +181,7 @@ static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)

			return -EFAULT;
	return 0;
}

+#endif

/*
* Initialize chip structure
@@ -236,6 +273,8 @@ static int ndfc_nand_probe(struct platform_device *pdev)

#ifndef CONFIG_PHYS_64BIT
	ndfc->ndfcbase = ioremap((phys_addr_t)phys, res->end - res->start + 1);
+#elif defined(CONFIG_PPC_MERGE)
+	ndfc->ndfcbase = ioremap(phys, res->end - res->start + 1);
#else
	ndfc->ndfcbase = ioremap64(phys, res->end - res->start + 1);

#endif

Josh Boyer

2008-01-09 18:43 UTC #8 lore raw

Quoted 33 lines · On Wed, 09 Jan 2008 13:05:35 -0500 Sean MacLennan <[email protected]>…

Quoted 21 lines · Stefan Roese
Quoted 8 lines · Sean MacLennan
This patch adds the maps for the taco. It also gets the ndfc.c NAND driver in a compilable state. The map is guaranteed to change since the exact NOR/NAND flash configuration is in flux right now when we found the 256M NAND flash won't boot properly.
Currently it configures the NOR in a reasonable fashion and leaves the NAND as one honkin' parition.
<snip>
Quoted 7 lines
drivers/mtd/nand/ndfc.c +0 -0
index 1c0e89f..f5e93cf 100644
@@ -24,11 +24,6 @@
 #include <linux/platform_device.h>
 
Quoted 11 lines
drivers/mtd/nand/ndfc.c line 27
Quoted 7 lines
drivers/mtd/nand/ndfc.c +0 -5
 #include <asm/io.h>
-#ifdef CONFIG_40x
-#include <asm/ibm405.h>
-#else
-#include <asm/ibm44x.h>
-#endif
 
You do break arch/ppc support with this patch. We have to still support arch/ppc a few month, so please don't break this support for now.

Quoted 8 lines

Here is an updated patch that does not break the ppc arch support. Unfortunately, it still has the taco specific patch. I can't find anything different between the taco and the sequoia and since this patch fixes the problem, the hardware guys refuse to look into it. I can't blame them as we are overloaded as it is.
But I will send out the patch anyway in case anybody needs the ndfc driver working under the powerpc arch.

This doesn't really get the addresses, etc from the device tree though, correct?

josh

Sean MacLennan

author

2008-01-09 18:50 UTC #9 lore raw

Quoted 46 lines · Josh Boyer

Quoted 37 lines · On Wed, 09 Jan 2008 13:05:35 -0500 Sean MacLennan <[email protected]>…
Quoted 23 lines · Stefan Roese
Quoted 9 lines · Sean MacLennan
This patch adds the maps for the taco. It also gets the ndfc.c NAND driver in a compilable state. The map is guaranteed to change since the exact NOR/NAND flash configuration is in flux right now when we found the 256M NAND flash won't boot properly.
Currently it configures the NOR in a reasonable fashion and leaves the NAND as one honkin' parition.
<snip>
Quoted 7 lines
drivers/mtd/nand/ndfc.c +0 -0
index 1c0e89f..f5e93cf 100644
@@ -24,11 +24,6 @@
 #include <linux/platform_device.h>
 
Quoted 12 lines
drivers/mtd/nand/ndfc.c line 28
Quoted 8 lines
drivers/mtd/nand/ndfc.c +0 -5
 #include <asm/io.h>
-#ifdef CONFIG_40x
-#include <asm/ibm405.h>
-#else
-#include <asm/ibm44x.h>
-#endif
 
 
You do break arch/ppc support with this patch. We have to still support arch/ppc a few month, so please don't break this support for now.
Quoted 4 lines
Here is an updated patch that does not break the ppc arch support. Unfortunately, it still has the taco specific patch. I can't find anything different between the taco and the sequoia and since this patch fixes the problem, the hardware guys refuse to look into it. I can't blame them as we are overloaded as it is.
But I will send out the patch anyway in case anybody needs the ndfc driver working under the powerpc arch.

Quoted 9 lines

This doesn't really get the addresses, etc from the device tree though, correct?
josh
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev

No. You have to setup everything the old way. This *just* gets it working so if you have the PPC layout, it will work.

Unless testing goes *really* well, I doubt I will have time in the short term to port it to the of_platform. I'm afraid I will have to live with "it works, don't touch it" for now :(

Cheers,
Sean

Josh Boyer

2008-01-09 19:04 UTC #10 lore raw

Quoted 6 lines · On Wed, 09 Jan 2008 13:50:41 -0500 Sean MacLennan <[email protected]>…

No. You have to setup everything the old way. This *just* gets it working so if you have the PPC layout, it will work.
Unless testing goes *really* well, I doubt I will have time in the short term to port it to the of_platform. I'm afraid I will have to live with "it works, don't touch it" for now :(

That's fine for you, nobody expects you to have to do all the porting for everything :).

I don't want it actually ported to of_platform anyway. What I really want, and what Stefan has started some time ago, is to make it get the values from the device tree but still create the regular platform devices.

josh

Sean MacLennan

author

2008-01-14 04:55 UTC #11 lore raw

In reply to Stefan Roese

Quoted 15 lines · Stefan Roese

drivers/mtd/nand/ndfc.c static int ndfc_calculate_ecc(struct mtd_info *mtd, · line 111
Quoted 6 lines
drivers/mtd/nand/ndfc.c +4 -0
+#ifdef CONFIG_TACO
+/* The NDFC may allow 32bit read/writes, but it sure doesn't work on
+ * the taco!
+ */
We definitely don't want to see such board specific stuff in the common NDFC driver. And I really doubt that you need this change for your board. We are using this ndfc driver on multiple boards, and all have no problems accessing the controller with 32bit read/writes. So you most likely have a problem with your board port. Perhaps something with with the EBC setup. Please re-check and compare with boards that are know to work, like Sequoia.

How about adding a config option that lets you specify 8 bit access? Something like CONFIG_NDFC_8BIT_ACCESS. We could default it to no and put a little blurb that says something like:

On some platforms the 32bit read/writes cause a machine access exception. If you get a machine access exception while reading the NAND bad block table, try turning on 8 bit access.

Cheers,
Sean

Josh Boyer

2008-01-14 08:44 UTC #12 lore raw

Quoted 16 lines · On Sun, 13 Jan 2008 23:55:21 -0500 Sean MacLennan <[email protected]>…

Quoted 15 lines · Stefan Roese
drivers/mtd/nand/ndfc.c static int ndfc_calculate_ecc(struct mtd_info *mtd, · line 111
Quoted 6 lines
drivers/mtd/nand/ndfc.c +4 -0
+#ifdef CONFIG_TACO
+/* The NDFC may allow 32bit read/writes, but it sure doesn't work on
+ * the taco!
+ */
We definitely don't want to see such board specific stuff in the common NDFC driver. And I really doubt that you need this change for your board. We are using this ndfc driver on multiple boards, and all have no problems accessing the controller with 32bit read/writes. So you most likely have a problem with your board port. Perhaps something with with the EBC setup. Please re-check and compare with boards that are know to work, like Sequoia.

Quoted 7 lines

How about adding a config option that lets you specify 8 bit access? Something like CONFIG_NDFC_8BIT_ACCESS. We could default it to no and put a little blurb that says something like:
On some platforms the 32bit read/writes cause a machine access exception. If you get a machine access exception while reading the NAND bad block table, try turning on 8 bit access.

But did you go back and verify the EBC settings were correct on your board? This shouldn't be needed at all if the EBC bank settings and timings are correct.

josh

Sean MacLennan

author

2008-01-14 17:32 UTC #13 lore raw

Quoted 7 lines · Josh Boyer

But did you go back and verify the EBC settings were correct on your board? This shouldn't be needed at all if the EBC bank settings and timings are correct.
josh

In the EBC0_CFG register we set the RTC (Ready Timeout Count) to 0 and the sequoia uses 7. Also we set the EMS (External Master Size) to none and the sequoia sets it to 8 bit.

This is in uboot, the dts are basically the same.

Cheers,
Sean

Stefan Roese

2008-01-14 19:43 UTC #14 lore raw

Quoted 5 lines · Sean MacLennan

Quoted 4 lines · Josh Boyer
But did you go back and verify the EBC settings were correct on your board? This shouldn't be needed at all if the EBC bank settings and timings are correct.

Quoted 7 lines

Quoted 1 line
josh
In the EBC0_CFG register we set the RTC (Ready Timeout Count) to 0 and the sequoia uses 7. Also we set the EMS (External Master Size) to none and the sequoia sets it to 8 bit.
This is in uboot, the dts are basically the same.

And the EBC0_BxCR & EBC0BxAP registers for the CS where the NAND is connected? How are they configured?

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: [email protected]
=====================================================================

Sean MacLennan

author

2008-01-14 20:04 UTC #15 lore raw

Quoted 2 lines · Stefan Roese

And the EBC0_BxCR & EBC0BxAP registers for the CS where the NAND is connected? How are they configured?

EBC0_B1CR d001c000
EBC0_B1AP 18003c0

Which matches the defines in include/configs/warp.h:

   #define CFG_EBC_PB1AP        0x018003c0
   #define CFG_EBC_PB1CR        (CFG_NAND_ADDR | 0x1c000)

It also matches the defines in sequoia.h except that we are on CS1 and the sequoia is on CS3.

Cheers,
Sean

Stefan Roese

2008-01-15 05:15 UTC #16 lore raw

Quoted 7 lines · Sean MacLennan

Quoted 2 lines · Stefan Roese
And the EBC0_BxCR & EBC0BxAP registers for the CS where the NAND is connected? How are they configured?
EBC0_B1CR d001c000
EBC0_B1AP 18003c0

Quoted 7 lines

Which matches the defines in include/configs/warp.h:
    #define CFG_EBC_PB1AP        0x018003c0
    #define CFG_EBC_PB1CR        (CFG_NAND_ADDR | 0x1c000)
It also matches the defines in sequoia.h except that we are on CS1 and the sequoia is on CS3.

Right. One thing I noticed though is, that you map the NAND to 0xd0000000, which is reserved for PCI in the 440EP address space. I suggest you map it to 0x90000000 as done on Bamboo. Please give it a try and let me know if this changes the 32bit access behavior.

Best regards,
Stefan

Sean MacLennan

author

2008-01-15 06:30 UTC #17 lore raw

Quoted 3 lines · Stefan Roese

Right. One thing I noticed though is, that you map the NAND to 0xd0000000, which is reserved for PCI in the 440EP address space. I suggest you map it to 0x90000000 as done on Bamboo. Please give it a try and let me know if this changes the 32bit access behavior.

I think I changed it right. The following code is obviously a hack:

static int warp_setup_nand_flash(void)

{
	unsigned data;

	mfebc(0x1, data);
	printk("EBC0_B1CR %x\n", data); // SAM DBG

	data = 0x9001c000;
	mtebc(0x1, data);

	mfebc(0x1, data);
	printk("after EBC0_B1CR %x\n", data); // SAM DBG

	mfebc(0x11, data);
	printk("EBC0_B1AP %x\n", data); // SAM DBG

	platform_device_register(&warp_ndfc_device);
	platform_device_register(&warp_nand_device);

	return 0;
}

device_initcall(warp_setup_nand_flash);

Then change the NAND base offset to 90000000. This change made no difference. It still works with 8-bit access and fails with 32-bit. The mtebc and mfebc macros where taken from u-boot.

Cheers,
Sean

Stefan Roese

2008-01-15 06:39 UTC #18 lore raw

Quoted 29 lines · Sean MacLennan

Quoted 4 lines · Stefan Roese
Right. One thing I noticed though is, that you map the NAND to 0xd0000000, which is reserved for PCI in the 440EP address space. I suggest you map it to 0x90000000 as done on Bamboo. Please give it a try and let me know if this changes the 32bit access behavior.
I think I changed it right. The following code is obviously a hack:
static int warp_setup_nand_flash(void)
{
	unsigned data;

	mfebc(0x1, data);
	printk("EBC0_B1CR %x\n", data); // SAM DBG

	data = 0x9001c000;
	mtebc(0x1, data);

	mfebc(0x1, data);
	printk("after EBC0_B1CR %x\n", data); // SAM DBG

	mfebc(0x11, data);
	printk("EBC0_B1AP %x\n", data); // SAM DBG

	platform_device_register(&warp_ndfc_device);
	platform_device_register(&warp_nand_device);

	return 0;
}

Quoted 6 lines

device_initcall(warp_setup_nand_flash);
Then change the NAND base offset to 90000000. This change made no difference. It still works with 8-bit access and fails with 32-bit. The mtebc and mfebc macros where taken from u-boot.

Bummer! Was worth a try though. I still don't see why this should fail on your platform. What error/exception do you get upon 32bit access btw?

Best regards,
Stefan

Sean MacLennan

author

2008-01-15 17:23 UTC #19 lore raw

Quoted 2 lines · Stefan Roese

Bummer! Was worth a try though. I still don't see why this should fail on your platform. What error/exception do you get upon 32bit access btw?

Ask and Ye Shall RX! Here is a complete trace of the crash including the NAND debug outputs.

Cheers,
Sean

NDFC NAND Driver initialized. Chip-Rev: 0x00000109
NAND device: Manufacturer ID: 0xec, Chip ID: 0x76 (Samsung NAND 64MiB 3,3V 8-bi) Scanning device for bad blocks
Data machine check in kernel mode.

Oops: Machine check, sig: 7 [#1]
Warp
Modules linked in:
NIP: 00000000 LR: c01559b8 CTR: c0159f64
REGS: c02c2f50 TRAP: 0202   Not tainted  (2.6.24-rc6-pika)
MSR: 00000000 <>  CR: 42000024  XER: 2000005f
TASK = cf81bba0[1] 'swapper' THREAD: cf81e000
GPR00: 70000000 cf81fd30 cf81bba0 c02daac4 cf9a5280 0000000c 00000240 00000000 GPR08: 122deea8 d1064000 000f4240 c02e0000 121f0cf6 00000000 0ffabe00 c02b0d14 GPR16: 0040044c 0000001f cf9a5080 00000000 00000000 00000010 00000002 00000000 GPR24: 00000010 00000000 00000001 c02daac4 cf81fda8 00000000 c02daac4 c0159f64 NIP [00000000] _start+0x40000000/0x30
LR [c01559b8] nand_read_oob_std+0x54/0x6c
Call Trace:
[cf81fd30] [c01559a0] nand_read_oob_std+0x3c/0x6c (unreliable)
[cf81fd50] [c0155fb4] nand_read_oob+0x204/0x334
[cf81fda0] [c01589c8] create_bbt+0x210/0x2f0
[cf81fe00] [c0159abc] nand_scan_bbt+0x464/0x4c4
[cf81fe50] [c0157a1c] nand_scan_tail+0x4b0/0x4f4
[cf81fe70] [c015a214] ndfc_chip_probe+0x168/0x1d4
[cf81fe90] [c012e014] platform_drv_probe+0x20/0x30
[cf81fea0] [c012bee8] driver_probe_device+0xb8/0x1cc
[cf81fec0] [c012c1d4] __driver_attach+0xcc/0xf8
[cf81fee0] [c012af8c] bus_for_each_dev+0x54/0x90
[cf81ff10] [c012bc14] driver_attach+0x24/0x34
[cf81ff20] [c012b64c] bus_add_driver+0x98/0x1b8
[cf81ff40] [c012c65c] driver_register+0x58/0xa0
[cf81ff50] [c012e190] platform_driver_register+0x98/0xa8
[cf81ff60] [c028c014] ndfc_nand_init+0x38/0x50
[cf81ff70] [c027486c] kernel_init+0xa4/0x290
[cf81fff0] [c000c320] kernel_thread+0x48/0x64

Instruction dump:
XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX ---[ end trace 99adfcc136bb75fe ]---

Sean MacLennan

author

2008-01-16 21:25 UTC #20 lore raw

In reply to Sean MacLennan

Quoted 4 lines · Sean MacLennan

How about adding a config option that lets you specify 8 bit access? Something like CONFIG_NDFC_8BIT_ACCESS. We could default it to no and put a little blurb that says something like:
On some platforms the 32bit read/writes cause a machine access exception. If you get a machine access exception while reading the NAND bad block table, try turning on 8 bit access.

I know it would be better if 32 bit access just worked, but nobody actively objected to this idea, so here is a patch ;)

Cheers,
Sean

Signed-off-by Sean MacLennan <[email protected]>

---

drivers/mtd/nand/Kconfig +0 -0

index 246d451..2809ded 100644
@@ -153,11 +153,21 @@ config MTD_NAND_S3C2410_HWECC

+config NDFC_8BIT_ACCESS
+ bool "NDFC 8-bit access only"
+ depends on MTD_NAND_NDFC
+ default n
+ help
+ The NDFC supports 32 bit read/writes to the NAND to improve
+ performance. On some platforms this does not work. If you get
+ a machine access exception while reading the bad block table,
+ try setting this to Y.
+
config MTD_NAND_S3C2410_CLKSTOP

	bool "S3C2410 NAND IDLE clock stop"
	depends on MTD_NAND_S3C2410

drivers/mtd/nand/ndfc.c +0 -0

index 1c0e89f..ac06099 100644
@@ -24,11 +24,13 @@

#include <linux/platform_device.h>

#include <asm/io.h>
+#ifndef CONFIG_PPC_MERGE
#ifdef CONFIG_40x
#include <asm/ibm405.h>
#else
#include <asm/ibm44x.h>
#endif
+#endif

struct ndfc_nand_mtd {
	struct mtd_info			mtd;
@@ -110,6 +112,37 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
	return 0;
}

+#ifdef CONFIG_NDFC_8BIT_ACCESS
+static void ndfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+ struct ndfc_controller *ndfc = &ndfc_ctrl;
+ uint8_t *p = (uint8_t *) buf;
+
+ for (; len > 0; len--)
+ *p++ = __raw_readb(ndfc->ndfcbase + NDFC_DATA);
+}
+
+static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+ struct ndfc_controller *ndfc = &ndfc_ctrl;
+ uint8_t *p = (uint8_t *) buf;
+
+ for (; len > 0; len--)
+ __raw_writeb(*p++, ndfc->ndfcbase + NDFC_DATA);
+}
+
+static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+ struct ndfc_controller *ndfc = &ndfc_ctrl;
+ uint8_t *p = (uint8_t *) buf;
+
+ for (; len > 0; len--)
+ if (*p++ != __raw_readb(ndfc->ndfcbase + NDFC_DATA))
+ return -EFAULT;
+
+ return 0;
+}
+#else
/*
* Speedups for buffer read/write/verify
*
@@ -145,6 +178,7 @@ static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)

			return -EFAULT;
	return 0;
}

+#endif

/*
* Initialize chip structure
@@ -236,6 +270,8 @@ static int ndfc_nand_probe(struct platform_device *pdev)

#ifndef CONFIG_PHYS_64BIT
	ndfc->ndfcbase = ioremap((phys_addr_t)phys, res->end - res->start + 1);
+#elif defined(CONFIG_PPC_MERGE)
+	ndfc->ndfcbase = ioremap(phys, res->end - res->start + 1);
#else
	ndfc->ndfcbase = ioremap64(phys, res->end - res->start + 1);

#endif

Josh Boyer

2008-01-16 23:36 UTC #21 lore raw

Quoted 11 lines · On Wed, 16 Jan 2008 16:25:23 -0500 Sean MacLennan <[email protected]>…

Quoted 8 lines · Sean MacLennan
How about adding a config option that lets you specify 8 bit access? Something like CONFIG_NDFC_8BIT_ACCESS. We could default it to no and put a little blurb that says something like:
On some platforms the 32bit read/writes cause a machine access exception. If you get a machine access exception while reading the NAND bad block table, try turning on 8 bit access.
I know it would be better if 32 bit access just worked, but nobody actively objected to this idea, so here is a patch ;)

I'm objecting, but not on the bit access reasons :).

1) Needs to go to the [email protected] list with the maintainer CC'd (Thomas Gleixner)

2) I don't want this driver enabled in mainline for boards in arch/powerpc until it fully understands how to parse device trees.

You could separate out the 8-bit access option and send that to linux-mtd though

josh

Sean MacLennan

author

2008-01-16 23:51 UTC #22 lore raw

Quoted 11 lines · Josh Boyer

Quoted 7 lines · On Wed, 16 Jan 2008 16:25:23 -0500 Sean MacLennan <[email protected]>…
Quoted 5 lines · Sean MacLennan
How about adding a config option that lets you specify 8 bit access? Something like CONFIG_NDFC_8BIT_ACCESS. We could default it to no and put a little blurb that says something like:
On some platforms the 32bit read/writes cause a machine access exception. If you get a machine access exception while reading the NAND bad block table, try turning on 8 bit access.

Quoted 8 lines

Quoted 2 lines
I know it would be better if 32 bit access just worked, but nobody actively objected to this idea, so here is a patch ;)
I'm objecting, but not on the bit access reasons :).
1) Needs to go to the [email protected] list with the maintainer CC'd (Thomas Gleixner)

Ok.

Quoted 3 lines

2) I don't want this driver enabled in mainline for boards in arch/powerpc until it fully understands how to parse device trees.

Fair enough. I thought it would be better to have a driver that compiles and is usable for arch/powerpc, even if not quite correct. We (PIKA) are going to be using a NAND flash, so we need the ndfc driver working. But I can understand wanting it "right" before you put it in the kernel.

Cheers,
Sean