[flashrom] [PATCH] Constify flashchips array
c-d.hailfinger.devel.2006 at gmx.net
Thu Mar 3 23:23:40 CET 2011
Auf 19.01.2011 20:00, Stefan Reinauer schrieb:
> * Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> [110119 08:49]:
>> Auf 19.01.2011 07:11, Stefan Reinauer schrieb:
>>> * Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> [110118 21:29]:
>>> You could have a "struct flash_instance" which is passed around and
>>> contains all the instance specific information of a flash chip plus a
>>> pointer to the struct flashchip element. This way you would cleanly
>>> separate the "database" from the flash chips found in the system (and
>>> could increase the number of flashes to 16 instead of 3). I know it
>>> requires an s,flashchip,flashchip_instance, across most of the code and
>>> some additional changes, but the readability might be worth the effort.
>> Given that almost all the code works with the instance data, it may be
>> better to use struct flashchip for the instance data and struct
>> flashchip_rodata (or flashchip_def) for the table. If flashchip_rodata
>> is an anonymous struct within flashchip, we have easy copying and can
>> keep all currently untouched code as is. As a nice side effect, the
>> flashrom .text size would shrink by a few percent.
Turns out that even gcc 4.5.0 chokes on this (but can support it with a
special feature switch), and older gcc versions can't handle it at all.
The pointer-to-flashchip_def variant breaks chips with variable size,
and even just duplicating struct flashchip into a slightly trimmed down
struct flashchip_def causes compilation problems on some targets.
>> My longterm plans call for programmer info (at least a pointer) in
>> struct flashchip so that we can handle multiple flashchips on sister
>> buses easier (e.g. software-switched "Dual BIOS" solutions) because
>> flashchip contains e.g. a enable_this_chip function pointer.
>> What do you think?
> Sounds great! This is a very good clean up of flashrom's "device model"
My last patch is the only variant I could find which works on all
currently supported compilers, and until I've figured out a way to
implement something better, this is still a step forward, so I'm going
to ask for a code review and an Ack.
More information about the flashrom