Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38941 )
Change subject: nb/intel/nehalem: Rename to ironlake ......................................................................
Patch Set 5:
Hi Martin,
Thanks for your review!
Patch Set 5:
Why? What does this name change improve?
Simply put, the code is not for Nehalem chips at all. Instead, it supports Arrandale processors, which are Westmere-based. Let me elaborate on the matter.
NEHALEM: Nehalem processors are manufactured in a 45 nm process, and lacks integrated graphics. The codename for mobile Nehalem processors is Clarksfield, and it consists of a single 45 nm die. Here is a picture: http://www.cpu-world.com/CPUs/Core_i7/Intel-Core i7 Mobile I7-740QM BY80607005259AA (BX80607I7740QM).html
The datasheets for Clarksfield are: Volume 1 (320765): https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/core... Volume 2 (320766): https://mafiadoc.com/download/intel-core-i7-900-mobile-processor-extreme-edi...
Note that these processors lack the usual MCHBAR register space to perform memory initialization. Instead, the registers are located on some PCI devices.
WESTMERE: Westmere is the die-shrunk version of Nehalem, and is manufactured in a 32 nm process. In addition, Intel HD graphics are included in the dual-core variants: Clarkdale on desktops (LGA1156) and Arrandale on laptops. Here is a picture of an Arrandale processor: http://www.cpu-world.com/CPUs/Core_i3/Intel-Core i3 Mobile I3-390M.html
As it can be seen, Arrandale has two dies. According to Wikipedia, their names are: - Hillel: 32 nm dual-core Westmere CPU die - Ironlake: 45 nm GMCH (Graphics and Memory Controller Hub)
The datasheets for Arrandale are: Volume 1 (322812): https://cdrdv2.intel.com/v1/dl/getContent/600229 Volume 2 (322813): https://cdrdv2.intel.com/v1/dl/getContent/600230
Like most other platforms, Arrandale has the memory controller registers in MCHBAR. I did some research, and the registers are very similar to the ones on Eaglelake (x4x), Pineview, Bearlake and Broadwater.
Given that the SA (System Agent, what used to be a northbridge) changes radically between Clarksfield and Arrandale, the difference between both is important. The raminit code makes heavy use of the MCHBAR to initialize the RAM, which is only present on Arrandale. This means that the SA can't be the one in Nehalem-based Clarksfield processors, as they lack a MCHBAR. Therefore, the SA must be the one on Arrandale processors, whose name is Ironlake.
But why Ironlake? Well, both desktop Clarkdale and mobile Arrandale use the same SA, whose name is Ironlake. This avoids a rename in the future, should anybody want to add support for Clarkdale.
I mean, I see that there's been a lot of work put into changing this, and I appreciate that, but it touches lots of files, creates a lot of merge conflicts, makes things harder to merge from/to older codebases, and for what?
I know it's a bit noisy. I've split the work in three different changes to ease review. This is by far the largest change, but it does not affect the resulting binary at all, so it is just a cosmetic change :)
Additionally, the renaming can be done with two invocations of `sed`, so it is not that much of an issue.
I'd say that we shouldn't create that much churn unless there's a compelling reason, and I'm not sure there is.
I have explained the rationale for this change above. I believe it is substantial enough to deserve this change.
In any case, should I explain the reasons for this change in the commit message?