2017年1月9日 下午6:30于 Andre Przywara <andre.przyw...@arm.com>写道：
>
> Hi,
>
> On 05/01/17 22:55, Icenowy Zheng wrote:
> >
> > 2017年1月6日 06:37于 Maxime Ripard <maxime.rip...@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 <icen...@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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
