Attention is currently required from: Thomas Heijligen, Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73825 )
Change subject: atavia: Use a atavia_offset locally to determin offset ......................................................................
Patch Set 1: Code-Review+1
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/73825/comment/dc3596a1_6eed7b46 PS1, Line 7: Use a atavia_offset locally to determin offset I think this should be more specific:
`Always initialise atavia_offset from programmer param`
And the below, in commit description you can add more clarification like
`Remove custom atavia_map function and detect the value of atavia_offset from programmer param. This means to use atavia_offset directly <and the the rest of your description>`
File atavia.c:
https://review.coreboot.org/c/flashrom/+/73825/comment/190bbf64_32432ff5 PS1, Line 58: try and error trial and error
https://review.coreboot.org/c/flashrom/+/73825/comment/4d91b4b7_fcc1d42b PS1, Line 57: /* The atavia_offset is some kind of magic to get the driver working. : * You can find the correct value in the documentation or by try and error. : */ I think you need to add new line:
/* * comment */
https://review.coreboot.org/c/flashrom/+/73825/comment/9fbb0a24_ed9e1e9c PS1, Line 140: char *arg = extract_programmer_param_str(cfg, "offset"); It seems like offset should be a required parameter now? I don't think the default offset 0 would ever work?
-
Anastasia Klimchuk (Code Review)