[U-Boot] [PATCH 0/3] OMAP3: Minor code clean up, no functional change

This patch series is an attempt (*) to help with [1] and the first part of [2].
It contains only minor code clean up, no functional changes.
Complete patch series boot tested on OMAP3 BeagleBoard and compile tested with ./MAKEALL ARM_CORTEX_A8.
I'm unsure about two topics:
* The initial ideas and patches for these changes are all from Jean-Christophe (*). So I added Jean-Christophe's From: and Signed-off-by:. If this is wrong, please correct.
* Patch 2/3 renames interrupts.c to timer.c. In git this can be done easier by just rename. But for testing and sending it as patch to be applied I had to do it this way. Maybe the rename could be done easier while applying the patch to git?
Best regards
Dirk
(*) Jean-Christophe told me that he is busy, hopefully I can help a little with this.
[1] http://lists.denx.de/pipermail/u-boot/2009-May/051948.html
[2] http://lists.denx.de/pipermail/u-boot/2009-May/051949.html

Remove duplicated interrupt code. Original, identical code can be found in lib_arm/interrupts.c
From: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com --- cpu/arm_cortexa8/omap3/interrupts.c | 128 ------------------------------------ 1 file changed, 128 deletions(-)
Index: u-boot-main/cpu/arm_cortexa8/omap3/interrupts.c =================================================================== --- u-boot-main.orig/cpu/arm_cortexa8/omap3/interrupts.c +++ u-boot-main/cpu/arm_cortexa8/omap3/interrupts.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;

After removal of dublicated interrupt code, rename file to what it really does now.
From: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com --- cpu/arm_cortexa8/omap3/Makefile | 2 cpu/arm_cortexa8/omap3/interrupts.c | 138 ------------------------------------ cpu/arm_cortexa8/omap3/timer.c | 138 ++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 139 deletions(-)
Index: u-boot-main/cpu/arm_cortexa8/omap3/Makefile =================================================================== --- u-boot-main.orig/cpu/arm_cortexa8/omap3/Makefile +++ u-boot-main/cpu/arm_cortexa8/omap3/Makefile @@ -26,7 +26,7 @@ include $(TOPDIR)/config.mk LIB = $(obj)lib$(SOC).a
SOBJS := lowlevel_init.o -COBJS := sys_info.o board.o clock.o interrupts.o mem.o syslib.o +COBJS := sys_info.o board.o clock.o timer.o mem.o syslib.o
SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS)) Index: u-boot-main/cpu/arm_cortexa8/omap3/timer.c =================================================================== --- /dev/null +++ u-boot-main/cpu/arm_cortexa8/omap3/timer.c @@ -0,0 +1,138 @@ +/* + * (C) Copyright 2008 + * Texas Instruments + * + * Richard Woodruff r-woodruff2@ti.com + * Syed Moahmmed Khasim khasim@ti.com + * + * (C) Copyright 2002 + * Sysgo Real-Time Solutions, GmbH <www.elinos.com> + * Marius Groeger mgroeger@sysgo.de + * Alex Zuepke azu@sysgo.de + * + * (C) Copyright 2002 + * Gary Jennejohn, DENX Software Engineering, gj@denx.de + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include <common.h> +#include <asm/io.h> + +static ulong timestamp; +static ulong lastinc; +static gptimer_t *timer_base = (gptimer_t *)CONFIG_SYS_TIMERBASE; + +/* + * Nothing really to do with interrupts, just starts up a counter. + * We run the counter with 13MHz, divided by 8, resulting in timer + * frequency of 1.625MHz. With 32bit counter register, counter + * overflows in ~44min + */ + +/* 13MHz / 8 = 1.625MHz */ +#define TIMER_CLOCK (V_SCLK / (2 << CONFIG_SYS_PTV)) +#define TIMER_LOAD_VAL 0xffffffff + +int interrupt_init(void) +{ + /* start the counter ticking up, reload value on overflow */ + writel(TIMER_LOAD_VAL, &timer_base->tldr); + /* enable timer */ + writel((CONFIG_SYS_PTV << 2) | TCLR_PRE | TCLR_AR | TCLR_ST, + &timer_base->tclr); + + reset_timer_masked(); /* init the timestamp and lastinc value */ + + return 0; +} + +/* + * timer without interrupts + */ +void reset_timer(void) +{ + reset_timer_masked(); +} + +ulong get_timer(ulong base) +{ + return get_timer_masked() - base; +} + +void set_timer(ulong t) +{ + timestamp = t; +} + +/* delay x useconds */ +void udelay(unsigned long usec) +{ + long tmo = usec * (TIMER_CLOCK / 1000) / 1000; + unsigned long now, last = readl(&timer_base->tcrr); + + while (tmo > 0) { + now = readl(&timer_base->tcrr); + if (last > now) /* count up timer overflow */ + tmo -= TIMER_LOAD_VAL - last + now; + else + tmo -= now - last; + last = now; + } +} + +void reset_timer_masked(void) +{ + /* reset time, capture current incrementer value time */ + lastinc = readl(&timer_base->tcrr) / (TIMER_CLOCK / CONFIG_SYS_HZ); + timestamp = 0; /* start "advancing" time stamp from 0 */ +} + +ulong get_timer_masked(void) +{ + /* current tick value */ + ulong now = readl(&timer_base->tcrr) / (TIMER_CLOCK / CONFIG_SYS_HZ); + + if (now >= lastinc) /* normal mode (non roll) */ + /* move stamp fordward with absoulte diff ticks */ + timestamp += (now - lastinc); + else /* we have rollover of incrementer */ + timestamp += ((TIMER_LOAD_VAL / (TIMER_CLOCK / CONFIG_SYS_HZ)) + - lastinc) + now; + lastinc = now; + return timestamp; +} + +/* + * This function is derived from PowerPC code (read timebase as long long). + * On ARM it just returns the timer value. + */ +unsigned long long get_ticks(void) +{ + return get_timer(0); +} + +/* + * This function is derived from PowerPC code (timebase clock frequency). + * On ARM it returns the number of timer ticks per second. + */ +ulong get_tbclk(void) +{ + return CONFIG_SYS_HZ; +} Index: u-boot-main/cpu/arm_cortexa8/omap3/interrupts.c =================================================================== --- u-boot-main.orig/cpu/arm_cortexa8/omap3/interrupts.c +++ /dev/null @@ -1,138 +0,0 @@ -/* - * (C) Copyright 2008 - * Texas Instruments - * - * Richard Woodruff r-woodruff2@ti.com - * Syed Moahmmed Khasim khasim@ti.com - * - * (C) Copyright 2002 - * Sysgo Real-Time Solutions, GmbH <www.elinos.com> - * Marius Groeger mgroeger@sysgo.de - * Alex Zuepke azu@sysgo.de - * - * (C) Copyright 2002 - * Gary Jennejohn, DENX Software Engineering, gj@denx.de - * - * See file CREDITS for list of people who contributed to this - * project. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; either version 2 of - * the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, - * MA 02111-1307 USA - */ - -#include <common.h> -#include <asm/io.h> - -static ulong timestamp; -static ulong lastinc; -static gptimer_t *timer_base = (gptimer_t *)CONFIG_SYS_TIMERBASE; - -/* - * Nothing really to do with interrupts, just starts up a counter. - * We run the counter with 13MHz, divided by 8, resulting in timer - * frequency of 1.625MHz. With 32bit counter register, counter - * overflows in ~44min - */ - -/* 13MHz / 8 = 1.625MHz */ -#define TIMER_CLOCK (V_SCLK / (2 << CONFIG_SYS_PTV)) -#define TIMER_LOAD_VAL 0xffffffff - -int interrupt_init(void) -{ - /* start the counter ticking up, reload value on overflow */ - writel(TIMER_LOAD_VAL, &timer_base->tldr); - /* enable timer */ - writel((CONFIG_SYS_PTV << 2) | TCLR_PRE | TCLR_AR | TCLR_ST, - &timer_base->tclr); - - reset_timer_masked(); /* init the timestamp and lastinc value */ - - return 0; -} - -/* - * timer without interrupts - */ -void reset_timer(void) -{ - reset_timer_masked(); -} - -ulong get_timer(ulong base) -{ - return get_timer_masked() - base; -} - -void set_timer(ulong t) -{ - timestamp = t; -} - -/* delay x useconds */ -void udelay(unsigned long usec) -{ - long tmo = usec * (TIMER_CLOCK / 1000) / 1000; - unsigned long now, last = readl(&timer_base->tcrr); - - while (tmo > 0) { - now = readl(&timer_base->tcrr); - if (last > now) /* count up timer overflow */ - tmo -= TIMER_LOAD_VAL - last + now; - else - tmo -= now - last; - last = now; - } -} - -void reset_timer_masked(void) -{ - /* reset time, capture current incrementer value time */ - lastinc = readl(&timer_base->tcrr) / (TIMER_CLOCK / CONFIG_SYS_HZ); - timestamp = 0; /* start "advancing" time stamp from 0 */ -} - -ulong get_timer_masked(void) -{ - /* current tick value */ - ulong now = readl(&timer_base->tcrr) / (TIMER_CLOCK / CONFIG_SYS_HZ); - - if (now >= lastinc) /* normal mode (non roll) */ - /* move stamp fordward with absoulte diff ticks */ - timestamp += (now - lastinc); - else /* we have rollover of incrementer */ - timestamp += ((TIMER_LOAD_VAL / (TIMER_CLOCK / CONFIG_SYS_HZ)) - - lastinc) + now; - lastinc = now; - return timestamp; -} - -/* - * This function is derived from PowerPC code (read timebase as long long). - * On ARM it just returns the timer value. - */ -unsigned long long get_ticks(void) -{ - return get_timer(0); -} - -/* - * This function is derived from PowerPC code (timebase clock frequency). - * On ARM it returns the number of timer ticks per second. - */ -ulong get_tbclk(void) -{ - return CONFIG_SYS_HZ; -}

Reformat COBJS handling.
From: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com Signed-off-by: Dirk Behme dirk.behme@googlemail.com --- cpu/arm_cortexa8/omap3/Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Index: u-boot-main/cpu/arm_cortexa8/omap3/Makefile =================================================================== --- u-boot-main.orig/cpu/arm_cortexa8/omap3/Makefile +++ u-boot-main/cpu/arm_cortexa8/omap3/Makefile @@ -26,7 +26,13 @@ include $(TOPDIR)/config.mk LIB = $(obj)lib$(SOC).a
SOBJS := lowlevel_init.o -COBJS := sys_info.o board.o clock.o timer.o mem.o syslib.o + +COBJS += board.o +COBJS += clock.o +COBJS += mem.o +COBJS += syslib.o +COBJS += sys_info.o +COBJS += timer.o
SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS))

Dear Jean-Christophe,
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 17:41 Tue 05 May , Dirk Behme wrote:
Reformat COBJS handling.
please rebase against you first patch as the 2 will be handle by an otherone
Sorry, but I don't understand what you want here. All three patches are fine in this order and apply cleanly on each other. What rebase will be neccesary?
Best regards
Dirk

On 17:41 Tue 05 May , Dirk Behme wrote:
After removal of dublicated interrupt code, rename file to what it really does now.
I prefer to rename all files at the same time
Btw please use git to show that you only rename the file without change
Best Regards, J.

Dear Jean-Christophe,
Jean-Christophe PLAGNIOL-VILLARD wrote:
On 17:41 Tue 05 May , Dirk Behme wrote:
After removal of dublicated interrupt code, rename file to what it really does now.
I prefer to rename all files at the same time
Then, please don't complain that you have "to do everything on your own and nobody helps you" ;) I would prefer that changes are done by the board maintainers and not by the custodian, though.
Btw please use git to show that you only rename the file without change
I'm not sure that this works for non-custodians. Custodians can do git-rename and then send a patch to the mailing list. When a custodian sends such a patch, it is mainly for review only, and not to be applied somewhere (because it is already in git).
This results then in something
cpu/sa1100/{interrupts.c => timer.c} | 5 +- rename cpu/arm925t/{interrupts.c => timer.c} (99%)
(from [1]). But this is only what normally 'diffstat' generates and what is totally ignored by 'patch' . I.e. in the part of the patch which is handled by 'patch' this info is totally missing. As far as I know 'patch' itself can't deal with file rename any other way than remove one file and create the other one with
--- u-boot-main.orig/cpu/arm_cortexa8/omap3/interrupts.c +++ /dev/null
--- /dev/null +++ u-boot-main/cpu/arm_cortexa8/omap3/timer.c
So it's my understanding that non-custodians have to send a patch which can be applied everywhere by everyone by 'patch' utility, i.e. the part 'patch' deals with contains all information necessary.
Do I miss anything?
Best regards
Dirk
[1] http://lists.denx.de/pipermail/u-boot/2009-May/051921.html

On Fri, May 08, 2009 at 05:47:27PM +0200, Dirk Behme wrote:
I'm not sure that this works for non-custodians. Custodians can do git-rename and then send a patch to the mailing list. When a custodian sends such a patch, it is mainly for review only, and not to be applied somewhere (because it is already in git).
This results then in something
cpu/sa1100/{interrupts.c => timer.c} | 5 +- rename cpu/arm925t/{interrupts.c => timer.c} (99%)
(from [1]). But this is only what normally 'diffstat' generates and what is totally ignored by 'patch' .
It can be applied with "git am" (well, not the diffstat, but the special rename diff hunk that git also generates).
-Scott

Scott Wood wrote:
On Fri, May 08, 2009 at 05:47:27PM +0200, Dirk Behme wrote:
I'm not sure that this works for non-custodians. Custodians can do git-rename and then send a patch to the mailing list. When a custodian sends such a patch, it is mainly for review only, and not to be applied somewhere (because it is already in git).
This results then in something
cpu/sa1100/{interrupts.c => timer.c} | 5 +- rename cpu/arm925t/{interrupts.c => timer.c} (99%)
(from [1]). But this is only what normally 'diffstat' generates and what is totally ignored by 'patch' .
It can be applied with "git am" (well, not the diffstat, but the special rename diff hunk that git also generates).
Ok, thanks. But not with other tools, like plain 'patch', correct?
Best regards
Dirk

Dirk Behme wrote:
Scott Wood wrote:
It can be applied with "git am" (well, not the diffstat, but the special rename diff hunk that git also generates).
Ok, thanks. But not with other tools, like plain 'patch', correct?
Right, until someone fixes that. :-)
Which I suppose could be a problem if "git am" refuses to accept the patch and the normal patch command needs to be used to resolve the conflict (I really wish git would treat it more like a merge from a UI perspective, even if it doesn't look that way internally).
-Scott

Dear Dirk,
In message 4A047B71.6080704@googlemail.com you wrote:
This results then in something
cpu/sa1100/{interrupts.c => timer.c} | 5 +- rename cpu/arm925t/{interrupts.c => timer.c} (99%)
(from [1]). But this is only what normally 'diffstat' generates and what is totally ignored by 'patch' .
It can be applied with "git am" (well, not the diffstat, but the special rename diff hunk that git also generates).
Ok, thanks. But not with other tools, like plain 'patch', correct?
Indeed. But why would you use such a tool in combination with a git repository? It makes no sense.
Best regards,
Wolfgang Denk

Dear Dirk,
In message 4A04540F.10507@googlemail.com you wrote:
Btw please use git to show that you only rename the file without change
I'm not sure that this works for non-custodians. Custodians can do git-rename and then send a patch to the mailing list. When a custodian sends such a patch, it is mainly for review only, and not to be applied somewhere (because it is already in git).
In which way is this rtestricted to custodians? everybody can do the same in his own git repositories and use this to provide patches and statistics.
(from [1]). But this is only what normally 'diffstat' generates and what is totally ignored by 'patch' . I.e. in the part of the patch
We don't use plain old "patch", we use git tools (like "git am"). And these do understand about rename patches.
which is handled by 'patch' this info is totally missing. As far as I know 'patch' itself can't deal with file rename any other way than remove one file and create the other one with
That's why we use git.
So it's my understanding that non-custodians have to send a patch which can be applied everywhere by everyone by 'patch' utility, i.e. the part 'patch' deals with contains all information necessary.
Do I miss anything?
Yes, you are missing that we use git these days, and git is more powerful.
Best regards,
Wolfgang Denk
participants (4)
-
Dirk Behme
-
Jean-Christophe PLAGNIOL-VILLARD
-
Scott Wood
-
Wolfgang Denk