| Message ID | 20210524190919.2616-1-rdunlap@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] OF: of_address: clean up OF stub & extern functions | expand |
On 5/24/21 4:02 PM, kernel test robot wrote: > Hi Randy, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on robh/for-next] > [also build test ERROR on linux/master linus/master v5.13-rc3 next-20210524] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: https://github.com/0day-ci/linux/commits/Randy-Dunlap/OF-of_address-clean-up-OF-stub-extern-functions/20210525-031115 > base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next > config: s390-randconfig-r001-20210524 (attached as .config) > compiler: s390-linux-gcc (GCC) 9.3.0 > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # https://github.com/0day-ci/linux/commit/8a9c54f1437e3af04c6de3b9606b46437ea69a12 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Randy-Dunlap/OF-of_address-clean-up-OF-stub-extern-functions/20210525-031115 > git checkout 8a9c54f1437e3af04c6de3b9606b46437ea69a12 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> Yes, this patch wasn't expected to fix all of these arch/s390/ build errors. Before this patch: (9 undefined symbols) ERROR: modpost: "devm_ioremap_resource" [drivers/crypto/ccree/ccree.ko] undefined! ERROR: modpost: "debugfs_create_regset32" [drivers/crypto/ccree/ccree.ko] undefined! ERROR: modpost: "devm_ioremap_resource" [drivers/media/rc/ir-hix5hd2.ko] undefined! ERROR: modpost: "ioremap" [drivers/pcmcia/pcmcia.ko] undefined! ERROR: modpost: "iounmap" [drivers/pcmcia/pcmcia.ko] undefined! ERROR: modpost: "devm_platform_ioremap_resource" [drivers/char/xillybus/xillybus_of.ko] undefined! ERROR: modpost: "devm_ioremap_resource" [drivers/dma/qcom/hdma.ko] undefined! ERROR: modpost: "devm_ioremap_resource" [drivers/dma/qcom/hdma_mgmt.ko] undefined! ERROR: modpost: "of_address_to_resource" [drivers/dma/qcom/hdma_mgmt.ko] undefined! This patch only affected the last symbol above. And it covers of_iomap() in my build testing as well. After this patch: (8 undefined symbols) > All errors (new ones prefixed by >>, old ones prefixed by <<): > > ERROR: modpost: "devm_ioremap_resource" [drivers/crypto/ccree/ccree.ko] undefined! > ERROR: modpost: "debugfs_create_regset32" [drivers/crypto/ccree/ccree.ko] undefined! > ERROR: modpost: "devm_ioremap_resource" [drivers/media/rc/ir-hix5hd2.ko] undefined! > ERROR: modpost: "ioremap" [drivers/pcmcia/pcmcia.ko] undefined! > ERROR: modpost: "iounmap" [drivers/pcmcia/pcmcia.ko] undefined! >>> ERROR: modpost: "devm_platform_ioremap_resource" [drivers/char/xillybus/xillybus_of.ko] undefined! > ERROR: modpost: "devm_ioremap_resource" [drivers/dma/qcom/hdma.ko] undefined! > ERROR: modpost: "devm_ioremap_resource" [drivers/dma/qcom/hdma_mgmt.ko] undefined! I think that all of these arch/s390/ builds undefined symbols when CONFIG_HAS_IOMEM is not set are low priority (or no priority) for most people. -- ~Randy
On Mon, May 24, 2021 at 2:09 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > Adjust <linux/of_address.h> so that stubs are present when > CONFIG_OF is not set *or* OF is set but OF_ADDRESS is not set. > > This eliminates 2 build errors on arch/s390/ when HAS_IOMEM > is not set (so OF_ADDRESS is not set). > I.e., it provides a stub for of_iomap() when one was previously > not provided as well as removing some duplicate externs. Personally, I think we should get rid of HAS_IOMEM or at least most of its usage in kconfig. It has little purpose beyond hiding drivers in kconfig and mainly for UML though I think UML no longer needs that IIRC. (I'm not wild about 'depends on OF' either). > s390-linux-ld: drivers/irqchip/irq-al-fic.o: in function `al_fic_init_dt': > irq-al-fic.c:(.init.text+0x7a): undefined reference to `of_iomap' > s390-linux-ld: drivers/clocksource/timer-of.o: in function `timer_of_init': > timer-of.c:(.init.text+0xa4): undefined reference to `of_iomap' > > Tested with many randconfig builds, but there could still be some > hidden problem here. > > Fixes: 4acf4b9cd453 ("of: move of_address_to_resource and of_iomap declarations from sparc") > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: devicetree@vger.kernel.org > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reported-by: kernel test robot <lkp@intel.com> > --- > v2: handle SPARC as a special case since it provides its own versions of > of_address_to_resource() and of_iomap(); > fix build errors reported by lkp/ktr and address comments from Laurent; > do more randconfig build testing; > > include/linux/of_address.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > --- linux-next-20210524.orig/include/linux/of_address.h > +++ linux-next-20210524/include/linux/of_address.h > @@ -106,11 +106,12 @@ static inline bool of_dma_is_coherent(st > } > #endif /* CONFIG_OF_ADDRESS */ > > -#ifdef CONFIG_OF > +#ifdef CONFIG_SPARC /* SPARC has its own versions of these */ The whole point of CONFIG_OF_ADDRESS is really just for SPARC. So I don't really like the mixture of the ifdefs here and in kconfig. It looks only more fragile. Can we drop the HAS_IOMEM dependency from CONFIG_OF_ADDRESS and then fix the fallout from that? That would also remove all the other build time dependencies on HAS_IOMEM. > extern int of_address_to_resource(struct device_node *dev, int index, > struct resource *r); > -void __iomem *of_iomap(struct device_node *node, int index); > -#else > +extern void __iomem *of_iomap(struct device_node *device, int index); > +#else /* !CONFIG_SPARC */ > +#if (defined(CONFIG_OF) && !defined(CONFIG_OF_ADDRESS)) || !defined(CONFIG_OF) > static inline int of_address_to_resource(struct device_node *dev, int index, > struct resource *r) > { > @@ -121,7 +122,9 @@ static inline void __iomem *of_iomap(str > { > return NULL; > } > -#endif > +#endif /* (defined(CONFIG_OF) && !defined(CONFIG_OF_ADDRESS)) || !defined(CONFIG_OF) */ > +#endif /* CONFIG_SPARC */ > + > #define of_range_parser_init of_pci_range_parser_init > > #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_PCI)
On 5/26/21 12:06 PM, Rob Herring wrote: > On Mon, May 24, 2021 at 2:09 PM Randy Dunlap <rdunlap@infradead.org> wrote: >> >> Adjust <linux/of_address.h> so that stubs are present when >> CONFIG_OF is not set *or* OF is set but OF_ADDRESS is not set. >> >> This eliminates 2 build errors on arch/s390/ when HAS_IOMEM >> is not set (so OF_ADDRESS is not set). >> I.e., it provides a stub for of_iomap() when one was previously >> not provided as well as removing some duplicate externs. > > Personally, I think we should get rid of HAS_IOMEM or at least most of > its usage in kconfig. It has little purpose beyond hiding drivers in > kconfig and mainly for UML though I think UML no longer needs that > IIRC. (I'm not wild about 'depends on OF' either). There are only over 800 of the latter. > >> s390-linux-ld: drivers/irqchip/irq-al-fic.o: in function `al_fic_init_dt': >> irq-al-fic.c:(.init.text+0x7a): undefined reference to `of_iomap' >> s390-linux-ld: drivers/clocksource/timer-of.o: in function `timer_of_init': >> timer-of.c:(.init.text+0xa4): undefined reference to `of_iomap' >> >> Tested with many randconfig builds, but there could still be some >> hidden problem here. >> >> Fixes: 4acf4b9cd453 ("of: move of_address_to_resource and of_iomap declarations from sparc") >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >> Cc: Rob Herring <robh+dt@kernel.org> >> Cc: Frank Rowand <frowand.list@gmail.com> >> Cc: devicetree@vger.kernel.org >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Reported-by: kernel test robot <lkp@intel.com> >> --- >> v2: handle SPARC as a special case since it provides its own versions of >> of_address_to_resource() and of_iomap(); >> fix build errors reported by lkp/ktr and address comments from Laurent; >> do more randconfig build testing; >> >> include/linux/of_address.h | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> --- linux-next-20210524.orig/include/linux/of_address.h >> +++ linux-next-20210524/include/linux/of_address.h >> @@ -106,11 +106,12 @@ static inline bool of_dma_is_coherent(st >> } >> #endif /* CONFIG_OF_ADDRESS */ >> >> -#ifdef CONFIG_OF >> +#ifdef CONFIG_SPARC /* SPARC has its own versions of these */ > > The whole point of CONFIG_OF_ADDRESS is really just for SPARC. So I > don't really like the mixture of the ifdefs here and in kconfig. It > looks only more fragile. > > Can we drop the HAS_IOMEM dependency from CONFIG_OF_ADDRESS and then > fix the fallout from that? That would also remove all the other build > time dependencies on HAS_IOMEM. so change config OF_ADDRESS def_bool y depends on !SPARC && (HAS_IOMEM || UML) to config OF_ADDRESS def_bool y depends on !SPARC I'll run some builds and see what all breaks. thanks. -- ~Randy
--- linux-next-20210524.orig/include/linux/of_address.h +++ linux-next-20210524/include/linux/of_address.h @@ -106,11 +106,12 @@ static inline bool of_dma_is_coherent(st } #endif /* CONFIG_OF_ADDRESS */ -#ifdef CONFIG_OF +#ifdef CONFIG_SPARC /* SPARC has its own versions of these */ extern int of_address_to_resource(struct device_node *dev, int index, struct resource *r); -void __iomem *of_iomap(struct device_node *node, int index); -#else +extern void __iomem *of_iomap(struct device_node *device, int index); +#else /* !CONFIG_SPARC */ +#if (defined(CONFIG_OF) && !defined(CONFIG_OF_ADDRESS)) || !defined(CONFIG_OF) static inline int of_address_to_resource(struct device_node *dev, int index, struct resource *r) { @@ -121,7 +122,9 @@ static inline void __iomem *of_iomap(str { return NULL; } -#endif +#endif /* (defined(CONFIG_OF) && !defined(CONFIG_OF_ADDRESS)) || !defined(CONFIG_OF) */ +#endif /* CONFIG_SPARC */ + #define of_range_parser_init of_pci_range_parser_init #if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_PCI)
Adjust <linux/of_address.h> so that stubs are present when CONFIG_OF is not set *or* OF is set but OF_ADDRESS is not set. This eliminates 2 build errors on arch/s390/ when HAS_IOMEM is not set (so OF_ADDRESS is not set). I.e., it provides a stub for of_iomap() when one was previously not provided as well as removing some duplicate externs. s390-linux-ld: drivers/irqchip/irq-al-fic.o: in function `al_fic_init_dt': irq-al-fic.c:(.init.text+0x7a): undefined reference to `of_iomap' s390-linux-ld: drivers/clocksource/timer-of.o: in function `timer_of_init': timer-of.c:(.init.text+0xa4): undefined reference to `of_iomap' Tested with many randconfig builds, but there could still be some hidden problem here. Fixes: 4acf4b9cd453 ("of: move of_address_to_resource and of_iomap declarations from sparc") Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: devicetree@vger.kernel.org Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reported-by: kernel test robot <lkp@intel.com> --- v2: handle SPARC as a special case since it provides its own versions of of_address_to_resource() and of_iomap(); fix build errors reported by lkp/ktr and address comments from Laurent; do more randconfig build testing; include/linux/of_address.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)