18 Jan
2019
18 Jan
'19
11:50 a.m.
Nico Huber has uploaded this change for review. (
https://review.coreboot.org/c/flashrom/+/30995
Change subject: ichspi: Add Apollo Lake support
......................................................................
ichspi: Add Apollo Lake support
It's almost identical to 100 series PCHs and later. There are some
addition FREGs (12..15). To not clutter the `if` conditions further,
make more use of `switch` statements.
Tested on Kontron mAL10. Mark it as DEP as usually the last sector
is not covered by the descriptor layout and can't be read.
Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
---
M chipset_enable.c
M ich_descriptors.c
M ichspi.c
3 files changed, 153 insertions(+), 62 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/30995/1
diff --git a/chipset_enable.c b/chipset_enable.c
index 547f21a..2f73ded 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -1965,7 +1965,7 @@
{0x8086, 0xa2c8, NT, "Intel", "B250", enable_flash_pch100},
{0x8086, 0xa2c9, NT, "Intel", "Z370", enable_flash_pch100},
{0x8086, 0xa2d2, NT, "Intel", "X299", enable_flash_pch100},
- {0x8086, 0x5ae8, BAD, "Intel", "Apollo Lake", enable_flash_apl},
+ {0x8086, 0x5ae8, DEP, "Intel", "Apollo Lake", enable_flash_apl},
#endif
{0},
};
diff --git a/ich_descriptors.c b/ich_descriptors.c
index 56e846f..dcbcfae 100644
--- a/ich_descriptors.c
+++ b/ich_descriptors.c
@@ -42,6 +42,8 @@
ssize_t ich_number_of_regions(const enum ich_chipset cs, const struct ich_desc_content
*const cont)
{
switch (cs) {
+ case CHIPSET_APOLLO_LAKE:
+ return 6;
case CHIPSET_C620_SERIES_LEWISBURG:
return 16;
case CHIPSET_100_SERIES_SUNRISE_POINT:
@@ -67,6 +69,7 @@
{
switch (cs) {
case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
if (cont->NM <= MAX_NUM_MASTERS)
return cont->NM;
default:
@@ -139,8 +142,8 @@
msg_pdbg2("FRBA (Flash Region Base Address): 0x%03x\n",
getFRBA(cont));
msg_pdbg2("NC (Number of Components): %5d\n",
cont->NC + 1);
msg_pdbg2("FCBA (Flash Component Base Address): 0x%03x\n",
getFCBA(cont));
- msg_pdbg2("ISL (ICH/PCH Strap Length): %5d\n",
cont->ISL);
- msg_pdbg2("FISBA/FPSBA (Flash ICH/PCH Strap Base Address): 0x%03x\n",
getFISBA(cont));
+ msg_pdbg2("ISL (ICH/PCH/SoC Strap Length): %5d\n",
cont->ISL);
+ msg_pdbg2("FISBA/FPSBA (Flash ICH/PCH/SoC Strap Base Addr): 0x%03x\n",
getFISBA(cont));
msg_pdbg2("NM (Number of Masters): %5zd\n",
ich_number_of_masters(cs, cont));
msg_pdbg2("FMBA (Flash Master Base Address): 0x%03x\n",
getFMBA(cont));
msg_pdbg2("MSL/PSL (MCH/PROC Strap Length): %5d\n",
cont->MSL);
@@ -193,7 +196,8 @@
case CHIPSET_9_SERIES_WILDCAT_POINT:
case CHIPSET_9_SERIES_WILDCAT_POINT_LP:
case CHIPSET_100_SERIES_SUNRISE_POINT:
- case CHIPSET_C620_SERIES_LEWISBURG: {
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE: {
uint8_t size_enc;
if (idx == 0) {
size_enc = desc->component.dens_new.comp1_density;
@@ -212,7 +216,7 @@
static const char *pprint_freq(enum ich_chipset cs, uint8_t value)
{
- static const char *const freq_str[2][8] = { {
+ static const char *const freq_str[3][8] = { {
"20 MHz", /* 000 */
"33 MHz", /* 001 */
"reserved", /* 010 */
@@ -230,6 +234,15 @@
"reserved", /* 101 */
"17 MHz", /* 110 */
"reserved" /* 111 */
+ }, {
+ "reserved", /* 000 */
+ "50 MHz", /* 001 */
+ "40 MHz", /* 010 */
+ "reserved", /* 011 */
+ "25 MHz", /* 100 */
+ "reserved", /* 101 */
+ "17 MHz", /* 110 */
+ "reserved" /* 111 */
} };
switch (cs) {
@@ -251,6 +264,8 @@
case CHIPSET_100_SERIES_SUNRISE_POINT:
case CHIPSET_C620_SERIES_LEWISBURG:
return freq_str[1][value];
+ case CHIPSET_APOLLO_LAKE:
+ return freq_str[2][value];
case CHIPSET_ICH_UNKNOWN:
default:
return "unknown";
@@ -259,11 +274,23 @@
void prettyprint_ich_descriptor_component(enum ich_chipset cs, const struct
ich_descriptors *desc)
{
+ bool has_flill1;
+
+ switch (cs) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ has_flill1 = true;
+ break;
+ default:
+ has_flill1 = false;
+ break;
+ }
msg_pdbg2("=== Component Section ===\n");
msg_pdbg2("FLCOMP 0x%08x\n", desc->component.FLCOMP);
msg_pdbg2("FLILL 0x%08x\n", desc->component.FLILL );
- if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || cs == CHIPSET_C620_SERIES_LEWISBURG)
+ if (has_flill1)
msg_pdbg2("FLILL1 0x%08x\n", desc->component.FLILL1);
msg_pdbg2("\n");
@@ -296,7 +323,7 @@
msg_pdbg2("Invalid instruction 3: 0x%02x\n",
desc->component.invalid_instr3);
}
- if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || cs == CHIPSET_C620_SERIES_LEWISBURG) {
+ if (has_flill1) {
if (desc->component.FLILL1 != 0) {
has_forbidden_opcode = 1;
msg_pdbg2("Invalid instruction 4: 0x%02x\n",
@@ -318,7 +345,7 @@
static void pprint_freg(const struct ich_desc_region *reg, uint32_t i)
{
static const char *const region_names[] = {
- "Descr.", "BIOS", "ME", "GbE",
"Platf.", "unknown", "BIOS2", "unknown",
+ "Descr.", "BIOS", "ME", "GbE",
"Platf.", "DevExt", "BIOS2", "unknown",
"EC/BMC", "unknown", "IE", "10GbE",
"unknown", "unknown", "unknown", "unknown"
};
if (i >= ARRAY_SIZE(region_names)) {
@@ -415,6 +442,23 @@
desc->master.mstr[i].write & (1 << j) ? 'w' : ' ');
msg_pdbg2("\n");
}
+ } else if (cs == CHIPSET_APOLLO_LAKE) {
+ const char *const master_names[] = { "BIOS", "TXE", };
+ if (nm > ARRAY_SIZE(master_names)) {
+ msg_pdbg2("%s: number of masters too high (%d).\n", __func__,
desc->content.NM);
+ return;
+ }
+
+ msg_pdbg2(" FD IFWI TXE n/a Platf DevExt\n");
+ for (i = 0; i < nm; i++) {
+ size_t j;
+ msg_pdbg2("%-4s", master_names[i]);
+ for (j = 0; j < ich_number_of_regions(cs, &desc->content); j++)
+ msg_pdbg2(" %c%c ",
+ desc->master.mstr[i].read & (1 << j) ? 'r' : ' ',
+ desc->master.mstr[i].write & (1 << j) ? 'w' : ' ');
+ msg_pdbg2("\n");
+ }
} else {
const struct ich_desc_master *const mstr = &desc->master;
msg_pdbg2(" Descr. BIOS ME GbE Platf.\n");
@@ -857,6 +901,11 @@
return CHIPSET_ICH10;
else if (content->ISL <= 16)
return CHIPSET_5_SERIES_IBEX_PEAK;
+ else if (content->FLMAP2 == 0) {
+ if (content->ISL != 19)
+ msg_pwarn("Peculiar firmware descriptor, assuming Apollo Lake
compatibility.\n");
+ return CHIPSET_APOLLO_LAKE;
+ }
msg_pwarn("Peculiar firmware descriptor, assuming Ibex Peak
compatibility.\n");
return CHIPSET_5_SERIES_IBEX_PEAK;
} else if (content->ICCRIBA < 0x31 && content->FMSBA < 0x30) {
@@ -886,8 +935,20 @@
{
const enum ich_chipset guess = guess_ich_chipset_from_content(content);
- if (component->modes.freq_read == 6) {
- if ((guess != CHIPSET_100_SERIES_SUNRISE_POINT) && (guess !=
CHIPSET_C620_SERIES_LEWISBURG)) {
+ switch (guess) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ if (component->modes.freq_read != 6) {
+ msg_pwarn("\nThe firmware descriptor looks like a Skylake/Sunrise Point
descriptor.\n"
+ "However, the read frequency isn't set to 17MHz (the only valid
value).\n"
+ "Please report this message, the output of `ich_descriptors_tool` for\n"
+ "your descriptor and the output of `lspci -nn` to
flashrom(a)flashrom.org\n\n")uot;);
+ return CHIPSET_9_SERIES_WILDCAT_POINT;
+ }
+ return guess;
+ default:
+ if (component->modes.freq_read == 6) {
msg_pwarn("\nThe firmware descriptor has the read frequency set to 17MHz.
However,\n"
"it doesn't look like a Skylake/Sunrise Point compatible
descriptor.\n"
"Please report this message, the output of `ich_descriptors_tool` for\n"
@@ -895,17 +956,7 @@
return CHIPSET_100_SERIES_SUNRISE_POINT;
}
return guess;
- } else {
- if (guess == CHIPSET_100_SERIES_SUNRISE_POINT || guess ==
CHIPSET_C620_SERIES_LEWISBURG) {
- msg_pwarn("\nThe firmware descriptor looks like a Skylake/Sunrise Point
descriptor.\n"
- "However, the read frequency isn't set to 17MHz (the only valid
value).\n"
- "Please report this message, the output of `ich_descriptors_tool` for\n"
- "your descriptor and the output of `lspci -nn` to
flashrom(a)flashrom.org\n\n")uot;);
- return CHIPSET_9_SERIES_WILDCAT_POINT;
- }
}
-
- return guess;
}
/* len is the length of dump in bytes */
@@ -1036,6 +1087,7 @@
case CHIPSET_9_SERIES_WILDCAT_POINT_LP:
case CHIPSET_100_SERIES_SUNRISE_POINT:
case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
if (idx == 0) {
size_enc = desc->component.dens_new.comp1_density;
} else {
@@ -1066,10 +1118,13 @@
uint32_t control = 0;
control |= (section << FDOC_FDSS_OFF) & FDOC_FDSS;
control |= (offset << FDOC_FDSI_OFF) & FDOC_FDSI;
- if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || cs == CHIPSET_C620_SERIES_LEWISBURG) {
+ switch (cs) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
mmio_le_writel(control, spibar + PCH100_REG_FDOC);
return mmio_le_readl(spibar + PCH100_REG_FDOD);
- } else {
+ default:
mmio_le_writel(control, spibar + ICH9_REG_FDOC);
return mmio_le_readl(spibar + ICH9_REG_FDOD);
}
diff --git a/ichspi.c b/ichspi.c
index 9eb24a9..2c8a89f 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -29,6 +29,9 @@
#include "spi.h"
#include "ich_descriptors.h"
+/* Apollo Lake */
+#define APL_REG_FREG12 0xe0 /* 32 Bytes Flash Region 12 */
+
/* Sunrise Point */
/* Added HSFS Status bits */
@@ -1564,15 +1567,17 @@
static enum ich_access_protection ich9_handle_frap(uint32_t frap, int i)
{
const int rwperms_unknown = ARRAY_SIZE(access_names);
- static const char *const region_names[5] = {
+ static const char *const region_names[6] = {
"Flash Descriptor", "BIOS", "Management Engine",
- "Gigabit Ethernet", "Platform Data"
+ "Gigabit Ethernet", "Platform Data", "Device Extension",
};
const char *const region_name = i < ARRAY_SIZE(region_names) ? region_names[i] :
"unknown";
uint32_t base, limit;
int rwperms;
- int offset = ICH9_REG_FREG0 + i * 4;
+ const int offset = i < 12
+ ? ICH9_REG_FREG0 + i * 4
+ : APL_REG_FREG12 + (i - 12) * 4;
uint32_t freg = mmio_readl(ich_spibar + offset);
if (i < 8) {
@@ -1716,8 +1721,10 @@
ich_spibar = spibar;
/* Moving registers / bits */
- if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT) {
- num_freg = 10;
+ switch (ich_generation) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
num_pr = 6; /* Includes GPR0 */
reg_pr0 = PCH100_REG_FPR0;
swseq_data.reg_ssfsc = PCH100_REG_SSFSC;
@@ -1727,19 +1734,8 @@
hwseq_data.addr_mask = PCH100_FADDR_FLA;
hwseq_data.only_4k = true;
hwseq_data.hsfc_fcycle = PCH100_HSFC_FCYCLE;
- } else if (ich_generation == CHIPSET_C620_SERIES_LEWISBURG) {
- num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */
- num_pr = 6; /* Includes GPR0 */
- reg_pr0 = PCH100_REG_FPR0;
- swseq_data.reg_ssfsc = PCH100_REG_SSFSC;
- swseq_data.reg_preop = PCH100_REG_PREOP;
- swseq_data.reg_optype = PCH100_REG_OPTYPE;
- swseq_data.reg_opmenu = PCH100_REG_OPMENU;
- hwseq_data.addr_mask = PCH100_FADDR_FLA;
- hwseq_data.only_4k = true;
- hwseq_data.hsfc_fcycle = PCH100_HSFC_FCYCLE;
- } else {
- num_freg = 5;
+ break;
+ default:
num_pr = 5;
reg_pr0 = ICH9_REG_PR0;
swseq_data.reg_ssfsc = ICH9_REG_SSFS;
@@ -1749,6 +1745,21 @@
hwseq_data.addr_mask = ICH9_FADDR_FLA;
hwseq_data.only_4k = false;
hwseq_data.hsfc_fcycle = HSFC_FCYCLE;
+ break;
+ }
+ switch (ich_generation) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ num_freg = 10;
+ break;
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */
+ break;
+ case CHIPSET_APOLLO_LAKE:
+ num_freg = 16;
+ break;
+ default:
+ num_freg = 5;
+ break;
}
switch (ich_generation) {
@@ -1834,10 +1845,16 @@
tmp = mmio_readl(ich_spibar + ICH9_REG_FADDR);
msg_pdbg2("0x08: 0x%08x (FADDR)\n", tmp);
- if (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT || ich_gen ==
CHIPSET_C620_SERIES_LEWISBURG) {
- const uint32_t dlock = mmio_readl(ich_spibar + PCH100_REG_DLOCK);
- msg_pdbg("0x0c: 0x%08x (DLOCK)\n", dlock);
- prettyprint_pch100_reg_dlock(dlock);
+ switch (ich_gen) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ tmp = mmio_readl(ich_spibar + PCH100_REG_DLOCK);
+ msg_pdbg("0x0c: 0x%08x (DLOCK)\n", tmp);
+ prettyprint_pch100_reg_dlock(tmp);
+ break;
+ default:
+ break;
}
if (desc_valid) {
@@ -1898,37 +1915,51 @@
swseq_data.reg_opmenu, mmio_readl(ich_spibar + swseq_data.reg_opmenu));
msg_pdbg("0x%zx: 0x%08x (OPMENU+4)\n",
swseq_data.reg_opmenu + 4, mmio_readl(ich_spibar + swseq_data.reg_opmenu + 4));
- if (ich_generation == CHIPSET_ICH8 && desc_valid) {
- tmp = mmio_readl(ich_spibar + ICH8_REG_VSCC);
- msg_pdbg("0xC1: 0x%08x (VSCC)\n", tmp);
- msg_pdbg("VSCC: ");
- prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true);
- } else if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT &&
- ich_generation != CHIPSET_C620_SERIES_LEWISBURG) {
- if (ich_generation != CHIPSET_BAYTRAIL && desc_valid) {
+
+ if (desc_valid) {
+ switch (ich_gen) {
+ case CHIPSET_ICH8:
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ case CHIPSET_BAYTRAIL:
+ break;
+ default:
ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR);
- msg_pdbg("0xA0: 0x%08x (BBAR)\n",
- ichspi_bbar);
+ msg_pdbg("0x%x: 0x%08x (BBAR)\n", ICH9_REG_BBAR, ichspi_bbar);
ich_set_bbar(0);
+ break;
}
- if (desc_valid) {
+ if (ich_gen == CHIPSET_ICH8) {
+ tmp = mmio_readl(ich_spibar + ICH8_REG_VSCC);
+ msg_pdbg("0x%x: 0x%08x (VSCC)\n", ICH8_REG_VSCC, tmp);
+ msg_pdbg("VSCC: ");
+ prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true);
+ } else {
tmp = mmio_readl(ich_spibar + ICH9_REG_LVSCC);
- msg_pdbg("0xC4: 0x%08x (LVSCC)\n", tmp);
+ msg_pdbg("0x%x: 0x%08x (LVSCC)\n", ICH9_REG_LVSCC, tmp);
msg_pdbg("LVSCC: ");
prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true);
tmp = mmio_readl(ich_spibar + ICH9_REG_UVSCC);
- msg_pdbg("0xC8: 0x%08x (UVSCC)\n", tmp);
+ msg_pdbg("0x%x: 0x%08x (UVSCC)\n", ICH9_REG_UVSCC, tmp);
msg_pdbg("UVSCC: ");
prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, false);
-
- tmp = mmio_readl(ich_spibar + ICH9_REG_FPB);
- msg_pdbg("0xD0: 0x%08x (FPB)\n", tmp);
}
- }
- if (desc_valid) {
+ switch (ich_gen) {
+ case CHIPSET_ICH8:
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ break;
+ default:
+ tmp = mmio_readl(ich_spibar + ICH9_REG_FPB);
+ msg_pdbg("0x%x: 0x%08x (FPB)\n", ICH9_REG_FPB, tmp);
+ break;
+ }
+
if (read_ich_descriptors_via_fdo(ich_gen, ich_spibar, &desc) == ICH_RET_OK)
prettyprint_ich_descriptors(ich_gen, &desc);
@@ -1955,6 +1986,11 @@
ich_spi_mode = ich_hwseq;
}
+ if (ich_spi_mode == ich_auto && ich_gen == CHIPSET_APOLLO_LAKE) {
+ msg_pdbg("Enabling hardware sequencing by default for Apollo Lake.\n");
+ ich_spi_mode = ich_hwseq;
+ }
+
if (ich_spi_mode == ich_hwseq) {
if (!desc_valid) {
msg_perr("Hardware sequencing was requested "
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: newchange
19 Jan
19 Jan
5:42 a.m.
Nico Huber has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c
File ich_descriptors.c:
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@348
PS1, Line 348: "Descr.", "BIOS", "ME", "GbE",
"Platf.", "DevExt", "BIOS2", "unknown",
Dev*Exp*
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@452
PS1, Line 452: msg_pdbg2(" FD IFWI TXE n/a Platf DevExt\n");
Dev*Exp*
https://review.coreboot.org/#/c/30995/1/ichspi.c
File ichspi.c:
https://review.coreboot.org/#/c/30995/1/ichspi.c@1572
PS1, Line 1572: "Gigabit Ethernet", "Platform Data", "Device
Extension",
Device *Expansion*
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sat, 19 Jan 2019 11:42:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
22 Jan
22 Jan
6:20 a.m.
Thomas Heijligen has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c
File ich_descriptors.c:
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@244
PS1, Line 244: 7
This should be correct. But please check this value again. My documentation says something
else. It might be a typo in the docs.
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@348
PS1, Line 348: ME
On APL this is not the place for the ME/TXE image. Only for the TXE ROM Bypass in pre
production
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@452
PS1, Line 452: TXE
This is only the TXE ROM Bypass for pre production not the whole TXE
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 22 Jan 2019 12:20:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
7:23 a.m.
Paul Menzel has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 22 Jan 2019 13:23:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
7:40 a.m.
Thomas Heijligen has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30995/1/ichspi.c
File ichspi.c:
https://review.coreboot.org/#/c/30995/1/ichspi.c@1758
PS1, Line 1758: num_freg = 16
Is this BIOS_FREGx from APL Datasheet 3 (334819-001)? There are only BIOS_FREG0 ...
BIOS_FREG11
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 22 Jan 2019 13:40:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
24 Jan
24 Jan
7:04 a.m.
Nico Huber has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c
File ich_descriptors.c:
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@244
PS1, Line 244: 7
This should be correct. But please check this value
again. My documentation says something else. […]
Ah, it's 17MHz for the read
speed, 14MHz for others. It's just an
informational string, should we make it "14 MHz / 17 MHz"?
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@348
PS1, Line 348: ME
On APL this is not the place for the ME/TXE image. […]
I don't think it makes it too confusing? We could make a distinction
by chipset, but is it worth the effort?
Naming it "ME/TXE" maybe? I'd say it should suffice to know that it
is ME/TXE related.
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@452
PS1, Line 452: TXE
This is only the TXE ROM Bypass for pre production not
the whole TXE
That's right, though, does it matter that much here?
https://review.coreboot.org/#/c/30995/1/ichspi.c
File ichspi.c:
https://review.coreboot.org/#/c/30995/1/ichspi.c@1758
PS1, Line 1758: num_freg = 16
Is this BIOS_FREGx from APL Datasheet 3 (334819-001)?
There are only BIOS_FREG0 ... […]
You might have to scroll down, there is another
set at 0xe0.
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 24 Jan 2019 13:04:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
1:13 p.m.
Thomas Heijligen has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c
File ich_descriptors.c:
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@348
PS1, Line 348: ME
I don't think it makes it too confusing? We could
make a distinction […]
On APL this region should be empty on production devices. The
TXE is inside the IFWI. When define this region just as ME/TXE it would say "put the
whole TXE here"
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 24 Jan 2019 19:13:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
27 Jan
27 Jan
2:33 a.m.
Angel Pons has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c
File ich_descriptors.c:
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@348
PS1, Line 348: ME
On APL this region should be empty on production
devices. The TXE is inside the IFWI. […]
Maybe name it ROMB on APL? But again,
APL-specific naming seems to be not trivial to implement.
And anyone who is working with APL stuff should bear this in mind anyway, so I don't
think leaving the name as ME/TXE is too much of an issue.
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 27 Jan 2019 08:33:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
28 Jan
28 Jan
4:01 a.m.
Thomas Heijligen has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c
File ich_descriptors.c:
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@348
PS1, Line 348: ME
Maybe name it ROMB on APL? But again, APL-specific
naming seems to be not trivial to implement. […]
Somewhere should be a hint that APL
differs from other platforms. It must not be here. Maybe on the APL specific string in
prettyprint_ich_descriptor_master()
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@452
PS1, Line 452: TXE
That's right, though, does it matter that much
here?
This string is APL specific. Here we could say TXE(ROM Bypass) to point out
that APL differs form the rest
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 28 Jan 2019 10:01:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
4:09 a.m.
Thomas Heijligen has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c
File ich_descriptors.c:
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@244
PS1, Line 244: 7
Ah, it's 17MHz for the read speed, 14MHz for
others. It's just an […]
I don't know if it matters. "14 MHz / 17
MHz" may be good.
https://review.coreboot.org/#/c/30995/1/ichspi.c
File ichspi.c:
https://review.coreboot.org/#/c/30995/1/ichspi.c@1758
PS1, Line 1758: num_freg = 16
You might have to scroll down, there is another set at
0xe0.
found it.
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 28 Jan 2019 10:09:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
6 Jun
6 Jun
9:19 a.m.
Hello Angel Pons, Thomas Heijligen, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/30995
to look at the new patch set (#2).
Change subject: ichspi: Add Apollo Lake support
......................................................................
ichspi: Add Apollo Lake support
It's almost identical to 100 series PCHs and later. There are some
addition FREGs (12..15). To not clutter the `if` conditions further,
make more use of `switch` statements.
Tested on Kontron mAL10. Mark it as DEP as usually the last sector
is not covered by the descriptor layout and can't be read.
Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
---
M chipset_enable.c
M ich_descriptors.c
M ichspi.c
3 files changed, 152 insertions(+), 61 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/30995/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
9:19 a.m.
Nico Huber has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Uploaded patch set 2.
(6 comments)
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c
File ich_descriptors.c:
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@244
PS1, Line 244: 7
I don't know if it matters. "14 MHz / 17
MHz" may be good.
Done
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@348
PS1, Line 348: "Descr.", "BIOS", "ME", "GbE",
"Platf.", "DevExt", "BIOS2", "unknown",
Dev*Exp*
Done
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@348
PS1, Line 348: ME
Somewhere should be a hint that APL differs from other
platforms. It must not be here. […]
I've decided to leave it as is (if that can
get approval):
It never was flashrom's purpose to care about the flash
contents. Actually, I think "ME" is most honest. While it
doesn't contain all possible software running on the ME,
it is what the ME might boot from.
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@452
PS1, Line 452: msg_pdbg2(" FD IFWI TXE n/a Platf DevExt\n");
Dev*Exp*
Done
https://review.coreboot.org/#/c/30995/1/ich_descriptors.c@452
PS1, Line 452: TXE
This string is APL specific. […]
These are the
terms used in the "SPI and SMIP Guide", I would like to keep
it consistent.
https://review.coreboot.org/#/c/30995/1/ichspi.c
File ichspi.c:
https://review.coreboot.org/#/c/30995/1/ichspi.c@1572
PS1, Line 1572: "Gigabit Ethernet", "Platform Data", "Device
Extension",
Device *Expansion*
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 2
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 06 Jun 2019 14:19:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
10:52 a.m.
Hello Angel Pons, Thomas Heijligen, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/30995
to look at the new patch set (#3).
Change subject: ichspi: Add Apollo Lake support
......................................................................
ichspi: Add Apollo Lake support
It's almost identical to 100 series PCHs and later. There are some
addition FREGs (12..15). To not clutter the `if` conditions further,
make more use of `switch` statements.
Tested on Kontron mAL10. Mark it as DEP as usually the last sector
is not covered by the descriptor layout and can't be read.
Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
---
M chipset_enable.c
M ich_descriptors.c
M ichspi.c
3 files changed, 153 insertions(+), 62 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/30995/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
7 Jun
7 Jun
7:46 a.m.
Hello Angel Pons, Thomas Heijligen, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/30995
to look at the new patch set (#4).
Change subject: ichspi: Add Apollo Lake support
......................................................................
ichspi: Add Apollo Lake support
It's almost identical to 100 series PCHs and later. There are some
addition FREGs (12..15). To not clutter the `if` conditions further,
make more use of `switch` statements.
Tested on Kontron mAL10. Mark it as DEP as usually the last sector
is not covered by the descriptor layout and can't be read.
Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
---
M chipset_enable.c
M ich_descriptors.c
M ichspi.c
3 files changed, 153 insertions(+), 62 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/30995/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 4
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
13 Jun
13 Jun
8 a.m.
Nico Huber has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 5:
Re-tested after rebase. Please review :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 5
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 13 Jun 2019 13:00:22 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
24 Jun
24 Jun
8:57 a.m.
Paul Menzel has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 5
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 24 Jun 2019 13:57:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
4 Jul
4 Jul
10:29 a.m.
New subject: Change in flashrom[master]: ichspi: Add Apollo Lake support
Hello Angel Pons, Arthur Heymans, Patrick Rudolph, Paul Menzel, David Hendricks, Thomas
Heijligen, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/30995
to look at the new patch set (#6).
Change subject: ichspi: Add Apollo Lake support
......................................................................
ichspi: Add Apollo Lake support
It's almost identical to 100 series PCHs and later. There are some
additional FREGs (12..15). To not clutter the `if` conditions further,
make more use of `switch` statements.
Tested on Kontron mAL10. Mark it as DEP as usually the last sector
is not covered by the descriptor layout and can't be read.
Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
---
M chipset_enable.c
M ich_descriptors.c
M ichspi.c
M util/ich_descriptors_tool/ich_descriptors_tool.c
4 files changed, 157 insertions(+), 63 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/30995/6
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 6
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
10:32 a.m.
New subject: Change in flashrom[master]: ichspi: Add Apollo Lake support
Nico Huber has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 6:
Added missing strings in two places and retested the
tip of this series on SKL, CFL and APL. Flashrom
works fine and logs and descriptor dumps look good.
Also, chipset and descriptor output agree on the flash
layout and permissions ;)
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 6
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 04 Jul 2019 15:32:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
6 Jul
6 Jul
11:45 a.m.
New subject: Change in flashrom[master]: ichspi: Add Apollo Lake support
Angel Pons has posted comments on this change. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
Patch Set 6: Code-Review+2
Looks good
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 6
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sat, 06 Jul 2019 16:45:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
12:23 p.m.
New subject: Change in flashrom[master]: ichspi: Add Apollo Lake support
Nico Huber has submitted this change and it was merged. (
https://review.coreboot.org/c/flashrom/+/30995 )
Change subject: ichspi: Add Apollo Lake support
......................................................................
ichspi: Add Apollo Lake support
It's almost identical to 100 series PCHs and later. There are some
additional FREGs (12..15). To not clutter the `if` conditions further,
make more use of `switch` statements.
Tested on Kontron mAL10. Mark it as DEP as usually the last sector
is not covered by the descriptor layout and can't be read.
Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/30995
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
---
M chipset_enable.c
M ich_descriptors.c
M ichspi.c
M util/ich_descriptors_tool/ich_descriptors_tool.c
4 files changed, 157 insertions(+), 63 deletions(-)
Approvals:
build bot (Jenkins): Verified
Angel Pons: Looks good to me, approved
diff --git a/chipset_enable.c b/chipset_enable.c
index 24ac64a..08feda5 100644
--- a/chipset_enable.c
+++ b/chipset_enable.c
@@ -2026,7 +2026,7 @@
{0x8086, 0xa2c8, B_S, NT, "Intel",
"B250", enable_flash_pch100},
{0x8086, 0xa2c9, B_S, NT, "Intel",
"Z370", enable_flash_pch100},
{0x8086, 0xa2d2, B_S, NT, "Intel",
"X299", enable_flash_pch100},
- {0x8086, 0x5ae8, B_S, BAD, "Intel", "Apollo
Lake", enable_flash_apl},
+ {0x8086, 0x5ae8, B_S, DEP, "Intel", "Apollo
Lake", enable_flash_apl},
#endif
{0},
};
diff --git a/ich_descriptors.c b/ich_descriptors.c
index bfe92bb..a757add 100644
--- a/ich_descriptors.c
+++ b/ich_descriptors.c
@@ -42,6 +42,8 @@
ssize_t ich_number_of_regions(const enum ich_chipset cs, const struct ich_desc_content
*const cont)
{
switch (cs) {
+ case CHIPSET_APOLLO_LAKE:
+ return 6;
case CHIPSET_C620_SERIES_LEWISBURG:
return 16;
case CHIPSET_100_SERIES_SUNRISE_POINT:
@@ -67,6 +69,7 @@
{
switch (cs) {
case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
if (cont->NM <= MAX_NUM_MASTERS)
return cont->NM;
break;
@@ -103,7 +106,7 @@
"5 series Ibex Peak", "6 series Cougar Point", "7 series
Panther Point",
"8 series Lynx Point", "Baytrail", "8 series Lynx Point
LP", "8 series Wellsburg",
"9 series Wildcat Point", "9 series Wildcat Point LP", "100
series Sunrise Point",
- "C620 series Lewisburg",
+ "C620 series Lewisburg", "Apollo Lake",
};
if (cs < CHIPSET_ICH8 || cs - CHIPSET_ICH8 + 1 >= ARRAY_SIZE(chipset_names))
cs = 0;
@@ -140,8 +143,8 @@
msg_pdbg2("FRBA (Flash Region Base Address): 0x%03x\n",
getFRBA(cont));
msg_pdbg2("NC (Number of Components): %5d\n",
cont->NC + 1);
msg_pdbg2("FCBA (Flash Component Base Address): 0x%03x\n",
getFCBA(cont));
- msg_pdbg2("ISL (ICH/PCH Strap Length): %5d\n",
cont->ISL);
- msg_pdbg2("FISBA/FPSBA (Flash ICH/PCH Strap Base Address): 0x%03x\n",
getFISBA(cont));
+ msg_pdbg2("ISL (ICH/PCH/SoC Strap Length): %5d\n",
cont->ISL);
+ msg_pdbg2("FISBA/FPSBA (Flash ICH/PCH/SoC Strap Base Addr): 0x%03x\n",
getFISBA(cont));
msg_pdbg2("NM (Number of Masters): %5zd\n",
ich_number_of_masters(cs, cont));
msg_pdbg2("FMBA (Flash Master Base Address): 0x%03x\n",
getFMBA(cont));
msg_pdbg2("MSL/PSL (MCH/PROC Strap Length): %5d\n",
cont->MSL);
@@ -194,7 +197,8 @@
case CHIPSET_9_SERIES_WILDCAT_POINT:
case CHIPSET_9_SERIES_WILDCAT_POINT_LP:
case CHIPSET_100_SERIES_SUNRISE_POINT:
- case CHIPSET_C620_SERIES_LEWISBURG: {
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE: {
uint8_t size_enc;
if (idx == 0) {
size_enc = desc->component.dens_new.comp1_density;
@@ -213,7 +217,7 @@
static const char *pprint_freq(enum ich_chipset cs, uint8_t value)
{
- static const char *const freq_str[2][8] = { {
+ static const char *const freq_str[3][8] = { {
"20 MHz",
"33 MHz",
"reserved",
@@ -231,6 +235,15 @@
"reserved",
"17 MHz",
"reserved"
+ }, {
+ "reserved",
+ "50 MHz",
+ "40 MHz",
+ "reserved",
+ "25 MHz",
+ "reserved",
+ "14 MHz / 17 MHz",
+ "reserved"
} };
switch (cs) {
@@ -253,6 +266,8 @@
case CHIPSET_100_SERIES_SUNRISE_POINT:
case CHIPSET_C620_SERIES_LEWISBURG:
return freq_str[1][value];
+ case CHIPSET_APOLLO_LAKE:
+ return freq_str[2][value];
case CHIPSET_ICH_UNKNOWN:
default:
return "unknown";
@@ -261,11 +276,23 @@
void prettyprint_ich_descriptor_component(enum ich_chipset cs, const struct
ich_descriptors *desc)
{
+ bool has_flill1;
+
+ switch (cs) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ has_flill1 = true;
+ break;
+ default:
+ has_flill1 = false;
+ break;
+ }
msg_pdbg2("=== Component Section ===\n");
msg_pdbg2("FLCOMP 0x%08x\n", desc->component.FLCOMP);
msg_pdbg2("FLILL 0x%08x\n", desc->component.FLILL );
- if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || cs == CHIPSET_C620_SERIES_LEWISBURG)
+ if (has_flill1)
msg_pdbg2("FLILL1 0x%08x\n", desc->component.FLILL1);
msg_pdbg2("\n");
@@ -298,7 +325,7 @@
msg_pdbg2("Invalid instruction 3: 0x%02x\n",
desc->component.invalid_instr3);
}
- if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || cs == CHIPSET_C620_SERIES_LEWISBURG) {
+ if (has_flill1) {
if (desc->component.FLILL1 != 0) {
has_forbidden_opcode = 1;
msg_pdbg2("Invalid instruction 4: 0x%02x\n",
@@ -320,7 +347,7 @@
static void pprint_freg(const struct ich_desc_region *reg, uint32_t i)
{
static const char *const region_names[] = {
- "Descr.", "BIOS", "ME", "GbE",
"Platf.", "unknown", "BIOS2", "unknown",
+ "Descr.", "BIOS", "ME", "GbE",
"Platf.", "DevExp", "BIOS2", "unknown",
"EC/BMC", "unknown", "IE", "10GbE",
"unknown", "unknown", "unknown", "unknown"
};
if (i >= ARRAY_SIZE(region_names)) {
@@ -417,6 +444,23 @@
desc->master.mstr[i].write & (1 << j) ? 'w' : ' ');
msg_pdbg2("\n");
}
+ } else if (cs == CHIPSET_APOLLO_LAKE) {
+ const char *const master_names[] = { "BIOS", "TXE", };
+ if (nm > ARRAY_SIZE(master_names)) {
+ msg_pdbg2("%s: number of masters too high (%d).\n", __func__,
desc->content.NM);
+ return;
+ }
+
+ msg_pdbg2(" FD IFWI TXE n/a Platf DevExp\n");
+ for (i = 0; i < nm; i++) {
+ size_t j;
+ msg_pdbg2("%-4s", master_names[i]);
+ for (j = 0; j < ich_number_of_regions(cs, &desc->content); j++)
+ msg_pdbg2(" %c%c ",
+ desc->master.mstr[i].read & (1 << j) ? 'r' : ' ',
+ desc->master.mstr[i].write & (1 << j) ? 'w' : ' ');
+ msg_pdbg2("\n");
+ }
} else {
const struct ich_desc_master *const mstr = &desc->master;
msg_pdbg2(" Descr. BIOS ME GbE Platf.\n");
@@ -859,6 +903,11 @@
return CHIPSET_ICH10;
else if (content->ISL <= 16)
return CHIPSET_5_SERIES_IBEX_PEAK;
+ else if (content->FLMAP2 == 0) {
+ if (content->ISL != 19)
+ msg_pwarn("Peculiar firmware descriptor, assuming Apollo Lake
compatibility.\n");
+ return CHIPSET_APOLLO_LAKE;
+ }
msg_pwarn("Peculiar firmware descriptor, assuming Ibex Peak
compatibility.\n");
return CHIPSET_5_SERIES_IBEX_PEAK;
} else if (content->ICCRIBA < 0x31 && content->FMSBA < 0x30) {
@@ -888,8 +937,20 @@
{
const enum ich_chipset guess = guess_ich_chipset_from_content(content);
- if (component->modes.freq_read == 6) {
- if ((guess != CHIPSET_100_SERIES_SUNRISE_POINT) && (guess !=
CHIPSET_C620_SERIES_LEWISBURG)) {
+ switch (guess) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ if (component->modes.freq_read != 6) {
+ msg_pwarn("\nThe firmware descriptor looks like a Skylake/Sunrise Point
descriptor.\n"
+ "However, the read frequency isn't set to 17MHz (the only valid
value).\n"
+ "Please report this message, the output of `ich_descriptors_tool` for\n"
+ "your descriptor and the output of `lspci -nn` to
flashrom(a)flashrom.org\n\n")uot;);
+ return CHIPSET_9_SERIES_WILDCAT_POINT;
+ }
+ return guess;
+ default:
+ if (component->modes.freq_read == 6) {
msg_pwarn("\nThe firmware descriptor has the read frequency set to 17MHz.
However,\n"
"it doesn't look like a Skylake/Sunrise Point compatible
descriptor.\n"
"Please report this message, the output of `ich_descriptors_tool` for\n"
@@ -897,17 +958,7 @@
return CHIPSET_100_SERIES_SUNRISE_POINT;
}
return guess;
- } else {
- if (guess == CHIPSET_100_SERIES_SUNRISE_POINT || guess ==
CHIPSET_C620_SERIES_LEWISBURG) {
- msg_pwarn("\nThe firmware descriptor looks like a Skylake/Sunrise Point
descriptor.\n"
- "However, the read frequency isn't set to 17MHz (the only valid
value).\n"
- "Please report this message, the output of `ich_descriptors_tool` for\n"
- "your descriptor and the output of `lspci -nn` to
flashrom(a)flashrom.org\n\n")uot;);
- return CHIPSET_9_SERIES_WILDCAT_POINT;
- }
}
-
- return guess;
}
/* len is the length of dump in bytes */
@@ -1038,6 +1089,7 @@
case CHIPSET_9_SERIES_WILDCAT_POINT_LP:
case CHIPSET_100_SERIES_SUNRISE_POINT:
case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
if (idx == 0) {
size_enc = desc->component.dens_new.comp1_density;
} else {
@@ -1068,10 +1120,13 @@
uint32_t control = 0;
control |= (section << FDOC_FDSS_OFF) & FDOC_FDSS;
control |= (offset << FDOC_FDSI_OFF) & FDOC_FDSI;
- if (cs == CHIPSET_100_SERIES_SUNRISE_POINT || cs == CHIPSET_C620_SERIES_LEWISBURG) {
+ switch (cs) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
mmio_le_writel(control, spibar + PCH100_REG_FDOC);
return mmio_le_readl(spibar + PCH100_REG_FDOD);
- } else {
+ default:
mmio_le_writel(control, spibar + ICH9_REG_FDOC);
return mmio_le_readl(spibar + ICH9_REG_FDOD);
}
diff --git a/ichspi.c b/ichspi.c
index 6601040..e1ede60 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -29,6 +29,9 @@
#include "spi.h"
#include "ich_descriptors.h"
+/* Apollo Lake */
+#define APL_REG_FREG12 0xe0 /* 32 Bytes Flash Region 12 */
+
/* Sunrise Point */
/* Added HSFS Status bits */
@@ -1564,15 +1567,17 @@
static enum ich_access_protection ich9_handle_frap(uint32_t frap, int i)
{
const int rwperms_unknown = ARRAY_SIZE(access_names);
- static const char *const region_names[5] = {
+ static const char *const region_names[6] = {
"Flash Descriptor", "BIOS", "Management Engine",
- "Gigabit Ethernet", "Platform Data"
+ "Gigabit Ethernet", "Platform Data", "Device Expansion",
};
const char *const region_name = i < ARRAY_SIZE(region_names) ? region_names[i] :
"unknown";
uint32_t base, limit;
int rwperms;
- int offset = ICH9_REG_FREG0 + i * 4;
+ const int offset = i < 12
+ ? ICH9_REG_FREG0 + i * 4
+ : APL_REG_FREG12 + (i - 12) * 4;
uint32_t freg = mmio_readl(ich_spibar + offset);
if (i < 8) {
@@ -1716,8 +1721,10 @@
memset(&desc, 0x00, sizeof(struct ich_descriptors));
/* Moving registers / bits */
- if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT) {
- num_freg = 10;
+ switch (ich_generation) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
num_pr = 6; /* Includes GPR0 */
reg_pr0 = PCH100_REG_FPR0;
swseq_data.reg_ssfsc = PCH100_REG_SSFSC;
@@ -1727,19 +1734,8 @@
hwseq_data.addr_mask = PCH100_FADDR_FLA;
hwseq_data.only_4k = true;
hwseq_data.hsfc_fcycle = PCH100_HSFC_FCYCLE;
- } else if (ich_generation == CHIPSET_C620_SERIES_LEWISBURG) {
- num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */
- num_pr = 6; /* Includes GPR0 */
- reg_pr0 = PCH100_REG_FPR0;
- swseq_data.reg_ssfsc = PCH100_REG_SSFSC;
- swseq_data.reg_preop = PCH100_REG_PREOP;
- swseq_data.reg_optype = PCH100_REG_OPTYPE;
- swseq_data.reg_opmenu = PCH100_REG_OPMENU;
- hwseq_data.addr_mask = PCH100_FADDR_FLA;
- hwseq_data.only_4k = true;
- hwseq_data.hsfc_fcycle = PCH100_HSFC_FCYCLE;
- } else {
- num_freg = 5;
+ break;
+ default:
num_pr = 5;
reg_pr0 = ICH9_REG_PR0;
swseq_data.reg_ssfsc = ICH9_REG_SSFS;
@@ -1749,6 +1745,21 @@
hwseq_data.addr_mask = ICH9_FADDR_FLA;
hwseq_data.only_4k = false;
hwseq_data.hsfc_fcycle = HSFC_FCYCLE;
+ break;
+ }
+ switch (ich_generation) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ num_freg = 10;
+ break;
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */
+ break;
+ case CHIPSET_APOLLO_LAKE:
+ num_freg = 16;
+ break;
+ default:
+ num_freg = 5;
+ break;
}
switch (ich_generation) {
@@ -1834,10 +1845,16 @@
tmp = mmio_readl(ich_spibar + ICH9_REG_FADDR);
msg_pdbg2("0x08: 0x%08x (FADDR)\n", tmp);
- if (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT || ich_gen ==
CHIPSET_C620_SERIES_LEWISBURG) {
- const uint32_t dlock = mmio_readl(ich_spibar + PCH100_REG_DLOCK);
- msg_pdbg("0x0c: 0x%08x (DLOCK)\n", dlock);
- prettyprint_pch100_reg_dlock(dlock);
+ switch (ich_gen) {
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ tmp = mmio_readl(ich_spibar + PCH100_REG_DLOCK);
+ msg_pdbg("0x0c: 0x%08x (DLOCK)\n", tmp);
+ prettyprint_pch100_reg_dlock(tmp);
+ break;
+ default:
+ break;
}
if (desc_valid) {
@@ -1898,37 +1915,51 @@
swseq_data.reg_opmenu, mmio_readl(ich_spibar + swseq_data.reg_opmenu));
msg_pdbg("0x%zx: 0x%08x (OPMENU+4)\n",
swseq_data.reg_opmenu + 4, mmio_readl(ich_spibar + swseq_data.reg_opmenu + 4));
- if (ich_generation == CHIPSET_ICH8 && desc_valid) {
- tmp = mmio_readl(ich_spibar + ICH8_REG_VSCC);
- msg_pdbg("0xC1: 0x%08x (VSCC)\n", tmp);
- msg_pdbg("VSCC: ");
- prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true);
- } else if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT &&
- ich_generation != CHIPSET_C620_SERIES_LEWISBURG) {
- if (ich_generation != CHIPSET_BAYTRAIL && desc_valid) {
+
+ if (desc_valid) {
+ switch (ich_gen) {
+ case CHIPSET_ICH8:
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ case CHIPSET_BAYTRAIL:
+ break;
+ default:
ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR);
- msg_pdbg("0xA0: 0x%08x (BBAR)\n",
- ichspi_bbar);
+ msg_pdbg("0x%x: 0x%08x (BBAR)\n", ICH9_REG_BBAR, ichspi_bbar);
ich_set_bbar(0);
+ break;
}
- if (desc_valid) {
+ if (ich_gen == CHIPSET_ICH8) {
+ tmp = mmio_readl(ich_spibar + ICH8_REG_VSCC);
+ msg_pdbg("0x%x: 0x%08x (VSCC)\n", ICH8_REG_VSCC, tmp);
+ msg_pdbg("VSCC: ");
+ prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true);
+ } else {
tmp = mmio_readl(ich_spibar + ICH9_REG_LVSCC);
- msg_pdbg("0xC4: 0x%08x (LVSCC)\n", tmp);
+ msg_pdbg("0x%x: 0x%08x (LVSCC)\n", ICH9_REG_LVSCC, tmp);
msg_pdbg("LVSCC: ");
prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true);
tmp = mmio_readl(ich_spibar + ICH9_REG_UVSCC);
- msg_pdbg("0xC8: 0x%08x (UVSCC)\n", tmp);
+ msg_pdbg("0x%x: 0x%08x (UVSCC)\n", ICH9_REG_UVSCC, tmp);
msg_pdbg("UVSCC: ");
prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, false);
-
- tmp = mmio_readl(ich_spibar + ICH9_REG_FPB);
- msg_pdbg("0xD0: 0x%08x (FPB)\n", tmp);
}
- }
- if (desc_valid) {
+ switch (ich_gen) {
+ case CHIPSET_ICH8:
+ case CHIPSET_100_SERIES_SUNRISE_POINT:
+ case CHIPSET_C620_SERIES_LEWISBURG:
+ case CHIPSET_APOLLO_LAKE:
+ break;
+ default:
+ tmp = mmio_readl(ich_spibar + ICH9_REG_FPB);
+ msg_pdbg("0x%x: 0x%08x (FPB)\n", ICH9_REG_FPB, tmp);
+ break;
+ }
+
if (read_ich_descriptors_via_fdo(ich_gen, ich_spibar, &desc) == ICH_RET_OK)
prettyprint_ich_descriptors(ich_gen, &desc);
@@ -1955,6 +1986,11 @@
ich_spi_mode = ich_hwseq;
}
+ if (ich_spi_mode == ich_auto && ich_gen == CHIPSET_APOLLO_LAKE) {
+ msg_pdbg("Enabling hardware sequencing by default for Apollo Lake.\n");
+ ich_spi_mode = ich_hwseq;
+ }
+
if (ich_spi_mode == ich_hwseq) {
if (!desc_valid) {
msg_perr("Hardware sequencing was requested "
diff --git a/util/ich_descriptors_tool/ich_descriptors_tool.c
b/util/ich_descriptors_tool/ich_descriptors_tool.c
index aa857c8..2c7966d 100644
--- a/util/ich_descriptors_tool/ich_descriptors_tool.c
+++ b/util/ich_descriptors_tool/ich_descriptors_tool.c
@@ -126,6 +126,7 @@
"\t- \"ich9\",\n"
"\t- \"ich10\",\n"
"\t- \"silvermont\" for chipsets from Intel's Silvermont architecture
(e.g. Bay Trail),\n"
+"\t- \"apollo\" for Intel's Apollo Lake SoC.\n"
"\t- \"5\" or \"ibex\" for Intel's 5 series
chipsets,\n"
"\t- \"6\" or \"cougar\" for Intel's 6 series
chipsets,\n"
"\t- \"7\" or \"panther\" for Intel's 7 series
chipsets.\n"
@@ -220,6 +221,8 @@
else if ((strcmp(csn, "100") == 0) ||
(strcmp(csn, "sunrise") == 0))
cs = CHIPSET_100_SERIES_SUNRISE_POINT;
+ else if (strcmp(csn, "apollo") == 0)
+ cs = CHIPSET_APOLLO_LAKE;
}
ret = read_ich_descriptors_from_dump(buf, len, &cs, &desc);
--
To view, visit https://review.coreboot.org/c/flashrom/+/30995
To unsubscribe, or for help writing mail filters, visit
https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c464b5b3d151e6d28d5db96495fe874a0a45718
Gerrit-Change-Number: 30995
Gerrit-PatchSet: 8
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
508
days inactive
677
days old
19 comments
4 participants
participants (4)
-
Angel Pons (Code Review) -
Nico Huber (Code Review)
-
Paul Menzel (Code Review)
-
Thomas Heijligen (Code Review)