Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39934 )
Change subject: nb/i945: Improve code formatting ......................................................................
Patch Set 6:
(1 comment)
Please no big all-at-once changes in the future. If the debatable changes would hold back the clear improvements, it's a burden to the reviewer to decide if they should start to debate.
https://review.coreboot.org/c/coreboot/+/39934/6/src/northbridge/intel/i945/... File src/northbridge/intel/i945/raminit.c:
https://review.coreboot.org/c/coreboot/+/39934/6/src/northbridge/intel/i945/... PS6, Line 1943: case 400: : printk(BIOS_DEBUG, "400"); idx += 0; break; : case 533: : printk(BIOS_DEBUG, "533"); idx += 6; break; : case 667: : printk(BIOS_DEBUG, "667"); idx += 12; break; : case 800: : printk(BIOS_DEBUG, "800"); idx += 18; break; : case 1066: : printk(BIOS_DEBUG, "1066"); idx += 24; break; : default: : printk(BIOS_DEBUG, "RSVD %x\n", fsbclk()); return; IMHO, easier for the eye. And I don't like to remove people's personal touch from code...
That you can write them as aligned tables is one of the great things about switch/case statements. I would probably have written it like this:
switch (fsbclk()) { case 400: printk(BIOS_DEBUG, "400"); idx += 0; break; case 533: printk(BIOS_DEBUG, "533"); idx += 6; break; ... case 1066: printk(BIOS_DEBUG, "1066"); idx += 24; break;