Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
nb/intel/ironlake: Add more host bridge PCI IDs
The host bridge PCI device ID can be changed by the firmware. There is no documentation about it, though. There's 'official' IDs, which appear in spec updates and Windows drivers, and 'mysterious' IDs, which Intel doesn't want OSes to know about and thus are not listed.
The current coreboot code seems to be able to change the device ID of the host bridge, but it seems to be missing a warm reset so that the device ID changes. Account for the 'mysterious' device IDs in the northbridge driver, so that booting an OS has a chance to work.
Change-Id: I93c9c47e2b0bf13d80c986c7d66b6cdf0e192b22 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/ironlake/northbridge.c 1 file changed, 25 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45562/1
diff --git a/src/northbridge/intel/ironlake/northbridge.c b/src/northbridge/intel/ironlake/northbridge.c index cf014fe..e0e926e 100644 --- a/src/northbridge/intel/ironlake/northbridge.c +++ b/src/northbridge/intel/ironlake/northbridge.c @@ -202,10 +202,34 @@ .ops_pci = &pci_dev_ops_pci, };
+/* + * The host bridge PCI device ID can be changed by the firmware. There + * is no documentation about it, though. There's 'official' IDs, which + * appear in spec updates and Windows drivers, and 'mysterious' IDs, + * which Intel doesn't want OSes to know about and thus are not listed. + * + * The current coreboot code seems to be able to change the device ID + * of the host bridge, but it seems to be missing a warm reset so that + * the device ID changes. Account for the 'mysterious' device IDs in + * the northbridge driver, so that booting an OS has a chance to work. + */ +static const unsigned short pci_device_ids[] = { + /* 'Official' DIDs */ + 0x0040, /* Clarkdale */ + 0x0044, /* Arrandale */ + 0x0048, /* Unknown, but it appears in OS drivers and raminit */ + + /* Mysterious DIDs, taken from Linux' intel-agp driver */ + 0x0062, /* Clarkdale A-? */ + 0x0069, /* Clarkdale K-0 */ + 0x006a, /* Arrandale K-0 */ + 0 +}; + static const struct pci_driver mc_driver_ard __pci_driver = { .ops = &mc_ops, .vendor = PCI_VENDOR_ID_INTEL, - .device = 0x0044, /* Arrandale DRAM controller */ + .device = pci_device_ids, };
static struct device_operations cpu_bus_ops = {
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45562
to look at the new patch set (#2).
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
nb/intel/ironlake: Add more host bridge PCI IDs
The host bridge PCI device ID can be changed by the firmware. There is no documentation about it, though. There's 'official' IDs, which appear in spec updates and Windows drivers, and 'mysterious' IDs, which Intel doesn't want OSes to know about and thus are not listed.
The current coreboot code seems to be able to change the device ID of the host bridge, but it seems to be missing a warm reset so that the device ID changes. Account for the 'mysterious' device IDs in the northbridge driver, so that booting an OS has a chance to work.
Change-Id: I93c9c47e2b0bf13d80c986c7d66b6cdf0e192b22 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/ironlake/northbridge.c 1 file changed, 27 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45562/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45562/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45562/2//COMMIT_MSG@18 PS2, Line 18: Maybe mention that only devices with broken ME firmware are affected? At least that is how I understood it.
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... PS2, Line 218: 0x0040, /* Clarkdale */ Why add Clarkdale? it's not supported by coreboot, or is it?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... PS2, Line 218: 0x0040, /* Clarkdale */
Why add Clarkdale? it's not supported by coreboot, or is it?
Why not? No one has ever tried to use coreboot on it, I know (IIRC these boards weren't very popular)
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... PS2, Line 230: .ops = &mc_ops, : .vendor = PCI_VENDOR_ID_INTEL, : .devices = pci_device_ids, Use either tabs or spaces for alignment?
Hello build bot (Jenkins), Nico Huber, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45562
to look at the new patch set (#3).
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
nb/intel/ironlake: Add more host bridge PCI IDs
The host bridge PCI device ID can be changed by the firmware. There is no documentation about it, though. There's 'official' IDs, which appear in spec updates and Windows drivers, and 'mysterious' IDs, which Intel doesn't want OSes to know about and thus are not listed.
The current coreboot code seems to be able to change the device ID of the host bridge, but it seems to be missing a warm reset so that the device ID changes. Account for the 'mysterious' device IDs in the northbridge driver, so that booting an OS has a chance to work.
It is possible that the Management Engine handles changing the PCI device ID, which would not happen when using a broken ME firmware.
Change-Id: I93c9c47e2b0bf13d80c986c7d66b6cdf0e192b22 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/ironlake/northbridge.c 1 file changed, 27 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45562/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45562/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45562/2//COMMIT_MSG@18 PS2, Line 18:
Maybe mention that only devices with broken ME firmware are affected? […]
Done, literally. (IMHO, me_cleaner breaks ME firmwares)
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... PS2, Line 230: .ops = &mc_ops, : .vendor = PCI_VENDOR_ID_INTEL, : .devices = pci_device_ids,
Use either tabs or spaces for alignment?
If I use tabs, then it flies off to the right (see where the tabstops end). If I use spaces, then people complain. If I don't align it, people complain...
Hello build bot (Jenkins), Nico Huber, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45562
to look at the new patch set (#4).
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
nb/intel/ironlake: Add more host bridge PCI IDs
The host bridge PCI device ID can be changed by the firmware. There is no documentation about it, though. There's 'official' IDs, which appear in spec updates and Windows drivers, and 'mysterious' IDs, which Intel doesn't want OSes to know about and thus are not listed.
The current coreboot code seems to be able to change the device ID of the host bridge, but it seems to be missing a warm reset so that the device ID changes. Account for the 'mysterious' device IDs in the northbridge driver, so that booting an OS has a chance to work.
For the sake of completeness, add the PCI device IDs for Clarkdale. Although only Arrandale is known to work, both of them are Ironlake.
It is possible that the Management Engine handles changing the PCI device ID, which would not happen when using a broken ME firmware.
Change-Id: I93c9c47e2b0bf13d80c986c7d66b6cdf0e192b22 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/ironlake/northbridge.c 1 file changed, 27 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45562/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... PS2, Line 218: 0x0040, /* Clarkdale */
Why not? No one has ever tried to use coreboot on it, I know (IIRC these boards weren't very popular […]
I added a comment explaining I've added Clarkdale for the sake of completeness. I mean, raminit checks for the official Clarkdale PCI ID at some point, so it might just work. I haven't acquired any Clarkdale hardware yet.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... PS2, Line 230: .ops = &mc_ops, : .vendor = PCI_VENDOR_ID_INTEL, : .devices = pci_device_ids,
If I use tabs, then it flies off to the right (see where the tabstops end). […]
Just write down somewhere that aligning with spaces is ok if tabs would be overkill. Then, nobody could complain anymore (or at least you could give a good reason to ignore them ^^).
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 4: Code-Review+1
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 4:
(1 comment)
What's the public names of Clarkdale? Or which spec-update do I have to read to review this?
https://review.coreboot.org/c/coreboot/+/45562/4/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/4/src/northbridge/intel/ironl... PS4, Line 252: CHIP_NAME("Intel i7 (Arrandale) integrated Northbridge") Needs an update, I guess.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 4:
(2 comments)
Patch Set 4:
(1 comment)
What's the public names of Clarkdale? Or which spec-update do I have to read to review this?
[0x40] Refer to the Clarkdale spec update: https://web.archive.org/web/20200418060904if_/https://www.intel.com/content/...
[0x44] Already present in the code, but refer to the Arrandale spec update: https://cdrdv2.intel.com/v1/dl/getContent/600249
[0x48] Download Intel chipset drivers for Windows, e.g. these: http://dl.drp.su/driverpacks/repack/Chipset/Intel/WinAll/Chipset/9.3.2.1020_... Open `IntelCP2.inf` and look for `0048`. Also see line 1523 of: https://raw.githubusercontent.com/coreboot/coreboot/f2c32515ee9a4f4db46ee512... Also see: https://pci-ids.ucw.cz/read/PC/8086/0048 Also see: https://lists.debian.org/debian-boot/2010/10/msg00567.html The last link indicates this PCI DID can be found on at least Clarkdale stepping C-2. However, it is not known whether the
[0x62] Refer to: https://github.com/torvalds/linux/blob/master/drivers/char/agp/intel-agp.h#L... Dig through commit history to find: https://github.com/torvalds/linux/commit/07fb6111e7af5fac6b6076e2658d0e32b67... Looks like this is a mobile variant, but I've tagged it as Clarkdale. I will fix. Stepping unknown.
[0x69] Refer to first link in [0x62], view blame for the "0x69" DID, which links to this commit: https://github.com/torvalds/linux/commit/67384fe3fd450536342330f684ea1f7dcae... Commit links to this bug report: https://bugs.freedesktop.org/show_bug.cgi?id=50381 View attachment 62133: https://bugs.freedesktop.org/attachment.cgi?id=62133 Grep for `CPU0`, second match is: [ 0.044673] CPU0: Intel(R) Core(TM) i3 CPU 540 @ 3.07GHz stepping 05 Refer to the Clarkdale spec update from [0x40] to identify this as Clarkdale stepping K-0.
[0x6a] This is the PCI DID which came up twice, and which this patch attempts to take care of. Refer to first link in [0x62] for another source of this ID. Refer to one of the CBMEM logs where this PCI DID showed up in: http://kurisu.sakamoto.pl/~milesedgeworth/cbmem.log Look for the part where microcode is updated, which contains the CPU model name and CPUID: microcode: sig=0x20655 pf=0x10 revision=0x7 CPU: Intel(R) Core(TM) i7 CPU L 640 @ 2.13GHz. Refer to the Arrandale spec update from [0x44] to identify this as Arrandale stepping K-0. Also see: https://github.com/torvalds/linux/commit/9eecabcb9a924f1e11ba670365fd4babe42...
https://review.coreboot.org/c/coreboot/+/45562/4/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/4/src/northbridge/intel/ironl... PS4, Line 223: Clarkdale Arrandale.
https://review.coreboot.org/c/coreboot/+/45562/4/src/northbridge/intel/ironl... PS4, Line 252: CHIP_NAME("Intel i7 (Arrandale) integrated Northbridge")
Needs an update, I guess.
Indeed, not everyone has an i7.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/2/src/northbridge/intel/ironl... PS2, Line 230: .ops = &mc_ops, : .vendor = PCI_VENDOR_ID_INTEL, : .devices = pci_device_ids,
Just write down somewhere that aligning with spaces is ok if tabs would […]
Alignment with only spaces is fine. Mixing is not good in my opinion.
Hello Marc Jones, build bot (Jenkins), Nico Huber, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45562
to look at the new patch set (#5).
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
nb/intel/ironlake: Add more host bridge PCI IDs
The host bridge PCI device ID can be changed by the firmware. There is no documentation about it, though. There's 'official' IDs, which appear in spec updates and Windows drivers, and 'mysterious' IDs, which Intel doesn't want OSes to know about and thus are not listed.
The current coreboot code seems to be able to change the device ID of the host bridge, but it seems to be missing a warm reset so that the device ID changes. Account for the 'mysterious' device IDs in the northbridge driver, so that booting an OS has a chance to work.
For the sake of completeness, add the PCI device IDs for Clarkdale. Although only Arrandale is known to work, both of them are Ironlake.
It is possible that the Management Engine handles changing the PCI device ID, which would not happen when using a broken ME firmware.
Change-Id: I93c9c47e2b0bf13d80c986c7d66b6cdf0e192b22 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/ironlake/northbridge.c 1 file changed, 27 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45562/5
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45562/4/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/4/src/northbridge/intel/ironl... PS4, Line 223: Clarkdale
Arrandale.
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 6: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45562/6/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/6/src/northbridge/intel/ironl... PS6, Line 220: 0x0048, /* Unknown, but it appears in OS drivers and raminit */ At least not seen much in laptops (you can tell when /usr/share/hwdata/pci.ids doesn't know any SSIDs)
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 6: Code-Review+2
Hello Marc Jones, build bot (Jenkins), Nico Huber, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45562
to look at the new patch set (#7).
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
nb/intel/ironlake: Add more host bridge PCI IDs
The host bridge PCI device ID can be changed by the firmware. There is no documentation about it, though. There's 'official' IDs, which appear in spec updates and Windows drivers, and 'mysterious' IDs, which Intel doesn't want OSes to know about and thus are not listed.
The current coreboot code seems to be able to change the device ID of the host bridge, but it seems to be missing a warm reset so that the device ID changes. Account for the 'mysterious' device IDs in the northbridge driver, so that booting an OS has a chance to work.
For the sake of completeness, add the PCI device IDs for Clarkdale. Although only Arrandale is known to work, both of them are Ironlake.
It is possible that the Management Engine handles changing the PCI device ID, which would not happen when using a broken ME firmware.
Change-Id: I93c9c47e2b0bf13d80c986c7d66b6cdf0e192b22 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/ironlake/northbridge.c 1 file changed, 28 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/45562/7
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45562/4/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/northbridge.c:
https://review.coreboot.org/c/coreboot/+/45562/4/src/northbridge/intel/ironl... PS4, Line 252: CHIP_NAME("Intel i7 (Arrandale) integrated Northbridge")
Indeed, not everyone has an i7.
CB:46664
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
Patch Set 8: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45562 )
Change subject: nb/intel/ironlake: Add more host bridge PCI IDs ......................................................................
nb/intel/ironlake: Add more host bridge PCI IDs
The host bridge PCI device ID can be changed by the firmware. There is no documentation about it, though. There's 'official' IDs, which appear in spec updates and Windows drivers, and 'mysterious' IDs, which Intel doesn't want OSes to know about and thus are not listed.
The current coreboot code seems to be able to change the device ID of the host bridge, but it seems to be missing a warm reset so that the device ID changes. Account for the 'mysterious' device IDs in the northbridge driver, so that booting an OS has a chance to work.
For the sake of completeness, add the PCI device IDs for Clarkdale. Although only Arrandale is known to work, both of them are Ironlake.
It is possible that the Management Engine handles changing the PCI device ID, which would not happen when using a broken ME firmware.
Change-Id: I93c9c47e2b0bf13d80c986c7d66b6cdf0e192b22 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45562 Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/ironlake/northbridge.c 1 file changed, 28 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/northbridge/intel/ironlake/northbridge.c b/src/northbridge/intel/ironlake/northbridge.c index 0047c2f..68dcf7d 100644 --- a/src/northbridge/intel/ironlake/northbridge.c +++ b/src/northbridge/intel/ironlake/northbridge.c @@ -202,10 +202,34 @@ .ops_pci = &pci_dev_ops_pci, };
-static const struct pci_driver mc_driver_ard __pci_driver = { - .ops = &mc_ops, - .vendor = PCI_VENDOR_ID_INTEL, - .device = 0x0044, /* Arrandale DRAM controller */ +/* + * The host bridge PCI device ID can be changed by the firmware. There + * is no documentation about it, though. There's 'official' IDs, which + * appear in spec updates and Windows drivers, and 'mysterious' IDs, + * which Intel doesn't want OSes to know about and thus are not listed. + * + * The current coreboot code seems to be able to change the device ID + * of the host bridge, but it seems to be missing a warm reset so that + * the device ID changes. Account for the 'mysterious' device IDs in + * the northbridge driver, so that booting an OS has a chance to work. + */ +static const unsigned short pci_device_ids[] = { + /* 'Official' DIDs */ + 0x0040, /* Clarkdale */ + 0x0044, /* Arrandale */ + 0x0048, /* Unknown, but it appears in OS drivers and raminit */ + + /* Mysterious DIDs, taken from Linux' intel-agp driver */ + 0x0062, /* Arrandale A-? */ + 0x0069, /* Clarkdale K-0 */ + 0x006a, /* Arrandale K-0 */ + 0 +}; + +static const struct pci_driver mc_driver_ilk __pci_driver = { + .ops = &mc_ops, + .vendor = PCI_VENDOR_ID_INTEL, + .devices = pci_device_ids, };
static struct device_operations cpu_bus_ops = {