On Thu, 2013-09-26 at 04:27 -0500, Xie Xiaobo-R63061 wrote: > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: Thursday, September 26, 2013 7:10 AM > > To: Xie Xiaobo-R63061 > > Cc: Wood Scott-B07421; linuxppc-dev@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@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@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@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@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. > > > > > > > > > > > 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,
The device tree describes the hardware, not the current state of Linux drivers. Especially drivers that aren't yet in Linux. :-) > 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. Ideally all devices (and bindings) should be described when the device tree is initally added, regardless of whether you have a driver yet. -Scott _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev