From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754967AbbLIOEh (ORCPT ); Wed, 9 Dec 2015 09:04:37 -0500 Received: from foss.arm.com ([217.140.101.70]:49908 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754518AbbLIOEf (ORCPT ); Wed, 9 Dec 2015 09:04:35 -0500 Message-ID: <566834F0.2000008@arm.com> Date: Wed, 09 Dec 2015 14:04:32 +0000 From: Marc Zyngier Organization: ARM Ltd User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.7.0 MIME-Version: 1.0 To: Linus Walleij , Thomas Gleixner , Jason Cooper CC: linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] irqchip/gic: support RealView variant setup References: <1445638553-8825-1-git-send-email-linus.walleij@linaro.org> <1445638553-8825-2-git-send-email-linus.walleij@linaro.org> In-Reply-To: <1445638553-8825-2-git-send-email-linus.walleij@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Cc: Jason Cooper > Cc: Marc Zyngier > Signed-off-by: Linus Walleij > --- > 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 > +#include > +#include > +#include > +#include > + > +#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 > > #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 > + > +/* > + * 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 #include #include - -#include "irq-gic.h" +#include #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 #include "irq-gic-common.h" -#include "irq-gic.h" #ifdef CONFIG_ARM64 #include 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 - -/* - * 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...