2022-08-09 03:33:22

by Luke D. Jones

[permalink] [raw]
Subject: [PATCH v3 1/2] asus-wmi: Adjust tablet/lidflip handling to use enum

Due to multiple types of tablet/lidflip, the existing code for
handling these events is refactored to use an enum for each type.

Signed-off-by: Luke D. Jones <[email protected]>
---
drivers/platform/x86/asus-nb-wmi.c | 13 +++---
drivers/platform/x86/asus-wmi.c | 67 ++++++++++++++++++++----------
drivers/platform/x86/asus-wmi.h | 9 +++-
3 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index a81dc4b191b7..3a93e056c4e1 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -115,12 +115,12 @@ static struct quirk_entry quirk_asus_forceals = {
};

static struct quirk_entry quirk_asus_use_kbd_dock_devid = {
- .use_kbd_dock_devid = true,
+ .tablet_switch_mode = asus_wmi_kbd_dock_devid,
};

static struct quirk_entry quirk_asus_use_lid_flip_devid = {
.wmi_backlight_set_devstate = true,
- .use_lid_flip_devid = true,
+ .tablet_switch_mode = asus_wmi_lid_flip_devid,
};

static int dmi_matched(const struct dmi_system_id *dmi)
@@ -492,16 +492,13 @@ static void asus_nb_wmi_quirks(struct asus_wmi_driver *driver)

switch (tablet_mode_sw) {
case 0:
- quirks->use_kbd_dock_devid = false;
- quirks->use_lid_flip_devid = false;
+ quirks->tablet_switch_mode = asus_wmi_no_tablet_switch;
break;
case 1:
- quirks->use_kbd_dock_devid = true;
- quirks->use_lid_flip_devid = false;
+ quirks->tablet_switch_mode = asus_wmi_kbd_dock_devid;
break;
case 2:
- quirks->use_kbd_dock_devid = false;
- quirks->use_lid_flip_devid = true;
+ quirks->tablet_switch_mode = asus_wmi_lid_flip_devid;
break;
}

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index c5fa21370481..029c26a218e1 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -507,8 +507,11 @@ static bool asus_wmi_dev_is_present(struct asus_wmi *asus, u32 dev_id)

static int asus_wmi_input_init(struct asus_wmi *asus)
{
+ struct device *dev;
int err, result;

+ dev = &asus->platform_device->dev;
+
asus->inputdev = input_allocate_device();
if (!asus->inputdev)
return -ENOMEM;
@@ -516,35 +519,38 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
asus->inputdev->name = asus->driver->input_name;
asus->inputdev->phys = asus->driver->input_phys;
asus->inputdev->id.bustype = BUS_HOST;
- asus->inputdev->dev.parent = &asus->platform_device->dev;
+ asus->inputdev->dev.parent = dev;
set_bit(EV_REP, asus->inputdev->evbit);

err = sparse_keymap_setup(asus->inputdev, asus->driver->keymap, NULL);
if (err)
goto err_free_dev;

- if (asus->driver->quirks->use_kbd_dock_devid) {
+ switch (asus->driver->quirks->tablet_switch_mode) {
+ case asus_wmi_no_tablet_switch:
+ break;
+ case asus_wmi_kbd_dock_devid:
result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_DOCK);
if (result >= 0) {
input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
input_report_switch(asus->inputdev, SW_TABLET_MODE, !result);
} else if (result != -ENODEV) {
- pr_err("Error checking for keyboard-dock: %d\n", result);
+ dev_err(dev, "Error checking for keyboard-dock: %d\n", result);
}
- }
-
- if (asus->driver->quirks->use_lid_flip_devid) {
+ break;
+ case asus_wmi_lid_flip_devid:
result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
if (result < 0)
- asus->driver->quirks->use_lid_flip_devid = 0;
+ asus->driver->quirks->tablet_switch_mode = asus_wmi_no_tablet_switch;
if (result >= 0) {
input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
} else if (result == -ENODEV) {
- pr_err("This device has lid_flip quirk but got ENODEV checking it. This is a bug.");
+ dev_err(dev, "This device has lid_flip quirk but got ENODEV checking it. This is a bug.");
} else {
- pr_err("Error checking for lid-flip: %d\n", result);
+ dev_err(dev, "Error checking for lid-flip: %d\n", result);
}
+ break;
}

err = input_register_device(asus->inputdev);
@@ -570,8 +576,9 @@ static void asus_wmi_input_exit(struct asus_wmi *asus)

static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus)
{
- int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
+ int result;

+ result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
if (result >= 0) {
input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
input_sync(asus->inputdev);
@@ -3404,20 +3411,26 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
return;
}

- if (asus->driver->quirks->use_kbd_dock_devid && code == NOTIFY_KBD_DOCK_CHANGE) {
- result = asus_wmi_get_devstate_simple(asus,
- ASUS_WMI_DEVID_KBD_DOCK);
+ switch (asus->driver->quirks->tablet_switch_mode) {
+ case asus_wmi_no_tablet_switch:
+ break;
+ case asus_wmi_kbd_dock_devid:
+ if (code == NOTIFY_KBD_DOCK_CHANGE) {
+ result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_KBD_DOCK);
if (result >= 0) {
input_report_switch(asus->inputdev, SW_TABLET_MODE,
- !result);
+ !result);
input_sync(asus->inputdev);
}
return;
- }
-
- if (asus->driver->quirks->use_lid_flip_devid && code == NOTIFY_LID_FLIP) {
- lid_flip_tablet_mode_get_state(asus);
- return;
+ }
+ break;
+ case asus_wmi_lid_flip_devid:
+ if (code == NOTIFY_LID_FLIP) {
+ lid_flip_tablet_mode_get_state(asus);
+ return;
+ }
+ break;
}

if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
@@ -4085,8 +4098,14 @@ static int asus_hotk_resume(struct device *device)
if (asus_wmi_has_fnlock_key(asus))
asus_wmi_fnlock_update(asus);

- if (asus->driver->quirks->use_lid_flip_devid)
+ switch (asus->driver->quirks->tablet_switch_mode) {
+ case asus_wmi_no_tablet_switch:
+ case asus_wmi_kbd_dock_devid:
+ break;
+ case asus_wmi_lid_flip_devid:
lid_flip_tablet_mode_get_state(asus);
+ break;
+ }

return 0;
}
@@ -4127,8 +4146,14 @@ static int asus_hotk_restore(struct device *device)
if (asus_wmi_has_fnlock_key(asus))
asus_wmi_fnlock_update(asus);

- if (asus->driver->quirks->use_lid_flip_devid)
+ switch (asus->driver->quirks->tablet_switch_mode) {
+ case asus_wmi_no_tablet_switch:
+ case asus_wmi_kbd_dock_devid:
+ break;
+ case asus_wmi_lid_flip_devid:
lid_flip_tablet_mode_get_state(asus);
+ break;
+ }

return 0;
}
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index b302415bf1d9..413920bad0c6 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -25,6 +25,12 @@ struct module;
struct key_entry;
struct asus_wmi;

+enum asus_wmi_tablet_switch_mode {
+ asus_wmi_no_tablet_switch,
+ asus_wmi_kbd_dock_devid,
+ asus_wmi_lid_flip_devid,
+};
+
struct quirk_entry {
bool hotplug_wireless;
bool scalar_panel_brightness;
@@ -33,8 +39,7 @@ struct quirk_entry {
bool wmi_backlight_native;
bool wmi_backlight_set_devstate;
bool wmi_force_als_set;
- bool use_kbd_dock_devid;
- bool use_lid_flip_devid;
+ enum asus_wmi_tablet_switch_mode tablet_switch_mode;
int wapf;
/*
* For machines with AMD graphic chips, it will send out WMI event
--
2.37.1


2022-08-09 08:44:14

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] asus-wmi: Adjust tablet/lidflip handling to use enum

On Tue, Aug 9, 2022 at 5:30 AM Luke D. Jones <[email protected]> wrote:
>
> Due to multiple types of tablet/lidflip, the existing code for
> handling these events is refactored to use an enum for each type.

...

> static int asus_wmi_input_init(struct asus_wmi *asus)
> {
> + struct device *dev;
> int err, result;
>
> + dev = &asus->platform_device->dev;
> +

While the discussed pattern of splitting assignments is a good
practice, for some cases we don't do it when we rely on the guarantee
by the callers that some of the stuff won't be problematic. Here the
device is part of the platform device and can't be NULL, there is no
point to split definition and assignment (and you may find plenty
examples in the kernel), so

struct device *dev = &asus->platform_device->dev;

is better to have as it's guaranteed not to be NULL and since that we
don't check it in the code anyway.


...

> input_report_switch(asus->inputdev, SW_TABLET_MODE,
> - !result);
> + !result);

Irrelevant change.

...

It also seems you switched to dev_err() here for the pr_err() that
aren't related to the change, either split that to a separate patch,
or don't do it right now. I.o.w. use dev_err() only in the lines your
change touches, otherwise it's irrelevant.

--
With Best Regards,
Andy Shevchenko