Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34689 )
Change subject: use JEDEC_SE as the default sector erase opcode for ICH southbridge ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
TLRD; I don't think this is needed.
Please look into old patches more carefully, especially if their commit message doesn't state what the change is about. Generally, I wouldn't reject this change per se. But it would need a commit message that makes sense in the history of the branch it is pushed to.
In this case, it seems the original problem was already solved in different ways upstream. So a commit on either side (upstream/ downstream) should state that its for alignment of the branches. e.g.
ichspi: Replace default JEDEC_BE_D8 with JEDEC_SE
This aligns the upstream master branch with chromium's. Both branches support on-the-fly opcode reprogramming by now, so it shouldn't matter which opcode is the default.
Or the other way around for a commit to the downstream branch.
NB. In my limited experience with porting things over the years over from chromium to upstream, it almost never made sense to take the downstream commits as is because of the loss of context. Unless the commits introduced completely new code, of course. It might be better to pick topics, gather the code changes and write new commits. Though, even that can be slower compared to a clean reimplementation of the topic for upstream.
https://review.coreboot.org/c/flashrom/+/34689/1//COMMIT_MSG Commit Message:
PS1: A commit message should state what is fixed by the change or why the change should be considered useful.
I know you are just forwarding very old commits. But that this one doesn't state its purpose obviously makes it very hard to reason about. In the new history of the upstream master branch it would be even harder because the commit message would be 9 years out- dated at the time of the rebase.
There is a hint, however, about the purpose of the change: The "Note: " below. If you take that together with the state of the code _before_ the original commit, it's starting to make sense.