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

From: Guenter Roeck
Date: Wed Jul 16 2014 - 21:58:29 EST


On 07/16/2014 06:27 PM, Chen Gang wrote:


On 07/15/2014 10:38 PM, Chen Gang wrote:
On 07/15/2014 09:11 AM, Chen Gang wrote:


On 07/15/2014 08:53 AM, Guenter Roeck wrote:
On 07/14/2014 05:34 PM, Chen Gang wrote:
On 07/14/2014 05:22 PM, Chen Gang wrote:

å 2014å7æ14æïäå4:57ïRichard Weinberger <richard@xxxxxx> åéï

Am 14.07.2014 10:48, schrieb Lars-Peter Clausen:
On 07/14/2014 10:31 AM, Richard Weinberger wrote:
Am 13.07.2014 22:17, schrieb Greg Kroah-Hartman:
On Sun, Jul 13, 2014 at 09:33:38PM +0200, Richard Weinberger wrote:
Maybe we could add COMPILE_TEST to the version string too?
Just to detect such kernels fast in user bug reports...

What kind of bug report are you going to get?

User manages to enable CONFIG_FOO by selecting COMPILE_TEST and
complains that it does not work. :)

These drivers are typically drivers for some SoC peripheral and the
device will simply physically not exist on a platform that does not
provide HAS_IOMEM. This is not really any
different from making the driver selectable via COMPILE_TEST for
any other platform. To hit the issue you'd have to instantiate a
device driver instance for a device that
physically does not exist. This will always result in a failure.

Okay, you have convinced me. :)


After search the history patches, I found one related patch which made
by myself (when I am in Asianux):

"https://lkml.org/lkml/2013/7/1/641";

For me, it is a long discussion, and forced many members have to join
in. Please help check again.


One thing you could try would be to return NULL (or where appropriate
an error) in the #else case of CONFIG_HAS_IOMEM and CONFIG_HAS_IOPORT,
ie dont take COMPILE_TEST into account at all. Obviously that means
you won't be able to dump a warning message in the COMPILE_TEST
case, but at least the code would compile. The rejection of above patch
would make a good case for this approach.


For me, only let 'devm_io*map*' support COMPILE_TEST is OK, that can fix
all related issues:


[PATCH] lib: devres: Add dumy functions to support COMPILE_TEST when no IOMEM

For some architectures which no IOMEM, 'devres' will be skipped. But
many drivers may still want COMPILE_TEST, so let 'devres' support it.

The related error (with allmodconfig under score):

MODPOST 1365 modules
ERROR: "devm_ioremap_resource" [drivers/watchdog/tegra_wdt.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/watchdog/of_xilinx_wdt.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/staging/iio/adc/mxs-lradc.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/pwm/pwm-clps711x.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/input/serio/olpc_apsp.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/input/serio/arc_ps2.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-xgene.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-stk17ta8.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-ds1742.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-ds1553.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-ds1511.ko] undefined!
ERROR: "devm_ioremap_resource" [drivers/rtc/rtc-ds1286.ko] undefined!
ERROR: "devm_ioremap" [drivers/rtc/rtc-rp5c01.ko] undefined!
ERROR: "devm_ioremap" [drivers/rtc/rtc-msm6242.ko] undefined!
ERROR: "devm_ioremap" [drivers/rtc/rtc-m48t59.ko] undefined!
ERROR: "devm_ioremap" [drivers/rtc/rtc-m48t35.ko] undefined!
ERROR: "devm_ioremap" [drivers/rtc/rtc-bq4802.ko] undefined!


Signed-off-by: Chen Gang <gang.chen.5i5j@xxxxxxxxx>
---
include/linux/device.h | 9 +++++++++
include/linux/io.h | 30 +++++++++++++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index c2421e0..a7500c3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -630,7 +630,16 @@ extern unsigned long devm_get_free_pages(struct device *dev,
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)

I would make it #else

+static inline void __iomem *devm_ioremap_resource(struct device *dev,
+ struct resource *res)
+{
+ pr_warn("no hardware io memory, only for COMPILE_TEST\n");

dev_warn

+ return (__force void __iomem *)ERR_PTR(-ENXIO);
+}
+#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */

/* allows to add/remove a custom action to devres stack */
int devm_add_action(struct device *dev, void (*action)(void *), void *data);
diff --git a/include/linux/io.h b/include/linux/io.h
index b76e6e5..59128aa 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -58,14 +58,42 @@ static inline void devm_ioport_unmap(struct device *dev, void __iomem *addr)
}
#endif

+#ifdef CONFIG_HAS_IOMEM
+
void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
unsigned long size);
void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
unsigned long size);
void devm_iounmap(struct device *dev, void __iomem *addr);
+void devm_ioremap_release(struct device *dev, void *res);
+
+#elif defined(CONFIG_COMPILE_TEST)
+

Same as above - I would suggest to use #else.

+static inline void __iomem *devm_ioremap(struct device *dev,
+ resource_size_t offset, unsigned long size)
+{
+ pr_warn("no hardware io memory, only for COMPILE_TEST\n");
+ return NULL;
+}
+static inline void __iomem *devm_ioremap_nocache(struct device *dev,
+ resource_size_t offset, unsigned long size)
+{
+ pr_warn("no hardware io memory, only for COMPILE_TEST\n");

dev_warn

Guenter

+ return NULL;
+}
+
+static inline void devm_iounmap(struct device *dev, void __iomem *addr)
+{
+}
+
+static inline void devm_ioremap_release(struct device *dev, void *res)
+{
+}
+
+#endif /* CONFIG_HAS_IOMEM || CONFIG_COMPILE_TEST */
+
int check_signature(const volatile void __iomem *io_addr,
const unsigned char *signature, int length);
-void devm_ioremap_release(struct device *dev, void *res);

/*
* Some systems do not have legacy ISA devices.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/