Dear Wolfgang,
On Monday 10 January 2011 04:27 AM, Wolfgang Denk wrote:
Dear Aneesh V,
In message1293018898-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.
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 modeWhy 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