[coreboot] PATCH: Add Spansion S25FL016A to flashrom
Stefan Reinauer
stepan at coresystems.de
Sun Jan 27 03:15:58 CET 2008
* Peter Stuge <peter at stuge.se> [080125 02:17]:
> On Thu, Jan 24, 2008 at 01:53:00PM +0100, Stefan Reinauer wrote:
> > This is unrelated to your patch, but I don't like that we list all
> > the IDs as defines in flash.h just to put that define in the same
> > place in flashchips.c.
> >
> > Can't we just put the ID directly into the array in flashchips.c
> > and rather add another string for the vendor name?
>
> #define ID is well tested but I'm not adamant in any way. You're
> basically asking for less information in source and more at run
> time. I like more info at run time but I still think it would be
> nice to have IDs in the code.
Not really less information in the source. Just a single place for that
information. flash.h is the only place where the VENDOR_ID defines are
defined and the array in flashchips.c is the only place where they are
used.
In fact i think it would add information if we replace the VENDOR_ID
defines with strings and drop the DEVICE_ID defines completely.
{"W29C011", WINBOND_ID, W_29C011, 128, 128, ...}
should rather read
{"Winbond", "W29C011", 0xda, 0xc1, 128, 128, ...}
and we suddenly know the vendor of that chip. gcc will make sure we only
store "Winbond" once in the binary.
> I was also thinking that it would/should be simple to tell flashrom
> about these flash chip parameters at run time so as to not need a
> rebuild with as-yet unsupported flash.
yes, this would be really neat.
> Also, I'll patch away the vendor check for -m so that it would be
> sufficient to say -m m57sli as long as no two vendors have boards
> with identical names. (but -m vend:prod will still always work)
sounds good.
--
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
mailing list