From: Armin Wolf <W_Armin@gmx.de>
To: mustafa <mustafa.eskieksi@gmail.com>,
hdegoede@redhat.com, ilpo.jarvinen@linux.intel.com,
jdelvare@suse.com, linux@roeck-us.net, pavel@ucw.cz,
lee@kernel.org, linux-kernel@vger.kernel.org,
platform-driver-x86@vger.kernel.org, linux-hwmon@vger.kernel.org,
linux-leds@vger.kernel.org
Subject: Re: [PATCH v5 0/1] platform/x86: Add wmi driver for Casper Excalibur laptops
Date: Mon, 25 Mar 2024 21:10:25 +0100 [thread overview]
Message-ID: <19661160-76d4-43ed-987f-dbf183e891ba@gmx.de> (raw)
In-Reply-To: <20240324181201.87882-1-mustafa.eskieksi@gmail.com>
Am 24.03.24 um 19:12 schrieb mustafa:
> From: Mustafa Ekşi <mustafa.eskieksi@gmail.com>
>
> 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
>
next prev parent reply other threads:[~2024-03-25 20:10 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-24 18:12 [PATCH v5 0/1] platform/x86: Add wmi driver for Casper Excalibur laptops mustafa
2024-03-24 18:12 ` [PATCH v5 1/1] " mustafa
2024-04-08 10:01 ` Hans de Goede
2024-03-25 20:10 ` Armin Wolf [this message]
2024-04-07 0:57 ` [PATCH v5 0/1] " Stella Bloom
2024-04-08 15:23 ` Mustafa Ekşi
2024-04-08 16:11 ` Stella Bloom
2024-04-08 16:13 ` Stella Bloom
2024-04-08 18:56 ` Armin Wolf
2024-04-08 19:32 ` Stella Bloom
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=19661160-76d4-43ed-987f-dbf183e891ba@gmx.de \
--to=w_armin@gmx.de \
--cc=hdegoede@redhat.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jdelvare@suse.com \
--cc=lee@kernel.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=mustafa.eskieksi@gmail.com \
--cc=pavel@ucw.cz \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).