| Message ID | 466a431b5430548a018c21222080ed4040596147.1499329461.git.christophe.leroy@c-s.fr |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
Hello Christophe, Am 06.07.2017 um 10:33 schrieb Christophe Leroy: > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> > --- > arch/powerpc/cpu/mpc8xx/cpu_init.c | 8 ++++++++ > 1 file changed, 8 insertions(+) Reviewed-by: Heiko Schocher <hs@denx.de> nitpick only > diff --git a/arch/powerpc/cpu/mpc8xx/cpu_init.c b/arch/powerpc/cpu/mpc8xx/cpu_init.c > index cf1280983a..52406e8483 100644 > --- a/arch/powerpc/cpu/mpc8xx/cpu_init.c > +++ b/arch/powerpc/cpu/mpc8xx/cpu_init.c > @@ -51,6 +51,14 @@ void cpu_init_f(immap_t __iomem *immr) > clrsetbits_be32(&immr->im_clkrst.car_sccr, ~SCCR_MASK, > CONFIG_SYS_SCCR); > > + /* BUG MPC866 GLL2 consideration */ > + reg = in_be32(&immr->im_clkrst.car_sccr); > + /* probably we use the mode 1:2:1 */ > + if ((reg & 0x00060000) == 0x00020000) { > + clrbits_be32(&immr->im_clkrst.car_sccr, 0x00060000); > + setbits_be32(&immr->im_clkrst.car_sccr, 0x00020000); You may can introduce defines for the magic values here. > + } > + > /* PLL (CPU clock) settings (15-30) */ > > out_be32(&immr->im_clkrstk.cark_plprcrk, KAPWR_KEY); > bye, Heiko
Dear Christophe, In message <466a431b5430548a018c21222080ed4040596147.1499329461.git.christophe.leroy@c-s.fr> you wrote: > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> > --- > arch/powerpc/cpu/mpc8xx/cpu_init.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/powerpc/cpu/mpc8xx/cpu_init.c b/arch/powerpc/cpu/mpc8xx/cpu_init.c > index cf1280983a..52406e8483 100644 > --- a/arch/powerpc/cpu/mpc8xx/cpu_init.c > +++ b/arch/powerpc/cpu/mpc8xx/cpu_init.c > @@ -51,6 +51,14 @@ void cpu_init_f(immap_t __iomem *immr) > clrsetbits_be32(&immr->im_clkrst.car_sccr, ~SCCR_MASK, > CONFIG_SYS_SCCR); > > + /* BUG MPC866 GLL2 consideration */ > + reg = in_be32(&immr->im_clkrst.car_sccr); > + /* probably we use the mode 1:2:1 */ > + if ((reg & 0x00060000) == 0x00020000) { > + clrbits_be32(&immr->im_clkrst.car_sccr, 0x00060000); > + setbits_be32(&immr->im_clkrst.car_sccr, 0x00020000); > + } Like a few lines above, you could/should use a single call to clrsetbits_be32() here. And as Heiko already commented, please use readable names istead of the magic numbers. Reviewed-by: Wolfgang Denk <wd@denx.de> Best regards, Wolfgang Denk
Le 06/07/2017 à 12:56, Wolfgang Denk a écrit : > Dear Christophe, > > In message <466a431b5430548a018c21222080ed4040596147.1499329461.git.christophe.leroy@c-s.fr> you wrote: >> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> >> --- >> arch/powerpc/cpu/mpc8xx/cpu_init.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/arch/powerpc/cpu/mpc8xx/cpu_init.c b/arch/powerpc/cpu/mpc8xx/cpu_init.c >> index cf1280983a..52406e8483 100644 >> --- a/arch/powerpc/cpu/mpc8xx/cpu_init.c >> +++ b/arch/powerpc/cpu/mpc8xx/cpu_init.c >> @@ -51,6 +51,14 @@ void cpu_init_f(immap_t __iomem *immr) >> clrsetbits_be32(&immr->im_clkrst.car_sccr, ~SCCR_MASK, >> CONFIG_SYS_SCCR); >> >> + /* BUG MPC866 GLL2 consideration */ >> + reg = in_be32(&immr->im_clkrst.car_sccr); >> + /* probably we use the mode 1:2:1 */ >> + if ((reg & 0x00060000) == 0x00020000) { >> + clrbits_be32(&immr->im_clkrst.car_sccr, 0x00060000); >> + setbits_be32(&immr->im_clkrst.car_sccr, 0x00020000); >> + } > > Like a few lines above, you could/should use a single call to > clrsetbits_be32() here. And as Heiko already commented, please use > readable names istead of the magic numbers. I shall not use clrsetbits_be32(), because the ERRATA says: Program the PLPRCR such that the PLL clock will change, then reprogram the PLPRCR value back to the desired value Christophe > > Reviewed-by: Wolfgang Denk <wd@denx.de> > > > Best regards, > > Wolfgang Denk >
Oops, I copied wrong alternative of the ERRATA. Correct one this time. Le 06/07/2017 à 13:12, Christophe LEROY a écrit : > > > Le 06/07/2017 à 12:56, Wolfgang Denk a écrit : >> Dear Christophe, >> >> In message >> <466a431b5430548a018c21222080ed4040596147.1499329461.git.christophe.leroy@c-s.fr> >> you wrote: >>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> >>> --- >>> arch/powerpc/cpu/mpc8xx/cpu_init.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/arch/powerpc/cpu/mpc8xx/cpu_init.c >>> b/arch/powerpc/cpu/mpc8xx/cpu_init.c >>> index cf1280983a..52406e8483 100644 >>> --- a/arch/powerpc/cpu/mpc8xx/cpu_init.c >>> +++ b/arch/powerpc/cpu/mpc8xx/cpu_init.c >>> @@ -51,6 +51,14 @@ void cpu_init_f(immap_t __iomem *immr) >>> clrsetbits_be32(&immr->im_clkrst.car_sccr, ~SCCR_MASK, >>> CONFIG_SYS_SCCR); >>> + /* BUG MPC866 GLL2 consideration */ >>> + reg = in_be32(&immr->im_clkrst.car_sccr); >>> + /* probably we use the mode 1:2:1 */ >>> + if ((reg & 0x00060000) == 0x00020000) { >>> + clrbits_be32(&immr->im_clkrst.car_sccr, 0x00060000); >>> + setbits_be32(&immr->im_clkrst.car_sccr, 0x00020000); >>> + } >> >> Like a few lines above, you could/should use a single call to >> clrsetbits_be32() here. And as Heiko already commented, please use >> readable names istead of the magic numbers. > > I shall not use clrsetbits_be32(), because the ERRATA says: The ERRATA says: Reprogram the SCCR: 1. Write 1'b00 to SCCR[EBDF]. 2. Write 1'b01 to SCCR[EBDF]. 3. Rewrite the desired value to the PLPRCR register Christophe > > Program the PLPRCR such that the PLL clock will change, then reprogram > the PLPRCR value back to the desired value > > Christophe > >> >> Reviewed-by: Wolfgang Denk <wd@denx.de> >> >> >> Best regards, >> >> Wolfgang Denk >> > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
Dear Christophe, In message <1e6c1b5c-2e49-6784-6d6d-f4532aa20434@c-s.fr> you wrote: > > > Like a few lines above, you could/should use a single call to > > clrsetbits_be32() here. And as Heiko already commented, please use > > readable names istead of the magic numbers. > > I shall not use clrsetbits_be32(), because the ERRATA says: > > Program the PLPRCR such that the PLL clock will change, then reprogram > the PLPRCR value back to the desired value Ah! This is critical information, so please add a comment to explain this. [Otherwise there is the risk some later "optimization" intro- duces a bug.] Best regards, Wolfgang Denk
diff --git a/arch/powerpc/cpu/mpc8xx/cpu_init.c b/arch/powerpc/cpu/mpc8xx/cpu_init.c index cf1280983a..52406e8483 100644 --- a/arch/powerpc/cpu/mpc8xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc8xx/cpu_init.c @@ -51,6 +51,14 @@ void cpu_init_f(immap_t __iomem *immr) clrsetbits_be32(&immr->im_clkrst.car_sccr, ~SCCR_MASK, CONFIG_SYS_SCCR); + /* BUG MPC866 GLL2 consideration */ + reg = in_be32(&immr->im_clkrst.car_sccr); + /* probably we use the mode 1:2:1 */ + if ((reg & 0x00060000) == 0x00020000) { + clrbits_be32(&immr->im_clkrst.car_sccr, 0x00060000); + setbits_be32(&immr->im_clkrst.car_sccr, 0x00020000); + } + /* PLL (CPU clock) settings (15-30) */ out_be32(&immr->im_clkrstk.cark_plprcrk, KAPWR_KEY);
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> --- arch/powerpc/cpu/mpc8xx/cpu_init.c | 8 ++++++++ 1 file changed, 8 insertions(+)