[U-Boot] [PATCH v2 04/10] powerpc, 8xx: Implement GLL2 ERRATA

Wolfgang Denk
2017-07-06T10:56:31Z

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

--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I haven't lost my mind - it's backed up on tape somewhere."