On Wed, 11 Mar 2026 14:22:36 +0200 Andy Shevchenko <[email protected]> wrote:
> On Wed, Mar 11, 2026 at 01:38:03PM +0200, Erikas Bitovtas wrote: > > Add supply, I2C and cathode voltage regulators to the sensor and enable > > them. This keeps the sensor powered on even after its only supply shared > > by another device shuts down. > > > Signed-off-by: Erikas Bitovtas <[email protected]> > > Reported-by: Raymond Hackley <[email protected]> > > Where was it reported? Do you need Closes tag? > > ... > > > +#include "linux/array_size.h" > > +#include "linux/regulator/consumer.h" > > Double quotes, huh?! > > > #include <linux/bitfield.h> > > #include <linux/module.h> > > #include <linux/i2c.h> > > Also, please keep the list ordered. > > ... > > > mutex_init(&data->vcnl4000_lock); > > + ret = devm_regulator_bulk_get_enable(&client->dev, > > + ARRAY_SIZE(regulator_names), > > + regulator_names); > > + if (ret < 0) > > + return ret; > > You can't add devm_ after non-devm calls. This one happens to be fine because there is no cleanup of the mutex_init(), so it is sort of not mixing devm and non devm. That is kind of a historical thing where I for one wasn't convinced it was worth the annoyance of mutex_destroy() until the devm easy way of doing it came along. Now, as the code is being touched anyway, I would like that moved to ret = devm_mutex_init(); if (ret) return ret; as a precursor patch both as it makes it obvious we are still devm and to get the advantage when lock debugging is turned on. Thanks Jonathan > Also it would help you to have > > struct device *dev = &client->dev; > > at the top of the function. > > ... > > With the above being said, I expect a series out of two patches at least. >
