Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45502 )
Change subject: nb/intel/sandybridge: Clean up `dram_freq` function ......................................................................
nb/intel/sandybridge: Clean up `dram_freq` function
The thing that this function initializes is the MPLL (Memory PLL). So, call it by its name. Also add a missing newline in a printk, and update a comment on the callsite of this function.
Tested on Asus P8Z77-V LX2, still boots.
Change-Id: I86ab643bc87253554346dfed3630eb9ddbd44eb3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/raminit_native.c 1 file changed, 11 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/45502/1
diff --git a/src/northbridge/intel/sandybridge/raminit_native.c b/src/northbridge/intel/sandybridge/raminit_native.c index 64a381d..6b74405 100644 --- a/src/northbridge/intel/sandybridge/raminit_native.c +++ b/src/northbridge/intel/sandybridge/raminit_native.c @@ -459,7 +459,7 @@ if (ctrl->tCK > TCK_400MHZ) { printk(BIOS_ERR, "DRAM frequency is under lowest supported frequency (400 MHz). " - "Increasing to 400 MHz as last resort"); + "Increasing to 400 MHz as last resort.\n"); ctrl->tCK = TCK_400MHZ; }
@@ -467,11 +467,11 @@ u8 val2; u32 reg1 = 0;
- /* Step 1 - Set target PCU frequency */ + /* Step 1 - Determine target MPLL frequency */ find_cas_tck(ctrl);
/* - * The PLL will never lock if the required frequency is already set. + * The MPLL will never lock if the requested frequency is already set. * Exit early to prevent a system hang. */ reg1 = MCHBAR32(MC_BIOS_DATA); @@ -479,16 +479,16 @@ if (val2) return;
- /* Step 2 - Select frequency in the MCU */ + /* Step 2 - Request MPLL frequency */ reg1 = ctrl->FRQ; if (ctrl->base_freq == 100) - reg1 |= 0x100; /* Enable 100Mhz REF clock */ + reg1 |= 0x100; /* Use 100MHz reference clock */
- reg1 |= 0x80000000; /* set running bit */ + reg1 |= (1 << 31); /* Set running bit */ MCHBAR32(MC_BIOS_REQ) = reg1; int i = 0; - printk(BIOS_DEBUG, "PLL busy... "); - while (reg1 & 0x80000000) { + printk(BIOS_DEBUG, "MPLL busy... "); + while (reg1 & (1 << 31)) { udelay(10); i++; reg1 = MCHBAR32(MC_BIOS_REQ); @@ -499,11 +499,11 @@ reg1 = MCHBAR32(MC_BIOS_DATA); val2 = (u8) reg1; if (val2 >= ctrl->FRQ) { - printk(BIOS_DEBUG, "MCU frequency is set at : %d MHz\n", + printk(BIOS_DEBUG, "MPLL frequency is set at : %d MHz\n", (1000 << 8) / ctrl->tCK); return; } - printk(BIOS_DEBUG, "PLL didn't lock. Retrying at lower frequency\n"); + printk(BIOS_DEBUG, "MPLL didn't lock. Retrying at lower frequency\n"); ctrl->tCK++; } } @@ -571,7 +571,7 @@ dram_dimm_mapping(ctrl); }
- /* Set MC frequency */ + /* Set MPLL frequency */ dram_freq(ctrl);
if (!fast_boot) {
Hello build bot (Jenkins), Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45502
to look at the new patch set (#2).
Change subject: nb/intel/sandybridge: Clean up `dram_freq` function ......................................................................
nb/intel/sandybridge: Clean up `dram_freq` function
The thing that this function initializes is the MPLL (Memory PLL). So, call it by its name. Also add a missing newline in a printk, and update a comment on the callsite of this function.
Tested on Asus P8Z77-V LX2, still boots.
Change-Id: I86ab643bc87253554346dfed3630eb9ddbd44eb3 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/raminit_native.c 1 file changed, 10 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/45502/2
Attention is currently required from: Angel Pons. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45502 )
Change subject: nb/intel/sandybridge: Clean up `dram_freq` function ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45502 )
Change subject: nb/intel/sandybridge: Clean up `dram_freq` function ......................................................................
nb/intel/sandybridge: Clean up `dram_freq` function
The thing that this function initializes is the MPLL (Memory PLL). So, call it by its name. Also add a missing newline in a printk, and update a comment on the callsite of this function.
Tested on Asus P8Z77-V LX2, still boots.
Change-Id: I86ab643bc87253554346dfed3630eb9ddbd44eb3 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45502 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org --- M src/northbridge/intel/sandybridge/raminit_native.c 1 file changed, 10 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved
diff --git a/src/northbridge/intel/sandybridge/raminit_native.c b/src/northbridge/intel/sandybridge/raminit_native.c index 9961e89..4c1fb8f 100644 --- a/src/northbridge/intel/sandybridge/raminit_native.c +++ b/src/northbridge/intel/sandybridge/raminit_native.c @@ -555,7 +555,7 @@ if (ctrl->tCK > TCK_400MHZ) { printk(BIOS_ERR, "DRAM frequency is under lowest supported frequency (400 MHz). " - "Increasing to 400 MHz as last resort"); + "Increasing to 400 MHz as last resort.\n"); ctrl->tCK = TCK_400MHZ; }
@@ -563,11 +563,11 @@ u8 val2; u32 reg1 = 0;
- /* Step 1 - Set target PCU frequency */ + /* Step 1 - Determine target MPLL frequency */ find_cas_tck(ctrl);
/* - * The PLL will never lock if the required frequency is already set. + * The MPLL will never lock if the requested frequency is already set. * Exit early to prevent a system hang. */ reg1 = MCHBAR32(MC_BIOS_DATA); @@ -575,15 +575,15 @@ if (val2) return;
- /* Step 2 - Select frequency in the MCU */ + /* Step 2 - Request MPLL frequency through the PCU */ reg1 = ctrl->FRQ; if (ctrl->base_freq == 100) - reg1 |= (1 << 8); /* Enable 100Mhz REF clock */ + reg1 |= (1 << 8); /* Use 100MHz reference clock */
- reg1 |= (1 << 31); /* set running bit */ + reg1 |= (1 << 31); /* Set running bit */ MCHBAR32(MC_BIOS_REQ) = reg1; int i = 0; - printk(BIOS_DEBUG, "PLL busy... "); + printk(BIOS_DEBUG, "MPLL busy... "); while (reg1 & (1 << 31)) { udelay(10); i++; @@ -595,11 +595,11 @@ reg1 = MCHBAR32(MC_BIOS_DATA); val2 = (u8) reg1; if (val2 >= ctrl->FRQ) { - printk(BIOS_DEBUG, "MCU frequency is set at : %d MHz\n", + printk(BIOS_DEBUG, "MPLL frequency is set at : %d MHz\n", (1000 << 8) / ctrl->tCK); return; } - printk(BIOS_DEBUG, "PLL didn't lock. Retrying at lower frequency\n"); + printk(BIOS_DEBUG, "MPLL didn't lock. Retrying at lower frequency\n"); ctrl->tCK++; } } @@ -661,7 +661,7 @@ dram_dimm_mapping(ctrl); }
- /* Set MC frequency */ + /* Set MPLL frequency */ dram_freq(ctrl);
if (!fast_boot) {
-
Angel Pons (Code Review) -
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)