Re: [PATCH] ndfc driver

Re: [PATCH] ndfc driver

[Josh Boyer]

On Wed, 3 Dec 2008 22:28:32 -0500
Sean MacLennan <smaclennan@pikatech.com> wrote:

Hi Sean,

A couple of comments/requests below.

> The current ndfc driver only compiles under arch/ppc. This arch was
> removed from the kernel. I notice the event entry for the ndfc in
> Kconfig has been removed in 2.6.28.
> 
> This patch converts the ndfc to a proper OF (OpenFirmware) driver. I
> can give a working example of the DTS if needed.

In addition to an example DTS patch (probably to warp itself), could
you briefly write up a binding and put it in
Documentation/powerpc/dts-bindings/amcc (or similar)?  Also please CC
the devicetree-discuss list on that part.

> The patch has been in production use on the PIKA Warp Appliance and is
> in use by others. The Warp basically boots from NAND, so the ndfc driver
> is very important to us.

Looking over the patch it seems pretty straight-forward and I don't see
anything immediately wrong with it.  You do have a number of
semi-unrelated changes to the actual port to of_platform though, like
the s/__raw_writel/out_be32 stuff, the addition of partition parsing,
etc.  I'm wondering if you could do those fixups separately from the
actual port.

Also, could you document why the data structures changed as they did in
the changelog or perhaps in a summary email.

You also seem to only support a single NAND chip, however the NDFC can
support multiple chips.  Have you looked at how the the fsl_elbc_nand
driver does multiple chip support?  If not, could you at least document
the limitation in the patch?

> This is a bi-monthly posting ;)

Keep at it Sean.  Your work is appreciated, even if the rest of us are
slow to review.

josh

Re: [PATCH] ndfc driver

[Sean MacLennan]

On Thu, 4 Dec 2008 09:01:07 -0500
"Josh Boyer" <jwboyer@linux.vnet.ibm.com> wrote:

> On Wed, 3 Dec 2008 22:28:32 -0500
> Sean MacLennan <smaclennan@pikatech.com> wrote:
> 
> Hi Sean,
> 
> A couple of comments/requests below.
> 
> In addition to an example DTS patch (probably to warp itself), could
> you briefly write up a binding and put it in
> Documentation/powerpc/dts-bindings/amcc (or similar)?  Also please CC
> the devicetree-discuss list on that part.

The DTS patch was a separate email to the linuxppc-dev list. I'll try to
find it.

I will look into writing up the binding.

> Looking over the patch it seems pretty straight-forward and I don't
> see anything immediately wrong with it.  You do have a number of
> semi-unrelated changes to the actual port to of_platform though, like
> the s/__raw_writel/out_be32 stuff, the addition of partition parsing,
> etc.  I'm wondering if you could do those fixups separately from the
> actual port.
> 
> Also, could you document why the data structures changed as they did
> in the changelog or perhaps in a summary email.

The __raw_writel changes where from feedback from this list. I will try
to find the email.

This patch originally goes back to January... so I have problems
remembering why all the changes where made ;) Believe it or not, we
have gone through 3 or 4 repositories since then. Finding history is
basically impossible :( So I have to try to rely on an email trail.

> You also seem to only support a single NAND chip, however the NDFC can
> support multiple chips.  Have you looked at how the the fsl_elbc_nand
> driver does multiple chip support?  If not, could you at least
> document the limitation in the patch?

I will document the limitation. I do not have access to a board with
multiple chips and I don't like submitting something I can't test.

Cheers,
   Sean

Re: [PATCH] ndfc driver

[Sean MacLennan]

On Thu, 4 Dec 2008 09:01:07 -0500
"Josh Boyer" <jwboyer@linux.vnet.ibm.com> wrote:

> In addition to an example DTS patch (probably to warp itself), could
> you briefly write up a binding and put it in
> Documentation/powerpc/dts-bindings/amcc (or similar)?  Also please CC
> the devicetree-discuss list on that part.

Here is a start at the doc. I have sent it as a patch, but could just
as easily send raw text.

The example comes from the warp dts, just with less partitions, so I
have not included a warp dts patch here.

Cheers,
   Sean

diff --git a/Documentation/powerpc/dts-bindings/amcc/ndfc.txt
b/Documentation/powerpc/dts-bindings/amcc/ndfc.txt new file mode 100644
index 0000000..668f4a9
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/amcc/ndfc.txt
@@ -0,0 +1,31 @@
+AMCC NDFC (NanD Flash Controller)
+
+Required properties:
+- compatible : "amcc,ndfc".
+- reg : should specify chip select and size used for the chip (0x2000).
+
+Optional properties:
+- ccr : NDFC config and control register value (default 0).
+- bank-settings : NDFC bank configuration register value (default 0).
+- partition(s) - follows the OF MTD standard for partitions
+
+Example:
+
+nand@1,0 {
+	compatible = "amcc,ndfc";
+	reg = <0x00000001 0x00000000 0x00002000>;
+	ccr = <0x00001000>;
+	bank-settings = <0x80002222>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	partition@0 {
+		label = "kernel";
+		reg = <0x00000000 0x00200000>;
+	};
+	partition@200000 {
+		label = "root";
+		reg = <0x00200000 0x03E00000>;
+	};
+};
+

Re: [PATCH] ndfc driver

[Anton Vorontsov]

Hi Sean,

On Mon, Dec 08, 2008 at 07:34:46PM -0500, Sean MacLennan wrote:
> On Thu, 4 Dec 2008 09:01:07 -0500
> "Josh Boyer" <jwboyer@linux.vnet.ibm.com> wrote:
> 
> > In addition to an example DTS patch (probably to warp itself), could
> > you briefly write up a binding and put it in
> > Documentation/powerpc/dts-bindings/amcc (or similar)?  Also please CC
> > the devicetree-discuss list on that part.
> 
> Here is a start at the doc. I have sent it as a patch, but could just
> as easily send raw text.
> 
> The example comes from the warp dts, just with less partitions, so I
> have not included a warp dts patch here.
> 
> Cheers,
>    Sean
> 
> diff --git a/Documentation/powerpc/dts-bindings/amcc/ndfc.txt
> b/Documentation/powerpc/dts-bindings/amcc/ndfc.txt new file mode 100644
> index 0000000..668f4a9
> --- /dev/null
> +++ b/Documentation/powerpc/dts-bindings/amcc/ndfc.txt
> @@ -0,0 +1,31 @@
> +AMCC NDFC (NanD Flash Controller)
> +
> +Required properties:
> +- compatible : "amcc,ndfc".
> +- reg : should specify chip select and size used for the chip (0x2000).
> +
> +Optional properties:
> +- ccr : NDFC config and control register value (default 0).
> +- bank-settings : NDFC bank configuration register value (default 0).
> +- partition(s) - follows the OF MTD standard for partitions
> +
> +Example:
> +
> +nand@1,0 {
> +	compatible = "amcc,ndfc";

The first line in this file says that this is a controller...

> +	reg = <0x00000001 0x00000000 0x00002000>;
> +	ccr = <0x00001000>;
> +	bank-settings = <0x80002222>;
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	partition@0 {

So this is a controller with partitions? ;-)

Recalling my own mistakes with the FSL UPM NAND controller bindings,
and Josh's comment:

http://www.mail-archive.com/linuxppc-dev@ozlabs.org/msg16572.html

I think the bindings should look like this:

nand-controller@.. {
	...controller's properties...

	nand-chip@... {
		...chip's properties...

		partition@... {
			...partition's properties...
		};
	};

	nand-chip@... {
		...
	};
};

Sure, I understand that there are plenty of boards with the "old"
scheme flashed into the firmware... Just something you might
want to consider for the future updates for the driver/bindings.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH] ndfc driver

[Sean MacLennan]

On Tue, 9 Dec 2008 05:11:15 +0300
"Anton Vorontsov" <avorontsov@ru.mvista.com> wrote:

> So this is a controller with partitions? ;-)

Actually, I did it this way to mimic the look of the NOR. Really, we
shouldn't care about the NAND chip.

Here is the complete NOR and NAND DTS:

nor@0,0 {
	compatible = "amd,s29gl032a", "cfi-flash";
	bank-width = <2>;
	reg = <0x00000000 0x00000000 0x00400000>;
	#address-cells = <1>;
	#size-cells = <1>;

	partition@0 {
		label = "splash";
		reg = <0x00000000 0x00020000>;
	};
	partition@300000 {
		label = "fpga";
		reg = <0x0300000 0x00040000>;
	};
	partition@340000 {
		label = "env";
		reg = <0x0340000 0x00040000>;
	};
	partition@380000 {
		label = "u-boot";
		reg = <0x0380000 0x00080000>;
	};
};

nand@1,0 {
	compatible = "amcc,ndfc";
	reg = <0x00000001 0x00000000 0x00002000>;
	ccr = <0x00001000>;
	bank-settings = <0x80002222>;
	#address-cells = <1>;
	#size-cells = <1>;

	partition@0 {
		label = "kernel";
		reg = <0x00000000 0x00200000>;
	};
	partition@200000 {
		label = "root";
		reg = <0x00200000 0x03E00000>;
	};
	partition@40000000 {
		label = "persistent";
		reg = <0x04000000 0x04000000>;
	};
	partition@80000000 {
		label = "persistent1";
		reg = <0x08000000 0x04000000>;
	};
	partition@C0000000 {
		label = "persistent2";
		reg = <0x0C000000 0x04000000>;
	};
};

Now I know I am cheating a bit.... but it does make it *look*
consistent.

But comments are welcome. I also could remove the partitions for now. A
partially supported NDFC is better than none at all.

Cheers,
   Sean

Re: [PATCH] ndfc driver

[Josh Boyer]

On Mon, 8 Dec 2008 21:45:12 -0500
Sean MacLennan <smaclennan@pikatech.com> wrote:

> On Tue, 9 Dec 2008 05:11:15 +0300
> "Anton Vorontsov" <avorontsov@ru.mvista.com> wrote:
> 
> > So this is a controller with partitions? ;-)
> 
> Actually, I did it this way to mimic the look of the NOR. Really, we
> shouldn't care about the NAND chip.

Except there is no controller in front of the NOR.  It's all just
MMIOs.  With NDFC, there is a controller, you have to do things to it
to talk to different chips, etc.

josh

Re: [PATCH] ndfc driver

[Sean MacLennan]

On Mon, 8 Dec 2008 22:32:27 -0500
"Josh Boyer" <jwboyer@linux.vnet.ibm.com> wrote:

> Except there is no controller in front of the NOR.  It's all just
> MMIOs.  With NDFC, there is a controller, you have to do things to it
> to talk to different chips, etc.

Ok, I have the following dts working... would this be better? It
basically follows the fsl,upm-nand model. I can produce a new patch to
ndfc.c for this:

ndfc@1,0 {
	compatible = "amcc,ndfc";
	reg = <0x00000001 0x00000000 0x00002000>;
	ccr = <0x00001000>;
	bank-settings = <0x80002222>;
	#address-cells = <1>;
	#size-cells = <1>;

	nand {
    		#address-cells = <1>;
    		#size-cells = <1>;

    		partition@0 {
    			label = "kernel";
    			reg = <0x00000000 0x00200000>;
    		};
    		partition@200000 {
    			label = "root";
    			reg = <0x00200000 0x03E00000>;
    		};	
    		partition@40000000 {
    			label = "persistent";
    			reg = <0x04000000 0x04000000>;
    		};
    		partition@80000000 {
    			label = "persistent1";
    			reg = <0x08000000 0x04000000>;
    		};
    		partition@C0000000 {
    			label = "persistent2";
    			reg = <0x0C000000 0x04000000>;
    		};
    	};
};


Here is the boot output for both the NOR and the NAND (just for
comparison):

ffc00000.nor: Found 1 x16 devices at 0x0 in 16-bit bank
 Amd/Fujitsu Extended Query Table at 0x0040
ffc00000.nor: CFI does not contain boot bank location. Assuming top.
number of CFI chips: 1
cfi_cmdset_0002: Disabling erase-suspend-program due to code brokenness.
RedBoot partition parsing not available
Creating 4 MTD partitions on "ffc00000.nor":
0x00000000-0x00020000 : "splash"
0x00300000-0x00340000 : "fpga"
0x00340000-0x00380000 : "env"
0x00380000-0x00400000 : "u-boot"
NAND device: Manufacturer ID: 0xec, Chip ID: 0xda (Samsung NAND 256MiB 3,3V 8-bit)
Scanning device for bad blocks
Creating 5 MTD partitions on "d0000000.ndfc.nand":
0x00000000-0x00200000 : "kernel"
0x00200000-0x04000000 : "root"
0x04000000-0x08000000 : "persistent"
0x08000000-0x0c000000 : "persistent1"
0x0c000000-0x10000000 : "persistent2"

If everybody likes this better, I can produce a code patch.

Cheers,
   Sean

Re: [PATCH] ndfc driver

[Mitch Bradley]

One address/size cell isn't enough for the next generation of NAND FLASH 
chips.

Re: [PATCH] ndfc driver

[Sean MacLennan]

On Mon, 08 Dec 2008 21:57:12 -1000
"Mitch Bradley" <wmb@firmworks.com> wrote:

> One address/size cell isn't enough for the next generation of NAND
> FLASH chips.
> 

I am no dts expert, but I thought I could put:

	nand {
		#address-cells = <1>;
		#size-cells = <1>;

in my dts and you could put:

	nand {
		#address-cells = <2>;
		#size-cells = <2>;

and, assuming we specified the reg entry right, everything would just
work. Is that assumption wrong?

And if the assumption is true, should I make a note in the doc that you
can make the address and size bigger?

Cheers,
   Sean

Re: [PATCH] ndfc driver

[Mitch Bradley]

>
> n Mon, 08 Dec 2008 21:57:12 -1000
> "Mitch Bradley" <wmb@firmworks.com> wrote:
>
>   
>> > One address/size cell isn't enough for the next generation of NAND
>> > FLASH chips.
>> > 
>>     
>
> I am no dts expert, but I thought I could put:
>
> 	nand {
> 		#address-cells = <1>;
> 		#size-cells = <1>;
>
> in my dts and you could put:
>
> 	nand {
> 		#address-cells = <2>;
> 		#size-cells = <2>;
>
> and, assuming we specified the reg entry right, everything would just
> work. Is that assumption wrong?
>
> And if the assumption is true, should I make a note in the doc that you
> can make the address and size bigger?
>
> Cheers,
>    Sean
>
>   


In principle that is correct, but the device tree partition parser in 
the Linux kernel assumes one address cell and one size cell, or at least 
it did the last time I looked.

I wrote a patch to fix that and circulated it on the linuxppc list, but 
since lost interest. OLPC (my main focus) is probably going to switch to 
managed NAND (SSD, LBA-NAND, eMMC, or some such thing with a built-in 
Flash Translation Layer) at some point.  Raw NAND is starting to go by 
the wayside.

Re: [PATCH] ndfc driver

[Stefan Roese]

On Tuesday 09 December 2008, Sean MacLennan wrote:
> On Thu, 4 Dec 2008 09:01:07 -0500
>
> "Josh Boyer" <jwboyer@linux.vnet.ibm.com> wrote:
> > In addition to an example DTS patch (probably to warp itself), could
> > you briefly write up a binding and put it in
> > Documentation/powerpc/dts-bindings/amcc (or similar)?  Also please CC
> > the devicetree-discuss list on that part.
>
> Here is a start at the doc. I have sent it as a patch, but could just
> as easily send raw text.
>
> The example comes from the warp dts, just with less partitions, so I
> have not included a warp dts patch here.
>
> Cheers,
>    Sean
>
> diff --git a/Documentation/powerpc/dts-bindings/amcc/ndfc.txt
> b/Documentation/powerpc/dts-bindings/amcc/ndfc.txt new file mode 100644
> index 0000000..668f4a9
> --- /dev/null
> +++ b/Documentation/powerpc/dts-bindings/amcc/ndfc.txt
> @@ -0,0 +1,31 @@
> +AMCC NDFC (NanD Flash Controller)
> +
> +Required properties:
> +- compatible : "amcc,ndfc".

The 4xx NAND controller was first implemented on the 440EP, IIRC. So I'm 
pretty sure that this controller is an IBM core and not am AMCC core. So this 
should be "ibm,ndfc".

And with this change it makes no sense to put this file "ndfc.txt" into the 
amcc directory.

Josh, where should this go then?

Best regards,
Stefan

Re: [PATCH] ndfc driver

[Josh Boyer]

On Tue, 9 Dec 2008 07:10:27 +0100
Stefan Roese <sr@denx.de> wrote:

> On Tuesday 09 December 2008, Sean MacLennan wrote:
> > On Thu, 4 Dec 2008 09:01:07 -0500
> >
> > "Josh Boyer" <jwboyer@linux.vnet.ibm.com> wrote:
> > > In addition to an example DTS patch (probably to warp itself), could
> > > you briefly write up a binding and put it in
> > > Documentation/powerpc/dts-bindings/amcc (or similar)?  Also please CC
> > > the devicetree-discuss list on that part.
> >
> > Here is a start at the doc. I have sent it as a patch, but could just
> > as easily send raw text.
> >
> > The example comes from the warp dts, just with less partitions, so I
> > have not included a warp dts patch here.
> >
> > Cheers,
> >    Sean
> >
> > diff --git a/Documentation/powerpc/dts-bindings/amcc/ndfc.txt
> > b/Documentation/powerpc/dts-bindings/amcc/ndfc.txt new file mode 100644
> > index 0000000..668f4a9
> > --- /dev/null
> > +++ b/Documentation/powerpc/dts-bindings/amcc/ndfc.txt
> > @@ -0,0 +1,31 @@
> > +AMCC NDFC (NanD Flash Controller)
> > +
> > +Required properties:
> > +- compatible : "amcc,ndfc".
> 
> The 4xx NAND controller was first implemented on the 440EP, IIRC. So I'm 
> pretty sure that this controller is an IBM core and not am AMCC core. So this 
> should be "ibm,ndfc".

That is true.  It's an IBM blue logic core.
 
> And with this change it makes no sense to put this file "ndfc.txt" into the 
> amcc directory.
> 
> Josh, where should this go then?

I declare it to be: dts-bindings/4xx/

mostly because I don't want the bindings scattered across two
directories simply because of the timeframe they showed up in the
marketplace.

If there are better ideas, I'm all ears.

josh

Re: [PATCH] ndfc driver

[Sean MacLennan]

Here is an updated patch. Doc has been moved to 4xx and amcc changed to
ibm.

Arnd: I removed your acked-by just in case you don't approve of the new
flash layout. Feel free to ack again.

Cheers,
   Sean

Port of the ndfc driver to an OF platform driver.

Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
diff --git a/Documentation/powerpc/dts-bindings/4xx/ndfc.txt b/Documentation/powerpc/dts-bindings/4xx/ndfc.txt
new file mode 100644
index 0000000..869f0b5
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/4xx/ndfc.txt
@@ -0,0 +1,39 @@
+AMCC NDFC (NanD Flash Controller)
+
+Required properties:
+- compatible : "ibm,ndfc".
+- reg : should specify chip select and size used for the chip (0x2000).
+
+Optional properties:
+- ccr : NDFC config and control register value (default 0).
+- bank-settings : NDFC bank configuration register value (default 0).
+
+Notes:
+- partition(s) - follows the OF MTD standard for partitions
+
+Example:
+
+ndfc@1,0 {
+	compatible = "ibm,ndfc";
+	reg = <0x00000001 0x00000000 0x00002000>;
+	ccr = <0x00001000>;
+	bank-settings = <0x80002222>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	nand {
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		partition@0 {
+			label = "kernel";
+			reg = <0x00000000 0x00200000>;
+		};
+		partition@200000 {
+			label = "root";
+			reg = <0x00200000 0x03E00000>;
+		};
+	};
+};
+
+
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 1c2e945..5705f85 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -163,6 +163,13 @@ config MTD_NAND_S3C2410_HWECC
 	  incorrect ECC generation, and if using these, the default of
 	  software ECC is preferable.
 
+config MTD_NAND_NDFC
+	tristate "NDFC NanD Flash Controller"
+	depends on 4xx
+	select MTD_NAND_ECC_SMC
+	help
+	 NDFC Nand Flash Controllers are integrated in IBM/AMCC's 4xx SoCs
+
 config MTD_NAND_S3C2410_CLKSTOP
 	bool "S3C2410 NAND IDLE clock stop"
 	depends on MTD_NAND_S3C2410
diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
index 955959e..582cf80 100644
--- a/drivers/mtd/nand/ndfc.c
+++ b/drivers/mtd/nand/ndfc.c
@@ -2,12 +2,20 @@
  *  drivers/mtd/ndfc.c
  *
  *  Overview:
- *   Platform independend driver for NDFC (NanD Flash Controller)
+ *   Platform independent driver for NDFC (NanD Flash Controller)
  *   integrated into EP440 cores
  *
+ *   Ported to an OF platform driver by Sean MacLennan
+ *
+ *   The NDFC supports multiple chips, but this driver only supports a
+ *   single chip since I do not have access to any boards with
+ *   multiple chips.
+ *
  *  Author: Thomas Gleixner
  *
  *  Copyright 2006 IBM
+ *  Copyright 2008 PIKA Technologies
+ *    Sean MacLennan <smaclennan@pikatech.com>
  *
  *  This program is free software; you can redistribute	 it and/or modify it
  *  under  the terms of	 the GNU General  Public License as published by the
@@ -21,27 +29,20 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/ndfc.h>
 #include <linux/mtd/mtd.h>
-#include <linux/platform_device.h>
-
+#include <linux/of_platform.h>
 #include <asm/io.h>
-#ifdef CONFIG_40x
-#include <asm/ibm405.h>
-#else
-#include <asm/ibm44x.h>
-#endif
-
-struct ndfc_nand_mtd {
-	struct mtd_info			mtd;
-	struct nand_chip		chip;
-	struct platform_nand_chip	*pl_chip;
-};
 
-static struct ndfc_nand_mtd ndfc_mtd[NDFC_MAX_BANKS];
 
 struct ndfc_controller {
-	void __iomem		*ndfcbase;
-	struct nand_hw_control	ndfc_control;
-	atomic_t		childs_active;
+	struct of_device *ofdev;
+	void __iomem *ndfcbase;
+	struct mtd_info mtd;
+	struct nand_chip chip;
+	int chip_select;
+	struct nand_hw_control ndfc_control;
+#ifdef CONFIG_MTD_PARTITIONS
+	struct mtd_partition *parts;
+#endif
 };
 
 static struct ndfc_controller ndfc_ctrl;
@@ -50,17 +51,14 @@ static void ndfc_select_chip(struct mtd_info *mtd, int chip)
 {
 	uint32_t ccr;
 	struct ndfc_controller *ndfc = &ndfc_ctrl;
-	struct nand_chip *nandchip = mtd->priv;
-	struct ndfc_nand_mtd *nandmtd = nandchip->priv;
-	struct platform_nand_chip *pchip = nandmtd->pl_chip;
 
-	ccr = __raw_readl(ndfc->ndfcbase + NDFC_CCR);
+	ccr = in_be32(ndfc->ndfcbase + NDFC_CCR);
 	if (chip >= 0) {
 		ccr &= ~NDFC_CCR_BS_MASK;
-		ccr |= NDFC_CCR_BS(chip + pchip->chip_offset);
+		ccr |= NDFC_CCR_BS(chip + ndfc->chip_select);
 	} else
 		ccr |= NDFC_CCR_RESET_CE;
-	__raw_writel(ccr, ndfc->ndfcbase + NDFC_CCR);
+	out_be32(ndfc->ndfcbase + NDFC_CCR, ccr);
 }
 
 static void ndfc_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
@@ -80,7 +78,7 @@ static int ndfc_ready(struct mtd_info *mtd)
 {
 	struct ndfc_controller *ndfc = &ndfc_ctrl;
 
-	return __raw_readl(ndfc->ndfcbase + NDFC_STAT) & NDFC_STAT_IS_READY;
+	return in_be32(ndfc->ndfcbase + NDFC_STAT) & NDFC_STAT_IS_READY;
 }
 
 static void ndfc_enable_hwecc(struct mtd_info *mtd, int mode)
@@ -88,9 +86,9 @@ static void ndfc_enable_hwecc(struct mtd_info *mtd, int mode)
 	uint32_t ccr;
 	struct ndfc_controller *ndfc = &ndfc_ctrl;
 
-	ccr = __raw_readl(ndfc->ndfcbase + NDFC_CCR);
+	ccr = in_be32(ndfc->ndfcbase + NDFC_CCR);
 	ccr |= NDFC_CCR_RESET_ECC;
-	__raw_writel(ccr, ndfc->ndfcbase + NDFC_CCR);
+	out_be32(ndfc->ndfcbase + NDFC_CCR, ccr);
 	wmb();
 }
 
@@ -102,9 +100,10 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
 	uint8_t *p = (uint8_t *)&ecc;
 
 	wmb();
-	ecc = __raw_readl(ndfc->ndfcbase + NDFC_ECC);
-	ecc_code[0] = p[1];
-	ecc_code[1] = p[2];
+	ecc = in_be32(ndfc->ndfcbase + NDFC_ECC);
+	/* The NDFC uses Smart Media (SMC) bytes order */
+	ecc_code[0] = p[2];
+	ecc_code[1] = p[1];
 	ecc_code[2] = p[3];
 
 	return 0;
@@ -123,7 +122,7 @@ static void ndfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 	uint32_t *p = (uint32_t *) buf;
 
 	for(;len > 0; len -= 4)
-		*p++ = __raw_readl(ndfc->ndfcbase + NDFC_DATA);
+		*p++ = in_be32(ndfc->ndfcbase + NDFC_DATA);
 }
 
 static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
@@ -132,7 +131,7 @@ static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 	uint32_t *p = (uint32_t *) buf;
 
 	for(;len > 0; len -= 4)
-		__raw_writel(*p++, ndfc->ndfcbase + NDFC_DATA);
+		out_be32(ndfc->ndfcbase + NDFC_DATA, *p++);
 }
 
 static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
@@ -141,7 +140,7 @@ static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 	uint32_t *p = (uint32_t *) buf;
 
 	for(;len > 0; len -= 4)
-		if (*p++ != __raw_readl(ndfc->ndfcbase + NDFC_DATA))
+		if (*p++ != in_be32(ndfc->ndfcbase + NDFC_DATA))
 			return -EFAULT;
 	return 0;
 }
@@ -149,10 +148,19 @@ static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 /*
  * Initialize chip structure
  */
-static void ndfc_chip_init(struct ndfc_nand_mtd *mtd)
+static int ndfc_chip_init(struct ndfc_controller *ndfc,
+			  struct device_node *node)
 {
-	struct ndfc_controller *ndfc = &ndfc_ctrl;
-	struct nand_chip *chip = &mtd->chip;
+#ifdef CONFIG_MTD_PARTITIONS
+#ifdef CONFIG_MTD_CMDLINE_PARTS
+	static const char *part_types[] = { "cmdlinepart", NULL };
+#else
+	static const char *part_types[] = { NULL };
+#endif
+#endif
+	struct device_node *flash_np;
+	struct nand_chip *chip = &ndfc->chip;
+	int ret;
 
 	chip->IO_ADDR_R = ndfc->ndfcbase + NDFC_DATA;
 	chip->IO_ADDR_W = ndfc->ndfcbase + NDFC_DATA;
@@ -160,8 +168,6 @@ static void ndfc_chip_init(struct ndfc_nand_mtd *mtd)
 	chip->dev_ready = ndfc_ready;
 	chip->select_chip = ndfc_select_chip;
 	chip->chip_delay = 50;
-	chip->priv = mtd;
-	chip->options = mtd->pl_chip->options;
 	chip->controller = &ndfc->ndfc_control;
 	chip->read_buf = ndfc_read_buf;
 	chip->write_buf = ndfc_write_buf;
@@ -172,143 +178,136 @@ static void ndfc_chip_init(struct ndfc_nand_mtd *mtd)
 	chip->ecc.mode = NAND_ECC_HW;
 	chip->ecc.size = 256;
 	chip->ecc.bytes = 3;
-	chip->ecclayout = chip->ecc.layout = mtd->pl_chip->ecclayout;
-	mtd->mtd.priv = chip;
-	mtd->mtd.owner = THIS_MODULE;
-}
-
-static int ndfc_chip_probe(struct platform_device *pdev)
-{
-	struct platform_nand_chip *nc = pdev->dev.platform_data;
-	struct ndfc_chip_settings *settings = nc->priv;
-	struct ndfc_controller *ndfc = &ndfc_ctrl;
-	struct ndfc_nand_mtd *nandmtd;
-
-	if (nc->chip_offset >= NDFC_MAX_BANKS || nc->nr_chips > NDFC_MAX_BANKS)
-		return -EINVAL;
-
-	/* Set the bank settings */
-	__raw_writel(settings->bank_settings,
-		     ndfc->ndfcbase + NDFC_BCFG0 + (nc->chip_offset << 2));
 
-	nandmtd = &ndfc_mtd[pdev->id];
-	if (nandmtd->pl_chip)
-		return -EBUSY;
+	ndfc->mtd.priv = chip;
+	ndfc->mtd.owner = THIS_MODULE;
 
-	nandmtd->pl_chip = nc;
-	ndfc_chip_init(nandmtd);
-
-	/* Scan for chips */
-	if (nand_scan(&nandmtd->mtd, nc->nr_chips)) {
-		nandmtd->pl_chip = NULL;
+	flash_np = of_get_next_child(node, NULL);
+	if (!flash_np)
 		return -ENODEV;
+
+	ndfc->mtd.name = kasprintf(GFP_KERNEL, "%s.%s",
+				   ndfc->ofdev->dev.bus_id, flash_np->name);
+	if (!ndfc->mtd.name) {
+		ret = -ENOMEM;
+		goto err;
 	}
 
-#ifdef CONFIG_MTD_PARTITIONS
-	printk("Number of partitions %d\n", nc->nr_partitions);
-	if (nc->nr_partitions) {
-		/* Add the full device, so complete dumps can be made */
-		add_mtd_device(&nandmtd->mtd);
-		add_mtd_partitions(&nandmtd->mtd, nc->partitions,
-				   nc->nr_partitions);
+	ret = nand_scan(&ndfc->mtd, 1);
+	if (ret)
+		goto err;
 
-	} else
-#else
-		add_mtd_device(&nandmtd->mtd);
+#ifdef CONFIG_MTD_PARTITIONS
+	ret = parse_mtd_partitions(&ndfc->mtd, part_types, &ndfc->parts, 0);
+	if (ret < 0)
+		goto err;
+
+#ifdef CONFIG_MTD_OF_PARTS
+	if (ret == 0) {
+		ret = of_mtd_parse_partitions(&ndfc->ofdev->dev, flash_np,
+					      &ndfc->parts);
+		if (ret < 0)
+			goto err;
+	}
 #endif
 
-	atomic_inc(&ndfc->childs_active);
-	return 0;
-}
+	if (ret > 0)
+		ret = add_mtd_partitions(&ndfc->mtd, ndfc->parts, ret);
+	else
+#endif
+		ret = add_mtd_device(&ndfc->mtd);
 
-static int ndfc_chip_remove(struct platform_device *pdev)
-{
-	return 0;
+err:
+	of_node_put(flash_np);
+	if (ret)
+		kfree(ndfc->mtd.name);
+	return ret;
 }
 
-static int ndfc_nand_probe(struct platform_device *pdev)
+static int __devinit ndfc_probe(struct of_device *ofdev,
+				const struct of_device_id *match)
 {
-	struct platform_nand_ctrl *nc = pdev->dev.platform_data;
-	struct ndfc_controller_settings *settings = nc->priv;
-	struct resource *res = pdev->resource;
 	struct ndfc_controller *ndfc = &ndfc_ctrl;
-	unsigned long long phys = settings->ndfc_erpn | res->start;
+	const u32 *reg;
+	u32 ccr;
+	int err, len;
 
-#ifndef CONFIG_PHYS_64BIT
-	ndfc->ndfcbase = ioremap((phys_addr_t)phys, res->end - res->start + 1);
-#else
-	ndfc->ndfcbase = ioremap64(phys, res->end - res->start + 1);
-#endif
+	spin_lock_init(&ndfc->ndfc_control.lock);
+	init_waitqueue_head(&ndfc->ndfc_control.wq);
+	ndfc->ofdev = ofdev;
+	dev_set_drvdata(&ofdev->dev, ndfc);
+
+	/* Read the reg property to get the chip select */
+	reg = of_get_property(ofdev->node, "reg", &len);
+	if (reg == NULL || len != 12) {
+		dev_err(&ofdev->dev, "unable read reg property (%d)\n", len);
+		return -ENOENT;
+	}
+	ndfc->chip_select = reg[0];
+
+	ndfc->ndfcbase = of_iomap(ofdev->node, 0);
 	if (!ndfc->ndfcbase) {
-		printk(KERN_ERR "NDFC: ioremap failed\n");
+		dev_err(&ofdev->dev, "failed to get memory\n");
 		return -EIO;
 	}
 
-	__raw_writel(settings->ccr_settings, ndfc->ndfcbase + NDFC_CCR);
+	ccr = NDFC_CCR_BS(ndfc->chip_select);
 
-	spin_lock_init(&ndfc->ndfc_control.lock);
-	init_waitqueue_head(&ndfc->ndfc_control.wq);
+	/* It is ok if ccr does not exist - just default to 0 */
+	reg = of_get_property(ofdev->node, "ccr", NULL);
+	if (reg)
+		ccr |= *reg;
 
-	platform_set_drvdata(pdev, ndfc);
+	out_be32(ndfc->ndfcbase + NDFC_CCR, ccr);
 
-	printk("NDFC NAND Driver initialized. Chip-Rev: 0x%08x\n",
-	       __raw_readl(ndfc->ndfcbase + NDFC_REVID));
+	/* Set the bank settings if given */
+	reg = of_get_property(ofdev->node, "bank-settings", NULL);
+	if (reg) {
+		int offset = NDFC_BCFG0 + (ndfc->chip_select << 2);
+		out_be32(ndfc->ndfcbase + offset, *reg);
+	}
+
+	err = ndfc_chip_init(ndfc, ofdev->node);
+	if (err) {
+		iounmap(ndfc->ndfcbase);
+		return err;
+	}
 
 	return 0;
 }
 
-static int ndfc_nand_remove(struct platform_device *pdev)
+static int __devexit ndfc_remove(struct of_device *ofdev)
 {
-	struct ndfc_controller *ndfc = platform_get_drvdata(pdev);
+	struct ndfc_controller *ndfc = dev_get_drvdata(&ofdev->dev);
 
-	if (atomic_read(&ndfc->childs_active))
-		return -EBUSY;
+	nand_release(&ndfc->mtd);
 
-	if (ndfc) {
-		platform_set_drvdata(pdev, NULL);
-		iounmap(ndfc_ctrl.ndfcbase);
-		ndfc_ctrl.ndfcbase = NULL;
-	}
 	return 0;
 }
 
-/* driver device registration */
-
-static struct platform_driver ndfc_chip_driver = {
-	.probe		= ndfc_chip_probe,
-	.remove		= ndfc_chip_remove,
-	.driver		= {
-		.name	= "ndfc-chip",
-		.owner	= THIS_MODULE,
-	},
+static const struct of_device_id ndfc_match[] = {
+	{ .compatible = "ibm,ndfc", },
+	{}
 };
+MODULE_DEVICE_TABLE(of, ndfc_match);
 
-static struct platform_driver ndfc_nand_driver = {
-	.probe		= ndfc_nand_probe,
-	.remove		= ndfc_nand_remove,
-	.driver		= {
-		.name	= "ndfc-nand",
-		.owner	= THIS_MODULE,
+static struct of_platform_driver ndfc_driver = {
+	.driver = {
+		.name	= "ndfc",
 	},
+	.match_table = ndfc_match,
+	.probe = ndfc_probe,
+	.remove = __devexit_p(ndfc_remove),
 };
 
 static int __init ndfc_nand_init(void)
 {
-	int ret;
-
-	spin_lock_init(&ndfc_ctrl.ndfc_control.lock);
-	init_waitqueue_head(&ndfc_ctrl.ndfc_control.wq);
-
-	ret = platform_driver_register(&ndfc_nand_driver);
-	if (!ret)
-		ret = platform_driver_register(&ndfc_chip_driver);
-	return ret;
+	return of_register_platform_driver(&ndfc_driver);
 }
 
 static void __exit ndfc_nand_exit(void)
 {
-	platform_driver_unregister(&ndfc_chip_driver);
-	platform_driver_unregister(&ndfc_nand_driver);
+	of_unregister_platform_driver(&ndfc_driver);
 }
 
 module_init(ndfc_nand_init);
@@ -316,6 +315,4 @@ module_exit(ndfc_nand_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Thomas Gleixner <tglx@linutronix.de>");
-MODULE_DESCRIPTION("Platform driver for NDFC");
-MODULE_ALIAS("platform:ndfc-chip");
-MODULE_ALIAS("platform:ndfc-nand");
+MODULE_DESCRIPTION("OF Platform driver for NDFC");

Re: [PATCH] ndfc driver

[Sean MacLennan]

On Wed, 10 Dec 2008 18:16:34 -0500
"Sean MacLennan" <sean.maclennan@ottawa.kanatek.ca> wrote:

> Here is an updated patch. Doc has been moved to 4xx and amcc changed
> to ibm.

Anybody? Even if it is not perfect, it would be better to have
a driver that at least compiles ;)

Cheers,
   Sean

Re: [PATCH] ndfc driver

[Josh Boyer]

On Tue, Dec 16, 2008 at 11:14:00PM -0500, Sean MacLennan wrote:
>On Wed, 10 Dec 2008 18:16:34 -0500
>"Sean MacLennan" <sean.maclennan@ottawa.kanatek.ca> wrote:
>
>> Here is an updated patch. Doc has been moved to 4xx and amcc changed
>> to ibm.
>
>Anybody? Even if it is not perfect, it would be better to have
>a driver that at least compiles ;)

Reviewing it today.  Looked pretty good at first glance.  It would be
nice to get David or Thomas to Ack it.

josh

Re: [PATCH] ndfc driver

[Josh Boyer]

On Wed, Dec 10, 2008 at 06:16:34PM -0500, Sean MacLennan wrote:
>Here is an updated patch. Doc has been moved to 4xx and amcc changed to
>ibm.
>
>Arnd: I removed your acked-by just in case you don't approve of the new
>flash layout. Feel free to ack again.
>
>Cheers,
>   Sean
>
>Port of the ndfc driver to an OF platform driver.
>
>Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>

Acked-By: Josh Boyer <jwboyer@linux.vnet.ibm.com>

David, if you have no objections I'd like to take this patch
through my tree.  I'll fixup the subject and commit log when
I do.

josh

>---
>diff --git a/Documentation/powerpc/dts-bindings/4xx/ndfc.txt b/Documentation/powerpc/dts-bindings/4xx/ndfc.txt
>new file mode 100644
>index 0000000..869f0b5
>--- /dev/null
>+++ b/Documentation/powerpc/dts-bindings/4xx/ndfc.txt
>@@ -0,0 +1,39 @@
>+AMCC NDFC (NanD Flash Controller)
>+
>+Required properties:
>+- compatible : "ibm,ndfc".
>+- reg : should specify chip select and size used for the chip (0x2000).
>+
>+Optional properties:
>+- ccr : NDFC config and control register value (default 0).
>+- bank-settings : NDFC bank configuration register value (default 0).
>+
>+Notes:
>+- partition(s) - follows the OF MTD standard for partitions
>+
>+Example:
>+
>+ndfc@1,0 {
>+	compatible = "ibm,ndfc";
>+	reg = <0x00000001 0x00000000 0x00002000>;
>+	ccr = <0x00001000>;
>+	bank-settings = <0x80002222>;
>+	#address-cells = <1>;
>+	#size-cells = <1>;
>+
>+	nand {
>+		#address-cells = <1>;
>+		#size-cells = <1>;
>+
>+		partition@0 {
>+			label = "kernel";
>+			reg = <0x00000000 0x00200000>;
>+		};
>+		partition@200000 {
>+			label = "root";
>+			reg = <0x00200000 0x03E00000>;
>+		};
>+	};
>+};
>+
>+
>diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
>index 1c2e945..5705f85 100644
>--- a/drivers/mtd/nand/Kconfig
>+++ b/drivers/mtd/nand/Kconfig
>@@ -163,6 +163,13 @@ config MTD_NAND_S3C2410_HWECC
> 	  incorrect ECC generation, and if using these, the default of
> 	  software ECC is preferable.
>
>+config MTD_NAND_NDFC
>+	tristate "NDFC NanD Flash Controller"
>+	depends on 4xx
>+	select MTD_NAND_ECC_SMC
>+	help
>+	 NDFC Nand Flash Controllers are integrated in IBM/AMCC's 4xx SoCs
>+
> config MTD_NAND_S3C2410_CLKSTOP
> 	bool "S3C2410 NAND IDLE clock stop"
> 	depends on MTD_NAND_S3C2410
>diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
>index 955959e..582cf80 100644
>--- a/drivers/mtd/nand/ndfc.c
>+++ b/drivers/mtd/nand/ndfc.c
>@@ -2,12 +2,20 @@
>  *  drivers/mtd/ndfc.c
>  *
>  *  Overview:
>- *   Platform independend driver for NDFC (NanD Flash Controller)
>+ *   Platform independent driver for NDFC (NanD Flash Controller)
>  *   integrated into EP440 cores
>  *
>+ *   Ported to an OF platform driver by Sean MacLennan
>+ *
>+ *   The NDFC supports multiple chips, but this driver only supports a
>+ *   single chip since I do not have access to any boards with
>+ *   multiple chips.
>+ *
>  *  Author: Thomas Gleixner
>  *
>  *  Copyright 2006 IBM
>+ *  Copyright 2008 PIKA Technologies
>+ *    Sean MacLennan <smaclennan@pikatech.com>
>  *
>  *  This program is free software; you can redistribute	 it and/or modify it
>  *  under  the terms of	 the GNU General  Public License as published by the
>@@ -21,27 +29,20 @@
> #include <linux/mtd/partitions.h>
> #include <linux/mtd/ndfc.h>
> #include <linux/mtd/mtd.h>
>-#include <linux/platform_device.h>
>-
>+#include <linux/of_platform.h>
> #include <asm/io.h>
>-#ifdef CONFIG_40x
>-#include <asm/ibm405.h>
>-#else
>-#include <asm/ibm44x.h>
>-#endif
>-
>-struct ndfc_nand_mtd {
>-	struct mtd_info			mtd;
>-	struct nand_chip		chip;
>-	struct platform_nand_chip	*pl_chip;
>-};
>
>-static struct ndfc_nand_mtd ndfc_mtd[NDFC_MAX_BANKS];
>
> struct ndfc_controller {
>-	void __iomem		*ndfcbase;
>-	struct nand_hw_control	ndfc_control;
>-	atomic_t		childs_active;
>+	struct of_device *ofdev;
>+	void __iomem *ndfcbase;
>+	struct mtd_info mtd;
>+	struct nand_chip chip;
>+	int chip_select;
>+	struct nand_hw_control ndfc_control;
>+#ifdef CONFIG_MTD_PARTITIONS
>+	struct mtd_partition *parts;
>+#endif
> };
>
> static struct ndfc_controller ndfc_ctrl;
>@@ -50,17 +51,14 @@ static void ndfc_select_chip(struct mtd_info *mtd, int chip)
> {
> 	uint32_t ccr;
> 	struct ndfc_controller *ndfc = &ndfc_ctrl;
>-	struct nand_chip *nandchip = mtd->priv;
>-	struct ndfc_nand_mtd *nandmtd = nandchip->priv;
>-	struct platform_nand_chip *pchip = nandmtd->pl_chip;
>
>-	ccr = __raw_readl(ndfc->ndfcbase + NDFC_CCR);
>+	ccr = in_be32(ndfc->ndfcbase + NDFC_CCR);
> 	if (chip >= 0) {
> 		ccr &= ~NDFC_CCR_BS_MASK;
>-		ccr |= NDFC_CCR_BS(chip + pchip->chip_offset);
>+		ccr |= NDFC_CCR_BS(chip + ndfc->chip_select);
> 	} else
> 		ccr |= NDFC_CCR_RESET_CE;
>-	__raw_writel(ccr, ndfc->ndfcbase + NDFC_CCR);
>+	out_be32(ndfc->ndfcbase + NDFC_CCR, ccr);
> }
>
> static void ndfc_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
>@@ -80,7 +78,7 @@ static int ndfc_ready(struct mtd_info *mtd)
> {
> 	struct ndfc_controller *ndfc = &ndfc_ctrl;
>
>-	return __raw_readl(ndfc->ndfcbase + NDFC_STAT) & NDFC_STAT_IS_READY;
>+	return in_be32(ndfc->ndfcbase + NDFC_STAT) & NDFC_STAT_IS_READY;
> }
>
> static void ndfc_enable_hwecc(struct mtd_info *mtd, int mode)
>@@ -88,9 +86,9 @@ static void ndfc_enable_hwecc(struct mtd_info *mtd, int mode)
> 	uint32_t ccr;
> 	struct ndfc_controller *ndfc = &ndfc_ctrl;
>
>-	ccr = __raw_readl(ndfc->ndfcbase + NDFC_CCR);
>+	ccr = in_be32(ndfc->ndfcbase + NDFC_CCR);
> 	ccr |= NDFC_CCR_RESET_ECC;
>-	__raw_writel(ccr, ndfc->ndfcbase + NDFC_CCR);
>+	out_be32(ndfc->ndfcbase + NDFC_CCR, ccr);
> 	wmb();
> }
>
>@@ -102,9 +100,10 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
> 	uint8_t *p = (uint8_t *)&ecc;
>
> 	wmb();
>-	ecc = __raw_readl(ndfc->ndfcbase + NDFC_ECC);
>-	ecc_code[0] = p[1];
>-	ecc_code[1] = p[2];
>+	ecc = in_be32(ndfc->ndfcbase + NDFC_ECC);
>+	/* The NDFC uses Smart Media (SMC) bytes order */
>+	ecc_code[0] = p[2];
>+	ecc_code[1] = p[1];
> 	ecc_code[2] = p[3];
>
> 	return 0;
>@@ -123,7 +122,7 @@ static void ndfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> 	uint32_t *p = (uint32_t *) buf;
>
> 	for(;len > 0; len -= 4)
>-		*p++ = __raw_readl(ndfc->ndfcbase + NDFC_DATA);
>+		*p++ = in_be32(ndfc->ndfcbase + NDFC_DATA);
> }
>
> static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
>@@ -132,7 +131,7 @@ static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> 	uint32_t *p = (uint32_t *) buf;
>
> 	for(;len > 0; len -= 4)
>-		__raw_writel(*p++, ndfc->ndfcbase + NDFC_DATA);
>+		out_be32(ndfc->ndfcbase + NDFC_DATA, *p++);
> }
>
> static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
>@@ -141,7 +140,7 @@ static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> 	uint32_t *p = (uint32_t *) buf;
>
> 	for(;len > 0; len -= 4)
>-		if (*p++ != __raw_readl(ndfc->ndfcbase + NDFC_DATA))
>+		if (*p++ != in_be32(ndfc->ndfcbase + NDFC_DATA))
> 			return -EFAULT;
> 	return 0;
> }
>@@ -149,10 +148,19 @@ static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
> /*
>  * Initialize chip structure
>  */
>-static void ndfc_chip_init(struct ndfc_nand_mtd *mtd)
>+static int ndfc_chip_init(struct ndfc_controller *ndfc,
>+			  struct device_node *node)
> {
>-	struct ndfc_controller *ndfc = &ndfc_ctrl;
>-	struct nand_chip *chip = &mtd->chip;
>+#ifdef CONFIG_MTD_PARTITIONS
>+#ifdef CONFIG_MTD_CMDLINE_PARTS
>+	static const char *part_types[] = { "cmdlinepart", NULL };
>+#else
>+	static const char *part_types[] = { NULL };
>+#endif
>+#endif
>+	struct device_node *flash_np;
>+	struct nand_chip *chip = &ndfc->chip;
>+	int ret;
>
> 	chip->IO_ADDR_R = ndfc->ndfcbase + NDFC_DATA;
> 	chip->IO_ADDR_W = ndfc->ndfcbase + NDFC_DATA;
>@@ -160,8 +168,6 @@ static void ndfc_chip_init(struct ndfc_nand_mtd *mtd)
> 	chip->dev_ready = ndfc_ready;
> 	chip->select_chip = ndfc_select_chip;
> 	chip->chip_delay = 50;
>-	chip->priv = mtd;
>-	chip->options = mtd->pl_chip->options;
> 	chip->controller = &ndfc->ndfc_control;
> 	chip->read_buf = ndfc_read_buf;
> 	chip->write_buf = ndfc_write_buf;
>@@ -172,143 +178,136 @@ static void ndfc_chip_init(struct ndfc_nand_mtd *mtd)
> 	chip->ecc.mode = NAND_ECC_HW;
> 	chip->ecc.size = 256;
> 	chip->ecc.bytes = 3;
>-	chip->ecclayout = chip->ecc.layout = mtd->pl_chip->ecclayout;
>-	mtd->mtd.priv = chip;
>-	mtd->mtd.owner = THIS_MODULE;
>-}
>-
>-static int ndfc_chip_probe(struct platform_device *pdev)
>-{
>-	struct platform_nand_chip *nc = pdev->dev.platform_data;
>-	struct ndfc_chip_settings *settings = nc->priv;
>-	struct ndfc_controller *ndfc = &ndfc_ctrl;
>-	struct ndfc_nand_mtd *nandmtd;
>-
>-	if (nc->chip_offset >= NDFC_MAX_BANKS || nc->nr_chips > NDFC_MAX_BANKS)
>-		return -EINVAL;
>-
>-	/* Set the bank settings */
>-	__raw_writel(settings->bank_settings,
>-		     ndfc->ndfcbase + NDFC_BCFG0 + (nc->chip_offset << 2));
>
>-	nandmtd = &ndfc_mtd[pdev->id];
>-	if (nandmtd->pl_chip)
>-		return -EBUSY;
>+	ndfc->mtd.priv = chip;
>+	ndfc->mtd.owner = THIS_MODULE;
>
>-	nandmtd->pl_chip = nc;
>-	ndfc_chip_init(nandmtd);
>-
>-	/* Scan for chips */
>-	if (nand_scan(&nandmtd->mtd, nc->nr_chips)) {
>-		nandmtd->pl_chip = NULL;
>+	flash_np = of_get_next_child(node, NULL);
>+	if (!flash_np)
> 		return -ENODEV;
>+
>+	ndfc->mtd.name = kasprintf(GFP_KERNEL, "%s.%s",
>+				   ndfc->ofdev->dev.bus_id, flash_np->name);
>+	if (!ndfc->mtd.name) {
>+		ret = -ENOMEM;
>+		goto err;
> 	}
>
>-#ifdef CONFIG_MTD_PARTITIONS
>-	printk("Number of partitions %d\n", nc->nr_partitions);
>-	if (nc->nr_partitions) {
>-		/* Add the full device, so complete dumps can be made */
>-		add_mtd_device(&nandmtd->mtd);
>-		add_mtd_partitions(&nandmtd->mtd, nc->partitions,
>-				   nc->nr_partitions);
>+	ret = nand_scan(&ndfc->mtd, 1);
>+	if (ret)
>+		goto err;
>
>-	} else
>-#else
>-		add_mtd_device(&nandmtd->mtd);
>+#ifdef CONFIG_MTD_PARTITIONS
>+	ret = parse_mtd_partitions(&ndfc->mtd, part_types, &ndfc->parts, 0);
>+	if (ret < 0)
>+		goto err;
>+
>+#ifdef CONFIG_MTD_OF_PARTS
>+	if (ret == 0) {
>+		ret = of_mtd_parse_partitions(&ndfc->ofdev->dev, flash_np,
>+					      &ndfc->parts);
>+		if (ret < 0)
>+			goto err;
>+	}
> #endif
>
>-	atomic_inc(&ndfc->childs_active);
>-	return 0;
>-}
>+	if (ret > 0)
>+		ret = add_mtd_partitions(&ndfc->mtd, ndfc->parts, ret);
>+	else
>+#endif
>+		ret = add_mtd_device(&ndfc->mtd);
>
>-static int ndfc_chip_remove(struct platform_device *pdev)
>-{
>-	return 0;
>+err:
>+	of_node_put(flash_np);
>+	if (ret)
>+		kfree(ndfc->mtd.name);
>+	return ret;
> }
>
>-static int ndfc_nand_probe(struct platform_device *pdev)
>+static int __devinit ndfc_probe(struct of_device *ofdev,
>+				const struct of_device_id *match)
> {
>-	struct platform_nand_ctrl *nc = pdev->dev.platform_data;
>-	struct ndfc_controller_settings *settings = nc->priv;
>-	struct resource *res = pdev->resource;
> 	struct ndfc_controller *ndfc = &ndfc_ctrl;
>-	unsigned long long phys = settings->ndfc_erpn | res->start;
>+	const u32 *reg;
>+	u32 ccr;
>+	int err, len;
>
>-#ifndef CONFIG_PHYS_64BIT
>-	ndfc->ndfcbase = ioremap((phys_addr_t)phys, res->end - res->start + 1);
>-#else
>-	ndfc->ndfcbase = ioremap64(phys, res->end - res->start + 1);
>-#endif
>+	spin_lock_init(&ndfc->ndfc_control.lock);
>+	init_waitqueue_head(&ndfc->ndfc_control.wq);
>+	ndfc->ofdev = ofdev;
>+	dev_set_drvdata(&ofdev->dev, ndfc);
>+
>+	/* Read the reg property to get the chip select */
>+	reg = of_get_property(ofdev->node, "reg", &len);
>+	if (reg == NULL || len != 12) {
>+		dev_err(&ofdev->dev, "unable read reg property (%d)\n", len);
>+		return -ENOENT;
>+	}
>+	ndfc->chip_select = reg[0];
>+
>+	ndfc->ndfcbase = of_iomap(ofdev->node, 0);
> 	if (!ndfc->ndfcbase) {
>-		printk(KERN_ERR "NDFC: ioremap failed\n");
>+		dev_err(&ofdev->dev, "failed to get memory\n");
> 		return -EIO;
> 	}
>
>-	__raw_writel(settings->ccr_settings, ndfc->ndfcbase + NDFC_CCR);
>+	ccr = NDFC_CCR_BS(ndfc->chip_select);
>
>-	spin_lock_init(&ndfc->ndfc_control.lock);
>-	init_waitqueue_head(&ndfc->ndfc_control.wq);
>+	/* It is ok if ccr does not exist - just default to 0 */
>+	reg = of_get_property(ofdev->node, "ccr", NULL);
>+	if (reg)
>+		ccr |= *reg;
>
>-	platform_set_drvdata(pdev, ndfc);
>+	out_be32(ndfc->ndfcbase + NDFC_CCR, ccr);
>
>-	printk("NDFC NAND Driver initialized. Chip-Rev: 0x%08x\n",
>-	       __raw_readl(ndfc->ndfcbase + NDFC_REVID));
>+	/* Set the bank settings if given */
>+	reg = of_get_property(ofdev->node, "bank-settings", NULL);
>+	if (reg) {
>+		int offset = NDFC_BCFG0 + (ndfc->chip_select << 2);
>+		out_be32(ndfc->ndfcbase + offset, *reg);
>+	}
>+
>+	err = ndfc_chip_init(ndfc, ofdev->node);
>+	if (err) {
>+		iounmap(ndfc->ndfcbase);
>+		return err;
>+	}
>
> 	return 0;
> }
>
>-static int ndfc_nand_remove(struct platform_device *pdev)
>+static int __devexit ndfc_remove(struct of_device *ofdev)
> {
>-	struct ndfc_controller *ndfc = platform_get_drvdata(pdev);
>+	struct ndfc_controller *ndfc = dev_get_drvdata(&ofdev->dev);
>
>-	if (atomic_read(&ndfc->childs_active))
>-		return -EBUSY;
>+	nand_release(&ndfc->mtd);
>
>-	if (ndfc) {
>-		platform_set_drvdata(pdev, NULL);
>-		iounmap(ndfc_ctrl.ndfcbase);
>-		ndfc_ctrl.ndfcbase = NULL;
>-	}
> 	return 0;
> }
>
>-/* driver device registration */
>-
>-static struct platform_driver ndfc_chip_driver = {
>-	.probe		= ndfc_chip_probe,
>-	.remove		= ndfc_chip_remove,
>-	.driver		= {
>-		.name	= "ndfc-chip",
>-		.owner	= THIS_MODULE,
>-	},
>+static const struct of_device_id ndfc_match[] = {
>+	{ .compatible = "ibm,ndfc", },
>+	{}
> };
>+MODULE_DEVICE_TABLE(of, ndfc_match);
>
>-static struct platform_driver ndfc_nand_driver = {
>-	.probe		= ndfc_nand_probe,
>-	.remove		= ndfc_nand_remove,
>-	.driver		= {
>-		.name	= "ndfc-nand",
>-		.owner	= THIS_MODULE,
>+static struct of_platform_driver ndfc_driver = {
>+	.driver = {
>+		.name	= "ndfc",
> 	},
>+	.match_table = ndfc_match,
>+	.probe = ndfc_probe,
>+	.remove = __devexit_p(ndfc_remove),
> };
>
> static int __init ndfc_nand_init(void)
> {
>-	int ret;
>-
>-	spin_lock_init(&ndfc_ctrl.ndfc_control.lock);
>-	init_waitqueue_head(&ndfc_ctrl.ndfc_control.wq);
>-
>-	ret = platform_driver_register(&ndfc_nand_driver);
>-	if (!ret)
>-		ret = platform_driver_register(&ndfc_chip_driver);
>-	return ret;
>+	return of_register_platform_driver(&ndfc_driver);
> }
>
> static void __exit ndfc_nand_exit(void)
> {
>-	platform_driver_unregister(&ndfc_chip_driver);
>-	platform_driver_unregister(&ndfc_nand_driver);
>+	of_unregister_platform_driver(&ndfc_driver);
> }
>
> module_init(ndfc_nand_init);
>@@ -316,6 +315,4 @@ module_exit(ndfc_nand_exit);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Thomas Gleixner <tglx@linutronix.de>");
>-MODULE_DESCRIPTION("Platform driver for NDFC");
>-MODULE_ALIAS("platform:ndfc-chip");
>-MODULE_ALIAS("platform:ndfc-nand");
>+MODULE_DESCRIPTION("OF Platform driver for NDFC");

Re: [PATCH] ndfc driver

[Sean MacLennan]

On Thu, 4 Dec 2008 09:01:07 -0500
"Josh Boyer" <jwboyer@linux.vnet.ibm.com> wrote:

> You also seem to only support a single NAND chip, however the NDFC can
> support multiple chips.  Have you looked at how the the fsl_elbc_nand
> driver does multiple chip support?  If not, could you at least
> document the limitation in the patch?

Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
index 0352d5c..3dd85e8 100644
--- a/drivers/mtd/nand/ndfc.c
+++ b/drivers/mtd/nand/ndfc.c
@@ -2,11 +2,15 @@
  *  drivers/mtd/ndfc.c
  *
  *  Overview:
- *   Platform independend driver for NDFC (NanD Flash Controller)
+ *   Platform independent driver for NDFC (NanD Flash Controller)
  *   integrated into EP440 cores
  *
  *   Ported to an OF platform driver by Sean MacLennan
  *
+ *   The NDFC supports multiple chips, but this driver only supports a
+ *   single chip since I do not have access to any boards with
+ *   multiple chips.
+ *
  *  Author: Thomas Gleixner
  *
  *  Copyright 2006 IBM

Re: [PATCH] ndfc driver

[Felix Radensky]

Hi,

I can confirm that this driver works fine on custom
4060EX board. The NAND is 256MiB Samsumg,
with 2K page size. I had to modify U-boot to provide
EBC "ranges" property for NAND, but otherwise
it worked from the beginning.

I didn't see any reaction to Sean's post, but this
driver is important IMHO, since several in-tree 44X
boards (e.g Canyonlands and Glacier) lack NAND
support, and Warp uses old platform device model
for its NAND.

Felix Radensky
Embedded Solutions Ltd.
http://www.embedded-sol.com

[PATCH] ndfc driver

[Sean MacLennan]

The current ndfc driver only compiles under arch/ppc. This arch was
merged with arch/powerpc and then removed from the kernel. I notice the
entry for the ndfc in Kconfig has been removed in 2.6.28.

This patch converts the ndfc to a proper OF (OpenFirmware) driver. I
can give a working example of the DTS if needed.

The patch has been in production use on the PIKA Warp Appliance. The
Warp basically boots from NAND, so the ndfc driver is very important to
us.

Cheers,
  Sean MacLennan

Port of the ndfc driver to an of platform driver.

Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 1c2e945..5705f85 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -163,6 +163,13 @@ config MTD_NAND_S3C2410_HWECC
 	  incorrect ECC generation, and if using these, the default of
 	  software ECC is preferable.
 
+config MTD_NAND_NDFC
+	tristate "NDFC NanD Flash Controller"
+	depends on 4xx
+	select MTD_NAND_ECC_SMC
+	help
+	 NDFC Nand Flash Controllers are integrated in IBM/AMCC's 4xx SoCs
+
 config MTD_NAND_S3C2410_CLKSTOP
 	bool "S3C2410 NAND IDLE clock stop"
 	depends on MTD_NAND_S3C2410
diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
index 955959e..0352d5c 100644
--- a/drivers/mtd/nand/ndfc.c
+++ b/drivers/mtd/nand/ndfc.c
@@ -5,9 +5,13 @@
  *   Platform independend driver for NDFC (NanD Flash Controller)
  *   integrated into EP440 cores
  *
+ *   Ported to an OF platform driver by Sean MacLennan
+ *
  *  Author: Thomas Gleixner
  *
  *  Copyright 2006 IBM
+ *  Copyright 2008 PIKA Technologies
+ *    Sean MacLennan <smaclennan@pikatech.com>
  *
  *  This program is free software; you can redistribute	 it and/or modify it
  *  under  the terms of	 the GNU General  Public License as published by the
@@ -21,27 +25,17 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/ndfc.h>
 #include <linux/mtd/mtd.h>
-#include <linux/platform_device.h>
-
+#include <linux/of_platform.h>
 #include <asm/io.h>
-#ifdef CONFIG_40x
-#include <asm/ibm405.h>
-#else
-#include <asm/ibm44x.h>
-#endif
-
-struct ndfc_nand_mtd {
-	struct mtd_info			mtd;
-	struct nand_chip		chip;
-	struct platform_nand_chip	*pl_chip;
-};
 
-static struct ndfc_nand_mtd ndfc_mtd[NDFC_MAX_BANKS];
 
 struct ndfc_controller {
-	void __iomem		*ndfcbase;
-	struct nand_hw_control	ndfc_control;
-	atomic_t		childs_active;
+	struct of_device *ofdev;
+	void __iomem *ndfcbase;
+	struct mtd_info mtd;
+	struct nand_chip chip;
+	int chip_select;
+	struct nand_hw_control ndfc_control;
 };
 
 static struct ndfc_controller ndfc_ctrl;
@@ -50,17 +44,14 @@ static void ndfc_select_chip(struct mtd_info *mtd, int chip)
 {
 	uint32_t ccr;
 	struct ndfc_controller *ndfc = &ndfc_ctrl;
-	struct nand_chip *nandchip = mtd->priv;
-	struct ndfc_nand_mtd *nandmtd = nandchip->priv;
-	struct platform_nand_chip *pchip = nandmtd->pl_chip;
 
-	ccr = __raw_readl(ndfc->ndfcbase + NDFC_CCR);
+	ccr = in_be32(ndfc->ndfcbase + NDFC_CCR);
 	if (chip >= 0) {
 		ccr &= ~NDFC_CCR_BS_MASK;
-		ccr |= NDFC_CCR_BS(chip + pchip->chip_offset);
+		ccr |= NDFC_CCR_BS(chip + ndfc->chip_select);
 	} else
 		ccr |= NDFC_CCR_RESET_CE;
-	__raw_writel(ccr, ndfc->ndfcbase + NDFC_CCR);
+	out_be32(ndfc->ndfcbase + NDFC_CCR, ccr);
 }
 
 static void ndfc_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
@@ -80,7 +71,7 @@ static int ndfc_ready(struct mtd_info *mtd)
 {
 	struct ndfc_controller *ndfc = &ndfc_ctrl;
 
-	return __raw_readl(ndfc->ndfcbase + NDFC_STAT) & NDFC_STAT_IS_READY;
+	return in_be32(ndfc->ndfcbase + NDFC_STAT) & NDFC_STAT_IS_READY;
 }
 
 static void ndfc_enable_hwecc(struct mtd_info *mtd, int mode)
@@ -88,9 +79,9 @@ static void ndfc_enable_hwecc(struct mtd_info *mtd, int mode)
 	uint32_t ccr;
 	struct ndfc_controller *ndfc = &ndfc_ctrl;
 
-	ccr = __raw_readl(ndfc->ndfcbase + NDFC_CCR);
+	ccr = in_be32(ndfc->ndfcbase + NDFC_CCR);
 	ccr |= NDFC_CCR_RESET_ECC;
-	__raw_writel(ccr, ndfc->ndfcbase + NDFC_CCR);
+	out_be32(ndfc->ndfcbase + NDFC_CCR, ccr);
 	wmb();
 }
 
@@ -102,9 +93,10 @@ static int ndfc_calculate_ecc(struct mtd_info *mtd,
 	uint8_t *p = (uint8_t *)&ecc;
 
 	wmb();
-	ecc = __raw_readl(ndfc->ndfcbase + NDFC_ECC);
-	ecc_code[0] = p[1];
-	ecc_code[1] = p[2];
+	ecc = in_be32(ndfc->ndfcbase + NDFC_ECC);
+	/* The NDFC uses Smart Media (SMC) bytes order */
+	ecc_code[0] = p[2];
+	ecc_code[1] = p[1];
 	ecc_code[2] = p[3];
 
 	return 0;
@@ -123,7 +115,7 @@ static void ndfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 	uint32_t *p = (uint32_t *) buf;
 
 	for(;len > 0; len -= 4)
-		*p++ = __raw_readl(ndfc->ndfcbase + NDFC_DATA);
+		*p++ = in_be32(ndfc->ndfcbase + NDFC_DATA);
 }
 
 static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
@@ -132,7 +124,7 @@ static void ndfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 	uint32_t *p = (uint32_t *) buf;
 
 	for(;len > 0; len -= 4)
-		__raw_writel(*p++, ndfc->ndfcbase + NDFC_DATA);
+		out_be32(ndfc->ndfcbase + NDFC_DATA, *p++);
 }
 
 static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
@@ -141,7 +133,7 @@ static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 	uint32_t *p = (uint32_t *) buf;
 
 	for(;len > 0; len -= 4)
-		if (*p++ != __raw_readl(ndfc->ndfcbase + NDFC_DATA))
+		if (*p++ != in_be32(ndfc->ndfcbase + NDFC_DATA))
 			return -EFAULT;
 	return 0;
 }
@@ -149,10 +141,19 @@ static int ndfc_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 /*
  * Initialize chip structure
  */
-static void ndfc_chip_init(struct ndfc_nand_mtd *mtd)
+static int ndfc_chip_init(struct ndfc_controller *ndfc,
+			  struct device_node *node)
 {
-	struct ndfc_controller *ndfc = &ndfc_ctrl;
-	struct nand_chip *chip = &mtd->chip;
+#ifdef CONFIG_MTD_PARTITIONS
+#ifdef CONFIG_MTD_CMDLINE_PARTS
+	static const char *part_types[] = { "cmdlinepart", NULL };
+#else
+	static const char *part_types[] = { NULL };
+#endif
+	struct mtd_partition *parts;
+#endif
+	struct nand_chip *chip = &ndfc->chip;
+	int ret;
 
 	chip->IO_ADDR_R = ndfc->ndfcbase + NDFC_DATA;
 	chip->IO_ADDR_W = ndfc->ndfcbase + NDFC_DATA;
@@ -160,8 +161,6 @@ static void ndfc_chip_init(struct ndfc_nand_mtd *mtd)
 	chip->dev_ready = ndfc_ready;
 	chip->select_chip = ndfc_select_chip;
 	chip->chip_delay = 50;
-	chip->priv = mtd;
-	chip->options = mtd->pl_chip->options;
 	chip->controller = &ndfc->ndfc_control;
 	chip->read_buf = ndfc_read_buf;
 	chip->write_buf = ndfc_write_buf;
@@ -172,143 +171,120 @@ static void ndfc_chip_init(struct ndfc_nand_mtd *mtd)
 	chip->ecc.mode = NAND_ECC_HW;
 	chip->ecc.size = 256;
 	chip->ecc.bytes = 3;
-	chip->ecclayout = chip->ecc.layout = mtd->pl_chip->ecclayout;
-	mtd->mtd.priv = chip;
-	mtd->mtd.owner = THIS_MODULE;
-}
-
-static int ndfc_chip_probe(struct platform_device *pdev)
-{
-	struct platform_nand_chip *nc = pdev->dev.platform_data;
-	struct ndfc_chip_settings *settings = nc->priv;
-	struct ndfc_controller *ndfc = &ndfc_ctrl;
-	struct ndfc_nand_mtd *nandmtd;
-
-	if (nc->chip_offset >= NDFC_MAX_BANKS || nc->nr_chips > NDFC_MAX_BANKS)
-		return -EINVAL;
 
-	/* Set the bank settings */
-	__raw_writel(settings->bank_settings,
-		     ndfc->ndfcbase + NDFC_BCFG0 + (nc->chip_offset << 2));
+	ndfc->mtd.priv = chip;
+	ndfc->mtd.owner = THIS_MODULE;
 
-	nandmtd = &ndfc_mtd[pdev->id];
-	if (nandmtd->pl_chip)
-		return -EBUSY;
+	ret = nand_scan(&ndfc->mtd, 1);
+	if (ret)
+		return ret;
 
-	nandmtd->pl_chip = nc;
-	ndfc_chip_init(nandmtd);
-
-	/* Scan for chips */
-	if (nand_scan(&nandmtd->mtd, nc->nr_chips)) {
-		nandmtd->pl_chip = NULL;
-		return -ENODEV;
-	}
+	ndfc->mtd.name = ndfc->ofdev->dev.bus_id;
 
 #ifdef CONFIG_MTD_PARTITIONS
-	printk("Number of partitions %d\n", nc->nr_partitions);
-	if (nc->nr_partitions) {
-		/* Add the full device, so complete dumps can be made */
-		add_mtd_device(&nandmtd->mtd);
-		add_mtd_partitions(&nandmtd->mtd, nc->partitions,
-				   nc->nr_partitions);
-
-	} else
-#else
-		add_mtd_device(&nandmtd->mtd);
+	ret = parse_mtd_partitions(&ndfc->mtd, part_types, &parts, 0);
+	if (ret < 0)
+		return ret;
+
+#ifdef CONFIG_MTD_OF_PARTS
+	if (ret == 0) {
+		ret = of_mtd_parse_partitions(&ndfc->ofdev->dev, node, &parts);
+		if (ret < 0)
+			return ret;
+	}
 #endif
 
-	atomic_inc(&ndfc->childs_active);
-	return 0;
-}
+	if (ret > 0)
+		return add_mtd_partitions(&ndfc->mtd, parts, ret);
+#endif
 
-static int ndfc_chip_remove(struct platform_device *pdev)
-{
-	return 0;
+	return add_mtd_device(&ndfc->mtd);
 }
 
-static int ndfc_nand_probe(struct platform_device *pdev)
+static int __devinit ndfc_probe(struct of_device *ofdev,
+				const struct of_device_id *match)
 {
-	struct platform_nand_ctrl *nc = pdev->dev.platform_data;
-	struct ndfc_controller_settings *settings = nc->priv;
-	struct resource *res = pdev->resource;
 	struct ndfc_controller *ndfc = &ndfc_ctrl;
-	unsigned long long phys = settings->ndfc_erpn | res->start;
+	const u32 *reg;
+	u32 ccr;
+	int err, len;
 
-#ifndef CONFIG_PHYS_64BIT
-	ndfc->ndfcbase = ioremap((phys_addr_t)phys, res->end - res->start + 1);
-#else
-	ndfc->ndfcbase = ioremap64(phys, res->end - res->start + 1);
-#endif
+	spin_lock_init(&ndfc->ndfc_control.lock);
+	init_waitqueue_head(&ndfc->ndfc_control.wq);
+	ndfc->ofdev = ofdev;
+	dev_set_drvdata(&ofdev->dev, ndfc);
+
+	/* Read the reg property to get the chip select */
+	reg = of_get_property(ofdev->node, "reg", &len);
+	if (reg == NULL || len != 12) {
+		dev_err(&ofdev->dev, "unable read reg property (%d)\n", len);
+		return -ENOENT;
+	}
+	ndfc->chip_select = reg[0];
+
+	ndfc->ndfcbase = of_iomap(ofdev->node, 0);
 	if (!ndfc->ndfcbase) {
-		printk(KERN_ERR "NDFC: ioremap failed\n");
+		dev_err(&ofdev->dev, "failed to get memory\n");
 		return -EIO;
 	}
 
-	__raw_writel(settings->ccr_settings, ndfc->ndfcbase + NDFC_CCR);
+	ccr = NDFC_CCR_BS(ndfc->chip_select);
 
-	spin_lock_init(&ndfc->ndfc_control.lock);
-	init_waitqueue_head(&ndfc->ndfc_control.wq);
+	/* It is ok if ccr does not exist - just default to 0 */
+	reg = of_get_property(ofdev->node, "ccr", NULL);
+	if (reg)
+		ccr |= *reg;
+
+	out_be32(ndfc->ndfcbase + NDFC_CCR, ccr);
 
-	platform_set_drvdata(pdev, ndfc);
+	/* Set the bank settings if given */
+	reg = of_get_property(ofdev->node, "bank-settings", NULL);
+	if (reg) {
+		int offset = NDFC_BCFG0 + (ndfc->chip_select << 2);
+		out_be32(ndfc->ndfcbase + offset, *reg);
+	}
 
-	printk("NDFC NAND Driver initialized. Chip-Rev: 0x%08x\n",
-	       __raw_readl(ndfc->ndfcbase + NDFC_REVID));
+	err = ndfc_chip_init(ndfc, ofdev->node);
+	if (err) {
+		iounmap(ndfc->ndfcbase);
+		return err;
+	}
 
 	return 0;
 }
 
-static int ndfc_nand_remove(struct platform_device *pdev)
+static int __devexit ndfc_remove(struct of_device *ofdev)
 {
-	struct ndfc_controller *ndfc = platform_get_drvdata(pdev);
+	struct ndfc_controller *ndfc = dev_get_drvdata(&ofdev->dev);
 
-	if (atomic_read(&ndfc->childs_active))
-		return -EBUSY;
+	nand_release(&ndfc->mtd);
 
-	if (ndfc) {
-		platform_set_drvdata(pdev, NULL);
-		iounmap(ndfc_ctrl.ndfcbase);
-		ndfc_ctrl.ndfcbase = NULL;
-	}
 	return 0;
 }
 
-/* driver device registration */
-
-static struct platform_driver ndfc_chip_driver = {
-	.probe		= ndfc_chip_probe,
-	.remove		= ndfc_chip_remove,
-	.driver		= {
-		.name	= "ndfc-chip",
-		.owner	= THIS_MODULE,
-	},
+static const struct of_device_id ndfc_match[] = {
+	{ .compatible = "amcc,ndfc", },
+	{}
 };
+MODULE_DEVICE_TABLE(of, ndfc_match);
 
-static struct platform_driver ndfc_nand_driver = {
-	.probe		= ndfc_nand_probe,
-	.remove		= ndfc_nand_remove,
-	.driver		= {
-		.name	= "ndfc-nand",
-		.owner	= THIS_MODULE,
+static struct of_platform_driver ndfc_driver = {
+	.driver = {
+		.name	= "ndfc",
 	},
+	.match_table = ndfc_match,
+	.probe = ndfc_probe,
+	.remove = __devexit_p(ndfc_remove),
 };
 
 static int __init ndfc_nand_init(void)
 {
-	int ret;
-
-	spin_lock_init(&ndfc_ctrl.ndfc_control.lock);
-	init_waitqueue_head(&ndfc_ctrl.ndfc_control.wq);
-
-	ret = platform_driver_register(&ndfc_nand_driver);
-	if (!ret)
-		ret = platform_driver_register(&ndfc_chip_driver);
-	return ret;
+	return of_register_platform_driver(&ndfc_driver);
 }
 
 static void __exit ndfc_nand_exit(void)
 {
-	platform_driver_unregister(&ndfc_chip_driver);
-	platform_driver_unregister(&ndfc_nand_driver);
+	of_unregister_platform_driver(&ndfc_driver);
 }
 
 module_init(ndfc_nand_init);
@@ -316,6 +292,4 @@ module_exit(ndfc_nand_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Thomas Gleixner <tglx@linutronix.de>");
-MODULE_DESCRIPTION("Platform driver for NDFC");
-MODULE_ALIAS("platform:ndfc-chip");
-MODULE_ALIAS("platform:ndfc-nand");
+MODULE_DESCRIPTION("OF Platform driver for NDFC");