Re: [PATCH v5 0/1] platform/x86: Add wmi driver for Casper Excalibur laptops

From: Armin Wolf
Date: Mon Mar 25 2024 - 16:11:47 EST


Am 24.03.24 um 19:12 schrieb mustafa:

From: Mustafa Ekşi <mustafa.eskieksi@xxxxxxxxx>

Hi,
I want to note that moving mutex_init to the bottom of the function
crashes the driver when mutex_lock is called. I didn't investigate it
further but I wanted to say that since Ai Chao also did it like that.

You mean like the lenovo-wmi-camera driver? In this case, the driver was
only using the mutex inside the WMI notify callback, which can only run
once the driver has finished probing.

In your case however, the mutex can already be used while the driver is still
probing, because it registers callbacks with other subsystems.
The subsystem (maybe led?) assumes that the device is already fully operational
and will begin calling the callbacks immediately, causing a crash.

Looking at the code, it seems that you are not calling mutex_destroy() in
casper_wmi_remove(). I suggest to use devm_add_action_or_reset() for handling this.

Thanks,
Armin Wolf

Driver sets all leds to white on start. Before that, when a led's
brightness is changed, that led's color gets set to white but others
keep their old colors which creates a bad user experience (at least for
me). Please inform me if this is a bad approach.
Also, this driver still lacks support for changing modes and I seek
advise for that.

Mustafa Ekşi (1):
platform/x86: Add wmi driver for Casper Excalibur laptops

MAINTAINERS | 6 +
drivers/platform/x86/Kconfig | 14 +
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/casper-wmi.c | 641 ++++++++++++++++++++++++++++++
4 files changed, 662 insertions(+)
create mode 100644 drivers/platform/x86/casper-wmi.c