Re: [PATCH 2/3] irqchip/gic: support RealView variant setup

From: Marc Zyngier
Date: Wed Dec 09 2015 - 09:04:40 EST


Hi Linus,

On 23/10/15 23:15, Linus Walleij wrote:
> The ARM RealView PB11MPCore reference design has some special
> bits in a system controller register to set up the GIC in one
> of three modes: legacy, new with DCC, new without DCC. The
> register is also used to enable FIQ.
>
> Since the platform will not boot unless this register is set
> up to "new with DCC" mode, we need a special quirk to be
> compiled-in for the RealView platforms.
>
> If we find the right compatible string on the GIC TestChip,
> we enable this quirk by looking up the system controller and
> enabling the special bits.
>
> We depend on the CONFIG_REALVIEW_DT Kconfig symbol as the old
> boardfile code has the same fix hardcoded, and this is only
> needed for the attempts to modernize the RealView code using
> device tree.
>
> After fixing this, the PB11MPCore boots with device tree
> only.
>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Jason Cooper <jason@xxxxxxxxxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> ChangeLog v1->v2:
> - Put the IRQCHIP_DECLARE() in the add-on irq-gic-realview.c file
> and have it call down to gic_of_init() after its special
> initialization
> - Created irq-gic.h to export functions inside irq-gic.c. Part of
> me wanted to use irq-gic-common.h so as not to proliferate the
> header files, but I felt it was encapsulating the functions in
> irq-gic-common.c so it seemed dirty, better to give irq-gic.c
> its own header file.
> - Broke out this irqchip stuff from the rest of the series so as
> not to stress the irqchip maintainers. It has no dependencies
> on the other patches anyways, and can be merged stand-alone.
> ---
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-gic-realview.c | 43 ++++++++++++++++++++++++++++++++++++++
> drivers/irqchip/irq-gic.c | 3 ++-
> drivers/irqchip/irq-gic.h | 7 +++++++
> 4 files changed, 53 insertions(+), 1 deletion(-)
> create mode 100644 drivers/irqchip/irq-gic-realview.c
> create mode 100644 drivers/irqchip/irq-gic.h
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index bb3048f00e64..7a7d4182777d 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_ARCH_SUNXI) += irq-sun4i.o
> obj-$(CONFIG_ARCH_SUNXI) += irq-sunxi-nmi.o
> obj-$(CONFIG_ARCH_SPEAR3XX) += spear-shirq.o
> obj-$(CONFIG_ARM_GIC) += irq-gic.o irq-gic-common.o
> +obj-$(CONFIG_REALVIEW_DT) += irq-gic-realview.o
> obj-$(CONFIG_ARM_GIC_V2M) += irq-gic-v2m.o
> obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o irq-gic-common.o
> obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o irq-gic-v3-its-pci-msi.o irq-gic-v3-its-platform-msi.o
> diff --git a/drivers/irqchip/irq-gic-realview.c b/drivers/irqchip/irq-gic-realview.c
> new file mode 100644
> index 000000000000..bb5583c07667
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-realview.c
> @@ -0,0 +1,43 @@
> +/*
> + * Special GIC quirks for the ARM RealView
> + * Copyright (C) 2015 Linus Walleij
> + */
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/bitops.h>
> +#include <linux/irqchip.h>
> +
> +#include "irq-gic.h"
> +
> +#define REALVIEW_SYS_LOCK_OFFSET 0x20
> +#define REALVIEW_PB11MP_SYS_PLD_CTRL1 0x74
> +#define VERSATILE_LOCK_VAL 0xA05F
> +#define PLD_INTMODE_MASK BIT(22)|BIT(23)|BIT(24)
> +#define PLD_INTMODE_LEGACY 0x0
> +#define PLD_INTMODE_NEW_DCC BIT(22)
> +#define PLD_INTMODE_NEW_NO_DCC BIT(23)
> +#define PLD_INTMODE_FIQ_ENABLE BIT(24)
> +
> +static int __init
> +realview_gic_of_init(struct device_node *node, struct device_node *parent)
> +{
> + static struct regmap *map;
> +
> + /* The PB11MPCore GIC needs to be configured in the syscon */
> + map = syscon_regmap_lookup_by_compatible("arm,realview-pb11mp-syscon");
> + if (!IS_ERR(map)) {
> + /* new irq mode with no DCC */
> + regmap_write(map, REALVIEW_SYS_LOCK_OFFSET,
> + VERSATILE_LOCK_VAL);
> + regmap_update_bits(map, REALVIEW_PB11MP_SYS_PLD_CTRL1,
> + PLD_INTMODE_NEW_NO_DCC,
> + PLD_INTMODE_MASK);
> + regmap_write(map, REALVIEW_SYS_LOCK_OFFSET, 0x0000);
> + pr_info("TC11MP GIC: set up interrupt controller to NEW mode, no DCC\n");
> + } else {
> + pr_err("TC11MP GIC setup: could not find syscon\n");

At this point, you probably want to error out, because the GIC is
probably not usable.

> + }
> + return gic_of_init(node, parent);
> +}
> +IRQCHIP_DECLARE(armtc11mp_gic, "arm,tc11mp-gic", realview_gic_of_init);
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 982c09c2d791..9ec8cf5137d9 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -50,6 +50,7 @@
> #include <asm/virt.h>
>
> #include "irq-gic-common.h"
> +#include "irq-gic.h"
>
> union gic_base {
> void __iomem *common_base;
> @@ -1141,7 +1142,7 @@ static bool gic_check_eoimode(struct device_node *node, void __iomem **base)
> return true;
> }
>
> -static int __init
> +int __init
> gic_of_init(struct device_node *node, struct device_node *parent)
> {
> void __iomem *cpu_base;
> diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
> new file mode 100644
> index 000000000000..3c45a540c235
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic.h
> @@ -0,0 +1,7 @@
> +#include <linux/of.h>
> +
> +/*
> + * Subdrivers that need some preparatory work can initialize their
> + * chips and call this to register their GICs.
> + */
> +int gic_of_init(struct device_node *node, struct device_node *parent);
>

I'm not exactly fond of yet another include file, and I rather put this
in include/irqchip/arm-gic.h (where this was until I recently removed
it).

How about the following (untested) patch on top of yours:

diff --git a/drivers/irqchip/irq-gic-realview.c b/drivers/irqchip/irq-gic-realview.c
index bb5583c..aa46eb2 100644
--- a/drivers/irqchip/irq-gic-realview.c
+++ b/drivers/irqchip/irq-gic-realview.c
@@ -7,8 +7,7 @@
#include <linux/mfd/syscon.h>
#include <linux/bitops.h>
#include <linux/irqchip.h>
-
-#include "irq-gic.h"
+#include <linux/irqchip/arm-gic.h>

#define REALVIEW_SYS_LOCK_OFFSET 0x20
#define REALVIEW_PB11MP_SYS_PLD_CTRL1 0x74
@@ -37,6 +36,7 @@ realview_gic_of_init(struct device_node *node, struct device_node *parent)
pr_info("TC11MP GIC: set up interrupt controller to NEW mode, no DCC\n");
} else {
pr_err("TC11MP GIC setup: could not find syscon\n");
+ return -ENXIO;
}
return gic_of_init(node, parent);
}
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index aea463e..428f9c1 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -49,7 +49,6 @@
#include <asm/virt.h>

#include "irq-gic-common.h"
-#include "irq-gic.h"

#ifdef CONFIG_ARM64
#include <asm/cpufeature.h>
diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
deleted file mode 100644
index 3c45a54..0000000
--- a/drivers/irqchip/irq-gic.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#include <linux/of.h>
-
-/*
- * Subdrivers that need some preparatory work can initialize their
- * chips and call this to register their GICs.
- */
-int gic_of_init(struct device_node *node, struct device_node *parent);
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index bae69e5..d0a29db 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -103,6 +103,16 @@ struct device_node;
void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
int gic_cpu_if_down(unsigned int gic_nr);

+/*
+ * Subdrivers that need some preparatory work can initialize their
+ * chips and call this to register their GICs.
+ */
+int gic_of_init(struct device_node *node, struct device_node *parent);
+
+/*
+ * Legacy platforms not converted to DT yet must use this to init
+ * their GIC
+ */
void gic_init(unsigned int nr, int start,
void __iomem *dist , void __iomem *cpu);

Thanks,

M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/