Sorry, hit the wrong key.  'T' and 'Y' are too close together.  (mutt)
----- Forwarded message from Russell King <rmk+lkml@arm.linux.org.uk> -----
Date: Fri, 28 Dec 2007 19:57:24 +0000

From: Russell King <rmk+lkml@arm.linux.org.uk>

To: Adrian Bunk <adrian.bunk@movial.fi>

Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>,

	Randy Dunlap <randy.dunlap@oracle.com>, phil.el@wanadoo.fr,

	oprofile-list@lists.sf.net, linux-kernel@vger.kernel.org

Subject: Re: [2.6.24 patch] restore ARMv6 OProfile support
Given where we are in the release cycle, I think this "cleanup" patch

should be reverted.  Once the issues with it are resolved (why was it

never even CC'd to the arch maintainers for review?) then it can go

into mainline.
Note that it also looks like (from the commitdiff) that the above

mentioned commit also removes:
CONFIG_HARDWARE_PM (blackfin)

CONFIG_OPROFILE_CELL (powerpc)
So they're probably subtly broken as well.
Linus, what do you think?  Should 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9
--

Russell King

 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/

 maintainer of:
----- End forwarded message -----
--

Russell King

 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/

 maintainer of:

--

Well, I have to say that I don't particularly like obviously

architecture-specific stuff in an obviously non-architecture file..
I'd almost prefer to revert the thing that caused the problem, because

with Adrian's patch, I think the end result may *work*, but it's uglier

than what we started out with.
However, I think the *cleanest* solution right now may be something like

the appended. Totally untested, of course. It basically just copies the

generic kernel/Kconfig.instrumentation file into the arm directory, makes

arm use its own instead of the generic one, and removes the dependencies

on ARM in there (including all of the KPROBES entry that apparently isn't

an issue on ARM anyway). It then adds back the ARM-specific ones.
This follows the sacred rules of good code:
 - generic code is either generic or not. If it's not generic, don't claim

   it is.
 - don't *force* people to use generic code if it doesn't suit them. Make

   it available for the cases it makes sense for, but don't shoe-horn it

   into cases where it doesn't work well.
So it allows the sharing of the common case and *many* architectures end

up using the generic Kconfig file, but hey, if it doesn't make sense for

ARM, it doesn't make sense for ARM. It's that simple.
But as mentioned, it's totally untested and I don't have (or really want

to have) a cross-compiling environment. And I don't care *that* much. I

just want something we can all live with.
So does something like this work for people?
		Linus
---

 arch/arm/Kconfig                 |    2 +-

 arch/arm/Kconfig.instrumentation |   52 ++++++++++++++++++++++++++++++++++++++

 2 files changed, 53 insertions(+), 1 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

index c4de2d4..3a75a0b 100644

--- a/arch/arm/Kconfig

+++ b/arch/arm/Kconfig

@@ -1076,7 +1076,7 @@ endmenu
 source "fs/Kconfig"
-source "kernel/Kconfig.instrumentation"

+source "arch/arm/Kconfig.instrumentation"
 source "arch/arm/Kconfig.d...
[ message continues ]

BTW, your patch may fix ARM, but the original commit broke blackfin as

well - it removed the Kconfig entry for HARDWARE_PM, which is still used:
$ grep HARDWARE_PM arch/blackfin/ -r

arch/blackfin/mach-common/interrupt.S:#ifdef CONFIG_HARDWARE_PM

arch/blackfin/mach-common/interrupt.S:#ifdef CONFIG_HARDWARE_PM

arch/blackfin/mach-common/irqpanic.c:#ifdef CONFIG_HARDWARE_PM

arch/blackfin/oprofile/Makefile:oprofile-$(CONFIG_HARDWARE_PM) += op_model_bf533.o

arch/blackfin/oprofile/common.c:#ifdef CONFIG_HARDWARE_PM
So blackfin also needs fixing.  I don't know if blackfin people are

aware of this.
--

Russell King

 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/

 maintainer of:

--

They are:
cu

Adrian
-- 
       "Is there not promise of rain?" Ling Tan asked suddenly out

        of the darkness. There had been need of rain for many days.

       "Only a promise," Lao Er said.

                                       Pearl S. Buck - Dragon Seed
--

Hi,
Well, it goes along the lines of the patch I suggested as a reply to

Adrian, with these differences :
- I still source the kernel/Kconfig.instrumentation file.

- I put back the missing OPROFILE options directly in arch/arm/Kconfig
Then end result is the same as your patch, but without the code

duplication.
Here is the patch :
Fix ARMv6 oprofile support
This patch restores the ARMv6 OProfile support that was killed by

commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
It puts the config options in arch/arm/Kconfig.
Thanks to Adrian Bunk for finding this bug and providing an initial

patch.
Changelog :

Use def_bool.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

CC: Adrian Bunk <adrian.bunk@movial.fi>

CC: Randy Dunlap <randy.dunlap@oracle.com>

CC: rmk@arm.linux.org.uk

CC: phil.el@wanadoo.fr

CC: oprofile-list@lists.sourceforge.net

---

 arch/arm/Kconfig |   17 +++++++++++++++++

 1 file changed, 17 insertions(+)
Index: linux-2.6-lttng/arch/arm/Kconfig

===================================================================

--- linux-2.6-lttng.orig/arch/arm/Kconfig	2007-12-29 16:58:32.000000000 -0500

+++ linux-2.6-lttng/arch/arm/Kconfig	2007-12-29 16:59:25.000000000 -0500

@@ -130,6 +130,23 @@ config FIQ

 config ARCH_MTD_XIP

 	bool
+if OPROFILE

+

+config OPROFILE_ARMV6

+	def_bool y

+	depends on CPU_V6 && !SMP

+	select OPROFILE_ARM11_CORE

+

+config OPROFILE_MPCORE

+	def_bool y

+	depends on CPU_V6 && SMP

+	select OPROFILE_ARM11_CORE

+

+config OPROFILE_ARM11_CORE

+	bool

+

+endif

+

 config VECTORS_BASE

 	hex

 	default 0xffff0000 if MMU || CPU_HIGH_VECTOR
-

Mathieu Desnoyers

Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal

OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

--

No it's not.
Now the config variables may all be there, but the UI for the *menu*

system is broken (ie all the ARM profiling config options are now outside

the profiling menu).
Is that menu really needed? I dunno. But since it exists, it should be

correct.
		Linus

--

There is an "instrumentation menu removal" patchset I've submitted to

Andrew for the next release cycle that moves the instrumentation menu

content into General setup (I did this following your advice).
Furthermore, on ARM, the OPROFILE_ARMV6, OPROFILE_MPCORE and

OPROFILE_ARM11_CORE are all "bool , default y" (equivalent to the

preferred def_bool y). Unless I am grossly mistaken, this is not

supposed to show up in the menus; it's just selected when the

dependencies are met.
Mathieu
--

Mathieu Desnoyers

Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal

OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

--

Ahh, I didn't notice that. If so, yes, it doesn't matter at all whether

it's inside the menu or not, of course.
		Linus

--

This was the patch:

+if OPROFILE

+

+config OPROFILE_ARMV6

+	def_bool y

+	depends on CPU_V6 && !SMP

+	select OPROFILE_ARM11_CORE

+

+config OPROFILE_MPCORE

+	def_bool y

+	depends on CPU_V6 && SMP

+	select OPROFILE_ARM11_CORE

+

+config OPROFILE_ARM11_CORE

+	bool

+

+endif
And none of these has a prompt defined so they do not show up

as a menu.

It is very easy to test if you have the patch applied.

No cross toolchin is needed - just do:

make ARCH=arm menuconfig
	Sam

--

Just tested with and without oprofile/MPCORE/CPU_V6 combinations and I

confirm that :
- no menu is showing up (as expected)

- the OPROFILE_* config options are selected (or not) as expected
Mathieu
--

Mathieu Desnoyers

Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal

OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

--

Just tested it and the right Kconfig symbols get set again - thanks.
--

Russell King

 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/

 maintainer of:

--

i like this approach better, not the least because it affects only one

architecture so late in the .24-rc cycle. Instrumentation bugs tend to

be found with a few weeks/months of delays. (because historically

instrumentation has been typically used by folks who cannot change their

kernel easily to debug it, i.e. by extension they dont run bleeding edge

kernels either.)
Acked-by: Ingo Molnar <mingo@elte.hu>
	Ingo

--

Below is the patch I already sent on 28 Dec 2007 that stuffs it into

Kconfig.instrumentation.
Technically it shouldn't make any difference whether this patch or

Mathieu's patch that stuffs it into arch/arm/Kconfig gets applied, but

one of them should be applied for 2.6.24 (plus either mine or Mathieu's

fix for the blackfin HARDWARE_PM support broken by the same commit).
I found the bugs in Mathieu's commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9,

I wrote patches that restore the status quo, Cc'ed all people even

remotely related to this issue, and I opened the Bugzilla bugs required

for getting them on the regression lists. Mathieu wants the regressions

he introduced fixed different from what my patches did and that's not a

problem for me (his patches are also OK).
What went wrong that his regression fixes did not land in Linus' tree?
cu

Adrian
<--  snip  -->
This patch restores the ARMv6 OProfile support that was killed by

commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
Signed-off-by: Adrian Bunk <adrian.bunk@movial.fi>
---
 kernel/Kconfig.instrumentation |   19 +++++++++++++++++++

 1 file changed, 19 insertions(+)
7fc221ef169610b5eac98e2ddd641811c0d53e4a

diff --git a/kernel/Kconfig.instrumentation b/kernel/Kconfig.instrumentation

index 468f47a..4453187 100644

--- a/kernel/Kconfig.instrumentation

+++ b/kernel/Kconfig.instrumentation

@@ -29,2 +29,17 @@ config OPROFILE
+config OPROFILE_ARMV6

+	bool

+	depends on OPROFILE && ARM && CPU_V6 && !SMP

+	default y

+	select OPROFILE_ARM11_CORE

+

+config OPROFILE_MPCORE

+	bool

+	depends on OPROFILE && ARM && CPU_V6 && SMP

+	default y

+	select OPROFILE_ARM11_CORE

+

+config OPROFILE_ARM11_CORE

+	bool

+

 config KPROBES

--

Yes Adrian, and I thank you very much for all this work. I only

suggested the alternative patches so it would remove menu options nobody

really needs (historical special-cases) and make it easier to apply the

following set of patches already in -mm.
One way or another, either your or my arm and blackfin patches should

get into 2.6.24 final.
--

Mathieu Desnoyers

Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal

OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

--