[brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[kernel test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
head:   7bb5f3a7ca8856c5c1fa26a6e3f58a1254019dc0
commit: 36e44186e0badfda499b65d4462c49783bf92314 [42/47] gpio: mmio: Support 64-bit BE access
config: um-allyesconfig (https://download.01.org/0day-ci/archive/20240223/202402232058.4eDf4GRs-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240223/202402232058.4eDf4GRs-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402232058.4eDf4GRs-lkp@intel.com/

All errors (new ones prefixed by >>):

   /usr/bin/ld: drivers/gpio/gpio-mmio.o: in function `bgpio_write64':
>> gpio-mmio.c:(.text+0x12e1): undefined reference to `iowrite64'
   /usr/bin/ld: drivers/gpio/gpio-mmio.o: in function `bgpio_read64':
>> gpio-mmio.c:(.text+0x131d): undefined reference to `ioread64'
   /usr/bin/ld: drivers/gpio/gpio-mmio.o: in function `bgpio_write64be':
>> gpio-mmio.c:(.text+0x1361): undefined reference to `iowrite64be'
   /usr/bin/ld: drivers/gpio/gpio-mmio.o: in function `bgpio_read64be':
>> gpio-mmio.c:(.text+0x139d): undefined reference to `ioread64be'
   collect2: error: ld returned 1 exit status

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[Linus Walleij]

On Fri, Feb 23, 2024 at 1:44 PM kernel test robot <lkp@intel.com> wrote:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
> head:   7bb5f3a7ca8856c5c1fa26a6e3f58a1254019dc0
> commit: 36e44186e0badfda499b65d4462c49783bf92314 [42/47] gpio: mmio: Support 64-bit BE access
> config: um-allyesconfig (https://download.01.org/0day-ci/archive/20240223/202402232058.4eDf4GRs-lkp@intel.com/config)

UM Linux now again.

gpio-mmio depends on HAS_IOMEM
and UM Linux has set HAS_IOMEM, but
also claims to support 64bit without providing the necessary 64bit
io-accessors.

Maybe UM Linux need to be fixed?

Yours,
Linus Walleij

Re: [brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[Bartosz Golaszewski]

On Sat, Feb 24, 2024 at 4:58 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Feb 23, 2024 at 1:44 PM kernel test robot <lkp@intel.com> wrote:
>
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
> > head:   7bb5f3a7ca8856c5c1fa26a6e3f58a1254019dc0
> > commit: 36e44186e0badfda499b65d4462c49783bf92314 [42/47] gpio: mmio: Support 64-bit BE access
> > config: um-allyesconfig (https://download.01.org/0day-ci/archive/20240223/202402232058.4eDf4GRs-lkp@intel.com/config)
>
> UM Linux now again.
>
> gpio-mmio depends on HAS_IOMEM
> and UM Linux has set HAS_IOMEM, but
> also claims to support 64bit without providing the necessary 64bit
> io-accessors.
>
> Maybe UM Linux need to be fixed?
>
> Yours,
> Linus Walleij
>

Nah, there were other reports for the same issues, at least for x86
and alpha. I hope Andy will have some time to look into it early this
week, otherwise we'll have to revert the offending patch.

Bart

Re: [brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[Andy Shevchenko]

On Sat, Feb 24, 2024 at 8:57 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Sat, Feb 24, 2024 at 4:58 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Fri, Feb 23, 2024 at 1:44 PM kernel test robot <lkp@intel.com> wrote:
> >
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
> > > head:   7bb5f3a7ca8856c5c1fa26a6e3f58a1254019dc0
> > > commit: 36e44186e0badfda499b65d4462c49783bf92314 [42/47] gpio: mmio: Support 64-bit BE access
> > > config: um-allyesconfig (https://download.01.org/0day-ci/archive/20240223/202402232058.4eDf4GRs-lkp@intel.com/config)
> >
> > UM Linux now again.
> >
> > gpio-mmio depends on HAS_IOMEM
> > and UM Linux has set HAS_IOMEM, but
> > also claims to support 64bit without providing the necessary 64bit
> > io-accessors.
> >
> > Maybe UM Linux need to be fixed?
>
> Nah, there were other reports for the same issues, at least for x86
> and alpha. I hope Andy will have some time to look into it early this
> week, otherwise we'll have to revert the offending patch.

So, the problem is that those architectures do define GENERIC_IOMAP
which does NOT have implementations of ioread64*()/iowrite64*(). And
we have dead code like drivers/vfio/pci/vfio_pci_rdwr.c (the part
related to 64-bit IO accessors), which nobody tried.

Arnd, maybe you can shed a light on why it is so?

P.S. The workaround is to open code using readq()/writeq() [with
swab64() for BE].

-- 
With Best Regards,
Andy Shevchenko

Re: [brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[Arnd Bergmann]

On Sun, Feb 25, 2024, at 00:23, Andy Shevchenko wrote:
> On Sat, Feb 24, 2024 at 8:57 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> On Sat, Feb 24, 2024 at 4:58 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>> > On Fri, Feb 23, 2024 at 1:44 PM kernel test robot <lkp@intel.com> wrote:
>> >
>> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
>> > > head:   7bb5f3a7ca8856c5c1fa26a6e3f58a1254019dc0
>> > > commit: 36e44186e0badfda499b65d4462c49783bf92314 [42/47] gpio: mmio: Support 64-bit BE access
>> > > config: um-allyesconfig (https://download.01.org/0day-ci/archive/20240223/202402232058.4eDf4GRs-lkp@intel.com/config)
>> >
>> > UM Linux now again.
>> >
>> > gpio-mmio depends on HAS_IOMEM
>> > and UM Linux has set HAS_IOMEM, but
>> > also claims to support 64bit without providing the necessary 64bit
>> > io-accessors.
>> >
>> > Maybe UM Linux need to be fixed?
>>
>> Nah, there were other reports for the same issues, at least for x86
>> and alpha. I hope Andy will have some time to look into it early this
>> week, otherwise we'll have to revert the offending patch.
>
> So, the problem is that those architectures do define GENERIC_IOMAP
> which does NOT have implementations of ioread64*()/iowrite64*().
>
> Arnd, maybe you can shed a light on why it is so?

The problem here is that x86 cannot do 64-bit accesses
to IORESOURCE_IO() mappings, so neither inq()/outq() nor
ioread64()/iowrite64() can be implemented for it without
splitting the access into two 32-bit ones.

If you have the specification for the device that tries
to use this, you should be able to work out whether
the top or the bottom half of the 64-bit register comes
first and replace it with a pair of 32-bit accesses that
work on both I/O and memory space, see
include/linux/io-64-nonatomic-{hi-lo,lo-hi}.h

> And we have dead code like drivers/vfio/pci/vfio_pci_rdwr.c (the
> part related to 64-bit IO accessors), which nobody tried.

I could not figure out what that code is even trying to do,
with its extra byteswap.

> P.S. The workaround is to open code using readq()/writeq() [with
> swab64() for BE].

readq()/writeq() are not generally a replace for
ioread64()/iowrite64() because they can't deal with ioport_map()
type mappings, though the reverse is true and you can always
replace readl()/writel() with ioread32()/iowrite32() if you
can live with the performance overhead on x86.

     Arnd

Re: [brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[Andy Shevchenko]

On Sun, Feb 25, 2024 at 11:09 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Sun, Feb 25, 2024, at 00:23, Andy Shevchenko wrote:
> > On Sat, Feb 24, 2024 at 8:57 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

...

> > So, the problem is that those architectures do define GENERIC_IOMAP
> > which does NOT have implementations of ioread64*()/iowrite64*().
> >
> > Arnd, maybe you can shed a light on why it is so?
>
> The problem here is that x86 cannot do 64-bit accesses
> to IORESOURCE_IO() mappings, so neither inq()/outq() nor
> ioread64()/iowrite64() can be implemented for it without
> splitting the access into two 32-bit ones.
>
> If you have the specification for the device that tries
> to use this, you should be able to work out whether
> the top or the bottom half of the 64-bit register comes
> first and replace it with a pair of 32-bit accesses that
> work on both I/O and memory space, see
> include/linux/io-64-nonatomic-{hi-lo,lo-hi}.h
>
> > And we have dead code like drivers/vfio/pci/vfio_pci_rdwr.c (the
> > part related to 64-bit IO accessors), which nobody tried.
>
> I could not figure out what that code is even trying to do,
> with its extra byteswap.
>
> > P.S. The workaround is to open code using readq()/writeq() [with
> > swab64() for BE].
>
> readq()/writeq() are not generally a replace for
> ioread64()/iowrite64() because they can't deal with ioport_map()
> type mappings, though the reverse is true and you can always
> replace readl()/writel() with ioread32()/iowrite32() if you
> can live with the performance overhead on x86.

The driver in question by name assumes that it won't perform IO port
access. Perhaps use of ioread*()/iowrite*() is not what we should even
consider there, Linus, Bart, do you know if gpio-mmio was ever used
for devices that want IO port rather than MMIO accesses?

-- 
With Best Regards,
Andy Shevchenko

Re: [brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[Arnd Bergmann]

On Mon, Feb 26, 2024, at 03:17, Andy Shevchenko wrote:
> On Sun, Feb 25, 2024 at 11:09 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Sun, Feb 25, 2024, at 00:23, Andy Shevchenko wrote:
>>
>> readq()/writeq() are not generally a replace for
>> ioread64()/iowrite64() because they can't deal with ioport_map()
>> type mappings, though the reverse is true and you can always
>> replace readl()/writel() with ioread32()/iowrite32() if you
>> can live with the performance overhead on x86.
>
> The driver in question by name assumes that it won't perform IO port
> access.

Do you have a link to the driver? Maybe it can just be
made 'depends on !GENERIC_IOMAP' if it doesn't run on x86
or um.

bgpio_setup_accessors() already has a special case for 64-bit
configs to allow bgpio_read64(), so it wouldn't be any
weirder to also require !GENERIC_IOMAP here.

> Perhaps use of ioread*()/iowrite*() is not what we should even
> consider there, Linus, Bart, do you know if gpio-mmio was ever used
> for devices that want IO port rather than MMIO accesses?

It looks like there is only one PCI driver using pci_iomap
with bgpio_init(), and this one is x86 specific:
drivers/gpio/gpio-sodaville.c. This one is little-endian
and 32-bit only.

    Arnd

Re: [brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[Bartosz Golaszewski]

On Mon, 26 Feb 2024 at 03:18, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> On Sun, Feb 25, 2024 at 11:09 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Sun, Feb 25, 2024, at 00:23, Andy Shevchenko wrote:
> > > On Sat, Feb 24, 2024 at 8:57 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> ...
>
> > > So, the problem is that those architectures do define GENERIC_IOMAP
> > > which does NOT have implementations of ioread64*()/iowrite64*().
> > >
> > > Arnd, maybe you can shed a light on why it is so?
> >
> > The problem here is that x86 cannot do 64-bit accesses
> > to IORESOURCE_IO() mappings, so neither inq()/outq() nor
> > ioread64()/iowrite64() can be implemented for it without
> > splitting the access into two 32-bit ones.
> >
> > If you have the specification for the device that tries
> > to use this, you should be able to work out whether
> > the top or the bottom half of the 64-bit register comes
> > first and replace it with a pair of 32-bit accesses that
> > work on both I/O and memory space, see
> > include/linux/io-64-nonatomic-{hi-lo,lo-hi}.h
> >
> > > And we have dead code like drivers/vfio/pci/vfio_pci_rdwr.c (the
> > > part related to 64-bit IO accessors), which nobody tried.
> >
> > I could not figure out what that code is even trying to do,
> > with its extra byteswap.
> >
> > > P.S. The workaround is to open code using readq()/writeq() [with
> > > swab64() for BE].
> >
> > readq()/writeq() are not generally a replace for
> > ioread64()/iowrite64() because they can't deal with ioport_map()
> > type mappings, though the reverse is true and you can always
> > replace readl()/writel() with ioread32()/iowrite32() if you
> > can live with the performance overhead on x86.
>
> The driver in question by name assumes that it won't perform IO port
> access. Perhaps use of ioread*()/iowrite*() is not what we should even
> consider there, Linus, Bart, do you know if gpio-mmio was ever used
> for devices that want IO port rather than MMIO accesses?
>

I don't know the answer. I'd also like to stabilize the for-next
branch as we're pretty late in the cycle so I'm really considering
backing this patch out unless you think you'll be able to fix it soon?
I'm quite busy ATM and will be travelling this week so can't really
spend time on it.

Bart

> --
> With Best Regards,
> Andy Shevchenko

Re: [brgl:gpio/for-next 42/47] gpio-mmio.c:undefined reference to `iowrite64'

[Andy Shevchenko]

On Mon, Feb 26, 2024 at 3:56 PM Bartosz Golaszewski
<bartosz.golaszewski@linaro.org> wrote:
> On Mon, 26 Feb 2024 at 03:18, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Sun, Feb 25, 2024 at 11:09 PM Arnd Bergmann <arnd@arndb.de> wrote:

...

> > The driver in question by name assumes that it won't perform IO port
> > access. Perhaps use of ioread*()/iowrite*() is not what we should even
> > consider there, Linus, Bart, do you know if gpio-mmio was ever used
> > for devices that want IO port rather than MMIO accesses?
>
> I don't know the answer. I'd also like to stabilize the for-next
> branch as we're pretty late in the cycle so I'm really considering
> backing this patch out unless you think you'll be able to fix it soon?
> I'm quite busy ATM and will be travelling this week so can't really
> spend time on it.

Then, revert it. Not a big deal to me right now, thanks!

-- 
With Best Regards,
Andy Shevchenko