Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Phil Terry]

Guys n Gals,

I'm new to this list so I hope this isn't the wrong place to be asking
these questions.

I'm using the MPC8641D in a Serial RapidIO backplane chassis design.

My last foray into kernel development (privately in an embedded company)
was back in 2.6.9 to 2.6.11 days for a MPC8540 so be gentle with me if
I've missed something in the intervening kernels. I've tried to catch up
as best I can with the archives.

So I thought it would be a simple task of taking Matt Porter's ppc rio
stuff and porting it to powerpc but then I discovered this dts/open
firmware thingy...

So I want to explain an overview of what I intend to do for this port
and if anyone can either tell me where its already been done (yippee),
tell me I'm on the right track (ok) or tell me I'm on the wrong track
(bad) that would be great.

So I'm taking the boot/dts/mpc8641_hpcn.dts and producing a new
mpc8641D_umem.dts with the following addition to the soc.

srio@c0000 {
                device_type = "srio";
                compatible = "86xx,85xx";
		reg = <c0000 20000>;
		law = <400000000 e00000000>;
		dbells = <0 ffff>;
		mboxs = <0 4>;
                interrupt-parent = <&mpic>;
                interrupts = <30 1 31 1 32 1 35 1 36 1 37 1 38 1>;
              };

where law is the 36-bit start/end address of the law for rapidio (yes I
do want to use 36-bit addressing as well but thats another story),
dbells is the range of doorbells to use and mboxs is the range of
mailboxes.

Then I can copy the old ppc/kernel/rio.c to powerpc/kernel/rio.c and
change...

void platform_rio_init(void)
{
	struct device_node *np;
	if ( (np = of_find_compatible_node(np, "srio", "86xx")) != NULL ) {
		mpc86xx_rio_setup(np);
	}
	else {
		printk(KERN_INFO "RIO: No platform_rio_init() present in dts\n");
	}
}

with mpc86xx_rio_setup being the old mpc85xx_rio_setup from
ppc/syslib/ppc85xx_rio.c modified to extract the laws, doorbell
resources, mailboxes etc, from the of_get_property instead of
hard-coding them.

/* void mpc86xx_rio_setup(int law_start, int law_size) */

void mpc86xx_rio_setup(struct device_node *np)
{
	...

	/*port->iores.start = law_start;*/
        port->iores.start = of_get_number(of_get_propert(np,"laws"));
	
	...
	/* mpc86xx_rio_doorbell_init(port); */
	mpc86xx_rio_doorbell_init(np,port);

}
etc, etc.

Then I should pass np into the various setup routines as above so that
they can find the interrupts, etc.

Is this the right kind of flavor or have I misunderstood how the dtb
stuff is supposed to integrate with the susbsys_initcall stuff. I'm
assuming we are supposed to do away with all the CONFIG_RAPIDIO,
CONFIG_MPCxyz etc so that the kernel is driven by the dtb?

Any pointers gratefully accepted.

Cheers
Phil

btw how to I get the dtc to accept 36-bit numbers as above for laws?
and am I allowed to invent properties like this or is there some keeper
of the property names who I should genuflect before?

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Segher Boessenkool]

> srio@c0000 {

Isn't there a nicer name, like "rapidio"?

>                 device_type = "srio";

No device_type please.

>                 compatible = "86xx,85xx";

compatible = "fsl,8641d-rapidio", "fsl,8540-rapidio";
and the latter only if the 8641d can actually be driven
as-if it were a 8540.

> 		reg = <c0000 20000>;
> 		law = <400000000 e00000000>;

What does "law" mean?  Please use a better name.  And
you write this as <4 0 e 0>.

> 		dbells = <0 ffff>;

"doorbells"

> 		mboxs = <0 4>;

"mailboxes"

> I'm
> assuming we are supposed to do away with all the CONFIG_RAPIDIO,

You still need config options to enable support for
rapidio etc.  The DTB is used at runtime to detect
what devices are actually there.

> btw how to I get the dtc to accept 36-bit numbers as above for laws?

The numbers inside < > are 32-bit integers.  64-bit
numbers are encoded as a pair of these.

> and am I allowed to invent properties like this or is there some keeper
> of the property names who I should genuflect before?

It would be nice to have a proper device binding for
rapidio, just like for PCI etc.  Since there isn't one
yet, try to invent one that would work for _all_ rapidio
hosts, so there won't be too many changes needed if
later a more generic device binding evolves.

Don't worry about it too much, just use common sense,
and you'll end up with an ad-hoc binding for this device
only that will work just fine; do the generic thing
later, when you have at least a chance to get it right ;-)


Segher

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Timur Tabi]

Phil Terry wrote:

> 
> srio@c0000 {
>                 device_type = "srio";
>                 compatible = "86xx,85xx";
> 		reg = <c0000 20000>;
> 		law = <400000000 e00000000>;
> 		dbells = <0 ffff>;
> 		mboxs = <0 4>;
>                 interrupt-parent = <&mpic>;
>                 interrupts = <30 1 31 1 32 1 35 1 36 1 37 1 38 1>;
>               };
> 
> where law is the 36-bit start/end address of the law for rapidio (yes I

Generally, the LAWs are configured in the bootloader, which is normally U-Boot.



-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Arnd Bergmann]

On Tuesday 22 May 2007, Phil Terry wrote:
> Guys n Gals,
> 
> I'm new to this list so I hope this isn't the wrong place to be asking
> these questions.

Hi Phil,

It's the right place, welcome here!

> So I want to explain an overview of what I intend to do for this port
> and if anyone can either tell me where its already been done (yippee),
> tell me I'm on the right track (ok) or tell me I'm on the wrong track
> (bad) that would be great.

I think you are on the right track. It depends a lot on what degree of
probing is possible on rapidio. For most of the bus types that we have
recently added, like plb5 or siliconbackplane, this would be "not at all",
but the code in drivers/rapidio suggests that this one is doing
it correctly and it allows you to probe every device on the bus as long
as you can find the bus controller.

> So I'm taking the boot/dts/mpc8641_hpcn.dts and producing a new
> mpc8641D_umem.dts with the following addition to the soc.
> 
> srio@c0000 {
>                 device_type = "srio";
>                 compatible = "86xx,85xx";
> 		reg = <c0000 20000>;
> 		law = <400000000 e00000000>;
> 		dbells = <0 ffff>;
> 		mboxs = <0 4>;
>                 interrupt-parent = <&mpic>;
>                 interrupts = <30 1 31 1 32 1 35 1 36 1 37 1 38 1>;
>               };
>
> where law is the 36-bit start/end address of the law for rapidio (yes I
> do want to use 36-bit addressing as well but thats another story),
> dbells is the range of doorbells to use and mboxs is the range of
> mailboxes.

This sounds like the "law" should actually be part of the reg property,
and both should be relative to the parent bus. The way you define
"reg" seems to be relative to some register area, which is not how
it's meant (depends on the parent of the srio device).

I would guess that the reg property should look more like

	reg = <0 abc0000 0 20000 /* regs_win */
	        4 0 0 400000	 /* maint_win */
		4 400000 0 10000 /* dbell_win */
		4 0 a 0>; /* all the law area */

If you want to describe the control registers being mapped at
0x000000000abc0000 and the law area at 0x0000000400000000, in the
global 36 bit address space, and the registers you want to
ioremap.

With this, your code can be simplified to
	regs_win = of_iomap(node, 0);
	maint_win = of_iomap(node, 1);
	dbell_wind = of_iomap(node, 2);

Since the law appears to define the address space for any
devices behind the rapidio bridge, it might also be an option
to define a "ranges" property that sets up a mapping between
addresses on the two sides of the bridge, like

     #address-cells <2>; /* 64 bit addresses behind the bridge */
     #size-cells <2>; /* 64 bit address length behind the bridge */
     ranges <0 0 4 0 a 0>; /* bus address 0 gets mapped to host
            address 400000000, length a00000000 */

This would be the only way if you wanted to describe rapidio
devices as child nodes, but if you don't do that, there may not
really be much value-add over simply defining a reg property.

What about the hdid? There is currently code that scans the kernel
command line for this, which would make it a candidate to get
written into the device tree by the boot wrapper instead.

> Then I can copy the old ppc/kernel/rio.c to powerpc/kernel/rio.c and
> change...
> 
> void platform_rio_init(void)
> {
> 	struct device_node *np;
> 	if ( (np = of_find_compatible_node(np, "srio", "86xx")) != NULL ) {
> 		mpc86xx_rio_setup(np);
> 	}
> 	else {
> 		printk(KERN_INFO "RIO: No platform_rio_init() present in dts\n");
> 	}
> }
> 
> with mpc86xx_rio_setup being the old mpc85xx_rio_setup from
> ppc/syslib/ppc85xx_rio.c modified to extract the laws, doorbell
> resources, mailboxes etc, from the of_get_property instead of
> hard-coding them.

There is no point in scanning all the device tree yourself for this,
just make sure you have the rapidio device in a place that gets
automatically scanned with of_platform_bus_probe at boot time,
and then register an of_platform_driver that matches your
compatible property.

The code from platform_rio_init then gets converted to the
->probe method of the of_platform_driver.
 
> /* void mpc86xx_rio_setup(int law_start, int law_size) */
> 
> void mpc86xx_rio_setup(struct device_node *np)
> {
> 	...
> 
> 	/*port->iores.start = law_start;*/
>         port->iores.start = of_get_number(of_get_propert(np,"laws"));

of_address_to_resource(np, 4, &port->iores);

> 	...
> 	/* mpc86xx_rio_doorbell_init(port); */
> 	mpc86xx_rio_doorbell_init(np,port);
> 
> }
> etc, etc.
> 
> Then I should pass np into the various setup routines as above so that
> they can find the interrupts, etc.
> 
> Is this the right kind of flavor or have I misunderstood how the dtb
> stuff is supposed to integrate with the susbsys_initcall stuff.

the thing you are missing is probably the of_platform_driver. Your
driver should have something like

static struct of_device_id mpc85xx_rio_ids[] = {
	{ .compatible = "fsl,8641d-rapidio", .data = RIO_8641 },
	{ .compatible = "fsl,8540-rapidio", .data = RIO_8540 },
	{ },
};
static struct of_platform_driver mpc85xx_rio_driver = {
	.name = "mpc85xx_rio",
	.match_table = &mpc85xx_rio_ids,
	.probe = mpc85xx_rio_probe,
};
static int __init mpc85xx_rio_init(void)
{
	return of_register_platform_driver(&mpc85xx_rio_driver);
}
subsys_initcall(mpc85xx_rio_init);

> I'm assuming we are supposed to do away with all the CONFIG_RAPIDIO,
> CONFIG_MPCxyz etc so that the kernel is driven by the dtb?

CONFIG_RAPIDIO is still needed to determine whether the code gets compiled
in, but you should make sure that there is no harm in enabling it
on a platform that doesn't actually have rapidio devices.

	Arnd <><

RE: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Zhang Wei-r63237]

> So I'm taking the boot/dts/mpc8641_hpcn.dts and producing a new
> mpc8641D_umem.dts with the following addition to the soc.
>=20
> srio@c0000 {
>                 device_type =3D "srio";
>                 compatible =3D "86xx,85xx";
> 		reg =3D <c0000 20000>;
> 		law =3D <400000000 e00000000>;

Please use range =3D <0 address_start size>

> 		dbells =3D <0 ffff>;
> 		mboxs =3D <0 4>;

The dbells and mboxs can be removed. The default setting in rio is okay.

>                 interrupt-parent =3D <&mpic>;
>                 interrupts =3D <30 1 31 1 32 1 35 1 36 1 37 1 38 1>;
>               };
>=20
Do you really use all of this interrupts? In my test, three <32 2 35 2
36 2> are okay, and the sense is 2.

I've finished the porting RIO from ppc to powerpc, do you need some
helps?

Best Regards,
Zhang Wei

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Mark A. Greer]

On Wed, May 23, 2007 at 09:26:59PM +0800, Zhang Wei-r63237 wrote:
> I've finished the porting RIO from ppc to powerpc, do you need some
> helps?

Care to share the patches?

Mark

RE: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Zhang Wei-r63237]

Hi, Mark

Yes, I'm glad to share it. I have added the memory mapping codes into
it.

These codes are reviewing and testing with our engineers. Could you
waiting for a little while? It will be soon.

Best Regards,
Zhang Wei

> -----Original Message-----
> From: Mark A. Greer [mailto:mgreer@mvista.com]=20
> Sent: Wednesday, May 23, 2007 9:33 PM
> To: Zhang Wei-r63237
> Cc: pterry@micromemory.com; linuxppc-dev@ozlabs.org
> Subject: Re: Porting RapidIO from ppc arch to powerpc arch in=20
> support of MPC8641D
>=20
> On Wed, May 23, 2007 at 09:26:59PM +0800, Zhang Wei-r63237 wrote:
> > I've finished the porting RIO from ppc to powerpc, do you need some
> > helps?
>=20
> Care to share the patches?
>=20
> Mark
>=20

RE: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Kumar Gala]

On Wed, 23 May 2007, Zhang Wei-r63237 wrote:

> > So I'm taking the boot/dts/mpc8641_hpcn.dts and producing a new
> > mpc8641D_umem.dts with the following addition to the soc.
> >
> > srio@c0000 {
> >                 device_type = "srio";
> >                 compatible = "86xx,85xx";

We really need to think about this, is their really any difference between
srio and prio from a software view point?

> > 		reg = <c0000 20000>;
> > 		law = <400000000 e00000000>;
>
> Please use range = <0 address_start size>

The law should really be removed.

>
> > 		dbells = <0 ffff>;
> > 		mboxs = <0 4>;
>
> The dbells and mboxs can be removed. The default setting in rio is okay.

this could possibly be useful.

>
> >                 interrupt-parent = <&mpic>;
> >                 interrupts = <30 1 31 1 32 1 35 1 36 1 37 1 38 1>;
> >               };
> >
> Do you really use all of this interrupts? In my test, three <32 2 35 2
> 36 2> are okay, and the sense is 2.

I think we need to list all the interrupts possible from RIO, not just the
ones the driver happens to use.

- k

RE: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Phil Terry]

On Wed, 2007-05-23 at 09:21 -0500, Kumar Gala wrote:
> On Wed, 23 May 2007, Zhang Wei-r63237 wrote:
> 
> > > So I'm taking the boot/dts/mpc8641_hpcn.dts and producing a new
> > > mpc8641D_umem.dts with the following addition to the soc.
> > >
> > > srio@c0000 {
> > >                 device_type = "srio";
> > >                 compatible = "86xx,85xx";
> 
> We really need to think about this, is their really any difference between
> srio and prio from a software view point?
I didn't mean to imply that, the above was just my strawman for
discussion purposes in my overview question. As per a suggestion I was
now calling it rapidio@c0000.

But maybe this is moot, the patches are on their way (yippee) Thanks
Zhang.

> 
> > > 		reg = <c0000 20000>;
> > > 		law = <400000000 e00000000>;
> >
> > Please use range = <0 address_start size>
> 
> The law should really be removed.
In my application I'm may be using a huge part of the 36-bit address
space to address multiple remote RIO boards in a peer DMA multicomputer
application. The default law just for maintenance messages isn't going
to cut it. Now I suppose we could say that my DMA oriented driver should
be responsible for another law for that purpose but that seemed wasteful
to me, there aren't that many laws to go around. So I wanted to hijack
and make this one configurable. But this was just my first idea so I'm
open to suggestions.

> 
> >
> > > 		dbells = <0 ffff>;
> > > 		mboxs = <0 4>;
> >
> > The dbells and mboxs can be removed. The default setting in rio is okay.
> 
> this could possibly be useful.

Again, the current setup seems to be the minimum required to support
rionet. The hardware can support multiple mailboxes as is not just mbox
0 used by rionet. As I understand it one message unit is dedicated to
mbox 0 and mboxes 1 to 63 are used by the second message unit. I want in
my application to use the "clean" mbox 0 for my apps messaging and
relegate rionet to mbox 1. So in the interests of generality I wanted to
make the number of mboxes supported configurable. Is this something I
should not do in dts but relegate somewhere else?

The doorbells is debatable. There is only a single unit supporting all
doorbells in extant hardware (AFAIK) so this was again for generality in
case a dual doorbell hw unit arrives.
> 
> >
> > >                 interrupt-parent = <&mpic>;
> > >                 interrupts = <30 1 31 1 32 1 35 1 36 1 37 1 38 1>;
> > >               };
> > >
> > Do you really use all of this interrupts? In my test, three <32 2 35 2
> > 36 2> are okay, and the sense is 2.
> 
> I think we need to list all the interrupts possible from RIO, not just the
> ones the driver happens to use.
Sorry about the senses, again I just threw that in the email as a
strawman to kick off discussion (which now seems moot as Wei has the
patches (yippee)). The second set of interrupts 37 and 38 are for the
second message unit which I want to use (see above). 30 is for
port-write/error which I will be using to get interrupts from my
switches for topology changes. 31 is the out doorbell done which the
driver doesn't use cos of the synchronous nature of the out doorbell I
suppose.
> 
> - k
> 
> 

RE: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Phil Terry]

On Wed, 2007-05-23 at 22:03 +0800, Zhang Wei-r63237 wrote:
> Hi, Mark
> 
> Yes, I'm glad to share it. I have added the memory mapping codes into
> it.
I have memory mapping requirements for my board above and beyond the
current driver. I'd like to see the above to see if its addressing my
needs.
> 
> These codes are reviewing and testing with our engineers. Could you
> waiting for a little while? It will be soon.
Well I'm stuck for hw to test on at the moment so I don't need tested
patches right now but I would like to see whats covered so I can check
what I still need to do for my application. Soon means???

Cheers
Phil

> 
> Best Regards,
> Zhang Wei
> 
> > -----Original Message-----
> > From: Mark A. Greer [mailto:mgreer@mvista.com] 
> > Sent: Wednesday, May 23, 2007 9:33 PM
> > To: Zhang Wei-r63237
> > Cc: pterry@micromemory.com; linuxppc-dev@ozlabs.org
> > Subject: Re: Porting RapidIO from ppc arch to powerpc arch in 
> > support of MPC8641D
> > 
> > On Wed, May 23, 2007 at 09:26:59PM +0800, Zhang Wei-r63237 wrote:
> > > I've finished the porting RIO from ppc to powerpc, do you need some
> > > helps?
> > 
> > Care to share the patches?
> > 
> > Mark
> > 
> 
> 

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Mark A. Greer]

On Wed, May 23, 2007 at 10:03:30PM +0800, Zhang Wei-r63237 wrote:
> Hi, Mark
> 
> Yes, I'm glad to share it. I have added the memory mapping codes into
> it.
> 
> These codes are reviewing and testing with our engineers. Could you
> waiting for a little while? It will be soon.

Of course.  I'm definitely interested in what you have, though, so
please share it as soon as you feel comfortable with it.

Mark

RE: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Phil Terry]

On Wed, 2007-05-23 at 22:03 +0800, Zhang Wei-r63237 wrote:
> Hi, Mark
> 
> Yes, I'm glad to share it. I have added the memory mapping codes into
> it.
> 
> These codes are reviewing and testing with our engineers. 

Can you tell me what your testing it on cos we need to get a good
interim base for our own development. The 8641_hpcn doesn't have support
for serial rapidio so I was looking at the 8548 eval system. The ltib
support is way back at 2.6.10 for that board but I was going to try and
make my powerpc/rapidio support both the 8641 and 8548 so I could test
in this way. Is your patch generic or 8641 specific?

Also either on this list or privately can you tell me what the plans are
for migrating the other freescale processors to powerpc and updating to
the latest kernel?

Cheers
Phil

> Could you
> waiting for a little while? It will be soon.
> 
> Best Regards,
> Zhang Wei
> 
> > -----Original Message-----
> > From: Mark A. Greer [mailto:mgreer@mvista.com] 
> > Sent: Wednesday, May 23, 2007 9:33 PM
> > To: Zhang Wei-r63237
> > Cc: pterry@micromemory.com; linuxppc-dev@ozlabs.org
> > Subject: Re: Porting RapidIO from ppc arch to powerpc arch in 
> > support of MPC8641D
> > 
> > On Wed, May 23, 2007 at 09:26:59PM +0800, Zhang Wei-r63237 wrote:
> > > I've finished the porting RIO from ppc to powerpc, do you need some
> > > helps?
> > 
> > Care to share the patches?
> > 
> > Mark
> > 
> 
> 

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Segher Boessenkool]

>>>                 interrupts = <30 1 31 1 32 1 35 1 36 1 37 1 38 1>;
>>>
>> Do you really use all of this interrupts? In my test, three <32 2 35 2
>> 36 2> are okay, and the sense is 2.
>
> I think we need to list all the interrupts possible from RIO, not just 
> the
> ones the driver happens to use.

Yes exactly; the device tree describes the hardware, not
the way that Linux (or any other OS) happens to use it.

The same holds for the "law", "doorbells", and "mailboxes"
properties -- I have no clue whether those are hardware
properties or what else, you guys figure it out :-)


Segher

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Segher Boessenkool]

>> The law should really be removed.
> In my application I'm may be using a huge part of the 36-bit address
> space to address multiple remote RIO boards in a peer DMA multicomputer
> application. The default law just for maintenance messages isn't going
> to cut it.

If the firmware sets up the "law", it should put a property
in the node describing the setting.  If Linux sets up the
laws, there shouldn't be a property (since it is a policy
decision).

>>> The dbells and mboxs can be removed. The default setting in rio is 
>>> okay.
>>
>> this could possibly be useful.

Same issue.  Even if the firmware uses a default setting,
it should still put it in the device tree _iff_ it is the
firmware's decision (and not the kernel's).


Segher

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Phil Terry]

On Wed, 2007-05-23 at 18:00 +0200, Segher Boessenkool wrote:
> >>>                 interrupts = <30 1 31 1 32 1 35 1 36 1 37 1 38 1>;
> >>>
> >> Do you really use all of this interrupts? In my test, three <32 2 35 2
> >> 36 2> are okay, and the sense is 2.
> >
> > I think we need to list all the interrupts possible from RIO, not just 
> > the
> > ones the driver happens to use.
> 
> Yes exactly; the device tree describes the hardware, not
> the way that Linux (or any other OS) happens to use it.
So a law is a bit like a BAR in PCI, its a configuration by software of
the memory map determining which accesses are directed at the hardware.

Unlike PCI its not something we can go around probing the hardware for
and then doling out memory space to them. Its purely up to software as a
whole to decide its system wide memory map. Its not that the device
"needs" x amount of memory to work, its a case of for your application
how much of your memory would you like this device to handle for you.

Is this something the dts as a central, early boottime thing should be
in charge of, ie telling the kernel how to organize its total memory
map?

Excuse the noobie questions but I haven't been able to work out the
provenance of the open firmware stuff, and the resolution between the
ppc32, ppc64, powerpc, apple, ibm, freescale, etc., camps on how this
stuff should be used from the archives, too much noise to signal.

Cheers
Phil

> 
> The same holds for the "law", "doorbells", and "mailboxes"
> properties -- I have no clue whether those are hardware
> properties or what else, you guys figure it out :-)
> 
> 
> Segher
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 
> 

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Kumar Gala]

On Wed, 23 May 2007, Phil Terry wrote:

> On Wed, 2007-05-23 at 18:05 +0200, Segher Boessenkool wrote:
> > >> The law should really be removed.
> > > In my application I'm may be using a huge part of the 36-bit address
> > > space to address multiple remote RIO boards in a peer DMA multicomputer
> > > application. The default law just for maintenance messages isn't going
> > > to cut it.
> >
> > If the firmware sets up the "law", it should put a property
> > in the node describing the setting.  If Linux sets up the
> > laws, there shouldn't be a property (since it is a policy
> > decision).
> Ooops, I just posted a question to you before I saw this pop up sorry?
>
> But when you say "firmware" do you mean u-boot or your kernel loading
> code or do you mean some aspect of the hardware, eg, its EEPROM program
> which can vary from hardware to hardware instantiation?
>
> If you mean u-boot I'm confused (so whats new?). AFAIK u-boot can't
> probe and set this up and even if it did, linux in the arch's I'm
> familiar with sets everything up anew regardless of what the boot loader
> did. AFAIK in the freescale processors the dtb is being passed in from
> u-boot as a supplied with the kernel build "blob" not because it built
> it dynamically. Or again have I got hold of the wrong end of the stick?

That's mostly true, but you are looking at it from one implementations
point of view, not from how its architected.  In the future we may (and I
will) have u-boot capable of doing more dynamic building of the device
tree or someother firmware.  All the kernel knows is its handed a device
tree.

- k

 > > Cheers
> Phil
>
> >
> > >>> The dbells and mboxs can be removed. The default setting in rio is
> > >>> okay.
> > >>
> > >> this could possibly be useful.
> >
> > Same issue.  Even if the firmware uses a default setting,
> > it should still put it in the device tree _iff_ it is the
> > firmware's decision (and not the kernel's).
> >
> >
> > Segher
> >
> >
> >
>
>

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Phil Terry]

On Wed, 2007-05-23 at 18:05 +0200, Segher Boessenkool wrote:
> >> The law should really be removed.
> > In my application I'm may be using a huge part of the 36-bit address
> > space to address multiple remote RIO boards in a peer DMA multicomputer
> > application. The default law just for maintenance messages isn't going
> > to cut it.
> 
> If the firmware sets up the "law", it should put a property
> in the node describing the setting.  If Linux sets up the
> laws, there shouldn't be a property (since it is a policy
> decision).
Ooops, I just posted a question to you before I saw this pop up sorry?

But when you say "firmware" do you mean u-boot or your kernel loading
code or do you mean some aspect of the hardware, eg, its EEPROM program
which can vary from hardware to hardware instantiation?

If you mean u-boot I'm confused (so whats new?). AFAIK u-boot can't
probe and set this up and even if it did, linux in the arch's I'm
familiar with sets everything up anew regardless of what the boot loader
did. AFAIK in the freescale processors the dtb is being passed in from
u-boot as a supplied with the kernel build "blob" not because it built
it dynamically. Or again have I got hold of the wrong end of the stick?

Cheers
Phil

> 
> >>> The dbells and mboxs can be removed. The default setting in rio is 
> >>> okay.
> >>
> >> this could possibly be useful.
> 
> Same issue.  Even if the firmware uses a default setting,
> it should still put it in the device tree _iff_ it is the
> firmware's decision (and not the kernel's).
> 
> 
> Segher
> 
> 
> 

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Phil Terry]

On Wed, 2007-05-23 at 11:20 -0500, Kumar Gala wrote:
> On Wed, 23 May 2007, Phil Terry wrote:
> 
> > On Wed, 2007-05-23 at 18:05 +0200, Segher Boessenkool wrote:
> > > >> The law should really be removed.
> > > > In my application I'm may be using a huge part of the 36-bit address
> > > > space to address multiple remote RIO boards in a peer DMA multicomputer
> > > > application. The default law just for maintenance messages isn't going
> > > > to cut it.
> > >
> > > If the firmware sets up the "law", it should put a property
> > > in the node describing the setting.  If Linux sets up the
> > > laws, there shouldn't be a property (since it is a policy
> > > decision).
> > Ooops, I just posted a question to you before I saw this pop up sorry?
> >
> > But when you say "firmware" do you mean u-boot or your kernel loading
> > code or do you mean some aspect of the hardware, eg, its EEPROM program
> > which can vary from hardware to hardware instantiation?
> >
> > If you mean u-boot I'm confused (so whats new?). AFAIK u-boot can't
> > probe and set this up and even if it did, linux in the arch's I'm
> > familiar with sets everything up anew regardless of what the boot loader
> > did. AFAIK in the freescale processors the dtb is being passed in from
> > u-boot as a supplied with the kernel build "blob" not because it built
> > it dynamically. Or again have I got hold of the wrong end of the stick?
> 
> That's mostly true, but you are looking at it from one implementations
> point of view, not from how its architected.  In the future we may (and I
> will) have u-boot capable of doing more dynamic building of the device
> tree or someother firmware.  All the kernel knows is its handed a device
> tree.

OK, but if the device tree is not allowed to dictate policy, to use
Segher's term, just hardware characteristics, how does that help us get
the embedded soc kernels away from being designed and built to specific
demo/eval board setups and make them more configurable? 

I got the impression that to some extent thats how you/we/?? were trying
to use the dts stuff. If my board is exactly like freescales xyz demo
board except I move my rio map to here, my pci map to here, change a few
sizes etc., why do I have to go and patchup the arch setup code, modify
ppc_md routines, etc. Isn't the plan that I just edit the dts, compile
with dtc and have u-boot pass in the dtb to the stock kernel and its
boots on my board? 

If dts can't do this because its not allowed policy statements then what
will do this?

What device in the dts represents the soc mpx/mpc where I can define
what the laws should be? Segher is right, its not a property of the
rapidio device, it doesn't have a BAR (though it does have ATMUs?). But
if I have a device representing the mpx/mpc (the soc itself?) then I can
specify ranges for child devices to operate in (the laws) and then in
the child device I can specify ranges for the ATMUs?

I'm probably trampling over a ton of old stuff here, sorry, excuse my
ramblings....

Please I'm not trying to start up any previous turf wars here, just
trying to understand the truce terms and the plan ahead so I can get
with the program. Appreciate the help for a newbie.

Cheers
Phil

> 
> - k
> 
>  > > Cheers
> > Phil
> >
> > >
> > > >>> The dbells and mboxs can be removed. The default setting in rio is
> > > >>> okay.
> > > >>
> > > >> this could possibly be useful.
> > >
> > > Same issue.  Even if the firmware uses a default setting,
> > > it should still put it in the device tree _iff_ it is the
> > > firmware's decision (and not the kernel's).
> > >
> > >
> > > Segher
> > >
> > >
> > >
> >
> >
> 
> 

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Segher Boessenkool]

>> If the firmware sets up the "law", it should put a property
>> in the node describing the setting.  If Linux sets up the
>> laws, there shouldn't be a property (since it is a policy
>> decision).
> Ooops, I just posted a question to you before I saw this pop up sorry?

No problem.

> But when you say "firmware" do you mean u-boot or your kernel loading
> code or do you mean some aspect of the hardware, eg, its EEPROM program
> which can vary from hardware to hardware instantiation?

Any of those.  Anything before the kernel takes control.

> If you mean u-boot I'm confused (so whats new?). AFAIK u-boot can't
> probe and set this up

Maybe it should.

>  and even if it did, linux in the arch's I'm
> familiar with sets everything up anew regardless of what the boot 
> loader
> did.

...and maybe that needs changing.


If it's a platform thing, it's the platform firmware code
that should configure it (and then it belongs in the device
tree).  If reasonably the kernel should configure it later,
_it_ should do it (and then there shouldn't be a device tree
property).

I have no clue which it is.


Segher

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[Segher Boessenkool]

> OK, but if the device tree is not allowed to dictate policy, to use
> Segher's term, just hardware characteristics, how does that help us get
> the embedded soc kernels away from being designed and built to specific
> demo/eval board setups and make them more configurable?

I don't see the problem really.  Could you point out one
specific problem, and then we solve it?  Repeat as necessary.

> I got the impression that to some extent thats how you/we/?? were 
> trying
> to use the dts stuff. If my board is exactly like freescales xyz demo
> board except I move my rio map to here, my pci map to here, change a 
> few
> sizes etc., why do I have to go and patchup the arch setup code, modify
> ppc_md routines, etc.

That shouldn't be necessary.

> Isn't the plan that I just edit the dts, compile
> with dtc and have u-boot pass in the dtb to the stock kernel and its
> boots on my board?

Mostly, yes.

> If dts can't do this because its not allowed policy statements then 
> what
> will do this?

You pass policy decisions to the kernel some other way.  Like,
on the kernel command line, for example.

> Segher is right,

Yes, I always am.

> Please I'm not trying to start up any previous turf wars here,

Much appreciated!

--

Okay, if I am to help at all here, please someone explain what
"law"s and mboxes and doorbells are (in the context of rapidio).
Also how they are used, etc.


Segher

Re: Porting RapidIO from ppc arch to powerpc arch in support of MPC8641D

[David Gibson]

On Wed, May 23, 2007 at 09:21:48AM -0500, Kumar Gala wrote:
> On Wed, 23 May 2007, Zhang Wei-r63237 wrote:
> 
> > > So I'm taking the boot/dts/mpc8641_hpcn.dts and producing a new
> > > mpc8641D_umem.dts with the following addition to the soc.
> > >
> > > srio@c0000 {
> > >                 device_type = "srio";
> > >                 compatible = "86xx,85xx";

This is quite tangential to the rest of this discussion, but "xx"s in
compatible properties always alarm me if they're the only thing
listed.  It's generally best to give a precise model, then a more
general one: the trade-off is that if you're too precise, it's fairly
easy for the driver to workaround by recognizing and binding to
multiple compatible strings, whereas if you're too general you risk
the driver not being able to determine whether it needs a
model-specific workaround or not.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

RE: Porting RapidIO from ppc arch to powerpc arch in support ofMPC8641D

[Zhang Wei-r63237]

Hi, Arnd,

> >=20
> > Then I should pass np into the various setup routines as=20
> above so that
> > they can find the interrupts, etc.
> >=20
> > Is this the right kind of flavor or have I misunderstood how the dtb
> > stuff is supposed to integrate with the susbsys_initcall stuff.
>=20
> the thing you are missing is probably the of_platform_driver. Your
> driver should have something like
>=20
> static struct of_device_id mpc85xx_rio_ids[] =3D {
> 	{ .compatible =3D "fsl,8641d-rapidio", .data =3D RIO_8641 },
> 	{ .compatible =3D "fsl,8540-rapidio", .data =3D RIO_8540 },
> 	{ },
> };
> static struct of_platform_driver mpc85xx_rio_driver =3D {
> 	.name =3D "mpc85xx_rio",
> 	.match_table =3D &mpc85xx_rio_ids,
> 	.probe =3D mpc85xx_rio_probe,
> };
> static int __init mpc85xx_rio_init(void)
> {
> 	return of_register_platform_driver(&mpc85xx_rio_driver);
> }
> subsys_initcall(mpc85xx_rio_init);
>=20
> > I'm assuming we are supposed to do away with all the CONFIG_RAPIDIO,
> > CONFIG_MPCxyz etc so that the kernel is driven by the dtb?
>=20


I'm not agree with you.

Of_platform_driver is a good framework. But this RapidIO driver is not a
device driver. It's a _bus_ driver very similar to PCI bus. So,
of_platform_driver is not fit here.

Thanks!
Wei.

RE: Porting RapidIO from ppc arch to powerpc arch in support ofMPC8641D

[Zhang Wei-r63237]

> > >                 interrupts =3D <30 1 31 1 32 1 35 1 36 1=20
> 37 1 38 1>;
> > > >               };
> > > >
> > > Do you really use all of this interrupts? In my test,=20
> three <32 2 35 2
> > > 36 2> are okay, and the sense is 2.
> >=20
> > I think we need to list all the interrupts possible from=20
> RIO, not just the
> > ones the driver happens to use.
> Sorry about the senses, again I just threw that in the email as a
> strawman to kick off discussion (which now seems moot as Wei has the
> patches (yippee)). The second set of interrupts 37 and 38 are for the
> second message unit which I want to use (see above). 30 is for
> port-write/error which I will be using to get interrupts from my
> switches for topology changes. 31 is the out doorbell done which the
> driver doesn't use cos of the synchronous nature of the out doorbell I
> suppose.

About this interrupts, I suggest to put the BELL, Tx, Rx to the front of
others. These three is already be used in ppc rio codes.
For example:
	/* bell_irq tx_irq rx_irq */
	interrupts =3D <32 2 35 2 36 2>;

Thanks!
Wei.

Re: Porting RapidIO from ppc arch to powerpc arch in support ofMPC8641D

[Arnd Bergmann]

On Thursday 24 May 2007, Zhang Wei-r63237 wrote:
> Of_platform_driver is a good framework. But this RapidIO driver is not a
> device driver. It's a _bus_ driver very similar to PCI bus. So,
> of_platform_driver is not fit here.

But there is not a fundamental difference between the two. In the Linux
driver model, a bus driver is a simply device driver whose devices have
other devices as child nodes.

Your example of PCI buses fits well here. Traditionally, each platform
had its own PCI scanning code that went looking for pci host bridges.
Now we have the of_pci_phb_driver in arch/powerpc/kernel/of_platform.c
that automatically sets up the bridges, and the platform only
needs to provide a ppc_md.pci_setup_phb() callback.

	Arnd <><

RE: Porting RapidIO from ppc arch to powerpc arch in support ofMPC8641D

[Zhang Wei-r63237]

>=20
> On Thursday 24 May 2007, Zhang Wei-r63237 wrote:
> > Of_platform_driver is a good framework. But this RapidIO=20
> driver is not a
> > device driver. It's a _bus_ driver very similar to PCI bus. So,
> > of_platform_driver is not fit here.
>=20
> But there is not a fundamental difference between the two. In=20
> the Linux
> driver model, a bus driver is a simply device driver whose=20
> devices have
> other devices as child nodes.
>=20
> Your example of PCI buses fits well here. Traditionally, each platform
> had its own PCI scanning code that went looking for pci host bridges.
> Now we have the of_pci_phb_driver in arch/powerpc/kernel/of_platform.c
> that automatically sets up the bridges, and the platform only
> needs to provide a ppc_md.pci_setup_phb() callback.
>=20

Add the rio scaning code to of_platform.c such as pci is ok?

Thanks!
-wei

Re: Porting RapidIO from ppc arch to powerpc arch in support ofMPC8641D

[Arnd Bergmann]

On Thursday 24 May 2007, Zhang Wei-r63237 wrote:
> 
> Add the rio scaning code to of_platform.c such as pci is ok?

I think it's better to have a separate file that consolidates
all the code specific to one rio bus bridge.

There is no technical reason why the of_pci_phb code is in
of_platform.c either, it more or less ended up in there by
accident ;-)

	Arnd <><