Attention is currently required from: Aarya, Anastasia Klimchuk, Carly Zlabek.
1 comment:
Patchset:
Vincent, Carly […]
In order to keep the code consistent, I think we can drop #2
```
// verify erase
ret = check_erased_range(flashctx, start_addr, block_len);
if (ret) {
msg_cerr("Verifying flash. Erase failed for range %#"PRIx32" : %#"PRIx32", Abort.\n",
start_addr, start_addr + block_len - 1);
goto _end;
}
```
This way the erase and verification are more tightly coupled and we avoid trying to verify ranges that are read/write protected.
I'll try to schedule this work for this iteration so we can get this in sooner rather than later.
--
As part of looking over #1 and #2, I developed some slight concerns about the loop on line 314 which is separate from this patchset. I don't know if it warrants an issue and further discussion, and I haven't spelunked into the erase function selection logic, but the concern I have there is we've already pre-calculated what erase functions we're using but `get_flash_region` could return a region with a shorter end range, meaning the selected erase block fn could erase part of the next region. So if we had a 32k erase block fn, it seems like if `region.end` forces the length of the erasable region to 4k, we don't actually select a 4k erase function and instead continue to use a 32k function. If the next region is write protected it seems like we'd have verification errors.
I think this currently only affects masters that have `get_region` defined, so `opaque_master_ich_hwseq` is at risk here. This may have been less of a risk with the legacy path because, from what I remember, it always used the smallest working erase block function, however the new path adjusts the function used based on the amount of changed data and coalesces blocks when possible (I think).
To view, visit change 79354. To unsubscribe, or for help writing mail filters, visit settings.