tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 2cf0f715623872823a72e451243bbf555d10d032
commit: f1a43aadb5a690e141a3b6700e2a40c1d4dbe088 watchdog: Enable COMPILE_TEST for more drivers
date: 5 weeks ago
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from arch/m68k/include/asm/io_mm.h:25,
from arch/m68k/include/asm/io.h:8,
from include/linux/io.h:13,
from drivers/watchdog/machzwd.c:39:
In function 'zf_set_timer',
inlined from 'zf_timer_on' at drivers/watchdog/machzwd.c:218:2:
>> arch/m68k/include/asm/raw_io.h:91:13: warning: array subscript 0 is outside array bounds of 'volatile u16[0]' {aka 'volatile short unsigned int[]'} [-Warray-bounds=]
91 | __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v >> 8)<<1)))); \
| ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/m68k/include/asm/io_mm.h:228:20: note: in expansion of macro 'rom_out_le16'
228 | : rom_out_le16(isa_itw(port), (val)))
| ^~~~~~~~~~~~
arch/m68k/include/asm/io_mm.h:356:42: note: in expansion of macro 'isa_rom_outw'
356 | #define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : out_le16((port), (val)))
| ^~~~~~~~~~~~
drivers/watchdog/machzwd.c:74:53: note: in expansion of macro 'outw'
74 | #define zf_writew(port, data) { outb(port, INDEX); outw(data, DATA_W); }
| ^~~~
drivers/watchdog/machzwd.c:173:17: note: in expansion of macro 'zf_writew'
173 | zf_writew(COUNTER_1, new);
| ^~~~~~~~~
In function 'zf_timer_on':
cc1: note: source object is likely at address zero
In function 'zf_set_timer',
inlined from 'zf_timer_on' at drivers/watchdog/machzwd.c:218:2:
arch/m68k/include/asm/raw_io.h:87:13: warning: array subscript 0 is outside array bounds of 'volatile u16[0]' {aka 'volatile short unsigned int[]'} [-Warray-bounds=]
87 | __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v & 0xFF)<<1)))); \
| ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/m68k/include/asm/io_mm.h:227:20: note: in expansion of macro 'rom_out_be16'
227 | (ISA_SEX ? rom_out_be16(isa_itw(port), (val)) \
| ^~~~~~~~~~~~
arch/m68k/include/asm/io_mm.h:356:42: note: in expansion of macro 'isa_rom_outw'
356 | #define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : out_le16((port), (val)))
| ^~~~~~~~~~~~
drivers/watchdog/machzwd.c:74:53: note: in expansion of macro 'outw'
74 | #define zf_writew(port, data) { outb(port, INDEX); outw(data, DATA_W); }
| ^~~~
drivers/watchdog/machzwd.c:173:17: note: in expansion of macro 'zf_writew'
173 | zf_writew(COUNTER_1, new);
| ^~~~~~~~~
In function 'zf_timer_on':
cc1: note: source object is likely at address zero
vim +91 arch/m68k/include/asm/raw_io.h
^1da177e4c3f41 include/asm-m68k/raw_io.h Linus Torvalds 2005-04-16 49
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 50 /*
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 51 * Atari ROM port (cartridge port) ISA adapter, used for the EtherNEC NE2000
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 52 * network card driver.
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 53 * The ISA adapter connects address lines A9-A13 to ISA address lines A0-A4,
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 54 * and hardwires the rest of the ISA addresses for a base address of 0x300.
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 55 *
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 56 * Data lines D8-D15 are connected to ISA data lines D0-D7 for reading.
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 57 * For writes, address lines A1-A8 are latched to ISA data lines D0-D7
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 58 * (meaning the bit pattern on A1-A8 can be read back as byte).
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 59 *
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 60 * Read and write operations are distinguished by the base address used:
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 61 * reads are from the ROM A side range, writes are through the B side range
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 62 * addresses (A side base + 0x10000).
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 63 *
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 64 * Reads and writes are byte only.
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 65 *
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 66 * 16 bit reads and writes are necessary for the NetUSBee adapter's USB
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 67 * chipset - 16 bit words are read straight off the ROM port while 16 bit
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 68 * reads are split into two byte writes. The low byte is latched to the
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 69 * NetUSBee buffer by a read from the _read_ window (with the data pattern
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 70 * asserted as A1-A8 address pattern). The high byte is then written to the
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 71 * write range as usual, completing the write cycle.
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 72 */
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 73
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 74 #if defined(CONFIG_ATARI_ROM_ISA)
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 75 #define rom_in_8(addr) \
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 76 ({ u16 __v = (*(__force volatile u16 *) (addr)); __v >>= 8; __v; })
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 77 #define rom_in_be16(addr) \
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 78 ({ u16 __v = (*(__force volatile u16 *) (addr)); __v; })
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 79 #define rom_in_le16(addr) \
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 80 ({ u16 __v = le16_to_cpu(*(__force volatile u16 *) (addr)); __v; })
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 81
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 82 #define rom_out_8(addr, b) \
30b5e6ef4a32ea arch/m68k/include/asm/raw_io.h Geert Uytterhoeven 2022-05-20 83 (void)({u8 __maybe_unused __w, __v = (b); u32 _addr = ((u32) (addr)); \
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 84 __w = ((*(__force volatile u8 *) ((_addr | 0x10000) + (__v<<1)))); })
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 85 #define rom_out_be16(addr, w) \
30b5e6ef4a32ea arch/m68k/include/asm/raw_io.h Geert Uytterhoeven 2022-05-20 86 (void)({u16 __maybe_unused __w, __v = (w); u32 _addr = ((u32) (addr)); \
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 87 __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v & 0xFF)<<1)))); \
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 88 __w = ((*(__force volatile u16 *) ((_addr | 0x10000) + ((__v >> 8)<<1)))); })
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 89 #define rom_out_le16(addr, w) \
30b5e6ef4a32ea arch/m68k/include/asm/raw_io.h Geert Uytterhoeven 2022-05-20 90 (void)({u16 __maybe_unused __w, __v = (w); u32 _addr = ((u32) (addr)); \
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 @91 __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v >> 8)<<1)))); \
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 92 __w = ((*(__force volatile u16 *) ((_addr | 0x10000) + ((__v & 0xFF)<<1)))); })
84b16b7b0d5c81 arch/m68k/include/asm/raw_io.h Michael Schmitz 2013-04-06 93
:::::: The code at line 91 was first introduced by commit
:::::: 84b16b7b0d5c818fadc731a69965dc76dce0c91e m68k/atari: ROM port ISA adapter support
:::::: TO: Michael Schmitz <[email protected]>
:::::: CC: Geert Uytterhoeven <[email protected]>
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Tue, Sep 19, 2023 at 7:09 AM kernel test robot <[email protected]> wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 2cf0f715623872823a72e451243bbf555d10d032
> commit: f1a43aadb5a690e141a3b6700e2a40c1d4dbe088 watchdog: Enable COMPILE_TEST for more drivers
> date: 5 weeks ago
> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
> In file included from arch/m68k/include/asm/io_mm.h:25,
> from arch/m68k/include/asm/io.h:8,
> from include/linux/io.h:13,
> from drivers/watchdog/machzwd.c:39:
> In function 'zf_set_timer',
> inlined from 'zf_timer_on' at drivers/watchdog/machzwd.c:218:2:
> >> arch/m68k/include/asm/raw_io.h:91:13: warning: array subscript 0 is outside array bounds of 'volatile u16[0]' {aka 'volatile short unsigned int[]'} [-Warray-bounds=]
> 91 | __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v >> 8)<<1)))); \
> | ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/m68k/include/asm/io_mm.h:228:20: note: in expansion of macro 'rom_out_le16'
> 228 | : rom_out_le16(isa_itw(port), (val)))
> | ^~~~~~~~~~~~
> arch/m68k/include/asm/io_mm.h:356:42: note: in expansion of macro 'isa_rom_outw'
> 356 | #define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : out_le16((port), (val)))
> | ^~~~~~~~~~~~
> drivers/watchdog/machzwd.c:74:53: note: in expansion of macro 'outw'
> 74 | #define zf_writew(port, data) { outb(port, INDEX); outw(data, DATA_W); }
> | ^~~~
> drivers/watchdog/machzwd.c:173:17: note: in expansion of macro 'zf_writew'
> 173 | zf_writew(COUNTER_1, new);
> | ^~~~~~~~~
> In function 'zf_timer_on':
> cc1: note: source object is likely at address zero
This seems to be some newish check in gcc which looks for fixed
pointers below 4KB[1]. The linked issue says more was planned for
gcc-13, but I haven't found what that is. AFAICT, that shouldn't
happen with this config because isa_itw() should be variable and the
compiler shouldn't be able to determine the value of _addr. However, a
config with CONFIG_Q40=n, CONFIG_AMIGA_PCMCIA=n, and
CONFIG_ATARI_ROM_ISA=n would have a fixed NULL value and could trigger
the warning. This should also have warnings everywhere outw() (and
others) are used with a constant port value.
Rob
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
On 9/19/23 11:37, Rob Herring wrote:
> On Tue, Sep 19, 2023 at 7:09 AM kernel test robot <[email protected]> wrote:
>>
>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> head: 2cf0f715623872823a72e451243bbf555d10d032
>> commit: f1a43aadb5a690e141a3b6700e2a40c1d4dbe088 watchdog: Enable COMPILE_TEST for more drivers
>> date: 5 weeks ago
>> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
>> compiler: m68k-linux-gcc (GCC) 13.2.0
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <[email protected]>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>>
>> All warnings (new ones prefixed by >>):
>>
>> In file included from arch/m68k/include/asm/io_mm.h:25,
>> from arch/m68k/include/asm/io.h:8,
>> from include/linux/io.h:13,
>> from drivers/watchdog/machzwd.c:39:
>> In function 'zf_set_timer',
>> inlined from 'zf_timer_on' at drivers/watchdog/machzwd.c:218:2:
>>>> arch/m68k/include/asm/raw_io.h:91:13: warning: array subscript 0 is outside array bounds of 'volatile u16[0]' {aka 'volatile short unsigned int[]'} [-Warray-bounds=]
>> 91 | __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v >> 8)<<1)))); \
>> | ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> arch/m68k/include/asm/io_mm.h:228:20: note: in expansion of macro 'rom_out_le16'
>> 228 | : rom_out_le16(isa_itw(port), (val)))
>> | ^~~~~~~~~~~~
>> arch/m68k/include/asm/io_mm.h:356:42: note: in expansion of macro 'isa_rom_outw'
>> 356 | #define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : out_le16((port), (val)))
>> | ^~~~~~~~~~~~
>> drivers/watchdog/machzwd.c:74:53: note: in expansion of macro 'outw'
>> 74 | #define zf_writew(port, data) { outb(port, INDEX); outw(data, DATA_W); }
>> | ^~~~
>> drivers/watchdog/machzwd.c:173:17: note: in expansion of macro 'zf_writew'
>> 173 | zf_writew(COUNTER_1, new);
>> | ^~~~~~~~~
>> In function 'zf_timer_on':
>> cc1: note: source object is likely at address zero
>
> This seems to be some newish check in gcc which looks for fixed
> pointers below 4KB[1]. The linked issue says more was planned for
> gcc-13, but I haven't found what that is. AFAICT, that shouldn't
> happen with this config because isa_itw() should be variable and the
> compiler shouldn't be able to determine the value of _addr. However, a
> config with CONFIG_Q40=n, CONFIG_AMIGA_PCMCIA=n, and
> CONFIG_ATARI_ROM_ISA=n would have a fixed NULL value and could trigger
> the warning. This should also have warnings everywhere outw() (and
> others) are used with a constant port value.
>
> Rob
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
A long time ago, when someone submitted a "cleanup: patch for the machzwd
watchdog driver, I approved it but added this comment.
> If anyone is still using the HW supported by this driver, it would
> be a prime target for conversion to use the watchdog subsystem.
> This would reduce code size and improve driver stability.
> _That_ would be useful.
> The only reason for replacing 0 with NULL is to make a static checker
> happy. This kind of change adds zero value to the code. Instead, it
> takes time from those who have to review the patches and apply them.
> If we ignore such patches, we'll get them again and again, taking
> away even more time. If we don't ignore them, we encourage submitters
> and get even more useless patches. If we try to discourage them, we
> risk being accused of being unfriendly. This is a perfect lose-lose
> situation for maintainers.
I do wonder if enabling BUILD_TEST on such drivers is any better.
Guenter
On Tue, Sep 19, 2023 at 6:14 PM Guenter Roeck <[email protected]> wrote:
>
> On 9/19/23 11:37, Rob Herring wrote:
> > On Tue, Sep 19, 2023 at 7:09 AM kernel test robot <[email protected]> wrote:
> >>
> >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> >> head: 2cf0f715623872823a72e451243bbf555d10d032
> >> commit: f1a43aadb5a690e141a3b6700e2a40c1d4dbe088 watchdog: Enable COMPILE_TEST for more drivers
> >> date: 5 weeks ago
> >> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
> >> compiler: m68k-linux-gcc (GCC) 13.2.0
> >> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)
> >>
> >> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> >> the same patch/commit), kindly add following tags
> >> | Reported-by: kernel test robot <[email protected]>
> >> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >>
> >> All warnings (new ones prefixed by >>):
> >>
> >> In file included from arch/m68k/include/asm/io_mm.h:25,
> >> from arch/m68k/include/asm/io.h:8,
> >> from include/linux/io.h:13,
> >> from drivers/watchdog/machzwd.c:39:
> >> In function 'zf_set_timer',
> >> inlined from 'zf_timer_on' at drivers/watchdog/machzwd.c:218:2:
> >>>> arch/m68k/include/asm/raw_io.h:91:13: warning: array subscript 0 is outside array bounds of 'volatile u16[0]' {aka 'volatile short unsigned int[]'} [-Warray-bounds=]
> >> 91 | __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v >> 8)<<1)))); \
> >> | ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> arch/m68k/include/asm/io_mm.h:228:20: note: in expansion of macro 'rom_out_le16'
> >> 228 | : rom_out_le16(isa_itw(port), (val)))
> >> | ^~~~~~~~~~~~
> >> arch/m68k/include/asm/io_mm.h:356:42: note: in expansion of macro 'isa_rom_outw'
> >> 356 | #define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : out_le16((port), (val)))
> >> | ^~~~~~~~~~~~
> >> drivers/watchdog/machzwd.c:74:53: note: in expansion of macro 'outw'
> >> 74 | #define zf_writew(port, data) { outb(port, INDEX); outw(data, DATA_W); }
> >> | ^~~~
> >> drivers/watchdog/machzwd.c:173:17: note: in expansion of macro 'zf_writew'
> >> 173 | zf_writew(COUNTER_1, new);
> >> | ^~~~~~~~~
> >> In function 'zf_timer_on':
> >> cc1: note: source object is likely at address zero
> >
> > This seems to be some newish check in gcc which looks for fixed
> > pointers below 4KB[1]. The linked issue says more was planned for
> > gcc-13, but I haven't found what that is. AFAICT, that shouldn't
> > happen with this config because isa_itw() should be variable and the
> > compiler shouldn't be able to determine the value of _addr. However, a
> > config with CONFIG_Q40=n, CONFIG_AMIGA_PCMCIA=n, and
> > CONFIG_ATARI_ROM_ISA=n would have a fixed NULL value and could trigger
> > the warning. This should also have warnings everywhere outw() (and
> > others) are used with a constant port value.
> >
> > Rob
> >
> > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
>
> A long time ago, when someone submitted a "cleanup: patch for the machzwd
> watchdog driver, I approved it but added this comment.
>
> > If anyone is still using the HW supported by this driver, it would
> > be a prime target for conversion to use the watchdog subsystem.
> > This would reduce code size and improve driver stability.
> > _That_ would be useful.
>
> > The only reason for replacing 0 with NULL is to make a static checker
> > happy. This kind of change adds zero value to the code. Instead, it
> > takes time from those who have to review the patches and apply them.
>
> > If we ignore such patches, we'll get them again and again, taking
> > away even more time. If we don't ignore them, we encourage submitters
> > and get even more useless patches. If we try to discourage them, we
> > risk being accused of being unfriendly. This is a perfect lose-lose
> > situation for maintainers.
Agreed. I keep getting one to fix "of of" in a comment. The fix is
always to drop an "of", but that's actually wrong because it's
supposed to be "OF". I keep pointing this out and *never* get the
right fix. I don't fix it myself because at this point, I find
continuing to get "fixes" entertaining.
> I do wonder if enabling BUILD_TEST on such drivers is any better.
I think that and trivial fixes on a specific driver are completely
different. If you want to test build one driver, that's not too hard.
Find its kconfig value, turn it on and build the right arch. For tree
wide stuff, it's a real pain to ensure you built everything. For
example, the only way to build many Cavium Octeon drivers is with the
Octeon defconfig which is just one of dozens configs for MIPS (which
is still a bigger mess than arm32 ever was).
Rob
On Wed, Sep 20, 2023 at 9:30 AM Guenter Roeck <[email protected]> wrote:
>
> On 9/20/23 05:44, Rob Herring wrote:
> > On Tue, Sep 19, 2023 at 6:14 PM Guenter Roeck <[email protected]> wrote:
> >>
> >> On 9/19/23 11:37, Rob Herring wrote:
> >>> On Tue, Sep 19, 2023 at 7:09 AM kernel test robot <[email protected]> wrote:
> >>>>
> >>>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> >>>> head: 2cf0f715623872823a72e451243bbf555d10d032
> >>>> commit: f1a43aadb5a690e141a3b6700e2a40c1d4dbe088 watchdog: Enable COMPILE_TEST for more drivers
> >>>> date: 5 weeks ago
> >>>> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
> >>>> compiler: m68k-linux-gcc (GCC) 13.2.0
> >>>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)
> >>>>
> >>>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> >>>> the same patch/commit), kindly add following tags
> >>>> | Reported-by: kernel test robot <[email protected]>
> >>>> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >>>>
> >>>> All warnings (new ones prefixed by >>):
> >>>>
> >>>> In file included from arch/m68k/include/asm/io_mm.h:25,
> >>>> from arch/m68k/include/asm/io.h:8,
> >>>> from include/linux/io.h:13,
> >>>> from drivers/watchdog/machzwd.c:39:
> >>>> In function 'zf_set_timer',
> >>>> inlined from 'zf_timer_on' at drivers/watchdog/machzwd.c:218:2:
> >>>>>> arch/m68k/include/asm/raw_io.h:91:13: warning: array subscript 0 is outside array bounds of 'volatile u16[0]' {aka 'volatile short unsigned int[]'} [-Warray-bounds=]
> >>>> 91 | __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v >> 8)<<1)))); \
> >>>> | ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>> arch/m68k/include/asm/io_mm.h:228:20: note: in expansion of macro 'rom_out_le16'
> >>>> 228 | : rom_out_le16(isa_itw(port), (val)))
> >>>> | ^~~~~~~~~~~~
> >>>> arch/m68k/include/asm/io_mm.h:356:42: note: in expansion of macro 'isa_rom_outw'
> >>>> 356 | #define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : out_le16((port), (val)))
> >>>> | ^~~~~~~~~~~~
> >>>> drivers/watchdog/machzwd.c:74:53: note: in expansion of macro 'outw'
> >>>> 74 | #define zf_writew(port, data) { outb(port, INDEX); outw(data, DATA_W); }
> >>>> | ^~~~
> >>>> drivers/watchdog/machzwd.c:173:17: note: in expansion of macro 'zf_writew'
> >>>> 173 | zf_writew(COUNTER_1, new);
> >>>> | ^~~~~~~~~
> >>>> In function 'zf_timer_on':
> >>>> cc1: note: source object is likely at address zero
> >>>
> >>> This seems to be some newish check in gcc which looks for fixed
> >>> pointers below 4KB[1]. The linked issue says more was planned for
> >>> gcc-13, but I haven't found what that is. AFAICT, that shouldn't
> >>> happen with this config because isa_itw() should be variable and the
> >>> compiler shouldn't be able to determine the value of _addr. However, a
> >>> config with CONFIG_Q40=n, CONFIG_AMIGA_PCMCIA=n, and
> >>> CONFIG_ATARI_ROM_ISA=n would have a fixed NULL value and could trigger
> >>> the warning. This should also have warnings everywhere outw() (and
> >>> others) are used with a constant port value.
> >>>
> >>> Rob
> >>>
> >>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
> >>
> >> A long time ago, when someone submitted a "cleanup: patch for the machzwd
> >> watchdog driver, I approved it but added this comment.
> >>
> >> > If anyone is still using the HW supported by this driver, it would
> >> > be a prime target for conversion to use the watchdog subsystem.
> >> > This would reduce code size and improve driver stability.
> >> > _That_ would be useful.
> >>
> >> > The only reason for replacing 0 with NULL is to make a static checker
> >> > happy. This kind of change adds zero value to the code. Instead, it
> >> > takes time from those who have to review the patches and apply them.
> >>
> >> > If we ignore such patches, we'll get them again and again, taking
> >> > away even more time. If we don't ignore them, we encourage submitters
> >> > and get even more useless patches. If we try to discourage them, we
> >> > risk being accused of being unfriendly. This is a perfect lose-lose
> >> > situation for maintainers.
> >
> > Agreed. I keep getting one to fix "of of" in a comment. The fix is
> > always to drop an "of", but that's actually wrong because it's
> > supposed to be "OF". I keep pointing this out and *never* get the
> > right fix. I don't fix it myself because at this point, I find
> > continuing to get "fixes" entertaining.
> >
> >> I do wonder if enabling BUILD_TEST on such drivers is any better.
> >
> > I think that and trivial fixes on a specific driver are completely
> > different. If you want to test build one driver, that's not too hard.
> > Find its kconfig value, turn it on and build the right arch. For tree
> > wide stuff, it's a real pain to ensure you built everything. For
> > example, the only way to build many Cavium Octeon drivers is with the
> > Octeon defconfig which is just one of dozens configs for MIPS (which
> > is still a bigger mess than arm32 ever was).
> >
>
> Sure, but I still argue that this isn't worth it for drivers like this one.
> Are you going to submit a fix ? Because otherwise I'll submit a patch
> to drop COMPILE_TEST from MACHZ_WDT.
I honestly don't know what the fix is. There's a compiler flag to
allow 0 address, but that seems like a big hammer. From what I read on
the fix for gcc-12, we shouldn't be getting this, but I haven't
confirmed. I was hoping for comment from Geert as the issue doesn't
appear to be the driver, but the arch code.
Furthermore, I just built the same HEAD and config as reported and
don't see this error. I'm using kernel.org nolibc gcc 13.2.0 which
should be the same version.
Rob
On 9/20/23 05:44, Rob Herring wrote:
> On Tue, Sep 19, 2023 at 6:14 PM Guenter Roeck <[email protected]> wrote:
>>
>> On 9/19/23 11:37, Rob Herring wrote:
>>> On Tue, Sep 19, 2023 at 7:09 AM kernel test robot <[email protected]> wrote:
>>>>
>>>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>>>> head: 2cf0f715623872823a72e451243bbf555d10d032
>>>> commit: f1a43aadb5a690e141a3b6700e2a40c1d4dbe088 watchdog: Enable COMPILE_TEST for more drivers
>>>> date: 5 weeks ago
>>>> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
>>>> compiler: m68k-linux-gcc (GCC) 13.2.0
>>>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)
>>>>
>>>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>>>> the same patch/commit), kindly add following tags
>>>> | Reported-by: kernel test robot <[email protected]>
>>>> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>> In file included from arch/m68k/include/asm/io_mm.h:25,
>>>> from arch/m68k/include/asm/io.h:8,
>>>> from include/linux/io.h:13,
>>>> from drivers/watchdog/machzwd.c:39:
>>>> In function 'zf_set_timer',
>>>> inlined from 'zf_timer_on' at drivers/watchdog/machzwd.c:218:2:
>>>>>> arch/m68k/include/asm/raw_io.h:91:13: warning: array subscript 0 is outside array bounds of 'volatile u16[0]' {aka 'volatile short unsigned int[]'} [-Warray-bounds=]
>>>> 91 | __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v >> 8)<<1)))); \
>>>> | ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> arch/m68k/include/asm/io_mm.h:228:20: note: in expansion of macro 'rom_out_le16'
>>>> 228 | : rom_out_le16(isa_itw(port), (val)))
>>>> | ^~~~~~~~~~~~
>>>> arch/m68k/include/asm/io_mm.h:356:42: note: in expansion of macro 'isa_rom_outw'
>>>> 356 | #define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : out_le16((port), (val)))
>>>> | ^~~~~~~~~~~~
>>>> drivers/watchdog/machzwd.c:74:53: note: in expansion of macro 'outw'
>>>> 74 | #define zf_writew(port, data) { outb(port, INDEX); outw(data, DATA_W); }
>>>> | ^~~~
>>>> drivers/watchdog/machzwd.c:173:17: note: in expansion of macro 'zf_writew'
>>>> 173 | zf_writew(COUNTER_1, new);
>>>> | ^~~~~~~~~
>>>> In function 'zf_timer_on':
>>>> cc1: note: source object is likely at address zero
>>>
>>> This seems to be some newish check in gcc which looks for fixed
>>> pointers below 4KB[1]. The linked issue says more was planned for
>>> gcc-13, but I haven't found what that is. AFAICT, that shouldn't
>>> happen with this config because isa_itw() should be variable and the
>>> compiler shouldn't be able to determine the value of _addr. However, a
>>> config with CONFIG_Q40=n, CONFIG_AMIGA_PCMCIA=n, and
>>> CONFIG_ATARI_ROM_ISA=n would have a fixed NULL value and could trigger
>>> the warning. This should also have warnings everywhere outw() (and
>>> others) are used with a constant port value.
>>>
>>> Rob
>>>
>>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
>>
>> A long time ago, when someone submitted a "cleanup: patch for the machzwd
>> watchdog driver, I approved it but added this comment.
>>
>> > If anyone is still using the HW supported by this driver, it would
>> > be a prime target for conversion to use the watchdog subsystem.
>> > This would reduce code size and improve driver stability.
>> > _That_ would be useful.
>>
>> > The only reason for replacing 0 with NULL is to make a static checker
>> > happy. This kind of change adds zero value to the code. Instead, it
>> > takes time from those who have to review the patches and apply them.
>>
>> > If we ignore such patches, we'll get them again and again, taking
>> > away even more time. If we don't ignore them, we encourage submitters
>> > and get even more useless patches. If we try to discourage them, we
>> > risk being accused of being unfriendly. This is a perfect lose-lose
>> > situation for maintainers.
>
> Agreed. I keep getting one to fix "of of" in a comment. The fix is
> always to drop an "of", but that's actually wrong because it's
> supposed to be "OF". I keep pointing this out and *never* get the
> right fix. I don't fix it myself because at this point, I find
> continuing to get "fixes" entertaining.
>
>> I do wonder if enabling BUILD_TEST on such drivers is any better.
>
> I think that and trivial fixes on a specific driver are completely
> different. If you want to test build one driver, that's not too hard.
> Find its kconfig value, turn it on and build the right arch. For tree
> wide stuff, it's a real pain to ensure you built everything. For
> example, the only way to build many Cavium Octeon drivers is with the
> Octeon defconfig which is just one of dozens configs for MIPS (which
> is still a bigger mess than arm32 ever was).
>
Sure, but I still argue that this isn't worth it for drivers like this one.
Are you going to submit a fix ? Because otherwise I'll submit a patch
to drop COMPILE_TEST from MACHZ_WDT.
Thanks,
Guenter
On 9/20/23 09:20, Rob Herring wrote:
[....]
>>
>> Sure, but I still argue that this isn't worth it for drivers like this one.
>> Are you going to submit a fix ? Because otherwise I'll submit a patch
>> to drop COMPILE_TEST from MACHZ_WDT.
>
> I honestly don't know what the fix is. There's a compiler flag to
> allow 0 address, but that seems like a big hammer. From what I read on
> the fix for gcc-12, we shouldn't be getting this, but I haven't
> confirmed. I was hoping for comment from Geert as the issue doesn't
> appear to be the driver, but the arch code.
>
> Furthermore, I just built the same HEAD and config as reported and
> don't see this error. I'm using kernel.org nolibc gcc 13.2.0 which
> should be the same version.
>
Exactly my point. So now we are stuck with a report like this on a
driver which probably has 0 users and we don't know how to fix it,
all to get the benefit of being able to compile it for an architecture
and platform which will never use it.
I seem to recall similar errors with m68k and COMPILE_TEST last time
I tried to enable it on watchdog drivers, so I am not sure if this is
entirely new.
Guenter
Hi Günter, Rob,
CC Michael
On Wed, Sep 20, 2023 at 11:09 PM Guenter Roeck <[email protected]> wrote:
> On 9/20/23 09:20, Rob Herring wrote:
> [....]
>
> >>
> >> Sure, but I still argue that this isn't worth it for drivers like this one.
> >> Are you going to submit a fix ? Because otherwise I'll submit a patch
> >> to drop COMPILE_TEST from MACHZ_WDT.
I think dropping COMPILE_TEST from MACHZ_WDT is the right thing to do
anyway. Unlike most other drivers, this is not a driver that can be
compiled in, and doesn't do anything if the hardware is not present.
In fact it is a very dangerous driver: its probe function "reads" the
ZF version register, but that involves doing an unconditional write,
which might crash any non-X86 system.
IMHO a driver must not be enabled for compile-testing if its presence
can harm the system.
> > I honestly don't know what the fix is. There's a compiler flag to
> > allow 0 address, but that seems like a big hammer. From what I read on
> > the fix for gcc-12, we shouldn't be getting this, but I haven't
> > confirmed. I was hoping for comment from Geert as the issue doesn't
> > appear to be the driver, but the arch code.
Well, Atari ROM port ISA accesses are really weird, due to the really
weird way the bus is wired to the address/data lines...
The issue is that gcc considers accessing these addresses as "not
done"...
> > Furthermore, I just built the same HEAD and config as reported and
> > don't see this error. I'm using kernel.org nolibc gcc 13.2.0 which
> > should be the same version.
>
> Exactly my point. So now we are stuck with a report like this on a
> driver which probably has 0 users and we don't know how to fix it,
> all to get the benefit of being able to compile it for an architecture
> and platform which will never use it.
>
> I seem to recall similar errors with m68k and COMPILE_TEST last time
> I tried to enable it on watchdog drivers, so I am not sure if this is
> entirely new.
Probably not.
Michael: original build failure report in
https://lore.kernel.org/r/[email protected]/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
Am 21.09.23 um 19:08 schrieb Geert Uytterhoeven:
> Hi Günter, Rob,
>
> CC Michael
>
> On Wed, Sep 20, 2023 at 11:09 PM Guenter Roeck <[email protected]> wrote:
>> On 9/20/23 09:20, Rob Herring wrote:
>> [....]
>>
>>>> Sure, but I still argue that this isn't worth it for drivers like this one.
>>>> Are you going to submit a fix ? Because otherwise I'll submit a patch
>>>> to drop COMPILE_TEST from MACHZ_WDT.
> I think dropping COMPILE_TEST from MACHZ_WDT is the right thing to do
> anyway. Unlike most other drivers, this is not a driver that can be
> compiled in, and doesn't do anything if the hardware is not present.
> In fact it is a very dangerous driver: its probe function "reads" the
> ZF version register, but that involves doing an unconditional write,
> which might crash any non-X86 system.
>
> IMHO a driver must not be enabled for compile-testing if its presence
> can harm the system.
>
>>> I honestly don't know what the fix is. There's a compiler flag to
>>> allow 0 address, but that seems like a big hammer. From what I read on
>>> the fix for gcc-12, we shouldn't be getting this, but I haven't
>>> confirmed. I was hoping for comment from Geert as the issue doesn't
>>> appear to be the driver, but the arch code.
> Well, Atari ROM port ISA accesses are really weird, due to the really
> weird way the bus is wired to the address/data lines...
Putting it mildly ...
> The issue is that gcc considers accessing these addresses as "not
> done"...
That an new GCC problem?
>>> Furthermore, I just built the same HEAD and config as reported and
>>> don't see this error. I'm using kernel.org nolibc gcc 13.2.0 which
>>> should be the same version.
>> Exactly my point. So now we are stuck with a report like this on a
>> driver which probably has 0 users and we don't know how to fix it,
>> all to get the benefit of being able to compile it for an architecture
>> and platform which will never use it.
>>
>> I seem to recall similar errors with m68k and COMPILE_TEST last time
>> I tried to enable it on watchdog drivers, so I am not sure if this is
>> entirely new.
> Probably not.
>
> Michael: original build failure report in
> https://lore.kernel.org/r/[email protected]/
From what I can see, DATA_W appears to be zero - maybe the driver needs
to set 'sane' values for IO ports if compile tested on non-ISA
architectures?
In the alternative, we might have to prevent setting ATARI_ROM_ISA if
COMPILE_TEST is set.
Cheers,
Michael
>
> Gr{oetje,eeting}s,
>
> Geert
>