Attention is currently required from: Felix Singer, Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Alexander Goncharov.
1 comment:
Commit Message:
Nico, Can I invite you to attempt to rewrite your wall of text without the condescending^[0] politic […]
Folks, can we please retry this review? We (Angel) agree with Nico's concerns regarding technical aspects of these patches, but we also agree with Edward's view that Nico's "communication style" (for lack of better words) is very unpleasant to work with.
The main problem here seems to be that `flashrom_flashctx` isn't the right structure to handle the state for `programmer_delay`: `flashrom_flashctx` only exists [?] once a chip was successfully probed, and the delay functions are used to probe the chip. From a quick glance, looks like `flashrom_flashctx` contains information regarding the flash chip that was probed: this "chip state" is, in a way, different from the "programmer state" which contains the `programmer_delay` state. This gets even messier when considering that "programmers" and "masters" are different things in flashrom: the former can encompass multiple of the latter.
Hmmm, feral thought: if `flashrom_flashctx` only exists [?] after a flash chip has been successfully probed, why does it contain a pointer to the "master" struct for the programmer? Wouldn't it make more sense to do it the other way around, i.e. have the "master" struct point to the `flashrom_flashctx`?
[?]: `flashrom_flashctx` exists during probing, but the CLI will use a "fresh" structure for subsequent probing after a chip has been successfully probed.
To view, visit change 67393. To unsubscribe, or for help writing mail filters, visit settings.