[coreboot] [patch] flashrom flashchips array modification

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Dec 15 02:50:21 CET 2008


On 14.12.2008 19:26, Stephan GUILLOUX wrote:
> Corey Osgood a écrit :
> > On Sun, Dec 14, 2008 at 12:53 PM, Stephan GUILLOUX <stephan.guilloux at free.fr>
> > wrote:
> >       Hello all,
> >
> >     Using flashrom for some times, I find that flashchips array becomes
> >     difficult to read.
> >     To be able to read a single entry, in this array, you need to expand your
> >     editor window
> >     to its maximum size and reduce the font size. Any new field in this array
> >     will result
> >     in a nightmare to read and for support.
> >
> >     My suggestion for this, is to detail each line with field names, like with
> >     the extract
> >     below :
> >     [...]
> >       {
> >           .vendor         = "AMD",
> >           .name           = "Am29F016D",
> >           .manufacture_id = AMD_ID,
> >           .model_id       = AM_29F016D,
> >           .total_size     = 2048,
> >           .page_size      = 64 * 1024,
> >           .tested         = TEST_UNTESTED,
> >           .probe          = probe_29f040b,
> >           .erase          = erase_29f040b,
> >           .write          = write_29f040b,
> >       },
> >     [...]
> >
> >
> > Good idea, but someone already had it ;) Take a look at a fresh checkout of 
> > flashrom, not quite the same but very similar.
>
> I know. This patch is based on this work too.
> But still difficult to parse to add/remove any field.
>   

Agreed.


> >
> > -Corey
> >
> >
> >     This kind of syntax makes flashchips.c rather longer in term of lines, but
> >     keeps it
> >     readable, and easily parsable with an external shell/tool...
> >
> >     Any comment ?
> >
> >     Also in attachement.
> >
> >       Stephan.
> >
> >     Signed-off-by: Stephan Guilloux <stephan.guilloux at free.fr>
>   

Awesome!
The patch improves machine parseability and human readability over
what's currently in flashrom HEAD. And the explicit initialization makes
sure any future struct flashchip reordering is not needed. (And I can
throw away some of my pending patches because this solution is cleaner
and fulfills all my requirements.)

Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>


> >
> >     Index: coreboot-v2/util/flashrom/flashchips.c
> >     ===================================================================
> >     --- coreboot-v2/util/flashrom/flashchips.c    (révision 3813)
> >     +++ coreboot-v2/util/flashrom/flashchips.c    (copie de travail)
> >     @@ -36,124 +36,278 @@
> >         * Probe function, Erase function, Write function, Read function
> >         */
> >
> >     -    {"AMD", "Am29F002(N)BB", AMD_ID, AM_29F002BB,
> >     -     256, 256,
> >     -     TEST_UNTESTED,
> >     -     probe_jedec, erase_chip_jedec, write_en29f002a
> >     +    {
> >     +        .vendor        = "AMD",
> >     +        .name        = "Am29F002(N)BB",
> >     +        .manufacture_id    = AMD_ID,
> >     +        .model_id    = AM_29F002BB,
> >     +        .total_size    = 256,
> >     +        .page_size    = 256,
> >     +        .tested        = TEST_UNTESTED,
> >     +        .probe        = probe_jedec,
> >     +        .erase        = erase_chip_jedec,
> >     +        .write        = write_en29f002a,
> >        },
> >
> >     -    {"AMD", "Am29F002(N)BT", AMD_ID, AM_29F002BT,
> >     -     256, 256,
> >     -     TEST_OK_PREW,
> >     -     probe_jedec, erase_chip_jedec, write_en29f002a
> >     +    {
> >     +        .vendor        = "AMD",
> >     +        .name        = "Am29F002(N)BT",
> >     +        .manufacture_id    = AMD_ID,
> >     +        .model_id    = AM_29F002BT,
> >     +        .total_size    = 256,
> >     +        .page_size    = 256,
> >     +        .tested        = TEST_OK_PREW,
> >     +        .probe        = probe_jedec,
> >     +        .erase        = erase_chip_jedec,
> >     +        .write        = write_en29f002a,
> >        },
> >
>   


-- 
http://www.hailfinger.org/





More information about the coreboot mailing list