coreboot-gerrit October 2020

coreboot-gerrit@coreboot.org

1 participants
3474 discussions

Change in coreboot[master]: mb/clevo/l140cu: Correct FSP-M UPDs
by Michael Niewöhner (Code Review) 11 Oct '20

11 Oct '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46249 ) Change subject: mb/clevo/l140cu: Correct FSP-M UPDs ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/c/coreboot/+/46249/1/src/mainboard/clevo/cml-u/… File src/mainboard/clevo/cml-u/variants/l140cu/romstage.c: https://review.coreboot.org/c/coreboot/+/46249/1/src/mainboard/clevo/cml-u/… PS1, Line 20: DRAM lol. no. https://review.coreboot.org/c/coreboot/+/46249/1/src/mainboard/clevo/cml-u/… PS1, Line 31: Indicates whether memory is interleaved. : * Set to 1 for an interleaved design, : * set to 0 for non-interleaved design. > https://i.imgur.com/eyvgl1S. […] 😄 -- To view, visit https://review.coreboot.org/c/coreboot/+/46249 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I32f1b7bb46eaaf0f0ecad1df310f5de988f64c85 Gerrit-Change-Number: 46249 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 11 Oct 2020 09:09:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/intel/{haswell,model_{1067,106c,206a,6e,6f}x}: Use SPEED_STEP_ENA...
by HAOUAS Elyes (Code Review) 11 Oct '20

11 Oct '20

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46271 ) Change subject: cpu/intel/{haswell,model_{1067,106c,206a,6e,6f}x}: Use SPEED_STEP_ENABLE_BIT macro ...................................................................... cpu/intel/{haswell,model_{1067,106c,206a,6e,6f}x}: Use SPEED_STEP_ENABLE_BIT macro Change-Id: Ib395a07cd7f028121d1c1a3f31fd5f5cb9a7926e Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/model_1067x/model_1067x_init.c M src/cpu/intel/model_106cx/model_106cx_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/cpu/intel/model_6ex/model_6ex_init.c M src/cpu/intel/model_6fx/model_6fx_init.c 6 files changed, 6 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/46271/1 diff --git a/src/cpu/intel/haswell/haswell_init.c b/src/cpu/intel/haswell/haswell_init.c index bf06add..9fd12ad 100644 --- a/src/cpu/intel/haswell/haswell_init.c +++ b/src/cpu/intel/haswell/haswell_init.c @@ -563,7 +563,7 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= FAST_STRINGS_ENABLE_BIT; msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; wrmsr(IA32_MISC_ENABLE, msr); /* Disable Thermal interrupts */ diff --git a/src/cpu/intel/model_1067x/model_1067x_init.c b/src/cpu/intel/model_1067x/model_1067x_init.c index c130894..563c226 100644 --- a/src/cpu/intel/model_1067x/model_1067x_init.c +++ b/src/cpu/intel/model_1067x/model_1067x_init.c @@ -184,7 +184,7 @@ msr.lo |= (1 << 10); /* FERR# multiplexing */ if (eist) - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; /* Enable C2E */ if (((sub_cstates >> (2 * 4)) & 0xf) >= 2) diff --git a/src/cpu/intel/model_106cx/model_106cx_init.c b/src/cpu/intel/model_106cx/model_106cx_init.c index 8c462f1..7be47a4 100644 --- a/src/cpu/intel/model_106cx/model_106cx_init.c +++ b/src/cpu/intel/model_106cx/model_106cx_init.c @@ -50,7 +50,7 @@ msr.lo |= (1 << 10); /* FERR# multiplexing */ // TODO: Only if IA32_PLATFORM_ID[17] = 0 and IA32_PLATFORM_ID[50] = 1 - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; // TODO Do we want Deep C4 and Dynamic L2 shrinking? wrmsr(IA32_MISC_ENABLE, msr); diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c index 6e229f6..86cea37 100644 --- a/src/cpu/intel/model_206ax/model_206ax_init.c +++ b/src/cpu/intel/model_206ax/model_206ax_init.c @@ -324,7 +324,7 @@ msr = rdmsr(IA32_MISC_ENABLE); msr.lo |= FAST_STRINGS_ENABLE_BIT; msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; wrmsr(IA32_MISC_ENABLE, msr); /* Disable Thermal interrupts */ diff --git a/src/cpu/intel/model_6ex/model_6ex_init.c b/src/cpu/intel/model_6ex/model_6ex_init.c index 16c6866..d007c92 100644 --- a/src/cpu/intel/model_6ex/model_6ex_init.c +++ b/src/cpu/intel/model_6ex/model_6ex_init.c @@ -51,7 +51,7 @@ msr.lo |= (1 << 10); /* FERR# multiplexing */ // TODO: Only if IA32_PLATFORM_ID[17] = 0 and IA32_PLATFORM_ID[50] = 1 - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; /* Enable C2E */ msr.lo |= (1 << 26); diff --git a/src/cpu/intel/model_6fx/model_6fx_init.c b/src/cpu/intel/model_6fx/model_6fx_init.c index d0987b4..950027b 100644 --- a/src/cpu/intel/model_6fx/model_6fx_init.c +++ b/src/cpu/intel/model_6fx/model_6fx_init.c @@ -54,7 +54,7 @@ msr.lo |= (1 << 10); /* FERR# multiplexing */ // TODO: Only if IA32_PLATFORM_ID[17] = 0 and IA32_PLATFORM_ID[50] = 1 - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; /* Enable C2E */ msr.lo |= (1 << 26); -- To view, visit https://review.coreboot.org/c/coreboot/+/46271 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib395a07cd7f028121d1c1a3f31fd5f5cb9a7926e Gerrit-Change-Number: 46271 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in ...coreboot[master]: cpu/intel: Use macro instead of numbers
by HAOUAS Elyes (Code Review) 11 Oct '20

11 Oct '20

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31196 Change subject: cpu/intel: Use macro instead of numbers ...................................................................... cpu/intel: Use macro instead of numbers Change-Id: Ic18c219beaec3dad197916a1e235a027c1c74a89 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/cpu/intel/fsp_model_406dx/model_406dx_init.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/model_1067x/model_1067x_init.c M src/cpu/intel/model_106cx/model_106cx_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/cpu/intel/model_6ex/model_6ex_init.c M src/cpu/intel/model_6fx/model_6fx_init.c 7 files changed, 10 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/31196/1 diff --git a/src/cpu/intel/fsp_model_406dx/model_406dx_init.c b/src/cpu/intel/fsp_model_406dx/model_406dx_init.c index 7994f0b..06b37a9 100644 --- a/src/cpu/intel/fsp_model_406dx/model_406dx_init.c +++ b/src/cpu/intel/fsp_model_406dx/model_406dx_init.c @@ -47,9 +47,9 @@ msr_t msr; msr = rdmsr(IA32_MISC_ENABLE); - msr.lo |= (1 << 0); /* Fast String enable */ + msr.lo |= FAST_STRINGS_ENABLE_BIT; msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; wrmsr(IA32_MISC_ENABLE, msr); /* Disable Thermal interrupts */ diff --git a/src/cpu/intel/haswell/haswell_init.c b/src/cpu/intel/haswell/haswell_init.c index aa77964..08abdf5 100644 --- a/src/cpu/intel/haswell/haswell_init.c +++ b/src/cpu/intel/haswell/haswell_init.c @@ -578,9 +578,9 @@ msr_t msr; msr = rdmsr(IA32_MISC_ENABLE); - msr.lo |= (1 << 0); /* Fast String enable */ + msr.lo |= FAST_STRINGS_ENABLE_BIT; msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; wrmsr(IA32_MISC_ENABLE, msr); /* Disable Thermal interrupts */ diff --git a/src/cpu/intel/model_1067x/model_1067x_init.c b/src/cpu/intel/model_1067x/model_1067x_init.c index 6068be1..97ed39d 100644 --- a/src/cpu/intel/model_1067x/model_1067x_init.c +++ b/src/cpu/intel/model_1067x/model_1067x_init.c @@ -199,7 +199,7 @@ msr.lo |= (1 << 10); /* FERR# multiplexing */ if (eist) - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; /* Enable C2E */ if (((sub_cstates >> (2 * 4)) & 0xf) >= 2) diff --git a/src/cpu/intel/model_106cx/model_106cx_init.c b/src/cpu/intel/model_106cx/model_106cx_init.c index e443026..1c0d081 100644 --- a/src/cpu/intel/model_106cx/model_106cx_init.c +++ b/src/cpu/intel/model_106cx/model_106cx_init.c @@ -65,7 +65,7 @@ msr.lo |= (1 << 10); /* FERR# multiplexing */ // TODO: Only if IA32_PLATFORM_ID[17] = 0 and IA32_PLATFORM_ID[50] = 1 - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; // TODO Do we want Deep C4 and Dynamic L2 shrinking? wrmsr(IA32_MISC_ENABLE, msr); diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c index 33ad1e7..c2e0364 100644 --- a/src/cpu/intel/model_206ax/model_206ax_init.c +++ b/src/cpu/intel/model_206ax/model_206ax_init.c @@ -338,9 +338,9 @@ msr_t msr; msr = rdmsr(IA32_MISC_ENABLE); - msr.lo |= (1 << 0); /* Fast String enable */ + msr.lo |= FAST_STRINGS_ENABLE_BIT; msr.lo |= (1 << 3); /* TM1/TM2/EMTTM enable */ - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; wrmsr(IA32_MISC_ENABLE, msr); /* Disable Thermal interrupts */ diff --git a/src/cpu/intel/model_6ex/model_6ex_init.c b/src/cpu/intel/model_6ex/model_6ex_init.c index 19a22f1..fbc6964 100644 --- a/src/cpu/intel/model_6ex/model_6ex_init.c +++ b/src/cpu/intel/model_6ex/model_6ex_init.c @@ -67,7 +67,7 @@ msr.lo |= (1 << 10); /* FERR# multiplexing */ // TODO: Only if IA32_PLATFORM_ID[17] = 0 and IA32_PLATFORM_ID[50] = 1 - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; /* Enable C2E */ msr.lo |= (1 << 26); diff --git a/src/cpu/intel/model_6fx/model_6fx_init.c b/src/cpu/intel/model_6fx/model_6fx_init.c index 4e19896..adc0265 100644 --- a/src/cpu/intel/model_6fx/model_6fx_init.c +++ b/src/cpu/intel/model_6fx/model_6fx_init.c @@ -70,7 +70,7 @@ msr.lo |= (1 << 10); /* FERR# multiplexing */ // TODO: Only if IA32_PLATFORM_ID[17] = 0 and IA32_PLATFORM_ID[50] = 1 - msr.lo |= (1 << 16); /* Enhanced SpeedStep Enable */ + msr.lo |= SPEED_STEP_ENABLE_BIT; /* Enable C2E */ msr.lo |= (1 << 26); -- To view, visit https://review.coreboot.org/c/coreboot/+/31196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic18c219beaec3dad197916a1e235a027c1c74a89 Gerrit-Change-Number: 31196 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

4 11

Change in coreboot[master]: sb/amd/*/*/smbus.c: Make 'smbus.c' uniform.
by HAOUAS Elyes (Code Review) 11 Oct '20

11 Oct '20

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45891 ) Change subject: sb/amd/*/*/smbus.c: Make 'smbus.c' uniform. ...................................................................... sb/amd/*/*/smbus.c: Make 'smbus.c' uniform. Reformat 'smbus.c' files and drop unuseful BIOS_DEBUG messages. Change-Id: I1efddef31dbbb97a7c75c7432d5fe8913a7c3e50 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/southbridge/amd/agesa/hudson/smbus.c M src/southbridge/amd/cimx/sb800/smbus.c M src/southbridge/amd/pi/hudson/smbus.c 3 files changed, 61 insertions(+), 63 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/45891/1 diff --git a/src/southbridge/amd/agesa/hudson/smbus.c b/src/southbridge/amd/agesa/hudson/smbus.c index c418492..a8bf85f 100644 --- a/src/southbridge/amd/agesa/hudson/smbus.c +++ b/src/southbridge/amd/agesa/hudson/smbus.c @@ -4,12 +4,15 @@ #define _HUDSON_SMBUS_C_ #include <arch/io.h> +#include <console/console.h> #include <stdint.h> + #include "smbus.h" static int smbus_wait_until_ready(u32 smbus_io_base) { u32 loops; + loops = SMBUS_TIMEOUT; do { u8 val; @@ -20,12 +23,14 @@ } outb(val, smbus_io_base + SMBHSTSTAT); } while (--loops); + return -2; /* time out */ } static int smbus_wait_until_done(u32 smbus_io_base) { u32 loops; + loops = SMBUS_TIMEOUT; do { u8 val; @@ -40,6 +45,7 @@ return 0; } } while (--loops); + return -3; /* timeout */ } @@ -48,10 +54,11 @@ u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - /* set the device I'm talking to */ + printk(BIOS_DEBUG, "%s - set the device I'm talking to.\n", __func__); outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR); byte = inb(smbus_io_base + SMBHSTCTRL); @@ -75,10 +82,11 @@ u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - /* set the command... */ + printk(BIOS_DEBUG, "%s - set the command.\n", __func__); outb(val, smbus_io_base + SMBHSTCMD); /* set the device I'm talking to */ @@ -97,16 +105,16 @@ return 0; } -int do_smbus_read_byte(u32 smbus_io_base, u32 device, - u32 address) +int do_smbus_read_byte(u32 smbus_io_base, u32 device, u32 address) { u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - /* set the command/address... */ + printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__); outb(address & 0xff, smbus_io_base + SMBHSTCMD); /* set the device I'm talking to */ @@ -128,16 +136,16 @@ return byte; } -int do_smbus_write_byte(u32 smbus_io_base, u32 device, - u32 address, u8 val) +int do_smbus_write_byte(u32 smbus_io_base, u32 device, u32 address, u8 val) { u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - /* set the command/address... */ + printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__); outb(address & 0xff, smbus_io_base + SMBHSTCMD); /* set the device I'm talking to */ @@ -159,8 +167,7 @@ return 0; } -void alink_ab_indx(u32 reg_space, u32 reg_addr, - u32 mask, u32 val) +void alink_ab_indx(u32 reg_space, u32 reg_addr, u32 mask, u32 val) { u32 tmp; @@ -181,8 +188,7 @@ outl(0, AB_INDX); } -void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, - u32 mask, u32 val) +void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, u32 mask, u32 val) { u32 tmp; @@ -206,8 +212,7 @@ /* space = 0: AX_INDXC, AX_DATAC * space = 1: AX_INDXP, AX_DATAP */ -void alink_ax_indx(u32 space /*c or p? */, u32 axindc, - u32 mask, u32 val) +void alink_ax_indx(u32 space /*c or p? */, u32 axindc, u32 mask, u32 val) { u32 tmp; diff --git a/src/southbridge/amd/cimx/sb800/smbus.c b/src/southbridge/amd/cimx/sb800/smbus.c index 7d1ffb9..460b96f 100644 --- a/src/southbridge/amd/cimx/sb800/smbus.c +++ b/src/southbridge/amd/cimx/sb800/smbus.c @@ -1,8 +1,10 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <arch/io.h> +#include <console/console.h> +#include <stdint.h> + #include "smbus.h" -#include <console/console.h> /* printk */ static int smbus_wait_until_ready(u32 smbus_io_base) { @@ -49,12 +51,11 @@ u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - smbus not ready.\n", __func__); - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__); - /* set the device I'm talking to */ + printk(BIOS_DEBUG, "%s - set the device I'm talking to.\n", __func__); outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR); byte = inb(smbus_io_base + SMBHSTCTRL); @@ -70,7 +71,6 @@ /* read results of transaction */ byte = inb(smbus_io_base + SMBHSTCMD); - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__); return byte; } @@ -79,12 +79,11 @@ u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - smbus not ready.\n", __func__); - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__); - /* set the command... */ + printk(BIOS_DEBUG, "%s - set the command.\n", __func__); outb(val, smbus_io_base + SMBHSTCMD); /* set the device I'm talking to */ @@ -100,7 +99,6 @@ return -3; /* timeout or error */ } - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__); return 0; } @@ -109,12 +107,11 @@ u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - smbus not ready.\n", __func__); - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__); - /* set the command/address... */ + printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__); outb(address & 0xff, smbus_io_base + SMBHSTCMD); /* set the device I'm talking to */ @@ -133,7 +130,6 @@ /* read results of transaction */ byte = inb(smbus_io_base + SMBHSTDAT0); - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__); return byte; } @@ -142,12 +138,11 @@ u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - smbus not ready.\n", __func__); - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__); - /* set the command/address... */ + printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__); outb(address & 0xff, smbus_io_base + SMBHSTCMD); /* set the device I'm talking to */ @@ -166,7 +161,6 @@ return -3; /* timeout or error */ } - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__); return 0; } @@ -174,7 +168,6 @@ { u32 tmp; - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__); outl((reg_space & 0x7) << 29 | reg_addr, AB_INDX); tmp = inl(AB_DATA); /* rpr 4.2 @@ -190,14 +183,12 @@ outl((reg_space & 0x7) << 29 | reg_addr, AB_INDX); /* probably we don't have to do it again. */ outl(tmp, AB_DATA); outl(0, AB_INDX); - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__); } void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, u32 mask, u32 val) { u32 tmp; - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__); outl((reg_space & 0x7) << 29 | (port & 3) << 24 | reg_addr, AB_INDX); tmp = inl(AB_DATA); /* rpr 4.2 @@ -213,7 +204,6 @@ outl((reg_space & 0x7) << 29 | (port & 3) << 24 | reg_addr, AB_INDX); /* probably we don't have to do it again. */ outl(tmp, AB_DATA); outl(0, AB_INDX); - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__); } /* space = 0: AX_INDXC, AX_DATAC @@ -223,7 +213,6 @@ { u32 tmp; - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - Start.\n", __func__); /* read axindc to tmp */ outl(space << 29 | space << 3 | 0x30, AB_INDX); outl(axindc, AB_DATA); @@ -242,5 +231,4 @@ outl(space << 29 | space << 3 | 0x34, AB_INDX); outl(tmp, AB_DATA); outl(0, AB_INDX); - printk(BIOS_DEBUG, "SB800 - Smbus.c - %s - End.\n", __func__); } diff --git a/src/southbridge/amd/pi/hudson/smbus.c b/src/southbridge/amd/pi/hudson/smbus.c index c418492..a8bf85f 100644 --- a/src/southbridge/amd/pi/hudson/smbus.c +++ b/src/southbridge/amd/pi/hudson/smbus.c @@ -4,12 +4,15 @@ #define _HUDSON_SMBUS_C_ #include <arch/io.h> +#include <console/console.h> #include <stdint.h> + #include "smbus.h" static int smbus_wait_until_ready(u32 smbus_io_base) { u32 loops; + loops = SMBUS_TIMEOUT; do { u8 val; @@ -20,12 +23,14 @@ } outb(val, smbus_io_base + SMBHSTSTAT); } while (--loops); + return -2; /* time out */ } static int smbus_wait_until_done(u32 smbus_io_base) { u32 loops; + loops = SMBUS_TIMEOUT; do { u8 val; @@ -40,6 +45,7 @@ return 0; } } while (--loops); + return -3; /* timeout */ } @@ -48,10 +54,11 @@ u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - /* set the device I'm talking to */ + printk(BIOS_DEBUG, "%s - set the device I'm talking to.\n", __func__); outb(((device & 0x7f) << 1) | 1, smbus_io_base + SMBHSTADDR); byte = inb(smbus_io_base + SMBHSTCTRL); @@ -75,10 +82,11 @@ u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - /* set the command... */ + printk(BIOS_DEBUG, "%s - set the command.\n", __func__); outb(val, smbus_io_base + SMBHSTCMD); /* set the device I'm talking to */ @@ -97,16 +105,16 @@ return 0; } -int do_smbus_read_byte(u32 smbus_io_base, u32 device, - u32 address) +int do_smbus_read_byte(u32 smbus_io_base, u32 device, u32 address) { u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - /* set the command/address... */ + printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__); outb(address & 0xff, smbus_io_base + SMBHSTCMD); /* set the device I'm talking to */ @@ -128,16 +136,16 @@ return byte; } -int do_smbus_write_byte(u32 smbus_io_base, u32 device, - u32 address, u8 val) +int do_smbus_write_byte(u32 smbus_io_base, u32 device, u32 address, u8 val) { u8 byte; if (smbus_wait_until_ready(smbus_io_base) < 0) { - return -2; /* not ready */ + printk(BIOS_DEBUG, "%s - smbus not ready.\n", __func__); + return -2; } - /* set the command/address... */ + printk(BIOS_DEBUG, "%s - set the command/address.\n", __func__); outb(address & 0xff, smbus_io_base + SMBHSTCMD); /* set the device I'm talking to */ @@ -159,8 +167,7 @@ return 0; } -void alink_ab_indx(u32 reg_space, u32 reg_addr, - u32 mask, u32 val) +void alink_ab_indx(u32 reg_space, u32 reg_addr, u32 mask, u32 val) { u32 tmp; @@ -181,8 +188,7 @@ outl(0, AB_INDX); } -void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, - u32 mask, u32 val) +void alink_rc_indx(u32 reg_space, u32 reg_addr, u32 port, u32 mask, u32 val) { u32 tmp; @@ -206,8 +212,7 @@ /* space = 0: AX_INDXC, AX_DATAC * space = 1: AX_INDXP, AX_DATAP */ -void alink_ax_indx(u32 space /*c or p? */, u32 axindc, - u32 mask, u32 val) +void alink_ax_indx(u32 space /*c or p? */, u32 axindc, u32 mask, u32 val) { u32 tmp; -- To view, visit https://review.coreboot.org/c/coreboot/+/45891 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1efddef31dbbb97a7c75c7432d5fe8913a7c3e50 Gerrit-Change-Number: 45891 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: drivers/wifi: Drop maxsleep parameter from chip config
by Furquan Shaikh (Code Review) 11 Oct '20

11 Oct '20

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46033 ) Change subject: drivers/wifi: Drop maxsleep parameter from chip config ...................................................................... drivers/wifi: Drop maxsleep parameter from chip config This change drops maxsleep parameter from chip config and instead hardcodes the deepest sleep state from which the WiFi device can wake the system up from to SLP_TYP_S3. This is similar to how other device drivers in coreboot report _PRW property in ACPI. It relieves the users from adding another register attribute to devicetree since all mainboards configure the same value. If this changes in the future, it should be easy to bring the maxsleep config parameter back. BUG=b:169802515 BRANCH=zork Change-Id: I42131fced008da0d51f0f777b7f2d99deaf68827 Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M src/drivers/intel/wifi/wifi.c M src/drivers/wifi/generic/chip.h M src/drivers/wifi/generic/generic.c M src/mainboard/google/dedede/variants/madoo/overridetree.cb M src/mainboard/google/dedede/variants/magolor/overridetree.cb M src/mainboard/google/dedede/variants/waddledee/overridetree.cb 6 files changed, 3 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/46033/1 diff --git a/src/drivers/intel/wifi/wifi.c b/src/drivers/intel/wifi/wifi.c index f768b04..6010673 100644 --- a/src/drivers/intel/wifi/wifi.c +++ b/src/drivers/intel/wifi/wifi.c @@ -49,11 +49,9 @@ struct drivers_intel_wifi_config *config = dev->chip_info; struct drivers_wifi_generic_config generic_config; - if (config) { + if (config) generic_config.wake = config->wake; - /* By default, all intel wifi chips wake from S3 */ - generic_config.maxsleep = 3; - } + wifi_generic_fill_ssdt(dev, config ? &generic_config : NULL); } #endif diff --git a/src/drivers/wifi/generic/chip.h b/src/drivers/wifi/generic/chip.h index fe3a1d1..02ab504 100644 --- a/src/drivers/wifi/generic/chip.h +++ b/src/drivers/wifi/generic/chip.h @@ -6,11 +6,9 @@ /** * struct drivers_wifi_generic_config - Data structure to contain generic wifi config * @wake: Wake pin for ACPI _PRW - * @maxsleep: Maximum sleep state to wake from */ struct drivers_wifi_generic_config { unsigned int wake; - unsigned int maxsleep; }; /** diff --git a/src/drivers/wifi/generic/generic.c b/src/drivers/wifi/generic/generic.c index 059185d..903afdc 100644 --- a/src/drivers/wifi/generic/generic.c +++ b/src/drivers/wifi/generic/generic.c @@ -189,7 +189,7 @@ /* Wake capabilities */ if (config) - acpigen_write_PRW(config->wake, config->maxsleep); + acpigen_write_PRW(config->wake, SLP_TYP_S3); /* Fill regulatory domain structure */ if (CONFIG(HAVE_REGULATORY_DOMAIN)) { diff --git a/src/mainboard/google/dedede/variants/madoo/overridetree.cb b/src/mainboard/google/dedede/variants/madoo/overridetree.cb index 039fd10..257f917 100644 --- a/src/mainboard/google/dedede/variants/madoo/overridetree.cb +++ b/src/mainboard/google/dedede/variants/madoo/overridetree.cb @@ -103,7 +103,6 @@ device pci 1c.7 on chip drivers/wifi/generic register "wake" = "GPE0_DW2_03" - register "maxsleep" = "3" device pci 00.0 on end end end # PCI Express Root Port 8 - WLAN diff --git a/src/mainboard/google/dedede/variants/magolor/overridetree.cb b/src/mainboard/google/dedede/variants/magolor/overridetree.cb index 8a83b83..f41e9fa 100644 --- a/src/mainboard/google/dedede/variants/magolor/overridetree.cb +++ b/src/mainboard/google/dedede/variants/magolor/overridetree.cb @@ -287,7 +287,6 @@ device pci 1c.7 on chip drivers/wifi/generic register "wake" = "GPE0_DW2_03" - register "maxsleep" = "3" device pci 00.0 on end end end # PCI Express Root Port 8 - WLAN diff --git a/src/mainboard/google/dedede/variants/waddledee/overridetree.cb b/src/mainboard/google/dedede/variants/waddledee/overridetree.cb index a525a0f..22a08a0 100644 --- a/src/mainboard/google/dedede/variants/waddledee/overridetree.cb +++ b/src/mainboard/google/dedede/variants/waddledee/overridetree.cb @@ -147,7 +147,6 @@ device pci 1c.7 on chip drivers/wifi/generic register "wake" = "GPE0_DW2_03" - register "maxsleep" = "3" device pci 00.0 on end end end # PCI Express Root Port 8 - WLAN -- To view, visit https://review.coreboot.org/c/coreboot/+/46033 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I42131fced008da0d51f0f777b7f2d99deaf68827 Gerrit-Change-Number: 46033 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

5 19

Change in coreboot[master]: drivers/wifi/generic: Log WiFi wake source to event log
by Furquan Shaikh (Code Review) 11 Oct '20

11 Oct '20

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46032 ) Change subject: drivers/wifi/generic: Log WiFi wake source to event log ...................................................................... drivers/wifi/generic: Log WiFi wake source to event log This change adds a call to `pci_dev_log_wake()` to log WiFi wake source to event log just like the Intel WiFi driver does. This is done in preparation to merge the generic and Intel WiFi drivers in follow up changes. BUG=b:169802515 BRANCH=zork Change-Id: I20528ae1f72ca633da31e01d777c46fd5f4a337f Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M src/drivers/wifi/generic/generic.c 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/46032/1 diff --git a/src/drivers/wifi/generic/generic.c b/src/drivers/wifi/generic/generic.c index b175f23..059185d 100644 --- a/src/drivers/wifi/generic/generic.c +++ b/src/drivers/wifi/generic/generic.c @@ -6,6 +6,7 @@ #include <device/device.h> #include <device/pci.h> #include <device/pci_def.h> +#include <elog.h> #include <sar.h> #include <string.h> #include <wrdd.h> @@ -237,10 +238,16 @@ wifi_generic_fill_ssdt(dev, dev->chip_info); } +static void wifi_pci_dev_init(struct device *dev) +{ + pci_dev_log_wake(dev, ELOG_WAKE_SOURCE_PME_WIFI, 0); +} + struct device_operations wifi_generic_ops = { .read_resources = pci_dev_read_resources, .set_resources = pci_dev_set_resources, .enable_resources = pci_dev_enable_resources, + .init = wifi_pci_dev_init, .ops_pci = &pci_dev_ops_pci, .acpi_name = wifi_generic_acpi_name, .acpi_fill_ssdt = wifi_generic_fill_ssdt_generator, -- To view, visit https://review.coreboot.org/c/coreboot/+/46032 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I20528ae1f72ca633da31e01d777c46fd5f4a337f Gerrit-Change-Number: 46032 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: newchange

5 7

Change in coreboot[master]: drivers/intel/wifi: Use newly added pci_dev_log_wake
by Furquan Shaikh (Code Review) 11 Oct '20

11 Oct '20

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46031 ) Change subject: drivers/intel/wifi: Use newly added pci_dev_log_wake ...................................................................... drivers/intel/wifi: Use newly added pci_dev_log_wake This change uses the newly added `pci_dev_log_wake()` helper function to determine and log WiFi wake source instead of assuming a hard-coded register value to check. This is done in preparation to merge the generic WiFi and Intel WiFi drivers in coreboot in follow up changes. BUG=b:169802515 BRANCH=zork Change-Id: I9bdb453092b4ce7bdab2969f13e0c0aa8166dc0a Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M src/drivers/intel/wifi/wifi.c 1 file changed, 1 insertion(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/46031/1 diff --git a/src/drivers/intel/wifi/wifi.c b/src/drivers/intel/wifi/wifi.c index e117ec0..f768b04 100644 --- a/src/drivers/intel/wifi/wifi.c +++ b/src/drivers/intel/wifi/wifi.c @@ -10,9 +10,6 @@ #include "chip.h" #include "drivers/wifi/generic/chip.h" -#define PMCS_DR 0xcc -#define PME_STS (1 << 15) - #if CONFIG(GENERATE_SMBIOS_TABLES) static int smbios_write_wifi(struct device *dev, int *handle, unsigned long *current) @@ -63,12 +60,7 @@ static void wifi_pci_dev_init(struct device *dev) { - if (CONFIG(ELOG)) { - uint32_t val; - val = pci_read_config16(dev, PMCS_DR); - if (val & PME_STS) - elog_add_event_wake(ELOG_WAKE_SOURCE_PME_WIFI, 0); - } + pci_dev_log_wake(dev, ELOG_WAKE_SOURCE_PME_WIFI, 0); } struct device_operations device_ops = { -- To view, visit https://review.coreboot.org/c/coreboot/+/46031 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9bdb453092b4ce7bdab2969f13e0c0aa8166dc0a Gerrit-Change-Number: 46031 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

5 9

Change in coreboot[master]: pci_device: Add a helper function for logging PCI device as wake source
by Furquan Shaikh (Code Review) 11 Oct '20

11 Oct '20

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46030 ) Change subject: pci_device: Add a helper function for logging PCI device as wake source ...................................................................... pci_device: Add a helper function for logging PCI device as wake source This change adds a helper function `pci_dev_log_wake()` that checks PME_STATUS and PME_ENABLE bits in PM control and status register to determine if the given device is the source of wake and logs it in eventlog if CONFIG_ELOG is selected. The caller is expected to pass in the source_id and instance that will be passed into `elog_add_event_wake()`. BUG=b:169802515 BRANCH=zork Change-Id: I06e9530b568543ab2f05a4f38dc5c3a527ff391e Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M src/device/pci_device.c M src/include/device/pci.h 2 files changed, 33 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/46030/1 diff --git a/src/device/pci_device.c b/src/device/pci_device.c index 8a6f123..06e1228 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -20,6 +20,7 @@ #include <device/pcix.h> #include <device/pciexp.h> #include <device/hypertransport.h> +#include <elog.h> #include <pc80/i8259.h> #include <security/vboot/vbnv.h> #include <timestamp.h> @@ -1643,3 +1644,28 @@ pci_update_config16(dev, PCI_COMMAND, ~PCI_COMMAND_MASTER, 0x0); } #endif + +void pci_dev_log_wake(const struct device *dev, uint8_t source_id, uint32_t instance) +{ + unsigned int pm_cap; + uint16_t pmcs; + + if (!CONFIG(ELOG)) + return; + + if (dev->path.type != DEVICE_PATH_PCI) + return; + + pm_cap = pci_find_capability(dev, PCI_CAP_ID_PM); + if (!pm_cap) + return; + + pmcs = pci_read_config16(dev, pm_cap + PCI_PM_CTRL); + if (!(pmcs & PCI_PM_CTRL_PME_ENABLE)) + return; + + if (!(pmcs & PCI_PM_CTRL_PME_STATUS)) + return; + + elog_add_event_wake(source_id, instance); +} diff --git a/src/include/device/pci.h b/src/include/device/pci.h index 6e28cb7..87702e2 100644 --- a/src/include/device/pci.h +++ b/src/include/device/pci.h @@ -80,6 +80,13 @@ struct device *pci_probe_dev(struct device *dev, struct bus *bus, unsigned int devfn); +/* + * Determine if the given PCI device is the source of wake from sleep by checking PME_STATUS and + * PME_ENABLE bits in PM control and status register and log the given source_id and instance in + * elog. + */ +void pci_dev_log_wake(const struct device *dev, uint8_t source_id, uint32_t instance); + void do_pci_scan_bridge(struct device *dev, void (*do_scan_bus)(struct bus *bus, unsigned int min_devfn, unsigned int max_devfn)); -- To view, visit https://review.coreboot.org/c/coreboot/+/46030 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I06e9530b568543ab2f05a4f38dc5c3a527ff391e Gerrit-Change-Number: 46030 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: newchange

4 7

Change in coreboot[master]: mb/google/puff: Enable SATA0 on wyvern
by Edward O'Callaghan (Code Review) 11 Oct '20

11 Oct '20

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46270 ) Change subject: mb/google/puff: Enable SATA0 on wyvern ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/46270 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a4ce2f89fa6d786358e01add15f2eedfbe4b20f Gerrit-Change-Number: 46270 Gerrit-PatchSet: 2 Gerrit-Owner: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 11 Oct 2020 01:57:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/puff: Enable SATA0 on wyvern
by Sam McNally (Code Review) 11 Oct '20

11 Oct '20

Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46270 ) Change subject: mb/google/puff: Enable SATA0 on wyvern ...................................................................... Patch Set 2: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/46270 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a4ce2f89fa6d786358e01add15f2eedfbe4b20f Gerrit-Change-Number: 46270 Gerrit-PatchSet: 2 Gerrit-Owner: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 11 Oct 2020 00:16:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2020