[PATCH 2/2] iio: light: vcnl4000: add regulator support

Erikas Bitovtas posted 2 patches 3 weeks, 6 days ago
There is a newer version of this series
[PATCH 2/2] iio: light: vcnl4000: add regulator support
Posted by Erikas Bitovtas 3 weeks, 6 days ago
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 <xerikasxx@gmail.com>
Reported-by: Raymond Hackley <raymondhackley@protonmail.com>
---
 drivers/iio/light/vcnl4000.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 5e03c3d8874b..967589d5f246 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -17,6 +17,8 @@
  *   interrupts (VCNL4040, VCNL4200)
  */
 
+#include "linux/array_size.h"
+#include "linux/regulator/consumer.h"
 #include <linux/bitfield.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
@@ -1983,6 +1985,7 @@ static int vcnl4010_probe_trigger(struct iio_dev *indio_dev)
 static int vcnl4000_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
+	const char * const regulator_names[] = { "vdd", "vddio", "vled" };
 	struct vcnl4000_data *data;
 	struct iio_dev *indio_dev;
 	int ret;
@@ -1998,6 +2001,11 @@ static int vcnl4000_probe(struct i2c_client *client)
 	data->chip_spec = &vcnl4000_chip_spec_cfg[data->id];
 
 	mutex_init(&data->vcnl4000_lock);
+	ret = devm_regulator_bulk_get_enable(&client->dev,
+				      ARRAY_SIZE(regulator_names),
+				      regulator_names);
+	if (ret < 0)
+		return ret;
 
 	ret = data->chip_spec->init(data);
 	if (ret < 0)

-- 
2.53.0
Re: [PATCH 2/2] iio: light: vcnl4000: add regulator support
Posted by Andy Shevchenko 3 weeks, 6 days ago
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 <xerikasxx@gmail.com>
> Reported-by: Raymond Hackley <raymondhackley@protonmail.com>

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.
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.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 2/2] iio: light: vcnl4000: add regulator support
Posted by Jonathan Cameron 3 weeks, 2 days ago
On Wed, 11 Mar 2026 14:22:36 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> 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 <xerikasxx@gmail.com>
> > Reported-by: Raymond Hackley <raymondhackley@protonmail.com>  
> 
> 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.
>
Re: [PATCH 2/2] iio: light: vcnl4000: add regulator support
Posted by Erikas Bitovtas 3 weeks, 6 days ago

On 3/11/26 2:22 PM, Andy Shevchenko 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 <xerikasxx@gmail.com>
>> Reported-by: Raymond Hackley <raymondhackley@protonmail.com>
> 
> Where was it reported? Do you need Closes tag?
> 
> ...
The report was done outside of LKML, in a Matrix channel.

Respectfully,
Erikas Bitovtas
Re: [PATCH 2/2] iio: light: vcnl4000: add regulator support
Posted by Andy Shevchenko 3 weeks, 6 days ago
On Wed, Mar 11, 2026 at 02:40:00PM +0200, Erikas Bitovtas wrote:
> On 3/11/26 2:22 PM, Andy Shevchenko 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 <xerikasxx@gmail.com>
> >> Reported-by: Raymond Hackley <raymondhackley@protonmail.com>
> > 
> > Where was it reported? Do you need Closes tag?

> The report was done outside of LKML, in a Matrix channel.

Good, next time don't forget to add this type of information in the comments
block, so we will know why Closes is not accompanied with the Reported-by.
But if that channel has web-available archives, it will be good to have a link
to it in the Closes tag.

-- 
With Best Regards,
Andy Shevchenko