Hi,
On 05/01/17 22:55, Icenowy Zheng wrote:
2017年1月6日 06:37于 Maxime Ripard maxime.ripard@free-electrons.com写道：
On Thu, Dec 29, 2016 at 03:00:58AM +0800, Icenowy Zheng wrote:
H3-like DRAM controller needs some special code to operate a DDR2 DRAM chip. Add the logic to probe such a chip.
Out of curiosity, how did you came up with those values? It would be good if this was mentioned in the commit log.
As there's no commercial boards available now with H3 and DDR2 DRAM, the patch is developed and tested on a V3s chip, which has in-package DDR2 DRAM.
Signed-off-by: Icenowy Zheng icenowy@aosc.xyz
It would have been great if your previous patch renaming the H3 symbol was part of that serie.
arch/arm/mach-sunxi/dram_sun8i_h3.c | 114 ++++++++++++++++++++++++++++++++++-- board/sunxi/Kconfig | 11 ++++ 2 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_sun8i_h3.c index 8e2527dee1..a48320e01c 100644 --- a/arch/arm/mach-sunxi/dram_sun8i_h3.c +++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c @@ -22,6 +22,9 @@ struct dram_para { u8 bus_width; u8 dual_rank; u8 row_bits; +#ifdef CONFIG_SUNXI_H3_DRAM_DDR2
- u8 bank_bits;
+#endif };
static inline int ns_to_t(int nanoseconds) @@ -136,36 +139,77 @@ static void mctl_set_timing_params(struct dram_para *para) struct sunxi_mctl_ctl_reg * const mctl_ctl = (struct sunxi_mctl_ctl_reg *)SUNXI_DRAM_CTL0_BASE;
+#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tccd = 2; +#else
- u8 tccd = 1;
+#endif u8 tfaw = ns_to_t(50); +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 trrd = max(ns_to_t(10), 4); u8 trcd = ns_to_t(15); u8 trc = ns_to_t(53); u8 txp = max(ns_to_t(8), 3); u8 twtr = max(ns_to_t(8), 4); u8 trtp = max(ns_to_t(8), 4); +#else
- u8 trrd = max(ns_to_t(10), 2);
- u8 trcd = ns_to_t(20);
- u8 trc = ns_to_t(65);
- u8 txp = 2;
- u8 twtr = max(ns_to_t(8), 2);
- u8 trtp = max(ns_to_t(8), 2);
+#endif u8 twr = max(ns_to_t(15), 3); u8 trp = ns_to_t(15); +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tras = ns_to_t(38); +#else
- u8 tras = ns_to_t(45);
+#endif u16 trefi = ns_to_t(7800) / 32; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u16 trfc = ns_to_t(350); +#else
- u16 trfc = ns_to_t(328);
+#endif
u8 tmrw = 0; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 tmrd = 4; +#else
- u8 tmrd = 2;
+#endif u8 tmod = 12; u8 tcke = 3; u8 tcksrx = 5; u8 tcksre = 5; u8 tckesr = 4; +#ifndef CONFIG_SUNXI_H3_DRAM_DDR2 u8 trasmax = 24; +#else
- u8 trasmax = 27;
+#endif
Can't that be moved into a structure that would have different declaration, this is barely readable.
I agree to that. If I got this correctly, the existing parameters here are actually DRAM chip specific. It just happens that we use DDR3-1333 parameters for all boards so far. Most A64 boards for instance sport DDR3-1600 capable chips, also we will need adjustments for the LPDDR3 chips used on the SoPine and Pinebook. So it would be good to address this properly.
Philipp has a quite elaborate rework to fix this and ease the way to allow board specific tunings of those parameters:
https://github.com/ptomsich/u-boot/commit/4ae474c756c3208a3bfaf36ed6f1850d46...
as part of that branch: https://github.com/ptomsich/u-boot/commits/a64uQ7-spl-wip
At some point in the future I wanted to clean this up and upstream it, I am wondering if you can take a look at this now?
Cheers, Andre.