On Sun, Feb 1, 2015 at 2:46 PM, Oleg Kolosov <bazurbat@xxxxxxxxx> wrote: > Hello MIPS gurus! > > I'm adding support for Sigma Designs SMP8652/SMP8654 (Tango3 family, > MIPS 24kf CPU) to newer kernel. I've selectively adapted patches from > 2.6.32.15 (the latest officially available for us) to the latest mips > 3.18 stable branch and things seem to work (it boots, runs simple test > programs), but there are few questions which I was not able to resolve > yet with my limited experience: > > 1. They (Sigma Designs) have overridden __fast_iob which is identical to > the default one except for one line: > > : "m" (*(int *)CKSEG1) > > is replaced with: > > : "m" (*(int *)(CKSEG1+CPU_REMAP_SPACE)) > > where CPU_REMAP_SPACE=0x4000000 is a compile time constant for Tango3 > and also called KERNEL_START_ADDRESS in Makefiles. The same value is > also written to ebase: > > ebase = KSEG0ADDR(CPU_REMAP_SPACE); > write_c0_ebase(ebase); > > in traps.c:per_cpu_trap_init() > > while writing ebase is really necessary for the kernel to boot, I've not > found any negative effects of not applying __fast_iob patch. What is it > supposed to do? I do not have any direct experience with these SoCs, but you might want to look at the memory map to try to figure this one out. i.e. if __fast_iob() normally performs an uncached dummy read from the first word of physical memory, does the address need to be adjusted by 64MB on the Sigma chips because system memory (or the memory allocated to the Linux application processor) starts at PA 0x0400_0000 instead of 0x0000_0000? That theory would also explain why the exception vectors were adjusted by the same offset. BTW, you can override ebase from the platform code, as was done in arch/mips/kernel/smp-bmips.c. It probably isn't necessary to change the common per_cpu_trap_init() code (but it may have been necessary way back in 2.6.32). > 2. In io.h they have added explicit __sync() to the end of > pfx##write##bwlq and pfx##out##bwlq##p. Is this really necessary? I've > not yet found any adverse effects of not doing so. Maybe this was a > workaround for some old kernel issue which was fixed since then? Adding a barrier in writel(), as was done on ARM, might have something to do with the SoC's busing or peripherals. Sometimes there are chip bugs that cause MMIO transaction ordering to break in unexpected ways. Or it could be there to compensate for missing barriers or bad assumptions in a driver somewhere. For #2 and #3, it is likely that somebody at Sigma could find a bug report or changelog explaining why it was added. In my experience these sorts of changes are usually made to work around subtle problems discovered in testing or production. Figuring out the exact problem that inspired the patch can be difficult without insider knowledge, unless you happened to run across the same failure. > 3. In c-r4k.c:r4k_cache_init() they assign: > > flush_icache_page = r4k_flush_icache_page; > > where: > > static void r4k_flush_icache_page(struct vm_area_struct *vma, > struct page *page) > { > r4k_flush_icache_range((unsigned long)page_address(page), > (unsigned long)page_address(page) + PAGE_SIZE); > } Hmm, this might not play nice with HIGHMEM. If it does, it would be helpful to include a comment explaining why.