[U-Boot] [PATCH v1 14/16] ddr: altera: stratix10: Add DDR support for Stratix10 SoC
Ley Foon Tan
lftan.linux at gmail.com
Thu May 10 07:47:30 UTC 2018
On Thu, Apr 19, 2018 at 11:02 AM, Marek Vasut <marex at denx.de> wrote:
> On 04/19/2018 11:50 AM, Ley Foon Tan wrote:
>> Add DDR support for Stratix SoC
>
> OT: How very different is the DDR controller on Stratix 10 and Arria 10?
The IP blocks are different, so can't combine both of them.
>
>> Signed-off-by: Chin Liang See <chin.liang.see at intel.com>
>> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
>> ---
>> arch/arm/mach-socfpga/include/mach/sdram_s10.h | 340 ++++++++++++++++++++
>
> If this is used only be the driver, no point having it in mach/
>
>> drivers/ddr/altera/Makefile | 1 +
>> drivers/ddr/altera/sdram_s10.c | 392 ++++++++++++++++++++++++
>> 3 files changed, 733 insertions(+), 0 deletions(-)
>> create mode 100644 arch/arm/mach-socfpga/include/mach/sdram_s10.h
>> create mode 100644 drivers/ddr/altera/sdram_s10.c
> [...]
>
>> +
>> +union caltiming9_reg {
>> + struct {
>> + u32 cfg_4_act_to_act:8;
>> + u32 reserved:24;
>> + };
>> + u32 word;
>> +};
>
> I wonder if this struct stuff is really a good idea.
This easier for driver to access each field in register.
>
>> +#define DDR_SCHED_DDRTIMING_ACTTOACT_OFFSET 0
>> +#define DDR_SCHED_DDRTIMING_RDTOMISS_OFFSET 6
>> +#define DDR_SCHED_DDRTIMING_WRTOMISS_OFFSET 12
>> +#define DDR_SCHED_DDRTIMING_BURSTLEN_OFFSET 18
>> +#define DDR_SCHED_DDRTIMING_RDTOWR_OFFSET 21
>> +#define DDR_SCHED_DDRTIMING_WRTORD_OFFSET 26
>> +#define DDR_SCHED_DDRTIMING_BWRATIO_OFFSET 31
>> +#define DDR_SCHED_DDRMOD_BWRATIOEXTENDED_OFFSET 1
>> +#define DDR_SCHED_ACTIVATE_RRD_OFFSET 0
>> +#define DDR_SCHED_ACTIVATE_FAW_OFFSET 4
>> +#define DDR_SCHED_ACTIVATE_FAWBANK_OFFSET 10
>> +#define DDR_SCHED_DEVTODEV_BUSRDTORD_OFFSET 0
>> +#define DDR_SCHED_DEVTODEV_BUSRDTOWR_OFFSET 2
>> +#define DDR_SCHED_DEVTODEV_BUSWRTORD_OFFSET 4
>> +#define DDR_HMC_DDRIOCTRL_IOSIZE_MSK 0x00000003
>> +#define DDR_HMC_DDRCALSTAT_CAL_MSK 0x00000001
>> +#define DDR_HMC_ECCCTL_AWB_CNT_RST_SET_MSK 0x00010000
>> +#define DDR_HMC_ECCCTL_CNT_RST_SET_MSK 0x00000100
>> +#define DDR_HMC_ECCCTL_ECC_EN_SET_MSK 0x00000001
>> +#define DDR_HMC_ECCCTL2_RMW_EN_SET_MSK 0x00000100
>> +#define DDR_HMC_ECCCTL2_AWB_EN_SET_MSK 0x00000001
>> +#define DDR_HMC_ECC_DIAGON_ECCDIAGON_EN_SET_MSK 0x00010000
>> +#define DDR_HMC_ECC_DIAGON_WRDIAGON_EN_SET_MSK 0x00000001
>> +#define DDR_HMC_ERRINTEN_SERRINTEN_EN_SET_MSK 0x00000001
>> +#define DDR_HMC_ERRINTEN_DERRINTEN_EN_SET_MSK 0x00000002
>> +#define DDR_HMC_INTSTAT_SERRPENA_SET_MSK 0x00000001
>> +#define DDR_HMC_INTSTAT_DERRPENA_SET_MSK 0x00000002
>> +#define DDR_HMC_INTSTAT_ADDRMTCFLG_SET_MSK 0x00010000
>> +#define DDR_HMC_INTMODE_INTMODE_SET_MSK 0x00000001
>> +#define DDR_HMC_RSTHANDSHAKE_MASK 0x000000ff
>> +#define DDR_HMC_CORE2SEQ_INT_REQ 0xF
>> +#define DDR_HMC_SEQ2CORE_INT_RESP_MASK 0x8
>> +#define DDR_HMC_HPSINTFCSEL_ENABLE_MASK 0x001f1f1f
>> +
>> +#define CCU_CPU0_MPRT_ADBASE_DDRREG_ADDR 0xf7004400
>> +#define CCU_CPU0_MPRT_ADBASE_MEMSPACE0_ADDR 0xf70045c0
>> +#define CCU_CPU0_MPRT_ADBASE_MEMSPACE1A_ADDR 0xf70045e0
>> +#define CCU_CPU0_MPRT_ADBASE_MEMSPACE1B_ADDR 0xf7004600
>> +#define CCU_CPU0_MPRT_ADBASE_MEMSPACE1C_ADDR 0xf7004620
>> +#define CCU_CPU0_MPRT_ADBASE_MEMSPACE1D_ADDR 0xf7004640
>> +#define CCU_CPU0_MPRT_ADBASE_MEMSPACE1E_ADDR 0xf7004660
>
> Can all this come from DT, maybe except for the offsets ?
Need to add a new node in DT if want get from DT. Or another way is
add CCU base address to base_addr_s10.h and remove all these defines
and use offset instead. Is it okay?
>
>> +#define CCU_IOM_MPRT_ADBASE_MEMSPACE0_ADDR 0xf7018560
>> +#define CCU_IOM_MPRT_ADBASE_MEMSPACE1A_ADDR 0xf7018580
>> +#define CCU_IOM_MPRT_ADBASE_MEMSPACE1B_ADDR 0xf70185a0
>> +#define CCU_IOM_MPRT_ADBASE_MEMSPACE1C_ADDR 0xf70185c0
>> +#define CCU_IOM_MPRT_ADBASE_MEMSPACE1D_ADDR 0xf70185e0
>> +#define CCU_IOM_MPRT_ADBASE_MEMSPACE1E_ADDR 0xf7018600
>> +
>> +#define CCU_ADBASE_DI_MASK 0x00000010
>
> [...]
>
>> +#define DDR_CONFIG(A, B, C, R) ((A << 24) | (B << 16) | (C << 8) | R)
>
> You need parenthesis around A, B, C, R in the macro.
Okay.
>
>> +/* The followring are the supported configurations */
>> +u32 ddr_config[] = {
>> + /* DDR_CONFIG(Address order,Bank,Column,Row) */
>> + /* List for DDR3 or LPDDR3 (pinout order > chip, row, bank, column) */
>> + DDR_CONFIG(0, 3, 10, 12),
>> + DDR_CONFIG(0, 3, 9, 13),
>> + DDR_CONFIG(0, 3, 10, 13),
>> + DDR_CONFIG(0, 3, 9, 14),
>> + DDR_CONFIG(0, 3, 10, 14),
>> + DDR_CONFIG(0, 3, 10, 15),
>> + DDR_CONFIG(0, 3, 11, 14),
>> + DDR_CONFIG(0, 3, 11, 15),
>> + DDR_CONFIG(0, 3, 10, 16),
>> + DDR_CONFIG(0, 3, 11, 16),
>> + DDR_CONFIG(0, 3, 12, 15), /* 0xa */
>> + /* List for DDR4 only (pinout order > chip, bank, row, column) */
>> + DDR_CONFIG(1, 3, 10, 14),
>> + DDR_CONFIG(1, 4, 10, 14),
>> + DDR_CONFIG(1, 3, 10, 15),
>> + DDR_CONFIG(1, 4, 10, 15),
>> + DDR_CONFIG(1, 3, 10, 16),
>> + DDR_CONFIG(1, 4, 10, 16),
>> + DDR_CONFIG(1, 3, 10, 17),
>> + DDR_CONFIG(1, 4, 10, 17),
>> +};
>> +
>> +#define DDR_CONFIG_ELEMENTS (sizeof(ddr_config) / sizeof(u32))
>
> Is that ARRAY_SIZE() ?
Yes, will change this.
>
>> +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;
>> +}
>> +
>> +static int emif_clear(void)
>> +{
>> + u32 s2c, i;
>> +
>> + writel(0, &socfpga_ecc_hmc_base->rsthandshakectrl);
>> + s2c = readl(&socfpga_ecc_hmc_base->rsthandshakestat) &
>> + DDR_HMC_RSTHANDSHAKE_MASK;
>> +
>> + for (i = 1000; (i > 0) && s2c; i--) {
>> + WATCHDOG_RESET();
>> + mdelay(1);
>> + s2c = readl(&socfpga_ecc_hmc_base->rsthandshakestat) &
>> + DDR_HMC_RSTHANDSHAKE_MASK;
>
> wait_for_bit() ?
Okay.
>
>> + }
>> + return !s2c;
>> +}
>> +
>> +static int emif_reset(void)
>> +{
>> + u32 c2s, s2c, i;
>> +
>> + c2s = readl(&socfpga_ecc_hmc_base->rsthandshakectrl) &
>> + DDR_HMC_RSTHANDSHAKE_MASK;
>> + s2c = readl(&socfpga_ecc_hmc_base->rsthandshakestat) &
>> + DDR_HMC_RSTHANDSHAKE_MASK;
>> +
>> + debug("DDR: c2s=%08x s2c=%08x nr0=%08x nr1=%08x nr2=%08x dst=%08x\n",
>> + c2s, s2c, readl(&socfpga_io48_mmr_base->niosreserve0),
>> + readl(&socfpga_io48_mmr_base->niosreserve1),
>> + readl(&socfpga_io48_mmr_base->niosreserve2),
>> + readl(&socfpga_io48_mmr_base->dramsts));
>> +
>> + if (s2c && emif_clear()) {
>> + printf("DDR: emif_clear() failed\n");
>> + return -1;
>> + }
>> +
>> + puts("DDR: Triggerring emif reset\n");
>> + writel(DDR_HMC_CORE2SEQ_INT_REQ,
>> + &socfpga_ecc_hmc_base->rsthandshakectrl);
>> +
>> + for (i = 1000; i > 0; i--) {
>> + /* if seq2core[3] = 0, we are good */
>> + if (!(readl(&socfpga_ecc_hmc_base->rsthandshakestat) &
>> + DDR_HMC_SEQ2CORE_INT_RESP_MASK))
>> + break;
>> + WATCHDOG_RESET();
>> + mdelay(1);
>
> Another wait_for_bit() ?
Okay.
>
>> + }
>> +
>> + if (!i) {
>> + printf("DDR: failed to get ack from EMIF\n");
>> + return -2;
>> + }
>> +
>> + if (emif_clear()) {
>> + printf("DDR: emif_clear() failed\n");
>> + return -3;
>> + }
>> +
>> + printf("DDR: %s triggered successly\n", __func__);
>
> debug() ?
Okay.
>
>> + return 0;
>> +}
> Comb through this and fix the things mentioned above globally.
>
> --
> Best regards,
> Marek Vasut
Regards
Ley Foon
More information about the U-Boot
mailing list