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

[Wolfgang Denk]

Dear Aneesh V,

In message <1293018898-13253-8-git-send-email-aneesh at 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.