linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 7/15] celleb: supporting interrupts
@ 2006-12-12  3:33 Ishizaki Kou
  2006-12-12 13:01 ` Arnd Bergmann
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Ishizaki Kou @ 2006-12-12  3:33 UTC (permalink / raw)
  To: paulus, linuxppc-dev

This patch creates Celleb platform dependent files to support interrupts.

Signed-off-by: Kou Ishizaki <kou.ishizaki.co.jp>
---

Index: linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.c
diff -u /dev/null linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.c:1.1
--- /dev/null	Mon Dec 11 20:37:34 2006
+++ linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.c	Wed Dec  6 08:43:15 2006
@@ -0,0 +1,256 @@
+/*
+ * Celleb/Beat Interrupt controller
+ *
+ * (C) Copyright 2006 TOSHIBA CORPORATION
+ *
+ * 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.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/percpu.h>
+#include <linux/types.h>
+
+#include <asm/io.h>
+#include <asm/pgtable.h>
+#include <asm/prom.h>
+#include <asm/ptrace.h>
+#include <asm/udbg.h>
+#include <asm/machdep.h>
+
+#include "interrupt.h"
+#include "beat.h"
+
+#define	MAX_IRQS	NR_IRQS
+static spinlock_t beatic_irq_mask_lock;
+static uint64_t	beatic_irq_mask[(MAX_IRQS+255)/64];
+
+static struct irq_host *beatic_host = NULL;
+
+/*
+ * In this implementation, "virq" == "IRQ plug number",
+ * "(irq_hw_number_t)hwirq" == "IRQ outlet number".
+ */
+
+static void beatic_unmask_irq(unsigned int irq_plug)
+{
+	int off;
+	unsigned long flags;
+
+	spin_lock_irqsave(&beatic_irq_mask_lock, flags);
+	off = (irq_plug / 256) * 4;
+	beatic_irq_mask[irq_plug/64] |= 1UL << (63 - (irq_plug%64));
+
+	if (beat_set_interrupt_mask(irq_plug&~255UL,
+		beatic_irq_mask[off + 0], beatic_irq_mask[off + 1],
+		beatic_irq_mask[off + 2], beatic_irq_mask[off + 3]) != 0)
+		panic("Failed to set mask IRQ!");
+	spin_unlock_irqrestore(&beatic_irq_mask_lock, flags);
+}
+
+static void beatic_mask_irq(unsigned int irq_plug)
+{
+	int off;
+	unsigned long flags;
+
+	spin_lock_irqsave(&beatic_irq_mask_lock, flags);
+	off = (irq_plug / 256) * 4;
+	beatic_irq_mask[irq_plug/64] &= ~(1UL << (63 - (irq_plug%64)));
+
+	if (beat_set_interrupt_mask(irq_plug&~255UL,
+		beatic_irq_mask[off + 0], beatic_irq_mask[off + 1],
+		beatic_irq_mask[off + 2], beatic_irq_mask[off + 3]) != 0)
+		panic("Failed to clear mask IRQ!");
+
+	spin_unlock_irqrestore(&beatic_irq_mask_lock, flags);
+}
+
+static void beatic_end_irq(unsigned int irq_plug)
+{
+	int64_t err;
+
+	if ((err = beat_downcount_of_interrupt(irq_plug)) != 0) {
+		if ((err & 0xFFFFFFFF) != 0xFFFFFFF5) /* -11: wrong state */
+			panic("Failed to downcount IRQ! Error = %16lx", err);
+
+		printk(KERN_ERR "IRQ over-downcounted, plug %d\n", irq_plug);
+	}
+	beatic_unmask_irq(irq_plug);
+}
+
+static struct irq_chip beatic_pic = {
+	.typename = " CELL-BEAT ",
+	.name = " CELL-BEAT ",
+	.unmask = beatic_unmask_irq,
+	.mask = beatic_mask_irq,
+	.eoi = beatic_end_irq,
+};
+
+/*
+ * Dispose binding hardware IRQ number (hw) and Virtuql IRQ number (virq),
+ * update flags.
+ *
+ * Note that the number (virq) is already assigned at upper layer.
+ */
+static void beatic_pic_host_unmap(struct irq_host *h, unsigned int virq)
+{
+	beat_destruct_irq_plug(virq);
+}
+
+/*
+ * Create or update binding hardware IRQ number (hw) and Virtuql
+ * IRQ number (virq). This is called only once for a given mapping.
+ *
+ * Note that the number (virq) is already assigned at upper layer.
+ */
+static int beatic_pic_host_map(struct irq_host *h, unsigned int virq,
+			       irq_hw_number_t hw)
+{
+	struct irq_desc *desc = get_irq_desc(virq);
+	int64_t	err;
+
+	if ((err = beat_construct_and_connect_irq_plug(virq, hw)) < 0)
+		return -EIO;
+
+	desc->status |= IRQ_LEVEL;
+	set_irq_chip_and_handler(virq, &beatic_pic, handle_fasteoi_irq);
+	return 0;
+}
+
+/*
+ * Update binding hardware IRQ number (hw) and Virtuql
+ * IRQ number (virq). This is called only once for a given mapping.
+ */
+static void beatic_pic_host_remap(struct irq_host *h, unsigned int virq,
+			       irq_hw_number_t hw)
+{
+	beat_construct_and_connect_irq_plug(virq, hw);
+}
+
+/*
+ * Translate device-tree interrupt spec to irq_hw_number_t style (ulong),
+ * to pass away to irq_create_mapping().
+ *
+ * Called from irq_create_of_mapping() only.
+ * Note: We have only 1 entry to translate.
+ */
+static int beatic_pic_host_xlate(struct irq_host *h, struct device_node *ct,
+				 u32 *intspec, unsigned int intsize,
+				 irq_hw_number_t *out_hwirq,
+				 unsigned int *out_flags)
+{
+	u64 *intspec2 = (u64 *)intspec;
+
+	*out_hwirq = *intspec2;
+	*out_flags |= IRQ_TYPE_LEVEL_LOW;
+	return 0;
+}
+
+static struct irq_host_ops beatic_pic_host_ops = {
+	.map = beatic_pic_host_map,
+	.remap = beatic_pic_host_remap,
+	.unmap = beatic_pic_host_unmap,
+	.xlate = beatic_pic_host_xlate,
+};
+
+/*
+ * Get an IRQ number
+ * Note: returns VIRQ
+ */
+static inline unsigned int
+beatic_get_irq_plug(void)
+{
+	int i;
+	uint64_t	pending[4], ub;
+
+	for (i = 0; i < MAX_IRQS; i += 256) {
+		beat_detect_pending_interrupts(i, pending);
+		__asm__ ("cntlzd %0,%1":"=r"(ub):
+			"r"(pending[0] & beatic_irq_mask[i/64+0]));
+		if (ub != 64)
+			return i + ub + 0;
+		__asm__ ("cntlzd %0,%1":"=r"(ub):
+			"r"(pending[1] & beatic_irq_mask[i/64+1]));
+		if (ub != 64)
+			return i + ub + 64;
+		__asm__ ("cntlzd %0,%1":"=r"(ub):
+			"r"(pending[2] & beatic_irq_mask[i/64+2]));
+		if (ub != 64)
+			return i + ub + 128;
+		__asm__ ("cntlzd %0,%1":"=r"(ub):
+			"r"(pending[3] & beatic_irq_mask[i/64+3]));
+		if (ub != 64)
+			return i + ub + 192;
+	}
+
+	return NO_IRQ;
+}
+unsigned int beatic_get_irq(void)
+{
+	unsigned int ret;
+
+	ret = beatic_get_irq_plug();
+	if (ret != NO_IRQ)
+		beatic_mask_irq(ret);
+	return ret;
+}
+
+/*
+ */
+void __init beatic_init_IRQ(void)
+{
+	int	i;
+
+	spin_lock_init(&beatic_irq_mask_lock);
+	memset(beatic_irq_mask, 0, sizeof(beatic_irq_mask));
+	for (i = 0; i < MAX_IRQS; i += 256)
+		beat_set_interrupt_mask(i, 0L, 0L, 0L, 0L);
+
+	/* Set out get_irq function */
+	ppc_md.get_irq = beatic_get_irq;
+
+	/* Allocate an irq host */
+	beatic_host = irq_alloc_host(IRQ_HOST_MAP_NOMAP, 0,
+					 &beatic_pic_host_ops,
+					 0);
+	BUG_ON(beatic_host == NULL);
+	irq_set_default_host(beatic_host);
+	spin_lock_init(&beatic_irq_mask_lock);
+}
+
+#ifdef CONFIG_SMP
+
+/* Nullified to compile with SMP mode */
+void beatic_setup_cpu(int cpu)
+{
+}
+
+void beatic_cause_IPI(int cpu, int mesg)
+{
+}
+
+void beatic_request_IPIs(void)
+{
+}
+#endif /* CONFIG_SMP */
+
+void beatic_deinit_IRQ(void)
+{
+	int	i;
+
+	for (i = 1; i < NR_IRQS; i++)
+		beat_destruct_irq_plug(i);
+}
Index: linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.h
diff -u /dev/null linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.h:1.1
--- /dev/null	Mon Dec 11 20:37:34 2006
+++ linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.h	Wed Dec  6 08:43:15 2006
@@ -0,0 +1,36 @@
+/*
+ * Celleb/Beat Interrupt controller
+ *
+ * (C) Copyright 2006 TOSHIBA CORPORATION
+ *
+ * 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.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef ASM_BEAT_PIC_H
+#define ASM_BEAT_PIC_H
+#ifdef __KERNEL__
+
+extern void beatic_init_IRQ(void);
+extern unsigned int beatic_get_irq(void);
+extern void beatic_cause_IPI(int cpu, int mesg);
+extern void beatic_request_IPIs(void);
+extern void beatic_setup_cpu(int);
+extern void beatic_local_enable(void);
+extern void beatic_local_disable(void);
+
+extern u8 beatic_get_target_id(int cpu);
+
+#endif
+#endif /* ASM_BEAT_PIC_H */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 7/15] celleb: supporting interrupts
  2006-12-12  3:33 [PATCH 7/15] celleb: supporting interrupts Ishizaki Kou
@ 2006-12-12 13:01 ` Arnd Bergmann
  2006-12-12 17:45 ` Geoff Levand
  2006-12-14  0:25 ` Benjamin Herrenschmidt
  2 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2006-12-12 13:01 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: paulus

On Tuesday 12 December 2006 04:33, Ishizaki Kou wrote:
> This patch creates Celleb platform dependent files to support interrupts.
> 
> Signed-off-by: Kou Ishizaki <kou.ishizaki.co.jp>

Acked-by: Arnd Bergmann <arnd.bergmann@de.ibm.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 7/15] celleb: supporting interrupts
  2006-12-12  3:33 [PATCH 7/15] celleb: supporting interrupts Ishizaki Kou
  2006-12-12 13:01 ` Arnd Bergmann
@ 2006-12-12 17:45 ` Geoff Levand
  2006-12-14  1:33   ` Ishizaki Kou
  2006-12-14  0:25 ` Benjamin Herrenschmidt
  2 siblings, 1 reply; 5+ messages in thread
From: Geoff Levand @ 2006-12-12 17:45 UTC (permalink / raw)
  To: Ishizaki Kou; +Cc: linuxppc-dev, paulus

Ishizaki Kou wrote:
> This patch creates Celleb platform dependent files to support interrupts.
> 
> Signed-off-by: Kou Ishizaki <kou.ishizaki.co.jp>
> ---
> 
> Index: linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.c
> diff -u /dev/null linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.c:1.1
> --- /dev/null	Mon Dec 11 20:37:34 2006
> +++ linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.c	Wed Dec  6 08:43:15 2006
> @@ -0,0 +1,256 @@
> +/*
> + * Celleb/Beat Interrupt controller
> + *
> + * (C) Copyright 2006 TOSHIBA CORPORATION
> + *
> + * 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.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/percpu.h>
> +#include <linux/types.h>
> +
> +#include <asm/io.h>
> +#include <asm/pgtable.h>
> +#include <asm/prom.h>
> +#include <asm/ptrace.h>
> +#include <asm/udbg.h>
> +#include <asm/machdep.h>
> +
> +#include "interrupt.h"
> +#include "beat.h"
> +
> +#define	MAX_IRQS	NR_IRQS
> +static spinlock_t beatic_irq_mask_lock;
> +static uint64_t	beatic_irq_mask[(MAX_IRQS+255)/64];
> +
> +static struct irq_host *beatic_host = NULL;
> +
> +/*
> + * In this implementation, "virq" == "IRQ plug number",
> + * "(irq_hw_number_t)hwirq" == "IRQ outlet number".
> + */
> +
> +static void beatic_unmask_irq(unsigned int irq_plug)
> +{
> +	int off;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&beatic_irq_mask_lock, flags);
> +	off = (irq_plug / 256) * 4;
> +	beatic_irq_mask[irq_plug/64] |= 1UL << (63 - (irq_plug%64));
> +
> +	if (beat_set_interrupt_mask(irq_plug&~255UL,
> +		beatic_irq_mask[off + 0], beatic_irq_mask[off + 1],
> +		beatic_irq_mask[off + 2], beatic_irq_mask[off + 3]) != 0)
> +		panic("Failed to set mask IRQ!");
> +	spin_unlock_irqrestore(&beatic_irq_mask_lock, flags);
> +}
> +
> +static void beatic_mask_irq(unsigned int irq_plug)
> +{
> +	int off;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&beatic_irq_mask_lock, flags);
> +	off = (irq_plug / 256) * 4;
> +	beatic_irq_mask[irq_plug/64] &= ~(1UL << (63 - (irq_plug%64)));
> +
> +	if (beat_set_interrupt_mask(irq_plug&~255UL,
> +		beatic_irq_mask[off + 0], beatic_irq_mask[off + 1],
> +		beatic_irq_mask[off + 2], beatic_irq_mask[off + 3]) != 0)
> +		panic("Failed to clear mask IRQ!");
> +
> +	spin_unlock_irqrestore(&beatic_irq_mask_lock, flags);
> +}
> +
> +static void beatic_end_irq(unsigned int irq_plug)
> +{
> +	int64_t err;
> +
> +	if ((err = beat_downcount_of_interrupt(irq_plug)) != 0) {
> +		if ((err & 0xFFFFFFFF) != 0xFFFFFFF5) /* -11: wrong state */
> +			panic("Failed to downcount IRQ! Error = %16lx", err);
> +
> +		printk(KERN_ERR "IRQ over-downcounted, plug %d\n", irq_plug);
> +	}
> +	beatic_unmask_irq(irq_plug);
> +}
> +
> +static struct irq_chip beatic_pic = {
> +	.typename = " CELL-BEAT ",
> +	.name = " CELL-BEAT ",


I don't think name is used anymore.


> +	struct irq_desc *desc = get_irq_desc(virq);
> +	int64_t	err;

Here you use int64_t...

> +{
> +	u64 *intspec2 = (u64 *)intspec;


...and here you use u64.  Please us a consistant scheme.  Standard
kernel convention is to use s64, u64, etc.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 7/15] celleb: supporting interrupts
  2006-12-12  3:33 [PATCH 7/15] celleb: supporting interrupts Ishizaki Kou
  2006-12-12 13:01 ` Arnd Bergmann
  2006-12-12 17:45 ` Geoff Levand
@ 2006-12-14  0:25 ` Benjamin Herrenschmidt
  2 siblings, 0 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2006-12-14  0:25 UTC (permalink / raw)
  To: Ishizaki Kou; +Cc: linuxppc-dev, paulus

I think there's a problem with interrupt handling...

> +static void beatic_end_irq(unsigned int irq_plug)
> +{
> +	int64_t err;
> +
> +	if ((err = beat_downcount_of_interrupt(irq_plug)) != 0) {
> +		if ((err & 0xFFFFFFFF) != 0xFFFFFFF5) /* -11: wrong state */
> +			panic("Failed to downcount IRQ! Error = %16lx", err);
> +
> +		printk(KERN_ERR "IRQ over-downcounted, plug %d\n", irq_plug);
> +	}
> +	beatic_unmask_irq(irq_plug);
> +}

 .../...

> +unsigned int beatic_get_irq(void)
> +{
> +	unsigned int ret;
> +
> +	ret = beatic_get_irq_plug();
> +	if (ret != NO_IRQ)
> +		beatic_mask_irq(ret);
> +	return ret;
> +}
> +

So you are masking the irq upon reception and unmasking it in eoi()...
This is not quite correct and I was hoping this wouldn't be necessary,
but it seems we are hitting a similar issues with Sony lv1...

The problem with the above approach is that the unconditional unmask in
eoi() is incorrect as the interrupt can have been disabled by software
between get_irq() and eoi().

In general, that means that the fasteoi handler isn't the right choice
for you as it assumes "smart" interrupt controllers that will not
re-emit an interrupt that has been already emited until EOI is called.

I can understand that there is a design issue with the hypervisor here,
as you have no way to "ack" an irq (so that the HV knows you actuall got
it, since a given 0x500 can be called for more than one interrupt). It's
generally done with other HV as part of get_irq() by an H-call to obtain
the next pending irq number, but your bitmask mecanism makes that
impossible.

Thus you need something which is pretty much a "bastard" of fasteoi and
level handlers... It's annoying as no existing handler matches exactly
what you need here. You can either write your own handler, possibly
based on the level one, but adding an eoi call to it, or use the eoi
one, add an empty ack() callback, and have unmask do an eoi
unconditionally if that isn't a problem for you.

Ben.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 7/15] celleb: supporting interrupts
  2006-12-12 17:45 ` Geoff Levand
@ 2006-12-14  1:33   ` Ishizaki Kou
  0 siblings, 0 replies; 5+ messages in thread
From: Ishizaki Kou @ 2006-12-14  1:33 UTC (permalink / raw)
  To: geoffrey.levand; +Cc: linuxppc-dev, paulus


Geoff-san,

Thanks for your comment.

> Ishizaki Kou wrote:
> > This patch creates Celleb platform dependent files to support interrupts.
> > 
> > Signed-off-by: Kou Ishizaki <kou.ishizaki.co.jp>
> > ---
> > 
> > Index: linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.c
> > diff -u /dev/null linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.c:1.1
> > --- /dev/null     Mon Dec 11 20:37:34 2006
> > +++ linux-powerpc-git/arch/powerpc/platforms/celleb/interrupt.c	Wed Dec  6 08:43:15 2006
> > @@ -0,0 +1,256 @@
> > +/*
> > + * Celleb/Beat Interrupt controller
> > + *
> > + * (C) Copyright 2006 TOSHIBA CORPORATION
> > + *
> > + * 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.,
> > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/irq.h>
> > +#include <linux/module.h>
> > +#include <linux/percpu.h>
> > +#include <linux/types.h>
> > +
> > +#include <asm/io.h>
> > +#include <asm/pgtable.h>
> > +#include <asm/prom.h>
> > +#include <asm/ptrace.h>
> > +#include <asm/udbg.h>
> > +#include <asm/machdep.h>
> > +
> > +#include "interrupt.h"
> > +#include "beat.h"
> > +
> > +#define	MAX_IRQS	NR_IRQS
> > +static spinlock_t beatic_irq_mask_lock;
> > +static uint64_t   beatic_irq_mask[(MAX_IRQS+255)/64];
> > +
> > +static struct irq_host *beatic_host = NULL;
> > +
> > +/*
> > + * In this implementation, "virq" == "IRQ plug number",
> > + * "(irq_hw_number_t)hwirq" == "IRQ outlet number".
> > + */
> > +
> > +static void beatic_unmask_irq(unsigned int irq_plug)
> > +{
> > +	int off;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&beatic_irq_mask_lock, flags);
> > +	off = (irq_plug / 256) * 4;
> > +	beatic_irq_mask[irq_plug/64] |= 1UL << (63 - (irq_plug%64));
> > +
> > +	if (beat_set_interrupt_mask(irq_plug&~255UL,
> > +	   beatic_irq_mask[off + 0], beatic_irq_mask[off + 1],
> > +			       beatic_irq_mask[off + 2], beatic_irq_mask[off + 3]) != 0)
> > +						   panic("Failed to set mask IRQ!");
> > +						   spin_unlock_irqrestore(&beatic_irq_mask_lock, flags);
> > +}
> > +
> > +static void beatic_mask_irq(unsigned int irq_plug)
> > +{
> > +	int off;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&beatic_irq_mask_lock, flags);
> > +	off = (irq_plug / 256) * 4;
> > +	beatic_irq_mask[irq_plug/64] &= ~(1UL << (63 - (irq_plug%64)));
> > +
> > +	if (beat_set_interrupt_mask(irq_plug&~255UL,
> > +	   beatic_irq_mask[off + 0], beatic_irq_mask[off + 1],
> > +			       beatic_irq_mask[off + 2], beatic_irq_mask[off + 3]) != 0)
> > +						   panic("Failed to clear mask IRQ!");
> > +
> > +	spin_unlock_irqrestore(&beatic_irq_mask_lock, flags);
> > +}
> > +
> > +static void beatic_end_irq(unsigned int irq_plug)
> > +{
> > +	int64_t err;
> > +
> > +	if ((err = beat_downcount_of_interrupt(irq_plug)) != 0) {
> > +	   if ((err & 0xFFFFFFFF) != 0xFFFFFFF5) /* -11: wrong state */
> > +		    panic("Failed to downcount IRQ! Error = %16lx", err);
> > +
> > +		printk(KERN_ERR "IRQ over-downcounted, plug %d\n", irq_plug);
> > +		}
> > +		beatic_unmask_irq(irq_plug);
> > +}
> > +
> > +static struct irq_chip beatic_pic = {
> > +	    .typename = " CELL-BEAT ",
> > +	    .name = " CELL-BEAT ",
> 
> 
> I don't think name is used anymore.

So it should be removed. I see.

> > + struct irq_desc *desc = get_irq_desc(virq);
> > + int64_t	      err;

> Here you use int64_t...

> > +{
> > +	u64 *intspec2 = (u64 *)intspec;

> ...and here you use u64.  Please us a consistant scheme.  Standard
> kernel convention is to use s64, u64, etc.

Ugh. "int64_t" comes from our calling convention on hypervisor call.
I will follow standard kernel convention.

With regards,
Kou Ishizaki

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2006-12-14  1:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-12-12  3:33 [PATCH 7/15] celleb: supporting interrupts Ishizaki Kou
2006-12-12 13:01 ` Arnd Bergmann
2006-12-12 17:45 ` Geoff Levand
2006-12-14  1:33   ` Ishizaki Kou
2006-12-14  0:25 ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).