* [U-Boot] [PATCH 1/1] omap3: remove copy&paste global arm interrupts code
@ 2009-05-01 23:09 Jean-Christophe PLAGNIOL-VILLARD
2009-05-02 6:36 ` Dirk Behme
0 siblings, 1 reply; 3+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-05-01 23:09 UTC (permalink / raw)
To: u-boot
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@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
@@ -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;
--
1.6.1.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH 1/1] omap3: remove copy&paste global arm interrupts code
2009-05-01 23:09 [U-Boot] [PATCH 1/1] omap3: remove copy&paste global arm interrupts code Jean-Christophe PLAGNIOL-VILLARD
@ 2009-05-02 6:36 ` Dirk Behme
2009-05-02 12:44 ` Dirk Behme
0 siblings, 1 reply; 3+ messages in thread
From: Dirk Behme @ 2009-05-02 6:36 UTC (permalink / raw)
To: u-boot
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@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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH 1/1] omap3: remove copy&paste global arm interrupts code
2009-05-02 6:36 ` Dirk Behme
@ 2009-05-02 12:44 ` Dirk Behme
0 siblings, 0 replies; 3+ messages in thread
From: Dirk Behme @ 2009-05-02 12:44 UTC (permalink / raw)
To: u-boot
Dear Jean-Christophe,
Dirk Behme wrote:
> 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"?
After some guessing, it seems to me that the the never given answer
might be "Please have a look to the interrupt implementation in file
lib_arm/interrupts.c".
Looking at lib_arm/interrupts.c, yes you are right, the code is
identical. With this, we can be sure that (TI's) copy & paste doesn't
add some valuable code to cpu/arm_cortexa8/omap3/interrupts.c.
Do you like to resend this as independent patch, doing the removal at
cpu/arm_cortexa8/omap3/interrupts.c (and not non-existing timer.c
like below)? After this, the rename of
cpu/arm_cortexa8/omap3/interrupts.c to cpu/arm_cortexa8/omap3/timer.c
will be fine, too.
So two patches, the first with the removal and the second with rename
(and Makefile adaption for this) would be nice.
As git commit message for the first patch I propose something like
"Remove duplicated interrupt code. Original, identical code can be
found in lib_arm/interrupts.c".
Do you have an OMAP3 based board for testing? If not, let me know, I
can test the both patches then.
Best regards
Dirk
> Jean-Christophe PLAGNIOL-VILLARD wrote:
>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@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;
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-05-02 12:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-01 23:09 [U-Boot] [PATCH 1/1] omap3: remove copy&paste global arm interrupts code Jean-Christophe PLAGNIOL-VILLARD
2009-05-02 6:36 ` Dirk Behme
2009-05-02 12:44 ` Dirk Behme
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox