diff mbox

intel-hid: remove duplicated acpi_remove_notify_handler and extra indents

Message ID 1467083382-31634-1-git-send-email-alex.hung@canonical.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Alex Hung June 28, 2016, 3:09 a.m. UTC
Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 drivers/platform/x86/intel-hid.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Darren Hart June 28, 2016, 4:35 a.m. UTC | #1
On Tue, Jun 28, 2016 at 11:09:42AM +0800, Alex Hung wrote:

What triggered this patch? Was an error/panic observed? Is this a theoretical
bug or an observed one? This will impact if it goes back to stable. Please
provide a complete change log.

> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
>  drivers/platform/x86/intel-hid.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c
> index a818db6..ed58742 100644
> --- a/drivers/platform/x86/intel-hid.c
> +++ b/drivers/platform/x86/intel-hid.c
> @@ -122,8 +122,8 @@ static int intel_hid_input_setup(struct platform_device *device)
>  	return 0;
>  
>  err_free_device:
> -		input_free_device(priv->input_dev);
> -		return ret;
> +	input_free_device(priv->input_dev);
> +	return ret;
>  }
>  
>  static void intel_hid_input_destroy(struct platform_device *device)
> @@ -224,7 +224,6 @@ static int intel_hid_remove(struct platform_device *device)
>  	acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
>  	intel_hid_input_destroy(device);
>  	intel_hid_set_enable(&device->dev, 0);
> -	acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
>  
>  	/*
>  	 * Even if we failed to shut off the event stream, we can still
> -- 
> 2.7.4
> 
>
Alex Hung June 28, 2016, 7:55 a.m. UTC | #2
It was an observed error and it does not generate panic.  I also
checked the function in acpica and it validates parameters (as below)
and therefore it shall not cause any troubles; however, it is still
worthwhile to get it fixed.

acpi_status
acpi_remove_notify_handler(acpi_handle device,
                           u32 handler_type, acpi_notify_handler handler)
{
        ....
        /* Parameter validation */

        if ((!device) || (!handler) || (!handler_type) ||
            (handler_type > ACPI_MAX_NOTIFY_HANDLER_TYPE)) {
                return_ACPI_STATUS(AE_BAD_PARAMETER);
        }
        ....

I will include more details for this patch in V2.

On Tue, Jun 28, 2016 at 12:35 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Jun 28, 2016 at 11:09:42AM +0800, Alex Hung wrote:
>
> What triggered this patch? Was an error/panic observed? Is this a theoretical
> bug or an observed one? This will impact if it goes back to stable. Please
> provide a complete change log.
>
>> Signed-off-by: Alex Hung <alex.hung@canonical.com>
>> ---
>>  drivers/platform/x86/intel-hid.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c
>> index a818db6..ed58742 100644
>> --- a/drivers/platform/x86/intel-hid.c
>> +++ b/drivers/platform/x86/intel-hid.c
>> @@ -122,8 +122,8 @@ static int intel_hid_input_setup(struct platform_device *device)
>>       return 0;
>>
>>  err_free_device:
>> -             input_free_device(priv->input_dev);
>> -             return ret;
>> +     input_free_device(priv->input_dev);
>> +     return ret;
>>  }
>>
>>  static void intel_hid_input_destroy(struct platform_device *device)
>> @@ -224,7 +224,6 @@ static int intel_hid_remove(struct platform_device *device)
>>       acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
>>       intel_hid_input_destroy(device);
>>       intel_hid_set_enable(&device->dev, 0);
>> -     acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
>>
>>       /*
>>        * Even if we failed to shut off the event stream, we can still
>> --
>> 2.7.4
>>
>>
>
> --
> Darren Hart
> Intel Open Source Technology Center
diff mbox

Patch

diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c
index a818db6..ed58742 100644
--- a/drivers/platform/x86/intel-hid.c
+++ b/drivers/platform/x86/intel-hid.c
@@ -122,8 +122,8 @@  static int intel_hid_input_setup(struct platform_device *device)
 	return 0;
 
 err_free_device:
-		input_free_device(priv->input_dev);
-		return ret;
+	input_free_device(priv->input_dev);
+	return ret;
 }
 
 static void intel_hid_input_destroy(struct platform_device *device)
@@ -224,7 +224,6 @@  static int intel_hid_remove(struct platform_device *device)
 	acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
 	intel_hid_input_destroy(device);
 	intel_hid_set_enable(&device->dev, 0);
-	acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
 
 	/*
 	 * Even if we failed to shut off the event stream, we can still