* Re: [PATCH] platform/x86/intel/hid: Add module-params for 5 button array + SW_TABLET_MODE reporting
[not found] ` <Y3djFvrET/4meoq/@smile.fi.intel.com>
@ 2022-11-20 22:47 ` Hans de Goede
0 siblings, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2022-11-20 22:47 UTC (permalink / raw
To: Andy Shevchenko; +Cc: Mark Gross, platform-driver-x86@vger.kernel.org
Hi,
On 11/18/22 11:48, Andy Shevchenko wrote:
> On Fri, Nov 18, 2022 at 10:25:50AM +0100, Hans de Goede wrote:
>> The driver has DMI-quirk tables for force-enabling 5 button array support
>> and for 2 different ways of enabling SW_TABLET_MODE reporting.
>>
>> Add module parameters to allow user to enable the driver behavior currently
>> only available through DMI quirks.
>>
>> This is useful for users to test this in bug-reports and for users to use
>> as a workaround while new DMI quirks find their way upstream.
>
> Lately you have been adding tons of module parameters here and there.
I'm not sure I would call it "tons of" I've added a few parameters to
allow users to test behavior which before then was only available through
DMI quirk tables.
This is useful for users to easily check if their model needs to be added
to a DMI quirk table.
> Taking into account that we discourage to do that, but at the same time
> understanding your point, wouldn't be better before doing that, provide
> a new type of the module parameters "for debug purposes only". One way
> is to provide necessary macros
>
> MODULE_PARAM_DEBUG()
>
> And always have the parameters suffixed with _debug. OR introduce an additional
> option that user may put before "open" debugging module / kernel command line
> parameters?
>
> In this case we delimit the old/existing parameters with the parameters for
> debugging. Moreover, it may be excluded by introducing Kconfig option at compile
> time.
Adding special infrastructure for just these 5-6 module parameters which I
have added recently seems a bit too much to me.
Regards,
Hans
p.s.
I notice that I have forgotten to Cc the list when submitting that
patch, I've added the list to the Cc now and I'll resend the patch
with the list in the Cc.
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] platform/x86/intel/hid: Add module-params for 5 button array + SW_TABLET_MODE reporting
@ 2022-11-20 22:48 Hans de Goede
2022-11-21 10:19 ` Hans de Goede
0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2022-11-20 22:48 UTC (permalink / raw
To: Mark Gross, Andy Shevchenko; +Cc: Hans de Goede, platform-driver-x86
The driver has DMI-quirk tables for force-enabling 5 button array support
and for 2 different ways of enabling SW_TABLET_MODE reporting.
Add module parameters to allow user to enable the driver behavior currently
only available through DMI quirks.
This is useful for users to test this in bug-reports and for users to use
as a workaround while new DMI quirks find their way upstream.
Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/822
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/intel/hid.c | 36 +++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/intel/hid.c b/drivers/platform/x86/intel/hid.c
index b6313ecd190c..b6c06b37862e 100644
--- a/drivers/platform/x86/intel/hid.c
+++ b/drivers/platform/x86/intel/hid.c
@@ -16,6 +16,25 @@
#include <linux/suspend.h>
#include "../dual_accel_detect.h"
+enum intel_hid_tablet_sw_mode {
+ TABLET_SW_AUTO = -1,
+ TABLET_SW_OFF = 0,
+ TABLET_SW_AT_EVENT,
+ TABLET_SW_AT_PROBE,
+};
+
+static bool enable_5_button_array;
+module_param(enable_5_button_array, bool, 0444);
+MODULE_PARM_DESC(enable_5_button_array,
+ "Enable 5 Button Array support. "
+ "If you need this please report this to: platform-driver-x86@vger.kernel.org");
+
+static int enable_sw_tablet_mode = TABLET_SW_AUTO;
+module_param(enable_sw_tablet_mode, int, 0444);
+MODULE_PARM_DESC(enable_sw_tablet_mode,
+ "Enable SW_TABLET_MODE reporting -1:auto 0:off 1:at-first-event 2:at-probe. "
+ "If you need this please report this to: platform-driver-x86@vger.kernel.org");
+
/* When NOT in tablet mode, VGBS returns with the flag 0x40 */
#define TABLET_MODE_FLAG BIT(6)
@@ -157,7 +176,6 @@ struct intel_hid_priv {
struct input_dev *array;
struct input_dev *switches;
bool wakeup_mode;
- bool auto_add_switch;
};
#define HID_EVENT_FILTER_UUID "eeec56b3-4442-408f-a792-4edd4d758054"
@@ -487,7 +505,8 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
* SW_TABLET_MODE report, in these cases we enable support when receiving
* the first event instead of during driver setup.
*/
- if (!priv->switches && priv->auto_add_switch && (event == 0xcc || event == 0xcd)) {
+ if (!priv->switches && enable_sw_tablet_mode == TABLET_SW_AT_EVENT &&
+ (event == 0xcc || event == 0xcd)) {
dev_info(&device->dev, "switch event received, enable switches supports\n");
err = intel_hid_switches_setup(device);
if (err)
@@ -592,7 +611,7 @@ static bool button_array_present(struct platform_device *device)
return true;
}
- if (dmi_check_system(button_array_table))
+ if (enable_5_button_array || dmi_check_system(button_array_table))
return true;
return false;
@@ -629,7 +648,14 @@ static int intel_hid_probe(struct platform_device *device)
dev_set_drvdata(&device->dev, priv);
/* See dual_accel_detect.h for more info on the dual_accel check. */
- priv->auto_add_switch = dmi_check_system(dmi_auto_add_switch) && !dual_accel_detect();
+ if (enable_sw_tablet_mode == TABLET_SW_AUTO) {
+ if (dmi_check_system(dmi_vgbs_allow_list))
+ enable_sw_tablet_mode = TABLET_SW_AT_PROBE;
+ else if (dmi_check_system(dmi_auto_add_switch) && !dual_accel_detect())
+ enable_sw_tablet_mode = TABLET_SW_AT_EVENT;
+ else
+ enable_sw_tablet_mode = TABLET_SW_OFF;
+ }
err = intel_hid_input_setup(device);
if (err) {
@@ -646,7 +672,7 @@ static int intel_hid_probe(struct platform_device *device)
}
/* Setup switches for devices that we know VGBS return correctly */
- if (dmi_check_system(dmi_vgbs_allow_list)) {
+ if (enable_sw_tablet_mode == TABLET_SW_AT_PROBE) {
dev_info(&device->dev, "platform supports switches\n");
err = intel_hid_switches_setup(device);
if (err)
--
2.38.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] platform/x86/intel/hid: Add module-params for 5 button array + SW_TABLET_MODE reporting
2022-11-20 22:48 Hans de Goede
@ 2022-11-21 10:19 ` Hans de Goede
0 siblings, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2022-11-21 10:19 UTC (permalink / raw
To: Mark Gross, Andy Shevchenko; +Cc: platform-driver-x86
Hi all,
On 11/20/22 23:48, Hans de Goede wrote:
> The driver has DMI-quirk tables for force-enabling 5 button array support
> and for 2 different ways of enabling SW_TABLET_MODE reporting.
>
> Add module parameters to allow user to enable the driver behavior currently
> only available through DMI quirks.
>
> This is useful for users to test this in bug-reports and for users to use
> as a workaround while new DMI quirks find their way upstream.
>
> Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/822
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
I have added this to my review-hans branch now.
Regards,
Hans
> ---
> drivers/platform/x86/intel/hid.c | 36 +++++++++++++++++++++++++++-----
> 1 file changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/hid.c b/drivers/platform/x86/intel/hid.c
> index b6313ecd190c..b6c06b37862e 100644
> --- a/drivers/platform/x86/intel/hid.c
> +++ b/drivers/platform/x86/intel/hid.c
> @@ -16,6 +16,25 @@
> #include <linux/suspend.h>
> #include "../dual_accel_detect.h"
>
> +enum intel_hid_tablet_sw_mode {
> + TABLET_SW_AUTO = -1,
> + TABLET_SW_OFF = 0,
> + TABLET_SW_AT_EVENT,
> + TABLET_SW_AT_PROBE,
> +};
> +
> +static bool enable_5_button_array;
> +module_param(enable_5_button_array, bool, 0444);
> +MODULE_PARM_DESC(enable_5_button_array,
> + "Enable 5 Button Array support. "
> + "If you need this please report this to: platform-driver-x86@vger.kernel.org");
> +
> +static int enable_sw_tablet_mode = TABLET_SW_AUTO;
> +module_param(enable_sw_tablet_mode, int, 0444);
> +MODULE_PARM_DESC(enable_sw_tablet_mode,
> + "Enable SW_TABLET_MODE reporting -1:auto 0:off 1:at-first-event 2:at-probe. "
> + "If you need this please report this to: platform-driver-x86@vger.kernel.org");
> +
> /* When NOT in tablet mode, VGBS returns with the flag 0x40 */
> #define TABLET_MODE_FLAG BIT(6)
>
> @@ -157,7 +176,6 @@ struct intel_hid_priv {
> struct input_dev *array;
> struct input_dev *switches;
> bool wakeup_mode;
> - bool auto_add_switch;
> };
>
> #define HID_EVENT_FILTER_UUID "eeec56b3-4442-408f-a792-4edd4d758054"
> @@ -487,7 +505,8 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
> * SW_TABLET_MODE report, in these cases we enable support when receiving
> * the first event instead of during driver setup.
> */
> - if (!priv->switches && priv->auto_add_switch && (event == 0xcc || event == 0xcd)) {
> + if (!priv->switches && enable_sw_tablet_mode == TABLET_SW_AT_EVENT &&
> + (event == 0xcc || event == 0xcd)) {
> dev_info(&device->dev, "switch event received, enable switches supports\n");
> err = intel_hid_switches_setup(device);
> if (err)
> @@ -592,7 +611,7 @@ static bool button_array_present(struct platform_device *device)
> return true;
> }
>
> - if (dmi_check_system(button_array_table))
> + if (enable_5_button_array || dmi_check_system(button_array_table))
> return true;
>
> return false;
> @@ -629,7 +648,14 @@ static int intel_hid_probe(struct platform_device *device)
> dev_set_drvdata(&device->dev, priv);
>
> /* See dual_accel_detect.h for more info on the dual_accel check. */
> - priv->auto_add_switch = dmi_check_system(dmi_auto_add_switch) && !dual_accel_detect();
> + if (enable_sw_tablet_mode == TABLET_SW_AUTO) {
> + if (dmi_check_system(dmi_vgbs_allow_list))
> + enable_sw_tablet_mode = TABLET_SW_AT_PROBE;
> + else if (dmi_check_system(dmi_auto_add_switch) && !dual_accel_detect())
> + enable_sw_tablet_mode = TABLET_SW_AT_EVENT;
> + else
> + enable_sw_tablet_mode = TABLET_SW_OFF;
> + }
>
> err = intel_hid_input_setup(device);
> if (err) {
> @@ -646,7 +672,7 @@ static int intel_hid_probe(struct platform_device *device)
> }
>
> /* Setup switches for devices that we know VGBS return correctly */
> - if (dmi_check_system(dmi_vgbs_allow_list)) {
> + if (enable_sw_tablet_mode == TABLET_SW_AT_PROBE) {
> dev_info(&device->dev, "platform supports switches\n");
> err = intel_hid_switches_setup(device);
> if (err)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-21 10:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20221118092550.48389-1-hdegoede@redhat.com>
[not found] ` <Y3djFvrET/4meoq/@smile.fi.intel.com>
2022-11-20 22:47 ` [PATCH] platform/x86/intel/hid: Add module-params for 5 button array + SW_TABLET_MODE reporting Hans de Goede
2022-11-20 22:48 Hans de Goede
2022-11-21 10:19 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.