Nico Huber has posted comments on this change. ( https://review.coreboot.org/17946 )
Change subject: Add a convenient libflashrom interface ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/#/c/17946/10/flash.h File flash.h:
PS10, Line 142: /* TODO: Agree on a name and convert all occurences.
I think the scope of this struct has grown beyond context of the flash chip
That depends on future design decisions, so I didn't want to go through a huge renaming twice. One question is, will we have one huge context for everything? or will we keep a per chip context that only lives from probing on? Stefan has patches to revise the probing btw (hadn't the time to look at them yet)...
The current libflashrom interface suggests that we have two types of contexts `flashrom_programmer` and `flashrom_flashctx`. But we could still use a single context in the implementation. I just wanted to keep all options open. With that said, a next step could be to implement `flashrom_programmer` (starting with the flags, they really don't belong in the chip's context).
With a single huge context, OTOH, we'd have aliases in the imple- mentation if we keep both structs in the interface (what I'd pre- fer). Then I'd agree to `flashrom_ctx` or `flashrom_context` in the implementation.
In either case, we might want to look for a better name for `flashrom_flashctx` in the interface too. Maybe `flashrom_flash` or `flashrom_flashchip`? IMO, both would be a better match for `flashrom_programmer`.
Last but not least, I don't want to annoy Stefan any further with unnecessary renaming atm. It would create a huge burden for anyone rebasing more invasive patches. And I think he's right with that. So we can discuss the names if you want, but I'd want to keep the alias until we have more of the larger patches in.
-
Nico Huber (Code Review)