coreboot-gerrit May 2017

coreboot-gerrit@coreboot.org

1 participants
3888 discussions

Change in coreboot[master]: nb/intel/x4x/raminit: Add DDR3 enhanced mode and power settings
by Arthur Heymans (Code Review) 25 May '17

25 May '17

Arthur Heymans has uploaded a new change for review. ( https://review.coreboot.org/19917 ) Change subject: nb/intel/x4x/raminit: Add DDR3 enhanced mode and power settings ...................................................................... nb/intel/x4x/raminit: Add DDR3 enhanced mode and power settings Change-Id: I9d34154d3ac1dd1e5400d692d4dcce70d95662c8 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/northbridge/intel/x4x/raminit_ddr23.c M src/northbridge/intel/x4x/raminit_tables.c M src/northbridge/intel/x4x/x4x.h 3 files changed, 143 insertions(+), 29 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/19917/1 diff --git a/src/northbridge/intel/x4x/raminit_ddr23.c b/src/northbridge/intel/x4x/raminit_ddr23.c index 7bfdc6a..fab0df6 100644 --- a/src/northbridge/intel/x4x/raminit_ddr23.c +++ b/src/northbridge/intel/x4x/raminit_ddr23.c @@ -2027,7 +2027,10 @@ MCHBAR32(0xfc4) = 0xfe22244; MCHBAR8(0x12f) = 0x5c; MCHBAR8(0xfb0) = (MCHBAR8(0xfb0) & ~1) | 1; - MCHBAR8(0x12f) = MCHBAR8(0x12f) | 0x2; + if (s->selected_timings.mem_clk <= MEM_CLOCK_800MHz) + MCHBAR8(0x12f) = MCHBAR8(0x12f) | 0x2; + else + MCHBAR8(0x12f) = MCHBAR8(0x12f) & 0x2; MCHBAR8(0x6c0) = (MCHBAR8(0x6c0) & ~0xf0) | 0xa0; MCHBAR32(0xfa8) = 0x30d400; @@ -2057,38 +2060,69 @@ pci_write_config8(PCI_DEV(0, 0, 0), 0xf0, reg8 & ~1); } -static void power_ddr2(struct sysinfo *s) +static void power_settings(struct sysinfo *s) { u32 reg1, reg2, reg3, reg4, clkgate, x592; u8 lane, ch; u8 twl = 0; u16 x264, x23c; - twl = s->selected_timings.CAS - 1; - x264 = 0x78; - switch (s->selected_timings.mem_clk) { - default: - case MEM_CLOCK_667MHz: - reg1 = 0x99; - reg2 = 0x1048a9; - clkgate = 0x230000; - x23c = 0x7a89; - break; - case MEM_CLOCK_800MHz: - if (s->selected_timings.CAS == 5) { - reg1 = 0x19a; - reg2 = 0x1048aa; - } else { - reg1 = 0x9a; - reg2 = 0x2158aa; + if (s->spd_type == DDR2) { + twl = s->selected_timings.CAS - 1; + x264 = 0x78; + + switch (s->selected_timings.mem_clk) { + default: + case MEM_CLOCK_667MHz: + reg1 = 0x99; + reg2 = 0x1048a9; + clkgate = 0x230000; + x23c = 0x7a89; + break; + case MEM_CLOCK_800MHz: + if (s->selected_timings.CAS == 5) { + reg1 = 0x19a; + reg2 = 0x1048aa; + } else { + reg1 = 0x9a; + reg2 = 0x2158aa; x264 = 0x89; + } + clkgate = 0x280000; + x23c = 0x7b89; + break; } - clkgate = 0x280000; - x23c = 0x7b89; - break; + reg3 = 0x232; + reg4 = 0x2864; + } else { /* DDR3 */ + twl = s->selected_timings.mem_clk - MEM_CLOCK_800MHz + 5; + reg1 = ddr3_c2_tab[s->nmode] + [s->selected_timings.mem_clk - MEM_CLOCK_800MHz] + [s->selected_timings.CAS - 5] + [0]; + reg2 = ddr3_c2_tab[s->nmode] + [s->selected_timings.mem_clk - MEM_CLOCK_800MHz] + [s->selected_timings.CAS - 5] + [1]; + reg3 = 0x764; + reg4 = 0x78c8; + x264 = ddr3_c2_x264[s->selected_timings.mem_clk - MEM_CLOCK_800MHz] + [s->selected_timings.CAS - 5]; + x23c = ddr3_c2_x23c[s->selected_timings.mem_clk - MEM_CLOCK_800MHz] + [s->selected_timings.CAS - 5]; + switch (s->selected_timings.mem_clk) { + case MEM_CLOCK_800MHz: + default: + clkgate = 0x280000; + break; + case MEM_CLOCK_1066MHz: + clkgate = 0x350000; + break; + case MEM_CLOCK_1333MHz: + clkgate = 0xff0000; + break; + } } - reg3 = 0x232; - reg4 = 0x2864; if (CHANNEL_IS_POPULATED(s->dimms, 0) && CHANNEL_IS_POPULATED(s->dimms, 1)) MCHBAR32(0x14) = 0x0010461f; @@ -2145,10 +2179,6 @@ MCHBAR32(0x110c) = 0x100; MCHBAR32(0x1110) = 0x10810350 & ~0x78; MCHBAR32(0x1114) = 0; - if (s->selected_timings.mem_clk == MEM_CLOCK_667MHz) - twl = 5; - else - twl = 6; x592 = 0xff; if (pci_read_config8(PCI_DEV(0, 0, 0), 0x8) < 3) x592 = ~0x4; @@ -2399,7 +2429,7 @@ printk(BIOS_DEBUG, "Done PRCOMP\n"); // Power settings - power_ddr2(s); + power_settings(s); printk(BIOS_DEBUG, "Done power settings\n"); // ME related diff --git a/src/northbridge/intel/x4x/raminit_tables.c b/src/northbridge/intel/x4x/raminit_tables.c index e3001f2..fabfe18 100644 --- a/src/northbridge/intel/x4x/raminit_tables.c +++ b/src/northbridge/intel/x4x/raminit_tables.c @@ -246,3 +246,84 @@ {0x81, 0x81, 0x81, 0x00}, /* 8D_16S */ {0x81, 0x00, 0x81, 0x00}, /* 16S_16S */ }; + +const u32 ddr3_c2_tab[2][3][6][2] = { /* [n-mode][ddr3 freq][CAS][reg] */ + /* 115h[15:0] 117h[23:0] */ + { /* 1N mode */ + { /* DDR3 400MHz */ + {0x0189, 0x000aaa}, /* CAS = 5 */ + {0x0189, 0x101aaa}, /* CAS = 6 */ + }, + { /* DDR3 533MHz */ + {0x0000, 0x000000}, /* CAS = 5 - Not supported */ + {0x0089, 0x000bbb}, /* CAS = 6 */ + {0x0099, 0x101bbb}, /* CAS = 7 */ + {0x0099, 0x202bbb} /* CAS = 8 */ + },{ /* DDR3 1333 */ + {0x0000, 0x000000}, /* CAS = 5 - Not supported */ + {0x0000, 0x000000}, /* CAS = 6 - Not supported */ + {0x0000, 0x000000}, /* CAS = 7 - Not supported */ + {0x129a, 0x0078dc}, /* CAS = 8 */ + {0x028a, 0x0078dc}, /* CAS = 9 */ + {0x028a, 0x1088dc}, /* CAS = 10 */ + }, + }, + { /* 2N mode */ + { /* DDR3 400MHz */ + {0x0189, 0x000aaa}, /* CAS = 5 */ + {0x0189, 0x101aaa}, /* CAS = 6 */ + {0x0000, 0x000000}, /* CAS = 7 - Not supported */ + {0x0000, 0x000000} /* CAS = 8 - Not suppported */ + }, + { /* DDR3 533 */ + {0x0000, 0x000000}, /* CAS = 5 - Not supported */ + {0x0089, 0x000bbb}, /* CAS = 6 */ + {0x0099, 0x101bbb}, /* CAS = 7 */ + {0x0099, 0x202bbb} /* CAS = 8 */ + },{ /* DDR3 666MHz */ + {0x0000, 0x000000}, /* CAS = 5 - Not supported */ + {0x0000, 0x000000}, /* CAS = 6 - Not supported */ + {0x0000, 0x000000}, /* CAS = 7 - Not supported */ + {0x019a, 0x0078dc}, /* CAS = 8 */ + {0x019a, 0x1088dc}, /* CAS = 9 */ + {0x019a, 0x2098dc}, /* CAS = 10 */ + }, + } +}; + +const u8 ddr3_c2_x264[3][6] = { /* [freq][cas] */ + /* DDR3 400MHz */ + {0x78, /* CAS = 5 */ + 0x89}, /* CAS = 6 */ + /* DDR3 1066 */ + {0x00, /* CAS = 5 - Not supported */ + 0xff, /* CAS = 6 */ + 0x8a, /* CAS = 7 */ + 0x9a}, /* CAS = 8 */ + /* DDR3 1333 */ + {0x00, /* CAS = 5 - Not supported */ + 0x00, /* CAS = 6 - Not supported */ + 0xff, /* CAS = 7 - Not supported */ + 0xff, /* CAS = 8 */ + 0xff, /* CAS = 9 */ + 0xff}, /* CAS = 10 */ +}; + +const u16 ddr3_c2_x23c[3][6]={ /* [freq][cas] */ + /* DDR3 400MHz */ + {0x9bbb, /* CAS = 5 */ + 0x8bbb}, /* CAS = 6 */ + /* DDR3 533MHz */ + {0x0000, /* CAS = 5 - Not supported */ + 0x9baa, /* CAS = 6 */ + 0x8caa, /* CAS = 7 */ + 0x7daa}, /* CAS = 8 */ + + /* DDR3 667MHz */ + {0x0000, /* CAS = 5 - Not supported */ + 0x0000, /* CAS = 6 - Not supported */ + 0x0000, /* CAS = 7 - Not supported */ + 0xaecb, /* CAS = 8 */ + 0x9fcb, /* CAS = 9 */ + 0x8fcb}, /* CAS = 10 */ +}; diff --git a/src/northbridge/intel/x4x/x4x.h b/src/northbridge/intel/x4x/x4x.h index 85879fb..a243e23 100644 --- a/src/northbridge/intel/x4x/x4x.h +++ b/src/northbridge/intel/x4x/x4x.h @@ -379,6 +379,9 @@ extern const struct dll_setting ddr3_dll_setting_1066[2][23]; extern const struct dll_setting ddr3_dll_setting_1333[2][23]; extern const u8 ddr3_emrs1_config[16][4]; +extern const u32 ddr3_c2_tab[2][3][6][2]; +extern const u8 ddr3_c2_x264[3][6]; +extern const u16 ddr3_c2_x23c[3][6]; struct acpi_rsdp; #ifndef __SIMPLE_DEVICE__ -- To view, visit https://review.coreboot.org/19917 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9d34154d3ac1dd1e5400d692d4dcce70d95662c8 Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

Change in coreboot[master]: nb/intel/x4x/raminit: Add DDR3 specific dra/drb settings
by Arthur Heymans (Code Review) 25 May '17

25 May '17

Arthur Heymans has uploaded a new change for review. ( https://review.coreboot.org/19916 ) Change subject: nb/intel/x4x/raminit: Add DDR3 specific dra/drb settings ...................................................................... nb/intel/x4x/raminit: Add DDR3 specific dra/drb settings Change-Id: Icf5bb4a8044bbf2f58c8c9a4774329a95bf3b070 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/northbridge/intel/x4x/raminit_ddr23.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/19916/1 diff --git a/src/northbridge/intel/x4x/raminit_ddr23.c b/src/northbridge/intel/x4x/raminit_ddr23.c index 4c15f62..7bfdc6a 100644 --- a/src/northbridge/intel/x4x/raminit_ddr23.c +++ b/src/northbridge/intel/x4x/raminit_ddr23.c @@ -1878,6 +1878,11 @@ MCHBAR8(0x262) = (MCHBAR8(0x262) & ~0xf0) | ((rankpop0 << 4) & 0xf0); MCHBAR8(0x662) = (MCHBAR8(0x662) & ~0xf0) | ((rankpop1 << 4) & 0xf0); + if (s->spd_type == DDR3) { + FOR_EACH_POPULATED_CHANNEL(s->dimms, ch) + MCHBAR32(0x269 + 0x400 * ch) = MCHBAR32(0x269 + 0x400 * ch) | (1 << 26); + } + if (ONLY_DIMMA_IS_POPULATED(s->dimms, 0) || ONLY_DIMMB_IS_POPULATED(s->dimms, 0)) MCHBAR8(0x260) = MCHBAR8(0x260) | 1; -- To view, visit https://review.coreboot.org/19916 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icf5bb4a8044bbf2f58c8c9a4774329a95bf3b070 Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

Change in coreboot[master]: nb/intel/x4x/raminit: Implement read and write DQ DQS training
by Arthur Heymans (Code Review) 25 May '17

25 May '17

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/19879 to look at the new patch set (#4). Change subject: nb/intel/x4x/raminit: Implement read and write DQ DQS training ...................................................................... nb/intel/x4x/raminit: Implement read and write DQ DQS training Positions the DQS in the eye of the DQ signal on both read and writes by taking a centered DLL value between two failing edges. This is not DDR3 specific and write training should also run on DDR2 at 400MHz and read training can run on all configurations. Change-Id: I806840445b5e768d079910fb9870a2cee7b9f1ca Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/northbridge/intel/x4x/dq_dqs_dll.c M src/northbridge/intel/x4x/raminit_ddr23.c M src/northbridge/intel/x4x/x4x.h 3 files changed, 484 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/19879/4 -- To view, visit https://review.coreboot.org/19879 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I806840445b5e768d079910fb9870a2cee7b9f1ca Gerrit-PatchSet: 4 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in coreboot[master]: nb/intel/x4x/raminit: Add write leveling
by Arthur Heymans (Code Review) 25 May '17

25 May '17

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/19878 to look at the new patch set (#4). Change subject: nb/intel/x4x/raminit: Add write leveling ...................................................................... nb/intel/x4x/raminit: Add write leveling DDR3 adapter a fly-by topology which allows for better signal integrity but at the same time requires additional calibration. This is done by settings the targeted rank in write leveling mode while disabling output buffer on the other ranks. DQS DLL settings are sampled until an edge is found over the DQ probe. The results are stored in the sysinfo struct for later S3 support. Change-Id: Ibfbaa235bc4eb08e9345321b851e880390a624e8 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/northbridge/intel/x4x/Makefile.inc A src/northbridge/intel/x4x/dq_dqs_dll.c M src/northbridge/intel/x4x/raminit_ddr23.c M src/northbridge/intel/x4x/raminit_tables.c M src/northbridge/intel/x4x/x4x.h 5 files changed, 472 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/19878/4 -- To view, visit https://review.coreboot.org/19878 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibfbaa235bc4eb08e9345321b851e880390a624e8 Gerrit-PatchSet: 4 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in coreboot[master]: nb/intel/x4x/raminit: Make programming launch ddr3 specific
by Arthur Heymans (Code Review) 25 May '17

25 May '17

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/19872 to look at the new patch set (#3). Change subject: nb/intel/x4x/raminit: Make programming launch ddr3 specific ...................................................................... nb/intel/x4x/raminit: Make programming launch ddr3 specific Change-Id: Ia2ca4a200a1c813b2133eb1004fbe248fa3de9ce Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/northbridge/intel/x4x/raminit_ddr23.c M src/northbridge/intel/x4x/x4x.h 2 files changed, 76 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/19872/3 -- To view, visit https://review.coreboot.org/19872 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia2ca4a200a1c813b2133eb1004fbe248fa3de9ce Gerrit-PatchSet: 3 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in coreboot[master]: nb/intel/x4x: Rename a things that are not ddr2 specific
by Arthur Heymans (Code Review) 25 May '17

25 May '17

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/19870 to look at the new patch set (#3). Change subject: nb/intel/x4x: Rename a things that are not ddr2 specific ...................................................................... nb/intel/x4x: Rename a things that are not ddr2 specific Change-Id: Ib3d10014f530905155e56fc52706edb4ab9f5630 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/northbridge/intel/x4x/Makefile.inc M src/northbridge/intel/x4x/raminit.c R src/northbridge/intel/x4x/raminit_ddr23.c M src/northbridge/intel/x4x/x4x.h 4 files changed, 20 insertions(+), 31 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/19870/3 -- To view, visit https://review.coreboot.org/19870 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib3d10014f530905155e56fc52706edb4ab9f5630 Gerrit-PatchSet: 3 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in coreboot[master]: pciexp_device: Prevent race condition with retrain link
by Youness Alaoui (Code Review) 25 May '17

25 May '17

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/19556 ) Change subject: pciexp_device: Prevent race condition with retrain link ...................................................................... Patch Set 1: (3 comments) https://review.coreboot.org/#/c/19556/1//COMMIT_MSG Commit Message: PS1, Line 9: The PCIe specification[1] describes a race condition that : can occur when using the Retrain Link bit in the Link : Control Register. : The race condition is avoided by checking the retrain link : bit in the link status register and waiting until it is : set to 0, before initiating a new link retraining. > Please add blank lines between paragraphs. Done https://review.coreboot.org/#/c/19556/1/src/device/pciexp_device.c File src/device/pciexp_device.c: Line 56: /* Implementation note (page 633) in PCIe Specification 3.0 suggests > Nit: Since try needs to be initialised at the same value twice would for lo Done PS1, Line 56: /* Implementation note (page 633) in PCIe Specification 3.0 suggests : * polling the Link Training bit in the Link Status register until the : * value returned is 0 before setting the Retrain Link bit to 1. : * This is meant to avoid a race condition when using the : * Retrain Link mechanism. : */ > Please use the style below. Done -- To view, visit https://review.coreboot.org/19556 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9d5840fb9a6e63838b5a4084d3bbe483f1d870ed Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: CBMEM: Clarify CBMEM_TOP_BACKUP function usage
by build bot (Jenkins) (Code Review) 25 May '17

25 May '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19377 ) Change subject: CBMEM: Clarify CBMEM_TOP_BACKUP function usage ...................................................................... Patch Set 7: Build Successful https://qa.coreboot.org/job/coreboot-checkpatch/10128/ : SUCCESS https://qa.coreboot.org/job/coreboot-gerrit/54405/ : SUCCESS -- To view, visit https://review.coreboot.org/19377 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61d20f94840ad61e9fd55976e5aa8c27040b8fb7 Gerrit-PatchSet: 7 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: No

1 0

Change in coreboot[master]: soc/amd/stoneyridge: Add CPU files
by Marc Jones (Code Review) 25 May '17

25 May '17

Marc Jones has posted comments on this change. ( https://review.coreboot.org/19723 ) Change subject: soc/amd/stoneyridge: Add CPU files ...................................................................... Patch Set 3: (2 comments) https://review.coreboot.org/#/c/19723/3/src/northbridge/amd/pi/Kconfig File src/northbridge/amd/pi/Kconfig: Line 19: default y if SOC_AMD_PI > Agreed - If we're moving things to SOC, why would we still need pieces in s yes, the northbridge is the next patch and this was needed to check the build and code worked moving the CPU code. https://review.coreboot.org/#/c/19723/3/src/soc/amd/common/Kconfig File src/soc/amd/common/Kconfig: Line 13 > But that doesn't really fix the problem entirely. Someone still has to incl So is there a solution already figured out or we should be doing something completely different here? -- To view, visit https://review.coreboot.org/19723 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b6b1991372c2c6a02709777a73615a86e78ac26 Gerrit-PatchSet: 3 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: soc/amd/stoneyridge: Add northbridge support
by Marc Jones (Code Review) 25 May '17

25 May '17

Marc Jones has posted comments on this change. ( https://review.coreboot.org/19724 ) Change subject: soc/amd/stoneyridge: Add northbridge support ...................................................................... Patch Set 3: (5 comments) Tried to answer some questions. lots of cleanup in follow-on patches. Maybe we will squash it all down. https://review.coreboot.org/#/c/19724/3/src/soc/amd/common/amd_late_init.c File src/soc/amd/common/amd_late_init.c: PS3, Line 23: #include <agesawrapper.h> : #include <agesawrapper_call.h> > These includes should use "" if you are wanting to pick up the files in the common/ is in the include path, so I left it <>. I don't understand what you mean by why they are directly under common. Do you think common/agesa/ or common/include/ or somewhere else? https://review.coreboot.org/#/c/19724/3/src/soc/amd/common/def_callouts.c File src/soc/amd/common/def_callouts.c: PS3, Line 20: #include <AGESA.h> : #include <amdlib.h> : #include <Ids.h> : #include <agesawrapper.h> : #include <BiosCallOuts.h> > All these headers should be namespaced in some matter. Why are these bare a They are in src/vendorcode/amd/pi/00670F00 and added by the vendorcode makefile. Line 84: LibAmdIoWrite (AccessWidth8, 0xCf9, &Value, StdHeader); > Is this really necessary? We can't use io_write8() ? Yes, we need to clean out the AMD functions and use coreboot functions. This is the reason some functions live in fixme.c and need to get fixed and moved. https://review.coreboot.org/#/c/19724/3/src/soc/amd/common/dimmSpd.h File src/soc/amd/common/dimmSpd.h: Line 20: AmdMemoryReadSPD (IN UINT32 Func, IN UINT32 Data, IN OUT AGESA_READ_SPD_PARAMS *SpdData); > IN and OUT ? agesa defines. https://review.coreboot.org/#/c/19724/3/src/soc/amd/stoneyridge/northbridge… File src/soc/amd/stoneyridge/northbridge.c: Line 731: > I have no words for this file. There is a lot of legacy here and still requires a lot more cleanup. The multinode HT init is overly complicated and didn't/doesn't fit well in coreboot. The PI removes the need for a lot of that logic in coreboot, but I still think there are a lot of problems putting the CPUs in PCI space and IO and MEMORY bus routing when there is more than a subtractive PCI bus on CPU bus. -- To view, visit https://review.coreboot.org/19724 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie86b4d744900f23502068517ece5bcea6c128993 Gerrit-PatchSet: 3 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-HasComments: Yes

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2017