Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41393 )
Change subject: util: add a tool to dump and insert MEC1322 firmware ......................................................................
Patch Set 3: Code-Review+1
(8 comments)
https://review.coreboot.org/c/coreboot/+/41393/3/util/mec1322/mec1322.c File util/mec1322/mec1322.c:
https://review.coreboot.org/c/coreboot/+/41393/3/util/mec1322/mec1322.c@9 PS3, Line 9: openssl Ah, I see why it's not enabled by default
https://review.coreboot.org/c/coreboot/+/41393/3/util/mec1322/mec1322.c@19 PS3, Line 19: image ec_fw ?
https://review.coreboot.org/c/coreboot/+/41393/3/util/mec1322/mec1322.c@38 PS3, Line 38: Why noy use tabs everywhere?
https://review.coreboot.org/c/coreboot/+/41393/3/util/mec1322/mec1322.c@44 PS3, Line 44: const uint8_t CRC_TABLE[] = {0x00, 0x07, 0x0e, 0x09, 0x1c, 0x1b, 0x12, 0x15, : 0x38, 0x3f, 0x36, 0x31, 0x24, 0x23, 0x2a, 0x2d}; nit: place the values in a new line?
const uint8_t CRC_TABLE[] = { 0x00, 0x07, 0x0e, 0x09, 0x1c, 0x1b, 0x12, 0x15, 0x38, 0x3f, 0x36, 0x31, 0x24, 0x23, 0x2a, 0x2d, };
https://review.coreboot.org/c/coreboot/+/41393/3/util/mec1322/mec1322.c@79 PS3, Line 79: 32 #define this?
https://review.coreboot.org/c/coreboot/+/41393/3/util/mec1322/mec1322.c@82 PS3, Line 82: 256 #define this?
https://review.coreboot.org/c/coreboot/+/41393/3/util/mec1322/mec1322.c@152 PS3, Line 152: fwTag nit: fw_tag
https://review.coreboot.org/c/coreboot/+/41393/3/util/mec1322/mec1322.c@157 PS3, Line 157: uint32_t Casting this shouldn't be necessary