Re: [PATCH v4 4/8] irqchip/irq-goldfish-pic: Add Goldfish PIC driver
On 18/08/17 14:08, Aleksandar Markovic wrote:
> From: Miodrag Dinic
>
> Add device driver for a virtual programmable interrupt controller
>
> The virtual PIC is designed as a device tree-based interrupt controller.
>
> The compatible string used by OS for binding the driver is
> "google,goldfish-pic".
>
> Signed-off-by: Miodrag Dinic
> Signed-off-by: Goran Ferenc
> Signed-off-by: Aleksandar Markovic
> ---
> MAINTAINERS| 1 +
> drivers/irqchip/Kconfig| 8 ++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-goldfish-pic.c | 145
> +
> 4 files changed, 155 insertions(+)
> create mode 100644 drivers/irqchip/irq-goldfish-pic.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 013da1d..6426875 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -844,6 +844,7 @@ ANDROID GOLDFISH PIC DRIVER
> M: Miodrag Dinic
> S: Supported
> F:
> Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
> +F: drivers/irqchip/irq-goldfish-pic.c
>
> ANDROID GOLDFISH RTC DRIVER
> M: Miodrag Dinic
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index f1fd5f4..21fab14 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -306,3 +306,11 @@ config QCOM_IRQ_COMBINER
> help
> Say yes here to add support for the IRQ combiner devices embedded
> in Qualcomm Technologies chips.
> +
> +config GOLDFISH_PIC
> + bool "Goldfish programmable interrupt controller"
> + depends on MIPS && (GOLDFISH || COMPILE_TEST)
> + select IRQ_DOMAIN
> + help
> + Say yes here to enable Goldfish interrupt controller driver used
> + for Goldfish based virtual platforms.
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index e88d856..ade04a1 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -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_GOLDFISH_PIC) += irq-goldfish-pic.o
> diff --git a/drivers/irqchip/irq-goldfish-pic.c
> b/drivers/irqchip/irq-goldfish-pic.c
> new file mode 100644
> index 000..948c35e
> --- /dev/null
> +++ b/drivers/irqchip/irq-goldfish-pic.c
> @@ -0,0 +1,145 @@
> +/*
> + * Copyright (C) 2017 Imagination Technologies Ltd. All rights reserved
> + * Author: Miodrag Dinic
> + *
> + * This file implements interrupt controller driver for MIPS Goldfish PIC.
> + *
> + * This program is free software; you can redistribute it and/or
> modify it
> + * under the terms of the GNU General Public License as published by
> the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +/* 0..7 MIPS CPU interrupts */
> +#define GF_CPU_IRQ_PIC (MIPS_CPU_IRQ_BASE + 2)
> +#define GF_CPU_IRQ_COMPARE (MIPS_CPU_IRQ_BASE + 7)
> +
> +#define GF_NR_IRQS 40
> +/* 8..39 Cascaded Goldfish PIC interrupts */
> +#define GF_IRQ_OFFSET8
> +
> +#define GF_PIC_NUMBER0x04
> +#define GF_PIC_DISABLE_ALL 0x08
> +#define GF_PIC_DISABLE 0x0c
> +#define GF_PIC_ENABLE0x10
> +
> +static struct irq_domain *irq_domain;
> +static void __iomem *gf_pic_base;
> +
> +static inline void unmask_goldfish_irq(struct irq_data *d)
> +{
> + writel(d->hwirq - GF_IRQ_OFFSET,
> +gf_pic_base + GF_PIC_ENABLE);
> + irq_enable_hazard();
> +}
> +
> +static inline void mask_goldfish_irq(struct irq_data *d)
> +{
> + writel(d->hwirq - GF_IRQ_OFFSET,
> +gf_pic_base + GF_PIC_DISABLE);
> + irq_disable_hazard();
> +}
> +
> +static struct irq_chip goldfish_irq_controller = {
> + .name = "Goldfish PIC",
> + .irq_ack= mask_goldfish_irq,
I'm slightly puzzled.
> + .irq_mask = mask_goldfish_irq,
> + .irq_mask_ack = mask_goldfish_irq,
What does it mean to have irq_mask_ack implemented as mask?
> + .irq_unmask = unmask_goldfish_irq,
> + .irq_eoi= unmask_goldfish_irq,
Really? Are you joking?
> + .irq_disable= mask_goldfish_irq,
> + .irq_enable = unmask_goldfish_irq,
If enable/disable are the same as mask/unmask, you don't need separate
entry points.
> +};
> +
> +static void goldfish_irq_dispatch(void)
> +{
> + u32 irq;
> + u32 virq;
> +
> + irq = readl(gf_pic_base + GF_PIC_NUMBER);
> + if (irq == 0) {
> + /* Timer interrupt */
> + do_IRQ(GF_CPU_IRQ_COMPARE);
> + return;
> + }
Why isn't this indirected through
[PATCH v4 4/8] irqchip/irq-goldfish-pic: Add Goldfish PIC driver
From: Miodrag Dinic
Add device driver for a virtual programmable interrupt controller
The virtual PIC is designed as a device tree-based interrupt controller.
The compatible string used by OS for binding the driver is
"google,goldfish-pic".
Signed-off-by: Miodrag Dinic
Signed-off-by: Goran Ferenc
Signed-off-by: Aleksandar Markovic
---
MAINTAINERS| 1 +
drivers/irqchip/Kconfig| 8 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-goldfish-pic.c | 145 +
4 files changed, 155 insertions(+)
create mode 100644 drivers/irqchip/irq-goldfish-pic.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 013da1d..6426875 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -844,6 +844,7 @@ ANDROID GOLDFISH PIC DRIVER
M: Miodrag Dinic
S: Supported
F:
Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
+F: drivers/irqchip/irq-goldfish-pic.c
ANDROID GOLDFISH RTC DRIVER
M: Miodrag Dinic
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index f1fd5f4..21fab14 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -306,3 +306,11 @@ config QCOM_IRQ_COMBINER
help
Say yes here to add support for the IRQ combiner devices embedded
in Qualcomm Technologies chips.
+
+config GOLDFISH_PIC
+ bool "Goldfish programmable interrupt controller"
+ depends on MIPS && (GOLDFISH || COMPILE_TEST)
+ select IRQ_DOMAIN
+ help
+ Say yes here to enable Goldfish interrupt controller driver used
+ for Goldfish based virtual platforms.
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e88d856..ade04a1 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -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_GOLDFISH_PIC) += irq-goldfish-pic.o
diff --git a/drivers/irqchip/irq-goldfish-pic.c
b/drivers/irqchip/irq-goldfish-pic.c
new file mode 100644
index 000..948c35e
--- /dev/null
+++ b/drivers/irqchip/irq-goldfish-pic.c
@@ -0,0 +1,145 @@
+/*
+ * Copyright (C) 2017 Imagination Technologies Ltd.All rights reserved
+ * Author: Miodrag Dinic
+ *
+ * This file implements interrupt controller driver for MIPS Goldfish PIC.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+/* 0..7 MIPS CPU interrupts */
+#define GF_CPU_IRQ_PIC (MIPS_CPU_IRQ_BASE + 2)
+#define GF_CPU_IRQ_COMPARE (MIPS_CPU_IRQ_BASE + 7)
+
+#define GF_NR_IRQS 40
+/* 8..39 Cascaded Goldfish PIC interrupts */
+#define GF_IRQ_OFFSET 8
+
+#define GF_PIC_NUMBER 0x04
+#define GF_PIC_DISABLE_ALL 0x08
+#define GF_PIC_DISABLE 0x0c
+#define GF_PIC_ENABLE 0x10
+
+static struct irq_domain *irq_domain;
+static void __iomem *gf_pic_base;
+
+static inline void unmask_goldfish_irq(struct irq_data *d)
+{
+ writel(d->hwirq - GF_IRQ_OFFSET,
+ gf_pic_base + GF_PIC_ENABLE);
+ irq_enable_hazard();
+}
+
+static inline void mask_goldfish_irq(struct irq_data *d)
+{
+ writel(d->hwirq - GF_IRQ_OFFSET,
+ gf_pic_base + GF_PIC_DISABLE);
+ irq_disable_hazard();
+}
+
+static struct irq_chip goldfish_irq_controller = {
+ .name = "Goldfish PIC",
+ .irq_ack= mask_goldfish_irq,
+ .irq_mask = mask_goldfish_irq,
+ .irq_mask_ack = mask_goldfish_irq,
+ .irq_unmask = unmask_goldfish_irq,
+ .irq_eoi= unmask_goldfish_irq,
+ .irq_disable= mask_goldfish_irq,
+ .irq_enable = unmask_goldfish_irq,
+};
+
+static void goldfish_irq_dispatch(void)
+{
+ u32 irq;
+ u32 virq;
+
+ irq = readl(gf_pic_base + GF_PIC_NUMBER);
+ if (irq == 0) {
+ /* Timer interrupt */
+ do_IRQ(GF_CPU_IRQ_COMPARE);
+ return;
+ }
+
+ virq = irq_linear_revmap(irq_domain, irq);
+ virq += GF_IRQ_OFFSET;
+ do_IRQ(virq);
+}
+
+static void goldfish_ip2_irq_dispatch(struct irq_desc *desc)
+{
+ unsigned long pending = read_c0_cause() & read_c0_status() & ST0_IM;
+
+ if (pending & CAUSEF_IP2)
+ goldfish_irq_dispatch();
+ else
+ spurious_interrupt();
+}
+
+static int goldfish_pic_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw)
+{
+ if (c
