linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] openrisc: irq: use irqchip framework
@ 2014-05-26 20:31 Stefan Kristiansson
  2014-05-26 20:52 ` Geert Uytterhoeven
  2014-06-21 20:02 ` Jason Cooper
  0 siblings, 2 replies; 9+ messages in thread
From: Stefan Kristiansson @ 2014-05-26 20:31 UTC (permalink / raw)
  To: linux-kernel, linux; +Cc: jonas, tglx, jason, geert, Stefan Kristiansson

In addition to consolidating the or1k-pic with other interrupt
controllers, this makes OpenRISC less tied to its on-cpu
interrupt controller.

All or1k-pic specific parts are moved out of irq.c and into
drivers/irqchip/irq-or1k-pic.c

In that transition, the funtionality have been divided into
three chip variants.
One that handles level triggered interrupts, one that handles edge
triggered interrupts and one that handles the interrupt
controller that is present in the or1200 OpenRISC cpu
implementation.

Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
---
Changes in v2:
 - Move or1k-pic related code into irq-or1k-pic
 - Add documentation for device tree bindings

Changes in v3:
 - Split level-, edge-triggered and or1200 implementation into seperate
   chip variants.

Changes in v4:
 - Fix typos in documentation
---
 .../interrupt-controller/opencores,or1k-pic.txt    |  23 +++
 arch/openrisc/Kconfig                              |   1 +
 arch/openrisc/include/asm/irq.h                    |   3 +
 arch/openrisc/kernel/irq.c                         | 146 ++---------------
 drivers/irqchip/Kconfig                            |   4 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-or1k-pic.c                     | 182 +++++++++++++++++++++
 7 files changed, 227 insertions(+), 133 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
 create mode 100644 drivers/irqchip/irq-or1k-pic.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt b/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
new file mode 100644
index 0000000..55c04fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
@@ -0,0 +1,23 @@
+OpenRISC 1000 Programmable Interrupt Controller
+
+Required properties:
+
+- compatible : should be "opencores,or1k-pic-level" for variants with
+  level triggered interrupt lines, "opencores,or1k-pic-edge" for variants with
+  edge triggered interrupt lines or "opencores,or1200-pic" for machines
+  with the non-spec compliant or1200 type implementation.
+
+  "opencores,or1k-pic" is also provided as an alias to "opencores,or1200-pic",
+  but this is only for backwards compatibility.
+
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+  interrupt source. The value shall be 1.
+
+Example:
+
+intc: interrupt-controller {
+	compatible = "opencores,or1k-pic-level";
+	interrupt-controller;
+	#interrupt-cells = <1>;
+};
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index e71d712..88e8336 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -22,6 +22,7 @@ config OPENRISC
 	select GENERIC_STRNLEN_USER
 	select MODULES_USE_ELF_RELA
 	select HAVE_DEBUG_STACKOVERFLOW
+	select OR1K_PIC
 
 config MMU
 	def_bool y
diff --git a/arch/openrisc/include/asm/irq.h b/arch/openrisc/include/asm/irq.h
index eb612b1..b84634c 100644
--- a/arch/openrisc/include/asm/irq.h
+++ b/arch/openrisc/include/asm/irq.h
@@ -24,4 +24,7 @@
 
 #define NO_IRQ		(-1)
 
+void handle_IRQ(unsigned int, struct pt_regs *);
+extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
+
 #endif /* __ASM_OPENRISC_IRQ_H__ */
diff --git a/arch/openrisc/kernel/irq.c b/arch/openrisc/kernel/irq.c
index 8ec77bc..967eb14 100644
--- a/arch/openrisc/kernel/irq.c
+++ b/arch/openrisc/kernel/irq.c
@@ -16,11 +16,10 @@
 
 #include <linux/interrupt.h>
 #include <linux/init.h>
-#include <linux/of.h>
 #include <linux/ftrace.h>
 #include <linux/irq.h>
+#include <linux/irqchip.h>
 #include <linux/export.h>
-#include <linux/irqdomain.h>
 #include <linux/irqflags.h>
 
 /* read interrupt enabled status */
@@ -37,150 +36,31 @@ void arch_local_irq_restore(unsigned long flags)
 }
 EXPORT_SYMBOL(arch_local_irq_restore);
 
-
-/* OR1K PIC implementation */
-
-/* We're a couple of cycles faster than the generic implementations with
- * these 'fast' versions.
- */
-
-static void or1k_pic_mask(struct irq_data *data)
-{
-	mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
-}
-
-static void or1k_pic_unmask(struct irq_data *data)
-{
-	mtspr(SPR_PICMR, mfspr(SPR_PICMR) | (1UL << data->hwirq));
-}
-
-static void or1k_pic_ack(struct irq_data *data)
-{
-	/* EDGE-triggered interrupts need to be ack'ed in order to clear
-	 * the latch.
-	 * LEVEL-triggered interrupts do not need to be ack'ed; however,
-	 * ack'ing the interrupt has no ill-effect and is quicker than
-	 * trying to figure out what type it is...
-	 */
-
-	/* The OpenRISC 1000 spec says to write a 1 to the bit to ack the
-	 * interrupt, but the OR1200 does this backwards and requires a 0
-	 * to be written...
-	 */
-
-#ifdef CONFIG_OR1K_1200
-	/* There are two oddities with the OR1200 PIC implementation:
-	 * i)  LEVEL-triggered interrupts are latched and need to be cleared
-	 * ii) the interrupt latch is cleared by writing a 0 to the bit,
-	 *     as opposed to a 1 as mandated by the spec
-	 */
-
-	mtspr(SPR_PICSR, mfspr(SPR_PICSR) & ~(1UL << data->hwirq));
-#else
-	WARN(1, "Interrupt handling possibly broken\n");
-	mtspr(SPR_PICSR, (1UL << data->hwirq));
-#endif
-}
-
-static void or1k_pic_mask_ack(struct irq_data *data)
-{
-	/* Comments for pic_ack apply here, too */
-
-#ifdef CONFIG_OR1K_1200
-	mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
-	mtspr(SPR_PICSR, mfspr(SPR_PICSR) & ~(1UL << data->hwirq));
-#else
-	WARN(1, "Interrupt handling possibly broken\n");
-	mtspr(SPR_PICMR, (1UL << data->hwirq));
-	mtspr(SPR_PICSR, (1UL << data->hwirq));
-#endif
-}
-
-#if 0
-static int or1k_pic_set_type(struct irq_data *data, unsigned int flow_type)
-{
-	/* There's nothing to do in the PIC configuration when changing
-	 * flow type.  Level and edge-triggered interrupts are both
-	 * supported, but it's PIC-implementation specific which type
-	 * is handled. */
-
-	return irq_setup_alt_chip(data, flow_type);
-}
-#endif
-
-static struct irq_chip or1k_dev = {
-	.name = "or1k-PIC",
-	.irq_unmask = or1k_pic_unmask,
-	.irq_mask = or1k_pic_mask,
-	.irq_ack = or1k_pic_ack,
-	.irq_mask_ack = or1k_pic_mask_ack,
-};
-
-static struct irq_domain *root_domain;
-
-static inline int pic_get_irq(int first)
-{
-	int hwirq;
-
-	hwirq = ffs(mfspr(SPR_PICSR) >> first);
-	if (!hwirq)
-		return NO_IRQ;
-	else
-		hwirq = hwirq + first -1;
-
-	return irq_find_mapping(root_domain, hwirq);
-}
-
-
-static int or1k_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
+void __init init_IRQ(void)
 {
-	irq_set_chip_and_handler_name(irq, &or1k_dev,
-				      handle_level_irq, "level");
-	irq_set_status_flags(irq, IRQ_LEVEL | IRQ_NOPROBE);
-
-	return 0;
+	irqchip_init();
 }
 
-static const struct irq_domain_ops or1k_irq_domain_ops = {
-	.xlate = irq_domain_xlate_onecell,
-	.map = or1k_map,
-};
-
-/*
- * This sets up the IRQ domain for the PIC built in to the OpenRISC
- * 1000 CPU.  This is the "root" domain as these are the interrupts
- * that directly trigger an exception in the CPU.
- */
-static void __init or1k_irq_init(void)
-{
-	struct device_node *intc = NULL;
-
-	/* The interrupt controller device node is mandatory */
-	intc = of_find_compatible_node(NULL, NULL, "opencores,or1k-pic");
-	BUG_ON(!intc);
-
-	/* Disable all interrupts until explicitly requested */
-	mtspr(SPR_PICMR, (0UL));
-
-	root_domain = irq_domain_add_linear(intc, 32,
-					    &or1k_irq_domain_ops, NULL);
-}
+static void (*handle_arch_irq)(struct pt_regs *);
 
-void __init init_IRQ(void)
+void __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
 {
-	or1k_irq_init();
+	handle_arch_irq = handle_irq;
 }
 
-void __irq_entry do_IRQ(struct pt_regs *regs)
+void handle_IRQ(unsigned int irq, struct pt_regs *regs)
 {
-	int irq = -1;
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
 	irq_enter();
 
-	while ((irq = pic_get_irq(irq + 1)) != NO_IRQ)
-		generic_handle_irq(irq);
+	generic_handle_irq(irq);
 
 	irq_exit();
 	set_irq_regs(old_regs);
 }
+
+void __irq_entry do_IRQ(struct pt_regs *regs)
+{
+	handle_arch_irq(regs);
+}
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index d770f74..f06e4c8 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -47,6 +47,10 @@ config CLPS711X_IRQCHIP
 	select SPARSE_IRQ
 	default y
 
+config OR1K_PIC
+	bool
+	select IRQ_DOMAIN
+
 config ORION_IRQCHIP
 	bool
 	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index f180f8d..4377695 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_METAG)			+= irq-metag-ext.o
 obj-$(CONFIG_METAG_PERFCOUNTER_IRQS)	+= irq-metag.o
 obj-$(CONFIG_ARCH_MOXART)		+= irq-moxart.o
 obj-$(CONFIG_CLPS711X_IRQCHIP)		+= irq-clps711x.o
+obj-$(CONFIG_OR1K_PIC)			+= irq-or1k-pic.o
 obj-$(CONFIG_ORION_IRQCHIP)		+= irq-orion.o
 obj-$(CONFIG_ARCH_SUNXI)		+= irq-sun4i.o
 obj-$(CONFIG_ARCH_SUNXI)		+= irq-sunxi-nmi.o
diff --git a/drivers/irqchip/irq-or1k-pic.c b/drivers/irqchip/irq-or1k-pic.c
new file mode 100644
index 0000000..17ff033
--- /dev/null
+++ b/drivers/irqchip/irq-or1k-pic.c
@@ -0,0 +1,182 @@
+/*
+ * Copyright (C) 2010-2011 Jonas Bonn <jonas@southpole.se>
+ * Copyright (C) 2014 Stefan Kristansson <stefan.kristiansson@saunalahti.fi>
+ *
+ * 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.
+ */
+
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+
+#include "irqchip.h"
+
+/* OR1K PIC implementation */
+
+struct or1k_pic_dev {
+	struct irq_chip chip;
+	irq_flow_handler_t handle;
+	unsigned long flags;
+};
+
+/*
+ * We're a couple of cycles faster than the generic implementations with
+ * these 'fast' versions.
+ */
+
+static void or1k_pic_mask(struct irq_data *data)
+{
+	mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
+}
+
+static void or1k_pic_unmask(struct irq_data *data)
+{
+	mtspr(SPR_PICMR, mfspr(SPR_PICMR) | (1UL << data->hwirq));
+}
+
+static void or1k_pic_ack(struct irq_data *data)
+{
+	mtspr(SPR_PICSR, (1UL << data->hwirq));
+}
+
+static void or1k_pic_mask_ack(struct irq_data *data)
+{
+	mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
+	mtspr(SPR_PICSR, (1UL << data->hwirq));
+}
+
+/*
+ * There are two oddities with the OR1200 PIC implementation:
+ * i)  LEVEL-triggered interrupts are latched and need to be cleared
+ * ii) the interrupt latch is cleared by writing a 0 to the bit,
+ *     as opposed to a 1 as mandated by the spec
+ */
+static void or1k_pic_or1200_ack(struct irq_data *data)
+{
+	mtspr(SPR_PICSR, mfspr(SPR_PICSR) & ~(1UL << data->hwirq));
+}
+
+static void or1k_pic_or1200_mask_ack(struct irq_data *data)
+{
+	mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
+	mtspr(SPR_PICSR, mfspr(SPR_PICSR) & ~(1UL << data->hwirq));
+}
+
+static struct or1k_pic_dev or1k_pic_level = {
+	.chip = {
+		.name = "or1k-PIC-level",
+		.irq_unmask = or1k_pic_unmask,
+		.irq_mask = or1k_pic_mask,
+		.irq_mask_ack = or1k_pic_mask,
+	},
+	.handle = handle_level_irq,
+	.flags = IRQ_LEVEL | IRQ_NOPROBE,
+};
+
+static struct or1k_pic_dev or1k_pic_edge = {
+	.chip = {
+		.name = "or1k-PIC-edge",
+		.irq_unmask = or1k_pic_unmask,
+		.irq_mask = or1k_pic_mask,
+		.irq_ack = or1k_pic_ack,
+		.irq_mask_ack = or1k_pic_mask_ack,
+	},
+	.handle = handle_edge_irq,
+	.flags = IRQ_LEVEL | IRQ_NOPROBE,
+};
+
+static struct or1k_pic_dev or1k_pic_or1200 = {
+	.chip = {
+		.name = "or1200-PIC",
+		.irq_unmask = or1k_pic_unmask,
+		.irq_mask = or1k_pic_mask,
+		.irq_ack = or1k_pic_or1200_ack,
+		.irq_mask_ack = or1k_pic_or1200_mask_ack,
+	},
+	.handle = handle_level_irq,
+	.flags = IRQ_LEVEL | IRQ_NOPROBE,
+};
+
+static struct irq_domain *root_domain;
+
+static inline int pic_get_irq(int first)
+{
+	int hwirq;
+
+	hwirq = ffs(mfspr(SPR_PICSR) >> first);
+	if (!hwirq)
+		return NO_IRQ;
+	else
+		hwirq = hwirq + first - 1;
+
+	return irq_find_mapping(root_domain, hwirq);
+}
+
+static void or1k_pic_handle_irq(struct pt_regs *regs)
+{
+	int irq = -1;
+
+	while ((irq = pic_get_irq(irq + 1)) != NO_IRQ)
+		handle_IRQ(irq, regs);
+}
+
+static int or1k_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
+{
+	struct or1k_pic_dev *pic = d->host_data;
+
+	irq_set_chip_and_handler(irq, &pic->chip, pic->handle);
+	irq_set_status_flags(irq, pic->flags);
+
+	return 0;
+}
+
+static const struct irq_domain_ops or1k_irq_domain_ops = {
+	.xlate = irq_domain_xlate_onecell,
+	.map = or1k_map,
+};
+
+/*
+ * This sets up the IRQ domain for the PIC built in to the OpenRISC
+ * 1000 CPU.  This is the "root" domain as these are the interrupts
+ * that directly trigger an exception in the CPU.
+ */
+static int __init or1k_pic_init(struct device_node *node,
+				 struct or1k_pic_dev *pic)
+{
+	/* Disable all interrupts until explicitly requested */
+	mtspr(SPR_PICMR, (0UL));
+
+	root_domain = irq_domain_add_linear(node, 32, &or1k_irq_domain_ops,
+					    pic);
+
+	set_handle_irq(or1k_pic_handle_irq);
+
+	return 0;
+}
+
+static int __init or1k_pic_or1200_init(struct device_node *node,
+				       struct device_node *parent)
+{
+	return or1k_pic_init(node, &or1k_pic_or1200);
+}
+IRQCHIP_DECLARE(or1k_pic_or1200, "opencores,or1200-pic", or1k_pic_or1200_init);
+IRQCHIP_DECLARE(or1k_pic, "opencores,or1k-pic", or1k_pic_or1200_init);
+
+static int __init or1k_pic_level_init(struct device_node *node,
+				      struct device_node *parent)
+{
+	return or1k_pic_init(node, &or1k_pic_level);
+}
+IRQCHIP_DECLARE(or1k_pic_level, "opencores,or1k-pic-level",
+		or1k_pic_level_init);
+
+static int __init or1k_pic_edge_init(struct device_node *node,
+				     struct device_node *parent)
+{
+	return or1k_pic_init(node, &or1k_pic_edge);
+}
+IRQCHIP_DECLARE(or1k_pic_edge, "opencores,or1k-pic-edge", or1k_pic_edge_init);
-- 
1.8.3.2


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

* Re: [PATCH v4] openrisc: irq: use irqchip framework
  2014-05-26 20:31 [PATCH v4] openrisc: irq: use irqchip framework Stefan Kristiansson
@ 2014-05-26 20:52 ` Geert Uytterhoeven
  2014-05-27  6:47   ` Jonas Bonn
  2014-06-21 20:02 ` Jason Cooper
  1 sibling, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-05-26 20:52 UTC (permalink / raw)
  To: Stefan Kristiansson
  Cc: linux-kernel, linux, Jonas Bonn, Thomas Gleixner, Jason Cooper,
	devicetree

CC devicetree for the bindings

On Mon, May 26, 2014 at 10:31 PM, Stefan Kristiansson
<stefan.kristiansson@saunalahti.fi> wrote:
> In addition to consolidating the or1k-pic with other interrupt
> controllers, this makes OpenRISC less tied to its on-cpu
> interrupt controller.
>
> All or1k-pic specific parts are moved out of irq.c and into
> drivers/irqchip/irq-or1k-pic.c
>
> In that transition, the funtionality have been divided into
> three chip variants.
> One that handles level triggered interrupts, one that handles edge
> triggered interrupts and one that handles the interrupt
> controller that is present in the or1200 OpenRISC cpu
> implementation.
>
> Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> ---
> Changes in v2:
>  - Move or1k-pic related code into irq-or1k-pic
>  - Add documentation for device tree bindings
>
> Changes in v3:
>  - Split level-, edge-triggered and or1200 implementation into seperate
>    chip variants.
>
> Changes in v4:
>  - Fix typos in documentation
> ---
>  .../interrupt-controller/opencores,or1k-pic.txt    |  23 +++
>  arch/openrisc/Kconfig                              |   1 +
>  arch/openrisc/include/asm/irq.h                    |   3 +
>  arch/openrisc/kernel/irq.c                         | 146 ++---------------
>  drivers/irqchip/Kconfig                            |   4 +
>  drivers/irqchip/Makefile                           |   1 +
>  drivers/irqchip/irq-or1k-pic.c                     | 182 +++++++++++++++++++++
>  7 files changed, 227 insertions(+), 133 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
>  create mode 100644 drivers/irqchip/irq-or1k-pic.c
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt b/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
> new file mode 100644
> index 0000000..55c04fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
> @@ -0,0 +1,23 @@
> +OpenRISC 1000 Programmable Interrupt Controller
> +
> +Required properties:
> +
> +- compatible : should be "opencores,or1k-pic-level" for variants with
> +  level triggered interrupt lines, "opencores,or1k-pic-edge" for variants with
> +  edge triggered interrupt lines or "opencores,or1200-pic" for machines
> +  with the non-spec compliant or1200 type implementation.
> +
> +  "opencores,or1k-pic" is also provided as an alias to "opencores,or1200-pic",
> +  but this is only for backwards compatibility.
> +
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +  interrupt source. The value shall be 1.
> +
> +Example:
> +
> +intc: interrupt-controller {
> +       compatible = "opencores,or1k-pic-level";
> +       interrupt-controller;
> +       #interrupt-cells = <1>;
> +};
> diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> index e71d712..88e8336 100644
> --- a/arch/openrisc/Kconfig
> +++ b/arch/openrisc/Kconfig
> @@ -22,6 +22,7 @@ config OPENRISC
>         select GENERIC_STRNLEN_USER
>         select MODULES_USE_ELF_RELA
>         select HAVE_DEBUG_STACKOVERFLOW
> +       select OR1K_PIC
>
>  config MMU
>         def_bool y
> diff --git a/arch/openrisc/include/asm/irq.h b/arch/openrisc/include/asm/irq.h
> index eb612b1..b84634c 100644
> --- a/arch/openrisc/include/asm/irq.h
> +++ b/arch/openrisc/include/asm/irq.h
> @@ -24,4 +24,7 @@
>
>  #define NO_IRQ         (-1)
>
> +void handle_IRQ(unsigned int, struct pt_regs *);
> +extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
> +
>  #endif /* __ASM_OPENRISC_IRQ_H__ */
> diff --git a/arch/openrisc/kernel/irq.c b/arch/openrisc/kernel/irq.c
> index 8ec77bc..967eb14 100644
> --- a/arch/openrisc/kernel/irq.c
> +++ b/arch/openrisc/kernel/irq.c
> @@ -16,11 +16,10 @@
>
>  #include <linux/interrupt.h>
>  #include <linux/init.h>
> -#include <linux/of.h>
>  #include <linux/ftrace.h>
>  #include <linux/irq.h>
> +#include <linux/irqchip.h>
>  #include <linux/export.h>
> -#include <linux/irqdomain.h>
>  #include <linux/irqflags.h>
>
>  /* read interrupt enabled status */
> @@ -37,150 +36,31 @@ void arch_local_irq_restore(unsigned long flags)
>  }
>  EXPORT_SYMBOL(arch_local_irq_restore);
>
> -
> -/* OR1K PIC implementation */
> -
> -/* We're a couple of cycles faster than the generic implementations with
> - * these 'fast' versions.
> - */
> -
> -static void or1k_pic_mask(struct irq_data *data)
> -{
> -       mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
> -}
> -
> -static void or1k_pic_unmask(struct irq_data *data)
> -{
> -       mtspr(SPR_PICMR, mfspr(SPR_PICMR) | (1UL << data->hwirq));
> -}
> -
> -static void or1k_pic_ack(struct irq_data *data)
> -{
> -       /* EDGE-triggered interrupts need to be ack'ed in order to clear
> -        * the latch.
> -        * LEVEL-triggered interrupts do not need to be ack'ed; however,
> -        * ack'ing the interrupt has no ill-effect and is quicker than
> -        * trying to figure out what type it is...
> -        */
> -
> -       /* The OpenRISC 1000 spec says to write a 1 to the bit to ack the
> -        * interrupt, but the OR1200 does this backwards and requires a 0
> -        * to be written...
> -        */
> -
> -#ifdef CONFIG_OR1K_1200
> -       /* There are two oddities with the OR1200 PIC implementation:
> -        * i)  LEVEL-triggered interrupts are latched and need to be cleared
> -        * ii) the interrupt latch is cleared by writing a 0 to the bit,
> -        *     as opposed to a 1 as mandated by the spec
> -        */
> -
> -       mtspr(SPR_PICSR, mfspr(SPR_PICSR) & ~(1UL << data->hwirq));
> -#else
> -       WARN(1, "Interrupt handling possibly broken\n");
> -       mtspr(SPR_PICSR, (1UL << data->hwirq));
> -#endif
> -}
> -
> -static void or1k_pic_mask_ack(struct irq_data *data)
> -{
> -       /* Comments for pic_ack apply here, too */
> -
> -#ifdef CONFIG_OR1K_1200
> -       mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
> -       mtspr(SPR_PICSR, mfspr(SPR_PICSR) & ~(1UL << data->hwirq));
> -#else
> -       WARN(1, "Interrupt handling possibly broken\n");
> -       mtspr(SPR_PICMR, (1UL << data->hwirq));
> -       mtspr(SPR_PICSR, (1UL << data->hwirq));
> -#endif
> -}
> -
> -#if 0
> -static int or1k_pic_set_type(struct irq_data *data, unsigned int flow_type)
> -{
> -       /* There's nothing to do in the PIC configuration when changing
> -        * flow type.  Level and edge-triggered interrupts are both
> -        * supported, but it's PIC-implementation specific which type
> -        * is handled. */
> -
> -       return irq_setup_alt_chip(data, flow_type);
> -}
> -#endif
> -
> -static struct irq_chip or1k_dev = {
> -       .name = "or1k-PIC",
> -       .irq_unmask = or1k_pic_unmask,
> -       .irq_mask = or1k_pic_mask,
> -       .irq_ack = or1k_pic_ack,
> -       .irq_mask_ack = or1k_pic_mask_ack,
> -};
> -
> -static struct irq_domain *root_domain;
> -
> -static inline int pic_get_irq(int first)
> -{
> -       int hwirq;
> -
> -       hwirq = ffs(mfspr(SPR_PICSR) >> first);
> -       if (!hwirq)
> -               return NO_IRQ;
> -       else
> -               hwirq = hwirq + first -1;
> -
> -       return irq_find_mapping(root_domain, hwirq);
> -}
> -
> -
> -static int or1k_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
> +void __init init_IRQ(void)
>  {
> -       irq_set_chip_and_handler_name(irq, &or1k_dev,
> -                                     handle_level_irq, "level");
> -       irq_set_status_flags(irq, IRQ_LEVEL | IRQ_NOPROBE);
> -
> -       return 0;
> +       irqchip_init();
>  }
>
> -static const struct irq_domain_ops or1k_irq_domain_ops = {
> -       .xlate = irq_domain_xlate_onecell,
> -       .map = or1k_map,
> -};
> -
> -/*
> - * This sets up the IRQ domain for the PIC built in to the OpenRISC
> - * 1000 CPU.  This is the "root" domain as these are the interrupts
> - * that directly trigger an exception in the CPU.
> - */
> -static void __init or1k_irq_init(void)
> -{
> -       struct device_node *intc = NULL;
> -
> -       /* The interrupt controller device node is mandatory */
> -       intc = of_find_compatible_node(NULL, NULL, "opencores,or1k-pic");
> -       BUG_ON(!intc);
> -
> -       /* Disable all interrupts until explicitly requested */
> -       mtspr(SPR_PICMR, (0UL));
> -
> -       root_domain = irq_domain_add_linear(intc, 32,
> -                                           &or1k_irq_domain_ops, NULL);
> -}
> +static void (*handle_arch_irq)(struct pt_regs *);
>
> -void __init init_IRQ(void)
> +void __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
>  {
> -       or1k_irq_init();
> +       handle_arch_irq = handle_irq;
>  }
>
> -void __irq_entry do_IRQ(struct pt_regs *regs)
> +void handle_IRQ(unsigned int irq, struct pt_regs *regs)
>  {
> -       int irq = -1;
>         struct pt_regs *old_regs = set_irq_regs(regs);
>
>         irq_enter();
>
> -       while ((irq = pic_get_irq(irq + 1)) != NO_IRQ)
> -               generic_handle_irq(irq);
> +       generic_handle_irq(irq);
>
>         irq_exit();
>         set_irq_regs(old_regs);
>  }
> +
> +void __irq_entry do_IRQ(struct pt_regs *regs)
> +{
> +       handle_arch_irq(regs);
> +}
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index d770f74..f06e4c8 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -47,6 +47,10 @@ config CLPS711X_IRQCHIP
>         select SPARSE_IRQ
>         default y
>
> +config OR1K_PIC
> +       bool
> +       select IRQ_DOMAIN
> +
>  config ORION_IRQCHIP
>         bool
>         select IRQ_DOMAIN
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index f180f8d..4377695 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_METAG)                   += irq-metag-ext.o
>  obj-$(CONFIG_METAG_PERFCOUNTER_IRQS)   += irq-metag.o
>  obj-$(CONFIG_ARCH_MOXART)              += irq-moxart.o
>  obj-$(CONFIG_CLPS711X_IRQCHIP)         += irq-clps711x.o
> +obj-$(CONFIG_OR1K_PIC)                 += irq-or1k-pic.o
>  obj-$(CONFIG_ORION_IRQCHIP)            += irq-orion.o
>  obj-$(CONFIG_ARCH_SUNXI)               += irq-sun4i.o
>  obj-$(CONFIG_ARCH_SUNXI)               += irq-sunxi-nmi.o
> diff --git a/drivers/irqchip/irq-or1k-pic.c b/drivers/irqchip/irq-or1k-pic.c
> new file mode 100644
> index 0000000..17ff033
> --- /dev/null
> +++ b/drivers/irqchip/irq-or1k-pic.c
> @@ -0,0 +1,182 @@
> +/*
> + * Copyright (C) 2010-2011 Jonas Bonn <jonas@southpole.se>
> + * Copyright (C) 2014 Stefan Kristansson <stefan.kristiansson@saunalahti.fi>
> + *
> + * 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.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +
> +#include "irqchip.h"
> +
> +/* OR1K PIC implementation */
> +
> +struct or1k_pic_dev {
> +       struct irq_chip chip;
> +       irq_flow_handler_t handle;
> +       unsigned long flags;
> +};
> +
> +/*
> + * We're a couple of cycles faster than the generic implementations with
> + * these 'fast' versions.
> + */
> +
> +static void or1k_pic_mask(struct irq_data *data)
> +{
> +       mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
> +}
> +
> +static void or1k_pic_unmask(struct irq_data *data)
> +{
> +       mtspr(SPR_PICMR, mfspr(SPR_PICMR) | (1UL << data->hwirq));
> +}
> +
> +static void or1k_pic_ack(struct irq_data *data)
> +{
> +       mtspr(SPR_PICSR, (1UL << data->hwirq));
> +}
> +
> +static void or1k_pic_mask_ack(struct irq_data *data)
> +{
> +       mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
> +       mtspr(SPR_PICSR, (1UL << data->hwirq));
> +}
> +
> +/*
> + * There are two oddities with the OR1200 PIC implementation:
> + * i)  LEVEL-triggered interrupts are latched and need to be cleared
> + * ii) the interrupt latch is cleared by writing a 0 to the bit,
> + *     as opposed to a 1 as mandated by the spec
> + */
> +static void or1k_pic_or1200_ack(struct irq_data *data)
> +{
> +       mtspr(SPR_PICSR, mfspr(SPR_PICSR) & ~(1UL << data->hwirq));
> +}
> +
> +static void or1k_pic_or1200_mask_ack(struct irq_data *data)
> +{
> +       mtspr(SPR_PICMR, mfspr(SPR_PICMR) & ~(1UL << data->hwirq));
> +       mtspr(SPR_PICSR, mfspr(SPR_PICSR) & ~(1UL << data->hwirq));
> +}
> +
> +static struct or1k_pic_dev or1k_pic_level = {
> +       .chip = {
> +               .name = "or1k-PIC-level",
> +               .irq_unmask = or1k_pic_unmask,
> +               .irq_mask = or1k_pic_mask,
> +               .irq_mask_ack = or1k_pic_mask,
> +       },
> +       .handle = handle_level_irq,
> +       .flags = IRQ_LEVEL | IRQ_NOPROBE,
> +};
> +
> +static struct or1k_pic_dev or1k_pic_edge = {
> +       .chip = {
> +               .name = "or1k-PIC-edge",
> +               .irq_unmask = or1k_pic_unmask,
> +               .irq_mask = or1k_pic_mask,
> +               .irq_ack = or1k_pic_ack,
> +               .irq_mask_ack = or1k_pic_mask_ack,
> +       },
> +       .handle = handle_edge_irq,
> +       .flags = IRQ_LEVEL | IRQ_NOPROBE,
> +};
> +
> +static struct or1k_pic_dev or1k_pic_or1200 = {
> +       .chip = {
> +               .name = "or1200-PIC",
> +               .irq_unmask = or1k_pic_unmask,
> +               .irq_mask = or1k_pic_mask,
> +               .irq_ack = or1k_pic_or1200_ack,
> +               .irq_mask_ack = or1k_pic_or1200_mask_ack,
> +       },
> +       .handle = handle_level_irq,
> +       .flags = IRQ_LEVEL | IRQ_NOPROBE,
> +};
> +
> +static struct irq_domain *root_domain;
> +
> +static inline int pic_get_irq(int first)
> +{
> +       int hwirq;
> +
> +       hwirq = ffs(mfspr(SPR_PICSR) >> first);
> +       if (!hwirq)
> +               return NO_IRQ;
> +       else
> +               hwirq = hwirq + first - 1;
> +
> +       return irq_find_mapping(root_domain, hwirq);
> +}
> +
> +static void or1k_pic_handle_irq(struct pt_regs *regs)
> +{
> +       int irq = -1;
> +
> +       while ((irq = pic_get_irq(irq + 1)) != NO_IRQ)
> +               handle_IRQ(irq, regs);
> +}
> +
> +static int or1k_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
> +{
> +       struct or1k_pic_dev *pic = d->host_data;
> +
> +       irq_set_chip_and_handler(irq, &pic->chip, pic->handle);
> +       irq_set_status_flags(irq, pic->flags);
> +
> +       return 0;
> +}
> +
> +static const struct irq_domain_ops or1k_irq_domain_ops = {
> +       .xlate = irq_domain_xlate_onecell,
> +       .map = or1k_map,
> +};
> +
> +/*
> + * This sets up the IRQ domain for the PIC built in to the OpenRISC
> + * 1000 CPU.  This is the "root" domain as these are the interrupts
> + * that directly trigger an exception in the CPU.
> + */
> +static int __init or1k_pic_init(struct device_node *node,
> +                                struct or1k_pic_dev *pic)
> +{
> +       /* Disable all interrupts until explicitly requested */
> +       mtspr(SPR_PICMR, (0UL));
> +
> +       root_domain = irq_domain_add_linear(node, 32, &or1k_irq_domain_ops,
> +                                           pic);
> +
> +       set_handle_irq(or1k_pic_handle_irq);
> +
> +       return 0;
> +}
> +
> +static int __init or1k_pic_or1200_init(struct device_node *node,
> +                                      struct device_node *parent)
> +{
> +       return or1k_pic_init(node, &or1k_pic_or1200);
> +}
> +IRQCHIP_DECLARE(or1k_pic_or1200, "opencores,or1200-pic", or1k_pic_or1200_init);
> +IRQCHIP_DECLARE(or1k_pic, "opencores,or1k-pic", or1k_pic_or1200_init);
> +
> +static int __init or1k_pic_level_init(struct device_node *node,
> +                                     struct device_node *parent)
> +{
> +       return or1k_pic_init(node, &or1k_pic_level);
> +}
> +IRQCHIP_DECLARE(or1k_pic_level, "opencores,or1k-pic-level",
> +               or1k_pic_level_init);
> +
> +static int __init or1k_pic_edge_init(struct device_node *node,
> +                                    struct device_node *parent)
> +{
> +       return or1k_pic_init(node, &or1k_pic_edge);
> +}
> +IRQCHIP_DECLARE(or1k_pic_edge, "opencores,or1k-pic-edge", or1k_pic_edge_init);
> --
> 1.8.3.2
>



-- 
Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH v4] openrisc: irq: use irqchip framework
  2014-05-26 20:52 ` Geert Uytterhoeven
@ 2014-05-27  6:47   ` Jonas Bonn
  2014-05-29 20:28     ` Stefan Kristiansson
  0 siblings, 1 reply; 9+ messages in thread
From: Jonas Bonn @ 2014-05-27  6:47 UTC (permalink / raw)
  To: Geert Uytterhoeven, Stefan Kristiansson
  Cc: linux-kernel, linux, Thomas Gleixner, Jason Cooper, devicetree

On 05/26/2014 10:52 PM, Geert Uytterhoeven wrote:
> CC devicetree for the bindings
> 
> On Mon, May 26, 2014 at 10:31 PM, Stefan Kristiansson
> <stefan.kristiansson@saunalahti.fi> wrote:
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
>> @@ -0,0 +1,23 @@
>> +OpenRISC 1000 Programmable Interrupt Controller
>> +
>> +Required properties:
>> +
>> +- compatible : should be "opencores,or1k-pic-level" for variants with
>> +  level triggered interrupt lines, "opencores,or1k-pic-edge" for variants with
>> +  edge triggered interrupt lines or "opencores,or1200-pic" for machines
>> +  with the non-spec compliant or1200 type implementation.
>> +
>> +  "opencores,or1k-pic" is also provided as an alias to "opencores,or1200-pic",
>> +  but this is only for backwards compatibility.

I still think this identifier needs to be versioned.  Use the same
version number as we have on the cpu identifier since the OR1200 PIC
hasn't changed since then; i.e. opencores,or1200-pic-rtlsvnXYZ.

/Jonas



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

* Re: [PATCH v4] openrisc: irq: use irqchip framework
  2014-05-27  6:47   ` Jonas Bonn
@ 2014-05-29 20:28     ` Stefan Kristiansson
  2014-05-30 11:08       ` Jonas Bonn
  2014-06-21  2:04       ` Jason Cooper
  0 siblings, 2 replies; 9+ messages in thread
From: Stefan Kristiansson @ 2014-05-29 20:28 UTC (permalink / raw)
  To: Jonas Bonn
  Cc: Geert Uytterhoeven, linux-kernel, linux, Thomas Gleixner,
	Jason Cooper, devicetree

On Tue, May 27, 2014 at 08:47:36AM +0200, Jonas Bonn wrote:
> On 05/26/2014 10:52 PM, Geert Uytterhoeven wrote:
> > CC devicetree for the bindings
> > 
> > On Mon, May 26, 2014 at 10:31 PM, Stefan Kristiansson
> > <stefan.kristiansson@saunalahti.fi> wrote:
> >> +++ b/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
> >> @@ -0,0 +1,23 @@
> >> +OpenRISC 1000 Programmable Interrupt Controller
> >> +
> >> +Required properties:
> >> +
> >> +- compatible : should be "opencores,or1k-pic-level" for variants with
> >> +  level triggered interrupt lines, "opencores,or1k-pic-edge" for variants with
> >> +  edge triggered interrupt lines or "opencores,or1200-pic" for machines
> >> +  with the non-spec compliant or1200 type implementation.
> >> +
> >> +  "opencores,or1k-pic" is also provided as an alias to "opencores,or1200-pic",
> >> +  but this is only for backwards compatibility.
> 
> I still think this identifier needs to be versioned.  Use the same
> version number as we have on the cpu identifier since the OR1200 PIC
> hasn't changed since then; i.e. opencores,or1200-pic-rtlsvnXYZ.
> 

I can change that if you *really* insist on it...
But I don't understand the purpose of the versioning here,
there will never be any other or1200-pic version than the one that currently
exists, so IMO "or1200" should be enough versioning information.

Stefan

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

* Re: [PATCH v4] openrisc: irq: use irqchip framework
  2014-05-29 20:28     ` Stefan Kristiansson
@ 2014-05-30 11:08       ` Jonas Bonn
  2014-06-21  2:04       ` Jason Cooper
  1 sibling, 0 replies; 9+ messages in thread
From: Jonas Bonn @ 2014-05-30 11:08 UTC (permalink / raw)
  To: Stefan Kristiansson
  Cc: Geert Uytterhoeven, linux-kernel, linux, Thomas Gleixner,
	Jason Cooper, devicetree

On 05/29/2014 10:28 PM, Stefan Kristiansson wrote:
> On Tue, May 27, 2014 at 08:47:36AM +0200, Jonas Bonn wrote:
>> On 05/26/2014 10:52 PM, Geert Uytterhoeven wrote:
>>> CC devicetree for the bindings
>>>
>>> On Mon, May 26, 2014 at 10:31 PM, Stefan Kristiansson
>>> <stefan.kristiansson@saunalahti.fi> wrote:
>>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
>>>> @@ -0,0 +1,23 @@
>>>> +OpenRISC 1000 Programmable Interrupt Controller
>>>> +
>>>> +Required properties:
>>>> +
>>>> +- compatible : should be "opencores,or1k-pic-level" for variants with
>>>> +  level triggered interrupt lines, "opencores,or1k-pic-edge" for variants with
>>>> +  edge triggered interrupt lines or "opencores,or1200-pic" for machines
>>>> +  with the non-spec compliant or1200 type implementation.
>>>> +
>>>> +  "opencores,or1k-pic" is also provided as an alias to "opencores,or1200-pic",
>>>> +  but this is only for backwards compatibility.
>>
>> I still think this identifier needs to be versioned.  Use the same
>> version number as we have on the cpu identifier since the OR1200 PIC
>> hasn't changed since then; i.e. opencores,or1200-pic-rtlsvnXYZ.
>>
> 
> I can change that if you *really* insist on it...
> But I don't understand the purpose of the versioning here,
> there will never be any other or1200-pic version than the one that currently
> exists, so IMO "or1200" should be enough versioning information.

Famous last words... ;)

It has been insisted upon earlier by upstream; for example, when we
submitted the openrisc architecture.  Aside from that, I am not the one
who is going to be doing any insisting here.

That said, given that this is a soft core CPU and what that entails, I
do think it is probably a good idea to version this thing.

Hopefully you will get some sage advice from someone else here...
otherwise, you may do as you see fit.

Your patch is otherwise fine by me and your effort here is much appreciated!

Acked-by: Jonas Bonn <jonas@southpole.se>

/Jonas

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

* Re: [PATCH v4] openrisc: irq: use irqchip framework
  2014-05-29 20:28     ` Stefan Kristiansson
  2014-05-30 11:08       ` Jonas Bonn
@ 2014-06-21  2:04       ` Jason Cooper
  2014-06-21  7:13         ` Stefan Kristiansson
  1 sibling, 1 reply; 9+ messages in thread
From: Jason Cooper @ 2014-06-21  2:04 UTC (permalink / raw)
  To: Stefan Kristiansson
  Cc: Jonas Bonn, Geert Uytterhoeven, linux-kernel, linux,
	Thomas Gleixner, devicetree

On Thu, May 29, 2014 at 11:28:08PM +0300, Stefan Kristiansson wrote:
> On Tue, May 27, 2014 at 08:47:36AM +0200, Jonas Bonn wrote:
> > On 05/26/2014 10:52 PM, Geert Uytterhoeven wrote:
> > > CC devicetree for the bindings
> > > 
> > > On Mon, May 26, 2014 at 10:31 PM, Stefan Kristiansson
> > > <stefan.kristiansson@saunalahti.fi> wrote:
> > >> +++ b/Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
> > >> @@ -0,0 +1,23 @@
> > >> +OpenRISC 1000 Programmable Interrupt Controller
> > >> +
> > >> +Required properties:
> > >> +
> > >> +- compatible : should be "opencores,or1k-pic-level" for variants with
> > >> +  level triggered interrupt lines, "opencores,or1k-pic-edge" for variants with
> > >> +  edge triggered interrupt lines or "opencores,or1200-pic" for machines
> > >> +  with the non-spec compliant or1200 type implementation.
> > >> +
> > >> +  "opencores,or1k-pic" is also provided as an alias to "opencores,or1200-pic",
> > >> +  but this is only for backwards compatibility.
> > 
> > I still think this identifier needs to be versioned.  Use the same
> > version number as we have on the cpu identifier since the OR1200 PIC
> > hasn't changed since then; i.e. opencores,or1200-pic-rtlsvnXYZ.
> > 
> 
> I can change that if you *really* insist on it...
> But I don't understand the purpose of the versioning here,
> there will never be any other or1200-pic version than the one that currently
> exists, so IMO "or1200" should be enough versioning information.

I'm horribly unfamiliar with openrisc, but compatible strings are
compatible strings. ;-)

Is the *actual* IP block called or1200-pic?  Or is it, eg or1235-pic, and
you're using or1200-pic as a generic catch-all?

Please use the specific IP name without wildcards.  That compatible
string will then be used on that IP and future IP that is compatible
with the original IP.  Once an incompatible change is introduced, then
we'll create a new compatible string, say or1300-pic, or or1237-pic.

When in doubt, be specific.  I don't think the '-rtlsvnXYZ' should be
necessary, though.

thx,

Jason.

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

* Re: [PATCH v4] openrisc: irq: use irqchip framework
  2014-06-21  2:04       ` Jason Cooper
@ 2014-06-21  7:13         ` Stefan Kristiansson
  0 siblings, 0 replies; 9+ messages in thread
From: Stefan Kristiansson @ 2014-06-21  7:13 UTC (permalink / raw)
  To: Jason Cooper
  Cc: Jonas Bonn, Geert Uytterhoeven, linux-kernel, linux,
	Thomas Gleixner, devicetree

On Fri, Jun 20, 2014 at 10:04:30PM -0400, Jason Cooper wrote:
> On Thu, May 29, 2014 at 11:28:08PM +0300, Stefan Kristiansson wrote:
> > But I don't understand the purpose of the versioning here,
> > there will never be any other or1200-pic version than the one that currently
> > exists, so IMO "or1200" should be enough versioning information.
> 
> I'm horribly unfamiliar with openrisc, but compatible strings are
> compatible strings. ;-)
> 
> Is the *actual* IP block called or1200-pic?  Or is it, eg or1235-pic, and
> you're using or1200-pic as a generic catch-all?
> 

No, the or1200-pic is a part of the or1200 cpu that is one
(there are several others) implementation of the OpenRISC 1000 architecture.
The quirk here is that the OpenRISC 1000 architecture specification specifies
how a PIC should be implemented, but or1200 doesn't follow that to 100%.
Thus, it need a bit of special care.

The implementations that follow the specification can use the
or1k-pic-level and or1k-pic-edge variants.

> Please use the specific IP name without wildcards.  That compatible
> string will then be used on that IP and future IP that is compatible
> with the original IP.  Once an incompatible change is introduced, then
> we'll create a new compatible string, say or1300-pic, or or1237-pic.
> 

The point I tried to make above was that if a change would be
made to the pic implementation in or1200, it should be a change to conform
to the specification.

> When in doubt, be specific.  I don't think the '-rtlsvnXYZ' should be
> necessary, though.
> 

I agree, and I think 'or1200-pic' is specific enough in this case.

Stefan

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

* Re: [PATCH v4] openrisc: irq: use irqchip framework
  2014-05-26 20:31 [PATCH v4] openrisc: irq: use irqchip framework Stefan Kristiansson
  2014-05-26 20:52 ` Geert Uytterhoeven
@ 2014-06-21 20:02 ` Jason Cooper
  2014-07-01 12:25   ` Jason Cooper
  1 sibling, 1 reply; 9+ messages in thread
From: Jason Cooper @ 2014-06-21 20:02 UTC (permalink / raw)
  To: Stefan Kristiansson; +Cc: linux-kernel, linux, jonas, tglx, geert

Stefan,

On Mon, May 26, 2014 at 11:31:42PM +0300, Stefan Kristiansson wrote:
> In addition to consolidating the or1k-pic with other interrupt
> controllers, this makes OpenRISC less tied to its on-cpu
> interrupt controller.
> 
> All or1k-pic specific parts are moved out of irq.c and into
> drivers/irqchip/irq-or1k-pic.c
> 
> In that transition, the funtionality have been divided into
> three chip variants.
> One that handles level triggered interrupts, one that handles edge
> triggered interrupts and one that handles the interrupt
> controller that is present in the or1200 OpenRISC cpu
> implementation.
> 
> Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> ---
> Changes in v2:
>  - Move or1k-pic related code into irq-or1k-pic
>  - Add documentation for device tree bindings
> 
> Changes in v3:
>  - Split level-, edge-triggered and or1200 implementation into seperate
>    chip variants.
> 
> Changes in v4:
>  - Fix typos in documentation
> ---
>  .../interrupt-controller/opencores,or1k-pic.txt    |  23 +++
>  arch/openrisc/Kconfig                              |   1 +
>  arch/openrisc/include/asm/irq.h                    |   3 +
>  arch/openrisc/kernel/irq.c                         | 146 ++---------------
>  drivers/irqchip/Kconfig                            |   4 +
>  drivers/irqchip/Makefile                           |   1 +
>  drivers/irqchip/irq-or1k-pic.c                     | 182 +++++++++++++++++++++
>  7 files changed, 227 insertions(+), 133 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
>  create mode 100644 drivers/irqchip/irq-or1k-pic.c

I've now applied this to irqchip/unstable with Jonas' Ack to get it some
time in -next.  As the name implies, I can rebase or edit this as
needed.

Since this crosses into arch/openrisc, you may have other changes
depending on it.  If so, just let me know and I'll create a stable topic
branch for you instead of merging it directly into irqchip/core.

thx,

Jason.

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

* Re: [PATCH v4] openrisc: irq: use irqchip framework
  2014-06-21 20:02 ` Jason Cooper
@ 2014-07-01 12:25   ` Jason Cooper
  0 siblings, 0 replies; 9+ messages in thread
From: Jason Cooper @ 2014-07-01 12:25 UTC (permalink / raw)
  To: Stefan Kristiansson; +Cc: linux-kernel, linux, jonas, tglx, geert

On Sat, Jun 21, 2014 at 04:02:02PM -0400, Jason Cooper wrote:
> Stefan,
> 
> On Mon, May 26, 2014 at 11:31:42PM +0300, Stefan Kristiansson wrote:
> > In addition to consolidating the or1k-pic with other interrupt
> > controllers, this makes OpenRISC less tied to its on-cpu
> > interrupt controller.
> > 
> > All or1k-pic specific parts are moved out of irq.c and into
> > drivers/irqchip/irq-or1k-pic.c
> > 
> > In that transition, the funtionality have been divided into
> > three chip variants.
> > One that handles level triggered interrupts, one that handles edge
> > triggered interrupts and one that handles the interrupt
> > controller that is present in the or1200 OpenRISC cpu
> > implementation.
> > 
> > Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> > ---
> > Changes in v2:
> >  - Move or1k-pic related code into irq-or1k-pic
> >  - Add documentation for device tree bindings
> > 
> > Changes in v3:
> >  - Split level-, edge-triggered and or1200 implementation into seperate
> >    chip variants.
> > 
> > Changes in v4:
> >  - Fix typos in documentation
> > ---
> >  .../interrupt-controller/opencores,or1k-pic.txt    |  23 +++
> >  arch/openrisc/Kconfig                              |   1 +
> >  arch/openrisc/include/asm/irq.h                    |   3 +
> >  arch/openrisc/kernel/irq.c                         | 146 ++---------------
> >  drivers/irqchip/Kconfig                            |   4 +
> >  drivers/irqchip/Makefile                           |   1 +
> >  drivers/irqchip/irq-or1k-pic.c                     | 182 +++++++++++++++++++++
> >  7 files changed, 227 insertions(+), 133 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/opencores,or1k-pic.txt
> >  create mode 100644 drivers/irqchip/irq-or1k-pic.c
> 
> I've now applied this to irqchip/unstable with Jonas' Ack to get it some
> time in -next.  As the name implies, I can rebase or edit this as
> needed.

Now moved into irqchip/core.

thx,

Jason.

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

end of thread, other threads:[~2014-07-01 12:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-26 20:31 [PATCH v4] openrisc: irq: use irqchip framework Stefan Kristiansson
2014-05-26 20:52 ` Geert Uytterhoeven
2014-05-27  6:47   ` Jonas Bonn
2014-05-29 20:28     ` Stefan Kristiansson
2014-05-30 11:08       ` Jonas Bonn
2014-06-21  2:04       ` Jason Cooper
2014-06-21  7:13         ` Stefan Kristiansson
2014-06-21 20:02 ` Jason Cooper
2014-07-01 12:25   ` Jason Cooper

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).