arm64: Realtek RTD1295 IRQ mux

Add binding for Realtek RTD1295 IRQ mux.

Signed-off-by Andreas F?rber <[email protected]>

---
v1 -> v2:
* Dropped reference to common interrupt.txt bindings (Rob)

 .../interrupt-controller/realtek,rtd119x-mux.txt   | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/realtek,rtd119x-mux.txt

This irq mux driver is derived from the RTD1295 vendor DT and assumes a linear mapping between intr_en and intr_status registers. Code for RTD119x indicates this may not always be the case (i2c_3).

Based in part on QNAP's arch/arm/mach-rtk119x/rtk_irq_mux.c code.

Signed-off-by Andreas F?rber <[email protected]>

---
v1 -> v2:
* Renamed struct fields to avoid ambiguity (Marc)
* Refactored offset lookup to avoid per-compatible init functions
* Inserted white lines to clarify balanced locking (Marc)
* Dropped forwarding of set_affinity to GIC (Marc)
* Added spinlocks for consistency (Marc)
* Limited initialization quirk to iso mux
* Fixed spinlock initialization (Andrew)

 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-rtd119x-mux.c | 204 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 205 insertions(+)
 create mode 100644 drivers/irqchip/irq-rtd119x-mux.c

drivers/irqchip/Makefile +1 -0

index e88d856cc09c..46202a0b7d96 100644
@@ -78,3 +78,4 @@ obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
 obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o irq-aspeed-i2c-ic.o
 obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
 obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
+obj-$(CONFIG_ARCH_REALTEK)		+= irq-rtd119x-mux.o

drivers/irqchip/irq-rtd119x-mux.c new file +30 -0

new file mode 100644
index 000000000000..65d22e163bef
@@ -0,0 +1,204 @@
+/*
+ * Realtek RTD129x IRQ mux
+ *
+ * Copyright (c) 2017 Andreas F?rber
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+struct rtd119x_irq_mux_info {
+	unsigned intr_status_offset;
+	unsigned intr_en_offset;
+};
+
+struct rtd119x_irq_mux_data {
+	void __iomem *intr_status;
+	void __iomem *intr_en;
+	int irq;
+	struct irq_domain *domain;
+	spinlock_t lock;
+};
+
+static void rtd119x_mux_irq_handle(struct irq_desc *desc)

drivers/irqchip/irq-rtd119x-mux.c new file +4 -0

+
+			spin_unlock(&data->lock);
+		}
+	}

drivers/irqchip/irq-rtd119x-mux.c new file +102 -0

+	intr_en = readl(mux_data->intr_en);
+	intr_en |= BIT(data->hwirq);
+	writel(intr_en, mux_data->intr_en);
+
+	spin_unlock(&mux_data->lock);
+}
+
+static int rtd119x_mux_set_affinity(struct irq_data *d,
+			const struct cpumask *mask_val, bool force)
+{
+	/* Forwarding the affinity to the parent would affect all 32 interrupts. */
+	return -EINVAL;
+}
+
+static struct irq_chip rtd119x_mux_irq_chip = {
+	.name			= "rtd119x-mux",
+	.irq_mask		= rtd119x_mux_mask_irq,
+	.irq_unmask		= rtd119x_mux_unmask_irq,
+	.irq_set_affinity	= rtd119x_mux_set_affinity,
+};
+
+static int rtd119x_mux_irq_domain_map(struct irq_domain *d,
+		unsigned int irq, irq_hw_number_t hw)
+{
+	struct rtd119x_irq_mux_data *data = d->host_data;
+
+	irq_set_chip_and_handler(irq, &rtd119x_mux_irq_chip, handle_level_irq);
+	irq_set_chip_data(irq, data);
+	irq_set_probe(irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops rtd119x_mux_irq_domain_ops = {
+	.xlate	= irq_domain_xlate_onecell,
+	.map	= rtd119x_mux_irq_domain_map,
+};
+
+static const struct rtd119x_irq_mux_info rtd1295_iso_irq_mux_info = {
+	.intr_status_offset	= 0x0,
+	.intr_en_offset		= 0x40,
+};
+
+static const struct rtd119x_irq_mux_info rtd1295_irq_mux_info = {
+	.intr_status_offset	= 0xc,
+	.intr_en_offset		= 0x80,
+};
+
+static const struct of_device_id rtd1295_irq_mux_dt_matches[] = {
+	{
+		.compatible = "realtek,rtd1295-iso-irq-mux",
+		.data = &rtd1295_iso_irq_mux_info,
+	}, {
+		.compatible = "realtek,rtd1295-irq-mux",
+		.data = &rtd1295_irq_mux_info,
+	}, {
+	}
+};
+
+static int __init rtd119x_irq_mux_init(struct device_node *node,
+				       struct device_node *parent)
+{
+	struct rtd119x_irq_mux_data *data;
+	const struct of_device_id *match;
+	const struct rtd119x_irq_mux_info *info;
+	void __iomem *base;
+	u32 val;
+
+	match = of_match_node(rtd1295_irq_mux_dt_matches, node);
+	if (!match)
+		return -EINVAL;
+
+	info = match->data;
+	if (!info)
+		return -EINVAL;
+
+	base = of_iomap(node, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->intr_status = base + info->intr_status_offset;
+	data->intr_en     = base + info->intr_en_offset;
+
+	data->irq = irq_of_parse_and_map(node, 0);
+	if (data->irq <= 0) {
+		kfree(data);
+		return -EINVAL;
+	}
+
+	spin_lock_init(&data->lock);
+
+	data->domain = irq_domain_add_linear(node, 32,
+				&rtd119x_mux_irq_domain_ops, data);
+	if (!data->domain) {
+		kfree(data);
+		return -ENOMEM;
+	}
+

Quoted 72 lines

This irq mux driver is derived from the RTD1295 vendor DT and assumes a linear mapping between intr_en and intr_status registers. Code for RTD119x indicates this may not always be the case (i2c_3).

Based in part on QNAP's arch/arm/mach-rtk119x/rtk_irq_mux.c code.

Signed-off-by Andreas F?rber <[email protected]>

---
v1 -> v2:
* Renamed struct fields to avoid ambiguity (Marc)
* Refactored offset lookup to avoid per-compatible init functions
* Inserted white lines to clarify balanced locking (Marc)
* Dropped forwarding of set_affinity to GIC (Marc)
* Added spinlocks for consistency (Marc)
* Limited initialization quirk to iso mux
* Fixed spinlock initialization (Andrew)

 drivers/irqchip/Makefile          |   1 +
 drivers/irqchip/irq-rtd119x-mux.c | 204 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 205 insertions(+)
 create mode 100644 drivers/irqchip/irq-rtd119x-mux.c

drivers/irqchip/Makefile +1 -0

index e88d856cc09c..46202a0b7d96 100644
@@ -78,3 +78,4 @@ obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o
 obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o irq-aspeed-i2c-ic.o
 obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
 obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
+obj-$(CONFIG_ARCH_REALTEK)		+= irq-rtd119x-mux.o

drivers/irqchip/irq-rtd119x-mux.c new file +35 -0

new file mode 100644
index 000000000000..65d22e163bef
@@ -0,0 +1,204 @@
+/*
+ * Realtek RTD129x IRQ mux
+ *
+ * Copyright (c) 2017 Andreas F?rber
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+struct rtd119x_irq_mux_info {
+	unsigned intr_status_offset;
+	unsigned intr_en_offset;
+};
+
+struct rtd119x_irq_mux_data {
+	void __iomem *intr_status;
+	void __iomem *intr_en;
+	int irq;
+	struct irq_domain *domain;
+	spinlock_t lock;
+};
+
+static void rtd119x_mux_irq_handle(struct irq_desc *desc)
+{
+	struct rtd119x_irq_mux_data *data = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	u32 intr_en, intr_status, status;
+	int ret;

drivers/irqchip/irq-rtd119x-mux.c line 40

Quoted 5 lines

drivers/irqchip/irq-rtd119x-mux.c new file +5 -0

+
+	chained_irq_enter(chip, desc);
+
+	spin_lock(&data->lock);
+	intr_en     = readl(data->intr_en);

I think that all the MMIO accessors in this file can advantageously turned into their _relaxed version (none of them require any barrier).

drivers/irqchip/irq-rtd119x-mux.c line 53

Quoted 13 lines

drivers/irqchip/irq-rtd119x-mux.c new file +13 -0

+	intr_status = readl(data->intr_status);
+	spin_unlock(&data->lock);
+
+	status = intr_status & intr_en;
+	if (status != 0) {
+		unsigned irq = __ffs(status);
+		ret = generic_handle_irq(irq_find_mapping(data->domain, irq));
+		if (ret == 0) {
+			spin_lock(&data->lock);
+
+			intr_status = readl(data->intr_status);
+			intr_status |= BIT(irq - 1);
+			writel(intr_status, data->intr_status);

This sequence feels a bit wrong: It seems to imply that writing to the status register is a way to EOI the interrupt. But what happens to the other bits that you've read? I fear that you are inadvertently signalling an EOI for interrupts that you may not have handled yet.

I'd rather see something like this:

	while (status) {
		irq = __ffs(status) - 1;
		writel_relaxed(BIT(irq), data->intr_status);
		generic_handle_irq(irq_find_mapping(data->domain, irq));
		status &= ~irq;
	}

assuming I've understood how the HW works. No need for additional locking.

Quoted 10 lines

drivers/irqchip/irq-rtd119x-mux.c new file +10 -0

+
+			spin_unlock(&data->lock);
+		}
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void rtd119x_mux_mask_irq(struct irq_data *data)
+{

drivers/irqchip/irq-rtd119x-mux.c line 67

Quoted 4 lines

drivers/irqchip/irq-rtd119x-mux.c new file +4 -0

+	struct rtd119x_irq_mux_data *mux_data = irq_data_get_irq_chip_data(data);
+	u32 intr_status;
+
+	spin_lock(&mux_data->lock);

Bang, you're dead. If you get the chained interrupt firing here on the same CPU, it will take the lock in the above function, and everything will grind to a halt. Use the irqsave version.

drivers/irqchip/irq-rtd119x-mux.c line 71

Quoted 5 lines

drivers/irqchip/irq-rtd119x-mux.c new file +4 -0

+
+	intr_status = readl(mux_data->intr_status);
+	intr_status |= BIT(data->hwirq);
+	writel(intr_status, mux_data->intr_status);

Or maybe I haven't understood how this works at all. Can you please explain? I'd expect masking to be the opposite of unmasking, but that's not the case...

drivers/irqchip/irq-rtd119x-mux.c line 82

Quoted 11 lines

drivers/irqchip/irq-rtd119x-mux.c new file +11 -0

+
+	spin_unlock(&mux_data->lock);
+}
+
+static void rtd119x_mux_unmask_irq(struct irq_data *data)
+{
+	struct rtd119x_irq_mux_data *mux_data = irq_data_get_irq_chip_data(data);
+	u32 intr_en;
+
+	spin_lock(&mux_data->lock);
+

Same here.

Quoted 108 lines

drivers/irqchip/irq-rtd119x-mux.c new file +108 -0

+	intr_en = readl(mux_data->intr_en);
+	intr_en |= BIT(data->hwirq);
+	writel(intr_en, mux_data->intr_en);
+
+	spin_unlock(&mux_data->lock);
+}
+
+static int rtd119x_mux_set_affinity(struct irq_data *d,
+			const struct cpumask *mask_val, bool force)
+{
+	/* Forwarding the affinity to the parent would affect all 32 interrupts. */
+	return -EINVAL;
+}
+
+static struct irq_chip rtd119x_mux_irq_chip = {
+	.name			= "rtd119x-mux",
+	.irq_mask		= rtd119x_mux_mask_irq,
+	.irq_unmask		= rtd119x_mux_unmask_irq,
+	.irq_set_affinity	= rtd119x_mux_set_affinity,
+};
+
+static int rtd119x_mux_irq_domain_map(struct irq_domain *d,
+		unsigned int irq, irq_hw_number_t hw)
+{
+	struct rtd119x_irq_mux_data *data = d->host_data;
+
+	irq_set_chip_and_handler(irq, &rtd119x_mux_irq_chip, handle_level_irq);
+	irq_set_chip_data(irq, data);
+	irq_set_probe(irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops rtd119x_mux_irq_domain_ops = {
+	.xlate	= irq_domain_xlate_onecell,
+	.map	= rtd119x_mux_irq_domain_map,
+};
+
+static const struct rtd119x_irq_mux_info rtd1295_iso_irq_mux_info = {
+	.intr_status_offset	= 0x0,
+	.intr_en_offset		= 0x40,
+};
+
+static const struct rtd119x_irq_mux_info rtd1295_irq_mux_info = {
+	.intr_status_offset	= 0xc,
+	.intr_en_offset		= 0x80,
+};
+
+static const struct of_device_id rtd1295_irq_mux_dt_matches[] = {
+	{
+		.compatible = "realtek,rtd1295-iso-irq-mux",
+		.data = &rtd1295_iso_irq_mux_info,
+	}, {
+		.compatible = "realtek,rtd1295-irq-mux",
+		.data = &rtd1295_irq_mux_info,
+	}, {
+	}
+};
+
+static int __init rtd119x_irq_mux_init(struct device_node *node,
+				       struct device_node *parent)
+{
+	struct rtd119x_irq_mux_data *data;
+	const struct of_device_id *match;
+	const struct rtd119x_irq_mux_info *info;
+	void __iomem *base;
+	u32 val;
+
+	match = of_match_node(rtd1295_irq_mux_dt_matches, node);
+	if (!match)
+		return -EINVAL;
+
+	info = match->data;
+	if (!info)
+		return -EINVAL;
+
+	base = of_iomap(node, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->intr_status = base + info->intr_status_offset;
+	data->intr_en     = base + info->intr_en_offset;
+
+	data->irq = irq_of_parse_and_map(node, 0);
+	if (data->irq <= 0) {
+		kfree(data);
+		return -EINVAL;
+	}
+
+	spin_lock_init(&data->lock);
+
+	data->domain = irq_domain_add_linear(node, 32,
+				&rtd119x_mux_irq_domain_ops, data);
+	if (!data->domain) {
+		kfree(data);
+		return -ENOMEM;
+	}
+
+	if (of_device_is_compatible(node, "realtek,rtd1295-iso-irq-mux")) {
+		const int uart0_irq = 2;
+
+		spin_lock(&data->lock);
+
+		val = readl(data->intr_en);

drivers/irqchip/irq-rtd119x-mux.c line 194

Quoted 4 lines

drivers/irqchip/irq-rtd119x-mux.c new file +4 -0

+		val &= ~BIT(uart0_irq);
+		writel(val, data->intr_en);
+
+		writel(BIT(uart0_irq), data->intr_status);

Same here. Can you please explain what you're trying to do? The locking seems a bit pointless (nobody can request the interrupt yet), and this uart0 needs at least a comment, and maybe a description in the device-tree.

drivers/irqchip/irq-rtd119x-mux.c line 204

Quoted 10 lines

drivers/irqchip/irq-rtd119x-mux.c new file +10 -0

+
+		spin_unlock(&data->lock);
+	}
+
+	irq_set_chained_handler_and_data(data->irq, rtd119x_mux_irq_handle, data);
+
+	return 0;
+}
+IRQCHIP_DECLARE(rtd1295_iso_mux, "realtek,rtd1295-iso-irq-mux", rtd119x_irq_mux_init);
+IRQCHIP_DECLARE(rtd1295_mux, "realtek,rtd1295-irq-mux", rtd119x_irq_mux_init);