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