Re: [U-Boot] [PATCH RESEND 1/3] driver/ddr/altera: Add DDR driver for Altera's SDRAM controller
Hi!
+#ifndef _SDRAM_H_
+#define _SDRAM_H_
+
+#ifndef __ASSEMBLY__
+
+/* function declaration */
You can delete this comment.
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0_LSB 0
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0_MASK \
+0x
+/* Register template: sdr::ctrlgrp::mpthresholdrst::mpthresholdrst_1 */
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32_LSB 0
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32_MASK \
+0x
+/* Register template: sdr::ctrlgrp::mpthresholdrst::mpthresholdrst_2 */
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64_LSB 0
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64_MASK \
+0x
Can we get slightly shorter define names?
+/* Register template: sdr::ctrlgrp::remappriority */
+#define SDR_CTRLGRP_REMAPPRIORITY_PRIORITYREMAP_LSB 0
+#define SDR_CTRLGRP_REMAPPRIORITY_PRIORITYREMAP_MASK 0x00ff
+/* Register template: sdr::ctrlgrp::phyctrl::phyctrl_0 */
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_SAMPLECOUNT_19_0_LSB 12
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_SAMPLECOUNT_19_0_WIDTH 20
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_SAMPLECOUNT_19_0_SET(x) \
+ (((x) 12) 0xf000)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_ADDLATSEL_SET(x) \
+ (((x) 10) 0x0c00)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_DQSLOGICDELAYEN_SET(x) \
+ (((x) 6) 0x00c0)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_RESETDELAYEN_SET(x) \
+ (((x) 8) 0x0100)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_LPDDRDIS_SET(x) \
+ (((x) 9) 0x0200)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_DQSDELAYEN_SET(x) \
+ (((x) 4) 0x0030)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_DQDELAYEN_SET(x) \
+ (((x) 2) 0x000c)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_ACDELAYEN_SET(x) \
+ (((x) 0) 0x0003)
+/* Register template: sdr::ctrlgrp::phyctrl::phyctrl_1 */
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_LONGIDLESAMPLECOUNT_19_0_WIDTH 20
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_LONGIDLESAMPLECOUNT_19_0_SET(x) \
+ (((x) 12) 0xf000)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_SAMPLECOUNT_31_20_SET(x) \
+ (((x) 0) 0x0fff)
+/* Register template: sdr::ctrlgrp::phyctrl::phyctrl_2 */
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_2_LONGIDLESAMPLECOUNT_31_20_SET(x) \
+ (((x) 0) 0x0fff)
Too long names here, too..
--- /dev/null
+++ b/arch/arm/include/asm/arch-socfpga/sdram_config.h
@@ -0,0 +1,100 @@
+/*
+ * Copyright Altera Corporation (C) 2012-2015
+ *
+ * SPDX-License-Identifier:BSD-3-Clause
+ */
+
If this file is autogenerated, you should mention it here.
+#ifdef CONFIG_SOCFPGA_ARRIA5
+/* The if..else... is not required if generated by tools */
What does this comment say?
+#define CONFIG_HPS_SDR_CTRLCFG_MPPACING_1_THRESHOLD2_3_0 0
+#define CONFIG_HPS_SDR_CTRLCFG_MPPACING_2_THRESHOLD2_35_40x41041041
+#define CONFIG_HPS_SDR_CTRLCFG_MPPACING_3_THRESHOLD2_59_36 0x410410
+#define CONFIG_HPS_SDR_CTRLCFG_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0 \
+0x01010101
+#define CONFIG_HPS_SDR_CTRLCFG_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32 \
+0x01010101
+#define CONFIG_HPS_SDR_CTRLCFG_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64 \
+0x0101
Drop HPS and CTRLCFG from the config names... they should still be
unique and you'll not hit 80 column limits with just the name?
+#define COMPARE_FAIL_ACTION return 1;
Macros that change control flow are nasty.
+/* Below function only applicable for SPL */
Function below?
Add ifdef so that it is not available for u-boot proper?
+typedef struct _sdram_prot_rule {
+ uint64_tsdram_start;/* SDRAM start address */
+ uint64_tsdram_end; /* SDRAM end address */
+ uint32_trule; /* SDRAM protection rule number: 0-19 */
+ int valid; /* Rule valid or not? 1 - valid, 0 not*/
+
+ uint32_tsecurity;
+ uint32_tportmask;
+ uint32_tresult;
+ uint32_tlo_prot_id;
+ uint32_thi_prot_id;
+} sdram_prot_rule, *psdram_prot_rule;
Struct typedefs are nasty. Just use struct sdram_prot_rule?
+static void sdram_get_rule(psdram_prot_rule prule)
+{
+ uint32_t protruleaddr;
+ uint32_t protruleid;
+ uint32_t protruledata;
Remove protrule from local variables, as it is clear from context?
+static void sdram_set_protection_config(uint64_t sdram_start, uint64_t
sdram_end)
+{
+ sdram_prot_rule rule;
+ int rules;
+
+ /* Start with accepting all SDRAM transaction */
+ writel(0x0, sdr_ctrl-protport_default);
+
+ /* Clear all protection rules for warm boot case */
+
+ rule.sdram_start = 0;
Kill last empty line. And actually... maybe memset?
+static void set_sdr_addr_rw(void)
+{
+ int cs =
Re: [U-Boot] [PATCH RESEND 1/3] driver/ddr/altera: Add DDR driver for Altera's SDRAM controller
Hi Pavel,
On 04/17/2015 07:31 AM, Pavel Machek wrote:
Hi!
+#ifndef _SDRAM_H_
+#define _SDRAM_H_
+
+#ifndef __ASSEMBLY__
+
+/* function declaration */
You can delete this comment.
Ok...
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0_LSB 0
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0_MASK \
+0x
+/* Register template: sdr::ctrlgrp::mpthresholdrst::mpthresholdrst_1
*/
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32_LSB 0
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32_MASK \
+0x
+/* Register template: sdr::ctrlgrp::mpthresholdrst::mpthresholdrst_2
*/
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64_LSB 0
+#define \
+SDR_CTRLGRP_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64_MASK \
+0x
Can we get slightly shorter define names?
I did think about shortening these defines a bit, but came to this
reason that I should leave these alone. These defines are generated from
the tools AFAICT. I don't think any sane person would try to have
defines this long. So I still want to try to save the use case that the
driver can still be used with the autogenerated header file from the
tools in some form.
+/* Register template: sdr::ctrlgrp::remappriority
*/
+#define SDR_CTRLGRP_REMAPPRIORITY_PRIORITYREMAP_LSB 0
+#define SDR_CTRLGRP_REMAPPRIORITY_PRIORITYREMAP_MASK 0x00ff
+/* Register template: sdr::ctrlgrp::phyctrl::phyctrl_0
*/
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_SAMPLECOUNT_19_0_LSB 12
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_SAMPLECOUNT_19_0_WIDTH 20
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_SAMPLECOUNT_19_0_SET(x) \
+ (((x) 12) 0xf000)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_ADDLATSEL_SET(x) \
+ (((x) 10) 0x0c00)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_DQSLOGICDELAYEN_SET(x) \
+ (((x) 6) 0x00c0)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_RESETDELAYEN_SET(x) \
+ (((x) 8) 0x0100)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_LPDDRDIS_SET(x) \
+ (((x) 9) 0x0200)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_DQSDELAYEN_SET(x) \
+ (((x) 4) 0x0030)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_DQDELAYEN_SET(x) \
+ (((x) 2) 0x000c)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_ACDELAYEN_SET(x) \
+ (((x) 0) 0x0003)
+/* Register template: sdr::ctrlgrp::phyctrl::phyctrl_1
*/
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_LONGIDLESAMPLECOUNT_19_0_WIDTH 20
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_LONGIDLESAMPLECOUNT_19_0_SET(x) \
+ (((x) 12) 0xf000)
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_SAMPLECOUNT_31_20_SET(x) \
+ (((x) 0) 0x0fff)
+/* Register template: sdr::ctrlgrp::phyctrl::phyctrl_2
*/
+#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_2_LONGIDLESAMPLECOUNT_31_20_SET(x) \
+ (((x) 0) 0x0fff)
Too long names here, too..
--- /dev/null
+++ b/arch/arm/include/asm/arch-socfpga/sdram_config.h
@@ -0,0 +1,100 @@
+/*
+ * Copyright Altera Corporation (C) 2012-2015
+ *
+ * SPDX-License-Identifier:BSD-3-Clause
+ */
+
If this file is autogenerated, you should mention it here.
Ok...
+#ifdef CONFIG_SOCFPGA_ARRIA5
+/* The if..else... is not required if generated by tools */
What does this comment say?
I have no idea, but will clean up.
+#define CONFIG_HPS_SDR_CTRLCFG_MPPACING_1_THRESHOLD2_3_00
+#define CONFIG_HPS_SDR_CTRLCFG_MPPACING_2_THRESHOLD2_35_4 0x41041041
+#define CONFIG_HPS_SDR_CTRLCFG_MPPACING_3_THRESHOLD2_59_36 0x410410
+#define CONFIG_HPS_SDR_CTRLCFG_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0 \
+0x01010101
+#define CONFIG_HPS_SDR_CTRLCFG_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32 \
+0x01010101
+#define CONFIG_HPS_SDR_CTRLCFG_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64 \
+0x0101
Drop HPS and CTRLCFG from the config names... they should still be
unique and you'll not hit 80 column limits with just the name?
Same reasoning as I had for previous comment regarding long define names.
+#define COMPARE_FAIL_ACTION return 1;
Macros that change control flow are nasty.
Will remove...
+/* Below function only applicable for SPL */
Function below?
Will clean..
Add ifdef so that it is not available for u-boot proper?
+typedef struct _sdram_prot_rule {
+uint64_tsdram_start;/* SDRAM start address */
+uint64_tsdram_end; /* SDRAM end address */
+uint32_trule; /* SDRAM protection rule number: 0-19 */
+int valid; /* Rule valid or not? 1 - valid, 0 not*/
+
+uint32_tsecurity;
+uint32_tportmask;
+uint32_tresult;
+uint32_tlo_prot_id;
+uint32_thi_prot_id;
+} sdram_prot_rule, *psdram_prot_rule;
Struct typedefs are nasty. Just use struct sdram_prot_rule?
Yeah...will clean up...
+static void sdram_get_rule(psdram_prot_rule prule)
+{
Re: [U-Boot] [PATCH RESEND 1/3] driver/ddr/altera: Add DDR driver for Altera's SDRAM controller
Hi! +#define \ +SDR_CTRLGRP_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0_LSB 0 +#define \ +SDR_CTRLGRP_MPTHRESHOLDRST_0_THRESHOLDRSTCYCLES_31_0_MASK \ +0x +/* Register template: sdr::ctrlgrp::mpthresholdrst::mpthresholdrst_1 */ +#define \ +SDR_CTRLGRP_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32_LSB 0 +#define \ +SDR_CTRLGRP_MPTHRESHOLDRST_1_THRESHOLDRSTCYCLES_63_32_MASK \ +0x +/* Register template: sdr::ctrlgrp::mpthresholdrst::mpthresholdrst_2 */ +#define \ +SDR_CTRLGRP_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64_LSB 0 +#define \ +SDR_CTRLGRP_MPTHRESHOLDRST_2_THRESHOLDRSTCYCLES_79_64_MASK \ +0x Can we get slightly shorter define names? I did think about shortening these defines a bit, but came to this reason that I should leave these alone. These defines are generated from the tools AFAICT. I don't think any sane person would try to have defines this long. So I still want to try to save the use case that the driver can still be used with the autogenerated header file from the tools in some form. Ok. [I'd suggest placing the defines on single lines, ignoring 80 column rule, but then checkpatch would scream, so I guess it is ok as it is.] Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH RESEND 1/3] driver/ddr/altera: Add DDR driver for Altera's SDRAM controller
From: Dinh Nguyen dingu...@opensource.altera.com
This patch enables the SDRAM controller that is used on Altera's SoCFPGA
family. This patch configures the SDRAM controller based on a configuration
file that is generated from the Quartus tool, sdram_config.h.
Signed-off-by: Dinh Nguyen dingu...@opensource.altera.com
---
Makefile | 1 +
arch/arm/include/asm/arch-socfpga/sdram.h| 294
arch/arm/include/asm/arch-socfpga/sdram_config.h | 100 +++
drivers/ddr/altera/sdram.c | 827 +++
scripts/Makefile.spl | 1 +
5 files changed, 1223 insertions(+)
create mode 100644 arch/arm/include/asm/arch-socfpga/sdram.h
create mode 100644 arch/arm/include/asm/arch-socfpga/sdram_config.h
create mode 100644 drivers/ddr/altera/sdram.c
diff --git a/Makefile b/Makefile
index 0f7d583..163d530 100644
--- a/Makefile
+++ b/Makefile
@@ -649,6 +649,7 @@ libs-y += drivers/power/ \
libs-y += drivers/spi/
libs-$(CONFIG_FMAN_ENET) += drivers/net/fm/
libs-$(CONFIG_SYS_FSL_DDR) += drivers/ddr/fsl/
+libs-$(CONFIG_ALTERA_SDRAM) += drivers/ddr/altera/
libs-y += drivers/serial/
libs-y += drivers/usb/dwc3/
libs-y += drivers/usb/eth/
diff --git a/arch/arm/include/asm/arch-socfpga/sdram.h
b/arch/arm/include/asm/arch-socfpga/sdram.h
new file mode 100644
index 000..f46c5f4
--- /dev/null
+++ b/arch/arm/include/asm/arch-socfpga/sdram.h
@@ -0,0 +1,294 @@
+/*
+ * Copyright Altera Corporation (C) 2014-2015
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+#ifndef_SDRAM_H_
+#define_SDRAM_H_
+
+#ifndef __ASSEMBLY__
+
+/* function declaration */
+unsigned long sdram_calculate_size(void);
+unsigned sdram_mmr_init_full(unsigned int sdr_phy_reg);
+int sdram_calibration_full(void);
+
+extern int sdram_calibration(void);
+
+#define SDR_CTRLGRP_ADDRESS 0x5000
+
+struct socfpga_sdr_ctrl {
+ u32 ctrl_cfg;
+ u32 dram_timing1;
+ u32 dram_timing2;
+ u32 dram_timing3;
+ u32 dram_timing4; /* 0x10 */
+ u32 lowpwr_timing;
+ u32 dram_odt;
+ u32 __padding0[4];
+ u32 dram_addrw; /* 0x2c */
+ u32 dram_if_width; /* 0x30 */
+ u32 dram_dev_width;
+ u32 dram_sts;
+ u32 dram_intr;
+ u32 sbe_count; /* 0x40 */
+ u32 dbe_count;
+ u32 err_addr;
+ u32 drop_count;
+ u32 drop_addr; /* 0x50 */
+ u32 lowpwr_eq;
+ u32 lowpwr_ack;
+ u32 static_cfg;
+ u32 ctrl_width; /* 0x60 */
+ u32 cport_width;
+ u32 cport_wmap;
+ u32 cport_rmap;
+ u32 rfifo_cmap; /* 0x70 */
+ u32 wfifo_cmap;
+ u32 cport_rdwr;
+ u32 port_cfg;
+ u32 fpgaport_rst; /* 0x80 */
+ u32 __padding1;
+ u32 fifo_cfg;
+ u32 protport_default;
+ u32 prot_rule_addr; /* 0x90 */
+ u32 prot_rule_id;
+ u32 prot_rule_data;
+ u32 prot_rule_rdwr;
+ u32 __padding2[3];
+ u32 mp_priority;/* 0xac */
+ u32 mp_weight0; /* 0xb0 */
+ u32 mp_weight1;
+ u32 mp_weight2;
+ u32 mp_weight3;
+ u32 mp_pacing0; /* 0xc0 */
+ u32 mp_pacing1;
+ u32 mp_pacing2;
+ u32 mp_pacing3;
+ u32 mp_threshold0; /* 0xd0 */
+ u32 mp_threshold1;
+ u32 mp_threshold2;
+ u32 __padding3[29];
+ u32 phy_ctrl0; /* 0x150 */
+ u32 phy_ctrl1;
+ u32 phy_ctrl2;
+};
+
+#define SDR_CTRLGRP_CTRLCFG_NODMPINS_LSB 23
+#define SDR_CTRLGRP_CTRLCFG_NODMPINS_MASK 0x0080
+#define SDR_CTRLGRP_CTRLCFG_DQSTRKEN_LSB 22
+#define SDR_CTRLGRP_CTRLCFG_DQSTRKEN_MASK 0x0040
+#define SDR_CTRLGRP_CTRLCFG_STARVELIMIT_LSB 16
+#define SDR_CTRLGRP_CTRLCFG_STARVELIMIT_MASK 0x003f
+#define SDR_CTRLGRP_CTRLCFG_REORDEREN_LSB 15
+#define SDR_CTRLGRP_CTRLCFG_REORDEREN_MASK 0x8000
+#define SDR_CTRLGRP_CTRLCFG_ECCCORREN_LSB 11
+#define SDR_CTRLGRP_CTRLCFG_ECCCORREN_MASK 0x0800
+#define SDR_CTRLGRP_CTRLCFG_ECCEN_LSB 10
+#define SDR_CTRLGRP_CTRLCFG_ECCEN_MASK 0x0400
+#define SDR_CTRLGRP_CTRLCFG_ADDRORDER_LSB 8
+#define SDR_CTRLGRP_CTRLCFG_ADDRORDER_MASK 0x0300
+#define SDR_CTRLGRP_CTRLCFG_MEMBL_LSB 3
+#define SDR_CTRLGRP_CTRLCFG_MEMBL_MASK 0x00f8
+#define SDR_CTRLGRP_CTRLCFG_MEMTYPE_LSB 0
+#define SDR_CTRLGRP_CTRLCFG_MEMTYPE_MASK 0x0007
+/* Register template: sdr::ctrlgrp::dramtiming1*/
+#define SDR_CTRLGRP_DRAMTIMING1_TRFC_LSB 24
+#define SDR_CTRLGRP_DRAMTIMING1_TRFC_MASK 0xff00
+#define SDR_CTRLGRP_DRAMTIMING1_TFAW_LSB 18
+#define SDR_CTRLGRP_DRAMTIMING1_TFAW_MASK 0x00fc
+#define SDR_CTRLGRP_DRAMTIMING1_TRRD_LSB 14
+#define SDR_CTRLGRP_DRAMTIMING1_TRRD_MASK 0x0003c000
+#define SDR_CTRLGRP_DRAMTIMING1_TCL_LSB 9
+#define
