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

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

[Thomas Petazzoni]

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

[PATCH 1/4] dt-bindings: interrupt-controller: add DT binding for Marvell 7K/8K PIC

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 .../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@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

Re: [PATCH 1/4] dt-bindings: interrupt-controller: add DT binding for Marvell 7K/8K PIC

[Rob Herring]

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 <thomas.petazzoni@free-electrons.com>
> ---
>  .../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>

[PATCH 2/4] irqchip: irq-mvebu-pic: new driver for Marvell Armada 7K/8K PIC

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 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

Re: [PATCH 2/4] irqchip: irq-mvebu-pic: new driver for Marvell Armada 7K/8K PIC

[Jason Cooper]

Hi Thomas,

On Fri, Aug 05, 2016 at 04:55:19PM +0200, Thomas Petazzoni wrote:
> 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 <thomas.petazzoni@free-electrons.com>
> ---
>  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

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

> +
>  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", },

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 <thomas.petazzoni@free-electrons.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:mvebu_pic");

thx,

Jason.

Re: [PATCH 2/4] irqchip: irq-mvebu-pic: new driver for Marvell Armada 7K/8K PIC

[Thomas Petazzoni]

Hello,

Thanks for the quick feedback!

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

> > +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.

> > +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

Re: [PATCH 2/4] irqchip: irq-mvebu-pic: new driver for Marvell Armada 7K/8K PIC

[Jason Cooper]

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:
> 
> > > +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.

> > > +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.

Re: [PATCH 2/4] irqchip: irq-mvebu-pic: new driver for Marvell Armada 7K/8K PIC

[Paul Gortmaker]

On Fri, Aug 5, 2016 at 10:55 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> 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 <thomas.petazzoni@free-electrons.com>
> ---
>  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

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.
--

> +
>  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>

[...]

> +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
>

Re: [PATCH 2/4] irqchip: irq-mvebu-pic: new driver for Marvell Armada 7K/8K PIC

[Paul Gortmaker]

On Fri, Aug 5, 2016 at 7:00 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:

>> 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.

[PATCH 3/4] arm64: marvell: enable the Marvell PIC driver

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 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

[PATCH 4/4] arm64: dts: marvell: describe the PIC and PMU on Armada 7K/8K

[Thomas Petazzoni]

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 <thomas.petazzoni@free-electrons.com>
---
 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@300000 {
 				compatible = "marvell,odmi-controller";
 				interrupt-controller;
@@ -140,6 +146,14 @@
 				marvell,spi-base = <128>, <136>, <144>, <152>;
 			};
 
+			pic: interrupt-controller@3f0100 {
+				compatible = "marvell,armada-8k-pic";
+				reg = <0x3f0100 0x10>;
+				#interrupt-cells = <1>;
+				interrupt-controller;
+				interrupts = <GIC_PPI 15 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
 			xor@400000 {
 				compatible = "marvell,armada-7k-xor", "marvell,xor-v2";
 				reg = <0x400000 0x1000>,
-- 
2.7.4

Re: [PATCH 4/4] arm64: dts: marvell: describe the PIC and PMU on Armada 7K/8K

[Gregory CLEMENT]

Hi Thomas,
 
 On ven., août 05 2016, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> 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 <thomas.petazzoni@free-electrons.com>

Applied on mvebu/dt64

Thanks,

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

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

[Jason Cooper]

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.

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

[Thomas Petazzoni]

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