| Message ID | 20220813092753.6635-1-luke@ljones.dev (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [1/2] asus-wmi: Adjust tablet/lidflip handling to use enum | expand |
Hi, On 8/13/22 11:27, Luke D. Jones wrote: > 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 <luke@ljones.dev> Thank you for the patch, I've merged this but with one change: > --- > 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 0421ffb81927..b4690ac39147 100644 > --- a/drivers/platform/x86/asus-wmi.c > +++ b/drivers/platform/x86/asus-wmi.c > @@ -485,8 +485,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; > @@ -494,35 +497,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); > @@ -548,8 +554,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); > @@ -3044,20 +3051,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) { Given that we need to check for both the tablet_switch_mode as well as different event codes, I believe that keep using something like: if (asus->driver->quirks->tablet_switch_mode == asus_wmi_kbd_dock_devid && code == NOTIFY_KBD_DOCK_CHANGE) { here is better then doing a switch-case with the code checkes nested under > - 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, Note the indent of these 2 lines is wrong now, it should be indentend one extra step because it is inside the if (code == NOTIFY_KBD_DOCK_CHANGE) { block now, sticking with a: if (asus->driver->quirks->tablet_switch_mode == asus_wmi_kbd_dock_devid && code == NOTIFY_KBD_DOCK_CHANGE) { is style instead of nesting avoids the need for this extra indentation. I have fixed this up while merging the patches. Regards, Hans > - !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) { > @@ -3700,8 +3713,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; > } > @@ -3742,8 +3761,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
On Wed, Aug 24 2022 at 16:12:42 +0200, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 8/13/22 11:27, Luke D. Jones wrote: >> 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 <luke@ljones.dev> > > Thank you for the patch, I've merged this but with one change: > >> --- >> 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 0421ffb81927..b4690ac39147 100644 >> --- a/drivers/platform/x86/asus-wmi.c >> +++ b/drivers/platform/x86/asus-wmi.c >> @@ -485,8 +485,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; >> @@ -494,35 +497,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); >> @@ -548,8 +554,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); >> @@ -3044,20 +3051,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) { > > Given that we need to check for both the tablet_switch_mode as well as > different event codes, I believe that keep using something like: > > if (asus->driver->quirks->tablet_switch_mode == > asus_wmi_kbd_dock_devid && > code == NOTIFY_KBD_DOCK_CHANGE) { > > here is better then doing a switch-case with the code checkes nested > under I think I recall that Andy requested use of the switch/case but I might be wrong. Thank you for your help and clean up of this. The patch is making quite a few people happy. > >> - 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, > > Note the indent of these 2 lines is wrong now, it should be indentend > one > extra step because it is inside the if (code == > NOTIFY_KBD_DOCK_CHANGE) { > block now, sticking with a: > > if (asus->driver->quirks->tablet_switch_mode == > asus_wmi_kbd_dock_devid && > code == NOTIFY_KBD_DOCK_CHANGE) { > > is style instead of nesting avoids the need for this extra > indentation. > > I have fixed this up while merging the patches. > > Regards, > > Hans > > > > >> - !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) { >> @@ -3700,8 +3713,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; >> } >> @@ -3742,8 +3761,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 >
Hi, On 8/25/22 22:50, Luke Jones wrote: > > > On Wed, Aug 24 2022 at 16:12:42 +0200, Hans de Goede <hdegoede@redhat.com> wrote: >> Hi, >> >> On 8/13/22 11:27, Luke D. Jones wrote: >>> 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 <luke@ljones.dev> >> >> Thank you for the patch, I've merged this but with one change: >> >>> --- >>> 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 0421ffb81927..b4690ac39147 100644 >>> --- a/drivers/platform/x86/asus-wmi.c >>> +++ b/drivers/platform/x86/asus-wmi.c >>> @@ -485,8 +485,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; >>> @@ -494,35 +497,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); >>> @@ -548,8 +554,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); >>> @@ -3044,20 +3051,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) { >> >> Given that we need to check for both the tablet_switch_mode as well as >> different event codes, I believe that keep using something like: >> >> if (asus->driver->quirks->tablet_switch_mode == asus_wmi_kbd_dock_devid && >> code == NOTIFY_KBD_DOCK_CHANGE) { >> >> here is better then doing a switch-case with the code checkes nested under > > I think I recall that Andy requested use of the switch/case but I might be wrong. Yeah, or maybe it was me even who asked that :) The code is not really ideal in either form. So as you've probably seen from the Cc I've done some further cleanup unifying the handling of all 3 tablet-mode-switch types. Those extra cleanup patches are also in my review-hans branch now. Regards, Hans > >> >>> - 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, >> >> Note the indent of these 2 lines is wrong now, it should be indentend one >> extra step because it is inside the if (code == NOTIFY_KBD_DOCK_CHANGE) { >> block now, sticking with a: >> >> if (asus->driver->quirks->tablet_switch_mode == asus_wmi_kbd_dock_devid && >> code == NOTIFY_KBD_DOCK_CHANGE) { >> >> is style instead of nesting avoids the need for this extra indentation. >> >> I have fixed this up while merging the patches. >> >> Regards, >> >> Hans >> >> >> >> >>> - !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) { >>> @@ -3700,8 +3713,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; >>> } >>> @@ -3742,8 +3761,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 >> > >
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 0421ffb81927..b4690ac39147 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -485,8 +485,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; @@ -494,35 +497,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); @@ -548,8 +554,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); @@ -3044,20 +3051,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) { @@ -3700,8 +3713,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; } @@ -3742,8 +3761,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
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 <luke@ljones.dev> --- 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(-)