+Bin Hi Andrew,
On 29 June 2015 at 09:10, <and...@bradfordembedded.com> wrote: > > From: Andrew Bradford <andrew.bradf...@kodakalaris.com> > > Allow for configuration of FSP UPD from the device tree which will > override any settings which the FSP was built with itself if the device > tree settings exist, otherwise simply trust the FSP's defaults. > > Modifies the MinnowMax board to transfer the FSP UPD hard-coded settings > to the MinnowMax dts. > > Signed-off-by: Andrew Bradford <andrew.bradf...@kodakalaris.com> > --- > arch/x86/cpu/baytrail/fsp_configs.c | 183 > +++++++++++++++++---- > arch/x86/dts/minnowmax.dts | 27 +++ > .../misc/intel,baytrail-fsp.txt | 113 +++++++++++++ > include/fdtdec.h | 1 + > lib/fdtdec.c | 1 + > 5 files changed, 295 insertions(+), 30 deletions(-) > create mode 100644 doc/device-tree-bindings/misc/intel,baytrail-fsp.txt This is a big step forward in flexibility, thanks for sending this. > > diff --git a/arch/x86/cpu/baytrail/fsp_configs.c > b/arch/x86/cpu/baytrail/fsp_configs.c > index 86b6926..fd07eca 100644 > --- a/arch/x86/cpu/baytrail/fsp_configs.c > +++ b/arch/x86/cpu/baytrail/fsp_configs.c > @@ -1,11 +1,13 @@ > /* > * Copyright (C) 2013, Intel Corporation > * Copyright (C) 2014, Bin Meng <bmeng...@gmail.com> > + * Copyright (C) 2015, Kodak Alaris > * > * SPDX-License-Identifier: Intel > */ > > #include <common.h> > +#include <fdtdec.h> > #include <asm/arch/fsp/azalia.h> > #include <asm/fsp/fsp_support.h> > > @@ -116,41 +118,162 @@ const struct pch_azalia_config azalia_config = { > .reset_wait_timer_us = 300 > }; > > +/** > + * Update the FSP's UPD. The FSP itself can be configured for defaults to > + * store in UPD through Intel's GUI configurator but likely a specific board > + * will want to update these from u-boot, so allow for that via device tree. > + * If the device tree does not specify a setting, trust the FSP's default. > + */ > void update_fsp_upd(struct upd_region *fsp_upd) > { > struct memory_down_data *mem; > + const void *blob = gd->fdt_blob; > + int node; > > - /* > - * Configure everything here to avoid the poor hard-pressed user > - * needing to run Intel's binary configuration tool. It may also allow > - * us to support the 1GB single core variant easily. > - * > - * TODO(s...@chromium.org): Move to device tree > - */ > - fsp_upd->mrc_init_tseg_size = 8; > - fsp_upd->mrc_init_mmio_size = 0x800; > - fsp_upd->emmc_boot_mode = 0xff; > - fsp_upd->enable_sdio = 1; > - fsp_upd->enable_sdcard = 1; > - fsp_upd->enable_hsuart0 = 1; > fsp_upd->azalia_config_ptr = (uint32_t)&azalia_config; > - fsp_upd->enable_i2_c0 = 0; > - fsp_upd->enable_i2_c2 = 0; > - fsp_upd->enable_i2_c3 = 0; > - fsp_upd->enable_i2_c4 = 0; > - fsp_upd->enable_xhci = 0; > - fsp_upd->igd_render_standby = 1; > + > + node = fdtdec_next_compatible(blob, 0, COMPAT_INTEL_BAYTRAIL_FSP); > + if (node < 0) { > + debug("%s: Cannot find FSP node\n", __func__); > + /* TODO: change return type for error indication */ > + return; > + } > + > + fsp_upd->mrc_init_tseg_size = fdtdec_get_int(blob, node, > + "mrc_int_tseg_size", > + > fsp_upd->mrc_init_tseg_size); > + fsp_upd->mrc_init_mmio_size = fdtdec_get_int(blob, node, > + "mrc_init_mmio_size", > + > fsp_upd->mrc_init_mmio_size); > + fsp_upd->mrc_init_spd_addr1 = fdtdec_get_int(blob, node, > + "mrc_init_spd_addr1", > + > fsp_upd->mrc_init_spd_addr1); > + fsp_upd->mrc_init_spd_addr2 = fdtdec_get_int(blob, node, > + "mrc_init_spd_addr2", > + > fsp_upd->mrc_init_spd_addr2); > + fsp_upd->emmc_boot_mode = fdtdec_get_int(blob, node, "emmc_boot_mode", > + fsp_upd->emmc_boot_mode); > + fsp_upd->enable_sdio = fdtdec_get_int(blob, node, "enable_sdio", > + fsp_upd->enable_sdio); Shouldn't these be booleans? > + fsp_upd->enable_sdcard = fdtdec_get_int(blob, node, "enable_sdcard", > + fsp_upd->enable_sdcard); > + fsp_upd->enable_hsuart0 = fdtdec_get_int(blob, node, "enable_hsuart0", > + fsp_upd->enable_hsuart0); > + fsp_upd->enable_hsuart1 = fdtdec_get_int(blob, node, "enable_hsuart1", > + fsp_upd->enable_hsuart1); > + fsp_upd->enable_spi = fdtdec_get_int(blob, node, "enable_spi", > + fsp_upd->enable_spi); > + fsp_upd->enable_sata = fdtdec_get_int(blob, node, "enable_sata", > + fsp_upd->enable_sata); > + fsp_upd->enable_azalia = fdtdec_get_int(blob, node, "enable_azalia", > + fsp_upd->enable_azalia); > + fsp_upd->enable_xhci = fdtdec_get_int(blob, node, "enable_xhci", > + fsp_upd->enable_xhci); > + fsp_upd->enable_lpe = fdtdec_get_int(blob, node, "enable_lpe", > + fsp_upd->enable_lpe); > + fsp_upd->lpss_sio_enable_pci_mode = fdtdec_get_int(blob, node, > + > "lpss_sio_enable_pci_mode", > + > fsp_upd->lpss_sio_enable_pci_mode); > + fsp_upd->enable_dma0 = fdtdec_get_int(blob, node, "enable_dma0", > + fsp_upd->enable_dma0); > + fsp_upd->enable_dma1 = fdtdec_get_int(blob, node, "enable_dma1", > + fsp_upd->enable_dma1); > + fsp_upd->enable_i2_c0 = fdtdec_get_int(blob, node, "enable_i2_c0", > + fsp_upd->enable_i2_c0); > + fsp_upd->enable_i2_c1 = fdtdec_get_int(blob, node, "enable_i2_c1", > + fsp_upd->enable_i2_c1); > + fsp_upd->enable_i2_c2 = fdtdec_get_int(blob, node, "enable_i2_c2", > + fsp_upd->enable_i2_c2); > + fsp_upd->enable_i2_c3 = fdtdec_get_int(blob, node, "enable_i2_c3", > + fsp_upd->enable_i2_c3); > + fsp_upd->enable_i2_c4 = fdtdec_get_int(blob, node, "enable_i2_c4", > + fsp_upd->enable_i2_c4); > + fsp_upd->enable_i2_c5 = fdtdec_get_int(blob, node, "enable_i2_c5", > + fsp_upd->enable_i2_c5); > + fsp_upd->enable_i2_c6 = fdtdec_get_int(blob, node, "enable_i2_c6", > + fsp_upd->enable_i2_c6); > + fsp_upd->enable_pwm0 = fdtdec_get_int(blob, node, "enable_pwm0", > + fsp_upd->enable_pwm0); > + fsp_upd->enable_pwm1 = fdtdec_get_int(blob, node, "enable_pwm1", > + fsp_upd->enable_pwm1); > + fsp_upd->enable_hsi = fdtdec_get_int(blob, node, "enable_hsi", > + fsp_upd->enable_hsi); > + fsp_upd->igd_dvmt50_pre_alloc = fdtdec_get_int(blob, node, > + "igd_dvmt50_pre_alloc", > + > fsp_upd->igd_dvmt50_pre_alloc); > + fsp_upd->aperture_size = fdtdec_get_int(blob, node, "aperture_size", > + fsp_upd->aperture_size); > + fsp_upd->gtt_size = fdtdec_get_int(blob, node, "gtt_size", > + fsp_upd->gtt_size); > + fsp_upd->serial_debug_port_address = fdtdec_get_int(blob, node, > + > "serial_debug_port_address", > + > fsp_upd->serial_debug_port_address); > + fsp_upd->serial_debug_port_type = fdtdec_get_int(blob, node, > + > "serial_debug_port_type", > + > fsp_upd->serial_debug_port_type); > + fsp_upd->mrc_debug_msg = fdtdec_get_int(blob, node, "mrc_debug_msg", > + fsp_upd->mrc_debug_msg); > + fsp_upd->isp_enable = fdtdec_get_int(blob, node, "isp_enable", > + fsp_upd->isp_enable); > + fsp_upd->scc_enable_pci_mode = fdtdec_get_int(blob, node, > + "scc_enable_pci_mode", > + > fsp_upd->scc_enable_pci_mode); > + fsp_upd->igd_render_standby = fdtdec_get_int(blob, node, > + "igd_render_standby", > + > fsp_upd->igd_render_standby); > + fsp_upd->txe_uma_enable = fdtdec_get_int(blob, node, "txe_uma_enable", > + fsp_upd->txe_uma_enable); > + fsp_upd->os_selection = fdtdec_get_int(blob, node, "os_selection", > + fsp_upd->os_selection); > + fsp_upd->emmc45_ddr50_enabled = fdtdec_get_int(blob, node, > + "emmc45_ddr50_enabled", > + > fsp_upd->emmc45_ddr50_enabled); > + fsp_upd->emmc45_hs200_enabled = fdtdec_get_int(blob, node, > + "emmc45_hs200_enabled", > + > fsp_upd->emmc45_hs200_enabled); > + fsp_upd->emmc45_retune_timer_value = fdtdec_get_int(blob, node, > + > "emmc45_retune_timer_value", > + > fsp_upd->emmc45_retune_timer_value); > + fsp_upd->igd_render_standby = fdtdec_get_int(blob, node, > + "igd_render_standby", > + > fsp_upd->igd_render_standby); > > mem = &fsp_upd->memory_params; > - mem->enable_memory_down = 1; > - mem->dram_speed = 1; > - mem->dimm_width = 1; > - mem->dimm_density = 2; > - mem->dimm_tcl = 0xb; > - mem->dimm_trpt_rcd = 0xb; > - mem->dimm_twr = 0xc; > - mem->dimm_twtr = 6; > - mem->dimm_trrd = 6; > - mem->dimm_trtp = 6; > - mem->dimm_tfaw = 0x14; > + mem->enable_memory_down = fdtdec_get_int(blob, node, > + "enable_memory_down", > + mem->enable_memory_down); Probably this should be fdtdec_get_bool(). > + if (mem->enable_memory_down) { > + mem->dram_speed = fdtdec_get_int(blob, node, "dram_speed", > + mem->dram_speed); > + mem->dram_type = fdtdec_get_int(blob, node, "dram_type", > + mem->dram_type); > + mem->dimm_0_enable = fdtdec_get_int(blob, node, > "dimm_0_enable", > + mem->dimm_0_enable); > + mem->dimm_1_enable = fdtdec_get_int(blob, node, > "dimm_1_enable", > + mem->dimm_1_enable); > + mem->dimm_width = fdtdec_get_int(blob, node, "dimm_width", > + mem->dimm_width); > + mem->dimm_density = fdtdec_get_int(blob, node, > + "dimm_density", > + mem->dimm_density); > + mem->dimm_bus_width = fdtdec_get_int(blob, node, > + "dimm_bus_width", > + mem->dimm_bus_width); > + mem->dimm_sides = fdtdec_get_int(blob, node, "dimm_sides", > + mem->dimm_sides); > + mem->dimm_tcl = fdtdec_get_int(blob, node, "dimm_tcl", > + mem->dimm_tcl); > + mem->dimm_trpt_rcd = fdtdec_get_int(blob, node, > "dimm_trpt_rcd", > + mem->dimm_trpt_rcd); > + mem->dimm_twr = fdtdec_get_int(blob, node, "dimm_twr", > + mem->dimm_twr); > + mem->dimm_twtr = fdtdec_get_int(blob, node, "dimm_twtr", > + mem->dimm_twtr); > + mem->dimm_trrd = fdtdec_get_int(blob, node, "dimm_trrd", > + mem->dimm_trrd); > + mem->dimm_trtp = fdtdec_get_int(blob, node, "dimm_trtp", > + mem->dimm_trtp); > + mem->dimm_tfaw = fdtdec_get_int(blob, node, "dimm_tfaw", > + mem->dimm_tfaw); > + } > } > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts > index bd21bfb..bb76e69 100644 > --- a/arch/x86/dts/minnowmax.dts > +++ b/arch/x86/dts/minnowmax.dts > @@ -111,6 +111,33 @@ > > }; > > + fsp { > + compatible = "intel,baytrail-fsp"; > + mrc_init_tseg_size = <8>; We tend to use hyphen in device tree and reserve underscore for phandles. So I think this should be mrc-init-tseg-size, and similarly for the others. Also how about an fsp, prefix, so: fsp,mrc-init-tseg-size = <8>; This indicates the context of the setting. > + mrc_init_mmio_size = <0x800>; > + emmc_boot_mode = <0xff>; > + enable_sdio = <1>; You can just have fsp,enable-sdio; meaning that it is true. The fdtdec_get_bool() function handles this. > + enable_sdcard = <1>; > + enable_hsuart0 = <1>; > + enable_i2_c0 = <0>; > + enable_i2_c2 = <0>; > + enable_i2_c3 = <0>; > + enable_i2_c4 = <0>; > + enable_xhci = <0>; > + igd_render_standby = <1>; > + enable_memory_down = <1>; How about putting the memory parameters in a separate subnode of your FSP node, like: memory-params { fsp,dram-speed = <1>; ... }; That groups them nicely into one area. > + dram_speed = <1>; > + dimm_width = <1>; > + dimm_density = <2>; > + dimm_tcl = <0xb>; > + dimm_trpt_rcd = <0xb>; > + dimm_twr = <0xc>; > + dimm_twtr = <6>; > + dimm_trrd = <6>; > + dimm_trtp = <6>; > + dimm_tfaw = <0x14>; > + }; > + > spi { > #address-cells = <1>; > #size-cells = <0>; > diff --git a/doc/device-tree-bindings/misc/intel,baytrail-fsp.txt > b/doc/device-tree-bindings/misc/intel,baytrail-fsp.txt > new file mode 100644 > index 0000000..a9841e7 > --- /dev/null > +++ b/doc/device-tree-bindings/misc/intel,baytrail-fsp.txt > @@ -0,0 +1,113 @@ > +Intel Baytrail FSP UPD Binding > +============================== > + > +The device tree node which describes the overriding of the Intel Baytrail FSP > +UPD data for configuring the SoC. > + > +All properties are optional, if unspecified then the FSP's preconfigured > choices > +will be used. > + > +All properties can be found within the `upd_region` struct in > +arch/x86/asm/arch-baytrail/fsp/fsp_vpd.h under the same names and in Intel's > FSP > +Binary Configuration Tool for Baytrail. > + > +Optional properties: > + > +- mrc_init_tseg_size > +- mrc_init_mmio_size > +- mrc_init_spd_addr1 > +- mrc_init_spd_addr2 > +- emmc_boot_mode > +- enable_sdio > +- enable_sdcard > +- enable_hsuart0 > +- enable_hsuart1 > +- enable_spi > +- enable_sata > +- sata_mode > +- enable_azalia > +- enable_xhci > +- enable_lpe > +- lpss_sio_enable_pci_mode > +- enable_dma0 > +- enable_dma1 > +- enable_i2_c0 > +- enable_i2_c1 > +- enable_i2_c2 > +- enable_i2_c3 > +- enable_i2_c4 > +- enable_i2_c5 > +- enable_i2_c6 > +- enable_pwm0 > +- enable_pwm1 > +- enable_hsi > +- igd_dvmt50_pre_alloc > +- aperture_size > +- gtt_size > +- serial_debug_port_address > +- serial_debug_port_type > +- mrc_debug_msg > +- isp_enable > +- scc_enable_pci_mode > +- igd_render_standby > +- txe_uma_enable > +- os_selection > +- emmc45_ddr50_enabled > +- emmc45_hs200_enabled > +- emmc45_retune_timer_value > +- enable_memory_down > + > + The following are only used if enable_memory_down is enabled, > otherwise > + the FSP will use the DIMM's SPD information to configure the memory: > + - dram_speed > + - dram_type > + - dimm_0_enable > + - dimm_1_enable > + - dimm_width > + - dimm_density > + - dimm_bus_width > + - dimm_sides > + - dimm_tcl > + - dimm_trpt_rcd > + - dimm_twr > + - dimm_twtr > + - dimm_trrd > + - dimm_trtp > + - dimm_tfaw > + > + > +Example (from MinnowMax Dual Core): > +----------------------------------- > + > +/ { > + ... > + > + fsp { > + compatible = "intel,baytrail-fsp"; > + mrc_init_tseg_size = <8>; > + mrc_init_mmio_size = <0x800>; > + emmc_boot_mode = <0xff>; > + enable_sdio = <1>; > + enable_sdcard = <1>; > + enable_hsuart0 = <1>; > + enable_i2_c0 = <0>; > + enable_i2_c2 = <0>; > + enable_i2_c3 = <0>; > + enable_i2_c4 = <0>; > + enable_xhci = <0>; > + igd_render_standby = <1>; > + enable_memory_down = <1>; > + dram_speed = <1>; > + dimm_width = <1>; > + dimm_density = <2>; > + dimm_tcl = <0xb>; > + dimm_trpt_rcd = <0xb>; > + dimm_twr = <0xc>; > + dimm_twtr = <6>; > + dimm_trrd = <6>; > + dimm_trtp = <6>; > + dimm_tfaw = <0x14>; > + }; > + > + ... > +}; > diff --git a/include/fdtdec.h b/include/fdtdec.h > index 2323603..2b09cce 100644 > --- a/include/fdtdec.h > +++ b/include/fdtdec.h > @@ -183,6 +183,7 @@ enum fdt_compat_id { > COMPAT_SOCIONEXT_XHCI, /* Socionext UniPhier xHCI */ > COMPAT_INTEL_PCH, /* Intel PCH */ > COMPAT_INTEL_IRQ_ROUTER, /* Intel Interrupt Router */ > + COMPAT_INTEL_BAYTRAIL_FSP, /* Intel Baytrail FSP */ > > COMPAT_COUNT, > }; > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > index 9877849..c7a27ac 100644 > --- a/lib/fdtdec.c > +++ b/lib/fdtdec.c > @@ -76,6 +76,7 @@ static const char * const compat_names[COMPAT_COUNT] = { > COMPAT(SOCIONEXT_XHCI, "socionext,uniphier-xhci"), > COMPAT(COMPAT_INTEL_PCH, "intel,bd82x6x"), > COMPAT(COMPAT_INTEL_IRQ_ROUTER, "intel,irq-router"), > + COMPAT(COMPAT_INTEL_BAYTRAIL_FSP, "intel,baytrail-fsp"), > }; > > const char *fdtdec_get_compatible(enum fdt_compat_id id) > -- > 1.9.1 > At some point we could move this to driver model, but that can come later. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot