Re: [microblaze-uclinux] Re: [microblaze-uclinux] RE: [PATCH v3] Device tree bindings for Xilinx devices

2007-10-23 Thread Michal Simek 
Hi David,
I remove some labels from my generator. I created fake system with some 
peripherals. 
There are 3 buses and 3 bridges. 
Can you check it and tell me what is wrong?

Thanks,
Michal Simek

/ {
model = mONStR;
chosen {
bootargs = root=/dev/xsysace/disc0/part2;
} ;
cpus {
#size-cells = 0;
#cpus =  0 ;
#address-cells = 1;
microblaze_0,[EMAIL PROTECTED] {
device_type = cpu;
reg = 0;
clock-frequency = 5f5e1000;
timebase-frequency = 1FCA055;
i-cache-line-size = 2000;
i-cache-size = 10;
d-cache-line-size = 2000;
d-cache-size = 10;
xilinx,pvr = 0;
xilinx,debug-enabled = 1;
xilinx,fsl-links = 0;
} ;
} ;

[EMAIL PROTECTED] {
compatible = opb_ethernet_1.04.a,opb_ethernet;
interrupts =  3 0 ;
reg =  1006 1 ;
device_type = network;
xilinx,cam-exist = 0;
xilinx,dev-blk-id = 1;
xilinx,dev-mir-enable = 1;
xilinx,dma-present = 1;
xilinx,include-dev-pencoder = 1;
xilinx,ipif-rdfifo-depth = 8000;
xilinx,ipif-wrfifo-depth = 8000;
xilinx,mii-exist = 1;
} ;
[EMAIL PROTECTED] {
memreg:reg =  2000 1000 ;
device_type = memory;
} ;
[EMAIL PROTECTED] {
compatible = opb_uart16550_1.00.d,opb_uart16550;
reg =  1003 1 ;
device_type = serial;
} ;
[EMAIL PROTECTED] {
compatible = opb_timer_1.00.b,opb_timer;
interrupts =  0 0 ;
reg =  1002 1 ;
xilinx,count-width = 20;
xilinx,one-timer-only = 0;
} ;
[EMAIL PROTECTED] {
ranges =  0 3000 1000 ;
[EMAIL PROTECTED] {
compatible = opb_gpio_3.01.b,opb_gpio;
reg =  3002 1 ;
xilinx,gpio-width = 4;
xilinx,is-dual = 0;
} ;
[EMAIL PROTECTED] {
compatible = opb_iic_1.02.a,opb_iic;
reg =  3003 1 ;
device_type = i2c;
} ;

[EMAIL PROTECTED] {
compatible = opb_gpio_3.01.b,opb_gpio;
reg =  3001 1 ;
xilinx,gpio-width = 20;
xilinx,is-dual = 0;
} ;
[EMAIL PROTECTED] {
compatible = 
opb_ethernetlite_1.01.b,opb_ethernetlite;
reg =  3004 1 ;
device_type = network;
xilinx,duplex = 1;
xilinx,rx-ping-pong = 0;
xilinx,tx-ping-pong = 0;
} ;
[EMAIL PROTECTED] {
compatible = opb_sysace_1.00.c,opb_sysace;
reg =  3005 1 ;
xilinx,mem-width = 10;
} ;
[EMAIL PROTECTED] {
compatible = 
opb_ps2_dual_ref_1.00.a,opb_ps2_dual_ref;
interrupts =  2 0 ;
interrupts =  1 0 ;
reg =  3006 1 ;
} ;
};
[EMAIL PROTECTED] {
compatible = opb_intc_1.00.c,opb_intc;
reg =  1001 1 ;
} ;
[EMAIL PROTECTED] {
compatible = opb_mdm_2.00.a,opb_mdm;
reg =  1005 1 ;
xilinx,mb-dbg-ports = 1;
xilinx,uart-width = 8;
xilinx,use-uart = 1;
} ;
[EMAIL PROTECTED] {
compatible = opb_uartlite_1.00.b,opb_uartlite;
interrupts =  4 0 ;
reg =  1004 1 ;
device_type = serial;
xilinx,baudrate = 2580;
xilinx,data-bits = 8;
xilinx,clk-freq = 5f5e100;
xilinx,odd-parity = 0;
xilinx,use-parity = 0;
} ;
[EMAIL PROTECTED] {
ranges =  0 8000 8000 ;
[EMAIL PROTECTED] {
compatible = plb_uart16550_1.00.c,plb_uart16550;
reg =  a002 1 ;
device_type = serial;
} ;
[EMAIL PROTECTED] {
compatible =

RE: [microblaze-uclinux] Re: [microblaze-uclinux] RE: [PATCH v3] Device tree bindings for Xilinx devices

2007-10-23 Thread Stephen Neuendorffer 
 

 -Original Message-
 From: 
 [EMAIL PROTECTED]
 g 
 [mailto:[EMAIL PROTECTED]
zlabs.org] On Behalf Of Michal Simek
 Sent: Monday, October 22, 2007 9:08 PM
 To: [EMAIL PROTECTED]
 Cc: Leonid; Wolfgang Reissnegger; Arnd Bergmann; 
 linuxppc-dev@ozlabs.org
 Subject: RE: [microblaze-uclinux] Re: [microblaze-uclinux] 
 RE: [PATCH v3] Device tree bindings for Xilinx devices
 
  In my opinion will be better generate only parameters which 
  you want not all.
  That smells with unusable parameters.
 
 In the long term, this may be true.  In the short term:
 1) dtb size is not the key problem
 Yes of course
 2) making sure that everything works is a key problem.
 3) The code that generates the dts should be as simple as possible,
 so that we can easily document what it does.
 Yes but you must document every parameter which your generate 
 do. The better way is 
 document only parameters which you want use.

No, that's exactly my point.  The generator should document what it
*does*
i.e.  When there is a parameter in the EDK file, then such and such
corresponding parameter will be generated in the dts.  The devices and
drivers
will inevitably change over time: your proposal would result in
an unnecessary maintenance headache...  The documentation of what the
individual
parameters are should be unambiguous from the EDK documentation.

The only things that the generator should handle 'specially', in my
opinion
are parameters that need to be munged to be standard names.

Steve

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [microblaze-uclinux] Re: [microblaze-uclinux] RE: [PATCH v3] Device tree bindings for Xilinx devices

2007-10-23 Thread David Gibson 
On Tue, Oct 23, 2007 at 09:34:37AM +0200, Michal Simek wrote:
 Hi David,
 I remove some labels from my generator. I created fake system with some 
 peripherals. 
 There are 3 buses and 3 bridges. 
 Can you check it and tell me what is wrong?

Grant's comments all seem reasonable, apologies if I've duplicated
some of them below.

 
 Thanks,
 Michal Simek
 
 / {
   model = mONStR;

You should have #address-cells and #size-cells properties.

   chosen {
   bootargs = root=/dev/xsysace/disc0/part2;
   } ;
   cpus {
   #size-cells = 0;
   #cpus =  0 ;
   #address-cells = 1;
   microblaze_0,[EMAIL PROTECTED] {

That name is acceptable, but I think just [EMAIL PROTECTED] would be better.  
The
generic names convention seems to be frequently ignored for cpus, but
I don't see a good reason to.

   device_type = cpu;
   reg = 0;
   clock-frequency = 5f5e1000;
   timebase-frequency = 1FCA055;
   i-cache-line-size = 2000;
   i-cache-size = 10;
   d-cache-line-size = 2000;
   d-cache-size = 10;
   xilinx,pvr = 0;
   xilinx,debug-enabled = 1;
   xilinx,fsl-links = 0;
   } ;
   } ;
 
   [EMAIL PROTECTED] {
   compatible = opb_ethernet_1.04.a,opb_ethernet;
   interrupts =  3 0 ;
   reg =  1006 1 ;
   device_type = network;
   xilinx,cam-exist = 0;
   xilinx,dev-blk-id = 1;
   xilinx,dev-mir-enable = 1;
   xilinx,dma-present = 1;
   xilinx,include-dev-pencoder = 1;
   xilinx,ipif-rdfifo-depth = 8000;
   xilinx,ipif-wrfifo-depth = 8000;
   xilinx,mii-exist = 1;
   } ;
   [EMAIL PROTECTED] {
   memreg:reg =  2000 1000 ;
   device_type = memory;
   } ;
   [EMAIL PROTECTED] {
   compatible = opb_uart16550_1.00.d,opb_uart16550;

Is this serial port actually 16550 compatible?  If so it should have
ns16550 in the compatible list.

   reg =  1003 1 ;
   device_type = serial;
   } ;
   [EMAIL PROTECTED] {
   compatible = opb_timer_1.00.b,opb_timer;
   interrupts =  0 0 ;
   reg =  1002 1 ;
   xilinx,count-width = 20;
   xilinx,one-timer-only = 0;
   } ;
   [EMAIL PROTECTED] {

This is a bus bridge, and so needs #address-cells and #size-cells
properties.  It should also have a compatible property to describe the
type of bridge.


   ranges =  0 3000 1000 ;
   [EMAIL PROTECTED] {
   compatible = opb_gpio_3.01.b,opb_gpio;
   reg =  3002 1 ;

This doesn't look quite right.  The reg property is translated by the
parent's ranges property.  So shouldn't this be just reg = 2
1, with the 3000 added by the parent?

   xilinx,gpio-width = 4;
   xilinx,is-dual = 0;
   } ;
   [EMAIL PROTECTED] {
   compatible = opb_iic_1.02.a,opb_iic;
   reg =  3003 1 ;
   device_type = i2c;

There was talk of an i2c device_type, but I don't think it ever
actually happened.  I think we should drop this.

   } ;
 
   [EMAIL PROTECTED] {
   compatible = opb_gpio_3.01.b,opb_gpio;
   reg =  3001 1 ;
   xilinx,gpio-width = 20;
   xilinx,is-dual = 0;
   } ;
   [EMAIL PROTECTED] {
   compatible = 
 opb_ethernetlite_1.01.b,opb_ethernetlite;
   reg =  3004 1 ;
   device_type = network;
   xilinx,duplex = 1;
   xilinx,rx-ping-pong = 0;
   xilinx,tx-ping-pong = 0;
   } ;
   [EMAIL PROTECTED] {
   compatible = opb_sysace_1.00.c,opb_sysace;
   reg =  3005 1 ;
   xilinx,mem-width = 10;
   } ;
   [EMAIL PROTECTED] {
   compatible = 
 opb_ps2_dual_ref_1.00.a,opb_ps2_dual_ref;
   interrupts =  2 0 ;
   interrupts =  1 0 ;

Uh... duplicate property names here.  Should be
interrupts = 2 0 1 0;

Also you need an interrupt-parent property somewhere.  Either here, or
in one of the ancestor bridges.

   reg =  3006 1 ;
   } ;
   };
   [EMAIL PROTECTED] {
   compatible = opb_intc_1.00.c,opb_intc;
   reg =  1001 1 ;

RE: [microblaze-uclinux] Re: [microblaze-uclinux] RE: [PATCH v3] Device tree bindings for Xilinx devices

2007-10-22 Thread Stephen Neuendorffer 
 -Original Message-
 From: [EMAIL PROTECTED] 
 [mailto:[EMAIL PROTECTED] On Behalf Of 
 Michal Simek
 Sent: Friday, October 19, 2007 7:28 PM
 To: [EMAIL PROTECTED]
 Cc: Stephen Neuendorffer; Grant Likely; Leonid; Arnd 
 Bergmann; linuxppc-dev@ozlabs.org; Wolfgang Reissnegger
 Subject: [microblaze-uclinux] Re: [microblaze-uclinux] RE: 
 [PATCH v3] Device tree bindings for Xilinx devices
 
 Hi Steve and all,
 Here's a full .dts generated using an updated version of
 gen_mhs_devtree.py, following the proposal.
 It happens to be a microblaze system, but you get the idea.
 
 I think that is no good idea generate dts with all information.
 Especially information about PVR - number 2 means - Full PVR 
 and you can
 obtain information directly from PVR. It is waste of memory space.
   xilinx,pvr = 2;

PowerPC does something with the powerpc equivalent of the PVR.
We should just do what they do...
 
 In my opinion will be better generate only parameters which 
 you want not all.
 That smells with unusable parameters.

In the long term, this may be true.  In the short term:
1) dtb size is not the key problem
2) making sure that everything works is a key problem.
3) The code that generates the dts should be as simple as possible,
so that we can easily document what it does.

In the long term, I'm all for optimizing the device tree that gets
built,
assuming that it appears to be a problem in real systems.

Steve

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

RE: [microblaze-uclinux] Re: [microblaze-uclinux] RE: [PATCH v3] Device tree bindings for Xilinx devices

2007-10-22 Thread Michal Simek 
 In my opinion will be better generate only parameters which 
 you want not all.
 That smells with unusable parameters.

In the long term, this may be true.  In the short term:
1) dtb size is not the key problem
Yes of course
2) making sure that everything works is a key problem.
3) The code that generates the dts should be as simple as possible,
so that we can easily document what it does.
Yes but you must document every parameter which your generate do. The better 
way is 
document only parameters which you want use.

In the long term, I'm all for optimizing the device tree that gets
built,
assuming that it appears to be a problem in real systems.
We can optimize now. I made new version of my generator  u-boot_v3_00_a 
(monstr.eu)
where you can simply add parameter which you want to use. 

If you want use any parameter add it. 
For microblaze cpu - line 1184. 
And for others peripherals lines 926-980. (The last parameter of function call).

Which parameters do you want for PPC405, Grant?

Regards,
Michal Simek

This is example of generator.

/ {
model = mONStR;
chosen {
linux,platform = 600;
bootargs = root=/dev/xsysace/disc0/part2;
} ;
cpus {
#size-cells = 0;
#cpus =  0 ;
#address-cells = 1;
microblaze_0,5.00.c {
32-bit;
linux,boot-cpu;
device_type = cpu;
reg = 0;
clock-frequency = 5f5e1000;
timebase-frequency = 1FCA055;
i-cache-line-size = 2000;
i-cache-size = 10;
d-cache-line-size = 2000;
d-cache-size = 10;
xilinx,pvr = 0;
xilinx,debug-enabled = 1;
xilinx,fsl-links = 0;
} ;
} ;

[EMAIL PROTECTED] {
compatible = opb_mdm_2.00.a\0opb_mdm;
name = debug_module;
reg =  4140 1 ;
device_type = opb_mdm;
xilinx,mb-dbg-ports = 1;
xilinx,uart-width = 8;
xilinx,use-uart = 1;
} ;
[EMAIL PROTECTED] {
compatible = opb_uartlite_1.00.b\0opb_uartlite;
name = RS232_Uart_1;
interrupts =  4 0 ;
reg =  4060 1 ;
device_type = opb_uartlite;
xilinx,baudrate = 2580;
xilinx,data-bits = 8;
xilinx,clk-freq = 5f5e100;
xilinx,odd-parity = 0;
xilinx,use-parity = 0;
} ;
[EMAIL PROTECTED] {
compatible = opb_ethernet_1.04.a\0opb_ethernet;
name = Ethernet_MAC;
interrupts =  3 0 ;
reg =  40c0 1 ;
device_type = opb_ethernet;
xilinx,cam-exist = 0;
xilinx,dev-blk-id = 1;
xilinx,dev-mir-enable = 1;
xilinx,dma-present = 1;
xilinx,include-dev-pencoder = 1;
xilinx,ipif-rdfifo-depth = 8000;
xilinx,ipif-wrfifo-depth = 8000;
xilinx,mii-exist = 1;
} ;
[EMAIL PROTECTED] {
compatible = opb_sysace_1.00.c\0opb_sysace;
name = SysACE_CompactFlash;
reg =  4182 1 ;
device_type = opb_sysace;
xilinx,mem-width = 10;
} ;
[EMAIL PROTECTED] {
compatible = opb_gpio_3.01.b\0opb_gpio;
name = LEDs_4Bit;
reg =  4000 1 ;
device_type = opb_gpio;
xilinx,gpio-width = 4;
xilinx,is-dual = 0;
} ;
[EMAIL PROTECTED] {
edk_name = DDR_256MB_32MX64_rank1_row13_col10_cl2_5;
compatible = opb_ddr;
memreg:reg =  3000 1000 ;
device_type = memory;
} ;
[EMAIL PROTECTED] {
compatible = opb_ps2_dual_ref_1.00.a\0opb_ps2_dual_ref;
name = PS2_Ports;
interrupts =  2 0 ;
interrupts =  1 0 ;
reg =  7a40 1 ;
device_type = opb_ps2_dual_ref;
} ;
[EMAIL PROTECTED] {
compatible = opb_timer_1.00.b\0opb_timer;
name = opb_timer_1;
interrupts =  0 0 ;
reg =  41c0 1 ;
device_type = opb_timer;
xilinx,count-width = 20;
xilinx,one-timer-only = 0;
} ;
[EMAIL PROTECTED] {
compatible = opb_intc_1.00.c\0opb_intc;
name = opb_intc_0;
reg =  4120 1 ;
device_type = opb_intc;

Re: [microblaze-uclinux] Re: [microblaze-uclinux] RE: [PATCH v3] Device tree bindings for Xilinx devices

2007-10-22 Thread David Gibson 
On Tue, Oct 23, 2007 at 06:07:56AM +0200, Michal Simek wrote:
  In my opinion will be better generate only parameters which 
  you want not all.
  That smells with unusable parameters.
 
 In the long term, this may be true.  In the short term:
 1) dtb size is not the key problem
 Yes of course
 2) making sure that everything works is a key problem.
 3) The code that generates the dts should be as simple as possible,
 so that we can easily document what it does.
 Yes but you must document every parameter which your generate do. The better 
 way is 
 document only parameters which you want use.
 
 In the long term, I'm all for optimizing the device tree that gets
 built,
 assuming that it appears to be a problem in real systems.
 We can optimize now. I made new version of my generator  u-boot_v3_00_a 
 (monstr.eu)
 where you can simply add parameter which you want to use. 
 
 If you want use any parameter add it. 
 For microblaze cpu - line 1184. 
 And for others peripherals lines 926-980. (The last parameter of function 
 call).
 
 Which parameters do you want for PPC405, Grant?
 
 Regards,
 Michal Simek
 
 This is example of generator.
 
 / {
   model = mONStR;

   chosen {
   linux,platform = 600;

linux,platform is long obsolete.  Kill it.

   bootargs = root=/dev/xsysace/disc0/part2;
   } ;
   cpus {
   #size-cells = 0;
   #cpus =  0 ;
   #address-cells = 1;
   microblaze_0,5.00.c {

Still missing an @0 here.  

   32-bit;
   linux,boot-cpu;

32-bit and linux,boot-cpu are both obsolete (the first was never
specified anywhere) and should be removed.

   device_type = cpu;
   reg = 0;
   clock-frequency = 5f5e1000;
   timebase-frequency = 1FCA055;
   i-cache-line-size = 2000;
   i-cache-size = 10;
   d-cache-line-size = 2000;
   d-cache-size = 10;
   xilinx,pvr = 0;
   xilinx,debug-enabled = 1;
   xilinx,fsl-links = 0;
   } ;
   } ;
 
   [EMAIL PROTECTED] {
   compatible = opb_mdm_2.00.a\0opb_mdm;

Please use the new opb_mdm_2.00.a, opb_mdm syntax.

   name = debug_module;

Don't create properties called 'name'.  In real OF the 'name' property
is a synonym for the node name without the @address part.  Although
it's possible to encode a name property in a flattened tree with a
different value, it's a bad idea since it will clash with the OF
notion of this property.  In Linux this will cause a collision when
the tree is unflattened.

We should probably fix dtc to reject this, in fact.

   reg =  4140 1 ;
   device_type = opb_mdm;

Get rid of all your device_type values, they're all bogus.  The only
devices which should have device_type at all are the ethernets and
maybe the uarts, and in these cases the values should be network and
serial respectively.

   xilinx,mb-dbg-ports = 1;
   xilinx,uart-width = 8;
   xilinx,use-uart = 1;
   } ;
   [EMAIL PROTECTED] {
   compatible = opb_uartlite_1.00.b\0opb_uartlite;
   name = RS232_Uart_1;
   interrupts =  4 0 ;
   reg =  4060 1 ;
   device_type = opb_uartlite;
   xilinx,baudrate = 2580;
   xilinx,data-bits = 8;
   xilinx,clk-freq = 5f5e100;
   xilinx,odd-parity = 0;
   xilinx,use-parity = 0;
   } ;
   [EMAIL PROTECTED] {

The generic names convention means this should be called
[EMAIL PROTECTED]


   compatible = opb_ethernet_1.04.a\0opb_ethernet;
   name = Ethernet_MAC;
   interrupts =  3 0 ;
   reg =  40c0 1 ;
   device_type = opb_ethernet;
   xilinx,cam-exist = 0;
   xilinx,dev-blk-id = 1;
   xilinx,dev-mir-enable = 1;
   xilinx,dma-present = 1;
   xilinx,include-dev-pencoder = 1;
   xilinx,ipif-rdfifo-depth = 8000;
   xilinx,ipif-wrfifo-depth = 8000;
   xilinx,mii-exist = 1;
   } ;
   [EMAIL PROTECTED] {
   compatible = opb_sysace_1.00.c\0opb_sysace;
   name = SysACE_CompactFlash;
   reg =  4182 1 ;
   device_type = opb_sysace;
   xilinx,mem-width = 10;
   } ;
   [EMAIL PROTECTED] {
   compatible = opb_gpio_3.01.b\0opb_gpio;
   name = LEDs_4Bit;
   reg =  4000 1 ;
   device_type = opb_gpio;
   xilinx,gpio-width = 4;
   xilinx,is-dual = 0;
   } ;
   [EMAIL PROTECTED] {
   edk_name = DDR_256MB_32MX64_rank1_row13_col10_cl2_5;