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

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

[Dan Carpenter]

[ Smatch checking is paused while we raise funding.  #SadFace
  https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]

Hello Ariana Lazar,

Commit bf394cc80369 ("iio: dac: adding support for Microchip
MCP47FEB02") from Dec 16, 2025 (linux-next), leads to the following
Smatch static checker warning:

	drivers/iio/dac/mcp47feb02.c:732 mcp47feb02_init_scales_avail()
	warn: passing zero to 'dev_err_probe'

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.  Return -EINVAL.

    733 
    734                 if (data->use_vref1)
    735                         tmp_vref = vref1_mV;
    736                 else
    737                         tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_mV;
    738 
    739                 mcp47feb02_init_scale(data, MCP47FEB02_SCALE_GAIN_X1,
    740                                       tmp_vref, data->scale_1);
    741                 mcp47feb02_init_scale(data, MCP47FEB02_SCALE_GAIN_X2,
    742                                       tmp_vref * 2, data->scale_1);
    743         }
    744 
    745         return 0;
    746 }

regards,
dan carpenter

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

[Andy Shevchenko]

On Fri, Feb 06, 2026 at 04:40:15PM +0300, Dan Carpenter wrote:
> [ Smatch checking is paused while we raise funding.  #SadFace
>   https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]
> 
> Hello Ariana Lazar,
> 
> Commit bf394cc80369 ("iio: dac: adding support for Microchip
> MCP47FEB02") from Dec 16, 2025 (linux-next), leads to the following
> Smatch static checker warning:
> 
> 	drivers/iio/dac/mcp47feb02.c:732 mcp47feb02_init_scales_avail()
> 	warn: passing zero to 'dev_err_probe'

Btw, why the bot mangles the patch, please?
Adding leading information (line number and some other markings) should not
mangle the code (tab-based indentation).

> 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.

> Return -EINVAL.

Probably true for the == 0 case.

With the above, this probably should be

>     734                 if (data->use_vref1)
>     735                         tmp_vref = vref1_mV;
>     736                 else
>     737                         tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_mV;

                 if (data->use_vref1) {
                         if (vref1_mV < 0)
                                  return dev_err_probe(dev, vref1_mV, "Can't get voltage for Vref1\n");
			 if (vref1_mV == 0)
                                  return dev_err_probe(dev, -ERANGE, "Invalid voltage for Vref1\n");
			 // or -EINVAL?
                         tmp_vref = vref1_mV;
		 } else {
                         tmp_vref = MCP47FEB02_INTERNAL_BAND_GAP_mV;
		 }


>     739                 mcp47feb02_init_scale(data, MCP47FEB02_SCALE_GAIN_X1,
>     740                                       tmp_vref, data->scale_1);
>     741                 mcp47feb02_init_scale(data, MCP47FEB02_SCALE_GAIN_X2,
>     742                                       tmp_vref * 2, data->scale_1);
>     743         }
>     744 
>     745         return 0;
>     746 }

-- 
With Best Regards,
Andy Shevchenko

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

[Dan Carpenter]

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.

regards,
dan carpenter

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

[Andy Shevchenko]

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?

-- 
With Best Regards,
Andy Shevchenko

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

[Dan Carpenter]

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

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

[Andy Shevchenko]

On Fri, Feb 6, 2026 at 5:32 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
> 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.

I see, thanks!

So, it means that the validation should be moved here on ret < 0 and
ret < 1000 (if positive).

>   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");

But... ret < 0  is checked here.
Hence the only one left is the range [0..999].

>   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;


-- 
With Best Regards,
Andy Shevchenko

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

[Ariana.Lazar]

On Fri, 2026-02-06 at 17:57 +0200, Andy Shevchenko wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Fri, Feb 6, 2026 at 5:32 PM Dan Carpenter
> <dan.carpenter@linaro.org> wrote:
> > 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.
> 
> I see, thanks!
> 
> So, it means that the validation should be moved here on ret < 0 and
> ret < 1000 (if positive).
> 
> >   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");
> 
> But... ret < 0  is checked here.
> Hence the only one left is the range [0..999].
> 
> >   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;
> 
> 
> --
> With Best Regards,
> Andy Shevchenko


Hello Dan and Andy,

Thank you for bringing to my attention this bug. I fixed it by storing
voltages
in microvolts instead of millivolts in order to avoid the [1, 999]
case.
I removed dividing by MILLI from the probe function and kept the
computation of
the scale values only in init_scale function.

I will send a follow on patch.

Best regards,
Ariana

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

[Jonathan Cameron]

On Tue, 10 Feb 2026 10:26:05 +0000
<Ariana.Lazar@microchip.com> wrote:

> On Fri, 2026-02-06 at 17:57 +0200, Andy Shevchenko wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > On Fri, Feb 6, 2026 at 5:32 PM Dan Carpenter
> > <dan.carpenter@linaro.org> wrote:  
> > > 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.  
> > 
> > I see, thanks!
> > 
> > So, it means that the validation should be moved here on ret < 0 and
> > ret < 1000 (if positive).
> >   
> > >   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");  
> > 
> > But... ret < 0  is checked here.
> > Hence the only one left is the range [0..999].
> >   
> > >   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;  
> > 
> > 
> > --
> > With Best Regards,
> > Andy Shevchenko  
> 
> 
> Hello Dan and Andy,
> 
> Thank you for bringing to my attention this bug. I fixed it by storing
> voltages
> in microvolts instead of millivolts in order to avoid the [1, 999]
> case.
> I removed dividing by MILLI from the probe function and kept the
> computation of
> the scale values only in init_scale function.
> 
> I will send a follow on patch.

Hi Ariana,

Just a reminder that this one still seems to be outstanding.
Maybe I missed a patch?

Thanks,

Jonathan

> 
> Best regards,
> Ariana
> 

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

[Ariana.Lazar]

> Just a reminder that this one still seems to be outstanding.
> Maybe I missed a patch?
> 
> Thanks,
> 
> Jonathan
> 
> > 
> > Best regards,
> > Ariana
> > 
> 

Hi Jonathan,

Given the latest reviews, I was wondering how do you prefer the next
patch to be sent. At the moment I am working on the version with three
modules in order to include both protocol families. If you prefer, I
will firstly send a patch to fix these bugs for MCP47FEB02 and then I
will send another one with the combined implementation.

Best regards,
Ariana

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

[Jonathan Cameron]

On Mon, 2 Mar 2026 10:28:04 +0000
<Ariana.Lazar@microchip.com> wrote:

> > Just a reminder that this one still seems to be outstanding.
> > Maybe I missed a patch?
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> > > 
> > > Best regards,
> > > Ariana
> > >   
> >   
> 
> Hi Jonathan,
> 
> Given the latest reviews, I was wondering how do you prefer the next
> patch to be sent. At the moment I am working on the version with three
> modules in order to include both protocol families. If you prefer, I
> will firstly send a patch to fix these bugs for MCP47FEB02 and then I
> will send another one with the combined implementation.

That last option sounds like the right approach. The fix will need to go upstream
first, then once that's available in upstream I can merge into the togreg
branch and apply new stuff on top of it.

Thanks,

Jonathan

> 
> Best regards,
> Ariana

[bug report] iio: adc: Add support for ad4062

[Dan Carpenter]

[ Smatch checking is paused while we raise funding.  #SadFace
  https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]

Hello Jorge Marques,

Commit d5284402d28f ("iio: adc: Add support for ad4062") from Dec 17,
2025 (linux-next), leads to the following Smatch static checker
warning:

	drivers/iio/adc/ad4062.c:1557 ad4062_probe()
	warn: passing positive error code 's32min-(-1),1-3' to 'dev_err_probe'

drivers/iio/adc/ad4062.c
    1547         pm_runtime_set_active(dev);
    1548         ret = devm_pm_runtime_enable(dev);
    1549         if (ret)
    1550                 return dev_err_probe(dev, ret, "Failed to enable pm_runtime\n");
    1551 
    1552         pm_runtime_set_autosuspend_delay(dev, 1000);
    1553         pm_runtime_use_autosuspend(dev);
    1554 
    1555         ret = ad4062_request_ibi(i3cdev);
    1556         if (ret)
--> 1557                 return dev_err_probe(dev, ret, "Failed to request i3c ibi\n");

The comments for ad4062_request_ibi() say it returns negative error codes
but the comments for i3c_master_enec_locked() say it returns "a positive
I3C error code if the error is one of the official Mx error codes, and
a negative error code otherwise."

    1558 
    1559         ret = ad4062_gpio_init(st);
    1560         if (ret)
    1561                 return ret;
    1562 
    1563         ret = devm_work_autocancel(dev, &st->trig_conv, ad4062_trigger_work);
    1564         if (ret)
    1565                 return ret;
    1566 
    1567         return devm_iio_device_register(dev, indio_dev);
    1568 }

regards,
dan carpenter

Re: [bug report] iio: adc: Add support for ad4062

[Andy Shevchenko]

On Fri, Feb 06, 2026 at 04:40:31PM +0300, Dan Carpenter wrote:
> [ Smatch checking is paused while we raise funding.  #SadFace
>   https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]

Oh, this is indeed sad. Wondering if LF can donate...

> Commit d5284402d28f ("iio: adc: Add support for ad4062") from Dec 17,
> 2025 (linux-next), leads to the following Smatch static checker
> warning:
> 
> 	drivers/iio/adc/ad4062.c:1557 ad4062_probe()
> 	warn: passing positive error code 's32min-(-1),1-3' to 'dev_err_probe'

>     1555         ret = ad4062_request_ibi(i3cdev);
>     1556         if (ret)

	if (ret < 0)

resolves immediate isssue, but...

> --> 1557                 return dev_err_probe(dev, ret, "Failed to request i3c ibi\n");
> 
> The comments for ad4062_request_ibi() say it returns negative error codes
> but the comments for i3c_master_enec_locked() say it returns "a positive
> I3C error code if the error is one of the official Mx error codes, and
> a negative error code otherwise."

...would be nice to have a conversion helper to get Linux error codes
from the Mx ones.

-- 
With Best Regards,
Andy Shevchenko

Re: [bug report] iio: adc: Add support for ad4062

[Jonathan Cameron]

On Fri, 6 Feb 2026 16:07:36 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Fri, Feb 06, 2026 at 04:40:31PM +0300, Dan Carpenter wrote:
> > [ Smatch checking is paused while we raise funding.  #SadFace
> >   https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]  
> 
> Oh, this is indeed sad. Wondering if LF can donate...
> 
> > Commit d5284402d28f ("iio: adc: Add support for ad4062") from Dec 17,
> > 2025 (linux-next), leads to the following Smatch static checker
> > warning:
> > 
> > 	drivers/iio/adc/ad4062.c:1557 ad4062_probe()
> > 	warn: passing positive error code 's32min-(-1),1-3' to 'dev_err_probe'  
> 
> >     1555         ret = ad4062_request_ibi(i3cdev);
> >     1556         if (ret)  
> 
> 	if (ret < 0)
> 
> resolves immediate isssue, but...
> 
> > --> 1557                 return dev_err_probe(dev, ret, "Failed to request i3c ibi\n");  
> > 
> > The comments for ad4062_request_ibi() say it returns negative error codes
> > but the comments for i3c_master_enec_locked() say it returns "a positive
> > I3C error code if the error is one of the official Mx error codes, and
> > a negative error code otherwise."  
> 
> ...would be nice to have a conversion helper to get Linux error codes
> from the Mx ones.
> 

@Jorge,  can you take a look at this please and ideally send a fix.

Thanks,

Jonathan

Re: [bug report] iio: adc: Add support for ad4062

[Jorge Marques]

On Sun, Mar 01, 2026 at 12:34:29PM +0000, Jonathan Cameron wrote:
> On Fri, 6 Feb 2026 16:07:36 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> 
> > On Fri, Feb 06, 2026 at 04:40:31PM +0300, Dan Carpenter wrote:
> > > [ Smatch checking is paused while we raise funding.  #SadFace
> > >   https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]  
> > 
> > Oh, this is indeed sad. Wondering if LF can donate...
> > 
> > > Commit d5284402d28f ("iio: adc: Add support for ad4062") from Dec 17,
> > > 2025 (linux-next), leads to the following Smatch static checker
> > > warning:
> > > 
> > > 	drivers/iio/adc/ad4062.c:1557 ad4062_probe()
> > > 	warn: passing positive error code 's32min-(-1),1-3' to 'dev_err_probe'  
> > 
> > >     1555         ret = ad4062_request_ibi(i3cdev);
> > >     1556         if (ret)  
> > 
> > 	if (ret < 0)
> > 
> > resolves immediate isssue, but...
> > 
> > > --> 1557                 return dev_err_probe(dev, ret, "Failed to request i3c ibi\n");  
> > > 
> > > The comments for ad4062_request_ibi() say it returns negative error codes
> > > but the comments for i3c_master_enec_locked() say it returns "a positive
> > > I3C error code if the error is one of the official Mx error codes, and
> > > a negative error code otherwise."  
> > 
> > ...would be nice to have a conversion helper to get Linux error codes
> > from the Mx ones.
> > 
> 
> @Jorge,  can you take a look at this please and ideally send a fix.

Hi Andy, Jonathan,

I found nine paths that returned positive Mx error codes when 0 or
negative was expected.

i3c/device.h suggests returning -EIO. The error codes range from M0 to
M2; only M2 is suppressed during [RST|ENT]DAA (means "no active device
on the bus"), which is already done internally but duplicated.

I will submit the changes to the i3c subsystem after CI/CD and hardware
tests are run to make sure, since many paths are involved.

Note that the immediate fix is not valid, if a Mx code was returned as
is, we would like to return an error still, since that would mean the
target device did not acknowledge the enable target events (instead we
would convert to -EIO).

Thanks,
Jorge
> 
> Thanks,
> 
> Jonathan