Thread (49 messages) 49 messages, 10 authors, 2008-06-30

Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration.

From: David Brownell <hidden>
Date: 2008-06-30 04:10:36
Also in: linux-spi, lkml

On Tuesday 17 June 2008, Grant Likely wrote:
quoted
quoted
quoted
This patch splits the allocation and registration portions of code out
of spi_new_device() and creates three new functions; spi_alloc_device(),
spi_register_device(), and spi_device_release().
I have no problem with the first two, but why the last?

If the devices are always allocated by spi_alloc_device() as
they should be -- probably through an intermediary -- the
only public function necessary for that cleanup should be
the existing spi_dev_put().
Ah, okay.  I'm still a bit fuzzy on the device model conventions.
I'll remove that then.
I've dug into this some more.  spi_alloc_device only allocates the
memory.  It doesn't call device_initialize() to initialize the kref.
Well, the driver model idiom is initialize() then add(), with
register() calls combining the two.  An alloc() is just a bit
outside those core idioms ...

But one alloc() example is platform_device_alloc(), which does
the device_initialize() call ... followed by platform_device_add().

The spi_new_device() call does a bunch of stuff beyond a register(),
but it also calls device_register().

All of that behaviour is handled within device_register().  Therefore
if a driver uses spi_alloc_device() and then if a later part of the
initialization fails before spi_register_device() is called, then the
alloc'd memory needs to be freed, but spi_dev_put() won't work because
the kobj isn't set up so I need another function to handle freeing it
in on a failure path.
I see ...

 
Should I switch things around to do device_initialize() in the alloc
function 
Yes.

and call device_add() instead of device_register() in the 
spi_register_device() function?
You should also rename it to spi_add_device(), since register()
calls always do the initialize() rather than having it done for
them in advance.  People rely on those names supporting that
pattern (as they should).

Is that sufficient to make put_device() work?
Looks like it to me.  Calling device_initialize() will
do a kobject_init(), which is documented as requiring
a kobject_put() to clean up ... that's all put_device()
will ever do.

- Dave
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help