Re: [PATCH v4] HID: appletb-kbd: support layer switching on Fn double press
From: sashiko-bot@kernel.org
Date: 2026-09-05 14:51:10
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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