Hello Guenter, Thanks for taking the time and doing review! On Tue, Jan 22, 2019 at 06:51:26AM -0800, Guenter Roeck wrote: > On 1/22/19 1:44 AM, Matti Vaittinen wrote: > > +static DEFINE_MUTEX(rtc_timer_mutex); // snip > > +static int bd70528_i2c_probe(struct i2c_client *i2c, > > + const struct i2c_device_id *id) > > +{ > > + struct bd70528 *bd70528; > > + struct regmap_irq_chip_data *irq_data; > > + int ret, i; > > + > > + if (!i2c->irq) { > > + dev_err(&i2c->dev, "No IRQ configured\n"); > > + return -EINVAL; > > + } > > + bd70528 = devm_kzalloc(&i2c->dev, sizeof(*bd70528), GFP_KERNEL); > > + > > + if (!bd70528) > > + return -ENOMEM; > > + > > + dev_set_drvdata(&i2c->dev, bd70528); > > + bd70528->rtc_timer_lock = &rtc_timer_mutex; > > One global mutex for all instances of this driver is odd. > Why isn't this just part of struct bd70528 ? > You are right. This was a brainfart from my side. I don't think there is many cases where this would har though. I don't expect to see many instances of PMIC drivers load. But you are correct nevertheless. I will fix this in future version. Thanks for pointing it out! Br, Matti Vaittinen -- Matti Vaittinen ROHM Semiconductors ~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~