Dear nitin.g...@freescale.com,

In message <1409516179-32553-2-git-send-email-nitin.g...@freescale.com> you 
wrote:
>
> +     reg = readl(&ccm_regs->CCGR0);
> +     reg |= (MXC_CCM_CCGR0_CAAM_WRAPPER_IPG_MASK |
> +                     MXC_CCM_CCGR0_CAAM_WRAPPER_ACLK_MASK |
> +                     MXC_CCM_CCGR0_CAAM_SECURE_MEM_MASK);
> +     writel(reg, &ccm_regs->CCGR0);

Is there any specific reason for not using setbits_le32() instead?

> +     /* Enable EMI slow clk */
> +     reg = readl(&ccm_regs->CCGR6);
> +     reg |= MXC_CCM_CCGR6_EMI_SLOW_MASK;
> +     writel(reg, &ccm_regs->CCGR6);

Ditto?

> +     reg = readl(&ccm_regs->CCGR0);
> +     reg &= ~(MXC_CCM_CCGR0_CAAM_WRAPPER_IPG_MASK |
> +                     MXC_CCM_CCGR0_CAAM_WRAPPER_ACLK_MASK |
> +                     MXC_CCM_CCGR0_CAAM_SECURE_MEM_MASK);
> +     writel(reg, &ccm_regs->CCGR0);

Why not using clrbits_le32() instead?

> +     /* Disable EMI slow clk */
> +     reg = readl(&ccm_regs->CCGR6);
> +     reg &= ~MXC_CCM_CCGR6_EMI_SLOW_MASK;
> +     writel(reg, &ccm_regs->CCGR6);

Ditto?

> +/* enable HAB (High Assurance Boot) debug */
> +/* #define DEBUG_AUTHENTICATE_IMAGE */
> +

Please do not add dead code.


> +#ifdef DEBUG_AUTHENTICATE_IMAGE
> +void dump_mem(uint32_t addr, int size)
> +{
> +     int i;
> +
> +     for (i = 0; i < size; i += 4) {
> +             if (i != 0) {
> +                     if (i % 16 == 0)
> +                             printf("\n");
> +                     else
> +                             printf(" ");
> +             }
> +
> +             printf("0x%08x", *(uint32_t *)addr);
> +             addr += 4;
> +     }
> +
> +     printf("\n");
> +
> +     return;
> +}
> +#endif

Is this really needed?  Do we not already have such code, like
print_buffer() ?


> @@ -60,4 +62,8 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num);
>  int enable_spi_clk(unsigned char enable, unsigned spi_num);
>  void enable_ipu_clock(void);
>  int enable_fec_anatop_clock(enum enet_freq freq);
> +#ifdef CONFIG_SECURE_BOOT
> +void hab_caam_clock_enable(void);
> +void hab_caam_clock_disable(void);
> +#endif

You can drop the #ifdef for prototypes.

> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h
> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h
> @@ -2,6 +2,8 @@
>   * (C) Copyright 2009
>   * Stefano Babic, DENX Software Engineering, sba...@denx.de.
>   *
> + * (C) Copyright 2014 Freescale Semiconductor, Inc.
> + *
>   * SPDX-License-Identifier:  GPL-2.0+
>   */
>  
> @@ -11,7 +13,7 @@
>  #include <asm/imx-common/regs-common.h>
>  #include "../arch-imx/cpu.h"
>  
> -#define soc_rev() (get_cpu_rev() & 0xFF)
> +#define soc_rev() ((int)(get_cpu_rev() & 0xFF))

Claiming copyright for adding a single cast is a bit excessive, isn't
it?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Your csh still thinks true is false. Write to your vendor  today  and
tell them that next year Configure ought to "rm /bin/csh" unless they
fix  their blasted shell. :-)
                 - Larry Wall in Configure from the perl distribution
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to