[PATCH v3 0/4] Add Actions Semi Owl family sirq support

[PATCH v3 0/4] Add Actions Semi Owl family sirq support

[Parthiban Nallathambi]

This patch series add support for external interrupt controller
in Actions Semi Owl famil of SoC's (S500, S700 and S900). Actions
provides support for external interrupt controller to be connected
with it's SoC's using 3 SIRQ pins.

Each line can be configures independently, i.e 3 independent external
interrupt controller can be connected and managed parallely.

Device tree node is created only for S700 after testing it in Cubieboard7.

Changelog in v3:
- Set default operating frequency to 24MHz
- Falling edge and Low Level interrupts translated to rising edge and high level
- Introduced common function with lock handling for register read and write
- Used direct GIC interrupt number for interrupt local hwirq and finding offset
using DT entry (range) when registers are shared 
- Changed irq_ack to irq_eoi
- Added translation method for irq_domain_ops
- Clearing interrupt pending based on bitmask for edge triggered
- Added pinctrl definition for sirq for cubieboard7. This depends on,
https://lore.kernel.org/patchwork/patch/1012859/

Changelog in v2:
- Added SIRQ as hierarchical chip
        GIC <----> SIRQ <----> External interrupt controller/Child devices
- Device binding updates with vendor prefix
- Register sharing handled globally and common init sequence/data for all
actions SoC family

Thanks,
Parthiban
Saravanan

Parthiban Nallathambi (4):
  dt-bindings: interrupt-controller: Actions external interrupt
    controller
  drivers/irqchip: Add Actions external interrupts support
  arm64: dts: actions: Add sirq node for Actions Semi S700
  arm64: dts: actions: s700-cubieboard7: Enable SIRQ

 .../interrupt-controller/actions,owl-sirq.txt |  57 ++++
 .../boot/dts/actions/s700-cubieboard7.dts     |  19 ++
 arch/arm64/boot/dts/actions/s700.dtsi         |  10 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-owl-sirq.c                | 301 ++++++++++++++++++
 5 files changed, 388 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
 create mode 100644 drivers/irqchip/irq-owl-sirq.c

-- 
2.17.2

[PATCH v3 1/4] dt-bindings: interrupt-controller: Actions external interrupt controller

[Parthiban Nallathambi]

Actions Semi OWL family SoC's provides support for external interrupt
controller to be connected and controlled using SIRQ pins. S500, S700
and S900 provides 3 SIRQ lines and works independently for 3 external
interrupt controllers.

Signed-off-by: Parthiban Nallathambi <pn@denx.de>
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 .../interrupt-controller/actions,owl-sirq.txt | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
new file mode 100644
index 000000000000..b3adc4bddf40
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
@@ -0,0 +1,57 @@
+Actions Semi Owl SoCs SIRQ interrupt controller
+
+S500, S700 and S900 SoC's from Actions provides 3 SPI's from GIC,
+in which external interrupt controller can be connected. 3 SPI's
+45, 46, 47 from GIC are directly exposed as SIRQ. It has
+the following properties:
+
+- inputs three interrupt signal from external interrupt controller
+
+Required properties:
+
+- compatible: should be "actions,owl-sirq"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupt-controller: identifies the node as an interrupt controller
+- #interrupt-cells: specifies the number of cells needed to encode an interrupt
+  source, should be 2.
+- actions,sirq-shared-reg: Applicable for S500 and S700 where SIRQ register
+  details are maintained at same offset/register.
+- actions,sirq-reg-offset: register offset for SIRQ interrupts. When registers are
+  shared, all the three offsets will be same (S500 and S700).
+- actions,ext-irq-range: Identifies external irq number range in different SoCs.
+
+Example for S900:
+
+sirq: interrupt-controller@e01b0000 {
+	compatible = "actions,owl-sirq";
+	reg = <0x0 0xe01b0000 0x0 0x1000>;
+	interrupt-controller;
+	#interrupt-cells = <3>;
+	actions,sirq-offset = <0x200 0x528 0x52c>;
+	actions,ext-irq-range = <13 15>;
+};
+
+Example for S700:
+
+sirq: interrupt-controller@e01b0000 {
+	compatible = "actions,owl-sirq";
+	reg = <0x0 0xe01b0000 0x0 0x1000>;
+	interrupt-controller;
+	#interrupt-cells = <3>;
+	actions,sirq-shared-reg;
+	actions,sirq-reg-offset = <0x200 0x200 0x200>;
+	actions,ext-irq-range = <13 15>;
+};
+
+Example for S500:
+
+sirq: interrupt-controller@b01b0000 {
+	compatible = "actions,owl-sirq";
+	reg = <0x0 0xb01b0000 0x0 0x1000>;
+	interrupt-controller;
+	#interrupt-cells = <3>;
+	actions,sirq-shared-reg;
+	actions,sirq-offset = <0x200 0x200 0x200>;
+	actions,ext-irq-range = <13 15>;
+};
-- 
2.17.2

[PATCH v3 2/4] drivers/irqchip: Add Actions external interrupts support

[Parthiban Nallathambi]

Actions Semi Owl family SoC's S500, S700 and S900 provides support
for 3 external interrupt controllers through SIRQ pins.

Each line can be independently configured as interrupt and triggers
on either of the edges (raising or falling) or either of the levels
(high or low) . Each line can also be masked independently.

Signed-off-by: Parthiban Nallathambi <pn@denx.de>
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 drivers/irqchip/Makefile       |   1 +
 drivers/irqchip/irq-owl-sirq.c | 301 +++++++++++++++++++++++++++++++++
 2 files changed, 302 insertions(+)
 create mode 100644 drivers/irqchip/irq-owl-sirq.c

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 794c13d3ac3d..d8015fc5c1a2 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_ATH79)			+= irq-ath79-misc.o
 obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2835.o
 obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2836.o
 obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
+obj-$(CONFIG_ARCH_ACTIONS)		+= irq-owl-sirq.o
 obj-$(CONFIG_FARADAY_FTINTC010)		+= irq-ftintc010.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_LPC32XX)		+= irq-lpc32xx.o
diff --git a/drivers/irqchip/irq-owl-sirq.c b/drivers/irqchip/irq-owl-sirq.c
new file mode 100644
index 000000000000..cc59e5743cda
--- /dev/null
+++ b/drivers/irqchip/irq-owl-sirq.c
@@ -0,0 +1,301 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Actions Semi Owl SoCs SIRQ interrupt controller driver
+ *
+ * Copyright (C) 2014 Actions Semi Inc.
+ * David Liu <liuwei@actions-semi.com>
+ *
+ * Author: Parthiban Nallathambi <pn@denx.de>
+ * Author: Saravanan Sekar <sravanhome@gmail.com>
+ */
+
+#include <linux/interrupt.h>
+#include <linux/irqchip.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+
+#define INTC_EXTCTL_PENDING		BIT(0)
+#define INTC_EXTCTL_CLK_SEL		BIT(4)
+#define INTC_EXTCTL_EN			BIT(5)
+#define	INTC_EXTCTL_TYPE_MASK		GENMASK(6, 7)
+#define	INTC_EXTCTL_TYPE_HIGH		0
+#define	INTC_EXTCTL_TYPE_LOW		BIT(6)
+#define	INTC_EXTCTL_TYPE_RISING		BIT(7)
+#define	INTC_EXTCTL_TYPE_FALLING	(BIT(6) | BIT(7))
+
+struct owl_sirq_chip_data {
+	void __iomem *base;
+	raw_spinlock_t lock;
+	/*
+	 * Some SoC's share the register for all SIRQ lines, so maintain
+	 * register is shared or not here. This value is from DT.
+	 */
+	bool shared_reg;
+	u32 ext_irq_start;
+	u32 ext_irq_end;
+	u16 offset[3];
+	u8 trigger;
+};
+static struct owl_sirq_chip_data *sirq_data;
+
+static u32 sirq_read_extctl(struct owl_sirq_chip_data *data, u32 index)
+{
+	u32 val;
+
+	val = readl_relaxed(data->base + data->offset[index]);
+	if (data->shared_reg)
+		val = (val >> (2 - index) * 8) & 0xff;
+
+	return val;
+}
+
+static void sirq_write_extctl(struct owl_sirq_chip_data *data,
+				u32 extctl, u32 index)
+{
+	u32 val;
+
+	if (data->shared_reg) {
+		val = readl_relaxed(data->base + data->offset[index]);
+		val &= ~(0xff << (2 - index) * 8);
+		extctl &= 0xff;
+		extctl = (extctl << (2 - index) * 8) | val;
+	}
+
+	writel_relaxed(extctl, data->base + data->offset[index]);
+}
+
+static void sirq_clear_set_extctl(struct owl_sirq_chip_data *d,
+					u32 clear, u32 set, u32 index)
+{
+	unsigned long flags;
+	u32 val;
+
+	raw_spin_lock_irqsave(&d->lock, flags);
+	val = sirq_read_extctl(d, index);
+	val &= ~clear;
+	val |= set;
+	sirq_write_extctl(d, val, index);
+	raw_spin_unlock_irqrestore(&d->lock, flags);
+}
+
+static void owl_sirq_eoi(struct irq_data *data)
+{
+	struct owl_sirq_chip_data *chip_data = data->chip_data;
+	u32 index = data->hwirq - chip_data->ext_irq_start;
+
+	/*
+	 * Software must clear external interrupt pending, when interrupt type
+	 * is edge triggered, so we need per SIRQ based clearing.
+	 */
+	if (chip_data->trigger & (1 << index))
+		sirq_clear_set_extctl(chip_data, 0, INTC_EXTCTL_PENDING, index);
+	irq_chip_eoi_parent(data);
+}
+
+static void owl_sirq_mask(struct irq_data *data)
+{
+	struct owl_sirq_chip_data *chip_data = data->chip_data;
+	u32 index = data->hwirq - chip_data->ext_irq_start;
+
+	sirq_clear_set_extctl(chip_data, INTC_EXTCTL_EN, 0, index);
+	irq_chip_mask_parent(data);
+}
+
+static void owl_sirq_unmask(struct irq_data *data)
+{
+	struct owl_sirq_chip_data *chip_data = data->chip_data;
+	u32 index = data->hwirq - chip_data->ext_irq_start;
+
+	sirq_clear_set_extctl(chip_data, 0, INTC_EXTCTL_EN, index);
+	irq_chip_unmask_parent(data);
+}
+
+/* PAD_PULLCTL needs to be defined in pinctrl */
+static int owl_sirq_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	struct owl_sirq_chip_data *chip_data = data->chip_data;
+	u32 index = data->hwirq - chip_data->ext_irq_start;
+	u32 type;
+
+	switch (flow_type) {
+	case IRQF_TRIGGER_LOW:
+		type = INTC_EXTCTL_TYPE_LOW;
+		chip_data->trigger &= ~(1 << index);
+		flow_type = IRQF_TRIGGER_HIGH;
+		break;
+	case IRQF_TRIGGER_HIGH:
+		type = INTC_EXTCTL_TYPE_HIGH;
+		chip_data->trigger &= ~(1 << index);
+		break;
+	case IRQF_TRIGGER_FALLING:
+		type = INTC_EXTCTL_TYPE_FALLING;
+		chip_data->trigger |= 1 << index;
+		flow_type = IRQF_TRIGGER_RISING;
+		break;
+	case IRQF_TRIGGER_RISING:
+		type = INTC_EXTCTL_TYPE_RISING;
+		chip_data->trigger |= 1 << index;
+		break;
+	default:
+		return  -EINVAL;
+	}
+
+	sirq_clear_set_extctl(chip_data, INTC_EXTCTL_TYPE_MASK, type, index);
+	return irq_chip_set_type_parent(data, flow_type);
+}
+
+static struct irq_chip owl_sirq_chip = {
+	.name		= "owl-sirq",
+	.irq_mask	= owl_sirq_mask,
+	.irq_unmask	= owl_sirq_unmask,
+	.irq_eoi	= owl_sirq_eoi,
+	.irq_set_type	= owl_sirq_set_type,
+	.irq_retrigger	= irq_chip_retrigger_hierarchy,
+};
+
+static int owl_sirq_domain_translate(struct irq_domain *d,
+				       struct irq_fwspec *fwspec,
+				       unsigned long *hwirq,
+				       unsigned int *type)
+{
+	if (is_of_node(fwspec->fwnode)) {
+		if (fwspec->param_count != 3)
+			return -EINVAL;
+
+		/* No PPI should point to this domain */
+		if (fwspec->param[0] != 0)
+			return -EINVAL;
+
+		/* sirq support irq number check */
+		if (fwspec->param[1] < sirq_data->ext_irq_start ||
+		    fwspec->param[1] > sirq_data->ext_irq_end)
+			return -EINVAL;
+
+		*hwirq = fwspec->param[1];
+		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int owl_sirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				   unsigned int nr_irqs, void *arg)
+{
+	int i, ret;
+	unsigned int type;
+	irq_hw_number_t hwirq;
+	struct irq_fwspec *fwspec = arg;
+	struct irq_fwspec gic_fwspec = *fwspec;
+
+	if (fwspec->param_count != 3)
+		return -EINVAL;
+
+	/* sysirq doesn't support PPI */
+	if (fwspec->param[0])
+		return -EINVAL;
+
+	ret = owl_sirq_domain_translate(domain, arg, &hwirq, &type);
+	if (ret)
+		return ret;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+	case IRQ_TYPE_LEVEL_HIGH:
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		type = IRQ_TYPE_EDGE_RISING;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		type = IRQ_TYPE_LEVEL_HIGH;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	for (i = 0; i < nr_irqs; i++)
+		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+					      &owl_sirq_chip,
+					      domain->host_data);
+
+	gic_fwspec.param[2] = type;
+	gic_fwspec.fwnode = domain->parent->fwnode;
+	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &gic_fwspec);
+}
+
+
+static const struct irq_domain_ops sirq_domain_ops = {
+	.translate	= owl_sirq_domain_translate,
+	.alloc		= owl_sirq_domain_alloc,
+	.free		= irq_domain_free_irqs_common,
+};
+
+static int __init owl_sirq_of_init(struct device_node *node,
+					struct device_node *parent)
+{
+	struct irq_domain *domain, *domain_parent;
+	int ret = 0, i, sirq_cnt = 0;
+	struct owl_sirq_chip_data *chip_data;
+
+	chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
+	if (!chip_data)
+		return -ENOMEM;
+
+	sirq_data = chip_data;
+	raw_spin_lock_init(&chip_data->lock);
+	chip_data->base = of_iomap(node, 0);
+	if (!chip_data->base) {
+		pr_err("owl_sirq: unable to map sirq register\n");
+		ret = -ENXIO;
+		goto out_free;
+	}
+
+	ret = of_property_read_u32_index(node, "actions,ext-irq-range", 0,
+					 &chip_data->ext_irq_start);
+	if (ret)
+		goto out_unmap;
+
+	ret = of_property_read_u32_index(node, "actions,ext-irq-range", 1,
+					 &chip_data->ext_irq_end);
+	if (ret)
+		goto out_unmap;
+
+	sirq_cnt = chip_data->ext_irq_end - chip_data->ext_irq_start + 1;
+	chip_data->shared_reg = of_property_read_bool(node,
+						"actions,sirq-shared-reg");
+	for (i = 0; i < sirq_cnt; i++) {
+		u32 value;
+
+		ret = of_property_read_u32_index(node, "actions,sirq-reg-offset",
+						i, &value);
+		if (ret)
+			goto out_unmap;
+
+		chip_data->offset[i] = (u16)value;
+		sirq_clear_set_extctl(chip_data, 0, INTC_EXTCTL_CLK_SEL, i);
+	}
+
+	domain_parent = irq_find_host(parent);
+	if (!domain_parent) {
+		pr_err("owl_sirq: interrupt-parent not found\n");
+		goto out_unmap;
+	}
+
+	domain = irq_domain_add_hierarchy(domain_parent, 0,
+			sirq_cnt, node,
+			&sirq_domain_ops, chip_data);
+	if (!domain) {
+		ret = -ENOMEM;
+		goto out_unmap;
+	}
+
+	return 0;
+
+out_unmap:
+	iounmap(chip_data->base);
+out_free:
+	kfree(chip_data);
+	return ret;
+}
+
+IRQCHIP_DECLARE(owl_sirq, "actions,owl-sirq", owl_sirq_of_init);
-- 
2.17.2

[PATCH v3 3/4] arm64: dts: actions: Add sirq node for Actions Semi S700

[Parthiban Nallathambi]

Add sirq node for Actions Semi S700 SoC with 3 SIRQ pins support,
in which external interrupt controllers can be connected.

Example:
atc260x: atc2603c@65 {
        interrupt-parent = <&sirq>;
        interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
};

Signed-off-by: Parthiban Nallathambi <pn@denx.de>
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
---
 arch/arm64/boot/dts/actions/s700.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/actions/s700.dtsi b/arch/arm64/boot/dts/actions/s700.dtsi
index 192c7b39c8c1..d87602ebd689 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -174,6 +174,16 @@
 			#clock-cells = <1>;
 		};
 
+		sirq: interrupt-controller@e01b0200 {
+			compatible = "actions,owl-sirq";
+			reg = <0x0 0xe01b0000 0x0 0x1000>;
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			actions,sirq-shared-reg;
+			actions,sirq-reg-offset = <0x200 0x200 0x200>;
+			actions,ext-irq-range = <13 15>;
+		};
+
 		sps: power-controller@e01b0100 {
 			compatible = "actions,s700-sps";
 			reg = <0x0 0xe01b0100 0x0 0x100>;
-- 
2.17.2

Re: [PATCH v3 2/4] drivers/irqchip: Add Actions external interrupts support

[Marc Zyngier]

On 26/11/2018 10:03, Parthiban Nallathambi wrote:
> Actions Semi Owl family SoC's S500, S700 and S900 provides support
> for 3 external interrupt controllers through SIRQ pins.
> 
> Each line can be independently configured as interrupt and triggers
> on either of the edges (raising or falling) or either of the levels
> (high or low) . Each line can also be masked independently.
> 
> Signed-off-by: Parthiban Nallathambi <pn@denx.de>
> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
> ---
>  drivers/irqchip/Makefile       |   1 +
>  drivers/irqchip/irq-owl-sirq.c | 301 +++++++++++++++++++++++++++++++++
>  2 files changed, 302 insertions(+)
>  create mode 100644 drivers/irqchip/irq-owl-sirq.c
> 
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 794c13d3ac3d..d8015fc5c1a2 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_ATH79)			+= irq-ath79-misc.o
>  obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2835.o
>  obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2836.o
>  obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
> +obj-$(CONFIG_ARCH_ACTIONS)		+= irq-owl-sirq.o
>  obj-$(CONFIG_FARADAY_FTINTC010)		+= irq-ftintc010.o
>  obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
>  obj-$(CONFIG_ARCH_LPC32XX)		+= irq-lpc32xx.o
> diff --git a/drivers/irqchip/irq-owl-sirq.c b/drivers/irqchip/irq-owl-sirq.c
> new file mode 100644
> index 000000000000..cc59e5743cda
> --- /dev/null
> +++ b/drivers/irqchip/irq-owl-sirq.c
> @@ -0,0 +1,301 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Actions Semi Owl SoCs SIRQ interrupt controller driver
> + *
> + * Copyright (C) 2014 Actions Semi Inc.
> + * David Liu <liuwei@actions-semi.com>
> + *
> + * Author: Parthiban Nallathambi <pn@denx.de>
> + * Author: Saravanan Sekar <sravanhome@gmail.com>
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/irqchip.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +
> +#define INTC_EXTCTL_PENDING		BIT(0)
> +#define INTC_EXTCTL_CLK_SEL		BIT(4)
> +#define INTC_EXTCTL_EN			BIT(5)
> +#define	INTC_EXTCTL_TYPE_MASK		GENMASK(6, 7)

#define GENMASK(h, l) [...]

I sense a problem here.

Thanks,

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

Re: [PATCH v3 1/4] dt-bindings: interrupt-controller: Actions external interrupt controller

[Rob Herring]

On Mon, Nov 26, 2018 at 11:03:53AM +0100, Parthiban Nallathambi wrote:
> Actions Semi OWL family SoC's provides support for external interrupt
> controller to be connected and controlled using SIRQ pins. S500, S700
> and S900 provides 3 SIRQ lines and works independently for 3 external
> interrupt controllers.
> 
> Signed-off-by: Parthiban Nallathambi <pn@denx.de>
> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
> ---
>  .../interrupt-controller/actions,owl-sirq.txt | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
> new file mode 100644
> index 000000000000..b3adc4bddf40
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
> @@ -0,0 +1,57 @@
> +Actions Semi Owl SoCs SIRQ interrupt controller
> +
> +S500, S700 and S900 SoC's from Actions provides 3 SPI's from GIC,

Listing SoCs here means you have to update this line for every new SoC.

> +in which external interrupt controller can be connected. 3 SPI's
> +45, 46, 47 from GIC are directly exposed as SIRQ. It has
> +the following properties:
> +
> +- inputs three interrupt signal from external interrupt controller
> +
> +Required properties:
> +
> +- compatible: should be "actions,owl-sirq"

SoC specific compatibles needed.

> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +- interrupt-controller: identifies the node as an interrupt controller
> +- #interrupt-cells: specifies the number of cells needed to encode an interrupt
> +  source, should be 2.
> +- actions,sirq-shared-reg: Applicable for S500 and S700 where SIRQ register
> +  details are maintained at same offset/register.
> +- actions,sirq-reg-offset: register offset for SIRQ interrupts. When registers are
> +  shared, all the three offsets will be same (S500 and S700).

These properties should be implied by the compatible string.

> +- actions,ext-irq-range: Identifies external irq number range in different SoCs.

Why is this needed? It appears to always be the same.

> +
> +Example for S900:
> +
> +sirq: interrupt-controller@e01b0000 {
> +	compatible = "actions,owl-sirq";
> +	reg = <0x0 0xe01b0000 0x0 0x1000>;
> +	interrupt-controller;
> +	#interrupt-cells = <3>;
> +	actions,sirq-offset = <0x200 0x528 0x52c>;
> +	actions,ext-irq-range = <13 15>;
> +};
> +
> +Example for S700:

Examples are examples, not an enumeration of all possible dts entries. 
So 1 should be sufficient.

> +
> +sirq: interrupt-controller@e01b0000 {
> +	compatible = "actions,owl-sirq";
> +	reg = <0x0 0xe01b0000 0x0 0x1000>;
> +	interrupt-controller;
> +	#interrupt-cells = <3>;
> +	actions,sirq-shared-reg;
> +	actions,sirq-reg-offset = <0x200 0x200 0x200>;
> +	actions,ext-irq-range = <13 15>;
> +};
> +
> +Example for S500:
> +
> +sirq: interrupt-controller@b01b0000 {
> +	compatible = "actions,owl-sirq";
> +	reg = <0x0 0xb01b0000 0x0 0x1000>;
> +	interrupt-controller;
> +	#interrupt-cells = <3>;
> +	actions,sirq-shared-reg;
> +	actions,sirq-offset = <0x200 0x200 0x200>;
> +	actions,ext-irq-range = <13 15>;
> +};
> -- 
> 2.17.2
> 

Re: [PATCH v3 1/4] dt-bindings: interrupt-controller: Actions external interrupt controller

[Parthiban Nallathambi]

Hello Rob,

On 12/8/18 12:29 AM, Rob Herring wrote:
> On Mon, Nov 26, 2018 at 11:03:53AM +0100, Parthiban Nallathambi wrote:
>> Actions Semi OWL family SoC's provides support for external interrupt
>> controller to be connected and controlled using SIRQ pins. S500, S700
>> and S900 provides 3 SIRQ lines and works independently for 3 external
>> interrupt controllers.
>>
>> Signed-off-by: Parthiban Nallathambi <pn@denx.de>
>> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
>> ---
>>  .../interrupt-controller/actions,owl-sirq.txt | 57 +++++++++++++++++++
>>  1 file changed, 57 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
>> new file mode 100644
>> index 000000000000..b3adc4bddf40
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.txt
>> @@ -0,0 +1,57 @@
>> +Actions Semi Owl SoCs SIRQ interrupt controller
>> +
>> +S500, S700 and S900 SoC's from Actions provides 3 SPI's from GIC,
> 
> Listing SoCs here means you have to update this line for every new SoC.

Ok, I will mark it as OWL SoC's here.

> 
>> +in which external interrupt controller can be connected. 3 SPI's
>> +45, 46, 47 from GIC are directly exposed as SIRQ. It has
>> +the following properties:
>> +
>> +- inputs three interrupt signal from external interrupt controller
>> +
>> +Required properties:
>> +
>> +- compatible: should be "actions,owl-sirq"
> 
> SoC specific compatibles needed.

Ok, I will change this into "actions,s700-sirq"

> 
>> +- reg: physical base address of the controller and length of memory mapped
>> +  region.
>> +- interrupt-controller: identifies the node as an interrupt controller
>> +- #interrupt-cells: specifies the number of cells needed to encode an interrupt
>> +  source, should be 2.
>> +- actions,sirq-shared-reg: Applicable for S500 and S700 where SIRQ register
>> +  details are maintained at same offset/register.
>> +- actions,sirq-reg-offset: register offset for SIRQ interrupts. When registers are
>> +  shared, all the three offsets will be same (S500 and S700).
> 
> These properties should be implied by the compatible string.

Agreed for sirq-shared-reg.

But for s900 sirq-reg-offset, the register offset will have different values.
So this shall not be removed.

> 
>> +- actions,ext-irq-range: Identifies external irq number range in different SoCs.
> 
> Why is this needed? It appears to always be the same.

Yes, I agree for all the existing Owl SoC's this remains same.

In the previous version we defined this as constant in the code. But based on Marc's
feedback I understood that this value should come from Device Tree instead on hard
coding in the code.

> 
>> +
>> +Example for S900:
>> +
>> +sirq: interrupt-controller@e01b0000 {
>> +	compatible = "actions,owl-sirq";
>> +	reg = <0x0 0xe01b0000 0x0 0x1000>;
>> +	interrupt-controller;
>> +	#interrupt-cells = <3>;
>> +	actions,sirq-offset = <0x200 0x528 0x52c>;
>> +	actions,ext-irq-range = <13 15>;
>> +};
>> +
>> +Example for S700:
> 
> Examples are examples, not an enumeration of all possible dts entries. 
> So 1 should be sufficient.

Sure, I will maintain only s700 here.

> 
>> +
>> +sirq: interrupt-controller@e01b0000 {
>> +	compatible = "actions,owl-sirq";
>> +	reg = <0x0 0xe01b0000 0x0 0x1000>;
>> +	interrupt-controller;
>> +	#interrupt-cells = <3>;
>> +	actions,sirq-shared-reg;
>> +	actions,sirq-reg-offset = <0x200 0x200 0x200>;
>> +	actions,ext-irq-range = <13 15>;
>> +};
>> +
>> +Example for S500:
>> +
>> +sirq: interrupt-controller@b01b0000 {
>> +	compatible = "actions,owl-sirq";
>> +	reg = <0x0 0xb01b0000 0x0 0x1000>;
>> +	interrupt-controller;
>> +	#interrupt-cells = <3>;
>> +	actions,sirq-shared-reg;
>> +	actions,sirq-offset = <0x200 0x200 0x200>;
>> +	actions,ext-irq-range = <13 15>;
>> +};
>> -- 
>> 2.17.2
>>
> 

-- 
Thanks,
Parthiban N

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-22 Fax: (+49)-8142-66989-80 Email: pn@denx.de