[XS] Change in flashrom[main]: erasure_layout: Erase larger block only when all sub-block need erase

[Anastasia Klimchuk (Code Review)]

Attention is currently required from: Aarya, Bill XIE.

Anastasia Klimchuk has posted comments on this change by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84614?usp=email )

Change subject: erasure_layout: Erase larger block only when all sub-block need erase
......................................................................

Patch Set 1:

(1 comment)

Patchset:

PS1: 
Bill, thank you so much for looking into the details of the erasure logic! And the patch with the test scenario, I appreciate so much!

My first thought is that this needs a test, and the fact that existing tests are passing both before and after the patch means this scenario is not really tested. It should be. I am going to inspect existing cases (in tests/erase_func_algo.c) and either add or modify something. I will post here how it goes.
As you said, this code is hardware independent, I have hopes to eventually get it fully covered with tests.

Also, very important two points that you described in commit message. I think that p.2 should be fixed regardless of p.1, because it creates the situation of redundant double erase. 
I would call it "nested sub-blocks" instead of "double sub-blocks", what do you think?

p.1 is a good question to discuss. My understanding, the idea was that if we have a region, and there are many small blocks inside that need erase, maybe it's faster to send one opcode once, and erase the whole region (instead of sending lots of opcodes for each small region).
How to decide when "maybe it's faster" was a threshold of how much area is covered by small blocks. And before the patch the threshold was 50% , you changed it to 100%.
I am thinking that maybe the optimal is in between?

And anyway, I think p.2 needs to be fixed. Not in this patch. I now want to make a patch which changes the code to fix p.2 and also write a test which would fail on head but succeed on the patch. Will you give me some time for this?

Also by the way, some time ago I discovered the opposite issue (smaller blocks selected instead of larger one), which I recorded as a failing test in CB:82723. Haven't started looking into it yet, but surely I want to. Just mentioning here because it belongs to the same topic.

Oh, it's a long comment, to summarise the questions are
> And before the patch the threshold was 50% , you changed it to 100%.

I am thinking that maybe the optimal is in between?
> I now want to make a patch which changes the code to fix p.2 and also write a test which would fail on head but succeed on the patch. Will you give me some time for this?