November 2021 - flashrom-gerrit

Change in flashrom[master]: cli_classic: add writeprotect CLI
by Anastasia Klimchuk (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 ) Change subject: cli_classic: add writeprotect CLI ...................................................................... Patch Set 7: (4 comments) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/b4ba353a_4a3bb1a2 PS7, Line 63: mode What <mode> is? I think this needs explanation, if someone runs `flashrom -h` they need to be able to understand how to use flashrom wp feature https://review.coreboot.org/c/flashrom/+/58738/comment/69c3b3a1_86cd318f PS7, Line 222: if (!any_wp_op) return 0; any_wp_op seems like a duplication of information, you could check if any of commands is present? https://review.coreboot.org/c/flashrom/+/58738/comment/16eb9ad8_c62c05c1 PS7, Line 257: return ret; Please move conditional body on the new line (and same for all other occurrences like this). https://review.coreboot.org/c/flashrom/+/58738/comment/b7601add_eb54b7fa PS7, Line 920: goto out_release; and the ones like this too, please move on a new line -- To view, visit https://review.coreboot.org/c/flashrom/+/58738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869 Gerrit-Change-Number: 58738 Gerrit-PatchSet: 7 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Wed, 17 Nov 2021 00:23:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add flashrom_wp_{config,range,mode}_to_str()
by Anastasia Klimchuk (Code Review) 16 Nov '21

16 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58484 ) Change subject: libflashrom,writeprotect: add flashrom_wp_{config,range,mode}_to_str() ...................................................................... Patch Set 11: (2 comments) File libflashrom.h: https://review.coreboot.org/c/flashrom/+/58484/comment/23d74627_935ca34c PS11, Line 138: #define FLASHROM_WP_OUT_STR_SIZE 1024 Why 1024? Same question for 128 above. An arbitrary number is totally fine, but a comment would be helpful, even if the comment like programmer.c#112 it is still helpful, gives information. File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58484/comment/ebad3c5b_0dd3537c PS11, Line 436: for (int i = cfg->srp_bit_count - 1; i >= 0; i--) { : buf += sprintf(buf, "SRP%d=%u ", i, cfg->srp[i]); : } : : if (cfg->cmp_bit_present) : buf += sprintf(buf, "CMP=%u ", cfg->cmp); : : if (cfg->sec_bit_present) : buf += sprintf(buf, "SEC=%u ", cfg->sec); : : if (cfg->tb_bit_present) : buf += sprintf(buf, "TB=%u ", cfg->tb); : : for (int i = cfg->bp_bit_count - 1; i >= 0; i--) { : buf += sprintf(buf, "BP%d=%u ", i, cfg->bp[i]); : } : I would prefer the same ordering as in wp_chip_struct defined in CB:58478 -- To view, visit https://review.coreboot.org/c/flashrom/+/58484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I82441c60c239a5b947b5a2a87b61e7bdf60bee89 Gerrit-Change-Number: 58484 Gerrit-PatchSet: 11 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 16 Nov 2021 23:34:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: writeprotect, cli_classic: delete old writeprotect code
by Angel Pons (Code Review) 16 Nov '21

16 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Nikolai Artemiev, Sergii Dmytruk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58474 ) Change subject: writeprotect, cli_classic: delete old writeprotect code ...................................................................... Patch Set 9: (1 comment) File libflashrom.h: https://review.coreboot.org/c/flashrom/+/58474/comment/1764fe7d_f3f312f1 PS9, Line 4: 2010 > It is 2021 now ;) I don't see why a commit that says `delete` in its summary should be adding anything. -- To view, visit https://review.coreboot.org/c/flashrom/+/58474 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I67e9b31f86465e5a8f7d3def637198671ee818a8 Gerrit-Change-Number: 58474 Gerrit-PatchSet: 9 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 16 Nov 2021 23:06:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add flashrom_wp_get_available_ranges()
by Anastasia Klimchuk (Code Review) 16 Nov '21

16 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58481 ) Change subject: libflashrom,writeprotect: add flashrom_wp_get_available_ranges() ...................................................................... Patch Set 10: (1 comment) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58481/comment/fb352f65_1ab37f18 PS10, Line 271: ranges[++last] = ranges[i]; > In the first iteration (last == 0, i == 0), it would compare the same […] Thank you! I took a paper and a pen and now I understand :) The code transforms AAABCD into ABCDCD and then cuts the array. This is why I got confused - the dupes are still there, I missed that array length is cut. Agree with empty array situation, maybe return early if count == 0 ? -- To view, visit https://review.coreboot.org/c/flashrom/+/58481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d Gerrit-Change-Number: 58481 Gerrit-PatchSet: 10 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 16 Nov 2021 22:52:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Angel Pons (Code Review) 16 Nov '21

16 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 16 Nov 2021 22:31:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Angel Pons (Code Review) 16 Nov '21

16 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Nikolai Artemiev. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I want to talk more about this patch. […] ICH7 does not support hardware sequencing (hwseq). -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 16 Nov 2021 22:31:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add flashrom_wp_get_available_ranges()
by Nico Huber (Code Review) 16 Nov '21

16 Nov '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58481 ) Change subject: libflashrom,writeprotect: add flashrom_wp_get_available_ranges() ...................................................................... Patch Set 10: (2 comments) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58481/comment/8c9d3160_7ece42e9 PS10, Line 271: ranges[++last] = ranges[i]; > I don't understand how it works :(( this assignment... […] In the first iteration (last == 0, i == 0), it would compare the same element, hence `duplicate` would be `true` and this line would be skipped. In further iterations, `last` would at least be one less than `i`, hence it would potentially overwrite duplicates (at `last + 1`) with later elements at `i`. It's hard to read, indeed. Starting the loop at `i == 1` might make things easier. And there is at least one (theoretical?) case where the code fails, see below. https://review.coreboot.org/c/flashrom/+/58481/comment/dfc5b99e_92f14bdb PS10, Line 274: *count = last + 1; If `*count` was 0 initially, i.e. an empty list, this would set it to `1`. -- To view, visit https://review.coreboot.org/c/flashrom/+/58481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d Gerrit-Change-Number: 58481 Gerrit-PatchSet: 10 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 16 Nov 2021 15:08:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add flashrom_wp_get_available_ranges()
by Anastasia Klimchuk (Code Review) 16 Nov '21

16 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58481 ) Change subject: libflashrom,writeprotect: add flashrom_wp_get_available_ranges() ...................................................................... Patch Set 10: (2 comments) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58481/comment/a3370e23_e7421b48 PS10, Line 192: if (ord == 0) ord = a->range.len - b->range.len; Same as before: please move conditional body on new line (and for all the rest of code in the chain). https://review.coreboot.org/c/flashrom/+/58481/comment/3c6e4f39_05999809 PS10, Line 271: ranges[++last] = ranges[i]; I don't understand how it works :(( this assignment... replaces a duplicate with another duplicate? -- To view, visit https://review.coreboot.org/c/flashrom/+/58481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d Gerrit-Change-Number: 58481 Gerrit-PatchSet: 10 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 16 Nov 2021 06:06:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flash: add structure to represent chip wp configuration
by Anastasia Klimchuk (Code Review) 16 Nov '21

16 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58478 ) Change subject: flash: add structure to represent chip wp configuration ...................................................................... Patch Set 9: (1 comment) File flash.h: https://review.coreboot.org/c/flashrom/+/58478/comment/e0c04f14_97501b70 PS9, Line 219: /* Used for intrepreting configuration bits above */ : const struct flashchip *chip; > A question: is interpreting config bits implemented in CB:58479 or there are other patches? In CB:58 […] Correction: I found implementation in CB:59183 and further in the chain (and added comments there). I still think this pointer shouldn't be in config struct. -- To view, visit https://review.coreboot.org/c/flashrom/+/58478 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I17dee630248ce7b51e624a6e46d7097d5d0de809 Gerrit-Change-Number: 58478 Gerrit-PatchSet: 9 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 16 Nov 2021 05:46:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add flashrom_wp_get_range()
by Anastasia Klimchuk (Code Review) 16 Nov '21

16 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59183 ) Change subject: libflashrom,writeprotect: add flashrom_wp_get_range() ...................................................................... Patch Set 2: (3 comments) File libflashrom.h: https://review.coreboot.org/c/flashrom/+/59183/comment/89ada889_6d4b9fbf PS2, Line 141: int flashrom_wp_get_range(const struct flashrom_wp_chip_config *cfg, struct flashrom_wp_range *range); You can omit param names here, and the same for the rest of the chain (since the rest of the chain is adding functions in this header) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/59183/comment/d8c94078_beba47aa PS2, Line 182: cfg->chip Unless people convince me otherwise, I think chip can be an argument to flashrom_wp_get_range , and no need to store a pointer in wp_chip_config. https://review.coreboot.org/c/flashrom/+/59183/comment/cc96580a_8fdc0160 PS2, Line 183: decode_range Yes, you can pass chip_len as one more argument, doesn't need to be in range struct. -- To view, visit https://review.coreboot.org/c/flashrom/+/59183 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5a1dfcf384166b1bac319d286306747e1dcaa000 Gerrit-Change-Number: 59183 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 16 Nov 2021 05:44:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2021