[coreboot] r3723 - trunk/util/flashrom

[Stefan Reinauer]

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".

Stefan

-- 
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