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? 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 > > /* > -- > 2.5.0.rc2.392.g76e840b _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
