[PATCH 0/4] Improve intel-hid's self-consistency

[PATCH 0/4] Improve intel-hid's self-consistency

[Michał Kępień]

Commit 2cfa4b311acc ("platform/x86: intel-hid: Support 5 button array")
contains some coding style cleanups compared to the code it was based
on.  This patch series "backports" these cleanups to the rest of the
driver in order to make it more self-consistent and removes a redundant
set_bit() call as a bonus.

This series was not tested on hardware supported by intel-hid.

 drivers/platform/x86/intel-hid.c | 48 ++++++++++------------------------------
 1 file changed, 12 insertions(+), 36 deletions(-)

-- 
2.11.1

[PATCH 1/4] platform/x86: intel-hid: simplify enabling/disabling HID events

[Michał Kępień]

ACPI method HDSM takes a single integer argument.  Use
acpi_execute_simple_method() instead of acpi_evaluate_object() for
calling that ACPI method to simplify code and reduce the number of local
variables inside intel_hid_set_enable().

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/intel-hid.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c
index bcf438f38781..4d1c5eb3a96d 100644
--- a/drivers/platform/x86/intel-hid.c
+++ b/drivers/platform/x86/intel-hid.c
@@ -79,12 +79,10 @@ struct intel_hid_priv {
 
 static int intel_hid_set_enable(struct device *device, int enable)
 {
-	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
-	struct acpi_object_list args = { 1, &arg0 };
 	acpi_status status;
 
-	arg0.integer.value = enable;
-	status = acpi_evaluate_object(ACPI_HANDLE(device), "HDSM", &args, NULL);
+	status = acpi_execute_simple_method(ACPI_HANDLE(device), "HDSM",
+					    enable);
 	if (ACPI_FAILURE(status)) {
 		dev_warn(device, "failed to %sable hotkeys\n",
 			 enable ? "en" : "dis");
-- 
2.11.1

[PATCH 2/4] platform/x86: intel-hid: make intel_hid_set_enable() take a boolean argument

[Michał Kępień]

As the integer value passed to intel_hid_set_enable() is always
explicitly passed and is used solely as a boolean value, make it a bool.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/intel-hid.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c
index 4d1c5eb3a96d..89d1d98e3214 100644
--- a/drivers/platform/x86/intel-hid.c
+++ b/drivers/platform/x86/intel-hid.c
@@ -77,7 +77,7 @@ struct intel_hid_priv {
 	struct input_dev *array;
 };
 
-static int intel_hid_set_enable(struct device *device, int enable)
+static int intel_hid_set_enable(struct device *device, bool enable)
 {
 	acpi_status status;
 
@@ -118,7 +118,7 @@ static void intel_button_array_enable(struct device *device, bool enable)
 
 static int intel_hid_pl_suspend_handler(struct device *device)
 {
-	intel_hid_set_enable(device, 0);
+	intel_hid_set_enable(device, false);
 	intel_button_array_enable(device, false);
 
 	return 0;
@@ -126,7 +126,7 @@ static int intel_hid_pl_suspend_handler(struct device *device)
 
 static int intel_hid_pl_resume_handler(struct device *device)
 {
-	intel_hid_set_enable(device, 1);
+	intel_hid_set_enable(device, true);
 	intel_button_array_enable(device, true);
 
 	return 0;
@@ -275,7 +275,7 @@ static int intel_hid_probe(struct platform_device *device)
 		goto err_remove_input;
 	}
 
-	err = intel_hid_set_enable(&device->dev, 1);
+	err = intel_hid_set_enable(&device->dev, true);
 	if (err)
 		goto err_remove_notify;
 
@@ -306,7 +306,7 @@ 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);
+	intel_hid_set_enable(&device->dev, false);
 	intel_button_array_enable(&device->dev, false);
 
 	/*
-- 
2.11.1

[PATCH 3/4] platform/x86: intel-hid: use devm_input_allocate_device() for HID events input device

[Michał Kępień]

intel_hid_input_setup() is only called from the platform driver's .probe
callback.  Use the devm variant of input_allocate_device() for
allocating memory for the HID events input device in order to simplify
two error paths and get rid of intel_hid_input_destroy().

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/intel-hid.c | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c
index 89d1d98e3214..b17f33b832c2 100644
--- a/drivers/platform/x86/intel-hid.c
+++ b/drivers/platform/x86/intel-hid.c
@@ -144,28 +144,20 @@ static int intel_hid_input_setup(struct platform_device *device)
 	struct intel_hid_priv *priv = dev_get_drvdata(&device->dev);
 	int ret;
 
-	priv->input_dev = input_allocate_device();
+	priv->input_dev = devm_input_allocate_device(&device->dev);
 	if (!priv->input_dev)
 		return -ENOMEM;
 
 	ret = sparse_keymap_setup(priv->input_dev, intel_hid_keymap, NULL);
 	if (ret)
-		goto err_free_device;
+		return ret;
 
 	priv->input_dev->dev.parent = &device->dev;
 	priv->input_dev->name = "Intel HID events";
 	priv->input_dev->id.bustype = BUS_HOST;
 	set_bit(KEY_RFKILL, priv->input_dev->keybit);
 
-	ret = input_register_device(priv->input_dev);
-	if (ret)
-		goto err_free_device;
-
-	return 0;
-
-err_free_device:
-	input_free_device(priv->input_dev);
-	return ret;
+	return input_register_device(priv->input_dev);
 }
 
 static int intel_button_array_input_setup(struct platform_device *device)
@@ -189,13 +181,6 @@ static int intel_button_array_input_setup(struct platform_device *device)
 	return input_register_device(priv->array);
 }
 
-static void intel_hid_input_destroy(struct platform_device *device)
-{
-	struct intel_hid_priv *priv = dev_get_drvdata(&device->dev);
-
-	input_unregister_device(priv->input_dev);
-}
-
 static void notify_handler(acpi_handle handle, u32 event, void *context)
 {
 	struct platform_device *device = context;
@@ -270,10 +255,8 @@ static int intel_hid_probe(struct platform_device *device)
 					     ACPI_DEVICE_NOTIFY,
 					     notify_handler,
 					     device);
-	if (ACPI_FAILURE(status)) {
-		err = -EBUSY;
-		goto err_remove_input;
-	}
+	if (ACPI_FAILURE(status))
+		return -EBUSY;
 
 	err = intel_hid_set_enable(&device->dev, true);
 	if (err)
@@ -294,9 +277,6 @@ static int intel_hid_probe(struct platform_device *device)
 err_remove_notify:
 	acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
 
-err_remove_input:
-	intel_hid_input_destroy(device);
-
 	return err;
 }
 
@@ -305,7 +285,6 @@ static int intel_hid_remove(struct platform_device *device)
 	acpi_handle handle = ACPI_HANDLE(&device->dev);
 
 	acpi_remove_notify_handler(handle, ACPI_DEVICE_NOTIFY, notify_handler);
-	intel_hid_input_destroy(device);
 	intel_hid_set_enable(&device->dev, false);
 	intel_button_array_enable(&device->dev, false);
 
-- 
2.11.1

[PATCH 4/4] platform/x86: intel-hid: remove redundant set_bit() call

[Michał Kępień]

Remove redundant set_bit() call for KEY_RFKILL as it is already made by
sparse_keymap_setup() due to KEY_RFKILL being assigned to event code 8
in intel_hid_keymap.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/intel-hid.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c
index b17f33b832c2..5eab31659cba 100644
--- a/drivers/platform/x86/intel-hid.c
+++ b/drivers/platform/x86/intel-hid.c
@@ -155,7 +155,6 @@ static int intel_hid_input_setup(struct platform_device *device)
 	priv->input_dev->dev.parent = &device->dev;
 	priv->input_dev->name = "Intel HID events";
 	priv->input_dev->id.bustype = BUS_HOST;
-	set_bit(KEY_RFKILL, priv->input_dev->keybit);
 
 	return input_register_device(priv->input_dev);
 }
-- 
2.11.1

Re: [PATCH 0/4] Improve intel-hid's self-consistency

[Andy Shevchenko]

On Fri, Feb 24, 2017 at 12:33 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> Commit 2cfa4b311acc ("platform/x86: intel-hid: Support 5 button array")
> contains some coding style cleanups compared to the code it was based
> on.  This patch series "backports" these cleanups to the rest of the
> driver in order to make it more self-consistent and removes a redundant
> set_bit() call as a bonus.
>
> This series was not tested on hardware supported by intel-hid.

Alex, do you have an access to hardware to test? I wouldn't going to
apply this until I get your tag(s) (in the best case
Reviewed-and-tested-by).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/4] Improve intel-hid's self-consistency

[Alex Hung]

On Thu, Mar 2, 2017 at 7:03 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Feb 24, 2017 at 12:33 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>> Commit 2cfa4b311acc ("platform/x86: intel-hid: Support 5 button array")
>> contains some coding style cleanups compared to the code it was based
>> on.  This patch series "backports" these cleanups to the rest of the
>> driver in order to make it more self-consistent and removes a redundant
>> set_bit() call as a bonus.
>>
>> This series was not tested on hardware supported by intel-hid.
>
> Alex, do you have an access to hardware to test? I wouldn't going to
> apply this until I get your tag(s) (in the best case
> Reviewed-and-tested-by).

The patches look good and tested on Latitude 7480

Reviewed-and-tested-by: Alex Hung <alex.hung@canonical.com>

>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH 0/4] Improve intel-hid's self-consistency

[Andy Shevchenko]

On Thu, Mar 2, 2017 at 12:17 PM, Alex Hung <alex.hung@canonical.com> wrote:
> On Thu, Mar 2, 2017 at 7:03 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Fri, Feb 24, 2017 at 12:33 PM, Michał Kępień <kernel@kempniu.pl> wrote:
>>> Commit 2cfa4b311acc ("platform/x86: intel-hid: Support 5 button array")
>>> contains some coding style cleanups compared to the code it was based
>>> on.  This patch series "backports" these cleanups to the rest of the
>>> driver in order to make it more self-consistent and removes a redundant
>>> set_bit() call as a bonus.
>>>
>>> This series was not tested on hardware supported by intel-hid.
>>
>> Alex, do you have an access to hardware to test? I wouldn't going to
>> apply this until I get your tag(s) (in the best case
>> Reviewed-and-tested-by).
>
> The patches look good and tested on Latitude 7480
>
> Reviewed-and-tested-by: Alex Hung <alex.hung@canonical.com>

Pushed to testing.
Thanks.

-- 
With Best Regards,
Andy Shevchenko