Re: [PATCH 6/6] of/device: populate platform_device (of_device) resource table on allocation

2 messages, 2 authors, 2010-06-10 · open the first message on its own page

Re: [PATCH 6/6] of/device: populate platform_device (of_device) resource table on allocation

From: M. Warner Losh <hidden>
Date: 2010-06-10 15:13:57

In message: [off-list ref]
            Grant Likely [off-list ref] writes:
: On Thu, Jun 10, 2010 at 12:17 AM, Benjamin Herrenschmidt
: [off-list ref] wrote:
: >
: >> You just introduced an unnamed structure of device + resources,
: >> it isn't declared anywhere but in the code itself (either via
: >> &foo[1] or buf + sizeof(*foo)).
: >>
: >> You're not the only one who hacks (or at least have to
: >> understand) the OF stuff, so let's try keep this stuff
: >> readable?
: >>
: >> I told you several ways of how to improve the code (based on
: >> the ideas from drivers/base/, so the ideas aren't even mine,
: >> fwiw).
: >
: > I tend to agree with Anton here.
: 
: The reason I'm confident doing it that way is that it is *not* a
: structure.  There is no structure relationship between the resource
: table and the platform_device other than they are allocated with the
: same kzalloc() call.  All the code that cares about that is contained
: within 4 lines of code.  I'm resistant to using a structure because it
: is adds an additional 5-6 lines of code to add a structure that won't
: be used anywhere else, and is only 4 lines to begin with.

I tend to agree with Grant here.  The idiom he's using is very wide
spread in the industry and works extremely well.  It keeps the
ugliness confined to a couple of lines and is less ugly than the
alternatives for this design pattern.  It is a little surprising when
you see the code the first time, granted, but I think its expressive
power trumps that small surprise.

Warner

Re: [PATCH 6/6] of/device: populate platform_device (of_device) resource table on allocation

From: Anton Vorontsov <hidden>
Date: 2010-06-10 15:47:41

On Thu, Jun 10, 2010 at 09:13:57AM -0600, M. Warner Losh wrote:
[...]
: >> I told you several ways of how to improve the code (based on
: >> the ideas from drivers/base/, so the ideas aren't even mine,
: >> fwiw).
: >
: > I tend to agree with Anton here.
: 
: The reason I'm confident doing it that way is that it is *not* a
: structure.  There is no structure relationship between the resource
: table and the platform_device other than they are allocated with the
: same kzalloc() call.  All the code that cares about that is contained
: within 4 lines of code.  I'm resistant to using a structure because it
: is adds an additional 5-6 lines of code to add a structure that won't
: be used anywhere else, and is only 4 lines to begin with.

I tend to agree with Grant here.  The idiom he's using is very wide
spread in the industry and works extremely well.  It keeps the
ugliness confined to a couple of lines and is less ugly than the
alternatives for this design pattern.  It is a little surprising when
you see the code the first time, granted, but I think its expressive
power trumps that small surprise.
Oh, come on. Both constructions are binary equivalent.

So how can people seriously be with *that* code:

	dev->resource = (void *)&dev[1];

which, semantically, is a nonsense and asks for a fix.

While
	dev_obj->dev.resource = dev_obj->resource;

simply makes sense.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help