[PATCH 1/6] platform/x86: acer-wmi: Drop no-op set_quirks call from find_quirks

[PATCH 1/6] platform/x86: acer-wmi: Drop no-op set_quirks call from find_quirks

[Hans de Goede]

set_quirks has a "if (!interface) return;" check at its beginning and
interface always is NULL when set_quirks is called from find_quirks,
so it is a no-op and we can drop it.

This also allows dropping the "if (!interface) return;" from set_quirks
since set_quirks now always is called with interface != NULL.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/acer-wmi.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 49f4b73be513..db6167a10981 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -302,9 +302,6 @@ static struct quirk_entry *quirks;
 
 static void __init set_quirks(void)
 {
-	if (!interface)
-		return;
-
 	if (quirks->mailled)
 		interface->capability |= ACER_CAP_MAILLED;
 
@@ -648,8 +645,6 @@ static void __init find_quirks(void)
 
 	if (quirks == NULL)
 		quirks = &quirk_unknown;
-
-	set_quirks();
 }
 
 /*
-- 
2.28.0

[PATCH 2/6] platform/x86: acer-wmi: Cleanup ACER_CAP_FOO defines

[Hans de Goede]

Cleanup the ACER_CAP_FOO defines:
-Switch to using BIT() macro.
-The ACER_CAP_RFBTN flag is set, but it is never checked anywhere, drop it.
-Drop the unused ACER_CAP_ANY define.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/acer-wmi.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index db6167a10981..2f69d418ed3b 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -205,14 +205,12 @@ struct hotkey_function_type_aa {
 /*
  * Interface capability flags
  */
-#define ACER_CAP_MAILLED		(1<<0)
-#define ACER_CAP_WIRELESS		(1<<1)
-#define ACER_CAP_BLUETOOTH		(1<<2)
-#define ACER_CAP_BRIGHTNESS		(1<<3)
-#define ACER_CAP_THREEG			(1<<4)
-#define ACER_CAP_ACCEL			(1<<5)
-#define ACER_CAP_RFBTN			(1<<6)
-#define ACER_CAP_ANY			(0xFFFFFFFF)
+#define ACER_CAP_MAILLED		BIT(0)
+#define ACER_CAP_WIRELESS		BIT(1)
+#define ACER_CAP_BLUETOOTH		BIT(2)
+#define ACER_CAP_BRIGHTNESS		BIT(3)
+#define ACER_CAP_THREEG			BIT(4)
+#define ACER_CAP_ACCEL			BIT(5)
 
 /*
  * Interface type flags
@@ -1247,10 +1245,8 @@ static void __init type_aa_dmi_decode(const struct dmi_header *header, void *d)
 		interface->capability |= ACER_CAP_THREEG;
 	if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_BLUETOOTH)
 		interface->capability |= ACER_CAP_BLUETOOTH;
-	if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_RFBTN) {
-		interface->capability |= ACER_CAP_RFBTN;
+	if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_RFBTN)
 		commun_func_bitmap &= ~ACER_WMID3_GDS_RFBTN;
-	}
 
 	commun_fn_key_number = type_aa->commun_fn_key_number;
 }
-- 
2.28.0

Re: [PATCH 2/6] platform/x86: acer-wmi: Cleanup ACER_CAP_FOO defines

[Andy Shevchenko]

On Mon, Oct 19, 2020 at 9:57 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Cleanup the ACER_CAP_FOO defines:
> -Switch to using BIT() macro.
> -The ACER_CAP_RFBTN flag is set, but it is never checked anywhere, drop it.
> -Drop the unused ACER_CAP_ANY define.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/platform/x86/acer-wmi.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index db6167a10981..2f69d418ed3b 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -205,14 +205,12 @@ struct hotkey_function_type_aa {

Should't we add bits.h as well?

>  /*
>   * Interface capability flags
>   */
> -#define ACER_CAP_MAILLED               (1<<0)
> -#define ACER_CAP_WIRELESS              (1<<1)
> -#define ACER_CAP_BLUETOOTH             (1<<2)
> -#define ACER_CAP_BRIGHTNESS            (1<<3)
> -#define ACER_CAP_THREEG                        (1<<4)
> -#define ACER_CAP_ACCEL                 (1<<5)
> -#define ACER_CAP_RFBTN                 (1<<6)
> -#define ACER_CAP_ANY                   (0xFFFFFFFF)
> +#define ACER_CAP_MAILLED               BIT(0)
> +#define ACER_CAP_WIRELESS              BIT(1)
> +#define ACER_CAP_BLUETOOTH             BIT(2)
> +#define ACER_CAP_BRIGHTNESS            BIT(3)
> +#define ACER_CAP_THREEG                        BIT(4)
> +#define ACER_CAP_ACCEL                 BIT(5)
>
>  /*
>   * Interface type flags
> @@ -1247,10 +1245,8 @@ static void __init type_aa_dmi_decode(const struct dmi_header *header, void *d)
>                 interface->capability |= ACER_CAP_THREEG;
>         if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_BLUETOOTH)
>                 interface->capability |= ACER_CAP_BLUETOOTH;
> -       if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_RFBTN) {
> -               interface->capability |= ACER_CAP_RFBTN;
> +       if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_RFBTN)
>                 commun_func_bitmap &= ~ACER_WMID3_GDS_RFBTN;
> -       }
>
>         commun_fn_key_number = type_aa->commun_fn_key_number;
>  }
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko

[PATCH 3/6] platform/x86: acer-wmi: Cleanup accelerometer device handling

[Hans de Goede]

Cleanup accelerometer device handling:
-Drop acer_wmi_accel_destroy instead directly call input_unregister_device.
-The information tracked by the CAP_ACCEL flag mirrors acer_wmi_accel_dev
 being NULL. Drop the CAP flag, this is a preparation change for allowing
 users to override the capability flags. Dropping the flag stops users
 from causing a NULL pointer dereference by forcing the capability.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/acer-wmi.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 2f69d418ed3b..f728cb5e5115 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -210,7 +210,6 @@ struct hotkey_function_type_aa {
 #define ACER_CAP_BLUETOOTH		BIT(2)
 #define ACER_CAP_BRIGHTNESS		BIT(3)
 #define ACER_CAP_THREEG			BIT(4)
-#define ACER_CAP_ACCEL			BIT(5)
 
 /*
  * Interface type flags
@@ -1510,7 +1509,7 @@ static int acer_gsensor_event(void)
 	struct acpi_buffer output;
 	union acpi_object out_obj[5];
 
-	if (!has_cap(ACER_CAP_ACCEL))
+	if (!acer_wmi_accel_dev)
 		return -1;
 
 	output.length = sizeof(out_obj);
@@ -1884,8 +1883,6 @@ static int __init acer_wmi_accel_setup(void)
 	gsensor_handle = acpi_device_handle(adev);
 	acpi_dev_put(adev);
 
-	interface->capability |= ACER_CAP_ACCEL;
-
 	acer_wmi_accel_dev = input_allocate_device();
 	if (!acer_wmi_accel_dev)
 		return -ENOMEM;
@@ -1911,11 +1908,6 @@ static int __init acer_wmi_accel_setup(void)
 	return err;
 }
 
-static void acer_wmi_accel_destroy(void)
-{
-	input_unregister_device(acer_wmi_accel_dev);
-}
-
 static int __init acer_wmi_input_setup(void)
 {
 	acpi_status status;
@@ -2070,7 +2062,7 @@ static int acer_resume(struct device *dev)
 	if (has_cap(ACER_CAP_BRIGHTNESS))
 		set_u32(data->brightness, ACER_CAP_BRIGHTNESS);
 
-	if (has_cap(ACER_CAP_ACCEL))
+	if (acer_wmi_accel_dev)
 		acer_gsensor_init();
 
 	return 0;
@@ -2260,8 +2252,8 @@ static int __init acer_wmi_init(void)
 error_platform_register:
 	if (wmi_has_guid(ACERWMID_EVENT_GUID))
 		acer_wmi_input_destroy();
-	if (has_cap(ACER_CAP_ACCEL))
-		acer_wmi_accel_destroy();
+	if (acer_wmi_accel_dev)
+		input_unregister_device(acer_wmi_accel_dev);
 
 	return err;
 }
@@ -2271,8 +2263,8 @@ static void __exit acer_wmi_exit(void)
 	if (wmi_has_guid(ACERWMID_EVENT_GUID))
 		acer_wmi_input_destroy();
 
-	if (has_cap(ACER_CAP_ACCEL))
-		acer_wmi_accel_destroy();
+	if (acer_wmi_accel_dev)
+		input_unregister_device(acer_wmi_accel_dev);
 
 	remove_debugfs();
 	platform_device_unregister(acer_platform_device);
-- 
2.28.0

[PATCH 4/6] platform/x86: acer-wmi: Add new force_caps module parameter

[Hans de Goede]

Add a new force_caps module parameter to allow overriding the drivers
builtin capability detection mechanism.

This can be used to for example:
-Disable rfkill functionality on devices where there is an AA OEM DMI
 record advertising non functional rfkill switches
-Force loading of the driver on devices with a missing AA OEM DMI record

Note that force_caps is -1 when unset, this allows forcing the
capability field to 0, which results in acer-wmi only providing WMI
hotkey handling while disabling all other (led, rfkill, backlight)
functionality.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/acer-wmi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index f728cb5e5115..87f64b91d283 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -232,6 +232,7 @@ static int mailled = -1;
 static int brightness = -1;
 static int threeg = -1;
 static int force_series;
+static int force_caps = -1;
 static bool ec_raw_mode;
 static bool has_type_aa;
 static u16 commun_func_bitmap;
@@ -241,11 +242,13 @@ module_param(mailled, int, 0444);
 module_param(brightness, int, 0444);
 module_param(threeg, int, 0444);
 module_param(force_series, int, 0444);
+module_param(force_caps, int, 0444);
 module_param(ec_raw_mode, bool, 0444);
 MODULE_PARM_DESC(mailled, "Set initial state of Mail LED");
 MODULE_PARM_DESC(brightness, "Set initial LCD backlight brightness");
 MODULE_PARM_DESC(threeg, "Set initial state of 3G hardware");
 MODULE_PARM_DESC(force_series, "Force a different laptop series");
+MODULE_PARM_DESC(force_caps, "Force the capability bitmask to this value");
 MODULE_PARM_DESC(ec_raw_mode, "Enable EC raw mode");
 
 struct acer_data {
@@ -2163,7 +2166,7 @@ static int __init acer_wmi_init(void)
 		}
 		/* WMID always provides brightness methods */
 		interface->capability |= ACER_CAP_BRIGHTNESS;
-	} else if (!wmi_has_guid(WMID_GUID2) && interface && !has_type_aa) {
+	} else if (!wmi_has_guid(WMID_GUID2) && interface && !has_type_aa && force_caps == -1) {
 		pr_err("No WMID device detection method found\n");
 		return -ENODEV;
 	}
@@ -2193,6 +2196,9 @@ static int __init acer_wmi_init(void)
 	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
 		interface->capability &= ~ACER_CAP_BRIGHTNESS;
 
+	if (force_caps != -1)
+		interface->capability = force_caps;
+
 	if (wmi_has_guid(WMID_GUID3)) {
 		if (ACPI_FAILURE(acer_wmi_enable_rf_button()))
 			pr_warn("Cannot enable RF Button Driver\n");
-- 
2.28.0

[PATCH 5/6] platform/x86: acer-wmi: Add ACER_CAP_SET_FUNCTION_MODE capability flag

[Hans de Goede]

Not all devices supporting WMID_GUID3 support the wmid3_set_function_mode()
call, leading to errors like these:

[   60.138358] acer_wmi: Enabling RF Button failed: 0x1 - 0xff
[   60.140036] acer_wmi: Enabling Launch Manager failed: 0x1 - 0xff

Add an ACER_CAP_SET_FUNCTION_MODE capability flag, so that these calls
can be disabled through the new force_caps mechanism.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/acer-wmi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 87f64b91d283..8bf9e6ed38a4 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -210,6 +210,7 @@ struct hotkey_function_type_aa {
 #define ACER_CAP_BLUETOOTH		BIT(2)
 #define ACER_CAP_BRIGHTNESS		BIT(3)
 #define ACER_CAP_THREEG			BIT(4)
+#define ACER_CAP_SET_FUNCTION_MODE	BIT(5)
 
 /*
  * Interface type flags
@@ -2196,10 +2197,14 @@ static int __init acer_wmi_init(void)
 	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
 		interface->capability &= ~ACER_CAP_BRIGHTNESS;
 
+	if (wmi_has_guid(WMID_GUID3))
+		interface->capability |= ACER_CAP_SET_FUNCTION_MODE;
+
 	if (force_caps != -1)
 		interface->capability = force_caps;
 
-	if (wmi_has_guid(WMID_GUID3)) {
+	if (wmi_has_guid(WMID_GUID3) &&
+	    (interface->capability & ACER_CAP_SET_FUNCTION_MODE)) {
 		if (ACPI_FAILURE(acer_wmi_enable_rf_button()))
 			pr_warn("Cannot enable RF Button Driver\n");
 
-- 
2.28.0

[PATCH 6/6] platform/x86: acer-wmi: Add support for SW_TABLET_MODE on Switch devices

[Hans de Goede]

Add support for SW_TABLET_MODE on the Acer Switch 10 (SW5-012) and the
acer Switch 10 (S1003) models.

There is no way to detect if this is supported, so this uses DMI based
quirks setting force_caps to ACER_CAP_KBD_DOCK (these devices have no
other acer-wmi based functionality).

The new SW_TABLET_MODE functionality can be tested on devices which
are not in the DMI table by passing acer_wmi.force_caps=0x40 on the
kernel commandline.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/acer-wmi.c | 108 +++++++++++++++++++++++++++++++-
 1 file changed, 105 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 8bf9e6ed38a4..68d1a0c8c205 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -30,6 +30,7 @@
 #include <linux/input/sparse-keymap.h>
 #include <acpi/video.h>
 
+ACPI_MODULE_NAME(KBUILD_MODNAME);
 MODULE_AUTHOR("Carlos Corbacho");
 MODULE_DESCRIPTION("Acer Laptop WMI Extras Driver");
 MODULE_LICENSE("GPL");
@@ -80,7 +81,7 @@ MODULE_ALIAS("wmi:676AA15E-6A47-4D9F-A2CC-1E6D18D14026");
 
 enum acer_wmi_event_ids {
 	WMID_HOTKEY_EVENT = 0x1,
-	WMID_ACCEL_EVENT = 0x5,
+	WMID_ACCEL_OR_KBD_DOCK_EVENT = 0x5,
 };
 
 static const struct key_entry acer_wmi_keymap[] __initconst = {
@@ -127,7 +128,9 @@ struct event_return_value {
 	u8 function;
 	u8 key_num;
 	u16 device_state;
-	u32 reserved;
+	u16 reserved1;
+	u8 kbd_dock_state;
+	u8 reserved2;
 } __attribute__((packed));
 
 /*
@@ -211,6 +214,7 @@ struct hotkey_function_type_aa {
 #define ACER_CAP_BRIGHTNESS		BIT(3)
 #define ACER_CAP_THREEG			BIT(4)
 #define ACER_CAP_SET_FUNCTION_MODE	BIT(5)
+#define ACER_CAP_KBD_DOCK		BIT(6)
 
 /*
  * Interface type flags
@@ -316,6 +320,15 @@ static int __init dmi_matched(const struct dmi_system_id *dmi)
 	return 1;
 }
 
+static int __init set_force_caps(const struct dmi_system_id *dmi)
+{
+	if (force_caps == -1) {
+		force_caps = (uintptr_t)dmi->driver_data;
+		pr_info("Found %s, set force_caps to 0x%x\n", dmi->ident, force_caps);
+	}
+	return 1;
+}
+
 static struct quirk_entry quirk_unknown = {
 };
 
@@ -494,6 +507,24 @@ static const struct dmi_system_id acer_quirks[] __initconst = {
 		},
 		.driver_data = &quirk_acer_travelmate_2490,
 	},
+	{
+		.callback = set_force_caps,
+		.ident = "Acer Aspire Switch 10 SW5-012",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire SW5-012"),
+		},
+		.driver_data = (void *)ACER_CAP_KBD_DOCK,
+	},
+	{
+		.callback = set_force_caps,
+		.ident = "Acer One 10 (S1003)",
+		.matches = {
+			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Acer"),
+			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "One S1003"),
+		},
+		.driver_data = (void *)ACER_CAP_KBD_DOCK,
+	},
 	{}
 };
 
@@ -1536,6 +1567,71 @@ static int acer_gsensor_event(void)
 	return 0;
 }
 
+/*
+ * Switch series keyboard dock status
+ */
+static int acer_kbd_dock_state_to_sw_tablet_mode(u8 kbd_dock_state)
+{
+	switch (kbd_dock_state) {
+	case 0x01: /* Docked, traditional clamshell laptop mode */
+		return 0;
+	case 0x04: /* Stand-alone tablet */
+	case 0x40: /* Docked, tent mode, keyboard not usable */
+		return 1;
+	default:
+		pr_warn("Unknown kbd_dock_state 0x%02x\n", kbd_dock_state);
+	}
+
+	return 0;
+}
+
+static void acer_kbd_dock_get_initial_state(void)
+{
+	u8 *output, input[8] = { 0x05, 0x00, };
+	struct acpi_buffer input_buf = { sizeof(input), input };
+	struct acpi_buffer output_buf = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	acpi_status status;
+	int sw_tablet_mode;
+
+	status = wmi_evaluate_method(WMID_GUID3, 0, 0x2, &input_buf, &output_buf);
+	if (ACPI_FAILURE(status)) {
+		ACPI_EXCEPTION((AE_INFO, status, "Error getting keyboard-dock initial status"));
+		return;
+	}
+
+	obj = output_buf.pointer;
+	if (!obj || obj->type != ACPI_TYPE_BUFFER || obj->buffer.length != 8) {
+		pr_err("Unexpected output format getting keyboard-dock initial status\n");
+		goto out_free_obj;
+	}
+
+	output = obj->buffer.pointer;
+	if (output[0] != 0x00 || (output[3] != 0x05 && output[3] != 0x45)) {
+		pr_err("Unexpected output [0]=0x%02x [3]=0x%02x getting keyboard-dock initial status\n",
+		       output[0], output[3]);
+		goto out_free_obj;
+	}
+
+	sw_tablet_mode = acer_kbd_dock_state_to_sw_tablet_mode(output[4]);
+	input_report_switch(acer_wmi_input_dev, SW_TABLET_MODE, sw_tablet_mode);
+
+out_free_obj:
+	kfree(obj);
+}
+
+static void acer_kbd_dock_event(const struct event_return_value *event)
+{
+	int sw_tablet_mode;
+
+	if (!has_cap(ACER_CAP_KBD_DOCK))
+		return;
+
+	sw_tablet_mode = acer_kbd_dock_state_to_sw_tablet_mode(event->kbd_dock_state);
+	input_report_switch(acer_wmi_input_dev, SW_TABLET_MODE, sw_tablet_mode);
+	input_sync(acer_wmi_input_dev);
+}
+
 /*
  * Rfkill devices
  */
@@ -1763,8 +1859,9 @@ static void acer_wmi_notify(u32 value, void *context)
 			sparse_keymap_report_event(acer_wmi_input_dev, scancode, 1, true);
 		}
 		break;
-	case WMID_ACCEL_EVENT:
+	case WMID_ACCEL_OR_KBD_DOCK_EVENT:
 		acer_gsensor_event();
+		acer_kbd_dock_event(&return_value);
 		break;
 	default:
 		pr_warn("Unknown function number - %d - %d\n",
@@ -1929,6 +2026,11 @@ static int __init acer_wmi_input_setup(void)
 	if (err)
 		goto err_free_dev;
 
+	if (has_cap(ACER_CAP_KBD_DOCK)) {
+		input_set_capability(acer_wmi_input_dev, EV_SW, SW_TABLET_MODE);
+		acer_kbd_dock_get_initial_state();
+	}
+
 	status = wmi_install_notify_handler(ACERWMID_EVENT_GUID,
 						acer_wmi_notify, NULL);
 	if (ACPI_FAILURE(status)) {
-- 
2.28.0

Re: [PATCH 1/6] platform/x86: acer-wmi: Drop no-op set_quirks call from find_quirks

[Andy Shevchenko]

On Mon, Oct 19, 2020 at 9:56 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> set_quirks has a "if (!interface) return;" check at its beginning and
> interface always is NULL when set_quirks is called from find_quirks,
> so it is a no-op and we can drop it.
>
> This also allows dropping the "if (!interface) return;" from set_quirks
> since set_quirks now always is called with interface != NULL.

Is there any cover letter for this?

In any case, LGTM, you may use my tag
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

I'm not sure about ACPI_EXCEPTION() use, though.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/platform/x86/acer-wmi.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 49f4b73be513..db6167a10981 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -302,9 +302,6 @@ static struct quirk_entry *quirks;
>
>  static void __init set_quirks(void)
>  {
> -       if (!interface)
> -               return;
> -
>         if (quirks->mailled)
>                 interface->capability |= ACER_CAP_MAILLED;
>
> @@ -648,8 +645,6 @@ static void __init find_quirks(void)
>
>         if (quirks == NULL)
>                 quirks = &quirk_unknown;
> -
> -       set_quirks();
>  }
>
>  /*
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/6] platform/x86: acer-wmi: Drop no-op set_quirks call from find_quirks

[Hans de Goede]

Hi,

On 10/26/20 4:15 PM, Andy Shevchenko wrote:
> On Mon, Oct 19, 2020 at 9:56 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> set_quirks has a "if (!interface) return;" check at its beginning and
>> interface always is NULL when set_quirks is called from find_quirks,
>> so it is a no-op and we can drop it.
>>
>> This also allows dropping the "if (!interface) return;" from set_quirks
>> since set_quirks now always is called with interface != NULL.
> 
> Is there any cover letter for this?

Nope, in hindsight I should have probably added one. The intention was
to only write patch 6/6 the rest is all prep work / cleanups to make
patch 6/6 possible.

> In any case, LGTM, you may use my tag
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Thank you, I assume that applies to the entire series ?

> I'm not sure about ACPI_EXCEPTION() use, though.

That is the only way I could find to easily print a sensible
error message given an acpi status code return...

Regards,

Hans




> 
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/platform/x86/acer-wmi.c | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
>> index 49f4b73be513..db6167a10981 100644
>> --- a/drivers/platform/x86/acer-wmi.c
>> +++ b/drivers/platform/x86/acer-wmi.c
>> @@ -302,9 +302,6 @@ static struct quirk_entry *quirks;
>>
>>  static void __init set_quirks(void)
>>  {
>> -       if (!interface)
>> -               return;
>> -
>>         if (quirks->mailled)
>>                 interface->capability |= ACER_CAP_MAILLED;
>>
>> @@ -648,8 +645,6 @@ static void __init find_quirks(void)
>>
>>         if (quirks == NULL)
>>                 quirks = &quirk_unknown;
>> -
>> -       set_quirks();
>>  }
>>
>>  /*
>> --
>> 2.28.0
>>
> 
> 

Re: [PATCH 1/6] platform/x86: acer-wmi: Drop no-op set_quirks call from find_quirks

[Andy Shevchenko]

On Mon, Oct 26, 2020 at 5:43 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 10/26/20 4:15 PM, Andy Shevchenko wrote:
> > On Mon, Oct 19, 2020 at 9:56 PM Hans de Goede <hdegoede@redhat.com> wrote:

> > In any case, LGTM, you may use my tag
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Thank you, I assume that applies to the entire series ?

Yes.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/6] platform/x86: acer-wmi: Drop no-op set_quirks call from find_quirks

[Hans de Goede]

Hi,

On 10/26/20 5:55 PM, Andy Shevchenko wrote:
> On Mon, Oct 26, 2020 at 5:43 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 10/26/20 4:15 PM, Andy Shevchenko wrote:
>>> On Mon, Oct 19, 2020 at 9:56 PM Hans de Goede <hdegoede@redhat.com> wrote:
> 
>>> In any case, LGTM, you may use my tag
>>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>> Thank you, I assume that applies to the entire series ?
> 
> Yes.

Great, I've added the series to my review-hans branch with your
Reviewed-by added.

Regards,

Hans