2013-06-26 06:32:15

by Chen Gang

[permalink] [raw]
Subject: [PATCH] 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(-)

diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index b30f34a..a34ea5d 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -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
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index d5afe96..e80331d 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -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 */

#ifdef CONFIG_HAS_IOPORT
#ifndef CONFIG_GENERIC_IOMAP
--
1.7.7.6


2013-06-26 06:54:45

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

Hi!

Am 26.06.2013 08:31, schrieb Chen Gang:
> 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(-)
>
> diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
> index b30f34a..a34ea5d 100644
> --- a/arch/um/include/asm/Kbuild
> +++ b/arch/um/include/asm/Kbuild
> @@ -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.

> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index d5afe96..e80331d 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -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 */
>
> #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.

Thanks,
//richard

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-February/039701.html

2013-06-26 07:38:51

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On Wed, Jun 26, 2013 at 8:54 AM, Richard Weinberger <[email protected]> wrote:
>> -#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)

> 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.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2013-06-26 07:56:57

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 06/26/2013 02:54 PM, Richard Weinberger wrote:
> Hi!
>
> Am 26.06.2013 08:31, schrieb Chen Gang:
>> 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(-)
>>
>> diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
>> index b30f34a..a34ea5d 100644
>> --- a/arch/um/include/asm/Kbuild
>> +++ b/arch/um/include/asm/Kbuild
>> @@ -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.

>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>> index d5afe96..e80331d 100644
>> --- a/include/asm-generic/io.h
>> +++ b/include/asm-generic/io.h
>> @@ -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 */
>>
>> #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).

So all together, I recommend the fix like below

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

diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index b30f34a..b282042 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -1,5 +1,5 @@
generic-y += bug.h cputime.h device.h emergency-restart.h futex.h hardirq.h
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 += ftrace.h pci.h param.h delay.h mutex.h current.h exec.h
generic-y += switch_to.h clkdev.h
generic-y += trace_clock.h
diff --git a/arch/um/include/asm/io.h b/arch/um/include/asm/io.h
new file mode 100644
index 0000000..00f3cd8
--- /dev/null
+++ b/arch/um/include/asm/io.h
@@ -0,0 +1,21 @@
+#ifndef UML_IO_H
+#define UML_IO_H
+
+#include <asm-generic/io.h>
+
+/*
+ * UML does not support io memory, so return NULL.
+ */
+static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
+{
+ return NULL;
+}
+
+#define ioremap_nocache ioremap
+#define ioremap_wc ioremap_nocache
+
+static inline void iounmap(void __iomem *addr)
+{
+}
+
+#endif /* UML_IO_H */

--------------------------diff end------------------------------------


Thanks.
--
Chen Gang

Asianux Corporation

2013-06-26 07:57:30

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 06/26/2013 03:38 PM, Geert Uytterhoeven wrote:
> On Wed, Jun 26, 2013 at 8:54 AM, Richard Weinberger <[email protected]> wrote:
>>> >> -#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)
>

Oh, really it is. :-)


Thanks.
--
Chen Gang

Asianux Corporation

2013-06-26 08:05:39

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

Hi!

Am 26.06.2013 09:56, schrieb Chen Gang:
> On 06/26/2013 02:54 PM, Richard Weinberger wrote:
>> Hi!
>>
>> Am 26.06.2013 08:31, schrieb Chen Gang:
>>> 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(-)
>>>
>>> diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
>>> index b30f34a..a34ea5d 100644
>>> --- a/arch/um/include/asm/Kbuild
>>> +++ b/arch/um/include/asm/Kbuild
>>> @@ -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.
>
>>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>>> index d5afe96..e80331d 100644
>>> --- a/include/asm-generic/io.h
>>> +++ b/include/asm-generic/io.h
>>> @@ -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 */
>>>
>>> #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.

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.


> 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

Thanks,
//richard

2013-06-26 08:35:36

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 06/26/2013 04:05 PM, Richard Weinberger wrote:
>>>> >>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>>>> >>> index d5afe96..e80331d 100644
>>>> >>> --- a/include/asm-generic/io.h
>>>> >>> +++ b/include/asm-generic/io.h
>>>> >>> @@ -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 */
>>>> >>>
>>>> >>> #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.
> 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.

>> > 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

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" ?


Thanks
--
Chen Gang

Asianux Corporation

2013-06-26 08:39:58

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

Am 26.06.2013 10:34, schrieb Chen Gang:
> On 06/26/2013 04:05 PM, Richard Weinberger wrote:
>>>>>>>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>>>>>>>> index d5afe96..e80331d 100644
>>>>>>>> --- a/include/asm-generic/io.h
>>>>>>>> +++ b/include/asm-generic/io.h
>>>>>>>> @@ -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 */
>>>>>>>>
>>>>>>>> #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.
>> 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.
>
>>>> 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
>
> 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().

> 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.

Thanks,
//richard

2013-06-26 08:59:01

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>> >>>> 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
>> >
>> > 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.


>> > 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.

I guess so, too.


Thanks.
--
Chen Gang

Asianux Corporation

2013-06-26 09:04:06

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

Am 26.06.2013 10:58, schrieb Chen Gang:
> On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>>>>>>> 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
>>>>
>>>> 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.

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.

Thanks,
//richard

2013-06-26 09:34:07

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 06/26/2013 05:03 PM, Richard Weinberger wrote:
> Am 26.06.2013 10:58, schrieb Chen Gang:
>> > On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>>>>>>>> >>>>>>>>>> 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
>>>>> >>>>
>>>>> >>>> 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.
> 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.


> 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. :-(


> 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.


Thanks.
--
Chen Gang

Asianux Corporation

2013-06-26 09:38:28

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

Am 26.06.2013 11:33, schrieb Chen Gang:
> On 06/26/2013 05:03 PM, Richard Weinberger wrote:
>> Am 26.06.2013 10:58, schrieb Chen Gang:
>>>> On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>>>>>>>>>>>>>>>>>>> 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
>>>>>>>>>>
>>>>>>>>>> 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.
>> 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).

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

>
> 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.
>
>
>> 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. :-(
>
>
>> 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.

It is.

Thanks,
//richard

2013-06-26 09:48:22

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On Wed, Jun 26, 2013 at 11:38 AM, Richard Weinberger <[email protected]> wrote:
>> 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.

>> 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.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2013-06-26 09:55:54

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 06/26/2013 05:38 PM, Richard Weinberger wrote:
> Am 26.06.2013 11:33, schrieb Chen Gang:
>> > On 06/26/2013 05:03 PM, Richard Weinberger wrote:
>>> >> Am 26.06.2013 10:58, schrieb Chen Gang:
>>>>> >>>> On 06/26/2013 04:39 PM, Richard Weinberger wrote:
>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> 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
>>>>>>>>>>> >>>>>>>>>>
>>>>>>>>>>> >>>>>>>>>> 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.
>>> >> 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).
> Because we don't even want to build these drivers and not make them fail while
> executing io memory related functions.
>

It is one reason, but it seems not quite enough for it, it depends on
our requirements.

If we are also the 'platform' of all kernel modules (not only for user
mode), and "include/asm-generic/*" plays an important role for it.

It is the modules their own choice to determine which architectures they
want to support, and which architecture they won't, not need be directed
or forced by 'platform'.

Also better to let kernel modules free to determine whether build in or
not, when actually do not support the specific features of architecture
currently, not need be directed or forced by 'platform'.


Thanks.
--
Chen Gang

Asianux Corporation

2013-06-26 10:01:57

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 06/26/2013 05:48 PM, Geert Uytterhoeven wrote:
> On Wed, Jun 26, 2013 at 11:38 AM, Richard Weinberger <[email protected]> wrote:
>>> >> 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.

>>> >> 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.


Thanks.
--
Chen Gang

Asianux Corporation

2013-06-26 10:17:25

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

Am 26.06.2013 12:01, schrieb Chen Gang:
> On 06/26/2013 05:48 PM, Geert Uytterhoeven wrote:
>> On Wed, Jun 26, 2013 at 11:38 AM, Richard Weinberger <[email protected]> wrote:
>>>>>> 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.
>
>>>>>> 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.

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.

Over and out,
//richard

2013-06-26 10:23:04

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 06/26/2013 06:17 PM, Richard Weinberger wrote:
> Am 26.06.2013 12:01, schrieb Chen Gang:
>> > On 06/26/2013 05:48 PM, Geert Uytterhoeven wrote:
>>> >> On Wed, Jun 26, 2013 at 11:38 AM, Richard Weinberger <[email protected]> wrote:
>>>>>>> >>>>>> 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.
>> >
>>>>>>> >>>>>> 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.
> 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.
--
Chen Gang

Asianux Corporation

2013-07-01 01:41:19

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 06/26/2013 06:22 PM, Chen Gang wrote:
> On 06/26/2013 06:17 PM, Richard Weinberger wrote:
>> Am 26.06.2013 12:01, schrieb Chen Gang:
>>>> On 06/26/2013 05:48 PM, Geert Uytterhoeven wrote:
>>>>>> On Wed, Jun 26, 2013 at 11:38 AM, Richard Weinberger <[email protected]> wrote:
>>>>>>>>>>>>>> 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.
>>>>
>>>>>>>>>>>>>> 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.
>> 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-----------------------------------

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index d5afe96..ede3775 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -303,13 +303,17 @@ 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(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;
}

#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-------------------------------------

> 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

2013-07-01 03:44:54

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add 'UML' case just like 'no-MMU'

On 07/01/2013 09:40 AM, Chen Gang wrote:
> On 06/26/2013 06:22 PM, Chen Gang wrote:
>> > On 06/26/2013 06:17 PM, Richard Weinberger wrote:
>>> >> Am 26.06.2013 12:01, schrieb Chen Gang:
>>>>> >>>> On 06/26/2013 05:48 PM, Geert Uytterhoeven wrote:
>>>>>>> >>>>>> On Wed, Jun 26, 2013 at 11:38 AM, Richard Weinberger <[email protected]> wrote:
>>>>>>>>>>>>>>> >>>>>>>>>>>>>> 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.
>>>>> >>>>
>>>>>>>>>>>>>>> >>>>>>>>>>>>>> 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.
>>> >> 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-----------------------------------
>
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index d5afe96..ede3775 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -303,13 +303,17 @@ 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(COMPILE_TEST))

Need use 'CONFIG_COMPILE_TEST' instead of '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;
> }
>
> #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-------------------------------------
>
>> > 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
>


--
Chen Gang

2013-07-02 02:14:53

by Chen Gang

[permalink] [raw]
Subject: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

'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]>
---
include/asm-generic/io.h | 22 ++++++++++++++++++----
1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index d5afe96..301ce80 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -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
}

#define __ioremap(offset, size, flags) ioremap(offset, size)
@@ -325,7 +330,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_HAS_IOMEM && CONFIG_COMPILE_TEST) */

#ifdef CONFIG_HAS_IOPORT
#ifndef CONFIG_GENERIC_IOMAP
@@ -341,6 +346,15 @@ static inline void ioport_unmap(void __iomem *p)
extern void __iomem *ioport_map(unsigned long port, unsigned int nr);
extern void ioport_unmap(void __iomem *p);
#endif /* CONFIG_GENERIC_IOMAP */
+#elif defined(CONFIG_COMPILE_TEST) /* CONFIG_HAS_IOPORT */
+static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
+{
+ return NULL;
+}
+
+static inline void ioport_unmap(void __iomem *p)
+{
+}
#endif /* CONFIG_HAS_IOPORT */

#ifndef xlate_dev_kmem_ptr
--
1.7.7.6

2013-07-02 07:19:54

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Tue, Jul 2, 2013 at 4:13 AM, Chen Gang <[email protected]> wrote:
> '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]>

NAKed-by: Geert Uytterhoeven <[email protected]>

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

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2013-07-02 08:01:09

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/02/2013 03:19 PM, Geert Uytterhoeven wrote:
> On Tue, Jul 2, 2013 at 4:13 AM, Chen Gang <[email protected]> wrote:
>> > '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]>
> 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 ?

Can we use 'asm-default' instead of 'asm-generic', since "if HW support,
it will provide default value, or it will provide nothing" ?


Thanks.
--
Chen Gang

2013-07-02 10:57:18

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Tue, Jul 2, 2013 at 10:00 AM, Chen Gang <[email protected]> wrote:
> On 07/02/2013 03:19 PM, Geert Uytterhoeven wrote:
>> On Tue, Jul 2, 2013 at 4:13 AM, Chen Gang <[email protected]> wrote:
>>> > '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]>
>> 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 ?

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.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2013-07-03 00:52:49

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/02/2013 06:57 PM, Geert Uytterhoeven wrote:
> On Tue, Jul 2, 2013 at 10:00 AM, Chen Gang <[email protected]> wrote:
>> > On 07/02/2013 03:19 PM, Geert Uytterhoeven wrote:
>>> >> On Tue, Jul 2, 2013 at 4:13 AM, Chen Gang <[email protected]> wrote:
>>>>> >>> > '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]>
>>> >> 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 ?
> 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'.


Thanks.
--
Chen Gang

2013-07-03 01:27:41

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/03/2013 08:51 AM, Chen Gang wrote:
> On 07/02/2013 06:57 PM, Geert Uytterhoeven wrote:
>> On Tue, Jul 2, 2013 at 10:00 AM, Chen Gang <[email protected]> wrote:
>>>> On 07/02/2013 03:19 PM, Geert Uytterhoeven wrote:
>>>>>> On Tue, Jul 2, 2013 at 4:13 AM, Chen Gang <[email protected]> wrote:
>>>>>>>>>> '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]>
>>>>>> 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 ?
>> 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'.
>
>

It is necessary to let the 'COMPILE_TEST' related members to know about
it (better to provide their own opinions), it may be helpful for our
discussion, so I list the details below.

-------------------------------commit begin----------------------------------

commit 4bb1667255a86360721291fe59991d033bbc2f2a
Author: Jiri Slaby <[email protected]>
Date: Wed May 22 10:56:24 2013 +0200

build some drivers only when compile-testing

Some drivers can be built on more platforms than they run on. This is
a burden for users and distributors who package a kernel. They have to
manually deselect some (for them useless) drivers when updating their
configs via oldconfig. And yet, sometimes it is even impossible to
disable the drivers without patching the kernel.

Introduce a new config option COMPILE_TEST and make all those drivers
to depend on the platform they run on, or on the COMPILE_TEST option.
Now, when users/distributors choose COMPILE_TEST=n they will not have
the drivers in their allmodconfig setups, but developers still can
compile-test them with COMPILE_TEST=y.

Now the drivers where we use this new option:
* PTP_1588_CLOCK_PCH: The PCH EG20T is only compatible with Intel Atom
processors so it should depend on x86.
* FB_GEODE: Geode is 32-bit only so only enable it for X86_32.
* USB_CHIPIDEA_IMX: The OF_DEVICE dependency will be met on powerpc
systems -- which do not actually support the hardware via that
method.
* INTEL_MID_PTI: It is specific to the Penwell type of Intel Atom
device.

[v2]
* remove EXPERT dependency

[gregkh - remove chipidea portion, as it's incorrect, and also doesn't
apply to my driver-core tree]

Signed-off-by: Jiri Slaby <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Jeff Mahoney <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: [email protected]
Cc: Florian Tobias Schandinat <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Richard Cochran <[email protected]>
Cc: [email protected]
Cc: Ben Hutchings <[email protected]>
Cc: "Keller, Jacob E" <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

diff --git a/init/Kconfig b/init/Kconfig
index 55ccdf6..1e825c2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -53,6 +53,20 @@ config CROSS_COMPILE
need to set this unless you want the configured kernel build
directory to select the cross-compiler automatically.

+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.
+

-------------------------------commit end------------------------------------

Thanks.
--
Chen Gang

2013-07-03 08:14:50

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Wednesday 03 July 2013, Chen Gang wrote:
> On 07/02/2013 06:57 PM, Geert Uytterhoeven wrote:
> > On Tue, Jul 2, 2013 at 10:00 AM, Chen Gang <[email protected]> wrote:
> >> > On 07/02/2013 03:19 PM, Geert Uytterhoeven wrote:
> >>> >> 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 ?
>
> 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.

> '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.

> 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.

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.

Arnd

2013-07-03 08:45:05

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/03/2013 04:14 PM, Arnd Bergmann wrote:
> On Wednesday 03 July 2013, Chen Gang wrote:
>> > On 07/02/2013 06:57 PM, Geert Uytterhoeven wrote:
>>> > > On Tue, Jul 2, 2013 at 10:00 AM, Chen Gang <[email protected]> wrote:
>>>>> > >> > On 07/02/2013 03:19 PM, Geert Uytterhoeven wrote:
>>>>>>> > >>> >> 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 ?
>> >
>> > 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.

>> > '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.

>> > 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.
> 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.
--
Chen Gang

2013-07-04 00:58:31

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

'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".


Thanks.

On 07/03/2013 04:43 PM, Chen Gang wrote:
> On 07/03/2013 04:14 PM, Arnd Bergmann wrote:
>> On Wednesday 03 July 2013, Chen Gang wrote:
>>>> On 07/02/2013 06:57 PM, Geert Uytterhoeven wrote:
>>>>>> On Tue, Jul 2, 2013 at 10:00 AM, Chen Gang <[email protected]> wrote:
>>>>>>>>>> On 07/02/2013 03:19 PM, Geert Uytterhoeven wrote:
>>>>>>>>>>>>>> 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 ?
>>>>
>>>> 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.
>
>>>> '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.
>
>>>> 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.
>> 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.
>


--
Chen Gang

2013-07-04 01:11:34

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Thu, Jul 04, 2013 at 08:57:34AM +0800, Chen Gang wrote:
> 'asm-generic' neither belongs to architectures nor belongs to modules,
> it provides public services to both modules and architectures.

That sentence does not make any sense to me.

> '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.

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

Huh?

> 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.

confused,

greg k-h

2013-07-04 01:23:30

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Wed, 2013-07-03 at 18:12 -0700, Greg KH wrote:

> confused,

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

-- Steve

2013-07-04 01:51:40

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/04/2013 09:12 AM, Greg KH wrote:
> On Thu, Jul 04, 2013 at 08:57:34AM +0800, Chen Gang wrote:
>> > '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.
>

"If add 'COMPILE_TEST=y' to 'asm-generic', it will provide self checking
services to both modules and architectures"

Is it correct ?

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

For developers, they may has hobby to let their code pass compiling as
much as possible in the integrating environments, although the modules
may not really load/run.


>> > 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 ?


Thanks.
--
Chen Gang

2013-07-04 02:03:14

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Thu, 2013-07-04 at 09:49 +0800, Chen Gang wrote:

> 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 ?

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.

-- Steve

2013-07-04 02:12:03

by Chen Gang F T

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/04/2013 10:03 AM, Steven Rostedt wrote:
> On Thu, 2013-07-04 at 09:49 +0800, Chen Gang wrote:
>
>> > 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 ?
> 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.
>

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.

> 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.

Oh, sorry for not reply the original mail, and did not see in my backup
mail, now, I can use my backup mail to continue.


Thanks.
--
Chen Gang

2013-07-04 02:21:03

by Chen Gang F T

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/04/2013 09:23 AM, Steven Rostedt wrote:
> On Wed, 2013-07-03 at 18:12 -0700, Greg KH wrote:
>
>> > confused,
> Good. I thought I was the only one. Confusion loves company, that way we
> can follow each other around in endless circles.

If you think, this mail has already make noises to many other members,
please reply mail next, which only contents the related members. And I
will follow with the address which you provide.

Thanks.
--
Chen Gang

2013-07-04 02:29:42

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Thu, 2013-07-04 at 10:10 +0800, Chen Gang F T wrote:

> 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.
>

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.

-- Steve

2013-07-04 02:44:14

by Chen Gang F T

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/04/2013 10:29 AM, Steven Rostedt wrote:
> On Thu, 2013-07-04 at 10:10 +0800, Chen Gang F T wrote:
>
>> > 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.
>> >
> 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.
>

Yes, I can understand.

But at present, it seems only 'asm-generic' can play the unlucky role
for current issues (at least, it is neither architectures issue nor
modules issue).

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.

> 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).

One of main issues is about it.


Thanks.
--
Chen Gang

2013-07-04 03:06:48

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Thu, 2013-07-04 at 10:42 +0800, Chen Gang F T wrote:

> 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.

>
> Or could you provide your suggestions or completions about it ?
>
> Thanks.
>
> > 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.

-- Steve

2013-07-04 03:28:22

by Chen Gang F T

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/04/2013 11:06 AM, Steven Rostedt wrote:
> On Thu, 2013-07-04 at 10:42 +0800, Chen Gang F T wrote:
>
>> > 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.
>

OK, thanks. I can understand.

Every contributors have their own focus areas, each area is valuable enough to go deeper and deeper.

>> >
>> > Or could you provide your suggestions or completions about it ?
>> >
>> > Thanks.
>> >
>>> > > 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?
>

Yeah, that is my current goal.

By this way, I can find more issues and try to solve them (it will be
good for public kernel), and also I can familiar the compiler step by
step (the cross-compilers also has their issues).

(In fact, I also want randconfig, and has already done for some
architectures). ;-)


> Well, one thing is, if a module doesn't build for an arch, then why not
> keep that module from building 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
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.


> 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.

If get rid of 'COMPILE_TEST', what you said above are reasonable.

When one module select "COMPILE_TEST=y", if it can not pass compiling
because of HW not support, it is not the module's issue, at least.

Hmm..., but at least for me, I still think, "COMPILE_TEST=y" is really
useful.


Thanks.
--
Chen Gang

2013-07-04 04:08:12

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Thu, Jul 04, 2013 at 11:26:53AM +0800, Chen Gang F T wrote:
> Please see the related comment in "init/Kconfig" of next-* tree.

This is now in Linus's tree for 3.11.

> 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.

greg k-h

2013-07-04 04:51:27

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/04/2013 12:08 PM, Greg KH wrote:
> On Thu, Jul 04, 2013 at 11:26:53AM +0800, Chen Gang F T wrote:
>> > Please see the related comment in "init/Kconfig" of next-* tree.
> This is now in Linus's tree for 3.11.
>

OK, thanks (at least for me, it is a good news).

>> > 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.


Thanks.
--
Chen Gang

2013-07-04 06:11:15

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Thu, Jul 04, 2013 at 12:50:31PM +0800, Chen Gang wrote:
> On 07/04/2013 12:08 PM, Greg KH wrote:
> >> > 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.

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.

greg k-h

2013-07-04 06:36:16

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/04/2013 02:12 PM, Greg KH wrote:
> On Thu, Jul 04, 2013 at 12:50:31PM +0800, Chen Gang wrote:
>> On 07/04/2013 12:08 PM, Greg KH wrote:
>>>>> 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.
>
> You seem to not understand what asm-generic is, or does, or I still do
> not understand what you are proposing.
>

Maybe both/neither of us. :-)

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

Please help to check the patch below, thanks.

--------------------------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(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index d5afe96..301ce80 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -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
}

#define __ioremap(offset, size, flags) ioremap(offset, size)
@@ -325,7 +330,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_HAS_IOMEM && CONFIG_COMPILE_TEST) */

#ifdef CONFIG_HAS_IOPORT
#ifndef CONFIG_GENERIC_IOMAP
@@ -341,6 +346,15 @@ static inline void ioport_unmap(void __iomem *p)
extern void __iomem *ioport_map(unsigned long port, unsigned int nr);
extern void ioport_unmap(void __iomem *p);
#endif /* CONFIG_GENERIC_IOMAP */
+#elif defined(CONFIG_COMPILE_TEST) /* CONFIG_HAS_IOPORT */
+static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
+{
+ return NULL;
+}
+
+static inline void ioport_unmap(void __iomem *p)
+{
+}
#endif /* CONFIG_HAS_IOPORT */

#ifndef xlate_dev_kmem_ptr
--
1.7.7.6



--------------------------patch end------------------------------------


Thanks.
--
Chen Gang

--
Chen Gang

2013-07-04 08:09:19

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Thu, Jul 4, 2013 at 8:12 AM, Greg KH <[email protected]> wrote:
> On Thu, Jul 04, 2013 at 12:50:31PM +0800, Chen Gang wrote:
>> On 07/04/2013 12:08 PM, Greg KH wrote:
>> >> > 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.
>
> 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

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2013-07-04 09:25:53

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Thursday 04 July 2013, Chen Gang wrote:

> --------------------------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'.

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

> Signed-off-by: Chen Gang <[email protected]>
> ---
> include/asm-generic/io.h | 22 ++++++++++++++++++----
> 1 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index d5afe96..301ce80 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -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
> }
>
> #define __ioremap(offset, size, flags) ioremap(offset, size)

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

NAK

2013-07-05 00:04:39

by Chen Gang F T

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/04/2013 05:25 PM, Arnd Bergmann wrote:
> On Thursday 04 July 2013, Chen Gang wrote:
>
>> > --------------------------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'.
> 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').


>> > Signed-off-by: Chen Gang <[email protected]>
>> > ---
>> > include/asm-generic/io.h | 22 ++++++++++++++++++----
>> > 1 files changed, 18 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>> > index d5afe96..301ce80 100644
>> > --- a/include/asm-generic/io.h
>> > +++ b/include/asm-generic/io.h
>> > @@ -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
>> > }
>> >
>> > #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.


Could you provide your suggestions or completions for this issue ?


Thanks.
--
Chen Gang

2013-07-05 00:11:30

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/04/2013 04:09 PM, Geert Uytterhoeven wrote:
> On Thu, Jul 4, 2013 at 8:12 AM, Greg KH <[email protected]> wrote:
>> > On Thu, Jul 04, 2013 at 12:50:31PM +0800, Chen Gang wrote:
>>> >> On 07/04/2013 12:08 PM, Greg KH wrote:
>>>>>> >> >> > 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.
>> >
>> > 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

Yeah, check history directly may be more precise.


Thanks.
--
Chen Gang

2013-07-05 00:11:52

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Fri, Jul 05, 2013 at 08:03:31AM +0800, Chen Gang F T wrote:
> On 07/04/2013 05:25 PM, Arnd Bergmann wrote:
> > On Thursday 04 July 2013, Chen Gang wrote:
> >
> >> > --------------------------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'.
> > 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').
>
>
> >> > Signed-off-by: Chen Gang <[email protected]>
> >> > ---
> >> > include/asm-generic/io.h | 22 ++++++++++++++++++----
> >> > 1 files changed, 18 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> >> > index d5afe96..301ce80 100644
> >> > --- a/include/asm-generic/io.h
> >> > +++ b/include/asm-generic/io.h
> >> > @@ -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
> >> > }
> >> >
> >> > #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.

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

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

What?

> 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...

greg k-h

2013-07-05 00:15:00

by Stephen Rothwell

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

Hi,

On Fri, 05 Jul 2013 08:03:31 +0800 Chen Gang F T <[email protected]> wrote:
>
> 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.

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

> 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.

--
Cheers,
Stephen Rothwell [email protected]


Attachments:
(No filename) (847.00 B)
(No filename) (836.00 B)
Download all attachments

2013-07-05 00:36:16

by Chen Gang

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/05/2013 08:12 AM, Greg KH wrote:
> On Fri, Jul 05, 2013 at 08:03:31AM +0800, Chen Gang F T wrote:
>> > On 07/04/2013 05:25 PM, Arnd Bergmann wrote:
>>> > > On Thursday 04 July 2013, Chen Gang wrote:
>>> > >
>>>>> > >> > --------------------------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'.
>>> > > 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').
>> >
>> >
>>>>> > >> > Signed-off-by: Chen Gang <[email protected]>
>>>>> > >> > ---
>>>>> > >> > include/asm-generic/io.h | 22 ++++++++++++++++++----
>>>>> > >> > 1 files changed, 18 insertions(+), 4 deletions(-)
>>>>> > >> >
>>>>> > >> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>>>>> > >> > index d5afe96..301ce80 100644
>>>>> > >> > --- a/include/asm-generic/io.h
>>>>> > >> > +++ b/include/asm-generic/io.h
>>>>> > >> > @@ -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
>>>>> > >> > }
>>>>> > >> >
>>>>> > >> > #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.
> That is not true at all, and is not what COMPILE_TEST means.
>

Below is the 'COMPILE_TEST' help contents.

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.

Does a module have no right to choose "COMPILE_TEST=y" under the
architecture which no related HW support ?

Or I misunderstand the help contents ?

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

If it can not pass compiling because of HW not support when
"COMPILE_TEST=y", 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.
> 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.
>

At least, we can not suspend this issue. It is not suitable to fix
module by force (if multiple modules want "COMPILE_TEST=y", we need fix
multiple various times in various areas).


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

We are just discussing, every member has right to reply or not.

If no members reply, I will (of cause) not continue to send repeated
contents.

And excuse me, some contents are really repeated multiple times because
of some of members need it for discussion.


Thanks.
--
Chen Gang

2013-07-05 00:50:00

by Chen Gang F T

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/05/2013 08:14 AM, Stephen Rothwell wrote:
> On Fri, 05 Jul 2013 08:03:31 +0800 Chen Gang F T <[email protected]> wrote:
>> >
>> > 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.
> 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').


>> > 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.
--
Chen Gang

2013-07-05 00:53:35

by Chen Gang F T

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On 07/05/2013 08:12 AM, Greg KH wrote:
> I'm done with this thread, it's madness..

Yeah, especially discussing with 'mad users' (e.g. allmodconfig,
randconfig, and me too). ;-)


Thanks.
--
Chen Gang

2013-07-05 08:03:17

by Chen Gang F T

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

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).

At last, I make an apologize to 'asm-generic' for my mad discussing.

Bye !!


On 07/05/2013 08:48 AM, Chen Gang F T wrote:
> On 07/05/2013 08:14 AM, Stephen Rothwell wrote:
>> On Fri, 05 Jul 2013 08:03:31 +0800 Chen Gang F T <[email protected]> wrote:
>>>>
>>>> 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.
>> 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').
>
>
>>>> 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.
>


--
Chen Gang

2013-07-05 11:14:09

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.

On Friday 05 July 2013, Chen Gang F T wrote:
> 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).

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.

Arnd

2013-07-08 02:12:16

by Chen Gang F T

[permalink] [raw]
Subject: Re: [PATCH] include/asm-generic/io.h: add dummy fuctions to support 'COMPILE_TEST' in 'asm-generic'.


Firstly, thank you very much for your reply.

On 07/05/2013 07:13 PM, Arnd Bergmann wrote:
> On Friday 05 July 2013, Chen Gang F T wrote:
>> > 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).
> 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.
>

Since we (at least my company) has already get benifits from Public Open
Source, we should try to provide the contribution back to Public Open
Source.

Providing contributions to Public Open Source is part of my duty (what
I should do).


> * 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.
>

We have to meet three roles: 'user', 'provider' and 'tester' (which is
may mad user, and not quite familiar with the details).

For 'user', they really need 'COMPILE_TEST', so it should be added into
kernel wide. And 'user' often thinks the 'tester' is just a 'mad user'
for 'tester' offten does "mad things" which will be useless in real
world.

For 'provider', he/she often feels the 'tester' is not familiar the
details (in fact, they really not familiar), but he/she has to discuss
with 'tester' (especially 'user' does not care about it). He/She really
feels it is just boring and waste time.

Instead of discussing the details, 'tester' should only provide the
related proof and only can discus the related API, he/she wants the
'provider' to explain about the proof and make the API clearer to every
one (not only for 'expert').

Now, I think, I may just play a 'tester' role for 'asm-generic'.


> * 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.

'tester' wants the 'provider' to explain the proof:

e.g. 'COMPILE_TEST' help contents whether really say: "can allow 'COMIPLE_TEST=y' in architectures which no related HW support"
e.g. If compile fails because of no HW support with "COMPILE_TEST=y", can we still let it suspending ?
e.g. Does it still count an issue, although in real world, it may not happen, at least now ?

'tester' also wants the 'provider' to explain 'asm-generic':

Arnd said:
"It's a set of examples for the architectures to look at and include if they want to"
Steven saind:
"The purpose of asm-generic is to add a standard infrastructure that some archs may be able to optimize."

(In fact, they are not precisely the same).

Did 'provider' have the related document for it (I guess so, could you provide the related location), thanks ?

according to the definition of 'asm-generic' (either of them):
the 'CONFIG_BUG' need really remove from 'asm-generic', for it is only related with some of architectures, not generic enough for all architectures
(we can continue to discus it in the related thread).
it belongs architectures wide, not kernel wide, is it better to move "./include/asm-generic" to "./arch/asm-default" or "./arch/asm-standard" ?

'tester' also wants to try something (but maybe incorrect).

one sample for string, its value has 4 kinds: 'volatile string', 'const string', '""', 'NULL'. ('""' is not equal to 'NULL').
may we also have the 4 kinds: 'HAS_IOMAP', 'NOMMU', 'COMPILE_TEST', 'N/A' ('COMPILE_TEST' is not equal to 'N/A')?

(this is maybe just boring or wast time, need not reply).


Thanks.
--
Chen Gang