SMC_CONFIG is needed before the device tree is ready and some people would rather not have mainboard settings like sio_gp1x_config in the device tree anyway. So found a nice united home for both in Kconfig, where users can change them without having to mess around in the C code.
Signed-off-by: Jens Rottmann JRottmann@LiPPERTEmbedded.de ---
Patrick, BTW, what happened to your patch which made the struct mainboard_config work again? I don't remember it getting committed. I don't use it mainly because SMC_CONFIG is needed early and I perfer not scattering 2 options in different places, but I _do_ think the mainboard logic is a kind of device and might be part (root??) of the tree. So IMHO your patch which reenables this is a very good thing to have!
Cheers, Jens
--- src/mainboard/lippert/roadrunner-lx/Kconfig (rev 5758) +++ src/mainboard/lippert/roadrunner-lx/Kconfig (working copy) @@ -11,6 +11,8 @@ select PIRQ_ROUTE select UDELAY_TSC select CACHE_AS_RAM + # Standard chip is a 512 KB FWH. Replacing it with a 1 MB + # SST 49LF008A is possible. select BOARD_ROMSIZE_KB_512
config MAINBOARD_DIR @@ -29,4 +31,11 @@ hex default 0x4000
+config ONBOARD_UARTS_RS485 + bool "Switch on-board serial ports to RS485" + default n + help + If selected, both on-board serial ports will operate in RS485 mode + instead of RS232. + endif # BOARD_LIPPERT_ROADRUNNER_LX --- src/mainboard/lippert/roadrunner-lx/chip.h (rev 5758) +++ src/mainboard/lippert/roadrunner-lx/chip.h (working copy) @@ -18,13 +18,6 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-/* Based on chip.h from AMD's DB800 mainboard. */ - -#include <stdint.h> - extern struct chip_operations mainboard_ops;
-struct mainboard_config { - /* bit5 = Live LED, bit2 = RS485_EN2, bit1 = RS485_EN1 */ - u8 sio_gp1x_config; -}; +struct mainboard_config {}; --- src/mainboard/lippert/roadrunner-lx/mainboard.c (rev 5758) +++ src/mainboard/lippert/roadrunner-lx/mainboard.c (working copy) @@ -29,6 +29,13 @@ #include <device/pci_ids.h> #include "chip.h"
+/* Bit1 switches Com1 to RS485, bit2 same for Com2, bit5 turns off the Live LED. */ +#if CONFIG_ONBOARD_UARTS_RS485 + #define SIO_GP1X_CONFIG 0x26 +#else + #define SIO_GP1X_CONFIG 0x20 +#endif + static const u16 ec_init_table[] = { /* hi=data, lo=index */ 0x1900, /* Enable monitoring */ 0x0351, /* TMPIN1,2 diode mode, TMPIN3 off */ @@ -40,7 +47,6 @@
static void init(struct device *dev) { - struct mainboard_config *mb = dev->chip_info; unsigned int gpio_base, i; printk(BIOS_DEBUG, "LiPPERT RoadRunner-LX ENTER %s\n", __func__);
@@ -61,7 +67,8 @@ outb(val >> 8, 0x0296); }
- outb(mb->sio_gp1x_config, 0x1220); /* Simple-I/O GP17-10 */ + /* bit5 = Live LED, bit2 = RS485_EN2, bit1 = RS485_EN1 */ + outb(SIO_GP1X_CONFIG, 0x1220); /* Simple-I/O GP17-10 */ printk(BIOS_DEBUG, "LiPPERT RoadRunner-LX EXIT %s\n", __func__); }
--- src/mainboard/lippert/roadrunner-lx/romstage.c (rev 5758) +++ src/mainboard/lippert/roadrunner-lx/romstage.c (working copy) @@ -132,4 +132,3 @@ /* Memory is setup. Return to cache_as_ram.inc and continue to boot. */ return; } - --- src/mainboard/lippert/spacerunner-lx/Kconfig (rev 5758) +++ src/mainboard/lippert/spacerunner-lx/Kconfig (working copy) @@ -12,6 +12,8 @@ select PIRQ_ROUTE select UDELAY_TSC select CACHE_AS_RAM + # Board is equipped with a 1 MB SPI flash, however, due to limitations + # of the IT8712F Super I/O, only the top 512 KB are directly mapped. select BOARD_ROMSIZE_KB_512
config MAINBOARD_DIR @@ -30,4 +32,17 @@ hex default 0x4000
+config ONBOARD_UARTS_RS485 + bool "Switch on-board serial ports to RS485" + default n + help + If selected, both on-board serial ports will operate in RS485 mode + instead of RS232. + +config ONBOARD_IDE_SLAVE + bool "Make on-board SSD act as Slave" + default n + help + If selected, the on-board SSD will act as IDE Slave instead of Master. + endif # BOARD_LIPPERT_SPACERUNNER_LX --- src/mainboard/lippert/spacerunner-lx/chip.h (rev 5758) +++ src/mainboard/lippert/spacerunner-lx/chip.h (working copy) @@ -18,13 +18,6 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-/* Based on chip.h from AMD's DB800 mainboard. */ - -#include <stdint.h> - extern struct chip_operations mainboard_ops;
-struct mainboard_config { - /* bit2 = RS485_EN2, bit1 = RS485_EN1, bit0 = Live LED */ - u8 sio_gp1x_config; -}; +struct mainboard_config {}; --- src/mainboard/lippert/spacerunner-lx/mainboard.c (rev 5758) +++ src/mainboard/lippert/spacerunner-lx/mainboard.c (working copy) @@ -29,6 +29,13 @@ #include <device/pci_ids.h> #include "chip.h"
+/* Bit0 turns off the Live LED, bit1 switches Com1 to RS485, bit2 same for Com2. */ +#if CONFIG_ONBOARD_UARTS_RS485 + #define SIO_GP1X_CONFIG 0x07 +#else + #define SIO_GP1X_CONFIG 0x01 +#endif + static const u16 ec_init_table[] = { /* hi=data, lo=index */ 0x1900, /* Enable monitoring */ 0x3050, /* VIN4,5 enabled */ @@ -41,7 +48,6 @@
static void init(struct device *dev) { - struct mainboard_config *mb = dev->chip_info; unsigned int gpio_base, i; printk(BIOS_DEBUG, "LiPPERT SpaceRunner-LX ENTER %s\n", __func__);
@@ -65,7 +71,9 @@ outb(val >> 8, 0x0296); }
- outb(mb->sio_gp1x_config, 0x1220); /* Simple-I/O GP17-10 */ + /* bit2 = RS485_EN2, bit1 = RS485_EN1, bit0 = Live LED */ + outb(SIO_GP1X_CONFIG, 0x1220); /* Simple-I/O GP17-10 */ + printk(BIOS_DEBUG, "LiPPERT SpaceRunner-LX EXIT %s\n", __func__); }
--- src/mainboard/lippert/spacerunner-lx/romstage.c (rev 5758) +++ src/mainboard/lippert/spacerunner-lx/romstage.c (working copy) @@ -41,7 +41,11 @@ #include "superio/ite/it8712f/it8712f_early_serial.c"
/* Bit0 enables Spread Spectrum, bit1 makes on-board SSD act as IDE slave. */ -#define SMC_CONFIG 0x01 +#if CONFIG_ONBOARD_IDE_SLAVE + #define SMC_CONFIG 0x03 +#else + #define SMC_CONFIG 0x01 +#endif
#define ManualConf 1 /* No automatic strapped PLL config */ #define PLLMSRhi 0x0000059C /* Manual settings for the PLL */ @@ -201,4 +205,3 @@ /* Memory is setup. Return to cache_as_ram.inc and continue to boot. */ return; } - _
Jens Rottmann wrote:
+++ src/mainboard/lippert/roadrunner-lx/Kconfig (working copy)
..
+config ONBOARD_UARTS_RS485
- bool "Switch on-board serial ports to RS485"
- default n
- help
If selected, both on-board serial ports will operate in RS485 modeinstead of RS232.
Maybe make that one option per port instead, with choices "RS232" and "RS485", since the code allows it very easily. Maybe also make an (expert?) option to disable the LED?
//Peter
On Tue, Aug 31, 2010 at 6:30 AM, Jens Rottmann JRottmann@lippertembedded.de wrote:
SMC_CONFIG is needed before the device tree is ready and some people would rather not have mainboard settings like sio_gp1x_config in the device tree anyway. So found a nice united home for both in Kconfig, where users can change them without having to mess around in the C code.
Signed-off-by: Jens Rottmann JRottmann@LiPPERTEmbedded.de
Acked-by: Myles Watson mylesgw@gmail.com
Thanks, Myles
On Tue, Aug 31, 2010 at 11:09 AM, Myles Watson mylesgw@gmail.com wrote:
On Tue, Aug 31, 2010 at 6:30 AM, Jens Rottmann JRottmann@lippertembedded.de wrote:
SMC_CONFIG is needed before the device tree is ready and some people would rather not have mainboard settings like sio_gp1x_config in the device tree anyway. So found a nice united home for both in Kconfig, where users can change them without having to mess around in the C code.
Signed-off-by: Jens Rottmann JRottmann@LiPPERTEmbedded.de
Acked-by: Myles Watson mylesgw@gmail.com
Rev 5760.
Thanks, Myles
Peter wrote:
Maybe make that one option per port instead, with choices "RS232" and "RS485", since the code allows it very easily.
I had considered this but decided against it. I'd guess that, like, 96.3% of all users will want both ports speak the same protocol. In fact most will want RS232, RS485 is already very rare, but mixed operation must be really, really exotic.
It is simply not feasible to make _everything_ configurable, one has to draw a line somewhere. That's why I didn't add an option to turn off the Spread Spectrum in SMC_CONFIG, either. Before I would consider this, there are lots of settings which are _much_ more likely to get changed first.
Like disabling devices completely to save power, I/O resources and IRQs. Or moving PCI INT A-D or legacy devices to a different IRQ line. (Our boards still feature ISA, provided by an IT8888 PCI-to-ISA bridge.) These are the things our customers request all the time. And even though our standard BIOSes (Insyde and Phoenix) have all this in the normal BIOS Setup, we _still_ keep providing many customers with variants of our standard BIOSes - like one making sure that a certain bit pattern is written as early as possible to the LPT data port (3rd assembler instruction actually) and is kept until OS has finished booting. (The customer in question is (ab)using the LPT data pins as GPIOs to control pieces of hardware in a satellite - embedded customers are ... umm ... different.) There is no way you can make _everything_ user-configurable.
I agree that easy access to all knobs would be great, but for that you'd have to start with devicetree.cb, and allow a flag added to each setting saying "do autogenerate Kconfig entry for this option".
Maybe also make an (expert?) option to disable the LED?
It's a red LED for free use of the customer's application - like "new mail has arrived" or "satellite ran out of fuel". ;-) I think most customers rather change the LED at runtime than by reconfiguring coreboot. The '1' I write to it makes sure it's initally off.
Cheers, Jens
Jens Rottmann wrote:
Maybe make that one option per port instead,
I had considered this but decided against it.
Oh well.
It is simply not feasible to make _everything_ configurable,
I'm not sure I agree about feasible. I agree it's not worthwhile though. You know your customers best, but from my experience with embedded customers, more configurability is better. :)
one has to draw a line somewhere.
Yes of course.
Like disabling devices completely to save power, I/O resources and IRQs. 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 do agree about disabling and I/O and interrupts as far as possible..
making sure that a certain bit pattern is written as early as possible to the LPT data port
This is a good point. It could make sense to have hooks already in the bootblock.
you'd have to start with devicetree.cb,
I know. This is one part of coreboot "infrastructure" that I think needs some improvement. A first step would be interrupt routing.
It might also be nice to make it a little easier to use devicetree.cb from the coreboot code.
Maybe also make an (expert?) option to disable the LED?
The '1' I write to it makes sure it's initally off.
That makes sense. Thanks for clarifying.
//Peter
Peter Stuge wrote:
Jens Rottmann wrote:
Maybe make that one option per port instead,
I had considered this but decided against it.
Oh well.
It is simply not feasible to make _everything_ configurable,
I'm not sure I agree about feasible. I agree it's not worthwhile though. You know your customers best, but from my experience with embedded customers, more configurability is better. :)
It also changes the code from "do the minimal effort for hardware init" to "parse a lot of different options and make sure all possible combinations make sense and work".
I'm not saying we shouldn't. I'm just saying it'll be a shift in principles. (And a lot of effort if done right)
you'd have to start with devicetree.cb,
I know. This is one part of coreboot "infrastructure" that I think needs some improvement. A first step would be interrupt routing.
It might also be nice to make it a little easier to use devicetree.cb from the coreboot code.
Now that we have a good grip on the device tree and sconfig is increasingly stable, we should discuss how to get the routing into our device tree. Suggestions?
Some brainstorming: - every device that has an APIC needs to have IRQ routing information. Every bridge, even? - what about 8259? Virtual Wire? - We need to represent the IO apics in the device tree so we can walk the system for dynamic boot time table creation. - any of the i945 ports would be an easy initial target, as they have i945_pci_irqs.asl, ich7_pci_irqs.asl, irq_tables.c and mptable.c in separate files, so they could be replaced easily by something auto generated. *.asl could (at least for now) be created from the device tree at compile time. - A good first start (and relatively easy, since its only refactoring) would also be to factor out interrupt routing from the K8/Fam10 boards into separate files, and move their acpi code to the north/southbridge like it's done on i945/ich7. Any takers?
Stefan
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
--- 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# */
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
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/kontro...
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
-
Jens Rottmann -
Myles Watson -
Patrick Georgi -
Peter Stuge -
Stefan Reinauer