[U-Boot] [PATCH 7/8] armv7: adapt omap3 to the new cache maintenance framework

Wolfgang Denk
2011-01-09T22:57:29Z

Dear Aneesh V,

In message <1293018898-13253-8-git-send-email-aneesh@ti.com> you wrote:
> adapt omap3 to the new layered cache maintenance framework
...

> +/* Declarations */

Please drop this comment. Everybody sees what this is.

> +#ifndef CONFIG_L2_OFF
> /*
> - * Writing to AuxCR in U-boot using SMI for GP DEV
> - * Currently SMI in Kernel on ES2 devices seems to have an issue
> - * Once that is resolved, we can postpone this config to kernel
> + * Invalidate L2-cache from secure mode
> */

Why not change this into simple

/* Invalidate L2-cache from secure mode */

?

...
> +static void omap3_emu_romcode_call(u32 service_id, u32 *parameters)
> +{
> + u32 i, num_params = *parameters;
> + u32 *sram_scratch_space = (u32 *)OMAP3_PUBLIC_SRAM_SCRATCH_AREA;
> + /*
> + * copy the parameters to an un-cached area to avoid coherency
> + * issues
> + */
> + for (i = 0; i < num_params; i++) {
> + __raw_writel(*parameters, sram_scratch_space);
> + parameters++;
> + sram_scratch_space++;
> + }

Do you have unlimited storage there? Or should you add some check not
to exceed some maximum size?

> + } else {
> + struct emu_hal_params emu_romcode_params;
> + emu_romcode_params.num_params = 1;
> + emu_romcode_params.param1 = acr;
> + omap3_emu_romcode_call(OMAP3_EMU_HAL_API_WRITE_ACR,
> + (u32 *)&emu_romcode_params);

Please add a blank line between declarations and code (fix globally).

> +static void omap3_setup_aux_cr(void)
> +{
> + /* Workaround for Cortex-A8 errata: #454179 #430973
> + * Set "IBE" bit
...
Incorrect multiline comment style.

...
> diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h b/arch/arm/include/asm/arch-omap3/sys_proto.h
> index 4a28ba1..25f54ea 100644
> --- a/arch/arm/include/asm/arch-omap3/sys_proto.h
> +++ b/arch/arm/include/asm/arch-omap3/sys_proto.h
> @@ -27,6 +27,11 @@ typedef struct {
> char *nand_string;
> } omap3_sysinfo;
>
> +struct __attribute__ ((__packed__)) emu_hal_params {
> + u32 num_params;
> + u32 param1;
> +};

Why exactly do we need the "__attribute__ ((__packed__))" here?

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: wd at denx.de
The years of peak mental activity are undoubtedly between the ages of
four and eighteen. At four we know all the questions, at eighteen all
the answers.