Hi, On 9/20/21 8:43 PM, Barnabás Pőcze wrote: > Hi > > > 2021. szeptember 20., hétfő 18:03 keltezéssel, José Expósito írta: >> Some devices, even non convertible ones, can send incorrect >> SW_TABLET_MODE reports. >> >> Add an allow list and accept such reports only from devices in it. >> >> Bug reported for Dell XPS 17 9710 on: >> https://gitlab.freedesktop.org/libinput/libinput/-/issues/662 >> >> Reported-by: Tobias Gurtzick <magic@xxxxxxxxxxxxxxx> >> Suggested-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> Tested-by: Tobias Gurtzick <magic@xxxxxxxxxxxxxxx> >> Signed-off-by: José Expósito <jose.exposito89@xxxxxxxxx> >> --- >> drivers/platform/x86/intel/hid.c | 33 +++++++++++++++++++++++++++----- >> 1 file changed, 28 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/platform/x86/intel/hid.c b/drivers/platform/x86/intel/hid.c >> index a33a5826e81a..24d26336e39a 100644 >> --- a/drivers/platform/x86/intel/hid.c >> +++ b/drivers/platform/x86/intel/hid.c >> @@ -118,6 +118,24 @@ static const struct dmi_system_id dmi_vgbs_allow_list[] = { >> { } >> }; >> >> +/* >> + * Some devices, even non convertible ones, can send incorrect SW_TABLET_MODE >> + * reports. Accept such reports only from devices in this list. >> + */ >> +static const struct dmi_system_id dmi_switches_auto_add_allow_list[] = { >> + { >> + .matches = { >> + DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "31" /* Convertible */), >> + }, >> + }, >> + { >> + .matches = { >> + DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "32" /* Detachable */), >> + }, >> + }, >> + {} /* Array terminator */ >> +}; >> + >> struct intel_hid_priv { >> struct input_dev *input_dev; >> struct input_dev *array; >> @@ -455,11 +473,16 @@ static void notify_handler(acpi_handle handle, u32 event, void *context) >> * >> * See dual_accel_detect.h for more info on the dual_accel check. >> */ >> - if (!priv->switches && !priv->dual_accel && (event == 0xcc || event == 0xcd)) { >> - dev_info(&device->dev, "switch event received, enable switches supports\n"); >> - err = intel_hid_switches_setup(device); >> - if (err) >> - pr_err("Failed to setup Intel HID switches\n"); >> + if (event == 0xcc || event == 0xcd) { >> + if (!dmi_check_system(dmi_switches_auto_add_allow_list)) >> + return; > > I think you should not check it every time. Maybe add a `bool` member > to `struct intel_hid_priv`. Or maybe better: rename `dual_accel` to something like > `autodetect_switch` and initialize it with `!dual_accel_detect() && dmi_check_system(...)`. These events don't happen all that often. But this still is a good suggestion. Since this is a regression fix of sorts I've gone ahead and made the suggested changes myself (keeping José as author) and I've applied this to my tree, see the version in the review-hans branch to see what the merged version looks like. Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans I will also add this to the pdx86 fixes branch and include it in my next pdx86-fixes for 5.15 pull-req to Linus. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > > >> + >> + if (!priv->switches && !priv->dual_accel) { >> + dev_info(&device->dev, "switch event received, enable switches supports\n"); >> + err = intel_hid_switches_setup(device); >> + if (err) >> + pr_err("Failed to setup Intel HID switches\n"); >> + } >> } >> >> if (priv->wakeup_mode) { >> -- >> 2.25.1 >> >> > > > Regards, > Barnabás Pőcze >
