[U-Boot] [PATCH 1/1] omap3: remove copy&paste global arm interrupts code

Dirk Behme dirk.behme at googlemail.com
Sat May 2 08:36:46 CEST 2009


Dear Jean-Christophe,

unfortunately, it's my feeling that this patch is impolite and makes 
me angry :(

I know, there should be no top posting, but hopefully it helps to make 
it clearer:

You tried this already! I asked several (*) times to just give us the 
file name where you think the first/original/copy source code of this 
is so that we can review if there are any differences. There was no 
answer for this filename question. It was then removed from pull 
request (what was fine). And now you try it again without any further 
comments :(

(*) I can find ~4 (!) requests for the filename (complete history in 
brackets):

( http://lists.denx.de/pipermail/u-boot/2009-March/049736.html )
http://lists.denx.de/pipermail/u-boot/2009-March/049758.html
( http://lists.denx.de/pipermail/u-boot/2009-March/049761.html )
http://lists.denx.de/pipermail/u-boot/2009-March/049762.html
( http://lists.denx.de/pipermail/u-boot/2009-March/049815.html )
( http://lists.denx.de/pipermail/u-boot/2009-March/049859.html )
( http://lists.denx.de/pipermail/u-boot/2009-March/049858.html )
( http://lists.denx.de/pipermail/u-boot/2009-March/049861.html )
http://lists.denx.de/pipermail/u-boot/2009-March/049883.html
( http://lists.denx.de/pipermail/u-boot/2009-March/049887.html )
http://lists.denx.de/pipermail/u-boot/2009-March/049943.html
( http://lists.denx.de/pipermail/u-boot/2009-April/050133.html )

To make it clear: I'm not basically against the removal of this code. 
But we have to review it first with the first/original/copy source 
code to be sure there are no differences and we don't remove any 
valuable code.

What's so difficult to just mention "original code is in file xxx"?

Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj at jcrosoft.com>
> ---
>  cpu/arm_cortexa8/omap3/timer.c |  128 ----------------------------------------
>  1 files changed, 0 insertions(+), 128 deletions(-)
> 
> diff --git a/cpu/arm_cortexa8/omap3/timer.c b/cpu/arm_cortexa8/omap3/timer.c
> index e99b149..a73916c 100644
> --- a/cpu/arm_cortexa8/omap3/timer.c
> +++ b/cpu/arm_cortexa8/omap3/timer.c

There is no file "cpu/arm_cortexa8/omap3/timer.c". It seems to me that 
you base this patch on an other one without mentioning it.

But this is just a minor issue in contrast to the above :(

Dirk

> @@ -34,134 +34,6 @@
>  
>  #include <common.h>
>  #include <asm/io.h>
> -#include <asm/proc-armv/ptrace.h>
> -
> -#ifdef CONFIG_USE_IRQ
> -/* enable IRQ interrupts */
> -void enable_interrupts(void)
> -{
> -	unsigned long temp;
> -	__asm__ __volatile__("mrs %0, cpsr\n"
> -			     "bic %0, %0, #0x80\n" "msr cpsr_c, %0":"=r"(temp)
> -			     ::"memory");
> -}
> -
> -/*
> - * disable IRQ/FIQ interrupts
> - * returns true if interrupts had been enabled before we disabled them
> - */
> -int disable_interrupts(void)
> -{
> -	unsigned long old, temp;
> -	__asm__ __volatile__("mrs %0, cpsr\n"
> -			     "orr %1, %0, #0xc0\n"
> -			     "msr cpsr_c, %1":"=r"(old), "=r"(temp)
> -			     ::"memory");
> -	return (old & 0x80) == 0;
> -}
> -#else
> -void enable_interrupts(void)
> -{
> -	return;
> -}
> -int disable_interrupts(void)
> -{
> -	return 0;
> -}
> -#endif
> -
> -void bad_mode(void)
> -{
> -	panic("Resetting CPU ...\n");
> -	reset_cpu(0);
> -}
> -
> -void show_regs(struct pt_regs *regs)
> -{
> -	unsigned long flags;
> -	const char *processor_modes[] = {
> -		"USER_26", "FIQ_26", "IRQ_26", "SVC_26",
> -		"UK4_26", "UK5_26", "UK6_26", "UK7_26",
> -		"UK8_26", "UK9_26", "UK10_26", "UK11_26",
> -		"UK12_26", "UK13_26", "UK14_26", "UK15_26",
> -		"USER_32", "FIQ_32", "IRQ_32", "SVC_32",
> -		"UK4_32", "UK5_32", "UK6_32", "ABT_32",
> -		"UK8_32", "UK9_32", "UK10_32", "UND_32",
> -		"UK12_32", "UK13_32", "UK14_32", "SYS_32",
> -	};
> -
> -	flags = condition_codes(regs);
> -
> -	printf("pc : [<%08lx>]    lr : [<%08lx>]\n"
> -		"sp : %08lx  ip : %08lx  fp : %08lx\n",
> -		instruction_pointer(regs),
> -		regs->ARM_lr, regs->ARM_sp, regs->ARM_ip, regs->ARM_fp);
> -	printf("r10: %08lx  r9 : %08lx  r8 : %08lx\n",
> -		regs->ARM_r10, regs->ARM_r9, regs->ARM_r8);
> -	printf("r7 : %08lx  r6 : %08lx  r5 : %08lx  r4 : %08lx\n",
> -		regs->ARM_r7, regs->ARM_r6, regs->ARM_r5, regs->ARM_r4);
> -	printf("r3 : %08lx  r2 : %08lx  r1 : %08lx  r0 : %08lx\n",
> -		regs->ARM_r3, regs->ARM_r2, regs->ARM_r1, regs->ARM_r0);
> -	printf("Flags: %c%c%c%c",
> -		flags & CC_N_BIT ? 'N' : 'n',
> -		flags & CC_Z_BIT ? 'Z' : 'z',
> -		flags & CC_C_BIT ? 'C' : 'c', flags & CC_V_BIT ? 'V' : 'v');
> -	printf("  IRQs %s  FIQs %s  Mode %s%s\n",
> -		interrupts_enabled(regs) ? "on" : "off",
> -		fast_interrupts_enabled(regs) ? "on" : "off",
> -		processor_modes[processor_mode(regs)],
> -		thumb_mode(regs) ? " (T)" : "");
> -}
> -
> -void do_undefined_instruction(struct pt_regs *pt_regs)
> -{
> -	printf("undefined instruction\n");
> -	show_regs(pt_regs);
> -	bad_mode();
> -}
> -
> -void do_software_interrupt(struct pt_regs *pt_regs)
> -{
> -	printf("software interrupt\n");
> -	show_regs(pt_regs);
> -	bad_mode();
> -}
> -
> -void do_prefetch_abort(struct pt_regs *pt_regs)
> -{
> -	printf("prefetch abort\n");
> -	show_regs(pt_regs);
> -	bad_mode();
> -}
> -
> -void do_data_abort(struct pt_regs *pt_regs)
> -{
> -	printf("data abort\n");
> -	show_regs(pt_regs);
> -	bad_mode();
> -}
> -
> -void do_not_used(struct pt_regs *pt_regs)
> -{
> -	printf("not used\n");
> -	show_regs(pt_regs);
> -	bad_mode();
> -}
> -
> -void do_fiq(struct pt_regs *pt_regs)
> -{
> -	printf("fast interrupt request\n");
> -	show_regs(pt_regs);
> -	bad_mode();
> -}
> -
> -void do_irq(struct pt_regs *pt_regs)
> -{
> -	printf("interrupt request\n");
> -	show_regs(pt_regs);
> -	bad_mode();
> -}
> -
>  
>  static ulong timestamp;
>  static ulong lastinc;



More information about the U-Boot mailing list