Re: [bug report] iio: dac: adding support for Microchip MCP47FEB02

[Dan Carpenter <dan.carpenter@linaro.org>]

On Fri, Feb 06, 2026 at 05:14:53PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 06, 2026 at 05:33:26PM +0300, Dan Carpenter wrote:
> > On Fri, Feb 06, 2026 at 04:04:07PM +0200, Andy Shevchenko wrote:
> > > > drivers/iio/dac/mcp47feb02.c
> > > >     712 static int mcp47feb02_init_scales_avail(struct mcp47feb02_data *data, int vdd_mV,
> > > >     713                                         int vref_mV, int vref1_mV)
> > > >     714 {
> > > >     715         struct device *dev = regmap_get_device(data->regmap);
> > > >     716         int tmp_vref;
> > > >     717 
> > > >     718         mcp47feb02_init_scale(data, MCP47FEB02_SCALE_VDD, vdd_mV, data->scale);
> > > >     719 
> > > >     720         if (data->use_vref)
> > > >     721                 tmp_vref = vref_mV;
> > > >     722         else
> > > >     723                 tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_mV;
> > > >     724 
> > > >     725         mcp47feb02_init_scale(data, MCP47FEB02_SCALE_GAIN_X1, tmp_vref, data->scale);
> > > >     726         mcp47feb02_init_scale(data, MCP47FEB02_SCALE_GAIN_X2, tmp_vref * 2, data->scale);
> > > >     727 
> > > >     728         if (data->phys_channels >= 4) {
> > > >     729                 mcp47feb02_init_scale(data, MCP47FEB02_SCALE_VDD, vdd_mV, data->scale_1);
> > > >     730 
> > > >     731                 if (data->use_vref1 && vref1_mV <= 0)
> > > > --> 732                         return dev_err_probe(dev, vref1_mV, "Invalid voltage for Vref1\n");
> > > >                                                           ^^^^^^^^
> > > > vref1_mV is not a valid error code.
> > > 
> > > Why not? When it's negative I believe the above statement is not true.
> > 
> > I saw this as just sanity checking the input.  vref1_mV is never
> > actually negative.  I don't know if devm_regulator_get_enable_read_voltage()
> > can return less than one millivolt.
> 
>  * In cases where the supply is not strictly required, callers can check for
>  * -ENODEV error and handle it accordingly.
>  *
>  * Returns: voltage in microvolts on success, or an negative error number on failure.
> 
> What did I miss?
> 

drivers/iio/dac/mcp47feb02.c
  1157          if (chip_features->have_ext_vref1) {
  1158                  ret = devm_regulator_get_enable_read_voltage(dev, "vref1");
  1159                  if (ret > 0) {
  1160                          vref1_mV = ret / MILLI;

Potentially, if ret is in the 1-999 range then vref1_mV could be zero,
but it can't be negative.

  1161                          data->use_vref1 = true;
  1162                  } else {
  1163                          dev_dbg(dev, "using internal band gap as voltage reference 1.\n");
  1164                          dev_dbg(dev, "Vref1 is unavailable.\n");
  1165                  }
  1166          }
  1167  
  1168          ret = mcp47feb02_init_ctrl_regs(data);
  1169          if (ret)
  1170                  return dev_err_probe(dev, ret, "Error initialising vref register\n");
  1171  
  1172          ret = mcp47feb02_init_ch_scales(data, vdd_mV, vref_mV, vref1_mV);
                                                                       ^^^^^^^^

  1173          if (ret)
  1174                  return ret;

regards,
dan carpenter