[PATCH 0/4] arm64: marvell: add support for the PIC and PMU

Hello,

This small patch series intends to add support for the PMU of the
Cortex-A72 cores found in the Marvell Armada 7K/8K SoCs.

However, the interrupt of the PMU is not directly connected to the
GIC, but goes through a secondary interrupt controller called the
PIC. Therefore, this series starts by adding a small driver for this
interrupt controller, before enabling the PIC and the PMU in the
Device Tree.

Thanks!

Thomas

Thomas Petazzoni (4):
  dt-bindings: interrupt-controller: add DT binding for Marvell 7K/8K
    PIC
  irqchip: irq-mvebu-pic: new driver for Marvell Armada 7K/8K PIC
  arm64: marvell: enable the Marvell PIC driver
  arm64: dts: marvell: describe the PIC and PMU on Armada 7K/8K

 .../interrupt-controller/marvell,armada-8k-pic.txt |  25 +++
 arch/arm64/Kconfig.platforms                       |   1 +
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi      |  14 ++
 drivers/irqchip/Kconfig                            |   3 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-mvebu-pic.c                    | 195 +++++++++++++++++++++
 6 files changed, 239 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,armada-8k-pic.txt
 create mode 100644 drivers/irqchip/irq-mvebu-pic.c

-- 
2.7.4

This commit adds the Device Tree binding description for the PIC
interrupt controller available in the ARM64 Marvell Armada 7K/8K SoCs.

Signed-off-by: Thomas Petazzoni <redacted>
---
 .../interrupt-controller/marvell,armada-8k-pic.txt | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,armada-8k-pic.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,armada-8k-pic.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,armada-8k-pic.txt
new file mode 100644
index 0000000..86a7b4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,armada-8k-pic.txt

@@ -0,0 +1,25 @@
+Marvell Armada 7K/8K PIC Interrupt controller
+---------------------------------------------
+
+This is the Device Tree binding for the PIC, a secondary interrupt
+controller available on the Marvell Armada 7K/8K ARM64 SoCs, and
+typically connected to the GIC as the primary interrupt controller.
+
+Required properties:
+- compatible: should be "marvell,armada-8k-pic"
+- interrupt-controller: identifies the node as an interrupt controller
+- #interrupt-cells: the number of cells to define interrupts on this
+  controller. Should be 1
+- reg: the register area for the PIC interrupt controller
+- interrupts: the interrupt to the primary interrupt controller,
+  typically the GIC
+
+Example:
+
+	pic: interrupt-controller at 3f0100 {
+		compatible = "marvell,armada-8k-pic";
+		reg = <0x3f0100 0x10>;
+		#interrupt-cells = <1>;
+		interrupt-controller;
+		interrupts = <GIC_PPI 15 IRQ_TYPE_LEVEL_HIGH>;
+	};

-- 
2.7.4

This commit makes sure the driver for the Marvell PIC interrupt
controller (used on Marvell Armada 7K/8K) is enabled.

Signed-off-by: Thomas Petazzoni <redacted>
---
 arch/arm64/Kconfig.platforms | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 7ef1d05..e395d03 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms

@@ -74,6 +74,7 @@ config ARCH_MVEBU
 	select ARMADA_AP806_SYSCON
 	select ARMADA_CP110_SYSCON
 	select MVEBU_ODMI
+	select MVEBU_PIC
 	help
 	  This enables support for Marvell EBU familly, including:
 	   - Armada 3700 SoC Family

-- 
2.7.4

This commit adds the necessary Device Tree description for the PIC
interrupt controller and the PMU available in the Marvell Armada 7K and
Armada 8K SoCs.

Signed-off-by: Thomas Petazzoni <redacted>
---
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
index eab1a42..e70b996 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi

@@ -128,6 +128,12 @@
 					     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_EDGE_RISING)>;
 			};
 
+			pmu {
+				compatible = "arm,cortex-a72-pmu";
+				interrupt-parent = <&pic>;
+				interrupts = <17>;
+			};
+
 			odmi: odmi at 300000 {
 				compatible = "marvell,odmi-controller";
 				interrupt-controller;

@@ -140,6 +146,14 @@
 				marvell,spi-base = <128>, <136>, <144>, <152>;
 			};
 
+			pic: interrupt-controller at 3f0100 {
+				compatible = "marvell,armada-8k-pic";
+				reg = <0x3f0100 0x10>;
+				#interrupt-cells = <1>;
+				interrupt-controller;
+				interrupts = <GIC_PPI 15 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
 			xor at 400000 {
 				compatible = "marvell,armada-7k-xor", "marvell,xor-v2";
 				reg = <0x400000 0x1000>,

-- 
2.7.4

The Marvell Armada 7K/8K integrates a secondary interrupt controller
very originally named "PIC". It is connected to the main GIC via a
PPI. Amongst other things, this PIC is used for the ARM PMU.

This commit adds a simple irqchip driver for this interrupt
controller. Since this interrupt controller is not needed early at boot
time, we make the driver a proper platform driver rather than use the
IRQCHIP_DECLARE() mechanism.

Signed-off-by: Thomas Petazzoni <redacted>
---
 drivers/irqchip/Kconfig         |   3 +
 drivers/irqchip/Makefile        |   1 +
 drivers/irqchip/irq-mvebu-pic.c | 195 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 199 insertions(+)
 create mode 100644 drivers/irqchip/irq-mvebu-pic.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index fa33c50..a6f90c5 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig

@@ -246,6 +246,9 @@ config MVEBU_ODMI
 	bool
 	select GENERIC_MSI_IRQ_DOMAIN
 
+config MVEBU_PIC
+	bool
+
 config LS_SCFG_MSI
 	def_bool y if SOC_LS1021A || ARCH_LAYERSCAPE
 	depends on PCI && PCI_MSI

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 38853a1..024a78d 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile

@@ -67,5 +67,6 @@ obj-$(CONFIG_INGENIC_IRQ)		+= irq-ingenic.o
 obj-$(CONFIG_IMX_GPCV2)			+= irq-imx-gpcv2.o
 obj-$(CONFIG_PIC32_EVIC)		+= irq-pic32-evic.o
 obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
+obj-$(CONFIG_MVEBU_PIC)			+= irq-mvebu-pic.o
 obj-$(CONFIG_LS_SCFG_MSI)		+= irq-ls-scfg-msi.o
 obj-$(CONFIG_EZNPS_GIC)			+= irq-eznps.o

diff --git a/drivers/irqchip/irq-mvebu-pic.c b/drivers/irqchip/irq-mvebu-pic.c
new file mode 100644
index 0000000..4a3aa7f
--- /dev/null
+++ b/drivers/irqchip/irq-mvebu-pic.c

@@ -0,0 +1,195 @@
+/*
+ * Copyright (C) 2016 Marvell
+ *
+ * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+
+#define PIC_CAUSE	       0x0
+#define PIC_MASK	       0x4
+
+#define PIC_MAX_IRQS		32
+#define PIC_MAX_IRQ_MASK	((1UL << PIC_MAX_IRQS) - 1)
+
+struct mvebu_pic {
+	void __iomem *base;
+	u32 parent_irq;
+	struct irq_domain *domain;
+	struct irq_chip irq_chip;
+};
+
+static void mvebu_pic_reset(struct mvebu_pic *pic)
+{
+	/* ACK and mask all interrupts */
+	writel(0, pic->base + PIC_MASK);
+	writel(PIC_MAX_IRQ_MASK, pic->base + PIC_CAUSE);
+}
+
+static void mvebu_pic_eoi_irq(struct irq_data *d)
+{
+	struct mvebu_pic *pic = irq_data_get_irq_chip_data(d);
+
+	writel(1 << d->hwirq, pic->base + PIC_CAUSE);
+}
+
+static void mvebu_pic_mask_irq(struct irq_data *d)
+{
+	struct mvebu_pic *pic = irq_data_get_irq_chip_data(d);
+	u32 reg;
+
+	reg =  readl(pic->base + PIC_MASK);
+	reg |= (1 << d->hwirq);
+	writel(reg, pic->base + PIC_MASK);
+}
+
+static void mvebu_pic_unmask_irq(struct irq_data *d)
+{
+	struct mvebu_pic *pic = irq_data_get_irq_chip_data(d);
+	u32 reg;
+
+	reg = readl(pic->base + PIC_MASK);
+	reg &= ~(1 << d->hwirq);
+	writel(reg, pic->base + PIC_MASK);
+}
+
+static int mvebu_pic_irq_map(struct irq_domain *domain, unsigned int virq,
+			     irq_hw_number_t hwirq)
+{
+	struct mvebu_pic *pic = domain->host_data;
+
+	irq_set_percpu_devid(virq);
+	irq_set_chip_data(virq, pic);
+	irq_set_chip_and_handler(virq, &pic->irq_chip,
+				 handle_percpu_devid_irq);
+	irq_set_status_flags(virq, IRQ_LEVEL);
+	irq_set_probe(virq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops mvebu_pic_domain_ops = {
+	.map = mvebu_pic_irq_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static void mvebu_pic_handle_cascade_irq(struct irq_desc *desc)
+{
+	struct mvebu_pic *pic = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned long irqmap, irqn;
+	unsigned int cascade_irq;
+
+	irqmap = readl_relaxed(pic->base + PIC_CAUSE);
+	chained_irq_enter(chip, desc);
+
+	for_each_set_bit(irqn, &irqmap, BITS_PER_LONG) {
+		cascade_irq = irq_find_mapping(pic->domain, irqn);
+		generic_handle_irq(cascade_irq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void mvebu_pic_enable_percpu_irq(void *data)
+{
+	struct mvebu_pic *pic = data;
+
+	mvebu_pic_reset(pic);
+	enable_percpu_irq(pic->parent_irq, IRQ_TYPE_NONE);
+}
+
+static void mvebu_pic_disable_percpu_irq(void *data)
+{
+	struct mvebu_pic *pic = data;
+
+	disable_percpu_irq(pic->parent_irq);
+}
+
+static int mvebu_pic_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct mvebu_pic *pic;
+	struct irq_chip *irq_chip;
+	struct resource *res;
+
+	pic = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_pic), GFP_KERNEL);
+	if (!pic)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pic->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pic->base))
+		return PTR_ERR(pic->base);
+
+	irq_chip = &pic->irq_chip;
+	irq_chip->name = dev_name(&pdev->dev);
+	irq_chip->irq_mask = mvebu_pic_mask_irq;
+	irq_chip->irq_unmask = mvebu_pic_unmask_irq;
+	irq_chip->irq_eoi = mvebu_pic_eoi_irq;
+
+	pic->parent_irq = irq_of_parse_and_map(node, 0);
+	if (pic->parent_irq <= 0) {
+		dev_err(&pdev->dev, "Failed to parse parent interrupt\n");
+		return -EINVAL;
+	}
+
+	pic->domain = irq_domain_add_linear(node, PIC_MAX_IRQS,
+					    &mvebu_pic_domain_ops, pic);
+	if (!pic->domain) {
+		dev_err(&pdev->dev, "Failed to allocate irq domain\n");
+		return -ENOMEM;
+	}
+
+	irq_set_chained_handler(pic->parent_irq, mvebu_pic_handle_cascade_irq);
+	irq_set_handler_data(pic->parent_irq, pic);
+
+	on_each_cpu(mvebu_pic_enable_percpu_irq, pic, 1);
+
+	platform_set_drvdata(pdev, pic);
+
+	return 0;
+}
+
+static int mvebu_pic_remove(struct platform_device *pdev)
+{
+	struct mvebu_pic *pic = platform_get_drvdata(pdev);
+
+	on_each_cpu(mvebu_pic_disable_percpu_irq, pic, 1);
+	irq_domain_remove(pic->domain);
+
+	return 0;
+}
+
+static const struct of_device_id mvebu_pic_of_match[] = {
+	{ .compatible = "marvell,armada-8k-pic", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mvebu_pic_of_match);
+
+static struct platform_driver mvebu_pic_driver = {
+	.probe  = mvebu_pic_probe,
+	.remove = mvebu_pic_remove,
+	.driver = {
+		.name = "mvebu-pic",
+		.of_match_table = mvebu_pic_of_match,
+	},
+};
+module_platform_driver(mvebu_pic_driver);
+
+MODULE_AUTHOR("Thomas Petazzoni <thomas.petazzoni@free-electrons.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mvebu_pic");
+

-- 
2.7.4

Hi Thomas,

On Fri, Aug 05, 2016 at 04:55:19PM +0200, Thomas Petazzoni wrote:

tri-state?  Is there anything else attached to the PIC besides the PMU?

You mention 7k in $subject, should you use that here as the youngest
compatible SoC generation?

+	{},
+};
+MODULE_DEVICE_TABLE(of, mvebu_pic_of_match);
+
+static struct platform_driver mvebu_pic_driver = {
+	.probe  = mvebu_pic_probe,
+	.remove = mvebu_pic_remove,
+	.driver = {
+		.name = "mvebu-pic",
+		.of_match_table = mvebu_pic_of_match,
+	},
+};
+module_platform_driver(mvebu_pic_driver);
+
+MODULE_AUTHOR("Thomas Petazzoni [off-list ref]");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mvebu_pic");

thx,

Jason.

Hello,

Thanks for the quick feedback!

On Fri, 5 Aug 2016 15:31:13 +0000, Jason Cooper wrote:

quoted

+config MVEBU_PIC
+	bool  

tri-state?  Is there anything else attached to the PIC besides the PMU?

tri-state would be fine I believe, it's indeed a secondary interrupt
controller, not essential for booting the platform.

But then I probably need to rework PATCH 3/4 and not have it
unconditionally selected by the platform Kconfig option, right?

Regarding what else is attached to the PIC, I have no idea, I don't
have this information.

quoted

+static const struct of_device_id mvebu_pic_of_match[] = {
+	{ .compatible = "marvell,armada-8k-pic", },  

You mention 7k in $subject, should you use that here as the youngest
compatible SoC generation?

There isn't anything youngest or oldest between 7K and 8K, they both
got released at the same time. They are really the same family of SoCs,
the 7K having only one CP110, the 8K having two of them, which provides
more I/Os.

For several other IPs, we're using armada-8k as the compatible string:

 * marvell,armada8k-pcie
 * marvell,armada-8k-xhci

(Yes you will already notice how consistent we are in the naming of the
compatible string :-/).

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Hi Thomas,

On Fri, Aug 05, 2016 at 05:58:12PM +0200, Thomas Petazzoni wrote:

On Fri, 5 Aug 2016 15:31:13 +0000, Jason Cooper wrote:

quoted

quoted

+config MVEBU_PIC
+	bool  

tri-state?  Is there anything else attached to the PIC besides the PMU?

tri-state would be fine I believe, it's indeed a secondary interrupt
controller, not essential for booting the platform.

But then I probably need to rework PATCH 3/4 and not have it
unconditionally selected by the platform Kconfig option, right?

meh.  I have no preference either way.  It's what works best for your
platform.  I've just seen one or two people on a tear lately regarding
module.h/MODULE_* and being boolean.  I figured I'd address it while I
was here. :-)

Regarding what else is attached to the PIC, I have no idea, I don't
have this information.

Ok, then which ever way you go is fine by me.

quoted

quoted

+static const struct of_device_id mvebu_pic_of_match[] = {
+	{ .compatible = "marvell,armada-8k-pic", },  

You mention 7k in $subject, should you use that here as the youngest
compatible SoC generation?

There isn't anything youngest or oldest between 7K and 8K, they both
got released at the same time. They are really the same family of SoCs,
the 7K having only one CP110, the 8K having two of them, which provides
more I/Os.

For several other IPs, we're using armada-8k as the compatible string:

 * marvell,armada8k-pcie
 * marvell,armada-8k-xhci

Ok, sure.  That was just a nit.  I know human nature, despite logic,
will assume 8k is newer that 7k, like SSLv3 being better than TLS v1.x
because the number is bigger. :-/

Consistency is better at this point.

The rest of it looks fine.

thx,

Jason.

On Fri, Aug 5, 2016 at 7:00 PM, Paul Gortmaker
[off-list ref] wrote:

quoted

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index fa33c50..a6f90c5 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig

@@ -246,6 +246,9 @@ config MVEBU_ODMI
        bool
        select GENERIC_MSI_IRQ_DOMAIN

+config MVEBU_PIC
+       bool

Please switch to a builtin registration call, and remove module.h and
all the MODULE_<xyz> references since this is a bool and not a
tristate Kconfig.

I overlooked that someone else beat me to making the bool/tristate
comment -- oh well, that is a good thing that it is on other people's
minds too now.  Apologies for the duplicated feedback.

Paul.

On Fri, Aug 5, 2016 at 10:55 AM, Thomas Petazzoni
[off-list ref] wrote:

Please switch to a builtin registration call, and remove module.h and
all the MODULE_<xyz> references since this is a bool and not a
tristate Kconfig.

Thanks,
Paul.
--

[...]

+static const struct of_device_id mvebu_pic_of_match[] = {
+       { .compatible = "marvell,armada-8k-pic", },
+       {},
+};
+MODULE_DEVICE_TABLE(of, mvebu_pic_of_match);
+
+static struct platform_driver mvebu_pic_driver = {
+       .probe  = mvebu_pic_probe,
+       .remove = mvebu_pic_remove,
+       .driver = {
+               .name = "mvebu-pic",
+               .of_match_table = mvebu_pic_of_match,
+       },
+};
+module_platform_driver(mvebu_pic_driver);
+
+MODULE_AUTHOR("Thomas Petazzoni [off-list ref]");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mvebu_pic");
+
--
2.7.4

On Fri, Aug 05, 2016 at 04:55:18PM +0200, Thomas Petazzoni wrote:

This commit adds the Device Tree binding description for the PIC
interrupt controller available in the ARM64 Marvell Armada 7K/8K SoCs.

Signed-off-by: Thomas Petazzoni <redacted>
---
 .../interrupt-controller/marvell,armada-8k-pic.txt | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,armada-8k-pic.txt

Acked-by: Rob Herring <robh@kernel.org>

Hi Thomas,

On Fri, Aug 05, 2016 at 04:55:17PM +0200, Thomas Petazzoni wrote:

Hello,

This small patch series intends to add support for the PMU of the
Cortex-A72 cores found in the Marvell Armada 7K/8K SoCs.

However, the interrupt of the PMU is not directly connected to the
GIC, but goes through a secondary interrupt controller called the
PIC. Therefore, this series starts by adding a small driver for this
interrupt controller, before enabling the PIC and the PMU in the
Device Tree.

Thanks!

Thomas

Thomas Petazzoni (4):
  dt-bindings: interrupt-controller: add DT binding for Marvell 7K/8K
    PIC
  irqchip: irq-mvebu-pic: new driver for Marvell Armada 7K/8K PIC
  arm64: marvell: enable the Marvell PIC driver
  arm64: dts: marvell: describe the PIC and PMU on Armada 7K/8K

 .../interrupt-controller/marvell,armada-8k-pic.txt |  25 +++
 arch/arm64/Kconfig.platforms                       |   1 +
 arch/arm64/boot/dts/marvell/armada-ap806.dtsi      |  14 ++
 drivers/irqchip/Kconfig                            |   3 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-mvebu-pic.c                    | 195 +++++++++++++++++++++
 6 files changed, 239 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,armada-8k-pic.txt
 create mode 100644 drivers/irqchip/irq-mvebu-pic.c

Patches 1-3 applied to irqchip/mvebu64.  Patch 1 has Rob's Ack.  I
assume you'll route the dtsi change through a separate branch.

thx,

Jason.

Hello,

On Tue, 16 Aug 2016 14:14:47 +0000, Jason Cooper wrote:

Patches 1-3 applied to irqchip/mvebu64.  Patch 1 has Rob's Ack.

Great, thanks! Did you fix the driver for the minor nit that you
pointed out, or do you want a follow-up to fix that?

I assume you'll route the dtsi change through a separate branch.

Yes, Gregory will take care of the .dtsi change, and pass it through
mvebu and arm-soc.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com