Hi, Segher,=20

=20

quoted

quoted

quoted

+    - device_type : Should be "rapidio"

There is no OF binding, so no.

So, we need to define it.

=20
If you want to.  Until that has been done, don't use
a "device_type".  Linux won't use it, anyway.

Do you have another ideas about that? Only remove it?

=20

quoted

quoted

quoted

+    - compatible : Should be "fsl,rapidio-v0.0" or

"fsl,rapidio-v1.0"

quoted

+      and so on. The version number is got from IP Block Revision
+      Register of RapidIO controller.

It's better to use real device names, just like everyone
else.

Some silicons of Freescale processor are the same RapidIO=20

controller,

quoted

such as mpc8540/mpc8560 are the same (v0.0), mpc8548/mpc8641 are the
same (v1.0). For v1.0 RapidIO controller, should we use mpc8548 or
mpc8641? Those will make people confused.

=20
Not at all.  On an 8641 it could be
=20
	compatible =3D "fsl,mpc8641-rapidio" "fsl,mpc8548-rapidio";
=20
which states "this is the 8641 thing and it is compatible
to the 8548 thing".  Perfectly clear.
=20

quoted

Using IP Block Revision is a
clear choice.

=20
I don't think so.  For one thing, it describes a version of
a cell design, not a version of an actual device.  For another
thing, if I hear "8641" I know what you're talking about (sort
of, anyway), but I draw a blank stare if you say "v1.0".  I'm
sure I'm not the only one.  Concrete names are good.
=20

From the different view ways, there are different results. Getting the

version from RapidIO IP revision register is clear to me. :)

quoted

quoted

quoted

+    - #address-cells : Address representation for

"rapidio" devices.

quoted

+      This field represents the number of cells needed=20

to represent

quoted

quoted

quoted

+      the RapidIO address of the registers.  For

supporting more than

quoted

+      36-bits RapidIO address, this field should be <2>.

More than 32 bit?

Yes, RapidIO bus address width is 34 bits.

=20
You said "more than 36 bit", I tried to ask if that is a typo
perhaps.

Ya, caught by you! I'll fix it in next version. :)

=20

quoted

quoted

No.  The format of an "interrupts" entry is defined by
the interrupt domain this device sits in, not by the
device itself.

Do you misunderstand the meaning of 'interrupts'?

=20
Hahaha.  No, I don't misunderstand what the "interrupts" property
means.  Perhaps you do?
=20

quoted

These interrupts is
issued from the RapidIO controller to the pic controller for tx, rx,
err, doorbell and message.

=20
But the rapidio node doesn't know or care what the interrupts
are connected to, and neither should it.  That's what the
interrupt mapping recommended practice is for.
=20

There are no rapidio device in it. Doorbell, msg are all parts of
rapidio controller.
For example, 8641 rapidio controller have 2 msg unit: msg0 and msg1.
They are not rapidio devices. Each msg unit has the tx_irq and rx_irq.

quoted

quoted

quoted

For this sector, interrupts order should be
+      <err_irq bell_outb_irq bell_inb_irq msg1_tx_irq msg1_rx_irq
+      msg2_tx_irq msg2_rx_irq ... msgN_tx_irq msgN_rx_irq>.

That's to be defined in the binding for your specific device,
not in a more generic rapidio binding.

These description is just for compatible=3D"fsl,rapidio-v*.*" =

rapidio

quoted

controller.

=20
Okay, good.  Please make that way more obvious then :-)
=20

quoted

quoted

quoted

+		#address-cells =3D <2>;

You want a #size-cells as well.

The size is not used in this sector, so no defined.

=20
The size _is_ used; in the "ranges" property in this node,
for example.  It is also needed to describe the "reg" for
any child node of this node.
=20
A non-existant "#size-cells" means 1, and "#address-cells"
means 2, so in principle you could do without these
properties; but Linux doesn't parse the tree correctly in
that case (which reminds me, I have some more patches to
send).
=20

Ok, I'll add it in the next version for more religious.

Thanks!
Wei.