On Tue, Dec 04, 2007 at 01:50:32PM +1100, David Gibson wrote:
> On Mon, Dec 03, 2007 at 07:10:26PM -0700, Mark A. Greer wrote:
> > On Mon, Dec 03, 2007 at 12:50:18PM +1100, David Gibson wrote:
> > > On Thu, Nov 29, 2007 at 06:28:36PM +0300, Andrei Dolnikov wrote:
> > > > + eth0 {
> > > > + device_type = "network";
> > > > + compatible = "marvell,mv64x60-eth";
> > > > + block-index = <0>;
> > >
> > > This block-index thing is crap. If you really need to subindex nodes
> > > like this, use "reg", with an appropriate #address-cells in the
> > > parent, then the nodes will also get sensible unit addresses.
> >
> > So how would that work for the "PHY Address Register 0x2000", say,
> > where bits 0-4 set the device addr for PHY 0; bits 5-9 set the device
> > addr for PHY 1; bts 10-14 set the devce addr for PHY 2?
>
> So use 'reg' to do the indexing. As long as you have no 'ranges'
> property in the parent 'ethernet' node, which you don't, you can use
> 'reg' as a private index. That's basically what non-translatable reg
> values are about.
>
> Incidentally you should probably call the subnodes "[EMAIL PROTECTED]"
> etc. and the parent one "multiethernet" or something. It's the
> subnodes that represent an individual ethernet interface, so they
> should take the "ethernet" name and not the parent, by generic names
> conventions.
Okay, thanks for the advice. I'll fix the prpmc2800 dts file.
Presumably Andrei will fix his.
> [snip]
> > > > + [EMAIL PROTECTED] {
> > > > + compatible = "marvell,mv64x60-sdma";
> > > > + reg = <4000 c18>;
> > > > + virtual-reg = <f8104000>;
> > >
> > > Why does this node have virtual-reg?
> >
> > "virtual-reg" is a special property that doesn't get translated thru
> > the parent mappings. It should never be used in the kernel. Its
> > purpose is to give code in the bootwrapper the exact address that it
> > should use to access a register or block of registers or ...
> > Its needed here because the MPSC (serial) driver uses the SDMA unit
> > to perform console I/O in the bootwrapper (e.g., cmdline editing, printf's).
> >
> > Yes, this needs to be documented.
>
> Ok. "it's used for serial in the bootwrapper" would have sufficed - I
> questioned it because it wasn't obvious that this was needed to use
> the mpsc.
Sorry :)
> >
> > > > + [EMAIL PROTECTED] {
> > > > + device_type = "serial";
> > > > + compatible = "marvell,mpsc";
> > > > + reg = <8000 38>;
> > > > + virtual-reg = <f8108000>;
> > > > + sdma = <&/mv64x60/[EMAIL PROTECTED]>;
> > > > + brg = <&/mv64x60/[EMAIL PROTECTED]>;
> > > > + cunit = <&/mv64x60/[EMAIL PROTECTED]>;
> > > > + mpscrouting = <&/mv64x60/[EMAIL PROTECTED]>;
> > > > + mpscintr = <&/mv64x60/[EMAIL PROTECTED]>;
> > > > + block-index = <0>;
> > >
> > > What is this block-index thing about here? Since the devices are
> > > disambiguated by their register address, why do you need it?
> >
> > This particular one is needed to access the correct MPSC interrupt reg.
> > Maybe it would be better to make a new property for this but it was only
> > one reg and block-index was already there and basically served that
> > purpose so I used it. I'd be happy to use an alternative if you have
> > something you think is better.
>
> No, that's an acceptable use for something like this, except that
> "cell-index" seems to be the name we've standardised on for other
> similar cases.
Yeah, I realize that but block-index was here first!
More seriously, I don't like "cell" because it isn't a cell, its a
block or an instance or an...I dunno but its not a cell IMHO.
Anyway, I'll think about changing it to cell but I already feel
dirty just thinking about it.
Mark
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
