[U-Boot] [linux-sunxi] Re: [RFC PATCH 1/4] sunxi: add DDR2 support to H3-like DRAM controller

[Icenowy Zheng]

2017年1月9日 下午6:30于 Andre Przywara <andre.przywara at arm.com>写道：
>
> Hi, 
>
> On 05/01/17 22:55, Icenowy Zheng wrote: 
> > 
> > 2017年1月6日 06:37于 Maxime Ripard <maxime.ripard at 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 at 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/4ae474c756c3208a3bfaf36ed6f1850d46b07429 
>
> as part of that branch: 
> https://github.com/ptomsich/u-boot/commits/a64uQ7-spl-wip 

I now think it worth to do a gaint rework for H3 DRAM code...

This patch may need to be splited, and supports of A64, H5, R40 are also worthful to be part of the refactor.

>
> 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.