Thread (70 messages) 70 messages, 11 authors, 2014-07-31

Re: [PATCH] drivers: Let several drivers depends on HAS_IOMEM for 'devm_ioremap_resource'

From: Arnd Bergmann <arnd@arndb.de>
Date: 2014-07-17 10:49:20
Also in: linux-iio, linux-pwm, linux-watchdog, lkml

On Thursday 17 July 2014 12:40:25 Lars-Peter Clausen wrote:
On 07/17/2014 11:20 AM, Arnd Bergmann wrote:
quoted
On Thursday 17 July 2014 09:27:58 Chen Gang wrote:
quoted
                                          gfp_t gfp_mask, unsigned int order);
  extern void devm_free_pages(struct device *dev, unsigned long addr);

+#ifdef CONFIG_HAS_IOMEM
  void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
+#elif defined(CONFIG_COMPILE_TEST)
+static inline void __iomem *devm_ioremap_resource(struct device *dev,
+                                               struct resource *res)
+{
+       pr_warn("no hardware io memory, only for COMPILE_TEST\n");
+       return (__force void __iomem *)ERR_PTR(-ENXIO);
+}
+#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */

  /* allows to add/remove a custom action to devres stack */
To be honest, I think it's a bad idea to introduce wrappers functions
that are only available when CONFIG_COMPILE_TEST is set.

COMPILE_TEST is a great tool in general, but it has its limits.
In particular, the case for !CONFIG_IOMEM is completely obscure
and we won't find any bugs by allowing more drivers to be built
in those configurations, but attempting to do it would cause
endless churn by changing each instance of 'depends on HAS_IOMEM'
to 'depends on HAS_IOMEM || COMPILE_TEST'.
The point of this exercise is that we do not have to replace a good chunk of 
'depends on COMPILE_TEST' with 'depends on COMPILE_TEST && HAS_IOMEM'
Ok, I see.
E.g. the typical Kconfig entry for your random SoC peripheral driver looks like

config ARCH_FOOBAR_DRIVER
        depends on ARCH_FOOBAR || COMPILE_TEST
        ...

Now when COMPILE_TEST is not set there is a implicit dependency on HAS_IOMEM 
since the architecture will provide it. If COMPILE_TEST is selected the 
driver will also be build-able on architectures that do no have HAS_IOMEM 
and hence linking the driver fails. One way to fix this is of course to 
replace the COMPILE_TEST with (COMPILE_TEST && HAS_IOMEM). But this is very 
often overlooked and only noticed later on when somebody actually builds a 
allyesconfig on an architecture that does not provide HAS_IOMEM. To avoid 
these kinds of build errors and tedious fixup patches the idea is to provide 
a stub function when HAS_IOMEM is not enabled, but COMPILE_TEST is enabled.
AFAICT, NO_IOMEM only has a real purpose on UML these days. Could we take
a shortcut here and make COMPILE_TEST depend on !UML? Getting random stuff
to build on UML seems pointless to me and we special-case it in a number of
places already.

	Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help