[PATCH v3 0/6] iio: light: vcnl4000: add regulator support

[PATCH v3 0/6] iio: light: vcnl4000: add regulator support

[Erikas Bitovtas]

This patch series introduces support for voltage supply, I2C and cathode
regulators. This fixes an issue where if a regulator is shared between
the proximity sensor and some other device, and the other device is
powered off, the proximity sensor would be powered off as well.

One of the commits includes a Reported-by: tag without a Closes: tag -
the report was done outside of LKML.

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
Changes in v3:
- Added a more detailed description for supplies in the dt-bindings commit.
- Separated sorting includes into a commit of its own.
- Replaced all occurrences of mutex_init with its device-managed
  counterpart.
- Moved client->dev variable declaration into a commit for adding
  regulators.
- Removed redundant dev_err messages in probe function.
- Replaced all direct usages of client->dev and data->client into usages
  by variable.
- Link to v2: https://lore.kernel.org/r/20260312-vcnl4000-regulators-v2-0-2bdebbcbb58a@gmail.com

Changes in v2:
- Removed double quotes in includes.
- Reordered includes alphabetically.
- Enabled regulators before the mutex is initialized.
- Replaced direct usage of &client->dev with a variable.
- Link to v1: https://lore.kernel.org/r/20260311-vcnl4000-regulators-v1-0-66b6038ce563@gmail.com

---
Erikas Bitovtas (6):
      dt-bindings: iio: light: vcnl4000: add regulators
      iio: light: vcnl4000: sort includes by their name
      iio: light: vcnl4000: replace mutex_init with devm_mutex_init
      iio: light: vcnl4000: add support for regulators
      iio: light: vcnl4000: remove error messages for trigger and irq
      iio: light: vcnl4000: use variables for I2C client and device instances

 .../bindings/iio/light/vishay,vcnl4000.yaml        |  7 ++
 drivers/iio/light/vcnl4000.c                       | 76 +++++++++++++---------
 2 files changed, 52 insertions(+), 31 deletions(-)
---
base-commit: 6e03baeeb160e6cfd72f2c39f26c50bcd925c7a0
change-id: 20260310-vcnl4000-regulators-bcf1b8a01ce6

Best regards,
-- 
Erikas Bitovtas <xerikasxx@gmail.com>

[PATCH v3 1/6] dt-bindings: iio: light: vcnl4000: add regulators

[Erikas Bitovtas]

This commit adds supply properties for the sensor, I2C bus and IR LED
anode supplies [1], which can be found in vcnl4000 ambient light and
proximity sensors, to ensure the sensor is powered on before proceeding
with setup.

[1] https://www.vishay.com/docs/84274/vcnl4040.pdf

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
 Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
index 2ba4d5de4ec4..a1b4c02db246 100644
--- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
+++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
@@ -33,6 +33,10 @@ properties:
   interrupts:
     maxItems: 1
 
+  vdd-supply: true
+  vddio-supply: true
+  vled-supply: true
+
   reg:
     maxItems: 1
 
@@ -54,6 +58,9 @@ examples:
             compatible = "vishay,vcnl4200";
             reg = <0x51>;
             proximity-near-level = <220>;
+            vdd-supply = <&reg_vdd>;
+            vddio-supply = <&reg_vddio>;
+            vled-supply = <&reg_vled>;
         };
     };
 ...

-- 
2.53.0

[PATCH v3 2/6] iio: light: vcnl4000: sort includes by their name

[Erikas Bitovtas]

Sort include headers by file name for better readability.

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
 drivers/iio/light/vcnl4000.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 5e03c3d8874b..939ff2d65105 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -18,12 +18,12 @@
  */
 
 #include <linux/bitfield.h>
-#include <linux/module.h>
-#include <linux/i2c.h>
-#include <linux/err.h>
 #include <linux/delay.h>
-#include <linux/pm_runtime.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
 #include <linux/units.h>
 
 #include <linux/iio/buffer.h>

-- 
2.53.0

[PATCH v3 3/6] iio: light: vcnl4000: replace mutex_init with devm_mutex_init

[Erikas Bitovtas]

Replace mutex_init used across driver with its device-managed
counterpart, so all assigned mutexes get destroyed.

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
 drivers/iio/light/vcnl4000.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 939ff2d65105..0ee307fc5ab7 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -356,6 +356,8 @@ static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on)
 
 static int vcnl4200_init(struct vcnl4000_data *data)
 {
+	struct i2c_client *client = data->client;
+	struct device *dev = &client->dev;
 	int ret, id;
 	u16 regval;
 
@@ -400,8 +402,14 @@ static int vcnl4200_init(struct vcnl4000_data *data)
 	}
 	data->al_scale = data->chip_spec->ulux_step;
 	data->ps_scale = 16;
-	mutex_init(&data->vcnl4200_al.lock);
-	mutex_init(&data->vcnl4200_ps.lock);
+
+	ret = devm_mutex_init(dev, &data->vcnl4200_al.lock);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_mutex_init(dev, &data->vcnl4200_ps.lock);
+	if (ret < 0)
+		return ret;
 
 	/* Use 16 bits proximity sensor readings */
 	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
@@ -1985,6 +1993,7 @@ static int vcnl4000_probe(struct i2c_client *client)
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
 	struct vcnl4000_data *data;
 	struct iio_dev *indio_dev;
+	struct device *dev = &client->dev;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
@@ -1997,7 +2006,9 @@ static int vcnl4000_probe(struct i2c_client *client)
 	data->id = id->driver_data;
 	data->chip_spec = &vcnl4000_chip_spec_cfg[data->id];
 
-	mutex_init(&data->vcnl4000_lock);
+	ret = devm_mutex_init(dev, &data->vcnl4000_lock);
+	if (ret < 0)
+		return ret;
 
 	ret = data->chip_spec->init(data);
 	if (ret < 0)

-- 
2.53.0

[PATCH v3 4/6] iio: light: vcnl4000: add support for regulators

[Erikas Bitovtas]

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 0ee307fc5ab7..e501db7249d7 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -24,6 +24,7 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 #include <linux/units.h>
 
 #include <linux/iio/buffer.h>
@@ -1991,6 +1992,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;
 	struct device *dev = &client->dev;
@@ -2006,6 +2008,12 @@ static int vcnl4000_probe(struct i2c_client *client)
 	data->id = id->driver_data;
 	data->chip_spec = &vcnl4000_chip_spec_cfg[data->id];
 
+	ret = devm_regulator_bulk_get_enable(dev,
+				      ARRAY_SIZE(regulator_names),
+				      regulator_names);
+	if (ret < 0)
+		return ret;
+
 	ret = devm_mutex_init(dev, &data->vcnl4000_lock);
 	if (ret < 0)
 		return ret;

-- 
2.53.0

[PATCH v3 5/6] iio: light: vcnl4000: remove error messages for trigger and irq

[Erikas Bitovtas]

The error code is available in the log after return. Remove duplicate
error messages to reduce noise in dmesg.

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
 drivers/iio/light/vcnl4000.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index e501db7249d7..c8bb1826b916 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -2041,11 +2041,8 @@ static int vcnl4000_probe(struct i2c_client *client)
 						      NULL,
 						      data->chip_spec->trig_buffer_func,
 						      data->chip_spec->buffer_setup_ops);
-		if (ret < 0) {
-			dev_err(&client->dev,
-				"unable to setup iio triggered buffer\n");
+		if (ret < 0)
 			return ret;
-		}
 	}
 
 	if (client->irq && data->chip_spec->irq_thread) {
@@ -2055,10 +2052,8 @@ static int vcnl4000_probe(struct i2c_client *client)
 						IRQF_ONESHOT,
 						"vcnl4000_irq",
 						indio_dev);
-		if (ret < 0) {
-			dev_err(&client->dev, "irq request failed\n");
+		if (ret < 0)
 			return ret;
-		}
 
 		ret = vcnl4010_probe_trigger(indio_dev);
 		if (ret < 0)

-- 
2.53.0

[PATCH v3 6/6] iio: light: vcnl4000: use variables for I2C client and device instances

[Erikas Bitovtas]

After moving data->client and client->dev into variables of their own,
replace all instances of data->client and client->dev being used in
vcnl4200_init and vcnl4000_probe by the said variables to reduce
clutter.

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
 drivers/iio/light/vcnl4000.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index c8bb1826b916..9b7cd3f0c8fb 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -362,14 +362,14 @@ static int vcnl4200_init(struct vcnl4000_data *data)
 	int ret, id;
 	u16 regval;
 
-	ret = i2c_smbus_read_word_data(data->client, VCNL4200_DEV_ID);
+	ret = i2c_smbus_read_word_data(client, VCNL4200_DEV_ID);
 	if (ret < 0)
 		return ret;
 
 	id = ret & 0xff;
 
 	if (id != VCNL4200_PROD_ID) {
-		ret = i2c_smbus_read_word_data(data->client, VCNL4040_DEV_ID);
+		ret = i2c_smbus_read_word_data(client, VCNL4040_DEV_ID);
 		if (ret < 0)
 			return ret;
 
@@ -379,7 +379,7 @@ static int vcnl4200_init(struct vcnl4000_data *data)
 			return -ENODEV;
 	}
 
-	dev_dbg(&data->client->dev, "device id 0x%x", id);
+	dev_dbg(dev, "device id 0x%x", id);
 
 	data->rev = (ret >> 8) & 0xf;
 	data->ps_int = 0;
@@ -413,23 +413,23 @@ static int vcnl4200_init(struct vcnl4000_data *data)
 		return ret;
 
 	/* Use 16 bits proximity sensor readings */
-	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
+	ret = i2c_smbus_read_word_data(client, VCNL4200_PS_CONF1);
 	if (ret < 0)
 		return ret;
 
 	regval = ret | VCNL4040_PS_CONF2_PS_HD;
-	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
+	ret = i2c_smbus_write_word_data(client, VCNL4200_PS_CONF1,
 					regval);
 	if (ret < 0)
 		return ret;
 
 	/* Align proximity sensor sample rate to 16 bits data width */
-	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF3);
+	ret = i2c_smbus_read_word_data(client, VCNL4200_PS_CONF3);
 	if (ret < 0)
 		return ret;
 
 	regval = ret | VCNL4040_CONF3_PS_SAMPLE_16BITS;
-	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
+	ret = i2c_smbus_write_word_data(client, VCNL4200_PS_CONF3,
 					regval);
 	if (ret < 0)
 		return ret;
@@ -1998,7 +1998,7 @@ static int vcnl4000_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -2022,10 +2022,10 @@ static int vcnl4000_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
-	dev_dbg(&client->dev, "%s Ambient light/proximity sensor, Rev: %02x\n",
+	dev_dbg(dev, "%s Ambient light/proximity sensor, Rev: %02x\n",
 		data->chip_spec->prod, data->rev);
 
-	if (device_property_read_u32(&client->dev, "proximity-near-level",
+	if (device_property_read_u32(dev, "proximity-near-level",
 				     &data->near_level))
 		data->near_level = 0;
 
@@ -2037,7 +2037,7 @@ static int vcnl4000_probe(struct i2c_client *client)
 
 	if (data->chip_spec->trig_buffer_func &&
 	    data->chip_spec->buffer_setup_ops) {
-		ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
+		ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
 						      NULL,
 						      data->chip_spec->trig_buffer_func,
 						      data->chip_spec->buffer_setup_ops);
@@ -2046,8 +2046,8 @@ static int vcnl4000_probe(struct i2c_client *client)
 	}
 
 	if (client->irq && data->chip_spec->irq_thread) {
-		ret = devm_request_threaded_irq(&client->dev, client->irq,
-						NULL, data->chip_spec->irq_thread,
+		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+						data->chip_spec->irq_thread,
 						IRQF_TRIGGER_FALLING |
 						IRQF_ONESHOT,
 						"vcnl4000_irq",
@@ -2060,7 +2060,7 @@ static int vcnl4000_probe(struct i2c_client *client)
 			return ret;
 	}
 
-	ret = pm_runtime_set_active(&client->dev);
+	ret = pm_runtime_set_active(dev);
 	if (ret < 0)
 		goto fail_poweroff;
 
@@ -2068,9 +2068,9 @@ static int vcnl4000_probe(struct i2c_client *client)
 	if (ret < 0)
 		goto fail_poweroff;
 
-	pm_runtime_enable(&client->dev);
-	pm_runtime_set_autosuspend_delay(&client->dev, VCNL4000_SLEEP_DELAY_MS);
-	pm_runtime_use_autosuspend(&client->dev);
+	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev, VCNL4000_SLEEP_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);
 
 	return 0;
 fail_poweroff:

-- 
2.53.0

Re: [PATCH v3 3/6] iio: light: vcnl4000: replace mutex_init with devm_mutex_init

[David Lechner]

On 3/14/26 11:06 AM, Erikas Bitovtas wrote:
> Replace mutex_init used across driver with its device-managed
> counterpart, so all assigned mutexes get destroyed.
> 

...

> @@ -1985,6 +1993,7 @@ static int vcnl4000_probe(struct i2c_client *client)
>  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
>  	struct vcnl4000_data *data;
>  	struct iio_dev *indio_dev;
> +	struct device *dev = &client->dev;

Purely cosmetic, but I would have put this two lines up (right after
*id) to keep assignments and longer lines together.

No need to v4 though, everything else look good.

>  	int ret;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));

Re: [PATCH v3 0/6] iio: light: vcnl4000: add regulator support

[David Lechner]

On 3/14/26 11:06 AM, Erikas Bitovtas wrote:
> This patch series introduces support for voltage supply, I2C and cathode
> regulators. This fixes an issue where if a regulator is shared between
> the proximity sensor and some other device, and the other device is
> powered off, the proximity sensor would be powered off as well.
> 
> One of the commits includes a Reported-by: tag without a Closes: tag -
> the report was done outside of LKML.

If it is on the public web, we can still add a link. You can just reply
to that patch with the tag (if there is one) and it will get picked up.

> 
> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
> ---

Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v3 1/6] dt-bindings: iio: light: vcnl4000: add regulators

[Krzysztof Kozlowski]

On Sat, Mar 14, 2026 at 06:06:30PM +0200, Erikas Bitovtas wrote:
> This commit adds supply properties for the sensor, I2C bus and IR LED

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94

> anode supplies [1], which can be found in vcnl4000 ambient light and

So other devices do not have these supplies? Or have only some?

You already received that question, so I assume you checked. Then check
again because 4040 has VDD.

thus in
that case 

> proximity sensors, to ensure the sensor is powered on before proceeding
> with setup.
> 
> [1] https://www.vishay.com/docs/84274/vcnl4040.pdf
> 
> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
> ---
>  Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> index 2ba4d5de4ec4..a1b4c02db246 100644
> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> @@ -33,6 +33,10 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  vdd-supply: true
> +  vddio-supply: true

VCNL4000 does not have VDDIO and VLED pins.

Best regards,
Krzysztof

Re: [PATCH v3 1/6] dt-bindings: iio: light: vcnl4000: add regulators

[Erikas Bitovtas]

On 3/15/26 10:49 AM, Krzysztof Kozlowski wrote:
> On Sat, Mar 14, 2026 at 06:06:30PM +0200, Erikas Bitovtas wrote:
>> This commit adds supply properties for the sensor, I2C bus and IR LED
> 
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94
> 
>> anode supplies [1], which can be found in vcnl4000 ambient light and
> 
> So other devices do not have these supplies? Or have only some?
> 
> You already received that question, so I assume you checked. Then check
> again because 4040 has VDD.
> 

VLED is supposed to be the IR anode pin. Every model has one. It is pin
1 on vcnl4000, vcnl4010 and vcnl4020, pin 6 on vcnl4200 and pin 4 on
vcnl4040.
VDD pin is pin 7 on vcnl4000 and vcnl4010, pin 5 on vcnl4020 and pin 3
on vcnl4040 and vcnl4200.
VDDIO is an external supply to the I2C bus for the sensor.

> thus in
> that case 
> 
>> proximity sensors, to ensure the sensor is powered on before proceeding
>> with setup.
>>
>> [1] https://www.vishay.com/docs/84274/vcnl4040.pdf
>>
>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>> index 2ba4d5de4ec4..a1b4c02db246 100644
>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>> @@ -33,6 +33,10 @@ properties:
>>    interrupts:
>>      maxItems: 1
>>  
>> +  vdd-supply: true
>> +  vddio-supply: true
> 
> VCNL4000 does not have VDDIO and VLED pins.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 3/6] iio: light: vcnl4000: replace mutex_init with devm_mutex_init

[Jonathan Cameron]

On Sat, 14 Mar 2026 18:06:32 +0200
Erikas Bitovtas <xerikasxx@gmail.com> wrote:

> Replace mutex_init used across driver with its device-managed
> counterpart, so all assigned mutexes get destroyed.
> 
> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
Hi Erikas,

One related area that I noticed whilst checking this patch was
safe wrt to ordering.  I think cleaning that up before this
patch would make this one more obviously correct.

Jonathan

> ---
>  drivers/iio/light/vcnl4000.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 939ff2d65105..0ee307fc5ab7 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -356,6 +356,8 @@ static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on)
>  
>  static int vcnl4200_init(struct vcnl4000_data *data)
>  {
> +	struct i2c_client *client = data->client;
> +	struct device *dev = &client->dev;
>  	int ret, id;
>  	u16 regval;
>  
> @@ -400,8 +402,14 @@ static int vcnl4200_init(struct vcnl4000_data *data)
>  	}
>  	data->al_scale = data->chip_spec->ulux_step;
>  	data->ps_scale = 16;
> -	mutex_init(&data->vcnl4200_al.lock);
> -	mutex_init(&data->vcnl4200_ps.lock);
> +
> +	ret = devm_mutex_init(dev, &data->vcnl4200_al.lock);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = devm_mutex_init(dev, &data->vcnl4200_ps.lock);
> +	if (ret < 0)
> +		return ret;
I think this is ok because the only thing undone in remove is the power state
setting that is the last call in this function but the mixture of non
devm and devm calls in init is less than helpful for readability.
Given both init() callbacks end with
return data->chip_spec->set_power_state(data, true) and the remove
just calls that callback without any wrapping up in different init functions
I'm thinking it would make it all more readable if we didn't consider
turning on the power as part of the _init() but instead called it
directly from probe().

That would perhaps give more readable code and avoid mix of devm cleanup
and other cleanup in those callbacks.


>  
>  	/* Use 16 bits proximity sensor readings */
>  	ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
> @@ -1985,6 +1993,7 @@ static int vcnl4000_probe(struct i2c_client *client)
>  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
>  	struct vcnl4000_data *data;
>  	struct iio_dev *indio_dev;
> +	struct device *dev = &client->dev;
>  	int ret;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> @@ -1997,7 +2006,9 @@ static int vcnl4000_probe(struct i2c_client *client)
>  	data->id = id->driver_data;
>  	data->chip_spec = &vcnl4000_chip_spec_cfg[data->id];
>  
> -	mutex_init(&data->vcnl4000_lock);
> +	ret = devm_mutex_init(dev, &data->vcnl4000_lock);
> +	if (ret < 0)
> +		return ret;
>  
>  	ret = data->chip_spec->init(data);
>  	if (ret < 0)
> 

Re: [PATCH v3 4/6] iio: light: vcnl4000: add support for regulators

[Jonathan Cameron]

On Sat, 14 Mar 2026 18:06:33 +0200
Erikas Bitovtas <xerikasxx@gmail.com> 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>

Mixing new features and cleanup through the series isn't ideal.
Can we move this one to the end?  So do all the cleanup first
then add this new handling?

Thanks,

Jonathan

> ---
>  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 0ee307fc5ab7..e501db7249d7 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -24,6 +24,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/units.h>
>  
>  #include <linux/iio/buffer.h>
> @@ -1991,6 +1992,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;
>  	struct device *dev = &client->dev;
> @@ -2006,6 +2008,12 @@ static int vcnl4000_probe(struct i2c_client *client)
>  	data->id = id->driver_data;
>  	data->chip_spec = &vcnl4000_chip_spec_cfg[data->id];
>  
> +	ret = devm_regulator_bulk_get_enable(dev,
> +				      ARRAY_SIZE(regulator_names),
> +				      regulator_names);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = devm_mutex_init(dev, &data->vcnl4000_lock);
>  	if (ret < 0)
>  		return ret;
> 

Re: [PATCH v3 5/6] iio: light: vcnl4000: remove error messages for trigger and irq

[Jonathan Cameron]

On Sat, 14 Mar 2026 18:06:34 +0200
Erikas Bitovtas <xerikasxx@gmail.com> wrote:

> The error code is available in the log after return. Remove duplicate
> error messages to reduce noise in dmesg.
> 
> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
> ---
>  drivers/iio/light/vcnl4000.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index e501db7249d7..c8bb1826b916 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -2041,11 +2041,8 @@ static int vcnl4000_probe(struct i2c_client *client)
>  						      NULL,
>  						      data->chip_spec->trig_buffer_func,
>  						      data->chip_spec->buffer_setup_ops);
> -		if (ret < 0) {
> -			dev_err(&client->dev,
> -				"unable to setup iio triggered buffer\n");

Is this one a duplicate? I don't recall us being particular verbose
in terms of error messages in iio_triggered_buffer_setup_ext() which is
where that ends up coming from.  I think there is only one path
where it can return anything other than -ENOMEM and that one is a duplicate
registration check (so fair to not print).  So perhaps all this patch
needs is a comment on what errors can surface from this call and why
it is therefore not worth printing anything.



> +		if (ret < 0)
>  			return ret;
> -		}
>  	}
>  
>  	if (client->irq && data->chip_spec->irq_thread) {
> @@ -2055,10 +2052,8 @@ static int vcnl4000_probe(struct i2c_client *client)
>  						IRQF_ONESHOT,
>  						"vcnl4000_irq",
>  						indio_dev);
> -		if (ret < 0) {
> -			dev_err(&client->dev, "irq request failed\n");
> +		if (ret < 0)
>  			return ret;
> -		}
>  
>  		ret = vcnl4010_probe_trigger(indio_dev);
>  		if (ret < 0)
> 

Re: [PATCH v3 5/6] iio: light: vcnl4000: remove error messages for trigger and irq

[Erikas Bitovtas]

On 3/15/26 8:31 PM, Jonathan Cameron wrote:
> On Sat, 14 Mar 2026 18:06:34 +0200
> Erikas Bitovtas <xerikasxx@gmail.com> wrote:
> 
>> The error code is available in the log after return. Remove duplicate
>> error messages to reduce noise in dmesg.
>>
>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
>> ---
>>  drivers/iio/light/vcnl4000.c | 9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
>> index e501db7249d7..c8bb1826b916 100644
>> --- a/drivers/iio/light/vcnl4000.c
>> +++ b/drivers/iio/light/vcnl4000.c
>> @@ -2041,11 +2041,8 @@ static int vcnl4000_probe(struct i2c_client *client)
>>  						      NULL,
>>  						      data->chip_spec->trig_buffer_func,
>>  						      data->chip_spec->buffer_setup_ops);
>> -		if (ret < 0) {
>> -			dev_err(&client->dev,
>> -				"unable to setup iio triggered buffer\n");
> 
> Is this one a duplicate? I don't recall us being particular verbose
> in terms of error messages in iio_triggered_buffer_setup_ext() which is
> where that ends up coming from.  I think there is only one path
> where it can return anything other than -ENOMEM and that one is a duplicate
> registration check (so fair to not print).  So perhaps all this patch
> needs is a comment on what errors can surface from this call and why
> it is therefore not worth printing anything.
> 

I do not see anything being printed on iio_triggered_buffer_setup_ext(),
so I guess this is not a duplicate. The function can return -EADDRINUSE
if a buffer is already assigned, to prevent cleanup function being
called on a buffer that wasn't allocated.
I will add this print back if necessary in v4.

Re: [PATCH v3 5/6] iio: light: vcnl4000: remove error messages for trigger and irq

[Jonathan Cameron]

On March 15, 2026 7:15:51 PM GMT, Erikas Bitovtas <xerikasxx@gmail.com> wrote:
>
>
>On 3/15/26 8:31 PM, Jonathan Cameron wrote:
>> On Sat, 14 Mar 2026 18:06:34 +0200
>> Erikas Bitovtas <xerikasxx@gmail.com> wrote:
>> 
>>> The error code is available in the log after return. Remove duplicate
>>> error messages to reduce noise in dmesg.
>>>
>>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
>>> ---
>>>  drivers/iio/light/vcnl4000.c | 9 ++-------
>>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
>>> index e501db7249d7..c8bb1826b916 100644
>>> --- a/drivers/iio/light/vcnl4000.c
>>> +++ b/drivers/iio/light/vcnl4000.c
>>> @@ -2041,11 +2041,8 @@ static int vcnl4000_probe(struct i2c_client *client)
>>>  						      NULL,
>>>  						      data->chip_spec->trig_buffer_func,
>>>  						      data->chip_spec->buffer_setup_ops);
>>> -		if (ret < 0) {
>>> -			dev_err(&client->dev,
>>> -				"unable to setup iio triggered buffer\n");
>> 
>> Is this one a duplicate? I don't recall us being particular verbose
>> in terms of error messages in iio_triggered_buffer_setup_ext() which is
>> where that ends up coming from.  I think there is only one path
>> where it can return anything other than -ENOMEM and that one is a duplicate
>> registration check (so fair to not print).  So perhaps all this patch
>> needs is a comment on what errors can surface from this call and why
>> it is therefore not worth printing anything.
>> 
>
>I do not see anything being printed on iio_triggered_buffer_setup_ext(),
>so I guess this is not a duplicate. The function can return -EADDRINUSE
>if a buffer is already assigned, to prevent cleanup function being
>called on a buffer that wasn't allocated.
>I will add this print back if necessary in v4.
>
I think it is fine to drop the print but more detail on why is needed for the commit message.

Re: [PATCH v3 3/6] iio: light: vcnl4000: replace mutex_init with devm_mutex_init

[Andy Shevchenko]

On Sat, Mar 14, 2026 at 06:06:32PM +0200, Erikas Bitovtas wrote:
> Replace mutex_init used across driver with its device-managed
> counterpart, so all assigned mutexes get destroyed.

...

> +	ret = devm_mutex_init(dev, &data->vcnl4200_al.lock);
> +	if (ret < 0)

Do we need ' < 0' parts?

> +		return ret;
> +
> +	ret = devm_mutex_init(dev, &data->vcnl4200_ps.lock);
> +	if (ret < 0)
> +		return ret;

...

> +	ret = devm_mutex_init(dev, &data->vcnl4000_lock);
> +	if (ret < 0)
> +		return ret;

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/6] iio: light: vcnl4000: add support for regulators

[Andy Shevchenko]

On Sat, Mar 14, 2026 at 06:06:33PM +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.

...

> +	ret = devm_regulator_bulk_get_enable(dev,

Below is broken indentation.

> +				      ARRAY_SIZE(regulator_names),

There is still some room on the previous line.

> +				      regulator_names);

	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names),
					     regulator_names);

OTOH the logical split is rather to have the last two on a single line, but
that one becomes too long, so your initial variant is okay with fixed
indentation.

	ret = devm_regulator_bulk_get_enable(dev,
					     ARRAY_SIZE(regulator_names),
					     regulator_names);


> +	if (ret < 0)

Do we need ' < 0' part?

> +		return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 6/6] iio: light: vcnl4000: use variables for I2C client and device instances

[Andy Shevchenko]

On Sat, Mar 14, 2026 at 06:06:35PM +0200, Erikas Bitovtas wrote:
> After moving data->client and client->dev into variables of their own,
> replace all instances of data->client and client->dev being used in
> vcnl4200_init and vcnl4000_probe by the said variables to reduce
> clutter.

...

> -	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
> +	ret = i2c_smbus_write_word_data(client, VCNL4200_PS_CONF1,
>  					regval);

Now it's perfectly a single line.

	ret = i2c_smbus_write_word_data(client, VCNL4200_PS_CONF1, regval);

>  	if (ret < 0)
>  		return ret;

...

>  	regval = ret | VCNL4040_CONF3_PS_SAMPLE_16BITS;
> -	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
> +	ret = i2c_smbus_write_word_data(client, VCNL4200_PS_CONF3,
>  					regval);

Ditto.

>  	if (ret < 0)
>  		return ret;

> -	if (device_property_read_u32(&client->dev, "proximity-near-level",
> +	if (device_property_read_u32(dev, "proximity-near-level",
>  				     &data->near_level))
>  		data->near_level = 0;

The 'if' is redundant, I think you can drop it at some point, probably to avoid
churn in the follow up

 -	if (device_property_read_u32(&client->dev, "proximity-near-level",
 -		data->near_level = 0;
 +	device_property_read_u32(dev, "proximity-near-level", &data->near_level);

Assuming data is allocated with kzalloc() or equivalent.

...

> -		ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
> +		ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
>  						      NULL,

Now NULL can be moved to upper line.

>  						      data->chip_spec->trig_buffer_func,
>  						      data->chip_spec->buffer_setup_ops);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 6/6] iio: light: vcnl4000: use variables for I2C client and device instances

[Erikas Bitovtas]

On 3/16/26 12:39 PM, Andy Shevchenko wrote:
> On Sat, Mar 14, 2026 at 06:06:35PM +0200, Erikas Bitovtas wrote:
>> After moving data->client and client->dev into variables of their own,
>> replace all instances of data->client and client->dev being used in
>> vcnl4200_init and vcnl4000_probe by the said variables to reduce
>> clutter.
> 
> ...
> 
>> -	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
>> +	ret = i2c_smbus_write_word_data(client, VCNL4200_PS_CONF1,
>>  					regval);
> 
> Now it's perfectly a single line.
> 
> 	ret = i2c_smbus_write_word_data(client, VCNL4200_PS_CONF1, regval);
> 
>>  	if (ret < 0)
>>  		return ret;
> 
> ...
> 
>>  	regval = ret | VCNL4040_CONF3_PS_SAMPLE_16BITS;
>> -	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF3,
>> +	ret = i2c_smbus_write_word_data(client, VCNL4200_PS_CONF3,
>>  					regval);
> 
> Ditto.
> 
>>  	if (ret < 0)
>>  		return ret;
> 
>> -	if (device_property_read_u32(&client->dev, "proximity-near-level",
>> +	if (device_property_read_u32(dev, "proximity-near-level",
>>  				     &data->near_level))
>>  		data->near_level = 0;
> 
> The 'if' is redundant, I think you can drop it at some point, probably to avoid
> churn in the follow up
> 
>  -	if (device_property_read_u32(&client->dev, "proximity-near-level",
>  -		data->near_level = 0;
>  +	device_property_read_u32(dev, "proximity-near-level", &data->near_level);
> 

device_property_read_u32() throws an error if a property is missing.
Would data->near_level be left without an assigned default value in that
case?

Re: [PATCH v3 6/6] iio: light: vcnl4000: use variables for I2C client and device instances

[Andy Shevchenko]

On Mon, Mar 16, 2026 at 12:50:48PM +0200, Erikas Bitovtas wrote:
> On 3/16/26 12:39 PM, Andy Shevchenko wrote:

First of all, remove the content you are not replying to!

...

> >> -	if (device_property_read_u32(&client->dev, "proximity-near-level",
> >> +	if (device_property_read_u32(dev, "proximity-near-level",
> >>  				     &data->near_level))
> >>  		data->near_level = 0;
> > 
> > The 'if' is redundant, I think you can drop it at some point, probably to avoid
> > churn in the follow up
> > 
> >  -	if (device_property_read_u32(&client->dev, "proximity-near-level",
> >  -		data->near_level = 0;
> >  +	device_property_read_u32(dev, "proximity-near-level", &data->near_level);
> > 
> 
> device_property_read_u32() throws an error if a property is missing.
> Would data->near_level be left without an assigned default value in that
> case?

Second, have you read my reply carefully?

-- 
With Best Regards,
Andy Shevchenko