* [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock
@ 2023-01-30 9:37 Mårten Lindahl
2023-01-30 12:39 ` Andy Shevchenko
2023-01-30 13:04 ` Jonathan Cameron
0 siblings, 2 replies; 8+ messages in thread
From: Mårten Lindahl @ 2023-01-30 9:37 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Andy Shevchenko, linux-iio, kernel,
Mårten Lindahl
There are different init functions for the sensors in this driver in
which only one initialize the generic vcnl4000_lock. With commit
e21b5b1f2 ("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)
WARNING: CPU: 1 PID: 8800 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
do_one_initcall
do_init_module
load_module
__do_sys_finit_module
...
Fix this by adding mutex_init on the lock in the init function used for
vcnl4040.
Fixes: e21b5b1f2 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
---
drivers/iio/light/vcnl4000.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index cc1a2062e76d..a8a9fc3b1a02 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -316,6 +316,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
}
mutex_init(&data->vcnl4200_al.lock);
mutex_init(&data->vcnl4200_ps.lock);
+ mutex_init(&data->vcnl4000_lock);
ret = data->chip_spec->set_power_state(data, true);
if (ret < 0)
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock
2023-01-30 9:37 [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock Mårten Lindahl
@ 2023-01-30 12:39 ` Andy Shevchenko
2023-01-30 12:40 ` Andy Shevchenko
2023-01-30 13:04 ` Jonathan Cameron
1 sibling, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2023-01-30 12:39 UTC (permalink / raw)
To: Mårten Lindahl
Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, kernel
On Mon, Jan 30, 2023 at 10:37:42AM +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
> e21b5b1f2 ("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)
> WARNING: CPU: 1 PID: 8800 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
> do_one_initcall
> do_init_module
> load_module
> __do_sys_finit_module
> ...
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
Otherwise looks good to me,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Fix this by adding mutex_init on the lock in the init function used for
> vcnl4040.
>
> Fixes: e21b5b1f2 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
> ---
> drivers/iio/light/vcnl4000.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index cc1a2062e76d..a8a9fc3b1a02 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -316,6 +316,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
> }
> mutex_init(&data->vcnl4200_al.lock);
> mutex_init(&data->vcnl4200_ps.lock);
> + mutex_init(&data->vcnl4000_lock);
>
> ret = data->chip_spec->set_power_state(data, true);
> if (ret < 0)
> --
> 2.30.2
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock
2023-01-30 12:39 ` Andy Shevchenko
@ 2023-01-30 12:40 ` Andy Shevchenko
2023-01-30 14:05 ` Mårten Lindahl
0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2023-01-30 12:40 UTC (permalink / raw)
To: Mårten Lindahl
Cc: Jonathan Cameron, Lars-Peter Clausen, linux-iio, kernel
On Mon, Jan 30, 2023 at 02:39:12PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 30, 2023 at 10:37:42AM +0100, Mårten Lindahl wrote:
...
> > Fixes: e21b5b1f2 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
Actually one more, please provide a proper Fixes tag.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
It's explained there.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock
2023-01-30 9:37 [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock Mårten Lindahl
2023-01-30 12:39 ` Andy Shevchenko
@ 2023-01-30 13:04 ` Jonathan Cameron
2023-01-30 14:10 ` Mårten Lindahl
1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2023-01-30 13:04 UTC (permalink / raw)
To: Mårten Lindahl
Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, linux-iio, kernel
On Mon, 30 Jan 2023 10:37:42 +0100
Mårten Lindahl <marten.lindahl@axis.com> wrote:
> There are different init functions for the sensors in this driver in
> which only one initialize the generic vcnl4000_lock. With commit
> e21b5b1f2 ("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)
> WARNING: CPU: 1 PID: 8800 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
> do_one_initcall
> do_init_module
> load_module
> __do_sys_finit_module
> ...
>
> Fix this by adding mutex_init on the lock in the init function used for
> vcnl4040.
>
> Fixes: e21b5b1f2 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
Perhaps better to pull the lock out of the device specific setup?
> ---
> drivers/iio/light/vcnl4000.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index cc1a2062e76d..a8a9fc3b1a02 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -316,6 +316,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
> }
> mutex_init(&data->vcnl4200_al.lock);
> mutex_init(&data->vcnl4200_ps.lock);
> + mutex_init(&data->vcnl4000_lock);
>
> ret = data->chip_spec->set_power_state(data, true);
> if (ret < 0)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock
2023-01-30 12:40 ` Andy Shevchenko
@ 2023-01-30 14:05 ` Mårten Lindahl
0 siblings, 0 replies; 8+ messages in thread
From: Mårten Lindahl @ 2023-01-30 14:05 UTC (permalink / raw)
To: Andy Shevchenko, Mårten Lindahl
Cc: linux-iio, Lars-Peter Clausen, kernel, Jonathan Cameron
On 1/30/23 13:40, Andy Shevchenko wrote:
> On Mon, Jan 30, 2023 at 02:39:12PM +0200, Andy Shevchenko wrote:
>> On Mon, Jan 30, 2023 at 10:37:42AM +0100, Mårten Lindahl wrote:
> ...
>
>>> Fixes: e21b5b1f2 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
> Actually one more, please provide a proper Fixes tag.
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>
> It's explained there.
>
Hi Andy! Sorry, I'll fix those.
Kind regards
Mårten
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock
2023-01-30 13:04 ` Jonathan Cameron
@ 2023-01-30 14:10 ` Mårten Lindahl
2023-01-30 17:30 ` Jonathan Cameron
0 siblings, 1 reply; 8+ messages in thread
From: Mårten Lindahl @ 2023-01-30 14:10 UTC (permalink / raw)
To: Jonathan Cameron, Mårten Lindahl
Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, linux-iio, kernel
On 1/30/23 14:04, Jonathan Cameron wrote:
> On Mon, 30 Jan 2023 10:37:42 +0100
> Mårten Lindahl <marten.lindahl@axis.com> wrote:
>
>> There are different init functions for the sensors in this driver in
>> which only one initialize the generic vcnl4000_lock. With commit
>> e21b5b1f2 ("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)
>> WARNING: CPU: 1 PID: 8800 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
>> do_one_initcall
>> do_init_module
>> load_module
>> __do_sys_finit_module
>> ...
>>
>> Fix this by adding mutex_init on the lock in the init function used for
>> vcnl4040.
>>
>> Fixes: e21b5b1f2 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
>> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
> Perhaps better to pull the lock out of the device specific setup?
Hi Jonathan!
Ok, having a
static DEFINE_MUTEX(vcnl4000_lock);
will make sure initializing it wont be forgotten for specific setups.
I'll test it.
Kind regards
Mårten
>
>> ---
>> drivers/iio/light/vcnl4000.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
>> index cc1a2062e76d..a8a9fc3b1a02 100644
>> --- a/drivers/iio/light/vcnl4000.c
>> +++ b/drivers/iio/light/vcnl4000.c
>> @@ -316,6 +316,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
>> }
>> mutex_init(&data->vcnl4200_al.lock);
>> mutex_init(&data->vcnl4200_ps.lock);
>> + mutex_init(&data->vcnl4000_lock);
>>
>> ret = data->chip_spec->set_power_state(data, true);
>> if (ret < 0)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock
2023-01-30 14:10 ` Mårten Lindahl
@ 2023-01-30 17:30 ` Jonathan Cameron
2023-01-31 7:49 ` Mårten Lindahl
0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2023-01-30 17:30 UTC (permalink / raw)
To: Mårten Lindahl
Cc: Mårten Lindahl, Jonathan Cameron, Lars-Peter Clausen,
Andy Shevchenko, linux-iio, kernel
On Mon, 30 Jan 2023 15:10:40 +0100
Mårten Lindahl <martenli@axis.com> wrote:
> On 1/30/23 14:04, Jonathan Cameron wrote:
> > On Mon, 30 Jan 2023 10:37:42 +0100
> > Mårten Lindahl <marten.lindahl@axis.com> wrote:
> >
> >> There are different init functions for the sensors in this driver in
> >> which only one initialize the generic vcnl4000_lock. With commit
> >> e21b5b1f2 ("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)
> >> WARNING: CPU: 1 PID: 8800 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
> >> do_one_initcall
> >> do_init_module
> >> load_module
> >> __do_sys_finit_module
> >> ...
> >>
> >> Fix this by adding mutex_init on the lock in the init function used for
> >> vcnl4040.
> >>
> >> Fixes: e21b5b1f2 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
> >> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
> > Perhaps better to pull the lock out of the device specific setup?
>
> Hi Jonathan!
>
> Ok, having a
>
> static DEFINE_MUTEX(vcnl4000_lock);
>
No. Definitely don't do that. We need one per device instance.
I just meant move the mutex_init() into the main probe function rather
than the chip specific init that is called from there.
> will make sure initializing it wont be forgotten for specific setups.
>
> I'll test it.
>
> Kind regards
>
> Mårten
>
> >
> >> ---
> >> drivers/iio/light/vcnl4000.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> >> index cc1a2062e76d..a8a9fc3b1a02 100644
> >> --- a/drivers/iio/light/vcnl4000.c
> >> +++ b/drivers/iio/light/vcnl4000.c
> >> @@ -316,6 +316,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
> >> }
> >> mutex_init(&data->vcnl4200_al.lock);
> >> mutex_init(&data->vcnl4200_ps.lock);
> >> + mutex_init(&data->vcnl4000_lock);
> >>
> >> ret = data->chip_spec->set_power_state(data, true);
> >> if (ret < 0)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock
2023-01-30 17:30 ` Jonathan Cameron
@ 2023-01-31 7:49 ` Mårten Lindahl
0 siblings, 0 replies; 8+ messages in thread
From: Mårten Lindahl @ 2023-01-31 7:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, linux-iio, kernel
On 1/30/23 18:30, Jonathan Cameron wrote:
> On Mon, 30 Jan 2023 15:10:40 +0100
> Mårten Lindahl <martenli@axis.com> wrote:
>
>> On 1/30/23 14:04, Jonathan Cameron wrote:
>>> On Mon, 30 Jan 2023 10:37:42 +0100
>>> Mårten Lindahl <marten.lindahl@axis.com> wrote:
>>>
>>>> There are different init functions for the sensors in this driver in
>>>> which only one initialize the generic vcnl4000_lock. With commit
>>>> e21b5b1f2 ("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)
>>>> WARNING: CPU: 1 PID: 8800 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
>>>> do_one_initcall
>>>> do_init_module
>>>> load_module
>>>> __do_sys_finit_module
>>>> ...
>>>>
>>>> Fix this by adding mutex_init on the lock in the init function used for
>>>> vcnl4040.
>>>>
>>>> Fixes: e21b5b1f2 ("iio: light: vcnl4000: Preserve conf bits when toggle power")
>>>> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
>>> Perhaps better to pull the lock out of the device specific setup?
>> Hi Jonathan!
>>
>> Ok, having a
>>
>> static DEFINE_MUTEX(vcnl4000_lock);
>>
> No. Definitely don't do that. We need one per device instance.
>
> I just meant move the mutex_init() into the main probe function rather
> than the chip specific init that is called from there.
Ok, sorry for the missunderstanding. I''ll fix it.
Kind regards
Mårten
>
>> will make sure initializing it wont be forgotten for specific setups.
>>
>> I'll test it.
>>
>> Kind regards
>>
>> Mårten
>>
>>>
>>>> ---
>>>> drivers/iio/light/vcnl4000.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
>>>> index cc1a2062e76d..a8a9fc3b1a02 100644
>>>> --- a/drivers/iio/light/vcnl4000.c
>>>> +++ b/drivers/iio/light/vcnl4000.c
>>>> @@ -316,6 +316,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
>>>> }
>>>> mutex_init(&data->vcnl4200_al.lock);
>>>> mutex_init(&data->vcnl4200_ps.lock);
>>>> + mutex_init(&data->vcnl4000_lock);
>>>>
>>>> ret = data->chip_spec->set_power_state(data, true);
>>>> if (ret < 0)
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-01-31 7:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30 9:37 [PATCH] iio: light: vcnl4000: Fix WARN_ON on uninitialized lock Mårten Lindahl
2023-01-30 12:39 ` Andy Shevchenko
2023-01-30 12:40 ` Andy Shevchenko
2023-01-30 14:05 ` Mårten Lindahl
2023-01-30 13:04 ` Jonathan Cameron
2023-01-30 14:10 ` Mårten Lindahl
2023-01-30 17:30 ` Jonathan Cameron
2023-01-31 7:49 ` Mårten Lindahl
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.