Re: [GIT PULL] Driver core patches for 5.1-rc1

[Linus Torvalds]

On Wed, Mar 6, 2019 at 2:33 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> Joe Perches (1):
>       device.h: Add __cold to dev_<level> logging functions

This is very funky, but that commit generates a new warning in a
totally unrelated area:

  drivers/iio/adc/qcom-pm8xxx-xoadc.c: In function âpm8xxx_xoadc_probeâ:
  drivers/iio/adc/qcom-pm8xxx-xoadc.c:633:8: warning: âchâ may be used
uninitialized in this function [-Wmaybe-uninitialized]
    ret = pm8xxx_read_channel_rsv(adc, ch, AMUX_RSV4,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
             &read_nomux_rsv4, true);
             ~~~~~~~~~~~~~~~~~~~~~~~
  drivers/iio/adc/qcom-pm8xxx-xoadc.c:426:27: note: âchâ was declared here
    struct pm8xxx_chan_info *ch;
                             ^~

and it all looks entirely insane if you look at that line 633 where
the ostensibly uninitialized variable is (it clearly _is_ initialized
there), but if you then look at that line 426 you notice that it
actually makes some kind of sense. The value comes from another
function that was apparently inlined, and that other function does not
"obviously" initialize it.

I wonder why this wasn't seen in linux-next? Yes, the connection is
odd, and maybe it's very compiler version dependent, but I do hope
people react to new warnings. The kernel is entirely warning-free for
me for an x86-64 allmodconfig build, and I want to keep it that way.

And _because_ I want to keep it that way (one of the things I do
during the merge window is look for oddities coming in during pulls,
and new warnings is a big deal for me), I applied the attached patch.
Just FYI.

                   Linus
From e0f0ae838a25464179d37f355d763f9ec139fc15 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Wed, 6 Mar 2019 15:41:29 -0800
Subject: [PATCH] iio: adc: fix warning in Qualcomm PM8xxx HK/XOADC driver
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The pm8xxx_get_channel() implementation is unclear, and causes gcc to
suddenly generate odd warnings.  The trigger for the warning (at least
for me) was the entirely unrelated commit 79a4e91d1bb2 ("device.h: Add
__cold to dev_<level> logging functions"), which apparently changes gcc
code generation in the caller function enough to cause this:

  drivers/iio/adc/qcom-pm8xxx-xoadc.c: In function âpm8xxx_xoadc_probeâ:
  drivers/iio/adc/qcom-pm8xxx-xoadc.c:633:8: warning: âchâ may be used uninitialized in this function [-Wmaybe-uninitialized]
    ret = pm8xxx_read_channel_rsv(adc, ch, AMUX_RSV4,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
             &read_nomux_rsv4, true);
             ~~~~~~~~~~~~~~~~~~~~~~~
  drivers/iio/adc/qcom-pm8xxx-xoadc.c:426:27: note: âchâ was declared here
    struct pm8xxx_chan_info *ch;
                             ^~

because gcc for some reason then isn't able to see that the termination
condition for the "for( )" loop in that function is also the condition
for returning NULL.

So it's not _actually_ uninitialized, but the function is admittedly
just unnecessarily oddly written.

Simplify and clarify the function, making gcc also see that it always
returns a valid initialized value.

Cc: Joe Perches <joe@xxxxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: Andy Gross <andy.gross@xxxxxxxxxx>
Cc: David Brown <david.brown@xxxxxxxxxx>
Cc: Jonathan Cameron <jic23@xxxxxxxxxx>
Cc: Hartmut Knaack <knaack.h@xxxxxx>
Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>
Cc: Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx>
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
 drivers/iio/adc/qcom-pm8xxx-xoadc.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
index c30c002f1fef..4735f8a1ca9d 100644
--- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
+++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
@@ -423,18 +423,14 @@ static irqreturn_t pm8xxx_eoc_irq(int irq, void *d)
 static struct pm8xxx_chan_info *
 pm8xxx_get_channel(struct pm8xxx_xoadc *adc, u8 chan)
 {
-	struct pm8xxx_chan_info *ch;
 	int i;
 
 	for (i = 0; i < adc->nchans; i++) {
-		ch = &adc->chans[i];
+		struct pm8xxx_chan_info *ch = &adc->chans[i];
 		if (ch->hwchan->amux_channel == chan)
-			break;
+			return ch;
 	}
-	if (i == adc->nchans)
-		return NULL;
-
-	return ch;
+	return NULL;
 }
 
 static int pm8xxx_read_channel_rsv(struct pm8xxx_xoadc *adc,
-- 
2.21.0.rc0.33.gfad1f114cd