[U-Boot] [PATCH v2 22/22] x86: Add support for Intel Minnowboard Max

[Simon Glass]

Hi Andrew,

On 26 May 2015 at 13:52, Andrew Bradford <andrew at bradfordembedded.com> 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---------------
>> diff --git a/arch/x86/cpu/baytrail/fsp_configs.c b/arch/x86/cpu/baytrail/fsp_configs.c
>> new file mode 100644
>> index 0000000..86b6926
>> --- /dev/null
>> +++ b/arch/x86/cpu/baytrail/fsp_configs.c
>> @@ -0,0 +1,156 @@
>> +/*
>> + * Copyright (C) 2013, Intel Corporation
>> + * Copyright (C) 2014, Bin Meng <bmeng.cn at gmail.com>
>> + *
>> + * SPDX-License-Identifier:  Intel
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/arch/fsp/azalia.h>
>> +#include <asm/fsp/fsp_support.h>
>> +
>> +/* ALC262 Verb Table - 10EC0262 */
>> +static const uint32_t verb_table_data13[] = {
>> +     /* Pin Complex (NID 0x11) */
>> +     0x01171cf0,
>> +     0x01171d11,
>> +     0x01171e11,
>> +     0x01171f41,
>> +     /* Pin Complex (NID 0x12) */
>> +     0x01271cf0,
>> +     0x01271d11,
>> +     0x01271e11,
>> +     0x01271f41,
>> +     /* Pin Complex (NID 0x14) */
>> +     0x01471c10,
>> +     0x01471d40,
>> +     0x01471e01,
>> +     0x01471f01,
>> +     /* Pin Complex (NID 0x15) */
>> +     0x01571cf0,
>> +     0x01571d11,
>> +     0x01571e11,
>> +     0x01571f41,
>> +     /* Pin Complex (NID 0x16) */
>> +     0x01671cf0,
>> +     0x01671d11,
>> +     0x01671e11,
>> +     0x01671f41,
>> +     /* Pin Complex (NID 0x18) */
>> +     0x01871c20,
>> +     0x01871d98,
>> +     0x01871ea1,
>> +     0x01871f01,
>> +     /* Pin Complex (NID 0x19) */
>> +     0x01971c21,
>> +     0x01971d98,
>> +     0x01971ea1,
>> +     0x01971f02,
>> +     /* Pin Complex (NID 0x1A) */
>> +     0x01a71c2f,
>> +     0x01a71d30,
>> +     0x01a71e81,
>> +     0x01a71f01,
>> +     /* Pin Complex */
>> +     0x01b71c1f,
>> +     0x01b71d40,
>> +     0x01b71e21,
>> +     0x01b71f02,
>> +     /* Pin Complex */
>> +     0x01c71cf0,
>> +     0x01c71d11,
>> +     0x01c71e11,
>> +     0x01c71f41,
>> +     /* Pin Complex */
>> +     0x01d71c01,
>> +     0x01d71dc6,
>> +     0x01d71e14,
>> +     0x01d71f40,
>> +     /* Pin Complex */
>> +     0x01e71cf0,
>> +     0x01e71d11,
>> +     0x01e71e11,
>> +     0x01e71f41,
>> +     /* Pin Complex */
>> +     0x01f71cf0,
>> +     0x01f71d11,
>> +     0x01f71e11,
>> +     0x01f71f41,
>> +};
>> +
>> +/*
>> + * This needs to be in ROM since if we put it in CAR, FSP init loses it when
>> + * it drops CAR.
>> + *
>> + * TODO(sjg at chromium.org): Move to device tree when FSP allows it
>> + *
>> + * VerbTable: (RealTek ALC262)
>> + * Revision ID = 0xFF, support all steps
>> + * Codec Verb Table For AZALIA
>> + * Codec Address: CAd value (0/1/2)
>> + * Codec Vendor: 0x10EC0262
>> + */
>> +static const struct pch_azalia_verb_table azalia_verb_table[] = {
>> +     {
>> +             {
>> +                     0x10ec0262,
>> +                     0x0000,
>> +                     0xff,
>> +                     0x01,
>> +                     0x000b,
>> +                     0x0002,
>> +             },
>> +             verb_table_data13
>> +     }
>> +};
>> +
>> +const struct pch_azalia_config azalia_config = {
>> +     .pme_enable = 1,
>> +     .docking_supported = 1,
>> +     .docking_attached = 0,
>> +     .hdmi_codec_enable = 1,
>> +     .azalia_v_ci_enable = 1,
>> +     .rsvdbits = 0,
>> +     .azalia_verb_table_num = 1,
>> +     .azalia_verb_table = azalia_verb_table,
>> +     .reset_wait_timer_us = 300
>> +};
>> +
>> +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(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;
>> +
>> +     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.

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.

I'm interested to hear what you figure out.

>
> Thanks for your help!
> -Andrew

Regards,
Simon