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:
(6 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/77999/comment/c52412b0_b232915d : PS2, Line 42: target=1 Since these are new values for `target` param, please document it on the manpage, here:
doc/classic_cli_manpage.rst
https://review.coreboot.org/c/flashrom/+/77999/comment/85410071_f9a4b2f8 : PS2, Line 45: archieves achieves (typo)
https://review.coreboot.org/c/flashrom/+/77999/comment/c94080aa_f2975417 : PS2, Line 48: run flashrom with HyperDebug Could you please list the exact commands that you ran? I read the commit message (thank you for providing so many details), and it would be ideal to run flashrom with both new and old values of `target` param. Run and then add to commit message, thank you!
Patchset:
PS2: Jes, thank you for the patch! I have few comments.
File raiden_debug_spi.c:
https://review.coreboot.org/c/flashrom/+/77999/comment/a675bd52_f8e5f8d6 : PS2, Line 1596: request_enable & 0xFF Does this still work for target values EC and AP ?
https://review.coreboot.org/c/flashrom/+/77999/comment/e9c85ba6_949f29ce : PS2, Line 1596: request_enable & 0xFF, : (request_enable >> 8) & 0xFF, A question: why do you initialise `request_enable` to a value that you never use by itself, just as `request_enable`? I see it is used as `request_enable & 0xFF` and as `(request_enable >> 8) & 0xFF` but never by itself? What is the meaning of `request_enable` ?
-
Anastasia Klimchuk (Code Review)