Re: [GIT PULL] Driver core patches for 5.1-rc1
Hi Linus, On Wed, 6 Mar 2019 15:47:11 -0800 Linus Torvalds wrote: > > 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, > ^~~ > _nomux_rsv4, true); > ~~~ > drivers/iio/adc/qcom-pm8xxx-xoadc.c:426:27: note: ‘ch’ was declared here > struct pm8xxx_chan_info *ch; > ^~ > > I wonder why this wasn't seen in linux-next? It is possible I missed it because I am currently building with -Wimplicit-fallthrough to help Kees and co with their attempt at making that a "normal" build flag for the kernel. Unfortunately it generates a few hundred warnings still. -- Cheers, Stephen Rothwell pgpg1BFCF8CTP.pgp Description: OpenPGP digital signature
Re: [GIT PULL] Driver core patches for 5.1-rc1
On Thu, Mar 7, 2019 at 12:48 AM Linus Torvalds wrote: > On Wed, Mar 6, 2019 at 2:33 AM Greg KH wrote: > > 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. I saw it in linux-next and sent a patch the other day, similar to yours, but with a less verbose changelog: https://patchwork.kernel.org/patch/10838499/ Overall, I had not done regular randconfig testing since the start of the year, and found 62 regressions that had crept in during that period. There was no significant uptick in -Wmaybe-uninitialized warnings, this is the only one I saw, so I'd classify this as random change in behavior due to inlining differences rather than a systematic issue. (there was a noticeable change in other warnings, particularly a stack size increase from the new structleak plugin changes, fix is coming). Arnd
Re: [GIT PULL] Driver core patches for 5.1-rc1
The pull request you sent on Wed, 6 Mar 2019 11:33:50 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git > tags/driver-core-5.1-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/e431f2d74e1b91e00e71e97cadcadffc4cda8a9b Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL] Driver core patches for 5.1-rc1
On Wed, Mar 6, 2019 at 4:19 PM Linus Torvalds wrote: > > > > btw; what compiler version? > > gcc version 8.2.1 20181215 (Red Hat 8.2.1-6) (GCC) Oh, and also did a dnf upgrade, and the same thing happens with gcc version 8.3.1 20190223 (Red Hat 8.3.1-2) (GCC) too. Linus
Re: [GIT PULL] Driver core patches for 5.1-rc1
On Wed, Mar 6, 2019 at 4:09 PM Joe Perches wrote: > > > > This is very funky, but that commit generates a new warning in a > > totally unrelated area: > > Very very funky. > > Are you sure it's the __cold marking of an > entirely unrelated function that isn't > even used in the code with the new warning? Yup, I bisected it, because it made no sense at all. There were no changes to the driver itself in that pull request, but there was a new warning. > btw; what compiler version? gcc version 8.2.1 20181215 (Red Hat 8.2.1-6) (GCC) FWIW, Linus
Re: [GIT PULL] Driver core patches for 5.1-rc1
On Wed, 2019-03-06 at 15:47 -0800, Linus Torvalds wrote: > On Wed, Mar 6, 2019 at 2:33 AM Greg KH wrote: > > Joe Perches (1): > > device.h: Add __cold to dev_ logging functions > > This is very funky, but that commit generates a new warning in a > totally unrelated area: Very very funky. Are you sure it's the __cold marking of an entirely unrelated function that isn't even used in the code with the new warning? btw; what compiler version? > 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, > ^~~ > _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
Re: [GIT PULL] Driver core patches for 5.1-rc1
On Wed, Mar 6, 2019 at 2:33 AM Greg KH wrote:
>
> Joe Perches (1):
> device.h: Add __cold to dev_ 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,
^~~
_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
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_ 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,
^~~
_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
Cc: Greg Kroah-Hartman
Cc: Andy Gross
Cc: David Brown
Cc: Jonathan Cameron
Cc: Hartmut Knaack
Cc: Lars-Peter Clausen
Cc: Peter Meerwald-Stadler
Signed-off-by: Linus Torvalds
---
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 = >chans[i];
+ struct pm8xxx_chan_info *ch = >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
[GIT PULL] Driver core patches for 5.1-rc1
The following changes since commit d13937116f1e82bf508a6325111b322c30c85eb9: Linux 5.0-rc6 (2019-02-10 14:42:20 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git tags/driver-core-5.1-rc1 for you to fetch changes up to 36cf3b1363f464c40f6ce647d3ac0ae9617d5fbc: driver core: platform: remove misleading err_alloc label (2019-03-01 18:08:06 +0100) Driver core patches for 5.1-rc1 Here is the big driver core patchset for 5.1-rc1 More patches than "normal" here this merge window, due to some work in the driver core by Alexander Duyck to rework the async probe functionality to work better for a number of devices, and independant work from Rafael for the device link functionality to make it work "correctly". Also in here is: - lots of BUS_ATTR() removals, the macro is about to go away - firmware test fixups - ihex fixups and simplification - component additions (also includes i915 patches) - lots of minor coding style fixups and cleanups. All of these have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Alexander Duyck (9): driver core: Establish order of operations for device_add and device_del via bitflag device core: Consolidate locking and unlocking of parent and device driver core: Probe devices asynchronously instead of the driver workqueue: Provide queue_work_node to queue work near a given NUMA node async: Add support for queueing on specific NUMA node driver core: Attach devices on CPU local to device node PM core: Use new async_schedule_dev command libnvdimm: Schedule device registration on node local to the device driver core: Rewrite test_async_driver_probe to cover serialization and NUMA affinity Andrey Smirnov (5): ihex: Share code between ihex_validate_fw() and ihex_next_binrec() ihex: Check if zero-length record is at the end of the blob ihex: Simplify next record offset calculation tools/firmware/ihex2fw: Simplify next record offset calculation tools/firmware/ihex2fw: Replace explicit alignment with ALIGN Ayush Mittal (1): kernfs: Allocating memory for kernfs_iattrs with kmem_cache. Bo YU (2): kobject: to repalce printk with pr_* style kobject: drop newline from msg string Daniel Vetter (4): component: Add documentation components: multiple components for a device i915/snd_hdac: I915 subcomponent for the snd_hdac drivers/component: kerneldoc polish David Engraf (1): device: Fix comment for driver_data in struct device Enrico Granata (1): driver: platform: Support parsing GpioInt 0 in platform_get_irq() Eric Biggers (1): kobject: make kset_get_ownership() 'static' Feng Tang (1): async: Add cmdline option to specify drivers to be async probed Geert Uytterhoeven (1): driver core: Postpone DMA tear-down until after devres release Greg Kroah-Hartman (11): driver core: bus: convert to use BUS_ATTR_WO and RW driver core: drop use of BUS_ATTR() Merge 5.0-rc2 into driver-core-next f2fs: no need to check return value of debugfs_create functions PCI: pci.c: convert to use BUS_ATTR_RW PCI: pci-sysfs.c: convert to use BUS_ATTR_WO pseries: ibmebus.c: convert to use BUS_ATTR_WO rapidio: rio-sysfs.c: convert to use BUS_ATTR_WO block: rbd: convert to use BUS_ATTR_WO and RO Merge 5.0-rc6 into driver-core-next Merge tag 'topic/component-typed-2019-02-11' of git://anongit.freedesktop.org/drm/drm-intel into driver-core-next Jerome Brunet (1): driver core: silence device link messages unless debugging Joe Perches (1): device.h: Add __cold to dev_ logging functions Johannes Berg (1): driver core: platform: remove misleading err_alloc label John Zhao (1): firmware: hardcode the debug message for -ENOENT Luis Chamberlain (3): Revert "selftests: firmware: add CONFIG_FW_LOADER_USER_HELPER_FALLBACK to config" Revert "selftests: firmware: remove use of non-standard diff -Z option" selftests: firmware: fix verify_reqs() return value Mans Rullgard (1): platform: set of_node in platform_device_register_full() Masahiro Yamada (2): firmware_loader: move CONFIG_FW_LOADER_USER_HELPER switch to Makefile firmware_loader: move firmware/ to drivers/base/firmware_loader/builtin/ Mathieu Malaterre (1): drivers: base: Use __printf markup to silence compiler Ondrej Mosnacek (1): sysfs: remove unused include of kernfs-internal.h Rafael J. Wysocki (15): driver core: Fix DL_FLAG_AUTOREMOVE_SUPPLIER device link flag handling driver core: Avoid careless re-use of existing device links driver core: Do not resume suppliers
