On Mon, Apr 29, 2013 at 11:07:28AM -0700, Dmitry Torokhov wrote:
On Mon, Apr 29, 2013 at 08:47:37PM +0300, Leon Romanovsky wrote:
On Mon, Apr 29, 2013 at 1:19 PM, Mark Brown broonie@kernel.org wrote:
On Sun, Apr 28, 2013 at 09:32:18PM +0300, Leon Romanovsky wrote:
I think the reason of our misunderstanding is due to the name of input_free_device call. From the code, it is device destroy function, and the freeing itself done as an error handling of input_register_device (http://lxr.free-electrons.com/source/drivers/input/input.c#L2114).
How do you think we need to proceed? Do I need to send patches with explicit call to input_free_device function?
I really think the input API needs to be looked at here, this is all way too error prone. Calling input_free_device() on something allocated using devm_ looks like an error itself...
In general, I agree with you, but I think we both agree that the current patch is not working as expected. The problem is that you allocated device with devm_ and later at the code you tried to register it, but failed. In this case no one will call to devres_destroy, because it is done at unregister stage only.
I see two possible solutions:
- short one - fix your patches
- long one - add input_free_device code into input_register_device
call (http://lxr.free-electrons.com/source/drivers/input/input.c#L2114).
The rules are pretty straightforward:
- If you are using devm_input_allocate_device() you do not need to call
input_free_device() nor input_unregister_device() - the core will create a devres structure for freeing the device and if input_register_device() succeeds it will also add a 2nd devres for unregistering. This way the "normal" unwind is a 2-step process with device is "half alive" and being able to survive input_event() calls from IRQ handlers if they are still alive.
IOW it should all "just work".
- If you are using input_allocate_device() then you need to call
input_free_device() until you called input_register_device(), afterward input_unregister_device() should be called.
BTW, looking at Marks patch it looks good to me.