Re: [flashrom] [PATCH] Constify flashchips array

[Carl-Daniel Hailfinger]

Ping?

The plans to create a more advanced version of the patch will break 
compilation with gcc 4.5.0 and older, so I'd like to keep the patch as 
is. Subsequent optimizations can be built on top of this.

Regards,
Carl-Daniel

Am 03.03.2011 23:23 schrieb 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