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

[Nico Huber (Code Review)]

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 <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Comment-Date: Fri, 26 Jan 2018 16:05:01 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No