[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e26afe39-5c92-45d0-9a53-057df4df7bec@tuxedocomputers.com>
Date: Tue, 4 Mar 2025 21:11:07 +0100
From: Werner Sembach <wse@...edocomputers.com>
To: Mario Limonciello <mario.limonciello@....com>,
Hans de Goede <hdegoede@...hat.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: dmitry.torokhov@...il.com, linux-kernel@...r.kernel.org,
platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v2 2/2] Input: atkbd - Fix TUXEDO NB02 notebook keyboards
FN-keys
Am 04.03.25 um 19:53 schrieb Mario Limonciello:
> On 3/3/2025 13:04, Werner Sembach wrote:
>> This small driver does 2 things:
>>
>> It remaps the touchpad toggle key from Control + Super +
>> Hangaku/Zenkaku to
>> F21 to conform with established userspace defaults. Note that the
>> Hangaku/Zenkaku scancode used here is usually unused, with real
>> Hangaku/Zenkaku keys using the tilde scancode.
>>
>> It suppresses the reserved scancode produced by pressing the FN-key
>> on its
>> own, which fixes a warning spamming the dmesg log otherwise.
>>
>> Signed-off-by: Werner Sembach <wse@...edocomputers.com>
>> ---
>> MAINTAINERS | 6 ++
>> drivers/platform/x86/Kconfig | 2 +
>> drivers/platform/x86/Makefile | 3 +
>> drivers/platform/x86/tuxedo/Kbuild | 6 ++
>> drivers/platform/x86/tuxedo/Kconfig | 6 ++
>> drivers/platform/x86/tuxedo/nb02/Kbuild | 7 ++
>> drivers/platform/x86/tuxedo/nb02/Kconfig | 15 ++++
>> drivers/platform/x86/tuxedo/nb02/platform.c | 94 +++++++++++++++++++++
>> 8 files changed, 139 insertions(+)
>> create mode 100644 drivers/platform/x86/tuxedo/Kbuild
>> create mode 100644 drivers/platform/x86/tuxedo/Kconfig
>> create mode 100644 drivers/platform/x86/tuxedo/nb02/Kbuild
>> create mode 100644 drivers/platform/x86/tuxedo/nb02/Kconfig
>> create mode 100644 drivers/platform/x86/tuxedo/nb02/platform.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 4ff26fa94895d..d3fbbcef813b0 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -24178,6 +24178,12 @@ T: git
>> git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git turbostat
>> F: tools/power/x86/turbostat/
>> F: tools/testing/selftests/turbostat/
>> +TUXEDO DRIVERS
>> +M: Werner Sembach <wse@...edocomputers.com>
>> +L: platform-driver-x86@...r.kernel.org
>> +S: Supported
>> +F: drivers/platform/x86/tuxedo/
>> +
>> TW5864 VIDEO4LINUX DRIVER
>> M: Bluecherry Maintainers <maintainers@...echerrydvr.com>
>> M: Andrey Utkin <andrey.utkin@...p.bluecherry.net>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 0258dd879d64b..9b78a1255c08e 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -1199,3 +1199,5 @@ config P2SB
>> The main purpose of this library is to unhide P2SB device in
>> case
>> firmware kept it hidden on some platforms in order to access
>> devices
>> behind it.
>> +
>> +source "drivers/platform/x86/tuxedo/Kconfig"
>> diff --git a/drivers/platform/x86/Makefile
>> b/drivers/platform/x86/Makefile
>> index e1b1429470674..1562dcd7ad9a5 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -153,3 +153,6 @@ obj-$(CONFIG_WINMATE_FM07_KEYS) +=
>> winmate-fm07-keys.o
>> # SEL
>> obj-$(CONFIG_SEL3350_PLATFORM) += sel3350-platform.o
>> +
>> +# TUXEDO
>> +obj-y += tuxedo/
>> diff --git a/drivers/platform/x86/tuxedo/Kbuild
>> b/drivers/platform/x86/tuxedo/Kbuild
>> new file mode 100644
>> index 0000000000000..e9c4243d438ba
>> --- /dev/null
>> +++ b/drivers/platform/x86/tuxedo/Kbuild
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +#
>> +# TUXEDO X86 Platform Specific Drivers
>> +#
>> +
>> +obj-y += nb02/
>> diff --git a/drivers/platform/x86/tuxedo/Kconfig
>> b/drivers/platform/x86/tuxedo/Kconfig
>> new file mode 100644
>> index 0000000000000..e463f92135780
>> --- /dev/null
>> +++ b/drivers/platform/x86/tuxedo/Kconfig
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# TUXEDO X86 Platform Specific Drivers
>> +#
>> +
>> +source "drivers/platform/x86/tuxedo/nb02/Kconfig"
>> diff --git a/drivers/platform/x86/tuxedo/nb02/Kbuild
>> b/drivers/platform/x86/tuxedo/nb02/Kbuild
>> new file mode 100644
>> index 0000000000000..8624a012cd683
>
> For a brand new directory isn't this structure a bit heavy handed for
> a single source file?
>
> Or are you envisioning a lot more coming to this structure and just
> want to be ready?
Exactly.
A rewrite and upstream of all of this is (eventually) planned:
https://gitlab.com/tuxedocomputers/development/packages/tuxedo-drivers/-/tree/67b781fbe0498fa6acba5b926d0083e00c287011/src
And already in progress:
https://lore.kernel.org/all/20250121225510.751444-1-wse@tuxedocomputers.com/
and
https://lore.kernel.org/all/20250205162109.222619-1-wse@tuxedocomputers.com/
The folder structure I envision is:
tuxedo/
nb01/
nb02/
nb04/
nb05/
nbxx/
NB03 doesn't exist as a DMI board_vendor string on TUXEDO devices for
historic reasons.
The different firmwares of the different board_vendor devices have very
little in common and so do their drivers.
nbxx is for the TUXI driver, an ACPI interface we plan on eventually
getting on more firmwares (currently only on the nb04 devices).
For nb02 there is also a WMI interface to control fans and NVIDIA cTGP
and stuff that I plan to implement in a separate driver, that would go
in the same folder.
>
> Why not just d/p/x86/tuxedo?
>
> And within there you can have exactly 3 files: nb02.c, Kconfig and
> Kbuild.
>
>> --- /dev/null
>> +++ b/drivers/platform/x86/tuxedo/nb02/Kbuild
>> @@ -0,0 +1,7 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# TUXEDO X86 Platform Specific Drivers
>> +#
>> +
>> +tuxedo_nb02_platform-y := platform.o
>> +obj-$(CONFIG_TUXEDO_NB02_PLATFORM) += tuxedo_nb02_platform.o
>> diff --git a/drivers/platform/x86/tuxedo/nb02/Kconfig
>> b/drivers/platform/x86/tuxedo/nb02/Kconfig
>> new file mode 100644
>> index 0000000000000..bed56276b9b36
>> --- /dev/null
>> +++ b/drivers/platform/x86/tuxedo/nb02/Kconfig
>> @@ -0,0 +1,15 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# TUXEDO X86 Platform Specific Drivers
>> +#
>> +
>> +menuconfig TUXEDO_NB02_PLATFORM
>> + tristate "TUXEDO NB02 Platform Driver"
>> + help
>> + This driver implements miscellaneous things found on TUXEDO
>> Notebooks
>> + with board vendor NB02. For the time being this is only
>> remapping the
>> + touchpad toggle key to something supported by most Linux distros
>> + out-of-the-box and suppressing an unsupported scancode from the
>> + FN-key.
>> +
>> + When compiled as a module it will be called tuxedo_nb02_platform.
>> diff --git a/drivers/platform/x86/tuxedo/nb02/platform.c
>> b/drivers/platform/x86/tuxedo/nb02/platform.c
>> new file mode 100644
>> index 0000000000000..68d83b9b4c2f5
>> --- /dev/null
>> +++ b/drivers/platform/x86/tuxedo/nb02/platform.c
>> @@ -0,0 +1,94 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2025 Werner Sembach wse@...edocomputers.com
>> + */
>> +
>> +#include <linux/dmi.h>
>> +#include <linux/i8042.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/serio.h>
>> +
>> +static u8 tux_nb02_touchp_toggle_seq[] = {
>> + 0xe0, 0x5b, // Super down
>> + 0x1d, // Control down
>> + 0x76, // Zenkaku/Hankaku down
>> + 0xf6, // Zenkaku/Hankaku up
>> + 0x9d, // Control up
>> + 0xe0, 0xdb // Super up
>> +};
>> +
>> +static bool tux_nb02_i8042_filter(unsigned char data,
>> + unsigned char str,
>> + struct serio *port,
>> + __always_unused void *context)
>> +{
>> + static u8 seq_pos;
>> +
>> + if (unlikely(str & I8042_STR_AUXDATA))
>> + return false;
>> +
>> + /* Replace touchpad toggle key sequence with a singular press of
>> the
>> + * F21-key.
>> + */
>> + if (unlikely(data == tux_nb02_touchp_toggle_seq[seq_pos])) {
>> + ++seq_pos;
>> + if (seq_pos == ARRAY_SIZE(tux_nb02_touchp_toggle_seq)) {
>> + seq_pos = 0;
>> + serio_interrupt(port, 0x6c, 0); // F21 down
>> + serio_interrupt(port, 0xec, 0); // F21 up
>> + }
>> + return true;
>> + }
>> +
>> + /* Ignore bogus scancode produced by the FN-key. Reuse seq_pos
>> as first
>> + * byte of that is just the "extended"-byte.
>> + */
>> + if (unlikely(seq_pos == 1 && (data == 0x78 || data == 0xf8))) {
>> + seq_pos = 0;
>> + return true;
>> + }
>> +
>> + /* Replay skipped sequence bytes if it did not finish and it was
>> not a
>> + * FN-key press.
>> + */
>> + if (unlikely(seq_pos)) {
>> + for (u8 i; i < seq_pos; ++i)
>> + serio_interrupt(port, tux_nb02_touchp_toggle_seq[i], 0);
>> + seq_pos = 0;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static const struct dmi_system_id tux_nb02_dmi_string_match[]
>> __initconst = {
>> + {
>> + .matches = {
>> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
>> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "NB02"),
>> + },
>> + },
>> + { }
>> +};
>> +
>> +static int __init tux_nb02_plat_init(void)
>> +{
>> + if (!dmi_check_system(tux_nb02_dmi_string_match))
>> + return -ENODEV;
>> +
>> + return i8042_install_filter(tux_nb02_i8042_filter, NULL);
>> +}
>> +
>> +static void __exit tux_nb02_plat_exit(void)
>> +{
>> + i8042_remove_filter(tux_nb02_i8042_filter);
>> +}
>> +
>> +module_init(tux_nb02_plat_init);
>> +module_exit(tux_nb02_plat_exit);
>> +
>> +MODULE_ALIAS("dmi:*:svnTUXEDO:*:rvnNB02:*");
>> +
>> +MODULE_DESCRIPTION("TUXEDO NB02 Platform");
>> +MODULE_AUTHOR("Werner Sembach <wse@...edocomputers.com>");
>> +MODULE_LICENSE("GPL");
>
Powered by blists - more mailing lists