Re: [PATCH] Input: twl4030 - convert to using managed resources

On Tuesday, January 07, 2014 3:07 AM, Dmitry Torokhov wrote:

On Mon, Jan 06, 2014 at 11:25:54AM +0900, Jingoo Han wrote:

quoted

On Saturday, January 04, 2014 6:02 PM, Dmitry Torokhov wrote:

quoted

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Compile-tested only.


 drivers/input/keyboard/twl4030_keypad.c | 70 +++++++++++----------------------
 1 file changed, 22 insertions(+), 48 deletions(-)

diff --git a/drivers/input/keyboard/twl4030_keypad.c b/drivers/input/keyboard/twl4030_keypad.c
index f663c19..71eb041 100644
--- a/drivers/input/keyboard/twl4030_keypad.c
+++ b/drivers/input/keyboard/twl4030_keypad.c

[.....]

quoted

-static int twl4030_kp_remove(struct platform_device *pdev)
-{
-	struct twl4030_keypad *kp = platform_get_drvdata(pdev);
-
-	free_irq(kp->irq, kp);
-	input_unregister_device(kp->input);

Hi Dmitry Torokhov,

IMHO, input_unregister_device() seems to be necessary,
because input_register_device() is still used.
If I am wrong, please let me know kindly.

No, input_unregister_device() is not needed (although it can be used)
with managed input devices (ones that are allocated with
devm_input_allocate_device()) because input_register_device() will
automatically insert devres entry for such devices. So for managed input
devices that were registered with input subsystem there are 2 separate
devres entries: one for unregistering the device and another is to
finally release all resources. If you had a sequence:

	devm_kzalloc(...);
	devm_input_allocate_device(...);
	devm_devm_ioremap_resource(...);
	devm_request_irq(...);
	input_register_device(...);

then unwind sequence will be:

	unregister input device (but input device structure is still
		present in memory and input_event() can be called, but
		events won't be delivered anywhere - that is done so
		that we can survive releasing IRQs after unregistering
		managed input device)
	free IRQ
	release resources
	free input device structure
	free kzalloc memory

Hope this helps.

Oh, I really appreciate your detailed and kind comments. :-)

I checked the following. As you said, input_register_device()
automatically inserts devres entry.

./drivers/input/input.c
int input_register_device(struct input_dev *dev)
{
        struct input_devres *devres = NULL;
        struct input_handler *handler;
        unsigned int packet_size;
        const char *path;
        int error;

        if (dev->devres_managed) {
                devres = devres_alloc(devm_input_device_unregister,
                                      sizeof(struct input_devres), GFP_KERNEL);

Thank you so much.

Best regards,
Jingoo Han