[PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support

[Xie Xiaobo-R63061]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, September 26, 2013 7:10 AM
> To: Xie Xiaobo-R63061
> Cc: Wood Scott-B07421; linuxppc-dev at lists.ozlabs.org; Johnston Michael-
> R49610
> Subject: Re: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support
> 
> On Wed, 2013-09-25 at 04:50 -0500, Xie Xiaobo-R63061 wrote:
> > Hi Scott,
> >
> > See the reply inline.
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Wednesday, September 25, 2013 7:22 AM
> > > To: Xie Xiaobo-R63061
> > > Cc: linuxppc-dev at 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:
> > > > +		partition at 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.
> >
> > >
> > > > +		partition at 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"
> >
> > >
> > > > +	/* CS2 for Display */
> > > > +	ssd1289 at 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 at 2,0".
> >
> > OK. The ssd1289 is a LCD controller.
> >
> > >
> > > 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?

[Xie] I review the ssd1289 driver code, and found the driver need two reg resources, if change the dts, the driver also should be modified accordingly. So I remove the ssd1289 node from this patch. I will submit new patch include the dts modification, ssd1289 driver and the binding.   

> 
> -Scott
>