Attention is currently required from: Anastasia Klimchuk, Bill XIE, Peter Marheine.
3 comments:
File ichspi.c:
Patch Set #1, Line 402: static OPCODE POSSIBLE_OPCODES[] = {
What is the meaning of this array ? It is called POSSIBLE_OPCODES , but for example in the logs of t […]
There's some logic in reprogram_opcode_on_the_fly() to handle the case where an opcode isn't found in this array (i.e. when lookup_spi_type() returns 0xFF). The array name is misleading since it doesn't contain all possible opcodes.
Patch Set #1, Line 664: int oppos = 2; // use original JEDEC_BE_D8 offset
I have questions in my head […]
The choice of index 2 here does seem suboptimal. It might be good to change it in a different CL since it could possibly break something.
It's worth noting that newer CPUs don't support this code path (they use hwseq) so it's hard to test any changes thoroughly.
static bool ich_spi_probe_opcode(const struct flashctx *flash, uint8_t opcode)
{
return find_opcode(curopcodes, opcode) >= 0;
Rather than restoring JEDEC_SE after every ich_spi_send_command() call, we could add a check here to see if the opcode is in POSSIBLE_OPCODES and return true if it is.
That way, ich_spi_send_command will automatically reprogram the required opcode if it is missing and we don't have to restore JEDEC_SE after every command that reprograms it.
To view, visit change 84253. To unsubscribe, or for help writing mail filters, visit settings.