Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40325 ) Change subject: Add writeprotect support ...................................................................... Patch Set 7: Code-Review+1 (5 comments) I'm not sure if this could be split up a bit. https://review.coreboot.org/c/flashrom/+/40325/7/writeprotect.h File writeprotect.h: https://review.coreboot.org/c/flashrom/+/40325/7/writeprotect.h@44 PS7, Line 44: wp_wpce775x Do we have this programmer upstream? https://review.coreboot.org/c/flashrom/+/40325/7/writeprotect.c File writeprotect.c: https://review.coreboot.org/c/flashrom/+/40325/7/writeprotect.c@873 PS7, Line 873: uint8_t mx25l_read_config_register(const struct flashctx *flash);//XXX Hmm? https://review.coreboot.org/c/flashrom/+/40325/7/writeprotect.c@1132 PS7, Line 1132: //case ATMEL_AT25SF128A: Hm? https://review.coreboot.org/c/flashrom/+/40325/7/writeprotect.c@1473 PS7, Line 1473: /* FIXME: this is NOT endian-free copy. */ How about providing a union instead of a struct? Here's an example: https://github.com/coreboot/coreboot/blob/dc0c08100124278efc9ed91952378b0190... https://review.coreboot.org/c/flashrom/+/40325/7/writeprotect.c@2236 PS7, Line 2236: //case SPANSION_ID: : // switch (flash->chip->model_id) { : // case SPANSION_S25FS128S_L: : // case SPANSION_S25FS128S_S: { : // *wp = &s25fs128s_wp; : // (*wp)->descrs = s25fs128s_ranges; : // *num_entries = ARRAY_SIZE(s25fs128s_ranges); : // break; : // } : // case SPANSION_S25FL256S_UL: : // case SPANSION_S25FL256S_US: { : // *wp = &s25fl256s_wp; : // (*wp)->descrs = s25fl256s_ranges; : // *num_entries = ARRAY_SIZE(s25fl256s_ranges); : // break; : // } : // default: : // msg_cerr("%s():%d Spansion flash chip mismatch (0x%04x)" : // ", aborting\n", __func__, __LINE__, : // flash->chip->model_id); : // return -1; : // } : // break; Um? -- To view, visit https://review.coreboot.org/c/flashrom/+/40325 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id93b5a1cb2da476fa8a7dde41d7b963024117474 Gerrit-Change-Number: 40325 Gerrit-PatchSet: 7 Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Damien Zammit Gerrit-CC: David Hendricks <david.hendricks@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-CC: Stefan Reinauer <stefan.reinauer@coreboot.org> Gerrit-Comment-Date: Wed, 16 Sep 2020 17:54:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment