From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755290AbcGERBS (ORCPT ); Tue, 5 Jul 2016 13:01:18 -0400 Received: from mail-pf0-f181.google.com ([209.85.192.181]:34859 "EHLO mail-pf0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752543AbcGERBQ (ORCPT ); Tue, 5 Jul 2016 13:01:16 -0400 Date: Tue, 5 Jul 2016 10:01:11 -0700 From: Bjorn Andersson To: Arvind Yadav Cc: dmitry.torokhov@gmail.com, ldewangan@nvidia.com, linus.walleij@linaro.org, van.freenix@gmail.com, bjorn.andersson@sonymobile.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] GPIO_KEYS: If devm_add_action() fails, we are explicitly calling the cleanup to freethe resources allocated. Use the helper devm_add_action_or_reset() and return directly in case of error, since the cleanup function has been already called by the helper if there was any error. Message-ID: <20160705170111.GO1190@tuxbot> References: <1467735500-5033-1-git-send-email-arvind.yadav.cs@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467735500-5033-1-git-send-email-arvind.yadav.cs@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 05 Jul 09:18 PDT 2016, Arvind Yadav wrote: Your subject line should be a short and descriptive summary, followed by an empty line and then potentially a longer description motivating your patch. Please read section #14 of Documentation/SubmittingPatches. > Signed-off-by: Arvind Yadav > --- > drivers/input/keyboard/gpio_keys.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c > index 2909365..a2fa3bb 100644 > --- a/drivers/input/keyboard/gpio_keys.c > +++ b/drivers/input/keyboard/gpio_keys.c > @@ -542,7 +542,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev, > * Install custom action to cancel release timer and > * workqueue item. > */ > - error = devm_add_action(&pdev->dev, gpio_keys_quiesce_key, bdata); > + error = devm_add_action_or_reset(&pdev->dev, gpio_keys_quiesce_key, > + bdata); The code leading up to this initializes the timer and work struct, but they are not triggered unless their respective isr is exectued. This would not happen until after the request_irq at the end of the function. As such there doesn't appear to be a reason for cancelling any work or timers. > if (error) { > dev_err(&pdev->dev, > "failed to register quiesce action, error: %d\n", Regards, Bjorn