diff mbox

ndfc driver

Message ID 20081208193446.37e27e26@lappy.seanm.ca
State Superseded
Headers show

Commit Message

Sean MacLennan Dec. 9, 2008, 12:34 a.m. UTC 
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

Comments

Anton Vorontsov Dec. 9, 2008, 2:11 a.m. UTC | #1 
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,
Sean MacLennan Dec. 9, 2008, 2:45 a.m. UTC | #2 
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
Josh Boyer Dec. 9, 2008, 3:32 a.m. UTC | #3 
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
Sean MacLennan Dec. 9, 2008, 4:54 a.m. UTC | #4 
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
Stefan Roese Dec. 9, 2008, 6:10 a.m. UTC | #5 
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
Mitch Bradley Dec. 9, 2008, 7:57 a.m. UTC | #6 
One address/size cell isn't enough for the next generation of NAND FLASH 
chips.
Josh Boyer Dec. 9, 2008, 11:24 a.m. UTC | #7 
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
Sean MacLennan Dec. 10, 2008, 4:01 a.m. UTC | #8 
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
Mitch Bradley Dec. 10, 2008, 8:28 a.m. UTC | #9 
>
> 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.
diff mbox

Patch

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>;
+	};
+};
+