All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/2] Move FSP-S configuration to device-tree
@ 2020-04-14  9:25 Bernhard Messerklinger
  2020-04-14  9:25 ` [RFC PATCH v2 1/2] arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled Bernhard Messerklinger
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Bernhard Messerklinger @ 2020-04-14  9:25 UTC (permalink / raw)
  To: u-boot

This patch series moves the configuration of FPS-S for Apollo Lake
based SoCs from the code to the device-tree.

This is similar to the previous patch series for FSP-M.
If wanted, I can also send FSP-M and FSP-S patch as a single series.

Changes in v2:
Remove FSP-M binding file

Bernhard Messerklinger (2):
  arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled
  arch: x86: apl: Read FSP-S configuration from device-tree

 arch/x86/cpu/apollolake/fsp_s.c               | 1084 +++++++++++------
 arch/x86/dts/chromebook_coral.dts             |   35 +-
 .../asm/arch-apollolake/fsp/fsp_s_upd.h       |  268 ++++
 .../fsp/fsp2/apollolake/fsp-s.txt             |  485 ++++++++
 4 files changed, 1497 insertions(+), 375 deletions(-)
 create mode 100644 doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt

-- 
2.26.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH v2 1/2] arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled
  2020-04-14  9:25 [RFC PATCH v2 0/2] Move FSP-S configuration to device-tree Bernhard Messerklinger
@ 2020-04-14  9:25 ` Bernhard Messerklinger
  2020-04-19 23:37   ` Simon Glass
  2020-04-14  9:25 ` [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree Bernhard Messerklinger
  2020-04-30  9:28 ` [RFC PATCH v2 0/2] Move FSP-S configuration to device-tree Bernhard Messerklinger
  2 siblings, 1 reply; 10+ messages in thread
From: Bernhard Messerklinger @ 2020-04-14  9:25 UTC (permalink / raw)
  To: u-boot

Only load VBT if it's present in the u-boot.rom.

Signed-off-by: Bernhard Messerklinger <bernhard.messerklinger@br-automation.com>
---

Changes in v2: None

 arch/x86/cpu/apollolake/fsp_s.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/cpu/apollolake/fsp_s.c b/arch/x86/cpu/apollolake/fsp_s.c
index 1f22c1ea3c..458825bc49 100644
--- a/arch/x86/cpu/apollolake/fsp_s.c
+++ b/arch/x86/cpu/apollolake/fsp_s.c
@@ -327,16 +327,17 @@ int fsps_update_config(struct udevice *dev, ulong rom_offset,
 {
 	struct fsp_s_config *cfg = &upd->config;
 	struct apl_config *apl;
+#ifdef CONFIG_HAVE_VBT
 	struct binman_entry vbt;
-	void *buf;
 	int ret;
+	void *vbt_buf;
 
 	ret = binman_entry_find("intel-vbt", &vbt);
 	if (ret)
 		return log_msg_ret("Cannot find VBT", ret);
 	vbt.image_pos += rom_offset;
-	buf = malloc(vbt.size);
-	if (!buf)
+	vbt_buf = malloc(vbt.size);
+	if (!vbt_buf)
 		return log_msg_ret("Alloc VBT", -ENOMEM);
 
 	/*
@@ -344,11 +345,12 @@ int fsps_update_config(struct udevice *dev, ulong rom_offset,
 	 * memory-mapped SPI at present.
 	 */
 	bootstage_start(BOOTSTAGE_ID_ACCUM_MMAP_SPI, "mmap_spi");
-	memcpy(buf, (void *)vbt.image_pos, vbt.size);
+	memcpy(vbt_buf, (void *)vbt.image_pos, vbt.size);
 	bootstage_accum(BOOTSTAGE_ID_ACCUM_MMAP_SPI);
-	if (*(u32 *)buf != VBT_SIGNATURE)
+	if (*(u32 *)vbt_buf != VBT_SIGNATURE)
 		return log_msg_ret("VBT signature", -EINVAL);
-	cfg->graphics_config_ptr = (ulong)buf;
+	cfg->graphics_config_ptr = (ulong)vbt_buf;
+#endif
 
 	apl = malloc(sizeof(*apl));
 	if (!apl)
-- 
2.26.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree
  2020-04-14  9:25 [RFC PATCH v2 0/2] Move FSP-S configuration to device-tree Bernhard Messerklinger
  2020-04-14  9:25 ` [RFC PATCH v2 1/2] arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled Bernhard Messerklinger
@ 2020-04-14  9:25 ` Bernhard Messerklinger
  2020-04-19 23:37   ` Simon Glass
  2020-04-20 13:11   ` Antwort: " Bernhard Messerklinger
  2020-04-30  9:28 ` [RFC PATCH v2 0/2] Move FSP-S configuration to device-tree Bernhard Messerklinger
  2 siblings, 2 replies; 10+ messages in thread
From: Bernhard Messerklinger @ 2020-04-14  9:25 UTC (permalink / raw)
  To: u-boot

Move FSP-S configuration to the device-tree like it's already done for
other SoCs (Baytrail).

Signed-off-by: Bernhard Messerklinger <bernhard.messerklinger@br-automation.com>
---

Changes in v2:
Remove FSP-M binding file

 arch/x86/cpu/apollolake/fsp_s.c               | 1070 +++++++++++------
 arch/x86/dts/chromebook_coral.dts             |   35 +-
 .../asm/arch-apollolake/fsp/fsp_s_upd.h       |  268 +++++
 .../fsp/fsp2/apollolake/fsp-s.txt             |  485 ++++++++
 4 files changed, 1489 insertions(+), 369 deletions(-)
 create mode 100644 doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt

diff --git a/arch/x86/cpu/apollolake/fsp_s.c b/arch/x86/cpu/apollolake/fsp_s.c
index 458825bc49..7d516adc92 100644
--- a/arch/x86/cpu/apollolake/fsp_s.c
+++ b/arch/x86/cpu/apollolake/fsp_s.c
@@ -27,309 +27,90 @@
 #define INTEL_GSPI_MAX		3
 #define MAX_USB2_PORTS		8
 
-enum {
-	CHIPSET_LOCKDOWN_FSP = 0, /* FSP handles locking per UPDs */
-	CHIPSET_LOCKDOWN_COREBOOT, /* coreboot handles locking */
-};
-
-/* Serial IRQ control. SERIRQ_QUIET is the default (0) */
-enum serirq_mode {
-	SERIRQ_QUIET,
-	SERIRQ_CONTINUOUS,
-	SERIRQ_OFF,
-};
-
-struct gspi_cfg {
-	/* Bus speed in MHz */
-	u32 speed_mhz;
-	/* Bus should be enabled prior to ramstage with temporary base */
-	u8 early_init;
-};
-
-/*
- * This structure will hold data required by common blocks.
- * These are soc specific configurations which will be filled by soc.
- * We'll fill this structure once during init and use the data in common block.
- */
-struct soc_intel_common_config {
-	int chipset_lockdown;
-	struct gspi_cfg gspi[INTEL_GSPI_MAX];
-};
-
-enum pnp_settings {
-	PNP_PERF,
-	PNP_POWER,
-	PNP_PERF_POWER,
-};
-
-struct usb2_eye_per_port {
-	u8 per_port_tx_pe_half;
-	u8 per_port_pe_txi_set;
-	u8 per_port_txi_set;
-	u8 hs_skew_sel;
-	u8 usb_tx_emphasis_en;
-	u8 per_port_rxi_set;
-	u8 hs_npre_drv_sel;
-	u8 override_en;
-};
-
-struct apl_config {
-	/* Common structure containing soc config data required by common code*/
-	struct soc_intel_common_config common_soc_config;
-
-	/*
-	 * Mapping from PCIe root port to CLKREQ input on the SOC. The SOC has
-	 * four CLKREQ inputs, but six root ports. Root ports without an
-	 * associated CLKREQ signal must be marked with "CLKREQ_DISABLED"
-	 */
-	u8 pcie_rp_clkreq_pin[MAX_PCIE_PORTS];
-
-	/* Enable/disable hot-plug for root ports (0 = disable, 1 = enable) */
-	u8 pcie_rp_hotplug_enable[MAX_PCIE_PORTS];
-
-	/* De-emphasis enable configuration for each PCIe root port */
-	u8 pcie_rp_deemphasis_enable[MAX_PCIE_PORTS];
-
-	/*
-	 * [14:8] DDR mode Number of dealy elements.Each = 125pSec.
-	 * [6:0] SDR mode Number of dealy elements.Each = 125pSec.
-	 */
-	u32 emmc_tx_cmd_cntl;
-
-	/*
-	 * [14:8] HS400 mode Number of dealy elements.Each = 125pSec.
-	 * [6:0] SDR104/HS200 mode Number of dealy elements.Each = 125pSec.
-	 */
-	u32 emmc_tx_data_cntl1;
-
-	/*
-	 * [30:24] SDR50 mode Number of dealy elements.Each = 125pSec.
-	 * [22:16] DDR50 mode Number of dealy elements.Each = 125pSec.
-	 * [14:8] SDR25/HS50 mode Number of dealy elements.Each = 125pSec.
-	 * [6:0] SDR12/Compatibility mode Number of dealy elements.
-	 *       Each = 125pSec.
-	 */
-	u32 emmc_tx_data_cntl2;
-
-	/*
-	 * [30:24] SDR50 mode Number of dealy elements.Each = 125pSec.
-	 * [22:16] DDR50 mode Number of dealy elements.Each = 125pSec.
-	 * [14:8] SDR25/HS50 mode Number of dealy elements.Each = 125pSec.
-	 * [6:0] SDR12/Compatibility mode Number of dealy elements.
-	 *       Each = 125pSec.
-	 */
-	u32 emmc_rx_cmd_data_cntl1;
-
-	/*
-	 * [14:8] HS400 mode 1 Number of dealy elements.Each = 125pSec.
-	 * [6:0] HS400 mode 2 Number of dealy elements.Each = 125pSec.
-	 */
-	u32 emmc_rx_strobe_cntl;
-
-	/*
-	 * [13:8] Auto Tuning mode Number of dealy elements.Each = 125pSec.
-	 * [6:0] SDR104/HS200 Number of dealy elements.Each = 125pSec.
-	 */
-	u32 emmc_rx_cmd_data_cntl2;
-
-	/* Select the eMMC max speed allowed */
-	u32 emmc_host_max_speed;
-
-	/* Specifies on which IRQ the SCI will internally appear */
-	u32 sci_irq;
-
-	/* Configure serial IRQ (SERIRQ) line */
-	enum serirq_mode serirq_mode;
-
-	/* Configure LPSS S0ix Enable */
-	bool lpss_s0ix_enable;
-
-	/* Enable DPTF support */
-	bool dptf_enable;
-
-	/* TCC activation offset value in degrees Celsius */
-	int tcc_offset;
-
-	/*
-	 * Configure Audio clk gate and power gate
-	 * IOSF-SB port ID 92 offset 0x530 [5] and [3]
-	 */
-	bool hdaudio_clk_gate_enable;
-	bool hdaudio_pwr_gate_enable;
-	bool hdaudio_bios_config_lockdown;
-
-	/* SLP S3 minimum assertion width */
-	int slp_s3_assertion_width_usecs;
-
-	/* GPIO pin for PERST_0 */
-	u32 prt0_gpio;
-
-	/* USB2 eye diagram settings per port */
-	struct usb2_eye_per_port usb2eye[MAX_USB2_PORTS];
-
-	/* GPIO SD card detect pin */
-	unsigned int sdcard_cd_gpio;
-
-	/*
-	 * PRMRR size setting with three options
-	 *  0x02000000 - 32MiB
-	 *  0x04000000 - 64MiB
-	 *  0x08000000 - 128MiB
-	 */
-	u32 PrmrrSize;
-
-	/*
-	 * Enable SGX feature.
-	 * Enabling SGX feature is 2 step process,
-	 * (1) set sgx_enable = 1
-	 * (2) set PrmrrSize to supported size
-	 */
-	bool sgx_enable;
-
-	/*
-	 * Select PNP Settings.
-	 * (0) Performance,
-	 * (1) Power
-	 * (2) Power & Performance
-	 */
-	enum pnp_settings pnp_settings;
-
-	/*
-	 * PMIC PCH_PWROK delay configuration - IPC Configuration
-	 * Upd for changing PCH_PWROK delay configuration : I2C_Slave_Address
-	 * (31:24) + Register_Offset (23:16) + OR Value (15:8) + AND Value (7:0)
-	 */
-	u32 pmic_pmc_ipc_ctrl;
-
-	/*
-	 * Options to disable XHCI Link Compliance Mode. Default is FALSE to not
-	 * disable Compliance Mode. Set TRUE to disable Compliance Mode.
-	 * 0:FALSE(Default), 1:True.
-	 */
-	bool disable_compliance_mode;
-
-	/*
-	 * Options to change USB3 ModPhy setting for the Integrated Filter (IF)
-	 * value. Default is 0 to not changing default IF value (0x12). Set
-	 * value with the range from 0x01 to 0xff to change IF value.
-	 */
-	u32 mod_phy_if_value;
-
-	/*
-	 * Options to bump USB3 LDO voltage. Default is FALSE to not increasing
-	 * LDO voltage. Set TRUE to increase LDO voltage with 40mV.
-	 * 0:FALSE (default), 1:True.
-	 */
-	bool mod_phy_voltage_bump;
-
-	/*
-	 * Options to adjust PMIC Vdd2 voltage. Default is 0 to not adjusting
-	 * the PMIC Vdd2 default voltage 1.20v. Upd for changing Vdd2 Voltage
-	 * configuration: I2C_Slave_Address (31:23) + Register_Offset (23:16)
-	 * + OR Value (15:8) + AND Value (7:0) through BUCK5_VID[3:2]:
-	 * 00=1.10v, 01=1.15v, 10=1.24v, 11=1.20v (default).
-	 */
-	u32 pmic_vdd2_voltage;
-
-	/* Option to enable VTD feature */
-	bool enable_vtd;
-};
-
-static int get_config(struct udevice *dev, struct apl_config *apl)
+#define FSP_I2C_COUNT 8
+#define FSP_HSUART_COUNT 4
+#define FSP_SPI_COUNT 3
+
+const u8 pcie_rp_clk_req_number_def[6] = { 0x4, 0x5, 0x0, 0x1, 0x2, 0x3 };
+const u8 physical_slot_number_def[6] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0x5 };
+const u8 ipc_def[16] = { 0xf8, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+			0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+const u8 port_usb20_per_port_pe_txi_set_def[8] = { 0x07, 0x06, 0x06, 0x06, 0x07,
+						  0x07, 0x07, 0x01 };
+const u8 port_usb20_per_port_txi_set_def[8] = { 0x00, 0x00, 0x00, 0x00, 0x00,
+					       0x00, 0x00, 0x03};
+const u8 port_usb20_hs_skew_sel_def[8] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+					  0x00, 0x01 };
+const u8 port_usb20_i_usb_tx_emphasis_en_def[8] = { 0x03, 0x03, 0x03, 0x03,
+						   0x03, 0x03, 0x03, 0x01 };
+const u8 port_usb20_hs_npre_drv_sel_def[8] = { 0x00, 0x00, 0x00, 0x00, 0x00,
+					      0x00, 0x00, 0x03 };
+
+static int read_u8_array(u8 prop[], ofnode node, const char *propname, int sz,
+			  u8 def)
 {
 	const u8 *ptr;
-	ofnode node;
-	u32 emmc[4];
-	int ret;
-
-	memset(apl, '\0', sizeof(*apl));
-
-	node = dev_read_subnode(dev, "fsp-s");
-	if (!ofnode_valid(node))
-		return log_msg_ret("fsp-s settings", -ENOENT);
 
-	ptr = ofnode_read_u8_array_ptr(node, "pcie-rp-clkreq-pin",
-				       MAX_PCIE_PORTS);
-	if (!ptr)
-		return log_msg_ret("pcie-rp-clkreq-pin", -EINVAL);
-	memcpy(apl->pcie_rp_clkreq_pin, ptr, MAX_PCIE_PORTS);
-
-	ret = ofnode_read_u32(node, "prt0-gpio", &apl->prt0_gpio);
-	if (ret)
-		return log_msg_ret("prt0-gpio", ret);
-	ret = ofnode_read_u32(node, "sdcard-cd-gpio", &apl->sdcard_cd_gpio);
-	if (ret)
-		return log_msg_ret("sdcard-cd-gpio", ret);
-
-	ret = ofnode_read_u32_array(node, "emmc", emmc, ARRAY_SIZE(emmc));
-	if (ret)
-		return log_msg_ret("emmc", ret);
-	apl->emmc_tx_data_cntl1 = emmc[0];
-	apl->emmc_tx_data_cntl2 = emmc[1];
-	apl->emmc_rx_cmd_data_cntl1 = emmc[2];
-	apl->emmc_rx_cmd_data_cntl2 = emmc[3];
-
-	apl->dptf_enable = ofnode_read_bool(node, "dptf-enable");
-
-	apl->hdaudio_clk_gate_enable = ofnode_read_bool(node,
-						"hdaudio-clk-gate-enable");
-	apl->hdaudio_pwr_gate_enable = ofnode_read_bool(node,
-						"hdaudio-pwr-gate-enable");
-	apl->hdaudio_bios_config_lockdown = ofnode_read_bool(node,
-					     "hdaudio-bios-config-lockdown");
-	apl->lpss_s0ix_enable = ofnode_read_bool(node, "lpss-s0ix-enable");
-
-	/* Santa */
-	apl->usb2eye[1].per_port_pe_txi_set = 7;
-	apl->usb2eye[1].per_port_txi_set = 2;
+	ptr = ofnode_read_u8_array_ptr(node, propname, sz);
+	if (ptr) {
+		memcpy(prop, ptr, sz);
+	} else {
+		memset(prop, def, sz);
+		return -EINVAL;
+	}
 
 	return 0;
 }
 
-static void apl_fsp_silicon_init_params_cb(struct apl_config *apl,
-					   struct fsp_s_config *cfg)
+static int read_u16_array(u16 prop[], ofnode node, const char *propname, int sz,
+			   u16 def)
 {
-	u8 port;
-
-	for (port = 0; port < MAX_USB2_PORTS; port++) {
-		if (apl->usb2eye[port].per_port_tx_pe_half)
-			cfg->port_usb20_per_port_tx_pe_half[port] =
-				apl->usb2eye[port].per_port_tx_pe_half;
-
-		if (apl->usb2eye[port].per_port_pe_txi_set)
-			cfg->port_usb20_per_port_pe_txi_set[port] =
-				apl->usb2eye[port].per_port_pe_txi_set;
-
-		if (apl->usb2eye[port].per_port_txi_set)
-			cfg->port_usb20_per_port_txi_set[port] =
-				apl->usb2eye[port].per_port_txi_set;
+	u32 *array_buf;
+	int ret;
 
-		if (apl->usb2eye[port].hs_skew_sel)
-			cfg->port_usb20_hs_skew_sel[port] =
-				apl->usb2eye[port].hs_skew_sel;
+	array_buf = malloc(sz * sizeof(u32));
+	if (!array_buf)
+		return -ENOMEM;
+
+	ret = ofnode_read_u32_array(node, propname, array_buf, sz);
+	if (!ret) {
+		for (int i = 0; i < sz; i++)
+			prop[i] = array_buf[i];
+	} else if (ret == -EINVAL) {
+		for (int i = 0; i < sz; i++)
+			prop[i] = def;
+		ret = 0;
+	}
+	free(array_buf);
 
-		if (apl->usb2eye[port].usb_tx_emphasis_en)
-			cfg->port_usb20_i_usb_tx_emphasis_en[port] =
-				apl->usb2eye[port].usb_tx_emphasis_en;
+	return ret;
+}
 
-		if (apl->usb2eye[port].per_port_rxi_set)
-			cfg->port_usb20_per_port_rxi_set[port] =
-				apl->usb2eye[port].per_port_rxi_set;
+static int read_u32_array(u32 prop[], ofnode node, const char *propname, int sz,
+			   u32 def)
+{
+	int ret;
 
-		if (apl->usb2eye[port].hs_npre_drv_sel)
-			cfg->port_usb20_hs_npre_drv_sel[port] =
-				apl->usb2eye[port].hs_npre_drv_sel;
+	ret = ofnode_read_u32_array(node, propname, prop, sz);
+	if (ret == -EINVAL) {
+		for (int i = 0; i < sz; i++)
+			prop[i] = def;
+		ret = 0;
 	}
+
+	return ret;
 }
 
 int fsps_update_config(struct udevice *dev, ulong rom_offset,
 		       struct fsps_upd *upd)
 {
 	struct fsp_s_config *cfg = &upd->config;
-	struct apl_config *apl;
+	int ret;
+	char buf[32];
+	ofnode node;
+
 #ifdef CONFIG_HAVE_VBT
 	struct binman_entry vbt;
-	int ret;
 	void *vbt_buf;
 
 	ret = binman_entry_find("intel-vbt", &vbt);
@@ -349,89 +130,654 @@ int fsps_update_config(struct udevice *dev, ulong rom_offset,
 	bootstage_accum(BOOTSTAGE_ID_ACCUM_MMAP_SPI);
 	if (*(u32 *)vbt_buf != VBT_SIGNATURE)
 		return log_msg_ret("VBT signature", -EINVAL);
-	cfg->graphics_config_ptr = (ulong)vbt_buf;
 #endif
 
-	apl = malloc(sizeof(*apl));
-	if (!apl)
-		return log_msg_ret("config", -ENOMEM);
-	get_config(dev, apl);
-
-	cfg->ish_enable = 0;
-	cfg->enable_sata = 0;
-	cfg->pcie_root_port_en[2] = 0;
-	cfg->pcie_rp_hot_plug[2] = 0;
-	cfg->pcie_root_port_en[3] = 0;
-	cfg->pcie_rp_hot_plug[3] = 0;
-	cfg->pcie_root_port_en[4] = 0;
-	cfg->pcie_rp_hot_plug[4] = 0;
-	cfg->pcie_root_port_en[5] = 0;
-	cfg->pcie_rp_hot_plug[5] = 0;
-	cfg->pcie_root_port_en[1] = 0;
-	cfg->pcie_rp_hot_plug[1] = 0;
-	cfg->usb_otg = 0;
-	cfg->i2c6_enable = 0;
-	cfg->i2c7_enable = 0;
-	cfg->hsuart3_enable = 0;
-	cfg->spi1_enable = 0;
-	cfg->spi2_enable = 0;
-	cfg->sdio_enabled = 0;
-
-	memcpy(cfg->pcie_rp_clk_req_number, apl->pcie_rp_clkreq_pin,
-	       sizeof(cfg->pcie_rp_clk_req_number));
-
-	memcpy(cfg->pcie_rp_hot_plug, apl->pcie_rp_hotplug_enable,
-	       sizeof(cfg->pcie_rp_hot_plug));
-
-	switch (apl->serirq_mode) {
-	case SERIRQ_QUIET:
-		cfg->sirq_enable = 1;
-		cfg->sirq_mode = 0;
-		break;
-	case SERIRQ_CONTINUOUS:
-		cfg->sirq_enable = 1;
-		cfg->sirq_mode = 1;
-		break;
-	case SERIRQ_OFF:
-	default:
-		cfg->sirq_enable = 0;
-		break;
-	}
+	node = dev_read_subnode(dev, "fsp-s");
+	if (!ofnode_valid(node))
+		return log_msg_ret("fsp-s settings", -ENOENT);
 
-	if (apl->emmc_tx_cmd_cntl)
-		cfg->emmc_tx_cmd_cntl = apl->emmc_tx_cmd_cntl;
-	if (apl->emmc_tx_data_cntl1)
-		cfg->emmc_tx_data_cntl1 = apl->emmc_tx_data_cntl1;
-	if (apl->emmc_tx_data_cntl2)
-		cfg->emmc_tx_data_cntl2 = apl->emmc_tx_data_cntl2;
-	if (apl->emmc_rx_cmd_data_cntl1)
-		cfg->emmc_rx_cmd_data_cntl1 = apl->emmc_rx_cmd_data_cntl1;
-	if (apl->emmc_rx_strobe_cntl)
-		cfg->emmc_rx_strobe_cntl = apl->emmc_rx_strobe_cntl;
-	if (apl->emmc_rx_cmd_data_cntl2)
-		cfg->emmc_rx_cmd_data_cntl2 = apl->emmc_rx_cmd_data_cntl2;
-	if (apl->emmc_host_max_speed)
-		cfg->e_mmc_host_max_speed = apl->emmc_host_max_speed;
-
-	cfg->lpss_s0ix_enable = apl->lpss_s0ix_enable;
-
-	cfg->skip_mp_init = true;
-
-	/* Disable setting of EISS bit in FSP */
-	cfg->spi_eiss = 0;
-
-	/* Disable FSP from locking access to the RTC NVRAM */
-	cfg->rtc_lock = 0;
-
-	/* Enable Audio clk gate and power gate */
-	cfg->hd_audio_clk_gate = apl->hdaudio_clk_gate_enable;
-	cfg->hd_audio_pwr_gate = apl->hdaudio_pwr_gate_enable;
-	/* Bios config lockdown Audio clk and power gate */
-	cfg->bios_cfg_lock_down = apl->hdaudio_bios_config_lockdown;
-	apl_fsp_silicon_init_params_cb(apl, cfg);
-
-	cfg->usb_otg = true;
-	cfg->vtd_enable = apl->enable_vtd;
+	cfg->active_processor_cores = ofnode_read_bool(node,
+					"fsps,enable-active-processor-cores");
+	cfg->disable_core1 = !ofnode_read_bool(node, "fsps,enable-core1");
+	cfg->disable_core2 = !ofnode_read_bool(node, "fsps,enable-core2");
+	cfg->disable_core3 = !ofnode_read_bool(node, "fsps,enable-core3");
+	cfg->vmx_enable = !ofnode_read_bool(node, "fsps,disable-vmx");
+	cfg->proc_trace_mem_size = ofnode_read_u32_default(node,
+						"fsps,proc-trace-mem-size",
+						PROC_TRACE_MEM_SIZE_DISABLE);
+	cfg->proc_trace_enable = ofnode_read_bool(node,
+						  "fsps,enable-proc-trace");
+	cfg->eist = !ofnode_read_bool(node, "fsps,disable-eist");
+	cfg->boot_p_state =  ofnode_read_u32_default(node, "fsps,boot-p-state",
+						     BOOT_P_STATE_HFM);
+	cfg->enable_cx = !ofnode_read_bool(node, "fsps,disable-cx");
+	cfg->c1e = ofnode_read_bool(node, "fsps,enable-c1e");
+	cfg->bi_proc_hot = !ofnode_read_bool(node, "fsps,disable-bi-proc-hot");
+	cfg->pkg_c_state_limit = ofnode_read_u32_default(node,
+						"fsps,pkg-c-state-limit",
+						PKG_C_STATE_LIMIT_C3);
+	cfg->c_state_auto_demotion = ofnode_read_u32_default(node,
+					"fsps,c-state-auto-demotion",
+					C_STATE_AUTO_DEMOTION_DISABLE_C1_C3);
+	cfg->c_state_un_demotion = ofnode_read_u32_default(node,
+					"fsps,c-state-un-demotion",
+					C_STATE_UN_DEMOTION_DISABLE_C1_C3);
+	cfg->max_core_c_state = ofnode_read_u32_default(node,
+					"fsps,max-core-c-state",
+					MAX_CORE_C_STATE_CCX);
+	cfg->pkg_c_state_demotion = ofnode_read_bool(node,
+					"fsps,enable-pkg-c-state-demotion");
+	cfg->pkg_c_state_un_demotion = !ofnode_read_bool(node,
+					"fsps,disable-pkg-c-state-un-demotion");
+	cfg->turbo_mode = !ofnode_read_bool(node, "fsps,disable-turbo-mode");
+	cfg->hda_verb_table_entry_num = ofnode_read_u32_default(node,
+					"fsps,hda-verb-table-entry-num",
+					HDA_VERB_TABLE_ENTRY_NUM_DEFAULT);
+	cfg->hda_verb_table_ptr = ofnode_read_u32_default(node,
+						"fsps,hda-verb-table-ptr",
+						0x00000000);
+	cfg->p2sb_unhide = ofnode_read_bool(node, "fsps,enable-p2sb-unhide");
+	cfg->ipu_en = !ofnode_read_bool(node, "fsps,disable-ipu");
+	cfg->ipu_acpi_mode = ofnode_read_u32_default(node,
+					"fsps,enable-ipu-acpi-mode",
+					IPU_ACPI_MODE_IGFX_CHILD_DEVICE);
+	cfg->force_wake = ofnode_read_bool(node, "fsps,enable-force-wake");
+	cfg->gtt_mm_adr = ofnode_read_u32_default(node, "fsps,gtt-mm-adr",
+						  GTT_MM_ADDRESS_DEFAULT);
+	cfg->gm_adr = ofnode_read_u32_default(node, "fsps,gm-adr",
+					      GM_ADDRESS_DEFAULT);
+	cfg->pavp_lock = ofnode_read_bool(node, "fsps,enable-pavp-lock");
+	cfg->graphics_freq_modify = ofnode_read_bool(node,
+					"fsps,enable-graphics-freq-modify");
+	cfg->graphics_freq_req = ofnode_read_bool(node,
+					"fsps,enable-graphics-freq-req");
+	cfg->graphics_video_freq = ofnode_read_bool(node,
+					"fsps,enable-graphics-video-freq");
+	cfg->pm_lock = ofnode_read_bool(node, "fsps,enable-pm-lock");
+	cfg->dop_clock_gating = ofnode_read_bool(node,
+					"fsps,enable-dop-clock-gating");
+	cfg->unsolicited_attack_override = ofnode_read_bool(node,
+				"fsps,enable-unsolicited-attack-override");
+	cfg->wopcm_support = ofnode_read_bool(node,
+				"fsps,enable-wopcm-support");
+	cfg->wopcm_size = ofnode_read_bool(node, "fsps,enable-wopcm-size");
+	cfg->power_gating = ofnode_read_bool(node, "fsps,enable-power-gating");
+	cfg->unit_level_clock_gating = ofnode_read_bool(node,
+					"fsps,enable-unit-level-clock-gating");
+	cfg->fast_boot = ofnode_read_bool(node, "fsps,enable-fast-boot");
+	cfg->dyn_sr = ofnode_read_bool(node, "fsps,enable-dyn-sr");
+	cfg->sa_ipu_enable = ofnode_read_bool(node,
+					      "fsps,enable-sa-ipu");
+	cfg->pm_support = !ofnode_read_bool(node, "fsps,disable-pm-support");
+	cfg->enable_render_standby = !ofnode_read_bool(node,
+						"fsps,disable-render-standby");
+	cfg->logo_size = ofnode_read_u32_default(node, "fsps,logo-size", 0);
+	cfg->logo_ptr = ofnode_read_u32_default(node, "fsps,logo-ptr", 0);
+#ifdef CONFIG_HAVE_VBT
+	cfg->graphics_config_ptr = (ulong)vbt_buf;
+#else
+	cfg->graphics_config_ptr = 0;
+#endif
+	cfg->pavp_enable = !ofnode_read_bool(node, "fsps,disable-pavp");
+	cfg->pavp_pr3 = !ofnode_read_bool(node, "fsps,disable-pavp-pr3");
+	cfg->cd_clock = ofnode_read_u32_default(node, "fsps,cd-clock",
+						CD_CLOCK_FREQ_624MHZ);
+	cfg->pei_graphics_peim_init = !ofnode_read_bool(node,
+					"fsps,disable-pei-graphics-peim-init");
+	read_u8_array(cfg->write_protection_enable, node,
+		      "fsps,enable-write-protection",
+		      ARRAY_SIZE(cfg->write_protection_enable), 0);
+	read_u8_array(cfg->read_protection_enable, node,
+		      "fsps,enable-read-protection",
+		      ARRAY_SIZE(cfg->read_protection_enable), 0);
+	read_u16_array(cfg->protected_range_limit, node,
+		       "fsps,protected-range-limit",
+		       ARRAY_SIZE(cfg->protected_range_limit),
+		       PROTECTED_RANGE_LIMITATION_DEFAULT);
+	read_u16_array(cfg->protected_range_base, node,
+		       "fsps,protected-range-base",
+		       ARRAY_SIZE(cfg->protected_range_base), 0);
+	cfg->gmm = !ofnode_read_bool(node, "fsps,disable-gmm");
+	cfg->clk_gating_pgcb_clk_trunk = !ofnode_read_bool(node,
+				"fsps,disable-clk-gating-pgcb-clk-trunk");
+	cfg->clk_gating_sb = !ofnode_read_bool(node,
+					       "fsps,disable-clk-gating-sb");
+	cfg->clk_gating_sb_clk_trunk = !ofnode_read_bool(node,
+					"fsps,disable-clk-gating-sb-clk-trunk");
+	cfg->clk_gating_sb_clk_partition = !ofnode_read_bool(node,
+				"fsps,disable-clk-gating-sb-clk-partition");
+	cfg->clk_gating_core = !ofnode_read_bool(node,
+					"fsps,disable-clk-gating-core");
+	cfg->clk_gating_dma = !ofnode_read_bool(node,
+						"fsps,disable-clk-gating-dma");
+	cfg->clk_gating_reg_access = !ofnode_read_bool(node,
+					"fsps,disable-clk-gating-reg-access");
+	cfg->clk_gating_host = !ofnode_read_bool(node,
+					"fsps,disable-clk-gating-host");
+	cfg->clk_gating_partition = !ofnode_read_bool(node,
+					"fsps,disable-clk-gating-partition");
+	cfg->clk_gating_trunk = !ofnode_read_bool(node,
+					"fsps,disable-clk-gating-trunk");
+	cfg->hda_enable = !ofnode_read_bool(node, "fsps,disable-hda");
+	cfg->dsp_enable = !ofnode_read_bool(node, "fsps,disable-dsp");
+	cfg->pme = ofnode_read_bool(node, "fsps,enable-pme");
+	cfg->hd_audio_io_buffer_ownership = ofnode_read_u32_default(node,
+					"fsps,hd-audio-io-buffer-ownership",
+					HDA_IO_BUFFER_OWNERSHIP_HDA_ALL_IO);
+	cfg->hd_audio_io_buffer_voltage = ofnode_read_u32_default(node,
+					"fsps,hd-audio-io-buffer-voltage",
+					HDA_IO_BUFFER_VOLTAGE_3V3);
+	cfg->hd_audio_vc_type = ofnode_read_u32_default(node,
+							"fsps,hd-audio-vc-type",
+							HDA_VC_TYPE_VC0);
+	cfg->hd_audio_link_frequency = ofnode_read_u32_default(node,
+						"fsps,hd-audio-link-frequency",
+						HDA_LINK_FREQ_6MHZ);
+	cfg->hd_audio_i_disp_link_frequency = ofnode_read_u32_default(node,
+					"fsps,hd-audio-i-disp-link-frequency",
+					HDA_I_DISP_LINK_FREQ_6MHZ);
+	cfg->hd_audio_i_disp_link_tmode = ofnode_read_u32_default(node,
+					"fsps,hd-audio-i-disp-link-tmode",
+					HDA_I_DISP_LINK_T_MODE_2T);
+	cfg->dsp_endpoint_dmic = ofnode_read_u32_default(node,
+						"fsps,dsp-endpoint-dmic",
+						HDA_DISP_DMIC_2CH_ARRAY);
+	cfg->dsp_endpoint_bluetooth = !ofnode_read_bool(node,
+					"fsps,disable-dsp-endpoint-bluetooth");
+	cfg->dsp_endpoint_i2s_skp = ofnode_read_bool(node,
+					"fsps,enable-dsp-endpoint-i2s-skp");
+	cfg->dsp_endpoint_i2s_hp = ofnode_read_bool(node,
+					"fsps,enable-dsp-endpoint-i2s-hp");
+	cfg->audio_ctl_pwr_gate = ofnode_read_bool(node,
+					"fsps,enable-audio-ctl-pwr-gate");
+	cfg->audio_dsp_pwr_gate = ofnode_read_bool(node,
+					"fsps,enable-audio-dsp-pwr-gate");
+	cfg->mmt = ofnode_read_u32_default(node, "fsps,mmt",
+					   HDA_CSE_MEM_TRANSFERS_VC0);
+	cfg->hmt = ofnode_read_u32_default(node, "fsps,hmt",
+					   HDA_HOST_MEM_TRANSFERS_VC0);
+	cfg->hd_audio_pwr_gate = ofnode_read_bool(node,
+					"fsps,enable-hd-audio-pwr-gate");
+	cfg->hd_audio_clk_gate = ofnode_read_bool(node,
+					"fsps,enable-hd-audio-clk-gate");
+	cfg->dsp_feature_mask = ofnode_read_u32_default(node,
+							"fsps,dsp-feature-mask",
+							0x00000000);
+	cfg->dsp_pp_module_mask = ofnode_read_u32_default(node,
+						"fsps,dsp-pp-module-mask",
+						 0x00000000);
+	cfg->bios_cfg_lock_down = ofnode_read_bool(node,
+					"fsps,enable-bios-cfg-lock-down");
+	cfg->hpet = !ofnode_read_bool(node, "fsps,disable-hpet");
+	cfg->hpet_bdf_valid = ofnode_read_bool(node,
+					       "fsps,enable-hpet-bdf-valid");
+	cfg->hpet_bus_number = ofnode_read_u32_default(node,
+						       "fsps,hpet-bus-number",
+						       HPET_BUS_NUMBER_DEFAULT);
+	cfg->hpet_device_number = ofnode_read_u32_default(node,
+						"fsps,hpet-device-number",
+						HPET_DEVICE_NUMBER_DEFAULT);
+	cfg->hpet_function_number = ofnode_read_u32_default(node,
+						"fsps,hpet-function-number",
+						HPET_FUNCTION_NUMBER_DEFAULT);
+	cfg->io_apic_bdf_valid = ofnode_read_bool(node,
+					       "fsps,enable-io-apic-bdf-valid");
+	cfg->io_apic_bus_number = ofnode_read_u32_default(node,
+						"fsps,io-apic-bus-number",
+						IOAPIC_BUS_NUMBER_DEFAULT);
+	cfg->io_apic_device_number = ofnode_read_u32_default(node,
+						"fsps,io-apic-device-number",
+						IOAPIC_DEVICE_NUMBER_DEFAULT);
+	cfg->io_apic_function_number = ofnode_read_u32_default(node,
+						"fsps,io-apic-function-number",
+						IOAPIC_FUNCTION_NUMBER_DEFAULT);
+	cfg->io_apic_entry24_119 = !ofnode_read_bool(node,
+					"fsps,disable-io-apic-entry24-119");
+	cfg->io_apic_id = ofnode_read_u32_default(node,
+						  "fsps,io-apic-id",
+						  IOAPIC_ID_DEFAULT);
+	cfg->io_apic_range_select = ofnode_read_u32_default(node,
+						  "fsps,io-apic-range-select",
+						  0);
+	cfg->ish_enable = !ofnode_read_bool(node, "fsps,disable-ish");
+	cfg->bios_interface = !ofnode_read_bool(node,
+					"fsps,disable-bios-interface");
+	cfg->bios_lock = ofnode_read_bool(node, "fsps,enable-bios-lock");
+	cfg->spi_eiss = !ofnode_read_bool(node, "fsps,disable-spi-eiss");
+	cfg->bios_lock_sw_smi_number = ofnode_read_u32_default(node,
+					"fsps,bios-lock-sw-smi-number",
+					BIOS_LOCK_SW_SMI_NUMBER_DEFAULT);
+	cfg->lpss_s0ix_enable = ofnode_read_bool(node, "fsps,enable-lpss-s0ix");
+	read_u8_array(cfg->i2c_clk_gate_cfg, node,
+		      "fsps,i2c-clk-gate-cfg",
+		      sizeof(cfg->i2c_clk_gate_cfg), 1);
+	read_u8_array(cfg->hsuart_clk_gate_cfg, node,
+		      "fsps,hsuart-clk-gate-cfg",
+		      sizeof(cfg->hsuart_clk_gate_cfg), 1);
+	read_u8_array(cfg->spi_clk_gate_cfg, node,
+		      "fsps,spi-clk-gate-cfg",
+		      sizeof(cfg->spi_clk_gate_cfg), 1);
+	for (int i = 0; i < FSP_I2C_COUNT; i++)  {
+		snprintf(buf, sizeof(buf), "fsps,enable-i2c%d", i);
+		*(&cfg->i2c0_enable + i) = ofnode_read_u32_default(node, buf,
+							I2CX_ENABLE_PCI_MODE);
+	}
+	for (int i = 0; i < FSP_HSUART_COUNT; i++)  {
+		snprintf(buf, sizeof(buf), "fsps,enable-hsuart%d", i);
+		*(&cfg->hsuart0_enable + i) = ofnode_read_u32_default(node, buf,
+						HSUARTX_ENABLE_PCI_MODE);
+	}
+	for (int i = 0; i < FSP_SPI_COUNT; i++)  {
+		snprintf(buf, sizeof(buf), "fsps,enable-spi%d", i);
+		*(&cfg->spi0_enable + i) = ofnode_read_u32_default(node, buf,
+							SPIX_ENABLE_PCI_MODE);
+	}
+	cfg->os_dbg_enable = ofnode_read_bool(node, "fsps,enable-os-dbg");
+	cfg->dci_en = ofnode_read_bool(node, "fsps,enable-dci");
+	cfg->uart2_kernel_debug_base_address = ofnode_read_u32_default(node,
+					"fsps,uart2-kernel-debug-base-address",
+					0);
+	cfg->pcie_clock_gating_disabled = !ofnode_read_bool(node,
+					"fsps,disable-pcie-clock-gating");
+	cfg->pcie_root_port8xh_decode = !ofnode_read_bool(node,
+				"fsps,disable-pcie-root-port8xh-decode");
+	cfg->pcie8xh_decode_port_index = ofnode_read_u32_default(node,
+					"fsps,pcie8xh-decode-port-index",
+					0);
+	cfg->pcie_root_port_peer_memory_write_enable = ofnode_read_bool(node,
+				"fsps,enable-pcie-root-port-peer-memory-write");
+	cfg->pcie_aspm_sw_smi_number = ofnode_read_u32_default(node,
+					"fsps,pcie-aspm-sw-smi-number",
+					PCIE_ASPM_SW_SMI_NUMBER_DEFAULT);
+	read_u8_array(cfg->pcie_root_port_en, node,
+		      "fsps,enable-pcie-root-port",
+		      ARRAY_SIZE(cfg->pcie_root_port_en), 1);
+	read_u8_array(cfg->pcie_rp_hide, node, "fsps,pcie-rp-hide",
+		      ARRAY_SIZE(cfg->pcie_rp_hide), 0);
+	read_u8_array(cfg->pcie_rp_slot_implemented, node,
+		      "fsps,pcie-rp-slot-implemented",
+		      ARRAY_SIZE(cfg->pcie_rp_slot_implemented), 1);
+	read_u8_array(cfg->pcie_rp_hot_plug, node, "fsps,pcie-rp-hot-plug",
+		      ARRAY_SIZE(cfg->pcie_rp_hot_plug), 1);
+	read_u8_array(cfg->pcie_rp_pm_sci, node, "fsps,pcie-rp-pm-sci",
+		      ARRAY_SIZE(cfg->pcie_rp_pm_sci), 0);
+	read_u8_array(cfg->pcie_rp_ext_sync, node, "fsps,pcie-rp-ext-sync",
+		      ARRAY_SIZE(cfg->pcie_rp_ext_sync), 1);
+	read_u8_array(cfg->pcie_rp_transmitter_half_swing, node,
+		      "fsps,pcie-rp-transmitter-half-swing",
+		      ARRAY_SIZE(cfg->pcie_rp_transmitter_half_swing), 1);
+	read_u8_array(cfg->pcie_rp_acs_enabled, node,
+		      "fsps,pcie-rp-acs",
+		      ARRAY_SIZE(cfg->pcie_rp_acs_enabled), 1);
+	read_u8_array(cfg->pcie_rp_clk_req_supported, node,
+		      "fsps,pcie-rp-clk-req-supported",
+		      ARRAY_SIZE(cfg->pcie_rp_clk_req_supported), 1);
+	ret = read_u8_array(cfg->pcie_rp_clk_req_number, node,
+		      "fsps,pcie-rp-clk-req-number",
+		      ARRAY_SIZE(cfg->pcie_rp_clk_req_number), 1);
+	if (ret)
+		memcpy(cfg->pcie_rp_clk_req_number, pcie_rp_clk_req_number_def,
+		       sizeof(cfg->pcie_rp_clk_req_number));
+	read_u8_array(cfg->pcie_rp_clk_req_detect, node,
+		      "fsps,pcie-rp-clk-req-detect",
+		      ARRAY_SIZE(cfg->pcie_rp_clk_req_detect), 0);
+	read_u8_array(cfg->advanced_error_reporting, node,
+		      "fsps,advanced-error-reportingt",
+		      ARRAY_SIZE(cfg->advanced_error_reporting), 0);
+	read_u8_array(cfg->pme_interrupt, node, "fsps,pme-interrupt",
+		      ARRAY_SIZE(cfg->pme_interrupt), 0);
+	read_u8_array(cfg->unsupported_request_report, node,
+		      "unsupported-request-report",
+		      ARRAY_SIZE(cfg->unsupported_request_report), 0);
+	read_u8_array(cfg->fatal_error_report, node,
+		      "fsps,fatal-error-report",
+		      ARRAY_SIZE(cfg->fatal_error_report), 0);
+	read_u8_array(cfg->no_fatal_error_report, node,
+		      "fsps,no-fatal-error-report",
+		      ARRAY_SIZE(cfg->no_fatal_error_report), 0);
+	read_u8_array(cfg->correctable_error_report, node,
+		      "fsps,correctable-error-report",
+		      ARRAY_SIZE(cfg->correctable_error_report), 0);
+	read_u8_array(cfg->system_error_on_fatal_error, node,
+		      "fsps,system-error-on-fatal-error",
+		      ARRAY_SIZE(cfg->system_error_on_fatal_error), 0);
+	read_u8_array(cfg->system_error_on_non_fatal_error, node,
+		      "fsps,system-error-on-non-fatal-error",
+		      ARRAY_SIZE(cfg->system_error_on_non_fatal_error), 0);
+	read_u8_array(cfg->system_error_on_correctable_error, node,
+		      "fsps,system-error-on-correctable-error",
+		      ARRAY_SIZE(cfg->system_error_on_correctable_error), 0);
+	read_u8_array(cfg->pcie_rp_speed, node, "fsps,pcie-rp-speed",
+		      ARRAY_SIZE(cfg->pcie_rp_speed), PCIE_RP_SPEED_AUTO);
+	ret = read_u8_array(cfg->physical_slot_number, node,
+			    "fsps,physical-slot-number",
+			    ARRAY_SIZE(cfg->physical_slot_number), 0);
+	if (ret)
+		memcpy(cfg->physical_slot_number, physical_slot_number_def,
+		       sizeof(cfg->physical_slot_number));
+	read_u8_array(cfg->pcie_rp_completion_timeout, node,
+		      "fsps,pcie-rp-completion-timeout",
+		      ARRAY_SIZE(cfg->pcie_rp_completion_timeout), 0);
+	read_u8_array(cfg->ptm_enable, node, "fsps,enable-ptm",
+		      ARRAY_SIZE(cfg->ptm_enable), 0);
+	read_u8_array(cfg->pcie_rp_aspm, node, "fsps,pcie-rp-aspm",
+		      ARRAY_SIZE(cfg->pcie_rp_aspm), PCIE_RP_ASPM_AUTO);
+	read_u8_array(cfg->pcie_rp_l1_substates, node,
+		      "fsps,pcie-rp-l1-substates",
+		      ARRAY_SIZE(cfg->pcie_rp_l1_substates),
+		      PCIE_RP_L1_SUBSTATES_L1_1_L1_2);
+	read_u8_array(cfg->pcie_rp_ltr_enable, node,
+		      "fsps,pcie-rp-ltr-enable",
+		      ARRAY_SIZE(cfg->pcie_rp_ltr_enable), 1);
+	read_u8_array(cfg->pcie_rp_ltr_config_lock, node,
+		      "fsps,pcie-rp-ltr-config-lock",
+		      ARRAY_SIZE(cfg->pcie_rp_ltr_config_lock), 0);
+	cfg->pme_b0_s5_dis = ofnode_read_bool(node,
+					       "fsps,disable-pme-b0-s5");
+	cfg->pci_clock_run = ofnode_read_bool(node,
+					       "fsps,enable-pci-clock-run");
+	cfg->timer8254_clk_setting = ofnode_read_bool(node,
+					"fsps,enable-timer8254-clk-setting");
+	cfg->enable_sata = !ofnode_read_bool(node, "fsps,disable-sata");
+	cfg->sata_mode = ofnode_read_u32_default(node, "fsps,sata-mode",
+						 SATA_MODE_AHCI);
+	cfg->sata_salp_support = !ofnode_read_bool(node,
+					"fsps,disable-sata-salp-support");
+	cfg->sata_pwr_opt_enable = ofnode_read_bool(node,
+					"fsps,enable-sata-pwr-opt");
+	cfg->e_sata_speed_limit = ofnode_read_bool(node,
+					"fsps,enable-e-sata-speed-limit");
+	cfg->speed_limit = ofnode_read_u32_default(node, "fsps,speed-limit",
+						SATA_SPEED_LIMIT_SC_SATA_SPEED);
+	read_u8_array(cfg->sata_ports_enable, node,
+		      "fsps,enable-sata-ports",
+		      ARRAY_SIZE(cfg->sata_ports_enable), 1);
+	read_u8_array(cfg->sata_ports_dev_slp, node,
+		      "fsps,sata-ports-dev-slp",
+		      ARRAY_SIZE(cfg->sata_ports_dev_slp), 0);
+	read_u8_array(cfg->sata_ports_hot_plug, node,
+		      "fsps,sata-ports-hot-plug",
+		      ARRAY_SIZE(cfg->sata_ports_hot_plug), 0);
+	read_u8_array(cfg->sata_ports_interlock_sw, node,
+		      "fsps,sata-ports-interlock-sw",
+		      ARRAY_SIZE(cfg->sata_ports_interlock_sw), 1);
+	read_u8_array(cfg->sata_ports_external, node,
+		      "fsps,sata-ports-external",
+		      ARRAY_SIZE(cfg->sata_ports_external), 0);
+	read_u8_array(cfg->sata_ports_spin_up, node,
+		      "fsps,sata-ports-spin-up",
+		      ARRAY_SIZE(cfg->sata_ports_spin_up), 0);
+	read_u8_array(cfg->sata_ports_solid_state_drive, node,
+		      "fsps,sata-ports-solid-state-drive",
+		      ARRAY_SIZE(cfg->sata_ports_solid_state_drive),
+		      SATA_PORT_SOLID_STATE_DRIVE_HARD_DISK_DRIVE);
+	read_u8_array(cfg->sata_ports_enable_dito_config, node,
+		      "fsps,enable-sata-ports-dito-config",
+		      ARRAY_SIZE(cfg->sata_ports_enable_dito_config), 0);
+	read_u8_array(cfg->sata_ports_dm_val, node,
+		      "fsps,sata-ports-dm-val",
+		      ARRAY_SIZE(cfg->sata_ports_dm_val),
+		      SATA_PORTS_DM_VAL_DEFAULT);
+	read_u16_array(cfg->sata_ports_dito_val, node,
+		      "fsps,sata-ports-dito-val",
+		      ARRAY_SIZE(cfg->sata_ports_dito_val),
+		      SATA_PORTS_DITO_VAL_DEFAULT);
+	cfg->sub_system_vendor_id = ofnode_read_u32_default(node,
+						"fsps,sub-system-vendor-id",
+						PCI_VENDOR_ID_INTEL);
+	cfg->sub_system_id = ofnode_read_u32_default(node,
+						"fsps,sub-system-id",
+						SUB_SYSTEM_ID_DEFAULT);
+	cfg->crid_settings = ofnode_read_u32_default(node,
+						"fsps,crid-settings",
+						CRID_SETTING_DISABLE);
+	cfg->reset_select = ofnode_read_u32_default(node,
+						"fsps,reset-select",
+						RESET_SELECT_WARM_RESET);
+	cfg->sdcard_enabled = !ofnode_read_bool(node, "fsps,disable-sdcard");
+	cfg->e_mmc_enabled = !ofnode_read_bool(node, "fsps,disable-emmc");
+	cfg->e_mmc_host_max_speed = ofnode_read_u32_default(node,
+						"fsps,emmc-host-max-speed",
+						EMMC_HOST_SPEED_MAX_HS400);
+	cfg->ufs_enabled = !ofnode_read_bool(node, "fsps,disable-ufs");
+	cfg->sdio_enabled = !ofnode_read_bool(node, "fsps,disable-sdio");
+	cfg->gpp_lock = ofnode_read_bool(node, "fsps,enable-gpp-lock");
+	cfg->sirq_enable = !ofnode_read_bool(node, "fsps,disable-sirq");
+	cfg->sirq_mode = ofnode_read_u32_default(node, "fsps,sirq-mode",
+						SERIAL_IRQ_MODE_QUIET_MODE);
+	cfg->start_frame_pulse = ofnode_read_u32_default(node,
+					"fsps,start-frame-pulse",
+					START_FRAME_PULSE_WIDTH_SCSFPW4CLK);
+	cfg->smbus_enable = !ofnode_read_bool(node, "fsps,disable-smbus");
+	cfg->arp_enable = !ofnode_read_bool(node, "fsps,disable-arp");
+	cfg->num_rsvd_smbus_addresses = ofnode_read_u32_default(node,
+					"fsps,num-rsvd-smbus-addresses",
+					NUM_RSVD_SMBUS_ADDRESSES_DEFAULT);
+	if (cfg->num_rsvd_smbus_addresses > 0) {
+		read_u8_array(cfg->rsvd_smbus_address_table, node,
+			      "fsps,rsvd-smbus-address-table",
+			      cfg->num_rsvd_smbus_addresses, 0x0000);
+	}
+	cfg->disable_compliance_mode = ofnode_read_bool(node,
+					"fsps,disable-compliance-mode");
+	cfg->usb_per_port_ctl = ofnode_read_bool(node,
+					"fsps,enable-usb-per-port-ctl");
+	cfg->usb30_mode = ofnode_read_u32_default(node,
+					"fsps,usb30-mode",
+					USB30_MODE_AUTO);
+	read_u8_array(cfg->port_usb20_enable, node,
+		      "fsps,enable-port-usb20",
+		      ARRAY_SIZE(cfg->port_usb20_enable), 1);
+	read_u8_array(cfg->port_us20b_over_current_pin, node,
+		      "fsps,port-usb20-over-current-pin",
+		      ARRAY_SIZE(cfg->port_us20b_over_current_pin),
+		      PORT_USB20_OVER_CURRENT_PIN_DEFAULT);
+	cfg->usb_otg = ofnode_read_u32_default(node, "fsps,usb-otg",
+					       USB_OTG_PCI_MODE);
+	cfg->hsic_support_enable = ofnode_read_bool(node,
+					"fsps,enable-hsic-support");
+	read_u8_array(cfg->port_usb30_enable, node,
+			      "fsps,enable-port-usb30",
+			      ARRAY_SIZE(cfg->port_usb30_enable), 1);
+	read_u8_array(cfg->port_us30b_over_current_pin, node,
+		      "fsps,port-usb30-over-current-pin",
+		      ARRAY_SIZE(cfg->port_us30b_over_current_pin),
+		      PORT_USB30_OVER_CURRENT_PIN_DEFAULT);
+	read_u8_array(cfg->ssic_port_enable, node,
+			      "fsps,enable-ssic-port",
+			      ARRAY_SIZE(cfg->ssic_port_enable),
+			      0);
+	cfg->dlane_pwr_gating = !ofnode_read_bool(node,
+					"fsps,disable-dlane-pwr-gating");
+	cfg->vtd_enable = ofnode_read_bool(node, "fsps,enable-vtd");
+	cfg->lock_down_global_smi = !ofnode_read_bool(node,
+					"fsps,disable-lock-down-global-smi");
+	cfg->reset_wait_timer = ofnode_read_u32_default(node,
+						"fsps,reset-wait-timer",
+						RESET_WAIT_TIMER_DEFAULT);
+	cfg->rtc_lock = !ofnode_read_bool(node, "fsps,disable-rtc-lock");
+	cfg->sata_test_mode = ofnode_read_bool(node,
+					       "fsps,enable-safe-test-mode");
+	read_u8_array(cfg->ssic_rate, node,
+				      "fsps,ssic-rate",
+				      ARRAY_SIZE(cfg->ssic_rate),
+				      SSIC_RATE_A_SERIES);
+	cfg->dynamic_power_gating = ofnode_read_bool(node,
+					"fsps,enable-dynamic-power-gating");
+	read_u16_array(cfg->pcie_rp_ltr_max_snoop_latency, node,
+			"fsps,pcie-rp-ltr-max-snoop-latency",
+			ARRAY_SIZE(cfg->pcie_rp_ltr_max_snoop_latency),
+			PCIE_RP_LTR_MAX_SNOOP_LATENCY_DEFAULT);
+	read_u8_array(cfg->pcie_rp_snoop_latency_override_mode, node,
+		      "fsps,pcie-rp-snoop-latency-override-mode",
+		      ARRAY_SIZE(cfg->pcie_rp_snoop_latency_override_mode),
+		      PCIE_RP_SNOOP_LATENCY_OVERRIDE_MODE_AUTO);
+	read_u16_array(cfg->pcie_rp_snoop_latency_override_value, node,
+		       "fsps,pcie-rp-snoop-latency-override-value",
+		       ARRAY_SIZE(cfg->pcie_rp_snoop_latency_override_value),
+		       PCIE_RP_SNOOP_LATENCY_OVERRIDE_VALUE_DEFAULT);
+	read_u8_array(cfg->pcie_rp_snoop_latency_override_multiplier, node,
+		"fsps,pcie-rp-snoop-latency-override-multiplier",
+		ARRAY_SIZE(cfg->pcie_rp_snoop_latency_override_multiplier),
+		PCIE_RP_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_1024NS);
+	cfg->skip_mp_init = ofnode_read_bool(node, "fsps,enable-skip-mp-init");
+	cfg->dci_auto_detect = !ofnode_read_bool(node,
+					"fsps,disable-dci-auto-detect");
+	read_u16_array(cfg->pcie_rp_ltr_max_non_snoop_latency, node,
+		       "fsps,pcie-rp-ltr-max-non-snoop-latency",
+		       ARRAY_SIZE(cfg->pcie_rp_ltr_max_non_snoop_latency),
+		       PCIE_RP_LTR_MAX_NON_SNOOP_LATENCY_DEFAULT);
+	read_u8_array(cfg->pcie_rp_non_snoop_latency_override_mode, node,
+		      "fsps,pcie-rp-non-snoop-latency-override-mode",
+		      ARRAY_SIZE(cfg->pcie_rp_non_snoop_latency_override_mode),
+		      PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MODE_AUTO);
+	cfg->tco_timer_halt_lock = !ofnode_read_bool(node,
+					"fsps,disable-tco-timer-halt-lock");
+	cfg->pwr_btn_override_period = ofnode_read_u32_default(node,
+					"fsps,pwr-btn-override-period",
+					PWR_BTN_OVERRIDE_PERIOD_4S);
+	read_u16_array(cfg->pcie_rp_non_snoop_latency_override_value, node,
+		"fsps,pcie-rp-non-snoop-latency-override-value",
+		ARRAY_SIZE(cfg->pcie_rp_non_snoop_latency_override_value),
+		PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_VALUE_DEFAULT);
+	read_u8_array(cfg->pcie_rp_non_snoop_latency_override_multiplier, node,
+		"fsps,pcie-rp-non-snoop-latency-override-multiplier",
+		ARRAY_SIZE(cfg->pcie_rp_non_snoop_latency_override_multiplier),
+		PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_1024NS);
+	read_u8_array(cfg->pcie_rp_slot_power_limit_scale, node,
+		      "fsps,pcie-rp-slot-power-limit-scale",
+		      ARRAY_SIZE(cfg->pcie_rp_slot_power_limit_scale),
+		      PCIE_RP_SLOT_POWER_LIMIT_SCALE_DEFAULT);
+	read_u8_array(cfg->pcie_rp_slot_power_limit_value, node,
+		      "fsps,pcie-rp-slot-power-limit-value",
+		      ARRAY_SIZE(cfg->pcie_rp_slot_power_limit_value),
+		      PCIE_RP_SLOT_POWER_LIMIT_VALUE_DEFAULT);
+	cfg->disable_native_power_button = ofnode_read_bool(node,
+					"fsps,disable-native-power-button");
+	cfg->power_butter_debounce_mode = !ofnode_read_bool(node,
+				"fsps,disable-power-button-debounce-mode");
+	cfg->sdio_tx_cmd_cntl = ofnode_read_u32_default(node,
+						"fsps,sdio-tx-cmd-cntl",
+						SDIO_TX_CMD_CNTL_DEFAULT);
+	cfg->sdio_tx_data_cntl1 = ofnode_read_u32_default(node,
+						"fsps,sdio-tx-data-cntl1",
+						SDIO_TX_DATA_CNTL1_DEFAULT);
+	cfg->sdio_tx_data_cntl2 = ofnode_read_u32_default(node,
+						"fsps,sdio-tx-data-cntl2",
+						SDIO_TX_DATA_CNTL2_DEFAULT);
+	cfg->sdio_rx_cmd_data_cntl1 = ofnode_read_u32_default(node,
+						"fsps,sdio-rx-cmd-data-cntl1",
+						SDIO_RX_CMD_DATA_CNTL1_DEFAULT);
+	cfg->sdio_rx_cmd_data_cntl2 = ofnode_read_u32_default(node,
+						"fsps,sdio-rx-cmd-data-cntl2",
+						SDIO_RX_CMD_DATA_CNTL2_DEFAULT);
+	cfg->sdcard_tx_cmd_cntl = ofnode_read_u32_default(node,
+						"fsps,sdcard-tx-cmd-cntl",
+						SDCARD_TX_CMD_CNTL_DEFAULT);
+	cfg->sdcard_tx_data_cntl1 = ofnode_read_u32_default(node,
+						"fsps,sdcard-tx-data-cntl1",
+						SDCARD_TX_DATA_CNTL1_DEFAULT);
+	cfg->sdcard_tx_data_cntl2 = ofnode_read_u32_default(node,
+						"fsps,sdcard-tx-data-cntl2",
+						SDCARD_TX_DATA_CNTL2_DEFAULT);
+	cfg->sdcard_rx_cmd_data_cntl1 = ofnode_read_u32_default(node,
+					"fsps,sdcard-rx-cmd-data-cntl1",
+					SDCARD_RX_CMD_DATA_CNTL1_DEFAULT);
+	cfg->sdcard_rx_strobe_cntl = ofnode_read_u32_default(node,
+						"fsps,sdcard-rx-strobe-cntl",
+						SDCARD_RX_STROBE_CNTL_DEFAULT);
+	cfg->sdcard_rx_cmd_data_cntl2 = ofnode_read_u32_default(node,
+					"fsps,sdcard-rx-cmd-data-cntl2",
+					SDCARD_RX_CMD_DATA_CNTL2_DEFAULT);
+	cfg->emmc_tx_cmd_cntl = ofnode_read_u32_default(node,
+						"fsps,emmc-tx-cmd-cntl",
+						EMMC_TX_CMD_CNTL_DEFAULT);
+	cfg->emmc_tx_data_cntl1 = ofnode_read_u32_default(node,
+						"fsps,emmc-tx-data-cntl1",
+						EMMC_TX_DATA_CNTL1_DEFAULT);
+	cfg->emmc_tx_data_cntl2 = ofnode_read_u32_default(node,
+						"fsps,emmc-tx-data-cntl2",
+						EMMC_TX_DATA_CNTL2_DEFAULT);
+	cfg->emmc_rx_cmd_data_cntl1 = ofnode_read_u32_default(node,
+						"fsps,emmc-rx-cmd-data-cntl1",
+						EMMC_RX_CMD_DATA_CNTL1_DEFAULT);
+	cfg->emmc_rx_strobe_cntl = ofnode_read_u32_default(node,
+						"fsps,emmc-rx-strobe-cntl",
+						EMMC_RX_STROBE_CNTL_DEFAULT);
+	cfg->emmc_rx_cmd_data_cntl2 = ofnode_read_u32_default(node,
+						"fsps,emmc-rx-cmd-data-cntl2",
+						EMMC_RX_CMD_DATA_CNTL2_DEFAULT);
+	cfg->emmc_master_sw_cntl = ofnode_read_u32_default(node,
+						"fsps,emmc-master-sw-cntl",
+						EMMC_MASTER_SW_CNTL_DEFAULT);
+	read_u8_array(cfg->pcie_rp_selectable_deemphasis, node,
+		      "fsps,pcie-rp-selectable-deemphasis",
+		      ARRAY_SIZE(cfg->pcie_rp_selectable_deemphasis),
+		      PCIE_RP_SELECTABLE_DEEMPHASIS_3_5_DB);
+	cfg->monitor_mwait_enable = !ofnode_read_bool(node,
+						"fsps,disable-monitor-mwait");
+	cfg->hd_audio_dsp_uaa_compliance = ofnode_read_bool(node,
+				"fsps,enable-hd-audio-dsp-uaa-compliance");
+	ret = read_u32_array(cfg->ipc, node, "fsps,ipc", ARRAY_SIZE(cfg->ipc),
+			     0);
+	if (ret)
+		memcpy(cfg->ipc, ipc_def, sizeof(cfg->ipc));
+
+	read_u8_array(cfg->sata_ports_disable_dynamic_pg, node,
+		      "fsps,sata-ports-disable-dynamic-pg",
+		      ARRAY_SIZE(cfg->sata_ports_disable_dynamic_pg),
+		      0);
+	cfg->init_s3_cpu = ofnode_read_bool(node, "fsps,enable-init-s3-cpu");
+	cfg->skip_punit_init = ofnode_read_bool(node,
+						"fsps,enable-skip-punit-init");
+	read_u8_array(cfg->port_usb20_per_port_tx_pe_half, node,
+		      "fsps,port-usb20-per-port-tx-pe-half",
+		      ARRAY_SIZE(cfg->port_usb20_per_port_tx_pe_half),
+		      0);
+	ret = read_u8_array(cfg->port_usb20_per_port_pe_txi_set, node,
+			    "fsps,port-usb20-per-port-pe-txi-set",
+			    ARRAY_SIZE(cfg->port_usb20_per_port_pe_txi_set),
+			    0);
+	if (ret)
+		memcpy(cfg->port_usb20_per_port_pe_txi_set,
+		       port_usb20_per_port_pe_txi_set_def,
+		       sizeof(cfg->port_usb20_per_port_pe_txi_set));
+	ret = read_u8_array(cfg->port_usb20_per_port_txi_set, node,
+			    "fsps,port-usb20-per-port-txi-set",
+			    ARRAY_SIZE(cfg->port_usb20_per_port_txi_set),
+			    0);
+	if (ret)
+		memcpy(cfg->port_usb20_per_port_txi_set,
+		       port_usb20_per_port_txi_set_def,
+		       sizeof(cfg->port_usb20_per_port_txi_set));
+	ret = read_u8_array(cfg->port_usb20_hs_skew_sel, node,
+			    "fsps,port-usb20-hs-skew-sel",
+			    ARRAY_SIZE(cfg->port_usb20_hs_skew_sel),
+			    0);
+	if (ret)
+		memcpy(cfg->port_usb20_hs_skew_sel,
+		       port_usb20_hs_skew_sel_def,
+		       sizeof(cfg->port_usb20_hs_skew_sel));
+	ret = read_u8_array(cfg->port_usb20_i_usb_tx_emphasis_en, node,
+			    "fsps,port-usb20-i-usb-tx-emphasis-en",
+			    ARRAY_SIZE(cfg->port_usb20_i_usb_tx_emphasis_en),
+			    0);
+	if (ret)
+		memcpy(cfg->port_usb20_i_usb_tx_emphasis_en,
+		       port_usb20_i_usb_tx_emphasis_en_def,
+		       sizeof(cfg->port_usb20_i_usb_tx_emphasis_en));
+	read_u8_array(cfg->port_usb20_per_port_rxi_set, node,
+		      "fsps,port-usb20-per-port-rxi-set",
+		      ARRAY_SIZE(cfg->port_usb20_per_port_rxi_set),
+		      0);
+	ret = read_u8_array(cfg->port_usb20_hs_npre_drv_sel, node,
+			    "fsps,port-usb20-hs-npre-drv-sel",
+			    ARRAY_SIZE(cfg->port_usb20_hs_npre_drv_sel),
+			    0);
+	if (ret)
+		memcpy(cfg->port_usb20_hs_npre_drv_sel,
+		       port_usb20_hs_npre_drv_sel_def,
+		       sizeof(cfg->port_usb20_hs_npre_drv_sel));
 
 	return 0;
 }
diff --git a/arch/x86/dts/chromebook_coral.dts b/arch/x86/dts/chromebook_coral.dts
index af52e11c89..f8ba932cc2 100644
--- a/arch/x86/dts/chromebook_coral.dts
+++ b/arch/x86/dts/chromebook_coral.dts
@@ -21,6 +21,7 @@
 #include <asm/arch-apollolake/iomap.h>
 #include <asm/arch-apollolake/pm.h>
 #include <dt-bindings/clock/intel-clock.h>
+#include <asm/arch-apollolake/fsp/fsp_s_upd.h>
 
 / {
 	model = "Google Coral";
@@ -484,8 +485,19 @@
 &fsp_s {
 	u-boot,dm-pre-proper;
 
+	fsps,disable-ish;
+	fsps,disable-sata;
+	fsps,enable-pcie-root-port = [00 00 00 00 00 01];
+	fsps,pcie-rp-hot-plug = [00 00 00 00 00 01];
+	fsps,enable-i2c6 = <I2CX_ENABLE_DISABLED>;
+	fsps,enable-i2c7 = <I2CX_ENABLE_DISABLED>;
+	fsps,enable-hsuart3 = <HSUARTX_ENABLE_DISABLED>;
+	fsps,enable-spi1 = <SPIX_ENABLE_DISABLED>;
+	fsps,enable-spi2 = <SPIX_ENABLE_DISABLED>;
+	fsps,disable-sdio;
+
 	/* Disable unused clkreq of PCIe root ports */
-	pcie-rp-clkreq-pin = /bits/ 8 <0 /* wifi/bt */
+	fsps,pcie-rp-clk-req-number = /bits/ 8 <0 /* wifi/bt */
 		CLKREQ_DISABLED
 		CLKREQ_DISABLED
 		CLKREQ_DISABLED
@@ -539,18 +551,27 @@
 	 * [14:8] steps of delay for Auto Tuning Mode, each 125ps
 	 * [6:0] steps of delay for HS200, each 125ps
 	 */
-	emmc = <0x0c16 0x28162828 0x00181717 0x10008>;
-
 	/* Enable DPTF */
 	dptf-enable;
+	fsps,emmc-tx-data-cntl1 = <0x0c16>;
+	fsps,emmc-tx-data-cntl2 = <0x28162828>;
+	fsps,emmc-rx-cmd-data-cntl1 = <0x00181717>;
+	fsps,emmc-rx-cmd-data-cntl2 = <0x10008>;
 
 	/* Enable Audio Clock and Power gating */
-	hdaudio-clk-gate-enable;
-	hdaudio-pwr-gate-enable;
-	hdaudio-bios-config-lockdown;
+	fsps,enable-hd-audio-clk-gate;
+	fsps,enable-hd-audio-pwr-gate;
+	fsps,enable-bios-cfg-lock-down;
 
 	/* Enable lpss s0ix */
-	lpss-s0ix-enable;
+	fsps,enable-lpss-s0ix;
+
+	fsps,enable-skip-mp-init;
+	fsps,disable-spi-eiss;
+	fsps,disable-rtc-lock;
+
+	fsps,port-usb20-per-port-pe-txi-set = [07 07 06 06 07 07 07 01];
+	fsps,port-usb20-per-port-txi-set = [00 02 00 00 00 00 00 03];
 
 	/*
 	 * TODO(sjg at chromium.org): Move this to the I2C nodes
diff --git a/arch/x86/include/asm/arch-apollolake/fsp/fsp_s_upd.h b/arch/x86/include/asm/arch-apollolake/fsp/fsp_s_upd.h
index 4a868e80ba..7a1799bc4f 100644
--- a/arch/x86/include/asm/arch-apollolake/fsp/fsp_s_upd.h
+++ b/arch/x86/include/asm/arch-apollolake/fsp/fsp_s_upd.h
@@ -6,6 +6,7 @@
 #ifndef __ASM_ARCH_FSP_S_UDP_H
 #define __ASM_ARCH_FSP_S_UDP_H
 
+#ifndef __ASSEMBLY__
 #include <asm/fsp2/fsp_api.h>
 
 struct __packed fsp_s_config {
@@ -288,5 +289,272 @@ struct __packed fsps_upd {
 	u8 unused_upd_space2[46];
 	u16 upd_terminator;
 };
+#endif
+
+#define PROC_TRACE_MEM_SIZE_DISABLE 0xff
+
+#define BOOT_P_STATE_HFM 0
+#define BOOT_P_STATE_LFM 1
+
+#define PKG_C_STATE_LIMIT_C0_C1 0
+#define PKG_C_STATE_LIMIT_C2 1
+#define PKG_C_STATE_LIMIT_C3 2
+#define PKG_C_STATE_LIMIT_C6 3
+#define PKG_C_STATE_LIMIT_C7 4
+#define PKG_C_STATE_LIMIT_C7S 5
+#define PKG_C_STATE_LIMIT_C8 6
+#define PKG_C_STATE_LIMIT_C9 7
+#define PKG_C_STATE_LIMIT_C10 8
+#define PKG_C_STATE_LIMIT_CMAX 9
+#define PKG_C_STATE_LIMIT_CPU_DEFAULT 254
+#define PKG_C_STATE_LIMIT_AUTO 255
+
+#define C_STATE_AUTO_DEMOTION_DISABLE_C1_C3 0
+#define C_STATE_AUTO_DEMOTION_ENABLE_C3_C6_C7_TO_C1 1
+#define C_STATE_AUTO_DEMOTION_ENABLE_C6_C7_TO_C3 2
+#define C_STATE_AUTO_DEMOTION_ENABLE_C6_C7_TO_C1_C3 3
+
+#define C_STATE_UN_DEMOTION_DISABLE_C1_C3 0
+#define C_STATE_UN_DEMOTION_ENABLE_C1 1
+#define C_STATE_UN_DEMOTION_ENABLE_C3 2
+#define C_STATE_UN_DEMOTION_ENABLE_C1_C3 3
+
+#define MAX_CORE_C_STATE_UNLIMITED 0
+#define MAX_CORE_C_STATE_C1 1
+#define MAX_CORE_C_STATE_C3 2
+#define MAX_CORE_C_STATE_C6 3
+#define MAX_CORE_C_STATE_C7 4
+#define MAX_CORE_C_STATE_C8 5
+#define MAX_CORE_C_STATE_C9 6
+#define MAX_CORE_C_STATE_C10 7
+#define MAX_CORE_C_STATE_CCX 8
+
+#define HDA_VERB_TABLE_ENTRY_NUM_DEFAULT 0
+
+#define IPU_ACPI_MODE_DISABLE 0
+#define IPU_ACPI_MODE_IGFX_CHILD_DEVICE 1
+#define IPU_ACPI_MODE_ACPI_DEVICE 1
+
+#define GTT_MM_ADDRESS_DEFAULT 0xbf000000
+
+#define GM_ADDRESS_DEFAULT 0xa0000000
+
+#define CD_CLOCK_FREQ_144MHZ 0
+#define CD_CLOCK_FREQ_288MHZ 1
+#define CD_CLOCK_FREQ_384MHZ 2
+#define CD_CLOCK_FREQ_576MHZ 3
+#define CD_CLOCK_FREQ_624MHZ 4
+
+#define PROTECTED_RANGE_LIMITATION_DEFAULT 0x0fff
+
+#define HDA_IO_BUFFER_OWNERSHIP_HDA_ALL_IO 0
+#define HDA_IO_BUFFER_OWNERSHIP_HDA_I2S_SPLIT 1
+#define HDA_IO_BUFFER_OWNERSHIP_I2S_ALL_IO 2
+
+#define HDA_IO_BUFFER_VOLTAGE_3V3 0
+#define HDA_IO_BUFFER_VOLTAGE_1V8 1
+
+#define HDA_VC_TYPE_VC0 0
+#define HDA_VC_TYPE_VC1 1
+
+#define HDA_LINK_FREQ_6MHZ 0
+#define HDA_LINK_FREQ_12MHZ 1
+#define HDA_LINK_FREQ_24MHZ 2
+#define HDA_LINK_FREQ_48MHZ 3
+#define HDA_LINK_FREQ_96MHZ 4
+#define HDA_LINK_FREQ_INVALID 5
+
+#define HDA_I_DISP_LINK_FREQ_6MHZ 0
+#define HDA_I_DISP_LINK_FREQ_12MHZ 1
+#define HDA_I_DISP_LINK_FREQ_24MHZ 2
+#define HDA_I_DISP_LINK_FREQ_48MHZ 3
+#define HDA_I_DISP_LINK_FREQ_96MHZ 4
+#define HDA_I_DISP_LINK_FREQ_INVALID 5
+
+#define HDA_I_DISP_LINK_T_MODE_2T 0
+#define HDA_I_DISP_LINK_T_MODE_1T 1
+
+#define HDA_DISP_DMIC_DISABLE 0
+#define HDA_DISP_DMIC_2CH_ARRAY 1
+#define HDA_DISP_DMIC_4CH_ARRAY 2
+
+#define HDA_CSE_MEM_TRANSFERS_VC0 0
+#define HDA_CSE_MEM_TRANSFERS_VC2 1
+
+#define HDA_HOST_MEM_TRANSFERS_VC0 0
+#define HDA_HOST_MEM_TRANSFERS_VC2 1
+
+#define HDA_DSP_FEATURE_MASK_WOV 0x1
+#define HDA_DSP_FEATURE_MASK_BT_SIDEBAND 0x2
+#define HDA_DSP_FEATURE_MASK_CODEC_VAD 0x4
+#define HDA_DSP_FEATURE_MASK_BT_INTEL_HFP 0x20
+#define HDA_DSP_FEATURE_MASK_BT_INTEL_A2DP 0x40
+#define HDA_DSP_FEATURE_MASK_DSP_BASED_PRE_PROC_DISABLE 0x80
+
+#define HDA_DSP_PP_MODULE_MASK_WOV 0x1
+#define HDA_DSP_PP_MODULE_MASK_BT_SIDEBAND 0x2
+#define HDA_DSP_PP_MODULE_MASK_CODEC_VAD 0x4
+#define HDA_DSP_PP_MODULE_MASK_BT_INTEL_HFP 0x20
+#define HDA_DSP_PP_MODULE_MASK_BT_INTEL_A2DP 0x40
+#define HDA_DSP_PP_MODULE_MASK_DSP_BASED_PRE_PROC_DISABLE 0x80
+
+#define HPET_BUS_NUMBER_DEFAULT 0xfa
+#define HPET_DEVICE_NUMBER_DEFAULT 0x1f
+#define HPET_FUNCTION_NUMBER_DEFAULT 0x00
+
+#define IOAPIC_BUS_NUMBER_DEFAULT 0xfa
+#define IOAPIC_DEVICE_NUMBER_DEFAULT 0x0f
+#define IOAPIC_FUNCTION_NUMBER_DEFAULT 0x00
+#define IOAPIC_ID_DEFAULT 0x01
+
+#define BIOS_LOCK_SW_SMI_NUMBER_DEFAULT 0xa9
+
+#define I2CX_ENABLE_DISABLED 0
+#define I2CX_ENABLE_PCI_MODE 1
+#define I2CX_ENABLE_ACPI_MODE 2
+
+#define HSUARTX_ENABLE_DISABLED 0
+#define HSUARTX_ENABLE_PCI_MODE 1
+#define HSUARTX_ENABLE_ACPI_MODE 2
+
+#define SPIX_ENABLE_DISABLED 0
+#define SPIX_ENABLE_PCI_MODE 1
+#define SPIX_ENABLE_ACPI_MODE 2
+
+#define PCIE_ASPM_SW_SMI_NUMBER_DEFAULT 0xaa
+
+#define PCIE_RP_SPEED_AUTO 0
+#define PCIE_RP_SPEED_GEN1 1
+#define PCIE_RP_SPEED_GEN2 2
+#define PCIE_RP_SPEED_GEN3 3
+
+#define PCIE_RP_ASPM_DISABLE 0
+#define PCIE_RP_ASPM_L0S 1
+#define PCIE_RP_ASPM_L1 2
+#define PCIE_RP_ASPM_L0S_L1 3
+#define PCIE_RP_ASPM_AUTO 4
+
+#define PCIE_RP_L1_SUBSTATES_DISABLE 0
+#define PCIE_RP_L1_SUBSTATES_L1_1 1
+#define PCIE_RP_L1_SUBSTATES_L1_2 2
+#define PCIE_RP_L1_SUBSTATES_L1_1_L1_2 3
+
+#define SATA_MODE_AHCI 0
+#define SATA_MODE_RAID 1
+
+#define SATA_SPEED_LIMIT_SC_SATA_SPEED 0
+#define SATA_SPEED_LIMIT_1_5GBS 1
+#define SATA_SPEED_LIMIT_3GBS 2
+#define SATA_SPEED_LIMIT_6GBS 3
+
+#define SATA_PORT_SOLID_STATE_DRIVE_HARD_DISK_DRIVE 0
+#define SATA_PORT_SOLID_STATE_DRIVE_SOLID_STATE_DRIVE 1
+
+#define SATA_PORTS_DM_VAL_DEFAULT 0x0f
+
+#define SATA_PORTS_DITO_VAL_DEFAULT 0x0271
+
+#define SUB_SYSTEM_ID_DEFAULT 0x7270
+
+#define CRID_SETTING_DISABLE 0
+#define CRID_SETTING_CRID_1 1
+#define CRID_SETTING_CRID_2 2
+#define CRID_SETTING_CRID_3 3
+
+#define RESET_SELECT_WARM_RESET 0x6
+#define RESET_SELECT_COLD_RESET 0xe
+
+#define EMMC_HOST_SPEED_MAX_HS400 0
+#define EMMC_HOST_SPEED_MAX_HS200 1
+#define EMMC_HOST_SPEED_MAX_DDR50 2
+
+#define SERIAL_IRQ_MODE_QUIET_MODE 0
+#define SERIAL_IRQ_MODE_CONTINUOUS_MODE 1
+
+#define START_FRAME_PULSE_WIDTH_SCSFPW4CLK 0
+#define START_FRAME_PULSE_WIDTH_SCSFPW6CLK 1
+#define START_FRAME_PULSE_WIDTH_SCSFPW8CLK 1
+
+#define NUM_RSVD_SMBUS_ADDRESSES_DEFAULT 0x80
+
+#define USB30_MODE_DISABLE 0
+#define USB30_MODE_ENABLE 1
+#define USB30_MODE_AUTO 2
+
+#define PORT_USB20_OVER_CURRENT_PIN_DEFAULT 0x00
+
+#define USB_OTG_DISABLE 0
+#define USB_OTG_PCI_MODE 1
+#define USB_OTG_ACPI_MODE 2
+
+#define PORT_USB30_OVER_CURRENT_PIN_DEFAULT 0x01
+
+#define RESET_WAIT_TIMER_DEFAULT 0x012c
+
+#define SSIC_RATE_A_SERIES 1
+#define SSIC_RATE_B_SERIES 2
+
+#define PCIE_RP_LTR_MAX_SNOOP_LATENCY_DEFAULT 0
+
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_MODE_DISABLE 0
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_MODE_ENABLE 1
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_MODE_AUTO 2
+
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_VALUE_DEFAULT 0x003c
+
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_1NS 0
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_32NS 1
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_1024NS 2
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_32768NS 3
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_1048576NS 4
+#define PCIE_RP_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_33554432NS 5
+
+#define PCIE_RP_LTR_MAX_NON_SNOOP_LATENCY_DEFAULT 0x0000
+
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MODE_DISABLE 0
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MODE_ENABLE 1
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MODE_AUTO 2
+
+#define PWR_BTN_OVERRIDE_PERIOD_4S 0
+#define PWR_BTN_OVERRIDE_PERIOD_6S 1
+#define PWR_BTN_OVERRIDE_PERIOD_8S 2
+#define PWR_BTN_OVERRIDE_PERIOD_10S 3
+#define PWR_BTN_OVERRIDE_PERIOD_12S 4
+#define PWR_BTN_OVERRIDE_PERIOD_14S 5
+
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_VALUE_DEFAULT 0x003c
+
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_1NS 0
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_32NS 1
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_1024NS 2
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_32768NS 3
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_1048576NS 4
+#define PCIE_RP_NON_SNOOP_LATENCY_OVERRIDE_MULTIPLIER_33554432NS 5
+
+#define PCIE_RP_SLOT_POWER_LIMIT_SCALE_DEFAULT 0
+
+#define PCIE_RP_SLOT_POWER_LIMIT_VALUE_DEFAULT 0
+
+#define SDIO_TX_CMD_CNTL_DEFAULT 0x505
+#define SDIO_TX_DATA_CNTL1_DEFAULT 0xe
+#define SDIO_TX_DATA_CNTL2_DEFAULT 0x22272828
+#define SDIO_RX_CMD_DATA_CNTL1_DEFAULT 0x16161616
+#define SDIO_RX_CMD_DATA_CNTL2_DEFAULT 0x10000
+#define SDCARD_TX_CMD_CNTL_DEFAULT 0x505
+#define SDCARD_TX_DATA_CNTL1_DEFAULT 0xa13
+#define SDCARD_TX_DATA_CNTL2_DEFAULT 0x24242828
+#define SDCARD_RX_CMD_DATA_CNTL1_DEFAULT 0x73a3637
+#define SDCARD_RX_STROBE_CNTL_DEFAULT 0x0
+#define SDCARD_RX_CMD_DATA_CNTL2_DEFAULT 0x10000
+#define EMMC_TX_CMD_CNTL_DEFAULT 0x505
+#define EMMC_TX_DATA_CNTL1_DEFAULT 0xc11
+#define EMMC_TX_DATA_CNTL2_DEFAULT 0x1c2a2927
+#define EMMC_RX_CMD_DATA_CNTL1_DEFAULT 0x000d162f
+#define EMMC_RX_STROBE_CNTL_DEFAULT 0x0a0a
+#define EMMC_RX_CMD_DATA_CNTL2_DEFAULT 0x1003b
+#define EMMC_MASTER_SW_CNTL_DEFAULT 0x001
+
+#define PCIE_RP_SELECTABLE_DEEMPHASIS_6_DB 0
+#define PCIE_RP_SELECTABLE_DEEMPHASIS_3_5_DB 1
 
 #endif
diff --git a/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt b/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
new file mode 100644
index 0000000000..e7dd2c262e
--- /dev/null
+++ b/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
@@ -0,0 +1,485 @@
+* Intel FSP-S configuration
+
+Several Intel platforms require the execution of the Intel FSP (Firmware
+Support Package) for initialization. The FSP consists of multiple parts, one
+of which is the FSP-S (Silicon initialization phase).
+
+This binding applies to the FSP-S for the Intel Apollo Lake SoC.
+
+The FSP-S is available on Github [1].
+For detailed information on the FSP-S parameters see the documentation in
+FSP/ApolloLakeFspBinPkg/Docs [2].
+
+The properties of this binding are all optional. If no properties are set the
+default values for the FSP-S are used. Default values were taken from the
+Fsp.bsf file for Apollo Lake[3]. Boolean properties are named after the
+non-default value, e.g. "CPU power states" is a boolean option in the FSP which
+is enabled by default, so the property in this binding is named
+"fsps,disable-cx". As a result, it is not required to add properties for which
+the default value should be used.
+
+[1] https://github.com/IntelFsp/FSP
+[2] https://github.com/IntelFsp/FSP/tree/master/ApolloLakeFspBinPkg/Docs
+[3] https://github.com/IntelFsp/FSP/blob/master/ApolloLakeFspBinPkg/FspBin/Fsp.bsf
+
+Optional properties:
+- fsps,enable-active-processor-cores: Enable number of active cores
+- fsps,enable-core1: Core1
+- fsps,enable-core2: Core2
+- fsps,enable-core3: Core3
+- fsps,disable-vmx: VMX
+- fsps,proc-trace-mem-size: Memory region allocation for Processor Trace
+  0xFF: Disable (default)
+- fsps,enable-proc-trace: Processor Trace
+- fsps,disable-eist: Eist
+- fsps,boot-p-state: Boot PState
+  0: HFM (default)
+  1: LFM
+- fsps,disable-cx: CPU power states (C-states)
+- fsps,enable-c1e: Enhanced C-states
+- fsps,disable-bi-proc-hot: Bi-Directional PROCHOT#
+- fsps,pkg-c-state-limit: Max Pkg Cstate
+  0: PkgC0C1
+  1: PkgC2
+  2: PkgC3 (default)
+  3: PkgC6
+  4: PkgC7
+  5: PkgC7s
+  6: PkgC8
+  7: PkgC9
+  8: PkgC10
+  9: PkgCMax
+  254: PkgCpuDefault
+  255: PkgAuto
+- fsps,c-state-auto-demotion: C-State auto-demotion
+  0: Disable C1 and C3 Auto-demotion (default)
+  1: Enable C3/C6/C7 Auto-demotion to C1
+  2: Enable C6/C7 Auto-demotion to C3
+  3: Enable C6/C7 Auto-demotion to C1 and C3
+- fsps,c-state-un-demotion: C-State un-demotion
+  0: Disable C1 and C3 Un-demotion (default)
+  1: Enable C1 Un-demotion
+  2: Enable C3 Un-demotion
+  3: Enable C1 and C3 Un-demotion
+- fsps,max-core-c-state: Max Core C-State
+  0: Unlimited
+  1: C1
+  2: C3
+  3: C6
+  4: C7
+  5: C8
+  6: C9
+  7: C10
+  8: CCx (default)
+- fsps,enable-pkg-c-state-demotion: Package C-State Demotion
+- fsps,disable-pkg-c-state-un-demotion: Package C-State Un-demotion
+- fsps,disable-turbo-mode: Turbo Mode
+- fsps,hda-verb-table-entry-num: SC HDA Verb Table Entry Number
+  0: (default)
+- fsps,hda-verb-table-ptr: SC HDA Verb Table Pointer
+  0x00000000: (default)
+- fsps,enable-p2sb-unhide: P2SB device hidden
+- fsps,disable-ipu: IPU
+- fsps,enable-ipu-acpi-mode: IMGU ACPI mode selection
+  0: Auto
+  1: IGFX Child device (default)
+  2: ACPI device
+- fsps,enable-force-wake: ForceWake
+- fsps,gtt-mm-adr: GttMmAdr
+  0xbf000000: (default)
+- fsps,gm-adr: GmAdr
+  0xa0000000: (default)
+- fsps,enable-pavp-lock: PavpLock
+- fsps,enable-graphics-freq-modify: GraphicsFreqModify
+- fsps,enable-graphics-freq-req: GraphicsFreqReq
+- fsps,enable-graphics-video-freq: GraphicsVideoFreq
+- fsps,enable-pm-lock: PmLock
+- fsps,enable-dop-clock-gating: DopClockGating
+- fsps,enable-unsolicited-attack-override: UnsolicitedAttackOverride
+- fsps,enable-wopcm-support: WOPCMSupport
+- fsps,enable-wopcm-size: WOPCMSize
+- fsps,enable-power-gating: PowerGating
+- fsps,enable-unit-level-clock-gating: UnitLevelClockGating
+- fsps,enable-fast-boot: FastBoot
+- fsps,enable-dyn-sr: DynSR
+- fsps,enable-sa-ipu: SaIpuEnable
+- fsps,disable-pm-support: GT PM Support
+- fsps,disable-render-standby: RC6(Render Standby)
+- fsps,disable-pavp: PavpLock
+- fsps,disable-pavp-pr3: PAVP PR3
+- fsps,cd-clock: CdClock Frequency selection
+  0: 144MHz
+  1: 288MHz
+  2: 384MHz
+  3: 576MHz
+  4: 624MHz (default)
+- fsps,disable-pei-graphics-peim-init: PeiGraphicsPeimInit
+- fsps,enable-write-protection: Write Protection Support
+- fsps,enable-read-protection: Read Protection Support
+- fsps,protected-range-limit: Protected Range Limitation
+  0x0FFF: (default)
+- fsps,protected-range-base: Protected Range Base
+  0x0000: (default)
+- fsps,disable-gmm: SC Gaussian Mixture Models
+- fsps,disable-clk-gating-pgcb-clk-trunk: GMM Clock Gating - PGCB Clock Trunk
+- fsps,disable-clk-gating-sb: GMM Clock Gating - Sideband
+- fsps,disable-clk-gating-sb-clk-trunk: GMM Clock Gating - Sideband
+- fsps,disable-clk-gating-sb-clk-partition: GMM Clock Gating - Sideband Clock
+  Partition
+- fsps,disable-clk-gating-core: GMM Clock Gating - Core
+- fsps,disable-clk-gating-dma: GMM Clock Gating - DMA
+- fsps,disable-clk-gating-reg-access: GMM Clock Gating - Register Access
+- fsps,disable-clk-gating-host: GMM Clock Gating - Host
+- fsps,disable-clk-gating-partition: GMM Clock Gating - Partition
+- fsps,disable-clk-gating-trunk: Clock Gating - Trunk
+- fsps,disable-hda: HD Audio Support
+- fsps,disable-dsp: HD Audio DSP Support
+- fsps,enable-pme: Azalia wake-on-ring
+- fsps,hd-audio-io-buffer-ownership: HD-Audio I/O Buffer Ownership
+  0: HD-Audio link owns all the I/O buffers (default)
+  1: HD-Audio link owns 4 I/O buffers and I2S port owns 4 I/O buffers
+  3: I2S port owns all the I/O buffers
+- fsps,hd-audio-io-buffer-voltage: HD-Audio I/O Buffer Voltage
+  0: 3.3V (default)
+  1: 1.8V
+- fsps,hd-audio-vc-type: HD-Audio Virtual Channel Type
+  0: VC0 (default)
+  1: VC1
+- fsps,hd-audio-link-frequency: HD-Audio Link Frequency
+  0: 6MHz (default)
+  1: 12MHz
+  2: 24MHz
+  3: 48MHz
+  4: 96MHz
+  5: Invalid
+- fsps,hd-audio-i-disp-link-frequency: HD-Audio iDisp-Link Frequency
+  0: 6MHz (default)
+  1: 12MHz
+  2: 24MHz
+  3: 48MHz
+  4: 96MHz
+  5: Invalid
+- fsps,hd-audio-i-disp-link-tmode: HD-Audio iDisp-Link T-Mode
+  0: 2T (default)
+  1: 1T
+- fsps,dsp-endpoint-dmic: HD-Audio Disp DMIC
+  0: dsable,
+  1: 2ch array (default)
+  2: 4ch array
+- fsps,disable-dsp-endpoint-bluetooth: HD-Audio Bluetooth
+- fsps,enable-dsp-endpoint-i2s-skp: HD-Audio I2S SHK
+- fsps,enable-dsp-endpoint-i2s-hp: HD-Audio I2S HP
+- fsps,enable-audio-ctl-pwr-gate: HD-Audio Controller Power Gating (deprecated)
+- fsps,enable-audio-dsp-pwr-gate: HD-Audio ADSP Power Gating (deprecated)
+- fsps,mmt: HD-Audio CSME Memory Transfers
+  0: VC0 (default)
+  1: VC2
+- fsps,hmt: HD-Audio Host Memory Transfers
+  0: VC0 (default)
+  1: VC2
+- fsps,enable-hd-audio-pwr-gate: HD-Audio Power Gating
+- fsps,enable-hd-audio-clk-gate: HD-Audio Clock Gating
+- fsps,dsp-feature-mask: Bitmask of DSP Feature
+  0x01: WoV
+  0x02: BT Sideband
+  0x04: Codec VAD
+  0x20: BT Intel HFP
+  0x40: BT Intel A2DP
+  0x80: DSP based speech pre-processing disabled
+- fsps,dsp-pp-module-mask: Bitmask of supported DSP Post-Processing Modules
+  0x01: WoV
+  0x02: BT Sideband
+  0x04: Codec VAD
+  0x20: BT Intel HFP
+  0x40: BT Intel A2DP
+  0x80: DSP based speech pre-processing disabled
+- fsps,enable-bios-cfg-lock-down: HD-Audio BIOS Configuration Lock Down
+- fsps,disable-hpet: Enable High Precision Timer
+- fsps,enable-hpet-bdf-valid: Hpet Valid BDF Value
+- fsps,hpet-bus-number: Bus Number of Hpet
+  0xFA: (default)
+- fsps,hpet-device-number: Device Number of Hpet
+  0x1F: (default)
+- fsps,hpet-function-number: Function Number of Hpet
+  0x00: (default)
+- fsps,enable-io-apic-bdf-valid: IoApic Valid BDF Value
+- fsps,io-apic-bus-number: Bus Number of IoApic
+  0xFA: (default)
+- fsps,io-apic-device-number: Device Number of IoApic
+  0x0F: (default)
+- fsps,io-apic-function-number: Function Number of IoApic
+  0x00: (default)
+- fsps,disable-io-apic-entry24-119: IOAPIC Entry 24-119
+- fsps,io-apic-id: IO APIC ID
+  0x01: (default)
+- fsps,io-apic-range-select: IoApic Range
+  0x00: (default)
+- fsps,disable-ish: ISH Controller
+- fsps,disable-bios-interface: BIOS Interface Lock Down
+- fsps,enable-bios-lock: Bios LockDown
+- fsps,disable-spi-eiss: SPI EISS Status
+- fsps,bios-lock-sw-smi-number: BiosLock SWSMI Number
+  0xA9: (default)
+- fsps,enable-lpss-s0ix: LPSS IOSF PMCTL S0ix Enable
+- fsps,i2c-clk-gate-cfg: LPSS I2C Clock Gating Configuration
+- fsps,hsuart-clk-gate-cfg: LPSS HSUART Clock Gating Configuration
+- fsps,spi-clk-gate-cfg: LPSS SPI Clock Gating Configuration
+- fsps,enable-i2cX: 2C Device X
+  0: Disabled
+  1: PCI Mode (default)
+  2: ACPI Mode
+- fsps,enable-hsuartX: UART Device X
+  0: Disabled
+  1: PCI Mode (default)
+  2: ACPI Mode
+- fsps,enable-spiX: SPI UART Device X
+  0: Disabled
+  1: PCI Mode (default)
+  2: ACPI Mode
+- fsps,enable-os-dbg: OS Debug Feature
+- fsps,enable-dci: DCI Feature
+- fsps,uart2-kernel-debug-base-address: UART Debug Base Address
+  0x00000000: (default)
+- fsps,disable-pcie-clock-gating: PCIE Clock Gating
+- fsps,disable-pcie-root-port8xh-decode: PCIE Root Port 8xh Decode
+- fsps,pcie8xh-decode-port-index: PCIE 8xh Decode Port Index
+  0x00: (default)
+- fsps,enable-pcie-root-port-peer-memory-write: CIE Root Port Peer Memory Write
+- fsps,pcie-aspm-sw-smi-number: PCIE SWSMI Number
+  0xAA: (default)
+- fsps,enable-pcie-root-port: PCI Express Root Port
+- fsps,pcie-rp-hide: Hide PCIE Root Port Configuration Space
+- fsps,pcie-rp-slot-implemented: PCIE Root Port Slot Implement
+- fsps,pcie-rp-hot-plug: Hot Plug
+- fsps,pcie-rp-pm-sci: PCIE PM SCI
+- fsps,pcie-rp-ext-sync: PCIE Root Port Extended Sync
+- fsps,pcie-rp-transmitter-half-swing: Transmitter Half Swing
+- fsps,pcie-rp-acs: ACS
+- fsps,pcie-rp-clk-req-supported: Clock Request Support
+- fsps,pcie-rp-clk-req-number: Configure CLKREQ Number
+- fsps,pcie-rp-clk-req-detect: CLKREQ# Detection
+- fsps,advanced-error-reportingt: Advanced Error Reporting
+- fsps,pme-interrupt: PME Interrupt
+- fsps,fatal-error-report: URR
+- fsps,no-fatal-error-report: FER
+- fsps,correctable-error-report: NFER
+- fsps,system-error-on-fatal-error: CER
+- fsps,system-error-on-non-fatal-error: SEFE
+- fsps,system-error-on-correctable-error: SENFE
+- fsps,pcie-rp-speed: SECE
+- fsps,physical-slot-number: PCIe Speed
+  0: Auto (default)
+  1: Gen1
+  2: Gen2
+  3: Gen3
+- fsps,pcie-rp-completion-timeout: Physical Slot Number
+  0x00, 0x01, 0x02, 0x03, 0x04, 0x05 (default)
+- fsps,enable-ptm: PTM Support
+- fsps,pcie-rp-aspm: ASPM
+- fsps,pcie-rp-l1-substates: L1 Substates
+- fsps,pcie-rp-ltr-enable: PCH PCIe LTR
+- fsps,pcie-rp-ltr-config-lock: PCIE LTR Lock
+- fsps,disable-pme-b0-s5: PME_B0_S5 Disable bit
+- fsps,enable-pci-clock-run: PCI Clock Run
+- fsps,enable-timer8254-clk-setting: Timer 8254 Clock Setting
+- fsps,disable-sata: Chipset SATA
+- fsps,sata-mode: SATA Mode Selection
+  0: AHCI (default)
+  1: RAID
+- fsps,disable-sata-salp-support: Aggressive LPM Support
+- fsps,enable-sata-pwr-opt: SATA Power Optimization
+- fsps,enable-e-sata-speed-limit: eSATA Speed Limit
+- fsps,speed-limit: SATA Speed Limit
+  0x1: 1.5Gb/s(Gen 1)
+  0x2: 3Gb/s(Gen 2)
+  0x3: 6Gb/s(Gen 3)
+- fsps,enable-sata-ports: SATA Port
+- fsps,sata-ports-dev-slp: SATA Port DevSlp
+- fsps,sata-ports-hot-plug: SATA Port HotPlug
+- fsps,sata-ports-interlock-sw: Mechanical Presence Switch
+- fsps,sata-ports-external: External SATA Ports
+- fsps,sata-ports-spin-up: Spin Up Device
+- fsps,sata-ports-solid-state-drive: SATA Solid State
+  0: Hard Disk Drive (default)
+  1: Solid State Drive
+- fsps,enable-sata-ports-dito-config: DITO Configuration
+- fsps,sata-ports-dm-val: DM Value
+  0x0F: Maximum (default)
+- fsps,sata-ports-dito-val: DITO Value
+  0x0271 (default)
+- fsps,sub-system-vendor-id: Subsystem Vendor ID
+  0x8086: (default)
+- fsps,sub-system-id: Subsystem ID
+  0x7270: (default)
+- fsps,crid-setting: CRIDSettings
+  0: Disable (default)
+  1: CRID_1
+  2: CRID_2
+  3: CRID_3
+- fsps,reset-select: ResetSelect
+  0x6: warm reset (default)
+  0xE: cold reset
+- fsps,disable-sdcard: SD Card Support (D27:F0)
+- fsps,disable-emmc: SeMMC Support (D28:F0)
+- fsps,emmc-host-max-speed: eMMC Max Speed
+  0: HS400(default)
+  1: HS200
+  2: DDR50
+- fsps,disable-ufs: UFS Support (D29:F0)
+- fsps,disable-sdio: SDIO Support (D30:F0)
+- fsps,enable-gpp-lock: GPP Lock Feature
+- fsps,disable-sirq: Serial IRQ
+- fsps,sirq-mode: Serial IRQ Mode
+  0: Quiet mode (default)
+  1: Continuous mode
+- fsps,start-frame-pulse: Start Frame Pulse Width
+  0: ScSfpw4Clk (default)
+  1: ScSfpw6Clk
+  2: ScSfpw8Clk
+- fsps,disable-smbus: SMBus
+- fsps,disable-arp: SMBus ARP Support
+- fsps,num-rsvd-smbus-addresses: SMBus Table Elements
+  0x0080: (default)
+- fsps,rsvd-smbus-address-table: Reserved SMBus Address Table
+   0x00: (default)
+- fsps,disable-compliance-mode: XHCI Disable Compliance Mode
+- fsps,enable-usb-per-port-ctl: USB Per-Port Control
+- fsps,usb30-mode: xHCI Mode
+  0: Disable
+  1: Enable
+  2: Auto (default)
+- fsps,enable-port-usb20: Enable USB2 ports
+- fsps,port-usb20-over-current-pin: USB20 Over Current Pin
+- fsps,usb-otg: XDCI Support
+  0: Disable
+  1: PCI_Mode (default)
+  2: ACPI_mode
+- fsps,enable-hsic-support: XHCI HSIC Support
+- fsps,enable-port-usb30: USB3 ports
+- fsps,port-usb30-over-current-pin: USB30 Over Current Pin
+- fsps,enable-ssic-port: XHCI SSIC Support
+- fsps,disable-dlane-pwr-gating: SSIC Dlane PowerGating
+- fsps,enable-vtd: VT-d
+- fsps,disable-lock-down-global-smi: SMI Lock bit
+- fsps,reset-wait-timer: HDAudio Delay Timer
+  0x012C: (default)
+- fsps,disable-rtc-lock: RTC Lock Bits
+- fsps,enable-safe-test-mode: SATA Test Mode Selection
+- fsps,ssic-rate: XHCI SSIC RATE
+  1: A Series (default)
+  2: B Series
+- fsps,enable-dynamic-power-gating: SMBus Dynamic Power Gating
+- fsps,pcie-rp-ltr-max-snoop-latency: Max Snoop Latency
+  0x0000: (default)
+- fsps,pcie-rp-snoop-latency-override-mode: Snoop Latency Override
+  0: Disable
+  1: Enable
+  2: Auto (default)
+- fsps,pcie-rp-snoop-latency-override-value: Snoop Latency Value
+  0x003C (default)
+- fsps,pcie-rp-snoop-latency-override-multiplier: Snoop Latency Multiplier
+  0: 1ns
+  1: 32ns
+  2: 1024ns (default)
+  3: 32768ns
+  4: 1048576ns
+  5: 33554432ns
+- fsps,enable-skip-mp-init: Skip Multi-Processor Initialization
+- fsps,disable-dci-auto-detect: DCI Auto Detect
+- fsps,pcie-rp-ltr-max-non-snoop-latency: Max Non-Snoop Latency
+  0x0000: (default)
+- fsps,pcie-rp-non-snoop-latency-override-mode: Non Snoop Latency Override
+- fsps,disable-tco-timer-halt-lock: Halt and Lock TCO Timer
+- fsps,pwr-btn-override-period: Power Button Override Period
+  000: 4s (default)
+  001: 6s
+  010: 8s
+  011: 10s
+  100: 12s
+  101: 14s
+- fsps,pcie-rp-non-snoop-latency-override-value:
+  0x003C: (default)
+- fsps,pcie-rp-non-snoop-latency-override-multiplier: Non Snoop Latency Value
+  0: 1ns
+  1: 32ns
+  2: 1024ns (default)
+  3: 32768ns
+  4: 1048576ns
+  5: 33554432ns
+- fsps,pcie-rp-slot-power-limit-scale: PCIE Root Port Slot Power Limit Scale
+  0x00: (default)
+- fsps,pcie-rp-slot-power-limit-value:
+  0x00: (default)
+- fsps,disable-native-power-button: Power Button Native Mode Disable
+- fsps,disable-power-button-debounce-mode: Power Button Debounce Mode
+- fsps,sdio-tx-cmd-cntl: SDIO_TX_CMD_DLL_CNTL
+  0x505: (default)
+- fsps,sdio-tx-data-cntl1: SDIO_TX_DATA_DLL_CNTL1
+  0xE: (default)
+- fsps,sdio-tx-data-cntl2: SDIO_TX_DATA_DLL_CNTL2
+  0x22272828: (default)
+- fsps,sdio-rx-cmd-data-cntl1: SDIO_RX_CMD_DATA_DLL_CNTL1
+  0x16161616: (default)
+- fsps,sdio-rx-cmd-data-cntl2: SDIO_RX_CMD_DATA_DLL_CNTL2
+  0x10000: (default)
+- fsps,sdcard-tx-cmd-cntl: SDCARD_TX_CMD_DLL_CNTL
+  0x505 (default)
+- fsps,sdcard-tx-data-cntl1: SDCARD_TX_DATA_DLL_CNTL1
+  0xA13: (default)
+- fsps,sdcard-tx-data-cntl2: SDCARD_TX_DATA_DLL_CNTL2
+  0x24242828: (default)
+- fsps,sdcard-rx-cmd-data-cntl1: SDCARD_RX_CMD_DATA_DLL_CNTL1
+  0x73A3637 (default)
+- fsps,sdcard-rx-strobe-cntl: SDCARD_RX_STROBE_DLL_CNTL
+  0x0: (default)
+- fsps,sdcard-rx-cmd-data-cntl2: SDCARD_RX_CMD_DATA_DLL_CNTL2
+  0x10000: (default)
+- fsps,emmc-tx-cmd-cntl: EMMC_TX_CMD_DLL_CNTL
+  0x505: (default)
+- fsps,emmc-tx-data-cntl1: EMMC_TX_DATA_DLL_CNTL1
+  0xC11: (default)
+- fsps,emmc-tx-data-cntl2: EMMC_TX_DATA_DLL_CNTL2
+  0x1C2A2927: (default)
+- fsps,emmc-rx-cmd-data-cntl1: EMMC_RX_CMD_DATA_DLL_CNTL1
+  0x000D162F: (default)
+- fsps,emmc-rx-strobe-cntl: EMMC_RX_STROBE_DLL_CNTL
+  0x0a0a: (default)
+- fsps,emmc-rx-cmd-data-cntl2: EMMC_RX_CMD_DATA_DLL_CNTL2
+  0x1003b: (default)
+- fsps,emmc-master-sw-cntl: EMMC_MASTER_DLL_CNTL
+  0x001: (default)
+- fsps,pcie-rp-selectable-deemphasis: PCIe Selectable De-emphasis
+  1: -3.5 dB (default)
+  0: -6 dB
+- fsps,disable-monitor-mwait: Monitor Mwait Enable
+- fsps,enable-hd-audio-dsp-uaa-compliance: Universal Audio Architecture
+  compliance for DSP enabled system
+- fsps,ipc: IRQ Interrupt Polarity Control
+- fsps,sata-ports-disable-dynamic-pg: Disable ModPHY dynamic power gate
+- fsps,enable-init-s3-cpu: Init CPU during S3 resume
+- fsps,enable-skip-punit-init: Skip P-unit Initialization
+- fsps,port-usb20-per-port-tx-pe-half: PerPort Half Bit Pre-emphasis
+- fsps,port-usb20-per-port-pe-txi-set: PerPort HS Pre-emphasis Bias
+- fsps,port-usb20-per-port-txi-set: PerPort HS Transmitter Bias
+- fsps,port-usb20-hs-skew-sel: Select the skew direction for HS transition
+- fsps,port-usb20-i-usb-tx-emphasis-en: PerPort HS Transmitter Emphasis
+- fsps,port-usb20-per-port-rxi-set: PerPort HS Receiver Bias
+- fsps,port-usb20-hs-npre-drv-sel: Delay/skew's strength control for HS driver
+
+Example:
+
+&fsp_s {
+	u-boot,dm-pre-proper;
+
+	fsps,disable-ish;
+	fsps,disable-sata;
+	fsps,enable-pcie-root-port = [00 00 00 00 00 01];
+	fsps,pcie-rp-hot-plug = [00 00 00 00 00 01];
+	fsps,enable-i2c6 = <I2CX_ENABLE_DISABLED>;
+	fsps,enable-i2c7 = <I2CX_ENABLE_DISABLED>;
+	fsps,enable-hsuart3 = <HSUARTX_ENABLE_DISABLED>;
+	fsps,enable-spi1 = <SPIX_ENABLE_DISABLED>;
+	fsps,enable-spi2 = <SPIX_ENABLE_DISABLED>;
+	fsps,disable-sdio;
+	...
+};
-- 
2.26.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC PATCH v2 1/2] arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled
  2020-04-14  9:25 ` [RFC PATCH v2 1/2] arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled Bernhard Messerklinger
@ 2020-04-19 23:37   ` Simon Glass
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2020-04-19 23:37 UTC (permalink / raw)
  To: u-boot

Hi Bernhard,

On Tue, 14 Apr 2020 at 03:26, Bernhard Messerklinger
<bernhard.messerklinger@br-automation.com> wrote:
>
> Only load VBT if it's present in the u-boot.rom.
>
> Signed-off-by: Bernhard Messerklinger <bernhard.messerklinger@br-automation.com>
> ---
>
> Changes in v2: None
>
>  arch/x86/cpu/apollolake/fsp_s.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>

Can this please use if(IS_ENABLED(....)) instead of #if?

Regards,
SImon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree
  2020-04-14  9:25 ` [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree Bernhard Messerklinger
@ 2020-04-19 23:37   ` Simon Glass
  2020-04-20 13:11   ` Antwort: " Bernhard Messerklinger
  1 sibling, 0 replies; 10+ messages in thread
From: Simon Glass @ 2020-04-19 23:37 UTC (permalink / raw)
  To: u-boot

Hi Bernhard,

On Tue, 14 Apr 2020 at 03:26, Bernhard Messerklinger
<bernhard.messerklinger@br-automation.com> wrote:
>
> Move FSP-S configuration to the device-tree like it's already done for
> other SoCs (Baytrail).
>
> Signed-off-by: Bernhard Messerklinger <bernhard.messerklinger@br-automation.com>
> ---
>
> Changes in v2:
> Remove FSP-M binding file
>
>  arch/x86/cpu/apollolake/fsp_s.c               | 1070 +++++++++++------
>  arch/x86/dts/chromebook_coral.dts             |   35 +-
>  .../asm/arch-apollolake/fsp/fsp_s_upd.h       |  268 +++++
>  .../fsp/fsp2/apollolake/fsp-s.txt             |  485 ++++++++
>  4 files changed, 1489 insertions(+), 369 deletions(-)
>  create mode 100644 doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt

Tested on chromebook-coral:
Tested-by: Simon Glass <sjg@chromium.org>

>
> diff --git a/arch/x86/cpu/apollolake/fsp_s.c b/arch/x86/cpu/apollolake/fsp_s.c
> index 458825bc49..7d516adc92 100644
> --- a/arch/x86/cpu/apollolake/fsp_s.c
> +++ b/arch/x86/cpu/apollolake/fsp_s.c
[..]

> +
> +const u8 pcie_rp_clk_req_number_def[6] = { 0x4, 0x5, 0x0, 0x1, 0x2, 0x3 };
> +const u8 physical_slot_number_def[6] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0x5 };
> +const u8 ipc_def[16] = { 0xf8, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> +                       0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
> +const u8 port_usb20_per_port_pe_txi_set_def[8] = { 0x07, 0x06, 0x06, 0x06, 0x07,
> +                                                 0x07, 0x07, 0x01 };
> +const u8 port_usb20_per_port_txi_set_def[8] = { 0x00, 0x00, 0x00, 0x00, 0x00,
> +                                              0x00, 0x00, 0x03};
> +const u8 port_usb20_hs_skew_sel_def[8] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +                                         0x00, 0x01 };
> +const u8 port_usb20_i_usb_tx_emphasis_en_def[8] = { 0x03, 0x03, 0x03, 0x03,
> +                                                  0x03, 0x03, 0x03, 0x01 };
> +const u8 port_usb20_hs_npre_drv_sel_def[8] = { 0x00, 0x00, 0x00, 0x00, 0x00,
> +                                             0x00, 0x00, 0x03 };

Do we actually need these, or does the FSP have these defaults in the
right place anyway?

> +
> +static int read_u8_array(u8 prop[], ofnode node, const char *propname, int sz,
> +                         u8 def)

Please add function comment

>  {
>         const u8 *ptr;

[..]


> +       ptr = ofnode_read_u8_array_ptr(node, propname, sz);
> +       if (ptr) {
> +               memcpy(prop, ptr, sz);
> +       } else {
> +               memset(prop, def, sz);
> +               return -EINVAL;
> +       }
>
>         return 0;
>  }
>
> -static void apl_fsp_silicon_init_params_cb(struct apl_config *apl,
> -                                          struct fsp_s_config *cfg)
> +static int read_u16_array(u16 prop[], ofnode node, const char *propname, int sz,
> +                          u16 def)
>  {

same here

> -       u8 port;
> -
> -       for (port = 0; port < MAX_USB2_PORTS; port++) {
> -               if (apl->usb2eye[port].per_port_tx_pe_half)
> -                       cfg->port_usb20_per_port_tx_pe_half[port] =
> -                               apl->usb2eye[port].per_port_tx_pe_half;
> -
> -               if (apl->usb2eye[port].per_port_pe_txi_set)
> -                       cfg->port_usb20_per_port_pe_txi_set[port] =
> -                               apl->usb2eye[port].per_port_pe_txi_set;
> -
> -               if (apl->usb2eye[port].per_port_txi_set)
> -                       cfg->port_usb20_per_port_txi_set[port] =
> -                               apl->usb2eye[port].per_port_txi_set;
> +       u32 *array_buf;
> +       int ret;
>
> -               if (apl->usb2eye[port].hs_skew_sel)
> -                       cfg->port_usb20_hs_skew_sel[port] =
> -                               apl->usb2eye[port].hs_skew_sel;
> +       array_buf = malloc(sz * sizeof(u32));

Is it possible to use a dynamic array, like

u32 array_buf[siz];

since these arrays are small and free() does not actually do anything
in SPL by default.

> +       if (!array_buf)
> +               return -ENOMEM;
> +
> +       ret = ofnode_read_u32_array(node, propname, array_buf, sz);
> +       if (!ret) {
> +               for (int i = 0; i < sz; i++)
> +                       prop[i] = array_buf[i];
> +       } else if (ret == -EINVAL) {
> +               for (int i = 0; i < sz; i++)
> +                       prop[i] = def;
> +               ret = 0;
> +       }
> +       free(array_buf);
>
> -               if (apl->usb2eye[port].usb_tx_emphasis_en)
> -                       cfg->port_usb20_i_usb_tx_emphasis_en[port] =
> -                               apl->usb2eye[port].usb_tx_emphasis_en;
> +       return ret;
> +}
>
> -               if (apl->usb2eye[port].per_port_rxi_set)
> -                       cfg->port_usb20_per_port_rxi_set[port] =
> -                               apl->usb2eye[port].per_port_rxi_set;
> +static int read_u32_array(u32 prop[], ofnode node, const char *propname, int sz,
> +                          u32 def)
> +{
> +       int ret;
>
> -               if (apl->usb2eye[port].hs_npre_drv_sel)
> -                       cfg->port_usb20_hs_npre_drv_sel[port] =
> -                               apl->usb2eye[port].hs_npre_drv_sel;
> +       ret = ofnode_read_u32_array(node, propname, prop, sz);
> +       if (ret == -EINVAL) {
> +               for (int i = 0; i < sz; i++)
> +                       prop[i] = def;
> +               ret = 0;
>         }
> +
> +       return ret;
>  }
>
[...]

> +#ifdef CONFIG_HAVE_VBT

if (IS_ENABLED(CONFIG_HAVE_VBT)

> +       cfg->graphics_config_ptr = (ulong)vbt_buf;
> +#else
> +       cfg->graphics_config_ptr = 0;
> +#endif

[..]

> +       for (int i = 0; i < FSP_I2C_COUNT; i++)  {
> +               snprintf(buf, sizeof(buf), "fsps,enable-i2c%d", i);
> +               *(&cfg->i2c0_enable + i) = ofnode_read_u32_default(node, buf,
> +                                                       I2CX_ENABLE_PCI_MODE);

cfg->i2c0_enable[i]

same below

> +       }
> +       for (int i = 0; i < FSP_HSUART_COUNT; i++)  {
> +               snprintf(buf, sizeof(buf), "fsps,enable-hsuart%d", i);
> +               *(&cfg->hsuart0_enable + i) = ofnode_read_u32_default(node, buf,
> +                                               HSUARTX_ENABLE_PCI_MODE);
> +       }
> +       for (int i = 0; i < FSP_SPI_COUNT; i++)  {
> +               snprintf(buf, sizeof(buf), "fsps,enable-spi%d", i);
> +               *(&cfg->spi0_enable + i) = ofnode_read_u32_default(node, buf,
> +                                                       SPIX_ENABLE_PCI_MODE);
> +       }
> +       cfg->os_dbg_enable

[..]

Regards,
Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Antwort: Re: [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree
  2020-04-14  9:25 ` [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree Bernhard Messerklinger
  2020-04-19 23:37   ` Simon Glass
@ 2020-04-20 13:11   ` Bernhard Messerklinger
  2020-04-20 14:41     ` Simon Glass
  2020-04-21  7:56     ` Bernhard Messerklinger
  1 sibling, 2 replies; 10+ messages in thread
From: Bernhard Messerklinger @ 2020-04-20 13:11 UTC (permalink / raw)
  To: u-boot

Hi Simon,

>Hi Bernhard,
>
>On Tue, 14 Apr 2020 at 03:26, Bernhard Messerklinger
><bernhard.messerklinger@br-automation.com> wrote:
>>
>> Move FSP-S configuration to the device-tree like it's already done
>for
>> other SoCs (Baytrail).
>>
>> Signed-off-by: Bernhard Messerklinger
><bernhard.messerklinger@br-automation.com>
>> ---
>>
>> Changes in v2:
>> Remove FSP-M binding file
>>
>>  arch/x86/cpu/apollolake/fsp_s.c               | 1070
>+++++++++++------
>>  arch/x86/dts/chromebook_coral.dts             |   35 +-
>>  .../asm/arch-apollolake/fsp/fsp_s_upd.h       |  268 +++++
>>  .../fsp/fsp2/apollolake/fsp-s.txt             |  485 ++++++++
>>  4 files changed, 1489 insertions(+), 369 deletions(-)
>>  create mode 100644
>doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
>
>Tested on chromebook-coral:
>Tested-by: Simon Glass <sjg@chromium.org>
>
>>
>> diff --git a/arch/x86/cpu/apollolake/fsp_s.c
>b/arch/x86/cpu/apollolake/fsp_s.c
>> index 458825bc49..7d516adc92 100644
>> --- a/arch/x86/cpu/apollolake/fsp_s.c
>> +++ b/arch/x86/cpu/apollolake/fsp_s.c
>[..]
>
>> +
>> +const u8 pcie_rp_clk_req_number_def[6] = { 0x4, 0x5, 0x0, 0x1,
>0x2, 0x3 };
>> +const u8 physical_slot_number_def[6] = { 0x0, 0x1, 0x2, 0x3, 0x4,
>0x5 };
>> +const u8 ipc_def[16] = { 0xf8, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff,
>0xff, 0xff,
>> +                       0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>> +const u8 port_usb20_per_port_pe_txi_set_def[8] = { 0x07, 0x06,
>0x06, 0x06, 0x07,
>> +                                                 0x07, 0x07, 0x01
>};
>> +const u8 port_usb20_per_port_txi_set_def[8] = { 0x00, 0x00, 0x00,
>0x00, 0x00,
>> +                                              0x00, 0x00, 0x03};
>> +const u8 port_usb20_hs_skew_sel_def[8] = { 0x00, 0x00, 0x00, 0x00,
>0x00, 0x00,
>> +                                         0x00, 0x01 };
>> +const u8 port_usb20_i_usb_tx_emphasis_en_def[8] = { 0x03, 0x03,
>0x03, 0x03,
>> +                                                  0x03, 0x03,
>0x03, 0x01 };
>> +const u8 port_usb20_hs_npre_drv_sel_def[8] = { 0x00, 0x00, 0x00,
>0x00, 0x00,
>> +                                             0x00, 0x00, 0x03 };
>
>Do we actually need these, or does the FSP have these defaults in the
>right place anyway?

The FSP would already have these default values included.
Whether we use them or not is a design decision.

My current approach tries the following:
 * only non-default values should require a devicetree entry
 * boolean FSP parameters are implemented with boolean
   devicetree properties

A limitation for boolean properties in devicetree is that they
are true when they are present, and false when they are not
present. But it is not possible to leave them out and use some
default value in this case.

--> For boolean properties, this patch uses the devicetree value
    (either the entry is present or it is not present)
    unconditionally and overwrites any default values included
    in the FSP.

Non-boolean devicetree properties on the other hand support
default values. But it needs to be decided where these default
values should come from:

   a) from the default values within FSP
   b) from default values within U-Boot

I have implemented option b) in this patch, as for this option
we don't rely on other tools to configure FSP default values
and IMHO it feels slightly more consistent with how boolean
properties are handled.
This is why the variables above are defined.

But I have no strong opinion on this topic, and could implement
it differently depending on the feedback I receive.

Open questions:
   * Should boolean properties of the FSP be implemented by
     boolean devicetree properties? The alternative would be
     to use u32 for everything.
     advantage: support for default values
     drawback:  devicetree gets bigger

   * For non-boolean properties: Should the default value from
     FSP be used, or a default value defined in U-Boot?

Feedback on these questions would be appreciated. 
 
>>> +
>> +static int read_u8_array(u8 prop[], ofnode node, const char
>*propname, int sz,
>> +                         u8 def)
>
>Please add function comment
>
>>  {
>>         const u8 *ptr;
>
>[..]
>
>
>> +       ptr = ofnode_read_u8_array_ptr(node, propname, sz);
>> +       if (ptr) {
>> +               memcpy(prop, ptr, sz);
>> +       } else {
>> +               memset(prop, def, sz);
>> +               return -EINVAL;
>> +       }
>>
>>         return 0;
>>  }
>>
>> -static void apl_fsp_silicon_init_params_cb(struct apl_config *apl,
>> -                                          struct fsp_s_config
>*cfg)
>> +static int read_u16_array(u16 prop[], ofnode node, const char
>*propname, int sz,
>> +                          u16 def)
>>  {
>
>same here
>
>> -       u8 port;
>> -
>> -       for (port = 0; port < MAX_USB2_PORTS; port++) {
>> -               if (apl->usb2eye[port].per_port_tx_pe_half)
>> -                       cfg->port_usb20_per_port_tx_pe_half[port] =
>> -
>apl->usb2eye[port].per_port_tx_pe_half;
>> -
>> -               if (apl->usb2eye[port].per_port_pe_txi_set)
>> -                       cfg->port_usb20_per_port_pe_txi_set[port] =
>> -
>apl->usb2eye[port].per_port_pe_txi_set;
>> -
>> -               if (apl->usb2eye[port].per_port_txi_set)
>> -                       cfg->port_usb20_per_port_txi_set[port] =
>> -
>apl->usb2eye[port].per_port_txi_set;
>> +       u32 *array_buf;
>> +       int ret;
>>
>> -               if (apl->usb2eye[port].hs_skew_sel)
>> -                       cfg->port_usb20_hs_skew_sel[port] =
>> -                               apl->usb2eye[port].hs_skew_sel;
>> +       array_buf = malloc(sz * sizeof(u32));
>
>Is it possible to use a dynamic array, like
>
>u32 array_buf[siz];
>
>since these arrays are small and free() does not actually do anything
>in SPL by default.
>
>> +       if (!array_buf)
>> +               return -ENOMEM;
>> +
>> +       ret = ofnode_read_u32_array(node, propname, array_buf, sz);
>> +       if (!ret) {
>> +               for (int i = 0; i < sz; i++)
>> +                       prop[i] = array_buf[i];
>> +       } else if (ret == -EINVAL) {
>> +               for (int i = 0; i < sz; i++)
>> +                       prop[i] = def;
>> +               ret = 0;
>> +       }
>> +       free(array_buf);
>>
>> -               if (apl->usb2eye[port].usb_tx_emphasis_en)
>> -                       cfg->port_usb20_i_usb_tx_emphasis_en[port]
>=
>> -
>apl->usb2eye[port].usb_tx_emphasis_en;
>> +       return ret;
>> +}
>>
>> -               if (apl->usb2eye[port].per_port_rxi_set)
>> -                       cfg->port_usb20_per_port_rxi_set[port] =
>> -
>apl->usb2eye[port].per_port_rxi_set;
>> +static int read_u32_array(u32 prop[], ofnode node, const char
>*propname, int sz,
>> +                          u32 def)
>> +{
>> +       int ret;
>>
>> -               if (apl->usb2eye[port].hs_npre_drv_sel)
>> -                       cfg->port_usb20_hs_npre_drv_sel[port] =
>> -                               apl->usb2eye[port].hs_npre_drv_sel;
>> +       ret = ofnode_read_u32_array(node, propname, prop, sz);
>> +       if (ret == -EINVAL) {
>> +               for (int i = 0; i < sz; i++)
>> +                       prop[i] = def;
>> +               ret = 0;
>>         }
>> +
>> +       return ret;
>>  }
>>
>[...]
>
>> +#ifdef CONFIG_HAVE_VBT
>
>if (IS_ENABLED(CONFIG_HAVE_VBT)
>
>> +       cfg->graphics_config_ptr = (ulong)vbt_buf;
>> +#else
>> +       cfg->graphics_config_ptr = 0;
>> +#endif
>
>[..]
>
>> +       for (int i = 0; i < FSP_I2C_COUNT; i++)  {
>> +               snprintf(buf, sizeof(buf), "fsps,enable-i2c%d", i);
>> +               *(&cfg->i2c0_enable + i) =
>ofnode_read_u32_default(node, buf,
>> +
>I2CX_ENABLE_PCI_MODE);
>
>cfg->i2c0_enable[i]
>
>same below
>
>> +       }
>> +       for (int i = 0; i < FSP_HSUART_COUNT; i++)  {
>> +               snprintf(buf, sizeof(buf), "fsps,enable-hsuart%d",
>i);
>> +               *(&cfg->hsuart0_enable + i) =
>ofnode_read_u32_default(node, buf,
>> +
>HSUARTX_ENABLE_PCI_MODE);
>> +       }
>> +       for (int i = 0; i < FSP_SPI_COUNT; i++)  {
>> +               snprintf(buf, sizeof(buf), "fsps,enable-spi%d", i);
>> +               *(&cfg->spi0_enable + i) =
>ofnode_read_u32_default(node, buf,
>> +
>SPIX_ENABLE_PCI_MODE);
>> +       }
>> +       cfg->os_dbg_enable
>
>[..]
>
>Regards,
>Simon
>

Regards,
Bernhard

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree
  2020-04-20 13:11   ` Antwort: " Bernhard Messerklinger
@ 2020-04-20 14:41     ` Simon Glass
  2020-04-21  7:56     ` Bernhard Messerklinger
  1 sibling, 0 replies; 10+ messages in thread
From: Simon Glass @ 2020-04-20 14:41 UTC (permalink / raw)
  To: u-boot

Hi Bernhard,

On Mon, 20 Apr 2020 at 07:11, Bernhard Messerklinger
<bernhard.messerklinger@br-automation.com> wrote:
>
> Hi Simon,
>
> >Hi Bernhard,
> >
> >On Tue, 14 Apr 2020 at 03:26, Bernhard Messerklinger
> ><bernhard.messerklinger@br-automation.com> wrote:
> >>
> >> Move FSP-S configuration to the device-tree like it's already done
> >for
> >> other SoCs (Baytrail).
> >>
> >> Signed-off-by: Bernhard Messerklinger
> ><bernhard.messerklinger@br-automation.com>
> >> ---
> >>
> >> Changes in v2:
> >> Remove FSP-M binding file
> >>
> >>  arch/x86/cpu/apollolake/fsp_s.c               | 1070
> >+++++++++++------
> >>  arch/x86/dts/chromebook_coral.dts             |   35 +-
> >>  .../asm/arch-apollolake/fsp/fsp_s_upd.h       |  268 +++++
> >>  .../fsp/fsp2/apollolake/fsp-s.txt             |  485 ++++++++
> >>  4 files changed, 1489 insertions(+), 369 deletions(-)
> >>  create mode 100644
> >doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
> >
> >Tested on chromebook-coral:
> >Tested-by: Simon Glass <sjg@chromium.org>
> >
> >>
> >> diff --git a/arch/x86/cpu/apollolake/fsp_s.c
> >b/arch/x86/cpu/apollolake/fsp_s.c
> >> index 458825bc49..7d516adc92 100644
> >> --- a/arch/x86/cpu/apollolake/fsp_s.c
> >> +++ b/arch/x86/cpu/apollolake/fsp_s.c
> >[..]
> >
> >> +
> >> +const u8 pcie_rp_clk_req_number_def[6] = { 0x4, 0x5, 0x0, 0x1,
> >0x2, 0x3 };
> >> +const u8 physical_slot_number_def[6] = { 0x0, 0x1, 0x2, 0x3, 0x4,
> >0x5 };
> >> +const u8 ipc_def[16] = { 0xf8, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff,
> >0xff, 0xff,
> >> +                       0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
> >> +const u8 port_usb20_per_port_pe_txi_set_def[8] = { 0x07, 0x06,
> >0x06, 0x06, 0x07,
> >> +                                                 0x07, 0x07, 0x01
> >};
> >> +const u8 port_usb20_per_port_txi_set_def[8] = { 0x00, 0x00, 0x00,
> >0x00, 0x00,
> >> +                                              0x00, 0x00, 0x03};
> >> +const u8 port_usb20_hs_skew_sel_def[8] = { 0x00, 0x00, 0x00, 0x00,
> >0x00, 0x00,
> >> +                                         0x00, 0x01 };
> >> +const u8 port_usb20_i_usb_tx_emphasis_en_def[8] = { 0x03, 0x03,
> >0x03, 0x03,
> >> +                                                  0x03, 0x03,
> >0x03, 0x01 };
> >> +const u8 port_usb20_hs_npre_drv_sel_def[8] = { 0x00, 0x00, 0x00,
> >0x00, 0x00,
> >> +                                             0x00, 0x00, 0x03 };
> >
> >Do we actually need these, or does the FSP have these defaults in the
> >right place anyway?
>
> The FSP would already have these default values included.
> Whether we use them or not is a design decision.
>
> My current approach tries the following:
>  * only non-default values should require a devicetree entry
>  * boolean FSP parameters are implemented with boolean
>    devicetree properties
>
> A limitation for boolean properties in devicetree is that they
> are true when they are present, and false when they are not
> present. But it is not possible to leave them out and use some
> default value in this case.
>
> --> For boolean properties, this patch uses the devicetree value
>     (either the entry is present or it is not present)
>     unconditionally and overwrites any default values included
>     in the FSP.

I think it would be better to use int properties for the booleans.
Perhaps we can add a new dev_read_opt_bool() to handle it.

>
> Non-boolean devicetree properties on the other hand support
> default values. But it needs to be decided where these default
> values should come from:
>
>    a) from the default values within FSP
>    b) from default values within U-Boot

OK. I had assumed that U-Boot itself wouldn't have any particular defaults.

>
> I have implemented option b) in this patch, as for this option
> we don't rely on other tools to configure FSP default values
> and IMHO it feels slightly more consistent with how boolean
> properties are handled.
> This is why the variables above are defined.
>
> But I have no strong opinion on this topic, and could implement
> it differently depending on the feedback I receive.

Neither do I, so let's keep it with the defaults in U-Boot as you have it.

>
> Open questions:
>    * Should boolean properties of the FSP be implemented by
>      boolean devicetree properties? The alternative would be
>      to use u32 for everything.
>      advantage: support for default values
>      drawback:  devicetree gets bigger

Yes we should support u32. It only increases the size by 4 bytes for each one.

>
>    * For non-boolean properties: Should the default value from
>      FSP be used, or a default value defined in U-Boot?

Let's go with what you have then as I don't have any info to suggest
we should change it.

>
> Feedback on these questions would be appreciated.
>

[..]

Regards,
Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree
  2020-04-20 13:11   ` Antwort: " Bernhard Messerklinger
  2020-04-20 14:41     ` Simon Glass
@ 2020-04-21  7:56     ` Bernhard Messerklinger
  2020-04-21 12:28       ` Simon Glass
  1 sibling, 1 reply; 10+ messages in thread
From: Bernhard Messerklinger @ 2020-04-21  7:56 UTC (permalink / raw)
  To: u-boot

Hi Simon,
>
>Hi Bernhard,
>
>On Mon, 20 Apr 2020 at 07:11, Bernhard Messerklinger
><bernhard.messerklinger@br-automation.com> wrote:
>>
>> Hi Simon,
>>
>> >Hi Bernhard,
>> >
>> >On Tue, 14 Apr 2020 at 03:26, Bernhard Messerklinger
>> ><bernhard.messerklinger@br-automation.com> wrote:
>> >>
>> >> Move FSP-S configuration to the device-tree like it's already
>done
>> >for
>> >> other SoCs (Baytrail).
>> >>
>> >> Signed-off-by: Bernhard Messerklinger
>> ><bernhard.messerklinger@br-automation.com>
>> >> ---
>> >>
>> >> Changes in v2:
>> >> Remove FSP-M binding file
>> >>
>> >>  arch/x86/cpu/apollolake/fsp_s.c               | 1070
>> >+++++++++++------
>> >>  arch/x86/dts/chromebook_coral.dts             |   35 +-
>> >>  .../asm/arch-apollolake/fsp/fsp_s_upd.h       |  268 +++++
>> >>  .../fsp/fsp2/apollolake/fsp-s.txt             |  485 ++++++++
>> >>  4 files changed, 1489 insertions(+), 369 deletions(-)
>> >>  create mode 100644
>> >doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
>> >
>> >Tested on chromebook-coral:
>> >Tested-by: Simon Glass <sjg@chromium.org>
>> >
>> >>
>> >> diff --git a/arch/x86/cpu/apollolake/fsp_s.c
>> >b/arch/x86/cpu/apollolake/fsp_s.c
>> >> index 458825bc49..7d516adc92 100644
>> >> --- a/arch/x86/cpu/apollolake/fsp_s.c
>> >> +++ b/arch/x86/cpu/apollolake/fsp_s.c
>> >[..]
>> >
>> >> +
>> >> +const u8 pcie_rp_clk_req_number_def[6] = { 0x4, 0x5, 0x0, 0x1,
>> >0x2, 0x3 };
>> >> +const u8 physical_slot_number_def[6] = { 0x0, 0x1, 0x2, 0x3,
>0x4,
>> >0x5 };
>> >> +const u8 ipc_def[16] = { 0xf8, 0xef, 0xff, 0xff, 0xff, 0xff,
>0xff,
>> >0xff, 0xff,
>> >> +                       0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
>};
>> >> +const u8 port_usb20_per_port_pe_txi_set_def[8] = { 0x07, 0x06,
>> >0x06, 0x06, 0x07,
>> >> +                                                 0x07, 0x07,
>0x01
>> >};
>> >> +const u8 port_usb20_per_port_txi_set_def[8] = { 0x00, 0x00,
>0x00,
>> >0x00, 0x00,
>> >> +                                              0x00, 0x00,
>0x03};
>> >> +const u8 port_usb20_hs_skew_sel_def[8] = { 0x00, 0x00, 0x00,
>0x00,
>> >0x00, 0x00,
>> >> +                                         0x00, 0x01 };
>> >> +const u8 port_usb20_i_usb_tx_emphasis_en_def[8] = { 0x03, 0x03,
>> >0x03, 0x03,
>> >> +                                                  0x03, 0x03,
>> >0x03, 0x01 };
>> >> +const u8 port_usb20_hs_npre_drv_sel_def[8] = { 0x00, 0x00,
>0x00,
>> >0x00, 0x00,
>> >> +                                             0x00, 0x00, 0x03
>};
>> >
>> >Do we actually need these, or does the FSP have these defaults in
>the
>> >right place anyway?
>>
>> The FSP would already have these default values included.
>> Whether we use them or not is a design decision.
>>
>> My current approach tries the following:
>>  * only non-default values should require a devicetree entry
>>  * boolean FSP parameters are implemented with boolean
>>    devicetree properties
>>
>> A limitation for boolean properties in devicetree is that they
>> are true when they are present, and false when they are not
>> present. But it is not possible to leave them out and use some
>> default value in this case.
>>
>> --> For boolean properties, this patch uses the devicetree value
>>     (either the entry is present or it is not present)
>>     unconditionally and overwrites any default values included
>>     in the FSP.
>
>I think it would be better to use int properties for the booleans.
>Perhaps we can add a new dev_read_opt_bool() to handle it.
>
>>
>> Non-boolean devicetree properties on the other hand support
>> default values. But it needs to be decided where these default
>> values should come from:
>>
>>    a) from the default values within FSP
>>    b) from default values within U-Boot
>
>OK. I had assumed that U-Boot itself wouldn't have any particular
>defaults.
>
>>
>> I have implemented option b) in this patch, as for this option
>> we don't rely on other tools to configure FSP default values
>> and IMHO it feels slightly more consistent with how boolean
>> properties are handled.
>> This is why the variables above are defined.
>>
>> But I have no strong opinion on this topic, and could implement
>> it differently depending on the feedback I receive.
>
>Neither do I, so let's keep it with the defaults in U-Boot as you
>have it.
>
>>
>> Open questions:
>>    * Should boolean properties of the FSP be implemented by
>>      boolean devicetree properties? The alternative would be
>>      to use u32 for everything.
>>      advantage: support for default values
>>      drawback:  devicetree gets bigger
>
>Yes we should support u32. It only increases the size by 4 bytes for
>each one.
>
>>
>>    * For non-boolean properties: Should the default value from
>>      FSP be used, or a default value defined in U-Boot?
>
>Let's go with what you have then as I don't have any info to suggest
>we should change it.

Thanks for your fast feedback.

The main reason why I added default values within U-Boot is
to avoid inconsistency between boolean and u32 properties.

I agree that we should use u32 for all parameters, but in this
case we can drop the default values in U-Boot and directly use
the default values in the FSP.

This would mean:
1.) All devicetree bool parameters are changed to u32.
2.) If a parameter is not specified in the devicetree, the
    default value configured in the FSP is used.

Advantages:
1.) U-boot can boot without any devicetree FPS parameters
if the FSP was already configured with the Intel bct tool.

2.) There is no need to maintain default FSP values in U-Boot.
New FSP releases can be used without checking for changes in
their default settings.

What do you think about this?

>> Feedback on these questions would be appreciated.
>>
>
>[..]
>
>Regards,
>Simon
>
Regards,
Bernhard

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree
  2020-04-21  7:56     ` Bernhard Messerklinger
@ 2020-04-21 12:28       ` Simon Glass
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2020-04-21 12:28 UTC (permalink / raw)
  To: u-boot

Hi Bernhard,

On Tue, 21 Apr 2020 at 01:57, Bernhard Messerklinger
<bernhard.messerklinger@br-automation.com> wrote:
>
> Hi Simon,
> >
> >Hi Bernhard,
> >
> >On Mon, 20 Apr 2020 at 07:11, Bernhard Messerklinger
> ><bernhard.messerklinger@br-automation.com> wrote:
> >>
> >> Hi Simon,
> >>
> >> >Hi Bernhard,
> >> >
> >> >On Tue, 14 Apr 2020 at 03:26, Bernhard Messerklinger
> >> ><bernhard.messerklinger@br-automation.com> wrote:
> >> >>
> >> >> Move FSP-S configuration to the device-tree like it's already
> >done
> >> >for
> >> >> other SoCs (Baytrail).
> >> >>
> >> >> Signed-off-by: Bernhard Messerklinger
> >> ><bernhard.messerklinger@br-automation.com>
> >> >> ---
> >> >>
> >> >> Changes in v2:
> >> >> Remove FSP-M binding file
> >> >>
> >> >>  arch/x86/cpu/apollolake/fsp_s.c               | 1070
> >> >+++++++++++------
> >> >>  arch/x86/dts/chromebook_coral.dts             |   35 +-
> >> >>  .../asm/arch-apollolake/fsp/fsp_s_upd.h       |  268 +++++
> >> >>  .../fsp/fsp2/apollolake/fsp-s.txt             |  485 ++++++++
> >> >>  4 files changed, 1489 insertions(+), 369 deletions(-)
> >> >>  create mode 100644
> >> >doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
> >> >
> >> >Tested on chromebook-coral:
> >> >Tested-by: Simon Glass <sjg@chromium.org>
> >> >
> >> >>
> >> >> diff --git a/arch/x86/cpu/apollolake/fsp_s.c
> >> >b/arch/x86/cpu/apollolake/fsp_s.c
> >> >> index 458825bc49..7d516adc92 100644
> >> >> --- a/arch/x86/cpu/apollolake/fsp_s.c
> >> >> +++ b/arch/x86/cpu/apollolake/fsp_s.c
> >> >[..]
> >> >
> >> >> +
> >> >> +const u8 pcie_rp_clk_req_number_def[6] = { 0x4, 0x5, 0x0, 0x1,
> >> >0x2, 0x3 };
> >> >> +const u8 physical_slot_number_def[6] = { 0x0, 0x1, 0x2, 0x3,
> >0x4,
> >> >0x5 };
> >> >> +const u8 ipc_def[16] = { 0xf8, 0xef, 0xff, 0xff, 0xff, 0xff,
> >0xff,
> >> >0xff, 0xff,
> >> >> +                       0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
> >};
> >> >> +const u8 port_usb20_per_port_pe_txi_set_def[8] = { 0x07, 0x06,
> >> >0x06, 0x06, 0x07,
> >> >> +                                                 0x07, 0x07,
> >0x01
> >> >};
> >> >> +const u8 port_usb20_per_port_txi_set_def[8] = { 0x00, 0x00,
> >0x00,
> >> >0x00, 0x00,
> >> >> +                                              0x00, 0x00,
> >0x03};
> >> >> +const u8 port_usb20_hs_skew_sel_def[8] = { 0x00, 0x00, 0x00,
> >0x00,
> >> >0x00, 0x00,
> >> >> +                                         0x00, 0x01 };
> >> >> +const u8 port_usb20_i_usb_tx_emphasis_en_def[8] = { 0x03, 0x03,
> >> >0x03, 0x03,
> >> >> +                                                  0x03, 0x03,
> >> >0x03, 0x01 };
> >> >> +const u8 port_usb20_hs_npre_drv_sel_def[8] = { 0x00, 0x00,
> >0x00,
> >> >0x00, 0x00,
> >> >> +                                             0x00, 0x00, 0x03
> >};
> >> >
> >> >Do we actually need these, or does the FSP have these defaults in
> >the
> >> >right place anyway?
> >>
> >> The FSP would already have these default values included.
> >> Whether we use them or not is a design decision.
> >>
> >> My current approach tries the following:
> >>  * only non-default values should require a devicetree entry
> >>  * boolean FSP parameters are implemented with boolean
> >>    devicetree properties
> >>
> >> A limitation for boolean properties in devicetree is that they
> >> are true when they are present, and false when they are not
> >> present. But it is not possible to leave them out and use some
> >> default value in this case.
> >>
> >> --> For boolean properties, this patch uses the devicetree value
> >>     (either the entry is present or it is not present)
> >>     unconditionally and overwrites any default values included
> >>     in the FSP.
> >
> >I think it would be better to use int properties for the booleans.
> >Perhaps we can add a new dev_read_opt_bool() to handle it.
> >
> >>
> >> Non-boolean devicetree properties on the other hand support
> >> default values. But it needs to be decided where these default
> >> values should come from:
> >>
> >>    a) from the default values within FSP
> >>    b) from default values within U-Boot
> >
> >OK. I had assumed that U-Boot itself wouldn't have any particular
> >defaults.
> >
> >>
> >> I have implemented option b) in this patch, as for this option
> >> we don't rely on other tools to configure FSP default values
> >> and IMHO it feels slightly more consistent with how boolean
> >> properties are handled.
> >> This is why the variables above are defined.
> >>
> >> But I have no strong opinion on this topic, and could implement
> >> it differently depending on the feedback I receive.
> >
> >Neither do I, so let's keep it with the defaults in U-Boot as you
> >have it.
> >
> >>
> >> Open questions:
> >>    * Should boolean properties of the FSP be implemented by
> >>      boolean devicetree properties? The alternative would be
> >>      to use u32 for everything.
> >>      advantage: support for default values
> >>      drawback:  devicetree gets bigger
> >
> >Yes we should support u32. It only increases the size by 4 bytes for
> >each one.
> >
> >>
> >>    * For non-boolean properties: Should the default value from
> >>      FSP be used, or a default value defined in U-Boot?
> >
> >Let's go with what you have then as I don't have any info to suggest
> >we should change it.
>
> Thanks for your fast feedback.
>
> The main reason why I added default values within U-Boot is
> to avoid inconsistency between boolean and u32 properties.
>
> I agree that we should use u32 for all parameters, but in this
> case we can drop the default values in U-Boot and directly use
> the default values in the FSP.
>
> This would mean:
> 1.) All devicetree bool parameters are changed to u32.
> 2.) If a parameter is not specified in the devicetree, the
>     default value configured in the FSP is used.
>
> Advantages:
> 1.) U-boot can boot without any devicetree FPS parameters
> if the FSP was already configured with the Intel bct tool.
>
> 2.) There is no need to maintain default FSP values in U-Boot.
> New FSP releases can be used without checking for changes in
> their default settings.
>
> What do you think about this?

It sounds good to me.

Bin, do you have any thoughts on this?

Regards,
Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH v2 0/2] Move FSP-S configuration to device-tree
  2020-04-14  9:25 [RFC PATCH v2 0/2] Move FSP-S configuration to device-tree Bernhard Messerklinger
  2020-04-14  9:25 ` [RFC PATCH v2 1/2] arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled Bernhard Messerklinger
  2020-04-14  9:25 ` [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree Bernhard Messerklinger
@ 2020-04-30  9:28 ` Bernhard Messerklinger
  2 siblings, 0 replies; 10+ messages in thread
From: Bernhard Messerklinger @ 2020-04-30  9:28 UTC (permalink / raw)
  To: u-boot

Hi Bin, Simon,

>This patch series moves the configuration of FPS-S for Apollo Lake
>based SoCs from the code to the device-tree.
>
>This is similar to the previous patch series for FSP-M.
>If wanted, I can also send FSP-M and FSP-S patch as a single series.
>

Please drop this series.
This series is superseded by:
"Move FSP configuration to devicetree" [1]

[1]: https://lists.denx.de/pipermail/u-boot/2020-April/409505.html

>Changes in v2:
>Remove FSP-M binding file
>
>Bernhard Messerklinger (2):
>  arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled
>  arch: x86: apl: Read FSP-S configuration from device-tree
>
> arch/x86/cpu/apollolake/fsp_s.c               | 1084
>+++++++++++------
> arch/x86/dts/chromebook_coral.dts             |   35 +-
> .../asm/arch-apollolake/fsp/fsp_s_upd.h       |  268 ++++
> .../fsp/fsp2/apollolake/fsp-s.txt             |  485 ++++++++
> 4 files changed, 1497 insertions(+), 375 deletions(-)
> create mode 100644
>doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
>
>-- 
>2.26.0
>
>
Regards,
Bernhard

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-04-30  9:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-14  9:25 [RFC PATCH v2 0/2] Move FSP-S configuration to device-tree Bernhard Messerklinger
2020-04-14  9:25 ` [RFC PATCH v2 1/2] arch: x86: apl: Only load VBT if CONFIG_HAVE_VBT is enabled Bernhard Messerklinger
2020-04-19 23:37   ` Simon Glass
2020-04-14  9:25 ` [RFC PATCH v2 2/2] arch: x86: apl: Read FSP-S configuration from device-tree Bernhard Messerklinger
2020-04-19 23:37   ` Simon Glass
2020-04-20 13:11   ` Antwort: " Bernhard Messerklinger
2020-04-20 14:41     ` Simon Glass
2020-04-21  7:56     ` Bernhard Messerklinger
2020-04-21 12:28       ` Simon Glass
2020-04-30  9:28 ` [RFC PATCH v2 0/2] Move FSP-S configuration to device-tree Bernhard Messerklinger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.