On 12 October 2018 at 01:01, Ryder Lee ryder.lee@mediatek.com wrote:
This patch adds a DDR3 driver for MT7629 SoC.
Signed-off-by: Wu Zou wu.zou@mediatek.com Signed-off-by: Ryder Lee ryder.lee@mediatek.com
drivers/ram/Makefile | 1 + drivers/ram/mediatek/Makefile | 7 + drivers/ram/mediatek/ddr3-mt7629.c | 766 +++++++++++++++++++++++++++++++++++++ 3 files changed, 774 insertions(+) create mode 100644 drivers/ram/mediatek/Makefile create mode 100644 drivers/ram/mediatek/ddr3-mt7629.c
Reviewed-by: Simon Glass sjg@chromium.org
Thoughts below.
[..]
+#define DDRPHY_PLL1 0x0000 +#define DDRPHY_PLL2 0x0004
Why not use a C struct for these registers?
[..]
writel(0x400, priv->dramc_ao + DRAMC_MRS);writel(0x1d7000, priv->dramc_ao + DRAMC_MRS);writel(0x1, priv->dramc_ao + DRAMC_SPCMD);writel(0x0, priv->dramc_ao + DRAMC_SPCMD);udelay(100);
Are these delays specified in a datasheet? Why did you chose 100? Perhaps add an enum for this value? Is there a way to check for when the hardware is ready, e.g. by reading a registers in a loop?
[..]
+static int mtk_ddr3_probe(struct udevice *dev) +{
struct mtk_ddr3_priv *priv = dev_get_priv(dev);priv->emi = dev_read_addr_index(dev, 0);if (priv->emi == FDT_ADDR_T_NONE)return -EINVAL;priv->ddrphy = dev_read_addr_index(dev, 1);if (priv->ddrphy == FDT_ADDR_T_NONE)return -EINVAL;priv->dramc_ao = dev_read_addr_index(dev, 2);if (priv->dramc_ao == FDT_ADDR_T_NONE)return -EINVAL;+#ifdef CONFIG_SPL_BUILD
int ret;ret = clk_get_by_index(dev, 0, &priv->phy);if (ret)return ret;ret = clk_get_by_index(dev, 1, &priv->phy_mux);if (ret)return ret;ret = clk_get_by_index(dev, 2, &priv->mem);if (ret)return ret;ret = clk_get_by_index(dev, 3, &priv->mem_mux);if (ret)return ret;
Do you have phandles for these clocks? I only worry that it is a bit brittle to have them numbered.
ret = mtk_ddr3_init(dev);if (ret)return ret;+#endif
return 0;+}
Regards, Simon