Hi Albert and Mike,
On Thu, Jul 19, 2012 at 7:10 AM, Mike Frysinger vapier@gentoo.org wrote:
On Thursday 12 July 2012 14:12:41 Albert ARIBAUD wrote:
On Thu, 12 Jul 2012 08:25:11 -0700, Simon Glass wrote:
--- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h
+/* options available for data cache on each page */ +enum dcache_option {
- DCACHE_OFF,
- DCACHE_WRITETHROUGH,
- DCACHE_WRITEBACK,
+};
--- a/arch/arm/lib/cache-cp15.c +++ b/arch/arm/lib/cache-cp15.c
-static inline void dram_bank_mmu_setup(int bank) +void set_section_dcache(int section, enum dcache_option option) {
- u32 value = section << MMU_SECTION_SHIFT | (3 << 10); u32 *page_table = (u32 *)gd->tlb_addr;
- switch (option) {
- case DCACHE_WRITETHROUGH:
value |= 0x1a;break;- case DCACHE_WRITEBACK:
value |= 0x1e;break;- case DCACHE_OFF:
value |= 0x12;break;- }
what's the benefit of introducing an arbitrary enum rather than defining DCACHE_WRITETHROUGH, DCACHE_WRITEBACK and DCACHE_OFF equal respecitvely to 0x1a, 0x1e and 0x12? All it does is force this switch case instead of a simple 'value |= option;' statement.
if these magic bitmasks are going to be the same for all arm cores (the enum is in common arm header), then you can get the advantages of both: enum dcache_option { DCACHE_OFF = 0x12, DCACHE_WRITETHROUGH = 0x1a, DCACHE_WRITEBACK = 0x1e, };
then just do: value |= option;
Yes you are both saying the same thing - sounds good. I will change it.
Regards, Simon
-mike