Re: [PATCH v2] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock

["Mårten Lindahl" <martenli@axis.com>]

On 1/31/23 13:33, Andy Shevchenko wrote:
> On Tue, Jan 31, 2023 at 11:29:51AM +0100, Mårten Lindahl wrote:
>> There are different init functions for the sensors in this driver in
>> which only one initialize the generic vcnl4000_lock. With commit
> initializes ?
>
>> e21b5b1f2669 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
>> the vcnl4040 sensor started to depend on the lock, but it was missed to
>> initialize it in vcnl4040's init function. This has not been visible
>> until we run lockdep on it:
>>
>>    DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>>    at kernel/locking/mutex.c:575 __mutex_lock+0x4f8/0x890
>>    Call trace:
>>    __mutex_lock
>>    mutex_lock_nested
>>    vcnl4200_set_power_state
>>    vcnl4200_init
>>    vcnl4000_probe
>>    i2c_device_probe
>>    really_probe
>>    __driver_probe_device
>>    driver_probe_device
>>    __driver_attach
>>    bus_for_each_dev
>>    driver_attach
>>    bus_add_driver
>>    driver_register
>>    i2c_register_driver
>>    vcnl4000_driver_init
> E.g. the following can be cut without losing significance of the data
> (see below as well).
>
>>    do_one_initcall
>>    do_init_module
>>    load_module
>>    __do_sys_finit_module
>> Fix this by initializing the lock in the probe function instead of doing
>> it in the chip specific init functions.
>>
>> Fixes: e21b5b1f2669 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
>> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
>> ---
>>
>> v2:
>>   - Trimmed backtrace in commit message
> Not enough, please try harder. The ideal is to have ~3-5 lines of traceback.

Hi Andy, Maybe I get it right soon ;). Could it perhaps be stripped like 
this?

Call trace:
__mutex_lock
mutex_lock_nested
vcnl4200_set_power_state
vcnl4200_init
vcnl4000_probe
i2c_device_probe


Kind regards

Mårten

>
>>   - Have 12 digit sha-1 id in Fixes tag
>>   - Make the lock initialization in probe instead of in _init function
> ...
>
>>   	data->client = client;
>>   	data->id = id->driver_data;
>>   	data->chip_spec = &vcnl4000_chip_spec_cfg[data->id];
> + Blank line.
>
>> +	mutex_init(&data->vcnl4000_lock);
>>   
>>   	ret = data->chip_spec->init(data);
>>   	if (ret < 0)