Re: [U-Boot] [PATCH v2 3/7] microblaze: intc: Registering interrupt should return value

2012-08-08 Thread Michal Simek 

On 08/07/2012 10:10 PM, Stephan Linz wrote:

Am Montag, den 06.08.2012, 09:46 +0200 schrieb Michal Simek:

Return value to find out if un/registration was succesful.

Signed-off-by: Michal Simek mon...@monstr.eu

---
v2: Add comment to header file to describe parameters and return codes
---
  arch/microblaze/cpu/interrupts.c  |   16 +---
  arch/microblaze/include/asm/microblaze_intc.h |   11 ++-
  2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c
index ee67082..08f6bad 100644
--- a/arch/microblaze/cpu/interrupts.c
+++ b/arch/microblaze/cpu/interrupts.c
@@ -91,14 +91,13 @@ static void disable_one_interrupt(int irq)
  #endif
  }

-/* 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 +105,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..e9640f5 100644
--- a/arch/microblaze/include/asm/microblaze_intc.h
+++ b/arch/microblaze/include/asm/microblaze_intc.h
@@ -39,7 +39,16 @@ struct irq_action {
int count; /* number of interrupt */
  };

-void install_interrupt_handler (int irq, interrupt_handler_t * hdlr,
+/**
+ * Register and unregister interrupt handler rutines
+ *
+ * @param irq  IRQ number
+ * @param hdlr Interrupt handler rutine
+ * @param arg  Pointer to argument which is passed to int. handler rutine
+ * @return 0 if registration pass, 1 if unregistration pass,
+ * or an error code  0 otherwise
+ */
+int install_interrupt_handler(int irq, interrupt_handler_t *hdlr,
   void *arg);


Hi Michal,

why not two different functions here, one for registration, another one
for unregistration? To mee it is puzzling to use a 'install' function
for unregistration ...


partially agree with that. Maybe we could introduce one macro for that.

#define uninstall_interrupt_handler(irq) install_interrupt_handler(irq, NULL, 
NULL)



... whatever, you should evaluate the return code in fsl_init2() too.


Not necessary to do it in this patch.

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 3/7] microblaze: intc: Registering interrupt should return value

2012-08-08 Thread Stephan Linz 
Am Mittwoch, den 08.08.2012, 10:27 +0200 schrieb Michal Simek: 
 On 08/07/2012 10:10 PM, Stephan Linz wrote:
  Am Montag, den 06.08.2012, 09:46 +0200 schrieb Michal Simek:
  Return value to find out if un/registration was succesful.
 
  Signed-off-by: Michal Simek mon...@monstr.eu
 
  ---
  v2: Add comment to header file to describe parameters and return codes
  ---
arch/microblaze/cpu/interrupts.c  |   16 +---
arch/microblaze/include/asm/microblaze_intc.h |   11 ++-
2 files changed, 19 insertions(+), 8 deletions(-)
 
  diff --git a/arch/microblaze/cpu/interrupts.c 
  b/arch/microblaze/cpu/interrupts.c
  index ee67082..08f6bad 100644
  --- a/arch/microblaze/cpu/interrupts.c
  +++ b/arch/microblaze/cpu/interrupts.c
  @@ -91,14 +91,13 @@ static void disable_one_interrupt(int irq)
#endif
}
 
  -/* 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 +105,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..e9640f5 100644
  --- a/arch/microblaze/include/asm/microblaze_intc.h
  +++ b/arch/microblaze/include/asm/microblaze_intc.h
  @@ -39,7 +39,16 @@ struct irq_action {
 int count; /* number of interrupt */
};
 
  -void install_interrupt_handler (int irq, interrupt_handler_t * hdlr,
  +/**
  + * Register and unregister interrupt handler rutines
  + *
  + * @param irq IRQ number
  + * @param hdlrInterrupt handler rutine
  + * @param arg Pointer to argument which is passed to int. handler 
  rutine
  + * @return0 if registration pass, 1 if unregistration pass,
  + *or an error code  0 otherwise
  + */
  +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr,
void *arg);
 
  Hi Michal,
 
  why not two different functions here, one for registration, another one
  for unregistration? To mee it is puzzling to use a 'install' function
  for unregistration ...
 
 partially agree with that. Maybe we could introduce one macro for that.
 
 #define uninstall_interrupt_handler(irq) install_interrupt_handler(irq, NULL, 
 NULL)

yes, that is ok ...

 
 
  ... whatever, you should evaluate the return code in fsl_init2() too.
 
 Not necessary to do it in this patch.

yes, it's another part ...

br,
Stephan

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 3/7] microblaze: intc: Registering interrupt should return value

2012-08-07 Thread Stephan Linz 
Am Montag, den 06.08.2012, 09:46 +0200 schrieb Michal Simek: 
 Return value to find out if un/registration was succesful.
 
 Signed-off-by: Michal Simek mon...@monstr.eu
 
 ---
 v2: Add comment to header file to describe parameters and return codes
 ---
  arch/microblaze/cpu/interrupts.c  |   16 +---
  arch/microblaze/include/asm/microblaze_intc.h |   11 ++-
  2 files changed, 19 insertions(+), 8 deletions(-)
 
 diff --git a/arch/microblaze/cpu/interrupts.c 
 b/arch/microblaze/cpu/interrupts.c
 index ee67082..08f6bad 100644
 --- a/arch/microblaze/cpu/interrupts.c
 +++ b/arch/microblaze/cpu/interrupts.c
 @@ -91,14 +91,13 @@ static void disable_one_interrupt(int irq)
  #endif
  }
  
 -/* 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 +105,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..e9640f5 100644
 --- a/arch/microblaze/include/asm/microblaze_intc.h
 +++ b/arch/microblaze/include/asm/microblaze_intc.h
 @@ -39,7 +39,16 @@ struct irq_action {
   int count; /* number of interrupt */
  };
  
 -void install_interrupt_handler (int irq, interrupt_handler_t * hdlr,
 +/**
 + * Register and unregister interrupt handler rutines
 + *
 + * @param irqIRQ number
 + * @param hdlr   Interrupt handler rutine
 + * @param argPointer to argument which is passed to int. handler 
 rutine
 + * @return   0 if registration pass, 1 if unregistration pass,
 + *   or an error code  0 otherwise
 + */
 +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr,
  void *arg);

Hi Michal,

why not two different functions here, one for registration, another one
for unregistration? To mee it is puzzling to use a 'install' function
for unregistration ...

... whatever, you should evaluate the return code in fsl_init2() too.

 
  int interrupts_init(void);


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 3/7] microblaze: intc: Registering interrupt should return value

2012-08-06 Thread Michal Simek 
Return value to find out if un/registration was succesful.

Signed-off-by: Michal Simek mon...@monstr.eu

---
v2: Add comment to header file to describe parameters and return codes
---
 arch/microblaze/cpu/interrupts.c  |   16 +---
 arch/microblaze/include/asm/microblaze_intc.h |   11 ++-
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c
index ee67082..08f6bad 100644
--- a/arch/microblaze/cpu/interrupts.c
+++ b/arch/microblaze/cpu/interrupts.c
@@ -91,14 +91,13 @@ static void disable_one_interrupt(int irq)
 #endif
 }
 
-/* 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 +105,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..e9640f5 100644
--- a/arch/microblaze/include/asm/microblaze_intc.h
+++ b/arch/microblaze/include/asm/microblaze_intc.h
@@ -39,7 +39,16 @@ struct irq_action {
int count; /* number of interrupt */
 };
 
-void install_interrupt_handler (int irq, interrupt_handler_t * hdlr,
+/**
+ * Register and unregister interrupt handler rutines
+ *
+ * @param irq  IRQ number
+ * @param hdlr Interrupt handler rutine
+ * @param arg  Pointer to argument which is passed to int. handler rutine
+ * @return 0 if registration pass, 1 if unregistration pass,
+ * or an error code  0 otherwise
+ */
+int install_interrupt_handler(int irq, interrupt_handler_t *hdlr,
   void *arg);
 
 int interrupts_init(void);
-- 
1.7.0.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot