Re: [flashrom] [PATCH] Constify flashchips array

[Carl-Daniel Hailfinger]

Auf 19.01.2011 20:00, Stefan Reinauer schrieb:
> Carl-Daniel Hailfinger  [110119 08:49]:
> > Auf 19.01.2011 07:11, Stefan Reinauer schrieb:
> > > Carl-Daniel Hailfinger  [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.

Regards,
Carl-Daniel