include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

For "User Mode Linux", it may enable 'MMU', but not need implement ioremap and iounmap, so "include/asm-generic/io.h" need notice this case to keep itself 'generic'.

The related error (with allmodconfig, without pcap):

CC [M] drivers/ptp/ptp_pch.o
drivers/ptp/ptp_pch.c: In function ‘pch_remove’:
drivers/ptp/ptp_pch.c:571:3: error: implicit declaration of function ‘iounmap’ [-Werror=implicit-function-declaration] drivers/ptp/ptp_pch.c: In function ‘pch_probe’:
drivers/ptp/ptp_pch.c:621:2: error: implicit declaration of function ‘ioremap’ [-Werror=implicit-function-declaration] drivers/ptp/ptp_pch.c:621:13: warning: assignment makes pointer from integer without a cast [enabled by default]
cc1: some warnings being treated as errors

Signed-off-by Chen Gang <[email protected]>

---
 arch/um/include/asm/Kbuild |    1 +
 include/asm-generic/io.h   |    6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

arch/um/include/asm/Kbuild +0 -0

index b30f34a..a34ea5d 100644

include/asm-generic/io.h +2 -2

index d5afe96..e80331d 100644
@@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
+ * This implementation is for the no-MMU or UML case only... if you have an MMU
  * you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !CONFIG_MMU || CONFIG_UML
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
 	return (void __iomem*) (unsigned long)offset;

include/asm-generic/io.h static inline void *phys_to_virt(unsigned long address) · line 309

Quoted 2 lines

include/asm-generic/io.h +1 -1

-#ifndef CONFIG_MMU
+#if !CONFIG_MMU || CONFIG_UML

UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies. _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1] to make nandsim work on UML. Maybe this helps.

Quoted 2 lines · Richard Weinberger

Quoted 2 lines

Quoted 2 lines

include/asm-generic/io.h static inline void *phys_to_virt(unsigned long address) · line 309

Quoted 2 lines

include/asm-generic/io.h +1 -1

-#ifndef CONFIG_MMU
+#if !CONFIG_MMU || CONFIG_UML

FWIW, the above syntax is not correct, it should be

#if !defined(CONFIG_MMU) || defined(CONFIG_UML)

Hi!

Am 26.06.2013 08:31, schrieb Chen Gang:

Quoted 21 lines

For "User Mode Linux", it may enable 'MMU', but not need implement ioremap and iounmap, so "include/asm-generic/io.h" need notice this case to keep itself 'generic'.

The related error (with allmodconfig, without pcap):

CC [M] drivers/ptp/ptp_pch.o
drivers/ptp/ptp_pch.c: In function �pch_remove�:
drivers/ptp/ptp_pch.c:571:3: error: implicit declaration of function �iounmap� [-Werror=implicit-function-declaration] drivers/ptp/ptp_pch.c: In function �pch_probe�:
drivers/ptp/ptp_pch.c:621:2: error: implicit declaration of function �ioremap� [-Werror=implicit-function-declaration] drivers/ptp/ptp_pch.c:621:13: warning: assignment makes pointer from integer without a cast [enabled by default]
cc1: some warnings being treated as errors

Signed-off-by Chen Gang <[email protected]>

---
 arch/um/include/asm/Kbuild |    1 +
 include/asm-generic/io.h   |    6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

arch/um/include/asm/Kbuild generic-y += hw_irq.h irq_regs.h kdebug.h percpu.h sections.h topology.h xor.h · line 6

Quoted 9 lines

arch/um/include/asm/Kbuild +1 -0

index b30f34a..a34ea5d 100644
@@ -3,3 +3,4 @@ generic-y += hw_irq.h irq_regs.h kdebug.h percpu.h sections.h topology.h xor.h
 generic-y += ftrace.h pci.h io.h param.h delay.h mutex.h current.h exec.h
 generic-y += switch_to.h clkdev.h
 generic-y += trace_clock.h
+generic-y += io.h

We include that file already. See three lines above.

Quoted 24 lines

include/asm-generic/io.h +3 -3

index d5afe96..e80331d 100644
@@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
+ * This implementation is for the no-MMU or UML case only... if you have an MMU
  * you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !CONFIG_MMU || CONFIG_UML
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
 	return (void __iomem*) (unsigned long)offset;

@@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 static inline void iounmap(void __iomem *addr)
 {
 }
-#endif /* CONFIG_MMU */
+#endif /* !CONFIG_MMU || CONFIG_UML */
 

include/asm-generic/io.h static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size) · line 331

Quoted 3 lines

include/asm-generic/io.h +0 -0

 #ifdef CONFIG_HAS_IOPORT
 #ifndef CONFIG_GENERIC_IOMAP

UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies. _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1] to make nandsim work on UML. Maybe this helps.

Quoted 24 lines

Hi!

Am 26.06.2013 08:31, schrieb Chen Gang:

Quoted 21 lines

For "User Mode Linux", it may enable 'MMU', but not need implement ioremap and iounmap, so "include/asm-generic/io.h" need notice this case to keep itself 'generic'.

The related error (with allmodconfig, without pcap):

CC [M] drivers/ptp/ptp_pch.o
drivers/ptp/ptp_pch.c: In function �pch_remove�:
drivers/ptp/ptp_pch.c:571:3: error: implicit declaration of function �iounmap� [-Werror=implicit-function-declaration] drivers/ptp/ptp_pch.c: In function �pch_probe�:
drivers/ptp/ptp_pch.c:621:2: error: implicit declaration of function �ioremap� [-Werror=implicit-function-declaration] drivers/ptp/ptp_pch.c:621:13: warning: assignment makes pointer from integer without a cast [enabled by default]
cc1: some warnings being treated as errors

Signed-off-by Chen Gang <[email protected]>

---
 arch/um/include/asm/Kbuild |    1 +
 include/asm-generic/io.h   |    6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

Quoted 12 lines

arch/um/include/asm/Kbuild generic-y += hw_irq.h irq_regs.h kdebug.h percpu.h sections.h topology.h xor.h · line 6

Quoted 9 lines

arch/um/include/asm/Kbuild +1 -0

index b30f34a..a34ea5d 100644
@@ -3,3 +3,4 @@ generic-y += hw_irq.h irq_regs.h kdebug.h percpu.h sections.h topology.h xor.h
 generic-y += ftrace.h pci.h io.h param.h delay.h mutex.h current.h exec.h
 generic-y += switch_to.h clkdev.h
 generic-y += trace_clock.h
+generic-y += io.h

We include that file already. See three lines above.

Oh, really it is, thanks.

Quoted 24 lines

Quoted 24 lines

include/asm-generic/io.h +3 -3

index d5afe96..e80331d 100644
@@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
+ * This implementation is for the no-MMU or UML case only... if you have an MMU
  * you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !CONFIG_MMU || CONFIG_UML
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
 	return (void __iomem*) (unsigned long)offset;

@@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 static inline void iounmap(void __iomem *addr)
 {
 }
-#endif /* CONFIG_MMU */
+#endif /* !CONFIG_MMU || CONFIG_UML */
 

Quoted 8 lines

include/asm-generic/io.h static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size) · line 331

Quoted 3 lines

include/asm-generic/io.h +0 -0

 #ifdef CONFIG_HAS_IOPORT
 #ifndef CONFIG_GENERIC_IOMAP

UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies. _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1] to make nandsim work on UML. Maybe this helps.

But "no io memory" is not the excuse to not define the related dummy function.

The drivers internal code has already check the related return value, so it is the architecture's duty to 'tell' the driver whether support io memory (e.g. define ioremap, but return NULL).

Quoted 33 lines

Quoted 33 lines

Quoted 32 lines

Quoted 32 lines

Quoted 24 lines

Quoted 24 lines

include/asm-generic/io.h +3 -3

index d5afe96..e80331d 100644
@@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
+ * This implementation is for the no-MMU or UML case only... if you have an MMU
  * you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !CONFIG_MMU || CONFIG_UML
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
 	return (void __iomem*) (unsigned long)offset;

@@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 static inline void iounmap(void __iomem *addr)
 {
 }
-#endif /* CONFIG_MMU */
+#endif /* !CONFIG_MMU || CONFIG_UML */
 

Quoted 3 lines

include/asm-generic/io.h static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size) · line 331

Quoted 3 lines

include/asm-generic/io.h +0 -0

 #ifdef CONFIG_HAS_IOPORT
 #ifndef CONFIG_GENERIC_IOMAP

UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies. _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1] to make nandsim work on UML. Maybe this helps.

Quoted 2 lines

Quoted 2 lines

But "no io memory" is not the excuse to not define the related dummy function.

UML has no io memory, period.
Same applies for s390, it also includes asm-generic/io.h in the !CONFIG_PCI case.
UML and s390 are very special here.

Quoted 3 lines

Quoted 3 lines

The drivers internal code has already check the related return value, so it is the architecture's duty to 'tell' the driver whether support io memory (e.g. define ioremap, but return NULL).

It does so already by setting CONFIG_HAS_IOMEM=n

Quoted 33 lines

Quoted 33 lines

Quoted 33 lines

Quoted 32 lines

Quoted 32 lines

Quoted 24 lines

Quoted 24 lines

include/asm-generic/io.h +3 -3

index d5afe96..e80331d 100644
@@ -303,10 +303,10 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
+ * This implementation is for the no-MMU or UML case only... if you have an MMU
  * you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !CONFIG_MMU || CONFIG_UML
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
 	return (void __iomem*) (unsigned long)offset;

@@ -325,7 +325,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 static inline void iounmap(void __iomem *addr)
 {
 }
-#endif /* CONFIG_MMU */
+#endif /* !CONFIG_MMU || CONFIG_UML */
 

Quoted 3 lines

include/asm-generic/io.h static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size) · line 331

Quoted 3 lines

include/asm-generic/io.h +0 -0

 #ifdef CONFIG_HAS_IOPORT
 #ifndef CONFIG_GENERIC_IOMAP

UML has no io memory but a MMU, so I'd argue that you better fix drivers/ptp/ptp_pch.c dependencies. _If_ ptp_pch.c really works without real io memory, you can look what I did in my GENERIC_IO series[1] to make nandsim work on UML. Maybe this helps.

Quoted 7 lines

Quoted 2 lines

Quoted 2 lines

But "no io memory" is not the excuse to not define the related dummy function.

UML has no io memory, period.
Same applies for s390, it also includes asm-generic/io.h in the !CONFIG_PCI case.
UML and s390 are very special here.

Oh, yes, really the same.

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

The drivers internal code has already check the related return value, so it is the architecture's duty to 'tell' the driver whether support io memory (e.g. define ioremap, but return NULL).

Quoted 1 line

It does so already by setting CONFIG_HAS_IOMEM=n

Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/" directory, but can not find the related definition for 'ioremap'.

Is there another declaration or definition way which I don't know ? (maybe it is).

For our case, the ".config" file does not define 'CONFIG_HAS_IOMEM', can I assume it means "CONFIG_HAS_IOMEM=n" ?

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

The drivers internal code has already check the related return value, so it is the architecture's duty to 'tell' the driver whether support io memory (e.g. define ioremap, but return NULL).

It does so already by setting CONFIG_HAS_IOMEM=n

Quoted 5 lines

Quoted 5 lines

Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/" directory, but can not find the related definition for 'ioremap'.

Is there another declaration or definition way which I don't know ? (maybe it is).

Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because without io memory you cannot have a ioremap().

Quoted 2 lines

Quoted 2 lines

For our case, the ".config" file does not define 'CONFIG_HAS_IOMEM', can I assume it means "CONFIG_HAS_IOMEM=n" ?

If I'm not mistaken it works the other way around. All archs except UML and s390 set CONFIG_HAS_IOMEM=y.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

The drivers internal code has already check the related return value, so it is the architecture's duty to 'tell' the driver whether support io memory (e.g. define ioremap, but return NULL).

It does so already by setting CONFIG_HAS_IOMEM=n

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/" directory, but can not find the related definition for 'ioremap'.

Is there another declaration or definition way which I don't know ? (maybe it is).

Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because without io memory you cannot have a ioremap().

I assume if ioremap() return NULL, it means "without io memory", is it correct ?

If it is correct, "define a dummy ioremap(), and return NULL" is just the meaning that you mentioned above.

If so, for UML, it is not requirement, but recommend to define a dummy ioremap() which return NULL, so can be generic enough to mach all cases.

Am 26.06.2013 10:58, schrieb Chen Gang:

Quoted 18 lines

Quoted 18 lines · Richard Weinberger

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

The drivers internal code has already check the related return value,
so it is the architecture's duty to 'tell' the driver whether support
io memory (e.g. define ioremap, but return NULL).
It does so already by setting CONFIG_HAS_IOMEM=n

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/" directory, but can not find the related definition for 'ioremap'.

Is there another declaration or definition way which I don't know ? (maybe it is).

Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because without io memory you cannot have a ioremap().

I assume if ioremap() return NULL, it means "without io memory", is it correct ?

Quoted 5 lines

Quoted 5 lines

If it is correct, "define a dummy ioremap(), and return NULL" is just the meaning that you mentioned above.

If so, for UML, it is not requirement, but recommend to define a dummy ioremap() which return NULL, so can be generic enough to mach all cases.

No.
Not setting CONFIG_HAS_IOMEM=y means "This arch has no io memory and therefore no functions to mess with it".

Let's get back to the real problem,
drivers/ptp/ptp_pch.c does not build on UML (and I'm very sure also not on S390). Fix the issue by making it depend on HAS_IOMEM.

Btw: Did you actually look at this driver? There is *zero* reason to have it on UML. ..like 99.9% of all other drivers which use io memory.

Quoted 19 lines

Am 26.06.2013 10:58, schrieb Chen Gang:

Quoted 18 lines

Quoted 18 lines · Richard Weinberger

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

The drivers internal code has already check the related return value,
so it is the architecture's duty to 'tell' the driver whether support
io memory (e.g. define ioremap, but return NULL).
It does so already by setting CONFIG_HAS_IOMEM=n

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/" directory, but can not find the related definition for 'ioremap'.

Is there another declaration or definition way which I don't know ? (maybe it is).

Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because without io memory you cannot have a ioremap().

I assume if ioremap() return NULL, it means "without io memory", is it correct ?

Quoted 9 lines

Quoted 5 lines

Quoted 5 lines

If it is correct, "define a dummy ioremap(), and return NULL" is just the meaning that you mentioned above.

If so, for UML, it is not requirement, but recommend to define a dummy ioremap() which return NULL, so can be generic enough to mach all cases.

No.
Not setting CONFIG_HAS_IOMEM=y means "This arch has no io memory and therefore no functions to mess with it".

Since the API itself already contents the meaning: "return NULL means the arch has no related io memory",

Why not define a generic dummy one in "include/asm-generic/io.h" instead of "HAS_IOMEM" (which has already spread many various places, and also, most of new drivers have to know about it).

e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a dummy ioremap() which return NULL ... (also need consider more details).

All together, I think: it is the duty of "asm-generic/io.h" to process this issue, not the duty of many drivers and some architectures.

Quoted 4 lines

Let's get back to the real problem,
drivers/ptp/ptp_pch.c does not build on UML (and I'm very sure also not on S390). Fix the issue by making it depend on HAS_IOMEM.

At least now, it seems it is the only suitable way to fix this issue. :-(

Quoted 2 lines

Btw: Did you actually look at this driver? There is *zero* reason to have it on UML. ..like 99.9% of all other drivers which use io memory.

Excuse me, I did not look at the details of this driver. Maybe it just like you said above.

Quoted 2 lines

Since the API itself already contents the meaning: "return NULL means the arch has no related io memory",

Quoted 6 lines

Why not define a generic dummy one in "include/asm-generic/io.h" instead of "HAS_IOMEM" (which has already spread many various places, and also, most of new drivers have to know about it).

e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a dummy ioremap() which return NULL ... (also need consider more details).

Because we don't even want to build these drivers and not make them fail while executing io memory related functions.

Quoted 2 lines · Richard Weinberger

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Since the API itself already contents the meaning: "return NULL means the arch has no related io memory",

No, NULL means it could not map the I/O memory.

Quoted 9 lines

Quoted 9 lines

Quoted 6 lines

Quoted 6 lines

Why not define a generic dummy one in "include/asm-generic/io.h" instead of "HAS_IOMEM" (which has already spread many various places, and also, most of new drivers have to know about it).

e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a dummy ioremap() which return NULL ... (also need consider more details).

Because we don't even want to build these drivers and not make them fail while executing io memory related functions.

Indeed, it doesn't make sense to build drivers that cannot work. And they may fail in a very bad way.

Quoted 5 lines · Richard Weinberger

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Since the API itself already contents the meaning: "return NULL means the arch has no related io memory",

No, NULL means it could not map the I/O memory.

"it could not map the I/O memory" includes "has no related io memory". So it is enough for our case.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

Quoted 6 lines

Why not define a generic dummy one in "include/asm-generic/io.h" instead of "HAS_IOMEM" (which has already spread many various places, and also, most of new drivers have to know about it).

e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a dummy ioremap() which return NULL ... (also need consider more details).

Quoted 4 lines

Quoted 2 lines

Quoted 2 lines

Because we don't even want to build these drivers and not make them fail while executing io memory related functions.

Indeed, it doesn't make sense to build drivers that cannot work. And they may fail in a very bad way.

That is our 'platform' guys feeling, not the 'module' guys, as 'platform' guys, it is better to provide the choice to 'module' guys, and let them decide by themselves, not forced by us.

Am 26.06.2013 12:01, schrieb Chen Gang:

Quoted 22 lines

Quoted 22 lines · Geert Uytterhoeven

Quoted 5 lines

Quoted 5 lines · Richard Weinberger

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Since the API itself already contents the meaning: "return NULL means
the arch has no related io memory",

No, NULL means it could not map the I/O memory.

"it could not map the I/O memory" includes "has no related io memory". So it is enough for our case.

Quoted 11 lines

Quoted 11 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Why not define a generic dummy one in "include/asm-generic/io.h" instead
of "HAS_IOMEM" (which has already spread many various places, and also,
most of new drivers have to know about it).

e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a
dummy ioremap() which return NULL ... (also need consider more details).

Because we don't even want to build these drivers and not make them fail while executing io memory related functions.

Indeed, it doesn't make sense to build drivers that cannot work. And they may fail in a very bad way.

Quoted 3 lines

Quoted 3 lines

That is our 'platform' guys feeling, not the 'module' guys, as 'platform' guys, it is better to provide the choice to 'module' guys, and let them decide by themselves, not forced by us.

FYI, this is my last reply to this thread.

As Geert and I said, drivers which need io memory have to depend on HAS_IOMEM=y. If an arch does not have io memory these drivers cannot work and therefore we don't want them built.

Quoted 23 lines · Richard Weinberger

Am 26.06.2013 12:01, schrieb Chen Gang:

Quoted 22 lines

Quoted 22 lines · Geert Uytterhoeven

Quoted 5 lines

Quoted 5 lines · Richard Weinberger

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Since the API itself already contents the meaning: "return NULL means
the arch has no related io memory",

No, NULL means it could not map the I/O memory.

"it could not map the I/O memory" includes "has no related io memory". So it is enough for our case.

Quoted 11 lines

Quoted 11 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Why not define a generic dummy one in "include/asm-generic/io.h" instead
of "HAS_IOMEM" (which has already spread many various places, and also,
most of new drivers have to know about it).

e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a
dummy ioremap() which return NULL ... (also need consider more details).

Because we don't even want to build these drivers and not make them fail while executing io memory related functions.

Indeed, it doesn't make sense to build drivers that cannot work. And they may fail in a very bad way.

Quoted 8 lines

Quoted 3 lines

Quoted 3 lines

That is our 'platform' guys feeling, not the 'module' guys, as 'platform' guys, it is better to provide the choice to 'module' guys, and let them decide by themselves, not forced by us.

FYI, this is my last reply to this thread.

As Geert and I said, drivers which need io memory have to depend on HAS_IOMEM=y. If an arch does not have io memory these drivers cannot work and therefore we don't want them built.

It is really the time for stopping discussion, and thank you all for spending your time resources on this discussion.

Next, I will send related patch for it (also cc to you all).

Thanks.

Quoted 27 lines · Chen Gang

Quoted 27 lines · Richard Weinberger

Quoted 26 lines

Quoted 26 lines

Am 26.06.2013 12:01, schrieb Chen Gang:

Quoted 25 lines

Quoted 25 lines

Quoted 25 lines

Quoted 25 lines · Geert Uytterhoeven

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines · Richard Weinberger

Since the API itself already contents the meaning: "return NULL means
the arch has no related io memory",
No, NULL means it could not map the I/O memory.

"it could not map the I/O memory" includes "has no related io memory". So it is enough for our case.

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Why not define a generic dummy one in "include/asm-generic/io.h" instead
of "HAS_IOMEM" (which has already spread many various places, and also,
most of new drivers have to know about it).

e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a
dummy ioremap() which return NULL ... (also need consider more details).

Because we don't even want to build these drivers and not make them fail while
executing io memory related functions.
Indeed, it doesn't make sense to build drivers that cannot work.
And they may fail in a very bad way.

That is our 'platform' guys feeling, not the 'module' guys, as 'platform' guys, it is better to provide the choice to 'module' guys, and let them decide by themselves, not forced by us.

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

Quoted 5 lines

FYI, this is my last reply to this thread.

As Geert and I said, drivers which need io memory have to depend on HAS_IOMEM=y. If an arch does not have io memory these drivers cannot work and therefore we don't want them built.

Hmm, if the modules select 'COMPILE_TEST', it is better to let asm-generic support this features (at least, not forbid it)

So how about the diff below ? ;-)

---------------------------diff begin-----------------------------------

include/asm-generic/io.h +0 -0

index d5afe96..ede3775 100644
@@ -303,13 +303,17 @@ static inline void *phys_to_virt(unsigned long address)
 /*

include/asm-generic/io.h +4 -2

 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
+#if !defined(CONFIG_MMU)
 	return (void __iomem*) (unsigned long)offset;
+#else
+	return NULL;
 }
 
 #define __ioremap(offset, size, flags)	ioremap(offset, size)

@@ -325,7 +329,7 @@ static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 static inline void iounmap(void __iomem *addr)
 {
 }
-#endif /* CONFIG_MMU */
+#endif /* !MMU || (!HAS_IOMEM && COMPILE_TEST) */
 
 #ifdef CONFIG_HAS_IOPORT
 #ifndef CONFIG_GENERIC_IOMAP
 
---------------------------diff end-------------------------------------

Quoted 9 lines

Quoted 9 lines

It is really the time for stopping discussion, and thank you all for spending your time resources on this discussion.

Next, I will send related patch for it (also cc to you all).

Thanks.

-- Chen Gang

'asm-generic' need provide necessary configuration checking, if can't pass checking, 'asm-generic' shouldn't implement it.

For 'COMPILE_TEST', according to its help contents, 'asm-generic' need let it pass configuration checking, and provide related dummy contents for it.

Part of 'COMPLE_TEST' help contents in "init/Kconfig":

"...Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support)..."

Signed-off-by Chen Gang <[email protected]>

Quoted 9 lines · Chen Gang

Quoted 9 lines

'asm-generic' need provide necessary configuration checking, if can't pass checking, 'asm-generic' shouldn't implement it.

For 'COMPILE_TEST', according to its help contents, 'asm-generic' need let it pass configuration checking, and provide related dummy contents for it.

Part of 'COMPLE_TEST' help contents in "init/Kconfig":

Quoted 4 lines

Quoted 4 lines

"...Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support)..."

Signed-off-by Chen Gang <[email protected]>

NAKed-by Geert Uytterhoeven <[email protected]>

Please don't clutter the code with checks for CONFIG_COMPILE_TEST.

Quoted 14 lines · Geert Uytterhoeven

Quoted 13 lines · Chen Gang

Quoted 13 lines

'asm-generic' need provide necessary configuration checking, if can't pass checking, 'asm-generic' shouldn't implement it.

For 'COMPILE_TEST', according to its help contents, 'asm-generic' need let it pass configuration checking, and provide related dummy contents for it.

Part of 'COMPLE_TEST' help contents in "init/Kconfig":

"...Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support)..."

Signed-off-by Chen Gang <[email protected]>

Quoted 3 lines

NAKed-by Geert Uytterhoeven <[email protected]>

Please don't clutter the code with checks for CONFIG_COMPILE_TEST.

Do you mean: 'asm-generic' should not support 'COMPILE_TEST' (the platform should not support 'COMPILE_TEST") ?

Or you mean: 'COMPILE_TEST' should not exist in kernel ?

Quoted 19 lines · Chen Gang

Quoted 19 lines · Geert Uytterhoeven

Quoted 17 lines

Quoted 17 lines · Chen Gang

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

'asm-generic' need provide necessary configuration checking, if can't pass checking, 'asm-generic' shouldn't implement it.

For 'COMPILE_TEST', according to its help contents, 'asm-generic' need let it pass configuration checking, and provide related dummy contents for it.

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Part of 'COMPLE_TEST' help contents in "init/Kconfig":

"...Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support)..."

Signed-off-by Chen Gang <[email protected]>

NAKed-by Geert Uytterhoeven <[email protected]>

Please don't clutter the code with checks for CONFIG_COMPILE_TEST.

Quoted 4 lines

Quoted 4 lines

Do you mean: 'asm-generic' should not support 'COMPILE_TEST' (the platform should not support 'COMPILE_TEST") ?

Or you mean: 'COMPILE_TEST' should not exist in kernel ?

I mean that COMPILE_TEST should exist in Kconfig files only. It's only meant to have more compile coverage, not to "fix" (through #ifdef) more code to make it compile.

Quoted 20 lines · Geert Uytterhoeven

Quoted 19 lines · Chen Gang

Quoted 19 lines · Geert Uytterhoeven

Quoted 17 lines

Quoted 17 lines · Chen Gang

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

'asm-generic' need provide necessary configuration checking, if can't pass checking, 'asm-generic' shouldn't implement it.

For 'COMPILE_TEST', according to its help contents, 'asm-generic' need let it pass configuration checking, and provide related dummy contents for it.

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Part of 'COMPLE_TEST' help contents in "init/Kconfig":

"...Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support)..."

Signed-off-by Chen Gang <[email protected]>

NAKed-by Geert Uytterhoeven <[email protected]>

Please don't clutter the code with checks for CONFIG_COMPILE_TEST.

Quoted 7 lines

Quoted 4 lines

Quoted 4 lines

Do you mean: 'asm-generic' should not support 'COMPILE_TEST' (the platform should not support 'COMPILE_TEST") ?

Or you mean: 'COMPILE_TEST' should not exist in kernel ?

I mean that COMPILE_TEST should exist in Kconfig files only. It's only meant to have more compile coverage, not to "fix" (through #ifdef) more code to make it compile.

If so, can we allow the module to 'COMPILE_TEST' under one platform which not support the related HW ?

e.g. "...Despite they cannot be loaded there (or even when they load
they cannot be used due to missing HW support)...".

'asm-generic' need provide generic layer to users (both architecture guys and module guys).

So for 'default', it can depend on some conditions (e.g. HW support); but for 'generic', it need try to be independent from any conditions.

And it is also necessary for 'generic' to provide the configuration checking features, but this checking must be no negative effect (or consistent) with its 'generic' services.

So it is necessary to check 'NOMMU', 'CONFIG_HAS_IOMEM' ..., but it also necessary to consider about 'COMPILE_TEST' to be consistent with its 'generic' services.

BTW: 20% code are for 80% features, but the left 20% features, need 80% code, if we have to make it complete, we have to face this 'rule'.

Quoted 6 lines · Geert Uytterhoeven

Quoted 2 lines · Chen Gang

Quoted 2 lines · Geert Uytterhoeven

Quoted 1 line

Quoted 1 line · Chen Gang

I mean that COMPILE_TEST should exist in Kconfig files only. It's only meant to have more compile coverage, not to "fix" (through #ifdef) more code to make it compile.

If so, can we allow the module to 'COMPILE_TEST' under one platform which not support the related HW ?

e.g. "...Despite they cannot be loaded there (or even when they load
they cannot be used due to missing HW support)...".

'asm-generic' need provide generic layer to users (both architecture guys and module guys).

So for 'default', it can depend on some conditions (e.g. HW support); but for 'generic', it need try to be independent from any conditions.

And it is also necessary for 'generic' to provide the configuration checking features, but this checking must be no negative effect (or consistent) with its 'generic' services.

So it is necessary to check 'NOMMU', 'CONFIG_HAS_IOMEM' ..., but it also necessary to consider about 'COMPILE_TEST' to be consistent with its 'generic' services.

Quoted 11 lines · Chen Gang

Quoted 11 lines · Geert Uytterhoeven

Quoted 6 lines

Quoted 6 lines · Chen Gang

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines · Geert Uytterhoeven

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line · Chen Gang

I mean that COMPILE_TEST should exist in Kconfig files only. It's only meant to have more compile coverage, not to "fix" (through #ifdef) more code to make it compile.

If so, can we allow the module to 'COMPILE_TEST' under one platform which not support the related HW ?

Quoted 2 lines

Quoted 2 lines

e.g. "...Despite they cannot be loaded there (or even when they load
they cannot be used due to missing HW support)...".

There is a reason why ioremap and the associated functions make no sense on UML, and it remains important to not provide them here so we can find drivers that accidentally use them and are missing a dependency on HAS_IOMEM.

Quoted 2 lines

Quoted 2 lines

'asm-generic' need provide generic layer to users (both architecture guys and module guys).

No. It's a set of examples for the architectures to look at and include if they want to.

Quoted 7 lines

Quoted 7 lines

So for 'default', it can depend on some conditions (e.g. HW support); but for 'generic', it need try to be independent from any conditions.

And it is also necessary for 'generic' to provide the configuration checking features, but this checking must be no negative effect (or consistent) with its 'generic' services.

Quoted 3 lines

Quoted 3 lines

So it is necessary to check 'NOMMU', 'CONFIG_HAS_IOMEM' ..., but it also necessary to consider about 'COMPILE_TEST' to be consistent with its 'generic' services.

The important distinction is between drivers we want to enable in COMPILE_TEST because they are written in a portable way but are just useless if you don't have the hardware, and other drivers that rely on an interface and that should not be built when that interface is not available.

Quoted 12 lines · Arnd Bergmann

Quoted 11 lines · Chen Gang

Quoted 11 lines · Geert Uytterhoeven

Quoted 6 lines

Quoted 6 lines · Chen Gang

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines · Geert Uytterhoeven

Quoted 1 line

Quoted 1 line

Quoted 1 line

On Tue, Jul 2, 2013 at 4:13 AM, Chen Gang <[email protected]> wrote:

I mean that COMPILE_TEST should exist in Kconfig files only. It's only meant to have more compile coverage, not to "fix" (through #ifdef) more code to make it compile.

If so, can we allow the module to 'COMPILE_TEST' under one platform which not support the related HW ?

Quoted 7 lines

Quoted 2 lines

Quoted 2 lines

e.g. "...Despite they cannot be loaded there (or even when they load
they cannot be used due to missing HW support)...".

There is a reason why ioremap and the associated functions make no sense on UML, and it remains important to not provide them here so we can find drivers that accidentally use them and are missing a dependency on HAS_IOMEM.

Yeah, it is necessary for "asm-generic" to provide the configuration checking features (e.g. check HAS_IOMEM, HAS_IOMAP, ...).

And it really make no sense on UML.

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

'asm-generic' need provide generic layer to users (both architecture guys and module guys).

No. It's a set of examples for the architectures to look at and include if they want to.

If really just like what you said, I recommend to use "asm-default" instead of "asm-generic".

And for module guys, they have to use another 'generic' files instead of current 'asm-generic', they really need some 'generic' things to prevent the various definitions/implementations spread into anywhere.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

So for 'default', it can depend on some conditions (e.g. HW support); but for 'generic', it need try to be independent from any conditions.

And it is also necessary for 'generic' to provide the configuration checking features, but this checking must be no negative effect (or consistent) with its 'generic' services.

Quoted 8 lines

Quoted 3 lines

Quoted 3 lines

So it is necessary to check 'NOMMU', 'CONFIG_HAS_IOMEM' ..., but it also necessary to consider about 'COMPILE_TEST' to be consistent with its 'generic' services.

The important distinction is between drivers we want to enable in COMPILE_TEST because they are written in a portable way but are just useless if you don't have the hardware, and other drivers that rely on an interface and that should not be built when that interface is not available.

For user/distributor, they are just like what you said above, but for some of developers ...

The comments of 'COMPILE_TEST' in init/Kconfig:

config COMPILE_TEST
bool "Compile also drivers which will not load"
default n
help
Some drivers can be compiled on a different platform than they are intended to be run on. Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support), developers still, opposing to distributors, might want to build such drivers to compile-test them.

If you are a developer and want to build everything available, say Y here. If you are a user/distributor, say N here to exclude useless drivers to be distributed.

Thanks.

'asm-generic' neither belongs to architectures nor belongs to modules, it provides public services to both modules and architectures.

'COMPILE_TEST=y' will let 'asm-generic' provide self checking sevices to both modules and architectures (especially with allmodconfig and "EXTRA_CFLAGS=-W")

For modules (especially which will run under the specific architecture soon), the developer can find more compiling issues before they really support it.

For architectures, can let modules compile as much as possible (if "COMPILE_TEST=y"), it will give a better check for the architectures.

At present, most of architectures (include various machine/cpu in an architecture) can not pass compiling with 'allmodconfig'. One of the main reasons is the HW of the specific architecture does not support.

It is neither architectures issue nor modules issue, the root cause is:
"now, 'asm-generic' doesn't provide the related necessary public services for it".

confused,

Quoted 1 line · Greg KH

Quoted 1 line

confused,

Good. I thought I was the only one. Confusion loves company, that way we can follow each other around in endless circles.

Quoted 3 lines · Chen Gang

Quoted 3 lines

'COMPILE_TEST=y' will let 'asm-generic' provide self checking sevices to both modules and architectures (especially with allmodconfig and "EXTRA_CFLAGS=-W")

No it doesn't.

Quoted 3 lines

Quoted 3 lines

For modules (especially which will run under the specific architecture soon), the developer can find more compiling issues before they really support it.

Huh?

Quoted 10 lines

Quoted 10 lines

For architectures, can let modules compile as much as possible (if "COMPILE_TEST=y"), it will give a better check for the architectures.

At present, most of architectures (include various machine/cpu in an architecture) can not pass compiling with 'allmodconfig'. One of the main reasons is the HW of the specific architecture does not support.

It is neither architectures issue nor modules issue, the root cause is:
"now, 'asm-generic' doesn't provide the related necessary public services for it".

That's not what asm-generic is for at all.

Hmm... at least, it is neither architectures issue nor modules issue.

So we have to look for who have duty for it, since it is a 'generic' issue for many architectures and modules, we have to find it in 'generic' area (e.g. "./include/*").

At least now, it seems only "asm-generic/*" can play the unlucky role !!

Or, do you think it is still the modules issue themselves ?

Quoted 6 lines · Chen Gang

Quoted 6 lines

Hmm... at least, it is neither architectures issue nor modules issue.

So we have to look for who have duty for it, since it is a 'generic' issue for many architectures and modules, we have to find it in 'generic' area (e.g. "./include/*").

Quoted 3 lines

Quoted 3 lines

At least now, it seems only "asm-generic/*" can play the unlucky role !!

Or, do you think it is still the modules issue themselves ?

What problem are you trying to solve? asm-generic has been around for a long time, and so has allmodconfig. I'm unaware of any issues with either of them.

But as my last email got blocked because your ISP thinks my ISP is a spambot (it's road runner, which I'm sure there are spammers), you wont get this anyway.

Select "COMPILE_TEST=y" with allmodconfig, but can not pass compiling in many architectures, one of the most reasons is "HW does not support".

'asm-generic' is really existent for a long time, and make an important role for both architectures and modules.

Quoted 3 lines · Chen Gang F T

Quoted 3 lines

Select "COMPILE_TEST=y" with allmodconfig, but can not pass compiling in many architectures, one of the most reasons is "HW does not support".

Quoted 3 lines

Quoted 3 lines

'asm-generic' is really existent for a long time, and make an important role for both architectures and modules.

The purpose of asm-generic is to add a standard infrastructure that some archs may be able to optimize. It's not so that all archs can compile all modules.

I'm still confused by what you are trying to accomplish.

Hmm..., I think maybe also has another way: get rid of 'COMPILE_TEST' (regress the related patch, which is only existent in next-* tree).

Or could you provide your suggestions or completions about it ?

Thanks.

Quoted 1 line

I'm still confused by what you are trying to accomplish.

Currently, I am trying to compile all architectures with allmodconfig in next-* tree (which will have "COMPILE_TEST=y").

So I can find and solve the related issues (I am one of contributors).

Quoted 2 lines · Chen Gang F T

Quoted 2 lines

Hmm..., I think maybe also has another way: get rid of 'COMPILE_TEST' (regress the related patch, which is only existent in next-* tree).

I'm not working on linux-next at the moment. Hmm, I'm not even working on mainline at the moment, the kernel I have is still 3.10-rc5.

Quoted 5 lines

Quoted 5 lines

Or could you provide your suggestions or completions about it ?

Thanks.

Quoted 7 lines

Quoted 7 lines

Quoted 1 line

Quoted 1 line

I'm still confused by what you are trying to accomplish.

Currently, I am trying to compile all architectures with allmodconfig in next-* tree (which will have "COMPILE_TEST=y").

So I can find and solve the related issues (I am one of contributors).

So, you want all archs to pass an allmodconfig?

Well, one thing is, if a module doesn't build for an arch, then why not keep that module from building for that arch.

If module foo.ko doesn't build for arch bazinga, then just add in the Kconfig for the module foo:

config FOO
depends on !BAZINGA

Then that module wont build for the specific arch, and all are happy. If someone someday wants to support module foo for arch bazinga, then they can fix module foo for that arch.

Please see the related comment in "init/Kconfig" of next-* tree.

config COMPILE_TEST
bool "Compile also drivers which will not load" default n

Quoted 1 line · Chen Gang F T

Quoted 1 line

Please see the related comment in "init/Kconfig" of next-* tree.

This is now in Linus's tree for 3.11.

Quoted 3 lines

Quoted 3 lines

config COMPILE_TEST
bool "Compile also drivers which will not load" default n

This has _nothing_ to do with asm-generic, sorry. Please don't confuse the issue.

Quoted 5 lines · Greg KH

Quoted 3 lines

Quoted 3 lines

config COMPILE_TEST
bool "Compile also drivers which will not load" default n

This has _nothing_ to do with asm-generic, sorry. Please don't confuse the issue.

But when I choose allmodconfig, then "COMPILE_TEST=y". this may allow a module to compile under the architectures which no related HW support.

When cause compiling issue (HW not support), it is not module's issue, we can not 'fix' module by force, and it is not architecture's issue, either.

So we have to look for who has duty for this issue. At least now, it seems only 'asm-generic' can be qualified to play this unlucky role.

Quoted 14 lines · Chen Gang

Quoted 5 lines · Greg KH

Quoted 3 lines

Quoted 3 lines

config COMPILE_TEST
bool "Compile also drivers which will not load" default n

This has _nothing_ to do with asm-generic, sorry. Please don't confuse the issue.

But when I choose allmodconfig, then "COMPILE_TEST=y". this may allow a module to compile under the architectures which no related HW support.

When cause compiling issue (HW not support), it is not module's issue, we can not 'fix' module by force, and it is not architecture's issue, either.

Quoted 2 lines

So we have to look for who has duty for this issue. At least now, it seems only 'asm-generic' can be qualified to play this unlucky role.

You seem to not understand what asm-generic is, or does, or I still do not understand what you are proposing.

Please send a patch showing what you are trying to do here, so that we can properly understand.

--------------------------patch begin----------------------------------

'asm-generic' need provide necessary configuration checking, if can't pass checking, 'asm-generic' shouldn't implement it.

For 'COMPILE_TEST', according to its help contents, 'asm-generic' need let it pass configuration checking, and provide related dummy contents for it.

Part of 'COMPLE_TEST' help contents in "init/Kconfig":

"...Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support)..."

One sample for using 'COMPILE_TEST':

'PTP_1588_CLOCK_PCH' in drivers/ptp/Kconfig, which need depend on 'HAS_IOMEM'.

Signed-off-by Chen Gang <[email protected]>

---
 include/asm-generic/io.h |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

include/asm-generic/io.h +4 -3

index d5afe96..301ce80 100644
@@ -303,13 +303,18 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
- * you'll need to provide your own definitions.
+ * This for the no-MMU, or no-IOMEM but still try to COMPILE_TEST cases.
+ * if you have an MMU and IOMEM, you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !defined(CONFIG_MMU) || \
+	(!defined(CONFIG_HAS_IOMEM) && defined(CONFIG_COMPILE_TEST))

Quoted 11 lines · Chen Gang

Quoted 11 lines

--------------------------patch begin----------------------------------

'asm-generic' need provide necessary configuration checking, if can't pass checking, 'asm-generic' shouldn't implement it.

For 'COMPILE_TEST', according to its help contents, 'asm-generic' need let it pass configuration checking, and provide related dummy contents for it.

Part of 'COMPLE_TEST' help contents in "init/Kconfig":

Quoted 5 lines

Quoted 5 lines

"...Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support)..."

One sample for using 'COMPILE_TEST':

'PTP_1588_CLOCK_PCH' in drivers/ptp/Kconfig, which need depend on 'HAS_IOMEM'.

Then please submit a patch that adds the 'depends on HAS_IOMEM' line there. That line was clearly left out by accident.

Quoted 30 lines

Quoted 30 lines

Signed-off-by Chen Gang <[email protected]>

---
 include/asm-generic/io.h |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

include/asm-generic/io.h +8 -3

index d5afe96..301ce80 100644
@@ -303,13 +303,18 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
- * you'll need to provide your own definitions.
+ * This for the no-MMU, or no-IOMEM but still try to COMPILE_TEST cases.
+ * if you have an MMU and IOMEM, you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !defined(CONFIG_MMU) || \
+	(!defined(CONFIG_HAS_IOMEM) && defined(CONFIG_COMPILE_TEST))
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
+#if !defined(CONFIG_MMU)
 	return (void __iomem*) (unsigned long)offset;
+#else
+	return NULL;
+#endif
 }
 

Quoted 1 line

include/asm-generic/io.h static inline void *phys_to_virt(unsigned long address) · line 320

Quoted 1 line

include/asm-generic/io.h +0 -0

 #define __ioremap(offset, size, flags)	ioremap(offset, size)

This is wrong for multiple reasons, all of which have been discussed in this thread before.

Quoted 13 lines · Arnd Bergmann

Quoted 11 lines · Chen Gang

Quoted 11 lines

--------------------------patch begin----------------------------------

'asm-generic' need provide necessary configuration checking, if can't pass checking, 'asm-generic' shouldn't implement it.

For 'COMPILE_TEST', according to its help contents, 'asm-generic' need let it pass configuration checking, and provide related dummy contents for it.

Part of 'COMPLE_TEST' help contents in "init/Kconfig":

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

"...Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support)..."

One sample for using 'COMPILE_TEST':

'PTP_1588_CLOCK_PCH' in drivers/ptp/Kconfig, which need depend on 'HAS_IOMEM'.

Then please submit a patch that adds the 'depends on HAS_IOMEM' line there. That line was clearly left out by accident.

Yes, I will send the related patch for it (I have sent one, but that seems incorrect, I will send patch v2 for that, after this patch finishes discussing).

But excluding 'PTP_1588_CLOCK_PCH' own issue, it is as a sample for our discussion (If "COMPILE_TEST=y", it should can be compiled under the archs which no 'HAS_IOMEM').

Quoted 30 lines

Quoted 30 lines

Quoted 30 lines

Signed-off-by Chen Gang <[email protected]>

---
 include/asm-generic/io.h |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

include/asm-generic/io.h +8 -3

index d5afe96..301ce80 100644
@@ -303,13 +303,18 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
- * you'll need to provide your own definitions.
+ * This for the no-MMU, or no-IOMEM but still try to COMPILE_TEST cases.
+ * if you have an MMU and IOMEM, you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !defined(CONFIG_MMU) || \
+	(!defined(CONFIG_HAS_IOMEM) && defined(CONFIG_COMPILE_TEST))
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
+#if !defined(CONFIG_MMU)
 	return (void __iomem*) (unsigned long)offset;
+#else
+	return NULL;
+#endif
 }
 

Quoted 3 lines

Quoted 1 line

include/asm-generic/io.h static inline void *phys_to_virt(unsigned long address) · line 320

Quoted 1 line

include/asm-generic/io.h +0 -0

 #define __ioremap(offset, size, flags)	ioremap(offset, size)

This is wrong for multiple reasons, all of which have been discussed in this thread before.

Hmm..., COMPILE_TEST has integrated into 3.11 (at least can be found in next tree).

When a module select "COMPILE_TEST=y" (e.g with allmodconfig), it has right to compile under the architecture which no related HW support.

If it can not pass compiling, at least it is not the module's issue, neither the architecture's issue.

We have to look for who has duty on it. At least now, it seems only 'asm-generic' can be qualified to play this unlucky role.

Quoted 66 lines · Chen Gang F T

Quoted 66 lines · Arnd Bergmann

Quoted 13 lines

Quoted 13 lines · Chen Gang

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

Quoted 12 lines

--------------------------patch begin----------------------------------

'asm-generic' need provide necessary configuration checking, if can't pass checking, 'asm-generic' shouldn't implement it.

For 'COMPILE_TEST', according to its help contents, 'asm-generic' need let it pass configuration checking, and provide related dummy contents for it.

Part of 'COMPLE_TEST' help contents in "init/Kconfig":

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

"...Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support)..."

One sample for using 'COMPILE_TEST':

'PTP_1588_CLOCK_PCH' in drivers/ptp/Kconfig, which need depend on 'HAS_IOMEM'.

Then please submit a patch that adds the 'depends on HAS_IOMEM' line there. That line was clearly left out by accident.

Yes, I will send the related patch for it (I have sent one, but that seems incorrect, I will send patch v2 for that, after this patch finishes discussing).

But excluding 'PTP_1588_CLOCK_PCH' own issue, it is as a sample for our discussion (If "COMPILE_TEST=y", it should can be compiled under the archs which no 'HAS_IOMEM').

Quoted 33 lines

Quoted 33 lines

Quoted 30 lines

Quoted 30 lines

Quoted 30 lines

Quoted 30 lines

Signed-off-by Chen Gang <[email protected]>

---
 include/asm-generic/io.h |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

include/asm-generic/io.h +8 -3

index d5afe96..301ce80 100644
@@ -303,13 +303,18 @@ static inline void *phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  *
- * This implementation is for the no-MMU case only... if you have an MMU
- * you'll need to provide your own definitions.
+ * This for the no-MMU, or no-IOMEM but still try to COMPILE_TEST cases.
+ * if you have an MMU and IOMEM, you'll need to provide your own definitions.
  */
-#ifndef CONFIG_MMU
+#if !defined(CONFIG_MMU) || \
+	(!defined(CONFIG_HAS_IOMEM) && defined(CONFIG_COMPILE_TEST))
 static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
 {
+#if !defined(CONFIG_MMU)
 	return (void __iomem*) (unsigned long)offset;
+#else
+	return NULL;
+#endif
 }
 

Quoted 1 line

Quoted 1 line

Quoted 1 line

include/asm-generic/io.h static inline void *phys_to_virt(unsigned long address) · line 320

Quoted 1 line

include/asm-generic/io.h +0 -0

 #define __ioremap(offset, size, flags)	ioremap(offset, size)

This is wrong for multiple reasons, all of which have been discussed in this thread before.

Quoted 5 lines

Quoted 5 lines

Hmm..., COMPILE_TEST has integrated into 3.11 (at least can be found in next tree).

When a module select "COMPILE_TEST=y" (e.g with allmodconfig), it has right to compile under the architecture which no related HW support.

That is not true at all, and is not what COMPILE_TEST means.

Quoted 2 lines

Quoted 2 lines

If it can not pass compiling, at least it is not the module's issue, neither the architecture's issue.

What?

Quoted 2 lines

Quoted 2 lines

We have to look for who has duty on it. At least now, it seems only 'asm-generic' can be qualified to play this unlucky role.

Huh?

Look, asm-generic is not what you think it is it seems, nor is COMPILE_TEST, which has caused this whole mess of a thread. Please start over, learn what asm-generic is there for, and used for today. Same goes for COMPILE_TEST.

I'm done with this thread, it's madness...

I'm done with this thread, it's madness..

When a module select "COMPILE_TEST=y" (e.g with allmodconfig), it has right to compile under the architecture which no related HW support.

If it can not pass compiling, at least it is not the module's issue, neither the architecture's issue.

We have to look for who has duty on it. At least now, it seems only 'asm-generic' can be qualified to play this unlucky role.

Could you provide your suggestions or completions for this issue ?

Quoted 4 lines · On Fri, 05 Jul 2013 08:03:31 +0800 Chen Gang F T…

Quoted 4 lines

When a module select "COMPILE_TEST=y" (e.g with allmodconfig), it has right to compile under the architecture which no related HW support.

Quoted 5 lines

Quoted 5 lines

If it can not pass compiling, at least it is not the module's issue, neither the architecture's issue.

We have to look for who has duty on it. At least now, it seems only 'asm-generic' can be qualified to play this unlucky role.

You keep saying this, but others have told you that this is not the problem.

Quoted 1 line

Quoted 1 line

Could you provide your suggestions or completions for this issue ?

If something doesn't build for a particular config, then either it needs to be fixed or excluded from building in that particular config.

Quoted 5 lines · Stephen Rothwell

Quoted 4 lines · On Fri, 05 Jul 2013 08:03:31 +0800 Chen Gang F T…

Quoted 4 lines

When a module select "COMPILE_TEST=y" (e.g with allmodconfig), it has right to compile under the architecture which no related HW support.

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

If it can not pass compiling, at least it is not the module's issue, neither the architecture's issue.

We have to look for who has duty on it. At least now, it seems only 'asm-generic' can be qualified to play this unlucky role.

You keep saying this, but others have told you that this is not the problem.

In real world, it is not the problem.

But for 'mad users' (e.g. allmodconfig, randconfig, and me too), they have not provided enough reason for it (prove that is not a problem for 'mad users').

Quoted 3 lines

Quoted 1 line

Quoted 1 line

Could you provide your suggestions or completions for this issue ?

If something doesn't build for a particular config, then either it needs to be fixed or excluded from building in that particular config.

I agree with you, if get rid of 'COMPILE_TEST'.

Thanks.

Hello All:

It seems 'asm-generic' dislikes 'mad users' (e.g allmodconfig, randconfig, and me).

I guess the main reason is: 'asm-generic' thinks what mad users talk about is useless in real world, so it is just noisy.

I can understand, at least what I talk about is not for urgent things. (maybe 'asm-generic' also thinks it not important either, every members have their own opinions).

Next, I still use allmodconfig/randconfig for some architectures which I am interested in (and also for learning compilers), but I will skip all things which are related with 'asm-generic', since it dislikes me (a mad user).

Quoted 8 lines · Chen Gang F T

Quoted 8 lines

Hello All:

It seems 'asm-generic' dislikes 'mad users' (e.g allmodconfig, randconfig, and me).

I guess the main reason is: 'asm-generic' thinks what mad users talk about is useless in real world, so it is just noisy.

Quoted 8 lines

Quoted 8 lines

I can understand, at least what I talk about is not for urgent things. (maybe 'asm-generic' also thinks it not important either, every members have their own opinions).

Next, I still use allmodconfig/randconfig for some architectures which I am interested in (and also for learning compilers), but I will skip all things which are related with 'asm-generic', since it dislikes me (a mad user).

As the asm-generic maintainer I think I have to clarify a few things:

* Build-time fixes for warnings and randconfig errors are good, and you have sent a number of good bug fixes all over the kernel, I definitely welcome getting more of those. In many cases the fix is trivial and obvious, in other cases you need to know much more of the background to understand what the underlying problem is and why you should not just fix the symptom.

* You have made an (understandable) mistake with this particular patch. It would have been nice if you had not made it, but I can see that you are still learning about these things. There is a fine line between what makes sense to be enabled as a 'compile test' and what should better be left disabled by Kconfig.

* When experienced developers tell you that you are mistaken, you need to make an effort to understand what the mistake was so you can learn from it and not make the same mistake again. If you make the same mistakes again, maintainers will get annoyed and ignore you (or worse), which is not a good situation to be in when you want to get your patches merged.

Quoted 14 lines · Chen Gang

Quoted 5 lines · Greg KH

Quoted 3 lines

Quoted 3 lines

config COMPILE_TEST
bool "Compile also drivers which will not load" default n

This has _nothing_ to do with asm-generic, sorry. Please don't confuse the issue.

But when I choose allmodconfig, then "COMPILE_TEST=y". this may allow a module to compile under the architectures which no related HW support.

When cause compiling issue (HW not support), it is not module's issue, we can not 'fix' module by force, and it is not architecture's issue, either.

Quoted 2 lines

So we have to look for who has duty for this issue. At least now, it seems only 'asm-generic' can be qualified to play this unlucky role.

You seem to not understand what asm-generic is, or does, or I still do not understand what you are proposing.

Please send a patch showing what you are trying to do here, so that we can properly understand.

Quoted 18 lines · Greg KH

Quoted 18 lines · Chen Gang

Quoted 16 lines

Quoted 16 lines · Greg KH

Quoted 5 lines

Quoted 5 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

config COMPILE_TEST
bool "Compile also drivers which will not load" default n

This has _nothing_ to do with asm-generic, sorry. Please don't confuse the issue.

But when I choose allmodconfig, then "COMPILE_TEST=y". this may allow a module to compile under the architectures which no related HW support.

When cause compiling issue (HW not support), it is not module's issue, we can not 'fix' module by force, and it is not architecture's issue, either.

So we have to look for who has duty for this issue. At least now, it seems only 'asm-generic' can be qualified to play this unlucky role.

Quoted 5 lines

Quoted 5 lines

You seem to not understand what asm-generic is, or does, or I still do not understand what you are proposing.

Please send a patch showing what you are trying to do here, so that we can properly understand.

The patch is in the first email of this thread:
https://lkml.org/lkml/2013/7/1/641

Am 26.06.2013 11:33, schrieb Chen Gang:

Quoted 33 lines

Quoted 33 lines · Richard Weinberger

Quoted 22 lines

Quoted 22 lines

Am 26.06.2013 10:58, schrieb Chen Gang:

Quoted 21 lines

Quoted 21 lines

Quoted 21 lines

Quoted 21 lines · Richard Weinberger

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

The drivers internal code has already check the related return value,
so it is the architecture's duty to 'tell' the driver whether support
io memory (e.g. define ioremap, but return NULL).
It does so already by setting CONFIG_HAS_IOMEM=n

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Excuse me, I use "grep -rn ioremap *" under "include/" and "arch/um/"
directory, but can not find the related definition for 'ioremap'.

Is there another declaration or definition way which I don't know ?
(maybe it is).
Both UML and s390 (in the !CONFIG_PCI) do not define ioremap() because
without io memory you cannot have a ioremap().

I assume if ioremap() return NULL, it means "without io memory", is it correct ?

If it is correct, "define a dummy ioremap(), and return NULL" is just the meaning that you mentioned above.

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

If so, for UML, it is not requirement, but recommend to define a dummy ioremap() which return NULL, so can be generic enough to mach all cases.

No.
Not setting CONFIG_HAS_IOMEM=y means "This arch has no io memory and therefore no functions to mess with it".

Since the API itself already contents the meaning: "return NULL means the arch has no related io memory",

Quoted 6 lines

Quoted 6 lines

Why not define a generic dummy one in "include/asm-generic/io.h" instead of "HAS_IOMEM" (which has already spread many various places, and also, most of new drivers have to know about it).

e.g: in "include/asm-generic/io.h", if "CONFIG_HAS_IOMEM=n", define a dummy ioremap() which return NULL ... (also need consider more details).

Because we don't even want to build these drivers and not make them fail while executing io memory related functions.