Add device-tree aware NDFC driver

This adds a device-tree aware PowerPC 44x NanD Flash Controller driver The code is based on the original NDFC driver by Thomas Gleixner, but since it's been changed much and has initialization/clean-up completely reworked it's been put into a separate ndfc_of.c file. This version supports both separate mtd devices on each chip attached to NDFC banks and single mtd device spread across identical chips (not using mtdconcat) as well. The choice is selected with device tree settings. This has been tested on PowerPC 440EPx Sequoia board.
Any comments are greatly appreciated.
Thanks,
Valentine.

This adds support of the built-in PowerPC 44x NanD Flash Controller (NDFC) based on the OF description. This version supports both separate mtd devices on each NDFC bank and mtd devices spread across identical chips attached to NDFC banks depending on the device tree settings. This is based on the original NDFC driver by Thomas Gleixner, but since a lot of things have been reworked it's been put to a separate ndfc_of file.

---
 drivers/mtd/nand/Kconfig   |    7 
 drivers/mtd/nand/Makefile  |    1 
 drivers/mtd/nand/ndfc_of.c |  599 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/ndfc.h   |    4 
 4 files changed, 611 insertions(+)

diff -pruN linux-2.6.orig/drivers/mtd/nand/Kconfig linux-2.6/drivers/mtd/nand/Kconfig
@@ -158,6 +158,13 @@ config MTD_NAND_NDFC
 	help
 	 NDFC Nand Flash Controllers are integrated in IBM/AMCC's 4xx SoCs
 
+config MTD_NAND_NDFC_OF
+	tristate "NDFC NanD Flash Controller based on OF"
+	depends on 44x && PPC_MERGE
+	select MTD_NAND_ECC_SMC
+	help
+	 NDFC Nand Flash Controllers are integrated in EP44x SoCs
+
 config MTD_NAND_S3C2410_CLKSTOP
 	bool "S3C2410 NAND IDLE clock stop"
 	depends on MTD_NAND_S3C2410

diff -pruN linux-2.6.orig/drivers/mtd/nand/Makefile linux-2.6/drivers/mtd/nand/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_MTD_NAND_TS7250)		+= ts7250
 obj-$(CONFIG_MTD_NAND_NANDSIM)		+= nandsim.o
 obj-$(CONFIG_MTD_NAND_CS553X)		+= cs553x_nand.o
 obj-$(CONFIG_MTD_NAND_NDFC)		+= ndfc.o
+obj-$(CONFIG_MTD_NAND_NDFC_OF)		+= ndfc_of.o
 obj-$(CONFIG_MTD_NAND_AT91)		+= at91_nand.o
 obj-$(CONFIG_MTD_NAND_CM_X270)		+= cmx270_nand.o
 obj-$(CONFIG_MTD_NAND_BASLER_EXCITE)	+= excite_nandflash.o

diff -pruN linux-2.6.orig/include/linux/mtd/ndfc.h linux-2.6/include/linux/mtd/ndfc.h
@@ -52,6 +52,10 @@
 #define NDFC_BxCFG_SZ_MASK	0x08000000 /* Bank Size */
 #define NDFC_BxCFG_SZ_8BIT	0x00000000 /* 8bit */
 #define NDFC_BxCFG_SZ_16BIT	0x08000000 /* 16bit */
+#define NDFC_BxCFG_RR(x)	((x) & 0x3 << 0)
+#define NDFC_BxCFG_RWH(x)	((x) & 0x3 << 4)
+#define NDFC_BxCFG_RWP(x)	((x) & 0x3 << 8)
+#define NDFC_BxCFG_CRW(x)	((x) & 0x3 << 12)
 
 #define NDFC_MAX_BANKS		4
 

PowerPC 44x NanD Flash Controller (NDFC) bindings.

---
 Documentation/powerpc/booting-without-of.txt |   64 +++++++++++++++++++++++++++
 1 files changed, 64 insertions(+)

@@ -52,6 +52,7 @@ Table of Contents
       i) Freescale QUICC Engine module (QE)
       j) CFI or JEDEC memory-mapped NOR flash
       k) Global Utilities Block
+      l) 44x NanD Flash Controller (NDFC)
 
   VII - Specifying interrupt information for devices
     1) interrupts property

@@ -2242,6 +2243,69 @@ platforms are moved over to use the flat
 			   available.
 			   For Axon: 0x0000012a
 
+  l) 44x NanD Flash Controller (NDFC)
+
+  Required properties:
+  - compatible : should be "ibm,ndfc".
+  - reg : should contain address and length of the NDFC registers.
+  
+  ndfc node should contain at least one ndfc-mtd sub-node describing
+  chip properties and bank settings for the chip(s) attached to particular
+  ndfc bank(s).
+
+  Required ndfc-mtd properties:
+  - bank-map : at least one value (less than number of NDFC banks available)
+    identifying bank number the chip is attached to. If we have several
+    identical chips and want to spread a single mtd device across all of them,
+    we need to specify here the bank numbers the chips are attached to.
+    E.g. "bank-map = <0 3>;" means a single mtd device will be created 
+    for 2 identical NAND chips attached to banks 0 and 3 of the NDFC.
+  Optional ndfc-mtd properties:
+  - chip-options : NAND chip options.
+  - chip-delay : NAND chip delay. Default is 50us.
+  - bank-settings : NDFC bank settings for the chip(s). This value is
+    written to the NDFC Bank Configuration Register.
+    If not specified, the default timings (RR=RWH=RWP=CRW=2) and 
+    bank width from the "bank-width" property will be used for the chip.
+  - bank-width : NAND chip bus width. Should be 1 for 8-bit NAND or 
+    2 for 16-bit NAND. By default an 8-bit width is set.
+  - #address-cells, #size-cells : Must be present if the flash has
+    sub-nodes representing partitions (see below).  In this case
+    both #address-cells and #size-cells must be equal to 1.
+  
+  ndfc_mtd can have partition sub-nodes, which are described the same way
+  as for the CFI or JEDEC memory-mapped NOR flash.
+
+   Example (Sequoia 440EPx):
+   NDFC is relocatable within EBC and should have EBC as a parent node.
+   Here we have NDFC on EBC CS3 bank and one NAND chip attached to bank 3
+   of the NanD Flash Controller:
+
+		ndfc@0,0 {
+			compatible = "ibm,ndfc-440epx", "ibm,ndfc";
+			reg = <3 000000 2000>;
+			ndfc-mtd0 {
+				bank-settings = <80002222>;
+				bank-map = <3>;
+				chip-delay = <32>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+				partition@0 {
+					label = "u-boot-nand";
+					reg = <0 0080000>;
+				};
+				partition@80000 {
+					label = "kernel-nand";
+					reg = <0080000 0180000>;
+				};
+				partition@200000 {
+					label = "filesystem";
+					reg = <0200000 1e00000>;
+				};
+			};
+		};
+
+
    More devices will be defined as this spec matures.
 
 VII - Specifying interrupt information for devices

NDFC DTS entry for PowerPC 440EPx Sequoia board.

---
 arch/powerpc/boot/dts/sequoia.dts |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+)

diff -pruN linux-2.6.orig/arch/powerpc/boot/dts/sequoia.dts linux-2.6/arch/powerpc/boot/dts/sequoia.dts
@@ -141,6 +141,29 @@
 				interrupts = <5 1>;
 				interrupt-parent = <&UIC1>;
 
+				ndfc@0,0 {
+					compatible = "ibm,ndfc-440epx", "ibm,ndfc";
+					reg = <3 000000 2000>;
+					ndfc-mtd0 {
+						bank-settings = <80002222>;
+						bank-map = <3>;
+						chip-delay = <32>;
+						#address-cells = <1>;
+						#size-cells = <1>;
+						partition@0 {
+							label = "u-boot-nand";
+							reg = <0 0080000>;
+						};
+						partition@80000 {
+							label = "kernel-nand";
+							reg = <0080000 0180000>;
+						};
+						partition@200000 {
+							label = "filesystem";
+							reg = <0200000 1e00000>;
+						};
+					};
+				};
 				nor_flash@0,0 {
 					compatible = "amd,s29gl256n", "cfi-flash";
 					bank-width = <2>;

Did I express myself not clear enough in my first reply or is this just a repeated epiphany in my inbox ?

You got plenty of comments to your patches, but you decided to ignore them silently.

Darn, fix it the right way once and forever and please don't try to tell me another heartrending "why I did it my way" story.

This all can be done with a nice series of incremental patches including a fixup to the existing users.

We have enough dump and run shit in the kernel already.

No thanks,

tglx

You know, you're really too tense Thomas. I'm not sure of the reason why you're being a complete nerve, but I'm feeling sorry for you.
I'm not saying my approach is the best, but I was hoping for a discussion.
I've reworked the patches according to the comments to the previous version and used my arguments to explain why I don't see much reason to mess with the code we currently have and added a separate _of version. I'm sure you'd find some time to do it yourself "the right way once and forever" with a "nice series of incremental patches" to fix what we currently have (call it a "dump" or anything you like) and even maybe add new device tree support. I'm sorry if for some reason I've made you feel bad.
This is the last time I disturb you with my e-mail, so please, forget it.
Thank you very much for your comments anyway.
It's been nice talking to you,
Valentine.

That is good. So please take a moment to listen.

Ok.

Thomas words were harsh. Nothing unusual so far. Some people deserve harsh words, some don't and some simply are doing a good job to invite them. You mails so far were inviting harsh words.

So how did you invite harsh words and what can you change to prevent this in the future?

1. Develop a thicker skin. No matter how well you do, there will always be the occasional harsh words, deserved or not. It is ok to get angry and call the person names - but do that at home and leave it out of your responses. Ignore the flamebait, at least in public.

2. Follow local customs. In particular you should follow the style of discussion commonly used in mailing lists:

>>> I'm doing this.
>> Crap! Never do that!
> Why not? Can you explain?
Because...

>>> Then I do that.
>> Wouldn't ABCD be better?
> ABCD wouldn't work for me because of XYZ. Fair enough.

Respond to individual comments _directly_following_the_comment_ do not collect comments from 10 mails, then respond to them all in a long paragraph. Noone will read that. I didn't read it either.

3. Be concise. When quoting someone, remove everything but the relevant bits. Respond only with relevant information. People regularly read 10+ mailing lists. Their attention span is short. If you manage to annoy them with irrelevant information in the first 10 lines, they will skip any amount of wisdom below.

4. Be polite. Even if the responses you get are not. Flamewars get you nowhere.

5. Have sound technical arguments. "mtd concat adds a slight overhead" is not for two reasons. First, it is lacking numbers. People's guesses about perceived overhead or usually wrong, so knowledgeable readers will immediately question such arguments. Secondly, you didn't explain _why_ mtdconcat adds overhead. In particular, why didn't you reduce the mtdconcat overhead instead of essentially copying its functionality?

6. Be structured. Empty lines are cheap. Use them. Add structure to your mails. Above you can see several paragraphs. You can quickly go back to a previous one. You can skip one after reading just the first sentence. After several attempts I still haven't read your 46-line monologue. I don't even know how far I made it because it is so damn hard to find the spot again.

If you are willing to change the style of your mails, maybe people will actually read them and respond to you in ways you expected. Otherwise it may indeed be a wise choice not to come back. Some people simply just don't get along. Sometimes a proxy is needed to translate between people. But my hope is that you are willing and able to work with us directly.

Jörn

There is no worse hell than that provided by the regrets
for wasted opportunities.
-- Andre-Louis Moreau in Scarabouche

Valentine,

You have a perception problem. I'm not tense, I'm grumpy.

Rest assured, that my nerves are completely fine despite of the fact that you try to rack them.

This is the exact point. You were asked to fix up the existing driver instead of replacing it and to do it with a series of incremental patches. You copied the old code anyway and modified it, so we want to have this documented in the history. This is not my obsession, it's common kernel coding practise. The fact that you do not see much reason to do it does not change this at all.

It might be time for you to try to understand how OSS development works.

What do you expect, after you abused my Signed-off-by in a way which might have tricked David into pulling your code as is? This was pointed out to you and you did not even bother to apologize.

Interesting. I thought you wanted to get the patch in, so you probably should ask yourself whether it is a good idea _not_ to talk to the responsible maintainer.

If you gave up on that, it just makes it more obvious that you do not want to work with the community and just wanted to dump your patch and move along.

tglx

:)

That's good to hear :)

Replacing the original driver is not my obsession either.
I just don't see the "right" way to modify the original driver to support both platform devices and the new OF implementation. I see some initialization order problems with the original version, and I think that the easiest way to handle ndfc and chips attached to it would be to
do it in a single probe() function instead of having separate chip
devices and a separate ndfc platform device on top of that.

So, my opinion is that modifying the original code involves ndfc users' modification. Considering the fact that ppc removal is scheduled for the next summer, I thought that we could leave the original version intact and build the new OF one.

I know the common kernel practice, but if we always followed it we'd never bring up arch/powerpc.
BTW, I've posted some of the ndfc questions to the MTD mailing list
(http://lists.infradead.org/pipermail/linux-mtd/2007-November/019769.html). If I had the answers I might get a more clear idea about the right way to do it. I'd appreciate if you could take a look.

I do understand how it works.

I apologize.
If I wanted to abuse or trick somebody and get my code in as fast as possible no matter what, I wouldn't cc the maintainer, the other people interested and send it to both mtd and ppc mailing lists. I don't see any possible way for a guy who hasn't worked with the MTD community much to trick someone and get his patches in.

As I said above, I don't see the "right" way to do it. And actually I didn't expect you to share your thoughts and reasons on how to support both implementations and why this way is preferred. All I heard were cursing and direct orders to stop tying to explain my reasons and do it the "right" way. It looked like the responsible maintainer just wouldn't listen. Why talk then?

I never give up ;) and I didn't say I was going to stop working with the community.

Cheers,
Valentine.