Store reference voltages in uV instead of mV to avoid invalid error code
in dev_err_probe() call. Vref variables store the actual value returned by
devm_regulator_get_enable_read_voltage() function instead of the results of
dividing it by MILLI. The corner case [1-999] divided by MILLI of the
voltage reference variable value would become 0 is covered too.

Fixes: bf394cc80369 ("iio: dac: adding support for Microchip MCP47FEB02")
Link: https://lore.kernel.org/all/aYXvP5FLA5BvkoVX@stanley.mountain/
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
Changes in v3:
- move assigments on else branch
- Link to v2: https://lore.kernel.org/r/20260309-mcp47feb02-fix1-v2-1-6a5ef30e4b27@microchip.com

Changes in v2:
- add () to functions mentioned in the commit message
- split vref_uV, vref1_uV assigments
- Link to v1: https://lore.kernel.org/r/20260305-mcp47feb02-fix1-v1-1-64b613260e09@microchip.com
---
 drivers/iio/dac/mcp47feb02.c | 49 ++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/iio/dac/mcp47feb02.c b/drivers/iio/dac/mcp47feb02.c
index b218f0c3a0bd79f43dc9f2da1027f49a04e1975b..e731acb616f58b118335b20659fcda1f4e5ec357 100644
--- a/drivers/iio/dac/mcp47feb02.c
+++ b/drivers/iio/dac/mcp47feb02.c
@@ -65,7 +65,7 @@
 #define MCP47FEB02_MAX_SCALES_CH			3
 #define MCP47FEB02_DAC_WIPER_UNLOCKED			0
 #define MCP47FEB02_NORMAL_OPERATION			0
-#define MCP47FEB02_INTERNAL_BAND_GAP_mV			2440
+#define MCP47FEB02_INTERNAL_BAND_GAP_uV			2440000
 #define NV_DAC_ADDR_OFFSET				0x10
 
 enum mcp47feb02_vref_mode {
@@ -697,44 +697,40 @@ static const struct iio_chan_spec mcp47febxx_ch_template = {
 };
 
 static void mcp47feb02_init_scale(struct mcp47feb02_data *data, enum mcp47feb02_scale scale,
-				  int vref_mV, int scale_avail[])
+				  int vref_uV, int scale_avail[])
 {
 	u32 value_micro, value_int;
 	u64 tmp;
 
-	/* vref_mV should not be negative */
-	tmp = (u64)vref_mV * MICRO >> data->chip_features->resolution;
+	/* vref_uV should not be negative */
+	tmp = (u64)vref_uV * MILLI >> data->chip_features->resolution;
 	value_int = div_u64_rem(tmp, MICRO, &value_micro);
 	scale_avail[scale * 2] = value_int;
 	scale_avail[scale * 2 + 1] = value_micro;
 }
 
-static int mcp47feb02_init_scales_avail(struct mcp47feb02_data *data, int vdd_mV,
-					int vref_mV, int vref1_mV)
+static int mcp47feb02_init_scales_avail(struct mcp47feb02_data *data, int vdd_uV,
+					int vref_uV, int vref1_uV)
 {
-	struct device *dev = regmap_get_device(data->regmap);
 	int tmp_vref;
 
-	mcp47feb02_init_scale(data, MCP47FEB02_SCALE_VDD, vdd_mV, data->scale);
+	mcp47feb02_init_scale(data, MCP47FEB02_SCALE_VDD, vdd_uV, data->scale);
 
 	if (data->use_vref)
-		tmp_vref = vref_mV;
+		tmp_vref = vref_uV;
 	else
-		tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_mV;
+		tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_uV;
 
 	mcp47feb02_init_scale(data, MCP47FEB02_SCALE_GAIN_X1, tmp_vref, data->scale);
 	mcp47feb02_init_scale(data, MCP47FEB02_SCALE_GAIN_X2, tmp_vref * 2, data->scale);
 
 	if (data->phys_channels >= 4) {
-		mcp47feb02_init_scale(data, MCP47FEB02_SCALE_VDD, vdd_mV, data->scale_1);
-
-		if (data->use_vref1 && vref1_mV <= 0)
-			return dev_err_probe(dev, vref1_mV, "Invalid voltage for Vref1\n");
+		mcp47feb02_init_scale(data, MCP47FEB02_SCALE_VDD, vdd_uV, data->scale_1);
 
 		if (data->use_vref1)
-			tmp_vref = vref1_mV;
+			tmp_vref = vref1_uV;
 		else
-			tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_mV;
+			tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_uV;
 
 		mcp47feb02_init_scale(data, MCP47FEB02_SCALE_GAIN_X1,
 				      tmp_vref, data->scale_1);
@@ -1080,8 +1076,8 @@ static int mcp47feb02_init_ctrl_regs(struct mcp47feb02_data *data)
 	return 0;
 }
 
-static int mcp47feb02_init_ch_scales(struct mcp47feb02_data *data, int vdd_mV,
-				     int vref_mV, int vref1_mV)
+static int mcp47feb02_init_ch_scales(struct mcp47feb02_data *data, int vdd_uV,
+				     int vref_uV, int vref1_uV)
 {
 	unsigned int i;
 
@@ -1089,7 +1085,7 @@ static int mcp47feb02_init_ch_scales(struct mcp47feb02_data *data, int vdd_mV,
 		struct device *dev = regmap_get_device(data->regmap);
 		int ret;
 
-		ret = mcp47feb02_init_scales_avail(data, vdd_mV, vref_mV, vref1_mV);
+		ret = mcp47feb02_init_scales_avail(data, vdd_uV, vref_uV, vref1_uV);
 		if (ret)
 			return dev_err_probe(dev, ret, "failed to init scales for ch %u\n", i);
 	}
@@ -1103,10 +1099,7 @@ static int mcp47feb02_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	struct mcp47feb02_data *data;
 	struct iio_dev *indio_dev;
-	int vref1_mV = 0;
-	int vref_mV = 0;
-	int vdd_mV;
-	int ret;
+	int vref1_uV, vref_uV, vdd_uV, ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
@@ -1143,13 +1136,14 @@ static int mcp47feb02_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
-	vdd_mV = ret / MILLI;
+	vdd_uV = ret;
 
 	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
 	if (ret > 0) {
-		vref_mV = ret / MILLI;
+		vref_uV = ret;
 		data->use_vref = true;
 	} else {
+		vref_uV = 0;
 		dev_dbg(dev, "using internal band gap as voltage reference.\n");
 		dev_dbg(dev, "Vref is unavailable.\n");
 	}
@@ -1157,9 +1151,10 @@ static int mcp47feb02_probe(struct i2c_client *client)
 	if (chip_features->have_ext_vref1) {
 		ret = devm_regulator_get_enable_read_voltage(dev, "vref1");
 		if (ret > 0) {
-			vref1_mV = ret / MILLI;
+			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");
 		}
@@ -1169,7 +1164,7 @@ static int mcp47feb02_probe(struct i2c_client *client)
 	if (ret)
 		return dev_err_probe(dev, ret, "Error initialising vref register\n");
 
-	ret = mcp47feb02_init_ch_scales(data, vdd_mV, vref_mV, vref1_mV);
+	ret = mcp47feb02_init_ch_scales(data, vdd_uV, vref_uV, vref1_uV);
 	if (ret)
 		return ret;
 

---
base-commit: bfd61205aca379121b1913885d4d06c51857119b
change-id: 20260304-mcp47feb02-fix1-20ddf9c71b66

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

On Tue, Mar 10, 2026 at 01:56:44PM +0200, Ariana Lazar wrote:
> Store reference voltages in uV instead of mV to avoid invalid error code
> in dev_err_probe() call. Vref variables store the actual value returned by
> devm_regulator_get_enable_read_voltage() function instead of the results of
> dividing it by MILLI. The corner case [1-999] divided by MILLI of the
> voltage reference variable value would become 0 is covered too.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

...

>  	struct device *dev = &client->dev;
>  	struct mcp47feb02_data *data;
>  	struct iio_dev *indio_dev;
> -	int vref1_mV = 0;
> -	int vref_mV = 0;
> -	int vdd_mV;
> -	int ret;
> +	int vref1_uV, vref_uV, vdd_uV, ret;

Hmm... Semantically ret != vrefXX_uV and putting on one line is questionable.
But I leave it to Jonathan and others.

...

>  	if (ret > 0) {
> -		vref_mV = ret / MILLI;
> +		vref_uV = ret;
>  		data->use_vref = true;
>  	} else {
> +		vref_uV = 0;
>  		dev_dbg(dev, "using internal band gap as voltage reference.\n");
>  		dev_dbg(dev, "Vref is unavailable.\n");
>  	}

Now a question here, why do we need use_vrefXX if we may deduct it
from vrefXX_uV?  Perhaps it deserves a separate change.


-- 
With Best Regards,
Andy Shevchenko

On Tue, 10 Mar 2026 14:21:16 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Mar 10, 2026 at 01:56:44PM +0200, Ariana Lazar wrote:
> > Store reference voltages in uV instead of mV to avoid invalid error code
> > in dev_err_probe() call. Vref variables store the actual value returned by
> > devm_regulator_get_enable_read_voltage() function instead of the results of
> > dividing it by MILLI. The corner case [1-999] divided by MILLI of the
> > voltage reference variable value would become 0 is covered too.  
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 
> ...
> 
> >  	struct device *dev = &client->dev;
> >  	struct mcp47feb02_data *data;
> >  	struct iio_dev *indio_dev;
> > -	int vref1_mV = 0;
> > -	int vref_mV = 0;
> > -	int vdd_mV;
> > -	int ret;
> > +	int vref1_uV, vref_uV, vdd_uV, ret;  
> 
> Hmm... Semantically ret != vrefXX_uV and putting on one line is questionable.
> But I leave it to Jonathan and others.
I would have left it on a separate line, but I don't care enough to
tweak it whilst applying.

So applied as is to the fixes-togreg branch of iio.git.
Thanks,

Jonathan

> 
> ...
> 
> >  	if (ret > 0) {
> > -		vref_mV = ret / MILLI;
> > +		vref_uV = ret;
> >  		data->use_vref = true;
> >  	} else {
> > +		vref_uV = 0;
> >  		dev_dbg(dev, "using internal band gap as voltage reference.\n");
> >  		dev_dbg(dev, "Vref is unavailable.\n");
> >  	}  
> 
> Now a question here, why do we need use_vrefXX if we may deduct it
> from vrefXX_uV?  Perhaps it deserves a separate change.
> 
>