On Mon, Apr 29, 2013 at 9:19 PM, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > 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@xxxxxxxxxx> 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: >> > 1. short one - fix your patches >> > 2. 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: >> >> 1. 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". >> >> 2. 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. Dmitry, thanks for the explanation. Mark, please take my apologies, I was mislead by the following comment on the code: http://lxr.free-electrons.com/source/drivers/input/input.c#L1825 1822 * input_free_device - free memory occupied by input_dev structure 1823 * @dev: input device to free 1824 * 1825 * This function should only be used if input_register_device() 1826 * was not called yet or if it failed. > > -- > Dmitry -- Leon Romanovsky | Independent Linux Consultant www.leon.nu | leon@xxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
