From: Richard Weinberger <richard@nod.at>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
Rob Herring <robh@kernel.org>, Lee Jones <lee.jones@linaro.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
user-mode-linux-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/2] asm-generic/io.h: provide default ioremap/iounmap for !HAS_IOMEM
Date: Wed, 30 Mar 2016 10:13:53 +0200 [thread overview]
Message-ID: <56FB8AC1.4070209@nod.at> (raw)
In-Reply-To: <6746826.YZLTqKTgkv@wuerfel>
Am 30.03.2016 um 10:04 schrieb Arnd Bergmann:
> On Wednesday 30 March 2016 09:50:22 Richard Weinberger wrote:
>> Am 29.03.2016 um 22:13 schrieb Rob Herring:
>>>>> Reuse the !MMU variants for !HAS_IOMEM as they are sufficient for our
>>>>> needs. This fixes build errors for UM allyesconfig:
>>>>>
>>>>> drivers/mfd/syscon.c:89:2: error: implicit declaration of function 'iounmap' [-Werror=implicit-function-declaration]
>>>>> iounmap(base);
>>>>>
>>>>> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
>>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>>> Cc: linux-arch@vger.kernel.org
>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>>
>>>> (adding Richard and the UML list to cc)
>>>>
>>>> I actually prototyped a patch that did the opposite: remove the readl/writel/...
>>>> definitions when HAS_IOMEM is unset. I didn't get far enough to submit it,
>>>> but see below for what I did.
>>>
>>> Ewww. Why do the opposite of what we do for every other Kconfig symbol
>>> which is provide empty functions? It really only functions as a
>>> disable on UML flag which runs counter to enabling drivers to build
>>> for all arches.
>>>
>>> I actually started a patch to remove the HAS_IOMEM dependency
>>> everywhere (or just the per driver cases). It didn't break as bad as I
>>> expected, but became more than I wanted to fix. Mainly, all the devm_
>>> variants also need empty versions or to be always enabled.
>>
>> The root cause of the problem is COMPILE_TEST. People who use it need to
>> get forced to think about the consequences.
>> COMPILE_TEST means that the driver will also be build on not so fancy archs
>> like UML, s390 and m68k where you don't have IOMEM or also DMA in every
>> possible configuration.
>> So a quick build test on x86 is not sufficient.
>>
>> This is why I'm absolutely not a fan of having stubs for iounmap() an friends
>> for UML. It only tries to bypass the root cause.
>> What is next? Stubs for DMA? PCI? For everything else that does not build?
>>
>> With COMPILE_TEST we have created a monster and now we have to deal with it in terms of
>> having correct dependencies when COMPILE_TEST is being used.
>>
>
> One way out of this would be accept that UML is just too different
> from the other architectures, and make COMPILE_TEST itself depend
> on !UML.
>
> COMPILE_TEST is highly valuable for me, because it means that all the
> ARM specific drivers can now be build-tested on x86 and they show up
> for anyone doing allmodconfig builds. If we introduce a regression
> in any of the drivers, it gets caught much quicker because many
> subsystem maintainers run an x86 allmodconfig build after pulling
> downstream maintainer trees. There is also stuff like Coverity that
> only ever runs on x86.
>
> It does make sense to have COMPILE_TEST imply that we are able
> to build things on all architectures, not just x86 in addition to
> the platform a driver is meant for, but the value of compile-testing
> a random driver with UML in particular approaches zero, and instead
> causes lots of work.
I fully understand your point of view. COMPILE_TEST is a monster that
can do the heavy lifting for you, but monsters also have claws and fangs. ;-)
Having COMPILE_TEST having depend on !UML works for me. But don't
we have other archs without io mem? At least a few years ago while
porting nandsim to UML I found s390 that lacks of io mem too.
Maybe we depend COMPILE_TEST on HAS_IOMEM?
Thanks,
//richard
------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140
WARNING: multiple messages have this Message-ID (diff)
From: Richard Weinberger <richard@nod.at>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh@kernel.org>, Lee Jones <lee.jones@linaro.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
user-mode-linux-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/2] asm-generic/io.h: provide default ioremap/iounmap for !HAS_IOMEM
Date: Wed, 30 Mar 2016 10:13:53 +0200 [thread overview]
Message-ID: <56FB8AC1.4070209@nod.at> (raw)
Message-ID: <20160330081353.mcADRxwQvpfJ4y0s_hOu1j6BURnl719R9tUHqTF8UF4@z> (raw)
In-Reply-To: <6746826.YZLTqKTgkv@wuerfel>
Am 30.03.2016 um 10:04 schrieb Arnd Bergmann:
> On Wednesday 30 March 2016 09:50:22 Richard Weinberger wrote:
>> Am 29.03.2016 um 22:13 schrieb Rob Herring:
>>>>> Reuse the !MMU variants for !HAS_IOMEM as they are sufficient for our
>>>>> needs. This fixes build errors for UM allyesconfig:
>>>>>
>>>>> drivers/mfd/syscon.c:89:2: error: implicit declaration of function 'iounmap' [-Werror=implicit-function-declaration]
>>>>> iounmap(base);
>>>>>
>>>>> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
>>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>>> Cc: linux-arch@vger.kernel.org
>>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>>
>>>> (adding Richard and the UML list to cc)
>>>>
>>>> I actually prototyped a patch that did the opposite: remove the readl/writel/...
>>>> definitions when HAS_IOMEM is unset. I didn't get far enough to submit it,
>>>> but see below for what I did.
>>>
>>> Ewww. Why do the opposite of what we do for every other Kconfig symbol
>>> which is provide empty functions? It really only functions as a
>>> disable on UML flag which runs counter to enabling drivers to build
>>> for all arches.
>>>
>>> I actually started a patch to remove the HAS_IOMEM dependency
>>> everywhere (or just the per driver cases). It didn't break as bad as I
>>> expected, but became more than I wanted to fix. Mainly, all the devm_
>>> variants also need empty versions or to be always enabled.
>>
>> The root cause of the problem is COMPILE_TEST. People who use it need to
>> get forced to think about the consequences.
>> COMPILE_TEST means that the driver will also be build on not so fancy archs
>> like UML, s390 and m68k where you don't have IOMEM or also DMA in every
>> possible configuration.
>> So a quick build test on x86 is not sufficient.
>>
>> This is why I'm absolutely not a fan of having stubs for iounmap() an friends
>> for UML. It only tries to bypass the root cause.
>> What is next? Stubs for DMA? PCI? For everything else that does not build?
>>
>> With COMPILE_TEST we have created a monster and now we have to deal with it in terms of
>> having correct dependencies when COMPILE_TEST is being used.
>>
>
> One way out of this would be accept that UML is just too different
> from the other architectures, and make COMPILE_TEST itself depend
> on !UML.
>
> COMPILE_TEST is highly valuable for me, because it means that all the
> ARM specific drivers can now be build-tested on x86 and they show up
> for anyone doing allmodconfig builds. If we introduce a regression
> in any of the drivers, it gets caught much quicker because many
> subsystem maintainers run an x86 allmodconfig build after pulling
> downstream maintainer trees. There is also stuff like Coverity that
> only ever runs on x86.
>
> It does make sense to have COMPILE_TEST imply that we are able
> to build things on all architectures, not just x86 in addition to
> the platform a driver is meant for, but the value of compile-testing
> a random driver with UML in particular approaches zero, and instead
> causes lots of work.
I fully understand your point of view. COMPILE_TEST is a monster that
can do the heavy lifting for you, but monsters also have claws and fangs. ;-)
Having COMPILE_TEST having depend on !UML works for me. But don't
we have other archs without io mem? At least a few years ago while
porting nandsim to UML I found s390 that lacks of io mem too.
Maybe we depend COMPILE_TEST on HAS_IOMEM?
Thanks,
//richard
next prev parent reply other threads:[~2016-03-30 8:13 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-29 18:23 [PATCH 1/2] asm-generic/io.h: provide default ioremap/iounmap for !HAS_IOMEM Rob Herring
2016-03-29 18:23 ` [PATCH 2/2] mfd: remove dependency on HAS_IOMEM Rob Herring
2016-03-30 4:23 ` Krzysztof Kozlowski
2016-03-29 19:37 ` [PATCH 1/2] asm-generic/io.h: provide default ioremap/iounmap for !HAS_IOMEM Arnd Bergmann
2016-03-29 19:50 ` Richard Weinberger
2016-03-29 20:13 ` Rob Herring
2016-03-30 7:50 ` Richard Weinberger
2016-03-30 8:04 ` Arnd Bergmann
2016-03-30 8:04 ` [uml-devel] " Arnd Bergmann
2016-03-30 8:13 ` Richard Weinberger [this message]
2016-03-30 8:13 ` Richard Weinberger
2016-03-30 10:03 ` Arnd Bergmann
2016-03-30 13:29 ` Rob Herring
2016-03-30 13:51 ` Arnd Bergmann
2016-03-30 20:08 ` Geert Uytterhoeven
2016-03-30 20:35 ` Richard Weinberger
2016-03-30 20:38 ` Arnd Bergmann
2016-03-30 21:20 ` Rob Herring
2016-03-31 8:39 ` Geert Uytterhoeven
2016-03-30 4:19 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56FB8AC1.4070209@nod.at \
--to=richard@nod.at \
--cc=arnd@arndb.de \
--cc=lee.jones@linaro.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh@kernel.org \
--cc=user-mode-linux-devel@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.