[PATCH v2] Make 83xx perfmon support selectable

Not all e300 cores support the performance monitors, and the ones
that don't will be confused by the mf/mtpmr instructions.  This
allows the support to be optional, so the 8349 can turn it off
while the 8379 can turn it on.  Sadly, those aren't config options,
so it will be left to the defconfigs and the users to make that
determination.

Signed-off-by: Andy Fleming <redacted>
---

Ugh.  The previous version was a little hasty.  Needed to base off
PPC_83xx rather than 83xx

 arch/powerpc/platforms/Kconfig         |    1 -
 arch/powerpc/platforms/Kconfig.cputype |    7 ++++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index 0afd225..a578b96 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig

@@ -22,7 +22,6 @@ config PPC_83xx
 	select FSL_SOC
 	select MPC83xx
 	select IPIC
-	select FSL_EMB_PERFMON
 
 config PPC_86xx
 	bool "Freescale 86xx"

diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 73d81ce..0c3face 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype

@@ -113,7 +113,12 @@ config FSL_BOOKE
 	default y
 
 config FSL_EMB_PERFMON
-	bool
+	bool "Freescale Embedded Perfmon"
+	depends on E500 || PPC_83xx
+	help
+	  This is the Performance Monitor support found on the e500 core
+	  and some e300 cores (c3 and c4).  Select this only if your
+	  core supports the Embedded Performance Monitor APU
 
 config PTE_64BIT
 	bool

-- 
1.5.4.23.gef5b9

Andy Fleming wrote:

Not all e300 cores support the performance monitors, and the ones
that don't will be confused by the mf/mtpmr instructions.  This
allows the support to be optional, so the 8349 can turn it off
while the 8379 can turn it on.  Sadly, those aren't config options,
so it will be left to the defconfigs and the users to make that
determination.

Signed-off-by: Andy Fleming <redacted>

Thanks, Andy!

FWIIW,
Acked-by: Gerald Van Baren <redacted>

On Mar 7, 2008, at 5:59 PM, Andy Fleming wrote:

Not all e300 cores support the performance monitors, and the ones
that don't will be confused by the mf/mtpmr instructions.  This
allows the support to be optional, so the 8349 can turn it off
while the 8379 can turn it on.  Sadly, those aren't config options,
so it will be left to the defconfigs and the users to make that
determination.

Signed-off-by: Andy Fleming <redacted>
---

Ugh.  The previous version was a little hasty.  Needed to base off
PPC_83xx rather than 83xx

arch/powerpc/platforms/Kconfig         |    1 -
arch/powerpc/platforms/Kconfig.cputype |    7 ++++++-
2 files changed, 6 insertions(+), 2 deletions(-)

applied.

- k

On Fri, Mar 07, 2008 at 05:59:03PM -0600, Andy Fleming wrote:

Not all e300 cores support the performance monitors, and the ones
that don't will be confused by the mf/mtpmr instructions.  This
allows the support to be optional, so the 8349 can turn it off
while the 8379 can turn it on.  Sadly, those aren't config options,
so it will be left to the defconfigs and the users to make that
determination.

So does this mean we can't do multiplatform of something with perfmon and
something without perfmon?  Seems like this should come from the device
tree, or PVR, or some other runtime check.

-Scott

On Mar 18, 2008, at 12:05 PM, Scott Wood wrote:

On Fri, Mar 07, 2008 at 05:59:03PM -0600, Andy Fleming wrote:

quoted

Not all e300 cores support the performance monitors, and the ones
that don't will be confused by the mf/mtpmr instructions.  This
allows the support to be optional, so the 8349 can turn it off
while the 8379 can turn it on.  Sadly, those aren't config options,
so it will be left to the defconfigs and the users to make that
determination.

So does this mean we can't do multiplatform of something with perfmon and
something without perfmon?  Seems like this should come from the device
tree, or PVR, or some other runtime check.

It possible if your binutils supports generating the instructions.  I believe Kim was going to look at doing a patch to use a #define MFPMR(x)/#define MTPMR() so we don't have to worry about toolchain versions.

- k

Kumar Gala wrote:

On Mar 18, 2008, at 12:05 PM, Scott Wood wrote:

quoted

On Fri, Mar 07, 2008 at 05:59:03PM -0600, Andy Fleming wrote:

quoted

Not all e300 cores support the performance monitors, and the ones
that don't will be confused by the mf/mtpmr instructions.  This
allows the support to be optional, so the 8349 can turn it off
while the 8379 can turn it on.  Sadly, those aren't config options,
so it will be left to the defconfigs and the users to make that
determination.

So does this mean we can't do multiplatform of something with perfmon and
something without perfmon?  Seems like this should come from the device
tree, or PVR, or some other runtime check.

It possible if your binutils supports generating the instructions.  I believe Kim was going to look at doing a patch to use a #define MFPMR(x)/#define MTPMR() so we don't have to worry about toolchain versions.

- k

Hi Kumar, Scott,

Per Andy (and my limited reading of the UMs), some 83xx have the PMR registers and some don't.  The compiler either supports the PMR register or it doesn't.  If you make it runtime configurable, people running CPUs that don't support the specific PMR will have to change compiler configurations in order to compile with the PMR register in there (could have unintended consequences).

Also, if you look at the code (arch/powerpc/kernel/pmc.c), there are several different types of PMR registers, based on the core and flavor of the core.  Finding or making a compiler setup that supports all of the possible PMR registers could be a problem.

You would still have to make the PMR read/write runtime selectable because the CPUs that don't support that register will raise an exception IIRC (an Really Bad Thing[tm]).

It crossed my mind /very briefly/ to use the FDT to select this but was not willing to put that much effort into it.
* Toolchain issues (first & second objection, above) - non-trivial
* Would have to read the enable/disable into a static (global?) variable
   to minimize the time impact for the runtime decision - ugh
* Need to add yet another definition to the booting-without-of.txt
   (bwoof) and appropriate .dts files - OK, I'm lazy

Best regards,
gvb

On Thu, Mar 20, 2008 at 07:59:41PM -0400, Jerry Van Baren wrote:

Per Andy (and my limited reading of the UMs), some 83xx have the PMR  
registers and some don't.  The compiler either supports the PMR register  
or it doesn't.  If you make it runtime configurable, people running CPUs  
that don't support the specific PMR will have to change compiler  
configurations in order to compile with the PMR register in there (could  
have unintended consequences).

I'm not saying make it runtime-only -- you can still have a config option to
determine whether to build a kernel that supports it.  I'm saying there
should be an additional runtime check so that if you run a multiplatform
kernel with perfmon enabled on a chip that doesn't support it, you won't
take a program check.

Also, if you look at the code (arch/powerpc/kernel/pmc.c), there are  
several different types of PMR registers, based on the core and flavor  
of the core.  Finding or making a compiler setup that supports all of  
the possible PMR registers could be a problem.

It only needs to support all possible registers within a class of hardware
over which we support multiplatform kernels.

You would still have to make the PMR read/write runtime selectable  
because the CPUs that don't support that register will raise an  
exception IIRC (an Really Bad Thing[tm]).

Yes, that was my point.  The changelog on the patch seemed to indicate that
the compile-time option was intended to address this, not just the toolchain
problem.

-Scott