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

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 14 Apr 2026 19:26:24 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Apr 14, 2026 at 02:26:45PM +0000, Ariana.Lazar@microchip.com wrote:
> > On Tue, 2026-04-14 at 15:48 +0300, Andy Shevchenko wrote:  
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > know the content is safe  
> 
> You should get rid of this message. It's incompatible with OSS development
> process.
> 
> > > On Tue, Apr 14, 2026 at 03:33:38PM +0300, Ariana Lazar wrote:  
> 
> ...
> 
> > > > +     vref1_uV = 0;
> > > >       if (chip_features->have_ext_vref1) {  
> > > 
> > > I'm wondering what will happen if we do the below unconditionally?
> > >   
> > > >               ret = devm_regulator_get_enable_read_voltage(dev,
> > > > "vref1");  
> > > 
> > > If we have no regulator, we get a dummy one, right? What is the
> > > voltage will
> > > be? 0?
> > >   
> > > >               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");
> > > >               }  
> > 
> > Thank you for the review.
> > 
> > This is a safety check to ensure the devicetree matches the available
> > hardware. If Vref1 was selected in devicetree but unavailable in
> > hardware, the scales MCP47FEB02_SCALE_GAIN_X1 and
> > MCP47FEB02_SCALE_GAIN_X2 and also voltage readings would be incorrect
> > for the channels that use Vref1.  
> 
> I didn't get how. What I recommend is to do regulator request unconditionally.
> 
> > I did something similiar to what you have suggested in the first patch
> > I have submitted for this driver and checking first was recommended.
> > 
> > https://lore.kernel.org/all/20250927185324.2f9e8061@jic23-huawei/  
> 
> I briefly read that. The check was there, Jonathan just asked to modify
> the check itself IIUC, i.o.w. the semantics of the check was commented
> and not the check presence in the first place.
> 
> Did I get it wrong? Jonathan, can we get rid of the check and ask for
> regulator unconditionally? (Maybe it would be good to print an error
> code in the debug message to be sure why it failed to get the regulator
> or its voltage.)
> 
Whilst it might be harmless I'm not keen to get a regulator at all if it
doesn't exist.  Sure we'll see an error and the code will work (assuming
the non existent regulator isn't in the DT!) but we'll also get a debug
message that is at best misleading.  Vref1 would indeed be unavailable
because the specific device doesn't have a pin to connect it to!

So I think the outer guard still makes sense. 

There is a reasonable question of why anything is using the value that
isn't initialized, and hence why the original bug exists?
I'm thinking it probably doesn't in practice, but that the
relationship between the flag and channel count etc is sufficiently complex
the compiler can't tell that.  So my assumption is this 'bug' is
a false positive.  We still want to tidy it up anyway on basis
it's not unreasonable that we are triggering static analysis warnings
on it!

J