Hi Bin, On 29 July 2015 at 08:08, Bin Meng <[email protected]> wrote: > Hi Andrew, Simon, > > On Wed, Jul 22, 2015 at 4:17 AM, Simon Glass <[email protected]> wrote: >> Hi Andrew, >> >> On 10 July 2015 at 12:24, Andrew Bradford <[email protected]> >> wrote: >>> Hi Simon, >>> >>> On 07/10 06:53, Simon Glass wrote: >>>> Hi, >>>> >>>> On 8 July 2015 at 05:30, Andrew Bradford <[email protected]> >>>> wrote: >>>> > Hi Bin, >>>> > >>>> > On 07/08 11:18, Bin Meng wrote: >>>> >> Hi Andrew, >>>> >> >>>> >> On Wed, Jul 8, 2015 at 3:16 AM, <[email protected]> wrote: >>>> >> > From: Andrew Bradford <[email protected]> >>>> >> > >>>> >> > 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 <[email protected]> >>>> >> > --- >>>> >> > >>>> >> > Changes from v1: >>>> >> > >>>> >> > - Use "-" rather than "_" in dt property names. >>>> >> > - Use "Bay Trail" for the formal name of the Intel product family. >>>> >> > - Use an "fsp," prefix for dt property names for clarity. >>>> >> > - Fix minor code indentation issues. >>>> >> > - Create a dt subnode for the memory-down-params. >>>> >> > - Clarify documentation that dt overrides the FSP's config, so we >>>> >> > don't >>>> >> > use booleans. >>>> >> > >>>> >> > arch/x86/cpu/baytrail/fsp_configs.c | 188 >>>> >> > +++++++++++++++++---- >>>> >> > arch/x86/dts/minnowmax.dts | 30 ++++ >>>> >> > .../misc/intel,baytrail-fsp.txt | 119 >>>> >> > +++++++++++++ >>>> >> > include/fdtdec.h | 2 + >>>> >> > lib/fdtdec.c | 2 + >>>> >> > 5 files changed, 311 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..fce76e6 100644 >>>> >> > --- a/arch/x86/cpu/baytrail/fsp_configs.c >>>> >> > +++ b/arch/x86/cpu/baytrail/fsp_configs.c >>>> >> > @@ -1,14 +1,18 @@ >>>> >> > /* >>>> >> > * Copyright (C) 2013, Intel Corporation >>>> >> > * Copyright (C) 2014, Bin Meng <[email protected]> >>>> >> > + * Copyright (C) 2015, Kodak Alaris, Inc >>>> >> > * >>>> >> > * SPDX-License-Identifier: Intel >>>> >> > */ >>>> >> > >>>> >> > #include <common.h> >>>> >> > +#include <fdtdec.h> >>>> >> > #include <asm/arch/fsp/azalia.h> >>>> >> > #include <asm/fsp/fsp_support.h> >>>> >> > >>>> >> > +DECLARE_GLOBAL_DATA_PTR; >>>> >> > + >>>> >> > /* ALC262 Verb Table - 10EC0262 */ >>>> >> > static const uint32_t verb_table_data13[] = { >>>> >> > /* Pin Complex (NID 0x11) */ >>>> >> > @@ -116,41 +120,165 @@ 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([email protected]): 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__); >>>> >> > + return; >>>> >> > + } >>>> >> > + >>>> >> > + fsp_upd->mrc_init_tseg_size = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,mrc-int-tseg-size", >>>> >> >>>> >> mrc-int? Guess it is mrc-init. >>>> > >>>> > Yes, thank you for catching this. Will fix in v3. >>>> > >>>> >> > + >>>> >> > fsp_upd->mrc_init_tseg_size); >>>> >> > + fsp_upd->mrc_init_mmio_size = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,mrc-init-mmio-size", >>>> >> > + >>>> >> > fsp_upd->mrc_init_mmio_size); >>>> >> > + fsp_upd->mrc_init_spd_addr1 = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,mrc-init-spd-addr1", >>>> >> > + >>>> >> > fsp_upd->mrc_init_spd_addr1); >>>> >> > + fsp_upd->mrc_init_spd_addr2 = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,mrc-init-spd-addr2", >>>> >> > + >>>> >> > fsp_upd->mrc_init_spd_addr2); >>>> >> > + fsp_upd->emmc_boot_mode = fdtdec_get_int(blob, node, >>>> >> > "fsp,emmc-boot-mode", >>>> >> > + >>>> >> > fsp_upd->emmc_boot_mode); >>>> >> > + fsp_upd->enable_sdio = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-sdio", >>>> >> > + fsp_upd->enable_sdio); >>>> >> >>>> >> I think we agreed that all these 'enable' should be accessed using >>>> >> fdtdec_get_bool(). >>>> > >>>> > I was under the impression that we decided to keep these as non-bool so >>>> > that the FSP could be overridden as needed (or you can fully specify all >>>> > the properties in dt if you want) as per Simon's comments [1] along with >>>> > adding additional comments in this version of my patch to clarify this.. >>>> > >>>> > [1]:http://lists.denx.de/pipermail/u-boot/2015-June/217802.html >>>> > >>>> > But I'm open to changing this to use bools. Additional thoughts on this >>>> > below... >>>> > >>>> >> > + fsp_upd->enable_sdcard = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-sdcard", >>>> >> > + >>>> >> > fsp_upd->enable_sdcard); >>>> >> > + fsp_upd->enable_hsuart0 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-hsuart0", >>>> >> > + >>>> >> > fsp_upd->enable_hsuart0); >>>> >> > + fsp_upd->enable_hsuart1 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-hsuart1", >>>> >> > + >>>> >> > fsp_upd->enable_hsuart1); >>>> >> > + fsp_upd->enable_spi = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-spi", >>>> >> > + fsp_upd->enable_spi); >>>> >> > + fsp_upd->enable_sata = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-sata", >>>> >> > + fsp_upd->enable_sata); >>>> >> > + fsp_upd->enable_azalia = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-azalia", >>>> >> > + >>>> >> > fsp_upd->enable_azalia); >>>> >> > + fsp_upd->enable_xhci = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-xhci", >>>> >> > + fsp_upd->enable_xhci); >>>> >> > + fsp_upd->enable_lpe = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-lpe", >>>> >> > + fsp_upd->enable_lpe); >>>> >> > + fsp_upd->lpss_sio_enable_pci_mode = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,lpss-sio-enable-pci-mode", >>>> >> > + >>>> >> > fsp_upd->lpss_sio_enable_pci_mode); >>>> >> > + fsp_upd->enable_dma0 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-dma0", >>>> >> > + fsp_upd->enable_dma0); >>>> >> > + fsp_upd->enable_dma1 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-dma1", >>>> >> > + fsp_upd->enable_dma1); >>>> >> > + fsp_upd->enable_i2_c0 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-i2-c0", >>>> >> > + fsp_upd->enable_i2_c0); >>>> >> > + fsp_upd->enable_i2_c1 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-i2-c1", >>>> >> > + fsp_upd->enable_i2_c1); >>>> >> > + fsp_upd->enable_i2_c2 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-i2-c2", >>>> >> > + fsp_upd->enable_i2_c2); >>>> >> > + fsp_upd->enable_i2_c3 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-i2-c3", >>>> >> > + fsp_upd->enable_i2_c3); >>>> >> > + fsp_upd->enable_i2_c4 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-i2-c4", >>>> >> > + fsp_upd->enable_i2_c4); >>>> >> > + fsp_upd->enable_i2_c5 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-i2-c5", >>>> >> > + fsp_upd->enable_i2_c5); >>>> >> > + fsp_upd->enable_i2_c6 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-i2-c6", >>>> >> > + fsp_upd->enable_i2_c6); >>>> >> > + fsp_upd->enable_pwm0 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-pwm0", >>>> >> > + fsp_upd->enable_pwm0); >>>> >> > + fsp_upd->enable_pwm1 = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-pwm1", >>>> >> > + fsp_upd->enable_pwm1); >>>> >> > + fsp_upd->enable_hsi = fdtdec_get_int(blob, node, >>>> >> > "fsp,enable-hsi", >>>> >> > + fsp_upd->enable_hsi); >>>> >> > + fsp_upd->igd_dvmt50_pre_alloc = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,igd-dvmt50-pre-alloc", >>>> >> > + >>>> >> > fsp_upd->igd_dvmt50_pre_alloc); >>>> >> > + fsp_upd->aperture_size = fdtdec_get_int(blob, node, >>>> >> > "fsp,aperture-size", >>>> >> > + >>>> >> > fsp_upd->aperture_size); >>>> >> > + fsp_upd->gtt_size = fdtdec_get_int(blob, node, "fsp,gtt-size", >>>> >> > + fsp_upd->gtt_size); >>>> >> > + fsp_upd->serial_debug_port_address = fdtdec_get_int(blob, >>>> >> > node, >>>> >> > + >>>> >> > "fsp,serial-debug-port-address", >>>> >> > + >>>> >> > fsp_upd->serial_debug_port_address); >>>> >> > + fsp_upd->serial_debug_port_type = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,serial-debug-port-type", >>>> >> > + >>>> >> > fsp_upd->serial_debug_port_type); >>>> >> > + fsp_upd->mrc_debug_msg = fdtdec_get_int(blob, node, >>>> >> > "fsp,mrc-debug-msg", >>>> >> > + >>>> >> > fsp_upd->mrc_debug_msg); >>>> >> > + fsp_upd->isp_enable = fdtdec_get_int(blob, node, >>>> >> > "fsp,isp-enable", >>>> >> > + fsp_upd->isp_enable); >>>> >> > + fsp_upd->scc_enable_pci_mode = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,scc-enable-pci-mode", >>>> >> > + >>>> >> > fsp_upd->scc_enable_pci_mode); >>>> >> > + fsp_upd->igd_render_standby = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,igd-render-standby", >>>> >> > + >>>> >> > fsp_upd->igd_render_standby); >>>> >> > + fsp_upd->txe_uma_enable = fdtdec_get_int(blob, node, >>>> >> > "fsp,txe-uma-enable", >>>> >> > + >>>> >> > fsp_upd->txe_uma_enable); >>>> >> > + fsp_upd->os_selection = fdtdec_get_int(blob, node, >>>> >> > "fsp,os-selection", >>>> >> > + fsp_upd->os_selection); >>>> >> > + fsp_upd->emmc45_ddr50_enabled = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,emmc45-ddr50-enabled", >>>> >> > + >>>> >> > fsp_upd->emmc45_ddr50_enabled); >>>> >> > + fsp_upd->emmc45_hs200_enabled = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,emmc45-hs200-enabled", >>>> >> > + >>>> >> > fsp_upd->emmc45_hs200_enabled); >>>> >> > + fsp_upd->emmc45_retune_timer_value = fdtdec_get_int(blob, >>>> >> > node, >>>> >> > + >>>> >> > "fsp,emmc45-retune-timer-value", >>>> >> > + >>>> >> > fsp_upd->emmc45_retune_timer_value); >>>> >> > + fsp_upd->igd_render_standby = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,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, >>>> >> > + >>>> >> > "fsp,enable-memory-down", >>>> >> > + >>>> >> > mem->enable_memory_down); >>>> >> > + node = fdtdec_next_compatible(blob, node, >>>> >> > + COMPAT_INTEL_BAYTRAIL_FSP_MDP); >>>> >> > + if (node < 0) { >>>> >> > + debug("%s: Cannot find FSP memory-down-params >>>> >> > node\n", __func__); >>>> >> > + } else { >>>> >> > + mem->dram_speed = fdtdec_get_int(blob, node, >>>> >> > "fsp,dram-speed", >>>> >> > + mem->dram_speed); >>>> >> > + mem->dram_type = fdtdec_get_int(blob, node, >>>> >> > "fsp,dram-type", >>>> >> > + mem->dram_type); >>>> >> > + mem->dimm_0_enable = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-0-enable", >>>> >> > + >>>> >> > mem->dimm_0_enable); >>>> >> > + mem->dimm_1_enable = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-1-enable", >>>> >> > + >>>> >> > mem->dimm_1_enable); >>>> >> > + mem->dimm_width = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-width", >>>> >> > + mem->dimm_width); >>>> >> > + mem->dimm_density = fdtdec_get_int(blob, node, >>>> >> > + "fsp,dimm-density", >>>> >> > + mem->dimm_density); >>>> >> > + mem->dimm_bus_width = fdtdec_get_int(blob, node, >>>> >> > + >>>> >> > "fsp,dimm-bus-width", >>>> >> > + >>>> >> > mem->dimm_bus_width); >>>> >> > + mem->dimm_sides = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-sides", >>>> >> > + mem->dimm_sides); >>>> >> > + mem->dimm_tcl = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-tcl", >>>> >> > + mem->dimm_tcl); >>>> >> > + mem->dimm_trpt_rcd = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-trpt-rcd", >>>> >> > + >>>> >> > mem->dimm_trpt_rcd); >>>> >> > + mem->dimm_twr = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-twr", >>>> >> > + mem->dimm_twr); >>>> >> > + mem->dimm_twtr = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-twtr", >>>> >> > + mem->dimm_twtr); >>>> >> > + mem->dimm_trrd = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-trrd", >>>> >> > + mem->dimm_trrd); >>>> >> > + mem->dimm_trtp = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-trtp", >>>> >> > + mem->dimm_trtp); >>>> >> > + mem->dimm_tfaw = fdtdec_get_int(blob, node, >>>> >> > "fsp,dimm-tfaw", >>>> >> > + mem->dimm_tfaw); >>>> >> > + } >>>> >> > } >>>> >> > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts >>>> >> > index 0e59b18..279d08d 100644 >>>> >> > --- a/arch/x86/dts/minnowmax.dts >>>> >> > +++ b/arch/x86/dts/minnowmax.dts >>>> >> > @@ -121,6 +121,36 @@ >>>> >> > 0x01000000 0x0 0x2000 0x2000 0 0xe000>; >>>> >> > }; >>>> >> > >>>> >> > + fsp { >>>> >> > + compatible = "intel,baytrail-fsp"; >>>> >> > + fsp,mrc-init-tseg-size = <8>; >>>> >> > + fsp,mrc-init-mmio-size = <0x800>; >>>> >> > + fsp,emmc-boot-mode = <0xff>; >>>> >> > + fsp,enable-sdio = <1>; >>>> >> > + fsp,enable-sdcard = <1>; >>>> >> > + fsp,enable-hsuart0 = <1>; >>>> >> > + fsp,enable-i2-c0 = <0>; >>>> >> > + fsp,enable-i2-c2 = <0>; >>>> >> > + fsp,enable-i2-c3 = <0>; >>>> >> > + fsp,enable-i2-c4 = <0>; >>>> >> > + fsp,enable-xhci = <0>; >>>> >> > + fsp,igd-render-standby = <1>; >>>> >> > + fsp,enable-memory-down = <1>; >>>> >> > + fsp,memory-down-params { >>>> >> > + compatible = "intel,baytrail-fsp-mdp"; >>>> >> > + fsp,dram-speed = <1>; >>>> >> > + fsp,dimm-width = <1>; >>>> >> > + fsp,dimm-density = <2>; >>>> >> > + fsp,dimm-tcl = <0xb>; >>>> >> > + fsp,dimm-trpt-rcd = <0xb>; >>>> >> > + fsp,dimm-twr = <0xc>; >>>> >> > + fsp,dimm-twtr = <6>; >>>> >> > + fsp,dimm-trrd = <6>; >>>> >> > + fsp,dimm-trtp = <6>; >>>> >> > + fsp,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..979d646 >>>> >> > --- /dev/null >>>> >> > +++ b/doc/device-tree-bindings/misc/intel,baytrail-fsp.txt >>>> >> > @@ -0,0 +1,119 @@ >>>> >> > +Intel Bay Trail FSP UPD Binding >>>> >> > +============================== >>>> >> > + >>>> >> > +The device tree node which describes the overriding of the Intel Bay >>>> >> > Trail FSP >>>> >> > +UPD data for configuring the SoC. >>>> >> > + >>>> >> > +All properties are optional, if a property is unspecified then the >>>> >> > FSP's >>>> >> > +preconfigured choices will be used. For this reason, using normal >>>> >> > boolean >>>> >> > +properties is not desired as the lack of a boolean property would >>>> >> > disable a >>>> >> > +given property and force the user to include all properties they >>>> >> > wish to enable. >>>> >> > + >>>> >> >>>> >> My understanding is that we either use device tree or the FSP >>>> >> defaults. So if we describe an FSP node in device tree, then we go >>>> >> with device tree as U-Boot's configuration. We cannot use partial >>>> >> configuration from device tree and partial from FSP defaults. That's >>>> >> confusing. >>>> > >>>> > I agree that it could be confusing, even just in writing this patch and >>>> > trying to test it I've run into the fact that in order to see if I'm >>>> > properly overriding the FSP's configuration that I would need to build a >>>> > bunch of different FSP configurations in Intel's Binary Configuration >>>> > Tool. I've not done this as it would be extremely time consuming. >>>> > >>>> > I'm open to making the properties into bools and then overriding all of >>>> > the FSP's UPD fields with sane defaults if the device tree does not >>>> > specify a non-bool property. I now think this might be a more sane >>>> > approach for the long run, but I would need some assistance to know what >>>> > the proper settings are for MinnowMax as I do not have one to test with >>>> > and I do not know the full backstory as to why the FSP that shipped with >>>> > MinnowMax was found to need to have its UPD overridden prior to my >>>> > patch. >>>> >>>> It is mostly that the FSP can be set to anything since it is >>>> configured by a proprietary binary tool. Having configuration in two >>>> places, particularly where one is uncertain, does seem confusing. >>>> >>>> So I think Bin's proposal is that we either override everything and >>>> don't use the FSP config, or we override nothing and use the entire >>>> FSP config. >>> >>> Yes, I agree, I think this will be the easiest to understand way. >>> >>> What I'll do for v3 is to make all of the "enable" properties into >>> bools. >> >> OK >> >>> >>> For the properties which are ints when the device tree does not have a >>> specification for the property, like dimm-width, do you think it >>> is better to trust the FSP's compiled in value or to simply set a sane >>> default value in fdtdec_get_int()? >>> >>>> Re the Minnowmax settings, I can dump out the list of current settings >>>> if you like. >>> >>> Yes, please. This would be very helpful. >> >> Sorry this has taken a while. It is quite painful. Here is my manual >> decoding of fsp.bin. I did it this way you can decode a different >> version, which might have things in different places. You can follow >> along with a hex editor. I suppose you could also write a little tool >> to display the UPD values. >> >> UPD >> FSP binary data >> >> see find_fsp_header() >> >> 0 struct fv_header >> u16 ext_hdr_off at 0x38 = 0x0060 >> 0 + 0x60 = 0x60 >> >> 0x60 has struct fv_ext_header >> ext_hdr_size at 0x70 = 0x14 >> 0x60 + 0x14 = 0x74 >> round up to multiple of 8 bytes = 0x78 >> >> 0x78 has struct ffs_file_header >> 16 byte guid at 0x78 - FSP_GUID_DATA1, etc. >> >> sizeof(struct ffs_file_header) = 0x18 >> 0x78 + 0x18 = 0x90 >> >> 0x90 has struct raw_section >> ->type at 0x93 = 0x19 >> sizeof(struct raw_section) = 4 >> 0x90 + 4 = 0x94 >> >> struct fsp_header at 0x94 >> 0x94 has 'FSPH' >> >> ->img_base at offset 0x1c = 0xb0 >> 0xb0 has 0xfffc0000 >> >> ->cfg_region_off at offset 0x24 = 0xb8 >> 0xb8 has 0x9dac >> >> ->cfg_region_size at offset 0x28 = 0xbc >> 0xbc has 0x36 >> >> struct struct vpd_region at 0x9dac >> ->sign at offset 0 = 0x9dac >> "VLYVIEW1" >> ->img_rev at offset 8 = 0x9db4 >> 0x303 >> >> ->upd_offset at offset 0xc = 9x9db8 >> -0x1f494 >> >> struct upd_region at 0x1f494 >> ->signature = "VLV2UPDR" >> data as per struct upd_region >> >> extends from 0x1f494 to 0x1f596 as per struct upd_region (ends with >> bytes 0xaa, 0x55) >> 00000000 56 4c 56 32 55 50 44 52 e4 ff ff ff 00 00 00 00 >> |VLV2UPDR........| >> 00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> |................| >> 00000020 01 00 00 08 a0 a2 02 01 01 00 01 01 01 01 01 00 >> |................| >> 00000030 00 00 00 00 01 01 01 01 01 01 01 01 01 01 01 01 >> |................| >> 00000040 01 01 00 02 02 02 f8 03 00 00 01 00 00 01 00 00 >> |................| >> 00000050 04 01 00 08 00 00 00 00 00 00 00 00 00 00 00 00 >> |................| >> 00000060 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> |................| >> * >> 000000f0 00 02 01 01 00 00 01 03 00 09 09 0a 05 04 05 14 >> |................| >> 00000100 aa 55 |.U| >> 00000102 >> > > Not sure if you already know, but if you installed the Intel Binary > Configuration Tool (BCT), the FSP binary default UPD settings can be > viewed easily by opening the shipped .bsf file within the FSP package > from the BCT. > > [snip]
I did manage to get that running a while back but now I'm not sure how now. On Ubuntu both the 32- and 64-bit versions crash with a X windows error, and on Windows I recently tried it and it required Windows 7, which I don't have in virtualbox (maybe I should try to figure that out one day). It would be much more accessible if there were an open source tool. Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
