New subject: Change in flashrom[master]: ichspi: Add Apollo Lake support

18 Jan 2019

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&gt;
---
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&quot;);
+			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&quot;);
-			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&gt;
Gerrit-MessageType: newchange



    

Change in ...flashrom[master]: ichspi: Add Apollo Lake support