Change in flashrom[master]: [v4, 4/6] ENE EDI - print debug info like others while probing for ENE...

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23261 ) Change subject: [v4,4/6] ENE EDI - print debug info like others while probing for ENE chips ...................................................................... Patch Set 1: (4 comments) https://review.coreboot.org/#/c/23261/1/edi.c File edi.c: https://review.coreboot.org/#/c/23261/1/edi.c@154 PS1, Line 154: 0x%x It's a signed integer, why print it in hex? https://review.coreboot.org/#/c/23261/1/edi.c@164 PS1, Line 164: msg_cdbg("%s: rc1 0x%x, rc2 0x%x; ", __func__, rc1, rc2); No need to print them, they are 0 on this path. https://review.coreboot.org/#/c/23261/1/edi.c@169 PS1, Line 169: } else Please add braces here too (they are not required by syntax but help to maintain visual balance). https://review.coreboot.org/#/c/23261/1/edi.c@171 PS1, Line 171: hwversion, chip->hwversion, ediid, chip->ediid); Actually, I'm not sure if we want this message at all. This function is supposed to be called multiple times with different ids. So it's expected that they don't match. If you want to keep it, please use a higher debug level, e.g. msg_cdbg2(). -- To view, visit https://review.coreboot.org/23261 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8e62bc9f6785b4bf0be0aaf0f74c8120d77c0d4 Gerrit-Change-Number: 23261 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Banon <mikebdp2@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Fri, 26 Jan 2018 16:05:01 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
participants (1)
-
Nico Huber (Code Review)