[PATCH v2] iio: dac: mcp47feb02: Fix passing uninitialized vref1_uV for no Vref1 case

[PATCH v2] iio: dac: mcp47feb02: Fix passing uninitialized vref1_uV for no Vref1 case

[Ariana Lazar]

Ensure that if a device has Vref1 but reading the regulator returns an
error, mcp47feb02_init_ctrl_regs() is not called with an uninitialized
vref1_uV value.

Also add a device_property_present() check for the Vref1 supply before
reading the regulator.

Fixes: dd154646d292 ("iio: dac: mcp47feb02: Fix Vref validation [1-999] case")
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/all/adiPnla0M5EzvgD-@stanley.mountain/
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
Changes in v2:
- return the reading regulator error to not use vref1_uV uninitialized in
   mcp47feb02_init_ctrl_regs() call 
- add device_property_present() check for the Vref1
- Link to v1: https://lore.kernel.org/r/20260414-mcp47feb02-fix4-v1-1-9d71badfd25e@microchip.com
---
 drivers/iio/dac/mcp47feb02.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c
index faccb804a5ed548088aaf83266b16ed45a92916c..a876ed71ecdf101784e0f98668a1681a1cd19ed0 100644
--- a/drivers/iio/dac/mcp47feb02.c
+++ b/drivers/iio/dac/mcp47feb02.c
@@ -1095,9 +1095,10 @@ static int mcp47feb02_probe(struct i2c_client *client)
 {
 	const struct mcp47feb02_features *chip_features;
 	struct device *dev = &client->dev;
+	int vdd_uV, vref_uV, vref1_uV;
 	struct mcp47feb02_data *data;
 	struct iio_dev *indio_dev;
-	int vref1_uV, vref_uV, vdd_uV, ret;
+	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
@@ -1146,16 +1147,16 @@ static int mcp47feb02_probe(struct i2c_client *client)
 		dev_dbg(dev, "Vref is unavailable.\n");
 	}
 
-	if (chip_features->have_ext_vref1) {
+	if (chip_features->have_ext_vref1 && device_property_present(dev, "vref1")) {
 		ret = devm_regulator_get_enable_read_voltage(dev, "vref1");
-		if (ret > 0) {
-			vref1_uV = ret;
-			data->use_vref1 = true;
-		} else {
-			vref1_uV = 0;
-			dev_dbg(dev, "using internal band gap as voltage reference 1.\n");
-			dev_dbg(dev, "Vref1 is unavailable.\n");
-		}
+		if (ret < 0)
+			return ret;
+
+		vref1_uV = ret;
+		data->use_vref1 = true;
+	} else {
+		vref1_uV = 0;
+		dev_dbg(dev, "using internal band gap as voltage reference 1.\n");
 	}
 
 	ret = mcp47feb02_init_ctrl_regs(data);

---
base-commit: 51e7665ab81f02adc80a1219c260ee678e9c6eb8
change-id: 20260414-mcp47feb02-fix4-614de9334f22

Best regards,
-- 
Ariana Lazar <ariana.lazar@microchip.com>

Re: [PATCH v2] iio: dac: mcp47feb02: Fix passing uninitialized vref1_uV for no Vref1 case

[Jonathan Cameron]

On Mon, 20 Apr 2026 11:04:12 +0300
Ariana Lazar <ariana.lazar@microchip.com> wrote:

> Ensure that if a device has Vref1 but reading the regulator returns an
> error, mcp47feb02_init_ctrl_regs() is not called with an uninitialized
> vref1_uV value.
Just to make it easier for folk to tell if this is critical or not:
Is it a real bug?  See my reply (just now) to v1.

Otherwise, same thing I just raised on v1.  Forgot to check if a new
version had come in today.

> 
> Also add a device_property_present() check for the Vref1 supply before
> reading the regulator.
> 
> Fixes: dd154646d292 ("iio: dac: mcp47feb02: Fix Vref validation [1-999] case")
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/all/adiPnla0M5EzvgD-@stanley.mountain/
> Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
> ---
> Changes in v2:
> - return the reading regulator error to not use vref1_uV uninitialized in
>    mcp47feb02_init_ctrl_regs() call 
> - add device_property_present() check for the Vref1
> - Link to v1: https://lore.kernel.org/r/20260414-mcp47feb02-fix4-v1-1-9d71badfd25e@microchip.com
> ---
>  drivers/iio/dac/mcp47feb02.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c
> index faccb804a5ed548088aaf83266b16ed45a92916c..a876ed71ecdf101784e0f98668a1681a1cd19ed0 100644
> --- a/drivers/iio/dac/mcp47feb02.c
> +++ b/drivers/iio/dac/mcp47feb02.c
> @@ -1095,9 +1095,10 @@ static int mcp47feb02_probe(struct i2c_client *client)
>  {
>  	const struct mcp47feb02_features *chip_features;
>  	struct device *dev = &client->dev;
> +	int vdd_uV, vref_uV, vref1_uV;
>  	struct mcp47feb02_data *data;
>  	struct iio_dev *indio_dev;
> -	int vref1_uV, vref_uV, vdd_uV, ret;
Solution is fine but this change seems to be unrelated noise.

> +	int ret;
>  
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>  	if (!indio_dev)
> @@ -1146,16 +1147,16 @@ static int mcp47feb02_probe(struct i2c_client *client)
>  		dev_dbg(dev, "Vref is unavailable.\n");
>  	}
>  
> -	if (chip_features->have_ext_vref1) {
> +	if (chip_features->have_ext_vref1 && device_property_present(dev, "vref1")) {
>  		ret = devm_regulator_get_enable_read_voltage(dev, "vref1");
> -		if (ret > 0) {
> -			vref1_uV = ret;
> -			data->use_vref1 = true;
> -		} else {
> -			vref1_uV = 0;
> -			dev_dbg(dev, "using internal band gap as voltage reference 1.\n");
> -			dev_dbg(dev, "Vref1 is unavailable.\n");
> -		}
> +		if (ret < 0)
> +			return ret;
> +
> +		vref1_uV = ret;
> +		data->use_vref1 = true;
> +	} else {
> +		vref1_uV = 0;
> +		dev_dbg(dev, "using internal band gap as voltage reference 1.\n");
>  	}
>  
>  	ret = mcp47feb02_init_ctrl_regs(data);
> 
> ---
> base-commit: 51e7665ab81f02adc80a1219c260ee678e9c6eb8
> change-id: 20260414-mcp47feb02-fix4-614de9334f22
> 
> Best regards,