[coreboot-gerrit] Change in coreboot[master]: nb/intel/i945: Remove dead code when channel B is empty

Nico Huber (Code Review) gerrit at coreboot.org
Fri Jun 22 16:18:16 CEST 2018


Nico Huber has posted comments on this change. ( https://review.coreboot.org/27204 )

Change subject: nb/intel/i945: Remove dead code when channel B is empty
......................................................................


Patch Set 7: Code-Review-1

(1 comment)

https://review.coreboot.org/#/c/27204/7/src/northbridge/intel/i945/raminit.c
File src/northbridge/intel/i945/raminit.c:

https://review.coreboot.org/#/c/27204/7/src/northbridge/intel/i945/raminit.c@1201
PS7, Line 1201: 	/* If Channel 1 is not populated. C1DRB are equal to C0DRB3 */
I hate comments.

The old comment, with the `#if 0` in mind, at least told us that
somebody expected it should be set to 0 (maybe because some docu-
mentation told him so), but somehow it didn't work.

The new comment, OTOH, makes the impression that it is supposed
to be like the code currently does and that the author knew what
he was doing. Which makes it worse than not commenting at all.
I even doubt that it adds any value.

So please, no new comment, or leave everything as it is.



-- 
To view, visit https://review.coreboot.org/27204
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic26103aac7f920e5696b445e125d33405df4f43b
Gerrit-Change-Number: 27204
Gerrit-PatchSet: 7
Gerrit-Owner: Elyes HAOUAS <ehaouas at noos.fr>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Elyes HAOUAS <ehaouas at noos.fr>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Fri, 22 Jun 2018 14:18:16 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180622/ddec54a2/attachment.html>


More information about the coreboot-gerrit mailing list