Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33407 )
Change subject: nb/intel/pineview: Remove dead code in switch ......................................................................
Patch Set 1: Code-Review+2
(2 comments)
Patch Set 1: Code-Review+2
Looks good, but we should follow-up with Martin's suggestion.
I think that's intended, see comment in the file.
Patch looks good though.
https://review.coreboot.org/c/coreboot/+/33407/1/src/northbridge/intel/pinev... File src/northbridge/intel/pineview/raminit.c:
https://review.coreboot.org/c/coreboot/+/33407/1/src/northbridge/intel/pinev... PS1, Line 469: break This and the other 'break' on the following case block can be omitted (just like you did on the code you changed. Can be on a separate patch.
https://review.coreboot.org/c/coreboot/+/33407/1/src/northbridge/intel/pinev... PS1, Line 490: if (freq == MEM_CLOCK_800MHz) {
Hmmm, if to avoid the die(), then probably yes
Nope.
The block before this one tries to find a value for CAS latency, at either 800 MHz or 667 MHz. If it is invalid (highcas < lowcas), it reduces the speed to 667 MHz and tries again. It is pointless to lower the speed if it is 667 MHz already, so that hits the die().