Hi Simon,
On Thu, Feb 5, 2015 at 12:24 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 February 2015 at 04:45, Bin Meng bmeng.cn@gmail.com wrote:
Add the main routines for Quark Memory Reference Code (MRC).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
The are 24 checkpatch warnings in this patch, which is:
warning: arch/x86/cpu/quark/mrc.c,43: line over 80 characters ...
I intentionally leave it as is now, as fixing these warnings make the mrc initialization table a little bit harder to read.
arch/x86/cpu/quark/mrc.c | 206 ++++++++++++++++++++++++++++++++++ arch/x86/include/asm/arch-quark/mrc.h | 189 +++++++++++++++++++++++++++++++ 2 files changed, 395 insertions(+) create mode 100644 arch/x86/cpu/quark/mrc.c create mode 100644 arch/x86/include/asm/arch-quark/mrc.h
diff --git a/arch/x86/cpu/quark/mrc.c b/arch/x86/cpu/quark/mrc.c new file mode 100644 index 0000000..6a82519 --- /dev/null +++ b/arch/x86/cpu/quark/mrc.c @@ -0,0 +1,206 @@ +/*
- Copyright (C) 2013, Intel Corporation
- Copyright (C) 2015, Bin Meng bmeng.cn@gmail.com
- Ported from Intel released Quark UEFI BIOS
- QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/
- SPDX-License-Identifier: Intel
- */
+/*
- This is the main Quark Memory Reference Code (MRC)
- These functions are generic and should work for any Quark based board.
Quark-based
Fixed
- MRC requires two data structures to be passed in which are initialized by
- mrc_adjust_params().
- The basic flow is as follows:
- Check for supported DDR speed configuration
- Set up Memory Manager buffer as pass-through (POR)
- Set Channel Interleaving Mode and Channel Stride to the most aggressive
setting possible
- Set up the Memory Controller logic
- Set up the DDR_PHY logic
- Initialise the DRAMs (JEDEC)
- Perform the Receive Enable Calibration algorithm
- Perform the Write Leveling algorithm
- Perform the Read Training algorithm (includes internal Vref)
- Perform the Write Training algorithm
- Set Channel Interleaving Mode and Channel Stride to the desired settings
- Dunit configuration based on Valleyview MRC.
What is Dunit?
Fixed. DRAM unit.
- */
+#include <common.h> +#include <asm/arch/mrc.h> +#include <asm/arch/msg_port.h> +#include "mrc_util.h" +#include "smc.h"
+static const struct mem_init init[] = {
{ 0x0101, BM_COLD | BM_FAST | BM_WARM | BM_S3, clear_self_refresh },{ 0x0200, BM_COLD | BM_FAST | BM_WARM | BM_S3, prog_ddr_timing_control },{ 0x0103, BM_COLD | BM_FAST , prog_decode_before_jedec },{ 0x0104, BM_COLD | BM_FAST , perform_ddr_reset },{ 0x0300, BM_COLD | BM_FAST | BM_S3, ddrphy_init },{ 0x0400, BM_COLD | BM_FAST , perform_jedec_init },{ 0x0105, BM_COLD | BM_FAST , set_ddr_init_complete },{ 0x0106, BM_FAST | BM_WARM | BM_S3, restore_timings },{ 0x0106, BM_COLD , default_timings },{ 0x0500, BM_COLD , rcvn_cal },{ 0x0600, BM_COLD , wr_level },{ 0x0120, BM_COLD , prog_page_ctrl },{ 0x0700, BM_COLD , rd_train },{ 0x0800, BM_COLD , wr_train },{ 0x010B, BM_COLD , store_timings },{ 0x010C, BM_COLD | BM_FAST | BM_WARM | BM_S3, enable_scrambling },{ 0x010D, BM_COLD | BM_FAST | BM_WARM | BM_S3, prog_ddr_control },{ 0x010E, BM_COLD | BM_FAST | BM_WARM | BM_S3, prog_dra_drb },{ 0x010F, BM_WARM | BM_S3, perform_wake },{ 0x0110, BM_COLD | BM_FAST | BM_WARM | BM_S3, change_refresh_period },{ 0x0111, BM_COLD | BM_FAST | BM_WARM | BM_S3, set_auto_refresh },{ 0x0112, BM_COLD | BM_FAST | BM_WARM | BM_S3, ecc_enable },{ 0x0113, BM_COLD | BM_FAST , memory_test },{ 0x0114, BM_COLD | BM_FAST | BM_WARM | BM_S3, lock_registers }What are the hex codes at the start? Ah I see they are post codes (we don't particularly need them, I'm just asking). Should there be #defines for these? Also how come they use all 16 bits?
Looks Intel chose random numbers for these post codes. Changing them to #define does not seem to work as I don't know how to name them. So I keep these unchanged in v2.
+};
+/* Adjust configuration parameters before initialization sequence */ +static void mrc_adjust_params(struct mrc_params *mrc_params) +{
const struct dram_params *dram_params;uint8_t dram_width;uint32_t rank_enables;uint32_t channel_width;ENTERFN();What is this?
Debug output for tracking function call.
/* initially expect success */mrc_params->status = MRC_SUCCESS;dram_width = mrc_params->dram_width;rank_enables = mrc_params->rank_enables;channel_width = mrc_params->channel_width;/** Setup board layout (must be reviewed as is selecting static timings)* 0 == R0 (DDR3 x16), 1 == R1 (DDR3 x16),* 2 == DV (DDR3 x8), 3 == SV (DDR3 x8).*/if (dram_width == X8)mrc_params->board_id = 2; /* select x8 layout */elsemrc_params->board_id = 0; /* select x16 layout *//* initially no memory */mrc_params->mem_size = 0;/* begin of channel settings */dram_params = &mrc_params->params;/** Determine Column Bits:** Column: 11 for 8Gbx8, else 10*/mrc_params->column_bits[0] =((dram_params[0].density == 4) &&(dram_width == X8)) ? (11) : (10);/** Determine Row Bits:Can we capitalise only the first word in these comments?
Fixed.
** 512Mbx16=12 512Mbx8=13* 1Gbx16=13 1Gbx8=14* 2Gbx16=14 2Gbx8=15* 4Gbx16=15 4Gbx8=16* 8Gbx16=16 8Gbx8=16*/mrc_params->row_bits[0] = 12 + (dram_params[0].density) +(((dram_params[0].density < 4) &&(dram_width == X8)) ? (1) : (0));/** Determine Per Channel Memory Size:per-channel
Fixed.
** (For 2 RANKs, multiply by 2)* (For 16 bit data bus, divide by 2)** DENSITY WIDTH MEM_AVAILABLE* 512Mb x16 0x008000000 ( 128MB)* 512Mb x8 0x010000000 ( 256MB)* 1Gb x16 0x010000000 ( 256MB)* 1Gb x8 0x020000000 ( 512MB)* 2Gb x16 0x020000000 ( 512MB)* 2Gb x8 0x040000000 (1024MB)* 4Gb x16 0x040000000 (1024MB)* 4Gb x8 0x080000000 (2048MB)*/mrc_params->channel_size[0] = (1 << dram_params[0].density);mrc_params->channel_size[0] *= (dram_width == X8) ? (2) : (1);mrc_params->channel_size[0] *= (rank_enables == 0x3) ? (2) : (1);mrc_params->channel_size[0] *= (channel_width == X16) ? (1) : (2);Remove () around 2 and 1.
Fixed
/* Determine memory size (convert number of 64MB/512Mb units) */mrc_params->mem_size += mrc_params->channel_size[0] << 26;LEAVEFN();?
Debug output for tracking function return.
+}
+static void mrc_init(struct mrc_params *mrc_params) +{
int i;ENTERFN();DPF(D_INFO, "mrc_init build %s %s\n", __DATE__, __TIME__);debug() I think, and below.
DPF is the MRC debug routine, and I wanted to keep using it for MRC codes. And in v2, I removed this line completely.
/* MRC started */mrc_post_code(0x01, 0x00);if (mrc_params->boot_mode != BM_COLD) {if (mrc_params->ddr_speed != mrc_params->timings.ddr_speed) {/* full training required as frequency changed */mrc_params->boot_mode = BM_COLD;}}for (i = 0; i < ARRAY_SIZE(init); i++) {uint64_t my_tsc;if (mrc_params->boot_mode & init[i].boot_path) {uint8_t major = init[i].post_code >> 8 & 0xFF;uint8_t minor = init[i].post_code >> 0 & 0xFF;Can we stick with lower case hex, and below?
Fixed.
mrc_post_code(major, minor);my_tsc = rdtsc();init[i].init_fn(mrc_params);DPF(D_TIME, "Execution time %llx", rdtsc() - my_tsc);}}/* display the timings */print_timings(mrc_params);/* MRC complete */mrc_post_code(0x01, 0xFF);
Fixed, using lower case hex.
LEAVEFN();+}
+void mrc(struct mrc_params *mrc_params) +{
ENTERFN();DPF(D_INFO, "MRC Version %04x %s %s\n",MRC_VERSION, __DATE__, __TIME__);Can you reformat so more args on first line?
Fixed.
/* Set up the data structures used by mrc_init() */mrc_adjust_params(mrc_params);/* Initialize system memory */mrc_init(mrc_params);LEAVEFN();+} diff --git a/arch/x86/include/asm/arch-quark/mrc.h b/arch/x86/include/asm/arch-quark/mrc.h new file mode 100644 index 0000000..690a800 --- /dev/null +++ b/arch/x86/include/asm/arch-quark/mrc.h @@ -0,0 +1,189 @@ +/*
- Copyright (C) 2013, Intel Corporation
- Copyright (C) 2015, Bin Meng bmeng.cn@gmail.com
- Ported from Intel released Quark UEFI BIOS
- QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/
- SPDX-License-Identifier: Intel
- */
+#ifndef _MRC_H_ +#define _MRC_H_
+/* MRC Version */
I think you can drop that comment!
Fixed.
+#define MRC_VERSION 0x0111
+/* architectural definitions */ +#define NUM_CHANNELS 1 /* number of channels */ +#define NUM_RANKS 2 /* number of ranks per channel */ +#define NUM_BYTE_LANES 4 /* number of byte lanes per channel */
+/* software limitations */ +#define MAX_CHANNELS 1 +#define MAX_RANKS 2 +#define MAX_BYTE_LANES 4
+/* only to mock MrcWrapper */
What does this mean?
I don't know, just removed this line in v2.
+#define MAX_SOCKETS 1 +#define MAX_SIDES 1 +#define MAX_ROWS (MAX_SIDES * MAX_SOCKETS)
+/* Specify DRAM of nenory channel width */
memory
Also this doesn't quite make sense - can you please reword it?
Changed the comment to: /* Specify DRAM and channel width */ in v2.
+enum {
X8, /* DRAM width */X16, /* DRAM width & Channel Width */X32 /* Channel Width */+};
+/* Specify DRAM speed */ +enum {
DDRFREQ_800,DDRFREQ_1066+};
+/* Specify DRAM type */ +enum {
DDR3,DDR3L+};
+/*
- density: 0=512Mb, 1=Gb, 2=2Gb, 3=4Gb
should either have @density in this header and all the others here too. Or move this comment below above density.
Fixed.
- cl is DRAM CAS Latency in clocks
- All other timings are in picoseconds
- Refer to JEDEC spec (or DRAM datasheet) when changing these values.
- */
+struct dram_params {
uint8_t density;/* CAS latency in clocks */uint8_t cl;/* ACT to PRE command period */uint32_t ras;/** Delay from start of internal write transaction to* internal read command*/uint32_t wtr;/* ACT to ACT command period (JESD79 specific to page size 1K/2K) */uint32_t rrd;/* Four activate window (JESD79 specific to page size 1K/2K) */uint32_t faw;+};
+/*
- Delay configuration for individual signals
- Vref setting
- Scrambler seed
What do the above two lines mean?
I think this is DDR technology term. I did not change this in v2.
- */
+struct mrc_timings {
uint32_t rcvn[NUM_CHANNELS][NUM_RANKS][NUM_BYTE_LANES];uint32_t rdqs[NUM_CHANNELS][NUM_RANKS][NUM_BYTE_LANES];uint32_t wdqs[NUM_CHANNELS][NUM_RANKS][NUM_BYTE_LANES];uint32_t wdq[NUM_CHANNELS][NUM_RANKS][NUM_BYTE_LANES];uint32_t vref[NUM_CHANNELS][NUM_BYTE_LANES];uint32_t wctl[NUM_CHANNELS][NUM_RANKS];uint32_t wcmd[NUM_CHANNELS];uint32_t scrambler_seed;Comments for the above?
Again too DDR-specific terms. I did not add any comment to this.
/* need to save for the case of frequency change */uint8_t ddr_speed;+};
+/* Boot mode defined as bit mask (1<<n) */ +enum {
BM_UNKNOWN,BM_COLD = 1, /* full training */BM_FAST = 2, /* restore timing parameters */BM_S3 = 4, /* resume from S3 */BM_WARM = 8+};
+/* MRC execution status */ +#define MRC_SUCCESS 0 /* initialization ok */ +#define MRC_E_MEMTEST 1 /* memtest failed */
+/* Input/output/context parameters for Memory Reference Code */ +struct mrc_params {
/* Global Settings *//* BM_COLD, BM_FAST, BM_WARM, BM_S3 */uint32_t boot_mode;uint8_t first_run;/* DRAM Parameters */Remove blank line
Fixed.
uint8_t dram_width; /* x8, x16 */uint8_t ddr_speed; /* DDRFREQ_800, DDRFREQ_1066 */uint8_t ddr_type; /* DDR3, DDR3L */uint8_t ecc_enables; /* 0, 1 (memory size reduced to 7/8) */uint8_t scrambling_enables; /* 0, 1 *//* 1, 3 (1'st rank has to be populated if 2'nd rank present) */uint32_t rank_enables;uint32_t channel_enables; /* 1 only */uint32_t channel_width; /* x16 only *//* 0, 1, 2 (mode 2 forced if ecc enabled) */uint32_t address_mode;/* REFRESH_RATE: 1=1.95us, 2=3.9us, 3=7.8us, others=RESERVED */uint8_t refresh_rate;/* SR_TEMP_RANGE: 0=normal, 1=extended, others=RESERVED */uint8_t sr_temp_range;/** RON_VALUE: 0=34ohm, 1=40ohm, others=RESERVED* (select MRS1.DIC driver impedance control)*/uint8_t ron_value;/* RTT_NOM_VALUE: 0=40ohm, 1=60ohm, 2=120ohm, others=RESERVED */uint8_t rtt_nom_value;/* RD_ODT_VALUE: 0=off, 1=60ohm, 2=120ohm, 3=180ohm, others=RESERVED */uint8_t rd_odt_value;struct dram_params params;/* Internally Used */I think I know what this means? It's unfortunate to have input/output/working data in the same structure but this seems to be the approach taken, so let's keep it. But can you add a comment above the struct saying how it is split into multiple parts?
Fixed.
/* internally used for board layout (use x8 or x16 memory) */uint32_t board_id;/* when set hte reconfiguration requested */uint32_t hte_setup:1;uint32_t menu_after_mrc:1;uint32_t power_down_disable:1;uint32_t tune_rcvn:1;Should these be bool? I'm not sure the :1 helps much - are you trying to save memory?
I just removed the :1 in the v2.
uint32_t channel_size[NUM_CHANNELS];uint32_t column_bits[NUM_CHANNELS];uint32_t row_bits[NUM_CHANNELS];/* register content saved during training */uint32_t mrs1;/* Output *//* initialization result (non zero specifies error code) */uint32_t status;/* total memory size in bytes (excludes ECC banks) */uint32_t mem_size;/* training results (also used on input) */struct mrc_timings timings;+};
This one needs comments:
Fixed.
+struct mem_init {
uint16_t post_code;uint16_t boot_path;void (*init_fn)(struct mrc_params *mrc_params);+};
+/* MRC platform data flags */ +#define MRC_FLAG_ECC_EN 0x00000001 +#define MRC_FLAG_SCRAMBLE_EN 0x00000002 +#define MRC_FLAG_MEMTEST_EN 0x00000004 +/* 0b DDR "fly-by" topology else 1b DDR "tree" topology */ +#define MRC_FLAG_TOP_TREE_EN 0x00000008 +/* If set ODR signal is asserted to DRAM devices on writes */ +#define MRC_FLAG_WR_ODT_EN 0x00000010
+/**
- mrc - Memory Reference Code entry routine
- @mrc_params: parameters for MRC
- */
+void mrc(struct mrc_params *mrc_params);
How about sdram_init() or mrc_init()?
Changed it to mrc_init() in v2.
+#endif /* _MRC_H_ */
1.8.2.1
Regards, Simon
Regards, Bin