Blackfin: architecture update patch
A message that is not in the archive (sent off-list or not yet received): 1174471618.5648.50.camel@roc-desktop
Review static inline void leds_switch(int flag) · line 104
Quoted 9 lines · Bryan
(quoted diff) +5
-0@@ -97,6 +97,11 @@ static inline void leds_switch(int flag) /* * The idle loop on BFIN */ +#ifdef CONFIG_IDLE_L1 +static inline void default_idle(void)__attribute__((l1_text)); +void cpu_idle(void)__attribute__((l1_text)); +#endif +
A forward declaration for an inline function seems rather pointless. Moreover, marking default_idle both l1_text and inline seems contradicting, right?
include/asm-blackfin/asm-offsets.h line 6
Quoted 10 lines
diff -purN
linux-2.6-orig/include/asm-blackfin/asm-offsets.hlinux-2.6/include/asm-blackfin/asm-offsets.hinclude/asm-blackfin/asm-offsets.h +6
-0@@ -0,0 +1,89 @@ +#ifndef __ASM_OFFSETS_H__ +#define __ASM_OFFSETS_H__ +/* +* DO NOT MODIFY. +* +* This file was generated by Kbuild
This file should be in the exclude list for your diff, it is generally not shipped with the kernel sources.
include/asm-blackfin/asm-offsets.h line 10
Quoted 4 lines
include/asm-blackfin/asm-offsets.h +4
-0+#ifndef __ASSEMBLY__ + +static inline unsigned char readb (volatile unsigned char *addr) +{
The prototype for this should normally contain an __iomem. This kind of error is normally caught by running 'make C=1' to use the 'sparse' tool. If you have not run that yet, you should start to, as it finds a number of common bugs.
include/asm-blackfin/asm-offsets.h line 18
Quoted 8 lines
include/asm-blackfin/asm-offsets.h +8
-0+/* +* Map some physical address range into the kernel address space. +*/ +static inline void *__ioremap (unsigned long physaddr,unsigned long size, +int cacheflag) +{ +return (void *)physaddr; +}
Likewise, this should return an __iomem pointer.
The rest of the patch looks good to me.
Arnd <><
Quoted 16 lines · Arnd Bergmann
Review static inline void leds_switch(int flag) · line 104
Quoted 9 lines · Bryan
(quoted diff) +5
-0@@ -97,6 +97,11 @@ static inline void leds_switch(int flag) /* * The idle loop on BFIN */ +#ifdef CONFIG_IDLE_L1 +static inline void default_idle(void)__attribute__((l1_text)); +void cpu_idle(void)__attribute__((l1_text)); +#endif +A forward declaration for an inline function seems rather pointless. Moreover, marking
default_idlebothl1_textand inline seems contradicting, right?
Quoted 13 lines
include/asm-blackfin/asm-offsets.h line 6
Quoted 10 lines
diff -purN
linux-2.6-orig/include/asm-blackfin/asm-offsets.hlinux-2.6/include/asm-blackfin/asm-offsets.hinclude/asm-blackfin/asm-offsets.h +6
-0@@ -0,0 +1,89 @@ +#ifndef __ASM_OFFSETS_H__ +#define __ASM_OFFSETS_H__ +/* +* DO NOT MODIFY. +* +* This file was generated by Kbuild This file should be in the exclude list for your diff, it is generally not shipped with the kernel sources.
Got, I will fix it.
Quoted 11 lines
include/asm-blackfin/asm-offsets.h line 10
Quoted 5 lines
include/asm-blackfin/asm-offsets.h +4
-0+#ifndef __ASSEMBLY__ + +static inline unsigned char readb (volatile unsigned char *addr) +{The prototype for this should normally contain an
__iomem. This kind of error is normally caught by running 'make C=1' to use the 'sparse' tool. If you have not run that yet, you should start to, as it finds a number of common bugs.
Quoted 10 lines
include/asm-blackfin/asm-offsets.h line 18
Quoted 8 lines
include/asm-blackfin/asm-offsets.h +8
-0+/* +* Map some physical address range into the kernel address space. +*/ +static inline void *__ioremap (unsigned long physaddr,unsigned long size, +int cacheflag) +{ +return (void *)physaddr; +}Likewise, this should return an
__iomempointer.
It will be fixed as soon as possible.
Quoted 4 lines
The rest of the patch looks good to me.
Arnd <><
I sent 4 mail to LKML, but this one lost. Arnd, can you receive this email from LKML.
Thanks a lot
-Bryan Wu
Quoted 2 lines · Bryan
I sent 4 mail to LKML, but this one lost. Arnd, can you receive this email from LKML.
The mail was around 400kb, while the limit for lkml is 100kb.
Arnd <><
Review config RWSEM_XCHGADD_ALGORITHM · line 30
Quoted 12 lines · Bryan
+++ linux-2.6/arch/blackfin/Kconfig 2007-03-21 14:38:42.000000000 +0800
(quoted diff) +4
-0@@ -21,6 +21,10 @@ config RWSEM_XCHGADD_ALGORITHM bool default n +config BLACKFIN + bool + default y + config BFIN bool default y
Again, there's no reason to have both of these. Pick one and stick with it.
Quoted 3 lines
diff -purN
linux-2.6-orig/arch/blackfin/kernel/flat.clinux-2.6/arch/blackfin/kernel/flat.carch/blackfin/kernel/flat.c +0
-0
[snip]
arch/blackfin/kernel/flat.c
Quoted 8 lines
arch/blackfin/kernel/flat.c +8
-0+switch (type) { +case FLAT_BFIN_RELOC_TYPE_16_BIT : +case FLAT_BFIN_RELOC_TYPE_16H_BIT : + usptr = (unsigned short *)ptr; +#ifdef DEBUG_BFIN_RELOC +pr_info (" *usptr = %x" ,get_unaligned (usptr)); +#endif + val =get_unaligned (usptr);
The debugging here looks broken, just use pr_debug() and DEBUG as normal, then you can kill off this DEBUG_BFIN_RELOC thing.
arch/blackfin/kernel/flat.c unsigned long get_wchan(struct task_stru · line 356
Quoted 7 lines
arch/blackfin/kernel/flat.c +3
-0@@ -346,3 +351,46 @@ unsigned long get_wchan(struct task_struwhile (count++ <16 );return 0 ; } + +#if !defined(CONFIG_NO_ACCESS_CHECK ) +int _access_ok (unsigned long addr,unsigned long size)
You may want to change this to CONFIG_ACCESS_CHECK instead, it's a bit more intuitive than checking ! CONFIG_NO_ACCESS_CHECK.
arch/blackfin/mach-bf533/boards/ezkit.c line 12
Quoted 11 lines
arch/blackfin/mach-bf533/boards/ezkit.c +1
-1@@ -6,7 +6,7 @@* Created: 2005 * Description: * -* Rev: $Id: ezkit.c,v 1.27 2006/11/24 10:23:54 aubrey Exp $ +* Rev: $Id: ezkit.c 2794 2007-03-05 05:27:47Z cooloney $ * * Modified: Robin Getz <[email protected]> - Named the boards * Copyright 2005 National ICT Australia (NICTA)
The RCS tags should be killed off as well, it's clear that they don't really mean anthing, even in this case.
arch/blackfin/mach-bf533/boards/generic_board.c line 12
Quoted 12 lines
diff -purN
linux-2.6-orig/arch/blackfin/mach-bf533/boards/generic_board.clinux-2.6/arch/blackfin/mach-bf533/boards/generic_board.carch/blackfin/mach-bf533/boards/generic_board.c +1
-1@@ -6,7 +6,7 @@* Created: 2005 * Description: * -* Rev: $Id: generic_board.c,v 1.9 2006/09/23 06:35:21 vapier Exp $ +* Rev: $Id: generic_board.c 2684 2007-01-22 03:59:04Z vapier $ * * Modified: * Copyright 2005 National ICT Australia (NICTA)
An then do-nothing patches like this can be left out..
arch/blackfin/mach-bf533/boards/generic_board.c
Quoted 13 lines
arch/blackfin/mach-bf533/boards/generic_board.c +13
-0+#if defined(CONFIG_USB_ISP1760_HCD ) || defined(CONFIG_USB_ISP1760_HCD_MODULE ) +static struct resource bfin_isp1761_resources[] = { + [0 ] = { + .name ="isp1761-regs" , + .start =ISP1761_BASE +0x00000000 , + .end =ISP1761_BASE +0x000fffff , + .flags =IORESOURCE_MEM , + }, + [1 ] = { + .start =ISP1761_IRQ , + .end =ISP1761_IRQ , + .flags =IORESOURCE_IRQ |IORESOURCE_IRQ_LOWLEVEL , + },
Here you have low level.
Quoted 10 lines
arch/blackfin/mach-bf533/boards/generic_board.c +10
-0+}; + +static struct platform_device bfin_isp1761_device = { + .name ="isp1761" , + .id =0 , + .num_resources =ARRAY_SIZE (bfin_isp1761_resources), + .resource = bfin_isp1761_resources, +}; + +static struct platform_device *bfin_isp1761_devices[] = {
arch/blackfin/mach-bf533/boards/generic_board.c
Quoted 10 lines
arch/blackfin/mach-bf533/boards/generic_board.c +10
-0+ &bfin_isp1761_device, +}; + +int __init bfin_isp1761_init (void ) +{ +unsigned int num_devices=ARRAY_SIZE (bfin_isp1761_devices); + +printk (KERN_INFO "%s(): registering device resources\n" , __FUNCTION__); +set_irq_type (ISP1761_IRQ ,IRQF_TRIGGER_FALLING ); +
But here you set it to falling?
include/asm-blackfin/asm-offsets.h
Quoted 3 lines
diff -purN
linux-2.6-orig/include/asm-blackfin/asm-offsets.hlinux-2.6/include/asm-blackfin/asm-offsets.hinclude/asm-blackfin/asm-offsets.h +0
-0
This is generated by Kbuild, there's no point in including it in the diff.
include/asm-blackfin/asm-offsets.h
Quoted 3 lines
include/asm-blackfin/asm-offsets.h +1
-1- +/* CSYNC implementation for C file */ #if defined(ANOMALY_05000312 ) && defined(ANOMALY_05000244 )
Is there some reason these aren't config options? Perhaps an errata sub-menu might be more intuitive.
Quoted 4 lines
diff -purN
linux-2.6-orig/include/asm-blackfin/entry.hlinux-2.6/include/asm-blackfin/entry.h
include/asm-blackfin/entry.h line 57
Quoted 10 lines
include/asm-blackfin/entry.h +2
-4@@ -52,10 +52,8 @@#define RESTORE_ALL_SYS restore_context_no_interrupts#define RESTORE_CONTEXT restore_context_with_interrupts -#define STR (X ) STR1(X) -#define STR1 (X ) #X -# define PT_OFF_ORIG_P0 208 -# define PT_OFF_SR 8 +#define PT_OFF_ORIG_P0 208 +#define PT_OFF_SR 8
Should these be in asm-offsets?
The rest of the patch looks good, this is certainly an improvement!
Quoted 7 lines · Paul Mundt
Quoted 3 lines · Bryan
+++ linux-2.6/arch/blackfin/Kconfig 2007-03-21 14:38:42.000000000 +0800
+config BLACKFIN
config BFINAgain, there's no reason to have both of these. Pick one and stick with it.
we're working on it ... not everything has been converted yet, thus they're still both alive :/
Quoted 5 lines
Quoted 2 lines
+/* CSYNC implementation for C file */
#ifdefined(ANOMALY_05000312)&&defined(ANOMALY_05000244)Is there some reason these aren't config options? Perhaps an errata sub-menu might be more intuitive.
there is ... in the Kconfig, you select the CPU and the silicon version ... we then have a header file that determines which anomalies need to be worked around since the anomaly/silicon binding is static -mike
PATCH 1/1 Blackfin: architecture update patch
In reply to Paul MundtHi folks:
This is the latest blackfin update patch. Because there are lots of issue fixing in this one, I put all modification in one update patch which is located in:
https://blackfin.uclinux.org/gf/download/frsrelease/39/2707/blackfin-arch-2.6.21-rc4-mm1-update.patch
Changlogs:
1) fixed most of issues according to Arnd and Paul's review.
2) Remove RCS tags
3) Remove unsupported BF535 machine.
4) applied and tested based on blackfin-arch-balance-parenthesis-in-macros.patch
---
arch/blackfin/Kconfig | 173 +- arch/blackfin/Makefile | 1 arch/blackfin/kernel/Makefile | 3 arch/blackfin/kernel/asm-offsets.c | 2 arch/blackfin/kernel/bfin_dma_5xx.c | 245 +-- arch/blackfin/kernel/bfin_gpio.c | 41 arch/blackfin/kernel/bfin_ksyms.c | 2 arch/blackfin/kernel/dma-mapping.c | 25 arch/blackfin/kernel/dualcore_test.c | 2 arch/blackfin/kernel/entry.S | 6 arch/blackfin/kernel/flat.c | 101 + arch/blackfin/kernel/init_task.c | 5 arch/blackfin/kernel/irqchip.c | 7 arch/blackfin/kernel/module.c | 2 arch/blackfin/kernel/process.c | 56 arch/blackfin/kernel/ptrace.c | 41 arch/blackfin/kernel/setup.c | 79 - arch/blackfin/kernel/signal.c | 193 -- arch/blackfin/kernel/sys_bfin.c | 18 arch/blackfin/kernel/time.c | 18 arch/blackfin/kernel/traps.c | 110 - arch/blackfin/kernel/vmlinux.lds.S | 2 arch/blackfin/lib/ashldi3.c | 6 arch/blackfin/lib/ashrdi3.c | 6 arch/blackfin/lib/checksum.c | 6 arch/blackfin/lib/divsi3.S | 9 arch/blackfin/lib/gcclib.h | 2 arch/blackfin/lib/ins.S | 2 arch/blackfin/lib/lshrdi3.c | 6 arch/blackfin/lib/memchr.S | 2 arch/blackfin/lib/memcmp.S | 2 arch/blackfin/lib/memcpy.S | 7 arch/blackfin/lib/memmove.S | 2 arch/blackfin/lib/memset.S | 8 arch/blackfin/lib/modsi3.S | 9 arch/blackfin/lib/muldi3.c | 6 arch/blackfin/lib/outs.S | 2 arch/blackfin/lib/smulsi3_highpart.S | 6 arch/blackfin/lib/udivsi3.S | 11 arch/blackfin/lib/umodsi3.S | 7 arch/blackfin/lib/umulsi3_highpart.S | 6 arch/blackfin/mach-bf533/boards/cm_bf533.c | 2 arch/blackfin/mach-bf533/boards/ezkit.c | 11 arch/blackfin/mach-bf533/boards/generic_board.c | 2 arch/blackfin/mach-bf533/boards/stamp.c | 17 arch/blackfin/mach-bf533/cpu.c | 2 arch/blackfin/mach-bf533/head.S | 2 arch/blackfin/mach-bf533/ints-priority.c | 2 arch/blackfin/mach-bf537/boards/Makefile | 9 arch/blackfin/mach-bf537/boards/cm_bf537.c | 13 arch/blackfin/mach-bf537/boards/eth_mac.c | 52 arch/blackfin/mach-bf537/boards/generic_board.c | 24 arch/blackfin/mach-bf537/boards/pnav10.c | 34 arch/blackfin/mach-bf537/boards/stamp.c | 95 - arch/blackfin/mach-bf537/cpu.c | 2 arch/blackfin/mach-bf537/head.S | 3 arch/blackfin/mach-bf537/ints-priority.c | 2 arch/blackfin/mach-bf561/boards/Makefile | 1 arch/blackfin/mach-bf561/boards/cm_bf561.c | 23 arch/blackfin/mach-bf561/boards/ezkit.c | 19 arch/blackfin/mach-bf561/boards/generic_board.c | 4 arch/blackfin/mach-bf561/coreb.c | 2 arch/blackfin/mach-bf561/head.S | 2 arch/blackfin/mach-bf561/ints-priority.c | 2 arch/blackfin/mach-common/cache.S | 4 arch/blackfin/mach-common/cacheinit.S | 2 arch/blackfin/mach-common/cplbhdlr.S | 6 arch/blackfin/mach-common/cplbinfo.c | 2 arch/blackfin/mach-common/cplbmgr.S | 6 arch/blackfin/mach-common/dpmc.S | 2 arch/blackfin/mach-common/entry.S | 19 arch/blackfin/mach-common/interrupt.S | 2 arch/blackfin/mach-common/ints-priority-dc.c | 29 arch/blackfin/mach-common/ints-priority-sc.c | 28 arch/blackfin/mach-common/irqpanic.c | 2 arch/blackfin/mach-common/lock.S | 2 arch/blackfin/mach-common/pm.c | 2 arch/blackfin/mm/Makefile | 2 arch/blackfin/mm/blackfin_sram.c | 23 arch/blackfin/mm/blackfin_sram.h | 2 arch/blackfin/mm/init.c | 5 arch/blackfin/mm/kmap.c | 84 - arch/blackfin/oprofile/common.c | 16 arch/blackfin/oprofile/op_blackfin.h | 2 arch/blackfin/oprofile/op_model_bf533.c | 4 arch/blackfin/oprofile/timer_int.c | 2 include/asm-blackfin/bfin-global.h | 3 include/asm-blackfin/bfin_spi_channel.h | 182 -- include/asm-blackfin/bfin_sport.h | 16 include/asm-blackfin/blackfin.h | 87 - include/asm-blackfin/bug.h | 11 include/asm-blackfin/cacheflush.h | 3 include/asm-blackfin/cplbinit.h | 395 ++--- include/asm-blackfin/delay.h | 5 include/asm-blackfin/dma-mapping.h | 21 include/asm-blackfin/dma.h | 33 include/asm-blackfin/dpmc.h | 4 include/asm-blackfin/entry.h | 5 include/asm-blackfin/flat.h | 88 - include/asm-blackfin/io.h | 107 + include/asm-blackfin/irq.h | 2 include/asm-blackfin/irq_handler.h | 22 include/asm-blackfin/mach-bf533/bf533.h | 13 include/asm-blackfin/mach-bf533/mem_map.h | 63 include/asm-blackfin/mach-bf535/bf535.h | 1277 ---------------- include/asm-blackfin/mach-bf535/bf535_serial.h | 109 - include/asm-blackfin/mach-bf535/blackfin.h | 43 include/asm-blackfin/mach-bf535/cdefBF535.h | 121 - include/asm-blackfin/mach-bf535/cdefblackfin.h | 69 include/asm-blackfin/mach-bf535/defBF535.h | 1818 ------------------------ include/asm-blackfin/mach-bf535/defblackfin.h | 444 ----- include/asm-blackfin/mach-bf535/irq.h | 125 - include/asm-blackfin/mach-bf537/bf537.h | 13 include/asm-blackfin/mach-bf537/defBF534.h | 3 include/asm-blackfin/mach-bf537/mem_map.h | 79 - include/asm-blackfin/mach-bf561/bf561.h | 7 include/asm-blackfin/mach-bf561/mem_map.h | 29 include/asm-blackfin/pgtable.h | 8 include/asm-blackfin/sigcontext.h | 25 include/asm-blackfin/uaccess.h | 58 include/asm-blackfin/ucontext.h | 15 include/asm-blackfin/unistd.h | 2 init/Kconfig | 3 123 files changed, 1518 insertions(+), 5598 deletions(-)
Thanks
-Bryan Wu
Quoted 5 lines · Bryan
This is the latest blackfin update patch. Because there are lots of issue fixing in this one, I put all modification in one update patch which is located in:
https://blackfin.uclinux.org/gf/download/frsrelease/39/2707/blackfin-arch-2.6.21-rc4-mm1-update.patch
I hope these will split up logically in the future so it's possible to reply to them without having to do manual mangling..
The patch generally looks fine, this is the only thing that really jumped out:
Quoted 7 lines
Index:
linux-2.6/include/asm-blackfin/pgtable.h
===================================================================include/asm-blackfin/pgtable.h +0
-0@@ -59,4 +59,12 @@#define VMALLOC_START 0#define VMALLOC_END 0xffffffff
include/asm-blackfin/pgtable.h line 70
Quoted 10 lines
include/asm-blackfin/pgtable.h +8
-0+#define __HAVE_ARCH_ENTER_LAZY_CPU_MODE +#define arch_enter_lazy_cpu_mode () do {} while (0) +#define arch_leave_lazy_cpu_mode () do {} while (0) + +#define __HAVE_ARCH_ENTER_LAZY_MMU_MODE +#define arch_enter_lazy_mmu_mode () do {} while (0) +#define arch_leave_lazy_mmu_mode () do {} while (0) +#endif /* _BLACKFIN_PGTABLE_H */
asm-generic/pgtable.h already does this if you don't explicitly define __HAVE_ARCH_ENTER_LAZY_{CPU,MMU}_MODE. So please kill this entirely. If you forgot to include asm-generic/pgtable.h, that's another matter..
Quoted 23 lines · Paul Mundt
Quoted 5 lines · Bryan
This is the latest blackfin update patch. Because there are lots of issue fixing in this one, I put all modification in one update patch which is located in:
https://blackfin.uclinux.org/gf/download/frsrelease/39/2707/blackfin-arch-2.6.21-rc4-mm1-update.patchI hope these will split up logically in the future so it's possible to reply to them without having to do manual mangling..
From now on, I will follow this rule. Thanks
The patch generally looks fine, this is the only thing that really jumped out:
Quoted 8 lines
Index:
linux-2.6/include/asm-blackfin/pgtable.h
===================================================================include/asm-blackfin/pgtable.h +0
-0@@ -59,4 +59,12 @@#define VMALLOC_START 0#define VMALLOC_END 0xffffffff
Quoted 13 lines
include/asm-blackfin/pgtable.h line 70
Quoted 9 lines
include/asm-blackfin/pgtable.h +8
-0+#define __HAVE_ARCH_ENTER_LAZY_CPU_MODE +#define arch_enter_lazy_cpu_mode () do {} while (0) +#define arch_leave_lazy_cpu_mode () do {} while (0) + +#define __HAVE_ARCH_ENTER_LAZY_MMU_MODE +#define arch_enter_lazy_mmu_mode () do {} while (0) +#define arch_leave_lazy_mmu_mode () do {} while (0) +#endif /* _BLACKFIN_PGTABLE_H */
asm-generic/pgtable.halready does this if you don't explicitly define __HAVE_ARCH_ENTER_LAZY_{CPU,MMU}_MODE. So please kill this entirely. If you forgot to includeasm-generic/pgtable.h, that's another matter..
OK, I will check with this. Adding this dummy function is because compiling will fail.
Best Regards,
-Bryan Wu
Quoted 1 line · Bryan
This is the latest blackfin update patch.
I think I'm going to give up on the present set of blackfin patches. I don't know whether what I have is up-to-date and various versions of various random patches keep on flying past.
So I think it's best if I drop everything and, when people have finished reviewing everything, you resend it all.
Quoted 8 lines · Andrew Morton
Quoted 1 line · Bryan
This is the latest blackfin update patch.
I think I'm going to give up on the present set of blackfin patches. I don't know whether what I have is up-to-date and various versions of various random patches keep on flying past.
Sorry for this mess up. You know, this is a big patch. After Arnd and Paul's review, it took lots of time to fix their request. But I think this update patch is the patch to satisfy Arnd and Paul's review and they almost think this one is fine to add to -mm.
Quoted 2 lines
So I think it's best if I drop everything and, when people have finished reviewing everything, you resend it all.
If this one is merged into -mm, I will follow the rule to send small and fixed patches. But you know incremental patch should have a base. I hope you can add this to -mm. And if Arnd, Paul or other kernel maintainers find some issues, we fix them and send small patches one by one.
Thanks Andrew.
-Bryan
Quoted 4 lines · Bryan
1) Some issues are fixed according to LKML patch review.
2) Remove not supported BF535 code
3) Fixed some bugs from blackfin.uclinux.org SVN update Here is the updated patch for 2.6.21-rc4-mm1
One rather general but important comment:
You need to get used to providing smaller, one fix per mail, patches. As long as the full tree is waiting in -mm, this is not as important, as I assume that Andrew will fold the whole architecture support into a big patch before submitting to Linus, but as soon as it's in, such big patches will not be acceptable any more.
If you're not already doing it, look into how 'quilt' or similar tools help you with this.
Arnd <><