Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
On Mon, 2013-03-11 at 09:33 -0700, Tony Lindgren wrote: * Paul Bolle pebo...@tiscali.nl [130309 11:52]: In the meantime, how do you prefer I solve the (trivial) issue of an useless select for MACH_NOKIA_RM696? Drop that select or add an (equally useless) config entry for MACH_NOKIA_RM696? Or should I try to ignore it for the time being? Just adding the config MACH_NOKIA_RM696 to Kconfig as bool should fix this unless I'm missing something here. Yes, that should do. I already promised to draft that small patch last week. I hope to actually do that shortly. Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
* Paul Bolle pebo...@tiscali.nl [130309 11:52]: On Sat, 2013-03-09 at 00:01 +, Russell King - ARM Linux wrote: However, going back to that MACH_NOKIA_RM696. If there exists only a select of this symbol and no config MACH_NOKIA_RM696 entry, then the symbol will never be generated in the output .config file. [...] My conclusion is... it's a mess. That mess can only be fully cleaned up if the code for the RM-696 that now is maintained in some unknown to me repository gets merged into mainline, can't it? In the meantime, how do you prefer I solve the (trivial) issue of an useless select for MACH_NOKIA_RM696? Drop that select or add an (equally useless) config entry for MACH_NOKIA_RM696? Or should I try to ignore it for the time being? Just adding the config MACH_NOKIA_RM696 to Kconfig as bool should fix this unless I'm missing something here. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
On Sat, 2013-03-09 at 00:01 +, Russell King - ARM Linux wrote:
It's actually quite clever. There's two levels to it.
The first is that CONFIG_MACH_xxx result in their machine_is_xxx() macros
being defined to constant zero if the CONFIG option is not enabled. That
allows the compiler to throw away code for disabled platforms because
the expression is always false.
Otherwise, they end up as (machine_arch_type == MACH_TYPE_xxx).
The second is the magic which happens when two CONFIG_MACH_xxx are
selected. If only one is selected, then machine_arch_type is defined
to the appropriate MACH_TYPE_xxx. This means that the above expression
becomes constant-true, and the conditional is eliminated.
If more than one is selected, then machine_arch_type is defined to a
variable which is appropriately set to one of the MACH_TYPE_xxx values.
At boot?
So, the result is that:
- de-selected platforms have their if (machine_is_xxx()) { } optimised
out of the kernel.
- for a kernel built targetting one platform, all the
if (machine_is_xxx()) tests are optimised away, leaving only the
relevant code behind.
- otherwise, we get the _appropriate_ conditional code for the
configuration generated.
Thanks for clarifying this. Quite clever indeed.
However, going back to that MACH_NOKIA_RM696. If there exists only a
select of this symbol and no config MACH_NOKIA_RM696 entry, then the
symbol will never be generated in the output .config file.
[...]
My conclusion is... it's a mess.
That mess can only be fully cleaned up if the code for the RM-696 that
now is maintained in some unknown to me repository gets merged into
mainline, can't it?
In the meantime, how do you prefer I solve the (trivial) issue of an
useless select for MACH_NOKIA_RM696? Drop that select or add an (equally
useless) config entry for MACH_NOKIA_RM696? Or should I try to ignore it
for the time being?
Paul Bolle
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
When support was added for Nokia N9 (RM-696), with commit 63fc5f3bb3d0ca9ab4767a801b518aa6335f87ad (ARM: OMAP: add minimal support for Nokia RM-696), a select statement for MACH_NOKIA_RM696 was added to the tree. But there's no Kconfig symbol with that name. That symbol would be superfluous, since support for that machine piggybacks on MACH_NOKIA_RM680. So drop that select. Signed-off-by: Paul Bolle pebo...@tiscali.nl --- 0) Tested with git grep. 1) Some searching on the web didn't return a config MACH_NOKIA_RM696. So apparently there's not even a development tree that uses this symbol. arch/arm/mach-omap2/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index 49ac3df..5cafa77 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -296,7 +296,6 @@ config MACH_NOKIA_RM680 bool Nokia N950 (RM-680) / N9 (RM-696) phones depends on ARCH_OMAP3 default y - select MACH_NOKIA_RM696 select OMAP_PACKAGE_CBB config MACH_NOKIA_RX51 -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
On Fri, Mar 08, 2013 at 11:29:56AM +0100, Paul Bolle wrote: When support was added for Nokia N9 (RM-696), with commit 63fc5f3bb3d0ca9ab4767a801b518aa6335f87ad (ARM: OMAP: add minimal support for Nokia RM-696), a select statement for MACH_NOKIA_RM696 was added to the tree. But there's no Kconfig symbol with that name. That symbol would be superfluous, since support for that machine piggybacks on MACH_NOKIA_RM680. So drop that select. This is needed because of arch/arm/tools/mach-types. See include/generated/mach-types.h. If you have just CONFIG_MACH_NOKIA_RM680 and run the kernel on RM-696, then machine_is_nokia_rm696() will return false. If I rememeber correctly, this broke at least early printk / uncompressor output at the time. I guess people may still want to use machine_is_... macros e.g. for debugging. Signed-off-by: Paul Bolle pebo...@tiscali.nl --- 0) Tested with git grep. git grep won't search generated source files. 1) Some searching on the web didn't return a config MACH_NOKIA_RM696. So apparently there's not even a development tree that uses this symbol. You can see machine_is_nokia_rm696() used in the public kernel source for Nokia N9 product. :-) A. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
* Aaro Koskinen aaro.koski...@iki.fi [130308 08:16]: On Fri, Mar 08, 2013 at 11:29:56AM +0100, Paul Bolle wrote: When support was added for Nokia N9 (RM-696), with commit 63fc5f3bb3d0ca9ab4767a801b518aa6335f87ad (ARM: OMAP: add minimal support for Nokia RM-696), a select statement for MACH_NOKIA_RM696 was added to the tree. But there's no Kconfig symbol with that name. That symbol would be superfluous, since support for that machine piggybacks on MACH_NOKIA_RM680. So drop that select. This is needed because of arch/arm/tools/mach-types. See include/generated/mach-types.h. If you have just CONFIG_MACH_NOKIA_RM680 and run the kernel on RM-696, then machine_is_nokia_rm696() will return false. If I rememeber correctly, this broke at least early printk / uncompressor output at the time. I guess people may still want to use machine_is_... macros e.g. for debugging. I think the righ fix is to just add config MACH_NOKIA_RM680 bool to the mach-omap2/Kconfig like we have for n8x0 also. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
On Fri, 2013-03-08 at 18:11 +0200, Aaro Koskinen wrote:
On Fri, Mar 08, 2013 at 11:29:56AM +0100, Paul Bolle wrote:
When support was added for Nokia N9 (RM-696), with commit
63fc5f3bb3d0ca9ab4767a801b518aa6335f87ad (ARM: OMAP: add minimal
support for Nokia RM-696), a select statement for MACH_NOKIA_RM696 was
added to the tree. But there's no Kconfig symbol with that name. That
symbol would be superfluous, since support for that machine piggybacks
on MACH_NOKIA_RM680. So drop that select.
This is needed because of arch/arm/tools/mach-types. See
include/generated/mach-types.h.
How does that file actually need this select statement?
If you have just CONFIG_MACH_NOKIA_RM680 and run the kernel on RM-696,
then machine_is_nokia_rm696() will return false. If I rememeber correctly,
this broke at least early printk / uncompressor output at the time.
I guess people may still want to use machine_is_... macros e.g. for
debugging.
What in the current mainline kernel cares about a
machine_is_nokia_rm696() macro?
$ git grep -n machine_is_nokia v3.9-rc1
v3.9-rc1:arch/arm/mach-omap2/board-n8x0.c:349: if (machine_is_nokia_n800()
|| slot == 0)
v3.9-rc1:arch/arm/mach-omap2/board-n8x0.c:395: if
(machine_is_nokia_n800()) {
v3.9-rc1:arch/arm/mach-omap2/board-n8x0.c:428: if (machine_is_nokia_n800())
v3.9-rc1:arch/arm/mach-omap2/board-n8x0.c:451: if
(machine_is_nokia_n800()) {
v3.9-rc1:arch/arm/mach-omap2/board-n8x0.c:478: if (machine_is_nokia_n800())
v3.9-rc1:arch/arm/mach-omap2/board-n8x0.c:493: if
(machine_is_nokia_n810()) {
v3.9-rc1:arch/arm/mach-omap2/board-n8x0.c:544: if
(machine_is_nokia_n810()) {
v3.9-rc1:arch/arm/mach-omap2/board-n8x0.c:562: if
(machine_is_nokia_n810()) {
v3.9-rc1:arch/arm/mach-omap2/board-n8x0.c:714: if (machine_is_nokia_n810())
v3.9-rc1:arch/arm/mach-omap2/board-rx51-video.c:71: if
(!machine_is_nokia_rx51())
v3.9-rc1:drivers/usb/host/ohci-omap.c:267: } else if
(machine_is_nokia770()) {
v3.9-rc1:sound/soc/omap/n810.c:309: if (!(machine_is_nokia_n810() ||
machine_is_nokia_n810_wimax()))
v3.9-rc1:sound/soc/omap/rx51.c:400: if (!machine_is_nokia_rx51())
$ git grep -n rm696 v3.9-rc1
v3.9-rc1:arch/arm/tools/mach-types:589:nokia_rm696
MACH_NOKIA_RM696NOKIA_RM696 3522
I don't find any actual usage of that macro. Are its uses also
generated?
0) Tested with git grep.
git grep won't search generated source files.
Of course. That's one of the reasons why I explicitly state that I only
tested it that way.
1) Some searching on the web didn't return a config MACH_NOKIA_RM696.
So apparently there's not even a development tree that uses this symbol.
You can see machine_is_nokia_rm696() used in the public kernel source
for Nokia N9 product. :-)
Where would that be? And does that tree actually have a Kconfig entry
MACH_NOKIA_RM696?
Paul Bolle
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
On Fri, 2013-03-08 at 08:35 -0800, Tony Lindgren wrote: I think the righ fix is to just add config MACH_NOKIA_RM680 bool I guess you meant MACH_NOKIA_RM696. to the mach-omap2/Kconfig like we have for n8x0 also. Should I draft a patch? Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
* Paul Bolle pebo...@tiscali.nl [130308 09:24]: On Fri, 2013-03-08 at 08:35 -0800, Tony Lindgren wrote: I think the righ fix is to just add config MACH_NOKIA_RM680 bool I guess you meant MACH_NOKIA_RM696. Ooops yes. to the mach-omap2/Kconfig like we have for n8x0 also. Should I draft a patch? Sure that would be nice. Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
On Fri, 2013-03-08 at 09:55 -0800, Tony Lindgren wrote: * Paul Bolle pebo...@tiscali.nl [130308 09:24]: Should I draft a patch? Sure that would be nice. One thing I couldn't determine is how the generated mach-types.h header handles multiple CONFIG_MACH_* macros. If both CONFIG_MACH_FOO and CONFIG_MACH_BAR are defined, and these both have a line in */mach-types, will the machine_is_foo() and machine_is_bar() macros both behave as one would expect? Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
* Paul Bolle pebo...@tiscali.nl [130308 10:06]: On Fri, 2013-03-08 at 09:55 -0800, Tony Lindgren wrote: * Paul Bolle pebo...@tiscali.nl [130308 09:24]: Should I draft a patch? Sure that would be nice. One thing I couldn't determine is how the generated mach-types.h header handles multiple CONFIG_MACH_* macros. If both CONFIG_MACH_FOO and CONFIG_MACH_BAR are defined, and these both have a line in */mach-types, will the machine_is_foo() and machine_is_bar() macros both behave as one would expect? Yes they do, for the selected ones the macro becomes machine_arch_type == MACH_TYPE_XYZ instead of 0. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: drop select MACH_NOKIA_RM696
On Fri, Mar 08, 2013 at 07:02:44PM +0100, Paul Bolle wrote:
On Fri, 2013-03-08 at 09:55 -0800, Tony Lindgren wrote:
* Paul Bolle pebo...@tiscali.nl [130308 09:24]:
Should I draft a patch?
Sure that would be nice.
One thing I couldn't determine is how the generated mach-types.h header
handles multiple CONFIG_MACH_* macros.
If both CONFIG_MACH_FOO and CONFIG_MACH_BAR are defined, and these both
have a line in */mach-types, will the machine_is_foo() and
machine_is_bar() macros both behave as one would expect?
It's actually quite clever. There's two levels to it.
The first is that CONFIG_MACH_xxx result in their machine_is_xxx() macros
being defined to constant zero if the CONFIG option is not enabled. That
allows the compiler to throw away code for disabled platforms because
the expression is always false.
Otherwise, they end up as (machine_arch_type == MACH_TYPE_xxx).
The second is the magic which happens when two CONFIG_MACH_xxx are
selected. If only one is selected, then machine_arch_type is defined
to the appropriate MACH_TYPE_xxx. This means that the above expression
becomes constant-true, and the conditional is eliminated.
If more than one is selected, then machine_arch_type is defined to a
variable which is appropriately set to one of the MACH_TYPE_xxx values.
So, the result is that:
- de-selected platforms have their if (machine_is_xxx()) { } optimised
out of the kernel.
- for a kernel built targetting one platform, all the
if (machine_is_xxx()) tests are optimised away, leaving only the
relevant code behind.
- otherwise, we get the _appropriate_ conditional code for the
configuration generated.
However, going back to that MACH_NOKIA_RM696. If there exists only a
select of this symbol and no config MACH_NOKIA_RM696 entry, then the
symbol will never be generated in the output .config file.
I too can find no trace of any use of machine_is_nokia_rm696 in the
mainline kernel. So, if there's nothing using the machine_is_()
symbol, and nothing using the CONFIG_MACH_NOKIA_RM696 symbol, then
any select of that is entirely superfluous.
Well, I did this:
$ git grep -i nokia_rm696
arch/arm/mach-omap2/Kconfig:select MACH_NOKIA_RM696
arch/arm/mach-omap2/board-rm680.c:MACHINE_START(NOKIA_RM696, Nokia RM-696
board)
arch/arm/tools/mach-types:nokia_rm696 MACH_NOKIA_RM696
NOKIA_RM696 3522
So, there exists platform support for this device, provided by the RM680
support, but there's no use of the machine_is_xxx() symbol - and if there
was, it would always be false.
My conclusion is... it's a mess.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
