| Message ID | 20230929164920.314849-1-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] fs: debugfs: fix build error at powerpc platform | expand |
On Fri, Sep 29, 2023 at 12:49:20PM -0400, Frank Li wrote: > ld: fs/debugfs/file.o: in function `debugfs_print_regs': > file.c:(.text+0x95a): undefined reference to `ioread64be' > >> ld: file.c:(.text+0x9dd): undefined reference to `ioread64' > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202309291322.3pZiyosI-lkp@intel.com/ > Signed-off-by: Frank Li <Frank.Li@nxp.com> What commit id does this fix? > --- > fs/debugfs/file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c > index 5b8d4fd7c747..b406283806d9 100644 > --- a/fs/debugfs/file.c > +++ b/fs/debugfs/file.c > @@ -1179,7 +1179,7 @@ void debugfs_print_regs(struct seq_file *s, const struct debugfs_reg *regs, > seq_printf(s, "%s = 0x%04x\n", regs->name, > b ? ioread16be(reg) : ioread16(reg)); > break; > -#ifdef CONFIG_64BIT > +#if defined(ioread64) && defined (ioread64be) Are you sure this is equivalent? What if these are functions? thanks, greg k-h
On Sat, Sep 30, 2023 at 09:11:04AM +0200, Greg KH wrote: > On Fri, Sep 29, 2023 at 12:49:20PM -0400, Frank Li wrote: > > ld: fs/debugfs/file.o: in function `debugfs_print_regs': > > file.c:(.text+0x95a): undefined reference to `ioread64be' > > >> ld: file.c:(.text+0x9dd): undefined reference to `ioread64' > > > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: https://lore.kernel.org/oe-kbuild-all/202309291322.3pZiyosI-lkp@intel.com/ > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > What commit id does this fix? In dmaengine tree https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git/log/?h=next commit 09289d0ad1226c4735f8d9f68c9c3e54cbaba3d4 Author: Frank Li <Frank.Li@nxp.com> Date: Thu Sep 21 11:01:42 2023 -0400 debugfs_create_regset32() support 8/16/64 bit width registers > > > --- > > fs/debugfs/file.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c > > index 5b8d4fd7c747..b406283806d9 100644 > > --- a/fs/debugfs/file.c > > +++ b/fs/debugfs/file.c > > @@ -1179,7 +1179,7 @@ void debugfs_print_regs(struct seq_file *s, const struct debugfs_reg *regs, > > seq_printf(s, "%s = 0x%04x\n", regs->name, > > b ? ioread16be(reg) : ioread16(reg)); > > break; > > -#ifdef CONFIG_64BIT > > +#if defined(ioread64) && defined (ioread64be) > > Are you sure this is equivalent? What if these are functions? Just dump 64bit register value. I am not sure why powerpc have not implement this function with CONFIG_64BIT. in io.h #ifndef ioread64 #define ioread64 ioread64 ... #endif I think it is better why to check if ioread64 exist. Frank > > thanks, > > greg k-h
On 09/29/23 at 12:49pm, Frank Li wrote: > ld: fs/debugfs/file.o: in function `debugfs_print_regs': > file.c:(.text+0x95a): undefined reference to `ioread64be' > >> ld: file.c:(.text+0x9dd): undefined reference to `ioread64' From your reproducer, on x86_64, GENERIC_IOMAP is selected. So the default version of ioread64 and ioread64be in asm-generic/io.h are bypassed. Except of those arch where ioread64 and ioread64be are implemented specifically like alpha, arm64, parisc, power, we may need include include/linux/io-64-nonatomic-hi-lo.h or include/linux/io-64-nonatomic-lo-hi.h to fix above linking issue? From my side, below change can fix the issue. However, I am not quite sure which one is chosen between io-64-nonatomic-hi-lo.h and io-64-nonatomic-hi-lo.h. diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 87b3753aa4b1..b433be134c67 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -15,6 +15,7 @@ #include <linux/pagemap.h> #include <linux/debugfs.h> #include <linux/io.h> +#include <linux/io-64-nonatomic-hi-lo.h> #include <linux/slab.h> #include <linux/atomic.h> #include <linux/device.h>
On Sun, Oct 1, 2023, at 07:22, Baoquan He wrote: > On 09/29/23 at 12:49pm, Frank Li wrote: >> ld: fs/debugfs/file.o: in function `debugfs_print_regs': >> file.c:(.text+0x95a): undefined reference to `ioread64be' >> >> ld: file.c:(.text+0x9dd): undefined reference to `ioread64' > > From your reproducer, on x86_64, GENERIC_IOMAP is selected. So the > default version of ioread64 and ioread64be in asm-generic/io.h are > bypassed. Except of those arch where ioread64 and ioread64be are > implemented specifically like alpha, arm64, parisc, power, we may need > include include/linux/io-64-nonatomic-hi-lo.h or > include/linux/io-64-nonatomic-lo-hi.h to fix above linking issue? > > From my side, below change can fix the issue. However, I am not quite > sure which one is chosen between io-64-nonatomic-hi-lo.h and > io-64-nonatomic-hi-lo.h. It looks like the latest version of the patch only calls it for 64-bit targets, so this question should not come up. On 32-bit targets, it is driver specific which one you need, so having it generic code would require passing a flag from a driver, but I think that adds more complexity than it help. Arnd
On Sun, Oct 01, 2023 at 07:22:33PM +0800, Baoquan He wrote: > On 09/29/23 at 12:49pm, Frank Li wrote: > > ld: fs/debugfs/file.o: in function `debugfs_print_regs': > > file.c:(.text+0x95a): undefined reference to `ioread64be' > > >> ld: file.c:(.text+0x9dd): undefined reference to `ioread64' > > >From your reproducer, on x86_64, GENERIC_IOMAP is selected. So the > default version of ioread64 and ioread64be in asm-generic/io.h are > bypassed. Except of those arch where ioread64 and ioread64be are > implemented specifically like alpha, arm64, parisc, power, we may need > include include/linux/io-64-nonatomic-hi-lo.h or > include/linux/io-64-nonatomic-lo-hi.h to fix above linking issue? Yes, it can fix this problem. I think hi-lo is more make sense. It is just show register value to help debug issue. It is not big issue even it is wrong. Let's fix it later if someone really need lo-hi in future. Frank > > >From my side, below change can fix the issue. However, I am not quite > sure which one is chosen between io-64-nonatomic-hi-lo.h and > io-64-nonatomic-hi-lo.h. > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c > index 87b3753aa4b1..b433be134c67 100644 > --- a/fs/debugfs/file.c > +++ b/fs/debugfs/file.c > @@ -15,6 +15,7 @@ > #include <linux/pagemap.h> > #include <linux/debugfs.h> > #include <linux/io.h> > +#include <linux/io-64-nonatomic-hi-lo.h> > #include <linux/slab.h> > #include <linux/atomic.h> > #include <linux/device.h> > -- > 2.41.0 >
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 5b8d4fd7c747..b406283806d9 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -1179,7 +1179,7 @@ void debugfs_print_regs(struct seq_file *s, const struct debugfs_reg *regs, seq_printf(s, "%s = 0x%04x\n", regs->name, b ? ioread16be(reg) : ioread16(reg)); break; -#ifdef CONFIG_64BIT +#if defined(ioread64) && defined (ioread64be) case sizeof(u64): seq_printf(s, "%s = 0x%016llx\n", regs->name, b ? ioread64be(reg) : ioread64(reg));