Carl-Daniel Hailfinger wrote:
On 03.11.2008 09:04, Stefan Reinauer wrote:
On 03.11.2008, at 01:35, Peter Stuge peter@stuge.se wrote:
svn@coreboot.org wrote:
+++ trunk/util/flashrom/chipset_enable.c 2008-11-03 00:20:22 UTC (rev 3723)
..
case BUS_TYPE_ICH9_SPI:
/* TODO: Add dumping function for ICH8/ICH9, or drop the* whole SPIBAR dumping from chipset_enable.c - There's* inteltool for this task already.*/
tmp2 = *(uint16_t *) (spibar + 0);printf_debug("0x00: 0x%04x (HSFS)\n", tmp2);printf_debug("FLOCKDN %i, ", (tmp >> 15 & 1));printf_debug("FDV %i, ", (tmp >> 14) & 1);printf_debug("FDOPSS %i, ", (tmp >> 13) & 1);Would have been nice if this had gone into ichspi.c.
It's not too late, Peter. Care to illustrate your idea with a patch?
Do we want to move all other chipset specific debug code to separate files as well? Then we need ck804.c and ichold.c and... IMHO chipset_enable.c is for stuff you want to run once per chipset: Some debug prints, some settings. If we move such code out of chipset_enable.c, where do we draw the line?
I think SPI is causing us headaches with our old concept of per-flash drivers since with SPI the _magic_ suddenly happens in the SPI controller and not in the flash device anymore. (Mostly)
I tend to agree with Carl-Daniel on this one, but we already "broke" the convention by creating ichspi.c Anyways, if someone comes up with code and a few reasons to apply that code, we have something to consider. That may well be interpreted as "No code, no discussion".
Stefan