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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/31/23 13:46, Cai Huoqing wrote:
On 31 1月 23 11:29:51, 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
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
   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@xxxxxxxx>
---

v2:
  - Trimmed backtrace in commit message
  - Have 12 digit sha-1 id in Fixes tag
  - Make the lock initialization in probe instead of in _init function

  drivers/iio/light/vcnl4000.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index b5d398228289..caa2fff9f486 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -208,7 +208,6 @@ static int vcnl4000_init(struct vcnl4000_data *data)
data->rev = ret & 0xf;
  	data->al_scale = 250000;
-	mutex_init(&data->vcnl4000_lock);
return data->chip_spec->set_power_state(data, true);
  };
@@ -1366,6 +1365,7 @@ static int vcnl4000_probe(struct i2c_client *client,
  	data->client = client;
  	data->id = id->driver_data;
  	data->chip_spec = &vcnl4000_chip_spec_cfg[data->id];
+	mutex_init(&data->vcnl4000_lock);
ret = data->chip_spec->init(data);
  	if (ret < 0)
Why not add mutex_init(&data->vcnl4000_lock) in vcnl4200_init.
like this

@@ -330,6 +330,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)

Perfer adding mutex_init to vcnl4200_init.

Hi Cai!

This is what I did in v1, but I got a suggestion to move it to the probe instead.

Having it in probe takes one mutex_init. Otherwise it needs to be in two places (both init functions).

Kind regards

Mårten

--
2.30.2




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux