Dear Wolfgang, On Monday 10 January 2011 04:27 AM, Wolfgang Denk wrote: > Dear Aneesh V, > > In message<1293018898-13253-8-git-send-email-ane...@ti.com> you wrote: >> adapt omap3 to the new layered cache maintenance framework > ... > >> +/* Declarations */ > > Please drop this comment. Everybody sees what this is.
ok. > >> +#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 */ > > ? ok. > > ... >> +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? Number of params is typically 1 or 2. We should have enough space unless the usage is wrong. > >> + } 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). ok. > >> +static void omap3_setup_aux_cr(void) >> +{ >> + /* Workaround for Cortex-A8 errata: #454179 #430973 >> + * Set "IBE" bit > ... > Incorrect multiline comment style. Will correct it. > > ... >> 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? Because a pointer to it has to be passed to ROM code and ROM code wouldn't expect any padding. > > > Best regards, > > Wolfgang Denk > Best regards, Aneesh _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot