Attention is currently required from: Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56721 )
Change subject: internal: Return early from map_flash for >16MiB on x86 ......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
Sorry that was Patchset 1, where I got asked to switch it to the physmap area... […]
I do admit I haven't read the full history of the patch (sorry), I added my comment just based on what I saw in the latest patchset.
Ok, so now I did read comments from the beginning. Is this what you are referring to:
platform specific code it probably should be encapsulated within internal.c itself. The flashrom.c implementation is generic dispatch and handling code and so should not really have platform specifics here.
Now I understand where is comes from, because in internal.c .map_flash_region = physmap,
In this case, if internal needs something special, can it have its own internal_map function which does all the special things? I can see why flashrom.c is not the right place, but I still think physmap_common is too deep (so, not the right place either). The problem is, at the moment there is nothing in between, but maybe let's create something in between?
What do people think?
-
Anastasia Klimchuk (Code Review)