public inbox for linux-iio@vger.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Ariana.Lazar@microchip.com
Cc: dlechner@baylibre.com, nuno.sa@analog.com,
	linux-iio@vger.kernel.org, Jonathan.Cameron@huawei.com,
	jic23@kernel.org, linux-kernel@vger.kernel.org, andy@kernel.org,
	error27@gmail.com
Subject: Re: [PATCH] iio: dac: Fix passing uninitialized vref1_uV for no Vref1 case
Date: Tue, 14 Apr 2026 19:26:24 +0300	[thread overview]
Message-ID: <ad5qsFP_7l3FATMA@ashevche-desk.local> (raw)
In-Reply-To: <30022c5e780b2312e764c9e25399d67b2eed165c.camel@microchip.com>

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

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2026-04-14 16:26 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-14 12:33 [PATCH] iio: dac: Fix passing uninitialized vref1_uV for no Vref1 case Ariana Lazar
2026-04-14 12:48 ` Andy Shevchenko
2026-04-14 13:21   ` David Lechner
2026-04-14 16:18     ` Andy Shevchenko
2026-04-14 16:19       ` Andy Shevchenko
2026-04-14 14:26   ` Ariana.Lazar
2026-04-14 16:26     ` Andy Shevchenko [this message]
2026-04-20 12:19       ` Jonathan Cameron
2026-04-14 15:32 ` David Lechner
2026-04-14 16:28   ` Andy Shevchenko
2026-04-20 12:11 ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ad5qsFP_7l3FATMA@ashevche-desk.local \
    --to=andriy.shevchenko@intel.com \
    --cc=Ariana.Lazar@microchip.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=error27@gmail.com \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox