Hello Marco Chen,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/33661
to review the following change.
Change subject: vendorcode/google: load sar config from CBFS first then VPD
......................................................................
vendorcode/google: load sar config from CBFS first then VPD
SAR config provisioned in RO VPD can be done in the factory only. Once
it is wrong, we can override the SAR config by updating FW RW which can
carry new SAR config in CBFS. As a result, we should check CBFS first
then VPD.
Change-Id: I5aa6235fb7a6d0b2ed52893a42f7bd57806af6c1
Signed-off-by: Marco Chen <marcochen(a)chromium.org>
---
M src/vendorcode/google/chromeos/sar.c
1 file changed, 23 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/33661/1
diff --git a/src/vendorcode/google/chromeos/sar.c b/src/vendorcode/google/chromeos/sar.c
index bbcb211..a5a2c3b 100644
--- a/src/vendorcode/google/chromeos/sar.c
+++ b/src/vendorcode/google/chromeos/sar.c
@@ -82,36 +82,37 @@
sizeof(struct wifi_sar_delta_table);
}
- /* Try to read the SAR limit entry from VPD */
- if (!vpd_gets(wifi_sar_limit_key, wifi_sar_limit_str,
- buffer_size, VPD_ANY)) {
- printk(BIOS_ERR, "Error: Could not locate '%s' in VPD.\n",
- wifi_sar_limit_key);
-
- if (!CONFIG(WIFI_SAR_CBFS))
- return -1;
-
+ if (CONFIG(WIFI_SAR_CBFS)) {
printk(BIOS_DEBUG, "Checking CBFS for default SAR values\n");
sar_cbfs_len = load_sar_file_from_cbfs(
(void *) wifi_sar_limit_str,
sar_expected_len);
- if (sar_cbfs_len != sar_expected_len) {
- printk(BIOS_ERR, "%s has bad len in CBFS\n",
- WIFI_SAR_CBFS_FILENAME);
- return -1;
- }
- } else {
- /* VPD key "wifi_sar" found. strlen is checked with addition of
- * 1 as we have created buffer size 1 char larger for the reason
- * mentioned at start of this function itself */
- if (strlen(wifi_sar_limit_str) + 1 != sar_expected_len) {
- printk(BIOS_ERR, "WIFI SAR key has bad len in VPD\n");
- return -1;
- }
+ if (sar_cbfs_len == sar_expected_len)
+ goto done;
+
+ printk(BIOS_ERR, "%s has bad len in CBFS\n",
+ WIFI_SAR_CBFS_FILENAME);
}
+ /* Try to read the SAR limit entry from VPD */
+ if (!vpd_gets(wifi_sar_limit_key, wifi_sar_limit_str,
+ buffer_size, VPD_ANY)) {
+ printk(BIOS_ERR, "Error: Could not locate '%s' in VPD.\n",
+ wifi_sar_limit_key);
+ return -1;
+ }
+
+ /* VPD key "wifi_sar" found. strlen is checked with addition of
+ * 1 as we have created buffer size 1 char larger for the reason
+ * mentioned at start of this function itself */
+ if (strlen(wifi_sar_limit_str) + 1 != sar_expected_len) {
+ printk(BIOS_ERR, "WIFI SAR key has bad len in VPD\n");
+ return -1;
+ }
+
+done:
/* Decode the heximal encoded string to binary values */
if (hexstrtobin(wifi_sar_limit_str, bin_buffer, bin_buff_adjusted_size)
< bin_buff_adjusted_size) {
--
To view, visit https://review.coreboot.org/c/coreboot/+/33661
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5aa6235fb7a6d0b2ed52893a42f7bd57806af6c1
Gerrit-Change-Number: 33661
Gerrit-PatchSet: 1
Gerrit-Owner: Marco Chen <marcochen(a)google.com>
Gerrit-Reviewer: Marco Chen <marcochen(a)chromium.org>
Gerrit-MessageType: newchange
Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41290 )
Change subject: Documentation: Add GbE and test results disabling it
......................................................................
Documentation: Add GbE and test results disabling it
The GbE region should not be disabled without further IFD modifications.
Change-Id: Ie7e3104bcf9cf28b32f15ec4268d8a3fa35c0d9f
Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com>
---
A Documentation/soc/intel/gbe.md
M Documentation/soc/intel/index.md
2 files changed, 25 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/41290/1
diff --git a/Documentation/soc/intel/gbe.md b/Documentation/soc/intel/gbe.md
new file mode 100644
index 0000000..19094c6
--- /dev/null
+++ b/Documentation/soc/intel/gbe.md
@@ -0,0 +1,21 @@
+# Intel Flash descriptor GbE region
+
+The gigabit ethernet (GbE) region is an 8 KiB reserved area of the x86 boot
+NOR SPI flash. The position withing the flash is specified by the IFD.
+
+The GbE regiong stores configuration data, for e.g. the MAC address
+of the Intel onboard network interface card. Thus if the Intel onboard
+network card isn't used by the mainboard, there's no GbE region present.
+
+The GbE region is named `gbe` in flashrom and ifdtool.
+
+# Removing the GbE
+
+Removing the GbE (marking the region as unused in the IFD) on a platform
+that has the onboard NIC enabled in fuses results in a no boot.
+
+# Clearing the GbE region
+
+Overwriting the GbE region allows to boot and the GbE PCI device shows
+up on GNU/Linux. The e1000e fails to load with 'error -3'.
+
diff --git a/Documentation/soc/intel/index.md b/Documentation/soc/intel/index.md
index f30ff9a..2df1fe6 100644
--- a/Documentation/soc/intel/index.md
+++ b/Documentation/soc/intel/index.md
@@ -10,3 +10,7 @@
- [MP Initialization](mp_init/mp_init.md)
- [Firmware Interface Table](fit.md)
- [Apollolake](apollolake/index.md)
+
+## IFD
+
+- [GbE region](gbe.md)
--
To view, visit https://review.coreboot.org/c/coreboot/+/41290
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie7e3104bcf9cf28b32f15ec4268d8a3fa35c0d9f
Gerrit-Change-Number: 41290
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-MessageType: newchange
Evgeny Zinoviev has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39761 )
Change subject: Doc/mb/lenovo/montevina_series: Describe GbE generating
......................................................................
Doc/mb/lenovo/montevina_series: Describe GbE generating
Instead of using the gbe region extracted from stock dump, one may want
to generate the new one. util/bincfg can do that, but it's not
documented anywhere yet.
This patch describes how to do that along with changing the MAC
address.
Change-Id: I9318514b125b53f8ad76ad99755690ec6f681441
Signed-off-by: Evgeny Zinoviev <me(a)ch1p.io>
---
M Documentation/mainboard/lenovo/montevina_series.md
1 file changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39761/1
diff --git a/Documentation/mainboard/lenovo/montevina_series.md b/Documentation/mainboard/lenovo/montevina_series.md
index 62e8796..39dee2f 100644
--- a/Documentation/mainboard/lenovo/montevina_series.md
+++ b/Documentation/mainboard/lenovo/montevina_series.md
@@ -33,6 +33,9 @@
$ ifdtool -x backup.rom
```
+Note that you can also [generate new GbE region](#generating-gbe-region) and use
+it instead.
+
Now you need to patch the flash descriptor. You can either [modify the one from
your backup with **ifdtool**](#modifying-flash-descriptor-using-ifdtool), or
[generate a completely new one with **bincfg**](#creating-a-new-flash-descriptor-using-bincfg).
@@ -122,6 +125,7 @@
(/path/to/flashregion_0_flashdescriptor.bin) Path and filename of the descriptor.bin file
[*] Add gigabit ethernet configuration
+ # Note: if you used bincfg, specify path to generated util/bincfg/flashregion_3_gbe.bin
(/path/to/flashregion_3_gbe.bin) Path to gigabit ethernet configuration
```
@@ -136,6 +140,32 @@
# flashrom -p YOUR_PROGRAMMER -w coreboot.rom --ifd -i bios
```
+## Generating GbE region
+
+GbE region contains configuration data for Intel integrated Gigabit Ethernet.
+You can generate new GbE Region by using **bincfg**. To do that, go to
+`util/bincfg`.
+
+It's good idea to set MAC address first, so open then `gbe-ich9m.set` file and
+change values of `macaddress0` .. `macaddress5` fields. For example, if you want
+to set MAC address to `00:01:02:03:04:05`:
+
+```
+ "macaddress0" = 0x0,
+ "macaddress1" = 0x1,
+ "macaddress2" = 0x2,
+ "macaddress3" = 0x3,
+ "macaddress4" = 0x4,
+ "macaddress5" = 0x5
+```
+
+Then run:
+```console
+$ make gen-gbe-ich9m
+```
+
+The new GbE region will be saved to `flashregion_3_gbe.bin`.
+
## Flash layout
The flash layouts of the OEM firmware are as follows:
--
To view, visit https://review.coreboot.org/c/coreboot/+/39761
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9318514b125b53f8ad76ad99755690ec6f681441
Gerrit-Change-Number: 39761
Gerrit-PatchSet: 1
Gerrit-Owner: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-MessageType: newchange
Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41877 )
Change subject: Add Kconfig option for ACPI S3 support
......................................................................
Add Kconfig option for ACPI S3 support
Being able to disable ACPI S3 saves 150 ms on the ASRock E350M1.
Change-Id: Ie25c6ab5cd927287f0e2ff00cce042a95f7d377c
Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de>
---
M src/Kconfig
M src/lib/hardwaremain.c
2 files changed, 20 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/41877/1
diff --git a/src/Kconfig b/src/Kconfig
index 2a2a144..0e89eb6 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -135,6 +135,22 @@
every boot. Use this if you want the NVRAM configuration to
never be modified from its default values.
+config DO_ACPI_RESUME
+ bool "Enable ACPI S3 resume"
+ default y
+ depends on HAVE_ACPI_RESUME
+ help
+ Adds S3 to the available sleepstates
+
+ Enabling ACPI S3 resume support delays the boot-up on certain boards.
+ For example, the delay it 200 ms on the AMD AGESA board ASRock E350M1.
+
+ If you do not need ACPI S3 resume support, select this option.
+
+ (And why should you, with coreboot, an SSD and an optimized OS, you
+ boot as fast as with resuming from ACPI S3, and have less code paths
+ to maintain and test.)
+
config COMPRESS_RAMSTAGE
bool "Compress ramstage with LZMA"
depends on HAVE_RAMSTAGE
diff --git a/src/lib/hardwaremain.c b/src/lib/hardwaremain.c
index 935393e..20b7c25 100644
--- a/src/lib/hardwaremain.c
+++ b/src/lib/hardwaremain.c
@@ -19,7 +19,7 @@
#include <stdlib.h>
#include <boot/tables.h>
#include <program_loading.h>
-#if CONFIG(HAVE_ACPI_RESUME)
+#if CONFIG(DO_ACPI_RESUME)
#include <acpi/acpi.h>
#endif
#include <timer.h>
@@ -151,7 +151,7 @@
static boot_state_t bs_os_resume_check(void *arg)
{
-#if CONFIG(HAVE_ACPI_RESUME)
+#if CONFIG(DO_ACPI_RESUME)
void *wake_vector;
wake_vector = acpi_find_wakeup_vector();
@@ -168,7 +168,7 @@
static boot_state_t bs_os_resume(void *wake_vector)
{
-#if CONFIG(HAVE_ACPI_RESUME)
+#if CONFIG(DO_ACPI_RESUME)
arch_bootstate_coreboot_exit();
acpi_resume(wake_vector);
#endif
@@ -445,7 +445,7 @@
post_code(POST_ENTRY_RAMSTAGE);
/* Handoff sleep type from romstage. */
-#if CONFIG(HAVE_ACPI_RESUME)
+#if CONFIG(DO_ACPI_RESUME)
acpi_is_wakeup();
#endif
threads_initialize();
--
To view, visit https://review.coreboot.org/c/coreboot/+/41877
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie25c6ab5cd927287f0e2ff00cce042a95f7d377c
Gerrit-Change-Number: 41877
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newchange