Attention is currently required from: Edward O'Callaghan, Jes Klinke.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77999?usp=email )
Change subject: raiden: Support target index with generic REQ_ENABLE ......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/77999/comment/2e179ba2_cc3e250a : PS2, Line 45: CL Just noticed: you need to say "this patch" "CL" is internal corp terminology, not applicable for upstream
File raiden_debug_spi.c:
https://review.coreboot.org/c/flashrom/+/77999/comment/0d981909_ab5554af : PS2, Line 1596: request_enable & 0xFF
See below. Yes, the EC and AP values fit inside 8 bits. […]
If this is a no-op, and given your idea to change the signature of `get_target` , maybe this is not needed and third argument can stay `request_enable` ?
https://review.coreboot.org/c/flashrom/+/77999/comment/80956f4e_cbbf7760 : PS2, Line 1596: request_enable & 0xFF, : (request_enable >> 8) & 0xFF, Jes, thank you for detailed explanation, I understand much better now!
Perhaps I should instead change the signature into
void get_target(const struct programmer_cfg *cfg, uint8_t *request_enable, uint16_t *request_parameter) such that the function can "return" two separate values.
I think this is a very good idea, and with it `request_enable` can keep the same meaning.
I have extended the Raiden protocol such that the lower byte of wValue can indicate which one of such an array of similar SPI ports to use
I just thought, there is a piece of documentation at the beginning of this file, does it all still apply? I looked through quickly, seems like it doesn't need to be changed with this patch, but maybe you can check as well. Thank you!
-
Anastasia Klimchuk (Code Review)