[PATCH v4] HID: appletb-kbd: support layer switching on Fn double press

[PATCH v4] HID: appletb-kbd: support layer switching on Fn double press

[Aditya Garg]

Holding Fn temporarily switches the Touch Bar between media controls and
function keys. Users who want to keep the alternate layer currently need
to change the mode through sysfs.

An optional double_press_switch_time module parameter makes a double press
persistently switch the default layer. Its value specifies the double-press
interval in milliseconds, while zero leaves the behavior disabled.

Signed-off-by: Aditya Garg <aditya.garg@linux.dev>
Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
---
Changes in v2:
- Added a check to ensure negative fn switch times are ignored.

Changes in v3:
- Renamed the backing variable to match the module parameter.
- Fixed the reported code style issues.

Changes in v4:
- Added missing "break;" in case statements.
- Change author's email to aditya.garg@linux.dev

 drivers/hid/hid-appletb-kbd.c | 56 ++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
index 5cc27066f..f10d093c2 100644
--- a/drivers/hid/hid-appletb-kbd.c
+++ b/drivers/hid/hid-appletb-kbd.c
@@ -56,6 +56,12 @@ static int appletb_tb_idle_timeout = 15;
 module_param_named(idle_timeout, appletb_tb_idle_timeout, int, 0644);
 MODULE_PARM_DESC(idle_timeout, "Idle timeout in sec");
 
+static int appletb_tb_double_press_switch_time;
+module_param_named(double_press_switch_time,
+		   appletb_tb_double_press_switch_time, int, 0644);
+MODULE_PARM_DESC(double_press_switch_time,
+		 "Fn double-press interval in ms (0 disables layer switching)");
+
 struct appletb_kbd {
 	struct hid_field *mode_field;
 	struct input_handler inp_handler;
@@ -68,6 +74,7 @@ struct appletb_kbd {
 	bool has_turned_off;
 	u8 saved_mode;
 	u8 current_mode;
+	unsigned long last_fn_press;
 };
 
 static const struct key_entry appletb_kbd_keymap[] = {
@@ -243,6 +250,20 @@ static int appletb_kbd_hid_event(struct hid_device *hdev, struct hid_field *fiel
 	return kbd->current_mode == APPLETB_KBD_MODE_OFF;
 }
 
+static u8 appletb_switch_mode(u8 mode)
+{
+	switch (mode) {
+	case APPLETB_KBD_MODE_SPCL:
+		return APPLETB_KBD_MODE_FN;
+		break;
+	case APPLETB_KBD_MODE_FN:
+		return APPLETB_KBD_MODE_SPCL;
+		break;
+	default:
+		return mode;
+	}
+}
+
 static void appletb_kbd_inp_event(struct input_handle *handle, unsigned int type,
 			      unsigned int code, int value)
 {
@@ -250,15 +271,36 @@ static void appletb_kbd_inp_event(struct input_handle *handle, unsigned int type
 
 	reset_inactivity_timer(kbd);
 
-	if (type == EV_KEY && code == KEY_FN && appletb_tb_fn_toggle &&
-		(kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
-		 kbd->current_mode == APPLETB_KBD_MODE_FN)) {
+	if (type == EV_KEY && code == KEY_FN &&
+	    (kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
+	     kbd->current_mode == APPLETB_KBD_MODE_FN)) {
 		if (value == 1) {
-			kbd->saved_mode = kbd->current_mode;
-			appletb_kbd_set_mode(kbd, kbd->current_mode == APPLETB_KBD_MODE_SPCL
-						? APPLETB_KBD_MODE_FN : APPLETB_KBD_MODE_SPCL);
+			if (appletb_tb_double_press_switch_time > 0) {
+				unsigned long now = jiffies;
+
+				if (time_before(now, kbd->last_fn_press +
+					msecs_to_jiffies(appletb_tb_double_press_switch_time))) {
+					appletb_tb_def_mode =
+						appletb_switch_mode(appletb_tb_def_mode);
+
+					appletb_kbd_set_mode(kbd, appletb_tb_def_mode);
+
+					kbd->saved_mode = appletb_tb_def_mode;
+					kbd->last_fn_press = 0;
+
+					return;
+				}
+
+				kbd->last_fn_press = now;
+			}
+			if (appletb_tb_fn_toggle) {
+				kbd->saved_mode = kbd->current_mode;
+
+				appletb_kbd_set_mode(kbd, appletb_switch_mode(kbd->current_mode));
+			}
 		} else if (value == 0) {
-			if (kbd->saved_mode != kbd->current_mode)
+			if (appletb_tb_fn_toggle &&
+			    kbd->saved_mode != kbd->current_mode)
 				appletb_kbd_set_mode(kbd, kbd->saved_mode);
 		}
 	}
-- 
2.53.0

Re: [PATCH v4] HID: appletb-kbd: support layer switching on Fn double press

[Aditya Garg]

On 9/5/26 8:07 PM, Aditya Garg wrote:
> Holding Fn temporarily switches the Touch Bar between media controls and
> function keys. Users who want to keep the alternate layer currently need
> to change the mode through sysfs.
> 
> An optional double_press_switch_time module parameter makes a double press
> persistently switch the default layer. Its value specifies the double-press
> interval in milliseconds, while zero leaves the behavior disabled.
> 
> Signed-off-by: Aditya Garg <aditya.garg@linux.dev>
> Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
> ---
> Changes in v2:
> - Added a check to ensure negative fn switch times are ignored.
> 
> Changes in v3:
> - Renamed the backing variable to match the module parameter.
> - Fixed the reported code style issues.
> 
> Changes in v4:
> - Added missing "break;" in case statements.
> - Change author's email to aditya.garg@linux.dev

Now that I see it, the case statements were ending in return. break was not needed. Sorry for the brainfart.

Re: [PATCH v4] HID: appletb-kbd: support layer switching on Fn double press

[Jiri Kosina]

On Sat, 5 Sep 2026, Aditya Garg wrote:

> > Holding Fn temporarily switches the Touch Bar between media controls and
> > function keys. Users who want to keep the alternate layer currently need
> > to change the mode through sysfs.
> > 
> > An optional double_press_switch_time module parameter makes a double press
> > persistently switch the default layer. Its value specifies the double-press
> > interval in milliseconds, while zero leaves the behavior disabled.
> > 
> > Signed-off-by: Aditya Garg <aditya.garg@linux.dev>
> > Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
> > ---
> > Changes in v2:
> > - Added a check to ensure negative fn switch times are ignored.
> > 
> > Changes in v3:
> > - Renamed the backing variable to match the module parameter.
> > - Fixed the reported code style issues.
> > 
> > Changes in v4:
> > - Added missing "break;" in case statements.
> > - Change author's email to aditya.garg@linux.dev
> 
> Now that I see it, the case statements were ending in return. break was 
> not needed. Sorry for the brainfart.

Please send v5 and I will apply it.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v4] HID: appletb-kbd: support layer switching on Fn double press

[Aditya Garg]

On 11/09/26 10:15 pm, Jiri Kosina wrote:
> On Sat, 5 Sep 2026, Aditya Garg wrote:
> 
>>> Holding Fn temporarily switches the Touch Bar between media controls and
>>> function keys. Users who want to keep the alternate layer currently need
>>> to change the mode through sysfs.
>>>
>>> An optional double_press_switch_time module parameter makes a double press
>>> persistently switch the default layer. Its value specifies the double-press
>>> interval in milliseconds, while zero leaves the behavior disabled.
>>>
>>> Signed-off-by: Aditya Garg <aditya.garg@linux.dev>
>>> Signed-off-by: Andre Eikmeyer <dev@deq.rocks>
>>> ---
>>> Changes in v2:
>>> - Added a check to ensure negative fn switch times are ignored.
>>>
>>> Changes in v3:
>>> - Renamed the backing variable to match the module parameter.
>>> - Fixed the reported code style issues.
>>>
>>> Changes in v4:
>>> - Added missing "break;" in case statements.
>>> - Change author's email to aditya.garg@linux.dev
>>
>> Now that I see it, the case statements were ending in return. break was 
>> not needed. Sorry for the brainfart.
> 
> Please send v5 and I will apply it.

Sent here:

https://lore.kernel.org/linux-input/20260911200019.17028-1-aditya.garg@linux.dev/T/#u

Re: [PATCH v4] HID: appletb-kbd: support layer switching on Fn double press

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Logic Error / Jiffies Wrap. Zero-initialized timer incorrectly triggers layer switch on every key press during early boot.
- [Low] Dead Code. The switch block contains unreachable break statements.

Pre-existing issues:
- [High] Sleeping in Atomic Context. The input event handler calls a sleepable HID power management function, causing a kernel panic.
--

commit 2e557c5f2297282525c11706211299ba134eef07
Author: Aditya Garg <aditya.garg@linux.dev>

HID: appletb-kbd: support layer switching on Fn double press

This commit adds an optional module parameter to allow a double press of the Fn
key to persistently switch the Touch Bar's default layer, avoiding the need
to change the mode through sysfs.

> diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
> index 5cc27066f6026..f10d093c2b3e3 100644
> --- a/drivers/hid/hid-appletb-kbd.c
> +++ b/drivers/hid/hid-appletb-kbd.c
[ ... ]
> @@ -243,6 +250,20 @@ static int appletb_kbd_hid_event(struct hid_device *hdev, struct hid_field *fiel
>  	return kbd->current_mode == APPLETB_KBD_MODE_OFF;
>  }
>  
> +static u8 appletb_switch_mode(u8 mode)
> +{
> +	switch (mode) {
> +	case APPLETB_KBD_MODE_SPCL:
> +		return APPLETB_KBD_MODE_FN;
> +		break;
[Severity: Low]
Are these break statements unreachable since the cases return directly?

> +	case APPLETB_KBD_MODE_FN:
> +		return APPLETB_KBD_MODE_SPCL;
> +		break;
> +	default:
> +		return mode;
> +	}
> +}
> +
>  static void appletb_kbd_inp_event(struct input_handle *handle, unsigned int type,
>  			      unsigned int code, int value)
>  {
[ ... ]
> +			if (appletb_tb_double_press_switch_time > 0) {
> +				unsigned long now = jiffies;
> +
> +				if (time_before(now, kbd->last_fn_press +
> +					msecs_to_jiffies(appletb_tb_double_press_switch_time))) {
[Severity: Medium]
Will this condition incorrectly evaluate to true on every key press during
early boot on 32-bit systems?

Since jiffies initializes to a large negative value (-300 seconds) on 32-bit
architectures, now will be negative. Because kbd->last_fn_press is initially
zero, time_before(negative, positive) will evaluate to true.

> +					appletb_tb_def_mode =
> +						appletb_switch_mode(appletb_tb_def_mode);
> +
> +					appletb_kbd_set_mode(kbd, appletb_tb_def_mode);
[Severity: High]
This isn't a bug introduced by this patch, but could calling
appletb_kbd_set_mode() from appletb_kbd_inp_event() result in sleeping
in atomic context?

The input subsystem invokes the ->event callback (which maps to
appletb_kbd_inp_event) with the per-device spinlock held and local interrupts
disabled.

appletb_kbd_set_mode() calls hid_hw_power(hdev, PM_HINT_FULLON), which
resolves to usbhid_power() and uses pm_runtime_get_sync() (a sleeping
function).

> +
> +					kbd->saved_mode = appletb_tb_def_mode;
> +					kbd->last_fn_press = 0;
[Severity: Medium]
Might resetting last_fn_press to 0 guarantee the above time_before() bug will
indefinitely trigger on every press during the 5 minute boot window before
jiffies crosses zero?

> +
> +					return;
> +				}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905143733.17819-1-aditya.garg@linux.dev?part=1