Patchwork use Kconfig for both options on Lippert boards

login
register
about
Submitter Jens Rottmann
Date 2010-09-01 09:31:49
Message ID <4C7E1D85.4060400@LiPPERTEmbedded.de>
Download mbox | patch
Permalink /patch/1840/
State New
Headers show

Comments

Jens Rottmann - 2010-09-01 09:31:49
Good morning Peter,

> > It is simply not feasible to make _everything_ configurable,
> I'm not sure I agree about feasible.

I mean that no matter how hard you try, there will _always_ be one
application left which needs something changed that no one had foreseen.
But that's exactly the strong point of an open source BIOS. The source
adds the last, ultimate layer of configurability.

> from my experience with embedded customers, more configurability
> is better. :)

Absolutely!!

> > Or moving PCI INT A-D or legacy devices to a different IRQ line.
> I guess you already know that PCI interrupts are locked to whatever
> the VSA blob configures for the virtualized devices.

I meant the mapping INT A --> IRQ 10 etc. I already had a patch for this
(attached) but also decided against it, because this should be generic
and _not_ done individually at the board level and the device tree is a
better place for it anyway.

> > you'd have to start with devicetree.cb,

> A first step would be interrupt routing.

I agree 100%!

Regards,
Jens
NACKed-by: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>
Patrick Georgi - 2010-09-01 11:22:21
Am 01.09.2010 11:31, schrieb Jens Rottmann:
> I meant the mapping INT A -->  IRQ 10 etc. I already had a patch for this
> (attached) but also decided against it, because this should be generic
> and _not_ done individually at the board level and the device tree is a
> better place for it anyway.
That data doesn't belong in code, yes. Unfortunately we don't have a 
very good place for it at all, at the moment.
The plan is to fix that, but I can't give a schedule, so _any_ support 
you provide will (very likely) need to be rewritten at some point (once 
we have a framework for that), so don't waste too much time to make it 
pretty.


Patrick
Stefan Reinauer - 2010-09-01 11:59:44
On 9/1/10 1:22 PM, Patrick Georgi wrote:
> Am 01.09.2010 11:31, schrieb Jens Rottmann:
>> I meant the mapping INT A --> IRQ 10 etc. I already had a patch for this
>> (attached) but also decided against it, because this should be generic
>> and _not_ done individually at the board level and the device tree is a
>> better place for it anyway.
> That data doesn't belong in code, yes. Unfortunately we don't have a
> very good place for it at all, at the moment.
> The plan is to fix that, but I can't give a schedule, so _any_ support
> you provide will (very likely) need to be rewritten at some point
> (once we have a framework for that), so don't waste too much time to
> make it pretty.
Actually, I think for the mapping we do have a mechanism in some
mainboards already, and it's stored in the device tree.

http://tracker.coreboot.org/trac/coreboot/browser/trunk/src/mainboard/kontron/986lcd-m/devicetree.cb

In addition, the IRQ routing information itself should also be stored in
the device tree, and that, unfortunately, lacks a framework to do so.

Stefan

Patch

--- src/mainboard/lippert/spacerunner-lx/Kconfig	(rev 5760)
+++ src/mainboard/lippert/spacerunner-lx/Kconfig	(working copy)
@@ -32,6 +32,22 @@ 
 	hex
 	default 0x4000
 
+config PIRQA
+	int "IRQ line for PCI INT A"
+	default 10
+
+config PIRQB
+	int "IRQ line for PCI INT B"
+	default 11
+
+config PIRQC
+	int "IRQ line for PCI INT C"
+	default 5
+
+config PIRQD
+	int "IRQ line for PCI INT D"
+	default 15
+
 config ONBOARD_UARTS_RS485
 	bool "Switch on-board serial ports to RS485"
 	default n
--- src/mainboard/lippert/spacerunner-lx/irq_tables.c	(rev 5760)
+++ src/mainboard/lippert/spacerunner-lx/irq_tables.c	(working copy)
@@ -26,17 +26,11 @@ 
 #include <arch/pirq_routing.h>
 #include "../../../southbridge/amd/cs5536/cs5536.h"
 
-/* Platform IRQs */
-#define PIRQA 10
-#define PIRQB 11
-#define PIRQC 5
-#define PIRQD 15
-
 /* Map */
-#define M_PIRQA (1 << PIRQA)	/* Bitmap of supported IRQs */
-#define M_PIRQB (1 << PIRQB)	/* Bitmap of supported IRQs */
-#define M_PIRQC (1 << PIRQC)	/* Bitmap of supported IRQs */
-#define M_PIRQD (1 << PIRQD)	/* Bitmap of supported IRQs */
+#define M_PIRQA (1 << CONFIG_PIRQA)	/* Bitmap of supported IRQs */
+#define M_PIRQB (1 << CONFIG_PIRQB)	/* Bitmap of supported IRQs */
+#define M_PIRQC (1 << CONFIG_PIRQC)	/* Bitmap of supported IRQs */
+#define M_PIRQD (1 << CONFIG_PIRQD)	/* Bitmap of supported IRQs */
 
 /* Link */
 #define L_PIRQA	 1		/* Means Slot INTx# Connects To Chipset INTA# */