Hi Simon, On 07/21 14:17, Simon Glass 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:
<snip> > >> >> 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 <snip> Thanks! :) I may not be able to send v3 of this patch until next week due to other obligations. Thanks for your help! -Andrew _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot

