[PATCH 2/6] MIPS: ip27-timer: fix unsigned irq < 0

[PATCH 2/6] MIPS: ip27-timer: fix unsigned irq < 0

[Roel Kluin]

irq is unsigned, cast to signed to evaluate the allocate_irqno() return value,
    
Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---   
diff --git a/arch/mips/sgi-ip27/ip27-timer.c b/arch/mips/sgi-ip27/ip27-timer.c
index 25d3baf..3c08afd 100644
--- a/arch/mips/sgi-ip27/ip27-timer.c
+++ b/arch/mips/sgi-ip27/ip27-timer.c
@@ -222,19 +222,19 @@ static void __init hub_rt_clock_event_global_init(void)
 	unsigned int irq;
 
 	do {
 		smp_wmb();
 		irq = rt_timer_irq;
 		if (irq)
 			break;
 
 		irq = allocate_irqno();
-		if (irq < 0)
+		if ((int) irq < 0)
 			panic("Allocation of irq number for timer failed");
 	} while (xchg(&rt_timer_irq, irq));
 
 	set_irq_chip_and_handler(irq, &rt_irq_type, handle_percpu_irq);
 	setup_irq(irq, &hub_rt_irqaction);
 }
 
 static cycle_t hub_rt_read(void)
 {

Re: [PATCH 2/6] MIPS: ip27-timer: fix unsigned irq < 0

[Thomas Bogendoerfer]

On Wed, Apr 16, 2008 at 03:43:56AM +0200, Roel Kluin wrote:
> irq is unsigned, cast to signed to evaluate the allocate_irqno() return value,
>     
> Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
> ---   
> diff --git a/arch/mips/sgi-ip27/ip27-timer.c b/arch/mips/sgi-ip27/ip27-timer.c
> index 25d3baf..3c08afd 100644
> --- a/arch/mips/sgi-ip27/ip27-timer.c
> +++ b/arch/mips/sgi-ip27/ip27-timer.c
> @@ -222,19 +222,19 @@ static void __init hub_rt_clock_event_global_init(void)
>  	unsigned int irq;
>  
>  	do {
>  		smp_wmb();
>  		irq = rt_timer_irq;
>  		if (irq)
>  			break;
>  
>  		irq = allocate_irqno();
> -		if (irq < 0)
> +		if ((int) irq < 0)

Why don't you just make irq and rt_timer_irq an int ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]

[PATCH 2/6 v2] MIPS: ip27-timer: unsigned irq to evaluate allocate_irqno()

[Roel Kluin]

Thomas Bogendoerfer wrote:
> On Wed, Apr 16, 2008 at 03:43:56AM +0200, Roel Kluin wrote:
>> irq is unsigned, cast to signed to evaluate the allocate_irqno() return value,
 
>> +		if ((int) irq < 0)
> 
> Why don't you just make irq and rt_timer_irq an int ?

Ok, thanks, It should be right, but I cannot test this (no hardware).
---
when allocate_irqno() returns a negative error value, but is stored in an
unsigned variable 'irq', the test '(irq < 0)' won't work.

Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/arch/mips/sgi-ip27/ip27-timer.c b/arch/mips/sgi-ip27/ip27-timer.c
index 25d3baf..9cebc9e 100644
--- a/arch/mips/sgi-ip27/ip27-timer.c
+++ b/arch/mips/sgi-ip27/ip27-timer.c
@@ -158,7 +158,7 @@ static void rt_set_mode(enum clock_event_mode mode,
 	}
 }
 
-unsigned int rt_timer_irq;
+int rt_timer_irq;
 
 static irqreturn_t hub_rt_counter_handler(int irq, void *dev_id)
 {
@@ -219,7 +219,7 @@ static void __cpuinit hub_rt_clock_event_init(void)
 
 static void __init hub_rt_clock_event_global_init(void)
 {
-	unsigned int irq;
+	int irq;
 
 	do {
 		smp_wmb();

Re: [PATCH 2/6 v2] MIPS: ip27-timer: unsigned irq to evaluate allocate_irqno()

[Thomas Bogendoerfer]

On Wed, Apr 16, 2008 at 05:09:58PM +0200, Roel Kluin wrote:
> Thomas Bogendoerfer wrote:
> > On Wed, Apr 16, 2008 at 03:43:56AM +0200, Roel Kluin wrote:
> >> irq is unsigned, cast to signed to evaluate the allocate_irqno() return value,
>  
> >> +		if ((int) irq < 0)
> > 
> > Why don't you just make irq and rt_timer_irq an int ?
> 
> Ok, thanks, It should be right, but I cannot test this (no hardware).

I've tested it on real hardware.

Acked-By: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH 2/6 v2] MIPS: ip27-timer: unsigned irq to evaluate allocate_irqno()

[Ralf Baechle]

On Thu, Apr 17, 2008 at 12:27:56AM +0200, Thomas Bogendoerfer wrote:

> On Wed, Apr 16, 2008 at 05:09:58PM +0200, Roel Kluin wrote:
> > Thomas Bogendoerfer wrote:
> > > On Wed, Apr 16, 2008 at 03:43:56AM +0200, Roel Kluin wrote:
> > >> irq is unsigned, cast to signed to evaluate the allocate_irqno() return value,
> >  
> > >> +		if ((int) irq < 0)
> > > 
> > > Why don't you just make irq and rt_timer_irq an int ?
> > 
> > Ok, thanks, It should be right, but I cannot test this (no hardware).
> 
> I've tested it on real hardware.
> 
> Acked-By: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

Thanks, applied.

  Ralf