irqchip: qcom: add support for PDC interrupt controller

From : Archana Sathyakumar <[email protected]>

From : Archana Sathyakumar <[email protected]>

The Power Domain Controller (PDC) hardware block on Qualcomm SoCs houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts.

The interrupt controller is on an always-on domain for the purpose of waking up the processor, but only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processor's domain, bringing the domain out of low power mode and replays the pending interrupts so the GIC may wake up the processor.

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

[Lina: Split out DT bindings target data and initialization changes]

---
 drivers/irqchip/Kconfig    |   9 ++
 drivers/irqchip/Makefile   |   1 +
 drivers/irqchip/qcom-pdc.c | 282 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/irqchip/qcom-pdc.h |  30 +++++
 4 files changed, 322 insertions(+)
 create mode 100644 drivers/irqchip/qcom-pdc.c
 create mode 100644 drivers/irqchip/qcom-pdc.h

drivers/irqchip/Kconfig +9 -0

index c70476b34a53..9d54151157d5 100644
@@ -343,4 +343,13 @@ config MESON_IRQ_GPIO
        help
          Support Meson SoC Family GPIO Interrupt Multiplexer
 
+config QCOM_PDC
+	bool "QCOM PDC"
+	depends on ARCH_QCOM
+	select IRQ_DOMAIN
+	select IRQ_DOMAIN_HIERARCHY
+	help
+	  Power Domain Controller driver to manage and configure wakeup
+	  IRQs for Qualcomm Technologies Inc.'s (QTI) mobile SoCs.
+
 endmenu

drivers/irqchip/Makefile +1 -0

index d2df34a54d38..280723d83916 100644
@@ -84,3 +84,4 @@ obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
 obj-$(CONFIG_IRQ_UNIPHIER_AIDET)	+= irq-uniphier-aidet.o
 obj-$(CONFIG_ARCH_SYNQUACER)		+= irq-sni-exiu.o
 obj-$(CONFIG_MESON_IRQ_GPIO)		+= irq-meson-gpio.o
+obj-$(CONFIG_QCOM_PDC)			+= qcom-pdc.o

drivers/irqchip/qcom-pdc.c new file +44 -0

new file mode 100644
index 000000000000..57f1348bd81c
@@ -0,0 +1,282 @@
+/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/spinlock.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "qcom-pdc.h"
+
+#define PDC_MAX_IRQS			126
+
+#define CLEAR_INTR(reg, intr)		(reg & ~(1 << intr))
+#define ENABLE_INTR(reg, intr)		(reg | (1 << intr))
+
+#define IRQ_ENABLE_BANK			0x10
+#define IRQ_i_CFG			0x110
+
+static DEFINE_SPINLOCK(pdc_lock);
+static void __iomem *pdc_base;
+
+static int get_pdc_pin(irq_hw_number_t hwirq, void *data)
+{

drivers/irqchip/qcom-pdc.c new file +12 -0

+
+static inline void pdc_enable_intr(struct irq_data *d, bool on)
+{
+	int pin_out = get_pdc_pin(d->hwirq, d->chip_data);
+	unsigned int index, mask;
+	u32 enable, r_enable;
+	unsigned long flags;
+
+	if (pin_out < 0)
+		return;
+
+	index = pin_out / 32;

drivers/irqchip/qcom-pdc.c new file +16 -0

+	WARN_ON(r_enable != enable);
+}
+
+static void qcom_pdc_gic_mask(struct irq_data *d)
+{
+	pdc_enable_intr(d, false);
+	irq_chip_mask_parent(d);
+}
+
+static void qcom_pdc_gic_unmask(struct irq_data *d)
+{
+	pdc_enable_intr(d, true);
+	irq_chip_unmask_parent(d);
+}
+
+static void qcom_pdc_gic_enable(struct irq_data *d)

drivers/irqchip/qcom-pdc.c new file +7 -0

+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		pdc_type = RISING_EDGE;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		pdc_type = FALLING_EDGE;

drivers/irqchip/qcom-pdc.c new file +4 -0

+
+	/*
+	 * If type is edge triggered, forward that as Rising edge as PDC
+	 * takes care of converting falling edge to rising edge signal

drivers/irqchip/qcom-pdc.c new file +17 -0

+
+fwd_to_parent:
+	return irq_chip_set_type_parent(d, type);
+}
+
+static struct irq_chip qcom_pdc_gic_chip = {
+	.name			= "pdc-gic",
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_mask		= qcom_pdc_gic_mask,
+	.irq_enable		= qcom_pdc_gic_enable,
+	.irq_unmask		= qcom_pdc_gic_unmask,
+	.irq_disable		= qcom_pdc_gic_disable,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_type		= qcom_pdc_gic_set_type,
+	.flags			= IRQCHIP_MASK_ON_SUSPEND |
+				  IRQCHIP_SET_TYPE_MASKED |
+				  IRQCHIP_SKIP_SET_WAKE,

drivers/irqchip/qcom-pdc.c new file +9 -0

+}
+
+static int qcom_pdc_alloc(struct irq_domain *domain,
+	unsigned int virq, unsigned int nr_irqs, void *data)
+{
+	struct irq_fwspec *fwspec = data;
+	struct irq_fwspec parent_fwspec;
+	irq_hw_number_t hwirq;
+	int i;

drivers/irqchip/qcom-pdc.c new file +36 -0

+
+	parent_fwspec = *fwspec;
+	parent_fwspec.fwnode = domain->parent->fwnode;
+
+	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
+						&parent_fwspec);
+}
+
+static const struct irq_domain_ops qcom_pdc_ops = {
+	.translate	= qcom_pdc_translate,
+	.alloc		= qcom_pdc_alloc,
+	.free		= irq_domain_free_irqs_common,
+};
+
+static const struct of_device_id pdc_table[] = {
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pdc_table);
+
+int qcom_pdc_init(struct device_node *node, struct device_node *parent)
+{
+	struct irq_domain *parent_domain,  *pdc_domain;
+	const struct pdc_pin *data;
+	const struct of_device_id *id;
+	int ret;
+
+	id = of_match_node(pdc_table, node);
+	if (id)
+		data = id->data;
+	else
+		return -ENODEV;
+
+	pdc_base = of_iomap(node, 0);
+	if (!pdc_base) {
+		pr_err("%s(): unable to map PDC registers\n", node->full_name);
+		return -ENXIO;

drivers/irqchip/qcom-pdc.c new file +6 -0

+	return 0;
+
+failure:
+	iounmap(pdc_base);
+	return ret;
+}

drivers/irqchip/qcom-pdc.h new file +20 -0

new file mode 100644
index 000000000000..b5b64390175e
@@ -0,0 +1,30 @@
+/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __QCOM_PDC_H__
+#define __QCOM_PDC_H__
+
+#include <linux/irq.h>
+
+/**
+ * pdc_pin: Mapping of PDC port to hwirq

Hi Lina, Archana,

Quoted 13 lines · Lina Iyer

From : Archana Sathyakumar <[email protected]>

The Power Domain Controller (PDC) hardware block on Qualcomm SoCs houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts.

The interrupt controller is on an always-on domain for the purpose of waking up the processor, but only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processor's domain, bringing the domain out of low power mode and replays the pending interrupts so the GIC may wake up the processor.

Quoted 4 lines

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

[Lina: Split out DT bindings target data and initialization changes]

---

There is one thing that worries me in this driver. You say that the PDC "replays the pending interrupts so the GIC may wake up the processor". How is that done without any PM hook allowing for a switch from GIC to PDC? How do you ensure that you transition from one to the other without loosing interrupts (edge interrupts, in particular)? Or can you get spurious interrupts instead?

Hi Lina, Archana,

Quoted 17 lines · Lina Iyer

From : Archana Sathyakumar <[email protected]>

The Power Domain Controller (PDC) hardware block on Qualcomm SoCs houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts.

The interrupt controller is on an always-on domain for the purpose of waking up the processor, but only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processor's domain, bringing the domain out of low power mode and replays the pending interrupts so the GIC may wake up the processor.

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

[Lina: Split out DT bindings target data and initialization changes]

---

drivers/irqchip/qcom-pdc.c line 203

Quoted 5 lines

drivers/irqchip/qcom-pdc.c new file +5 -0

+
+static int qcom_pdc_translate(struct irq_domain *d,
+	struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type)
+{
+	return d->parent->ops->translate(d->parent, fwspec, hwirq, type);

No way. The translate operation is local to your domain. You don't go and fish in another domain's private stuff. Please implement your own. The reason you're getting away with it is because you're in the DT by providing the GIC SPI instead of the pin into the PDC.

Don't do that. Expose the pin in the DT, use the alloc method to map the PDC pin into the GIC pin.

Hi Mark,

Quoted 20 lines · Marc Zyngier

Hi Lina, Archana,

Quoted 17 lines · Lina Iyer

From : Archana Sathyakumar <[email protected]>

The Power Domain Controller (PDC) hardware block on Qualcomm SoCs houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts.

The interrupt controller is on an always-on domain for the purpose of waking up the processor, but only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processor's domain, bringing the domain out of low power mode and replays the pending interrupts so the GIC may wake up the processor.

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

[Lina: Split out DT bindings target data and initialization changes]

---

[...]

Quoted 11 lines

drivers/irqchip/qcom-pdc.c line 203

Quoted 5 lines

drivers/irqchip/qcom-pdc.c new file +5 -0

+
+static int qcom_pdc_translate(struct irq_domain *d,
+	struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type)
+{
+	return d->parent->ops->translate(d->parent, fwspec, hwirq, type);

No way. The translate operation is local to your domain. You don't go and fish in another domain's private stuff. Please implement your own. The reason you're getting away with it is because you're in the DT by providing the GIC SPI instead of the pin into the PDC.

I am looking into this approach. Was hoping to get some clarifications from you.

The PDC sits between the device and the the GIC. Platform drivers receive their interrupts from GIC. They are not aware of the fact that the GIC may lose power and hand over its job to PDC. The platform

drivers may configure an interrupt as a wakeup interrupt, in which case, we would wake up the CPU even if we are in system sleep or suspend mode. Platform driver don't know about the PDC pin or its configuration information. It makes it easier to keep that information contained within the PDC driver. Instead of getting the pin-hwirq map from the table as in patch #4, I can get that information cleanly from DT.

Quoted 4 lines

Don't do that. Expose the pin in the DT, use the alloc method to map the PDC pin into the GIC pin.

I would like to understand how you mean by this. I am thinking something like this in the dts -

/ {
	interrupt-controller = <&pdc>;
	soc {
		intc: interrupt-controller@176000 {
			[...]
			interrupt-controller;
			interrupt-parent = <&intc>;
		};

		pdc: interrupt-controller@210000 {

			[...]
			interrupt-controller;
			interrupt-parent = <&intc>;
			qcm,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
		};

		foo-device {
			interrupts = <GIC_SPI 481 IRQ_TYPE_NONE>;

		};
	};

};

Where the qcom,pdc-ranges is defined as -
<pdc-pin-start interrupt-vector size-of-sequence>.

For this example, the PDC map is established for pin0-pin93 using 94 interrupts in sequence starting from 512 and so on. This allows for holes in the map per the hardware interrupt topology.

I am not sure if you were asking to specify the pin in the 'interrupts' property in each device. I would like to avoid that as this may be an information that the device author may care less about. Would you agree?

If I completely missed your point, would you please care to elaborate?

Quoted 37 lines · Lina Iyer

Hi Mark,

Quoted 20 lines · Marc Zyngier

Hi Lina, Archana,

Quoted 17 lines · Lina Iyer

From : Archana Sathyakumar <[email protected]>

The Power Domain Controller (PDC) hardware block on Qualcomm SoCs houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts.

The interrupt controller is on an always-on domain for the purpose of waking up the processor, but only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processor's domain, bringing the domain out of low power mode and replays the pending interrupts so the GIC may wake up the processor.

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

[Lina: Split out DT bindings target data and initialization changes]

---

[...]

Quoted 11 lines

drivers/irqchip/qcom-pdc.c line 203

Quoted 5 lines

drivers/irqchip/qcom-pdc.c new file +5 -0

+
+static int qcom_pdc_translate(struct irq_domain *d,
+	struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type)
+{
+	return d->parent->ops->translate(d->parent, fwspec, hwirq, type);

No way. The translate operation is local to your domain. You don't go and fish in another domain's private stuff. Please implement your own. The reason you're getting away with it is because you're in the DT by providing the GIC SPI instead of the pin into the PDC.

Quoted 6 lines

I am looking into this approach. Was hoping to get some clarifications from you.

The PDC sits between the device and the the GIC. Platform drivers receive their interrupts from GIC. They are not aware of the fact that the GIC may lose power and hand over its job to PDC. The platform

I disagree here. If the PDC is between the device and the GIC, then the device interrupt line is routed to the PDC, and nothing else. The PDC itself is tied to the GIC, but that's none of the device's business.

In general, the device shouldn't care about what interrupt controller it is connected to. So please just describe the HW. There is about 10 similar configurations in the tree at the moment for the exact same thing. They are simpler because their PDC equivalent has been designed to exactly align with the GIC pins, but that's absolutely not a requirement.

Quoted 6 lines

drivers may configure an interrupt as a wakeup interrupt, in which case, we would wake up the CPU even if we are in system sleep or suspend mode. Platform driver don't know about the PDC pin or its configuration information. It makes it easier to keep that information contained within the PDC driver. Instead of getting the pin-hwirq map from the table as in patch #4, I can get that information cleanly from DT.

That part is fine.

Quoted 23 lines

Quoted 4 lines

Don't do that. Expose the pin in the DT, use the alloc method to map the PDC pin into the GIC pin.

I would like to understand how you mean by this. I am thinking something like this in the dts -

/ {
	interrupt-controller = <&pdc>;
	soc {
		intc: interrupt-controller@176000 {
			[...]
			interrupt-controller;
			interrupt-parent = <&intc>;
		};

		pdc: interrupt-controller@210000 {
			[...]
			interrupt-controller;
			interrupt-parent = <&intc>;
			qcm,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
		};

Quoted 2 lines

		foo-device {
			interrupts = <GIC_SPI 481 IRQ_TYPE_NONE>;

Drop the GIC_SPI (you should only use it in the PDC driver itself when requesting the GIC version of the interrupt), make sure 481 is a PDC pin number (it looks like a GIC SPI to me), and *never* encode IRQ_TYPE_NONE in DT (put the actual trigger type).

Quoted 11 lines

		};
	};

};

Where the qcom,pdc-ranges is defined as -
<pdc-pin-start interrupt-vector size-of-sequence>.

For this example, the PDC map is established for pin0-pin93 using 94 interrupts in sequence starting from 512 and so on. This allows for holes in the map per the hardware interrupt topology.

Quoted 3 lines

I am not sure if you were asking to specify the pin in the 'interrupts' property in each device. I would like to avoid that as this may be an information that the device author may care less about. Would you agree?

The way we represent these stacked interrupt controllers is by tying the device to the closest interrupt controller. Look at

Quoted 49 lines · Marc Zyngier

Quoted 37 lines · Lina Iyer

Hi Mark,

Quoted 20 lines · Marc Zyngier

Hi Lina, Archana,

Quoted 17 lines · Lina Iyer

From : Archana Sathyakumar <[email protected]>

The Power Domain Controller (PDC) hardware block on Qualcomm SoCs houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts.

The interrupt controller is on an always-on domain for the purpose of waking up the processor, but only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processor's domain, bringing the domain out of low power mode and replays the pending interrupts so the GIC may wake up the processor.

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

[Lina: Split out DT bindings target data and initialization changes]

---

[...]

Quoted 11 lines

drivers/irqchip/qcom-pdc.c line 203

Quoted 5 lines

drivers/irqchip/qcom-pdc.c new file +5 -0

+
+static int qcom_pdc_translate(struct irq_domain *d,
+	struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type)
+{
+	return d->parent->ops->translate(d->parent, fwspec, hwirq, type);

No way. The translate operation is local to your domain. You don't go and fish in another domain's private stuff. Please implement your own. The reason you're getting away with it is because you're in the DT by providing the GIC SPI instead of the pin into the PDC.

Quoted 6 lines

I am looking into this approach. Was hoping to get some clarifications from you.

The PDC sits between the device and the the GIC. Platform drivers receive their interrupts from GIC. They are not aware of the fact that the GIC may lose power and hand over its job to PDC. The platform

I disagree here. If the PDC is between the device and the GIC, then the device interrupt line is routed to the PDC, and nothing else. The PDC itself is tied to the GIC, but that's none of the device's business.

Quoted 6 lines

In general, the device shouldn't care about what interrupt controller it is connected to. So please just describe the HW. There is about 10 similar configurations in the tree at the moment for the exact same thing. They are simpler because their PDC equivalent has been designed to exactly align with the GIC pins, but that's absolutely not a requirement.

This is exactly the case. The PDC is not aligned with GIC for all interrupts. To be a bit more clear than what I seem to conveyed, some interrupts are routed to the GIC and to the PDC and some are not.

Interrupts like USB that need to be detected for a falling edge or level low (i.e., not detected at the GIC) are routed through the PDC as well. The PDC detects these and re-triggers the GIC interrupts with the correct polarity so the GIC may sense it.

In another case, when the GIC is powered off, the PDC will continue to montior the interrupts routed through the PDC interrupt controller and if any of the enabled ones trigger, the PDC will wake up the GIC and replay the interrupts at the GIC.

To the device the presence of PDC is transparent, nor it is aware if the GIC was powered off while the interrupt was enabled.

Quoted 33 lines

Quoted 7 lines

drivers may configure an interrupt as a wakeup interrupt, in which case, we would wake up the CPU even if we are in system sleep or suspend mode. Platform driver don't know about the PDC pin or its configuration information. It makes it easier to keep that information contained within the PDC driver. Instead of getting the pin-hwirq map from the table as in patch #4, I can get that information cleanly from DT.

That part is fine.

Quoted 23 lines

Quoted 4 lines

Don't do that. Expose the pin in the DT, use the alloc method to map the PDC pin into the GIC pin.

I would like to understand how you mean by this. I am thinking something like this in the dts -

/ {
	interrupt-controller = <&pdc>;
	soc {
		intc: interrupt-controller@176000 {
			[...]
			interrupt-controller;
			interrupt-parent = <&intc>;
		};

		pdc: interrupt-controller@210000 {
			[...]
			interrupt-controller;
			interrupt-parent = <&intc>;
			qcm,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
		};

Quoted 8 lines

Quoted 2 lines

		foo-device {
			interrupts = <GIC_SPI 481 IRQ_TYPE_NONE>;

Drop the GIC_SPI (you should only use it in the PDC driver itself when requesting the GIC version of the interrupt), make sure 481 is a PDC pin number (it looks like a GIC SPI to me), and *never* encode IRQ_TYPE_NONE in DT (put the actual trigger type).

Sure, I just picked it up as an example.Yes, this is a GIC SPI and not a PDC port. This is what the platform drivers desire. They have an interrupt number assigned for their device and it would be preferrable for them to use this interrupt vector at the GIC in DT. They dont't want to know the PDC pin (even though the interrupt controller for the SoC may be the PDC in DT). This is because the devices that do not have their interrupt routed through the PDC, would anyways have only the GIC interrupt vector to provide. They have no PDC port available for them. The foo-device above is an example of one such device that does not have it's interrupt routed through the PDC (it doesn't fall in the pdc-ranges).

The PDC behaves more like arch extn to the GIC than a stacked interrupt controller in the hierarchy.

Quoted 12 lines

Quoted 12 lines

		};
	};

};

Where the qcom,pdc-ranges is defined as -
<pdc-pin-start interrupt-vector size-of-sequence>.

For this example, the PDC map is established for pin0-pin93 using 94 interrupts in sequence starting from 512 and so on. This allows for holes in the map per the hardware interrupt topology.

Quoted 7 lines

Quoted 3 lines

I am not sure if you were asking to specify the pin in the 'interrupts' property in each device. I would like to avoid that as this may be an information that the device author may care less about. Would you agree?

The way we represent these stacked interrupt controllers is by tying the device to the closest interrupt controller. Look at

Did you miss a reference here?

We looked at tegra's implementation and it appears similar, thought they don't seemt to have the same complexity.

Quoted 22 lines · Lina Iyer

Quoted 21 lines · Marc Zyngier

Quoted 20 lines · Lina Iyer

Quoted 18 lines · Marc Zyngier

Quoted 17 lines · Lina Iyer

From : Archana Sathyakumar <[email protected]>

The Power Domain Controller (PDC) hardware block on Qualcomm SoCs houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts.

The interrupt controller is on an always-on domain for the purpose of waking up the processor, but only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processor's domain, bringing the domain out of low power mode and replays the pending interrupts so the GIC may wake up the processor.

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

[Lina: Split out DT bindings target data and initialization changes]

---

Quoted 2 lines

We looked at tegra's implementation and it appears similar, thought they don't seemt to have the same complexity.

Exactly. They are pretty similar. Hence my suggestion to adopt the same approach, which you don't seem to agree with.

From: Archana Sathyakumar <[email protected]>

Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational.

Cc [email protected]

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 .../bindings/interrupt-controller/qcom,pdc.txt     | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt

Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file +10 -0

new file mode 100644
index 000000000000..c4592bbf678d
@@ -0,0 +1,55 @@
+PDC interrupt controller
+
+Qualcomm Technologies Inc. SoCs based on the RPM Hardened archicture have a
+Power Domain Controller (PDC) that is on always-on domain. In addition to
+providing power control for the power domains, the hardware also has an
+interrupt controller that can be used to help detect edge low interrupts as
+well detect interrupts when the GIC is non-operational.
+
+GIC is parent interrupt controller at the highest level. Platform interrupt
+controller PDC is next in hierarchy, followed by others.

This driver only configures the interrupts, does not handle them.

Quoted 31 lines · Lina Iyer

From: Archana Sathyakumar <[email protected]>

Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational.

Cc [email protected]

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 .../bindings/interrupt-controller/qcom,pdc.txt     | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt

Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file +10 -0

new file mode 100644
index 000000000000..c4592bbf678d
@@ -0,0 +1,55 @@
+PDC interrupt controller
+
+Qualcomm Technologies Inc. SoCs based on the RPM Hardened archicture have a
+Power Domain Controller (PDC) that is on always-on domain. In addition to
+providing power control for the power domains, the hardware also has an
+interrupt controller that can be used to help detect edge low interrupts as
+well detect interrupts when the GIC is non-operational.
+
+GIC is parent interrupt controller at the highest level. Platform interrupt
+controller PDC is next in hierarchy, followed by others.

Quoted 1 line

This driver only configures the interrupts, does not handle them.

Not sure if the above statement belongs to the binding.

From: Archana Sathyakumar <[email protected]>

Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational.

Cc [email protected]

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 .../bindings/interrupt-controller/qcom,pdc.txt     | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt

Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file +0 -0

new file mode 100644
index 000000000000..c4592bbf678d

Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file +19 -0

+Power Domain Controller (PDC) that is on always-on domain. In addition to
+providing power control for the power domains, the hardware also has an
+interrupt controller that can be used to help detect edge low interrupts as
+well detect interrupts when the GIC is non-operational.
+
+GIC is parent interrupt controller at the highest level. Platform interrupt
+controller PDC is next in hierarchy, followed by others. This driver only
+configures the interrupts, does not handle them.
+
+Properties:
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: Should contain "qcom,pdc" and "qcom,pdc-<target>"
+		    - "qcom,pdc-sdm845": For sdm845 pin data
+
+- reg:
+	Usage: required

Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file +13 -0

+
+- interrupt-parent:
+	Usage: required
+	Value type: <phandle>
+	Definition: Specifies the interrupt parent necessary for hierarchical
+		    domain to operate.
+
+- interrupt-controller:
+	Usage: required
+	Value type: <bool>
+	Definition: Identifies the node as an interrupt controller.
+
+Example:

From: Archana Sathyakumar <[email protected]>

Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational.

Cc [email protected]

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 .../bindings/interrupt-controller/qcom,pdc.txt     | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt

Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file +9 -0

new file mode 100644
index 000000000000..c4592bbf678d
@@ -0,0 +1,55 @@
+PDC interrupt controller
+
+Qualcomm Technologies Inc. SoCs based on the RPM Hardened archicture have a
+Power Domain Controller (PDC) that is on always-on domain. In addition to
+providing power control for the power domains, the hardware also has an
+interrupt controller that can be used to help detect edge low interrupts as
+well detect interrupts when the GIC is non-operational.
+
+GIC is parent interrupt controller at the highest level. Platform interrupt

Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file +19 -0

+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: Specifies the base physical address for PDC hardware.
+
+- interrupt-cells:
+	Usage: required
+	Value type: <u32>
+	Definition: Specifies the number of cells needed to encode an interrupt
+		    source.
+		    Value must be 3.
+		    The encoding of these cells are same as described in [1].
+
+- interrupt-parent:
+	Usage: required
+	Value type: <phandle>
+	Definition: Specifies the interrupt parent necessary for hierarchical
+		    domain to operate.

Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file +7 -0

+		reg = <0xb220000 0x30000>;
+		#interrupt-cells = <3>;
+		interrupt-parent = <&intc>;
+		interrupt-controller;
+	};
+
+[1]. Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt

Signature · 7 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Archana Sathyakumar <[email protected]>

Log key PDC pin configuration in FTRACE.

Cc Steven Rostedt <[email protected]>

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/irqchip/qcom-pdc.c |  6 ++++++
 include/trace/events/pdc.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 include/trace/events/pdc.h

drivers/irqchip/qcom-pdc.c +2 -0

index 57f1348bd81c..9b626e9f3a29 100644
@@ -26,6 +26,8 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#define CREATE_TRACE_POINTS
+#include "trace/events/pdc.h"
 
 #include "qcom-pdc.h"
 

drivers/irqchip/qcom-pdc.c +2 -0

+				pdc_type, 0);
+
 	/*
 	 * If type is edge triggered, forward that as Rising edge as PDC
 	 * takes care of converting falling edge to rising edge signal

include/trace/events/pdc.h new file +33 -0

new file mode 100644
index 000000000000..bec54c414880
@@ -0,0 +1,50 @@
+/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM pdc
+
+#if !defined(_TRACE_PDC_) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PDC_H_
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(irq_pin_config,
+
+	TP_PROTO(char *func, u32 pin, u32 hwirq, u32 type, u32 enable),
+
+	TP_ARGS(func, pin, hwirq, type, enable),
+
+	TP_STRUCT__entry(
+		__field(char *, func)
+		__field(u32, pin)
+		__field(u32, hwirq)
+		__field(u32, type)
+		__field(u32, enable)
+	),

include/trace/events/pdc.h new file +7 -0

+		__entry->func, __entry->pin, __entry->hwirq, __entry->type,
+		__entry->enable)
+);
+
+#endif
+#define TRACE_INCLUDE_FILE pdc
+#include <trace/define_trace.h>

Quoted 1 line · On Tue, 23 Jan 2018 10:56:55 -0700 Lina Iyer <[email protected]> wrote:

From: Archana Sathyakumar <[email protected]>

Hi Lina and Archana,

Quoted 26 lines

Log key PDC pin configuration in FTRACE.

Cc Steven Rostedt <[email protected]>

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/irqchip/qcom-pdc.c |  6 ++++++
 include/trace/events/pdc.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 include/trace/events/pdc.h

drivers/irqchip/qcom-pdc.c +2 -0

index 57f1348bd81c..9b626e9f3a29 100644
@@ -26,6 +26,8 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#define CREATE_TRACE_POINTS
+#include "trace/events/pdc.h"
 
 #include "qcom-pdc.h"
 

drivers/irqchip/qcom-pdc.c static inline void pdc_enable_intr(struct irq_data *d, bool on) · line 83

Quoted 4 lines

drivers/irqchip/qcom-pdc.c +1 -0

@@ -78,6 +80,7 @@ static inline void pdc_enable_intr(struct irq_data *d, bool on)
 			break;
 		udelay(5);
 	} while (1);
+	trace_irq_pin_config("enable", (u32)pin_out, (u32)d->hwirq, 0, on);

I'm concerned with that string you are passing in.

drivers/irqchip/qcom-pdc.c static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type) · line 167

Quoted 8 lines

drivers/irqchip/qcom-pdc.c +1 -0

 	spin_unlock_irqrestore(&pdc_lock, flags);
 	WARN_ON(r_enable != enable);
 }

@@ -161,6 +164,9 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
 		udelay(5);
 	} while (1);
 
+	trace_irq_pin_config("type_config", (u32)pin_out, (u32)d->hwirq,

This one too.

Quoted 44 lines

drivers/irqchip/qcom-pdc.c +2 -0

+				pdc_type, 0);
+
 	/*
 	 * If type is edge triggered, forward that as Rising edge as PDC
 	 * takes care of converting falling edge to rising edge signal

include/trace/events/pdc.h new file +33 -0

new file mode 100644
index 000000000000..bec54c414880
@@ -0,0 +1,50 @@
+/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM pdc
+
+#if !defined(_TRACE_PDC_) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PDC_H_
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(irq_pin_config,
+
+	TP_PROTO(char *func, u32 pin, u32 hwirq, u32 type, u32 enable),
+
+	TP_ARGS(func, pin, hwirq, type, enable),
+
+	TP_STRUCT__entry(
+		__field(char *, func)
+		__field(u32, pin)
+		__field(u32, hwirq)
+		__field(u32, type)
+		__field(u32, enable)
+	),

include/trace/events/pdc.h line 43

Quoted 10 lines

include/trace/events/pdc.h new file +10 -0

+
+	TP_fast_assign(
+		__entry->pin = pin;
+		__entry->func = func;
+		__entry->hwirq = hwirq;
+		__entry->type = type;
+		__entry->enable = enable;
+	),
+
+	TP_printk("%s hwirq:%u pin:%u type:%u enable:%u",

%s is dereferencing a pointer, which works fine when reading the ASCII translated trace files (/sys/kernel/debug/tracing/trace and friends). But it breaks when we use trace-cmd or perf. Because they record the binary data and they do not have access to memory inside the kernel.

What I would recommend is in pdc.h have:

#define PDC_ENTRY 1
#define PDC_TYPE_CONFIG 2

Then have the tracepoints do instead of:

	trace_irq_pin_config("enable", ...);
	trace_irq_pin_config("type_config", ...);

Have them do:

	trace_irq_pin_config(PDC_ENTRY, ...);
	trace_irq_pin_config(PDC_TYPE_CONFIG, ...);

And then have:

	TP_PROTO(u32 func, ...)

	TP_STRUCT__entry(
		__field(u32, func)
		[...]

	TP_fast_assign(
		__entry->func = func;
		[...]

	TP_printk("%s hwirq...",
		print_symbolic(__entry->func,
				{ PDC_ENTRY, "entry" },
				{ PDC_TYPE_CONFIG, "type_config" }),
		[...]

And you will get the same effect, with trace-cmd and perf still working as expected.

-- Steve

Quoted 7 lines

include/trace/events/pdc.h new file +7 -0

+		__entry->func, __entry->pin, __entry->hwirq, __entry->type,
+		__entry->enable)
+);
+
+#endif
+#define TRACE_INCLUDE_FILE pdc
+#include <trace/define_trace.h>

From: Archana Sathyakumar <[email protected]>

Add PDC pin information for SDM845. Interrupts listed are wake up sources for the processor, when the processor and GIC are powered down.

Signed-off-by Archana Sathyakumar <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/irqchip/Makefile          |   2 +-
 drivers/irqchip/qcom-pdc-sdm845.c | 130 ++++++++++++++++++++++++++++++++++++++
 drivers/irqchip/qcom-pdc.c        |   3 +
 drivers/irqchip/qcom-pdc.h        |   1 +
 4 files changed, 135 insertions(+), 1 deletion(-)
 create mode 100644 drivers/irqchip/qcom-pdc-sdm845.c

drivers/irqchip/Makefile +1 -1

index 280723d83916..0cf8f3feb47f 100644
@@ -84,4 +84,4 @@ obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
 obj-$(CONFIG_IRQ_UNIPHIER_AIDET)	+= irq-uniphier-aidet.o
 obj-$(CONFIG_ARCH_SYNQUACER)		+= irq-sni-exiu.o
 obj-$(CONFIG_MESON_IRQ_GPIO)		+= irq-meson-gpio.o
-obj-$(CONFIG_QCOM_PDC)			+= qcom-pdc.o
+obj-$(CONFIG_QCOM_PDC)			+= qcom-pdc.o qcom-pdc-sdm845.o

drivers/irqchip/qcom-pdc-sdm845.c new file +13 -0

new file mode 100644
index 000000000000..57d0fb51d5ac
@@ -0,0 +1,130 @@
+/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+

drivers/irqchip/qcom-pdc-sdm845.c new file +1 -0

+};

drivers/irqchip/qcom-pdc.c +3 -0

index 9b626e9f3a29..0d54db4e915b 100644
@@ -241,6 +241,7 @@ static const struct irq_domain_ops qcom_pdc_ops = {
 };
 
 static const struct of_device_id pdc_table[] = {
+	{ .compatible = "qcom,pdc-sdm845", .data = sdm845_data, },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, pdc_table);

@@ -286,3 +287,5 @@ int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 	iounmap(pdc_base);
 	return ret;
 }
+
+IRQCHIP_DECLARE(pdc_sdm845, "qcom,pdc-sdm845", qcom_pdc_init);

drivers/irqchip/qcom-pdc.h +0 -0

index b5b64390175e..6a9d9f9eb4f1 100644

Quoted 12 lines · Lina Iyer

From: Archana Sathyakumar <[email protected]>

drivers/irqchip/qcom-pdc-sdm845.c new file +10 -0

+#include "qcom-pdc.h"
+
+const struct pdc_pin sdm845_data[] = {
+	{0, 512}, /* rpmh_wake */
+	{1, 513}, /* ee0_apps_hlos_spmi_periph_irq */
+	{2, 514}, /* ee1_apps_trustzone_spmi_periph_irq */
+	{3, 515}, /* secure_wdog_expired */
+	{4, 516}, /* secure_wdog_bark_irq */
+	{5, 517}, /* aop_wdog_expired_irq */
+	{6, 518}, /* qmp_usb3_lfps_rxterm_irq */

[...]

Nice try, but no. This is DT material. Please use the pin number in your DT, which the driver uses as a hwirq. When it comes to mapping it to the corresponding GIC SPI, use a per-soc helper that comes from DT too (you seem to only have two ranges here, so that's pretty easy to do).

On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a power domain that can be powered off when not needed. Interrupts that need to be sensed even when the GIC is powered off, are routed through an interrupt controller in an always-on domain called the Power Domain Controller a.k.a PDC. This series adds support for the PDC's interrupt controller.

Hi Lina,

Quoted 6 lines · Lina Iyer

On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a power domain that can be powered off when not needed. Interrupts that need to be sensed even when the GIC is powered off, are routed through an interrupt controller in an always-on domain called the Power Domain Controller a.k.a PDC. This series adds support for the PDC's interrupt controller.

Sorry for the basic questions:

1. Will the GIC be powered off in any other state other than System suspend ?

2. Why this needs to be done in Linux, why can't it be transparent and hidden in the firmware doing the actual GIC power down ? I assume Linux is not powering down the GIC.

3. I see some bits that enable secure interrupts in one of the patch. Is that even
safe to allow Linux to enable some secure interrupts in PDC ?

Quoted 14 lines · Sudeep Holla

Hi Lina,

Quoted 10 lines · Lina Iyer

On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a
power domain that can be powered off when not needed. Interrupts that need to
be sensed even when the GIC is powered off, are routed through an interrupt
controller in an always-on domain called the Power Domain Controller a.k.a PDC.
This series adds support for the PDC's interrupt controller.

Quoted 5 lines

Sorry for the basic questions:

1. Will the GIC be powered off in any other state other than System suspend ?

Yes. When all the CPUs are in idle, there is an opportunity to power off the CPU's power domain and the GIC. QCOM SoCs have been doing that for many generations now.

Quoted 5 lines

2. Why this needs to be done in Linux, why can't it be transparent and hidden
   in the firmware doing the actual GIC power down ? I assume Linux is not
   powering down the GIC.

No. You are right, Linux is not powering off the GIC directly. A dedicated processor for power management in the SoC does that. Platform drivers in Linux, know and configure the wakeup interrupts (depending on the usecase). This is runtime specific and this is the way to tell the SoC to wake up the processor even if the GIC and the CPU domain were powered off.

Quoted 5 lines

3. I see some bits that enable secure interrupts in one of the patch. Is that even
   safe to allow Linux to enable some secure interrupts in PDC ?

Linux should not and would not configure secure interrupts. We would not have permissions for secure interrupts. The interrupt names might be a misnomer, but the interrupts listed in patch #4 are all non-secure interrupts.

Quoted 20 lines · Lina Iyer

Quoted 19 lines · Sudeep Holla

Hi Lina,

Quoted 10 lines · Lina Iyer

On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a
power domain that can be powered off when not needed. Interrupts that need to
be sensed even when the GIC is powered off, are routed through an interrupt
controller in an always-on domain called the Power Domain Controller a.k.a PDC.
This series adds support for the PDC's interrupt controller.

Sorry for the basic questions:

1. Will the GIC be powered off in any other state other than System suspend ?

Quoted 4 lines

Yes. When all the CPUs are in idle, there is an opportunity to power off the CPU's power domain and the GIC. QCOM SoCs have been doing that for many generations now.

OK interesting, in that case so either GIC state is saved/restored with some out of tree patches or the firmware takes care of it and it's transparent to Linux ?

Also when will this PDC wakeup interrupts get configured ?

Quoted 5 lines

Quoted 5 lines

2. Why this needs to be done in Linux, why can't it be transparent and hidden
   in the firmware doing the actual GIC power down ? I assume Linux is not
   powering down the GIC.

Quoted 7 lines

No. You are right, Linux is not powering off the GIC directly. A dedicated processor for power management in the SoC does that. Platform drivers in Linux, know and configure the wakeup interrupts (depending on the usecase). This is runtime specific and this is the way to tell the SoC to wake up the processor even if the GIC and the CPU domain were powered off.

OK, understood. By transparent, I mean firmware can copy the interrupts enabled in the GIC to the PDC. It need not be kernel driven.

Quoted 10 lines

Quoted 5 lines

3. I see some bits that enable secure interrupts in one of the patch. Is that even
   safe to allow Linux to enable some secure interrupts in PDC ?

Linux should not and would not configure secure interrupts. We would not have permissions for secure interrupts. The interrupt names might be a misnomer, but the interrupts listed in patch #4 are all non-secure interrupts.

OK. So I can assume PDC is partitioned in secure and non-secure. If not it's safe not have any access for PDC in the kernel. Couple of designs of similar PDC I have seen is system wide and does handle even secure part of the system. That was the main reason for checking.

Quoted 29 lines · Sudeep Holla

Quoted 25 lines · Lina Iyer

Quoted 15 lines · Sudeep Holla

Hi Lina,

Quoted 10 lines · Lina Iyer

On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a
power domain that can be powered off when not needed. Interrupts that need to
be sensed even when the GIC is powered off, are routed through an interrupt
controller in an always-on domain called the Power Domain Controller a.k.a PDC.
This series adds support for the PDC's interrupt controller.

Quoted 5 lines

Sorry for the basic questions:

1. Will the GIC be powered off in any other state other than System suspend ?

Yes. When all the CPUs are in idle, there is an opportunity to power off the CPU's power domain and the GIC. QCOM SoCs have been doing that for many generations now.

Quoted 4 lines

OK interesting, in that case so either GIC state is saved/restored with some out of tree patches or the firmware takes care of it and it's transparent to Linux ?

Yes. It is handled by a remote processor, which is aware that the application processor subsystem has been powered off.

Quoted 2 lines

Also when will this PDC wakeup interrupts get configured ?

The platform drivers configure the IRQ as a wake source and if the IRQ is one of those listed as routed to the PDC, the PDC is configured to sense the interrupt and when the application processor domain is powered on and the GIC can sense the interrupts, it is replayed to the GIC, which then wakes up the processor.

Quoted 13 lines

Quoted 12 lines

Quoted 5 lines

2. Why this needs to be done in Linux, why can't it be transparent and hidden
   in the firmware doing the actual GIC power down ? I assume Linux is not
   powering down the GIC.

No. You are right, Linux is not powering off the GIC directly. A dedicated processor for power management in the SoC does that. Platform drivers in Linux, know and configure the wakeup interrupts (depending on the usecase). This is runtime specific and this is the way to tell the SoC to wake up the processor even if the GIC and the CPU domain were powered off.

Quoted 3 lines

OK, understood. By transparent, I mean firmware can copy the interrupts enabled in the GIC to the PDC. It need not be kernel driven.

Yes, through the hierarchy.

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

3. I see some bits that enable secure interrupts in one of the patch. Is that even
   safe to allow Linux to enable some secure interrupts in PDC ?

Linux should not and would not configure secure interrupts. We would not have permissions for secure interrupts. The interrupt names might be a misnomer, but the interrupts listed in patch #4 are all non-secure interrupts.

Quoted 5 lines

OK. So I can assume PDC is partitioned in secure and non-secure. If not it's safe not have any access for PDC in the kernel. Couple of designs of similar PDC I have seen is system wide and does handle even secure part of the system. That was the main reason for checking.

Yes. There is a partition and protected. So only permitted ELs can write to the registers. This is done by the firmware at boot.

Quoted 6 lines · Lina Iyer

Quoted 4 lines · Sudeep Holla

Quoted 1 line · Lina Iyer

On Tue, Jan 23 2018 at 18:15 +0000, Sudeep Holla wrote:

Quoted 8 lines

Quoted 2 lines

Also when will this PDC wakeup interrupts get configured ?

The platform drivers configure the IRQ as a wake source and if the IRQ is one of those listed as routed to the PDC, the PDC is configured to sense the interrupt and when the application processor domain is powered on and the GIC can sense the interrupts, it is replayed to the GIC, which then wakes up the processor.

Now why can't this be done in the firmware entirely, if it can save/restore GIC state.

Quoted 5 lines

Quoted 3 lines

OK, understood. By transparent, I mean firmware can copy the interrupts enabled in the GIC to the PDC. It need not be kernel driven.

Yes, through the hierarchy.

/me confused. Are you saying it's possible for f/w to copy wakeup sources from GIC to PDC or not ?

Quoted 3 lines

Yes. There is a partition and protected. So only permitted ELs can write to the registers. This is done by the firmware at boot.

Just for myself to understand better, so you have multiple partitions in PDC and one of them is given to EL1 or it just has one partition and that can be configured so that only permitted ELx is allowed to access it(in your case it's EL1)

Quoted 18 lines · Sudeep Holla

Quoted 6 lines · Lina Iyer

Quoted 4 lines · Sudeep Holla

Quoted 1 line · Lina Iyer

On Tue, Jan 23 2018 at 18:15 +0000, Sudeep Holla wrote:

Quoted 8 lines

Quoted 2 lines

Also when will this PDC wakeup interrupts get configured ?

The platform drivers configure the IRQ as a wake source and if the IRQ is one of those listed as routed to the PDC, the PDC is configured to sense the interrupt and when the application processor domain is powered on and the GIC can sense the interrupts, it is replayed to the GIC, which then wakes up the processor.

Quoted 3 lines

Now why can't this be done in the firmware entirely, if it can save/restore GIC state.

That is because platform drivers make the choice of which interrupts are wake up capable at runtime, based on the usecase. They have to have a way to tell the firmware to do that. Earlier we used IPC to tell the remote processor to handle that. Now we do that by writing to the registers locally from Linux.

Quoted 9 lines

Quoted 5 lines

Quoted 3 lines

OK, understood. By transparent, I mean firmware can copy the interrupts enabled in the GIC to the PDC. It need not be kernel driven.

Yes, through the hierarchy.

/me confused. Are you saying it's possible for f/w to copy wakeup sources from GIC to PDC or not ?

Platform drivers leave their interrupts enabled at the GIC. Only interrupts that are wakeup capable are of interest when the processor is powered down. The remote processor (or f/w) will need to know the set of wakeup capable interrupts and then configure the PDC by copying from GIC. As mentioned earlier, it is simplified by letting Linux write to the PDC reqisters directly.

Quoted 9 lines

Quoted 3 lines

Yes. There is a partition and protected. So only permitted ELs can write to the registers. This is done by the firmware at boot.

Just for myself to understand better, so you have multiple partitions in PDC and one of them is given to EL1 or it just has one partition and that can be configured so that only permitted ELx is allowed to access it(in your case it's EL1)

Yes.

Quoted 8 lines · Lina Iyer

Quoted 7 lines · Sudeep Holla

Quoted 4 lines · Lina Iyer

Quoted 3 lines · Sudeep Holla

On 23/01/18 18:44, Lina Iyer wrote:

Quoted 9 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line · Sudeep Holla

Platform drivers leave their interrupts enabled at the GIC. Only interrupts that are wakeup capable are of interest when the processor is powered down. The remote processor (or f/w) will need to know the set of wakeup capable interrupts and then configure the PDC by copying from GIC. As mentioned earlier, it is simplified by letting Linux write to the PDC reqisters directly.

OK looks like I have not properly conveyed what I wanted to :( So lets take a peripheral A which has GIC interrupt X and a corresponding PDC interrupt Y.

IIUC you want to configure Y from Linux while X is still enabled.

1. GICv3 masks all the interrupts(~X) that are not wakeup sources. So when you say "platform drivers leave their interrupts enabled at the GIC", it's not completely correct.

2. GIC CPU interface is disabled in firmware, so it's better to copy the wakeup source to PDC just before that in the firmware.

3. Remote f/w must just know the mapping to PDC(X) for all the enabled interrupts(Y) at the GIC and enable them accordingly at PDC. Is that not what you have in the array in patch 4 ?

I find above approach simpler instead of getting those wakeup interrupts defined per peripheral in DT. Further if there are any secure wakeup interrupts the firmware can also deal with that.

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

Yes. There is a partition and protected. So only permitted ELs can write to the registers. This is done by the firmware at boot.

Quoted 6 lines

Quoted 5 lines

Just for myself to understand better, so you have multiple partitions in PDC and one of them is given to EL1 or it just has one partition and that can be configured so that only permitted ELx is allowed to access it(in your case it's EL1)

Yes.

Yes for the former or the latter :) ?

Quoted 27 lines · Sudeep Holla

Quoted 8 lines · Lina Iyer

Quoted 7 lines · Sudeep Holla

Quoted 4 lines · Lina Iyer

Quoted 3 lines · Sudeep Holla

On 23/01/18 18:44, Lina Iyer wrote:

Quoted 9 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line · Sudeep Holla

Platform drivers leave their interrupts enabled at the GIC. Only interrupts that are wakeup capable are of interest when the processor is powered down. The remote processor (or f/w) will need to know the set of wakeup capable interrupts and then configure the PDC by copying from GIC. As mentioned earlier, it is simplified by letting Linux write to the PDC reqisters directly.

OK looks like I have not properly conveyed what I wanted to :( So lets take a peripheral A which has GIC interrupt X and a corresponding PDC interrupt Y.

IIUC you want to configure Y from Linux while X is still enabled.

Quoted 4 lines

1. GICv3 masks all the interrupts(~X) that are not wakeup sources. So when you say "platform drivers leave their interrupts enabled at the GIC", it's not completely correct.

During idle path, the interrupts remain enabled. The GIC configuration is retained, but the controller cannot sense interrupts because the GIC logic is powered off. The PDC takes over for GIC at this time.

Quoted 11 lines

2. GIC CPU interface is disabled in firmware, so it's better to copy the wakeup source to PDC just before that in the firmware.

3. Remote f/w must just know the mapping to PDC(X) for all the enabled interrupts(Y) at the GIC and enable them accordingly at PDC. Is that not what you have in the array in patch 4 ?

I find above approach simpler instead of getting those wakeup interrupts defined per peripheral in DT. Further if there are any secure wakeup interrupts the firmware can also deal with that.

You assume that the remote processor is capable of doing all that. It is better to de-centralize all this and have individual processors do the work of configuring their wake up sources. We used to do that in earlier SoCs but with SDM845, we moved to de-centralized model to reduce latency and take the load off from time-critical idle path at the remote processor. Dumping all this work into the firmware/PSCI is not desirable either.

Quoted 7 lines · Lina Iyer

Quoted 6 lines · Sudeep Holla

Quoted 3 lines · Lina Iyer

Quoted 2 lines · Sudeep Holla

Quoted 7 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines · Lina Iyer

Quoted 5 lines · Sudeep Holla

Quoted 4 lines · Lina Iyer

Quoted 1 line · Sudeep Holla

Ah OK, so PDC interrupts needs to be enabled all the time then. I was missing that.

Quoted 11 lines

Quoted 11 lines

2. GIC CPU interface is disabled in firmware, so it's better to copy the wakeup source to PDC just before that in the firmware.

3. Remote f/w must just know the mapping to PDC(X) for all the enabled interrupts(Y) at the GIC and enable them accordingly at PDC. Is that not what you have in the array in patch 4 ?

I find above approach simpler instead of getting those wakeup interrupts defined per peripheral in DT. Further if there are any secure wakeup interrupts the firmware can also deal with that.

Quoted 8 lines

You assume that the remote processor is capable of doing all that. It is better to de-centralize all this and have individual processors do the work of configuring their wake up sources. We used to do that in earlier SoCs but with SDM845, we moved to de-centralized model to reduce latency and take the load off from time-critical idle path at the remote processor. Dumping all this work into the firmware/PSCI is not desirable either.

It may have some advantages to decentralize but will that not cause issues in complex systems ? I assume even modem and other processors can access and configure these wakeup interrupts. What happens if 2 such processors try to access it at the same time ?

Thanks for you patience and taking time to help me understand the design.

Quoted 1 line · Sudeep Holla

Quoted 5 lines

It may have some advantages to decentralize but will that not cause issues in complex systems ? I assume even modem and other processors can access and configure these wakeup interrupts. What happens if 2 such processors try to access it at the same time ?

Every processor in the SoC has its own PDC and does exactly the same thing in SW. The hardware blocks are replicated for each of the 'subsystem' and they behave similarly.