Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration.
On Friday 23 May 2008, Grant Likely wrote: Question: spi_alloc_device() (and the original code) does a spi_master_get() on the spi_master device. Doesn't spi_master_put() need to be called when the device is discarded? spi_dev_put() doesn't do that explicitly; is it an implicit operation after a device has been deregistered from the spi_master? Depends whether or not the add() has been done to hook things into the driver model tree, as I recall. The add() presumes things are properly refcounted. When you make a driver model tree node vanish, its associated refcounts get updated too. - Dave ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration.
On Tuesday 17 June 2008, Grant Likely wrote: 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration.
On Sat, May 24, 2008 at 12:43 AM, Grant Likely [EMAIL PROTECTED] wrote: On Wed, May 21, 2008 at 6:17 PM, David Brownell [EMAIL PROTECTED] wrote: On Friday 16 May 2008, Grant Likely wrote: 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. 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. Should I switch things around to do device_initialize() in the alloc function and call device_add() instead of device_register() in the spi_register_device() function? Is that sufficient to make put_device() work? Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration.
On Wed, May 21, 2008 at 6:17 PM, David Brownell [EMAIL PROTECTED] wrote: On Friday 16 May 2008, Grant Likely wrote: 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. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration.
On Sat, May 24, 2008 at 12:43 AM, Grant Likely [EMAIL PROTECTED] wrote: On Wed, May 21, 2008 at 6:17 PM, David Brownell [EMAIL PROTECTED] wrote: On Friday 16 May 2008, Grant Likely wrote: 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. Question: spi_alloc_device() (and the original code) does a spi_master_get() on the spi_master device. Doesn't spi_master_put() need to be called when the device is discarded? spi_dev_put() doesn't do that explicitly; is it an implicit operation after a device has been deregistered from the spi_master? Thanks, g. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/4] spi: split up spi_new_device() to allow two stage registration.
On Friday 16 May 2008, Grant Likely wrote: 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(). - Dave ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
