Re: [U-Boot] [PATCH v2 12/19] arm: socfpga: Add DDR driver for Arria 10

On 09/25/2017 10:40 AM, tien.fong.chee@intel.com wrote:

From: Tien Fong Chee tien.fong.chee@intel.com

Add DDR driver suppport for Arria 10.

Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com

arch/arm/mach-socfpga/include/mach/sdram.h | 2 + arch/arm/mach-socfpga/include/mach/sdram_arria10.h | 103 ++- drivers/ddr/altera/sdram_arria10.c | 735 +++++++++++++++++++++ 3 files changed, 839 insertions(+), 1 deletion(-) create mode 100644 drivers/ddr/altera/sdram_arria10.c

diff --git a/arch/arm/mach-socfpga/include/mach/sdram.h b/arch/arm/mach-socfpga/include/mach/sdram.h index 4a9754e..b833fc2 100644 --- a/arch/arm/mach-socfpga/include/mach/sdram.h +++ b/arch/arm/mach-socfpga/include/mach/sdram.h @@ -10,6 +10,8 @@

#if defined(CONFIG_TARGET_SOCFPGA_GEN5) #include <asm/arch/sdram_gen5.h> +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10) +#include <asm/arch/sdram_arria10.h> #endif

#endif diff --git a/arch/arm/mach-socfpga/include/mach/sdram_arria10.h b/arch/arm/mach-socfpga/include/mach/sdram_arria10.h index 1d7b7c1..7af9431 100644 --- a/arch/arm/mach-socfpga/include/mach/sdram_arria10.h +++ b/arch/arm/mach-socfpga/include/mach/sdram_arria10.h @@ -1,5 +1,5 @@ /*

• • Copyright (C) 2015-2017 Intel Corporation <www.intel.com>

• • Copyright (C) 2017 Intel Corporation <www.intel.com>
  • SPDX-License-Identifier: GPL-2.0
  */

@@ -8,6 +8,7 @@ #define _SOCFPGA_SDRAM_ARRIA10_H_

#ifndef __ASSEMBLY__ +int ddr_calibration_sequence(void);

struct socfpga_ecc_hmc { u32 ip_rev_id; @@ -204,6 +205,106 @@ struct socfpga_io48_mmr { u32 niosreserve1; u32 niosreserve2; };

+union dramaddrw_reg {

• struct {

• u32 cfg_col_addr_width:5;
  

• u32 cfg_row_addr_width:5;
  

• u32 cfg_bank_addr_width:4;
  

• u32 cfg_bank_group_addr_width:2;
  

• u32 cfg_cs_addr_width:3;
  

• u32 reserved:13;
  

• };

Use regular macros for bitfields, not this crap.

• u32 word;

+};

+union ctrlcfg0_reg {

• struct {

• u32 cfg_mem_type:4;
  

• u32 cfg_dimm_type:3;
  

• u32 cfg_ac_pos:2;
  

• u32 cfg_ctrl_burst_len:5;
  

• u32 reserved:18;  /* Other fields unused */
  

• };
• u32 word;

+};

[...]

diff --git a/drivers/ddr/altera/sdram_arria10.c b/drivers/ddr/altera/sdram_arria10.c new file mode 100644 index 0000000..f22a726 --- /dev/null +++ b/drivers/ddr/altera/sdram_arria10.c @@ -0,0 +1,735 @@ +/*

• • Copyright (C) 2017 Intel Corporation <www.intel.com>
• • SPDX-License-Identifier: GPL-2.0
• */

+#include <common.h> +#include <fdtdec.h> +#include <malloc.h> +#include <mmc.h> +#include <nand.h> +#include <watchdog.h> +#include <ns16550.h>

Why does an SDRAM init driver need access to ns16550 UART , MMC and NAND ?

+#include <asm/io.h> +#include <asm/arch/fpga_manager.h> +#include <asm/arch/misc.h> +#include <asm/arch/reset_manager.h> +#include <asm/arch/sdram.h>

+DECLARE_GLOBAL_DATA_PTR;

+static void sdram_mmr_init(void); +static unsigned long long sdram_size_calc(void);

+/* FAWBANK - Number of Bank of a given device involved in the FAW period. */ +#define ARRIA10_SDR_ACTIVATE_FAWBANK (0x1)

+#define ARRIA_DDR_CONFIG(A, B, C, R) ((A<<24)|(B<<16)|(C<<8)|R)

Missing parenthesis around (A), (B) etc. ...

+#define DDR_CONFIG_ELEMENTS (sizeof(ddr_config)/sizeof(u32))

So this is ARRAY_SIZE() ?

+#define DDR_REG_SEQ2CORE 0xFFD0507C +#define DDR_REG_CORE2SEQ 0xFFD05078 +#define DDR_READ_LATENCY_DELAY 40 +#define DDR_SIZE_2GB_HEX 0x80000000 +#define DDR_MAX_TRIES 0x00100000

+#define IO48_MMR_DRAMSTS 0xFFCFA0EC +#define IO48_MMR_NIOS2_RESERVE0 0xFFCFA110 +#define IO48_MMR_NIOS2_RESERVE1 0xFFCFA114 +#define IO48_MMR_NIOS2_RESERVE2 0xFFCFA118

+#define SEQ2CORE_MASK 0xF +#define CORE2SEQ_INT_REQ 0xF +#define SEQ2CORE_INT_RESP_BIT 3

+static const struct socfpga_ecc_hmc *socfpga_ecc_hmc_base =

• (void *)SOCFPGA_SDR_ADDRESS;
  

+static const struct socfpga_noc_ddr_scheduler *socfpga_noc_ddr_scheduler_base =

• (void *)SOCFPGA_SDR_SCHEDULER_ADDRESS;
  

+static const struct socfpga_noc_fw_ddr_mpu_fpga2sdram

• *socfpga_noc_fw_ddr_mpu_fpga2sdram_base =
  

• (void *)SOCFPGA_SDR_FIREWALL_MPU_FPGA_ADDRESS;
  

+static const struct socfpga_noc_fw_ddr_l3 *socfpga_noc_fw_ddr_l3_base =

• (void *)SOCFPGA_SDR_FIREWALL_L3_ADDRESS;
  

+static const struct socfpga_io48_mmr *socfpga_io48_mmr_base =

• (void *)SOCFPGA_HMC_MMR_IO48_ADDRESS;
  

+/* The followring are the supported configurations */ +static u32 ddr_config[] = {

• /* Chip - Row - Bank - Column Style */
• /* All Types */
• ARRIA_DDR_CONFIG(0, 3, 10, 12),
• ARRIA_DDR_CONFIG(0, 3, 10, 13),
• ARRIA_DDR_CONFIG(0, 3, 10, 14),
• ARRIA_DDR_CONFIG(0, 3, 10, 15),
• ARRIA_DDR_CONFIG(0, 3, 10, 16),
• ARRIA_DDR_CONFIG(0, 3, 10, 17),
• /* LPDDR x16 */
• ARRIA_DDR_CONFIG(0, 3, 11, 14),
• ARRIA_DDR_CONFIG(0, 3, 11, 15),
• ARRIA_DDR_CONFIG(0, 3, 11, 16),
• ARRIA_DDR_CONFIG(0, 3, 12, 15),
• /* DDR4 Only */
• ARRIA_DDR_CONFIG(0, 4, 10, 14),
• ARRIA_DDR_CONFIG(0, 4, 10, 15),
• ARRIA_DDR_CONFIG(0, 4, 10, 16),
• ARRIA_DDR_CONFIG(0, 4, 10, 17), /* 14 */
• /* Chip - Bank - Row - Column Style */
• ARRIA_DDR_CONFIG(1, 3, 10, 12),
• ARRIA_DDR_CONFIG(1, 3, 10, 13),
• ARRIA_DDR_CONFIG(1, 3, 10, 14),
• ARRIA_DDR_CONFIG(1, 3, 10, 15),
• ARRIA_DDR_CONFIG(1, 3, 10, 16),
• ARRIA_DDR_CONFIG(1, 3, 10, 17),
• ARRIA_DDR_CONFIG(1, 3, 11, 14),
• ARRIA_DDR_CONFIG(1, 3, 11, 15),
• ARRIA_DDR_CONFIG(1, 3, 11, 16),
• ARRIA_DDR_CONFIG(1, 3, 12, 15),
• /* DDR4 Only */
• ARRIA_DDR_CONFIG(1, 4, 10, 14),
• ARRIA_DDR_CONFIG(1, 4, 10, 15),
• ARRIA_DDR_CONFIG(1, 4, 10, 16),
• ARRIA_DDR_CONFIG(1, 4, 10, 17),

+};

+static int match_ddr_conf(u32 ddr_conf) +{

• int i;
• for (i = 0; i < DDR_CONFIG_ELEMENTS; i++) {

• if (ddr_conf == ddr_config[i])
  

• 	return i;
  

• }
• return 0;

+}

+/* Check whether SDRAM is successfully Calibrated */ +static int is_sdram_cal_success(void) +{

• return readl(&socfpga_ecc_hmc_base->ddrcalstat);

+}

+static unsigned char ddr_get_bit(u32 ereg, unsigned char bit) +{

• u32 reg = readl(ereg);
• return (reg & (1 << bit)) ? 1 : 0;

Urgh, BIT() and just use it inline and drop this

+}

+static unsigned char ddr_wait_bit(u32 ereg, u32 bit,

• 	   u32 expected, u32 timeout_usec)
  

+{

• u32 tmr;
• for (tmr = 0; tmr < timeout_usec; tmr += 100) {

• udelay(100);
  

• WATCHDOG_RESET();
  

• if (ddr_get_bit(ereg, bit) == expected)
  

• 	return 0;
  

• }
• return 1;

+}

+static void ddr_delay(u32 delay) +{

• int tmr;
• for (tmr = 0; tmr < delay; tmr++) {

• udelay(1000);
  

• WATCHDOG_RESET();
  

So this is mdelay(delay) ?

• }

+}

+static int emif_clear(void) +{

• u32 s2c;
• u32 i = DDR_MAX_TRIES;
• writel(0, DDR_REG_CORE2SEQ);
• do {

• ddr_delay(50);
  

• s2c = readl(DDR_REG_SEQ2CORE);
  

• } while ((s2c & SEQ2CORE_MASK) && (--i > 0));

wait_for_bit() or something ?

• return !i;

+}

+static int emif_reset(void) +{

• u32 c2s, s2c;
• c2s = readl(DDR_REG_CORE2SEQ);
• s2c = readl(DDR_REG_SEQ2CORE);
• debug("c2s=%08x s2c=%08x nr0=%08x nr1=%08x nr2=%08x dst=%08x\n",

• c2s, s2c, readl(IO48_MMR_NIOS2_RESERVE0),
  

• readl(IO48_MMR_NIOS2_RESERVE1),
  

• readl(IO48_MMR_NIOS2_RESERVE2),
  

• readl(IO48_MMR_DRAMSTS));
  

• if ((s2c & SEQ2CORE_MASK) && emif_clear()) {

• printf("failed emif_clear()\n");
  

• return -1;
  

• }
• writel(CORE2SEQ_INT_REQ, DDR_REG_CORE2SEQ);
• if (ddr_wait_bit(DDR_REG_SEQ2CORE, SEQ2CORE_INT_RESP_BIT, 0, 1000000)) {

• printf("emif_reset failed to see interrupt acknowledge\n");
  

• return -2;
  

Use proper errno.h codes

• } else {

• printf("emif_reset interrupt acknowledged\n");
  

• }
• if (emif_clear()) {

• printf("emif_clear() failed\n");
  

• return -3;
  

• }
• debug("emif_reset interrupt cleared\n");
• debug("nr0=%08x nr1=%08x nr2=%08x\n",

• readl(IO48_MMR_NIOS2_RESERVE0),
  

• readl(IO48_MMR_NIOS2_RESERVE1),
  

• readl(IO48_MMR_NIOS2_RESERVE2));
  

• return 0;

+}

[...]

+struct firewall_entry {

• const char *prop_name;
• const u32 cfg_addr;
• const u32 en_addr;
• const u32 en_bit;

+}; +#define FW_MPU_FPGA_ADDRESS \

• ((const struct socfpga_noc_fw_ddr_mpu_fpga2sdram *)\
• SOCFPGA_SDR_FIREWALL_MPU_FPGA_ADDRESS)

+const struct firewall_entry firewall_table[] = {

• {

This seems to be mostly copy-paste of the same stuff ... fix with macro.

• "altr,mpu0",
  

• SOCFPGA_SDR_FIREWALL_MPU_FPGA_ADDRESS +
  

• offsetof(struct socfpga_noc_fw_ddr_mpu_fpga2sdram,
  

• 	 mpuregion0addr),
  

• SOCFPGA_SDR_FIREWALL_MPU_FPGA_ADDRESS +
  

• offsetof(struct socfpga_noc_fw_ddr_mpu_fpga2sdram,
  

• 	 enable),
  

• ALT_NOC_FW_DDR_SCR_EN_MPUREG0EN_SET_MSK
  

• },
• {

• "altr,mpu1",
  

• SOCFPGA_SDR_FIREWALL_MPU_FPGA_ADDRESS +
  

• offsetof(struct socfpga_noc_fw_ddr_mpu_fpga2sdram,
  

• 	 mpuregion1addr),
  

• SOCFPGA_SDR_FIREWALL_MPU_FPGA_ADDRESS +
  

• offsetof(struct socfpga_noc_fw_ddr_mpu_fpga2sdram,
  

• 	 enable),
  

• ALT_NOC_FW_DDR_SCR_EN_MPUREG1EN_SET_MSK
  

• },

[...]