Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42009 )
Change subject: sb/intel/bd82x6x: Align some ME functions ......................................................................
sb/intel/bd82x6x: Align some ME functions
This eliminates the differences in the first part of the file.
Change-Id: Ifb7d57da08e02664a28819e65bc8e9697ed38c4c Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/me.c M src/southbridge/intel/bd82x6x/me_8.x.c 2 files changed, 11 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/42009/1
diff --git a/src/southbridge/intel/bd82x6x/me.c b/src/southbridge/intel/bd82x6x/me.c index 4550f7e..f961bfba 100644 --- a/src/southbridge/intel/bd82x6x/me.c +++ b/src/southbridge/intel/bd82x6x/me.c @@ -10,10 +10,10 @@
#include <acpi/acpi.h> #include <device/mmio.h> -#include <device/pci_ops.h> -#include <console/console.h> #include <device/device.h> #include <device/pci.h> +#include <device/pci_ops.h> +#include <console/console.h> #include <device/pci_ids.h> #include <device/pci_def.h> #include <string.h> @@ -597,7 +597,7 @@ printk(BIOS_DEBUG, "ME: MEI resource not present!\n"); return -1; } - mei_base_address = (u32*)(uintptr_t)res->base; + mei_base_address = (u32 *)(uintptr_t)res->base;
/* Ensure Memory and Bus Master bits are set */ pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY); diff --git a/src/southbridge/intel/bd82x6x/me_8.x.c b/src/southbridge/intel/bd82x6x/me_8.x.c index 4fcc090..32c96ad 100644 --- a/src/southbridge/intel/bd82x6x/me_8.x.c +++ b/src/southbridge/intel/bd82x6x/me_8.x.c @@ -25,12 +25,11 @@ #include "pch.h"
#if CONFIG(CHROMEOS) -#include <vendorcode/google/chromeos/chromeos.h> #include <vendorcode/google/chromeos/gnvs.h> #endif
/* Path that the BIOS should take based on ME state */ -static const char *me_bios_path_values[] __unused = { +static const char *me_bios_path_values[] __unused = { [ME_NORMAL_BIOS_PATH] = "Normal", [ME_S3WAKE_BIOS_PATH] = "S3 Wake", [ME_ERROR_BIOS_PATH] = "Error", @@ -38,12 +37,10 @@ [ME_DISABLE_BIOS_PATH] = "Disable", [ME_FIRMWARE_UPDATE_BIOS_PATH] = "Firmware Update", }; -static int intel_me_read_mbp(me_bios_payload *mbp_data);
/* MMIO base address for MEI interface */ static u32 *mei_base_address;
- static void mei_dump(void *ptr, int dword, int offset, const char *type) { struct mei_csr *csr; @@ -292,7 +289,7 @@ if (!mkhi_rsp.is_response || mkhi->group_id != mkhi_rsp.group_id || mkhi->command != mkhi_rsp.command) { - printk(BIOS_ERR, "ME: invalid response, group %u ?= %u," + printk(BIOS_ERR, "ME: invalid response, group %u ?= %u, " "command %u ?= %u, is_response %u\n", mkhi->group_id, mkhi_rsp.group_id, mkhi->command, mkhi_rsp.command, mkhi_rsp.is_response); @@ -568,6 +565,10 @@ return 0; }
+#if CONFIG(CHROMEOS) +#include <vendorcode/google/chromeos/chromeos.h> +#endif + /* Read the Extend register hash of ME firmware */ static int intel_me_extend_valid(struct device *dev) { @@ -622,6 +623,8 @@ pch_enable(dev); }
+static int intel_me_read_mbp(me_bios_payload *mbp_data); + /* Check whether ME is present and do basic init */ static void intel_me_init(struct device *dev) {
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42009 )
Change subject: sb/intel/bd82x6x: Align some ME functions ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42009/1/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_8.x.c:
https://review.coreboot.org/c/coreboot/+/42009/1/src/southbridge/intel/bd82x... PS1, Line 626: static int intel_me_read_mbp(me_bios_payload *mbp_data); need consistent spacing around '*' (ctx:WxV)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42009 )
Change subject: sb/intel/bd82x6x: Align some ME functions ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42009/2/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_8.x.c:
https://review.coreboot.org/c/coreboot/+/42009/2/src/southbridge/intel/bd82x... PS2, Line 626: static int intel_me_read_mbp(me_bios_payload *mbp_data); need consistent spacing around '*' (ctx:WxV)
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42009 )
Change subject: sb/intel/bd82x6x: Align some ME functions ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42009/2/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_8.x.c:
https://review.coreboot.org/c/coreboot/+/42009/2/src/southbridge/intel/bd82x... PS2, Line 568: #if CONFIG(CHROMEOS) : #include <vendorcode/google/chromeos/chromeos.h> : #endif please, why this is moved here ?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42009 )
Change subject: sb/intel/bd82x6x: Align some ME functions ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42009/2/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_8.x.c:
https://review.coreboot.org/c/coreboot/+/42009/2/src/southbridge/intel/bd82x... PS2, Line 568: #if CONFIG(CHROMEOS) : #include <vendorcode/google/chromeos/chromeos.h> : #endif
please, why this is moved here ?
I moved this here to make the diff simpler. We can remove all unneeded headers in a follow-up (I'm pretty sure there will be lots of unused stuff). Does it sound good?
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42009 )
Change subject: sb/intel/bd82x6x: Align some ME functions ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/42009/2/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/me_8.x.c:
https://review.coreboot.org/c/coreboot/+/42009/2/src/southbridge/intel/bd82x... PS2, Line 568: #if CONFIG(CHROMEOS) : #include <vendorcode/google/chromeos/chromeos.h> : #endif
I moved this here to make the diff simpler. […]
yep. thank you.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42009 )
Change subject: sb/intel/bd82x6x: Align some ME functions ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42009 )
Change subject: sb/intel/bd82x6x: Align some ME functions ......................................................................
sb/intel/bd82x6x: Align some ME functions
This eliminates the differences in the first part of the file.
Change-Id: Ifb7d57da08e02664a28819e65bc8e9697ed38c4c Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42009 Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/southbridge/intel/bd82x6x/me.c M src/southbridge/intel/bd82x6x/me_8.x.c 2 files changed, 11 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified HAOUAS Elyes: Looks good to me, approved
diff --git a/src/southbridge/intel/bd82x6x/me.c b/src/southbridge/intel/bd82x6x/me.c index bc0a71e..ebb9db9 100644 --- a/src/southbridge/intel/bd82x6x/me.c +++ b/src/southbridge/intel/bd82x6x/me.c @@ -10,10 +10,10 @@
#include <acpi/acpi.h> #include <device/mmio.h> -#include <device/pci_ops.h> -#include <console/console.h> #include <device/device.h> #include <device/pci.h> +#include <device/pci_ops.h> +#include <console/console.h> #include <device/pci_ids.h> #include <device/pci_def.h> #include <string.h> @@ -564,7 +564,7 @@ printk(BIOS_DEBUG, "ME: MEI resource not present!\n"); return -1; } - mei_base_address = (u32*)(uintptr_t)res->base; + mei_base_address = (u32 *)(uintptr_t)res->base;
/* Ensure Memory and Bus Master bits are set */ pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY); diff --git a/src/southbridge/intel/bd82x6x/me_8.x.c b/src/southbridge/intel/bd82x6x/me_8.x.c index f64e29c..dd972d2 100644 --- a/src/southbridge/intel/bd82x6x/me_8.x.c +++ b/src/southbridge/intel/bd82x6x/me_8.x.c @@ -25,12 +25,11 @@ #include "pch.h"
#if CONFIG(CHROMEOS) -#include <vendorcode/google/chromeos/chromeos.h> #include <vendorcode/google/chromeos/gnvs.h> #endif
/* Path that the BIOS should take based on ME state */ -static const char *me_bios_path_values[] __unused = { +static const char *me_bios_path_values[] __unused = { [ME_NORMAL_BIOS_PATH] = "Normal", [ME_S3WAKE_BIOS_PATH] = "S3 Wake", [ME_ERROR_BIOS_PATH] = "Error", @@ -38,12 +37,10 @@ [ME_DISABLE_BIOS_PATH] = "Disable", [ME_FIRMWARE_UPDATE_BIOS_PATH] = "Firmware Update", }; -static int intel_me_read_mbp(me_bios_payload *mbp_data);
/* MMIO base address for MEI interface */ static u32 *mei_base_address;
- static void mei_dump(void *ptr, int dword, int offset, const char *type) { struct mei_csr *csr; @@ -292,7 +289,7 @@ if (!mkhi_rsp.is_response || mkhi->group_id != mkhi_rsp.group_id || mkhi->command != mkhi_rsp.command) { - printk(BIOS_ERR, "ME: invalid response, group %u ?= %u," + printk(BIOS_ERR, "ME: invalid response, group %u ?= %u, " "command %u ?= %u, is_response %u\n", mkhi->group_id, mkhi_rsp.group_id, mkhi->command, mkhi_rsp.command, mkhi_rsp.is_response); @@ -568,6 +565,10 @@ return 0; }
+#if CONFIG(CHROMEOS) +#include <vendorcode/google/chromeos/chromeos.h> +#endif + /* Read the Extend register hash of ME firmware */ static int intel_me_extend_valid(struct device *dev) { @@ -622,6 +623,8 @@ pch_enable(dev); }
+static int intel_me_read_mbp(me_bios_payload *mbp_data); + /* Check whether ME is present and do basic init */ static void intel_me_init(struct device *dev) {
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42009 )
Change subject: sb/intel/bd82x6x: Align some ME functions ......................................................................
Patch Set 3:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/5221 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/5220 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/5219 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/5218
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
build bot (Jenkins) (Code Review)
-
HAOUAS Elyes (Code Review)
-
Patrick Georgi (Code Review)