Re: [PATCH 0/4] powerpc: Fix minor build issues on 2.6.32-rc7 without CONFIG_XICS set

Previous thread: [PATCH 1/4] powerpc: Add Kconfig dependency for PCI_MSI as needed by XICS by Mel Gorman on Tuesday, November 17, 2009 - 5:07 am. (2 messages)

Next thread: Out of order kernel messages in sd detection by =?utf-8?Q?=22J.A._Magall=C3=B3n=22?= on Tuesday, November 17, 2009 - 6:21 am. (2 messages)
From: Mel Gorman
Date: Tuesday, November 17, 2009 - 5:07 am

KConfig doesn't require CONFIG_XICS to be set with CONFIG_PSERIES but if
it's not set, there are numerous small build errors. This is a small series
of patches to allow CONFIG_XICS to be unset in the config.

XICS appears to be some sort of interrupt controller but I'm not sure how
common it is on systems that require CONFIG_PSERIES. If CONFIG_PSERIES
universally requires CONFIG_XICS, the better path might be to force it to
be set.

Testing was building with make oldconfig a configuration from 2.6.31 on
a PPC970.
--

From: Mel Gorman
Date: Tuesday, November 17, 2009 - 5:07 am

If CONFIG_XICS is set, then smp_pSeries_cpu_bootable() is unnecessary.
As warnings are treated as errors, this fails to build.

This patch only defines smp_pSeries_cpu_bootable() when CONFIG_XICS is
set.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---
 arch/powerpc/platforms/pseries/smp.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index da78ea5..62b1141 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -130,6 +130,15 @@ static void __devinit smp_pSeries_kick_cpu(int nr)
 	paca[nr].cpu_start = 1;
 }
 
+#ifdef CONFIG_MPIC
+static struct smp_ops_t pSeries_mpic_smp_ops = {
+	.message_pass	= smp_mpic_message_pass,
+	.probe		= smp_mpic_probe,
+	.kick_cpu	= smp_pSeries_kick_cpu,
+	.setup_cpu	= smp_mpic_setup_cpu,
+};
+#endif
+#ifdef CONFIG_XICS
 static int smp_pSeries_cpu_bootable(unsigned int nr)
 {
 	/* Special case - we inhibit secondary thread startup
@@ -142,15 +151,7 @@ static int smp_pSeries_cpu_bootable(unsigned int nr)
 
 	return 1;
 }
-#ifdef CONFIG_MPIC
-static struct smp_ops_t pSeries_mpic_smp_ops = {
-	.message_pass	= smp_mpic_message_pass,
-	.probe		= smp_mpic_probe,
-	.kick_cpu	= smp_pSeries_kick_cpu,
-	.setup_cpu	= smp_mpic_setup_cpu,
-};
-#endif
-#ifdef CONFIG_XICS
+
 static struct smp_ops_t pSeries_xics_smp_ops = {
 	.message_pass	= smp_xics_message_pass,
 	.probe		= smp_xics_probe,
-- 
1.6.5

--

From: Michael Ellerman
Date: Tuesday, November 17, 2009 - 5:47 am

I'm not sure, but I would guess that was just an oversight by whoever
added the cpu_bootable check. By which I mean it should be in the mpic
ops as well - avoiding the need for any ifdefery.

cheers

From: Michael Ellerman
Date: Tuesday, November 17, 2009 - 5:39 am

It is indeed 'some sort of interrupt controller' :)

All the virtualised PSERIES systems have, or appear to have, a XICS. So
it's pretty common.

I think the only time you see an MPIC is on older machines, and even
then maybe only if you're running bare metal.

So it is pretty much required for a PSERIES kernel, but perhaps there
are cases where the BML guys want to be able to turn it off.

cheers
From: Michael Ellerman
Date: Tuesday, November 17, 2009 - 5:52 am

In fact this series makes me wonder whether we can drop support for a
single kernel image with pseries XICS & MPIC support.

If we could drop that requirement we could have a single set of names,
ie. API, for the irq routines and build either the XICS or MPIC
versions.

That would avoid all the code that needs a setup_foo_xics() and
setup_foo_mpic() - it'd just be setup_foo(), implemented by either the
XICS or MPIC code.

cheers


From: Benjamin Herrenschmidt
Date: Tuesday, November 17, 2009 - 11:00 pm

Nope. Not happening. We should just hide CONFIG_XICS just like
CONFIG_MPIC, it should be select'ed by the platform (which today is only

And so we would have the ability to build a kernel that supports in a
single binary every platform, as is the case today, ie, pseries,
powermac, pa6t, cell, etc... _BUT_ for pseries support, we would have to
choose at compile time whether to support old or new machines ? Sounds

And that would save us what ? one page on a pSeries machine ? yeah !

Cheers,
Ben.

--

From: Mel Gorman
Date: Tuesday, November 17, 2009 - 6:25 am

Do you want to take ownership of the series as I'm a bit unclear on how
and when XICS should be used? If not, I don't mind fixing the series up
as you suggested but I wouldn't be in the best position to determine if
the final result is sensible or not.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab
--

From: Benjamin Herrenschmidt
Date: Tuesday, November 17, 2009 - 11:05 pm

CONFIG_XICS should be made invisible and selected by PSERIES.

Cheers,
Ben.


--

From: Mel Gorman
Date: Wednesday, November 18, 2009 - 10:05 am

Like so?

==== CUT HERE ====

powerpc: Add Kconfig dependency on PCI_MSI for XICS and select for PSERIES

It's possible to set CONFIG_XICS without CONFIG_PCI_MSI. When that
happens, the kernel fails to build with

arch/powerpc/platforms/built-in.o: In function `.xics_startup':
xics.c:(.text+0x12f60): undefined reference to `.unmask_msi_irq'
make: *** [.tmp_vmlinux1] Error 1

Furthermore, as noted by Benjamin Herrenschmidt, "CONFIG_XICS should be
made invisible and selected by PSERIES."

This patch adds the dependency in KConfig for XICS on PCI_MSI. When
PSERIES support is being configured, both options are silently selected.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
--- 
 arch/powerpc/platforms/Kconfig         |    4 ++--
 arch/powerpc/platforms/pseries/Kconfig |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index 04a8061..a82c470 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -52,9 +52,9 @@ config PPC_UDBG_BEAT
 	default n
 
 config XICS
-	depends on PPC_PSERIES
+	depends on PCI_MSI
 	bool
-	default y
+	default n
 
 config IPIC
 	bool
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index f0e6f28..81c2289 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -5,6 +5,8 @@ config PPC_PSERIES
 	select PPC_I8259
 	select PPC_RTAS
 	select RTAS_ERROR_LOGGING
+	select PCI_MSI
+	select XICS
 	select PPC_UDBG_16550
 	select PPC_NATIVE
 	select PPC_PCI_CHOICE if EMBEDDED
--

From: Benjamin Herrenschmidt
Date: Thursday, November 26, 2009 - 7:40 pm

Why the above ? XICS only exist on PSERIES and select bypass depends in

The above bits look plenty enough. Can you also stick it next to the
MPIC one ?

Cheers,


--

From: Mel Gorman
Date: Friday, November 27, 2009 - 9:33 am

You're right. When I made that change, I assumed that a "select foo"
would also resolve the dependencies. It doesn't but I failed to back

They are. I can.

==== CUT HERE ====
powerpc: Add Kconfig dependency on PCI_MSI for XICS and select for PSERIES

It's possible to set CONFIG_XICS without CONFIG_PCI_MSI. When that happens,
the kernel fails to build with

arch/powerpc/platforms/built-in.o: In function `.xics_startup':
xics.c:(.text+0x12f60): undefined reference to `.unmask_msi_irq' make: ***
[.tmp_vmlinux1] Error 1

Furthermore, as noted by Benjamin Herrenschmidt, "CONFIG_XICS should be
made invisible and selected by PSERIES."

This patch adds the dependency in KConfig for XICS on PCI_MSI. When PSERIES
support is being configured, both options are silently selected.

Signed-off-by: Mel Gorman <mel[at]csn.ul.ie>
---
 arch/powerpc/platforms/pseries/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index f0e6f28..60d53ed 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -2,6 +2,8 @@ config PPC_PSERIES
 	depends on PPC64 && PPC_BOOK3S
 	bool "IBM pSeries & new (POWER5-based) iSeries"
 	select MPIC
+	select PCI_MSI
+	select XICS
 	select PPC_I8259
 	select PPC_RTAS
 	select RTAS_ERROR_LOGGING
--

Previous thread: [PATCH 1/4] powerpc: Add Kconfig dependency for PCI_MSI as needed by XICS by Mel Gorman on Tuesday, November 17, 2009 - 5:07 am. (2 messages)

Next thread: Out of order kernel messages in sd detection by =?utf-8?Q?=22J.A._Magall=C3=B3n=22?= on Tuesday, November 17, 2009 - 6:21 am. (2 messages)