Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39599 )
Change subject: nb/intel/sandybridge: Tidy up code and comments ......................................................................
Patch Set 5: Code-Review+1
(9 comments)
https://review.coreboot.org/c/coreboot/+/39599/4/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/gma.c:
https://review.coreboot.org/c/coreboot/+/39599/4/src/northbridge/intel/sandy... PS4, Line 722: i'd still use tabs and not spaces here
https://review.coreboot.org/c/coreboot/+/39599/4/src/northbridge/intel/sandy... PS4, Line 743: tabs
https://review.coreboot.org/c/coreboot/+/39599/4/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/northbridge.c:
https://review.coreboot.org/c/coreboot/+/39599/4/src/northbridge/intel/sandy... PS4, Line 259: tabs?
https://review.coreboot.org/c/coreboot/+/39599/4/src/northbridge/intel/sandy... PS4, Line 455: tabs?
https://review.coreboot.org/c/coreboot/+/39599/4/src/northbridge/intel/sandy... PS4, Line 471: tabs?
https://review.coreboot.org/c/coreboot/+/39599/4/src/northbridge/intel/sandy... PS4, Line 477: tabs?
https://review.coreboot.org/c/coreboot/+/39599/2/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_common.c:
https://review.coreboot.org/c/coreboot/+/39599/2/src/northbridge/intel/sandy... PS2, Line 3309: =
I ended up placing a zero and a chain of or-statements. […]
i find it slightly odd. i'd drop just the 0 and put the | at the ends of the last lines. probably the main reason why like to have the = or | at the end of a line is that it's clearer to me that the statement will continue in the next line and that it isn't just a missing ;
https://review.coreboot.org/c/coreboot/+/39599/2/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_ivy.c:
https://review.coreboot.org/c/coreboot/+/39599/2/src/northbridge/intel/sandy... PS2, Line 51: tREFI = 7.8usec not sure, but this might be some information that should be kept as comment?
https://review.coreboot.org/c/coreboot/+/39599/2/src/northbridge/intel/sandy... PS2, Line 79: tXS-offset: tXS = tRFC+10ns same?