On Wed, 2013-09-25 at 04:50 -0500, Xie Xiaobo-R63061 wrote:

Hi Scott,

See the reply inline.

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, September 25, 2013 7:22 AM
To: Xie Xiaobo-R63061
Cc: linuxppc-dev@lists.ozlabs.org; Johnston Michael-R49610
Subject: Re: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support

On Tue, 2013-09-24 at 18:48 +0800, Xie Xiaobo wrote:

quoted

+		partition@80000 {
+			/* 3.5 MB for Linux Kernel Image */
+			reg = <0x00080000 0x00380000>;
+			label = "NOR Linux Kernel Image";
+		};

Is this enough?

I will enlarge it to 6MB.
 

quoted

quoted

+		partition@400000 {
+			/* 58.75MB for JFFS2 based Root file System */
+			reg = <0x00400000 0x03ac0000>;
+			label = "NOR Root File System";
+		};

Don't specify jffs2.

OK, I will remove "jffs2"

quoted

quoted

+	/* CS2 for Display */
+	ssd1289@2,0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "ssd1289";
+		reg = <0x2 0x0000 0x0002
+		       0x2 0x0002 0x0002>;
+	};

Node names should be generic.  What does ssd1289 do?  If this is actually
the display device, then it should be called "display@2,0".

OK. The ssd1289 is a LCD controller.

quoted

How about a vendor prefix on that compatible?  Why #address-cells/#size-
cells despite no child nodes?  Where is a binding that says what each of
those two reg resources mean?

I will add the vendor prefix. I review the ssd1289 driver, and the #address-cells/#size-cells were un-used. I will remove them.

And a binding?

Why do you need two separate reg resources rather than just <2 0 4>?
Will they ever be discontiguous?

-Scott