[coreboot] r3723 - trunk/util/flashrom
stepan at coresystems.de
Mon Nov 3 12:30:59 CET 2008
Carl-Daniel Hailfinger wrote:
> On 03.11.2008 09:04, Stefan Reinauer wrote:
>> On 03.11.2008, at 01:35,
>> Peter Stuge <peter at stuge.se> wrote:
>>> svn at 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".
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
More information about the coreboot