[U-Boot] [PATCH v2] x86: baytrail: Configure FSP UPD from device tree

Bin Meng bmeng.cn at gmail.com
Wed Jul 29 16:08:56 CEST 2015


Hi Andrew, Simon,

On Wed, Jul 22, 2015 at 4:17 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Andrew,
>
> On 10 July 2015 at 12:24, Andrew Bradford <andrew at bradfordembedded.com> wrote:
>> Hi Simon,
>>
>> On 07/10 06:53, Simon Glass wrote:
>>> Hi,
>>>
>>> On 8 July 2015 at 05:30, Andrew Bradford <andrew at bradfordembedded.com> wrote:
>>> > Hi Bin,
>>> >
>>> > On 07/08 11:18, Bin Meng wrote:
>>> >> Hi Andrew,
>>> >>
>>> >> On Wed, Jul 8, 2015 at 3:16 AM,  <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>
>>> >> > ---
>>> >> >
>>> >> > 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 <bmeng.cn at gmail.com>
>>> >> > + * 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(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;
>>> >> > -       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]

Regards,
Bin


More information about the U-Boot mailing list