Skip to content

Commit 5c08303

Browse files
sangyun0110gregkh
authored andcommitted
HID: appletb-kbd: run inactivity autodim from workqueues
commit 1654e53 upstream. The autodim code in hid-appletb-kbd takes backlight_device->ops_lock via backlight_device_set_brightness() -> mutex_lock() from two different atomic contexts: * appletb_inactivity_timer() is a struct timer_list callback, so it runs in softirq context. Every expiry triggers BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591 Call Trace: <IRQ> __might_resched __mutex_lock backlight_device_set_brightness appletb_inactivity_timer call_timer_fn run_timer_softirq * reset_inactivity_timer() is called from appletb_kbd_hid_event() and appletb_kbd_inp_event(). On real USB hardware these run in softirq/IRQ context (URB completion and input-event dispatch). When the Touch Bar has already been dimmed or turned off, the reset path calls backlight_device_set_brightness() directly to restore brightness, producing the same warning. Both call sites hit the same mutex_lock()-from-atomic bug. Fix them together by moving the blocking work onto the system workqueue: * Convert the inactivity timer from struct timer_list to struct delayed_work; the callback (appletb_inactivity_work) now runs in process context where mutex_lock() is legal. * Add a dedicated struct work_struct restore_brightness_work and have reset_inactivity_timer() schedule it instead of calling backlight_device_set_brightness() directly. Cancel both works synchronously during driver tear-down alongside the existing backlight reference drop. The semantics are unchanged (same delays, same state transitions on dim, turn-off and user activity); only the execution context of the sleeping call changes. The timer field and callback are renamed to match their new type; reset_inactivity_timer() keeps its name because it is invoked from input event paths that read naturally as "reset the inactivity timer". Fixes: 93a0fc4 ("HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar") Cc: stable@vger.kernel.org Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr> Signed-off-by: Jiri Kosina <jkosina@suse.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 59a7993 commit 5c08303

1 file changed

Lines changed: 30 additions & 14 deletions

File tree

drivers/hid/hid-appletb-kbd.c

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
#include <linux/module.h>
1818
#include <linux/string.h>
1919
#include <linux/backlight.h>
20-
#include <linux/timer.h>
20+
#include <linux/workqueue.h>
2121
#include <linux/input/sparse-keymap.h>
2222

2323
#include "hid-ids.h"
@@ -62,7 +62,8 @@ struct appletb_kbd {
6262
struct input_handle kbd_handle;
6363
struct input_handle tpd_handle;
6464
struct backlight_device *backlight_dev;
65-
struct timer_list inactivity_timer;
65+
struct delayed_work inactivity_work;
66+
struct work_struct restore_brightness_work;
6667
bool has_dimmed;
6768
bool has_turned_off;
6869
u8 saved_mode;
@@ -164,33 +165,44 @@ static int appletb_tb_key_to_slot(unsigned int code)
164165
}
165166
}
166167

167-
static void appletb_inactivity_timer(struct timer_list *t)
168+
static void appletb_inactivity_work(struct work_struct *work)
168169
{
169-
struct appletb_kbd *kbd = timer_container_of(kbd, t, inactivity_timer);
170+
struct appletb_kbd *kbd = container_of(to_delayed_work(work),
171+
struct appletb_kbd,
172+
inactivity_work);
170173

171174
if (kbd->backlight_dev && appletb_tb_autodim) {
172175
if (!kbd->has_dimmed) {
173176
backlight_device_set_brightness(kbd->backlight_dev, 1);
174177
kbd->has_dimmed = true;
175-
mod_timer(&kbd->inactivity_timer,
176-
jiffies + secs_to_jiffies(appletb_tb_idle_timeout));
178+
mod_delayed_work(system_wq, &kbd->inactivity_work,
179+
secs_to_jiffies(appletb_tb_idle_timeout));
177180
} else if (!kbd->has_turned_off) {
178181
backlight_device_set_brightness(kbd->backlight_dev, 0);
179182
kbd->has_turned_off = true;
180183
}
181184
}
182185
}
183186

187+
static void appletb_restore_brightness_work(struct work_struct *work)
188+
{
189+
struct appletb_kbd *kbd = container_of(work, struct appletb_kbd,
190+
restore_brightness_work);
191+
192+
if (kbd->backlight_dev)
193+
backlight_device_set_brightness(kbd->backlight_dev, 2);
194+
}
195+
184196
static void reset_inactivity_timer(struct appletb_kbd *kbd)
185197
{
186198
if (kbd->backlight_dev && appletb_tb_autodim) {
187199
if (kbd->has_dimmed || kbd->has_turned_off) {
188-
backlight_device_set_brightness(kbd->backlight_dev, 2);
189200
kbd->has_dimmed = false;
190201
kbd->has_turned_off = false;
202+
schedule_work(&kbd->restore_brightness_work);
191203
}
192-
mod_timer(&kbd->inactivity_timer,
193-
jiffies + secs_to_jiffies(appletb_tb_dim_timeout));
204+
mod_delayed_work(system_wq, &kbd->inactivity_work,
205+
secs_to_jiffies(appletb_tb_dim_timeout));
194206
}
195207
}
196208

@@ -408,9 +420,11 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id
408420
dev_err_probe(dev, -ENODEV, "Failed to get backlight device\n");
409421
} else {
410422
backlight_device_set_brightness(kbd->backlight_dev, 2);
411-
timer_setup(&kbd->inactivity_timer, appletb_inactivity_timer, 0);
412-
mod_timer(&kbd->inactivity_timer,
413-
jiffies + secs_to_jiffies(appletb_tb_dim_timeout));
423+
INIT_DELAYED_WORK(&kbd->inactivity_work, appletb_inactivity_work);
424+
INIT_WORK(&kbd->restore_brightness_work,
425+
appletb_restore_brightness_work);
426+
mod_delayed_work(system_wq, &kbd->inactivity_work,
427+
secs_to_jiffies(appletb_tb_dim_timeout));
414428
}
415429

416430
kbd->inp_handler.event = appletb_kbd_inp_event;
@@ -444,7 +458,8 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id
444458
stop_hw:
445459
hid_hw_stop(hdev);
446460
if (kbd->backlight_dev) {
447-
timer_delete_sync(&kbd->inactivity_timer);
461+
cancel_delayed_work_sync(&kbd->inactivity_work);
462+
cancel_work_sync(&kbd->restore_brightness_work);
448463
put_device(&kbd->backlight_dev->dev);
449464
}
450465
return ret;
@@ -461,7 +476,8 @@ static void appletb_kbd_remove(struct hid_device *hdev)
461476
hid_hw_stop(hdev);
462477

463478
if (kbd->backlight_dev) {
464-
timer_delete_sync(&kbd->inactivity_timer);
479+
cancel_delayed_work_sync(&kbd->inactivity_work);
480+
cancel_work_sync(&kbd->restore_brightness_work);
465481
put_device(&kbd->backlight_dev->dev);
466482
}
467483
}

0 commit comments

Comments
 (0)