2019-07-23 20:06:37

by Mischa Jonker

[permalink] [raw]
Subject: [PATCH 1/2] ARCv2: IDU-intc: Add support for edge-triggered interrupts

This adds support for an optional extra interrupt cell to specify edge
vs level triggered. It is backward compatible with dts files with only
one cell, and will default to level-triggered in such a case.

Signed-off-by: Mischa Jonker <[email protected]>
---
arch/arc/kernel/mcip.c | 56 ++++++++++++++++++++++++++++++++++++++++++++------
include/soc/arc/mcip.h | 11 ++++++++++
2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index 18b493d..fc45564 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -202,8 +202,8 @@ static void idu_set_dest(unsigned int cmn_irq, unsigned int cpu_mask)
__mcip_cmd_data(CMD_IDU_SET_DEST, cmn_irq, cpu_mask);
}

-static void idu_set_mode(unsigned int cmn_irq, unsigned int lvl,
- unsigned int distr)
+static void idu_set_mode(unsigned int cmn_irq, bool set_lvl, unsigned int lvl,
+ bool set_distr, unsigned int distr)
{
union {
unsigned int word;
@@ -212,8 +212,11 @@ static void idu_set_mode(unsigned int cmn_irq, unsigned int lvl,
};
} data;

- data.distr = distr;
- data.lvl = lvl;
+ data.word = __mcip_cmd_read(CMD_IDU_READ_MODE, cmn_irq);
+ if (set_distr)
+ data.distr = distr;
+ if (set_lvl)
+ data.lvl = lvl;
__mcip_cmd_data(CMD_IDU_SET_MODE, cmn_irq, data.word);
}

@@ -240,6 +243,25 @@ static void idu_irq_unmask(struct irq_data *data)
raw_spin_unlock_irqrestore(&mcip_lock, flags);
}

+static void idu_irq_ack(struct irq_data *data)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&mcip_lock, flags);
+ __mcip_cmd(CMD_IDU_ACK_CIRQ, data->hwirq);
+ raw_spin_unlock_irqrestore(&mcip_lock, flags);
+}
+
+static void idu_irq_mask_ack(struct irq_data *data)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&mcip_lock, flags);
+ __mcip_cmd_data(CMD_IDU_SET_MASK, data->hwirq, 1);
+ __mcip_cmd(CMD_IDU_ACK_CIRQ, data->hwirq);
+ raw_spin_unlock_irqrestore(&mcip_lock, flags);
+}
+
static int
idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
bool force)
@@ -263,13 +285,32 @@ idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
else
distribution_mode = IDU_M_DISTRI_RR;

- idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL, distribution_mode);
+ idu_set_mode(data->hwirq, false, 0, true, distribution_mode);

raw_spin_unlock_irqrestore(&mcip_lock, flags);

return IRQ_SET_MASK_OK;
}

+static int idu_irq_set_type(struct irq_data *data, u32 type)
+{
+ unsigned long flags;
+
+ if (type & ~(IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH))
+ return -EINVAL;
+
+ raw_spin_lock_irqsave(&mcip_lock, flags);
+
+ idu_set_mode(data->hwirq, true,
+ type & IRQ_TYPE_EDGE_RISING ? IDU_M_TRIG_EDGE :
+ IDU_M_TRIG_LEVEL,
+ false, 0);
+
+ raw_spin_unlock_irqrestore(&mcip_lock, flags);
+
+ return 0;
+}
+
static void idu_irq_enable(struct irq_data *data)
{
/*
@@ -289,7 +330,10 @@ static struct irq_chip idu_irq_chip = {
.name = "MCIP IDU Intc",
.irq_mask = idu_irq_mask,
.irq_unmask = idu_irq_unmask,
+ .irq_ack = idu_irq_ack,
+ .irq_mask_ack = idu_irq_mask_ack,
.irq_enable = idu_irq_enable,
+ .irq_set_type = idu_irq_set_type,
#ifdef CONFIG_SMP
.irq_set_affinity = idu_irq_set_affinity,
#endif
@@ -317,7 +361,7 @@ static int idu_irq_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t
}

static const struct irq_domain_ops idu_irq_ops = {
- .xlate = irq_domain_xlate_onecell,
+ .xlate = irq_domain_xlate_onetwocell,
.map = idu_irq_map,
};

diff --git a/include/soc/arc/mcip.h b/include/soc/arc/mcip.h
index 50f49e0..d1a93c7 100644
--- a/include/soc/arc/mcip.h
+++ b/include/soc/arc/mcip.h
@@ -46,7 +46,9 @@ struct mcip_cmd {
#define CMD_IDU_ENABLE 0x71
#define CMD_IDU_DISABLE 0x72
#define CMD_IDU_SET_MODE 0x74
+#define CMD_IDU_READ_MODE 0x75
#define CMD_IDU_SET_DEST 0x76
+#define CMD_IDU_ACK_CIRQ 0x79
#define CMD_IDU_SET_MASK 0x7C

#define IDU_M_TRIG_LEVEL 0x0
@@ -119,4 +121,13 @@ static inline void __mcip_cmd_data(unsigned int cmd, unsigned int param,
__mcip_cmd(cmd, param);
}

+/*
+ * Read MCIP register
+ */
+static inline unsigned int __mcip_cmd_read(unsigned int cmd, unsigned int param)
+{
+ __mcip_cmd(cmd, param);
+ return read_aux_reg(ARC_REG_MCIP_READBACK);
+}
+
#endif
--
2.8.3


2019-07-23 20:25:21

by Mischa Jonker

[permalink] [raw]
Subject: [PATCH 2/2] dt-bindings: IDU-intc: Add support for edge-triggered interrupts

This updates the documentation for supporting a optional extra interrupt
cell to specify edge vs level triggered.

Signed-off-by: Mischa Jonker <[email protected]>
---
.../interrupt-controller/snps,archs-idu-intc.txt | 30 ++++++++++++++--------
1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
index 09fc02b..a5c1db9 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
@@ -1,20 +1,30 @@
* ARC-HS Interrupt Distribution Unit

- This optional 2nd level interrupt controller can be used in SMP configurations for
- dynamic IRQ routing, load balancing of common/external IRQs towards core intc.
+ This optional 2nd level interrupt controller can be used in SMP configurations
+ for dynamic IRQ routing, load balancing of common/external IRQs towards core
+ intc.

Properties:

- compatible: "snps,archs-idu-intc"
- interrupt-controller: This is an interrupt controller.
-- #interrupt-cells: Must be <1>.
-
- Value of the cell specifies the "common" IRQ from peripheral to IDU. Number N
- of the particular interrupt line of IDU corresponds to the line N+24 of the
- core interrupt controller.
-
- intc accessed via the special ARC AUX register interface, hence "reg" property
- is not specified.
+- #interrupt-cells: Must be <1> or <2>.
+
+ Value of the first cell specifies the "common" IRQ from peripheral to IDU.
+ Number N of the particular interrupt line of IDU corresponds to the line N+24
+ of the core interrupt controller.
+
+ The (optional) second cell specifies any of the following flags:
+ - bits[3:0] trigger type and level flags
+ 1 = low-to-high edge triggered
+ 2 = NOT SUPPORTED (high-to-low edge triggered)
+ 4 = active high level-sensitive <<< DEFAULT
+ 8 = NOT SUPPORTED (active low level-sensitive)
+ When no second cell is specified, the interrupt is assumed to be level
+ sensitive.
+
+ The interrupt controller is accessed via the special ARC AUX register
+ interface, hence "reg" property is not specified.

Example:
core_intc: core-interrupt-controller {
--
2.8.3

2019-07-24 10:51:22

by Alexey Brodkin

[permalink] [raw]
Subject: RE: [PATCH 1/2] ARCv2: IDU-intc: Add support for edge-triggered interrupts

Hi Mischa,

> -----Original Message-----
> From: Mischa Jonker <[email protected]>
> Sent: Tuesday, July 23, 2019 1:26 PM
> To: Vineet Gupta <[email protected]>; Alexey Brodkin <[email protected]>;
> [email protected]; [email protected]; [email protected]; linux-snps-
> [email protected]; [email protected]; [email protected]
> Cc: Mischa Jonker <[email protected]>
> Subject: [PATCH 1/2] ARCv2: IDU-intc: Add support for edge-triggered interrupts
>
> This adds support for an optional extra interrupt cell to specify edge
> vs level triggered. It is backward compatible with dts files with only
> one cell, and will default to level-triggered in such a case.

In general LGTM. Still a couple of comments.

It might be useful to explain changes
made to idu_irq_set_affinity() as it's not immediately clear what affinity
has to do with IRQ modes (in theory it should be orthogonal).

But what happens we're actually fixing previously implemented short-cut
when instead of a separately set IRQ mode we were doing it together with
setup of distribution since it's done with the same one command
(anyways we relied on the one and only IRQ type being supported).

And now we have a proper implementation with separated setup of IRQ mode and
affinity.

> static int
> idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
> bool force)
> @@ -263,13 +285,32 @@ idu_irq_set_affinity(struct irq_data *data, const struct cpumask *cpumask,
> else
> distribution_mode = IDU_M_DISTRI_RR;
>
> - idu_set_mode(data->hwirq, IDU_M_TRIG_LEVEL, distribution_mode);
> + idu_set_mode(data->hwirq, false, 0, true, distribution_mode);
>
> raw_spin_unlock_irqrestore(&mcip_lock, flags);
>
> return IRQ_SET_MASK_OK;
> }
>
> +static int idu_irq_set_type(struct irq_data *data, u32 type)
> +{
> + unsigned long flags;
> +
> + if (type & ~(IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH))
> + return -EINVAL;

Maybe add an explanation why only these types are supported?

-Alexey

2019-07-24 10:55:02

by Alexey Brodkin

[permalink] [raw]
Subject: RE: [PATCH 2/2] dt-bindings: IDU-intc: Add support for edge-triggered interrupts

Hi Mischa,

> -----Original Message-----
> From: Mischa Jonker <[email protected]>
> Sent: Tuesday, July 23, 2019 1:26 PM
> To: Vineet Gupta <[email protected]>; Alexey Brodkin <[email protected]>;
> [email protected]; [email protected]; [email protected]; linux-snps-
> [email protected]; [email protected]; [email protected]
> Cc: Mischa Jonker <[email protected]>
> Subject: [PATCH 2/2] dt-bindings: IDU-intc: Add support for edge-triggered interrupts
>
> This updates the documentation for supporting a optional extra interrupt
> cell to specify edge vs level triggered.

LGTM as well. But maybe split pure clean-up changes from addition of
the new property description so that info about addition of new property is
clearly seen? Otherwise it gets a bit lost among nice and useful generic fixes.

-Alexey