[U-Boot] [PATCH] x86: baytrail: Configure FSP UPD from device tree
Andrew Bradford
andrew at bradfordembedded.com
Wed Jul 1 22:39:49 CEST 2015
Hi Bin,
On 07/01 17:45, Bin Meng wrote:
> Hi Andrew,
>
> On Mon, Jun 29, 2015 at 11:10 PM, <andrew at bradfordembedded.com> wrote:
> > From: Andrew Bradford <andrew.bradford at 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.bradford at 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
> >
> > 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.cn at 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>
> >
>
> Please add DECLARE_GLOBAL_DATA_PTR here.
OK, will do in v2.
> > @@ -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(sjg at 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;
>
> Should the azalia config be moved to device tree too?
Probably, yes. Is there a good reason to *not* move it to the device tree
along with the moves I'm making in this patch?
If there's no objection, I'll work to include moving the azalia config
to device tree in v2.
> > - 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 */
>
> Do we need return error? Since you mentioned that if no device tree
> settings the default FSP values will be used.
Probably we don't need to return an error, I will remove the TODO in v2.
At some point, if it is desired for runtime to know if the device tree
settings or the built-in FSP settings were used, then it might be worth
returning an error.
> > + 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);
> > + 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);
> > + 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>;
> > + 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>;
> > + };
> > +
> > 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
>
> Baytrail -> BayTrail. Please fix this globally.
Should it be "Bay Trail" as per [1] when written out like this?
[1]:http://ark.intel.com/products/codename/55844/Bay-Trail#@All
> > +==============================
> > +
> > +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
>
> arch/x86/include/asm/ ...
Yes, thank you for catching this! :)
Will fix in v2.
> > +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)
> > --
Thanks for taking the time to review this! :)
-Andrew
More information about the U-Boot
mailing list