[U-Boot] [RFC PATCH 9/9] x86: quark: Call MRC in dram_init()

[Bin Meng]

Hi Simon,

On Thu, Feb 5, 2015 at 12:25 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 3 February 2015 at 04:45, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Now that we have added Quark MRC codes, call MRC in dram_init() so
>> that DRAM can be initialized on a Quark based board.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>
>> ---
>>
>>  arch/x86/cpu/quark/dram.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++-
>>  arch/x86/dts/galileo.dts  | 25 ++++++++++++
>>  2 files changed, 120 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/cpu/quark/dram.c b/arch/x86/cpu/quark/dram.c
>> index fbdc3cd..3ed1d20 100644
>> --- a/arch/x86/cpu/quark/dram.c
>> +++ b/arch/x86/cpu/quark/dram.c
>> @@ -5,15 +5,108 @@
>>   */
>>
>>  #include <common.h>
>> +#include <errno.h>
>> +#include <fdtdec.h>
>>  #include <asm/post.h>
>> +#include <asm/arch/mrc.h>
>>  #include <asm/arch/quark.h>
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> +static int mrc_configure_params(struct mrc_params *mrc_params)
>> +{
>> +       const void *blob = gd->fdt_blob;
>> +       int node;
>> +       int mrc_flags;
>> +
>> +       node = fdtdec_next_compatible(blob, 0, COMPAT_INTEL_QRK_MRC);
>> +       if (node < 0) {
>> +               debug("%s: Cannot find MRC node\n", __func__);
>> +               return -EINVAL;
>> +       }
>> +
>> +       /*
>> +        * TODO:
>> +        *
>> +        * We need support fast boot (MRC cache) in the future.
>> +        *
>> +        * Set boot mode to cold boot for now
>> +        */
>> +       mrc_params->boot_mode = BM_COLD;
>> +
>> +       /*
>> +        * TODO:
>> +        *
>> +        * We need determine ECC by pin strap state
>> +        *
>> +        * Disable ECC by default for now
>> +        */
>> +       mrc_params->ecc_enables = 0;
>> +
>> +       mrc_flags = fdtdec_get_int(blob, node, "flags", 0);
>> +       if (mrc_flags & MRC_FLAG_SCRAMBLE_EN)
>> +               mrc_params->scrambling_enables = 1;
>> +       else
>> +               mrc_params->scrambling_enables = 0;
>> +
>> +       mrc_params->dram_width = fdtdec_get_int(blob, node, "dram-width", 0);
>> +       mrc_params->ddr_speed = fdtdec_get_int(blob, node, "dram-speed", 0);
>> +       mrc_params->ddr_type = fdtdec_get_int(blob, node, "dram-type", 0);
>> +
>> +       mrc_params->rank_enables = fdtdec_get_int(blob, node, "rank-mask", 0);
>> +       mrc_params->channel_enables = fdtdec_get_int(blob, node,
>> +               "chan-mask", 0);
>> +       mrc_params->channel_width = fdtdec_get_int(blob, node,
>> +               "chan-width", 0);
>> +       mrc_params->address_mode = fdtdec_get_int(blob, node, "addr-mode", 0);
>> +
>> +       mrc_params->refresh_rate = fdtdec_get_int(blob, node,
>> +               "refresh-rate", 0);
>> +       mrc_params->sr_temp_range = fdtdec_get_int(blob, node,
>> +               "sr-temp-range", 0);
>> +       mrc_params->ron_value = fdtdec_get_int(blob, node,
>> +               "ron-value", 0);
>> +       mrc_params->rtt_nom_value = fdtdec_get_int(blob, node,
>> +               "rtt-nom-value", 0);
>> +       mrc_params->rd_odt_value = fdtdec_get_int(blob, node,
>> +               "rd-odt-value", 0);
>> +
>> +       mrc_params->params.density = fdtdec_get_int(blob, node,
>> +               "dram-density", 0);
>> +       mrc_params->params.cl = fdtdec_get_int(blob, node, "dram-cl", 0);
>> +       mrc_params->params.ras = fdtdec_get_int(blob, node, "dram-ras", 0);
>> +       mrc_params->params.wtr = fdtdec_get_int(blob, node, "dram-wtr", 0);
>> +       mrc_params->params.rrd = fdtdec_get_int(blob, node, "dram-rrd", 0);
>> +       mrc_params->params.faw = fdtdec_get_int(blob, node, "dram-faw", 0);
>> +
>> +       debug("MRC dram_width %d\n", mrc_params->dram_width);
>> +       debug("MRC rank_enables %d\n", mrc_params->rank_enables);
>> +       debug("MRC ddr_speed %d\n", mrc_params->ddr_speed);
>> +       debug("MRC flags: %s\n",
>> +             (mrc_params->scrambling_enables) ? "SCRAMBLE_EN" : "");
>> +
>> +       debug("MRC density=%d tCL=%d tRAS=%d tWTR=%d tRRD=%d tFAW=%d\n",
>> +             mrc_params->params.density, mrc_params->params.cl,
>> +             mrc_params->params.ras, mrc_params->params.wtr,
>> +             mrc_params->params.rrd, mrc_params->params.faw);
>> +
>> +       return 0;
>> +}
>> +
>>  int dram_init(void)
>>  {
>> -       /* hardcode the DRAM size for now */
>> -       gd->ram_size = DRAM_MAX_SIZE;
>> +       struct mrc_params mrc_params;
>> +       int ret;
>> +
>> +       memset(&mrc_params, 0, sizeof(struct mrc_params));
>> +       ret = mrc_configure_params(&mrc_params);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* Call MRC */
>
> How about something like:
>
> /* Set up the SDRAM by calling the memory reference code */
>

OK.

>> +       mrc(&mrc_params);
>
> Can this fail?
>

Probably. mrc() itself is a function returning void. If you check the
'struct mrc_params', there is a 'status' member which is assigned
after memory_test() which is supposed to indicate memory is
initialized correctly or not. However during my debug, when the memory
is not initialized correctly (some mistakes during the port) and
U-Boot hangs at relocating fdt, the output message showed that the
status is 0, which I don't understand. The original Intel codes do not
check this status after calling MRC, but I think I can add a check
here.

[snip]

Regards,
Bin