Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1437165 > unrolled thread

[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.

Started byArvind Yadav <arvind.yadav.cs@gmail.com>
First post2016-07-05 18:30 +0200
Last post2016-07-06 20:10 +0200
Articles 4 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [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. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2016-07-05 18:30 +0200
    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. Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-07-05 19:10 +0200
      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. arvind Yadav <arvind.yadav.cs@gmail.com> - 2016-07-06 13:30 +0200
      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. Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-07-06 20:10 +0200

#1437165 — [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.

FromArvind Yadav <arvind.yadav.cs@gmail.com>
Date2016-07-05 18:30 +0200
Subject[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<rRBou-5cR-27@gated-at.bofh.it>
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 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);
 	if (error) {
 		dev_err(&pdev->dev,
 			"failed to register quiesce action, error: %d\n",
-- 
1.9.1

[toc] | [next] | [standalone]


#1437197 — 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.

FromBjorn Andersson <bjorn.andersson@linaro.org>
Date2016-07-05 19:10 +0200
SubjectRe: [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<rRC1c-5HP-33@gated-at.bofh.it>
In reply to#1437165
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 <arvind.yadav.cs@gmail.com>
> ---
>  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

[toc] | [prev] | [next] | [standalone]


#1437648 — 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.

Fromarvind Yadav <arvind.yadav.cs@gmail.com>
Date2016-07-06 13:30 +0200
SubjectRe: [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<rRTbH-71-7@gated-at.bofh.it>
In reply to#1437197
Sorry for subject. Thanks for reviewing my patch. I will take care and 
follow the standard now.

regards,
Arvind


On Tuesday 05 July 2016 10:31 PM, Bjorn Andersson wrote:
> 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 <arvind.yadav.cs@gmail.com>
>> ---
>>   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

[toc] | [prev] | [next] | [standalone]


#1437893 — 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.

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2016-07-06 20:10 +0200
SubjectRe: [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<rRZqO-47a-17@gated-at.bofh.it>
In reply to#1437197
On Tue, Jul 05, 2016 at 10:01:11AM -0700, Bjorn Andersson wrote:
> 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 <arvind.yadav.cs@gmail.com>
> > ---
> >  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.

Agreed.

> 
> >  	if (error) {
> >  		dev_err(&pdev->dev,
> >  			"failed to register quiesce action, error: %d\n",
> 
> Regards,
> Bjorn

-- 
Dmitry

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web