[PATCH 0/3] genirq: Saving/restoring the irqchip state of an irq line

[PATCH 0/3] genirq: Saving/restoring the irqchip state of an irq line

[Marc Zyngier]

Despite Linux offering a rather fine grained control over the life
cycle of an interrupt, there is a few cases where it would be very
useful to snapshot (or even set) the internal state of the interrupt
controller for a given interrupt line:

- With KVM, a device shared between VMs must have its whole context
  switched, and that includes the interrupt line state. KVM/arm is
  moving to using this.
- Some GPIO controllers seem to require peeking into the interrupt controller
  they are connected to to report their internal state.

Instead of letting people facing this situation doing horrible
(controller specific) hacks in their code, let's offer a couple of new
entry points that allow a few attributes to be read and set.

Of course, this is a very dangerous thing to do if you don't know what
you doing, and I wouldn't expect most drivers to use this. But this
can also be a life saver at times.

This patch series implement said API, and adds support for this to the
two main ARM interrupt controllers (GIC and GICv3).

Based on 3.18-rc1, tested on arm/arm64, and also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/irqchip_state

Marc Zyngier (3):
  genirq: Allow the irqchip state of an IRQ to be save/restored
  irqchip: GIC: Add support for irq_{get,set}_irqchip_state
  irqchip: GICv3: Add support for irq_{get,set}_irqchip_state

 drivers/irqchip/irq-gic-v3.c | 78 ++++++++++++++++++++++++++++++++++++--------
 drivers/irqchip/irq-gic.c    | 70 ++++++++++++++++++++++++++++++++++++---
 include/linux/interrupt.h    |  2 ++
 include/linux/irq.h          | 18 ++++++++++
 kernel/irq/manage.c          | 71 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 221 insertions(+), 18 deletions(-)

-- 
2.1.0

[PATCH 0/3] genirq: Saving/restoring the irqchip state of an irq line

[Marc Zyngier]

Despite Linux offering a rather fine grained control over the life
cycle of an interrupt, there is a few cases where it would be very
useful to snapshot (or even set) the internal state of the interrupt
controller for a given interrupt line:

- With KVM, a device shared between VMs must have its whole context
  switched, and that includes the interrupt line state. KVM/arm is
  moving to using this.
- Some GPIO controllers seem to require peeking into the interrupt controller
  they are connected to to report their internal state.

Instead of letting people facing this situation doing horrible
(controller specific) hacks in their code, let's offer a couple of new
entry points that allow a few attributes to be read and set.

Of course, this is a very dangerous thing to do if you don't know what
you doing, and I wouldn't expect most drivers to use this. But this
can also be a life saver at times.

This patch series implement said API, and adds support for this to the
two main ARM interrupt controllers (GIC and GICv3).

Based on 3.18-rc1, tested on arm/arm64, and also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/irqchip_state

Marc Zyngier (3):
  genirq: Allow the irqchip state of an IRQ to be save/restored
  irqchip: GIC: Add support for irq_{get,set}_irqchip_state
  irqchip: GICv3: Add support for irq_{get,set}_irqchip_state

 drivers/irqchip/irq-gic-v3.c | 78 ++++++++++++++++++++++++++++++++++++--------
 drivers/irqchip/irq-gic.c    | 70 ++++++++++++++++++++++++++++++++++++---
 include/linux/interrupt.h    |  2 ++
 include/linux/irq.h          | 18 ++++++++++
 kernel/irq/manage.c          | 71 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 221 insertions(+), 18 deletions(-)

-- 
2.1.0

[PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Marc Zyngier]

There is a number of cases where a kernel subsystem may want to
introspect the state of an interrupt at the irqchip level:

- When a peripheral is shared between virtual machines, its interrupt
  state becomes part of the guest's state, and must be switched accordingly.
  KVM on arm/arm64 requires this for its guest-visible timer
- Some GPIO controllers seem to require peeking into the interrupt controller
  they are connected to to report their internal state

This seem to be a pattern that is common enough for the core code to try and
support this without too many horrible hacks. Introduce a pair of accessors
(irq_get_irqchip_state/irq_set_irqchip_state) to retrieve the bits that can
be of interest to another subsystem: pending, active, and masked.

- irq_get_irqchip_state returns the state of the interrupt according to a
  state parameter set to IRQCHIP_STATE_PENDING, IRQCHIP_STATE_ACTIVE
  or IRQCHIP_STATE_MASKED.
- irq_set_irqchip_state sets the state of the interrupt according to
  a similar state.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/interrupt.h |  2 ++
 include/linux/irq.h       | 18 ++++++++++++
 kernel/irq/manage.c       | 71 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 69517a2..80818b4 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -356,6 +356,8 @@ static inline int disable_irq_wake(unsigned int irq)
 	return irq_set_irq_wake(irq, 0);
 }
 
+extern int irq_get_irqchip_state(unsigned int irq, int state);
+extern int irq_set_irqchip_state(unsigned int irq, int state, int val);
 
 #ifdef CONFIG_IRQ_FORCED_THREADING
 extern bool force_irqthreads;
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 03f48d9..257d59a 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -315,6 +315,8 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
  *				any other callback related to this irq
  * @irq_release_resources:	optional to release resources acquired with
  *				irq_request_resources
+ * @irq_get_irqchip_state:	return the internal state of an interrupt
+ * @irq_set_irqchip_state:	set the internal state of a interrupt
  * @flags:		chip specific flags
  */
 struct irq_chip {
@@ -351,6 +353,9 @@ struct irq_chip {
 	int		(*irq_request_resources)(struct irq_data *data);
 	void		(*irq_release_resources)(struct irq_data *data);
 
+	int		(*irq_get_irqchip_state)(struct irq_data *data, int state);
+	void		(*irq_set_irqchip_state)(struct irq_data *data, int state, int val);
+
 	unsigned long	flags;
 };
 
@@ -376,6 +381,19 @@ enum {
 	IRQCHIP_EOI_THREADED		= (1 <<  6),
 };
 
+/*
+ * irq_get_irqchip_state/irq_set_irqchip_state specific flags:
+ *
+ * IRQCHIP_STATE_PENDING:	Interrupt asserted at the pin level
+ * IRQCHIP_STATE_ACTIVE:	Interrupt in progress (ACKed, but not EOIed)
+ * IRQCHIP_STATE_MASKED:	Interrupt is masked
+ */
+enum {
+	IRQCHIP_STATE_PENDING,
+	IRQCHIP_STATE_ACTIVE,
+	IRQCHIP_STATE_MASKED,
+};
+
 /* This include will go away once we isolated irq_desc usage to core code */
 #include <linux/irqdesc.h>
 
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 0a9104b..6a4c03f 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1756,3 +1756,74 @@ int request_percpu_irq(unsigned int irq, irq_handler_t handler,
 
 	return retval;
 }
+
+/**
+ *	irq_get_irqchip_state - returns the irqchip state of a interrupt.
+ *	@irq: Interrupt line that is forwarded to a VM
+ *	@state: One of IRQCHIP_STATE_* the caller wants to know about
+ *
+ *	This call snapshots the internal irqchip state of an
+ *	interrupt, returning the bit corresponding to the requested
+ *	@state.
+ *
+ *	This function should be called with preemption disabled if the
+ *	interrupt controller has per-cpu registers.
+ */
+int irq_get_irqchip_state(unsigned int irq, int state)
+{
+	struct irq_desc *desc;
+	struct irq_data *data;
+	struct irq_chip *chip;
+	int val;
+
+	desc = irq_to_desc(irq);
+	if (!desc)
+		return -EINVAL;
+
+	data = irq_desc_get_irq_data(desc);
+
+	chip = irq_desc_get_chip(desc);
+	if (!chip->irq_get_irqchip_state)
+		return -EINVAL;
+
+	chip_bus_lock(desc);
+	val = chip->irq_get_irqchip_state(data, state);
+	chip_bus_sync_unlock(desc);
+
+	return val;
+}
+
+/**
+ *	irq_set_irqchip_state - set the state of a forwarded interrupt.
+ *	@irq: Interrupt line that is forwarded to a VM
+ *	@state: State to be restored (one of IRQCHIP_STATE_*)
+ *	@val: value corresponding to @state
+ *
+ *	This call sets the internal irqchip state of an interrupt,
+ *	depending on the value of @state.
+ *
+ *	This function should be called with preemption disabled if the
+ *	interrupt controller has per-cpu registers.
+ */
+int irq_set_irqchip_state(unsigned int irq, int state, int val)
+{
+	struct irq_desc *desc;
+	struct irq_data *data;
+	struct irq_chip *chip;
+
+	desc = irq_to_desc(irq);
+	if (!desc)
+		return -EINVAL;
+
+	data = irq_desc_get_irq_data(desc);
+
+	chip = irq_desc_get_chip(desc);
+	if (!chip->irq_set_irqchip_state)
+		return -EINVAL;
+
+	chip_bus_lock(desc);
+	chip->irq_set_irqchip_state(data, state, val);
+	chip_bus_sync_unlock(desc);
+
+	return 0;
+}
-- 
2.1.0

[PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Marc Zyngier]

There is a number of cases where a kernel subsystem may want to
introspect the state of an interrupt at the irqchip level:

- When a peripheral is shared between virtual machines, its interrupt
  state becomes part of the guest's state, and must be switched accordingly.
  KVM on arm/arm64 requires this for its guest-visible timer
- Some GPIO controllers seem to require peeking into the interrupt controller
  they are connected to to report their internal state

This seem to be a pattern that is common enough for the core code to try and
support this without too many horrible hacks. Introduce a pair of accessors
(irq_get_irqchip_state/irq_set_irqchip_state) to retrieve the bits that can
be of interest to another subsystem: pending, active, and masked.

- irq_get_irqchip_state returns the state of the interrupt according to a
  state parameter set to IRQCHIP_STATE_PENDING, IRQCHIP_STATE_ACTIVE
  or IRQCHIP_STATE_MASKED.
- irq_set_irqchip_state sets the state of the interrupt according to
  a similar state.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/interrupt.h |  2 ++
 include/linux/irq.h       | 18 ++++++++++++
 kernel/irq/manage.c       | 71 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 69517a2..80818b4 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -356,6 +356,8 @@ static inline int disable_irq_wake(unsigned int irq)
 	return irq_set_irq_wake(irq, 0);
 }
 
+extern int irq_get_irqchip_state(unsigned int irq, int state);
+extern int irq_set_irqchip_state(unsigned int irq, int state, int val);
 
 #ifdef CONFIG_IRQ_FORCED_THREADING
 extern bool force_irqthreads;
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 03f48d9..257d59a 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -315,6 +315,8 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
  *				any other callback related to this irq
  * @irq_release_resources:	optional to release resources acquired with
  *				irq_request_resources
+ * @irq_get_irqchip_state:	return the internal state of an interrupt
+ * @irq_set_irqchip_state:	set the internal state of a interrupt
  * @flags:		chip specific flags
  */
 struct irq_chip {
@@ -351,6 +353,9 @@ struct irq_chip {
 	int		(*irq_request_resources)(struct irq_data *data);
 	void		(*irq_release_resources)(struct irq_data *data);
 
+	int		(*irq_get_irqchip_state)(struct irq_data *data, int state);
+	void		(*irq_set_irqchip_state)(struct irq_data *data, int state, int val);
+
 	unsigned long	flags;
 };
 
@@ -376,6 +381,19 @@ enum {
 	IRQCHIP_EOI_THREADED		= (1 <<  6),
 };
 
+/*
+ * irq_get_irqchip_state/irq_set_irqchip_state specific flags:
+ *
+ * IRQCHIP_STATE_PENDING:	Interrupt asserted@the pin level
+ * IRQCHIP_STATE_ACTIVE:	Interrupt in progress (ACKed, but not EOIed)
+ * IRQCHIP_STATE_MASKED:	Interrupt is masked
+ */
+enum {
+	IRQCHIP_STATE_PENDING,
+	IRQCHIP_STATE_ACTIVE,
+	IRQCHIP_STATE_MASKED,
+};
+
 /* This include will go away once we isolated irq_desc usage to core code */
 #include <linux/irqdesc.h>
 
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 0a9104b..6a4c03f 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1756,3 +1756,74 @@ int request_percpu_irq(unsigned int irq, irq_handler_t handler,
 
 	return retval;
 }
+
+/**
+ *	irq_get_irqchip_state - returns the irqchip state of a interrupt.
+ *	@irq: Interrupt line that is forwarded to a VM
+ *	@state: One of IRQCHIP_STATE_* the caller wants to know about
+ *
+ *	This call snapshots the internal irqchip state of an
+ *	interrupt, returning the bit corresponding to the requested
+ *	@state.
+ *
+ *	This function should be called with preemption disabled if the
+ *	interrupt controller has per-cpu registers.
+ */
+int irq_get_irqchip_state(unsigned int irq, int state)
+{
+	struct irq_desc *desc;
+	struct irq_data *data;
+	struct irq_chip *chip;
+	int val;
+
+	desc = irq_to_desc(irq);
+	if (!desc)
+		return -EINVAL;
+
+	data = irq_desc_get_irq_data(desc);
+
+	chip = irq_desc_get_chip(desc);
+	if (!chip->irq_get_irqchip_state)
+		return -EINVAL;
+
+	chip_bus_lock(desc);
+	val = chip->irq_get_irqchip_state(data, state);
+	chip_bus_sync_unlock(desc);
+
+	return val;
+}
+
+/**
+ *	irq_set_irqchip_state - set the state of a forwarded interrupt.
+ *	@irq: Interrupt line that is forwarded to a VM
+ *	@state: State to be restored (one of IRQCHIP_STATE_*)
+ *	@val: value corresponding to @state
+ *
+ *	This call sets the internal irqchip state of an interrupt,
+ *	depending on the value of @state.
+ *
+ *	This function should be called with preemption disabled if the
+ *	interrupt controller has per-cpu registers.
+ */
+int irq_set_irqchip_state(unsigned int irq, int state, int val)
+{
+	struct irq_desc *desc;
+	struct irq_data *data;
+	struct irq_chip *chip;
+
+	desc = irq_to_desc(irq);
+	if (!desc)
+		return -EINVAL;
+
+	data = irq_desc_get_irq_data(desc);
+
+	chip = irq_desc_get_chip(desc);
+	if (!chip->irq_set_irqchip_state)
+		return -EINVAL;
+
+	chip_bus_lock(desc);
+	chip->irq_set_irqchip_state(data, state, val);
+	chip_bus_sync_unlock(desc);
+
+	return 0;
+}
-- 
2.1.0

[PATCH 2/3] irqchip: GIC: Add support for irq_{get, set}_irqchip_state

[Marc Zyngier]

Add the required hooks for the internal state of an interrupt
to be exposed to other subsystems.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic.c | 70 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 38493ff..d78169e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -151,12 +151,22 @@ static inline unsigned int gic_irq(struct irq_data *d)
 /*
  * Routines to acknowledge, disable and enable interrupts
  */
-static void gic_mask_irq(struct irq_data *d)
+static void gic_poke_irq(struct irq_data *d, u32 offset)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
+	writel_relaxed(mask, gic_dist_base(d) + offset + (gic_irq(d) / 32) * 4);
+}
 
+static int gic_peek_irq(struct irq_data *d, u32 offset)
+{
+	u32 mask = 1 << (gic_irq(d) % 32);
+	return !!(readl_relaxed(gic_dist_base(d) + offset + (gic_irq(d) / 32) * 4) & mask);
+}
+
+static void gic_mask_irq(struct irq_data *d)
+{
 	raw_spin_lock(&irq_controller_lock);
-	writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_CLEAR + (gic_irq(d) / 32) * 4);
+	gic_poke_irq(d, GIC_DIST_ENABLE_CLEAR);
 	if (gic_arch_extn.irq_mask)
 		gic_arch_extn.irq_mask(d);
 	raw_spin_unlock(&irq_controller_lock);
@@ -164,12 +174,10 @@ static void gic_mask_irq(struct irq_data *d)
 
 static void gic_unmask_irq(struct irq_data *d)
 {
-	u32 mask = 1 << (gic_irq(d) % 32);
-
 	raw_spin_lock(&irq_controller_lock);
 	if (gic_arch_extn.irq_unmask)
 		gic_arch_extn.irq_unmask(d);
-	writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_SET + (gic_irq(d) / 32) * 4);
+	gic_poke_irq(d, GIC_DIST_ENABLE_SET);
 	raw_spin_unlock(&irq_controller_lock);
 }
 
@@ -184,6 +192,56 @@ static void gic_eoi_irq(struct irq_data *d)
 	writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
 }
 
+static void gic_irq_set_irqchip_state(struct irq_data *d, int state, int val)
+{
+	u32 reg;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		reg = val ? GIC_DIST_PENDING_SET : GIC_DIST_PENDING_CLEAR;
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		reg = val ? GIC_DIST_ACTIVE_SET : GIC_DIST_ACTIVE_CLEAR;
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		reg = val ? GIC_DIST_ENABLE_CLEAR : GIC_DIST_ENABLE_SET;
+		break;
+
+	default:
+		WARN_ON(1);
+		return;
+	}
+
+	gic_poke_irq(d, reg);
+}
+
+static int gic_irq_get_irqchip_state(struct irq_data *d, int state)
+{
+	int val;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		val = gic_peek_irq(d, GIC_DIST_PENDING_SET);
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		val = gic_peek_irq(d, GIC_DIST_ACTIVE_SET);
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		val = !gic_peek_irq(d, GIC_DIST_ENABLE_SET);
+		break;
+
+	default:
+		WARN_ON(1);
+		val = 0;
+	}
+
+	return val;
+}
+
 static int gic_set_type(struct irq_data *d, unsigned int type)
 {
 	void __iomem *base = gic_dist_base(d);
@@ -322,6 +380,8 @@ static struct irq_chip gic_chip = {
 	.irq_set_affinity	= gic_set_affinity,
 #endif
 	.irq_set_wake		= gic_set_wake,
+	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
+	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
 };
 
 void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
-- 
2.1.0

[PATCH 2/3] irqchip: GIC: Add support for irq_{get,set}_irqchip_state

[Marc Zyngier]

Add the required hooks for the internal state of an interrupt
to be exposed to other subsystems.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic.c | 70 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 38493ff..d78169e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -151,12 +151,22 @@ static inline unsigned int gic_irq(struct irq_data *d)
 /*
  * Routines to acknowledge, disable and enable interrupts
  */
-static void gic_mask_irq(struct irq_data *d)
+static void gic_poke_irq(struct irq_data *d, u32 offset)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
+	writel_relaxed(mask, gic_dist_base(d) + offset + (gic_irq(d) / 32) * 4);
+}
 
+static int gic_peek_irq(struct irq_data *d, u32 offset)
+{
+	u32 mask = 1 << (gic_irq(d) % 32);
+	return !!(readl_relaxed(gic_dist_base(d) + offset + (gic_irq(d) / 32) * 4) & mask);
+}
+
+static void gic_mask_irq(struct irq_data *d)
+{
 	raw_spin_lock(&irq_controller_lock);
-	writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_CLEAR + (gic_irq(d) / 32) * 4);
+	gic_poke_irq(d, GIC_DIST_ENABLE_CLEAR);
 	if (gic_arch_extn.irq_mask)
 		gic_arch_extn.irq_mask(d);
 	raw_spin_unlock(&irq_controller_lock);
@@ -164,12 +174,10 @@ static void gic_mask_irq(struct irq_data *d)
 
 static void gic_unmask_irq(struct irq_data *d)
 {
-	u32 mask = 1 << (gic_irq(d) % 32);
-
 	raw_spin_lock(&irq_controller_lock);
 	if (gic_arch_extn.irq_unmask)
 		gic_arch_extn.irq_unmask(d);
-	writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_SET + (gic_irq(d) / 32) * 4);
+	gic_poke_irq(d, GIC_DIST_ENABLE_SET);
 	raw_spin_unlock(&irq_controller_lock);
 }
 
@@ -184,6 +192,56 @@ static void gic_eoi_irq(struct irq_data *d)
 	writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
 }
 
+static void gic_irq_set_irqchip_state(struct irq_data *d, int state, int val)
+{
+	u32 reg;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		reg = val ? GIC_DIST_PENDING_SET : GIC_DIST_PENDING_CLEAR;
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		reg = val ? GIC_DIST_ACTIVE_SET : GIC_DIST_ACTIVE_CLEAR;
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		reg = val ? GIC_DIST_ENABLE_CLEAR : GIC_DIST_ENABLE_SET;
+		break;
+
+	default:
+		WARN_ON(1);
+		return;
+	}
+
+	gic_poke_irq(d, reg);
+}
+
+static int gic_irq_get_irqchip_state(struct irq_data *d, int state)
+{
+	int val;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		val = gic_peek_irq(d, GIC_DIST_PENDING_SET);
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		val = gic_peek_irq(d, GIC_DIST_ACTIVE_SET);
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		val = !gic_peek_irq(d, GIC_DIST_ENABLE_SET);
+		break;
+
+	default:
+		WARN_ON(1);
+		val = 0;
+	}
+
+	return val;
+}
+
 static int gic_set_type(struct irq_data *d, unsigned int type)
 {
 	void __iomem *base = gic_dist_base(d);
@@ -322,6 +380,8 @@ static struct irq_chip gic_chip = {
 	.irq_set_affinity	= gic_set_affinity,
 #endif
 	.irq_set_wake		= gic_set_wake,
+	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
+	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
 };
 
 void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
-- 
2.1.0

[PATCH 2/3] irqchip: GIC: Add support for irq_{get, set}_irqchip_state

[Marc Zyngier]

Add the required hooks for the internal state of an interrupt
to be exposed to other subsystems.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic.c | 70 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 38493ff..d78169e 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -151,12 +151,22 @@ static inline unsigned int gic_irq(struct irq_data *d)
 /*
  * Routines to acknowledge, disable and enable interrupts
  */
-static void gic_mask_irq(struct irq_data *d)
+static void gic_poke_irq(struct irq_data *d, u32 offset)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
+	writel_relaxed(mask, gic_dist_base(d) + offset + (gic_irq(d) / 32) * 4);
+}
 
+static int gic_peek_irq(struct irq_data *d, u32 offset)
+{
+	u32 mask = 1 << (gic_irq(d) % 32);
+	return !!(readl_relaxed(gic_dist_base(d) + offset + (gic_irq(d) / 32) * 4) & mask);
+}
+
+static void gic_mask_irq(struct irq_data *d)
+{
 	raw_spin_lock(&irq_controller_lock);
-	writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_CLEAR + (gic_irq(d) / 32) * 4);
+	gic_poke_irq(d, GIC_DIST_ENABLE_CLEAR);
 	if (gic_arch_extn.irq_mask)
 		gic_arch_extn.irq_mask(d);
 	raw_spin_unlock(&irq_controller_lock);
@@ -164,12 +174,10 @@ static void gic_mask_irq(struct irq_data *d)
 
 static void gic_unmask_irq(struct irq_data *d)
 {
-	u32 mask = 1 << (gic_irq(d) % 32);
-
 	raw_spin_lock(&irq_controller_lock);
 	if (gic_arch_extn.irq_unmask)
 		gic_arch_extn.irq_unmask(d);
-	writel_relaxed(mask, gic_dist_base(d) + GIC_DIST_ENABLE_SET + (gic_irq(d) / 32) * 4);
+	gic_poke_irq(d, GIC_DIST_ENABLE_SET);
 	raw_spin_unlock(&irq_controller_lock);
 }
 
@@ -184,6 +192,56 @@ static void gic_eoi_irq(struct irq_data *d)
 	writel_relaxed(gic_irq(d), gic_cpu_base(d) + GIC_CPU_EOI);
 }
 
+static void gic_irq_set_irqchip_state(struct irq_data *d, int state, int val)
+{
+	u32 reg;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		reg = val ? GIC_DIST_PENDING_SET : GIC_DIST_PENDING_CLEAR;
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		reg = val ? GIC_DIST_ACTIVE_SET : GIC_DIST_ACTIVE_CLEAR;
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		reg = val ? GIC_DIST_ENABLE_CLEAR : GIC_DIST_ENABLE_SET;
+		break;
+
+	default:
+		WARN_ON(1);
+		return;
+	}
+
+	gic_poke_irq(d, reg);
+}
+
+static int gic_irq_get_irqchip_state(struct irq_data *d, int state)
+{
+	int val;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		val = gic_peek_irq(d, GIC_DIST_PENDING_SET);
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		val = gic_peek_irq(d, GIC_DIST_ACTIVE_SET);
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		val = !gic_peek_irq(d, GIC_DIST_ENABLE_SET);
+		break;
+
+	default:
+		WARN_ON(1);
+		val = 0;
+	}
+
+	return val;
+}
+
 static int gic_set_type(struct irq_data *d, unsigned int type)
 {
 	void __iomem *base = gic_dist_base(d);
@@ -322,6 +380,8 @@ static struct irq_chip gic_chip = {
 	.irq_set_affinity	= gic_set_affinity,
 #endif
 	.irq_set_wake		= gic_set_wake,
+	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
+	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
 };
 
 void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
-- 
2.1.0

[PATCH 3/3] irqchip: GICv3: Add support for irq_{get, set}_irqchip_state

[Marc Zyngier]

Add the required hooks for the internal state of an interrupt
to be exposed to other subsystems.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3.c | 78 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index aa17ae8..666c14e 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -193,6 +193,19 @@ static void gic_enable_redist(bool enable)
 /*
  * Routines to disable, enable, EOI and route interrupts
  */
+static int gic_peek_irq(struct irq_data *d, u32 offset)
+{
+	u32 mask = 1 << (gic_irq(d) % 32);
+	void __iomem *base;
+
+	if (gic_irq_in_rdist(d))
+		base = gic_data_rdist_sgi_base();
+	else
+		base = gic_data.dist_base;
+
+	return !!(readl_relaxed(base + offset + (gic_irq(d) / 32) * 4) & mask);
+}
+
 static void gic_poke_irq(struct irq_data *d, u32 offset)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
@@ -221,6 +234,56 @@ static void gic_unmask_irq(struct irq_data *d)
 	gic_poke_irq(d, GICD_ISENABLER);
 }
 
+static void gic_irq_set_irqchip_state(struct irq_data *d, int state, int val)
+{
+	u32 reg;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		reg = val ? GICD_ISPENDR : GICD_ICPENDR;
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		reg = val ? GICD_ISACTIVER : GICD_ICACTIVER;
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		reg = val ? GICD_ICENABLER : GICD_ISENABLER;
+		break;
+
+	default:
+		WARN_ON(1);
+		return;
+	}
+
+	gic_poke_irq(d, reg);
+}
+
+static int gic_irq_get_irqchip_state(struct irq_data *d, int state)
+{
+	int val;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		val = gic_peek_irq(d, GICD_ISPENDR);
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		val = gic_peek_irq(d, GICD_ISACTIVER);
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		val = !gic_peek_irq(d, GICD_ISENABLER);
+		break;
+
+	default:
+		WARN_ON(1);
+		val = 0;
+	}
+
+	return val;
+}
+
 static void gic_eoi_irq(struct irq_data *d)
 {
 	gic_write_eoir(gic_irq(d));
@@ -404,19 +467,6 @@ static void gic_cpu_init(void)
 }
 
 #ifdef CONFIG_SMP
-static int gic_peek_irq(struct irq_data *d, u32 offset)
-{
-	u32 mask = 1 << (gic_irq(d) % 32);
-	void __iomem *base;
-
-	if (gic_irq_in_rdist(d))
-		base = gic_data_rdist_sgi_base();
-	else
-		base = gic_data.dist_base;
-
-	return !!(readl_relaxed(base + offset + (gic_irq(d) / 32) * 4) & mask);
-}
-
 static int gic_secondary_init(struct notifier_block *nfb,
 			      unsigned long action, void *hcpu)
 {
@@ -583,6 +633,8 @@ static struct irq_chip gic_chip = {
 	.irq_eoi		= gic_eoi_irq,
 	.irq_set_type		= gic_set_type,
 	.irq_set_affinity	= gic_set_affinity,
+	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
+	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
 };
 
 static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
-- 
2.1.0

[PATCH 3/3] irqchip: GICv3: Add support for irq_{get,set}_irqchip_state

[Marc Zyngier]

Add the required hooks for the internal state of an interrupt
to be exposed to other subsystems.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3.c | 78 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index aa17ae8..666c14e 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -193,6 +193,19 @@ static void gic_enable_redist(bool enable)
 /*
  * Routines to disable, enable, EOI and route interrupts
  */
+static int gic_peek_irq(struct irq_data *d, u32 offset)
+{
+	u32 mask = 1 << (gic_irq(d) % 32);
+	void __iomem *base;
+
+	if (gic_irq_in_rdist(d))
+		base = gic_data_rdist_sgi_base();
+	else
+		base = gic_data.dist_base;
+
+	return !!(readl_relaxed(base + offset + (gic_irq(d) / 32) * 4) & mask);
+}
+
 static void gic_poke_irq(struct irq_data *d, u32 offset)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
@@ -221,6 +234,56 @@ static void gic_unmask_irq(struct irq_data *d)
 	gic_poke_irq(d, GICD_ISENABLER);
 }
 
+static void gic_irq_set_irqchip_state(struct irq_data *d, int state, int val)
+{
+	u32 reg;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		reg = val ? GICD_ISPENDR : GICD_ICPENDR;
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		reg = val ? GICD_ISACTIVER : GICD_ICACTIVER;
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		reg = val ? GICD_ICENABLER : GICD_ISENABLER;
+		break;
+
+	default:
+		WARN_ON(1);
+		return;
+	}
+
+	gic_poke_irq(d, reg);
+}
+
+static int gic_irq_get_irqchip_state(struct irq_data *d, int state)
+{
+	int val;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		val = gic_peek_irq(d, GICD_ISPENDR);
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		val = gic_peek_irq(d, GICD_ISACTIVER);
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		val = !gic_peek_irq(d, GICD_ISENABLER);
+		break;
+
+	default:
+		WARN_ON(1);
+		val = 0;
+	}
+
+	return val;
+}
+
 static void gic_eoi_irq(struct irq_data *d)
 {
 	gic_write_eoir(gic_irq(d));
@@ -404,19 +467,6 @@ static void gic_cpu_init(void)
 }
 
 #ifdef CONFIG_SMP
-static int gic_peek_irq(struct irq_data *d, u32 offset)
-{
-	u32 mask = 1 << (gic_irq(d) % 32);
-	void __iomem *base;
-
-	if (gic_irq_in_rdist(d))
-		base = gic_data_rdist_sgi_base();
-	else
-		base = gic_data.dist_base;
-
-	return !!(readl_relaxed(base + offset + (gic_irq(d) / 32) * 4) & mask);
-}
-
 static int gic_secondary_init(struct notifier_block *nfb,
 			      unsigned long action, void *hcpu)
 {
@@ -583,6 +633,8 @@ static struct irq_chip gic_chip = {
 	.irq_eoi		= gic_eoi_irq,
 	.irq_set_type		= gic_set_type,
 	.irq_set_affinity	= gic_set_affinity,
+	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
+	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
 };
 
 static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
-- 
2.1.0

[PATCH 3/3] irqchip: GICv3: Add support for irq_{get, set}_irqchip_state

[Marc Zyngier]

Add the required hooks for the internal state of an interrupt
to be exposed to other subsystems.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3.c | 78 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index aa17ae8..666c14e 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -193,6 +193,19 @@ static void gic_enable_redist(bool enable)
 /*
  * Routines to disable, enable, EOI and route interrupts
  */
+static int gic_peek_irq(struct irq_data *d, u32 offset)
+{
+	u32 mask = 1 << (gic_irq(d) % 32);
+	void __iomem *base;
+
+	if (gic_irq_in_rdist(d))
+		base = gic_data_rdist_sgi_base();
+	else
+		base = gic_data.dist_base;
+
+	return !!(readl_relaxed(base + offset + (gic_irq(d) / 32) * 4) & mask);
+}
+
 static void gic_poke_irq(struct irq_data *d, u32 offset)
 {
 	u32 mask = 1 << (gic_irq(d) % 32);
@@ -221,6 +234,56 @@ static void gic_unmask_irq(struct irq_data *d)
 	gic_poke_irq(d, GICD_ISENABLER);
 }
 
+static void gic_irq_set_irqchip_state(struct irq_data *d, int state, int val)
+{
+	u32 reg;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		reg = val ? GICD_ISPENDR : GICD_ICPENDR;
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		reg = val ? GICD_ISACTIVER : GICD_ICACTIVER;
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		reg = val ? GICD_ICENABLER : GICD_ISENABLER;
+		break;
+
+	default:
+		WARN_ON(1);
+		return;
+	}
+
+	gic_poke_irq(d, reg);
+}
+
+static int gic_irq_get_irqchip_state(struct irq_data *d, int state)
+{
+	int val;
+
+	switch (state) {
+	case IRQCHIP_STATE_PENDING:
+		val = gic_peek_irq(d, GICD_ISPENDR);
+		break;
+
+	case IRQCHIP_STATE_ACTIVE:
+		val = gic_peek_irq(d, GICD_ISACTIVER);
+		break;
+
+	case IRQCHIP_STATE_MASKED:
+		val = !gic_peek_irq(d, GICD_ISENABLER);
+		break;
+
+	default:
+		WARN_ON(1);
+		val = 0;
+	}
+
+	return val;
+}
+
 static void gic_eoi_irq(struct irq_data *d)
 {
 	gic_write_eoir(gic_irq(d));
@@ -404,19 +467,6 @@ static void gic_cpu_init(void)
 }
 
 #ifdef CONFIG_SMP
-static int gic_peek_irq(struct irq_data *d, u32 offset)
-{
-	u32 mask = 1 << (gic_irq(d) % 32);
-	void __iomem *base;
-
-	if (gic_irq_in_rdist(d))
-		base = gic_data_rdist_sgi_base();
-	else
-		base = gic_data.dist_base;
-
-	return !!(readl_relaxed(base + offset + (gic_irq(d) / 32) * 4) & mask);
-}
-
 static int gic_secondary_init(struct notifier_block *nfb,
 			      unsigned long action, void *hcpu)
 {
@@ -583,6 +633,8 @@ static struct irq_chip gic_chip = {
 	.irq_eoi		= gic_eoi_irq,
 	.irq_set_type		= gic_set_type,
 	.irq_set_affinity	= gic_set_affinity,
+	.irq_get_irqchip_state	= gic_irq_get_irqchip_state,
+	.irq_set_irqchip_state	= gic_irq_set_irqchip_state,
 };
 
 static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
-- 
2.1.0

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Thomas Gleixner]

On Sat, 25 Oct 2014, Marc Zyngier wrote:
> +int irq_get_irqchip_state(unsigned int irq, int state)
> +{
> +	struct irq_desc *desc;
> +	struct irq_data *data;
> +	struct irq_chip *chip;
> +	int val;
> +
> +	desc = irq_to_desc(irq);
> +	if (!desc)
> +		return -EINVAL;
> +
> +	data = irq_desc_get_irq_data(desc);
> +
> +	chip = irq_desc_get_chip(desc);
> +	if (!chip->irq_get_irqchip_state)
> +		return -EINVAL;
> +
> +	chip_bus_lock(desc);
> +	val = chip->irq_get_irqchip_state(data, state);

Hmm. What's the irq_get_irqchip_state() callback supposed to return?

Thanks,

	tglx

[PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Thomas Gleixner]

On Sat, 25 Oct 2014, Marc Zyngier wrote:
> +int irq_get_irqchip_state(unsigned int irq, int state)
> +{
> +	struct irq_desc *desc;
> +	struct irq_data *data;
> +	struct irq_chip *chip;
> +	int val;
> +
> +	desc = irq_to_desc(irq);
> +	if (!desc)
> +		return -EINVAL;
> +
> +	data = irq_desc_get_irq_data(desc);
> +
> +	chip = irq_desc_get_chip(desc);
> +	if (!chip->irq_get_irqchip_state)
> +		return -EINVAL;
> +
> +	chip_bus_lock(desc);
> +	val = chip->irq_get_irqchip_state(data, state);

Hmm. What's the irq_get_irqchip_state() callback supposed to return?

Thanks,

	tglx

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Thomas Gleixner]

On Sat, 25 Oct 2014, Thomas Gleixner wrote:

Bah, hit send way too fast :)

> On Sat, 25 Oct 2014, Marc Zyngier wrote:
> > +int irq_get_irqchip_state(unsigned int irq, int state)

get_state(state) does not make sense. get_state(which) probably more
so. And 'which' wants to be an enum btw.

> > +	chip_bus_lock(desc);
> > +	val = chip->irq_get_irqchip_state(data, state);
> 
> Hmm. What's the irq_get_irqchip_state() callback supposed to return?

Either an error code or a boolean value, right? Does not mix very well
I think. 

int irq_get_irqchip_state(unsigned int irq, enum xxx which, bool *val)

Might be a more clear interface.

Thanks,

	tglx

[PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Thomas Gleixner]

On Sat, 25 Oct 2014, Thomas Gleixner wrote:

Bah, hit send way too fast :)

> On Sat, 25 Oct 2014, Marc Zyngier wrote:
> > +int irq_get_irqchip_state(unsigned int irq, int state)

get_state(state) does not make sense. get_state(which) probably more
so. And 'which' wants to be an enum btw.

> > +	chip_bus_lock(desc);
> > +	val = chip->irq_get_irqchip_state(data, state);
> 
> Hmm. What's the irq_get_irqchip_state() callback supposed to return?

Either an error code or a boolean value, right? Does not mix very well
I think. 

int irq_get_irqchip_state(unsigned int irq, enum xxx which, bool *val)

Might be a more clear interface.

Thanks,

	tglx

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Marc Zyngier]

On 25/10/14 20:42, Thomas Gleixner wrote:
> On Sat, 25 Oct 2014, Thomas Gleixner wrote:
> 
> Bah, hit send way too fast :)
> 
>> On Sat, 25 Oct 2014, Marc Zyngier wrote:
>>> +int irq_get_irqchip_state(unsigned int irq, int state)
> 
> get_state(state) does not make sense. get_state(which) probably more
> so. And 'which' wants to be an enum btw.
> 
>>> +	chip_bus_lock(desc);
>>> +	val = chip->irq_get_irqchip_state(data, state);
>>
>> Hmm. What's the irq_get_irqchip_state() callback supposed to return?
> 
> Either an error code or a boolean value, right? Does not mix very well
> I think. 
> 
> int irq_get_irqchip_state(unsigned int irq, enum xxx which, bool *val)
> 
> Might be a more clear interface.

Agreed, this makes a lot of sense. Will respin it.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Marc Zyngier]

On 25/10/14 20:42, Thomas Gleixner wrote:
> On Sat, 25 Oct 2014, Thomas Gleixner wrote:
> 
> Bah, hit send way too fast :)
> 
>> On Sat, 25 Oct 2014, Marc Zyngier wrote:
>>> +int irq_get_irqchip_state(unsigned int irq, int state)
> 
> get_state(state) does not make sense. get_state(which) probably more
> so. And 'which' wants to be an enum btw.
> 
>>> +	chip_bus_lock(desc);
>>> +	val = chip->irq_get_irqchip_state(data, state);
>>
>> Hmm. What's the irq_get_irqchip_state() callback supposed to return?
> 
> Either an error code or a boolean value, right? Does not mix very well
> I think. 
> 
> int irq_get_irqchip_state(unsigned int irq, enum xxx which, bool *val)
> 
> Might be a more clear interface.

Agreed, this makes a lot of sense. Will respin it.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Marc Zyngier]

On 25/10/14 20:42, Thomas Gleixner wrote:
> On Sat, 25 Oct 2014, Thomas Gleixner wrote:
> 
> Bah, hit send way too fast :)
> 
>> On Sat, 25 Oct 2014, Marc Zyngier wrote:
>>> +int irq_get_irqchip_state(unsigned int irq, int state)
> 
> get_state(state) does not make sense. get_state(which) probably more
> so. And 'which' wants to be an enum btw.
> 
>>> +	chip_bus_lock(desc);
>>> +	val = chip->irq_get_irqchip_state(data, state);
>>
>> Hmm. What's the irq_get_irqchip_state() callback supposed to return?
> 
> Either an error code or a boolean value, right? Does not mix very well
> I think. 
> 
> int irq_get_irqchip_state(unsigned int irq, enum xxx which, bool *val)
> 
> Might be a more clear interface.

Agreed, this makes a lot of sense. Will respin it.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Linus Walleij]

On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

> There is a number of cases where a kernel subsystem may want to
> introspect the state of an interrupt at the irqchip level:
>
> - When a peripheral is shared between virtual machines, its interrupt
>   state becomes part of the guest's state, and must be switched accordingly.
>   KVM on arm/arm64 requires this for its guest-visible timer
> - Some GPIO controllers seem to require peeking into the interrupt controller
>   they are connected to to report their internal state

I'd like to know exactly what this means, for GPIO. As mentioned in
conversation with Arnd, there is since before the case where a GPIO
irqchip gets its irqs "stolen" by some other hardware that is in the
always-on domain, and I call these "latent irqs".

There is a third usecase here since ages (pre-git) in
arch/arm/mach-integrator/integrator_cp.c:

/*
 * It seems that the card insertion interrupt remains active after
 * we've acknowledged it.  We therefore ignore the interrupt, and
 * rely on reading it from the SIC.  This also means that we must
 * clear the latched interrupt.
 */
static unsigned int mmc_status(struct device *dev)
{
        unsigned int status = readl(__io_address(0xca000000 + 4));
        writel(8, intcp_con_base + 8);

        return status & 8;
}

static struct mmci_platform_data mmc_data = {
        .ocr_mask       = MMC_VDD_32_33|MMC_VDD_33_34,
        .status         = mmc_status,
        .gpio_wp        = -1,
        .gpio_cd        = -1,
};

This just goes in and peeks around in the Integrator SIC, this
patch would solve also this I think. Or are the added calls good
for clearing the latched IRQ too?

Yours,
Linus Walleij

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Linus Walleij]

On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

> There is a number of cases where a kernel subsystem may want to
> introspect the state of an interrupt at the irqchip level:
>
> - When a peripheral is shared between virtual machines, its interrupt
>   state becomes part of the guest's state, and must be switched accordingly.
>   KVM on arm/arm64 requires this for its guest-visible timer
> - Some GPIO controllers seem to require peeking into the interrupt controller
>   they are connected to to report their internal state

I'd like to know exactly what this means, for GPIO. As mentioned in
conversation with Arnd, there is since before the case where a GPIO
irqchip gets its irqs "stolen" by some other hardware that is in the
always-on domain, and I call these "latent irqs".

There is a third usecase here since ages (pre-git) in
arch/arm/mach-integrator/integrator_cp.c:

/*
 * It seems that the card insertion interrupt remains active after
 * we've acknowledged it.  We therefore ignore the interrupt, and
 * rely on reading it from the SIC.  This also means that we must
 * clear the latched interrupt.
 */
static unsigned int mmc_status(struct device *dev)
{
        unsigned int status = readl(__io_address(0xca000000 + 4));
        writel(8, intcp_con_base + 8);

        return status & 8;
}

static struct mmci_platform_data mmc_data = {
        .ocr_mask       = MMC_VDD_32_33|MMC_VDD_33_34,
        .status         = mmc_status,
        .gpio_wp        = -1,
        .gpio_cd        = -1,
};

This just goes in and peeks around in the Integrator SIC, this
patch would solve also this I think. Or are the added calls good
for clearing the latched IRQ too?

Yours,
Linus Walleij

[PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Linus Walleij]

On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

> There is a number of cases where a kernel subsystem may want to
> introspect the state of an interrupt at the irqchip level:
>
> - When a peripheral is shared between virtual machines, its interrupt
>   state becomes part of the guest's state, and must be switched accordingly.
>   KVM on arm/arm64 requires this for its guest-visible timer
> - Some GPIO controllers seem to require peeking into the interrupt controller
>   they are connected to to report their internal state

I'd like to know exactly what this means, for GPIO. As mentioned in
conversation with Arnd, there is since before the case where a GPIO
irqchip gets its irqs "stolen" by some other hardware that is in the
always-on domain, and I call these "latent irqs".

There is a third usecase here since ages (pre-git) in
arch/arm/mach-integrator/integrator_cp.c:

/*
 * It seems that the card insertion interrupt remains active after
 * we've acknowledged it.  We therefore ignore the interrupt, and
 * rely on reading it from the SIC.  This also means that we must
 * clear the latched interrupt.
 */
static unsigned int mmc_status(struct device *dev)
{
        unsigned int status = readl(__io_address(0xca000000 + 4));
        writel(8, intcp_con_base + 8);

        return status & 8;
}

static struct mmci_platform_data mmc_data = {
        .ocr_mask       = MMC_VDD_32_33|MMC_VDD_33_34,
        .status         = mmc_status,
        .gpio_wp        = -1,
        .gpio_cd        = -1,
};

This just goes in and peeks around in the Integrator SIC, this
patch would solve also this I think. Or are the added calls good
for clearing the latched IRQ too?

Yours,
Linus Walleij

Re: [PATCH 2/3] irqchip: GIC: Add support for irq_{get,set}_irqchip_state

[Linus Walleij]

On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

> Add the required hooks for the internal state of an interrupt
> to be exposed to other subsystems.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

(...)
> +static void gic_poke_irq(struct irq_data *d, u32 offset)
> +static int gic_peek_irq(struct irq_data *d, u32 offset)

Awesome terminology Marc, very CBM BASIC v2 :)

Yours,
Linus Walleij

Re: [PATCH 2/3] irqchip: GIC: Add support for irq_{get,set}_irqchip_state

[Linus Walleij]

On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

> Add the required hooks for the internal state of an interrupt
> to be exposed to other subsystems.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

(...)
> +static void gic_poke_irq(struct irq_data *d, u32 offset)
> +static int gic_peek_irq(struct irq_data *d, u32 offset)

Awesome terminology Marc, very CBM BASIC v2 :)

Yours,
Linus Walleij

[PATCH 2/3] irqchip: GIC: Add support for irq_{get, set}_irqchip_state

[Linus Walleij]

On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:

> Add the required hooks for the internal state of an interrupt
> to be exposed to other subsystems.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

(...)
> +static void gic_poke_irq(struct irq_data *d, u32 offset)
> +static int gic_peek_irq(struct irq_data *d, u32 offset)

Awesome terminology Marc, very CBM BASIC v2 :)

Yours,
Linus Walleij

Re: [PATCH 2/3] irqchip: GIC: Add support for irq_{get,set}_irqchip_state

[Marc Zyngier]

On 29/10/14 10:14, Linus Walleij wrote:
> On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> Add the required hooks for the internal state of an interrupt
>> to be exposed to other subsystems.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> (...)
>> +static void gic_poke_irq(struct irq_data *d, u32 offset)
>> +static int gic_peek_irq(struct irq_data *d, u32 offset)
> 
> Awesome terminology Marc, very CBM BASIC v2 :)

Sorry Linus, I'm strictly an ORIC EXTENDED BASIC V1.0 kind of guy. :-)

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 2/3] irqchip: GIC: Add support for irq_{get,set}_irqchip_state

[Marc Zyngier]

On 29/10/14 10:14, Linus Walleij wrote:
> On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> Add the required hooks for the internal state of an interrupt
>> to be exposed to other subsystems.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> (...)
>> +static void gic_poke_irq(struct irq_data *d, u32 offset)
>> +static int gic_peek_irq(struct irq_data *d, u32 offset)
> 
> Awesome terminology Marc, very CBM BASIC v2 :)

Sorry Linus, I'm strictly an ORIC EXTENDED BASIC V1.0 kind of guy. :-)

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 2/3] irqchip: GIC: Add support for irq_{get, set}_irqchip_state

[Marc Zyngier]

On 29/10/14 10:14, Linus Walleij wrote:
> On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> Add the required hooks for the internal state of an interrupt
>> to be exposed to other subsystems.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> (...)
>> +static void gic_poke_irq(struct irq_data *d, u32 offset)
>> +static int gic_peek_irq(struct irq_data *d, u32 offset)
> 
> Awesome terminology Marc, very CBM BASIC v2 :)

Sorry Linus, I'm strictly an ORIC EXTENDED BASIC V1.0 kind of guy. :-)

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Marc Zyngier]

On 29/10/14 10:12, Linus Walleij wrote:
> On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> There is a number of cases where a kernel subsystem may want to
>> introspect the state of an interrupt at the irqchip level:
>>
>> - When a peripheral is shared between virtual machines, its interrupt
>>   state becomes part of the guest's state, and must be switched accordingly.
>>   KVM on arm/arm64 requires this for its guest-visible timer
>> - Some GPIO controllers seem to require peeking into the interrupt controller
>>   they are connected to to report their internal state
> 
> I'd like to know exactly what this means, for GPIO. As mentioned in
> conversation with Arnd, there is since before the case where a GPIO
> irqchip gets its irqs "stolen" by some other hardware that is in the
> always-on domain, and I call these "latent irqs".

It looks like a slightly different issue:
http://patchwork.ozlabs.org/patch/397657/

Basically, the GPIO chip cannot report its own state, and has to
introspect the parent irqchip to find out.

> There is a third usecase here since ages (pre-git) in
> arch/arm/mach-integrator/integrator_cp.c:
> 
> /*
>  * It seems that the card insertion interrupt remains active after
>  * we've acknowledged it.  We therefore ignore the interrupt, and
>  * rely on reading it from the SIC.  This also means that we must
>  * clear the latched interrupt.
>  */
> static unsigned int mmc_status(struct device *dev)
> {
>         unsigned int status = readl(__io_address(0xca000000 + 4));
>         writel(8, intcp_con_base + 8);
> 
>         return status & 8;
> }
> 
> static struct mmci_platform_data mmc_data = {
>         .ocr_mask       = MMC_VDD_32_33|MMC_VDD_33_34,
>         .status         = mmc_status,
>         .gpio_wp        = -1,
>         .gpio_cd        = -1,
> };
> 
> This just goes in and peeks around in the Integrator SIC, this
> patch would solve also this I think. Or are the added calls good
> for clearing the latched IRQ too?

Pretty funky. You could also use this to clear the pending bit (assuming
there is one on the CP). I'm amazed at the number of similar hacks that
are coming out of the wood now...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Marc Zyngier]

On 29/10/14 10:12, Linus Walleij wrote:
> On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> There is a number of cases where a kernel subsystem may want to
>> introspect the state of an interrupt at the irqchip level:
>>
>> - When a peripheral is shared between virtual machines, its interrupt
>>   state becomes part of the guest's state, and must be switched accordingly.
>>   KVM on arm/arm64 requires this for its guest-visible timer
>> - Some GPIO controllers seem to require peeking into the interrupt controller
>>   they are connected to to report their internal state
> 
> I'd like to know exactly what this means, for GPIO. As mentioned in
> conversation with Arnd, there is since before the case where a GPIO
> irqchip gets its irqs "stolen" by some other hardware that is in the
> always-on domain, and I call these "latent irqs".

It looks like a slightly different issue:
http://patchwork.ozlabs.org/patch/397657/

Basically, the GPIO chip cannot report its own state, and has to
introspect the parent irqchip to find out.

> There is a third usecase here since ages (pre-git) in
> arch/arm/mach-integrator/integrator_cp.c:
> 
> /*
>  * It seems that the card insertion interrupt remains active after
>  * we've acknowledged it.  We therefore ignore the interrupt, and
>  * rely on reading it from the SIC.  This also means that we must
>  * clear the latched interrupt.
>  */
> static unsigned int mmc_status(struct device *dev)
> {
>         unsigned int status = readl(__io_address(0xca000000 + 4));
>         writel(8, intcp_con_base + 8);
> 
>         return status & 8;
> }
> 
> static struct mmci_platform_data mmc_data = {
>         .ocr_mask       = MMC_VDD_32_33|MMC_VDD_33_34,
>         .status         = mmc_status,
>         .gpio_wp        = -1,
>         .gpio_cd        = -1,
> };
> 
> This just goes in and peeks around in the Integrator SIC, this
> patch would solve also this I think. Or are the added calls good
> for clearing the latched IRQ too?

Pretty funky. You could also use this to clear the pending bit (assuming
there is one on the CP). I'm amazed at the number of similar hacks that
are coming out of the wood now...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Marc Zyngier]

On 29/10/14 10:12, Linus Walleij wrote:
> On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
>> There is a number of cases where a kernel subsystem may want to
>> introspect the state of an interrupt at the irqchip level:
>>
>> - When a peripheral is shared between virtual machines, its interrupt
>>   state becomes part of the guest's state, and must be switched accordingly.
>>   KVM on arm/arm64 requires this for its guest-visible timer
>> - Some GPIO controllers seem to require peeking into the interrupt controller
>>   they are connected to to report their internal state
> 
> I'd like to know exactly what this means, for GPIO. As mentioned in
> conversation with Arnd, there is since before the case where a GPIO
> irqchip gets its irqs "stolen" by some other hardware that is in the
> always-on domain, and I call these "latent irqs".

It looks like a slightly different issue:
http://patchwork.ozlabs.org/patch/397657/

Basically, the GPIO chip cannot report its own state, and has to
introspect the parent irqchip to find out.

> There is a third usecase here since ages (pre-git) in
> arch/arm/mach-integrator/integrator_cp.c:
> 
> /*
>  * It seems that the card insertion interrupt remains active after
>  * we've acknowledged it.  We therefore ignore the interrupt, and
>  * rely on reading it from the SIC.  This also means that we must
>  * clear the latched interrupt.
>  */
> static unsigned int mmc_status(struct device *dev)
> {
>         unsigned int status = readl(__io_address(0xca000000 + 4));
>         writel(8, intcp_con_base + 8);
> 
>         return status & 8;
> }
> 
> static struct mmci_platform_data mmc_data = {
>         .ocr_mask       = MMC_VDD_32_33|MMC_VDD_33_34,
>         .status         = mmc_status,
>         .gpio_wp        = -1,
>         .gpio_cd        = -1,
> };
> 
> This just goes in and peeks around in the Integrator SIC, this
> patch would solve also this I think. Or are the added calls good
> for clearing the latched IRQ too?

Pretty funky. You could also use this to clear the pending bit (assuming
there is one on the CP). I'm amazed at the number of similar hacks that
are coming out of the wood now...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Linus Walleij]

On Wed, Oct 29, 2014 at 12:17 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 29/10/14 10:12, Linus Walleij wrote:
>> On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>
>>> There is a number of cases where a kernel subsystem may want to
>>> introspect the state of an interrupt at the irqchip level:
>>>
>>> - When a peripheral is shared between virtual machines, its interrupt
>>>   state becomes part of the guest's state, and must be switched accordingly.
>>>   KVM on arm/arm64 requires this for its guest-visible timer
>>> - Some GPIO controllers seem to require peeking into the interrupt controller
>>>   they are connected to to report their internal state
>>
>> I'd like to know exactly what this means, for GPIO. As mentioned in
>> conversation with Arnd, there is since before the case where a GPIO
>> irqchip gets its irqs "stolen" by some other hardware that is in the
>> always-on domain, and I call these "latent irqs".
>
> It looks like a slightly different issue:
> http://patchwork.ozlabs.org/patch/397657/
>
> Basically, the GPIO chip cannot report its own state, and has to
> introspect the parent irqchip to find out.

That's incidentally the same problem as in the Integrator/CP,
wowsers. What goes around comes around.

>> This just goes in and peeks around in the Integrator SIC, this
>> patch would solve also this I think. Or are the added calls good
>> for clearing the latched IRQ too?
>
> Pretty funky. You could also use this to clear the pending bit (assuming
> there is one on the CP). I'm amazed at the number of similar hacks that
> are coming out of the wood now...

Yeah we should have fixed this with the Integrator in 2001 or so.
Not too late anyways, let's clean it out now :)

Yours,
Linus Walleij

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Linus Walleij]

On Wed, Oct 29, 2014 at 12:17 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 29/10/14 10:12, Linus Walleij wrote:
>> On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>
>>> There is a number of cases where a kernel subsystem may want to
>>> introspect the state of an interrupt at the irqchip level:
>>>
>>> - When a peripheral is shared between virtual machines, its interrupt
>>>   state becomes part of the guest's state, and must be switched accordingly.
>>>   KVM on arm/arm64 requires this for its guest-visible timer
>>> - Some GPIO controllers seem to require peeking into the interrupt controller
>>>   they are connected to to report their internal state
>>
>> I'd like to know exactly what this means, for GPIO. As mentioned in
>> conversation with Arnd, there is since before the case where a GPIO
>> irqchip gets its irqs "stolen" by some other hardware that is in the
>> always-on domain, and I call these "latent irqs".
>
> It looks like a slightly different issue:
> http://patchwork.ozlabs.org/patch/397657/
>
> Basically, the GPIO chip cannot report its own state, and has to
> introspect the parent irqchip to find out.

That's incidentally the same problem as in the Integrator/CP,
wowsers. What goes around comes around.

>> This just goes in and peeks around in the Integrator SIC, this
>> patch would solve also this I think. Or are the added calls good
>> for clearing the latched IRQ too?
>
> Pretty funky. You could also use this to clear the pending bit (assuming
> there is one on the CP). I'm amazed at the number of similar hacks that
> are coming out of the wood now...

Yeah we should have fixed this with the Integrator in 2001 or so.
Not too late anyways, let's clean it out now :)

Yours,
Linus Walleij

[PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Linus Walleij]

On Wed, Oct 29, 2014 at 12:17 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 29/10/14 10:12, Linus Walleij wrote:
>> On Sat, Oct 25, 2014 at 12:14 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>
>>> There is a number of cases where a kernel subsystem may want to
>>> introspect the state of an interrupt at the irqchip level:
>>>
>>> - When a peripheral is shared between virtual machines, its interrupt
>>>   state becomes part of the guest's state, and must be switched accordingly.
>>>   KVM on arm/arm64 requires this for its guest-visible timer
>>> - Some GPIO controllers seem to require peeking into the interrupt controller
>>>   they are connected to to report their internal state
>>
>> I'd like to know exactly what this means, for GPIO. As mentioned in
>> conversation with Arnd, there is since before the case where a GPIO
>> irqchip gets its irqs "stolen" by some other hardware that is in the
>> always-on domain, and I call these "latent irqs".
>
> It looks like a slightly different issue:
> http://patchwork.ozlabs.org/patch/397657/
>
> Basically, the GPIO chip cannot report its own state, and has to
> introspect the parent irqchip to find out.

That's incidentally the same problem as in the Integrator/CP,
wowsers. What goes around comes around.

>> This just goes in and peeks around in the Integrator SIC, this
>> patch would solve also this I think. Or are the added calls good
>> for clearing the latched IRQ too?
>
> Pretty funky. You could also use this to clear the pending bit (assuming
> there is one on the CP). I'm amazed at the number of similar hacks that
> are coming out of the wood now...

Yeah we should have fixed this with the Integrator in 2001 or so.
Not too late anyways, let's clean it out now :)

Yours,
Linus Walleij

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Bjorn Andersson]

On Mon, Oct 27, 2014 at 4:47 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
[...]
>
> Agreed, this makes a lot of sense. Will respin it.
>

Any update on this? I have a couple of drivers on the way that needs
this (querying level of an interrupt line).

Regards,
Bjorn

Re: [PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Bjorn Andersson]

On Mon, Oct 27, 2014 at 4:47 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
[...]
>
> Agreed, this makes a lot of sense. Will respin it.
>

Any update on this? I have a couple of drivers on the way that needs
this (querying level of an interrupt line).

Regards,
Bjorn

[PATCH 1/3] genirq: Allow the irqchip state of an IRQ to be save/restored

[Bjorn Andersson]

On Mon, Oct 27, 2014 at 4:47 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
[...]
>
> Agreed, this makes a lot of sense. Will respin it.
>

Any update on this? I have a couple of drivers on the way that needs
this (querying level of an interrupt line).

Regards,
Bjorn