Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45435 )
Change subject: nb/intel/gm45: Guard macro parameters ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45435/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45435/4//COMMIT_MSG@11 PS4, Line 11: the resulting coreboot image. The reasons have not been determined. Simple: because it's broken. It happens exactly what the brackets should prevent: `(2 + 1 % 2) * 16` is not a good shift for 32-bit regs ;)
So, if I read the code correctly, we program the last rank boundary always to 0 (because the boundary gets shifted far away). Intel definitely wanted it to be programmed, even though the chip only has 2 CS lines per channel.
Feel free to fix it. I guess it makes no difference, though.
https://review.coreboot.org/c/coreboot/+/45435/4/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/gm45.h:
https://review.coreboot.org/c/coreboot/+/45435/4/src/northbridge/intel/gm45/... PS4, Line 234: #define MCHBAR8(x) *((volatile u8 *)(DEFAULT_MCHBAR + (x)))
Macros with complex values should be enclosed in parentheses
As mentioned on IRC, there probably is no operator that could take precedence and make sense wrt. the types. But we can as well make Jenkins happy and be sure.
https://review.coreboot.org/c/coreboot/+/45435/4/src/northbridge/intel/gm45/... PS4, Line 267: \ looks like this was manually aligned to match the next line