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 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..5615b4f50e6527023da2f492f12268f64e59fab9 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,11 +1136,12 @@ static int mcp47feb02_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
-	vdd_mV = ret / MILLI;
+	vdd_uV = ret;
 
+	vref_uV = 0;
 	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
 	if (ret > 0) {
-		vref_mV = ret / MILLI;
+		vref_uV = ret;
 		data->use_vref = true;
 	} else {
 		dev_dbg(dev, "using internal band gap as voltage reference.\n");
@@ -1155,9 +1149,10 @@ static int mcp47feb02_probe(struct i2c_client *client)
 	}
 
 	if (chip_features->have_ext_vref1) {
+		vref1_uV = 0;
 		ret = devm_regulator_get_enable_read_voltage(dev, "vref1");
 		if (ret > 0) {
-			vref1_mV = ret / MILLI;
+			vref1_uV = ret;
 			data->use_vref1 = true;
 		} else {
 			dev_dbg(dev, "using internal band gap as voltage reference 1.\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 Mon, Mar 09, 2026 at 02:12:23PM +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.

...

> +	vref_uV = 0;
>  	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
>  	if (ret > 0) {
> +		vref_uV = ret;
>  		data->use_vref = true;
>  	} else {
>  		dev_dbg(dev, "using internal band gap as voltage reference.\n");

>  	}

Thanks, but in some cases this will be rewritten. Why not put the assignment
into the 'else' branch?

...


> +		vref1_uV = 0;

Ditto.

>  		ret = devm_regulator_get_enable_read_voltage(dev, "vref1");
>  		if (ret > 0) {
> -			vref1_mV = ret / MILLI;
> +			vref1_uV = ret;
>  			data->use_vref1 = true;
>  		} else {
>  			dev_dbg(dev, "using internal band gap as voltage reference 1.\n");

-- 
With Best Regards,
Andy Shevchenko