- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: flashrom.c: Add wrapper function to use the erase algorithm
by Aarya (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Simon Buhrow, Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66104 ) Change subject: flashrom.c: Add wrapper function to use the erase algorithm ...................................................................... Patch Set 92: (9 comments) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/66104/comment/5d1239da_3226feb6 PS91, Line 165: s > `const s` Done https://review.coreboot.org/c/flashrom/+/66104/comment/69d274b5_cd3f5364 PS91, Line 170: old_start_buf > allocation never checked. Done https://review.coreboot.org/c/flashrom/+/66104/comment/0c6b856a_48af033c PS91, Line 182: // > add some `\n` to help readability of the function. Done https://review.coreboot.org/c/flashrom/+/66104/comment/4356146b_96083abd PS91, Line 188: ( > trivial: ` (` Done https://review.coreboot.org/c/flashrom/+/66104/comment/99016884_ade85873 PS91, Line 189: ( > trivial: ` (` Done https://review.coreboot.org/c/flashrom/+/66104/comment/392bd159_c6341e53 PS91, Line 194: if(erase_layout[i].layout_list[j].selected) { > consider identifying more base-cases to early return/continue/break to avoid deep loops and branches […] Done https://review.coreboot.org/c/flashrom/+/66104/comment/50998fbf_0c5cfff2 PS91, Line 195: //execute erase : erasefunc_t *erasefn = lookup_erase_func_ptr(erase_layout[i].eraser); : ret = erasefn(flashctx, start_addr, block_len); : if (ret) { : msg_cerr("Failed to execute erase command for offset %#x to %#x.\n", start_addr, start_addr + block_len); : ret = -1; : goto _end; : } : //adjust curcontents : memset(curcontents+start_addr, erased_value, block_len); : //after erase make it unselected again : erase_layout[i].layout_list[j].selected = false; : msg_cdbg("E(%x:%x)", start_addr, start_addr + block_len - 1); : //verify erase : ret = check_erased_range(flashctx, start_addr, block_len); : if (ret) { : msg_cerr("Verifying flash. Erase failed for range %#x : %#x, Abort.\n", start_addr, start_addr + block_len - 1); : goto _end; : } > integrate CB:70517 ?? File flashrom.c: https://review.coreboot.org/c/flashrom/+/66104/comment/e8e49652_4f8388c6 PS72, Line 1349: static int erase_by_layout > Yeah sorry for the delay. I'm starting to move the implementation to a separate file now. Done File flashrom.c: https://review.coreboot.org/c/flashrom/+/66104/comment/1cf011ca_364dd6ae PS78, Line 1422: write_by_layout > in a separate commit before this one, rename as `write_by_layout_legacy()` and, […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/66104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8 Gerrit-Change-Number: 66104 Gerrit-PatchSet: 92 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Dec 2022 17:01:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c:Add a function to get list of sectors that need erasing
by Aarya (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Simon Buhrow, Edward O'Callaghan. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65844 ) Change subject: flashrom.c:Add a function to get list of sectors that need erasing ...................................................................... Patch Set 74: (6 comments) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/65844/comment/151a1291_97ff5db6 PS73, Line 127: region_end > `rend` and Doxygen. Done https://review.coreboot.org/c/flashrom/+/65844/comment/0816ab4d_167f38e7 PS73, Line 127: void * > `void *` in type system speak is mostly like putting on a blind-fold and jumping between a large can […] Done https://review.coreboot.org/c/flashrom/+/65844/comment/fdee2ac5_28cd8b26 PS73, Line 127: region_start > `rstart` and Doxygen. Done https://review.coreboot.org/c/flashrom/+/65844/comment/fd0be5ef_5bc380ff PS73, Line 127: function_index > `findex` and in the Doxygen comment you can say "function index". Done https://review.coreboot.org/c/flashrom/+/65844/comment/a432e29b_bd28e321 PS73, Line 127: struct flashctx *flashctx, > by convention, keep the `flashctx` as the first argument to the function, it helps keep the code-bas […] Done https://review.coreboot.org/c/flashrom/+/65844/comment/75d3e6c3_55578ac9 PS73, Line 130: function_index == 0 > `!foo` is canonical for the 0 or NULL case. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 74 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Dec 2022 17:01:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom.c:Add function to get a flattened view of the chip erase blocks
by Aarya (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Simon Buhrow, Nico Huber, Thomas Heijligen, Paul Menzel, Edward O'Callaghan. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65879 ) Change subject: flashrom.c:Add function to get a flattened view of the chip erase blocks ...................................................................... Patch Set 55: (11 comments) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/65879/comment/f296e9ed_59988bca PS54, Line 24: int > `unsigned int` Done https://review.coreboot.org/c/flashrom/+/65879/comment/64923c4e_2eddf29b PS54, Line 26: if (layout) { > rewrite your function like this: […] Done https://review.coreboot.org/c/flashrom/+/65879/comment/7ee44179_11644b3e PS54, Line 34: ) > The signature of this function should be, […] Done https://review.coreboot.org/c/flashrom/+/65879/comment/73e83c48_5ae2e8ae PS54, Line 48: ; > \n Done https://review.coreboot.org/c/flashrom/+/65879/comment/1653c5f8_c44e0b67 PS54, Line 54: int j = 0; : while(addr < chip->total_size * 1024) { > ok `j` is shadowed below. Rewrite this while-loop construct as a `for` loop construct as: […] Done https://review.coreboot.org/c/flashrom/+/65879/comment/4a83b380_8a84abed PS54, Line 64: ) : { > trivial: `) {` and `if (!ptr)` is canonical. Done https://review.coreboot.org/c/flashrom/+/65879/comment/c7338721_7c338bb5 PS54, Line 71: chipoff_t start_addr = 0; > does this belong to the outer block_count loop? should it be reset when the inner loop finishes for […] Yes it belongs to outer loop and resets every iteration. https://review.coreboot.org/c/flashrom/+/65879/comment/57cc57f6_de7f650a PS54, Line 72: chipoff_t end_addr = 0; > delete and also don't unnecessarily initialise variables as you obscure compiler warnings. Done https://review.coreboot.org/c/flashrom/+/65879/comment/65606c19_03c74f8f PS54, Line 74: block_num < block_count > since `block_num` is initialised to zero above doesn't this predicate become `0 < block_count` and t […] No, `block_num` increases. https://review.coreboot.org/c/flashrom/+/65879/comment/d8ac57a9_360f9b7f PS54, Line 79: e > Keep minimal scope to variables so declare here, […] Done https://review.coreboot.org/c/flashrom/+/65879/comment/4918a0b3_68be5692 PS54, Line 84: if (layout_idx > 0) { : layout[layout_idx].layout_list[block_num].first_sub_block_index = sub_block_index; : while (layout[layout_idx-1].layout_list[sub_block_index].start_addr >= start_addr && : layout[layout_idx-1].layout_list[sub_block_index].end_addr <= end_addr && : sub_block_index < layout[layout_idx-1].block_count) { : sub_block_index++; : } : layout[layout_idx].layout_list[block_num].last_sub_block_index = sub_block_index - 1; : } > looks like another function with a base case of `layout_idx == 0 => return`. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/65879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafe78de00daa55f7114bd4ce09465dd88074ece4 Gerrit-Change-Number: 65879 Gerrit-PatchSet: 55 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Dec 2022 17:01:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom.c: Add new erase_by_layout and walk_bylayout implementations
by Aarya (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Simon Buhrow, Thomas Heijligen, Aarya. Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen, Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71173 to look at the new patch set (#7). Change subject: flashrom.c: Add new erase_by_layout and walk_bylayout implementations ...................................................................... flashrom.c: Add new erase_by_layout and walk_bylayout implementations Add [erase,walk]_by_layout_new to use optimised implementations of the erase function selection algorithm. Change-Id: Id79ae943eb9d6a817da28381db477725834faaf6 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c 1 file changed, 92 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/71173/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/71173 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id79ae943eb9d6a817da28381db477725834faaf6 Gerrit-Change-Number: 71173 Gerrit-PatchSet: 7 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom.c: Add wrapper function to use the erase algorithm
by Aarya (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Simon Buhrow, Nico Huber, Aarya, Anastasia Klimchuk. Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/66104 to look at the new patch set (#92). Change subject: flashrom.c: Add wrapper function to use the erase algorithm ...................................................................... flashrom.c: Add wrapper function to use the erase algorithm Add a function to call the erase algorithm. Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M erasure_layout.c M erasure_layout.h 2 files changed, 116 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/66104/92 -- To view, visit https://review.coreboot.org/c/flashrom/+/66104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8 Gerrit-Change-Number: 66104 Gerrit-PatchSet: 92 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c:Add a function to get list of sectors that need erasing
by Aarya (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Simon Buhrow, Aarya. Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen, Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/65844 to look at the new patch set (#74). Change subject: flashrom.c:Add a function to get list of sectors that need erasing ...................................................................... flashrom.c:Add a function to get list of sectors that need erasing Add a function that returns a list of sectors (as seen by the first erase function) that need erasing. Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M erasure_layout.c 1 file changed, 48 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/65844/74 -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 74 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom.c:Add function to get a flattened view of the chip erase blocks
by Aarya (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Simon Buhrow, Nico Huber, Thomas Heijligen, Paul Menzel, Aarya. Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen, Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/65879 to look at the new patch set (#55). Change subject: flashrom.c:Add function to get a flattened view of the chip erase blocks ...................................................................... flashrom.c:Add function to get a flattened view of the chip erase blocks Add a function to flatten out the addresses of the flash chip as per the different erase functions. This function will return a list of layouts which is dynamically allocated. So after use all the layouts as well as the list itself should be freed. The free_erase_layout function does that. Change-Id: Iafe78de00daa55f7114bd4ce09465dd88074ece4 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- A erasure_layout.c A erasure_layout.h 2 files changed, 158 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/65879/55 -- To view, visit https://review.coreboot.org/c/flashrom/+/65879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafe78de00daa55f7114bd4ce09465dd88074ece4 Gerrit-Change-Number: 65879 Gerrit-PatchSet: 55 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom.c: Add wrapper function to use the erase algorithm
by Edward O'Callaghan (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Simon Buhrow, Nico Huber, Aarya, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66104 ) Change subject: flashrom.c: Add wrapper function to use the erase algorithm ...................................................................... Patch Set 91: (8 comments) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/66104/comment/8fb97c19_b1d7589d PS91, Line 1: /* you can use clang-format to help with formatting like line wraps or do it manually but whatever you do you should try to keep consistent with generally kernel C style and you wont be far off. https://www.kernel.org/doc/html/v4.10/process/coding-style.html https://review.coreboot.org/c/flashrom/+/66104/comment/45429ca9_5bcb04ee PS91, Line 165: s `const s` https://review.coreboot.org/c/flashrom/+/66104/comment/e47d5bf7_b747cf71 PS91, Line 170: old_start_buf allocation never checked. https://review.coreboot.org/c/flashrom/+/66104/comment/698e2517_e9b37ae5 PS91, Line 182: // add some `\n` to help readability of the function. https://review.coreboot.org/c/flashrom/+/66104/comment/f1466463_521e44ef PS91, Line 188: ( trivial: ` (` https://review.coreboot.org/c/flashrom/+/66104/comment/07bc1f69_04439fea PS91, Line 189: ( trivial: ` (` https://review.coreboot.org/c/flashrom/+/66104/comment/37d0fadf_f54d1d10 PS91, Line 194: if(erase_layout[i].layout_list[j].selected) { consider identifying more base-cases to early return/continue/break to avoid deep loops and branches. For example in this case, the inverse case gives you a continue base case: ``` if (!erase_layout[i].layout_list[j].selected) { continue; } [..] ``` https://review.coreboot.org/c/flashrom/+/66104/comment/953096fc_fba6f404 PS91, Line 195: //execute erase : erasefunc_t *erasefn = lookup_erase_func_ptr(erase_layout[i].eraser); : ret = erasefn(flashctx, start_addr, block_len); : if (ret) { : msg_cerr("Failed to execute erase command for offset %#x to %#x.\n", start_addr, start_addr + block_len); : ret = -1; : goto _end; : } : //adjust curcontents : memset(curcontents+start_addr, erased_value, block_len); : //after erase make it unselected again : erase_layout[i].layout_list[j].selected = false; : msg_cdbg("E(%x:%x)", start_addr, start_addr + block_len - 1); : //verify erase : ret = check_erased_range(flashctx, start_addr, block_len); : if (ret) { : msg_cerr("Verifying flash. Erase failed for range %#x : %#x, Abort.\n", start_addr, start_addr + block_len - 1); : goto _end; : } integrate CB:70517 -- To view, visit https://review.coreboot.org/c/flashrom/+/66104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8 Gerrit-Change-Number: 66104 Gerrit-PatchSet: 91 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 23 Dec 2022 10:50:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: spi: Make 'default_spi_send_multicommand' the default unless defined
by Edward O'Callaghan (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67481 ) Change subject: spi: Make 'default_spi_send_multicommand' the default unless defined ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/67481/comment/04dd808a_04c1c39f PS1, Line 9: Drop the explicit need to specify the default > Please mention the struct where you perform the changes on Done Patchset: PS1: > The same as in CB:67480 applies here. see CB:67480 for dicussion. -- To view, visit https://review.coreboot.org/c/flashrom/+/67481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6cc24bf982da3d5251d391eb397db43dd10280e8 Gerrit-Change-Number: 67481 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Fri, 23 Dec 2022 08:06:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: spi: Make 'default_spi_send_multicommand' the default unless defined
by Edward O'Callaghan (Code Review) 23 Dec '22

23 Dec '22

Attention is currently required from: Edward O'Callaghan. Hello build bot (Jenkins), Thomas Heijligen, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67481 to look at the new patch set (#2). Change subject: spi: Make 'default_spi_send_multicommand' the default unless defined ...................................................................... spi: Make 'default_spi_send_multicommand' the default unless defined A NULL func pointer is necessary and sufficient for the condition `NULL func pointer => default_spi_send_multicommand' as to not need this explicit specification of 'default'. Therefore drop the explicit need to specify the 'default_spi_send_multicommand' callback function pointer in the spi_master struct. This is a reasonable default for every other driver in the tree with only a few exceptions. This simplifies the code and driver development. Change-Id: I6cc24bf982da3d5251d391eb397db43dd10280e8 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M dediprog.c M digilent_spi.c M dummyflasher.c M it87spi.c M jlink_spi.c M linux_spi.c M mediatek_i2c_spi.c M mstarddc_spi.c M ni845x_spi.c M parade_lspcon.c M pickit2_spi.c M raiden_debug_spi.c M realtek_mst_i2c_spi.c M sb600spi.c M serprog.c M spi.c M stlinkv3_spi.c M usbblaster_spi.c M wbsio_spi.c 22 files changed, 24 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/67481/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6cc24bf982da3d5251d391eb397db43dd10280e8 Gerrit-Change-Number: 67481 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit