Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40429 )
Change subject: src/northbridge/intel: Drop duplicated branches ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40429/3/src/northbridge/intel/pinev... File src/northbridge/intel/pineview/raminit.c:
https://review.coreboot.org/c/coreboot/+/40429/3/src/northbridge/intel/pinev... PS3, Line 158: if (s->dimms[chan>>1].sides == 0) { : // NC : if (s->dimms[(chan>>1) + 1].sides == 0) { : // NC/NC : s->dimm_config[chan] = 0; : } else if (s->dimms[(chan>>1) + 1].sides == 1) { : // NC/SS : //if (s->dimms[(chan>>1) + 1].width == 0) { : // NC/8SS : // s->dimm_config[chan] = 1; : //} else { : // NC/16SS : s->dimm_config[chan] = 1; : //} : } else { : // NC/DS : if (s->dimms[(chan>>1) + 1].width == 0) { : // NC/8DS : s->dimm_config[chan] = 5; : } else { : // NC/16DS : s->dimm_config[chan] = 2; : } : } : } else if (s->dimms[chan>>1].sides == 1) { : // SS : if (s->dimms[(chan>>1) + 1].sides == 0) { : // SS/NC : //if (s->dimms[chan>>1].width == 0) { : // 8SS/NC : // s->dimm_config[chan] = 1; : //} else { : // 16SS/NC : s->dimm_config[chan] = 1; : //} : } else if (s->dimms[(chan>>1) + 1].sides == 1) { : // SS/SS : if (s->dimms[chan>>1].width == 0) { : if (s->dimms[(chan>>1) + 1].width == 0) { : // 8SS/8SS : s->dimm_config[chan] = 3; : } else { : // 8SS/16SS : die("Mixed Not supported\n"); : } : } else { : if (s->dimms[(chan>>1) + 1].width == 0) { : // 16SS/8SS : die("Mixed Not supported\n"); : } else { : // 16SS/16SS : s->dimm_config[chan] = 3; : } : } : } else { : // SS/DS : //if (s->dimms[chan>>1].width == 0) { : //if (s->dimms[(chan>>1) + 1].width == 0) { : // 8SS/8DS : // die("Mixed Not supported\n"); : //} else { : // die("Mixed Not supported\n"); : //} : //} else { : //if (s->dimms[(chan>>1) + 1].width == 0) { : // 16SS/8DS : // die("Mixed Not supported\n"); : //} else { : die("Mixed Not supported\n"); : //} : //} : } : } else { : // DS : if (s->dimms[(chan>>1) + 1].sides == 0) { : // DS/NC : if (s->dimms[chan>>1].width == 0) { : // 8DS/NC : s->dimm_config[chan] = 5; : } else { : s->dimm_config[chan] = 4; : } : } else if (s->dimms[(chan>>1) + 1].sides == 1) { : // DS/SS : if (s->dimms[chan>>1].width == 0) { : //if (s->dimms[(chan>>1) + 1].width == 0) { : // 8DS/8SS : // die("Mixed Not supported\n"); : //} else { : // 8DS/16SS : die("Mixed Not supported\n"); : //} : } else { : if (s->dimms[(chan>>1) + 1].width == 0) { : die("Mixed Not supported\n"); : } else { : // 16DS/16DS : s->dimm_config[chan] = 4; : } : } : } else { : // DS/DS : if (s->dimms[chan>>1].width == 0 && s->dimms[(chan>>1)+1].width == 0) { : // 8DS/8DS : s->dimm_config[chan] = 6; : } : } : } This whole part is quite hard to read in general and should probably be refactored...