Hi Bin, On 08/07 08:11, Andrew Bradford wrote: > Hi Bin, > > On 08/07 08:23, Bin Meng wrote: > > Hi Andrew, > > > > On Fri, Aug 7, 2015 at 4:08 AM, Andrew Bradford > > <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. > > > > > > Modify the MinnowMax and BayleyBay boards to transfer sensible UPD > > > settings from the Intel FSPv4 Gold release to the respective dts files, > > > with the condition that the memory-down parameters for MinnowMax are > > > also used. > > > > > > > Thanks for updating BayleyBay dts as well :) > > You're welcome :) > My custom board actually boots with the Bayley Bay configuration, once I > change out the dts to use your D0 stepping microcode patch, so that was > a good check for me. > > > > Signed-off-by: Andrew Bradford <andrew.bradf...@kodakalaris.com> > > > --- > > > > Reviewed-by: Bin Meng <bmeng...@gmail.com> > > Tested-by: Bin Meng <bmeng...@gmail.com> > > > > But please see some comments/nits below. > > Would it be best if I fix your comments/nits and send a v4? > > > > > > > Changes from v2: > > > - Switch to using booleans in dts, where appropriate. > > > - Add Bayley Bay dts modifications. > > > - Clarify docs to show bool versus int properties. > > > - Include enable-igt property from FSPv4. > > > > > > 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 | 167 > > > +++++++++++++++++---- > > > arch/x86/dts/bayleybay.dts | 40 +++++ > > > arch/x86/dts/minnowmax.dts | 58 +++++++ > > > .../misc/intel,baytrail-fsp.txt | 158 > > > +++++++++++++++++++ > > > include/fdtdec.h | 2 + > > > lib/fdtdec.c | 2 + > > > 6 files changed, 397 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..1e5656f 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...@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,144 @@ const struct pch_azalia_config azalia_config = { > > > .reset_wait_timer_us = 300 > > > }; > > > > > > +/** > > > + * Override the FSP's UPD. > > > + * If the device tree does not specify a integer setting, use the default > > > > Nits: an integer > > > > > + * provided in Intel's Baytrail_FSP_Gold4.tgz release > > > FSP/BayleyBayFsp.bsf file. > > > + */ > > > 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; > > > > One general comment: I think we should replace these hardcoded numbers > > with meaningful macros (eg: 0x800 means MMIO uses 2GB space, that's > > why we see previous the pci does not work on a 4GB DIMM as FSP only > > maps 2GB RAM per this setting). See Intel's Baytrail_FSP_Gold4.tgz > > release FSP/BayleyBayFsp.bsf file, it has the details of how each > > magic number maps to what meaning. Will you do a follow-up patch for > > this? I can do that as well. > > I can do this.
I'm sorry, but I have been busy with other tasks for the past week and haven't gotten a chance to craft or send out a patch to turn the magic numbers into #defines in a header. If you are able to send a patch which #defines the magic numbers quickly, I would greatly appreciate it, as I likely won't be able to get a patch out until (at best) the end of this week. I don't want to hold anyone up with their development waiting for me to send such a patch. My Bay Trail project has been put on-hold by my employer so I'm not able to dedicate much time to it now. Sorry. <snip> Thanks, Andrew _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot