[flashrom] [RFC] Add struct flashchip * everywhere

[Carl-Daniel Hailfinger]

Am 09.12.2011 02:19 schrieb Carl-Daniel Hailfinger:
> Am 09.11.2011 01:16 schrieb Carl-Daniel Hailfinger:
>> Am 03.11.2011 23:07 schrieb Carl-Daniel Hailfinger:
>>> Am 02.11.2011 13:41 schrieb Stefan Tauner:
>>>> another layer of redirection is - as always - also a
>>>> possibility: introducing a new struct with pointers to the actual chip
>>>> and the programmer to be used (and other information related to the
>>>> actual situation/probing... e.g. access right ranges). but that's
>>>> probably not needed (yet) and the splitting could be done later anyway
>>>> if need be. OTOH if it is clear that there will be more information
>>>> stuffed into struct flashchip, that is not really static and does not
>>>> need to/should not reside in flashchips.c/struct flashchip, we may
>>>> better discuss a separation now(?).
>>> We have a big problem: There is almost no information in struct
>>> flashchip which is constant in all cases.
>>> The name, size and erase structures could be filled in automatically for
>>> SFDP stuff. That alone kills the separation idea IMHO.
>> Turns out separating struct flashchip and struct flashctx (the flash
>> context) is possible, but it required some sed and manual care.
>>
>> The only difference between struct flashchip and struct flashctx right
>> now are the virtual_memory and virtual_registers members.
>> Compared to my earlier patch, no function signatures have been changed
>> except for flashchip->flashctx replacements. This should make reviews
>> easier.
>>
>> TODO:
>> Test if flashing still works on hardware (I tested with dummy).
>> Test if printing and wiki printing still works as expected.
>>
>> Deferred to another patch:
>> Adding struct flashctx to the remaining function signatures.
>> Check if it makes sense to convert some function signatures to use constant.
>>
>> If you want to verify this patch, run the following command in an
>> unmodified tree:
>> sed -i "s/struct flashchip/struct flashctx/g" *.[ch]
>> and then compare the result to this patch. A few places like print.c and
>> print_wiki.c kept struct flashchip because they don't care about the
>> context and act directly on the flashchips array.
> New version, updated to apply against svn HEAD.
> Please note that this one still duplicates the struct flashchip members
> manually in struct flashctx. An alternative version will be posted as
> followup.

I take that back. Moving struct flashchip members to a separate file did
not work for reasons which are not immediately obvious:
struct block_eraser, struct eraseblock and struct voltage are declared
ad-hoc inside struct flashchip. This improves readability because every
member of struct flashchip is defined (and commented) in one central
place without having to hunt down struct definitions elsewhere. However,
repeating that block of code causes compilers to warn about redefining
those same structs. I don't see any good solution for that.

The version I posted a few minutes ago has a check in selfcheck() to
ensure at least some basic sanity:

/* Check that virtual_memory in struct flashctx is placed directly
 * after the members copied from struct flashchip.
 */
if (sizeof(struct flashchip) != offsetof(struct flashctx, virtual_memory)) {
        msg_gerr("struct flashctx broken!\n");
        ret = 1;
}


Regards,
Carl-Daniel

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