Re: [U-Boot] [PATCH 03/12] microblaze: intc: Clear interrupt code

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