Hi Andrew, On 27 May 2015 at 08:22, Andrew Bradford <[email protected]> wrote: > Hi Simon & Bin, > > On 05/26 15:37, Simon Glass wrote: >> Hi Andrew, >> >> On 26 May 2015 at 13:52, Andrew Bradford <[email protected]> wrote: >> > Hi Simon and Bin (sorry for bringing this back from the dead), >> > >> > But I have a question about fsp_configs.c down below: >> > >> > On 01/27 22:13, Simon Glass wrote: >> > ------------->8--------------- >> >> +void update_fsp_upd(struct upd_region *fsp_upd) >> >> +{ >> >> + struct memory_down_data *mem; >> >> + >> >> + /* >> >> + * 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; >> >> + >> >> + 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; >> >> +} >> > >> > I am trying to move this fsp upd to use device tree as I am trying to >> > create a patch set to add the Intel Valley Island E38xx board (which >> > uses a SODIMM rather than memory down). In doing so, I've found that >> > global data doesn't seem to be available when update_fsp_upd() is called >> > and generally it seems that gd->fdt_blob is used to get a reference to >> > the flattened device tree. >> > >> > I'm not super familiar with device tree, but I was attempting to use >> > fdtdec_next_compatible(gd->fdt_blob, 0, COMPAT_INTEL_BAYTRAIL_FSP) in a >> > similar way that Quark does in my patchset (I've properly created the >> > COMPAT_INTEL_BAYTRAIL_FSP define and some device tree nodes in my dts >> > file). When I call fdtdec_next_compatible() the board does something >> > which I'm unable to debug (Valley Island does not have the early UART >> > pins connected so I have no early UART capability) but things just seem >> > to stop. >> > >> > In manually tracing the calls which lead to update_fsp_upd(), it seems >> > that we haven't yet set up global data, so it makes sense that I can't >> > reference it. But the device tree should be available in NOR flash or >> > in some other way which we can access in order to get the FSP UPD >> > settings. >> > >> > Is there a simple way to access the device tree while it's still in NOR >> > flash so I can avoid using gd? Or can global data be setup prior to >> > calling update_fsp_upd() (I believe we're still in CAR at this point)? >> > Or am I misunderstanding something basic here? >> > >> > Did you have a rough outline of how this could be moved to device tree? >> >> This is a bit tricky. I would like to move fsp_init() later in the >> init sequence (e.g. to board_init_f()). See this TODO in the code: >> >> /* >> * TODO: >> * >> * According to FSP architecture spec, the fsp_init() will not return >> * to its caller, instead it requires the bootloader to provide a >> * so-called continuation function to pass into the FSP as a parameter >> * of fsp_init, and fsp_init() will call that continuation function >> * directly. >> * >> * The call to fsp_init() may need to be moved out of the car_init() >> * to cpu_init_f() with the help of some inline assembly codes. >> * Note there is another issue that fsp_init() will setup another stack >> * using the fsp_init parameter stack_top after DRAM is initialized, >> * which means any data on the previous stack (on the CAR) gets lost >> * (ie: U-Boot global_data). FSP is supposed to support such scenario, >> * however it does not work. This should be revisited in the future. >> */ >> >> The primary issues are: >> 1. The need to recover the global_data >> 2. The need to change to a new stack >> >> Re 1, my reading of the HOB stuff is that it is supposed to provide >> you with a pointer to the CAR RAM (all ~128KB of it) so that you can >> go back and find your old stack (and in our case, global_data). >> >> Bin mentioned that this doesn't work - his is the comment above after >> I asked him about it. > > OK, if it doesn't work then that's frustrating. > > I do see that HOB 15 on my Valley Island board has the right GUID to be > the FSP_BOOTLOADER_TEMPORARY_MEMORY_HOB and its size is 16408. So that > has me hopeful, but likely I'll run into similar things that you two > have seen before.
Well if you can get access to the old global_data here then you can memcpy it to a new place in DRAM. That helps. But I forgot about issue 3: 3. Driver model will have created a few devices in temporary RAM and now they are gone. The easiest solution is to run the board_init_f() sequence again from the start, this time skipping the dram init. It's horrible but pretty easy to implement. So you end up running dram_init() twice. The first time it runs the FSP call and then calls board_init_f() (or returns a special value such that the sequence restarts). The second time it does nothing. Better might be to redo just the early malloc and driver model init: initf_malloc() and initf_dm(). That might work. > > Any pointers on why it was determined that the CAR RAM pointer doesn't > work correctly so I can avoid spending too much time investigating > things which have already been done? > >> But if it could be made to work, then we could delay the init. >> >> Re 2, U-Boot expects to change to a new stack when it wants to, which >> is at the boundary of board_init_f() and board_init_r(). The Intel FSP >> should not mandate a stack change over a C function call. IMO that is >> just bad design. Dealing with it is not very easy, but one option is >> to run with the new stack for the rest of the board_init_f() sequence >> and then change stack again at the end of the sequence. Ick. >> >> To specifically address your problem, global_data is not available >> until board_init_f() is called, and the device tree comes into >> existence soon after. We could hack around it - e.g. with microcode we >> find it in the device tree and stick a pointer to it in a special >> place. But the real solution is to figure out how to move this >> fsp_init() stuff to later in the sequence. For non-FSP boards we don't >> have this problem - e.g. ivybridge does RAM init long after we have >> global_data and device tree. Note it is still running from flash at >> this point, but CAR is set up and that is where global_data resides. > > Assuming that we can recover the CAR RAM data, would it make sense to > setup global data while in CAR so we can get access to the device tree > and then call fsp_init(), and once we're running from main memory just > set our global data pointer to the insides of the CAR RAM? CAR becomes unavailable when U-Boot relocates, since we want to use the cache for actual caching rather than for RAM. Currently this happens in init_cache_f_r() for non-FSP platforms, and it would be great if Intel could fix it so that this happens for FSP platforms too (as the link to the new FSP spec suggests). > > Likely I'm missing a few steps, here. But this feels a lot like how SPL > works, except that u-boot is not in charge of how the relocation > happens, we're just along for the ride. Exactly. Note that the device tree is actually available very early - see the fdtdec_setup() call which is one of the first things in the init sequence. The problem is just how to move the FSP init into that sequence, so that global_data is available. Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
