How to add platform specific data to a of_device

How to add platform specific data to a of_device

[Juergen Beisert]

Hi,

I'm trying to use the drivers/spi/mpc52xx_psc_spi.c as an open firmware device
(ARCH=powerpc). This device needs some platform specific data (the devices
connected to the SPI bus and how to drive the chipselects to these devices).

The driver itself get a "struct of_device *op" in his probe function and
does something like this:

struct fsl_spi_platform_data *pdata = op->dev.platform_data;

My question is: How is the correct way to bring the platform specific data
into this device structure? Is there a way to do it in the OFTree (dts file)?

Or in a way like this?

static struct fsl_spi_platform_data my_spi_master_info = {
[....]
}

static int __init my_platform_register_spi(void)
{
       struct device_node *np = NULL;
       struct of_device *of_dev;

       if ((np = of_find_compatible_node(np, "spi", "mpc5200-psc-spi")) == NULL) {
               printk("couldn't find of tree node\n");
               return -1;
       }
       if ((of_dev = of_find_device_by_node(np)) == NULL) {
               printk("couldn't find device by node\n");
               return -1;
       }

       of_dev->dev.platform_data = &my_spi_master_info;

       return 0;
}

Or is there any other way?

Regards
Juergen

Re: How to add platform specific data to a of_device

[Benjamin Herrenschmidt]

On Sat, 2007-07-14 at 18:31 +0200, Juergen Beisert wrote:
> Hi,
> 
> I'm trying to use the drivers/spi/mpc52xx_psc_spi.c as an open firmware device
> (ARCH=powerpc). This device needs some platform specific data (the devices
> connected to the SPI bus and how to drive the chipselects to these devices).
> 
> The driver itself get a "struct of_device *op" in his probe function and
> does something like this:
> 
> struct fsl_spi_platform_data *pdata = op->dev.platform_data;
> 
> My question is: How is the correct way to bring the platform specific data
> into this device structure? Is there a way to do it in the OFTree (dts file)?

Your approach would work I suppose.... though it's a bit ugly. I've long
considered removing platform_data from struct device, and make it part
strictly of platform device...

I'm not sure what you actually need here... if it's to know what your
child devices are, you can always walk the device-tree, though for most
things, it would be the child devices themslves who would call into your
SPI driver with whatever identification they retreived from there.

Ben.

Re: How to add platform specific data to a of_device

[Juergen Beisert]

Hi Ben,

On Saturday 14 July 2007 22:48, Benjamin Herrenschmidt wrote:
> On Sat, 2007-07-14 at 18:31 +0200, Juergen Beisert wrote:
> > Hi,
> >
> > I'm trying to use the drivers/spi/mpc52xx_psc_spi.c as an open firmware
> > device (ARCH=powerpc). This device needs some platform specific data (the
> > devices connected to the SPI bus and how to drive the chipselects to
> > these devices).
> >
> > The driver itself get a "struct of_device *op" in his probe function and
> > does something like this:
> >
> > struct fsl_spi_platform_data *pdata = op->dev.platform_data;
> >
> > My question is: How is the correct way to bring the platform specific
> > data into this device structure? Is there a way to do it in the OFTree
> > (dts file)?
>
> Your approach would work I suppose.... though it's a bit ugly.

Yes, I know it works (I'm currently using it), but also yes, its very ugly. So 
I want to change it.

> I'm not sure what you actually need here...

What the SPI master driver needs its something like the SPI bus number it 
controls, how many chipselect lines are available (=how many SPI slave 
devices are connected) and what platform specific functions it must call to 
set or reset the chipselect lines to make a data transfer on its bus.
It does not need the types of connected slave devices, as this will be 
controlled by the SPI framework.

struct fsl_spi_platform_data spi_master_platform_info =
{
	.initial_spmode = <something SPI related>,
	.sysclk = <mpc5200 internal systemclock - platform specific >,
	.bus_num = <SPI bus number for this master controller>,
	.max_chipselect = <platforms chipselect count>,
	.activate_cs = <platform function to activate one of the chipselects>,
	.deactivate_cs = <platform function to activate one of the chipselects>
};

Would it be possible to query these informations from the oftree? Can someone 
give an example how the oftree dts syntax would look like for this case?

> if it's to know what your 
> child devices are, you can always walk the device-tree, though for most
> things, it would be the child devices themslves who would call into your
> SPI driver with whatever identification they retreived from there.

Hmm, as I stated above: Handling the SPI slave devices is done in the SPI 
framework. But including this data into the dts sounds better than including 
it into the BSP. Is there some documentation around how to describe such a 
SPI bus with its devices in the dts?

Thanks so far
Juergen

Re: How to add platform specific data to a of_device

[Benjamin Herrenschmidt]

> Hmm, as I stated above: Handling the SPI slave devices is done in the SPI 
> framework. But including this data into the dts sounds better than including 
> it into the BSP. Is there some documentation around how to describe such a 
> SPI bus with its devices in the dts?

I don't think so, I doubt there's an SPI OF binding. You'll have to
invent something and discuss it on the list.

Ben.

Re: How to add platform specific data to a of_device

[Robert Schwebel]

On Sun, Jul 15, 2007 at 06:48:53AM +1000, Benjamin Herrenschmidt wrote:
> Your approach would work I suppose.... though it's a bit ugly.

Speaking of uggly, I'm still wondering why this oftree stuff for powerpc
must be soooo complicated. If you come from the ARM-linux world like we
do, the whole powerpc BSP stuff looks like a completely overengineered
piece of code, introducing complexity where it isn't necessary. But it
may be that it's just me not knowing powerpc kernel requirements deeply
enough :)

For most of the devices on for example the MPC5200B and MPC8260 I would
just model them as platform devices; there could be a simple

	oftree -> oftree-interpreter -> bunch of platform devices

mapping. Is there a reason why there is sooo much interaction of the
platform code with the oftree? We usually have the situation that, if
something goes wrong, you have to change

	- the driver
	- the platform code
	- the oftree

and they often contain redundant information (like names of oftree
nodes, which change more often than some people's panties).

Robert
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: How to add platform specific data to a of_device

[Benjamin Herrenschmidt]

On Mon, 2007-07-16 at 08:51 +0200, Robert Schwebel wrote:
> On Sun, Jul 15, 2007 at 06:48:53AM +1000, Benjamin Herrenschmidt wrote:
> > Your approach would work I suppose.... though it's a bit ugly.
> 
> Speaking of uggly, I'm still wondering why this oftree stuff for powerpc
> must be soooo complicated. If you come from the ARM-linux world like we
> do, the whole powerpc BSP stuff looks like a completely overengineered
> piece of code, introducing complexity where it isn't necessary. But it
> may be that it's just me not knowing powerpc kernel requirements deeply
> enough :)
> 
> For most of the devices on for example the MPC5200B and MPC8260 I would
> just model them as platform devices; there could be a simple
> 
> 	oftree -> oftree-interpreter -> bunch of platform devices

As I wrote a couple of times already, it's a perfectly acceptable
approach to have "constructors" (what you call oftree-interpreter) that
generate platform devices from the OF tree.

Since any struct device in the system can be associated with an OF
device node, it's actually not that interesting anymore to use something
such as of_platform_device or in general, subclasses of of_device unless
this is a platform native bus (such as sbus on sparc, or ebus on power)
that makes no sense to use without OF.

One of the idea I have for the long term but didn't have time to
implement is typically to have a bunch of generic constructors that
register to match against device name/type/compatible triplets, and are
called as part a boot time initial device-tree walk, generating the
various linux devices on the fly. This could also generate PCI devices,
thus replacing the separate walk path done by ppc64, but it could be
used to generate any type of linux device, not necessarily OF-derivative
ones, but platform devices too etc...

I just haven't had time to work on that yet. You are welcome to beat me
to it.

Note that a lot of the complexity is mostly perceived complexity due to
some of the stupid endless debates we've been having on this list. For
things like interrupt mapping, for example, the OF tree is a very useful
and very flexible representation that makes things a lot simpler on the
kernel side when you start having multiple levels of cascaded interrupt
controllers.

> mapping. Is there a reason why there is sooo much interaction of the
> platform code with the oftree? We usually have the situation that, if
> something goes wrong, you have to change
> 
> 	- the driver
> 	- the platform code
> 	- the oftree

There should generally be no need to change the platform code.

> and they often contain redundant information (like names of oftree
> nodes, which change more often than some people's panties).

Well, they aren't supposed to :-) The problem at this point is more due
to the fact that for things that haven't been specified by official OF
bindings, people are going all over trying to define their own and
sometimes conflicting bindings and then changing them.

Ben.

Re: How to add platform specific data to a of_device

[Robert Schwebel]

On Mon, Jul 16, 2007 at 05:09:12PM +1000, Benjamin Herrenschmidt wrote:
> As I wrote a couple of times already, it's a perfectly acceptable
> approach to have "constructors" (what you call oftree-interpreter) that
> generate platform devices from the OF tree.

Great!

> > mapping. Is there a reason why there is sooo much interaction of the
> > platform code with the oftree? We usually have the situation that, if
> > something goes wrong, you have to change
> > 
> > 	- the driver
> > 	- the platform code
> > 	- the oftree
> 
> There should generally be no need to change the platform code.

Well, in reality it is, because for example the MPC52xx PSC SPI
controller we are currently working was obviously never tested with
oftree before it hit the mainline ...

> > and they often contain redundant information (like names of oftree
> > nodes, which change more often than some people's panties).
> 
> Well, they aren't supposed to :-) The problem at this point is more due
> to the fact that for things that haven't been specified by official OF
> bindings, people are going all over trying to define their own and
> sometimes conflicting bindings and then changing them.

I think it is a fundamental thing: the "we just have to wait long
enough, until oftree definitions have settled" proposal just isn't
right. It may be right for big irons, being well defined. But for the
embedded processors, too less people are working on it, plus we have too
much things which could be defined. Speaking of the MPC5200, look at how
often device tree names change, e.g. for mpc5200 vs. mpc52xx vs.
whatever. As long as things change, you have to keep the three locations
in sync manually, and that's bad.

Robert
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: How to add platform specific data to a of_device

[Benjamin Herrenschmidt]

On Mon, 2007-07-16 at 09:19 +0200, Robert Schwebel wrote:

> I think it is a fundamental thing: the "we just have to wait long
> enough, until oftree definitions have settled" proposal just isn't
> right. It may be right for big irons, being well defined. But for the
> embedded processors, too less people are working on it, plus we have too
> much things which could be defined. Speaking of the MPC5200, look at how
> often device tree names change, e.g. for mpc5200 vs. mpc52xx vs.
> whatever. As long as things change, you have to keep the three locations
> in sync manually, and that's b 

I wouldn't expect things to change that much. I think MPC52xx is a bad
example of a worst case scenario. Also, as the core group of people
working on linux/ppc get more familiar with the device-tree, we should
get things right more quickly.

In the end, the problem with the device-tree is also it's strongest
advantage: it's extremely flexible :-) So yes, that causes that sort of
problem, but don't ignore the whole lot of problems that it solves by
not having to hard code knowledge of the gazillion ways a given chip can
be setup in drivers or the ability to pass along ancilliary informations
such as MAC addresses, UUIDs, etc... from the firmware to selected
devices in the tree, or the sane interrupt & address mapping (that's
really the two main reasons in fact).

Ben.

Re: How to add platform specific data to a of_device

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]

On Mon, 2007-07-16 at 08:51 +0200, Robert Schwebel wrote:
> On Sun, Jul 15, 2007 at 06:48:53AM +1000, Benjamin Herrenschmidt wrote:
> > Your approach would work I suppose.... though it's a bit ugly.
> 
> Speaking of uggly, I'm still wondering why this oftree stuff for powerpc
> must be soooo complicated. If you come from the ARM-linux world like we
> do, the whole powerpc BSP stuff looks like a completely overengineered
> piece of code, introducing complexity where it isn't necessary. But it
> may be that it's just me not knowing powerpc kernel requirements deeply
> enough :)

And I don't know anything about ARM, so let's trade uninformed
opinions ... :)

# cd arch/arm
# du -sk kernel/ mach-* | sort -nk 1
16      mach-l7200
16      mach-s3c2400
24      mach-clps7500
24      mach-s3c2442
32      mach-iop33x
32      mach-rpc
32      mach-shark
36      mach-ebsa110
40      mach-aaec2000
48      mach-h720x
48      mach-ks8695
52      mach-iop32x
56      mach-davinci
60      mach-ixp23xx
60      mach-ns9xxx
60      mach-s3c2443
68      mach-clps711x
68      mach-s3c2412
72      mach-netx
72      mach-realview
72      mach-versatile
76      mach-s3c2440
80      mach-ep93xx
80      mach-imx
88      mach-ixp2000
96      mach-iop13xx
96      mach-lh7a40x
108     mach-pnx4008
124     mach-footbridge
124     mach-integrator
140     mach-ixp4xx
140     mach-s3c2410
252     mach-sa1100
272     mach-omap2
296     mach-omap1
296     mach-pxa
332     mach-at91
428     kernel/

# cd arch/powerpc
# du -sk kernel/ sysdev/ platforms/* | sort -nk 1
8       platforms/prep
12      platforms/4xx
24      platforms/44x
40      platforms/82xx
44      platforms/86xx
52      platforms/8xx
52      platforms/embedded6xx
56      platforms/maple
60      platforms/83xx
64      platforms/85xx
64      platforms/chrp
72      platforms/52xx
72      platforms/pasemi
140     platforms/celleb
212     platforms/ps3
280     platforms/iseries
300     platforms/pseries
412     platforms/powermac
412     sysdev/
544     platforms/cell		(spufs shared with ps3 (and celleb?))
1636    kernel/

Not to mention that ~8 of those platforms can be built into a single
vmlinux that will boot on any of the supported machines, including five
different hypervisors.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: How to add platform specific data to a of_device

[Segher Boessenkool]

>> Hmm, as I stated above: Handling the SPI slave devices is done in  
>> the SPI
>> framework. But including this data into the dts sounds better than  
>> including
>> it into the BSP. Is there some documentation around how to  
>> describe such a
>> SPI bus with its devices in the dts?
>
> I don't think so, I doubt there's an SPI OF binding.

That's correct.


Segher

Re: How to add platform specific data to a of_device

[Segher Boessenkool]

> Is there a reason why there is sooo much interaction of the
> platform code with the oftree?

Yes.  Since the decision has been made to require a device
tree for every platform, arch/powerpc is in the luxury
position that we can actually exploit the benefits of _having_
a tree for every platform.


Segher

Re: How to add platform specific data to a of_device

[Segher Boessenkool]

> As I wrote a couple of times already, it's a perfectly acceptable
> approach to have "constructors" (what you call oftree-interpreter)  
> that
> generate platform devices from the OF tree.

Yes, lots of the "glue" code could be made into some
helper library.  Also, while that glue code might be
perceived as being "a lot of stuff", it isn't really,
and it is quite simple anyway; it's just a layer that
sits there to translate for the impedance mismatch of
the device tree on one hand, and the kernel interfaces
on the other.  Because we do have such a layer, interface
changes aren't a big deal; and since the device tree
interface is a way flexible, mostly text-based associative
array tree thing, we have none of the problems that
binary firmware interfaces on other platforms have.  Oh,
and the Open Firmware device tree works perfectly fine
across different architectures, too.

>> and they often contain redundant information (like names of oftree
>> nodes, which change more often than some people's panties).
>
> Well, they aren't supposed to :-) The problem at this point is more  
> due
> to the fact that for things that haven't been specified by official OF
> bindings, people are going all over trying to define their own and
> sometimes conflicting bindings and then changing them.

This is most of the "endless debate", too.


Segher

Re: How to add platform specific data to a of_device

[Segher Boessenkool]

>> Well, they aren't supposed to :-) The problem at this point is  
>> more due
>> to the fact that for things that haven't been specified by  
>> official OF
>> bindings, people are going all over trying to define their own and
>> sometimes conflicting bindings and then changing them.
>
> I think it is a fundamental thing: the "we just have to wait long
> enough, until oftree definitions have settled" proposal just isn't
> right.

The big problem here is that lots of people got the _basic_
stuff wrong.  I feel that this is getting much better the
last few months though :-)

> It may be right for big irons, being well defined.

OF is being successfully used on many many more systems
than just "big iron"; and many of those do have really weird
quirks.  arch/powerpc also deals with many systems that don't
get their device trees quite right (*cough* powermac *cough*)
and that is doable just fine.  The quirks should be separated
more from the normal code though, in the Linux OF layer.

> But for the
> embedded processors, too less people are working on it, plus we  
> have too
> much things which could be defined.

For embedded, too often the whole bloody thing is dropped
onto the "bigger Linux community" after it has been developed
in-house for many many months.  And usually, lots of core
things are very wrong.  This is a problem with "embedded"
in general, nothing new for OF.

> Speaking of the MPC5200, look at how
> often device tree names change, e.g. for mpc5200 vs. mpc52xx vs.
> whatever. As long as things change, you have to keep the three  
> locations
> in sync manually, and that's bad.

No, you *do not* have to keep them in synch, that's part of
the beauty of the device tree definition.  Sure, you have to
make sure the consumer is at least as new as the producer,
unless people took pains to try to create wrong-way-around
compatibility.  Nothing new there.


Segher