Re: [PATCH v5 3/5] iio: adc: ad7949: add support for internal vref

From: "Liam Beguin" <liambeguin@gmail.com>
To: "Andy Shevchenko" <andy.shevchenko@gmail.com>
Cc: "Jonathan Cameron" <jic23@kernel.org>, <lars@metafoo.de>,
	<Michael.Hennerich@analog.com>,
	<charles-antoine.couret@essensium.com>, <Nuno.Sa@analog.com>,
	<linux-kernel@vger.kernel.org>, <linux-iio@vger.kernel.org>,
	<devicetree@vger.kernel.org>, <robh+dt@kernel.org>
Subject: Re: [PATCH v5 3/5] iio: adc: ad7949: add support for internal vref
Date: Tue, 10 Aug 2021 15:46:16 -0400	[thread overview]
Message-ID: <CDG3ATRV00VV.57ZPL3OPU5N4@shaak> (raw)
In-Reply-To: <CAHp75Vc3o-RrHD_wt_CfJY3P8hoRhJkdiRRySFS_O_7VdtJTQA@mail.gmail.com>

On Tue Aug 10, 2021 at 8:15 AM EDT, Andy Shevchenko wrote:
> On Mon, Aug 9, 2021 at 1:50 AM Liam Beguin <liambeguin@gmail.com> wrote:
> > On Sun Aug 8, 2021 at 12:36 PM EDT, Jonathan Cameron wrote:
> > > On Sat, 7 Aug 2021 21:56:57 -0400
> >         ret = fwnode_property_read_u32(child, "adi,internal-ref-microvolt", &tmp);
> >         if (ret == -EINVAL && mode & AD7949_CFG_VAL_REF_EXTERNAL) {
> >                 continue;
>
> >         } else if (ret < 0) {
>

Hi Andy,

> Side note, redundant 'else'

Are you asking to add an 'else' statement?

because, unless I'm mistaken, in this case ret can have other negative values
that we want to catch with this 'else if'.

Thanks for your time,
Liam

>
> >                 dev_err(dev, "invalid voltage reference in %pfw\n", child);
> >                 fwnode_handle_put(child);
> >                 return ret;
> >         }
>
>
> --
> With Best Regards,
> Andy Shevchenko