Dear dinguyen@opensource.altera.com,
In message 1427752878-18426-2-git-send-email-dinguyen@opensource.altera.com you wrote:
...
+/* Register: sdr.ctrlgrp.ctrlcfg */ +#define SDR_CTRLGRP_CTRLCFG_ADDRESS 0x5000 +/* Register: sdr.ctrlgrp.dramtiming1 */ +#define SDR_CTRLGRP_DRAMTIMING1_ADDRESS 0x5004 +/* Register: sdr.ctrlgrp.dramtiming2 */ +#define SDR_CTRLGRP_DRAMTIMING2_ADDRESS 0x5008 +/* Register: sdr.ctrlgrp.dramtiming3 */ +#define SDR_CTRLGRP_DRAMTIMING3_ADDRESS 0x500c +/* Register: sdr.ctrlgrp.dramtiming4 */ +#define SDR_CTRLGRP_DRAMTIMING4_ADDRESS 0x5010 +/* Register: sdr.ctrlgrp.lowpwrtiming */ +#define SDR_CTRLGRP_LOWPWRTIMING_ADDRESS 0x5014 +/* Register: sdr.ctrlgrp.dramodt */ +#define SDR_CTRLGRP_DRAMODT_ADDRESS 0x5018 +/* Register: sdr.ctrlgrp.dramaddrw */ +#define SDR_CTRLGRP_DRAMADDRW_ADDRESS 0x502c
...
First, this whole block of registers should probably made a C struct. Also, the comments are pretty much redundant - they do not add any new information that is not already included in the #define, so they could be omitted to make the code easier to read.
+#define SDR_CTRLGRP_PHYCTRL_ADDRESS 0x5150 +/* Register: sdr.ctrlgrp.phyctrl.phyctrl_0 */ +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_ADDRESS 0x5150 +/* Register: sdr.ctrlgrp.phyctrl.phyctrl_1 */ +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_ADDRESS 0x5154 +/* Register: sdr.ctrlgrp.phyctrl.phyctrl_2 */ +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_2_ADDRESS 0x5158 +/* Register instance: sdr::ctrlgrp::phyctrl.phyctrl_0 */ +/* Register template referenced: sdr::ctrlgrp::phyctrl::phyctrl_0 */ +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_0_OFFSET 0x150 +/* Register instance: sdr::ctrlgrp::phyctrl.phyctrl_1 */ +/* Register template referenced: sdr::ctrlgrp::phyctrl::phyctrl_1 */ +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_1_OFFSET 0x154 +/* Register instance: sdr::ctrlgrp::phyctrl.phyctrl_2 */ +/* Register template referenced: sdr::ctrlgrp::phyctrl::phyctrl_2 */ +#define SDR_CTRLGRP_PHYCTRL_PHYCTRL_2_OFFSET 0x158
Is it not redundant to define both the address and the offset? Decide for one. And again, this should probably be rather a C struct.
+/* Register template: sdr::ctrlgrp::ctrlcfg */ +#define SDR_CTRLGRP_CTRLCFG_OUTPUTREG_LSB 26 +#define SDR_CTRLGRP_CTRLCFG_OUTPUTREG_MASK 0x04000000 +#define SDR_CTRLGRP_CTRLCFG_BURSTTERMEN_LSB 25 +#define SDR_CTRLGRP_CTRLCFG_BURSTTERMEN_MASK 0x02000000 +#define SDR_CTRLGRP_CTRLCFG_BURSTINTREN_LSB 24 +#define SDR_CTRLGRP_CTRLCFG_BURSTINTREN_MASK 0x01000000 +#define SDR_CTRLGRP_CTRLCFG_NODMPINS_LSB 23 +#define SDR_CTRLGRP_CTRLCFG_NODMPINS_MASK 0x00800000 +#define SDR_CTRLGRP_CTRLCFG_DQSTRKEN_LSB 22 +#define SDR_CTRLGRP_CTRLCFG_DQSTRKEN_MASK 0x00400000 +#define SDR_CTRLGRP_CTRLCFG_STARVELIMIT_LSB 16 +#define SDR_CTRLGRP_CTRLCFG_STARVELIMIT_MASK 0x003f0000 +#define SDR_CTRLGRP_CTRLCFG_REORDEREN_LSB 15 +#define SDR_CTRLGRP_CTRLCFG_REORDEREN_MASK 0x00008000 +#define SDR_CTRLGRP_CTRLCFG_GENDBE_LSB 14 +#define SDR_CTRLGRP_CTRLCFG_GENDBE_MASK 0x00004000 +#define SDR_CTRLGRP_CTRLCFG_GENSBE_LSB 13 +#define SDR_CTRLGRP_CTRLCFG_GENSBE_MASK 0x00002000 +#define SDR_CTRLGRP_CTRLCFG_CFG_ENABLE_ECC_CODE_OVERWRITES_LSB 12 +#define SDR_CTRLGRP_CTRLCFG_CFG_ENABLE_ECC_CODE_OVERWRITES_MASK 0x00001000 +#define SDR_CTRLGRP_CTRLCFG_ECCCORREN_LSB 11 +#define SDR_CTRLGRP_CTRLCFG_ECCCORREN_MASK 0x00000800 +#define SDR_CTRLGRP_CTRLCFG_ECCEN_LSB 10 +#define SDR_CTRLGRP_CTRLCFG_ECCEN_MASK 0x00000400 +#define SDR_CTRLGRP_CTRLCFG_ADDRORDER_LSB 8 +#define SDR_CTRLGRP_CTRLCFG_ADDRORDER_MASK 0x00000300 +#define SDR_CTRLGRP_CTRLCFG_MEMBL_LSB 3 +#define SDR_CTRLGRP_CTRLCFG_MEMBL_MASK 0x000000f8 +#define SDR_CTRLGRP_CTRLCFG_MEMTYPE_LSB 0 +#define SDR_CTRLGRP_CTRLCFG_MEMTYPE_MASK 0x00000007 +/* Register template: sdr::ctrlgrp::dramtiming1 */ +#define SDR_CTRLGRP_DRAMTIMING1_TRFC_LSB 24 +#define SDR_CTRLGRP_DRAMTIMING1_TRFC_MASK 0xff000000 +#define SDR_CTRLGRP_DRAMTIMING1_TFAW_LSB 18 +#define SDR_CTRLGRP_DRAMTIMING1_TFAW_MASK 0x00fc0000 +#define SDR_CTRLGRP_DRAMTIMING1_TRRD_LSB 14 +#define SDR_CTRLGRP_DRAMTIMING1_TRRD_MASK 0x0003c000 +#define SDR_CTRLGRP_DRAMTIMING1_TCL_LSB 9 +#define SDR_CTRLGRP_DRAMTIMING1_TCL_MASK 0x00003e00 +#define SDR_CTRLGRP_DRAMTIMING1_TAL_LSB 4 +#define SDR_CTRLGRP_DRAMTIMING1_TAL_MASK 0x000001f0 +#define SDR_CTRLGRP_DRAMTIMING1_TCWL_LSB 0 +#define SDR_CTRLGRP_DRAMTIMING1_TCWL_MASK 0x0000000f +/* Register template: sdr::ctrlgrp::dramtiming2 */ +#define SDR_CTRLGRP_DRAMTIMING2_TWTR_LSB 25 +#define SDR_CTRLGRP_DRAMTIMING2_TWTR_MASK 0x1e000000 +#define SDR_CTRLGRP_DRAMTIMING2_TWR_LSB 21 +#define SDR_CTRLGRP_DRAMTIMING2_TWR_MASK 0x01e00000 +#define SDR_CTRLGRP_DRAMTIMING2_TRP_LSB 17 +#define SDR_CTRLGRP_DRAMTIMING2_TRP_MASK 0x001e0000 +#define SDR_CTRLGRP_DRAMTIMING2_TRCD_LSB 13 +#define SDR_CTRLGRP_DRAMTIMING2_TRCD_MASK 0x0001e000 +#define SDR_CTRLGRP_DRAMTIMING2_TREFI_LSB 0 +#define SDR_CTRLGRP_DRAMTIMING2_TREFI_MASK 0x00001fff +/* Register template: sdr::ctrlgrp::dramtiming3 */ +#define SDR_CTRLGRP_DRAMTIMING3_TCCD_LSB 19 +#define SDR_CTRLGRP_DRAMTIMING3_TCCD_MASK 0x00780000 +#define SDR_CTRLGRP_DRAMTIMING3_TMRD_LSB 15 +#define SDR_CTRLGRP_DRAMTIMING3_TMRD_MASK 0x00078000 +#define SDR_CTRLGRP_DRAMTIMING3_TRC_LSB 9 +#define SDR_CTRLGRP_DRAMTIMING3_TRC_MASK 0x00007e00 +#define SDR_CTRLGRP_DRAMTIMING3_TRAS_LSB 4 +#define SDR_CTRLGRP_DRAMTIMING3_TRAS_MASK 0x000001f0 +#define SDR_CTRLGRP_DRAMTIMING3_TRTP_LSB 0 +#define SDR_CTRLGRP_DRAMTIMING3_TRTP_MASK 0x0000000f +/* Register template: sdr::ctrlgrp::dramtiming4 */ +#define SDR_CTRLGRP_DRAMTIMING4_MINPWRSAVECYCLES_LSB 20 +#define SDR_CTRLGRP_DRAMTIMING4_MINPWRSAVECYCLES_MASK 0x00f00000 +#define SDR_CTRLGRP_DRAMTIMING4_PWRDOWNEXIT_LSB 10 +#define SDR_CTRLGRP_DRAMTIMING4_PWRDOWNEXIT_MASK 0x000ffc00 +#define SDR_CTRLGRP_DRAMTIMING4_SELFRFSHEXIT_LSB 0 +#define SDR_CTRLGRP_DRAMTIMING4_SELFRFSHEXIT_MASK 0x000003ff +/* Register template: sdr::ctrlgrp::lowpwrtiming */ +#define SDR_CTRLGRP_LOWPWRTIMING_CLKDISABLECYCLES_LSB 16 +#define SDR_CTRLGRP_LOWPWRTIMING_CLKDISABLECYCLES_MASK 0x000f0000 +#define SDR_CTRLGRP_LOWPWRTIMING_AUTOPDCYCLES_LSB 0 +#define SDR_CTRLGRP_LOWPWRTIMING_AUTOPDCYCLES_MASK 0x0000ffff +/* Register template: sdr::ctrlgrp::dramaddrw */ +#define SDR_CTRLGRP_DRAMADDRW_CSBITS_LSB 13 +#define SDR_CTRLGRP_DRAMADDRW_CSBITS_MASK 0x0000e000 +#define SDR_CTRLGRP_DRAMADDRW_BANKBITS_LSB 10 +#define SDR_CTRLGRP_DRAMADDRW_BANKBITS_MASK 0x00001c00 +#define SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB 5 +#define SDR_CTRLGRP_DRAMADDRW_ROWBITS_MASK 0x000003e0 +#define SDR_CTRLGRP_DRAMADDRW_COLBITS_LSB 0 +#define SDR_CTRLGRP_DRAMADDRW_COLBITS_MASK 0x0000001f
...
Are all these #defines actually used in the code? Maybe we can remove some that will never play a role here?
+#define CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_MEMTYPE (2) +#define CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_MEMBL (8) +#define CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ADDRORDER (0) +#define CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ECCEN (1) +#define CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ECCCORREN (1) +#define CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_REORDEREN (1) +#define CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_STARVELIMIT (10)
...
Please remove parens from all such simple definitions. Please fix globally.
+#define ADDRORDER2_INFO \
- "INFO: Changing address order to 2 (row, chip, bank, column)\n"
+#define ADDRORDER0_INFO \
- "INFO: Changing address order to 0 (chip, row, bank, column)\n"
Please insert the strings where they are used, this makes the code much easier to read.
+typedef struct _sdram_prot_rule {
- uint32_t rule; /* SDRAM protection rule number: 0-19 */
- uint64_t sdram_start; /* SDRAM start address */
- uint64_t sdram_end; /* SDRAM end address */
- int valid; /* Rule valid or not? 1 - valid, 0 not*/
- uint32_t security;
- uint32_t portmask;
- uint32_t result;
- uint32_t lo_prot_id;
- uint32_t hi_prot_id;
+} sdram_prot_rule, *psdram_prot_rule;
Would it make sense to move the "uint32_t rule" after the uint64_t entries to avoid a gap in the struct?
- /* Select the rule */
- regoffs = SDR_CTRLGRP_PROTRULERDWR_ADDRESS;
- writel(ruleno, (SOCFPGA_SDR_ADDRESS + regoffs));
NAK. We do not allow such syntax. Please use a C struct. Please fix globally.
+/* Function to update the field within variable */ +static unsigned sdram_write_register_field(unsigned masked_value,
- unsigned data, unsigned shift, unsigned mask)
+{
- masked_value &= ~(mask);
- masked_value |= (data << shift) & mask;
- return masked_value;
+}
Is this really needed? I guess you could use standard I/O accessors like clrsetbits-*() instead?
+#if defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_CSBITS) && \ +defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_ROWBITS) && \ +defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_BANKBITS) && \ +defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_COLBITS) && \ +defined(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_ROWBITS)
- int cs = CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_CSBITS;
- int width = 8;
- int rows = CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_ROWBITS;
- int banks = CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_BANKBITS;
- int cols = CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_COLBITS;
- unsigned long long workaround_memsize = MEMSIZE_4G;
- writel(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_ROWBITS,
&sysmgr_regs->iswgrp_handoff[4]);+#endif
- /***** CTRLCFG *****/
+#if defined(CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_MEMTYPE) || \ +defined(CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_MEMBL) || \ +defined(CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ADDRORDER) || \ +defined(CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ECCEN) || \ +defined(CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ECCCORREN) || \ +defined(CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_REORDEREN) || \ +defined(CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_STARVELIMIT) || \ +defined(CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_DQSTRKEN) || \ +defined(CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_NODMPINS)
- debug("\nConfiguring CTRLCFG\n");
- register_offset = SDR_CTRLGRP_CTRLCFG_ADDRESS;
- /* Read original register value */
- reg_value = readl(SOCFPGA_SDR_ADDRESS + register_offset);
+#ifdef CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_MEMTYPE
This is an unreadable #ifdef mess. Can we somehow clean up this code, please?
+#ifdef CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ECCEN
- reg_value = sdram_write_register_field(reg_value,
CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ECCEN,SDR_CTRLGRP_CTRLCFG_ECCEN_LSB,SDR_CTRLGRP_CTRLCFG_ECCEN_MASK);+#endif +#ifdef CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ECCCORREN
- reg_value = sdram_write_register_field(reg_value,
CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_ECCCORREN,SDR_CTRLGRP_CTRLCFG_ECCCORREN_LSB,SDR_CTRLGRP_CTRLCFG_ECCCORREN_MASK);+#endif +#ifdef CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_REORDEREN
- reg_value = sdram_write_register_field(reg_value,
CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_REORDEREN,SDR_CTRLGRP_CTRLCFG_REORDEREN_LSB,SDR_CTRLGRP_CTRLCFG_REORDEREN_MASK);+#endif +#ifdef CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_STARVELIMIT
- reg_value = sdram_write_register_field(reg_value,
CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_STARVELIMIT,SDR_CTRLGRP_CTRLCFG_STARVELIMIT_LSB,SDR_CTRLGRP_CTRLCFG_STARVELIMIT_MASK);+#endif +#ifdef CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_DQSTRKEN
- reg_value = sdram_write_register_field(reg_value,
CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_DQSTRKEN,SDR_CTRLGRP_CTRLCFG_DQSTRKEN_LSB,SDR_CTRLGRP_CTRLCFG_DQSTRKEN_MASK);+#endif +#ifdef CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_NODMPINS
- reg_value = sdram_write_register_field(reg_value,
CONFIG_HPS_SDR_CTRLCFG_CTRLCFG_NODMPINS,SDR_CTRLGRP_CTRLCFG_NODMPINS_LSB,SDR_CTRLGRP_CTRLCFG_NODMPINS_MASK);+#endif
You would get much simpler code if you did not define shift and mask, but the direct hex value instead - this just became one big |.
+/*
- space around comma is required for varargs macro to remove comma if args
- is empty
- */
+#define BFM_GBL_SET(field, value)
where do we have a varargs macro here?
+#define DYNAMIC_CALIB_STEPS (dyn_calib_steps)
The following code uses a mix of DYNAMIC_CALIB_STEPS and dyn_calib_steps. Please drop DYNAMIC_CALIB_STEPS and use dyn_calib_steps consistently.
+static int sdr_ops(u32 *base, u32 offset, u32 data, bool write) +{
Is this needed? It appears we could use standard I/O accessors instead?
+static inline void scc_mgr_set_dqs_bypass(uint32_t write_group, uint32_t bypass) +{
- /* Load the setting in the SCC manager */
- WRITE_SCC_DQS_BYPASS(write_group, bypass);
+}
+inline void scc_mgr_set_dq_out1_delay(uint32_t write_group,
uint32_t dq_in_group, uint32_t delay)+{
- /* Load the setting in the SCC manager */
- sdr_ops((u32 *)SCC_MGR_IO_OUT1_DELAY, dq_in_group << 2, delay, SDR_WRITE);
+}
+inline void scc_mgr_set_dq_out2_delay(uint32_t write_group,
uint32_t dq_in_group, uint32_t delay)+{
- /* Load the setting in the SCC manager */
- WRITE_SCC_DQ_OUT2_DELAY(dq_in_group, delay);
+}
+inline void scc_mgr_set_dq_in_delay(uint32_t write_group,
- uint32_t dq_in_group, uint32_t delay)
+{
- /* Load the setting in the SCC manager */
- sdr_ops((u32 *)SCC_MGR_IO_IN_DELAY, dq_in_group << 2, delay, SDR_WRITE);
+}
+static inline void scc_mgr_set_dq_bypass(uint32_t write_group,
uint32_t dq_in_group, uint32_t bypass)+{
- /* Load the setting in the SCC manager */
- WRITE_SCC_DQ_BYPASS(dq_in_group, bypass);
+}
+static inline void scc_mgr_set_rfifo_mode(uint32_t write_group,
uint32_t dq_in_group, uint32_t mode)+{
- /* Load the setting in the SCC manager */
- WRITE_SCC_RFIFO_MODE(dq_in_group, mode);
+}
+static inline void scc_mgr_set_hhp_extras(void) +{
- /*
* Load the fixed setting in the SCC manager* bits: 0:0 = 1'b1 - dqs bypass* bits: 1:1 = 1'b1 - dq bypass* bits: 4:2 = 3'b001 - rfifo_mode* bits: 6:5 = 2'b01 - rfifo clock_select* bits: 7:7 = 1'b0 - separate gating from ungating setting* bits: 8:8 = 1'b0 - separate OE from Output delay setting*/- uint32_t value = (0<<8) | (0<<7) | (1<<5) | (1<<2) | (1<<1) | (1<<0);
- sdr_ops((u32 *)SCC_MGR_HHP_GLOBALS, SCC_MGR_HHP_EXTRAS_OFFSET,
value, SDR_WRITE);+}
+static inline void scc_mgr_set_hhp_dqse_map(void) +{
- /* Load the fixed setting in the SCC manager */
- sdr_ops((u32 *)SCC_MGR_HHP_GLOBALS, SCC_MGR_HHP_DQSE_MAP_OFFSET, 0,
SDR_WRITE);+}
+static inline void scc_mgr_set_dqs_out1_delay(uint32_t write_group,
uint32_t delay)+{
- /* Load the setting in the SCC manager */
- sdr_ops((u32 *)SCC_MGR_IO_OUT1_DELAY, RW_MGR_MEM_DQ_PER_WRITE_DQS << 2,
delay, SDR_WRITE);+}
+static inline void scc_mgr_set_dqs_out2_delay(uint32_t write_group,
uint32_t delay)+{
- /* Load the setting in the SCC manager */
- WRITE_SCC_DQS_IO_OUT2_DELAY(delay);
+}
+static inline void scc_mgr_set_dm_out1_delay(uint32_t write_group,
uint32_t dm, uint32_t delay)+{
- /* Load the setting in the SCC manager */
- sdr_ops((u32 *)SCC_MGR_IO_OUT1_DELAY,
(RW_MGR_MEM_DQ_PER_WRITE_DQS + 1 + dm) << 2, delay, SDR_WRITE);+}
+static inline void scc_mgr_set_dm_out2_delay(uint32_t write_group, uint32_t dm,
uint32_t delay)+{
- /* Load the setting in the SCC manager */
- WRITE_SCC_DM_IO_OUT2_DELAY(dm, delay);
+}
+static inline void scc_mgr_set_dm_in_delay(uint32_t write_group,
uint32_t dm, uint32_t delay)+{
- /* Load the setting in the SCC manager */
- sdr_ops((u32 *)SCC_MGR_IO_IN_DELAY,
(RW_MGR_MEM_DQ_PER_WRITE_DQS + 1 + dm) << 2, delay, SDR_WRITE);+}
+static inline void scc_mgr_set_dm_bypass(uint32_t write_group, uint32_t dm,
uint32_t bypass)+{
- /* Load the setting in the SCC manager */
- WRITE_SCC_DM_BYPASS(dm, bypass);
+}
All these one-line functions should probably rather be inlined?
...
drivers/ddr/altera/sdram.c is 1,300+ lines; and drivers/ddr/altera/sequencer.c is nearly 4,000 lines. Have you considered splitting this off into smaller units? Eventually this might help to reduce the #ifdef mess, too ?
+#define SCC_MGR_DQS_ENA (BASE_SCC_MGR + 0x0E00) +#define SCC_MGR_DQS_IO_ENA (BASE_SCC_MGR + 0x0E04) +#define SCC_MGR_DQ_ENA (BASE_SCC_MGR + 0x0E08) +#define SCC_MGR_DM_ENA (BASE_SCC_MGR + 0x0E0C) +#define SCC_MGR_UPD (BASE_SCC_MGR + 0x0E20) +#define SCC_MGR_ACTIVE_RANK (BASE_SCC_MGR + 0x0E40)
Guess this should be a C struct?
diff --git a/drivers/ddr/altera/sequencer_auto.h b/drivers/ddr/altera/sequencer_auto.h new file mode 100644 index 0000000..de26b6b --- /dev/null +++ b/drivers/ddr/altera/sequencer_auto.h @@ -0,0 +1,216 @@ +/*
- Copyright Altera Corporation (C) 2012-2014. All rights reserved
Can we please get rid of the non-sense "All rights reserved" clause here (and in the other fiels)? Thanks!
- SPDX-License-Identifier: BSD-3-Clause
- */
+#define __RW_MGR_ac_mrs1 0x04 +#define __RW_MGR_ac_mrs3 0x06 +#define __RW_MGR_ac_write_bank_0_col_0_nodata_wl_1 0x1C +#define __RW_MGR_ac_act_1 0x11
Macro names must be all caps. Please fix globally.
Also, please be aware of the special meaning of a __ prefix for C macros. I think your usage here might be incorrect.
Best regards,
Wolfgang Denk