[U-Boot] [PATCH V7 1/3] MX5: clock: Add clock config interface

[U-Boot] [PATCH V7 1/3] MX5: clock: Add clock config interface

[Jason Liu]

Add clock config interface support, so that we
can configure CPU or DDR clock in the later init

Signed-off-by: Jason Liu <jason.hui@linaro.org>
---
 arch/arm/cpu/armv7/mx5/clock.c           |  551 +++++++++++++++++++++++++++++-
 arch/arm/include/asm/arch-mx5/clock.h    |    4 +
 arch/arm/include/asm/arch-mx5/crm_regs.h |    6 +
 arch/arm/include/asm/arch-mx5/imx-regs.h |    1 +
 4 files changed, 559 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx5/clock.c b/arch/arm/cpu/armv7/mx5/clock.c
index 0b04a88..04d9f71 100644
--- a/arch/arm/cpu/armv7/mx5/clock.c
+++ b/arch/arm/cpu/armv7/mx5/clock.c
@@ -24,6 +24,7 @@
  */
 
 #include <common.h>
+#include <div64.h>
 #include <asm/io.h>
 #include <asm/errno.h>
 #include <asm/arch/imx-regs.h>
@@ -34,6 +35,7 @@ enum pll_clocks {
 	PLL1_CLOCK = 0,
 	PLL2_CLOCK,
 	PLL3_CLOCK,
+	PLL4_CLOCK,
 	PLL_CLOCKS,
 };
 
@@ -41,8 +43,42 @@ struct mxc_pll_reg *mxc_plls[PLL_CLOCKS] = {
 	[PLL1_CLOCK] = (struct mxc_pll_reg *)PLL1_BASE_ADDR,
 	[PLL2_CLOCK] = (struct mxc_pll_reg *)PLL2_BASE_ADDR,
 	[PLL3_CLOCK] = (struct mxc_pll_reg *)PLL3_BASE_ADDR,
+	[PLL4_CLOCK] = (struct mxc_pll_reg *)PLL4_BASE_ADDR,
 };
 
+#define AHB_CLK_ROOT    133333333
+#define SZ_DEC_1M       1000000
+#define PLL_PD_MAX      16      /* Actual pd+1 */
+#define PLL_MFI_MAX     15
+#define PLL_MFI_MIN     5
+#define ARM_DIV_MAX     8
+#define IPG_DIV_MAX     4
+#define AHB_DIV_MAX     8
+#define EMI_DIV_MAX     8
+#define NFC_DIV_MAX     8
+
+struct fixed_pll_mfd {
+	u32 ref_clk_hz;
+	u32 mfd;
+};
+
+const struct fixed_pll_mfd fixed_mfd[] = {
+	{CONFIG_SYS_MX5_HCLK, 24 * 16},
+};
+
+struct pll_param {
+	u32 pd;
+	u32 mfi;
+	u32 mfn;
+	u32 mfd;
+};
+
+#define PLL_FREQ_MAX(ref_clk)  (4 * (ref_clk) * PLL_MFI_MAX)
+#define PLL_FREQ_MIN(ref_clk) \
+		((2 * (ref_clk) * (PLL_MFI_MIN - 1)) / PLL_PD_MAX)
+#define MAX_DDR_CLK     420000000
+#define NFC_CLK_MAX     34000000
+
 struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)MXC_CCM_BASE;
 
 /*
@@ -175,7 +211,7 @@ static u32 get_uart_clk(void)
 /*
  * This function returns the low power audio clock.
  */
-u32 get_lp_apm(void)
+static u32 get_lp_apm(void)
 {
 	u32 ret_val = 0;
 	u32 ccsr = __raw_readl(&mxc_ccm->ccsr);
@@ -191,7 +227,7 @@ u32 get_lp_apm(void)
 /*
  * get cspi clock rate.
  */
-u32 imx_get_cspiclk(void)
+static u32 get_cspi_clk(void)
 {
 	u32 ret_val = 0, pdf, pre_pdf, clk_sel;
 	u32 cscmr1 = __raw_readl(&mxc_ccm->cscmr1);
@@ -228,6 +264,94 @@ u32 imx_get_cspiclk(void)
 	return ret_val;
 }
 
+static u32 get_axi_a_clk(void)
+{
+	u32 cbcdr =  __raw_readl(&mxc_ccm->cbcdr);
+	u32 pdf = (cbcdr & MXC_CCM_CBCDR_AXI_A_PODF_MASK) \
+			>> MXC_CCM_CBCDR_AXI_A_PODF_OFFSET;
+
+	return  get_periph_clk() / (pdf + 1);
+}
+
+static u32 get_axi_b_clk(void)
+{
+	u32 cbcdr =  __raw_readl(&mxc_ccm->cbcdr);
+	u32 pdf = (cbcdr & MXC_CCM_CBCDR_AXI_B_PODF_MASK) \
+			>> MXC_CCM_CBCDR_AXI_B_PODF_OFFSET;
+
+	return  get_periph_clk() / (pdf + 1);
+}
+
+static u32 get_ahb_clk(void)
+{
+	u32 cbcdr = __raw_readl(&mxc_ccm->cbcdr);
+	u32 pdf = (cbcdr & MXC_CCM_CBCDR_AHB_PODF_MASK) \
+			>> MXC_CCM_CBCDR_AHB_PODF_OFFSET;
+
+	return  get_periph_clk() / (pdf + 1);
+}
+
+static u32 get_emi_slow_clk(void)
+{
+	u32 cbcdr = __raw_readl(&mxc_ccm->cbcdr);
+	u32 emi_clk_sel = cbcdr & MXC_CCM_CBCDR_EMI_CLK_SEL;
+	u32 pdf = (cbcdr & MXC_CCM_CBCDR_EMI_PODF_MASK) \
+			>> MXC_CCM_CBCDR_EMI_PODF_OFFSET;
+
+	if (emi_clk_sel)
+		return  get_ahb_clk() / (pdf + 1);
+
+	return  get_periph_clk() / (pdf + 1);
+}
+
+static u32 get_nfc_clk(void)
+{
+	u32 cbcdr = __raw_readl(&mxc_ccm->cbcdr);
+	u32 pdf = (cbcdr & MXC_CCM_CBCDR_NFC_PODF_MASK) \
+			>> MXC_CCM_CBCDR_NFC_PODF_OFFSET;
+
+	return  get_emi_slow_clk() / (pdf + 1);
+}
+
+static u32 get_ddr_clk(void)
+{
+	u32 ret_val = 0;
+	u32 cbcmr = __raw_readl(&mxc_ccm->cbcmr);
+	u32 ddr_clk_sel = (cbcmr & MXC_CCM_CBCMR_DDR_CLK_SEL_MASK) \
+				>> MXC_CCM_CBCMR_DDR_CLK_SEL_OFFSET;
+#ifdef CONFIG_MX51
+	u32 cbcdr = __raw_readl(&mxc_ccm->cbcdr);
+	if (cbcdr & MXC_CCM_CBCDR_DDR_HIFREQ_SEL) {
+		u32 ddr_clk_podf = (cbcdr & MXC_CCM_CBCDR_DDR_PODF_MASK) >> \
+					MXC_CCM_CBCDR_DDR_PODF_OFFSET;
+
+		ret_val = decode_pll(mxc_plls[PLL1_CLOCK], CONFIG_SYS_MX5_HCLK);
+		ret_val /= ddr_clk_podf + 1;
+
+		return ret_val;
+	}
+#endif
+	switch (ddr_clk_sel) {
+	case 0:
+		ret_val = get_axi_a_clk();
+		break;
+	case 1:
+		ret_val = get_axi_b_clk();
+		break;
+	case 2:
+		ret_val = get_emi_slow_clk();
+		break;
+	case 3:
+		ret_val = get_ahb_clk();
+		break;
+	default:
+		break;
+	}
+
+	return ret_val;
+}
+
+
 /*
  * The API of get mxc clockes.
  */
@@ -245,10 +369,12 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
 	case MXC_UART_CLK:
 		return get_uart_clk();
 	case MXC_CSPI_CLK:
-		return imx_get_cspiclk();
+		return get_cspi_clk();
 	case MXC_FEC_CLK:
 		return decode_pll(mxc_plls[PLL1_CLOCK],
 				    CONFIG_SYS_MX5_HCLK);
+	case MXC_DDR_CLK:
+		return get_ddr_clk();
 	default:
 		break;
 	}
@@ -267,6 +393,424 @@ u32 imx_get_fecclk(void)
 }
 
 /*
+ * Clock config code start here
+ */
+
+/* precondition: m>0 and n>0.  Let g=gcd(m,n). */
+static int gcd(int m, int n)
+{
+	int t;
+	while (m > 0) {
+		if (n > m) {
+			t = m;
+			m = n;
+			n = t;
+		} /* swap */
+		m -= n;
+	}
+	return n;
+}
+
+/*
+ * This is to calculate various parameters based on reference clock and
+ * targeted clock based on the equation:
+ *      t_clk = 2*ref_freq*(mfi + mfn/(mfd+1))/(pd+1)
+ * This calculation is based on a fixed MFD value for simplicity.
+ *
+ * @param ref       reference clock freq in Hz
+ * @param target    targeted clock in Hz
+ * @param pll       pll_param structure.
+ *
+ * @return          0 if successful; non-zero otherwise.
+ */
+static int calc_pll_params(u32 ref, u32 target, struct pll_param *pll)
+{
+	u64 pd, mfi = 1, mfn, mfd, t1;
+	u32 n_target = target;
+	u32 n_ref = ref, i;
+
+	/*
+	 * Make sure targeted freq is in the valid range.
+	 * Otherwise the following calculation might be wrong!!!
+	 */
+	if (n_target < PLL_FREQ_MIN(ref) ||
+		n_target > PLL_FREQ_MAX(ref)) {
+		printf("Targeted peripheral clock should be"
+			"within [%d - %d]\n",
+			PLL_FREQ_MIN(ref) / SZ_DEC_1M,
+			PLL_FREQ_MAX(ref) / SZ_DEC_1M);
+		return -1;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(fixed_mfd); i++) {
+		if (fixed_mfd[i].ref_clk_hz == ref) {
+			mfd = fixed_mfd[i].mfd;
+			break;
+		}
+	}
+
+	if (i == ARRAY_SIZE(fixed_mfd))
+		return -1;
+
+	/* Use n_target and n_ref to avoid overflow */
+	for (pd = 1; pd <= PLL_PD_MAX; pd++) {
+		t1 = n_target * pd;
+		do_div(t1, (4 * n_ref));
+		mfi = t1;
+		if (mfi > PLL_MFI_MAX)
+			return -1;
+		else if (mfi < 5)
+			continue;
+		break;
+	}
+	/* Now got pd and mfi already */
+	/*
+	mfn = (((n_target * pd) / 4 - n_ref * mfi) * mfd) / n_ref;
+	*/
+	t1 = n_target * pd;
+	do_div(t1, 4);
+	t1 -= n_ref * mfi;
+	t1 *= mfd;
+	do_div(t1, n_ref);
+	mfn = t1;
+#ifdef CMD_CLOCK_DEBUG
+	printf("ref=%d, target=%d, pd=%d," "mfi=%d,mfn=%d, mfd=%d\n",
+		ref, n_target, (u32)pd, (u32)mfi, (u32)mfn, (u32)mfd);
+#endif
+	i = 1;
+	if (mfn != 0)
+		i = gcd(mfd, mfn);
+	pll->pd = (u32)pd;
+	pll->mfi = (u32)mfi;
+	do_div(mfn, i);
+	pll->mfn = (u32)mfn;
+	do_div(mfd, i);
+	pll->mfd = (u32)mfd;
+
+	return 0;
+}
+
+#define calc_div(tgt_clk, src_clk, limit) ({		\
+		u32 v = 0;				\
+		if (((src_clk) % (tgt_clk)) <= 100)	\
+			v = (src_clk) / (tgt_clk);	\
+		else					\
+			v = ((src_clk) / (tgt_clk)) + 1;\
+		if (v > limit)				\
+			v = limit;			\
+		(v - 1);				\
+	})
+
+static u32 calc_per_cbcdr_val(u32 per_clk)
+{
+	u32 cbcdr = __raw_readl(&mxc_ccm->cbcdr);
+	u32 tmp_clk = 0, div = 0, clk_sel = 0;
+
+	cbcdr &= ~MXC_CCM_CBCDR_PERIPH_CLK_SEL;
+
+	/* emi_slow_podf divider */
+	tmp_clk = get_emi_slow_clk();
+	clk_sel = cbcdr & MXC_CCM_CBCDR_EMI_CLK_SEL;
+	if (clk_sel) {
+		div = calc_div(tmp_clk, per_clk, 8);
+		cbcdr &= ~MXC_CCM_CBCDR_EMI_PODF_MASK;
+		cbcdr |= (div << MXC_CCM_CBCDR_EMI_PODF_OFFSET);
+	}
+
+	/* axi_b_podf divider */
+	tmp_clk = get_axi_b_clk();
+	div = calc_div(tmp_clk, per_clk, 8);
+	cbcdr &= ~MXC_CCM_CBCDR_AXI_B_PODF_MASK;
+	cbcdr |= (div << MXC_CCM_CBCDR_AXI_B_PODF_OFFSET);
+
+	/* axi_b_podf divider */
+	tmp_clk = get_axi_a_clk();
+	div = calc_div(tmp_clk, per_clk, 8);
+	cbcdr &= ~MXC_CCM_CBCDR_AXI_A_PODF_MASK;
+	cbcdr |= (div << MXC_CCM_CBCDR_AXI_A_PODF_OFFSET);
+
+	/* ahb podf divider */
+	tmp_clk = AHB_CLK_ROOT;
+	div = calc_div(tmp_clk, per_clk, 8);
+	cbcdr &= ~MXC_CCM_CBCDR_AHB_PODF_MASK;
+	cbcdr |= (div << MXC_CCM_CBCDR_AHB_PODF_OFFSET);
+
+	return cbcdr;
+}
+
+#define CHANGE_PLL_SETTINGS(pll, pd, fi, fn, fd) \
+	{	\
+		__raw_writel(0x1232, &pll->ctrl);		\
+		__raw_writel(0x2, &pll->config);		\
+		__raw_writel((((pd) - 1) << 0) | ((fi) << 4),	\
+			&pll->op);				\
+		__raw_writel(fn, &(pll->mfn));			\
+		__raw_writel((fd) - 1, &pll->mfd);		\
+		__raw_writel((((pd) - 1) << 0) | ((fi) << 4),	\
+			&pll->hfs_op);				\
+		__raw_writel(fn, &pll->hfs_mfn);		\
+		__raw_writel((fd) - 1, &pll->hfs_mfd);		\
+		__raw_writel(0x1232, &pll->ctrl);		\
+		while (!__raw_readl(&pll->ctrl) & 0x1)		\
+			;\
+	}
+
+static int config_pll_clk(enum pll_clocks index, struct pll_param *pll_param)
+{
+	u32 ccsr = __raw_readl(&mxc_ccm->ccsr);
+	struct mxc_pll_reg *pll = mxc_plls[index];
+
+	switch (index) {
+	case PLL1_CLOCK:
+		/* Switch ARM to PLL2 clock */
+		__raw_writel(ccsr | 0x4, &mxc_ccm->ccsr);
+		CHANGE_PLL_SETTINGS(pll, pll_param->pd,
+					pll_param->mfi, pll_param->mfn,
+					pll_param->mfd);
+		/* Switch back */
+		__raw_writel(ccsr & ~0x4, &mxc_ccm->ccsr);
+		break;
+	case PLL2_CLOCK:
+		/* Switch to pll2 bypass clock */
+		__raw_writel(ccsr | 0x2, &mxc_ccm->ccsr);
+		CHANGE_PLL_SETTINGS(pll, pll_param->pd,
+					pll_param->mfi, pll_param->mfn,
+					pll_param->mfd);
+		/* Switch back */
+		__raw_writel(ccsr & ~0x2, &mxc_ccm->ccsr);
+		break;
+	case PLL3_CLOCK:
+		/* Switch to pll3 bypass clock */
+		__raw_writel(ccsr | 0x1, &mxc_ccm->ccsr);
+		CHANGE_PLL_SETTINGS(pll, pll_param->pd,
+					pll_param->mfi, pll_param->mfn,
+					pll_param->mfd);
+		/* Switch back */
+		__raw_writel(ccsr & ~0x1, &mxc_ccm->ccsr);
+		break;
+	case PLL4_CLOCK:
+		/* Switch to pll4 bypass clock */
+		__raw_writel(ccsr | 0x20, &mxc_ccm->ccsr);
+		CHANGE_PLL_SETTINGS(pll, pll_param->pd,
+					pll_param->mfi, pll_param->mfn,
+					pll_param->mfd);
+		/* Switch back */
+		__raw_writel(ccsr & ~0x20, &mxc_ccm->ccsr);
+		break;
+	default:
+		return -1;
+	}
+
+	return 0;
+}
+
+/* Config CPU clock */
+static int config_core_clk(u32 ref, u32 freq)
+{
+	int ret = 0;
+	struct pll_param pll_param;
+
+	memset(&pll_param, 0, sizeof(struct pll_param));
+
+	/* The case that periph uses PLL1 is not considered here */
+	ret = calc_pll_params(ref, freq, &pll_param);
+	if (ret != 0) {
+		printf("Error:Can't find pll parameters: %d\n", ret);
+		return ret;
+	}
+
+	return config_pll_clk(PLL1_CLOCK, &pll_param);
+}
+
+static int config_nfc_clk(u32 nfc_clk)
+{
+	u32 reg;
+	u32 parent_rate = get_emi_slow_clk();
+	u32 div = parent_rate / nfc_clk;
+
+	if (nfc_clk <= 0)
+		return -1;
+	if (div == 0)
+		div++;
+	if (parent_rate / div > NFC_CLK_MAX)
+		div++;
+	reg = __raw_readl(&mxc_ccm->cbcdr);
+	reg &= ~MXC_CCM_CBCDR_NFC_PODF_MASK;
+	reg |= (div - 1) << MXC_CCM_CBCDR_NFC_PODF_OFFSET;
+	__raw_writel(reg, &mxc_ccm->cbcdr);
+	while (__raw_readl(&mxc_ccm->cdhipr) != 0)
+		;
+	return 0;
+}
+
+/* Config main_bus_clock for periphs */
+static int config_periph_clk(u32 ref, u32 freq)
+{
+	int ret = 0;
+	struct pll_param pll_param;
+
+	memset(&pll_param, 0, sizeof(struct pll_param));
+
+	if (__raw_readl(&mxc_ccm->cbcdr) & MXC_CCM_CBCDR_PERIPH_CLK_SEL) {
+		ret = calc_pll_params(ref, freq, &pll_param);
+		if (ret != 0) {
+			printf("Error:Can't find pll parameters: %d\n",
+				ret);
+			return ret;
+		}
+		switch ((__raw_readl(&mxc_ccm->cbcmr) & \
+			MXC_CCM_CBCMR_PERIPH_CLK_SEL_MASK) >> \
+			MXC_CCM_CBCMR_PERIPH_CLK_SEL_OFFSET) {
+		case 0:
+			return config_pll_clk(PLL1_CLOCK, &pll_param);
+			break;
+		case 1:
+			return config_pll_clk(PLL3_CLOCK, &pll_param);
+			break;
+		default:
+			return -1;
+		}
+	} else {
+		u32 old_cbcmr = __raw_readl(&mxc_ccm->cbcmr);
+		u32 new_cbcdr = calc_per_cbcdr_val(freq);
+		u32 old_nfc = get_nfc_clk();
+
+		/* Switch peripheral to PLL3 */
+		__raw_writel(0x00015154, &mxc_ccm->cbcmr);
+		__raw_writel(0x02888945, &mxc_ccm->cbcdr);
+
+		/* Make sure change is effective */
+		while (__raw_readl(&mxc_ccm->cdhipr) != 0)
+			;
+
+		/* Setup PLL2 */
+		ret = calc_pll_params(ref, freq, &pll_param);
+		if (ret != 0) {
+			printf("Error:Can't find pll parameters: %d\n",
+				ret);
+			return ret;
+		}
+		config_pll_clk(PLL2_CLOCK, &pll_param);
+
+		/* Switch peripheral back */
+		__raw_writel(new_cbcdr, &mxc_ccm->cbcdr);
+		__raw_writel(old_cbcmr, &mxc_ccm->cbcmr);
+
+		/* Make sure change is effective */
+		while (__raw_readl(&mxc_ccm->cdhipr) != 0)
+			;
+		/* restore to old NFC clock */
+		config_nfc_clk(old_nfc);
+	}
+
+	return 0;
+}
+
+static int config_ddr_clk(u32 emi_clk)
+{
+	u32 clk_src;
+	s32 shift = 0, clk_sel, div = 1;
+	u32 cbcmr = __raw_readl(&mxc_ccm->cbcmr);
+	u32 cbcdr = __raw_readl(&mxc_ccm->cbcdr);
+
+	if (emi_clk > MAX_DDR_CLK) {
+		printf("Warning:DDR clock should not exceed %d MHz\n",
+			MAX_DDR_CLK / SZ_DEC_1M);
+		emi_clk = MAX_DDR_CLK;
+	}
+
+	clk_src = get_periph_clk();
+	/* Find DDR clock input */
+	clk_sel = (cbcmr >> 10) & 0x3;
+	switch (clk_sel) {
+	case 0:
+		shift = 16;
+		break;
+	case 1:
+		shift = 19;
+		break;
+	case 2:
+		shift = 22;
+		break;
+	case 3:
+		shift = 10;
+		break;
+	default:
+		return -1;
+	}
+
+	if ((clk_src % emi_clk) < 10000000)
+		div = clk_src / emi_clk;
+	else
+		div = (clk_src / emi_clk) + 1;
+	if (div > 8)
+		div = 8;
+
+	cbcdr = cbcdr & ~(0x7 << shift);
+	cbcdr |= ((div - 1) << shift);
+	__raw_writel(cbcdr, &mxc_ccm->cbcdr);
+	while (__raw_readl(&mxc_ccm->cdhipr) != 0)
+		;
+	__raw_writel(0x0, &mxc_ccm->ccdr);
+
+	return 0;
+}
+
+/*!
+ * This function assumes the expected core clock has to be changed by
+ * modifying the PLL. This is NOT true always but for most of the times,
+ * it is. So it assumes the PLL output freq is the same as the expected
+ * core clock (presc=1) unless the core clock is less than PLL_FREQ_MIN.
+ * In the latter case, it will try to increase the presc value until
+ * (presc*core_clk) is greater than PLL_FREQ_MIN. It then makes call to
+ * calc_pll_params() and obtains the values of PD, MFI,MFN, MFD based
+ * on the targeted PLL and reference input clock to the PLL. Lastly,
+ * it sets the register based on these values along with the dividers.
+ * Note 1) There is no value checking for the passed-in divider values
+ *         so the caller has to make sure those values are sensible.
+ *      2) Also adjust the NFC divider such that the NFC clock doesn't
+ *         exceed NFC_CLK_MAX.
+ *      3) IPU HSP clock is independent of AHB clock. Even it can go up to
+ *         177MHz for higher voltage, this function fixes the max to 133MHz.
+ *      4) This function should not have allowed diag_printf() calls since
+ *         the serial driver has been stoped. But leave then here to allow
+ *         easy debugging by NOT calling the cyg_hal_plf_serial_stop().
+ *
+ * @param ref   pll input reference clock (24MHz)
+ * @param freq  core clock in Hz
+ * @param clk   clock type, e.g CPU_CLK, DDR_CLK, etc.
+ * @return      0 if successful; non-zero otherwise
+ */
+int mxc_set_clock(u32 ref, u32 freq, enum mxc_clock clk)
+{
+	freq *= SZ_DEC_1M;
+
+	switch (clk) {
+	case MXC_ARM_CLK:
+		if (config_core_clk(ref, freq))
+			return -1;
+		break;
+	case MXC_PERIPH_CLK:
+		if (config_periph_clk(ref, freq))
+			return -1;
+		break;
+	case MXC_DDR_CLK:
+		if (config_ddr_clk(freq))
+			return -1;
+		break;
+	case MXC_NFC_CLK:
+		if (config_nfc_clk(freq))
+			return -1;
+		break;
+	default:
+		printf("Warning:Unsupported or invalid clock type\n");
+	}
+
+	return 0;
+}
+
+
+/*
  * Dump some core clockes.
  */
 int do_mx5_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
@@ -281,6 +825,7 @@ int do_mx5_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	printf("pll3: %dMHz\n", freq / 1000000);
 	printf("ipg clock     : %dHz\n", mxc_get_clock(MXC_IPG_CLK));
 	printf("ipg per clock : %dHz\n", mxc_get_clock(MXC_IPG_PERCLK));
+	printf("ddr clock     : %dHz\n", mxc_get_clock(MXC_DDR_CLK));
 
 	return 0;
 }
diff --git a/arch/arm/include/asm/arch-mx5/clock.h b/arch/arm/include/asm/arch-mx5/clock.h
index 1f8a537..bcecd45 100644
--- a/arch/arm/include/asm/arch-mx5/clock.h
+++ b/arch/arm/include/asm/arch-mx5/clock.h
@@ -32,6 +32,9 @@ enum mxc_clock {
 	MXC_UART_CLK,
 	MXC_CSPI_CLK,
 	MXC_FEC_CLK,
+	MXC_DDR_CLK,
+	MXC_NFC_CLK,
+	MXC_PERIPH_CLK,
 };
 
 unsigned int imx_decode_pll(unsigned int pll, unsigned int f_ref);
@@ -39,5 +42,6 @@ unsigned int imx_decode_pll(unsigned int pll, unsigned int f_ref);
 u32 imx_get_uartclk(void);
 u32 imx_get_fecclk(void);
 unsigned int mxc_get_clock(enum mxc_clock clk);
+int mxc_set_clock(u32 ref, u32 freq, u32 clk_type);
 
 #endif /* __ASM_ARCH_CLOCK_H */
diff --git a/arch/arm/include/asm/arch-mx5/crm_regs.h b/arch/arm/include/asm/arch-mx5/crm_regs.h
index 4ed8eb3..735e4bd 100644
--- a/arch/arm/include/asm/arch-mx5/crm_regs.h
+++ b/arch/arm/include/asm/arch-mx5/crm_regs.h
@@ -76,6 +76,9 @@ struct mxc_ccm_reg {
 	u32 CCGR4;
 	u32 CCGR5;
 	u32 CCGR6;	/* 0x0080 */
+#ifdef CONFIG_MX53
+	u32 CCGR7;      /* 0x0084 */
+#endif
 	u32 cmeor;
 };
 
@@ -84,6 +87,9 @@ struct mxc_ccm_reg {
 #define MXC_CCM_CACRR_ARM_PODF_MASK		0x7
 
 /* Define the bits in register CBCDR */
+#define MXC_CCM_CBCDR_DDR_HIFREQ_SEL		(0x1 << 30)
+#define MXC_CCM_CBCDR_DDR_PODF_MASK		(0x7 << 27)
+#define MXC_CCM_CBCDR_DDR_PODF_OFFSET		27
 #define MXC_CCM_CBCDR_EMI_CLK_SEL		(0x1 << 26)
 #define MXC_CCM_CBCDR_PERIPH_CLK_SEL		(0x1 << 25)
 #define MXC_CCM_CBCDR_EMI_PODF_OFFSET		22
diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h b/arch/arm/include/asm/arch-mx5/imx-regs.h
index a1849f8..896a229 100644
--- a/arch/arm/include/asm/arch-mx5/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx5/imx-regs.h
@@ -98,6 +98,7 @@
 #define PLL1_BASE_ADDR		(AIPS2_BASE_ADDR + 0x00080000)
 #define PLL2_BASE_ADDR		(AIPS2_BASE_ADDR + 0x00084000)
 #define PLL3_BASE_ADDR		(AIPS2_BASE_ADDR + 0x00088000)
+#define PLL4_BASE_ADDR          (AIPS2_BASE_ADDR + 0x0008c000)
 #define AHBMAX_BASE_ADDR	(AIPS2_BASE_ADDR + 0x00094000)
 #define IIM_BASE_ADDR		(AIPS2_BASE_ADDR + 0x00098000)
 #define CSU_BASE_ADDR		(AIPS2_BASE_ADDR + 0x0009C000)
-- 
1.7.1

[U-Boot] [PATCH V7 2/3] PMIC: Add dialog pmic support

[Jason Liu]

This patch add dialog pmic(DA9053) driver with I2C interface support
In order to not duplicate code and according to the discussion on the
mail-list, change fsl_pmic.c to spi_i2c_pmic.c.Actaully,spi_i2c_pmic.c
is just a wrapper for PMIC communication when SPI or I2C is used.

Signed-off-by: Jason Liu <jason.hui@linaro.org>
Cc: sbabic at denx.de <sbabic@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
---
change since v1:
- use one file to support fsl pmic and dialog pmic
- rename fsl_pmic.c to spi_i2c_pmic.c
---
 drivers/misc/Makefile                       |    3 +-
 drivers/misc/{fsl_pmic.c => spi_i2c_pmic.c} |   72 +++++++++--
 include/da9053.h                            |  186 +++++++++++++++++++++++++++
 3 files changed, 248 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index b152486..b2e150e 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -26,9 +26,10 @@ include $(TOPDIR)/config.mk
 LIB	:= $(obj)libmisc.o
 
 COBJS-$(CONFIG_ALI152X) += ali512x.o
+COBJS-$(CONFIG_DIALOG_PMIC) += spi_i2c_pmic.o
 COBJS-$(CONFIG_DS4510)  += ds4510.o
 COBJS-$(CONFIG_FSL_LAW) += fsl_law.o
-COBJS-$(CONFIG_FSL_PMIC) += fsl_pmic.o
+COBJS-$(CONFIG_FSL_PMIC) += spi_i2c_pmic.o
 COBJS-$(CONFIG_GPIO_LED) += gpio_led.o
 COBJS-$(CONFIG_FSL_MC9SDZ60) += mc9sdz60.o
 COBJS-$(CONFIG_NS87308) += ns87308.o
diff --git a/drivers/misc/fsl_pmic.c b/drivers/misc/spi_i2c_pmic.c
similarity index 76%
rename from drivers/misc/fsl_pmic.c
rename to drivers/misc/spi_i2c_pmic.c
index ef80ad9..5fbfb40 100644
--- a/drivers/misc/fsl_pmic.c
+++ b/drivers/misc/spi_i2c_pmic.c
@@ -22,13 +22,16 @@
 
 #include <config.h>
 #include <common.h>
+#include <i2c.h>
 #include <asm/errno.h>
 #include <linux/types.h>
+#if defined(CONFIG_FSL_PMIC)
 #include <fsl_pmic.h>
+#endif
 
-static int check_param(u32 reg, u32 write)
+static int check_param(u32 reg, u32 write, u32 max_reg)
 {
-	if (reg > 63 || write > 1) {
+	if (reg > max_reg || write > 1) {
 		printf("<reg num> = %d is invalid. Should be less then 63\n",
 			reg);
 		return -1;
@@ -37,15 +40,13 @@ static int check_param(u32 reg, u32 write)
 	return 0;
 }
 
-#ifdef CONFIG_FSL_PMIC_I2C
-#include <i2c.h>
-
-u32 pmic_reg(u32 reg, u32 val, u32 write)
+#if defined(CONFIG_FSL_PMIC_I2C)
+u32 fsl_pmic_reg(u32 reg, u32 val, u32 write)
 {
-	unsigned char buf[4] = { 0 };
 	u32 ret_val = 0;
+	unsigned char buf[4] = { 0 };
 
-	if (check_param(reg, write))
+	if (check_param(reg, write, 63))
 		return -1;
 
 	if (write) {
@@ -62,7 +63,44 @@ u32 pmic_reg(u32 reg, u32 val, u32 write)
 
 	return ret_val;
 }
-#else /* SPI interface */
+#endif
+
+#if defined(CONFIG_DIALOG_PMIC_I2C)
+u32 dlg_pmic_reg(u32 reg, u32 val, u32 write)
+{
+	u32 ret_val = 0;
+	unsigned char buf[1] = { 0 };
+
+	if (check_param(reg, write, 142))
+		return -1;
+
+	if (write) {
+		buf[0] = (val) & 0xff;
+		if (i2c_write(CONFIG_SYS_DIALOG_PMIC_I2C_ADDR, reg, 1, buf, 1))
+			return -1;
+	} else {
+		if (i2c_read(CONFIG_SYS_DIALOG_PMIC_I2C_ADDR, reg, 1, buf, 1))
+			return -1;
+		ret_val = buf[0];
+	}
+
+	return ret_val;
+}
+#endif
+
+#if defined(CONFIG_FSL_PMIC_I2C) || defined(CONFIG_DIALOG_PMIC_I2C)
+#include <i2c.h>
+u32 pmic_reg(u32 reg, u32 val, u32 write)
+{
+#if defined(CONFIG_FSL_PMIC_I2C)
+	return fsl_pmic_reg(reg, val, write);
+#endif
+
+#if defined(CONFIG_DIALOG_PMIC_I2C)
+	return dlg_pmic_reg(reg, val, write);
+#endif
+}
+#elif defined(CONFIG_FSL_PMIC) /* SPI interface */
 #include <spi.h>
 static struct spi_slave *slave;
 
@@ -92,7 +130,7 @@ u32 pmic_reg(u32 reg, u32 val, u32 write)
 			return -1;
 	}
 
-	if (check_param(reg, write))
+	if (check_param(reg, write, 63))
 		return -1;
 
 	if (spi_claim_bus(slave))
@@ -135,6 +173,7 @@ u32 pmic_reg_read(u32 reg)
 
 void pmic_show_pmic_info(void)
 {
+#if defined(CONFIG_FSL_PMIC)
 	u32 rev_id;
 
 	rev_id = pmic_reg_read(REG_IDENTIFICATION);
@@ -178,6 +217,7 @@ void pmic_show_pmic_info(void)
 		break;
 	}
 	puts("]\n");
+#endif
 }
 
 static void pmic_dump(int numregs)
@@ -189,7 +229,7 @@ static void pmic_dump(int numregs)
 	for (i = 0; i < numregs; i++) {
 		val = pmic_reg_read(i);
 		if (!(i % 8))
-			printf ("\n0x%02x: ", i);
+			printf("\n0x%02x: ", i);
 		printf("%08x ", val);
 	}
 	puts("\n");
@@ -227,9 +267,17 @@ int do_pmic(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return 1;
 }
 
+#if defined(CONFIG_FSL_PMIC)
+#define PMIC_NAME "Freescale PMIC (Atlas)"
+#elif defined(CONFIG_DIALOG_PMIC)
+#define PMIC_NAME "Dialog PMIC (DA905x)"
+#else
+#error "Unkown PMIC name"
+#endif
+
 U_BOOT_CMD(
 	pmic,	CONFIG_SYS_MAXARGS, 1, do_pmic,
-	"Freescale PMIC (Atlas)",
+	PMIC_NAME,
 	"dump [numregs] dump registers\n"
 	"pmic write <reg> <value> - write register"
 );
diff --git a/include/da9053.h b/include/da9053.h
new file mode 100644
index 0000000..f69408a
--- /dev/null
+++ b/include/da9053.h
@@ -0,0 +1,186 @@
+/*
+ * da9053 register declarations.
+ *
+ * Copyright(c) 2009 Dialog Semiconductor Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+#ifndef __DA9053_REG_H__
+#define __DA9053_REG_H__
+
+enum {
+	DA9053_PAGECON0_REG = 0,
+	DA9053_STATUSA_REG,
+	DA9053_STATUSB_REG,
+	DA9053_STATUSC_REG,
+	DA9053_STATUSD_REG,
+	DA9053_EVENTA_REG,
+	DA9053_EVENTB_REG,
+	DA9053_EVENTC_REG,
+	DA9053_EVENTD_REG,
+	DA9053_FAULTLOG_REG,
+	DA9053_IRQMASKA_REG,
+	DA9053_IRQMASKB_REG,
+	DA9053_IRQMASKC_REG,
+	DA9053_IRQMASKD_REG,
+	DA9053_CONTROLA_REG,
+	DA9053_CONTROLB_REG,
+	DA9053_CONTROLC_REG,
+	DA9053_CONTROLD_REG,
+	DA9053_PDDIS_REG,
+	DA9053_INTERFACE_REG,
+	DA9053_RESET_REG,
+	DA9053_GPIO0001_REG,
+	DA9053_GPIO0203_REG,
+	DA9053_GPIO0405_REG,
+	DA9053_GPIO0607_REG,
+	DA9053_GPIO0809_REG,
+	DA9053_GPIO1011_REG,
+	DA9053_GPIO1213_REG,
+	DA9053_GPIO1415_REG,
+	DA9053_ID01_REG,
+	DA9053_ID23_REG,
+	DA9053_ID45_REG,
+	DA9053_ID67_REG,
+	DA9053_ID89_REG,
+	DA9053_ID1011_REG,
+	DA9053_ID1213_REG,
+	DA9053_ID1415_REG,
+	DA9053_ID1617_REG,
+	DA9053_ID1819_REG,
+	DA9053_ID2021_REG,
+	DA9053_SEQSTATUS_REG,
+	DA9053_SEQA_REG,
+	DA9053_SEQB_REG,
+	DA9053_SEQTIMER_REG,
+	DA9053_BUCKA_REG,
+	DA9053_BUCKB_REG,
+	DA9053_BUCKCORE_REG,
+	DA9053_BUCKPRO_REG,
+	DA9053_BUCKMEM_REG,
+	DA9053_BUCKPERI_REG,
+	DA9053_LDO1_REG,
+	DA9053_LDO2_REG,
+	DA9053_LDO3_REG,
+	DA9053_LDO4_REG,
+	DA9053_LDO5_REG,
+	DA9053_LDO6_REG,
+	DA9053_LDO7_REG,
+	DA9053_LDO8_REG,
+	DA9053_LDO9_REG,
+	DA9053_LDO10_REG,
+	DA9053_SUPPLY_REG,
+	DA9053_PULLDOWN_REG,
+	DA9053_CHGBUCK_REG,
+	DA9053_WAITCONT_REG,
+	DA9053_ISET_REG,
+	DA9053_BATCHG_REG,
+	DA9053_CHGCONT_REG,
+	DA9053_INPUTCONT_REG,
+	DA9053_CHGTIME_REG,
+	DA9053_BBATCONT_REG,
+	DA9053_BOOST_REG,
+	DA9053_LEDCONT_REG,
+	DA9053_LEDMIN123_REG,
+	DA9053_LED1CONF_REG,
+	DA9053_LED2CONF_REG,
+	DA9053_LED3CONF_REG,
+	DA9053_LED1CONT_REG,
+	DA9053_LED2CONT_REG,
+	DA9053_LED3CONT_REG,
+	DA9053_LED4CONT_REG,
+	DA9053_LED5CONT_REG,
+	DA9053_ADCMAN_REG,
+	DA9053_ADCCONT_REG,
+	DA9053_ADCRESL_REG,
+	DA9053_ADCRESH_REG,
+	DA9053_VDDRES_REG,
+	DA9053_VDDMON_REG,
+	DA9053_ICHGAV_REG,
+	DA9053_ICHGTHD_REG,
+	DA9053_ICHGEND_REG,
+	DA9053_TBATRES_REG,
+	DA9053_TBATHIGHP_REG,
+	DA9053_TBATHIGHIN_REG,
+	DA9053_TBATLOW_REG,
+	DA9053_TOFFSET_REG,
+	DA9053_ADCIN4RES_REG,
+	DA9053_AUTO4HIGH_REG,
+	DA9053_AUTO4LOW_REG,
+	DA9053_ADCIN5RES_REG,
+	DA9053_AUTO5HIGH_REG,
+	DA9053_AUTO5LOW_REG,
+	DA9053_ADCIN6RES_REG,
+	DA9053_AUTO6HIGH_REG,
+	DA9053_AUTO6LOW_REG,
+	DA9053_TJUNCRES_REG,
+	DA9053_TSICONTA_REG,
+	DA9053_TSICONTB_REG,
+	DA9053_TSIXMSB_REG,
+	DA9053_TSIYMSB_REG,
+	DA9053_TSILSB_REG,
+	DA9053_TSIZMSB_REG,
+	DA9053_COUNTS_REG,
+	DA9053_COUNTMI_REG,
+	DA9053_COUNTH_REG,
+	DA9053_COUNTD_REG,
+	DA9053_COUNTMO_REG,
+	DA9053_COUNTY_REG,
+	DA9053_ALARMMI_REG,
+	DA9053_ALARMH_REG,
+	DA9053_ALARMD_REG,
+	DA9053_ALARMMO_REG,
+	DA9053_ALARMY_REG,
+	DA9053_SECONDA_REG,
+	DA9053_SECONDB_REG,
+	DA9053_SECONDC_REG,
+	DA9053_SECONDD_REG,
+	DA9053_PAGECON128_REG,
+	DA9053_CHIPID_REG,
+	DA9053_CONFIGID_REG,
+	DA9053_OTPCONT_REG,
+	DA9053_OSCTRIM_REG,
+	DA9053_GPID0_REG,
+	DA9053_GPID1_REG,
+	DA9053_GPID2_REG,
+	DA9053_GPID3_REG,
+	DA9053_GPID4_REG,
+	DA9053_GPID5_REG,
+	DA9053_GPID6_REG,
+	DA9053_GPID7_REG,
+	DA9053_GPID8_REG,
+	DA9053_GPID9_REG,
+};
+
+#define DA_BUCKCORE_VBCORE_1_250V 0x1e
+
+/* BUCKCORE REGISTER */
+#define DA9052_BUCKCORE_BCORECONF               (1<<7)
+#define DA9052_BUCKCORE_BCOREEN                 (1<<6)
+#define DA9052_BUCKCORE_VBCORE                  (63<<0)
+
+/* SUPPLY REGISTER */
+#define DA9052_SUPPLY_VLOCK                     (1<<7)
+#define DA9052_SUPPLY_VMEMSWEN                  (1<<6)
+#define DA9052_SUPPLY_VPERISWEN                 (1<<5)
+#define DA9052_SUPPLY_VLDO3GO                   (1<<4)
+#define DA9052_SUPPLY_VLDO2GO                   (1<<3)
+#define DA9052_SUPPLY_VBMEMGO                   (1<<2)
+#define DA9052_SUPPLY_VBPROGO                   (1<<1)
+#define DA9052_SUPPLY_VBCOREGO                  (1<<0)
+
+#endif /* __DA9053_REG_H__ */
-- 
1.7.1

[U-Boot] [PATCH V7 3/3] MX53: support for freescale MX53LOCO board

[Jason Liu]

This patch add initial support for freescale MX53LOCO board.
Network(FEC),SD/MMC,UART have been supported by this patch

The patch also config CPU:1GHZ,DDR:400MHZ for better peformance

Signed-off-by: Jason Liu <jason.hui@linaro.org>
---
Changes since v5:
- merge the "Add power init support" patch
changes since v4:
- remove the boot reset cause from board support
changes since v3:
- include other two small patch into this commit,
mx53loco: set mmc env to MMC slot1
MX5: Enable flat-device-tree support on mx53 loco board
changes since V2:
-factor out the boot cause function to common code,
-fix gd->ram_size with full memory size
-remove the '1' from all #defines that just enable features
-correct memory test end address value
---
 MAINTAINERS                               |    1 +
 arch/arm/cpu/armv7/mx5/soc.c              |    2 +-
 arch/arm/include/asm/arch-mx5/sys_proto.h |    2 +
 board/freescale/mx53loco/Makefile         |   47 ++++
 board/freescale/mx53loco/imximage.cfg     |   96 +++++++
 board/freescale/mx53loco/mx53loco.c       |  395 +++++++++++++++++++++++++++++
 boards.cfg                                |    1 +
 include/configs/mx53loco.h                |  199 +++++++++++++++
 8 files changed, 742 insertions(+), 1 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e2a4ba9..42c5048 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -574,6 +574,7 @@ Stefano Babic <sbabic@denx.de>
 Jason Liu <r64343@freescale.com>
 
 	mx53evk         i.MX53
+	mx53loco        i.MX53
 
 Enric Balletbo i Serra <eballetbo@iseebcn.com>
 
diff --git a/arch/arm/cpu/armv7/mx5/soc.c b/arch/arm/cpu/armv7/mx5/soc.c
index 6f4e8db..9c03474 100644
--- a/arch/arm/cpu/armv7/mx5/soc.c
+++ b/arch/arm/cpu/armv7/mx5/soc.c
@@ -116,7 +116,7 @@ int print_cpuinfo(void)
 		(cpurev & 0x000F0) >> 4,
 		(cpurev & 0x0000F) >> 0,
 		mxc_get_clock(MXC_ARM_CLK) / 1000000);
-	printf("Reset cause: %s\n", get_reset_cause());
+	printf("Reset  cause: %s\n", get_reset_cause());
 	return 0;
 }
 #endif
diff --git a/arch/arm/include/asm/arch-mx5/sys_proto.h b/arch/arm/include/asm/arch-mx5/sys_proto.h
index f687503..2d7e9ed 100644
--- a/arch/arm/include/asm/arch-mx5/sys_proto.h
+++ b/arch/arm/include/asm/arch-mx5/sys_proto.h
@@ -27,5 +27,7 @@
 u32 get_cpu_rev(void);
 #define is_soc_rev(rev)	((get_cpu_rev() & 0xFF) - rev)
 void sdelay(unsigned long);
+void pmic_reg_write(u32 reg, u32 value);
+u32 pmic_reg_read(u32 reg);
 
 #endif
diff --git a/board/freescale/mx53loco/Makefile b/board/freescale/mx53loco/Makefile
new file mode 100644
index 0000000..2088a48
--- /dev/null
+++ b/board/freescale/mx53loco/Makefile
@@ -0,0 +1,47 @@
+#
+# (C) Copyright 2011 Freescale Semiconductor, Inc.
+# Jason Liu <r64343@freescale.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)lib$(BOARD).o
+
+COBJS	:= mx53loco.o
+
+SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+SOBJS	:= $(addprefix $(obj),$(SOBJS))
+
+$(LIB):	$(obj).depend $(OBJS) $(SOBJS)
+	$(call cmd_link_o_target, $(OBJS) $(SOBJS))
+
+clean:
+	rm -f $(SOBJS) $(OBJS)
+
+distclean:	clean
+	rm -f $(LIB) core *.bak .depend
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/board/freescale/mx53loco/imximage.cfg b/board/freescale/mx53loco/imximage.cfg
new file mode 100644
index 0000000..ce9c8fc
--- /dev/null
+++ b/board/freescale/mx53loco/imximage.cfg
@@ -0,0 +1,96 @@
+# Copyright (C) 2011 Freescale Semiconductor, Inc.
+# Jason Liu <r64343@freescale.com>
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not write to the Free Software
+# Foundation Inc. 51 Franklin Street Fifth Floor Boston,
+# MA 02110-1301 USA
+#
+# Refer docs/README.imxmage for more details about how-to configure
+# and create imximage boot image
+#
+# The syntax is taken as close as possible with the kwbimage
+
+# image version
+
+IMAGE_VERSION 2
+
+# Boot Device : one of
+# spi, sd (the board has no nand neither onenand)
+
+BOOT_FROM	sd
+
+# Device Configuration Data (DCD)
+#
+# Each entry must have the format:
+# Addr-type           Address        Value
+#
+# where:
+#	Addr-type register length (1,2 or 4 bytes)
+#	Address	  absolute address of the register
+#	value	  value to be stored in the register
+
+DATA 4 0x53fa8554 0x00300000
+DATA 4 0x53fa8558 0x00300040
+DATA 4 0x53fa8560 0x00300000
+DATA 4 0x53fa8564 0x00300040
+DATA 4 0x53fa8568 0x00300040
+DATA 4 0x53fa8570 0x00300000
+DATA 4 0x53fa8574 0x00300000
+DATA 4 0x53fa8578 0x00300000
+DATA 4 0x53fa857c 0x00300040
+DATA 4 0x53fa8580 0x00300040
+DATA 4 0x53fa8584 0x00300000
+DATA 4 0x53fa8588 0x00300000
+DATA 4 0x53fa8590 0x00300040
+DATA 4 0x53fa8594 0x00300000
+DATA 4 0x53fa86f0 0x00300000
+DATA 4 0x53fa86f4 0x00000000
+DATA 4 0x53fa86fc 0x00000000
+DATA 4 0x53fa8714 0x00000000
+DATA 4 0x53fa8718 0x00300000
+DATA 4 0x53fa871c 0x00300000
+DATA 4 0x53fa8720 0x00300000
+DATA 4 0x53fa8724 0x04000000
+DATA 4 0x53fa8728 0x00300000
+DATA 4 0x53fa872c 0x00300000
+DATA 4 0x63fd9088 0x35343535
+DATA 4 0x63fd9090 0x4d444c44
+DATA 4 0x63fd907c 0x01370138
+DATA 4 0x63fd9080 0x013b013c
+DATA 4 0x63fd9018 0x00011740
+DATA 4 0x63fd9000 0xc3190000
+DATA 4 0x63fd900c 0x9f5152e3
+DATA 4 0x63fd9010 0xb68e8a63
+DATA 4 0x63fd9014 0x01ff00db
+DATA 4 0x63fd902c 0x000026d2
+DATA 4 0x63fd9030 0x009f0e21
+DATA 4 0x63fd9008 0x12273030
+DATA 4 0x63fd9004 0x0002002d
+DATA 4 0x63fd901c 0x00008032
+DATA 4 0x63fd901c 0x00008033
+DATA 4 0x63fd901c 0x00028031
+DATA 4 0x63fd901c 0x092080b0
+DATA 4 0x63fd901c 0x04008040
+DATA 4 0x63fd901c 0x0000803a
+DATA 4 0x63fd901c 0x0000803b
+DATA 4 0x63fd901c 0x00028039
+DATA 4 0x63fd901c 0x09208138
+DATA 4 0x63fd901c 0x04008048
+DATA 4 0x63fd9020 0x00001800
+DATA 4 0x63fd9040 0x04b80003
+DATA 4 0x63fd9058 0x00022227
+DATA 4 0x63fd901c 0x00000000
diff --git a/board/freescale/mx53loco/mx53loco.c b/board/freescale/mx53loco/mx53loco.c
new file mode 100644
index 0000000..62ed400
--- /dev/null
+++ b/board/freescale/mx53loco/mx53loco.c
@@ -0,0 +1,395 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc.
+ * Jason Liu <r64343@freescale.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/imx-regs.h>
+#include <asm/arch/mx5x_pins.h>
+#include <asm/arch/sys_proto.h>
+#include <asm/arch/crm_regs.h>
+#include <asm/arch/iomux.h>
+#include <asm/arch/clock.h>
+#include <asm/errno.h>
+#include <netdev.h>
+#include <i2c.h>
+#include <mmc.h>
+#include <fsl_esdhc.h>
+#include <mxc_gpio.h>
+#include <da9053.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+u32 get_board_rev(void)
+{
+	return get_cpu_rev();
+}
+
+int dram_init(void)
+{
+	u32 size1, size2;
+
+	size1 = get_ram_size((volatile void *)PHYS_SDRAM_1, PHYS_SDRAM_1_SIZE);
+	size2 = get_ram_size((volatile void *)PHYS_SDRAM_2, PHYS_SDRAM_2_SIZE);
+
+	gd->ram_size = size1 + size2;
+
+	return 0;
+}
+void dram_init_banksize(void)
+{
+	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
+	gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
+
+	gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
+	gd->bd->bi_dram[1].size = PHYS_SDRAM_2_SIZE;
+}
+
+static void setup_iomux_uart(void)
+{
+	/* UART1 RXD */
+	mxc_request_iomux(MX53_PIN_CSI0_D11, IOMUX_CONFIG_ALT2);
+	mxc_iomux_set_pad(MX53_PIN_CSI0_D11,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_100K_PU |
+				PAD_CTL_ODE_OPENDRAIN_ENABLE);
+	mxc_iomux_set_input(MX53_UART1_IPP_UART_RXD_MUX_SELECT_INPUT, 0x1);
+
+	/* UART1 TXD */
+	mxc_request_iomux(MX53_PIN_CSI0_D10, IOMUX_CONFIG_ALT2);
+	mxc_iomux_set_pad(MX53_PIN_CSI0_D10,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_100K_PU |
+				PAD_CTL_ODE_OPENDRAIN_ENABLE);
+}
+
+static void setup_iomux_fec(void)
+{
+	/*FEC_MDIO*/
+	mxc_request_iomux(MX53_PIN_FEC_MDIO, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_MDIO,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_22K_PU | PAD_CTL_ODE_OPENDRAIN_ENABLE);
+	mxc_iomux_set_input(MX53_FEC_FEC_MDI_SELECT_INPUT, 0x1);
+
+	/*FEC_MDC*/
+	mxc_request_iomux(MX53_PIN_FEC_MDC, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_MDC, PAD_CTL_DRV_HIGH);
+
+	/* FEC RXD1 */
+	mxc_request_iomux(MX53_PIN_FEC_RXD1, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_RXD1,
+			PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE);
+
+	/* FEC RXD0 */
+	mxc_request_iomux(MX53_PIN_FEC_RXD0, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_RXD0,
+			PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE);
+
+	 /* FEC TXD1 */
+	mxc_request_iomux(MX53_PIN_FEC_TXD1, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_TXD1, PAD_CTL_DRV_HIGH);
+
+	/* FEC TXD0 */
+	mxc_request_iomux(MX53_PIN_FEC_TXD0, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_TXD0, PAD_CTL_DRV_HIGH);
+
+	/* FEC TX_EN */
+	mxc_request_iomux(MX53_PIN_FEC_TX_EN, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_TX_EN, PAD_CTL_DRV_HIGH);
+
+	/* FEC TX_CLK */
+	mxc_request_iomux(MX53_PIN_FEC_REF_CLK, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_REF_CLK,
+			PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE);
+
+	/* FEC RX_ER */
+	mxc_request_iomux(MX53_PIN_FEC_RX_ER, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_RX_ER,
+			PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE);
+
+	/* FEC CRS */
+	mxc_request_iomux(MX53_PIN_FEC_CRS_DV, IOMUX_CONFIG_ALT0);
+	mxc_iomux_set_pad(MX53_PIN_FEC_CRS_DV,
+			PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE);
+}
+
+#ifdef CONFIG_FSL_ESDHC
+struct fsl_esdhc_cfg esdhc_cfg[2] = {
+	{MMC_SDHC1_BASE_ADDR, 1},
+	{MMC_SDHC3_BASE_ADDR, 1},
+};
+
+int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+{
+	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
+
+	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
+		*cd = mxc_gpio_get(77); /*GPIO3_13*/
+	else
+		*cd = mxc_gpio_get(75); /*GPIO3_11*/
+
+	return 0;
+}
+
+int board_mmc_init(bd_t *bis)
+{
+	u32 index;
+	s32 status = 0;
+
+	for (index = 0; index < CONFIG_SYS_FSL_ESDHC_NUM; index++) {
+		switch (index) {
+		case 0:
+			mxc_request_iomux(MX53_PIN_SD1_CMD, IOMUX_CONFIG_ALT0);
+			mxc_request_iomux(MX53_PIN_SD1_CLK, IOMUX_CONFIG_ALT0);
+			mxc_request_iomux(MX53_PIN_SD1_DATA0,
+						IOMUX_CONFIG_ALT0);
+			mxc_request_iomux(MX53_PIN_SD1_DATA1,
+						IOMUX_CONFIG_ALT0);
+			mxc_request_iomux(MX53_PIN_SD1_DATA2,
+						IOMUX_CONFIG_ALT0);
+			mxc_request_iomux(MX53_PIN_SD1_DATA3,
+						IOMUX_CONFIG_ALT0);
+			mxc_request_iomux(MX53_PIN_EIM_DA13,
+						IOMUX_CONFIG_ALT1);
+
+			mxc_iomux_set_pad(MX53_PIN_SD1_CMD,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_100K_PU);
+			mxc_iomux_set_pad(MX53_PIN_SD1_CLK,
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU |
+				PAD_CTL_DRV_HIGH);
+			mxc_iomux_set_pad(MX53_PIN_SD1_DATA0,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_SD1_DATA1,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_SD1_DATA2,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_SD1_DATA3,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			break;
+		case 1:
+			mxc_request_iomux(MX53_PIN_ATA_RESET_B,
+						IOMUX_CONFIG_ALT2);
+			mxc_request_iomux(MX53_PIN_ATA_IORDY,
+						IOMUX_CONFIG_ALT2);
+			mxc_request_iomux(MX53_PIN_ATA_DATA8,
+						IOMUX_CONFIG_ALT4);
+			mxc_request_iomux(MX53_PIN_ATA_DATA9,
+						IOMUX_CONFIG_ALT4);
+			mxc_request_iomux(MX53_PIN_ATA_DATA10,
+						IOMUX_CONFIG_ALT4);
+			mxc_request_iomux(MX53_PIN_ATA_DATA11,
+						IOMUX_CONFIG_ALT4);
+			mxc_request_iomux(MX53_PIN_ATA_DATA0,
+						IOMUX_CONFIG_ALT4);
+			mxc_request_iomux(MX53_PIN_ATA_DATA1,
+						IOMUX_CONFIG_ALT4);
+			mxc_request_iomux(MX53_PIN_ATA_DATA2,
+						IOMUX_CONFIG_ALT4);
+			mxc_request_iomux(MX53_PIN_ATA_DATA3,
+						IOMUX_CONFIG_ALT4);
+			mxc_request_iomux(MX53_PIN_EIM_DA11,
+						IOMUX_CONFIG_ALT1);
+
+			mxc_iomux_set_pad(MX53_PIN_ATA_RESET_B,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_100K_PU);
+			mxc_iomux_set_pad(MX53_PIN_ATA_IORDY,
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU |
+				PAD_CTL_DRV_HIGH);
+			mxc_iomux_set_pad(MX53_PIN_ATA_DATA8,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_ATA_DATA9,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_ATA_DATA10,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_ATA_DATA11,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_ATA_DATA0,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_ATA_DATA1,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_ATA_DATA2,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+			mxc_iomux_set_pad(MX53_PIN_ATA_DATA3,
+				PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH |
+				PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_HYS_ENABLE | PAD_CTL_47K_PU);
+
+			break;
+		default:
+			printf("Warning: you configured more ESDHC controller"
+				"(%d) as supported by the board(2)\n",
+				CONFIG_SYS_FSL_ESDHC_NUM);
+			return status;
+		}
+		status |= fsl_esdhc_initialize(bis, &esdhc_cfg[index]);
+	}
+
+	return status;
+}
+#endif
+
+static void setup_i2c(unsigned int port_number)
+{
+	switch (port_number) {
+	case 0:
+		/* i2c1 SDA */
+		mxc_request_iomux(MX53_PIN_CSI0_D8,
+				IOMUX_CONFIG_ALT5 | IOMUX_CONFIG_SION);
+		mxc_iomux_set_input(MX53_I2C1_IPP_SDA_IN_SELECT_INPUT,
+				INPUT_CTL_PATH0);
+		mxc_iomux_set_pad(MX53_PIN_CSI0_D8,
+				PAD_CTL_SRE_FAST | PAD_CTL_DRV_HIGH |
+				PAD_CTL_100K_PU | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_PUE_PULL |
+				PAD_CTL_ODE_OPENDRAIN_ENABLE);
+		/* i2c1 SCL */
+		mxc_request_iomux(MX53_PIN_CSI0_D9,
+				IOMUX_CONFIG_ALT5 | IOMUX_CONFIG_SION);
+		mxc_iomux_set_input(MX53_I2C1_IPP_SCL_IN_SELECT_INPUT,
+				INPUT_CTL_PATH0);
+		mxc_iomux_set_pad(MX53_PIN_CSI0_D9,
+				PAD_CTL_SRE_FAST | PAD_CTL_DRV_HIGH |
+				PAD_CTL_100K_PU | PAD_CTL_PKE_ENABLE |
+				PAD_CTL_PUE_PULL |
+				PAD_CTL_ODE_OPENDRAIN_ENABLE);
+		break;
+	case 1:
+		/* i2c2 SDA */
+		mxc_request_iomux(MX53_PIN_KEY_ROW3,
+				IOMUX_CONFIG_ALT4 | IOMUX_CONFIG_SION);
+		mxc_iomux_set_input(MX53_I2C2_IPP_SDA_IN_SELECT_INPUT,
+				INPUT_CTL_PATH0);
+		mxc_iomux_set_pad(MX53_PIN_KEY_ROW3,
+				PAD_CTL_SRE_FAST | PAD_CTL_DRV_HIGH |
+				PAD_CTL_100K_PU | PAD_CTL_HYS_ENABLE |
+				PAD_CTL_ODE_OPENDRAIN_ENABLE);
+
+		/* i2c2 SCL */
+		mxc_request_iomux(MX53_PIN_KEY_COL3,
+				IOMUX_CONFIG_ALT4 | IOMUX_CONFIG_SION);
+		mxc_iomux_set_input(MX53_I2C2_IPP_SCL_IN_SELECT_INPUT,
+				INPUT_CTL_PATH0);
+		mxc_iomux_set_pad(MX53_PIN_KEY_COL3,
+				PAD_CTL_SRE_FAST | PAD_CTL_DRV_HIGH |
+				PAD_CTL_100K_PU | PAD_CTL_HYS_ENABLE |
+				PAD_CTL_ODE_OPENDRAIN_ENABLE);
+		break;
+	default:
+		printf("Warning: Wrong I2C port number\n");
+		break;
+	}
+}
+
+static void clock_init(void)
+{
+	int ret;
+	u32 ref_clk = CONFIG_SYS_MX5_HCLK;
+	/*
+	 * After increase voltage to 1.25V, We can switch
+	 * CPU clokc to 1Ghz and DDR to 400Mhz safely now
+	 */
+	ret = mxc_set_clock(ref_clk, 1000, MXC_ARM_CLK);
+	if (!ret)
+		printf("CPU:   Switch CPU clock to 1GHZ OK\n");
+	else
+		printf("CPU:   Switch CPU clock to 1GHZ failed\n");
+
+	ret = mxc_set_clock(ref_clk, 400, MXC_PERIPH_CLK);
+	ret |= mxc_set_clock(ref_clk, 400, MXC_DDR_CLK);
+	if (!ret)
+		printf("DDR:   Switch DDR clock to 400MHz OK\n");
+	else
+		printf("CPU:   Switch DDR clock to 1GHZ failed\n");
+}
+
+static void power_init(void)
+{
+	unsigned int val;
+
+	/* Set VDDA to 1.25V */
+	val = DA9052_BUCKCORE_BCOREEN;
+	val |= DA_BUCKCORE_VBCORE_1_250V;
+	pmic_reg_write(DA9053_BUCKCORE_REG, val);
+	val = pmic_reg_read(DA9053_SUPPLY_REG);
+	val |= DA9052_SUPPLY_VBCOREGO;
+	pmic_reg_write(DA9053_SUPPLY_REG, val);
+}
+
+int board_early_init_f(void)
+{
+	setup_iomux_uart();
+	setup_iomux_fec();
+
+	return 0;
+}
+
+int board_init(void)
+{
+	gd->bd->bi_arch_number = MACH_TYPE_MX53_LOCO;
+	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
+
+#ifdef CONFIG_I2C_MXC
+	setup_i2c(0);
+	power_init();
+	clock_init();
+#endif
+	return 0;
+}
+
+int checkboard(void)
+{
+	puts("Board: MX53 LOCO\n");
+
+	return 0;
+}
diff --git a/boards.cfg b/boards.cfg
index 2b0900a..67fd163 100644
--- a/boards.cfg
+++ b/boards.cfg
@@ -124,6 +124,7 @@ ca9x4_ct_vxp                 arm         armv7       vexpress            armltd
 efikamx                      arm         armv7       efikamx             -              mx5
 mx51evk                      arm         armv7       mx51evk             freescale      mx5		mx51evk:IMX_CONFIG=board/freescale/mx51evk/imximage.cfg
 mx53evk                      arm         armv7       mx53evk             freescale      mx5		mx53evk:IMX_CONFIG=board/freescale/mx53evk/imximage.cfg
+mx53loco                     arm         armv7       mx53loco            freescale      mx5		mx53loco:IMX_CONFIG=board/freescale/mx53loco/imximage.cfg
 vision2                      arm         armv7       vision2             ttcontrol      mx5
 cm_t35                       arm         armv7       cm_t35              -              omap3
 omap3_overo                  arm         armv7       overo               -              omap3
diff --git a/include/configs/mx53loco.h b/include/configs/mx53loco.h
new file mode 100644
index 0000000..116af6d
--- /dev/null
+++ b/include/configs/mx53loco.h
@@ -0,0 +1,199 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc.
+ * Jason Liu <r64343@freescale.com>
+ *
+ * Configuration settings for Freescale MX53 low cost board.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __CONFIG_H
+#define __CONFIG_H
+
+#define CONFIG_MX53
+
+#define CONFIG_SYS_MX5_HCLK	24000000
+#define CONFIG_SYS_MX5_CLK32		32768
+#define CONFIG_DISPLAY_CPUINFO
+#define CONFIG_DISPLAY_BOARDINFO
+
+#define CONFIG_L2_OFF
+
+#include <asm/arch/imx-regs.h>
+
+#define CONFIG_CMDLINE_TAG
+#define CONFIG_REVISION_TAG
+#define CONFIG_SETUP_MEMORY_TAGS
+#define CONFIG_INITRD_TAG
+
+/* Size of malloc() pool */
+#define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + 2 * 1024 * 1024)
+
+#define CONFIG_BOARD_EARLY_INIT_F
+#define CONFIG_MXC_GPIO
+
+#define CONFIG_MXC_UART
+#define CONFIG_SYS_MX53_UART1
+
+/* I2C Configs */
+#define CONFIG_CMD_I2C
+#define CONFIG_HARD_I2C
+#define CONFIG_I2C_MXC
+#define CONFIG_SYS_I2C_MX53_PORT1
+#define CONFIG_SYS_I2C_SPEED            100000
+#define CONFIG_SYS_I2C_SLAVE            0xfe
+
+/* PMIC Configs */
+#define CONFIG_DIALOG_PMIC
+#define CONFIG_DIALOG_PMIC_I2C
+#define CONFIG_SYS_DIALOG_PMIC_I2C_ADDR    0x48
+
+/* MMC Configs */
+#define CONFIG_FSL_ESDHC
+#define CONFIG_SYS_FSL_ESDHC_ADDR	0
+#define CONFIG_SYS_FSL_ESDHC_NUM	2
+
+#define CONFIG_MMC
+#define CONFIG_CMD_MMC
+#define CONFIG_GENERIC_MMC
+#define CONFIG_CMD_FAT
+#define CONFIG_DOS_PARTITION
+
+/* Eth Configs */
+#define CONFIG_HAS_ETH1
+#define CONFIG_NET_MULTI
+#define CONFIG_MII
+#define CONFIG_DISCOVER_PHY
+
+#define CONFIG_FEC_MXC
+#define IMX_FEC_BASE	FEC_BASE_ADDR
+#define CONFIG_FEC_MXC_PHYADDR	0x1F
+
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_DHCP
+#define CONFIG_CMD_MII
+#define CONFIG_CMD_NET
+
+/* allow to overwrite serial and ethaddr */
+#define CONFIG_ENV_OVERWRITE
+#define CONFIG_CONS_INDEX		1
+#define CONFIG_BAUDRATE			115200
+#define CONFIG_SYS_BAUDRATE_TABLE	{9600, 19200, 38400, 57600, 115200}
+
+/* Command definition */
+#include <config_cmd_default.h>
+
+#undef CONFIG_CMD_IMLS
+
+#define CONFIG_BOOTDELAY	3
+
+#define CONFIG_PRIME	"FEC0"
+
+#define CONFIG_LOADADDR		0x70800000	/* loadaddr env var */
+#define CONFIG_SYS_TEXT_BASE    0x77800000
+
+#define CONFIG_EXTRA_ENV_SETTINGS \
+	"script=boot.scr\0" \
+	"uimage=uImage\0" \
+	"mmcdev=0\0" \
+	"mmcpart=2\0" \
+	"mmcroot=/dev/mmcblk0p3 rw\0" \
+	"mmcrootfstype=ext3 rootwait\0" \
+	"mmcargs=setenv bootargs console=ttymxc0,${baudrate} " \
+		"root=${mmcroot} " \
+		"rootfstype=${mmcrootfstype}\0" \
+	"loadbootscript=" \
+		"fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${script};\0" \
+	"bootscript=echo Running bootscript from mmc ...; " \
+		"source\0" \
+	"loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \
+	"mmcboot=echo Booting from mmc ...; " \
+		"run mmcargs; " \
+		"bootm\0" \
+	"netargs=setenv bootargs console=ttymxc0,${baudrate} " \
+		"root=/dev/nfs " \
+		"ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \
+	"netboot=echo Booting from net ...; " \
+		"run netargs; " \
+		"dhcp ${uimage}; bootm\0" \
+
+#define CONFIG_BOOTCOMMAND \
+	"if mmc rescan ${mmcdev}; then " \
+		"if run loadbootscript; then " \
+			"run bootscript; " \
+		"else " \
+			"if run loaduimage; then " \
+				"run mmcboot; " \
+			"else run netboot; " \
+			"fi; " \
+		"fi; " \
+	"else run netboot; fi"
+
+#define CONFIG_ARP_TIMEOUT	200UL
+
+/* Miscellaneous configurable options */
+#define CONFIG_SYS_LONGHELP		/* undef to save memory */
+#define CONFIG_SYS_HUSH_PARSER		/* use "hush" command parser */
+#define CONFIG_SYS_PROMPT_HUSH_PS2	"> "
+#define CONFIG_SYS_PROMPT		"MX53LOCO U-Boot > "
+#define CONFIG_AUTO_COMPLETE
+#define CONFIG_SYS_CBSIZE		256	/* Console I/O Buffer Size */
+
+/* Print Buffer Size */
+#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16)
+#define CONFIG_SYS_MAXARGS	16	/* max number of command args */
+#define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE /* Boot Argument Buffer Size */
+
+#define CONFIG_SYS_MEMTEST_START       0x70000000
+#define CONFIG_SYS_MEMTEST_END         0x70010000
+
+#define CONFIG_SYS_LOAD_ADDR		CONFIG_LOADADDR
+
+#define CONFIG_SYS_HZ		1000
+#define CONFIG_CMDLINE_EDITING
+
+/* Stack sizes */
+#define CONFIG_STACKSIZE	(128 * 1024)	/* regular stack */
+
+/* Physical Memory Map */
+#define CONFIG_NR_DRAM_BANKS	2
+#define PHYS_SDRAM_1		CSD0_BASE_ADDR
+#define PHYS_SDRAM_1_SIZE	(512 * 1024 * 1024)
+#define PHYS_SDRAM_2		CSD1_BASE_ADDR
+#define PHYS_SDRAM_2_SIZE	(512 * 1024 * 1024)
+#define PHYS_SDRAM_SIZE         (PHYS_SDRAM_1_SIZE + PHYS_SDRAM_2_SIZE)
+
+#define CONFIG_SYS_SDRAM_BASE		(PHYS_SDRAM_1)
+#define CONFIG_SYS_INIT_RAM_ADDR	(IRAM_BASE_ADDR)
+#define CONFIG_SYS_INIT_RAM_SIZE	(IRAM_SIZE)
+
+#define CONFIG_SYS_INIT_SP_OFFSET \
+	(CONFIG_SYS_INIT_RAM_SIZE - GENERATED_GBL_DATA_SIZE)
+#define CONFIG_SYS_INIT_SP_ADDR \
+	(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET)
+
+/* FLASH and environment organization */
+#define CONFIG_SYS_NO_FLASH
+
+#define CONFIG_ENV_OFFSET      (6 * 64 * 1024)
+#define CONFIG_ENV_SIZE        (8 * 1024)
+#define CONFIG_ENV_IS_IN_MMC
+#define CONFIG_SYS_MMC_ENV_DEV 0
+
+#define CONFIG_OF_LIBFDT
+#define CONFIG_SYS_BOOTMAPSZ   0x800000
+
+#endif				/* __CONFIG_H */
-- 
1.7.1

[U-Boot] [PATCH V7 3/3] MX53: support for freescale MX53LOCO board

[Fabio Estevam]

Hi Jason,

--- On Wed, 5/11/11, Jason Liu <jason.hui@linaro.org> wrote:
... 
> diff --git a/arch/arm/cpu/armv7/mx5/soc.c
> b/arch/arm/cpu/armv7/mx5/soc.c
> index 6f4e8db..9c03474 100644
> --- a/arch/arm/cpu/armv7/mx5/soc.c
> +++ b/arch/arm/cpu/armv7/mx5/soc.c
...

This file is not MX53Loco specific, so it should be part of another patch.
 ??? ???
> mxc_get_clock(MXC_ARM_CLK) / 1000000);
> -??? printf("Reset cause: %s\n", get_reset_cause());
> +??? printf("Reset? cause: %s\n", get_reset_cause());
>  ??? return 0;

What is the purpose of this change?

> index f687503..2d7e9ed 100644
> --- a/arch/arm/include/asm/arch-mx5/sys_proto.h
> +++ b/arch/arm/include/asm/arch-mx5/sys_proto.h

This file is not MX53Loco specific, so it should be part of another patch.

Regards,

Fabio Estevam

[U-Boot] [PATCH V7 3/3] MX53: support for freescale MX53LOCO board

[Stefano Babic]

On 05/11/2011 10:03 AM, Jason Liu wrote:
> This patch add initial support for freescale MX53LOCO board.
> Network(FEC),SD/MMC,UART have been supported by this patch
> 
> The patch also config CPU:1GHZ,DDR:400MHZ for better peformance
> 
> Signed-off-by: Jason Liu <jason.hui@linaro.org>

Hi Jason,

> diff --git a/arch/arm/cpu/armv7/mx5/soc.c b/arch/arm/cpu/armv7/mx5/soc.c
> index 6f4e8db..9c03474 100644
> --- a/arch/arm/cpu/armv7/mx5/soc.c
> +++ b/arch/arm/cpu/armv7/mx5/soc.c
> @@ -116,7 +116,7 @@ int print_cpuinfo(void)
>  		(cpurev & 0x000F0) >> 4,
>  		(cpurev & 0x0000F) >> 0,
>  		mxc_get_clock(MXC_ARM_CLK) / 1000000);
> -	printf("Reset cause: %s\n", get_reset_cause());
> +	printf("Reset  cause: %s\n", get_reset_cause());
>  	return 0;

It seems to me this file slips into your patch, but it was not supposed
to be.

>  }
>  #endif
> diff --git a/arch/arm/include/asm/arch-mx5/sys_proto.h b/arch/arm/include/asm/arch-mx5/sys_proto.h
> index f687503..2d7e9ed 100644
> --- a/arch/arm/include/asm/arch-mx5/sys_proto.h
> +++ b/arch/arm/include/asm/arch-mx5/sys_proto.h
> @@ -27,5 +27,7 @@
>  u32 get_cpu_rev(void);
>  #define is_soc_rev(rev)	((get_cpu_rev() & 0xFF) - rev)
>  void sdelay(unsigned long);
> +void pmic_reg_write(u32 reg, u32 value);
> +u32 pmic_reg_read(u32 reg);

The pmic_ prototypes have nothing to do with the Soc prototype, as they
are specific for a driver. You should move them in the dialog header.

> +int board_init(void)
> +{
> +	gd->bd->bi_arch_number = MACH_TYPE_MX53_LOCO;
> +	gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
> +
> +#ifdef CONFIG_I2C_MXC
> +	setup_i2c(0);
> +	power_init();
> +	clock_init();
> +#endif

Probably it does not make a lot of sense to build this board without I2C
support. If this is the case, you should drop the #ifdef, as your board
must always be compiled with I2C.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V7 3/3] MX53: support for freescale MX53LOCO board

[Jason Hui]

Hi, Fabio,

On Wed, May 11, 2011 at 8:03 PM, Fabio Estevam <fabioestevam@yahoo.com> wrote:
> Hi Jason,
>
> --- On Wed, 5/11/11, Jason Liu <jason.hui@linaro.org> wrote:
> ...
>> diff --git a/arch/arm/cpu/armv7/mx5/soc.c
>> b/arch/arm/cpu/armv7/mx5/soc.c
>> index 6f4e8db..9c03474 100644
>> --- a/arch/arm/cpu/armv7/mx5/soc.c
>> +++ b/arch/arm/cpu/armv7/mx5/soc.c
> ...
>
> This file is not MX53Loco specific, so it should be part of another patch.
>
>> mxc_get_clock(MXC_ARM_CLK) / 1000000);
>> -??? printf("Reset cause: %s\n", get_reset_cause());
>> +??? printf("Reset? cause: %s\n", get_reset_cause());
>> ???? return 0;
>
> What is the purpose of this change?

Just make the print looks good. Nothing else. I can remove it.

>
>> index f687503..2d7e9ed 100644
>> --- a/arch/arm/include/asm/arch-mx5/sys_proto.h
>> +++ b/arch/arm/include/asm/arch-mx5/sys_proto.h
>
> This file is not MX53Loco specific, so it should be part of another patch.

Will move to another file as Stefano suggest.

Jason

>
> Regards,
>
> Fabio Estevam
>

[U-Boot] [PATCH V7 3/3] MX53: support for freescale MX53LOCO board

[Jason Hui]

Hi, Stefano,

On Wed, May 11, 2011 at 8:37 PM, Stefano Babic <sbabic@denx.de> wrote:
> On 05/11/2011 10:03 AM, Jason Liu wrote:
>> This patch add initial support for freescale MX53LOCO board.
>> Network(FEC),SD/MMC,UART have been supported by this patch
>>
>> The patch also config CPU:1GHZ,DDR:400MHZ for better peformance
>>
>> Signed-off-by: Jason Liu <jason.hui@linaro.org>
>
> Hi Jason,
>
>> diff --git a/arch/arm/cpu/armv7/mx5/soc.c b/arch/arm/cpu/armv7/mx5/soc.c
>> index 6f4e8db..9c03474 100644
>> --- a/arch/arm/cpu/armv7/mx5/soc.c
>> +++ b/arch/arm/cpu/armv7/mx5/soc.c
>> @@ -116,7 +116,7 @@ int print_cpuinfo(void)
>> ? ? ? ? ? ? ? (cpurev & 0x000F0) >> 4,
>> ? ? ? ? ? ? ? (cpurev & 0x0000F) >> 0,
>> ? ? ? ? ? ? ? mxc_get_clock(MXC_ARM_CLK) / 1000000);
>> - ? ? printf("Reset cause: %s\n", get_reset_cause());
>> + ? ? printf("Reset ?cause: %s\n", get_reset_cause());
>> ? ? ? return 0;
>
> It seems to me this file slips into your patch, but it was not supposed
> to be.

Yes, I will remove it.

>
>> ?}
>> ?#endif
>> diff --git a/arch/arm/include/asm/arch-mx5/sys_proto.h b/arch/arm/include/asm/arch-mx5/sys_proto.h
>> index f687503..2d7e9ed 100644
>> --- a/arch/arm/include/asm/arch-mx5/sys_proto.h
>> +++ b/arch/arm/include/asm/arch-mx5/sys_proto.h
>> @@ -27,5 +27,7 @@
>> ?u32 get_cpu_rev(void);
>> ?#define is_soc_rev(rev) ? ? ?((get_cpu_rev() & 0xFF) - rev)
>> ?void sdelay(unsigned long);
>> +void pmic_reg_write(u32 reg, u32 value);
>> +u32 pmic_reg_read(u32 reg);
>
> The pmic_ prototypes have nothing to do with the Soc prototype, as they
> are specific for a driver. You should move them in the dialog header.

OK,

>
>> +int board_init(void)
>> +{
>> + ? ? gd->bd->bi_arch_number = MACH_TYPE_MX53_LOCO;
>> + ? ? gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
>> +
>> +#ifdef CONFIG_I2C_MXC
>> + ? ? setup_i2c(0);
>> + ? ? power_init();
>> + ? ? clock_init();
>> +#endif
>
> Probably it does not make a lot of sense to build this board without I2C
> support. If this is the case, you should drop the #ifdef, as your board
> must always be compiled with I2C.

OK,

Jason

>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 ?Email: office at denx.de
> =====================================================================
>

[U-Boot] [PATCH V7 3/3] MX53: support for freescale MX53LOCO board

[Jason Liu]

Hi, Stefano,

2011/5/11 Stefano Babic <sbabic@denx.de>:
> On 05/11/2011 10:03 AM, Jason Liu wrote:
>> This patch add initial support for freescale MX53LOCO board.
>> Network(FEC),SD/MMC,UART have been supported by this patch
>>
>> The patch also config CPU:1GHZ,DDR:400MHZ for better peformance
>>
>> Signed-off-by: Jason Liu <jason.hui@linaro.org>
>
> Hi Jason,
>
...
>> ?u32 get_cpu_rev(void);
>> ?#define is_soc_rev(rev) ? ? ?((get_cpu_rev() & 0xFF) - rev)
>> ?void sdelay(unsigned long);
>> +void pmic_reg_write(u32 reg, u32 value);
>> +u32 pmic_reg_read(u32 reg);
>
> The pmic_ prototypes have nothing to do with the Soc prototype, as they
> are specific for a driver. You should move them in the dialog header.
>

I think I need create one head file named: include/dlg_pmic.h to include the
pmic_reg_write/read declaration and not just put the declaration to
da9053.h file, what's your idea?

Jason

>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 ?Email: office at denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH V7 3/3] MX53: support for freescale MX53LOCO board

[Stefano Babic]

On 05/12/2011 08:13 AM, Jason Liu wrote:
> Hi, Stefano,
> 

Hi Jason,

>>>  u32 get_cpu_rev(void);
>>>  #define is_soc_rev(rev)      ((get_cpu_rev() & 0xFF) - rev)
>>>  void sdelay(unsigned long);
>>> +void pmic_reg_write(u32 reg, u32 value);
>>> +u32 pmic_reg_read(u32 reg);
>>
>> The pmic_ prototypes have nothing to do with the Soc prototype, as they
>> are specific for a driver. You should move them in the dialog header.
>>
> 
> I think I need create one head file named: include/dlg_pmic.h to include the
> pmic_reg_write/read declaration and not just put the declaration to
> da9053.h file, what's your idea?

You are right, I have only concerns to add a new file only to put three
prototypes.... Anyway, it seems to me also a cleaner solution.

However, the name of file should be general, as the prototypes are used
for all PMICs using the SPI/I2C interfaces. Probably spi_i2c_pmic.h to
stay coherent with the driver name.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V7 2/3] PMIC: Add dialog pmic support

[Stefano Babic]

On 05/11/2011 10:03 AM, Jason Liu wrote:
> This patch add dialog pmic(DA9053) driver with I2C interface support
> In order to not duplicate code and according to the discussion on the
> mail-list, change fsl_pmic.c to spi_i2c_pmic.c.Actaully,spi_i2c_pmic.c
> is just a wrapper for PMIC communication when SPI or I2C is used.
> 
> Signed-off-by: Jason Liu <jason.hui@linaro.org>
> Cc: sbabic at denx.de <sbabic@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>

Hi Jason,

> --- a/drivers/misc/fsl_pmic.c
> +++ b/drivers/misc/spi_i2c_pmic.c
> @@ -22,13 +22,16 @@
>  
>  #include <config.h>
>  #include <common.h>
> +#include <i2c.h>
>  #include <asm/errno.h>
>  #include <linux/types.h>
> +#if defined(CONFIG_FSL_PMIC)
>  #include <fsl_pmic.h>
> +#endif
>  
> -static int check_param(u32 reg, u32 write)
> +static int check_param(u32 reg, u32 write, u32 max_reg)
>  {
> -	if (reg > 63 || write > 1) {
> +	if (reg > max_reg || write > 1) {
>  		printf("<reg num> = %d is invalid. Should be less then 63\n",
>  			reg);
>  		return -1;
> @@ -37,15 +40,13 @@ static int check_param(u32 reg, u32 write)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_FSL_PMIC_I2C
> -#include <i2c.h>
> -
> -u32 pmic_reg(u32 reg, u32 val, u32 write)
> +#if defined(CONFIG_FSL_PMIC_I2C)
> +u32 fsl_pmic_reg(u32 reg, u32 val, u32 write)
>  {
> -	unsigned char buf[4] = { 0 };
>  	u32 ret_val = 0;
> +	unsigned char buf[4] = { 0 };
>  
> -	if (check_param(reg, write))
> +	if (check_param(reg, write, 63))
>  		return -1;
>  
>  	if (write) {
> @@ -62,7 +63,44 @@ u32 pmic_reg(u32 reg, u32 val, u32 write)
>  
>  	return ret_val;
>  }
> -#else /* SPI interface */
> +#endif
> +
> +#if defined(CONFIG_DIALOG_PMIC_I2C)
> +u32 dlg_pmic_reg(u32 reg, u32 val, u32 write)
> +{
> +	u32 ret_val = 0;
> +	unsigned char buf[1] = { 0 };
> +
> +	if (check_param(reg, write, 142))
> +		return -1;
> +
> +	if (write) {
> +		buf[0] = (val) & 0xff;
> +		if (i2c_write(CONFIG_SYS_DIALOG_PMIC_I2C_ADDR, reg, 1, buf, 1))
> +			return -1;
> +	} else {
> +		if (i2c_read(CONFIG_SYS_DIALOG_PMIC_I2C_ADDR, reg, 1, buf, 1))
> +			return -1;
> +		ret_val = buf[0];
> +	}
> +
> +	return ret_val;

This is not what I meant. You have duplicated the code, instead of merge
the two functions together. And I think the switch is wrong.
This file provides a general access to PMIc using SPI/I2C. There should
not be #ifdef related to a single PMIC. Instead of that, You need
additional CONFIG_ to select how to access the PMIC (8 bit or 32 bit).

IMHO we can rid of the check_param() function, as this is a constraint
to have an implementation independent from a single PMIC.

I would prefer something like this:

u32 pmic_reg(u32 reg, u32 val, u32 write) {

	........
#ifdef CONFIG_SYS_PMIC_8BIT
	
	<read / write 8 bit register>
#else

	<actual implementation for fsl PMICs>
#endif


> +#if defined(CONFIG_FSL_PMIC_I2C) || defined(CONFIG_DIALOG_PMIC_I2C)

Same comments apply here. We should select only between I2C and SPI, not
the chip.

> +#if defined(CONFIG_FSL_PMIC)
> +#define PMIC_NAME "Freescale PMIC (Atlas)"
> +#elif defined(CONFIG_DIALOG_PMIC)
> +#define PMIC_NAME "Dialog PMIC (DA905x)"
> +#else
> +#error "Unkown PMIC name"
> +#endif

Instead of that, we can set a general name or put the PMIC_NAME inside
the specific PMIC header file.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V7 2/3] PMIC: Add dialog pmic support

[Jason Liu]

Hi, Stefano,

2011/5/12 Stefano Babic <sbabic@denx.de>:
> On 05/11/2011 10:03 AM, Jason Liu wrote:
>> This patch add dialog pmic(DA9053) driver with I2C interface support
>> In order to not duplicate code and according to the discussion on the
>> mail-list, change fsl_pmic.c to spi_i2c_pmic.c.Actaully,spi_i2c_pmic.c
>> is just a wrapper for PMIC communication when SPI or I2C is used.
>>
>> Signed-off-by: Jason Liu <jason.hui@linaro.org>
>> Cc: sbabic at denx.de <sbabic@denx.de>
>> Cc: Detlev Zundel <dzu@denx.de>
>
> Hi Jason,
>
>> --- a/drivers/misc/fsl_pmic.c
>> +++ b/drivers/misc/spi_i2c_pmic.c
>> @@ -22,13 +22,16 @@
>>
>> ?#include <config.h>
>> ?#include <common.h>
>> +#include <i2c.h>
>> ?#include <asm/errno.h>
>> ?#include <linux/types.h>
>> +#if defined(CONFIG_FSL_PMIC)
>> ?#include <fsl_pmic.h>
>> +#endif
>>
>> -static int check_param(u32 reg, u32 write)
>> +static int check_param(u32 reg, u32 write, u32 max_reg)
>> ?{
>> - ? ? if (reg > 63 || write > 1) {
>> + ? ? if (reg > max_reg || write > 1) {
>> ? ? ? ? ? ? ? printf("<reg num> = %d is invalid. Should be less then 63\n",
>> ? ? ? ? ? ? ? ? ? ? ? reg);
>> ? ? ? ? ? ? ? return -1;
>> @@ -37,15 +40,13 @@ static int check_param(u32 reg, u32 write)
>> ? ? ? return 0;
>> ?}
>>
>> -#ifdef CONFIG_FSL_PMIC_I2C
>> -#include <i2c.h>
>> -
>> -u32 pmic_reg(u32 reg, u32 val, u32 write)
>> +#if defined(CONFIG_FSL_PMIC_I2C)
>> +u32 fsl_pmic_reg(u32 reg, u32 val, u32 write)
>> ?{
>> - ? ? unsigned char buf[4] = { 0 };
>> ? ? ? u32 ret_val = 0;
>> + ? ? unsigned char buf[4] = { 0 };
>>
>> - ? ? if (check_param(reg, write))
>> + ? ? if (check_param(reg, write, 63))
>> ? ? ? ? ? ? ? return -1;
>>
>> ? ? ? if (write) {
>> @@ -62,7 +63,44 @@ u32 pmic_reg(u32 reg, u32 val, u32 write)
>>
>> ? ? ? return ret_val;
>> ?}
>> -#else /* SPI interface */
>> +#endif
>> +
>> +#if defined(CONFIG_DIALOG_PMIC_I2C)
>> +u32 dlg_pmic_reg(u32 reg, u32 val, u32 write)
>> +{
>> + ? ? u32 ret_val = 0;
>> + ? ? unsigned char buf[1] = { 0 };
>> +
>> + ? ? if (check_param(reg, write, 142))
>> + ? ? ? ? ? ? return -1;
>> +
>> + ? ? if (write) {
>> + ? ? ? ? ? ? buf[0] = (val) & 0xff;
>> + ? ? ? ? ? ? if (i2c_write(CONFIG_SYS_DIALOG_PMIC_I2C_ADDR, reg, 1, buf, 1))
>> + ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? } else {
>> + ? ? ? ? ? ? if (i2c_read(CONFIG_SYS_DIALOG_PMIC_I2C_ADDR, reg, 1, buf, 1))
>> + ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? ret_val = buf[0];
>> + ? ? }
>> +
>> + ? ? return ret_val;
>
> This is not what I meant. You have duplicated the code, instead of merge
> the two functions together. And I think the switch is wrong.
> This file provides a general access to PMIc using SPI/I2C. There should
> not be #ifdef related to a single PMIC. Instead of that, You need
> additional CONFIG_ to select how to access the PMIC (8 bit or 32 bit).

Then can we have the CONFIG_SYS_DIALOG_PMIC_I2C_ADDR or
CONFIG_SYS_FSL_PMIC_I2C_ADDR in this file?

Please tell clear about your mean, don't let me guess what you mean?

>
> IMHO we can rid of the check_param() function, as this is a constraint
> to have an implementation independent from a single PMIC.

If you think we don't need check_param, then It's ok. I can remove it.
Since this function is added by you before.

>
> I would prefer something like this:
>
> u32 pmic_reg(u32 reg, u32 val, u32 write) {
>
> ? ? ? ?........
> #ifdef CONFIG_SYS_PMIC_8BIT
>
> ? ? ? ?<read / write 8 bit register>
> #else
>
> ? ? ? ?<actual implementation for fsl PMICs>
> #endif

Then what you prefer is function pointer or something else?
Please tell clear, otherwise, it will cost another time to do it.

>
>
>> +#if defined(CONFIG_FSL_PMIC_I2C) || defined(CONFIG_DIALOG_PMIC_I2C)
>
> Same comments apply here. We should select only between I2C and SPI, not
> the chip.
>
>> +#if defined(CONFIG_FSL_PMIC)
>> +#define PMIC_NAME "Freescale PMIC (Atlas)"
>> +#elif defined(CONFIG_DIALOG_PMIC)
>> +#define PMIC_NAME "Dialog PMIC (DA905x)"
>> +#else
>> +#error "Unkown PMIC name"
>> +#endif
>
> Instead of that, we can set a general name or put the PMIC_NAME inside
> the specific PMIC header file.

Then what general name you think it's good?

I have seen the painful for restructure with the second time, another
is the make imximage.
Can we avoid it in the future?

And BTW, do you have any comments about the 1/3 clock patch? If you
have, please tell it
as early as possible and I don't want to let the patch version goes up
very bigger and the time endless.

Thanks,

Jason

>
> Best regards,
> Stefano
>
> --
> =====================================================================
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 ?Email: office at denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH V7 3/3] MX53: support for freescale MX53LOCO board

[Jason Hui]

Hi, Stefano,

On Thu, May 12, 2011 at 5:03 PM, Stefano Babic <sbabic@denx.de> wrote:
> On 05/12/2011 08:13 AM, Jason Liu wrote:
>> Hi, Stefano,
>>
>
> Hi Jason,
>
>>>> ?u32 get_cpu_rev(void);
>>>> ?#define is_soc_rev(rev) ? ? ?((get_cpu_rev() & 0xFF) - rev)
>>>> ?void sdelay(unsigned long);
>>>> +void pmic_reg_write(u32 reg, u32 value);
>>>> +u32 pmic_reg_read(u32 reg);
>>>
>>> The pmic_ prototypes have nothing to do with the Soc prototype, as they
>>> are specific for a driver. You should move them in the dialog header.
>>>
>>
>> I think I need create one head file named: include/dlg_pmic.h to include the
>> pmic_reg_write/read declaration and not just put the declaration to
>> da9053.h file, what's your idea?
>
> You are right, I have only concerns to add a new file only to put three
> prototypes.... Anyway, it seems to me also a cleaner solution.
>
> However, the name of file should be general, as the prototypes are used
> for all PMICs using the SPI/I2C interfaces. Probably spi_i2c_pmic.h to
> stay coherent with the driver name.

Then for more cleaner, I will remove the duplicated declaration from fsl_pmic.h

Jason

>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 ?Email: office at denx.de
> =====================================================================
>

[U-Boot] [PATCH V7 2/3] PMIC: Add dialog pmic support

[Stefano Babic]

On 05/13/2011 05:08 AM, Jason Liu wrote:
> Hi, Stefano,

Hi Jason,

>> This is not what I meant. You have duplicated the code, instead of merge
>> the two functions together. And I think the switch is wrong.
>> This file provides a general access to PMIc using SPI/I2C. There should
>> not be #ifdef related to a single PMIC. Instead of that, You need
>> additional CONFIG_ to select how to access the PMIC (8 bit or 32 bit).
> 
> Then can we have the CONFIG_SYS_DIALOG_PMIC_I2C_ADDR or
> CONFIG_SYS_FSL_PMIC_I2C_ADDR in this file?
> 
> Please tell clear about your mean, don't let me guess what you mean?

I thought it was already clear, but probably not enough. We do not need
special CONFIG_ to select the same attribute for different PMICs. What
is the functional difference between CONFIG_SYS_DIALOG_PMIC_I2C_ADDR and
CONFIG_SYS_FSL_PMIC_I2C_ADDR ? They set exactly the same thing. If we
add several other PMIC, should each of them have a separate CONFIG_ for
the I2C address ? Surely not.

We need only one of them. It seems to me you get confused due to the
original name, that contains the FSL_ string. But think at what this
driver provides: an API to access the PMICs using SPI or I2C, unrelated
to the specific chosen chip.

By the way, you can still use the old name CONFIG_SYS_FSL_PMIC_I2C_ADDR
for your patch. I understand that changing names here means also to
change other boards, that are not related to your patch.

I will then send a patch to reorganize the names after integrating your
patch in the tree.

> 
>>
>> IMHO we can rid of the check_param() function, as this is a constraint
>> to have an implementation independent from a single PMIC.
> 
> If you think we don't need check_param, then It's ok. I can remove it.
> Since this function is added by you before.

The function seemed ok when I write the first version, but it seems
overkilling now.

> 
>>
>> I would prefer something like this:
>>
>> u32 pmic_reg(u32 reg, u32 val, u32 write) {
>>
>>        ........
>> #ifdef CONFIG_SYS_PMIC_8BIT
>>
>>        <read / write 8 bit register>
>> #else
>>
>>        <actual implementation for fsl PMICs>
>> #endif
> 
> Then what you prefer is function pointer or something else?
> Please tell clear, otherwise, it will cost another time to do it.

To be more clearer: this driver should not have inside different
implementations based on a chip type, but based on the specific
*features* or options that are needed to implement. As you see in my
example, it should be possible to access to a PMIC with 8bit with your
implementation also with other PMICs. If we have to add several other
chips, the code (that is real simple) becomes easy unreadable.

As I said, do not change the CONFIG_ names in your patch, as this can
create some confusion, as I see. I will do it later.

>>> +#if defined(CONFIG_FSL_PMIC_I2C) || defined(CONFIG_DIALOG_PMIC_I2C)
>>
>> Same comments apply here. We should select only between I2C and SPI, not
>> the chip.
>>
>>> +#if defined(CONFIG_FSL_PMIC)
>>> +#define PMIC_NAME "Freescale PMIC (Atlas)"
>>> +#elif defined(CONFIG_DIALOG_PMIC)
>>> +#define PMIC_NAME "Dialog PMIC (DA905x)"
>>> +#else
>>> +#error "Unkown PMIC name"
>>> +#endif
>>
>> Instead of that, we can set a general name or put the PMIC_NAME inside
>> the specific PMIC header file.
> 
> Then what general name you think it's good?

It should be unrelated from the specific chip (if you chose this
solution), such as "SPI/I2C PMIC", or dropped from this file and put
into the PMIC header file if you select solution 2).

> 
> I have seen the painful for restructure with the second time, another
> is the make imximage.

If the code in a review is considered wrong, it must be changed. I
cannot avoid it.

> Can we avoid it in the future?

Check in the archive, and you see that changes were requested by several
reviewers an issue was discovered. And for the LOCO the patches were
removed after beeing accepted due to issues found later in the code,
that are considered unaccetable for mainline. I cannot foresee how many
reviews one patch requires, and how many issues a single patch raises.

On the other side, feel free to ask if something is not enough clear
*before* implementing. On my site, I will do my best to explain my
position, as all other reviewers in the list.

> 
> And BTW, do you have any comments about the 1/3 clock patch? If you
> have, please tell it
> as early as possible and I don't want to let the patch version goes up
> very bigger and the time endless.

The patch adds several functions that are strictly related to the
processor and I am checking with the reference manuals to understand
them. I need more time for it.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V7 2/3] PMIC: Add dialog pmic support

[Jason Liu]

Hi, Stefano,

2011/5/13 Stefano Babic <sbabic@denx.de>:
> On 05/13/2011 05:08 AM, Jason Liu wrote:
>> Hi, Stefano,
>
> Hi Jason,
>
[...]
>>
>> And BTW, do you have any comments about the 1/3 clock patch? If you
>> have, please tell it
>> as early as possible and I don't want to let the patch version goes up
>> very bigger and the time endless.
>
> The patch adds several functions that are strictly related to the
> processor and I am checking with the reference manuals to understand
> them. I need more time for it.

If that, can we consider that I just add the basic mx53loco support:
which exclude:

Dialog PMIC support, which is just useful to increase the CPU to 1GHZ,
by default voltage,
CPU can only run up to 800MHZ,

clock code(patch 1/3) , which is just useful to increase DDR run up to
400Mhz, by default it's 333Mhz.

I just don't want to break the basic mx53loco board support to
mainline for so-long time.

Please advise, thanks,

Jason

>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 ?Email: office at denx.de
> =====================================================================
>

[U-Boot] [PATCH V7 2/3] PMIC: Add dialog pmic support

[Stefano Babic]

On 05/13/2011 10:41 AM, Jason Liu wrote:

>> The patch adds several functions that are strictly related to the
>> processor and I am checking with the reference manuals to understand
>> them. I need more time for it.
> 
> If that, can we consider that I just add the basic mx53loco support:
> which exclude:
> 
> Dialog PMIC support, which is just useful to increase the CPU to 1GHZ,
> by default voltage,
> CPU can only run up to 800MHZ,
> 
> clock code(patch 1/3) , which is just useful to increase DDR run up to
> 400Mhz, by default it's 333Mhz.
> 
> I just don't want to break the basic mx53loco board support to
> mainline for so-long time.
> 
> Please advise, thanks,

If you prefer to proceed in this way, no problem at all, we can do it.
You can add support for clock/dialog later. In the meantime I will send
to you my feedback for the clock patch.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V7 2/3] PMIC: Add dialog pmic support

[Jason Hui]

Hi, Stefano

On Fri, May 13, 2011 at 4:47 PM, Stefano Babic <sbabic@denx.de> wrote:
> On 05/13/2011 10:41 AM, Jason Liu wrote:
>
>>> The patch adds several functions that are strictly related to the
>>> processor and I am checking with the reference manuals to understand
>>> them. I need more time for it.
>>
>> If that, can we consider that I just add the basic mx53loco support:
>> which exclude:
>>
>> Dialog PMIC support, which is just useful to increase the CPU to 1GHZ,
>> by default voltage,
>> CPU can only run up to 800MHZ,
>>
>> clock code(patch 1/3) , which is just useful to increase DDR run up to
>> 400Mhz, by default it's 333Mhz.
>>
>> I just don't want to break the basic mx53loco board support to
>> mainline for so-long time.
>>
>> Please advise, thanks,
>
> If you prefer to proceed in this way, no problem at all, we can do it.
> You can add support for clock/dialog later. In the meantime I will send
> to you my feedback for the clock patch.

OK, I will do it now.

>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 ?Email: office at denx.de
> =====================================================================
>

[U-Boot] [PATCH V7 1/3] MX5: clock: Add clock config interface

[Stefano Babic]

On 05/11/2011 10:03 AM, Jason Liu wrote:

> diff --git a/arch/arm/cpu/armv7/mx5/clock.c b/arch/arm/cpu/armv7/mx5/clock.c
> index 0b04a88..04d9f71 100644
> --- a/arch/arm/cpu/armv7/mx5/clock.c
> +++ b/arch/arm/cpu/armv7/mx5/clock.c
> @@ -24,6 +24,7 @@
>   */

>  
> +#define AHB_CLK_ROOT    133333333
> +#define SZ_DEC_1M       1000000

Is this define used only to get the value in Mhz from the PLL clock ? If
it is the case, the name is quite confusing, as it refers to a size. If
it is not the case, please explain.

> +#define PLL_PD_MAX      16      /* Actual pd+1 */
> +#define PLL_MFI_MAX     15
> +#define PLL_MFI_MIN     5
> +#define ARM_DIV_MAX     8
> +#define IPG_DIV_MAX     4
> +#define AHB_DIV_MAX     8
> +#define EMI_DIV_MAX     8
> +#define NFC_DIV_MAX     8

Definitions for clock registers are in the crm_regs.h file. These are
the maximum values for some fields in the registers. Should they not be
put inside the crm_regs.h ?

> +
> +struct fixed_pll_mfd {
> +	u32 ref_clk_hz;
> +	u32 mfd;
> +};
> +
> +const struct fixed_pll_mfd fixed_mfd[] = {
> +	{CONFIG_SYS_MX5_HCLK, 24 * 16},
> +};

Not understood the need of an array (I have not said it is wrong, simply
I have not understood !). You use in the code (this is another issue)
"ref" as parameter for your functions for the reference clock, but is
seems to me that the only possible value is CONFIG_SYS_MX5_HCLK.

Are there other use case for this array, that makes sense to define and
maybe to extend it ?

Can you add a reference to the manual explaining where these values are
coming from ?

> +
> +struct pll_param {
> +	u32 pd;
> +	u32 mfi;
> +	u32 mfn;
> +	u32 mfd;
> +};
> +
> +#define PLL_FREQ_MAX(ref_clk)  (4 * (ref_clk) * PLL_MFI_MAX)
> +#define PLL_FREQ_MIN(ref_clk) \
> +		((2 * (ref_clk) * (PLL_MFI_MIN - 1)) / PLL_PD_MAX)

I understand what it is done here, but only after I have finally found
where it is described in the manual. Can you add useful comments here
and reference to the manual, too ? Such as describing these values are
for the registers DP_OP, DP_MFN and DP-MFD, and a reference to the
formula (Eqn. 22-1) helps to understand it.

> +#define MAX_DDR_CLK     420000000
> +#define NFC_CLK_MAX     34000000

Where do these values come from ? I understand they are computed values,
depending on pll clock. It seems to me (at least for DDR clock) they are
absolute maximum rates, but it could be that MAX_DDR_CLK could be set to
a lower value depending on the PLL value. Is it correct ? In other words
: should be possible to set a PLL (you provide an API for changing it)
to a lower value, and then even the defines you set here do not
correspond to the real maximum value ?

> +
>  /*
>   * The API of get mxc clockes.
>   */
> @@ -245,10 +369,12 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
>  	case MXC_UART_CLK:
>  		return get_uart_clk();
>  	case MXC_CSPI_CLK:
> -		return imx_get_cspiclk();
> +		return get_cspi_clk();
>  	case MXC_FEC_CLK:
>  		return decode_pll(mxc_plls[PLL1_CLOCK],
>  				    CONFIG_SYS_MX5_HCLK);
> +	case MXC_DDR_CLK:
> +		return get_ddr_clk();

You extended the enum for the clocks, as I see also MXC_NFC_CLK. You
should add the MXC_NFC_CLK case, too.

Is it worth to export the other getter functions, too (get_axi_a/b_clk,
get_ahb_clk) ?

>  /*
> + * Clock config code start here
> + */
> +
> +/* precondition: m>0 and n>0.  Let g=gcd(m,n). */
> +static int gcd(int m, int n)
> +{
> +	int t;
> +	while (m > 0) {
> +		if (n > m) {
> +			t = m;
> +			m = n;
> +			n = t;
> +		} /* swap */
> +		m -= n;
> +	}
> +	return n;
> +}

This function has nothing to do with MX5 code. This is a general
function, and should be add to lib/. I think you have to remove it from
here and put it in a separate patch.

Add a comment to explain you are computing the greatest common divisor.
Why do you not have taken the implementation from linux ? It uses the
Euclid method (using a reminder) and it takes less iterations to get the
result as this implementation.

> +
> +/*
> + * This is to calculate various parameters based on reference clock and
> + * targeted clock based on the equation:
> + *      t_clk = 2*ref_freq*(mfi + mfn/(mfd+1))/(pd+1)

Where does this formula come from ?

> + * This calculation is based on a fixed MFD value for simplicity.
> + *
> + * @param ref       reference clock freq in Hz
> + * @param target    targeted clock in Hz
> + * @param pll       pll_param structure.
> + *
> + * @return          0 if successful; non-zero otherwise.
> + */
> +static int calc_pll_params(u32 ref, u32 target, struct pll_param *pll)
> +{
> +	u64 pd, mfi = 1, mfn, mfd, t1;
> +	u32 n_target = target;
> +	u32 n_ref = ref, i;

In your code you pass always a struct pll_param *pll previously filled
with zeroes. You can at least move the memset inside this function, if
it is used by all callers.

> +	for (i = 0; i < ARRAY_SIZE(fixed_mfd); i++) {
> +		if (fixed_mfd[i].ref_clk_hz == ref) {
> +			mfd = fixed_mfd[i].mfd;
> +			break;
> +		}
> +	}
> +
> +	if (i == ARRAY_SIZE(fixed_mfd))
> +		return -1;
> +
> +	/* Use n_target and n_ref to avoid overflow */
> +	for (pd = 1; pd <= PLL_PD_MAX; pd++) {
> +		t1 = n_target * pd;
> +		do_div(t1, (4 * n_ref));
> +		mfi = t1;
> +		if (mfi > PLL_MFI_MAX)
> +			return -1;
> +		else if (mfi < 5)
> +			continue;
> +		break;
> +	}
> +	/* Now got pd and mfi already */
> +	/*
> +	mfn = (((n_target * pd) / 4 - n_ref * mfi) * mfd) / n_ref;
> +	*/

Wrong multiline comment. Can you explain to me (or telling where is
described) this formula ?


> +#ifdef CMD_CLOCK_DEBUG

Use debug() instead of this. Do not add further personal DEBUG_* macro.

> +#define calc_div(tgt_clk, src_clk, limit) ({		\
> +		u32 v = 0;				\
> +		if (((src_clk) % (tgt_clk)) <= 100)	\
> +			v = (src_clk) / (tgt_clk);	\
> +		else					\
> +			v = ((src_clk) / (tgt_clk)) + 1;\
> +		if (v > limit)				\
> +			v = limit;			\
> +		(v - 1);				\
> +	})

Please move the macro at the beginning of the file. I do not know if
there are some rules in u-boot to returning values from macro as you do
here. My personal taste is to use a function in this case.

> +
> +#define CHANGE_PLL_SETTINGS(pll, pd, fi, fn, fd) \
> +	{	\
> +		__raw_writel(0x1232, &pll->ctrl);		\
> +		__raw_writel(0x2, &pll->config);		\
> +		__raw_writel((((pd) - 1) << 0) | ((fi) << 4),	\
> +			&pll->op);				\
> +		__raw_writel(fn, &(pll->mfn));			\
> +		__raw_writel((fd) - 1, &pll->mfd);		\
> +		__raw_writel((((pd) - 1) << 0) | ((fi) << 4),	\
> +			&pll->hfs_op);				\
> +		__raw_writel(fn, &pll->hfs_mfn);		\
> +		__raw_writel((fd) - 1, &pll->hfs_mfd);		\
> +		__raw_writel(0x1232, &pll->ctrl);		\
> +		while (!__raw_readl(&pll->ctrl) & 0x1)		\
> +			;\
> +	}

I have tried to understand, I give up now. Can you please add useful
comments for this part and explain also what the magic number 0x1232 is
supposed to do. Is there a part in the manual explaining this ? Can you
add in the comment ?
And move the macro at the beginning of the file.

> +
> +static int config_pll_clk(enum pll_clocks index, struct pll_param *pll_param)
> +{
> +	u32 ccsr = __raw_readl(&mxc_ccm->ccsr);
> +	struct mxc_pll_reg *pll = mxc_plls[index];
> +
> +	switch (index) {
> +	case PLL1_CLOCK:
> +		/* Switch ARM to PLL2 clock */
> +		__raw_writel(ccsr | 0x4, &mxc_ccm->ccsr);
> +		CHANGE_PLL_SETTINGS(pll, pll_param->pd,
> +					pll_param->mfi, pll_param->mfn,
> +					pll_param->mfd);
> +		/* Switch back */
> +		__raw_writel(ccsr & ~0x4, &mxc_ccm->ccsr);

Offsets and fields for CCM module are defined in crm_regs.h. I see there
are no definition for the Clcock Switch Register (ccsr). Please add them
instead of fix constants. The same in the rest of the function.

It make also easier to understand that your code switches clocks to
another PLL before changing frequency.

> +/* Config CPU clock */
> +static int config_core_clk(u32 ref, u32 freq)
> +{
> +	int ret = 0;
> +	struct pll_param pll_param;

See my comments for the ref parameter later. is it correct or must be
always CONFIG_SYS_MX5_HCLK ?

> +
> +/* Config main_bus_clock for periphs */
> +static int config_periph_clk(u32 ref, u32 freq)
> +{
> +	int ret = 0;
> +	struct pll_param pll_param;
> +
> +	memset(&pll_param, 0, sizeof(struct pll_param));
> +
> +	if (__raw_readl(&mxc_ccm->cbcdr) & MXC_CCM_CBCDR_PERIPH_CLK_SEL) {
> +		ret = calc_pll_params(ref, freq, &pll_param);
> +		if (ret != 0) {
> +			printf("Error:Can't find pll parameters: %d\n",
> +				ret);
> +			return ret;
> +		}
> +		switch ((__raw_readl(&mxc_ccm->cbcmr) & \
> +			MXC_CCM_CBCMR_PERIPH_CLK_SEL_MASK) >> \
> +			MXC_CCM_CBCMR_PERIPH_CLK_SEL_OFFSET) {
> +		case 0:
> +			return config_pll_clk(PLL1_CLOCK, &pll_param);
> +			break;
> +		case 1:
> +			return config_pll_clk(PLL3_CLOCK, &pll_param);
> +			break;
> +		default:
> +			return -1;
> +		}
> +	} else {
> +		u32 old_cbcmr = __raw_readl(&mxc_ccm->cbcmr);
> +		u32 new_cbcdr = calc_per_cbcdr_val(freq);
> +		u32 old_nfc = get_nfc_clk();
> +
> +		/* Switch peripheral to PLL3 */
> +		__raw_writel(0x00015154, &mxc_ccm->cbcmr);
> +		__raw_writel(0x02888945, &mxc_ccm->cbcdr);

Use defines to set registers. Or read-modify-write, as you changed only
PERIPH_APM_SEL from the reset value. However, you reset to 0
DDR_CLK_SEL.  I see you switch values back at the end of the function,
but my doubt is if it is correct to set in this transition DDR_CLK_SEL
always to zero. Can it work with all boards or there are configurations
where it does not work ?

> +
> +static int config_ddr_clk(u32 emi_clk)
> +{
> +	u32 clk_src;
> +	s32 shift = 0, clk_sel, div = 1;
> +	u32 cbcmr = __raw_readl(&mxc_ccm->cbcmr);
> +	u32 cbcdr = __raw_readl(&mxc_ccm->cbcdr);
> +
> +	if (emi_clk > MAX_DDR_CLK) {
> +		printf("Warning:DDR clock should not exceed %d MHz\n",
> +			MAX_DDR_CLK / SZ_DEC_1M);
> +		emi_clk = MAX_DDR_CLK;
> +	}
> +
> +	clk_src = get_periph_clk();
> +	/* Find DDR clock input */
> +	clk_sel = (cbcmr >> 10) & 0x3;

Add #defines for this. This must be fixed globally.

> +	__raw_writel(0x0, &mxc_ccm->ccdr);
> +
> +	return 0;
> +}
> +
> +/*!

Drop the ! from comment

> + * This function assumes the expected core clock has to be changed by
> + * modifying the PLL. This is NOT true always but for most of the times,
> + * it is. So it assumes the PLL output freq is the same as the expected
> + * core clock (presc=1) unless the core clock is less than PLL_FREQ_MIN.
> + * In the latter case, it will try to increase the presc value until
> + * (presc*core_clk) is greater than PLL_FREQ_MIN. It then makes call to
> + * calc_pll_params() and obtains the values of PD, MFI,MFN, MFD based
> + * on the targeted PLL and reference input clock to the PLL. Lastly,
> + * it sets the register based on these values along with the dividers.
> + * Note 1) There is no value checking for the passed-in divider values
> + *         so the caller has to make sure those values are sensible.
> + *      2) Also adjust the NFC divider such that the NFC clock doesn't
> + *         exceed NFC_CLK_MAX.
> + *      3) IPU HSP clock is independent of AHB clock. Even it can go up to
> + *         177MHz for higher voltage, this function fixes the max to 133MHz.
> + *      4) This function should not have allowed diag_printf() calls since
> + *         the serial driver has been stoped. But leave then here to allow
> + *         easy debugging by NOT calling the cyg_hal_plf_serial_stop().

???

The code is clear taken from another context. Which is diag_printf ?
Does point 4 apply here ?

> + *
> + * @param ref   pll input reference clock (24MHz)

Reference clock is defined as CONFIG_SYS_MX5_HCLK. Is it really possible
to pass a parameter to this function different as CONFIG_SYS_MX5_HCLK ?
What happens if CONFIG_SYS_MX5_HCLK and the ref parameter in this
function have two different values ? It seems to me this is not a use
case, and then we must avoid it. If I am right, you should drop the ref
parameter and use CONFIG_SYS_MX5_HCLK.

> + * @param freq  core clock in Hz

If the freq is in Hz, why do you multiply it for SZ_DEC_1M ?
It seems to me that freq is in Mhz, not Hz. Please be consistent.

> + * @param clk   clock type, e.g CPU_CLK, DDR_CLK, etc.
> + * @return      0 if successful; non-zero otherwise
> + */
> +int mxc_set_clock(u32 ref, u32 freq, enum mxc_clock clk)
> +{
> +	freq *= SZ_DEC_1M;
> +
> +	switch (clk) {
> +	case MXC_ARM_CLK:
> +		if (config_core_clk(ref, freq))
> +			return -1;
> +		break;
> +	case MXC_PERIPH_CLK:
> +		if (config_periph_clk(ref, freq))
> +			return -1;
> +		break;
> +	case MXC_DDR_CLK:
> +		if (config_ddr_clk(freq))
> +			return -1;
> +		break;
> +	case MXC_NFC_CLK:
> +		if (config_nfc_clk(freq))
> +			return -1;
> +		break;
> +	default:
> +		printf("Warning:Unsupported or invalid clock type\n");

This is the error case, you must not return 0 as you do.

> +/*
>   * Dump some core clockes.
>   */
>  int do_mx5_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> @@ -281,6 +825,7 @@ int do_mx5_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	printf("pll3: %dMHz\n", freq / 1000000);
>  	printf("ipg clock     : %dHz\n", mxc_get_clock(MXC_IPG_CLK));
>  	printf("ipg per clock : %dHz\n", mxc_get_clock(MXC_IPG_PERCLK));
> +	printf("ddr clock     : %dHz\n", mxc_get_clock(MXC_DDR_CLK));

What about the other clocks you added in your patch (MXC_NFC_CLK /
MXC_PERIPH_CLK) ?

>  /* Define the bits in register CBCDR */
> +#define MXC_CCM_CBCDR_DDR_HIFREQ_SEL		(0x1 << 30)
> +#define MXC_CCM_CBCDR_DDR_PODF_MASK		(0x7 << 27)
> +#define MXC_CCM_CBCDR_DDR_PODF_OFFSET		27
>  #define MXC_CCM_CBCDR_EMI_CLK_SEL		(0x1 << 26)
>  #define MXC_CCM_CBCDR_PERIPH_CLK_SEL		(0x1 << 25)
>  #define MXC_CCM_CBCDR_EMI_PODF_OFFSET		22

Agree with these changes in this file. Please extend this file with the
missing constants you use in your code.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V7 1/3] MX5: clock: Add clock config interface

[Jason Liu]

Hi, Stefano,

2011/5/16 Stefano Babic <sbabic@denx.de>:
> On 05/11/2011 10:03 AM, Jason Liu wrote:
>
>> diff --git a/arch/arm/cpu/armv7/mx5/clock.c b/arch/arm/cpu/armv7/mx5/clock.c
>> index 0b04a88..04d9f71 100644
>> --- a/arch/arm/cpu/armv7/mx5/clock.c
>> +++ b/arch/arm/cpu/armv7/mx5/clock.c
>> @@ -24,6 +24,7 @@
>> ? */
>
>>
>> +#define AHB_CLK_ROOT ? ?133333333
>> +#define SZ_DEC_1M ? ? ? 1000000
>
> Is this define used only to get the value in Mhz from the PLL clock ? If
> it is the case, the name is quite confusing, as it refers to a size. If
> it is not the case, please explain.

I will get rid of the definition for AHB_CLK_ROOT and it's no need to
define here
since it only use once in the code.

I will rename the SZ_DEC_1M or get rid of it also.

>
>> +#define PLL_PD_MAX ? ? ?16 ? ? ?/* Actual pd+1 */
>> +#define PLL_MFI_MAX ? ? 15
>> +#define PLL_MFI_MIN ? ? 5
>> +#define ARM_DIV_MAX ? ? 8
>> +#define IPG_DIV_MAX ? ? 4
>> +#define AHB_DIV_MAX ? ? 8
>> +#define EMI_DIV_MAX ? ? 8
>> +#define NFC_DIV_MAX ? ? 8
>
> Definitions for clock registers are in the crm_regs.h file. These are
> the maximum values for some fields in the registers. Should they not be
> put inside the crm_regs.h ?

Yes, make sense, I will put it to crm_regs.h file.

>
>> +
>> +struct fixed_pll_mfd {
>> + ? ? u32 ref_clk_hz;
>> + ? ? u32 mfd;
>> +};
>> +
>> +const struct fixed_pll_mfd fixed_mfd[] = {
>> + ? ? {CONFIG_SYS_MX5_HCLK, 24 * 16},
>> +};
>
> Not understood the need of an array (I have not said it is wrong, simply
> I have not understood !). You use in the code (this is another issue)
> "ref" as parameter for your functions for the reference clock, but is
> seems to me that the only possible value is CONFIG_SYS_MX5_HCLK.
>

I use array here just for the case we will have another element in the array
(currently we only have one) in the future. Currently, we use 24MHz as ref.


> Are there other use case for this array, that makes sense to define and
> maybe to extend it ?

Just in case the PLL ref clock is not from 24MHZ.

>
> Can you add a reference to the manual explaining where these values are
> coming from ?

Do you mean the value 24 * 16? If yes, it's just for the simpler calculation.

>
>> +
>> +struct pll_param {
>> + ? ? u32 pd;
>> + ? ? u32 mfi;
>> + ? ? u32 mfn;
>> + ? ? u32 mfd;
>> +};
>> +
>> +#define PLL_FREQ_MAX(ref_clk) ?(4 * (ref_clk) * PLL_MFI_MAX)
>> +#define PLL_FREQ_MIN(ref_clk) \
>> + ? ? ? ? ? ? ((2 * (ref_clk) * (PLL_MFI_MIN - 1)) / PLL_PD_MAX)
>
> I understand what it is done here, but only after I have finally found
> where it is described in the manual. Can you add useful comments here
> and reference to the manual, too ? Such as describing these values are
> for the registers DP_OP, DP_MFN and DP-MFD, and a reference to the
> formula (Eqn. 22-1) helps to understand it.

OK,  I will add the comments here.

>
>> +#define MAX_DDR_CLK ? ? 420000000
>> +#define NFC_CLK_MAX ? ? 34000000
>
> Where do these values come from ? I understand they are computed values,
> depending on pll clock. It seems to me (at least for DDR clock) they are
> absolute maximum rates, but it could be that MAX_DDR_CLK could be set to
> a lower value depending on the PLL value. Is it correct ? In other words
> : should be possible to set a PLL (you provide an API for changing it)
> to a lower value, and then even the defines you set here do not
> correspond to the real maximum value ?

Yes, this is the absolute maximum rate. When you clk_api to config
clock, it should not
exceed the max value, for example,

if (emi_clk > MAX_DDR_CLK) {
                printf("DDR clock should be less than"
                        "%d MHz, assuming max value \n",
...


>
>> +
>> ?/*
>> ? * The API of get mxc clockes.
>> ? */
>> @@ -245,10 +369,12 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
>> ? ? ? case MXC_UART_CLK:
>> ? ? ? ? ? ? ? return get_uart_clk();
>> ? ? ? case MXC_CSPI_CLK:
>> - ? ? ? ? ? ? return imx_get_cspiclk();
>> + ? ? ? ? ? ? return get_cspi_clk();
>> ? ? ? case MXC_FEC_CLK:
>> ? ? ? ? ? ? ? return decode_pll(mxc_plls[PLL1_CLOCK],
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? CONFIG_SYS_MX5_HCLK);
>> + ? ? case MXC_DDR_CLK:
>> + ? ? ? ? ? ? return get_ddr_clk();
>
> You extended the enum for the clocks, as I see also MXC_NFC_CLK. You
> should add the MXC_NFC_CLK case, too.

OK,  I will add it.

>
> Is it worth to export the other getter functions, too (get_axi_a/b_clk,
> get_ahb_clk) ?

Yes, I think it's valuable to export it and print it, thus user can
know clearly what
the axi_a/b and ahb clk rate are.

>
>> ?/*
>> + * Clock config code start here
>> + */
>> +
>> +/* precondition: m>0 and n>0. ?Let g=gcd(m,n). */
>> +static int gcd(int m, int n)
>> +{
>> + ? ? int t;
>> + ? ? while (m > 0) {
>> + ? ? ? ? ? ? if (n > m) {
>> + ? ? ? ? ? ? ? ? ? ? t = m;
>> + ? ? ? ? ? ? ? ? ? ? m = n;
>> + ? ? ? ? ? ? ? ? ? ? n = t;
>> + ? ? ? ? ? ? } /* swap */
>> + ? ? ? ? ? ? m -= n;
>> + ? ? }
>> + ? ? return n;
>> +}
>
> This function has nothing to do with MX5 code. This is a general
> function, and should be add to lib/. I think you have to remove it from
> here and put it in a separate patch.

I don't think it should be put to lib since only the mx5 clock code use it here.


>
> Add a comment to explain you are computing the greatest common divisor.
> Why do you not have taken the implementation from linux ? It uses the
> Euclid method (using a reminder) and it takes less iterations to get the
> result as this implementation.

OK, I will check linux implementation. Do you know does u-boot also has such
kind of function already, I did not find it?

>
>> +
>> +/*
>> + * This is to calculate various parameters based on reference clock and
>> + * targeted clock based on the equation:
>> + * ? ? ?t_clk = 2*ref_freq*(mfi + mfn/(mfd+1))/(pd+1)
>
> Where does this formula come from ?

The comments is not correct, I will fix it, it should be 4*ref_freq
not 2 * ref_req.
The reference manual: 12.2.2.3 DPLL Operation Register, elvis user guide,
Registers DP_OP, DP_MFN, and DP_MFD calculate the output frequency by
the formula:

The chapter number will not the same as you, but you still can find it
in the reference manual.

>
>> + * This calculation is based on a fixed MFD value for simplicity.
>> + *
>> + * @param ref ? ? ? reference clock freq in Hz
>> + * @param target ? ?targeted clock in Hz
>> + * @param pll ? ? ? pll_param structure.
>> + *
>> + * @return ? ? ? ? ?0 if successful; non-zero otherwise.
>> + */
>> +static int calc_pll_params(u32 ref, u32 target, struct pll_param *pll)
>> +{
>> + ? ? u64 pd, mfi = 1, mfn, mfd, t1;
>> + ? ? u32 n_target = target;
>> + ? ? u32 n_ref = ref, i;
>
> In your code you pass always a struct pll_param *pll previously filled
> with zeroes. You can at least move the memset inside this function, if
> it is used by all callers.

Yes, good point.

>
>> + ? ? for (i = 0; i < ARRAY_SIZE(fixed_mfd); i++) {
>> + ? ? ? ? ? ? if (fixed_mfd[i].ref_clk_hz == ref) {
>> + ? ? ? ? ? ? ? ? ? ? mfd = fixed_mfd[i].mfd;
>> + ? ? ? ? ? ? ? ? ? ? break;
>> + ? ? ? ? ? ? }
>> + ? ? }
>> +
>> + ? ? if (i == ARRAY_SIZE(fixed_mfd))
>> + ? ? ? ? ? ? return -1;
>> +
>> + ? ? /* Use n_target and n_ref to avoid overflow */
>> + ? ? for (pd = 1; pd <= PLL_PD_MAX; pd++) {
>> + ? ? ? ? ? ? t1 = n_target * pd;
>> + ? ? ? ? ? ? do_div(t1, (4 * n_ref));
>> + ? ? ? ? ? ? mfi = t1;
>> + ? ? ? ? ? ? if (mfi > PLL_MFI_MAX)
>> + ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? else if (mfi < 5)
>> + ? ? ? ? ? ? ? ? ? ? continue;
>> + ? ? ? ? ? ? break;
>> + ? ? }
>> + ? ? /* Now got pd and mfi already */
>> + ? ? /*
>> + ? ? mfn = (((n_target * pd) / 4 - n_ref * mfi) * mfd) / n_ref;
>> + ? ? */
>
> Wrong multiline comment. Can you explain to me (or telling where is
> described) this formula ?

Yes, I will fix the multi-line comments. As for the formula, it was based on
the the reference manual: 12.2.2.3 DPLL Operation Register, elvis user guide,
Registers DP_OP, DP_MFN, and DP_MFD calculate the output frequency by
the formula:

>
>
>> +#ifdef CMD_CLOCK_DEBUG
>
> Use debug() instead of this. Do not add further personal DEBUG_* macro.
>
>> +#define calc_div(tgt_clk, src_clk, limit) ({ ? ? ? ? \
>> + ? ? ? ? ? ? u32 v = 0; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? if (((src_clk) % (tgt_clk)) <= 100) ? ? \
>> + ? ? ? ? ? ? ? ? ? ? v = (src_clk) / (tgt_clk); ? ? ?\
>> + ? ? ? ? ? ? else ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? ? ? ? ? v = ((src_clk) / (tgt_clk)) + 1;\
>> + ? ? ? ? ? ? if (v > limit) ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? ? ? ? ? v = limit; ? ? ? ? ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? (v - 1); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> + ? ? })
>
> Please move the macro at the beginning of the file. I do not know if
> there are some rules in u-boot to returning values from macro as you do
> here. My personal taste is to use a function in this case.

OK,  I will move it.

>
>> +
>> +#define CHANGE_PLL_SETTINGS(pll, pd, fi, fn, fd) \
>> + ? ? { ? ? ? \
>> + ? ? ? ? ? ? __raw_writel(0x1232, &pll->ctrl); ? ? ? ? ? ? ? \
>> + ? ? ? ? ? ? __raw_writel(0x2, &pll->config); ? ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? __raw_writel((((pd) - 1) << 0) | ((fi) << 4), ? \
>> + ? ? ? ? ? ? ? ? ? ? &pll->op); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? __raw_writel(fn, &(pll->mfn)); ? ? ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? __raw_writel((fd) - 1, &pll->mfd); ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? __raw_writel((((pd) - 1) << 0) | ((fi) << 4), ? \
>> + ? ? ? ? ? ? ? ? ? ? &pll->hfs_op); ? ? ? ? ? ? ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? __raw_writel(fn, &pll->hfs_mfn); ? ? ? ? ? ? ? ?\
>> + ? ? ? ? ? ? __raw_writel((fd) - 1, &pll->hfs_mfd); ? ? ? ? ?\
>> + ? ? ? ? ? ? __raw_writel(0x1232, &pll->ctrl); ? ? ? ? ? ? ? \
>> + ? ? ? ? ? ? while (!__raw_readl(&pll->ctrl) & 0x1) ? ? ? ? ?\
>> + ? ? ? ? ? ? ? ? ? ? ;\
>> + ? ? }
>
> I have tried to understand, I give up now. Can you please add useful
> comments for this part and explain also what the magic number 0x1232 is
> supposed to do. Is there a part in the manual explaining this ? Can you
> add in the comment ?
> And move the macro at the beginning of the file.

Reference manual tell it clear and you need read the 12.2.2.1 DPLL
Control Register
but you reminder me to put some macro definition for the pain of reading.

>
>> +
>> +static int config_pll_clk(enum pll_clocks index, struct pll_param *pll_param)
>> +{
>> + ? ? u32 ccsr = __raw_readl(&mxc_ccm->ccsr);
>> + ? ? struct mxc_pll_reg *pll = mxc_plls[index];
>> +
>> + ? ? switch (index) {
>> + ? ? case PLL1_CLOCK:
>> + ? ? ? ? ? ? /* Switch ARM to PLL2 clock */
>> + ? ? ? ? ? ? __raw_writel(ccsr | 0x4, &mxc_ccm->ccsr);
>> + ? ? ? ? ? ? CHANGE_PLL_SETTINGS(pll, pll_param->pd,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pll_param->mfi, pll_param->mfn,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pll_param->mfd);
>> + ? ? ? ? ? ? /* Switch back */
>> + ? ? ? ? ? ? __raw_writel(ccsr & ~0x4, &mxc_ccm->ccsr);
>
> Offsets and fields for CCM module are defined in crm_regs.h. I see there
> are no definition for the Clcock Switch Register (ccsr). Please add them
> instead of fix constants. The same in the rest of the function.

OK,

>
> It make also easier to understand that your code switches clocks to
> another PLL before changing frequency.
>
>> +/* Config CPU clock */
>> +static int config_core_clk(u32 ref, u32 freq)
>> +{
>> + ? ? int ret = 0;
>> + ? ? struct pll_param pll_param;
>
> See my comments for the ref parameter later. is it correct or must be
> always CONFIG_SYS_MX5_HCLK ?
>
>> +
>> +/* Config main_bus_clock for periphs */
>> +static int config_periph_clk(u32 ref, u32 freq)
>> +{
>> + ? ? int ret = 0;
>> + ? ? struct pll_param pll_param;
>> +
>> + ? ? memset(&pll_param, 0, sizeof(struct pll_param));
>> +
>> + ? ? if (__raw_readl(&mxc_ccm->cbcdr) & MXC_CCM_CBCDR_PERIPH_CLK_SEL) {
>> + ? ? ? ? ? ? ret = calc_pll_params(ref, freq, &pll_param);
>> + ? ? ? ? ? ? if (ret != 0) {
>> + ? ? ? ? ? ? ? ? ? ? printf("Error:Can't find pll parameters: %d\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ret);
>> + ? ? ? ? ? ? ? ? ? ? return ret;
>> + ? ? ? ? ? ? }
>> + ? ? ? ? ? ? switch ((__raw_readl(&mxc_ccm->cbcmr) & \
>> + ? ? ? ? ? ? ? ? ? ? MXC_CCM_CBCMR_PERIPH_CLK_SEL_MASK) >> \
>> + ? ? ? ? ? ? ? ? ? ? MXC_CCM_CBCMR_PERIPH_CLK_SEL_OFFSET) {
>> + ? ? ? ? ? ? case 0:
>> + ? ? ? ? ? ? ? ? ? ? return config_pll_clk(PLL1_CLOCK, &pll_param);
>> + ? ? ? ? ? ? ? ? ? ? break;
>> + ? ? ? ? ? ? case 1:
>> + ? ? ? ? ? ? ? ? ? ? return config_pll_clk(PLL3_CLOCK, &pll_param);
>> + ? ? ? ? ? ? ? ? ? ? break;
>> + ? ? ? ? ? ? default:
>> + ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? }
>> + ? ? } else {
>> + ? ? ? ? ? ? u32 old_cbcmr = __raw_readl(&mxc_ccm->cbcmr);
>> + ? ? ? ? ? ? u32 new_cbcdr = calc_per_cbcdr_val(freq);
>> + ? ? ? ? ? ? u32 old_nfc = get_nfc_clk();
>> +
>> + ? ? ? ? ? ? /* Switch peripheral to PLL3 */
>> + ? ? ? ? ? ? __raw_writel(0x00015154, &mxc_ccm->cbcmr);
>> + ? ? ? ? ? ? __raw_writel(0x02888945, &mxc_ccm->cbcdr);
>
> Use defines to set registers. Or read-modify-write, as you changed only
> PERIPH_APM_SEL from the reset value. However, you reset to 0
> DDR_CLK_SEL. ?I see you switch values back at the end of the function,
> but my doubt is if it is correct to set in this transition DDR_CLK_SEL
> always to zero. Can it work with all boards or there are configurations
> where it does not work ?

I think it should work since DDR_CLK_SEL:00 derive clock from axi a
is the default value when boot up.

>
>> +
>> +static int config_ddr_clk(u32 emi_clk)
>> +{
>> + ? ? u32 clk_src;
>> + ? ? s32 shift = 0, clk_sel, div = 1;
>> + ? ? u32 cbcmr = __raw_readl(&mxc_ccm->cbcmr);
>> + ? ? u32 cbcdr = __raw_readl(&mxc_ccm->cbcdr);
>> +
>> + ? ? if (emi_clk > MAX_DDR_CLK) {
>> + ? ? ? ? ? ? printf("Warning:DDR clock should not exceed %d MHz\n",
>> + ? ? ? ? ? ? ? ? ? ? MAX_DDR_CLK / SZ_DEC_1M);
>> + ? ? ? ? ? ? emi_clk = MAX_DDR_CLK;
>> + ? ? }
>> +
>> + ? ? clk_src = get_periph_clk();
>> + ? ? /* Find DDR clock input */
>> + ? ? clk_sel = (cbcmr >> 10) & 0x3;
>
> Add #defines for this. This must be fixed globally.
>
>> + ? ? __raw_writel(0x0, &mxc_ccm->ccdr);
>> +
>> + ? ? return 0;
>> +}
>> +
>> +/*!
>
> Drop the ! from comment

Sure.

>
>> + * This function assumes the expected core clock has to be changed by
>> + * modifying the PLL. This is NOT true always but for most of the times,
>> + * it is. So it assumes the PLL output freq is the same as the expected
>> + * core clock (presc=1) unless the core clock is less than PLL_FREQ_MIN.
>> + * In the latter case, it will try to increase the presc value until
>> + * (presc*core_clk) is greater than PLL_FREQ_MIN. It then makes call to
>> + * calc_pll_params() and obtains the values of PD, MFI,MFN, MFD based
>> + * on the targeted PLL and reference input clock to the PLL. Lastly,
>> + * it sets the register based on these values along with the dividers.
>> + * Note 1) There is no value checking for the passed-in divider values
>> + * ? ? ? ? so the caller has to make sure those values are sensible.
>> + * ? ? ?2) Also adjust the NFC divider such that the NFC clock doesn't
>> + * ? ? ? ? exceed NFC_CLK_MAX.
>> + * ? ? ?3) IPU HSP clock is independent of AHB clock. Even it can go up to
>> + * ? ? ? ? 177MHz for higher voltage, this function fixes the max to 133MHz.
>> + * ? ? ?4) This function should not have allowed diag_printf() calls since
>> + * ? ? ? ? the serial driver has been stoped. But leave then here to allow
>> + * ? ? ? ? easy debugging by NOT calling the cyg_hal_plf_serial_stop().
>
> ???
>
> The code is clear taken from another context. Which is diag_printf ?
> Does point 4 apply here ?

Yes, good catch. I will remove it.

>
>> + *
>> + * @param ref ? pll input reference clock (24MHz)
>
> Reference clock is defined as CONFIG_SYS_MX5_HCLK. Is it really possible
> to pass a parameter to this function different as CONFIG_SYS_MX5_HCLK ?
> What happens if CONFIG_SYS_MX5_HCLK and the ref parameter in this
> function have two different values ? It seems to me this is not a use
> case, and then we must avoid it. If I am right, you should drop the ref
> parameter and use CONFIG_SYS_MX5_HCLK.

As the reference manual tell, the pll can have 4 different clock
source. The most
common use case is using 24Mhz OSC as the input.

>
>> + * @param freq ?core clock in Hz
>
> If the freq is in Hz, why do you multiply it for SZ_DEC_1M ?
> It seems to me that freq is in Mhz, not Hz. Please be consistent.

Will fix the comments.

>
>> + * @param clk ? clock type, e.g CPU_CLK, DDR_CLK, etc.
>> + * @return ? ? ?0 if successful; non-zero otherwise
>> + */
>> +int mxc_set_clock(u32 ref, u32 freq, enum mxc_clock clk)
>> +{
>> + ? ? freq *= SZ_DEC_1M;
>> +
>> + ? ? switch (clk) {
>> + ? ? case MXC_ARM_CLK:
>> + ? ? ? ? ? ? if (config_core_clk(ref, freq))
>> + ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? break;
>> + ? ? case MXC_PERIPH_CLK:
>> + ? ? ? ? ? ? if (config_periph_clk(ref, freq))
>> + ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? break;
>> + ? ? case MXC_DDR_CLK:
>> + ? ? ? ? ? ? if (config_ddr_clk(freq))
>> + ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? break;
>> + ? ? case MXC_NFC_CLK:
>> + ? ? ? ? ? ? if (config_nfc_clk(freq))
>> + ? ? ? ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? break;
>> + ? ? default:
>> + ? ? ? ? ? ? printf("Warning:Unsupported or invalid clock type\n");
>
> This is the error case, you must not return 0 as you do.

OK,

>
>> +/*
>> ? * Dump some core clockes.
>> ? */
>> ?int do_mx5_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> @@ -281,6 +825,7 @@ int do_mx5_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> ? ? ? printf("pll3: %dMHz\n", freq / 1000000);
>> ? ? ? printf("ipg clock ? ? : %dHz\n", mxc_get_clock(MXC_IPG_CLK));
>> ? ? ? printf("ipg per clock : %dHz\n", mxc_get_clock(MXC_IPG_PERCLK));
>> + ? ? printf("ddr clock ? ? : %dHz\n", mxc_get_clock(MXC_DDR_CLK));
>
> What about the other clocks you added in your patch (MXC_NFC_CLK /
> MXC_PERIPH_CLK) ?

I will add the these clock print.

>
>> ?/* Define the bits in register CBCDR */
>> +#define MXC_CCM_CBCDR_DDR_HIFREQ_SEL ? ? ? ? (0x1 << 30)
>> +#define MXC_CCM_CBCDR_DDR_PODF_MASK ? ? ? ? ?(0x7 << 27)
>> +#define MXC_CCM_CBCDR_DDR_PODF_OFFSET ? ? ? ? ? ? ? ?27
>> ?#define MXC_CCM_CBCDR_EMI_CLK_SEL ? ? ? ? ? ?(0x1 << 26)
>> ?#define MXC_CCM_CBCDR_PERIPH_CLK_SEL ? ? ? ? (0x1 << 25)
>> ?#define MXC_CCM_CBCDR_EMI_PODF_OFFSET ? ? ? ? ? ? ? ?22
>
> Agree with these changes in this file. Please extend this file with the
> missing constants you use in your code.

OK,

>
> Best regards,
> Stefano Babic
>
> --
> =====================================================================
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 ?Email: office at denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH V7 1/3] MX5: clock: Add clock config interface

[Stefano Babic]

On 05/17/2011 08:25 AM, Jason Liu wrote:
> Hi, Stefano,

Hi Jason,

> I use array here just for the case we will have another element in the array
> (currently we only have one) in the future. Currently, we use 24MHz as ref.

Understood, thanks.

>>>  /*
>>> + * Clock config code start here
>>> + */
>>> +
>>> +/* precondition: m>0 and n>0.  Let g=gcd(m,n). */
>>> +static int gcd(int m, int n)
>>> +{
>>> +     int t;
>>> +     while (m > 0) {
>>> +             if (n > m) {
>>> +                     t = m;
>>> +                     m = n;
>>> +                     n = t;
>>> +             } /* swap */
>>> +             m -= n;
>>> +     }
>>> +     return n;
>>> +}
>>
>> This function has nothing to do with MX5 code. This is a general
>> function, and should be add to lib/. I think you have to remove it from
>> here and put it in a separate patch.
> 
> I don't think it should be put to lib since only the mx5 clock code use it here.

This function computes the GCD of two numbers, and has nothing to do
with MX5 and clock. It is a general function, that others can use.

> OK, I will check linux implementation. Do you know does u-boot also has such
> kind of function already, I did not find it?

No, this function is not yet implemented in u-boot. However, it could be
that there is in some drivers an analog function doing the same. For
this reason and to avoid multiple instances of "quite" same code doing
"quite" the same thing, I suggest to move this function from here from
strictly related IMX code to a general lib. Let's see what Wolfgang
whinks about it.

>>> + *      t_clk = 2*ref_freq*(mfi + mfn/(mfd+1))/(pd+1)
>>
>> Where does this formula come from ?
> 
> The comments is not correct, I will fix it, it should be 4*ref_freq
> not 2 * ref_req.
> The reference manual: 12.2.2.3 DPLL Operation Register, elvis user guide,
> Registers DP_OP, DP_MFN, and DP_MFD calculate the output frequency by
> the formula:
> 
> The chapter number will not the same as you, but you still can find it
> in the reference manual.

Ok. I think it is better to add a comment with the chapter title, as to
set a reference number, because this is changing.

I will check in the manual, thanks for hint.

>> Use defines to set registers. Or read-modify-write, as you changed only
>> PERIPH_APM_SEL from the reset value. However, you reset to 0
>> DDR_CLK_SEL.  I see you switch values back at the end of the function,
>> but my doubt is if it is correct to set in this transition DDR_CLK_SEL
>> always to zero. Can it work with all boards or there are configurations
>> where it does not work ?
> 
> I think it should work since DDR_CLK_SEL:00 derive clock from axi a
> is the default value when boot up.

Then I think it is enough you add as comment your explanation, and
mention that the code does not support if the clock is not derived from
axi-a. If someone adds a custom board with this set-up (and I do not
know if it will be a use case), he is at least warned that must adapt
this function.


>>> + *
>>> + * @param ref   pll input reference clock (24MHz)
>>
>> Reference clock is defined as CONFIG_SYS_MX5_HCLK. Is it really possible
>> to pass a parameter to this function different as CONFIG_SYS_MX5_HCLK ?
>> What happens if CONFIG_SYS_MX5_HCLK and the ref parameter in this
>> function have two different values ? It seems to me this is not a use
>> case, and then we must avoid it. If I am right, you should drop the ref
>> parameter and use CONFIG_SYS_MX5_HCLK.
> 
> As the reference manual tell, the pll can have 4 different clock
> source. The most
> common use case is using 24Mhz OSC as the input.

Understood, thanks.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH V7 3/3] MX53: support for freescale MX53LOCO board

[Albert ARIBAUD]

Hi Jason,

Sorry for chiming in so late, but...

Just one question. What is this with the patch numbering, i.e. 1/1, 2/2, 
3/3 ? I am having a hard time following what is what, especially 
considering the patch subjects are identical apart from version and part 
numbers.

Amicalement,
-- 
Albert.