Moving per-arch IRQ handling code into common directories

Moving per-arch IRQ handling code into common directories

[Peter Chubb]

Hi Folks,
  Inside each arch-specific kernel/irq.c, there's a comment something like, 
  /* (mostly architecture independent, will move to kernel/irq.c in 2.5.) */

This obviously hasn't happened, even though there was a patch by
Andrey Panin floating about around a year ago.  Is there some
fundamental objection to consolidating the IRQ handling as far as
possible, or was it just that the patch didn't get high enough profile?


-- 
Dr Peter Chubb  http://www.gelato.unsw.edu.au  peterc AT gelato.unsw.edu.au
The technical we do immediately,  the political takes *forever*

Re: Moving per-arch IRQ handling code into common directories

[Anton Blanchard]

Hi,

>   Inside each arch-specific kernel/irq.c, there's a comment something like, 
>   /* (mostly architecture independent, will move to kernel/irq.c in 2.5.) */
> 
> This obviously hasn't happened, even though there was a patch by
> Andrey Panin floating about around a year ago.  Is there some
> fundamental objection to consolidating the IRQ handling as far as
> possible, or was it just that the patch didn't get high enough profile?

I think it died because we were in a freeze at the time. Id like to see
it happen again, perhaps we can get something together to go into -mm.

Anton

Re: Moving per-arch IRQ handling code into common directories

[Andrey Panin]

[-- Attachment #1.1: Type: text/plain, Size: 991 bytes --]

On 193, 07 11, 2004 at 09:09:20PM +1000, Anton Blanchard wrote:
> 
> Hi,
> 
> >   Inside each arch-specific kernel/irq.c, there's a comment something like, 
> >   /* (mostly architecture independent, will move to kernel/irq.c in 2.5.) */
> > 
> > This obviously hasn't happened, even though there was a patch by
> > Andrey Panin floating about around a year ago.  Is there some
> > fundamental objection to consolidating the IRQ handling as far as
> > possible, or was it just that the patch didn't get high enough profile?
> 
> I think it died because we were in a freeze at the time. Id like to see
> it happen again, perhaps we can get something together to go into -mm.

No, it almost died because of lack of time to track changes in so many
architectures. BTW can you take a look at attached patch, which removes
do_free_irq() crap from ppc64 irq handling code ?

-- 
Andrey Panin		| Linux and UNIX system administrator
pazke@donpac.ru		| PGP key: wwwkeys.pgp.net

[-- Attachment #1.2: patch-ppc64-free_irq-cleanup --]
[-- Type: text/plain, Size: 2821 bytes --]

diff -urpN -X /usr/share/dontdiff linux-2.6.7.vanilla/arch/ppc64/kernel/irq.c linux-2.6.7-ppc64/arch/ppc64/kernel/irq.c
--- linux-2.6.7.vanilla/arch/ppc64/kernel/irq.c	Sat May 22 14:58:15 2004
+++ linux-2.6.7-ppc64/arch/ppc64/kernel/irq.c	Sat May 22 19:58:35 2004
@@ -143,47 +143,6 @@ EXPORT_SYMBOL(synchronize_irq);
 
 #endif /* CONFIG_SMP */
 
-/* XXX Make this into free_irq() - Anton */
-
-/* This could be promoted to a real free_irq() ... */
-static int
-do_free_irq(int irq, void* dev_id)
-{
-	irq_desc_t *desc = get_irq_desc(irq);
-	struct irqaction **p;
-	unsigned long flags;
-
-	spin_lock_irqsave(&desc->lock,flags);
-	p = &desc->action;
-	for (;;) {
-		struct irqaction * action = *p;
-		if (action) {
-			struct irqaction **pp = p;
-			p = &action->next;
-			if (action->dev_id != dev_id)
-				continue;
-
-			/* Found it - now remove it from the list of entries */
-			*pp = action->next;
-			if (!desc->action) {
-				desc->status |= IRQ_DISABLED;
-				mask_irq(irq);
-			}
-			spin_unlock_irqrestore(&desc->lock,flags);
-
-			/* Wait to make sure it's not being used on another CPU */
-			synchronize_irq(irq);
-			kfree(action);
-			return 0;
-		}
-		printk("Trying to free free IRQ%d\n",irq);
-		spin_unlock_irqrestore(&desc->lock,flags);
-		break;
-	}
-	return -ENOENT;
-}
-
-
 int request_irq(unsigned int irq,
 	irqreturn_t (*handler)(int, void *, struct pt_regs *),
 	unsigned long irqflags, const char * devname, void *dev_id)
@@ -194,8 +153,7 @@ int request_irq(unsigned int irq,
 	if (irq >= NR_IRQS)
 		return -EINVAL;
 	if (!handler)
-		/* We could implement really free_irq() instead of that... */
-		return do_free_irq(irq, dev_id);
+		return -EINVAL;
 
 	action = (struct irqaction *)
 		kmalloc(sizeof(struct irqaction), GFP_KERNEL);
@@ -222,7 +180,38 @@ EXPORT_SYMBOL(request_irq);
 
 void free_irq(unsigned int irq, void *dev_id)
 {
-	request_irq(irq, NULL, 0, NULL, dev_id);
+	irq_desc_t *desc = get_irq_desc(irq);
+	struct irqaction **p;
+	unsigned long flags;
+
+	spin_lock_irqsave(&desc->lock,flags);
+	p = &desc->action;
+	for (;;) {
+		struct irqaction * action = *p;
+		if (action) {
+			struct irqaction **pp = p;
+			p = &action->next;
+			if (action->dev_id != dev_id)
+				continue;
+
+			/* Found it - now remove it from the list of entries */
+			*pp = action->next;
+			if (!desc->action) {
+				desc->status |= IRQ_DISABLED;
+				mask_irq(irq);
+			}
+			spin_unlock_irqrestore(&desc->lock,flags);
+
+			/* Wait to make sure it's not being used on another CPU */
+			synchronize_irq(irq);
+			kfree(action);
+			return;
+		}
+		printk("Trying to free free IRQ%d\n",irq);
+		spin_unlock_irqrestore(&desc->lock,flags);
+		break;
+	}
+	return;
 }
 
 EXPORT_SYMBOL(free_irq);

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Moving per-arch IRQ handling code into common directories

[Anton Blanchard]

 
Hi,

> No, it almost died because of lack of time to track changes in so many
> architectures. BTW can you take a look at attached patch, which removes
> do_free_irq() crap from ppc64 irq handling code ?

Thanks, looks good.

Anton

--

From: Andrey Panin <pazke@donpac.ru>

Fix ppc64 free_irq.

Signed-off-by: Anton Blanchard <anton@samba.org>

diff -urpN -X /usr/share/dontdiff linux-2.6.7.vanilla/arch/ppc64/kernel/irq.c linux-2.6.7-ppc64/arch/ppc64/kernel/irq.c
--- linux-2.6.7.vanilla/arch/ppc64/kernel/irq.c	Sat May 22 14:58:15 2004
+++ linux-2.6.7-ppc64/arch/ppc64/kernel/irq.c	Sat May 22 19:58:35 2004
@@ -143,47 +143,6 @@ EXPORT_SYMBOL(synchronize_irq);
 
 #endif /* CONFIG_SMP */
 
-/* XXX Make this into free_irq() - Anton */
-
-/* This could be promoted to a real free_irq() ... */
-static int
-do_free_irq(int irq, void* dev_id)
-{
-	irq_desc_t *desc = get_irq_desc(irq);
-	struct irqaction **p;
-	unsigned long flags;
-
-	spin_lock_irqsave(&desc->lock,flags);
-	p = &desc->action;
-	for (;;) {
-		struct irqaction * action = *p;
-		if (action) {
-			struct irqaction **pp = p;
-			p = &action->next;
-			if (action->dev_id != dev_id)
-				continue;
-
-			/* Found it - now remove it from the list of entries */
-			*pp = action->next;
-			if (!desc->action) {
-				desc->status |= IRQ_DISABLED;
-				mask_irq(irq);
-			}
-			spin_unlock_irqrestore(&desc->lock,flags);
-
-			/* Wait to make sure it's not being used on another CPU */
-			synchronize_irq(irq);
-			kfree(action);
-			return 0;
-		}
-		printk("Trying to free free IRQ%d\n",irq);
-		spin_unlock_irqrestore(&desc->lock,flags);
-		break;
-	}
-	return -ENOENT;
-}
-
-
 int request_irq(unsigned int irq,
 	irqreturn_t (*handler)(int, void *, struct pt_regs *),
 	unsigned long irqflags, const char * devname, void *dev_id)
@@ -194,8 +153,7 @@ int request_irq(unsigned int irq,
 	if (irq >= NR_IRQS)
 		return -EINVAL;
 	if (!handler)
-		/* We could implement really free_irq() instead of that... */
-		return do_free_irq(irq, dev_id);
+		return -EINVAL;
 
 	action = (struct irqaction *)
 		kmalloc(sizeof(struct irqaction), GFP_KERNEL);
@@ -222,7 +180,38 @@ EXPORT_SYMBOL(request_irq);
 
 void free_irq(unsigned int irq, void *dev_id)
 {
-	request_irq(irq, NULL, 0, NULL, dev_id);
+	irq_desc_t *desc = get_irq_desc(irq);
+	struct irqaction **p;
+	unsigned long flags;
+
+	spin_lock_irqsave(&desc->lock,flags);
+	p = &desc->action;
+	for (;;) {
+		struct irqaction * action = *p;
+		if (action) {
+			struct irqaction **pp = p;
+			p = &action->next;
+			if (action->dev_id != dev_id)
+				continue;
+
+			/* Found it - now remove it from the list of entries */
+			*pp = action->next;
+			if (!desc->action) {
+				desc->status |= IRQ_DISABLED;
+				mask_irq(irq);
+			}
+			spin_unlock_irqrestore(&desc->lock,flags);
+
+			/* Wait to make sure it's not being used on another CPU */
+			synchronize_irq(irq);
+			kfree(action);
+			return;
+		}
+		printk("Trying to free free IRQ%d\n",irq);
+		spin_unlock_irqrestore(&desc->lock,flags);
+		break;
+	}
+	return;
 }
 
 EXPORT_SYMBOL(free_irq);