[U-Boot] [PATCH 01/12] microblaze: board: Remove compilation warning
Variable is used when CONFIG_SYS_FLASH_CHECKSUM is used.
Warning log: board.c: In function 'board_init': board.c:101: warning: unused variable 's'
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/lib/board.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index a8ed7ce..4146f5c 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -98,7 +98,6 @@ void board_init (void) gd = (gd_t *) (CONFIG_SYS_SDRAM_BASE + CONFIG_SYS_GBL_DATA_OFFSET); bd = (bd_t *) (CONFIG_SYS_SDRAM_BASE + CONFIG_SYS_GBL_DATA_OFFSET \ - GENERATED_BD_INFO_SIZE); - char *s; #if defined(CONFIG_CMD_FLASH) ulong flash_size = 0; #endif @@ -157,9 +156,9 @@ void board_init (void) puts ("Flash: "); bd->bi_flashstart = CONFIG_SYS_FLASH_BASE; if (0 < (flash_size = flash_init ())) { - bd->bi_flashsize = flash_size; - bd->bi_flashoffset = CONFIG_SYS_FLASH_BASE + flash_size; # ifdef CONFIG_SYS_FLASH_CHECKSUM + char *s; + print_size (flash_size, ""); /* * Compute and print flash CRC if flashchecksum is set to 'y' @@ -176,6 +175,8 @@ void board_init (void) # else /* !CONFIG_SYS_FLASH_CHECKSUM */ print_size (flash_size, "\n"); # endif /* CONFIG_SYS_FLASH_CHECKSUM */ + bd->bi_flashsize = flash_size; + bd->bi_flashoffset = CONFIG_SYS_FLASH_BASE + flash_size; } else { puts ("Flash init FAILED"); bd->bi_flashstart = 0;
eth_init() is defined at include/net.h.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/lib/board.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index 4146f5c..1cf895b 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -42,10 +42,6 @@ extern int gpio_init (void); #ifdef CONFIG_SYS_INTC_0 extern int interrupts_init (void); #endif - -#if defined(CONFIG_CMD_NET) -extern int eth_init (bd_t * bis); -#endif #ifdef CONFIG_SYS_TIMER_0 extern int timer_init (void); #endif
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek monstr@monstr.eu wrote:
eth_init() is defined at include/net.h.
Signed-off-by: Michal Simek monstr@monstr.eu
Acked-by: Simon Glass sjg@chromium.org
arch/microblaze/lib/board.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index 4146f5c..1cf895b 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -42,10 +42,6 @@ extern int gpio_init (void); #ifdef CONFIG_SYS_INTC_0 extern int interrupts_init (void); #endif
-#if defined(CONFIG_CMD_NET) -extern int eth_init (bd_t * bis); -#endif #ifdef CONFIG_SYS_TIMER_0 extern int timer_init (void);
#endif
1.7.0.4
On 07/09/2012 11:10 PM, Simon Glass wrote:
Acked-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
Applied.
Thanks, Michal
Clear and prepare for device-tree driven configuration. Remove CONFIG_SYS_INTC_0 definition Use dynamic allocation instead of static.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/interrupts.c | 88 ++++++++++++++----------- arch/microblaze/cpu/start.S | 2 - arch/microblaze/cpu/timer.c | 2 - arch/microblaze/include/asm/microblaze_intc.h | 3 + arch/microblaze/lib/board.c | 6 +-- include/configs/microblaze-generic.h | 1 - 6 files changed, 54 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index e7ca859..ee67082 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -26,6 +26,7 @@
#include <common.h> #include <command.h> +#include <malloc.h> #include <asm/microblaze_intc.h> #include <asm/asm.h>
@@ -48,20 +49,19 @@ int disable_interrupts (void) return (msr & 0x2) != 0; }
-#ifdef CONFIG_SYS_INTC_0 - -static struct irq_action vecs[CONFIG_SYS_INTC_0_NUM]; +static struct irq_action *vecs; +static u32 irq_no;
/* mapping structure to interrupt controller */ -microblaze_intc_t *intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); +microblaze_intc_t *intc;
/* default handler */ -void def_hdlr (void) +static void def_hdlr(void) { puts ("def_hdlr\n"); }
-void enable_one_interrupt (int irq) +static void enable_one_interrupt(int irq) { int mask; int offset = 1; @@ -76,7 +76,7 @@ void enable_one_interrupt (int irq) #endif }
-void disable_one_interrupt (int irq) +static void disable_one_interrupt(int irq) { int mask; int offset = 1; @@ -96,7 +96,7 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) { struct irq_action *act; /* irq out of range */ - if ((irq < 0) || (irq > CONFIG_SYS_INTC_0_NUM)) { + if ((irq < 0) || (irq > irq_no)) { puts ("IRQ out of range\n"); return; } @@ -114,7 +114,7 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) }
/* initialization interrupt controller - hardware */ -void intc_init (void) +static void intc_init(void) { intc->mer = 0; intc->ier = 0; @@ -127,18 +127,33 @@ void intc_init (void) #endif }
-int interrupts_init (void) +int interrupts_init(void) { int i; - /* initialize irq list */ - for (i = 0; i < CONFIG_SYS_INTC_0_NUM; i++) { - vecs[i].handler = (interrupt_handler_t *) def_hdlr; - vecs[i].arg = (void *)i; - vecs[i].count = 0; + +#if defined(CONFIG_SYS_INTC_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) + intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); + irq_no = CONFIG_SYS_INTC_0_NUM; +#endif + if (irq_no) { + vecs = calloc(1, sizeof(struct irq_action) * irq_no); + if (vecs == NULL) { + puts("Interrupt vector allocation failed\n"); + return -1; + } + + /* initialize irq list */ + for (i = 0; i < irq_no; i++) { + vecs[i].handler = (interrupt_handler_t *) def_hdlr; + vecs[i].arg = (void *)i; + vecs[i].count = 0; + } + /* initialize intc controller */ + intc_init(); + enable_interrupts(); + } else { + puts("Undefined interrupt controller\n"); } - /* initialize intc controller */ - intc_init (); - enable_interrupts (); return 0; }
@@ -172,33 +187,30 @@ void interrupt_handler (void) printf ("Interrupt handler on %x line, r14 %x\n", irqs, value); #endif } -#endif
#if defined(CONFIG_CMD_IRQ) -#ifdef CONFIG_SYS_INTC_0 -int do_irqinfo (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) +int do_irqinfo(cmd_tbl_t *cmdtp, int flag, int argc, const char *argv[]) { int i; struct irq_action *act = vecs;
- puts ("\nInterrupt-Information:\n\n" - "Nr Routine Arg Count\n" - "-----------------------------\n"); - - for (i = 0; i < CONFIG_SYS_INTC_0_NUM; i++) { - if (act->handler != (interrupt_handler_t*) def_hdlr) { - printf ("%02d %08x %08x %d\n", i, - (int)act->handler, (int)act->arg, act->count); + if (irq_no) { + puts("\nInterrupt-Information:\n\n" + "Nr Routine Arg Count\n" + "-----------------------------\n"); + + for (i = 0; i < irq_no; i++) { + if (act->handler != (interrupt_handler_t *) def_hdlr) { + printf("%02d %08x %08x %d\n", i, + (int)act->handler, (int)act->arg, + act->count); + } + act++; } - act++; + puts("\n"); + } else { + puts("Undefined interrupt controller\n"); } - puts ("\n"); - return (0); -} -#else -int do_irqinfo (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) -{ - puts ("Undefined interrupt controller\n"); + return 0; } #endif -#endif diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S index 9077f74..8a2f634 100644 --- a/arch/microblaze/cpu/start.S +++ b/arch/microblaze/cpu/start.S @@ -108,7 +108,6 @@ _start: sh r6, r0, r8 #endif
-#ifdef CONFIG_SYS_INTC_0 /* interrupt_handler */ swi r2, r0, 0x10 /* interrupt - imm opcode */ swi r3, r0, 0x14 /* interrupt - brai opcode */ @@ -120,7 +119,6 @@ _start: sh r7, r0, r8 rsubi r8, r10, 0x16 sh r6, r0, r8 -#endif
/* hardware exception */ swi r2, r0, 0x20 /* hardware exception - imm opcode */ diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index 1952804..ae4ffe0 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -40,7 +40,6 @@ ulong get_timer (ulong base) } #endif
-#ifdef CONFIG_SYS_INTC_0 #ifdef CONFIG_SYS_TIMER_0 microblaze_timer_t *tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
@@ -61,7 +60,6 @@ int timer_init (void) return 0; } #endif -#endif
/* * This function is derived from PowerPC code (read timebase as long long). diff --git a/arch/microblaze/include/asm/microblaze_intc.h b/arch/microblaze/include/asm/microblaze_intc.h index 4c385aa..6142b9c 100644 --- a/arch/microblaze/include/asm/microblaze_intc.h +++ b/arch/microblaze/include/asm/microblaze_intc.h @@ -41,3 +41,6 @@ struct irq_action {
void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg); + +int interrupts_init(void); + diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index 1cf895b..8cf4266 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -32,6 +32,7 @@ #include <stdio_dev.h> #include <net.h> #include <asm/processor.h> +#include <asm/microblaze_intc.h> #include <fdtdec.h>
DECLARE_GLOBAL_DATA_PTR; @@ -39,9 +40,6 @@ DECLARE_GLOBAL_DATA_PTR; #ifdef CONFIG_SYS_GPIO_0 extern int gpio_init (void); #endif -#ifdef CONFIG_SYS_INTC_0 -extern int interrupts_init (void); -#endif #ifdef CONFIG_SYS_TIMER_0 extern int timer_init (void); #endif @@ -73,9 +71,7 @@ init_fnc_t *init_sequence[] = { #ifdef CONFIG_SYS_GPIO_0 gpio_init, #endif -#ifdef CONFIG_SYS_INTC_0 interrupts_init, -#endif #ifdef CONFIG_SYS_TIMER_0 timer_init, #endif diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h index e20eb08..44934eb 100644 --- a/include/configs/microblaze-generic.h +++ b/include/configs/microblaze-generic.h @@ -97,7 +97,6 @@
/* interrupt controller */ #ifdef XILINX_INTC_BASEADDR -# define CONFIG_SYS_INTC_0 1 # define CONFIG_SYS_INTC_0_ADDR XILINX_INTC_BASEADDR # define CONFIG_SYS_INTC_0_NUM XILINX_INTC_NUM_INTR_INPUTS #endif
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek monstr@monstr.eu wrote:
Clear and prepare for device-tree driven configuration. Remove CONFIG_SYS_INTC_0 definition Use dynamic allocation instead of static.
Signed-off-by: Michal Simek monstr@monstr.eu
I'm not really qualified to review this, but it seems reasonable.
Acked-by: Simon Glass sjg@chromium.org
arch/microblaze/cpu/interrupts.c | 88 ++++++++++++++----------- arch/microblaze/cpu/start.S | 2 - arch/microblaze/cpu/timer.c | 2 - arch/microblaze/include/asm/microblaze_intc.h | 3 + arch/microblaze/lib/board.c | 6 +-- include/configs/microblaze-generic.h | 1 - 6 files changed, 54 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index e7ca859..ee67082 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -26,6 +26,7 @@
#include <common.h> #include <command.h> +#include <malloc.h> #include <asm/microblaze_intc.h> #include <asm/asm.h>
@@ -48,20 +49,19 @@ int disable_interrupts (void) return (msr & 0x2) != 0; }
-#ifdef CONFIG_SYS_INTC_0
-static struct irq_action vecs[CONFIG_SYS_INTC_0_NUM]; +static struct irq_action *vecs; +static u32 irq_no;
/* mapping structure to interrupt controller */ -microblaze_intc_t *intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); +microblaze_intc_t *intc;
/* default handler */ -void def_hdlr (void) +static void def_hdlr(void) { puts ("def_hdlr\n"); }
-void enable_one_interrupt (int irq) +static void enable_one_interrupt(int irq) { int mask; int offset = 1; @@ -76,7 +76,7 @@ void enable_one_interrupt (int irq) #endif }
-void disable_one_interrupt (int irq) +static void disable_one_interrupt(int irq) { int mask; int offset = 1; @@ -96,7 +96,7 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) { struct irq_action *act; /* irq out of range */
if ((irq < 0) || (irq > CONFIG_SYS_INTC_0_NUM)) {
if ((irq < 0) || (irq > irq_no)) { puts ("IRQ out of range\n"); return; }@@ -114,7 +114,7 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) }
/* initialization interrupt controller - hardware */ -void intc_init (void) +static void intc_init(void) { intc->mer = 0; intc->ier = 0; @@ -127,18 +127,33 @@ void intc_init (void) #endif }
-int interrupts_init (void) +int interrupts_init(void) { int i;
/* initialize irq list */for (i = 0; i < CONFIG_SYS_INTC_0_NUM; i++) {vecs[i].handler = (interrupt_handler_t *) def_hdlr;vecs[i].arg = (void *)i;vecs[i].count = 0;
+#if defined(CONFIG_SYS_INTC_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM)
intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR);irq_no = CONFIG_SYS_INTC_0_NUM;+#endif
if (irq_no) {vecs = calloc(1, sizeof(struct irq_action) * irq_no);
This is fine, since I assume it is called after memalloc_init(), but you could set an arbitrary maximum limit if you prefer.
if (vecs == NULL) {puts("Interrupt vector allocation failed\n");return -1;}/* initialize irq list */for (i = 0; i < irq_no; i++) {vecs[i].handler = (interrupt_handler_t *) def_hdlr;vecs[i].arg = (void *)i;vecs[i].count = 0;}/* initialize intc controller */intc_init();enable_interrupts();} else {puts("Undefined interrupt controller\n"); }
/* initialize intc controller */intc_init ();enable_interrupts (); return 0;}
@@ -172,33 +187,30 @@ void interrupt_handler (void) printf ("Interrupt handler on %x line, r14 %x\n", irqs, value); #endif } -#endif
#if defined(CONFIG_CMD_IRQ) -#ifdef CONFIG_SYS_INTC_0 -int do_irqinfo (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) +int do_irqinfo(cmd_tbl_t *cmdtp, int flag, int argc, const char *argv[]) { int i; struct irq_action *act = vecs;
puts ("\nInterrupt-Information:\n\n""Nr Routine Arg Count\n""-----------------------------\n");for (i = 0; i < CONFIG_SYS_INTC_0_NUM; i++) {if (act->handler != (interrupt_handler_t*) def_hdlr) {printf ("%02d %08x %08x %d\n", i,(int)act->handler, (int)act->arg,act->count);
if (irq_no) {puts("\nInterrupt-Information:\n\n""Nr Routine Arg Count\n""-----------------------------\n");for (i = 0; i < irq_no; i++) {if (act->handler != (interrupt_handler_t *)def_hdlr) {
printf("%02d %08x %08x %d\n", i,(int)act->handler, (int)act->arg,act->count);
}act++; }
act++;
puts("\n");} else {puts("Undefined interrupt controller\n"); }
puts ("\n");return (0);-} -#else -int do_irqinfo (cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) -{
puts ("Undefined interrupt controller\n");
return 0;} #endif -#endif diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S index 9077f74..8a2f634 100644 --- a/arch/microblaze/cpu/start.S +++ b/arch/microblaze/cpu/start.S @@ -108,7 +108,6 @@ _start: sh r6, r0, r8 #endif
-#ifdef CONFIG_SYS_INTC_0 /* interrupt_handler */ swi r2, r0, 0x10 /* interrupt - imm opcode */ swi r3, r0, 0x14 /* interrupt - brai opcode */ @@ -120,7 +119,6 @@ _start: sh r7, r0, r8 rsubi r8, r10, 0x16 sh r6, r0, r8 -#endif
/* hardware exception */ swi r2, r0, 0x20 /* hardware exception - imm opcode */diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index 1952804..ae4ffe0 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -40,7 +40,6 @@ ulong get_timer (ulong base) } #endif
-#ifdef CONFIG_SYS_INTC_0 #ifdef CONFIG_SYS_TIMER_0 microblaze_timer_t *tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
@@ -61,7 +60,6 @@ int timer_init (void) return 0; } #endif -#endif
/*
- This function is derived from PowerPC code (read timebase as long
long). diff --git a/arch/microblaze/include/asm/microblaze_intc.h b/arch/microblaze/include/asm/microblaze_intc.h index 4c385aa..6142b9c 100644 --- a/arch/microblaze/include/asm/microblaze_intc.h +++ b/arch/microblaze/include/asm/microblaze_intc.h @@ -41,3 +41,6 @@ struct irq_action {
void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg);
+int interrupts_init(void);
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index 1cf895b..8cf4266 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -32,6 +32,7 @@ #include <stdio_dev.h> #include <net.h> #include <asm/processor.h> +#include <asm/microblaze_intc.h> #include <fdtdec.h>
DECLARE_GLOBAL_DATA_PTR; @@ -39,9 +40,6 @@ DECLARE_GLOBAL_DATA_PTR; #ifdef CONFIG_SYS_GPIO_0 extern int gpio_init (void); #endif -#ifdef CONFIG_SYS_INTC_0 -extern int interrupts_init (void); -#endif #ifdef CONFIG_SYS_TIMER_0 extern int timer_init (void); #endif @@ -73,9 +71,7 @@ init_fnc_t *init_sequence[] = { #ifdef CONFIG_SYS_GPIO_0 gpio_init, #endif -#ifdef CONFIG_SYS_INTC_0 interrupts_init, -#endif #ifdef CONFIG_SYS_TIMER_0 timer_init, #endif diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h index e20eb08..44934eb 100644 --- a/include/configs/microblaze-generic.h +++ b/include/configs/microblaze-generic.h @@ -97,7 +97,6 @@
/* interrupt controller */ #ifdef XILINX_INTC_BASEADDR -# define CONFIG_SYS_INTC_0 1 # define CONFIG_SYS_INTC_0_ADDR XILINX_INTC_BASEADDR # define CONFIG_SYS_INTC_0_NUM XILINX_INTC_NUM_INTR_INPUTS
#endif
1.7.0.4
Regards,
Simon
On 07/09/2012 11:21 PM, Simon Glass wrote:
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu mailto:monstr@monstr.eu> wrote:
Clear and prepare for device-tree driven configuration. Remove CONFIG_SYS_INTC_0 definition Use dynamic allocation instead of static. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>>I'm not really qualified to review this, but it seems reasonable.
Acked-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
thanks.
--- arch/microblaze/cpu/interrupts.c | 88 ++++++++++++++----------- arch/microblaze/cpu/start.S | 2 - arch/microblaze/cpu/timer.c | 2 - arch/microblaze/include/asm/microblaze_intc.h | 3 + arch/microblaze/lib/board.c | 6 +-- include/configs/microblaze-generic.h | 1 - 6 files changed, 54 insertions(+), 48 deletions(-) diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index e7ca859..ee67082 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -26,6 +26,7 @@ #include <common.h> #include <command.h> +#include <malloc.h> #include <asm/microblaze_intc.h> #include <asm/asm.h> @@ -48,20 +49,19 @@ int disable_interrupts (void) return (msr & 0x2) != 0; } -#ifdef CONFIG_SYS_INTC_0 - -static struct irq_action vecs[CONFIG_SYS_INTC_0_NUM]; +static struct irq_action *vecs; +static u32 irq_no; /* mapping structure to interrupt controller */ -microblaze_intc_t *intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); +microblaze_intc_t *intc; /* default handler */ -void def_hdlr (void) +static void def_hdlr(void) { puts ("def_hdlr\n"); } -void enable_one_interrupt (int irq) +static void enable_one_interrupt(int irq) { int mask; int offset = 1; @@ -76,7 +76,7 @@ void enable_one_interrupt (int irq) #endif } -void disable_one_interrupt (int irq) +static void disable_one_interrupt(int irq) { int mask; int offset = 1; @@ -96,7 +96,7 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) { struct irq_action *act; /* irq out of range */ - if ((irq < 0) || (irq > CONFIG_SYS_INTC_0_NUM)) { + if ((irq < 0) || (irq > irq_no)) { puts ("IRQ out of range\n"); return; } @@ -114,7 +114,7 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) } /* initialization interrupt controller - hardware */ -void intc_init (void) +static void intc_init(void) { intc->mer = 0; intc->ier = 0; @@ -127,18 +127,33 @@ void intc_init (void) #endif } -int interrupts_init (void) +int interrupts_init(void) { int i; - /* initialize irq list */ - for (i = 0; i < CONFIG_SYS_INTC_0_NUM; i++) { - vecs[i].handler = (interrupt_handler_t *) def_hdlr; - vecs[i].arg = (void *)i; - vecs[i].count = 0; + +#if defined(CONFIG_SYS_INTC_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) + intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); + irq_no = CONFIG_SYS_INTC_0_NUM; +#endif + if (irq_no) { + vecs = calloc(1, sizeof(struct irq_action) * irq_no);This is fine, since I assume it is called after memalloc_init(), but you could set an arbitrary maximum limit if you prefer.
Yes, it is called after mem_malloc_init. The maximum limit is 32 but it won't be reached. This checking is done by u-boot BSP.
Applied.
Thanks, Michal
Return value to find out if un/registration was succesful.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/interrupts.c | 15 +++++++++------ arch/microblaze/include/asm/microblaze_intc.h | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index ee67082..871cefb 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -92,13 +92,13 @@ static void disable_one_interrupt(int irq) }
/* adding new handler for interrupt */ -void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, void *arg) { struct irq_action *act; /* irq out of range */ if ((irq < 0) || (irq > irq_no)) { puts ("IRQ out of range\n"); - return; + return -1; } act = &vecs[irq]; if (hdlr) { /* enable */ @@ -106,11 +106,14 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) act->arg = arg; act->count = 0; enable_one_interrupt (irq); - } else { /* disable */ - act->handler = (interrupt_handler_t *) def_hdlr; - act->arg = (void *)irq; - disable_one_interrupt (irq); + return 0; } + + /* Disable */ + act->handler = (interrupt_handler_t *) def_hdlr; + act->arg = (void *)irq; + disable_one_interrupt(irq); + return 1; }
/* initialization interrupt controller - hardware */ diff --git a/arch/microblaze/include/asm/microblaze_intc.h b/arch/microblaze/include/asm/microblaze_intc.h index 6142b9c..359efe4 100644 --- a/arch/microblaze/include/asm/microblaze_intc.h +++ b/arch/microblaze/include/asm/microblaze_intc.h @@ -39,7 +39,7 @@ struct irq_action { int count; /* number of interrupt */ };
-void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, void *arg);
int interrupts_init(void);
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek monstr@monstr.eu wrote:
Return value to find out if un/registration was succesful.
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/cpu/interrupts.c | 15 +++++++++------ arch/microblaze/include/asm/microblaze_intc.h | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index ee67082..871cefb 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -92,13 +92,13 @@ static void disable_one_interrupt(int irq) }
/* adding new handler for interrupt */ -void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, void *arg) { struct irq_action *act; /* irq out of range */ if ((irq < 0) || (irq > irq_no)) { puts ("IRQ out of range\n");
return;
return -1; } act = &vecs[irq]; if (hdlr) { /* enable */@@ -106,11 +106,14 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) act->arg = arg; act->count = 0; enable_one_interrupt (irq);
} else { /* disable */act->handler = (interrupt_handler_t *) def_hdlr;act->arg = (void *)irq;disable_one_interrupt (irq);
return 0; }/* Disable */act->handler = (interrupt_handler_t *) def_hdlr;act->arg = (void *)irq;disable_one_interrupt(irq);return 1;}
/* initialization interrupt controller - hardware */ diff --git a/arch/microblaze/include/asm/microblaze_intc.h b/arch/microblaze/include/asm/microblaze_intc.h index 6142b9c..359efe4 100644 --- a/arch/microblaze/include/asm/microblaze_intc.h +++ b/arch/microblaze/include/asm/microblaze_intc.h @@ -39,7 +39,7 @@ struct irq_action { int count; /* number of interrupt */ };
-void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, void *arg);
Perhaps you should add a comment here as to what the return value is, and
maybe the other args also?
int interrupts_init(void);
1.7.0.4
Regards,
Simon
On 07/09/2012 11:22 PM, Simon Glass wrote:
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu mailto:monstr@monstr.eu> wrote:
Return value to find out if un/registration was succesful. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>> --- arch/microblaze/cpu/interrupts.c | 15 +++++++++------ arch/microblaze/include/asm/microblaze_intc.h | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index ee67082..871cefb 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -92,13 +92,13 @@ static void disable_one_interrupt(int irq) } /* adding new handler for interrupt */ -void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, void *arg) { struct irq_action *act; /* irq out of range */ if ((irq < 0) || (irq > irq_no)) { puts ("IRQ out of range\n"); - return; + return -1; } act = &vecs[irq]; if (hdlr) { /* enable */ @@ -106,11 +106,14 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) act->arg = arg; act->count = 0; enable_one_interrupt (irq); - } else { /* disable */ - act->handler = (interrupt_handler_t *) def_hdlr; - act->arg = (void *)irq; - disable_one_interrupt (irq); + return 0; } + + /* Disable */ + act->handler = (interrupt_handler_t *) def_hdlr; + act->arg = (void *)irq; + disable_one_interrupt(irq); + return 1; } /* initialization interrupt controller - hardware */ diff --git a/arch/microblaze/include/asm/microblaze_intc.h b/arch/microblaze/include/asm/microblaze_intc.h index 6142b9c..359efe4 100644 --- a/arch/microblaze/include/asm/microblaze_intc.h +++ b/arch/microblaze/include/asm/microblaze_intc.h @@ -39,7 +39,7 @@ struct irq_action { int count; /* number of interrupt */ }; -void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, void *arg);Perhaps you should add a comment here as to what the return value is, and maybe the other args also?
Make sense. Will be in v2.
Thanks, Michal
Just coding style cleanup. Remove unneeded externs.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/interrupts.c | 26 +++++++++++++------------- 1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index 871cefb..79ee96a 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -32,15 +32,12 @@
#undef DEBUG_INT
-extern void microblaze_disable_interrupts (void); -extern void microblaze_enable_interrupts (void); - -void enable_interrupts (void) +void enable_interrupts(void) { MSRSET(0x2); }
-int disable_interrupts (void) +int disable_interrupts(void) { unsigned int msr;
@@ -58,20 +55,21 @@ microblaze_intc_t *intc; /* default handler */ static void def_hdlr(void) { - puts ("def_hdlr\n"); + puts("def_hdlr\n"); }
static void enable_one_interrupt(int irq) { int mask; int offset = 1; + offset <<= irq; mask = intc->ier; intc->ier = (mask | offset); #ifdef DEBUG_INT - printf ("Enable one interrupt irq %x - mask %x,ier %x\n", offset, mask, + printf("Enable one interrupt irq %x - mask %x,ier %x\n", offset, mask, intc->ier); - printf ("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr, intc->ier, + printf("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr, intc->ier, intc->iar, intc->mer); #endif } @@ -80,13 +78,14 @@ static void disable_one_interrupt(int irq) { int mask; int offset = 1; + offset <<= irq; mask = intc->ier; intc->ier = (mask & ~offset); #ifdef DEBUG_INT - printf ("Disable one interrupt irq %x - mask %x,ier %x\n", irq, mask, + printf("Disable one interrupt irq %x - mask %x,ier %x\n", irq, mask, intc->ier); - printf ("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr, intc->ier, + printf("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr, intc->ier, intc->iar, intc->mer); #endif } @@ -95,9 +94,10 @@ static void disable_one_interrupt(int irq) int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, void *arg) { struct irq_action *act; + /* irq out of range */ if ((irq < 0) || (irq > irq_no)) { - puts ("IRQ out of range\n"); + puts("IRQ out of range\n"); return -1; } act = &vecs[irq]; @@ -125,7 +125,7 @@ static void intc_init(void) /* XIntc_Start - hw_interrupt enable and all interrupt enable */ intc->mer = 0x3; #ifdef DEBUG_INT - printf ("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr, intc->ier, + printf("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr, intc->ier, intc->iar, intc->mer); #endif } @@ -160,7 +160,7 @@ int interrupts_init(void) return 0; }
-void interrupt_handler (void) +void interrupt_handler(void) { int irqs = intc->ivr; /* find active interrupt */ int mask = 1;
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek monstr@monstr.eu wrote:
Just coding style cleanup. Remove unneeded externs.
Signed-off-by: Michal Simek monstr@monstr.eu
Acked-by: Simon Glass sjg@chromium.org
arch/microblaze/cpu/interrupts.c | 26 +++++++++++++------------- 1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index 871cefb..79ee96a 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -32,15 +32,12 @@
#undef DEBUG_INT
-extern void microblaze_disable_interrupts (void); -extern void microblaze_enable_interrupts (void);
-void enable_interrupts (void) +void enable_interrupts(void) { MSRSET(0x2); }
-int disable_interrupts (void) +int disable_interrupts(void) { unsigned int msr;
@@ -58,20 +55,21 @@ microblaze_intc_t *intc; /* default handler */ static void def_hdlr(void) {
puts ("def_hdlr\n");
puts("def_hdlr\n");}
static void enable_one_interrupt(int irq) { int mask; int offset = 1;
offset <<= irq; mask = intc->ier; intc->ier = (mask | offset);#ifdef DEBUG_INT
printf ("Enable one interrupt irq %x - mask %x,ier %x\n", offset,mask,
printf("Enable one interrupt irq %x - mask %x,ier %x\n", offset,mask, intc->ier);
printf ("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr,intc->ier,
printf("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr,intc->ier, intc->iar, intc->mer); #endif } @@ -80,13 +78,14 @@ static void disable_one_interrupt(int irq) { int mask; int offset = 1;
offset <<= irq; mask = intc->ier; intc->ier = (mask & ~offset);#ifdef DEBUG_INT
printf ("Disable one interrupt irq %x - mask %x,ier %x\n", irq,mask,
printf("Disable one interrupt irq %x - mask %x,ier %x\n", irq,mask, intc->ier);
printf ("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr,intc->ier,
printf("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr,intc->ier, intc->iar, intc->mer); #endif } @@ -95,9 +94,10 @@ static void disable_one_interrupt(int irq) int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, void *arg) { struct irq_action *act;
/* irq out of range */ if ((irq < 0) || (irq > irq_no)) {
puts ("IRQ out of range\n");
puts("IRQ out of range\n"); return -1; } act = &vecs[irq];@@ -125,7 +125,7 @@ static void intc_init(void) /* XIntc_Start - hw_interrupt enable and all interrupt enable */ intc->mer = 0x3; #ifdef DEBUG_INT
printf ("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr,intc->ier,
printf("INTC isr %x, ier %x, iar %x, mer %x\n", intc->isr,intc->ier, intc->iar, intc->mer); #endif } @@ -160,7 +160,7 @@ int interrupts_init(void) return 0; }
-void interrupt_handler (void) +void interrupt_handler(void) { int irqs = intc->ivr; /* find active interrupt */ int mask = 1; -- 1.7.0.4
On 07/09/2012 11:23 PM, Simon Glass wrote:
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu mailto:monstr@monstr.eu> wrote:
Just coding style cleanup. Remove unneeded externs. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>>Acked-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
It depends on previous patch that's why I will send it in v2 package.
Thanks, Michal
Read configuration from DTB.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/interrupts.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index 79ee96a..98c9110 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -29,6 +29,9 @@ #include <malloc.h> #include <asm/microblaze_intc.h> #include <asm/asm.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR;
#undef DEBUG_INT
@@ -134,10 +137,26 @@ int interrupts_init(void) { int i;
+#ifndef CONFIG_OF_CONTROL #if defined(CONFIG_SYS_INTC_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); irq_no = CONFIG_SYS_INTC_0_NUM; #endif +#else + int temp; + int offset = 0; + + offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset, + "xlnx,xps-intc-1.00.a"); + if (offset > 0) { + temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg"); + if (temp != FDT_ADDR_T_NONE) { + intc = (microblaze_intc_t *)temp; + irq_no = fdtdec_get_int(gd->fdt_blob, offset, + "xlnx,num-intr-inputs", 0); + } + } +#endif if (irq_no) { vecs = calloc(1, sizeof(struct irq_action) * irq_no); if (vecs == NULL) {
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek monstr@monstr.eu wrote:
Read configuration from DTB.
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/cpu/interrupts.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index 79ee96a..98c9110 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -29,6 +29,9 @@ #include <malloc.h> #include <asm/microblaze_intc.h> #include <asm/asm.h> +#include <fdtdec.h>
+DECLARE_GLOBAL_DATA_PTR;
#undef DEBUG_INT
@@ -134,10 +137,26 @@ int interrupts_init(void) { int i;
+#ifndef CONFIG_OF_CONTROL #if defined(CONFIG_SYS_INTC_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); irq_no = CONFIG_SYS_INTC_0_NUM; #endif +#else
int temp;int offset = 0;
I don't think you need the = 0.
offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset,"xlnx,xps-intc-1.00.a");if (offset > 0) {temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg");if (temp != FDT_ADDR_T_NONE) {intc = (microblaze_intc_t *)temp;irq_no = fdtdec_get_int(gd->fdt_blob, offset,"xlnx,num-intr-inputs", 0);}}+#endif if (irq_no) { vecs = calloc(1, sizeof(struct irq_action) * irq_no); if (vecs == NULL) {
I'm not completely clear about whether this should be doing a realloc() (copying the existing array and adding a new member) or not.
-- 1.7.0.4
Regards, Simon
On 07/09/2012 11:26 PM, Simon Glass wrote:
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu mailto:monstr@monstr.eu> wrote:
Read configuration from DTB. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>> --- arch/microblaze/cpu/interrupts.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-) diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c index 79ee96a..98c9110 100644 --- a/arch/microblaze/cpu/interrupts.c +++ b/arch/microblaze/cpu/interrupts.c @@ -29,6 +29,9 @@ #include <malloc.h> #include <asm/microblaze_intc.h> #include <asm/asm.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR; #undef DEBUG_INT @@ -134,10 +137,26 @@ int interrupts_init(void) { int i; +#ifndef CONFIG_OF_CONTROL #if defined(CONFIG_SYS_INTC_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); irq_no = CONFIG_SYS_INTC_0_NUM; #endif +#else + int temp; + int offset = 0;I don't think you need the = 0.
Agree. Will remove it.
+ + offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset, + "xlnx,xps-intc-1.00.a"); + if (offset > 0) { + temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg"); + if (temp != FDT_ADDR_T_NONE) { + intc = (microblaze_intc_t *)temp; + irq_no = fdtdec_get_int(gd->fdt_blob, offset, + "xlnx,num-intr-inputs", 0); + } + } +#endif if (irq_no) { vecs = calloc(1, sizeof(struct irq_action) * irq_no); if (vecs == NULL) {I'm not completely clear about whether this should be doing a realloc() (copying the existing array and adding a new member) or not.
I am not sure what you mean by that. This is done after mem_alloc_init and microblaze u-boot does not any reallocation like other platforms. Is it what did you mean?
Thanks, Michal
Hi Michal,
On Tue, Jul 10, 2012 at 11:07 AM, Michal Simek monstr@monstr.eu wrote:
On 07/09/2012 11:26 PM, Simon Glass wrote:
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu mailto: monstr@monstr.eu> wrote:
Read configuration from DTB. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>>
--- arch/microblaze/cpu/**interrupts.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-) diff --git a/arch/microblaze/cpu/**interrupts.cb/arch/microblaze/cpu/**interrupts.c index 79ee96a..98c9110 100644 --- a/arch/microblaze/cpu/**interrupts.c +++ b/arch/microblaze/cpu/**interrupts.c @@ -29,6 +29,9 @@ #include <malloc.h> #include <asm/microblaze_intc.h> #include <asm/asm.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR;
#undef DEBUG_INT @@ -134,10 +137,26 @@ int interrupts_init(void) { int i; +#ifndef CONFIG_OF_CONTROL #if defined(CONFIG_SYS_INTC_0_**ADDR) &&defined(CONFIG_SYS_INTC_0_NUM) intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); irq_no = CONFIG_SYS_INTC_0_NUM; #endif +#else + int temp; + int offset = 0;
I don't think you need the = 0.
Agree. Will remove it.
+ + offset = fdt_node_offset_by_compatible(**gd->fdt_blob,offset, + "xlnx,xps-intc-1.00.a"); + if (offset > 0) { + temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg"); + if (temp != FDT_ADDR_T_NONE) { + intc = (microblaze_intc_t *)temp; + irq_no = fdtdec_get_int(gd->fdt_blob, offset, + "xlnx,num-intr-inputs", 0); + } + } +#endif if (irq_no) { vecs = calloc(1, sizeof(struct irq_action) * irq_no); if (vecs == NULL) {
I'm not completely clear about whether this should be doing a realloc() (copying the existing array and adding a new member) or not.
I am not sure what you mean by that. This is done after mem_alloc_init and microblaze u-boot does not any reallocation like other platforms. Is it what did you mean?
No I just meant that if you had several calls to this it would need to add
to the existing list. But I see now that this is just a single item being alloced, so it is fine.
Thanks, Michal
-- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian
Regards, Simon
On 07/10/2012 11:02 PM, Simon Glass wrote:
Hi Michal,
On Tue, Jul 10, 2012 at 11:07 AM, Michal Simek <monstr@monstr.eu mailto:monstr@monstr.eu> wrote:
On 07/09/2012 11:26 PM, Simon Glass wrote: Hi Michal, On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu <mailto:monstr@monstr.eu>>> wrote: Read configuration from DTB. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu <mailto:monstr@monstr.eu>>> --- arch/microblaze/cpu/__interrupts.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-) diff --git a/arch/microblaze/cpu/__interrupts.c b/arch/microblaze/cpu/__interrupts.c index 79ee96a..98c9110 100644 --- a/arch/microblaze/cpu/__interrupts.c +++ b/arch/microblaze/cpu/__interrupts.c @@ -29,6 +29,9 @@ #include <malloc.h> #include <asm/microblaze_intc.h> #include <asm/asm.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR; #undef DEBUG_INT @@ -134,10 +137,26 @@ int interrupts_init(void) { int i; +#ifndef CONFIG_OF_CONTROL #if defined(CONFIG_SYS_INTC_0___ADDR) && defined(CONFIG_SYS_INTC_0_NUM) intc = (microblaze_intc_t *) (CONFIG_SYS_INTC_0_ADDR); irq_no = CONFIG_SYS_INTC_0_NUM; #endif +#else + int temp; + int offset = 0; I don't think you need the = 0. Agree. Will remove it. + + offset = fdt_node_offset_by_compatible(__gd->fdt_blob, offset, + "xlnx,xps-intc-1.00.a"); + if (offset > 0) { + temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg"); + if (temp != FDT_ADDR_T_NONE) { + intc = (microblaze_intc_t *)temp; + irq_no = fdtdec_get_int(gd->fdt_blob, offset, + "xlnx,num-intr-inputs", 0); + } + } +#endif if (irq_no) { vecs = calloc(1, sizeof(struct irq_action) * irq_no); if (vecs == NULL) { I'm not completely clear about whether this should be doing a realloc() (copying the existing array and adding a new member) or not. I am not sure what you mean by that. This is done after mem_alloc_init and microblaze u-boot does not any reallocation like other platforms. Is it what did you mean?No I just meant that if you had several calls to this it would need to add to the existing list. But I see now that this is just a single item being alloced, so it is fine.
ok.
Thanks, Michal
Move __udelay to the timer code because of unification. And clean coding style because of checkpatch.pl.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/timer.c | 19 +++++++++++++++++++ arch/microblaze/lib/Makefile | 1 - arch/microblaze/lib/time.c | 42 ------------------------------------------ 3 files changed, 19 insertions(+), 43 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index ae4ffe0..cc6b897 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -41,6 +41,25 @@ ulong get_timer (ulong base) #endif
#ifdef CONFIG_SYS_TIMER_0 +void __udelay(unsigned long usec) +{ + int i; + + i = get_timer(0); + while ((get_timer(0) - i) < (usec / 1000)) + ; +} +#else +void __udelay(unsigned long usec) +{ + unsigned int i; + + for (i = 0; i < (usec * CONFIG_XILINX_CLOCK_FREQ / 10000000); i++) + ; +} +#endif + +#ifdef CONFIG_SYS_TIMER_0 microblaze_timer_t *tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
void timer_isr (void *arg) diff --git a/arch/microblaze/lib/Makefile b/arch/microblaze/lib/Makefile index de0a7d3..7730695 100644 --- a/arch/microblaze/lib/Makefile +++ b/arch/microblaze/lib/Makefile @@ -29,7 +29,6 @@ SOBJS-y +=
COBJS-y += board.o COBJS-y += bootm.o -COBJS-y += time.o
SRCS := $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y)) diff --git a/arch/microblaze/lib/time.c b/arch/microblaze/lib/time.c index da016a0..e69de29 100644 --- a/arch/microblaze/lib/time.c +++ b/arch/microblaze/lib/time.c @@ -1,42 +0,0 @@ -/* - * (C) Copyright 2007 Michal Simek - * (C) Copyright 2004 Atmark Techno, Inc. - * - * Michal SIMEK monstr@monstr.eu - * Yasushi SHOJI yashi@atmark-techno.com - * - * 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> - -#ifdef CONFIG_SYS_TIMER_0 -void __udelay (unsigned long usec) -{ - int i; - i = get_timer (0); - while ((get_timer (0) - i) < (usec / 1000)) ; -} -#else -void __udelay (unsigned long usec) -{ - unsigned int i; - for (i = 0; i < (usec * CONFIG_XILINX_CLOCK_FREQ / 10000000); i++); -} -#endif
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek monstr@monstr.eu wrote:
Move __udelay to the timer code because of unification. And clean coding style because of checkpatch.pl.
Signed-off-by: Michal Simek monstr@monstr.eu
Acked-by: Simon Glass sjg@chromium.org
arch/microblaze/cpu/timer.c | 19 +++++++++++++++++++ arch/microblaze/lib/Makefile | 1 - arch/microblaze/lib/time.c | 42
3 files changed, 19 insertions(+), 43 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index ae4ffe0..cc6b897 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -41,6 +41,25 @@ ulong get_timer (ulong base) #endif
#ifdef CONFIG_SYS_TIMER_0 +void __udelay(unsigned long usec) +{
int i;i = get_timer(0);while ((get_timer(0) - i) < (usec / 1000));+} +#else +void __udelay(unsigned long usec) +{
unsigned int i;for (i = 0; i < (usec * CONFIG_XILINX_CLOCK_FREQ / 10000000); i++);+} +#endif
+#ifdef CONFIG_SYS_TIMER_0 microblaze_timer_t *tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
void timer_isr (void *arg) diff --git a/arch/microblaze/lib/Makefile b/arch/microblaze/lib/Makefile index de0a7d3..7730695 100644 --- a/arch/microblaze/lib/Makefile +++ b/arch/microblaze/lib/Makefile @@ -29,7 +29,6 @@ SOBJS-y +=
COBJS-y += board.o COBJS-y += bootm.o -COBJS-y += time.o
SRCS := $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y)) diff --git a/arch/microblaze/lib/time.c b/arch/microblaze/lib/time.c index da016a0..e69de29 100644 --- a/arch/microblaze/lib/time.c +++ b/arch/microblaze/lib/time.c @@ -1,42 +0,0 @@ -/*
- (C) Copyright 2007 Michal Simek
- (C) Copyright 2004 Atmark Techno, Inc.
- Michal SIMEK monstr@monstr.eu
- Yasushi SHOJI yashi@atmark-techno.com
- 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>
-#ifdef CONFIG_SYS_TIMER_0 -void __udelay (unsigned long usec) -{
int i;i = get_timer (0);while ((get_timer (0) - i) < (usec / 1000)) ;-} -#else -void __udelay (unsigned long usec) -{
unsigned int i;for (i = 0; i < (usec * CONFIG_XILINX_CLOCK_FREQ / 10000000); i++);-}
-#endif
1.7.0.4
On 07/09/2012 11:28 PM, Simon Glass wrote:
Acked-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
Applied.
Thanks, Michal
microblaze: Fix CONFIG_SYS_HZ usage in board config
Do not use hardcoded value. Use CONFIG_SYS_HZ instead. Separate static configuration to single block.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/timer.c | 69 ++++++++++++----------- arch/microblaze/include/asm/microblaze_timer.h | 3 + arch/microblaze/lib/board.c | 5 -- include/configs/microblaze-generic.h | 12 +---- 4 files changed, 41 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index cc6b897..dfaaaf5 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -27,42 +27,30 @@ #include <asm/microblaze_intc.h>
volatile int timestamp = 0; +microblaze_timer_t *tmr;
-#ifdef CONFIG_SYS_TIMER_0 ulong get_timer (ulong base) { - return (timestamp - base); + if (tmr) + return timestamp - base; + return timestamp++ - base; } -#else -ulong get_timer (ulong base) -{ - return (timestamp++ - base); -} -#endif
-#ifdef CONFIG_SYS_TIMER_0 void __udelay(unsigned long usec) { - int i; + u32 i;
- i = get_timer(0); - while ((get_timer(0) - i) < (usec / 1000)) - ; + if (tmr) { + i = get_timer(0); + while ((get_timer(0) - i) < (usec / 1000)) + ; + } else { + for (i = 0; i < (usec * XILINX_CLOCK_FREQ / 10000000); i++) + ; + } } -#else -void __udelay(unsigned long usec) -{ - unsigned int i;
- for (i = 0; i < (usec * CONFIG_XILINX_CLOCK_FREQ / 10000000); i++) - ; -} -#endif - -#ifdef CONFIG_SYS_TIMER_0 -microblaze_timer_t *tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR); - -void timer_isr (void *arg) +static void timer_isr(void *arg) { timestamp++; tmr->control = tmr->control | TIMER_INTERRUPT; @@ -70,15 +58,30 @@ void timer_isr (void *arg)
int timer_init (void) { - tmr->loadreg = CONFIG_SYS_TIMER_0_PRELOAD; - tmr->control = TIMER_INTERRUPT | TIMER_RESET; - tmr->control = - TIMER_ENABLE | TIMER_ENABLE_INTR | TIMER_RELOAD | TIMER_DOWN_COUNT; - timestamp = 0; - install_interrupt_handler (CONFIG_SYS_TIMER_0_IRQ, timer_isr, (void *)tmr); + u32 irq = 0; + u32 preload = 0; + u32 ret = 0; + +#if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) + preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ; + irq = CONFIG_SYS_TIMER_0_IRQ; + tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR); +#endif + + if (tmr && irq && preload) { + tmr->loadreg = preload; + tmr->control = TIMER_INTERRUPT | TIMER_RESET; + tmr->control = TIMER_ENABLE | TIMER_ENABLE_INTR |\ + TIMER_RELOAD | TIMER_DOWN_COUNT; + timestamp = 0; + ret = install_interrupt_handler (irq, timer_isr, (void *)tmr); + if (ret) + tmr = NULL; + } + + /* No problem if timer is not found/initialized */ return 0; } -#endif
/* * This function is derived from PowerPC code (read timebase as long long). diff --git a/arch/microblaze/include/asm/microblaze_timer.h b/arch/microblaze/include/asm/microblaze_timer.h index 844c8db..28e8b02 100644 --- a/arch/microblaze/include/asm/microblaze_timer.h +++ b/arch/microblaze/include/asm/microblaze_timer.h @@ -39,3 +39,6 @@ typedef volatile struct microblaze_timer_t { int loadreg; /* load register TLR */ int counter; /* timer/counter register */ } microblaze_timer_t; + +int timer_init(void); + diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index 8cf4266..ddbc862 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -40,9 +40,6 @@ DECLARE_GLOBAL_DATA_PTR; #ifdef CONFIG_SYS_GPIO_0 extern int gpio_init (void); #endif -#ifdef CONFIG_SYS_TIMER_0 -extern int timer_init (void); -#endif #ifdef CONFIG_SYS_FSL_2 extern void fsl_init2 (void); #endif @@ -72,9 +69,7 @@ init_fnc_t *init_sequence[] = { gpio_init, #endif interrupts_init, -#ifdef CONFIG_SYS_TIMER_0 timer_init, -#endif #ifdef CONFIG_SYS_FSL_2 fsl_init2, #endif diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h index 44934eb..9f90107 100644 --- a/include/configs/microblaze-generic.h +++ b/include/configs/microblaze-generic.h @@ -102,19 +102,11 @@ #endif
/* timer */ -#ifdef XILINX_TIMER_BASEADDR -# if (XILINX_TIMER_IRQ != -1) -# define CONFIG_SYS_TIMER_0 1 +#if defined(XILINX_TIMER_BASEADD) && defined(XILINX_TIMER_IRQ) # define CONFIG_SYS_TIMER_0_ADDR XILINX_TIMER_BASEADDR # define CONFIG_SYS_TIMER_0_IRQ XILINX_TIMER_IRQ -# define FREQUENCE XILINX_CLOCK_FREQ -# define CONFIG_SYS_TIMER_0_PRELOAD ( FREQUENCE/1000 ) -# endif -#elif XILINX_CLOCK_FREQ -# define CONFIG_XILINX_CLOCK_FREQ XILINX_CLOCK_FREQ -#else -# error BAD CLOCK FREQ #endif + /* FSL */ /* #define CONFIG_SYS_FSL_2 */ /* #define FSL_INTR_2 1 */
Hi Michal,
Am Montag, den 09.07.2012, 11:20 +0200 schrieb Michal Simek:
microblaze: Fix CONFIG_SYS_HZ usage in board config
Do not use hardcoded value. Use CONFIG_SYS_HZ instead. Separate static configuration to single block.
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/cpu/timer.c | 69 ++++++++++++----------- arch/microblaze/include/asm/microblaze_timer.h | 3 + arch/microblaze/lib/board.c | 5 -- include/configs/microblaze-generic.h | 12 +---- 4 files changed, 41 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index cc6b897..dfaaaf5 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -27,42 +27,30 @@ #include <asm/microblaze_intc.h>
volatile int timestamp = 0; +microblaze_timer_t *tmr;
-#ifdef CONFIG_SYS_TIMER_0 ulong get_timer (ulong base) {
- return (timestamp - base);
- if (tmr)
return timestamp - base;- return timestamp++ - base;
} -#else -ulong get_timer (ulong base) -{
- return (timestamp++ - base);
-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 void __udelay(unsigned long usec) {
- int i;
- u32 i;
- i = get_timer(0);
- while ((get_timer(0) - i) < (usec / 1000))
;
- if (tmr) {
i = get_timer(0);while ((get_timer(0) - i) < (usec / 1000));- } else {
for (i = 0; i < (usec * XILINX_CLOCK_FREQ / 10000000); i++);- }
} -#else -void __udelay(unsigned long usec) -{
unsigned int i;
for (i = 0; i < (usec * CONFIG_XILINX_CLOCK_FREQ / 10000000); i++)
;-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 -microblaze_timer_t *tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
-void timer_isr (void *arg) +static void timer_isr(void *arg) { timestamp++; tmr->control = tmr->control | TIMER_INTERRUPT; @@ -70,15 +58,30 @@ void timer_isr (void *arg)
int timer_init (void) {
- tmr->loadreg = CONFIG_SYS_TIMER_0_PRELOAD;
- tmr->control = TIMER_INTERRUPT | TIMER_RESET;
- tmr->control =
TIMER_ENABLE | TIMER_ENABLE_INTR | TIMER_RELOAD | TIMER_DOWN_COUNT;- timestamp = 0;
- install_interrupt_handler (CONFIG_SYS_TIMER_0_IRQ, timer_isr, (void *)tmr);
- u32 irq = 0;
should be int irq = -1; (see below)
- u32 preload = 0;
- u32 ret = 0;
+#if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM)
- preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
- irq = CONFIG_SYS_TIMER_0_IRQ;
- tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
+#endif
- if (tmr && irq && preload) {
it should be:
if (tmr && irq >= 0 && preload) {
irq valid --> irq >= 0 irq invalid --> irq == -1
br, Stephan
tmr->loadreg = preload;tmr->control = TIMER_INTERRUPT | TIMER_RESET;tmr->control = TIMER_ENABLE | TIMER_ENABLE_INTR |\TIMER_RELOAD | TIMER_DOWN_COUNT;timestamp = 0;ret = install_interrupt_handler (irq, timer_isr, (void *)tmr);if (ret)tmr = NULL;- }
- /* No problem if timer is not found/initialized */ return 0;
} -#endif
/*
- This function is derived from PowerPC code (read timebase as long long).
diff --git a/arch/microblaze/include/asm/microblaze_timer.h b/arch/microblaze/include/asm/microblaze_timer.h index 844c8db..28e8b02 100644 --- a/arch/microblaze/include/asm/microblaze_timer.h +++ b/arch/microblaze/include/asm/microblaze_timer.h @@ -39,3 +39,6 @@ typedef volatile struct microblaze_timer_t { int loadreg; /* load register TLR */ int counter; /* timer/counter register */ } microblaze_timer_t;
+int timer_init(void);
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index 8cf4266..ddbc862 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -40,9 +40,6 @@ DECLARE_GLOBAL_DATA_PTR; #ifdef CONFIG_SYS_GPIO_0 extern int gpio_init (void); #endif -#ifdef CONFIG_SYS_TIMER_0 -extern int timer_init (void); -#endif #ifdef CONFIG_SYS_FSL_2 extern void fsl_init2 (void); #endif @@ -72,9 +69,7 @@ init_fnc_t *init_sequence[] = { gpio_init, #endif interrupts_init, -#ifdef CONFIG_SYS_TIMER_0 timer_init, -#endif #ifdef CONFIG_SYS_FSL_2 fsl_init2, #endif diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h index 44934eb..9f90107 100644 --- a/include/configs/microblaze-generic.h +++ b/include/configs/microblaze-generic.h @@ -102,19 +102,11 @@ #endif
/* timer */ -#ifdef XILINX_TIMER_BASEADDR -# if (XILINX_TIMER_IRQ != -1) -# define CONFIG_SYS_TIMER_0 1 +#if defined(XILINX_TIMER_BASEADD) && defined(XILINX_TIMER_IRQ) # define CONFIG_SYS_TIMER_0_ADDR XILINX_TIMER_BASEADDR # define CONFIG_SYS_TIMER_0_IRQ XILINX_TIMER_IRQ -# define FREQUENCE XILINX_CLOCK_FREQ -# define CONFIG_SYS_TIMER_0_PRELOAD ( FREQUENCE/1000 ) -# endif -#elif XILINX_CLOCK_FREQ -# define CONFIG_XILINX_CLOCK_FREQ XILINX_CLOCK_FREQ -#else -# error BAD CLOCK FREQ #endif
/* FSL */ /* #define CONFIG_SYS_FSL_2 */ /* #define FSL_INTR_2 1 */
On 07/09/2012 08:26 PM, Stephan Linz wrote:
Hi Michal,
Am Montag, den 09.07.2012, 11:20 +0200 schrieb Michal Simek:
microblaze: Fix CONFIG_SYS_HZ usage in board config
Do not use hardcoded value. Use CONFIG_SYS_HZ instead. Separate static configuration to single block.
Signed-off-by: Michal Simekmonstr@monstr.eu
arch/microblaze/cpu/timer.c | 69 ++++++++++++----------- arch/microblaze/include/asm/microblaze_timer.h | 3 + arch/microblaze/lib/board.c | 5 -- include/configs/microblaze-generic.h | 12 +---- 4 files changed, 41 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index cc6b897..dfaaaf5 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -27,42 +27,30 @@ #include<asm/microblaze_intc.h>
volatile int timestamp = 0; +microblaze_timer_t *tmr;
-#ifdef CONFIG_SYS_TIMER_0 ulong get_timer (ulong base) {
- return (timestamp - base);
- if (tmr)
return timestamp - base;- return timestamp++ - base; }
-#else -ulong get_timer (ulong base) -{
- return (timestamp++ - base);
-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 void __udelay(unsigned long usec) {
- int i;
- u32 i;
- i = get_timer(0);
- while ((get_timer(0) - i)< (usec / 1000))
;
- if (tmr) {
i = get_timer(0);while ((get_timer(0) - i)< (usec / 1000));- } else {
for (i = 0; i< (usec * XILINX_CLOCK_FREQ / 10000000); i++);- } }
-#else -void __udelay(unsigned long usec) -{
unsigned int i;
for (i = 0; i< (usec * CONFIG_XILINX_CLOCK_FREQ / 10000000); i++)
;-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 -microblaze_timer_t *tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
-void timer_isr (void *arg) +static void timer_isr(void *arg) { timestamp++; tmr->control = tmr->control | TIMER_INTERRUPT; @@ -70,15 +58,30 @@ void timer_isr (void *arg)
int timer_init (void) {
- tmr->loadreg = CONFIG_SYS_TIMER_0_PRELOAD;
- tmr->control = TIMER_INTERRUPT | TIMER_RESET;
- tmr->control =
TIMER_ENABLE | TIMER_ENABLE_INTR | TIMER_RELOAD | TIMER_DOWN_COUNT;- timestamp = 0;
- install_interrupt_handler (CONFIG_SYS_TIMER_0_IRQ, timer_isr, (void *)tmr);
- u32 irq = 0;
should be int irq = -1; (see below)
- u32 preload = 0;
- u32 ret = 0;
+#if defined(CONFIG_SYS_TIMER_0_ADDR)&& defined(CONFIG_SYS_INTC_0_NUM)
- preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
- irq = CONFIG_SYS_TIMER_0_IRQ;
- tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
+#endif
- if (tmr&& irq&& preload) {
it should be:
if (tmr&& irq>= 0&& preload) {
irq valid --> irq>= 0 irq invalid --> irq == -1
Correct - will be in the v2.
Thanks, Michal
Am Montag, den 09.07.2012, 11:20 +0200 schrieb Michal Simek:
microblaze: Fix CONFIG_SYS_HZ usage in board config
Do not use hardcoded value. Use CONFIG_SYS_HZ instead. Separate static configuration to single block.
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/cpu/timer.c | 69 ++++++++++++----------- arch/microblaze/include/asm/microblaze_timer.h | 3 + arch/microblaze/lib/board.c | 5 -- include/configs/microblaze-generic.h | 12 +---- 4 files changed, 41 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index cc6b897..dfaaaf5 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -27,42 +27,30 @@ #include <asm/microblaze_intc.h>
volatile int timestamp = 0; +microblaze_timer_t *tmr;
-#ifdef CONFIG_SYS_TIMER_0 ulong get_timer (ulong base) {
- return (timestamp - base);
- if (tmr)
return timestamp - base;- return timestamp++ - base;
} -#else -ulong get_timer (ulong base) -{
- return (timestamp++ - base);
-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 void __udelay(unsigned long usec) {
- int i;
- u32 i;
- i = get_timer(0);
- while ((get_timer(0) - i) < (usec / 1000))
;
- if (tmr) {
i = get_timer(0);while ((get_timer(0) - i) < (usec / 1000));
Hi Michal,
- } else {
for (i = 0; i < (usec * XILINX_CLOCK_FREQ / 10000000); i++);
this part should be enclosed by #ifdef XILINX_CLOCK_FREQ
br, Stephan Linz
- }
} -#else -void __udelay(unsigned long usec) -{
unsigned int i;
for (i = 0; i < (usec * CONFIG_XILINX_CLOCK_FREQ / 10000000); i++)
;-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 -microblaze_timer_t *tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
-void timer_isr (void *arg) +static void timer_isr(void *arg) { timestamp++; tmr->control = tmr->control | TIMER_INTERRUPT; @@ -70,15 +58,30 @@ void timer_isr (void *arg)
int timer_init (void) {
- tmr->loadreg = CONFIG_SYS_TIMER_0_PRELOAD;
- tmr->control = TIMER_INTERRUPT | TIMER_RESET;
- tmr->control =
TIMER_ENABLE | TIMER_ENABLE_INTR | TIMER_RELOAD | TIMER_DOWN_COUNT;- timestamp = 0;
- install_interrupt_handler (CONFIG_SYS_TIMER_0_IRQ, timer_isr, (void *)tmr);
- u32 irq = 0;
- u32 preload = 0;
- u32 ret = 0;
+#if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM)
- preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ;
- irq = CONFIG_SYS_TIMER_0_IRQ;
- tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR);
+#endif
- if (tmr && irq && preload) {
tmr->loadreg = preload;tmr->control = TIMER_INTERRUPT | TIMER_RESET;tmr->control = TIMER_ENABLE | TIMER_ENABLE_INTR |\TIMER_RELOAD | TIMER_DOWN_COUNT;timestamp = 0;ret = install_interrupt_handler (irq, timer_isr, (void *)tmr);if (ret)tmr = NULL;- }
- /* No problem if timer is not found/initialized */ return 0;
} -#endif
/*
- This function is derived from PowerPC code (read timebase as long long).
diff --git a/arch/microblaze/include/asm/microblaze_timer.h b/arch/microblaze/include/asm/microblaze_timer.h index 844c8db..28e8b02 100644 --- a/arch/microblaze/include/asm/microblaze_timer.h +++ b/arch/microblaze/include/asm/microblaze_timer.h @@ -39,3 +39,6 @@ typedef volatile struct microblaze_timer_t { int loadreg; /* load register TLR */ int counter; /* timer/counter register */ } microblaze_timer_t;
+int timer_init(void);
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index 8cf4266..ddbc862 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -40,9 +40,6 @@ DECLARE_GLOBAL_DATA_PTR; #ifdef CONFIG_SYS_GPIO_0 extern int gpio_init (void); #endif -#ifdef CONFIG_SYS_TIMER_0 -extern int timer_init (void); -#endif #ifdef CONFIG_SYS_FSL_2 extern void fsl_init2 (void); #endif @@ -72,9 +69,7 @@ init_fnc_t *init_sequence[] = { gpio_init, #endif interrupts_init, -#ifdef CONFIG_SYS_TIMER_0 timer_init, -#endif #ifdef CONFIG_SYS_FSL_2 fsl_init2, #endif diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h index 44934eb..9f90107 100644 --- a/include/configs/microblaze-generic.h +++ b/include/configs/microblaze-generic.h @@ -102,19 +102,11 @@ #endif
/* timer */ -#ifdef XILINX_TIMER_BASEADDR -# if (XILINX_TIMER_IRQ != -1) -# define CONFIG_SYS_TIMER_0 1 +#if defined(XILINX_TIMER_BASEADD) && defined(XILINX_TIMER_IRQ) # define CONFIG_SYS_TIMER_0_ADDR XILINX_TIMER_BASEADDR # define CONFIG_SYS_TIMER_0_IRQ XILINX_TIMER_IRQ -# define FREQUENCE XILINX_CLOCK_FREQ -# define CONFIG_SYS_TIMER_0_PRELOAD ( FREQUENCE/1000 ) -# endif -#elif XILINX_CLOCK_FREQ -# define CONFIG_XILINX_CLOCK_FREQ XILINX_CLOCK_FREQ -#else -# error BAD CLOCK FREQ #endif
/* FSL */ /* #define CONFIG_SYS_FSL_2 */ /* #define FSL_INTR_2 1 */
On 07/09/2012 09:06 PM, Stephan Linz wrote:
Am Montag, den 09.07.2012, 11:20 +0200 schrieb Michal Simek:
microblaze: Fix CONFIG_SYS_HZ usage in board config
Do not use hardcoded value. Use CONFIG_SYS_HZ instead. Separate static configuration to single block.
Signed-off-by: Michal Simekmonstr@monstr.eu
arch/microblaze/cpu/timer.c | 69 ++++++++++++----------- arch/microblaze/include/asm/microblaze_timer.h | 3 + arch/microblaze/lib/board.c | 5 -- include/configs/microblaze-generic.h | 12 +---- 4 files changed, 41 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index cc6b897..dfaaaf5 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -27,42 +27,30 @@ #include<asm/microblaze_intc.h>
volatile int timestamp = 0; +microblaze_timer_t *tmr;
-#ifdef CONFIG_SYS_TIMER_0 ulong get_timer (ulong base) {
- return (timestamp - base);
- if (tmr)
return timestamp - base;- return timestamp++ - base; }
-#else -ulong get_timer (ulong base) -{
- return (timestamp++ - base);
-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 void __udelay(unsigned long usec) {
- int i;
- u32 i;
- i = get_timer(0);
- while ((get_timer(0) - i)< (usec / 1000))
;
- if (tmr) {
i = get_timer(0);while ((get_timer(0) - i)< (usec / 1000));Hi Michal,
- } else {
for (i = 0; i< (usec * XILINX_CLOCK_FREQ / 10000000); i++);this part should be enclosed by #ifdef XILINX_CLOCK_FREQ
It was intentional because XILINX_CLOCK_FREQ must be define. Maybe it could be handle by
#ifndef XILINX_CLOCK_FREQ # error Please define XILINX_CLOCK_FREQ #endif
Thanks, Michal
Am Dienstag, den 10.07.2012, 10:16 +0200 schrieb Michal Simek:
On 07/09/2012 09:06 PM, Stephan Linz wrote:
Am Montag, den 09.07.2012, 11:20 +0200 schrieb Michal Simek:
microblaze: Fix CONFIG_SYS_HZ usage in board config
Do not use hardcoded value. Use CONFIG_SYS_HZ instead. Separate static configuration to single block.
Signed-off-by: Michal Simekmonstr@monstr.eu
arch/microblaze/cpu/timer.c | 69 ++++++++++++----------- arch/microblaze/include/asm/microblaze_timer.h | 3 + arch/microblaze/lib/board.c | 5 -- include/configs/microblaze-generic.h | 12 +---- 4 files changed, 41 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index cc6b897..dfaaaf5 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -27,42 +27,30 @@ #include<asm/microblaze_intc.h>
volatile int timestamp = 0; +microblaze_timer_t *tmr;
-#ifdef CONFIG_SYS_TIMER_0 ulong get_timer (ulong base) {
- return (timestamp - base);
- if (tmr)
return timestamp - base;- return timestamp++ - base; }
-#else -ulong get_timer (ulong base) -{
- return (timestamp++ - base);
-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 void __udelay(unsigned long usec) {
- int i;
- u32 i;
- i = get_timer(0);
- while ((get_timer(0) - i)< (usec / 1000))
;
- if (tmr) {
i = get_timer(0);while ((get_timer(0) - i)< (usec / 1000));Hi Michal,
- } else {
for (i = 0; i< (usec * XILINX_CLOCK_FREQ / 10000000); i++);this part should be enclosed by #ifdef XILINX_CLOCK_FREQ
It was intentional because XILINX_CLOCK_FREQ must be define.
Hm, it's a catch-22 situation in between compilation and run time.
With a prober fdt setup you will implicitly fetch this value from dts (as tmr->loadreg) and you will never reach this code. In the case of a wrong dts content, for example a missing "clock-frequency" entry, this code snipped could be a fall back solution. But what is wrong and what is right?
I think you should make a hard cut here and make a strict distinguish between the old (undef CONFIG_OF_CONTROL) and the new timer setup. Your current code base leads to a hardware description with overdetermined definition of timer clock frequency:
first one here: board/xilinx/microblaze-generic/xparameters.h second one here: board/xilinx/microblaze-generic/dts/microblaze.dts
Maybe it could be handle by
#ifndef XILINX_CLOCK_FREQ # error Please define XILINX_CLOCK_FREQ #endif
better would be:
#if !defined(CONFIG_OF_CONTROL) && !defined(XILINX_CLOCK_FREQ) # error Please define XILINX_CLOCK_FREQ #endif
br, Stephan
On 07/10/2012 08:36 PM, Stephan Linz wrote:
Am Dienstag, den 10.07.2012, 10:16 +0200 schrieb Michal Simek:
On 07/09/2012 09:06 PM, Stephan Linz wrote:
Am Montag, den 09.07.2012, 11:20 +0200 schrieb Michal Simek:
microblaze: Fix CONFIG_SYS_HZ usage in board config
Do not use hardcoded value. Use CONFIG_SYS_HZ instead. Separate static configuration to single block.
Signed-off-by: Michal Simekmonstr@monstr.eu
arch/microblaze/cpu/timer.c | 69 ++++++++++++----------- arch/microblaze/include/asm/microblaze_timer.h | 3 + arch/microblaze/lib/board.c | 5 -- include/configs/microblaze-generic.h | 12 +---- 4 files changed, 41 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index cc6b897..dfaaaf5 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -27,42 +27,30 @@ #include<asm/microblaze_intc.h>
volatile int timestamp = 0; +microblaze_timer_t *tmr;
-#ifdef CONFIG_SYS_TIMER_0 ulong get_timer (ulong base) {
- return (timestamp - base);
- if (tmr)
return timestamp - base;- return timestamp++ - base; }
-#else -ulong get_timer (ulong base) -{
- return (timestamp++ - base);
-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 void __udelay(unsigned long usec) {
- int i;
- u32 i;
- i = get_timer(0);
- while ((get_timer(0) - i)< (usec / 1000))
;
- if (tmr) {
i = get_timer(0);while ((get_timer(0) - i)< (usec / 1000));Hi Michal,
- } else {
for (i = 0; i< (usec * XILINX_CLOCK_FREQ / 10000000); i++);this part should be enclosed by #ifdef XILINX_CLOCK_FREQ
It was intentional because XILINX_CLOCK_FREQ must be define.
Hm, it's a catch-22 situation in between compilation and run time.
With a prober fdt setup you will implicitly fetch this value from dts (as tmr->loadreg) and you will never reach this code. In the case of a wrong dts content, for example a missing "clock-frequency" entry, this code snipped could be a fall back solution. But what is wrong and what is right?
Correct - u-boot is able to work without timer and it is fall back solution. My point was that XILINX_CLOCK_FREQ will be setup to any default value just for sure.
I think you should make a hard cut here and make a strict distinguish between the old (undef CONFIG_OF_CONTROL) and the new timer setup. Your current code base leads to a hardware description with overdetermined definition of timer clock frequency:
Not going to do hard cut. We can do it in future that we will use only DTS initialization but it is long way to go.
first one here: board/xilinx/microblaze-generic/xparameters.h
It should be define here for non FDT platform and my preference is completely remove xparameters for FDT platform.
second one here: board/xilinx/microblaze-generic/dts/microblaze.dts
For FDT platform with timer connected to intc.
Maybe it could be handle by
#ifndef XILINX_CLOCK_FREQ # error Please define XILINX_CLOCK_FREQ #endif
better would be:
#if !defined(CONFIG_OF_CONTROL)&& !defined(XILINX_CLOCK_FREQ) # error Please define XILINX_CLOCK_FREQ #endif
Still make sense to define default value for CONFIG_OF_CONTROL for that fall back solution which should be there.
There is also one more case for system which contains several timers where some of them have IRQ connected and some not. This parsing just read the first timer with selected compatible property. But this is different topic.
Thanks, Michal
Read configuration from DTB.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/timer.c | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index dfaaaf5..91ca42b 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -25,6 +25,9 @@ #include <common.h> #include <asm/microblaze_timer.h> #include <asm/microblaze_intc.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR;
volatile int timestamp = 0; microblaze_timer_t *tmr; @@ -62,11 +65,33 @@ int timer_init (void) u32 preload = 0; u32 ret = 0;
+#ifndef CONFIG_OF_CONTROL #if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ; irq = CONFIG_SYS_TIMER_0_IRQ; tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR); #endif +#else + int temp; + int offset = 0; + + offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset, + "xlnx,xps-timer-1.00.a"); + if (offset > 0) { + temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg"); + if (temp != FDT_ADDR_T_NONE) { + tmr = (microblaze_timer_t *)temp; + irq = fdtdec_get_int(gd->fdt_blob, offset, + "interrupts", -1); + if (irq == -1) + panic("Connect IRQ to system timer\n"); + /* Set default clock frequency */ + temp = fdtdec_get_int(gd->fdt_blob, offset, + "clock-frequency", 0); + preload = temp / CONFIG_SYS_HZ; + } + } +#endif
if (tmr && irq && preload) { tmr->loadreg = preload;
Hi Michal,
Am Montag, den 09.07.2012, 11:20 +0200 schrieb Michal Simek:
Read configuration from DTB.
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/cpu/timer.c | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index dfaaaf5..91ca42b 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -25,6 +25,9 @@ #include <common.h> #include <asm/microblaze_timer.h> #include <asm/microblaze_intc.h> +#include <fdtdec.h>
+DECLARE_GLOBAL_DATA_PTR;
volatile int timestamp = 0; microblaze_timer_t *tmr; @@ -62,11 +65,33 @@ int timer_init (void) u32 preload = 0; u32 ret = 0;
+#ifndef CONFIG_OF_CONTROL #if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ; irq = CONFIG_SYS_TIMER_0_IRQ; tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR); #endif +#else
- int temp;
- int offset = 0;
- offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset,
"xlnx,xps-timer-1.00.a");- if (offset > 0) {
temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg");if (temp != FDT_ADDR_T_NONE) {tmr = (microblaze_timer_t *)temp;irq = fdtdec_get_int(gd->fdt_blob, offset,"interrupts", -1);if (irq == -1)
irq is u32 (unsigned) -- my be a problem. But yes, I'm missing a compilation warning here ...
br, Stephan
panic("Connect IRQ to system timer\n");/* Set default clock frequency */temp = fdtdec_get_int(gd->fdt_blob, offset,"clock-frequency", 0);preload = temp / CONFIG_SYS_HZ;}- }
+#endif
if (tmr && irq && preload) { tmr->loadreg = preload;
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek monstr@monstr.eu wrote:
Read configuration from DTB.
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/cpu/timer.c | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index dfaaaf5..91ca42b 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -25,6 +25,9 @@ #include <common.h> #include <asm/microblaze_timer.h> #include <asm/microblaze_intc.h> +#include <fdtdec.h>
+DECLARE_GLOBAL_DATA_PTR;
volatile int timestamp = 0; microblaze_timer_t *tmr; @@ -62,11 +65,33 @@ int timer_init (void) u32 preload = 0; u32 ret = 0;
+#ifndef CONFIG_OF_CONTROL
Maybe make this #ifdef and put the fdt code first?
#if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ; irq = CONFIG_SYS_TIMER_0_IRQ; tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR); #endif +#else
int temp;int offset = 0;
Can remove =0 I think.
offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset,"xlnx,xps-timer-1.00.a");if (offset > 0) {temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg");if (temp != FDT_ADDR_T_NONE) {tmr = (microblaze_timer_t *)temp;irq = fdtdec_get_int(gd->fdt_blob, offset,"interrupts", -1);if (irq == -1)panic("Connect IRQ to system timer\n");
That's a little cryptic - maybe should mention fdt?
/* Set default clock frequency */temp = fdtdec_get_int(gd->fdt_blob, offset,"clock-frequency", 0);preload = temp / CONFIG_SYS_HZ;}}+#endif
if (tmr && irq && preload) { tmr->loadreg = preload;-- 1.7.0.4
Regards, Simon
On 07/09/2012 11:32 PM, Simon Glass wrote:
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu mailto:monstr@monstr.eu> wrote:
Read configuration from DTB. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>> --- arch/microblaze/cpu/timer.c | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index dfaaaf5..91ca42b 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -25,6 +25,9 @@ #include <common.h> #include <asm/microblaze_timer.h> #include <asm/microblaze_intc.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR; volatile int timestamp = 0; microblaze_timer_t *tmr; @@ -62,11 +65,33 @@ int timer_init (void) u32 preload = 0; u32 ret = 0; +#ifndef CONFIG_OF_CONTROLMaybe make this #ifdef and put the fdt code first?
I see only one reason to do it (and that's why I have done it in intc code too) which is debug purpose. Because you can simple comment ifdefs and hardcoded values below fdt code which you can debug.
#if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ; irq = CONFIG_SYS_TIMER_0_IRQ; tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR); #endif +#else + int temp; + int offset = 0;Can remove =0 I think.
done.
+ + offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset, + "xlnx,xps-timer-1.00.a"); + if (offset > 0) { + temp = fdtdec_get_addr(gd->fdt_blob, offset, "reg"); + if (temp != FDT_ADDR_T_NONE) { + tmr = (microblaze_timer_t *)temp; + irq = fdtdec_get_int(gd->fdt_blob, offset, + "interrupts", -1); + if (irq == -1) + panic("Connect IRQ to system timer\n");That's a little cryptic - maybe should mention fdt?
What about this? panic("Timer IRQ is not found in DTS/DTB\n");
I don't want to write there novels but this is also checked in different tool which generates DTS that's why simple panic should be eliminated for most cases.
Thanks, Michal
On 07/09/2012 11:32 PM, Simon Glass wrote:
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu mailto:monstr@monstr.eu> wrote:
Read configuration from DTB. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>> --- arch/microblaze/cpu/timer.c | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index dfaaaf5..91ca42b 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -25,6 +25,9 @@ #include <common.h> #include <asm/microblaze_timer.h> #include <asm/microblaze_intc.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR; volatile int timestamp = 0; microblaze_timer_t *tmr; @@ -62,11 +65,33 @@ int timer_init (void) u32 preload = 0; u32 ret = 0; +#ifndef CONFIG_OF_CONTROLMaybe make this #ifdef and put the fdt code first?
#if defined(CONFIG_SYS_TIMER_0_ADDR) && defined(CONFIG_SYS_INTC_0_NUM) preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ; irq = CONFIG_SYS_TIMER_0_IRQ; tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR); #endif +#else + int temp; + int offset = 0;Can remove =0 I think.
+ + offset = fdt_node_offset_by_compatible(gd->fdt_blob, offset, + "xlnx,xps-timer-1.00.a");
ok. It must be initialized because offset is passed as argument to the next function. For interrupt code is fine to add there zero but for timer I would keep there offset because system can contain more timers and we can change it to while loop soon.
(NOTE: We have had discussion to use aliases for system timer but we haven't finished it.
Thanks, Michal
Hi Mich
On Tue, Jul 10, 2012 at 11:34 AM, Michal Simek monstr@monstr.eu wrote:
On 07/09/2012 11:32 PM, Simon Glass wrote:
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu mailto: monstr@monstr.eu> wrote:
Read configuration from DTB. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>>
--- arch/microblaze/cpu/timer.c | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index dfaaaf5..91ca42b 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -25,6 +25,9 @@ #include <common.h> #include <asm/microblaze_timer.h> #include <asm/microblaze_intc.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR; volatile int timestamp = 0; microblaze_timer_t *tmr; @@ -62,11 +65,33 @@ int timer_init (void) u32 preload = 0; u32 ret = 0; +#ifndef CONFIG_OF_CONTROLMaybe make this #ifdef and put the fdt code first?
#if defined(CONFIG_SYS_TIMER_0_**ADDR) &&defined(CONFIG_SYS_INTC_0_NUM) preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ; irq = CONFIG_SYS_TIMER_0_IRQ; tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR); #endif +#else + int temp; + int offset = 0;
Can remove =0 I think.
+ + offset = fdt_node_offset_by_compatible(**gd->fdt_blob,offset, + "xlnx,xps-timer-1.00.a");
ok. It must be initialized because offset is passed as argument to the next function. For interrupt code is fine to add there zero but for timer I would keep there offset because system can contain more timers and we can change it to while loop soon.
Yes, well maybe then:
+ offset = fdt_node_offset_by_compatible(**gd->fdt_blob, 0,
(NOTE: We have had discussion to use aliases for system timer but we haven't finished it.
Thanks, Michal
-- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian
Regards, Simon
On 07/10/2012 11:04 PM, Simon Glass wrote:
Hi Mich
On Tue, Jul 10, 2012 at 11:34 AM, Michal Simek <monstr@monstr.eu mailto:monstr@monstr.eu> wrote:
On 07/09/2012 11:32 PM, Simon Glass wrote: Hi Michal, On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu <mailto:monstr@monstr.eu>>> wrote: Read configuration from DTB. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu <mailto:monstr@monstr.eu>>> --- arch/microblaze/cpu/timer.c | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index dfaaaf5..91ca42b 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -25,6 +25,9 @@ #include <common.h> #include <asm/microblaze_timer.h> #include <asm/microblaze_intc.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR; volatile int timestamp = 0; microblaze_timer_t *tmr; @@ -62,11 +65,33 @@ int timer_init (void) u32 preload = 0; u32 ret = 0; +#ifndef CONFIG_OF_CONTROL Maybe make this #ifdef and put the fdt code first? #if defined(CONFIG_SYS_TIMER_0___ADDR) && defined(CONFIG_SYS_INTC_0_NUM) preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ; irq = CONFIG_SYS_TIMER_0_IRQ; tmr = (microblaze_timer_t *) (CONFIG_SYS_TIMER_0_ADDR); #endif +#else + int temp; + int offset = 0; Can remove =0 I think. + + offset = fdt_node_offset_by_compatible(__gd->fdt_blob, offset, + "xlnx,xps-timer-1.00.a"); ok. It must be initialized because offset is passed as argument to the next function. For interrupt code is fine to add there zero but for timer I would keep there offset because system can contain more timers and we can change it to while loop soon.Yes, well maybe then:
+ offset = fdt_node_offset_by_compatible(__gd->fdt_blob, 0,
Both options just works. I can look at asm dump to see which solution is faster and better to use.
Thanks, Michal
Hi Michal,
On Wed, Jul 11, 2012 at 8:18 AM, Michal Simek monstr@monstr.eu wrote:
On 07/10/2012 11:04 PM, Simon Glass wrote:
Hi Mich
On Tue, Jul 10, 2012 at 11:34 AM, Michal Simek <monstr@monstr.eu mailto: monstr@monstr.eu> wrote:
On 07/09/2012 11:32 PM, Simon Glass wrote: Hi Michal, On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu<mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu mailto:monstr@monstr.eu>> wrote:
Read configuration from DTB. Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu> <mailto:monstr@monstr.eu mailto:monstr@monstr.eu>>
--- arch/microblaze/cpu/timer.c | 25+++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/cpu/timer.cb/arch/microblaze/cpu/timer.c index dfaaaf5..91ca42b 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -25,6 +25,9 @@ #include <common.h> #include <asm/microblaze_timer.h> #include <asm/microblaze_intc.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR;
volatile int timestamp = 0; microblaze_timer_t *tmr; @@ -62,11 +65,33 @@ int timer_init (void) u32 preload = 0; u32 ret = 0; +#ifndef CONFIG_OF_CONTROL Maybe make this #ifdef and put the fdt code first? #if defined(CONFIG_SYS_TIMER_0___**ADDR) &&defined(CONFIG_SYS_INTC_0_NUM)
preload = XILINX_CLOCK_FREQ / CONFIG_SYS_HZ; irq = CONFIG_SYS_TIMER_0_IRQ; tmr = (microblaze_timer_t *)(CONFIG_SYS_TIMER_0_ADDR); #endif +#else + int temp; + int offset = 0;
Can remove =0 I think. + + offset = fdt_node_offset_by_compatible(**__gd->fdt_blob,offset,
+ "xlnx,xps-timer-1.00.a"); ok. It must be initialized because offset is passed as argument tothe next function. For interrupt code is fine to add there zero but for timer I would keep there offset because system can contain more timers and we can change it to while loop soon.
Yes, well maybe then:
+ offset = fdt_node_offset_by_compatible(**__gd->fdt_blob, 0,Both options just works. I can look at asm dump to see which solution is faster and better to use.
I doubt there will be any difference there, but OK.
Thanks, Michal
-- Michal Simek, Ing. (M.Eng) w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/ Microblaze U-BOOT custodian
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/lib/board.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index ddbc862..37ec665 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -30,6 +30,7 @@ #include <version.h> #include <watchdog.h> #include <stdio_dev.h> +#include <serial.h> #include <net.h> #include <asm/processor.h> #include <asm/microblaze_intc.h> @@ -118,6 +119,10 @@ void board_init (void) */ mem_malloc_init (CONFIG_SYS_MALLOC_BASE, CONFIG_SYS_MALLOC_LEN);
+#ifdef CONFIG_SERIAL_MULTI + serial_initialize(); +#endif + for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { WATCHDOG_RESET (); if ((*init_fnc_ptr) () != 0) {
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek monstr@monstr.eu wrote:
Signed-off-by: Michal Simek monstr@monstr.eu
Acked-by: Simon Glass sjg@chromium.org
arch/microblaze/lib/board.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index ddbc862..37ec665 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -30,6 +30,7 @@ #include <version.h> #include <watchdog.h> #include <stdio_dev.h> +#include <serial.h> #include <net.h> #include <asm/processor.h> #include <asm/microblaze_intc.h> @@ -118,6 +119,10 @@ void board_init (void) */ mem_malloc_init (CONFIG_SYS_MALLOC_BASE, CONFIG_SYS_MALLOC_LEN);
+#ifdef CONFIG_SERIAL_MULTI
serial_initialize();+#endif
for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { WATCHDOG_RESET (); if ((*init_fnc_ptr) () != 0) {-- 1.7.0.4
On 07/09/2012 11:32 PM, Simon Glass wrote:
Acked-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
Applied.
Thanks, Michal
Move board specific function to board_init function in board/ folder Remove externs from generic board.c Use board_init_f function in board.c file.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/cpu/start.S | 2 +- arch/microblaze/lib/board.c | 17 +++-------------- .../xilinx/microblaze-generic/microblaze-generic.c | 10 ++++++++++ 3 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S index 8a2f634..8564c4e 100644 --- a/arch/microblaze/cpu/start.S +++ b/arch/microblaze/cpu/start.S @@ -149,7 +149,7 @@ clear_bss: cmp r6, r5, r4 /* check if we have reach the end */ bnei r6, 2b 3: /* jumping to board_init */ - brai board_init + brai board_init_f 1: bri 1b
/* diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index 37ec665..1dee830 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -38,13 +38,6 @@
DECLARE_GLOBAL_DATA_PTR;
-#ifdef CONFIG_SYS_GPIO_0 -extern int gpio_init (void); -#endif -#ifdef CONFIG_SYS_FSL_2 -extern void fsl_init2 (void); -#endif - /* * All attempts to come up with a "common" initialization sequence * that works for all boards and architectures failed: some of the @@ -66,20 +59,14 @@ init_fnc_t *init_sequence[] = { #endif serial_init, console_init_f, -#ifdef CONFIG_SYS_GPIO_0 - gpio_init, -#endif interrupts_init, timer_init, -#ifdef CONFIG_SYS_FSL_2 - fsl_init2, -#endif NULL, };
unsigned long monitor_flash_len;
-void board_init (void) +void board_init_f(ulong not_used) { bd_t *bd; init_fnc_t **init_fnc_ptr; @@ -189,6 +176,8 @@ void board_init (void) /* Initialize the console (after the relocation and devices init) */ console_init_r();
+ board_init(); + /* Initialize from environment */ load_addr = getenv_ulong("loadaddr", 16, load_addr);
diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c b/board/xilinx/microblaze-generic/microblaze-generic.c index 4a719ba..608fb20 100644 --- a/board/xilinx/microblaze-generic/microblaze-generic.c +++ b/board/xilinx/microblaze-generic/microblaze-generic.c @@ -31,6 +31,8 @@ #include <asm/microblaze_intc.h> #include <asm/asm.h>
+DECLARE_GLOBAL_DATA_PTR; + int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { #ifdef CONFIG_SYS_GPIO_0 @@ -69,6 +71,14 @@ int fsl_init2 (void) { } #endif
+void board_init(void) +{ + gpio_init(); +#ifdef CONFIG_SYS_FSL_2 + fsl_init2(); +#endif +} + int board_eth_init(bd_t *bis) { int ret = 0;
Prepare for device-tree driven configuration.
Signed-off-by: Michal Simek monstr@monstr.eu --- arch/microblaze/lib/board.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index 1dee830..03ebc97 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -134,7 +134,8 @@ void board_init_f(ulong not_used) #if defined(CONFIG_CMD_FLASH) puts ("Flash: "); bd->bi_flashstart = CONFIG_SYS_FLASH_BASE; - if (0 < (flash_size = flash_init ())) { + flash_size = flash_init(); + if (bd->bi_flashstart && flash_size > 0) { # ifdef CONFIG_SYS_FLASH_CHECKSUM char *s;
@@ -147,7 +148,8 @@ void board_init_f(ulong not_used) s = getenv ("flashchecksum"); if (s && (*s == 'y')) { printf (" CRC: %08X", - crc32 (0, (const unsigned char *) CONFIG_SYS_FLASH_BASE, flash_size) + crc32(0, (const u8 *)bd->bi_flashstart, + flash_size) ); } putc ('\n'); @@ -155,7 +157,7 @@ void board_init_f(ulong not_used) print_size (flash_size, "\n"); # endif /* CONFIG_SYS_FLASH_CHECKSUM */ bd->bi_flashsize = flash_size; - bd->bi_flashoffset = CONFIG_SYS_FLASH_BASE + flash_size; + bd->bi_flashoffset = bd->bi_flashstart + flash_size; } else { puts ("Flash init FAILED"); bd->bi_flashstart = 0;
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek monstr@monstr.eu wrote:
Variable is used when CONFIG_SYS_FLASH_CHECKSUM is used.
Warning log: board.c: In function 'board_init': board.c:101: warning: unused variable 's'
Signed-off-by: Michal Simek monstr@monstr.eu
arch/microblaze/lib/board.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index a8ed7ce..4146f5c 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -98,7 +98,6 @@ void board_init (void) gd = (gd_t *) (CONFIG_SYS_SDRAM_BASE + CONFIG_SYS_GBL_DATA_OFFSET); bd = (bd_t *) (CONFIG_SYS_SDRAM_BASE + CONFIG_SYS_GBL_DATA_OFFSET \ - GENERATED_BD_INFO_SIZE);
char *s;#if defined(CONFIG_CMD_FLASH) ulong flash_size = 0; #endif @@ -157,9 +156,9 @@ void board_init (void) puts ("Flash: "); bd->bi_flashstart = CONFIG_SYS_FLASH_BASE; if (0 < (flash_size = flash_init ())) {
bd->bi_flashsize = flash_size;bd->bi_flashoffset = CONFIG_SYS_FLASH_BASE + flash_size;# ifdef CONFIG_SYS_FLASH_CHECKSUM
char *s;
Is it a good idea to put declarations in the middle of the code? I don't think that is U-Boot style.
You could instead use __maybe_unused on the variable, or #ifdef the declaration :-(
print_size (flash_size, ""); /* * Compute and print flash CRC if flashchecksum is set to'y' @@ -176,6 +175,8 @@ void board_init (void) # else /* !CONFIG_SYS_FLASH_CHECKSUM */ print_size (flash_size, "\n"); # endif /* CONFIG_SYS_FLASH_CHECKSUM */
bd->bi_flashsize = flash_size;bd->bi_flashoffset = CONFIG_SYS_FLASH_BASE + flash_size; } else { puts ("Flash init FAILED"); bd->bi_flashstart = 0;-- 1.7.0.4
Regards,
Simon
On 07/09/2012 11:08 PM, Simon Glass wrote:
Hi Michal,
On Mon, Jul 9, 2012 at 2:20 AM, Michal Simek <monstr@monstr.eu mailto:monstr@monstr.eu> wrote:
Variable is used when CONFIG_SYS_FLASH_CHECKSUM is used. Warning log: board.c: In function 'board_init': board.c:101: warning: unused variable 's' Signed-off-by: Michal Simek <monstr@monstr.eu <mailto:monstr@monstr.eu>> --- arch/microblaze/lib/board.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c index a8ed7ce..4146f5c 100644 --- a/arch/microblaze/lib/board.c +++ b/arch/microblaze/lib/board.c @@ -98,7 +98,6 @@ void board_init (void) gd = (gd_t *) (CONFIG_SYS_SDRAM_BASE + CONFIG_SYS_GBL_DATA_OFFSET); bd = (bd_t *) (CONFIG_SYS_SDRAM_BASE + CONFIG_SYS_GBL_DATA_OFFSET \ - GENERATED_BD_INFO_SIZE); - char *s; #if defined(CONFIG_CMD_FLASH) ulong flash_size = 0; #endif @@ -157,9 +156,9 @@ void board_init (void) puts ("Flash: "); bd->bi_flashstart = CONFIG_SYS_FLASH_BASE; if (0 < (flash_size = flash_init ())) { - bd->bi_flashsize = flash_size; - bd->bi_flashoffset = CONFIG_SYS_FLASH_BASE + flash_size; # ifdef CONFIG_SYS_FLASH_CHECKSUM + char *s; +Is it a good idea to put declarations in the middle of the code? I don't think that is U-Boot style.
You could instead use __maybe_unused on the variable, or #ifdef the declaration :-(
I wanted to avoid that ifdef mess. Will look at __maybe_unused. I wasn't aware about it.
Thanks, Michal
participants (3)
-
Michal Simek -
Simon Glass -
Stephan Linz