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

[Marek Vasut <marex@denx.de>]

On 09/25/2017 10:40 AM, tien.fong.chee at 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
> +	},
[...]


-- 
Best regards,
Marek Vasut