Deprecating of_platform, the path from here...

Deprecating of_platform, the path from here...

[Grant Likely]

Hi all,

This is a summary of a discussion we had on #mklinux last night.

On PowerPC, SPARC and Microblaze, most system (non-discoverable)
devices are registered on the of_platform bus.  In the rest of the
kernel, the same kind of devices are registered on the platform bus.
The of_platform and platform busses essentially perform the same task;
proved a method for platform code to register a device that will be
bound to a driver at a later point in time.  The most significant
difference is that the of_platform bus understands how probe drivers
The OF Way using the 'compatible' property.  In the OF way, more than
one driver could match the device and the OS is responsible to choose
the best one.

The platform bus only supports matching a device to a driver based on
the device's name.  It has no mechanism for capturing the OF binding
model.

Maintaining of_platform comes at a cost.  Many device drivers end up
with two bus bindings, both of_platform_driver and platform_driver.
In practice this means a bunch of duplicated boilerplate and some ugly
#ifdef blocks.  Device driver authors don't seem thrilled to have
of_platform stuff "invade" their platform drivers.  Also, the
of_platform/platform split doesn't match the pattern used for other
bus subsystems (i2c, spi, mdio, etc).  On other busses, the same
driver is used regardless of whether the device was allocated
statically in platform code, obtained from the device tree, or
something else entirely.

So, the desire is to deprecate of_platform bus and use platform bus
for system devices in the device tree.  However, doing this requires
two problems to be solved:

1) device to driver matching - Given an OF style compatible list, how
is the device matched to the correct platform driver?
2) data translation - how and where is the device tree data
transformed into something usable by the platform driver?

The 2nd problem is the easy one to solve.  In most cases, any code to
translate device tree data into a driver platform_data structure is
driver specific.  A hook can be added to the top of the driver's
.probe() routine.  If no pdata pointer is set in the driver structure,
then it can call out to a helper function to create one from the
device tree data.  Something along the lines of:

pdata = pdev->dev.platform_data;
if (!pdata)
        pdata = pdriver_get_of_pdata(pdev);

It makes sense for this code to be called from the driver itself so
that the translation code only gets run when the device is actually
used.  By running the translators at probe time (instead of device
registration time) it means the translators don't need to be built
into the kernel.  On a kernel config with a lot of of-enabled drivers
this would be significant.

To solve the 1st problem the possibility was discussed is to add
compatible list parsing to both the platform bus and modutils.  Doing
so would make it easy to create platform devices from nodes in the
device tree, but it would be hard to get 'right'.  There is a risk
that it would break existing modutils (completely unacceptable).  It
is unclear how to make sure Linux probes the correct driver.  ie.  If
a compatible driver is loaded, but a 'better' driver is available,
then how is the kernel prevented from binding against the 'worse'
driver before the better driver is loaded?  Finally, while it may
solve the problem for the platform bus, the exact same problem needs
to also be solved for i2c, spi, mdio, etc.

In the end, the decision was to just use a simple lookup table of
compatible values to driver names.  When registering devices, platform
code will use the table to choose the best name.  Existing platform
devices can then bind against them as-is.  The downside is that the
table must be statically compiled into the kernel.  However, the
solution is simple and the table can probably be put into an initdata
section to be discarded after boot.  At a later date something more
sophisticated can be designed.

So, to summarize the decisions made:

1) of_platform will be deprecated in preference of the platform bus.
2) platform code will register platform devices at boot time
 - A lookup table will be used to translate 'compatible' values to
driver names used by the kernel
 - If no suitable compatible values exist in the table, then choose a
name programatically (ie. of_modalias_node()).  Userspace can force
bind to a driver after boot if desirable.
 - helper functions to be written to make registration simple (ie.
of_platform_bus_probe())
 - helper functions will fill in common resources (memory ranges and
irqs) but leave complex stuff to the driver
3) platform drivers will probe the same way they are probed now.  No
changes to the platform bus.
4) If drivers needs extra platform_data, then a call will be added to
the top of the .probe() hook to go and create one.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Deprecating of_platform, the path from here...

[David Miller]

From: Grant Likely <grant.likely@secretlab.ca>
Date: Wed, 9 Dec 2009 15:06:29 -0700

> 1) of_platform will be deprecated in preference of the platform bus.

What a shame, it's one of the cleanest driver probing models
in the tree.

Re: Deprecating of_platform, the path from here...

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Wed, 09 Dec 2009 16:15:50 -0800 (PST)

> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Wed, 9 Dec 2009 15:06:29 -0700
> 
>> 1) of_platform will be deprecated in preference of the platform bus.
> 
> What a shame, it's one of the cleanest driver probing models
> in the tree.

And BTW, have you folks who "decided" this considered at all the fact
that it is much easier to describe represent platform devices using
of OF devices rather than the other way around?

The platform device pdata mechanism requires data structure changes
and is not dynamically extensible, whereas OF devices are
fundamentally so.

I don't like the idea to get rid of of_platform devices at all.

OF devices are really clean, much like netlink messages, where
arbitrary named attributes can be added or removed without any data
structure changes at all.

Re: Deprecating of_platform, the path from here...

[Grant Likely]

Hi David,

On Wed, Dec 9, 2009 at 5:21 PM, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 09 Dec 2009 16:15:50 -0800 (PST)
>
>> From: Grant Likely <grant.likely@secretlab.ca>
>> Date: Wed, 9 Dec 2009 15:06:29 -0700
>>
>>> 1) of_platform will be deprecated in preference of the platform bus.
>>
>> What a shame, it's one of the cleanest driver probing models
>> in the tree.
>
> And BTW, have you folks who "decided" this considered at all the fact
> that it is much easier to describe represent platform devices using
> of OF devices rather than the other way around?

Yup.  I also think of_platform is a cleaner implementation than
platform, but the fact remains that there are far more platform
drivers than there are of_platform.  So, as nice as of_platform is, it
still does pretty much exactly the same job as platform.  I'd rather
see of_platform features migrated to platform than creating drivers
with dual registrations to be used on both OF and non-OF platforms.

Trying to go the other way around (deprecate platform and encouraging
of_platform instead) I don't think will gain much traction; whereas I
think bringing of_platform features into platform will be an easier
sell.  I'm trying to be pragmatic here.

> The platform device pdata mechanism requires data structure changes
> and is not dynamically extensible, whereas OF devices are
> fundamentally so.
>
> I don't like the idea to get rid of of_platform devices at all.

There are no plans to actually remove of_platform.  I certainly don't
plan to try and force SPARC to switch from of_platform to platform
bus.  But on PowerPC (and probably Microblaze) the plan is to move
away from of_platform for all the reasons discussed, and I'm not be
bringing of_platform over as I work on ARM support.

> OF devices are really clean, much like netlink messages, where
> arbitrary named attributes can be added or removed without any data
> structure changes at all.

No argument here.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Deprecating of_platform, the path from here...

[David Miller]

From: Grant Likely <grant.likely@secretlab.ca>
Date: Thu, 10 Dec 2009 13:47:33 -0700

> Trying to go the other way around (deprecate platform and encouraging
> of_platform instead) I don't think will gain much traction; whereas I
> think bringing of_platform features into platform will be an easier
> sell.  I'm trying to be pragmatic here.

When people use words like "traction" and "pragmatic" that means they
are making decisions for reasons other than technical ones. :-)

Re: Deprecating of_platform, the path from here...

[Grant Likely]

On Thu, Dec 10, 2009 at 2:56 PM, David Miller <davem@davemloft.net> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Thu, 10 Dec 2009 13:47:33 -0700
>
>> Trying to go the other way around (deprecate platform and encouraging
>> of_platform instead) I don't think will gain much traction; whereas I
>> think bringing of_platform features into platform will be an easier
>> sell. =A0I'm trying to be pragmatic here.
>
> When people use words like "traction" and "pragmatic" that means they
> are making decisions for reasons other than technical ones. :-)

100% true.  :-)

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Deprecating of_platform, the path from here...

[Arnd Bergmann]

On Thursday 10 December 2009, Grant Likely wrote:
> On Wed, Dec 9, 2009 at 5:21 PM, David Miller <davem@davemloft.net> wrote:
> > From: David Miller <davem@davemloft.net>
> > Date: Wed, 09 Dec 2009 16:15:50 -0800 (PST)
> >> What a shame, it's one of the cleanest driver probing models
> >> in the tree.
> >
> > And BTW, have you folks who "decided" this considered at all the fact
> > that it is much easier to describe represent platform devices using
> > of OF devices rather than the other way around?
> 
> Yup.  I also think of_platform is a cleaner implementation than
> platform, but the fact remains that there are far more platform
> drivers than there are of_platform.  So, as nice as of_platform is, it
> still does pretty much exactly the same job as platform.  I'd rather
> see of_platform features migrated to platform than creating drivers
> with dual registrations to be used on both OF and non-OF platforms.

As far as I can tell, there is another path to allow unifying the drivers
that currently need to register to both platform_bus and of_bus, without
destroying the features we currently have in those two.

> Trying to go the other way around (deprecate platform and encouraging
> of_platform instead) I don't think will gain much traction; whereas I
> think bringing of_platform features into platform will be an easier
> sell.  I'm trying to be pragmatic here.

The key to the solution IMHO is the ability to create an of_platform_device
in a hardcoded way without data from a device tree, like we create a
platform_device today. All these static of_devices would then be rooted
in /sys/platform by default, while those that come from a device tree are
in the hierarchy defined there.

With your generalized device tree support, you have come most of
the way there, so the next step could be to create an
of_platform_device_register or of_platform_device_{alloc,add} function
that behaves like platform_device_register but adds an of_device
instead.

Something along the lines of

struct of_platform_device {
	of_device dev;			  /* this is what gets registered */
	struct device_node node; /* can't point to the device tree */
	struct property compatible;
};

int of_platform_device_register(struct of_platform_device *ofpdev,
					const char *name,	struct property *properties,
					struct device *parent)
{
	/* fill out the of_device */
	ofpdev->dev.node = &ofpdev->node;
	ofpdev->dev.dev.bus = &of_platform_bus_type;
	ofpdev->dev.dev.parent = parent;
	ofpdev->dev.dev.archdata.of_node = &ofpdev->node;

	/* fill out only the parts of the node that are absolutely required */
	ofpdev->node.name = name;
	if (!ofpdev->node.type)
		ofpdev->node.type = "platform"; /* not sure */
	ofpdev->node.properties = &ofpdev->compatible;
	kref_init(&ofpdev->node.kref);

	/* add properties that we always want */
	ofpdev->compatible.name = "compatible";
	ofpdev->compatible.length = strlen(name);
	ofpdev->compatible.value = name;

	/* add any user-specified properties and chain them */
	ofpdev->compatible.next = properties;
	while (properties->name)
		properties->next = ++properties;


	return of_device_register(&ofpdev->dev);
}

There are obviously detailed that need to be worked out for this,
but it should give you an easy way of defining of_platform_devices.
All existing platform_drivers can stay unchanged, but if any driver
should deal with both of_device and platform_device, you just
kill the platform_driver in there and transform the platform_device
definitions for this driver to of_platform_device definitions.

	Arnd

Re: Deprecating of_platform, the path from here...

[Joakim Tjernlund]

>
> On Thursday 10 December 2009, Grant Likely wrote:
> > On Wed, Dec 9, 2009 at 5:21 PM, David Miller <davem@davemloft.net> wrote:
> > > From: David Miller <davem@davemloft.net>
> > > Date: Wed, 09 Dec 2009 16:15:50 -0800 (PST)
> > >> What a shame, it's one of the cleanest driver probing models
> > >> in the tree.
> > >
> > > And BTW, have you folks who "decided" this considered at all the fact
> > > that it is much easier to describe represent platform devices using
> > > of OF devices rather than the other way around?
> >
> > Yup.  I also think of_platform is a cleaner implementation than
> > platform, but the fact remains that there are far more platform
> > drivers than there are of_platform.  So, as nice as of_platform is, it
> > still does pretty much exactly the same job as platform.  I'd rather
> > see of_platform features migrated to platform than creating drivers
> > with dual registrations to be used on both OF and non-OF platforms.
>
> As far as I can tell, there is another path to allow unifying the drivers
> that currently need to register to both platform_bus and of_bus, without
> destroying the features we currently have in those two.
>
> > Trying to go the other way around (deprecate platform and encouraging
> > of_platform instead) I don't think will gain much traction; whereas I
> > think bringing of_platform features into platform will be an easier
> > sell.  I'm trying to be pragmatic here.
>
> The key to the solution IMHO is the ability to create an of_platform_device
> in a hardcoded way without data from a device tree, like we create a
> platform_device today. All these static of_devices would then be rooted
> in /sys/platform by default, while those that come from a device tree are
> in the hierarchy defined there.

Yes, please. I don't like the direction some drivers are heading, namely OF only.
I always tried to view OF as an layer on top of generic methods. If
you look at spi_mpc83xx.c, it is actually less capable today since
platform has been declared legacy.

    Jocke

Re: Deprecating of_platform, the path from here...

[Grant Likely]

On Fri, Dec 11, 2009 at 8:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 10 December 2009, Grant Likely wrote:
>> On Wed, Dec 9, 2009 at 5:21 PM, David Miller <davem@davemloft.net> wrote=
:
>> > From: David Miller <davem@davemloft.net>
>> > Date: Wed, 09 Dec 2009 16:15:50 -0800 (PST)
>> >> What a shame, it's one of the cleanest driver probing models
>> >> in the tree.
>> >
>> > And BTW, have you folks who "decided" this considered at all the fact
>> > that it is much easier to describe represent platform devices using
>> > of OF devices rather than the other way around?
>>
>> Yup. =A0I also think of_platform is a cleaner implementation than
>> platform, but the fact remains that there are far more platform
>> drivers than there are of_platform. =A0So, as nice as of_platform is, it
>> still does pretty much exactly the same job as platform. =A0I'd rather
>> see of_platform features migrated to platform than creating drivers
>> with dual registrations to be used on both OF and non-OF platforms.
>
> As far as I can tell, there is another path to allow unifying the drivers
> that currently need to register to both platform_bus and of_bus, without
> destroying the features we currently have in those two.
>
>> Trying to go the other way around (deprecate platform and encouraging
>> of_platform instead) I don't think will gain much traction; whereas I
>> think bringing of_platform features into platform will be an easier
>> sell. =A0I'm trying to be pragmatic here.
>
> The key to the solution IMHO is the ability to create an of_platform_devi=
ce
> in a hardcoded way without data from a device tree, like we create a
> platform_device today. All these static of_devices would then be rooted
> in /sys/platform by default, while those that come from a device tree are
> in the hierarchy defined there.
>
> With your generalized device tree support, you have come most of
> the way there, so the next step could be to create an
> of_platform_device_register or of_platform_device_{alloc,add} function
> that behaves like platform_device_register but adds an of_device
> instead.

The problem remains that of_platform and platform essentially do
exactly the same job.  It's not interesting to me for it to be
'normal' to use both in the same kernel.  My goal is to have a single
bus and not have to choose between them when writing a driver.  I can
already hear the howls of protest when the first patch to migrate a
platform driver to of_platform get posted... and requires all users of
the driver to now include the of_platform infrastructure.  Basically
it means adding more code, imposing code churn, and increasing the
size of the kernel for no measurable benefit for the existing
architecture users.

platform users far outnumber of_platform users.  I actually don't care
which becomes the 'preferred' bus, just as long as one is chosen.  It
is easy to migrate features between them.  When I look at the work
required though, I think it is far more feasible to fold of_platform
features into platform bus than it is to ask current platform users to
migrate over to of_platform.

Now, if consensus can be reached among architecture maintainers to
make of_platform the preferred approach, and to deprecate platform
bus, then I'm all for it and I'll work towards it  However, I
personally don't think it will fly and so I'm not spending any effort
on that direction.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Deprecating of_platform, the path from here...

[Arnd Bergmann]

On Friday 11 December 2009 16:44:32 Grant Likely wrote:

> platform users far outnumber of_platform users.  I actually don't care
> which becomes the 'preferred' bus, just as long as one is chosen.  It
> is easy to migrate features between them.  When I look at the work
> required though, I think it is far more feasible to fold of_platform
> features into platform bus than it is to ask current platform users to
> migrate over to of_platform.

Yes, I think you have convinced me. For me the key argument is that
we can extend platform_bus to do everything that of_platform_bus
does today. 

If we can automatically turn "reg" and "interrupt" properties into
resources for the platform_devices created from a device tree,
and add interfaces to platform_device to operate directly on properties
of the underlying device, I'm happy.

DMA address translation is something that will require some care
to get right with platform_device, and it's important that we come
up with a nice syntax to define properties for regular
platform_devices that do not come from a device tree.

> Now, if consensus can be reached among architecture maintainers to
> make of_platform the preferred approach, and to deprecate platform
> bus, then I'm all for it and I'll work towards it  However, I
> personally don't think it will fly and so I'm not spending any effort
> on that direction.

Right.

	Arnd <><

Re: Deprecating of_platform, the path from here...

[Benjamin Herrenschmidt]

> The key to the solution IMHO is the ability to create an of_platform_device
> in a hardcoded way without data from a device tree, like we create a
> platform_device today. All these static of_devices would then be rooted
> in /sys/platform by default, while those that come from a device tree are
> in the hierarchy defined there.

 .../...

But that has the exact same problem as the other approaches proposed,
which is that in order to convert a driver, all the platforms in all
archs that use it need to be converted to use the new mechanisms,
properties etc...

This is going to be really hard since we don't have access to all of
that HW and will introduce huge lag as I can expect the various embedded
platforms maintainers being in no hurry to help initially.

Cheers,
Ben.

Re: Deprecating of_platform, the path from here...

[Benjamin Herrenschmidt]

On Wed, 2009-12-09 at 16:15 -0800, David Miller wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Wed, 9 Dec 2009 15:06:29 -0700
> 
> > 1) of_platform will be deprecated in preference of the platform bus.
> 
> What a shame, it's one of the cleanest driver probing models
> in the tree.

It is indeed. However, I'm not 100% convinced it requires keeping a
separate bus & device type, if we can make the base platform_device
benefit from it transparently.

So here's a (long) email summarizing my thoughts in those matters. I
would really appreciate if you took the time to read it through, think
it through and then speak your mind. As you will notice, I'm not myself
completely sold on the way to go there myself and some of your ideas do
tempt me very much indeed :-)

First let's clearly separate the two different aspects of this
discussion for the sake of lurkers :-) One is the probing & matching
mechanism.  The other one is how the driver retrieves it's various
parameters, either from a device node, or from pdata. From our IRC
discussion you have valid points on both aspect of the argument so I'll
try to discuss this here separately and we'll see what others think too.

First the probing because that's the real important issue, I believe the
other one is mostly academic and can be dealt on a per driver basis
(I'll discuss it later too).

I'm not totally sold on the idea of the table that Grant proposed. I
think I proposed it initially in a discussion we had on IRC as one
possible option and in fact it was suggested by Paulus so don't
completely blame Grant for it though. It does mean that either platform
code or core code will have some kind of big table that associates a
whole pile of of_device_id with a platform device name. It somewhat
sucks when I think more about it and it does feel like a step backward
from of_platform_device.

If we could make platform_driver grow an of_device_id match list it
would be indeed nicer. Of course you'll reply to that "let's just use
of_platform" instead :-)

I would then argue that I don't like having two different bus types,
platform and of_platform for "generic platform" devices that aren't
typed on a bus type. There's a lot of existing "platform device" and it
would be quite hard to convert them all. Especially since a lot of them
are used today on archs that don't have device trees and may never grow
one.

IE. I believe it's going to be an easier path to grow platform_device
into something that has the of_device probing functionality rather than
turn everything into of_platform. (I'm not talking here about the pdata
and retrieval of informations from the tree, this is the second part of
the discussion, discussed below).

The main problem with moving existing platform_device to of_platform is
how do we deal with archs that don't have the device-tree infrastructure
and use those drivers today ?

We could I suppose create a helper that looks a lot like the current
platform device creation one, which would create an of_platform device
instead, along with a struct device_node attached (which isn't part of a
tree) to it, and create a single "compatible" property whose content is
the platform data name.

But that means that for every driver we want to be able to use a
device-tree probing for, we would have to convert all archs & platforms
that may instanciate it to use the new helpers, in addition to replacing
whatever pdata they have statically stored into a device node (see the
second part of the discussion).

It's possible I suppose. I just feel that it's going to be a tougher
sell to the rest of the world.

There's one nit to be careful of. Some drivers (sadly) have the fact
that they appear under /sys/bus/platform/ as a userspace ABI thingy. Sad
but a fact. This is one of the reasons why rather than actually
converting to of_platform I'd rather find a way to add the of_device_id
match mechanism to the existing platform_device and deprecate the pdata
over time. 

It will also provide with an easier transition. Basically transform
platform_device into of_platform_device by first adding the missing bits
and -then- trimming the crufty remains.

Now let's move to the second part of the discussion which is the
retrieval of various configuration informations by the driver.

Here too, our model is better, I think there's little argument there. A
named list of properties is just so much more flexible than a statically
data structure that has to be in sync between the driver and all
platforms using it leaves little room for improvement or adding platform
specific attributes which some drivers might need, etc...

Grant proposal is to have drivers create the pdata from the device-tree.
This is something I believe we both disagree with, though you more
vehemently than me I suppose :-)

There are various things at play here:

First, let me make it clear that imho, the device type
(of_platform_device vs. platform_device) is irrelevant to that aspect of
the problem since nowadays we have the ability to carry a device node
pointer in any descendant of struct device (and we use that heavily for
devices using specific bus types already).

If you take an existing platform driver that you want to use on a
device-tree enabled platform (other than just creating it and pdata from
arch code which we all agree sucks), the two choices have different
consequences:

In one case, converting to of_platform_device, you pretty much _have_ to
get rid of pdata, and convert the driver into using of_get_property()
instead. This is probably not a bad thing in the long run, except that
this means you also -have- to convert all platforms in all archs that
use that specific platform driver to also generate a device node
(possibly statically in many cases).

This can be a lot of code churn deep into platform code for things like
ARM which can be pretty nasty, for which none of us have any way to test
on the relevant hardware. Other arch people will (maybe rightfully so)
protest especially if they have no intent to use the device-tree stuff
in their architecture or not yet anyways. And that for each platform
driver involved.

On the other case, converting to platform_device, adding the device
node, we have the ability to do an easier transition and easier to sell.
yes, we do take the risk of getting in that limbo land where drivers end
up forever in the "transition" state though. That's a con of this
approach, I do admit.

I don't agree with grant idea however that just converting the content
of the device node into properties is the way to go.

I do prefer your proposed approach (from our IRC discussion) which is
instead to allocate a struct device-node, convert pdata into properties,
and modify the drier to use these properties.

The main difference thus between the two type of conversions (convert to
of_platform vs convert to platform) is that in the first case, you have
to convert the driver to use properties -and- convert all platforms in
all archs including gory ARM cell phone stuff you really don't want to
go anywhere near. In the second case, you still convert the driver to
use properties natively, but you keep a "wart" to turn pdata into a
device-node -inside the driver-, protected by a CONFIG option maybe, so
that those archs can be left alone until it becomes so obvious to
everybody what approach is better that they'll end up being converted
too and the wart can go.

I believe the second approach, while less "clean" in the absolute is a
more realistic path to take.

Now, orthogonally to that, I do believe it's still nice to provide a way
to statically lay out a device node in platform code, to allow archs
that don't otherwise have the device-tree to replace pdata with
something nicer and get rid of the wart quicker.

We could either find a way with macros to layout an actual struct
device_node statically along with all the properties etc... but that
sounds a tad hard.

We could have something that convert an entirely ASCII representation
into a struct device_node, but that would be akin of having dtc in the
kernel, might be a bit bloated no ? Though it could be made simpler and
more restrictive.

Or we could find an in-between .. .A different struct type that is more
suitable for being laid out statically (a name, a type, and an enum of
structs for various property "types", ie, strings, words, bytes, ...)
with a little helper function that conver that into a device node at
runtime ?

What do you think ?

Cheers,
Ben.

Re: Deprecating of_platform, the path from here...

[Benjamin Herrenschmidt]

On Thu, 2009-12-10 at 12:45 +1100, Benjamin Herrenschmidt wrote:

> I don't agree with grant idea however that just converting the content
> of the device node into properties is the way to go.

And here of course I meant " converting the content of the device node
into into pdata" ...

> I do prefer your proposed approach (from our IRC discussion) which is
> instead to allocate a struct device-node, convert pdata into properties,
> and modify the drier to use these properties.
> 
> The main difference thus between the two type of conversions (convert to
> of_platform vs convert to platform) is that in the first case, you have
> to convert the driver to use properties -and- convert all platforms in
> all archs including gory ARM cell phone stuff you really don't want to
> go anywhere near. In the second case, you still convert the driver to
> use properties natively, but you keep a "wart" to turn pdata into a
> device-node -inside the driver-, protected by a CONFIG option maybe, so
> that those archs can be left alone until it becomes so obvious to
> everybody what approach is better that they'll end up being converted
> too and the wart can go.
> 
> I believe the second approach, while less "clean" in the absolute is a
> more realistic path to take.
> 
> Now, orthogonally to that, I do believe it's still nice to provide a way
> to statically lay out a device node in platform code, to allow archs
> that don't otherwise have the device-tree to replace pdata with
> something nicer and get rid of the wart quicker.
> 
> We could either find a way with macros to layout an actual struct
> device_node statically along with all the properties etc... but that
> sounds a tad hard.
> 
> We could have something that convert an entirely ASCII representation
> into a struct device_node, but that would be akin of having dtc in the
> kernel, might be a bit bloated no ? Though it could be made simpler and
> more restrictive.
> 
> Or we could find an in-between .. .A different struct type that is more
> suitable for being laid out statically (a name, a type, and an enum of
> structs for various property "types", ie, strings, words, bytes, ...)
> with a little helper function that conver that into a device node at
> runtime ?
> 
> What do you think ?
> 
> Cheers,
> Ben.
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: Deprecating of_platform, the path from here...

[Grant Likely]

Hey guys, some more thoughts below...

On Wed, Dec 9, 2009 at 6:45 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> First the probing because that's the real important issue, I believe the
> other one is mostly academic and can be dealt on a per driver basis
> (I'll discuss it later too).
>
> I'm not totally sold on the idea of the table that Grant proposed. I
> think I proposed it initially in a discussion we had on IRC as one
> possible option and in fact it was suggested by Paulus so don't
> completely blame Grant for it though. It does mean that either platform
> code or core code will have some kind of big table that associates a
> whole pile of of_device_id with a platform device name. It somewhat
> sucks when I think more about it and it does feel like a step backward
> from of_platform_device.

For the record, I don't like it either.  But it is the pragmatic thing
to do because it is simple to implement, and can easily be replaced
later with a better scheme with little impact on drivers.  Basically,
it's a migration plan.

> If we could make platform_driver grow an of_device_id match list it
> would be indeed nicer. Of course you'll reply to that "let's just use
> of_platform" instead :-)
>
> I would then argue that I don't like having two different bus types,
> platform and of_platform for "generic platform" devices that aren't
> typed on a bus type. There's a lot of existing "platform device" and it
> would be quite hard to convert them all. Especially since a lot of them
> are used today on archs that don't have device trees and may never grow
> one.
>
> IE. I believe it's going to be an easier path to grow platform_device
> into something that has the of_device probing functionality rather than
> turn everything into of_platform. (I'm not talking here about the pdata
> and retrieval of informations from the tree, this is the second part of
> the discussion, discussed below).
>
> The main problem with moving existing platform_device to of_platform is
> how do we deal with archs that don't have the device-tree infrastructure
> and use those drivers today ?
>
> We could I suppose create a helper that looks a lot like the current
> platform device creation one, which would create an of_platform device
> instead, along with a struct device_node attached (which isn't part of a
> tree) to it, and create a single "compatible" property whose content is
> the platform data name.
>
> But that means that for every driver we want to be able to use a
> device-tree probing for, we would have to convert all archs & platforms
> that may instanciate it to use the new helpers, in addition to replacing
> whatever pdata they have statically stored into a device node (see the
> second part of the discussion).
>
> It's possible I suppose. I just feel that it's going to be a tougher
> sell to the rest of the world.
>
> There's one nit to be careful of. Some drivers (sadly) have the fact
> that they appear under /sys/bus/platform/ as a userspace ABI thingy. Sad
> but a fact. This is one of the reasons why rather than actually
> converting to of_platform I'd rather find a way to add the of_device_id
> match mechanism to the existing platform_device and deprecate the pdata
> over time.
>
> It will also provide with an easier transition. Basically transform
> platform_device into of_platform_device by first adding the missing bits
> and -then- trimming the crufty remains.

I 100% agree with you here on the matching problem.

> Now let's move to the second part of the discussion which is the
> retrieval of various configuration informations by the driver.
>
> Here too, our model is better, I think there's little argument there. A
> named list of properties is just so much more flexible than a statically
> data structure that has to be in sync between the driver and all
> platforms using it leaves little room for improvement or adding platform
> specific attributes which some drivers might need, etc...
>
> Grant proposal is to have drivers create the pdata from the device-tree.
> This is something I believe we both disagree with, though you more
> vehemently than me I suppose :-)

See my pdata comment lower down.

> There are various things at play here:
>
> First, let me make it clear that imho, the device type
> (of_platform_device vs. platform_device) is irrelevant to that aspect of
> the problem since nowadays we have the ability to carry a device node
> pointer in any descendant of struct device (and we use that heavily for
> devices using specific bus types already).

And even if we kept of_platform; the same problem of parsing device
tree data exists for all other bus types.  Places where of_platform
isn't used today (i2c, spi, pci, mdio, etc).  It makes sense to
establish a pattern for matching and probing with device tree data
that is usable by all bus types.

> If you take an existing platform driver that you want to use on a
> device-tree enabled platform (other than just creating it and pdata from
> arch code which we all agree sucks), the two choices have different
> consequences:
>
> In one case, converting to of_platform_device, you pretty much _have_ to
> get rid of pdata, and convert the driver into using of_get_property()
> instead. This is probably not a bad thing in the long run, except that
> this means you also -have- to convert all platforms in all archs that
> use that specific platform driver to also generate a device node
> (possibly statically in many cases).
>
> This can be a lot of code churn deep into platform code for things like
> ARM which can be pretty nasty, for which none of us have any way to test
> on the relevant hardware. Other arch people will (maybe rightfully so)
> protest especially if they have no intent to use the device-tree stuff
> in their architecture or not yet anyways. And that for each platform
> driver involved.
>
> On the other case, converting to platform_device, adding the device
> node, we have the ability to do an easier transition and easier to sell.
> yes, we do take the risk of getting in that limbo land where drivers end
> up forever in the "transition" state though. That's a con of this
> approach, I do admit.
>
> I don't agree with grant idea however that just converting the content
> of the device node into properties is the way to go.

s/properties/pdata

I'm being pragmatic here.  pdata sucks, but the lowest impact path to
getting device tree data into a lot of existing drivers is to populate
the data source it is already using.  However, at least by putting the
translator into the driver itself, the pdata isn't an anonymous
pointer anymore and the compiler can correctly type check it.

But this is a minor point.  It really is contained within the driver
with no external impact.  pdata can be eliminated one driver at a
time.

> I do prefer your proposed approach (from our IRC discussion) which is
> instead to allocate a struct device-node, convert pdata into properties,
> and modify the drier to use these properties.

I like this approach.  It would actually be quite easy to convert on a
driver-by-driver basis from .pdata to device nodes.  Even platforms
that statically declare their platform devices could be changed by
statically declaring the sub tree for the device.  For (a completely
bogus) example:

  static struct platform_device omap3beagle_nand_device = {
  	.name		= "omap2-nand",
  	.id		= -1,
- 	.dev		= {
- 		.platform_data	= &omap3beagle_nand_data,
- 	},
- 	.num_resources	= 1,
- 	.resource	= &omap3beagle_nand_resource,
+ 	.device_node = (struct device_node *) {
+ 		.name = "nand",
+ 		.properties = (struct property *) {
+ 			.name = "blah",
+ 			.length = sizeof("foo"),
+ 			.value = "foo"
+ 			.next = (struct property *)
+ 		{
+ 			.name = "bar",
+ 			.length = sizeof("wazzit"),
+ 			.value = "wazzit"
+ 		} }
+ 	},
  };

Naturally this example is brutally ugly, and it could be done in a far
cleaner manner; but you get the idea.  As you and I just discussed on
IRC, the device_node structure isn't really designed for static
declarations, but it could work.  Basically it would allow the
migration to a better model that ugly, non-type-checked anonymous
pdata pointers.

> The main difference thus between the two type of conversions (convert to
> of_platform vs convert to platform) is that in the first case, you have
> to convert the driver to use properties -and- convert all platforms in
> all archs including gory ARM cell phone stuff you really don't want to
> go anywhere near. In the second case, you still convert the driver to
> use properties natively, but you keep a "wart" to turn pdata into a
> device-node -inside the driver-, protected by a CONFIG option maybe, so
> that those archs can be left alone until it becomes so obvious to
> everybody what approach is better that they'll end up being converted
> too and the wart can go.
>
> I believe the second approach, while less "clean" in the absolute is a
> more realistic path to take.
>
> Now, orthogonally to that, I do believe it's still nice to provide a way
> to statically lay out a device node in platform code, to allow archs
> that don't otherwise have the device-tree to replace pdata with
> something nicer and get rid of the wart quicker.
>
> We could either find a way with macros to layout an actual struct
> device_node statically along with all the properties etc... but that
> sounds a tad hard.

Heh.  I hacked out my example above before reading this far.

> We could have something that convert an entirely ASCII representation
> into a struct device_node, but that would be akin of having dtc in the
> kernel, might be a bit bloated no ? Though it could be made simpler and
> more restrictive.

Ewh.  Ugly.

> Or we could find an in-between .. .A different struct type that is more
> suitable for being laid out statically (a name, a type, and an enum of
> structs for various property "types", ie, strings, words, bytes, ...)
> with a little helper function that conver that into a device node at
> runtime ?

I think this bears some investigation.  A slightly different layout
(ie, using arrays instead of linked list for properties) would be
easier to prepare.  Worth some investigation.

> What do you think ?

I liked what I read.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.