| Message ID | 1426238380-28958-2-git-send-email-voice.shen@atmel.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Andreas Bießmann |
| Headers | show |
Hello Bo, Am 13.03.2015 10:19, schrieb Bo Shen: > Config MCKR according to the datasheet sequence, or else it > will cause the MCKR configuration failed. > > Remove timeout checking for clock configuration, if configure > the clock failed, let the system hang while not run in wrong > clock configuration. > > Signed-off-by: Bo Shen <voice.shen@atmel.com> > --- > > arch/arm/mach-at91/arm926ejs/clock.c | 54 +++++++++++++++++++----------------- > 1 file changed, 28 insertions(+), 26 deletions(-) Tested on the corvus and taurus board. Tested-by: Heiko Schocher <hs@denx.de> > diff --git a/arch/arm/mach-at91/arm926ejs/clock.c b/arch/arm/mach-at91/arm926ejs/clock.c > index f363982..8d6934e 100644 > --- a/arch/arm/mach-at91/arm926ejs/clock.c > +++ b/arch/arm/mach-at91/arm926ejs/clock.c > @@ -195,50 +195,52 @@ int at91_clock_init(unsigned long main_clock) > void at91_plla_init(u32 pllar) > { > struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; > - int timeout = AT91_PLL_LOCK_TIMEOUT; > > writel(pllar, &pmc->pllar); > - while (!(readl(&pmc->sr) & (AT91_PMC_LOCKA | AT91_PMC_MCKRDY))) { > - timeout--; > - if (timeout == 0) > - break; > - } > + while (!(readl(&pmc->sr) & AT91_PMC_LOCKA)) > + ; just hanging is maybe also bad ... could we hang with adding a if (timeout == 0) { debug("could not set PLL(A|B)\n"); timeout = AT91_PLL_LOCK_TIMEOUT; } Thinking about it ... have we setup here the debug uart already? If not, forget my comment bye, Heiko > } > void at91_pllb_init(u32 pllbr) > { > struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; > - int timeout = AT91_PLL_LOCK_TIMEOUT; > > writel(pllbr, &pmc->pllbr); > - while (!(readl(&pmc->sr) & (AT91_PMC_LOCKB | AT91_PMC_MCKRDY))) { > - timeout--; > - if (timeout == 0) > - break; > - } > + while (!(readl(&pmc->sr) & AT91_PMC_LOCKB)) > + ; > } > > void at91_mck_init(u32 mckr) > { > struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; > - int timeout = AT91_PLL_LOCK_TIMEOUT; > u32 tmp; > > tmp = readl(&pmc->mckr); > - tmp &= ~(AT91_PMC_MCKR_PRES_MASK | > - AT91_PMC_MCKR_MDIV_MASK | > - AT91_PMC_MCKR_PLLADIV_MASK | > - AT91_PMC_MCKR_CSS_MASK); > - tmp |= mckr & (AT91_PMC_MCKR_PRES_MASK | > - AT91_PMC_MCKR_MDIV_MASK | > - AT91_PMC_MCKR_PLLADIV_MASK | > - AT91_PMC_MCKR_CSS_MASK); > + tmp &= ~AT91_PMC_MCKR_PRES_MASK; > + tmp |= mckr & AT91_PMC_MCKR_PRES_MASK; > writel(tmp, &pmc->mckr); > + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) > + ; > > - while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) { > - timeout--; > - if (timeout == 0) > - break; > - } > + tmp = readl(&pmc->mckr); > + tmp &= ~AT91_PMC_MCKR_MDIV_MASK; > + tmp |= mckr & AT91_PMC_MCKR_MDIV_MASK; > + writel(tmp, &pmc->mckr); > + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) > + ; > + > + tmp = readl(&pmc->mckr); > + tmp &= ~AT91_PMC_MCKR_PLLADIV_MASK; > + tmp |= mckr & AT91_PMC_MCKR_PLLADIV_MASK; > + writel(tmp, &pmc->mckr); > + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) > + ; > + > + tmp = readl(&pmc->mckr); > + tmp &= ~AT91_PMC_MCKR_CSS_MASK; > + tmp |= mckr & AT91_PMC_MCKR_CSS_MASK; > + writel(tmp, &pmc->mckr); > + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) > + ; > } > > void at91_periph_clk_enable(int id) >
Hi Heiko, On 03/17/2015 03:45 PM, Heiko Schocher wrote: > Hello Bo, > > Am 13.03.2015 10:19, schrieb Bo Shen: >> Config MCKR according to the datasheet sequence, or else it >> will cause the MCKR configuration failed. >> >> Remove timeout checking for clock configuration, if configure >> the clock failed, let the system hang while not run in wrong >> clock configuration. >> >> Signed-off-by: Bo Shen <voice.shen@atmel.com> >> --- >> >> arch/arm/mach-at91/arm926ejs/clock.c | 54 >> +++++++++++++++++++----------------- >> 1 file changed, 28 insertions(+), 26 deletions(-) > > Tested on the corvus and taurus board. > > Tested-by: Heiko Schocher <hs@denx.de> Thanks. >> diff --git a/arch/arm/mach-at91/arm926ejs/clock.c >> b/arch/arm/mach-at91/arm926ejs/clock.c >> index f363982..8d6934e 100644 >> --- a/arch/arm/mach-at91/arm926ejs/clock.c >> +++ b/arch/arm/mach-at91/arm926ejs/clock.c >> @@ -195,50 +195,52 @@ int at91_clock_init(unsigned long main_clock) >> void at91_plla_init(u32 pllar) >> { >> struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; >> - int timeout = AT91_PLL_LOCK_TIMEOUT; >> >> writel(pllar, &pmc->pllar); >> - while (!(readl(&pmc->sr) & (AT91_PMC_LOCKA | AT91_PMC_MCKRDY))) { >> - timeout--; >> - if (timeout == 0) >> - break; >> - } >> + while (!(readl(&pmc->sr) & AT91_PMC_LOCKA)) >> + ; > > just hanging is maybe also bad ... could we hang with adding a > > if (timeout == 0) { > debug("could not set PLL(A|B)\n"); > timeout = AT91_PLL_LOCK_TIMEOUT; > } > > Thinking about it ... have we setup here the debug uart already? > If not, forget my comment Yes the uart is not ready. And one more thing, if the clock is not setup correctly, then the system will run in abnormal status. So, I remove the timeout check here. Best Regards, Bo Shen > bye, > Heiko >> } >> void at91_pllb_init(u32 pllbr) >> { >> struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; >> - int timeout = AT91_PLL_LOCK_TIMEOUT; >> >> writel(pllbr, &pmc->pllbr); >> - while (!(readl(&pmc->sr) & (AT91_PMC_LOCKB | AT91_PMC_MCKRDY))) { >> - timeout--; >> - if (timeout == 0) >> - break; >> - } >> + while (!(readl(&pmc->sr) & AT91_PMC_LOCKB)) >> + ; >> } >> >> void at91_mck_init(u32 mckr) >> { >> struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; >> - int timeout = AT91_PLL_LOCK_TIMEOUT; >> u32 tmp; >> >> tmp = readl(&pmc->mckr); >> - tmp &= ~(AT91_PMC_MCKR_PRES_MASK | >> - AT91_PMC_MCKR_MDIV_MASK | >> - AT91_PMC_MCKR_PLLADIV_MASK | >> - AT91_PMC_MCKR_CSS_MASK); >> - tmp |= mckr & (AT91_PMC_MCKR_PRES_MASK | >> - AT91_PMC_MCKR_MDIV_MASK | >> - AT91_PMC_MCKR_PLLADIV_MASK | >> - AT91_PMC_MCKR_CSS_MASK); >> + tmp &= ~AT91_PMC_MCKR_PRES_MASK; >> + tmp |= mckr & AT91_PMC_MCKR_PRES_MASK; >> writel(tmp, &pmc->mckr); >> + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) >> + ; >> >> - while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) { >> - timeout--; >> - if (timeout == 0) >> - break; >> - } >> + tmp = readl(&pmc->mckr); >> + tmp &= ~AT91_PMC_MCKR_MDIV_MASK; >> + tmp |= mckr & AT91_PMC_MCKR_MDIV_MASK; >> + writel(tmp, &pmc->mckr); >> + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) >> + ; >> + >> + tmp = readl(&pmc->mckr); >> + tmp &= ~AT91_PMC_MCKR_PLLADIV_MASK; >> + tmp |= mckr & AT91_PMC_MCKR_PLLADIV_MASK; >> + writel(tmp, &pmc->mckr); >> + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) >> + ; >> + >> + tmp = readl(&pmc->mckr); >> + tmp &= ~AT91_PMC_MCKR_CSS_MASK; >> + tmp |= mckr & AT91_PMC_MCKR_CSS_MASK; >> + writel(tmp, &pmc->mckr); >> + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) >> + ; >> } >> >> void at91_periph_clk_enable(int id) >> >
Hello Bo, Am 17.03.2015 08:58, schrieb Bo Shen: > Hi Heiko, > > On 03/17/2015 03:45 PM, Heiko Schocher wrote: >> Hello Bo, >> >> Am 13.03.2015 10:19, schrieb Bo Shen: >>> Config MCKR according to the datasheet sequence, or else it >>> will cause the MCKR configuration failed. >>> >>> Remove timeout checking for clock configuration, if configure >>> the clock failed, let the system hang while not run in wrong >>> clock configuration. >>> >>> Signed-off-by: Bo Shen <voice.shen@atmel.com> >>> --- >>> >>> arch/arm/mach-at91/arm926ejs/clock.c | 54 >>> +++++++++++++++++++----------------- >>> 1 file changed, 28 insertions(+), 26 deletions(-) >> >> Tested on the corvus and taurus board. >> >> Tested-by: Heiko Schocher <hs@denx.de> > > Thanks. > >>> diff --git a/arch/arm/mach-at91/arm926ejs/clock.c >>> b/arch/arm/mach-at91/arm926ejs/clock.c >>> index f363982..8d6934e 100644 >>> --- a/arch/arm/mach-at91/arm926ejs/clock.c >>> +++ b/arch/arm/mach-at91/arm926ejs/clock.c >>> @@ -195,50 +195,52 @@ int at91_clock_init(unsigned long main_clock) >>> void at91_plla_init(u32 pllar) >>> { >>> struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; >>> - int timeout = AT91_PLL_LOCK_TIMEOUT; >>> >>> writel(pllar, &pmc->pllar); >>> - while (!(readl(&pmc->sr) & (AT91_PMC_LOCKA | AT91_PMC_MCKRDY))) { >>> - timeout--; >>> - if (timeout == 0) >>> - break; >>> - } >>> + while (!(readl(&pmc->sr) & AT91_PMC_LOCKA)) >>> + ; >> >> just hanging is maybe also bad ... could we hang with adding a >> >> if (timeout == 0) { >> debug("could not set PLL(A|B)\n"); >> timeout = AT91_PLL_LOCK_TIMEOUT; >> } >> >> Thinking about it ... have we setup here the debug uart already? >> If not, forget my comment > > Yes the uart is not ready. And one more thing, if the clock is not setup correctly, then the system will run in abnormal status. So, I remove the timeout check here. Ok, thanks! bye, Heiko
diff --git a/arch/arm/mach-at91/arm926ejs/clock.c b/arch/arm/mach-at91/arm926ejs/clock.c index f363982..8d6934e 100644 --- a/arch/arm/mach-at91/arm926ejs/clock.c +++ b/arch/arm/mach-at91/arm926ejs/clock.c @@ -195,50 +195,52 @@ int at91_clock_init(unsigned long main_clock) void at91_plla_init(u32 pllar) { struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; - int timeout = AT91_PLL_LOCK_TIMEOUT; writel(pllar, &pmc->pllar); - while (!(readl(&pmc->sr) & (AT91_PMC_LOCKA | AT91_PMC_MCKRDY))) { - timeout--; - if (timeout == 0) - break; - } + while (!(readl(&pmc->sr) & AT91_PMC_LOCKA)) + ; } void at91_pllb_init(u32 pllbr) { struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; - int timeout = AT91_PLL_LOCK_TIMEOUT; writel(pllbr, &pmc->pllbr); - while (!(readl(&pmc->sr) & (AT91_PMC_LOCKB | AT91_PMC_MCKRDY))) { - timeout--; - if (timeout == 0) - break; - } + while (!(readl(&pmc->sr) & AT91_PMC_LOCKB)) + ; } void at91_mck_init(u32 mckr) { struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; - int timeout = AT91_PLL_LOCK_TIMEOUT; u32 tmp; tmp = readl(&pmc->mckr); - tmp &= ~(AT91_PMC_MCKR_PRES_MASK | - AT91_PMC_MCKR_MDIV_MASK | - AT91_PMC_MCKR_PLLADIV_MASK | - AT91_PMC_MCKR_CSS_MASK); - tmp |= mckr & (AT91_PMC_MCKR_PRES_MASK | - AT91_PMC_MCKR_MDIV_MASK | - AT91_PMC_MCKR_PLLADIV_MASK | - AT91_PMC_MCKR_CSS_MASK); + tmp &= ~AT91_PMC_MCKR_PRES_MASK; + tmp |= mckr & AT91_PMC_MCKR_PRES_MASK; writel(tmp, &pmc->mckr); + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) + ; - while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) { - timeout--; - if (timeout == 0) - break; - } + tmp = readl(&pmc->mckr); + tmp &= ~AT91_PMC_MCKR_MDIV_MASK; + tmp |= mckr & AT91_PMC_MCKR_MDIV_MASK; + writel(tmp, &pmc->mckr); + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) + ; + + tmp = readl(&pmc->mckr); + tmp &= ~AT91_PMC_MCKR_PLLADIV_MASK; + tmp |= mckr & AT91_PMC_MCKR_PLLADIV_MASK; + writel(tmp, &pmc->mckr); + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) + ; + + tmp = readl(&pmc->mckr); + tmp &= ~AT91_PMC_MCKR_CSS_MASK; + tmp |= mckr & AT91_PMC_MCKR_CSS_MASK; + writel(tmp, &pmc->mckr); + while (!(readl(&pmc->sr) & AT91_PMC_MCKRDY)) + ; } void at91_periph_clk_enable(int id)
Config MCKR according to the datasheet sequence, or else it will cause the MCKR configuration failed. Remove timeout checking for clock configuration, if configure the clock failed, let the system hang while not run in wrong clock configuration. Signed-off-by: Bo Shen <voice.shen@atmel.com> --- arch/arm/mach-at91/arm926ejs/clock.c | 54 +++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 26 deletions(-)