Skip to contentSkip to conversation

irqchip: gic/realview: support more RealView DCC variants

PATCH Superseded Linus Walleij 4 participants 8 messages Feb 18, 2016 – Feb 22, 2016

Subsystems (3)
Device tree bindings irqchip drivers Arm generic interrupt controller drivers

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Linus Walleij

author
#0 lore raw

PATCH 1/3 irqchip: gic/realview: support more RealView DCC variants

In the add-on file for the GIC dealing with the RealView family we currently only handle the PB11MPCore, let's extend this to manage the RealView EB ARM11MPCore as well. The Revision B of the ARM11MPCore core tile is a bit special and needs special handling as it moves a system control register around at random.

Cc Arnd Bergmann <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc devicetree at vger.kernel.org
Signed-off-by Linus Walleij <[email protected]>

---
This can be applied in isolation from the other patches so Marc one you're happy with it, please take it into the IRQchip tree.

There are two compatible strings getting added to the device tree bindings so CC to the DT list. No biggie though, just figures out exactly what ARM custom GIC flavor it is.

---
 .../bindings/interrupt-controller/arm,gic.txt      |  2 ++
 drivers/irqchip/irq-gic-realview.c                 | 35 ++++++++++++++++++----
 2 files changed, 32 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt +2 -0
index 5a1cb4bc3dfe..0c80e6870645 100644
@@ -16,6 +16,8 @@ Main node required properties:
 	"arm,cortex-a15-gic"
 	"arm,cortex-a7-gic"
 	"arm,cortex-a9-gic"
+	"arm,eb11mp-gic"
+	"arm,eb11mp-revb-gic"
 	"arm,gic-400"
 	"arm,pl390"
 	"arm,tc11mp-gic"
drivers/irqchip/irq-gic-realview.c +30 -5
index aa46eb280a7f..224e83d5c056 100644
@@ -10,7 +10,8 @@
 #include <linux/irqchip/arm-gic.h>
 
 #define REALVIEW_SYS_LOCK_OFFSET	0x20
-#define REALVIEW_PB11MP_SYS_PLD_CTRL1	0x74
+#define REALVIEW_SYS_PLD_CTRL1		0x74
+#define REALVIEW_EB_REVB_SYS_PLD_CTRL1	0xD8
 #define VERSATILE_LOCK_VAL		0xA05F
 #define PLD_INTMODE_MASK		BIT(22)|BIT(23)|BIT(24)
 #define PLD_INTMODE_LEGACY		0x0

@@ -18,26 +19,50 @@
 #define PLD_INTMODE_NEW_NO_DCC		BIT(23)
 #define PLD_INTMODE_FIQ_ENABLE		BIT(24)
 
+static const struct of_device_id syscon_pldset_of_match[] = {
+	{
+		.compatible = "arm,realview-eb-syscon",
+	},
+	{
+		.compatible = "arm,realview-pb11mp-syscon",
+	},
+	{},
+};
+
 static int __init
 realview_gic_of_init(struct device_node *node, struct device_node *parent)
 {
 	static struct regmap *map;
+	struct device_node *np;
+	struct pld_setting *pldset;
+	u32 pld1_ctrl = REALVIEW_SYS_PLD_CTRL1;
+
+	np = of_find_matching_node_and_match(NULL, syscon_pldset_of_match,
+					     (void *)&pldset);
+	if (!np)
+		return -ENODEV;
+
+	/* For some reason RealView EB Rev B moved this register */
+	if (of_device_is_compatible(np, "arm,eb11mp-revb-gic"))
+		pld1_ctrl = REALVIEW_EB_REVB_SYS_PLD_CTRL1;
 
 	/* The PB11MPCore GIC needs to be configured in the syscon */
-	map = syscon_regmap_lookup_by_compatible("arm,realview-pb11mp-syscon");
+	map = syscon_node_to_regmap(np);
 	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,
+		regmap_update_bits(map, pld1_ctrl,
 				   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");
+		pr_info("RealView GIC: set up interrupt controller to NEW mode, no DCC\n");
 	} else {
-		pr_err("TC11MP GIC setup: could not find syscon\n");
+		pr_err("RealView GIC setup: could not find syscon\n");
 		return -ENXIO;
 	}
 	return gic_of_init(node, parent);
 }
 IRQCHIP_DECLARE(armtc11mp_gic, "arm,tc11mp-gic", realview_gic_of_init);
+IRQCHIP_DECLARE(armeb11mp_gic, "arm,eb11mp-gic", realview_gic_of_init);
+IRQCHIP_DECLARE(armeb11mp_revb_gic, "arm,eb11mp-revb-gic", realview_gic_of_init);
Signature · 1 line
2.4.3
#1 lore raw

PATCH 1/3 irqchip: gic/realview: support more RealView DCC variants

Quoted 10 lines · On Thursday 18 February 2016 14:46:12 Linus Walleij wrote:

In the add-on file for the GIC dealing with the RealView family we currently only handle the PB11MPCore, let's extend this to manage the RealView EB ARM11MPCore as well. The Revision B of the ARM11MPCore core tile is a bit special and needs special handling as it moves a system control register around at random.

Cc Arnd Bergmann <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc devicetree at vger.kernel.org
Signed-off-by Linus Walleij <[email protected]>
Quoted 8 lines

---
This can be applied in isolation from the other patches so Marc one you're happy with it, please take it into the IRQchip tree.

There are two compatible strings getting added to the device tree bindings so CC to the DT list. No biggie though, just figures out exactly what ARM custom GIC flavor it is.

Looks good to me.

All three patches:

Acked-by Arnd Bergmann <[email protected]>
#2 lore raw

PATCH 1/3 irqchip: gic/realview: support more RealView DCC variants

In reply to Linus Walleij

Hi Linus,

Quoted 5 lines · Linus Walleij

In the add-on file for the GIC dealing with the RealView family we currently only handle the PB11MPCore, let's extend this to manage the RealView EB ARM11MPCore as well. The Revision B of the ARM11MPCore core tile is a bit special and needs special handling as it moves a system control register around at random.

Ah, 11MPCore RevB. That thing. Tried to locate one in the magic cupboard, and failed. Oh well...

Quoted 64 lines
Cc Arnd Bergmann <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc devicetree at vger.kernel.org
Signed-off-by Linus Walleij <[email protected]>

---
This can be applied in isolation from the other patches so Marc one you're happy with it, please take it into the IRQchip tree.

There are two compatible strings getting added to the device tree bindings so CC to the DT list. No biggie though, just figures out exactly what ARM custom GIC flavor it is.

---
 .../bindings/interrupt-controller/arm,gic.txt      |  2 ++
 drivers/irqchip/irq-gic-realview.c                 | 35 ++++++++++++++++++----
 2 files changed, 32 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt +2 -0
index 5a1cb4bc3dfe..0c80e6870645 100644
@@ -16,6 +16,8 @@ Main node required properties:
 	"arm,cortex-a15-gic"
 	"arm,cortex-a7-gic"
 	"arm,cortex-a9-gic"
+	"arm,eb11mp-gic"
+	"arm,eb11mp-revb-gic"
 	"arm,gic-400"
 	"arm,pl390"
 	"arm,tc11mp-gic"
drivers/irqchip/irq-gic-realview.c +14 -1
index aa46eb280a7f..224e83d5c056 100644
@@ -10,7 +10,8 @@
 #include <linux/irqchip/arm-gic.h>
 
 #define REALVIEW_SYS_LOCK_OFFSET	0x20
-#define REALVIEW_PB11MP_SYS_PLD_CTRL1	0x74
+#define REALVIEW_SYS_PLD_CTRL1		0x74
+#define REALVIEW_EB_REVB_SYS_PLD_CTRL1	0xD8
 #define VERSATILE_LOCK_VAL		0xA05F
 #define PLD_INTMODE_MASK		BIT(22)|BIT(23)|BIT(24)
 #define PLD_INTMODE_LEGACY		0x0

@@ -18,26 +19,50 @@
 #define PLD_INTMODE_NEW_NO_DCC		BIT(23)
 #define PLD_INTMODE_FIQ_ENABLE		BIT(24)
 
+static const struct of_device_id syscon_pldset_of_match[] = {
+	{
+		.compatible = "arm,realview-eb-syscon",
+	},
+	{
+		.compatible = "arm,realview-pb11mp-syscon",
+	},
+	{},
+};
+
 static int __init
 realview_gic_of_init(struct device_node *node, struct device_node *parent)
 {
 	static struct regmap *map;
+	struct device_node *np;
+	struct pld_setting *pldset;

drivers/irqchip/irq-gic-realview.c line 47

Quoted 10 lines
drivers/irqchip/irq-gic-realview.c +10 -0
+	u32 pld1_ctrl = REALVIEW_SYS_PLD_CTRL1;
+
+	np = of_find_matching_node_and_match(NULL, syscon_pldset_of_match,
+					     (void *)&pldset);
+	if (!np)
+		return -ENODEV;
+
+	/* For some reason RealView EB Rev B moved this register */
+	if (of_device_is_compatible(np, "arm,eb11mp-revb-gic"))
+		pld1_ctrl = REALVIEW_EB_REVB_SYS_PLD_CTRL1;

Associating the PLD control register with the GIC compatible string is a bit odd, as they are conceptually two different devices. Why not storing the register address as the data field in the syscon_pldset_of_match array? One less check, and you get it for free (sort of).

Quoted 9 lines
drivers/irqchip/irq-gic-realview.c +1 -2
 	/* The PB11MPCore GIC needs to be configured in the syscon */
-	map = syscon_regmap_lookup_by_compatible("arm,realview-pb11mp-syscon");
+	map = syscon_node_to_regmap(np);
 	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,

drivers/irqchip/irq-gic-realview.c line 62

Quoted 10 lines
drivers/irqchip/irq-gic-realview.c +3 -2
+		regmap_update_bits(map, pld1_ctrl,
 				   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");
+		pr_info("RealView GIC: set up interrupt controller to NEW mode, no DCC\n");
 	} else {
-		pr_err("TC11MP GIC setup: could not find syscon\n");
+		pr_err("RealView GIC setup: could not find syscon\n");
 		return -ENXIO;

Is that even reachable now that you check for the syscon early in the function? Also, you are now returning -ENODEV while you were returning -ENXIO before. Which one is the most correct (I have no idea)?

drivers/irqchip/irq-gic-realview.c

Quoted 7 lines
drivers/irqchip/irq-gic-realview.c +2 -0
 	}
 	return gic_of_init(node, parent);
 }
 IRQCHIP_DECLARE(armtc11mp_gic, "arm,tc11mp-gic", realview_gic_of_init);
+IRQCHIP_DECLARE(armeb11mp_gic, "arm,eb11mp-gic", realview_gic_of_init);
+IRQCHIP_DECLARE(armeb11mp_revb_gic, "arm,eb11mp-revb-gic", realview_gic_of_init);

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
#3 lore raw

PATCH 1/3 irqchip: gic/realview: support more RealView DCC variants

Quoted 9 lines · On Thursday 18 February 2016 14:19:48 Marc Zyngier wrote:
Quoted 5 lines · Linus Walleij

In the add-on file for the GIC dealing with the RealView family we currently only handle the PB11MPCore, let's extend this to manage the RealView EB ARM11MPCore as well. The Revision B of the ARM11MPCore core tile is a bit special and needs special handling as it moves a system control register around at random.

Ah, 11MPCore RevB. That thing. Tried to locate one in the magic cupboard, and failed. Oh well...

What surprised me though is that it's enabled by default:

$ grep REVB arch/arm/configs/realview*
arch/arm/configs/realview_defconfig:CONFIG_REALVIEW_EB_ARM11MP_REVB=y
arch/arm/configs/realview-smp_defconfig:CONFIG_REALVIEW_EB_ARM11MP_REVB=y

With that option set, the defconfig kernel should not be able to boot on RevC machines. Do you have a RevC machine that works with the defconfig?

Arnd

#4 lore raw

PATCH 1/3 irqchip: gic/realview: support more RealView DCC variants

Quoted 13 lines · Arnd Bergmann
Quoted 9 lines · On Thursday 18 February 2016 14:19:48 Marc Zyngier wrote:
Quoted 5 lines · Linus Walleij

In the add-on file for the GIC dealing with the RealView family we currently only handle the PB11MPCore, let's extend this to manage the RealView EB ARM11MPCore as well. The Revision B of the ARM11MPCore core tile is a bit special and needs special handling as it moves a system control register around at random.

Ah, 11MPCore RevB. That thing. Tried to locate one in the magic cupboard, and failed. Oh well...

What surprised me though is that it's enabled by default:

Quoted 7 lines
$ grep REVB arch/arm/configs/realview*
arch/arm/configs/realview_defconfig:CONFIG_REALVIEW_EB_ARM11MP_REVB=y
arch/arm/configs/realview-smp_defconfig:CONFIG_REALVIEW_EB_ARM11MP_REVB=y

With that option set, the defconfig kernel should not be able to boot on RevC machines. Do you have a RevC machine that works with the defconfig?

No, it looks like someone helped themselves to *all* of the EB-11MPs - I have a bunch of empty baseboards, and no tile to slap on. I also have a box of unsoldered 11MPs, but that a different story... ;-)

I guess I should plan for another dumpster diving session.

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
author
#5 lore raw

PATCH 1/3 irqchip: gic/realview: support more RealView DCC variants

In reply to Arnd Bergmann
Quoted 8 lines · Arnd Bergmann
Quoted 7 lines · On Thursday 18 February 2016 14:19:48 Marc Zyngier wrote:
Quoted 5 lines · Linus Walleij

In the add-on file for the GIC dealing with the RealView family we currently only handle the PB11MPCore, let's extend this to manage the RealView EB ARM11MPCore as well. The Revision B of the ARM11MPCore core tile is a bit special and needs special handling as it moves a system control register around at random.

Quoted 4 lines
Quoted 2 lines

Ah, 11MPCore RevB. That thing. Tried to locate one in the magic cupboard, and failed. Oh well...

What surprised me though is that it's enabled by default:

Looks like my fault according to git-blame...

I think this happened when making cover-all defconfigs for multiplatform. I think I just turned everything on... not realizing that turning on RevB disabled RevA+RevC.

However since it is the *only* version that works with QEMU it actually makes some kind of weird sense to have it enabled since the QEMU version is probably more common than RevA+RevC...

Yours,
Linus Walleij

#6 lore raw

PATCH 1/3 irqchip: gic/realview: support more RealView DCC variants

Quoted 16 lines · On Thursday 18 February 2016 19:30:27 Linus Walleij wrote:
Quoted 8 lines · Arnd Bergmann
Quoted 7 lines · On Thursday 18 February 2016 14:19:48 Marc Zyngier wrote:
Quoted 5 lines · Linus Walleij

In the add-on file for the GIC dealing with the RealView family we currently only handle the PB11MPCore, let's extend this to manage the RealView EB ARM11MPCore as well. The Revision B of the ARM11MPCore core tile is a bit special and needs special handling as it moves a system control register around at random.

Quoted 4 lines
Quoted 2 lines

Ah, 11MPCore RevB. That thing. Tried to locate one in the magic cupboard, and failed. Oh well...

What surprised me though is that it's enabled by default:

Looks like my fault according to git-blame...

Quoted 8 lines

I think this happened when making cover-all defconfigs for multiplatform. I think I just turned everything on... not realizing that turning on RevB disabled RevA+RevC.

However since it is the *only* version that works with QEMU it actually makes some kind of weird sense to have it enabled since the QEMU version is probably more common than RevA+RevC...

Do we have reason to believe that any RevA exists?

My understanding from reading the source was that RevB was everything there was until support for RevC got added and RevB gained a separate option, or possibly that RevA was the same as RevB but it was never common enough to even get mentioned in the source.

Arnd

maintainer
#7 lore raw

PATCH 1/3 irqchip: gic/realview: support more RealView DCC variants

In reply to Linus Walleij
Quoted 14 lines · Linus Walleij

In the add-on file for the GIC dealing with the RealView family we currently only handle the PB11MPCore, let's extend this to manage the RealView EB ARM11MPCore as well. The Revision B of the ARM11MPCore core tile is a bit special and needs special handling as it moves a system control register around at random.

Cc Arnd Bergmann <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc devicetree at vger.kernel.org
Signed-off-by Linus Walleij <[email protected]>

---
This can be applied in isolation from the other patches so Marc one you're happy with it, please take it into the IRQchip tree.

Quoted 5 lines

There are two compatible strings getting added to the device tree bindings so CC to the DT list. No biggie though, just figures out exactly what ARM custom GIC flavor it is.

---
 .../bindings/interrupt-controller/arm,gic.txt      |  2 ++
Acked-by Rob Herring <[email protected]>
Quoted 2 lines
 drivers/irqchip/irq-gic-realview.c                 | 35 ++++++++++++++++++----
 2 files changed, 32 insertions(+), 5 deletions(-)