On Tue, Jan 31, 2023 at 02:33:38PM +0100, Mårten Lindahl wrote: > On 1/31/23 13:46, Cai Huoqing wrote: > > On 31 1月 23 11:29:51, Mårten Lindahl wrote: ... > > 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). Exactly and if one more device will be added, we won't miss it, so it's less error prone. -- With Best Regards, Andy Shevchenko