Change in coreboot[master]: mb/intel/kunimitsu: drop support for FSP 1.1 - coreboot-gerrit

9 Oct 2019

Hello Matt DeVillier,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/35922
to review the following change.
Change subject: mb/intel/kunimitsu: drop support for FSP 1.1
......................................................................
mb/intel/kunimitsu: drop support for FSP 1.1
This patch is part of the patch series to drop support for FSP 1.1 in
soc/intel/skylake.
The following modifications have been done to migrate the board(s) from
FSP 1.1 to FSP 2.0:
- remove deprecated devicetree VR_RING domain (only 4 domains in FSP 2.0)
- add AC/DC loadline values to devicetree (see TODO)
- drop FSP-1.1-only romstage.c and spd.c
- add FSP parameter DmiVc1 (see TODO)
- set FSP board type to "mobile"
TODO:
- check loadline values again
- loadline values in devicetree maybe can dropped again, as Maxim is
  working on adding missing loadline values in CB:35167
- find out why some boards set DmiVc1
- testing
Change-Id: I9d312ac959a7dac4b018d5ca1d007b1347bcf1dd
Signed-off-by: Michael Niewöhner foss@mniewoehner.de
Signed-off-by: Matt DeVillier matt.devillier@gmail.com
---
M src/mainboard/intel/kunimitsu/Kconfig
M src/mainboard/intel/kunimitsu/Makefile.inc
M src/mainboard/intel/kunimitsu/devicetree.cb
D src/mainboard/intel/kunimitsu/romstage.c
M src/mainboard/intel/kunimitsu/romstage_fsp20.c
M src/mainboard/intel/kunimitsu/spd/Makefile.inc
D src/mainboard/intel/kunimitsu/spd/spd.c
M src/mainboard/intel/kunimitsu/spd/spd.h
8 files changed, 67 insertions(+), 208 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/35922/1

diff --git a/src/mainboard/intel/kunimitsu/Kconfig b/src/mainboard/intel/kunimitsu/Kconfig
index 6396151..26cfe96 100644
--- a/src/mainboard/intel/kunimitsu/Kconfig
+++ b/src/mainboard/intel/kunimitsu/Kconfig
@@ -19,20 +19,8 @@
    select MAINBOARD_HAS_CHROMEOS
    select MAINBOARD_HAS_LPC_TPM
    select SOC_INTEL_SKYLAKE
-
-choice
-	prompt "FSP driver"
-	default KUNIMITSU_USES_FSP1_1
-
-config KUNIMITSU_USES_FSP1_1
-	bool "FSP driver 1.1"
-
-config KUNIMITSU_USES_FSP2_0
-	bool "FSP driver 2.0"
    select MAINBOARD_USES_FSP2_0
-endchoice
-
 config VBOOT
    select EC_GOOGLE_CHROMEEC_SWITCHES
    select VBOOT_LID_SWITCH
diff --git a/src/mainboard/intel/kunimitsu/Makefile.inc b/src/mainboard/intel/kunimitsu/Makefile.inc
index 933074b..826c958 100644
--- a/src/mainboard/intel/kunimitsu/Makefile.inc
+++ b/src/mainboard/intel/kunimitsu/Makefile.inc
@@ -30,6 +30,4 @@
smm-y += smihandler.c
-ifeq ($(CONFIG_PLATFORM_USES_FSP2_0),y)
 romstage-srcs := $(subst $(MAINBOARDDIR)/romstage.c,$(MAINBOARDDIR)/romstage_fsp20.c,$(romstage-srcs))
-endif
diff --git a/src/mainboard/intel/kunimitsu/devicetree.cb b/src/mainboard/intel/kunimitsu/devicetree.cb
index f2c9f43..31dfec1 100644
--- a/src/mainboard/intel/kunimitsu/devicetree.cb
+++ b/src/mainboard/intel/kunimitsu/devicetree.cb
@@ -60,82 +60,80 @@
    register "PmConfigSlpAMinAssert" = "0x03"
-	# VR Settings Configuration for 5 Domains
-	#+----------------+-------+-------+-------------+-------------+-------+
-	#| Domain/Setting |  SA   |  IA   | Ring Sliced | GT Unsliced |  GT   |
-	#+----------------+-------+-------+-------------+-------------+-------+
-	#| Psi1Threshold  | 20A   | 20A   | 20A         | 20A         | 20A   |
-	#| Psi2Threshold  | 4A    | 5A    | 5A          | 5A          | 5A    |
-	#| Psi3Threshold  | 1A    | 1A    | 1A          | 1A          | 1A    |
-	#| Psi3Enable     | 1     | 1     | 1           | 1           | 1     |
-	#| Psi4Enable     | 1     | 1     | 1           | 1           | 1     |
-	#| ImonSlope      | 0     | 0     | 0           | 0           | 0     |
-	#| ImonOffset     | 0     | 0     | 0           | 0           | 0     |
-	#| IccMax         | 7A    | 34A   | 34A         | 35A         | 35A   |
-	#| VrVoltageLimit | 1.52V | 1.52V | 1.52V       | 1.52V       | 1.52V |
-	#+----------------+-------+-------+-------------+-------------+-------+
+	# VR Settings Configuration for 4 Domains
+	#+----------------+-----------+-----------+-------------+----------+
+	#| Domain/Setting |     SA    |    IA     | GT Unsliced |    GT    |
+	#+----------------+-----------+-----------+-------------+----------+
+	#| Psi1Threshold  | 20A       | 20A       | 20A         | 20A      |
+	#| Psi2Threshold  | 4A        | 5A        | 5A          | 5A       |
+	#| Psi3Threshold  | 1A        | 1A        | 1A          | 1A       |
+	#| Psi3Enable     | 1         | 1         | 1           | 1        |
+	#| Psi4Enable     | 1         | 1         | 1           | 1        |
+	#| ImonSlope      | 0         | 0         | 0           | 0        |
+	#| ImonOffset     | 0         | 0         | 0           | 0        |
+	#| IccMax         | 7A        | 34A       | 35A         | 35A      |
+	#| VrVoltageLimit | 1.52V     | 1.52V     | 1.52V       | 1.52V    |
+	#| AC LoadLine    | 15 mOhm   | 5.7 mOhm  | 5.2 mOhm    | 5.2 mOhm |
+	#| DC LoadLine    | 14.3 mOhm | 4.83 mOhm | 4.2 mOhm    | 4.2 mOhm |
+	#+----------------+-----------+-----------+-------------+----------+
    register "domain_vr_config[VR_SYSTEM_AGENT]" = "{
-		.vr_config_enable = 1, \
-		.psi1threshold = 0x50, \
-		.psi2threshold = 0x10, \
-		.psi3threshold = 0x4, \
-		.psi3enable = 1, \
-		.psi4enable = 1, \
-		.imon_slope = 0x0, \
-		.imon_offset = 0x0, \
-		.icc_max = 0x1C, \
-		.voltage_limit = 0x5F0 \
+		.vr_config_enable = 1,
+		.psi1threshold = VR_CFG_AMP(20),
+		.psi2threshold = VR_CFG_AMP(4),
+		.psi3threshold = VR_CFG_AMP(1),
+		.psi3enable = 1,
+		.psi4enable = 1,
+		.imon_slope = 0x0,
+		.imon_offset = 0x0,
+		.icc_max = VR_CFG_AMP(7),
+		.voltage_limit = 1520,
+		.ac_loadline = 1500,
+		.dc_loadline = 1430,
    }"
register "domain_vr_config[VR_IA_CORE]" = "{
-		.vr_config_enable = 1, \
-		.psi1threshold = 0x50, \
-		.psi2threshold = 0x14, \
-		.psi3threshold = 0x4, \
-		.psi3enable = 1, \
-		.psi4enable = 1, \
-		.imon_slope = 0x0, \
-		.imon_offset = 0x0, \
-		.icc_max = 0x88, \
-		.voltage_limit = 0x5F0 \
-	}"
-	register "domain_vr_config[VR_RING]" = "{
-		.vr_config_enable = 1, \
-		.psi1threshold = 0x50, \
-		.psi2threshold = 0x14, \
-		.psi3threshold = 0x4, \
-		.psi3enable = 1, \
-		.psi4enable = 1, \
-		.imon_slope = 0x0, \
-		.imon_offset = 0x0, \
-		.icc_max = 0x88, \
-		.voltage_limit = 0x5F0, \
+		.vr_config_enable = 1,
+		.psi1threshold = VR_CFG_AMP(20),
+		.psi2threshold = VR_CFG_AMP(5),
+		.psi3threshold = VR_CFG_AMP(1),
+		.psi3enable = 1,
+		.psi4enable = 1,
+		.imon_slope = 0x0,
+		.imon_offset = 0x0,
+		.icc_max = VR_CFG_AMP(34),
+		.voltage_limit = 1520,
+		.ac_loadline = 570,
+		.dc_loadline = 483,
    }"
register "domain_vr_config[VR_GT_UNSLICED]" = "{
-		.vr_config_enable = 1, \
-		.psi1threshold = 0x50, \
-		.psi2threshold = 0x14, \
-		.psi3threshold = 0x4, \
-		.psi3enable = 1, \
-		.psi4enable = 1, \
-		.imon_slope = 0x0, \
-		.imon_offset = 0x0, \
-		.icc_max = 0x8C ,\
-		.voltage_limit = 0x5F0 \
+		.vr_config_enable = 1,
+		.psi1threshold = VR_CFG_AMP(20),
+		.psi2threshold = VR_CFG_AMP(5),
+		.psi3threshold = VR_CFG_AMP(1),
+		.psi3enable = 1,
+		.psi4enable = 1,
+		.imon_slope = 0x0,
+		.imon_offset = 0x0,
+		.icc_max = VR_CFG_AMP(35),
+		.voltage_limit = 1520,
+		.ac_loadline = 520,
+		.dc_loadline = 420,
    }"
register "domain_vr_config[VR_GT_SLICED]" = "{
-		.vr_config_enable = 1, \
-		.psi1threshold = 0x50, \
-		.psi2threshold = 0x14, \
-		.psi3threshold = 0x4, \
-		.psi3enable = 1, \
-		.psi4enable = 1, \
-		.imon_slope = 0x0, \
-		.imon_offset = 0x0, \
-		.icc_max = 0x8C, \
-		.voltage_limit = 0x5F0 \
+		.vr_config_enable = 1,
+		.psi1threshold = VR_CFG_AMP(20),
+		.psi2threshold = VR_CFG_AMP(5),
+		.psi3threshold = VR_CFG_AMP(1),
+		.psi3enable = 1,
+		.psi4enable = 1,
+		.imon_slope = 0x0,
+		.imon_offset = 0x0,
+		.icc_max = VR_CFG_AMP(35),
+		.voltage_limit = 1520,
+		.ac_loadline = 520,
+		.dc_loadline = 420,
    }"
# Enable Root port 1 and 5.
diff --git a/src/mainboard/intel/kunimitsu/romstage.c b/src/mainboard/intel/kunimitsu/romstage.c
deleted file mode 100644
index 0312ad1..0000000
--- a/src/mainboard/intel/kunimitsu/romstage.c
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * Copyright (C) 2007-2010 coresystems GmbH
- * Copyright (C) 2014 Google Inc.
- * Copyright (C) 2015 Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <gpio.h>
-#include <soc/romstage.h>
-#include "gpio.h"
-#include "spd/spd.h"
-
-void mainboard_memory_init_params(struct romstage_params *params,
-				  MEMORY_INIT_UPD *memory_params)
-{
-	spd_memory_init_params(memory_params);
-	mainboard_fill_dq_map_data(&memory_params->DqByteMapCh0,
-				   &memory_params->DqByteMapCh1);
-	mainboard_fill_dqs_map_data(&memory_params->DqsMapCpu2DramCh0,
-				    &memory_params->DqsMapCpu2DramCh1);
-	mainboard_fill_rcomp_res_data(&memory_params->RcompResistor);
-	mainboard_fill_rcomp_strength_data(&memory_params->RcompTarget);
-	memory_params->MemorySpdDataLen = SPD_LEN;
-	memory_params->DqPinsInterleaved = FALSE;
-}
diff --git a/src/mainboard/intel/kunimitsu/romstage_fsp20.c b/src/mainboard/intel/kunimitsu/romstage_fsp20.c
index 2a4474e..ebecdb2 100644
--- a/src/mainboard/intel/kunimitsu/romstage_fsp20.c
+++ b/src/mainboard/intel/kunimitsu/romstage_fsp20.c
@@ -37,4 +37,7 @@
    if (mainboard_has_dual_channel_mem())
    	mem_cfg->MemorySpdPtr10 = mem_cfg->MemorySpdPtr00;
    mem_cfg->MemorySpdDataLen = SPD_LEN;
+	mem_cfg->UserBd = BOARD_TYPE_MOBILE;
+
+	mupd->FspmTestConfig.DmiVc1 = 1;
 }
diff --git a/src/mainboard/intel/kunimitsu/spd/Makefile.inc b/src/mainboard/intel/kunimitsu/spd/Makefile.inc
index 9856368e..bbcf183 100644
--- a/src/mainboard/intel/kunimitsu/spd/Makefile.inc
+++ b/src/mainboard/intel/kunimitsu/spd/Makefile.inc
@@ -14,7 +14,6 @@
 ## GNU General Public License for more details.
 ##
-romstage-$(CONFIG_PLATFORM_USES_FSP1_1) += spd.c
 romstage-y += spd_util.c
SPD_BIN = $(obj)/spd.bin
diff --git a/src/mainboard/intel/kunimitsu/spd/spd.c b/src/mainboard/intel/kunimitsu/spd/spd.c
deleted file mode 100644
index db5e24e..0000000
--- a/src/mainboard/intel/kunimitsu/spd/spd.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * Copyright (C) 2014 Google Inc.
- * Copyright (C) 2015 Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <console/console.h>
-#include <fsp/soc_binding.h>
-#include <soc/romstage.h>
-#include <stdint.h>
-#include <string.h>
-
-#include "spd.h"
-
-static void mainboard_print_spd_info(uint8_t spd[])
-{
-	const int spd_banks[8] = {  8, 16, 32, 64, -1, -1, -1, -1 };
-	const int spd_capmb[8] = {  1,  2,  4,  8, 16, 32, 64,  0 };
-	const int spd_rows[8]  = { 12, 13, 14, 15, 16, -1, -1, -1 };
-	const int spd_cols[8]  = {  9, 10, 11, 12, -1, -1, -1, -1 };
-	const int spd_ranks[8] = {  1,  2,  3,  4, -1, -1, -1, -1 };
-	const int spd_devw[8]  = {  4,  8, 16, 32, -1, -1, -1, -1 };
-	const int spd_busw[8]  = {  8, 16, 32, 64, -1, -1, -1, -1 };
-	char spd_name[SPD_PART_LEN+1] = { 0 };
-
-	int banks = spd_banks[(spd[SPD_DENSITY_BANKS] >> 4) & 7];
-	int capmb = spd_capmb[spd[SPD_DENSITY_BANKS] & 7] * 256;
-	int rows  = spd_rows[(spd[SPD_ADDRESSING] >> 3) & 7];
-	int cols  = spd_cols[spd[SPD_ADDRESSING] & 7];
-	int ranks = spd_ranks[(spd[SPD_ORGANIZATION] >> 3) & 7];
-	int devw  = spd_devw[spd[SPD_ORGANIZATION] & 7];
-	int busw  = spd_busw[spd[SPD_BUS_DEV_WIDTH] & 7];
-
-	/* Module type */
-	printk(BIOS_INFO, "SPD: module type is ");
-	switch (spd[SPD_DRAM_TYPE]) {
-	case SPD_DRAM_DDR3:
-		printk(BIOS_INFO, "DDR3\n");
-		break;
-	case SPD_DRAM_LPDDR3:
-		printk(BIOS_INFO, "LPDDR3\n");
-		break;
-	default:
-		printk(BIOS_INFO, "Unknown (%02x)\n", spd[SPD_DRAM_TYPE]);
-		break;
-	}
-
-	/* Module Part Number */
-	memcpy(spd_name, &spd[SPD_PART_OFF], SPD_PART_LEN);
-	spd_name[SPD_PART_LEN] = 0;
-	printk(BIOS_INFO, "SPD: module part is %s\n", spd_name);
-
-	printk(BIOS_INFO,
-		"SPD: banks %d, ranks %d, rows %d, columns %d, density %d Mb\n",
-		banks, ranks, rows, cols, capmb);
-	printk(BIOS_INFO, "SPD: device width %d bits, bus width %d bits\n",
-		devw, busw);
-
-	if (capmb > 0 && busw > 0 && devw > 0 && ranks > 0) {
-		/* SIZE = DENSITY / 8 * BUS_WIDTH / SDRAM_WIDTH * RANKS */
-		printk(BIOS_INFO, "SPD: module size is %u MB (per channel)\n",
-		       capmb / 8 * busw / devw * ranks);
-	}
-}
-
-/* Fill SPD pointers for on-board memory */
-void spd_memory_init_params(MEMORY_INIT_UPD *memory_params)
-{
-	uintptr_t spd_data;
-	spd_data = mainboard_get_spd_data();
-
-	/* Make sure a valid SPD was found */
-	if (*(uint8_t *)spd_data == 0)
-		die("Invalid SPD data.");
-
-	memory_params->MemorySpdPtr00 = spd_data;
-	if (mainboard_has_dual_channel_mem())
-		memory_params->MemorySpdPtr10 = spd_data;
-
-	mainboard_print_spd_info((uint8_t *)spd_data);
-}
diff --git a/src/mainboard/intel/kunimitsu/spd/spd.h b/src/mainboard/intel/kunimitsu/spd/spd.h
index 8bc7336..c6e47f0 100644
--- a/src/mainboard/intel/kunimitsu/spd/spd.h
+++ b/src/mainboard/intel/kunimitsu/spd/spd.h
@@ -54,7 +54,6 @@
    };
    return (gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)));
 }
-void spd_memory_init_params(MEMORY_INIT_UPD *memory_params);
 void mainboard_fill_dq_map_data(void *dq_map_ch0, void *dq_map_ch1);
 void mainboard_fill_dqs_map_data(void *dqs_map_ch0, void *dqs_map_ch1);
 void mainboard_fill_rcomp_res_data(void *rcomp_ptr);
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/35922
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9d312ac959a7dac4b018d5ca1d007b1347bcf1dd
Gerrit-Change-Number: 35922
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Martin Roth martinroth@google.com
Gerrit-Reviewer: Matt DeVillier matt.devillier@gmail.com
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com
Gerrit-MessageType: newchange



    