czw., 23 gru 2021 o 16:51 Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> napisał(a): > > On Wed, Dec 22, 2021 at 10:45:57AM +0100, Jan Dabros wrote: > > All accesses to controller's registers should be protected on > > probe, disable and xfer paths. This is needed for i2c bus controllers > > that are shared with but not controlled by kernel. > > ... > > > + ret = i2c_dw_acquire_lock(dev); > > + if (ret) > > + return ret; > > + > > ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, ¶m); > > if (ret) > > return ret; > > + i2c_dw_release_lock(dev); > > Not sure this part is fully correct. Please, fix the leakage. Correct, I will move release() right below regmap_read() call, before checking return code. > > ... > > > + ret = i2c_dw_acquire_lock(dev); > > + if (ret) > > + return; > > > > /* Disable controller */ > > __i2c_dw_disable(dev); > > @@ -614,6 +624,8 @@ void i2c_dw_disable(struct dw_i2c_dev *dev) > > /* Disable all interrupts */ > > regmap_write(dev->map, DW_IC_INTR_MASK, 0); > > regmap_read(dev->map, DW_IC_CLR_INTR, &dummy); > > > + i2c_dw_release_lock(dev); > > Not enough context here, bu I believe there is the same issue(s). Since we are ignoring values returned by regmap_write()/read() and also __i2c_dw_disable() returns void I don't see a leakage here. Don't have more function calls here.
