Hi Tom, On 11 August 2015 at 11:41, Tom Warren <twar...@nvidia.com> wrote: > Simon, > >> -----Original Message----- >> From: Simon Glass [mailto:s...@google.com] On Behalf Of Simon Glass >> Sent: Monday, August 10, 2015 6:15 AM >> To: U-Boot Mailing List >> Cc: Simon Glass; Tom Warren; Thierry Reding; Masahiro Yamada; Stephen >> Warren; Tom Rini; Albert Aribaud; Marcel Ziswiler; Stephen Warren >> Subject: [PATCH] tegra: Correct logic for reading pll_misc in >> clock_start_pll() >> >> The logic for simple PLLs on T124 was broken by this commit: >> >> 722e000c Tegra: PLL: use per-SoC pllinfo table instead of PLL_DIVM/N/P, >> etc. >> >> Correct it by reading from the same pll_misc register that it writes to and >> adding an entry for the DP PLL. >> >> Signed-off-by: Simon Glass <s...@chromium.org> >> --- >> >> arch/arm/mach-tegra/clock.c | 33 +++++++++++++++++++++------------ >> arch/arm/mach-tegra/tegra124/clock.c | 2 ++ >> 2 files changed, 23 insertions(+), 12 deletions(-) >> >> diff --git a/arch/arm/mach-tegra/clock.c b/arch/arm/mach-tegra/clock.c index >> 3b2b4ff..f014434 100644 >> --- a/arch/arm/mach-tegra/clock.c >> +++ b/arch/arm/mach-tegra/clock.c >> @@ -126,19 +126,34 @@ unsigned long clock_start_pll(enum clock_id clkid, u32 >> divm, u32 divn, { >> struct clk_pll *pll = NULL; >> struct clk_pll_info *pllinfo = &tegra_pll_info_table[clkid]; >> + struct clk_pll_simple *simple_pll = NULL; >> u32 misc_data, data; >> >> - if (clkid < (enum clock_id)TEGRA_CLK_PLLS) >> + if (clkid < (enum clock_id)TEGRA_CLK_PLLS) { >> pll = get_pll(clkid); >> + } else { >> + simple_pll = clock_get_simple_pll(clkid); >> + if (!simple_pll) { >> + debug("%s: Uknown simple PLL %d\n", __func__, >> clkid); >> + return 0; >> + } >> + } >> >> /* >> * pllinfo has the m/n/p and kcp/kvco mask and shift >> * values for all of the PLLs used in U-Boot, with any >> * SoC differences accounted for. >> + * >> + * Preserve EN_LOCKDET, etc. >> */ >> - misc_data = readl(&pll->pll_misc); /* preserve EN_LOCKDET, etc. >> */ >> - misc_data &= ~(pllinfo->kcp_mask << pllinfo->kcp_shift) | (cpcon << >> pllinfo->kcp_shift); >> - misc_data &= ~(pllinfo->kvco_mask << pllinfo->kvco_shift) | (lfcon << >> pllinfo->kvco_shift); >> + if (pll) >> + misc_data = readl(&pll->pll_misc); >> + else >> + misc_data = readl(&simple_pll->pll_misc); >> + misc_data &= ~(pllinfo->kcp_mask << pllinfo->kcp_shift); >> + misc_data |= cpcon << pllinfo->kcp_shift; >> + misc_data &= ~(pllinfo->kvco_mask << pllinfo->kvco_shift); >> + misc_data |= lfcon << pllinfo->kvco_shift; >> >> data = (divm << pllinfo->m_shift) | (divn << pllinfo->n_shift); >> data |= divp << pllinfo->p_shift; >> @@ -148,14 +163,8 @@ unsigned long clock_start_pll(enum clock_id clkid, u32 >> divm, u32 divn, >> writel(misc_data, &pll->pll_misc); >> writel(data, &pll->pll_base); >> } else { >> - struct clk_pll_simple *pll = clock_get_simple_pll(clkid); >> - >> - if (!pll) { >> - debug("%s: Uknown simple PLL %d\n", __func__, >> clkid); >> - return 0; >> - } >> - writel(misc_data, &pll->pll_misc); >> - writel(data, &pll->pll_base); >> + writel(misc_data, &simple_pll->pll_misc); >> + writel(data, &simple_pll->pll_base); >> } >> >> /* calculate the stable time */ >> diff --git a/arch/arm/mach-tegra/tegra124/clock.c b/arch/arm/mach- >> tegra/tegra124/clock.c >> index 9126218..42000ae 100644 >> --- a/arch/arm/mach-tegra/tegra124/clock.c >> +++ b/arch/arm/mach-tegra/tegra124/clock.c >> @@ -593,6 +593,8 @@ struct clk_pll_info >> tegra_pll_info_table[CLOCK_ID_PLL_COUNT] = { >> .lock_ena = 9, .lock_det = 11, .kcp_shift = 6, .kcp_mask = 3, >> .kvco_shift = 0, .kvco_mask = 1 }, /* PLLE */ >> { .m_shift = 0, .m_mask = 0x0F, .n_shift = 8, .n_mask = 0x3FF, >> .p_shift = >> 20, .p_mask = 0x07, >> .lock_ena = 18, .lock_det = 27, .kcp_shift = 8, .kcp_mask = 0xF, >> .kvco_shift = 4, .kvco_mask = 0xF }, /* PLLS (RESERVED) */ >> + { .m_shift = 0, .m_mask = 0x1F, .n_shift = 8, .n_mask = 0x3FF, >> .p_shift >> = 20, .p_mask = 7, >> + .lock_ena = 18, .lock_det = 27, .kcp_shift = 8, .kcp_mask = 0xF, >> .kvco_shift = 4, .kvco_mask = 0xF }, /* PLLDP */ >> }; > I was looking at adding a PLLDP table entry for T210 based on your T124 > table, and your settings don't match what I have in my T124 TRM. > > PLLDP_MDIV is 8 bits wide, starting at bit 0, so .m_shift = 0 but .m_mask s/b > 0xFF. > PLLDP_NDIV is 8 bits wide, starting at bit 8, so .n_shift = 8, but .n_mask > s/b 0xFF. > .lock_det is OK at bit 27, but .lock_ena s/b bit 30 (PLLDP_LOCK_ENABLE). > PLLDP_KCP is 2 bits wide, starting at bit 25 with a mask of 0x3, so > .kcp_shift s/b 25 and .kcp_mask s/b 3. > PLLDP_KVCO is 1 bit wide, starting at bit 24 with a mask of 1, so .kvco_shift > s/b 24 and .kvco_mask s/b 1. > > Not sure where your values came from - maybe a cut&paste error?
Actually I just copied the pre-existing defines as I was trying to revert the behaviour. But please go ahead and update the change, since what you have comes from the datasheet. > > Please take a look. I've got this patch in u-boot-tegra/next right now, but I > can update it when you've confirmed my findings. > > Tom > -- > nvpublic Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
