Re: [U-Boot] [PATCH 4/4] Exynos5420: DMC: Add software read leveling

2014-05-23 Thread Simon Glass
Hi Akshay,

On 21 May 2014 23:33, Akshay Saraswat aksha...@samsung.com wrote:
 Sometimes Read DQ and DQS are not in phase. Since, this
 phase shift differs from board to board, we need to
 calibrate it at DRAM init phase, that's read DQ calibration.
 This patch adds SW Read DQ calibration routine to compensate
 this skew.

The English could be a bit better here - also suggest formatting to 72
columns or more.


 Signed-off-by: Alim Akhtar alim.akh...@samsung.com
 Signed-off-by: Akshay Saraswat aksha...@samsung.com
 ---
  arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c | 244 
 +-
  arch/arm/cpu/armv7/exynos/exynos5_setup.h |   5 +-
  arch/arm/include/asm/arch-exynos/dmc.h|   3 +
  arch/arm/include/asm/arch-exynos/power.h  |   4 +-
  4 files changed, 252 insertions(+), 4 deletions(-)

 diff --git a/arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c 
 b/arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c
 index 0654c6a..a4f6099 100644
 --- a/arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c
 +++ b/arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c
 @@ -6,6 +6,7 @@
   * SPDX-License-Identifier:GPL-2.0+
   */

 +#include common.h
  #include config.h
  #include asm/io.h
  #include asm/arch/clock.h
 @@ -16,7 +17,12 @@
  #include exynos5_setup.h
  #include clock_init.h

 -#define TIMEOUT1
 +#define TIMEOUT1

Is this in milliseconds? If so TIMEOUT_MS, or maybe microseconds, TIMEOUT _US

 +#define NUM_BYTE_LANES 4
 +#define DEFAULT_DQS8
 +#define DEFAULT_DQS_X4 0x08080808

What does this mean?

 +#define FALSE  0
 +#define TRUE   1

You can just use true and false in U-Boot now (the bool type).


  #ifdef CONFIG_EXYNOS5250
  static void reset_phy_ctrl(void)
 @@ -222,6 +228,219 @@ int ddr3_mem_ctrl_init(int reset)
  #endif

  #ifdef CONFIG_EXYNOS5420
 +/*
 + * RAM address to use in the test.
 + *
 + * We'll use 4 words at this address and 4 at this address + 0x80 (Ares
 + * interleaves channels every 128 bytes).  This will allow us to evaluate 
 all of
 + * the chips in a 1 chip per channel (2GB) system and half the chips in a 2
 + * chip per channel (4GB) system.  We can't test the 2nd chip since we need 
 to
 + * do tests before the 2nd chip is enabled.  Looking at the 2nd chip isn't
 + * critical because the 1st and 2nd chip have very similar timings (they'd
 + * better have similar timings, since there's only a single adjustment that 
 is
 + * shared by both chips).
 + */
 +const unsigned int test_addr = 0x2000;

Isn't this CONFIG_SYS_SDRAM_BASE?

 +
 +/* Test pattern with which RAM will be tested */
 +static const unsigned int test_pattern[] = {
 +   0x5A5A5A5A,
 +   0xA5A5A5A5,
 +   0xF0F0F0F0,
 +   0x0F0F0F0F,

Lower case hex throughout?

 +};
 +
 +/*

/**

Should fix for each function.

 + * This function is a test vector for sw read leveling,
 + * it compares the read data with the written data.
 + *
 + * @param ch   DMC channel number
 + * @param byte_lanewhich DQS byte offset,
 + * possible values are 0,1,2,3
 + * @returnsTRUE if memory was good, FALSE if not.

@return

 + */
 +static int dmc_valid_window_test_vector(int ch, int byte_lane)
 +{
 +   unsigned int read_data;
 +   unsigned int mask;
 +   int i;
 +
 +   mask = 0xFF  (8 * byte_lane);
 +
 +   for (i = 0; i  ARRAY_SIZE(test_pattern); i++) {
 +   read_data = readl(test_addr + i*4 + ch*0x80);

Spaces around * operators

 +   if ((read_data  mask) != (test_pattern[i]  mask))
 +   return FALSE;
 +   }
 +
 +   return TRUE;
 +}
 +
 +/*
 + * This function returns current read offset value.
 + *
 + * @param phy_ctrl pointer to the current phy controller
 + */
 +static unsigned int dmc_get_read_offset_value(struct exynos5420_phy_control
 +  *phy_ctrl)
 +{
 +   return readl(phy_ctrl-phy_con4);
 +}
 +
 +/*
 + * This function performs resync, so that slave DLL is updated.
 + *
 + * @param phy_ctrl pointer to the current phy controller
 + */
 +static void ddr_phy_set_do_resync(struct exynos5420_phy_control *phy_ctrl)
 +{
 +   setbits_le32(phy_ctrl-phy_con10, PHY_CON10_CTRL_OFFSETR3);
 +   clrbits_le32(phy_ctrl-phy_con10, PHY_CON10_CTRL_OFFSETR3);
 +}
 +
 +/*
 + * This function sets read offset value register with 'offset'.
 + *
 + * ...we also call call ddr_phy_set_do_resync().
 + *
 + * @param phy_ctrl pointer to the current phy controller
 + * @param offset   offset to read DQS
 + */
 +static void dmc_set_read_offset_value(struct exynos5420_phy_control 
 *phy_ctrl,
 + unsigned int offset)
 +{
 +   writel(offset, phy_ctrl-phy_con4);
 +   ddr_phy_set_do_resync(phy_ctrl);
 +}
 +
 +/*
 + * Convert a 2s complement byte to a byte with a sign bit.
 + *
 + * NOTE: you shouldn't use normal math 

[U-Boot] [PATCH 4/4] Exynos5420: DMC: Add software read leveling

2014-05-22 Thread Akshay Saraswat
Sometimes Read DQ and DQS are not in phase. Since, this
phase shift differs from board to board, we need to
calibrate it at DRAM init phase, that's read DQ calibration.
This patch adds SW Read DQ calibration routine to compensate
this skew.

Signed-off-by: Alim Akhtar alim.akh...@samsung.com
Signed-off-by: Akshay Saraswat aksha...@samsung.com
---
 arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c | 244 +-
 arch/arm/cpu/armv7/exynos/exynos5_setup.h |   5 +-
 arch/arm/include/asm/arch-exynos/dmc.h|   3 +
 arch/arm/include/asm/arch-exynos/power.h  |   4 +-
 4 files changed, 252 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c 
b/arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c
index 0654c6a..a4f6099 100644
--- a/arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c
+++ b/arch/arm/cpu/armv7/exynos/dmc_init_ddr3.c
@@ -6,6 +6,7 @@
  * SPDX-License-Identifier:GPL-2.0+
  */
 
+#include common.h
 #include config.h
 #include asm/io.h
 #include asm/arch/clock.h
@@ -16,7 +17,12 @@
 #include exynos5_setup.h
 #include clock_init.h
 
-#define TIMEOUT1
+#define TIMEOUT1
+#define NUM_BYTE_LANES 4
+#define DEFAULT_DQS8
+#define DEFAULT_DQS_X4 0x08080808
+#define FALSE  0
+#define TRUE   1
 
 #ifdef CONFIG_EXYNOS5250
 static void reset_phy_ctrl(void)
@@ -222,6 +228,219 @@ int ddr3_mem_ctrl_init(int reset)
 #endif
 
 #ifdef CONFIG_EXYNOS5420
+/*
+ * RAM address to use in the test.
+ *
+ * We'll use 4 words at this address and 4 at this address + 0x80 (Ares
+ * interleaves channels every 128 bytes).  This will allow us to evaluate all 
of
+ * the chips in a 1 chip per channel (2GB) system and half the chips in a 2
+ * chip per channel (4GB) system.  We can't test the 2nd chip since we need to
+ * do tests before the 2nd chip is enabled.  Looking at the 2nd chip isn't
+ * critical because the 1st and 2nd chip have very similar timings (they'd
+ * better have similar timings, since there's only a single adjustment that is
+ * shared by both chips).
+ */
+const unsigned int test_addr = 0x2000;
+
+/* Test pattern with which RAM will be tested */
+static const unsigned int test_pattern[] = {
+   0x5A5A5A5A,
+   0xA5A5A5A5,
+   0xF0F0F0F0,
+   0x0F0F0F0F,
+};
+
+/*
+ * This function is a test vector for sw read leveling,
+ * it compares the read data with the written data.
+ *
+ * @param ch   DMC channel number
+ * @param byte_lanewhich DQS byte offset,
+ * possible values are 0,1,2,3
+ * @returnsTRUE if memory was good, FALSE if not.
+ */
+static int dmc_valid_window_test_vector(int ch, int byte_lane)
+{
+   unsigned int read_data;
+   unsigned int mask;
+   int i;
+
+   mask = 0xFF  (8 * byte_lane);
+
+   for (i = 0; i  ARRAY_SIZE(test_pattern); i++) {
+   read_data = readl(test_addr + i*4 + ch*0x80);
+   if ((read_data  mask) != (test_pattern[i]  mask))
+   return FALSE;
+   }
+
+   return TRUE;
+}
+
+/*
+ * This function returns current read offset value.
+ *
+ * @param phy_ctrl pointer to the current phy controller
+ */
+static unsigned int dmc_get_read_offset_value(struct exynos5420_phy_control
+  *phy_ctrl)
+{
+   return readl(phy_ctrl-phy_con4);
+}
+
+/*
+ * This function performs resync, so that slave DLL is updated.
+ *
+ * @param phy_ctrl pointer to the current phy controller
+ */
+static void ddr_phy_set_do_resync(struct exynos5420_phy_control *phy_ctrl)
+{
+   setbits_le32(phy_ctrl-phy_con10, PHY_CON10_CTRL_OFFSETR3);
+   clrbits_le32(phy_ctrl-phy_con10, PHY_CON10_CTRL_OFFSETR3);
+}
+
+/*
+ * This function sets read offset value register with 'offset'.
+ *
+ * ...we also call call ddr_phy_set_do_resync().
+ *
+ * @param phy_ctrl pointer to the current phy controller
+ * @param offset   offset to read DQS
+ */
+static void dmc_set_read_offset_value(struct exynos5420_phy_control *phy_ctrl,
+ unsigned int offset)
+{
+   writel(offset, phy_ctrl-phy_con4);
+   ddr_phy_set_do_resync(phy_ctrl);
+}
+
+/*
+ * Convert a 2s complement byte to a byte with a sign bit.
+ *
+ * NOTE: you shouldn't use normal math on the number returned by this function.
+ *   As an example, -10 = 0xf6.  After this function -10 = 0x8a.  If you wanted
+ *   to do math and get the average of 10 and -10 (should be 0):
+ * 0x8a + 0xa = 0x94 (-108)
+ * 0x94 / 2   = 0xca (-54)
+ *   ...and 0xca = sign bit plus 0x4a, or -74
+ *
+ * Also note that you lose the ability to represent -128 since there are two
+ * representations of 0.
+ *
+ * @param bThe byte to convert in two's complement.
+ * @returnsThe 7-bit value + sign bit.
+ */
+
+unsigned char make_signed_byte(signed char b)
+{
+   if (b  0)
+   return