From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chee, Tien Fong Date: Tue, 26 Sep 2017 08:20:32 +0000 Subject: [U-Boot] [PATCH v2 12/19] arm: socfpga: Add DDR driver for Arria 10 In-Reply-To: <34f4649f-e202-3a63-4aff-e2c14d5dff82@denx.de> References: <1506328815-23733-1-git-send-email-tien.fong.chee@intel.com> <1506328815-23733-13-git-send-email-tien.fong.chee@intel.com> <34f4649f-e202-3a63-4aff-e2c14d5dff82@denx.de> Message-ID: <1506414031.27760.20.camel@intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Isn, 2017-09-25 at 11:19 +0200, Marek Vasut wrote: > On 09/25/2017 10:40 AM, tien.fong.chee at intel.com wrote: > > > > From: Tien Fong Chee > > > > Add DDR driver suppport for Arria 10. > > > > Signed-off-by: Tien Fong Chee > > --- > >  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 > > +#elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > > +#include > >  #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 > > + * Copyright (C) 2017 Intel Corporation > >   * > >   * 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. > Why regular macros is prefered? Above implementation improve readability, simplify the implementation and saving memory. > > > > + 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 > > + * > > + * SPDX-License-Identifier:    GPL-2.0 > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > Why does an SDRAM init driver need access to ns16550 UART , MMC and > NAND ? > I will remove them. > > > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +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. ... > Okay. > > > > +#define DDR_CONFIG_ELEMENTS (sizeof(ddr_config)/sizeof(u32) > > ) > So this is ARRAY_SIZE() ? > Yes. i will use 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 > Okay. > > > > +} > > + > > +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) ? > Yes, i can use mdelay. > > > > + } > > +} > > + > > +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 ? > Okay. > > > > + 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 > Okay. > > > > + } 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. > Okay. > > > > + "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 > > + }, > [...] > >