Received-SPF: pass (google.com: domain of linux-kernel+bounces-117891-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <19661160-76d4-43ed-987f-dbf183e891ba@gmx.de>
Date: Mon, 25 Mar 2024 21:10:25 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 0/1] platform/x86: Add wmi driver for Casper Excalibur
 laptops
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
References: <20240324181201.87882-1-mustafa.eskieksi@gmail.com>
Content-Language: en-US
From: Armin Wolf <W_Armin@gmx.de>
In-Reply-To: <20240324181201.87882-1-mustafa.eskieksi@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:4Q7DKlFfmFE=;1gutw6nxLW14lkJsyKYuM4sLje8
 z/GVi1B240WDw392PqTI8PmIGLBIaP6em5iITJ+UL7Q+rIYiJevgcuBJ2cu0uFjwUrcoYGyXs
 s/zKD4WK7gRXFWfWmvd3t4nGLT8p3bijTGAk3STO5TDxGfjU8vK9x/sEjcVmA5M2AwrAIgVzR
 BNxJmpqbWStWlkBuTuJxjrSZ3PT6Izl42brBi4/z6kNBLrFXzu3E3oXLQc8MIvHFSYWb+spCC
 XZb+yXWnZc2XSTnuRsFdqA/5g8x2vDLMxyBkqrfZ7yn4IZQoL46n/dsdVgEikTB068zmGl0/o
 KQ3PrJ4jb+VvOd4Ohs5fzkMJlA1QpN7T5oLQgj0McqNMMhdfz1GnMWNm63jHiQNkKL5UOMP6h
 WFacbdwoLlJNFcKcFHOHUtp8TutqM/YRAD5v+Nc07bIjelQBV+SMokkst+l7VSqoL7/f36szu
 kvBeL2vw286XGsMWOwv0oMrnyQggLdHdVhse9NaK5PFiHCNMLAUg6ba5XcDs1TR3MbiyO+pXX
 gq3YGulqT4AnsvN9O93y/HZIQshKjEeclZJ8WzCSImYHXBXY6b8DhRWI2hqTlbvskXRjQYbnP
 Nj0J0HFCMyz7xdRwYu+ht/lRbn9WEBr46eWI94dtDidmFu7/YUUQqt7YncO7ZATaax4nC8/nz
 OcU1IYjBo9+XIFba9EdJxqBZOMJWKDv1GdI1CgyVdzEyniUDtTyXyn4O+F7Vw/NegtDr6rAOJ
 TCjfULJos8hMDcNteij1eMjoqN4mccnxAF3Ml6tyyPMnbyXnZsb42ed/Ne0+QYEooslCF+8aw
 d5+zTZpHGYt/N4+6u2FKNQrd1/F3taRwYzruGb54CmmKU=

Am 24.03.24 um 19:12 schrieb mustafa:

> From: Mustafa Ek=C5=9Fi <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 st=
ill
probing, because it registers callbacks with other subsystems.
The subsystem (maybe led?) assumes that the device is already fully operat=
ional
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 handl=
ing 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=C5=9Fi (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
>