[RFC 3/3] powerpc: Add DTS file for the Motorola PrPMC2800 platform
Yoder Stuart-B08248
stuart.yoder at freescale.com
Thu Mar 29 02:43:16 EST 2007
Biggest concern is that I see a lot of undocumented device
types and properties...
Device types that I do not recognize include:
+ device_type = "brg";
+ device_type = "mv64x60-cunit";
+ device_type = "mpscrouting";
+ device_type = "mpscintr";
+ device_type = "mv64x60-pic";
+ device_type = "mv64x60-mpp";
+ device_type = "mv64x60-gpp";
+ device_type = "mv64x60-cpu-error";
+ device_type = "mv64x60-sram-error";
+ device_type = "mv64x60-pci-error";
+ device_type = "mv64x60-memctrl";
These should ideally be documented with required properties
and what they mean.
I think a document similar to Grant Likely's
Documentation/mpc52xx-device-tree-bindings.txt would be
very helpful here. People using this DTS as a starting
point are going to have a lot of questions.
comments inline below--
[snip]
> +
> + mv64x60 at f1000000 { /* Marvell Discovery */
> + #address-cells = <1>;
> + #size-cells = <1>;
> + #interrupt-cells = <1>;
> + device_type = "mv64360";
> + compatible = "mv64x60";
> + clock-frequency = <7f28155>; /*
> 133.333333 Mhz */
> + reg = <f1000000 00010000>;
> + virtual-reg = <f1000000>;
What is virtual-reg for? I gather it is related to the bootwrapper
needed virtual addresses for devices reg blocks. But, I counted
12 instances of the property in this file. Are all needed by the
bootwrapper??
Does anyone where this property came from historically? If it's
specifically for a Linux bootwrapper the name should reflect
that-- linux,bootwrapper-reg-vaddr -- or something...
I also wonder if there is a better place to put this information.
The device nodes strictly speaking should be describing the devices
_physical_ characteristics.
It also needs to be documented in booting-without-of.txt.
[snip]
> + reg = <2000 2000>;
> + virtual-reg = <f1002000>;
> + eth0 {
> + device_type = "network";
> + compatible = "mv64x60-eth";
> + block-index = <0>;
> + interrupts = <20>;
> + interrupt-parent = <&/mv64x60/pic>;
> + phy = <&/mv64x60/mdio/ethernet-phy at 1>;
> + local-mac-address = [ 00 00 00
> 00 00 00 ];
> + };
> + eth1 {
> + device_type = "network";
> + compatible = "mv64x60-eth";
> + block-index = <1>;
> + interrupts = <21>;
> + interrupt-parent = <&/mv64x60/pic>;
> + phy = <&/mv64x60/mdio/ethernet-phy at 3>;
> + local-mac-address = [ 00 00 00
> 00 00 00 ];
> + };
> + };
Based on some offline dicussions I think I know what block-index
is, but I'd like to see this property documented in
booting-without-of.txt.
[snip]
> + i2c at c000 {
> + device_type = "i2c";
> + compatible = "mv64x60-i2c";
> + reg = <c000 20>;
> + virtual-reg = <f100c000>;
> + freq_m = <8>;
> + freq_n = <3>;
> + timeout = <3e8>; /* 1000
If freq_m and freq_n are Marvell specific should have
a "Marvel," prepended to them?
[snip]
> + pci at 80000000 {
> + #address-cells = <3>;
> + #size-cells = <2>;
> + #interrupt-cells = <1>;
> + device_type = "pci";
> + compatible = "mv64x60-pci";
> + reg = <0cf8 8>;
> + ranges = <01000000 0 0 88000000
> 0 01000000
> + 02000000 0 80000000 80000000
> 0 08000000>;
> + bus-range = <0 ff>;
> + clock-frequency = <3EF1480>;
> + interrupt-pci-iack = <0c34>;
I don't think interrupt-pci-iack is standard. If it's generally
applicable add it to booting-without-of.txt. If its MV specific
prepend with "Marvel,".
Regards,
Stuart Yoder
More information about the Linuxppc-dev
mailing list