[coreboot-gerrit] Change in coreboot[master]: pi/hudson: Add LPC IO decode enable function
Marc Jones (Code Review)
gerrit at coreboot.org
Mon Apr 10 02:26:38 CEST 2017
Marc Jones has posted comments on this change. ( https://review.coreboot.org/19160 )
Change subject: pi/hudson: Add LPC IO decode enable function
......................................................................
Patch Set 1:
(9 comments)
https://review.coreboot.org/#/c/19160/1/src/southbridge/amd/pi/hudson/early_setup.c
File src/southbridge/amd/pi/hudson/early_setup.c:
PS1, Line 155: u8 port, uint16_t
> Why are uX and uintX_t types being mixed?
Done
Line 159: LPC_WIDEIO2_ENABLE};
> array[] = {
Done
PS1, Line 164: = 0
> tmp doesn't need to be initialized.
Done
PS1, Line 168: port
> Validate port before using it to access the array? Coverity will definitel
Do we have a macro for this? Seems we should.
PS1, Line 181: = 0
> not needed
Done
PS1, Line 184: assert
> By default, assert just prints a warning message to the log if it fails. I
Doesn't it die and tell you the file function and line? This should always be found by the developer. Not a user.
PS1, Line 184: )
> Please put semi-colons at the end of statements even though our assert() ma
Done
PS1, Line 200: +=
> I know it's resulting in the same thing, but |= is more consistent with set
Done
PS1, Line 204: assert(0);
> Maybe print something more useful?
either the developer finds this and it stops or we just continue on if asserts were disabled. It seems like we were getting away from warnings and using asserts.
--
To view, visit https://review.coreboot.org/19160
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2bed3a99180188101e00b4431d634227e488cbda
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd at gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes
More information about the coreboot-gerrit
mailing list