Patch Set 1:

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.

Hi Nico,

Just so I understand you correctly and I can help Mayur with his patch here.. The two issues here are that the commit message is essentially obsolete for the context of today and this change isn't 'strictly' needed because both trees support on-the-fly opcode reprogramming? However, it should be harmless for the purposes of resync the two trees and making the comment make a bit more sense with the define name? Thus, provided the commit message is something aliken your suggestion then this commit is still alright to go forward with?

Yes, you can go forward. The change really seems like a no-op to me.

However, people can fail, and upstream has very limited reviewing
capacities. So generally, I would prefer a downstream change in such
cases.

Understood about upstream review bandwidth. this is my first patch Edward is helping me with so I am just considering this a learning experience on how to get a patch into flashrom.

I shall update the commit description to follow the advice

View Change

To view, visit change 34689. To unsubscribe, or for help writing mail filters, visit settings.