Changes since v3:
- Code clean up as suggested by Marc
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.
Please consider reviewing these patches.
RFC v1: https://patchwork.kernel.org/patch/10180857/
RFC v2: https://www.mail-archive.com/[email protected]/msg1600634.html
v3: https://lkml.org/lkml/2018/2/6/595
Lina Iyer (2):
drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
dt-bindings/interrupt-controller: pdc: descibe PDC device binding
.../bindings/interrupt-controller/qcom,pdc.txt | 80 ++++++
drivers/irqchip/Kconfig | 9 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/qcom-pdc.c | 311 +++++++++++++++++++++
4 files changed, 401 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
create mode 100644 drivers/irqchip/qcom-pdc.c
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
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 | 80 ++++++++++++++++++++++
1 file changed, 80 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
new file mode 100644
index 000000000000..21c4f2fbcd0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
@@ -0,0 +1,80 @@
+PDC interrupt controller
+
+Qualcomm Technologies Inc. SoCs based on the RPM Hardened architecture 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. Drivers requiring
+wakeup capabilities of their device interrupts routed through the PDC, must
+specify PDC as their interrupt controller and request the PDC port associated
+with the GIC interrupt. See example below.
+
+Properties:
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: Should contain "qcom,<soc>-pdc"
+ - "qcom,sdm845-pdc": For SDM845
+
+- 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.
+ Must be 2.
+ The first element of the tuple is the PDC pin for the
+ interrupt.
+ The second element is the trigger type.
+
+- 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.
+
+- qcom,pdc-ranges:
+ Usage: required
+ Value type: <u32 array>
+ Definition: Specifies the PDC pin offset and the number of PDC ports.
+ The tuples indicates the valid mapping of valid PDC ports
+ and their hwirq mapping.
+ The first element of the tuple is the starting PDC port.
+ The second element is the GIC hwirq number for the PDC port.
+ The third element is the number of interrupts in sequence.
+
+Example:
+
+ pdc: interrupt-controller@b220000 {
+ compatible = "qcom,sdm845-pdc";
+ reg = <0xb220000 0x30000>;
+ qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
+ #interrupt-cells = <2>;
+ interrupt-parent = <&intc>;
+ interrupt-controller;
+ };
+
+DT binding of a device that wants to use the GIC SPI 514 as a wakeup
+interrupt, must do -
+
+ wake-device {
+ [...]
+ interrupt-controller = <&pdc>;
+ interrupt = <2 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+In this case interrupt 514 would be mapped to port 2 on the PDC as defined by
+the qcom,pdc-ranges property.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
From : Archana Sathyakumar <[email protected]>
The Power Domain Controller (PDC) on QTI SoCs like SDM845 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. Only a subset of the processor's interrupts are
routed through the PDC to the GIC. The PDC powers on the processors'
domain, when in low power mode and replays pending interrupts so the GIC
may wake up the processor.
Signed-off-by: Archana Sathyakumar <[email protected]>
Signed-off-by: Lina Iyer <[email protected]>
---
drivers/irqchip/Kconfig | 9 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/qcom-pdc.c | 311 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 321 insertions(+)
create mode 100644 drivers/irqchip/qcom-pdc.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index c70476b34a53..506c6aa7f0b4 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -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 (QTI) mobile chips.
+
endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index d2df34a54d38..280723d83916 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -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
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
new file mode 100644
index 000000000000..376e66c47868
--- /dev/null
+++ b/drivers/irqchip/qcom-pdc.c
@@ -0,0 +1,311 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Coyright (c) 2017-2018, The Linux Foundation. All rights reserved. */
+
+#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>
+
+#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
+
+struct pdc_pin_region {
+ u32 pin_base;
+ u32 parent_base;
+ u32 cnt;
+};
+
+static DEFINE_RAW_SPINLOCK(pdc_lock);
+static void __iomem *pdc_base;
+static struct pdc_pin_region *pdc_region;
+static int pdc_region_cnt;
+
+static void pdc_reg_write(int reg, u32 i, u32 val)
+{
+ writel_relaxed(val, pdc_base + reg + i * sizeof(u32));
+}
+
+static u32 pdc_reg_read(int reg, u32 i)
+{
+ return readl_relaxed(pdc_base + reg + i * sizeof(u32));
+}
+
+static void pdc_enable_intr(struct irq_data *d, bool on)
+{
+ int pin_out = d->hwirq;
+ u32 index, mask;
+ u32 enable;
+
+ index = pin_out / 32;
+ mask = pin_out % 32;
+
+ raw_spin_lock(&pdc_lock);
+ enable = pdc_reg_read(IRQ_ENABLE_BANK, index);
+ enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask);
+ pdc_reg_write(IRQ_ENABLE_BANK, index, enable);
+ raw_spin_unlock(&pdc_lock);
+}
+
+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);
+}
+
+/*
+ * GIC does not handle falling edge or active low. To allow falling edge and
+ * active low interrupts to be handled at GIC, PDC has an inverter that inverts
+ * falling edge into a rising edge and active low into an active high.
+ * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to
+ * set as per the table below.
+ * (polarity, falling edge, rising edge ) POLARITY
+ * 3'b0 00 Level sensitive active low LOW
+ * 3'b0 01 Rising edge sensitive NOT USED
+ * 3'b0 10 Falling edge sensitive LOW
+ * 3'b0 11 Dual Edge sensitive NOT USED
+ * 3'b1 00 Level sensitive active High HIGH
+ * 3'b1 01 Falling Edge sensitive NOT USED
+ * 3'b1 10 Rising edge sensitive HIGH
+ * 3'b1 11 Dual Edge sensitive HIGH
+ */
+enum pdc_irq_config_bits {
+ PDC_POLARITY_LOW = 0,
+ PDC_FALLING_EDGE = 2,
+ PDC_POLARITY_HIGH = 4,
+ PDC_RISING_EDGE = 6,
+ PDC_DUAL_EDGE = 7,
+};
+
+/**
+ * qcom_pdc_gic_set_type: Configure PDC for the interrupt
+ *
+ * @d: the interrupt data
+ * @type: the interrupt type
+ *
+ * If @type is edge triggered, forward that as Rising edge as PDC
+ * takes care of converting falling edge to rising edge signal
+ * If @type is level, then forward that as level high as PDC
+ * takes care of converting falling edge to rising edge signal
+ */
+static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
+{
+ int pin_out = d->hwirq;
+ enum pdc_irq_config_bits pdc_type;
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ pdc_type = PDC_RISING_EDGE;
+ type = IRQ_TYPE_EDGE_RISING;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ pdc_type = PDC_FALLING_EDGE;
+ type = IRQ_TYPE_EDGE_RISING;
+ break;
+ case IRQ_TYPE_EDGE_BOTH:
+ pdc_type = PDC_DUAL_EDGE;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ pdc_type = PDC_POLARITY_HIGH;
+ type = IRQ_TYPE_LEVEL_HIGH;
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ pdc_type = PDC_POLARITY_LOW;
+ type = IRQ_TYPE_LEVEL_HIGH;
+ break;
+ default:
+ WARN_ON(1);
+ return -EINVAL;
+ }
+
+ pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type);
+
+ return irq_chip_set_type_parent(d, type);
+}
+
+static struct irq_chip qcom_pdc_gic_chip = {
+ .name = "PDC",
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_mask = qcom_pdc_gic_mask,
+ .irq_unmask = qcom_pdc_gic_unmask,
+ .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,
+ .irq_set_vcpu_affinity = irq_chip_set_vcpu_affinity_parent,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+};
+
+static irq_hw_number_t get_parent_hwirq(int pin)
+{
+ int i;
+ struct pdc_pin_region *region;
+
+ for (i = 0; i < pdc_region_cnt; i++) {
+ region = &pdc_region[i];
+ if (pin >= region->pin_base &&
+ pin < region->pin_base + region->cnt)
+ return (region->parent_base + pin - region->pin_base);
+ }
+
+ WARN_ON(1);
+ return ~0UL;
+}
+
+static int qcom_pdc_translate(struct irq_domain *d,
+ struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type)
+{
+ if (is_of_node(fwspec->fwnode)) {
+ if (fwspec->param_count != 2)
+ return -EINVAL;
+
+ *hwirq = fwspec->param[0];
+ *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+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, parent_hwirq;
+ unsigned int type;
+ int ret;
+
+ ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
+ if (ret)
+ return -EINVAL;
+
+ parent_hwirq = get_parent_hwirq(hwirq);
+ ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+ &qcom_pdc_gic_chip, NULL);
+ if (ret)
+ return ret;
+
+ if (type & IRQ_TYPE_EDGE_BOTH)
+ type = IRQ_TYPE_EDGE_RISING;
+
+ if (type & IRQ_TYPE_LEVEL_MASK)
+ type = IRQ_TYPE_LEVEL_HIGH;
+
+ parent_fwspec.fwnode = domain->parent->fwnode;
+ parent_fwspec.param_count = 3;
+ parent_fwspec.param[0] = 0;
+ parent_fwspec.param[1] = parent_hwirq;
+ parent_fwspec.param[2] = type;
+
+ 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 int pdc_setup_pin_mapping(struct device_node *np)
+{
+ u32 *region = NULL;
+ int ret, n;
+
+ n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
+ if (n <= 0 || n % 3)
+ return -EINVAL;
+
+ region = kcalloc(n, sizeof(*region), GFP_KERNEL);
+ if (!region)
+ return -ENOMEM;
+
+ ret = of_property_read_u32_array(np, "qcom,pdc-ranges", region, n);
+ if (ret)
+ goto fail;
+
+ pdc_region_cnt = n / 3;
+ pdc_region = kcalloc(pdc_region_cnt, sizeof(*pdc_region), GFP_KERNEL);
+ if (!pdc_region) {
+ pdc_region_cnt = 0;
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+ for (n = 0; n < pdc_region_cnt; n++, region += 3) {
+ pdc_region[n].pin_base = region[0];
+ pdc_region[n].parent_base = region[1];
+ pdc_region[n].cnt = region[2];
+ }
+
+fail:
+ kfree(region);
+ return ret;
+}
+
+static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
+{
+ struct irq_domain *parent_domain, *pdc_domain;
+ int ret;
+
+ pdc_base = of_iomap(node, 0);
+ if (!pdc_base) {
+ pr_err("%pOF: unable to map PDC registers\n", node->full_name);
+ return -ENXIO;
+ }
+
+ parent_domain = irq_find_host(parent);
+ if (!parent_domain) {
+ pr_err("%pOF: unable to find PDC's parent domain\n",
+ node->full_name);
+ ret = -ENXIO;
+ goto failure;
+ }
+
+ ret = pdc_setup_pin_mapping(node);
+ if (ret) {
+ pr_err("%pOF: failed to init PDC pin-hwirq mapping\n",
+ node->full_name);
+ goto failure;
+ }
+
+ pdc_domain = irq_domain_create_hierarchy(parent_domain, 0, PDC_MAX_IRQS,
+ of_fwnode_handle(node),
+ &qcom_pdc_ops, NULL);
+ if (!pdc_domain) {
+ pr_err("%pOF: GIC domain add failed\n", node->full_name);
+ ret = -ENOMEM;
+ goto failure;
+ }
+
+ return 0;
+
+failure:
+ kfree(pdc_region);
+ iounmap(pdc_base);
+ return ret;
+}
+
+IRQCHIP_DECLARE(pdc_sdm845, "qcom,sdm845-pdc", qcom_pdc_init);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
On Wed 07 Feb 07:49 PST 2018, Lina Iyer wrote:
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
[..]
> +Example:
[..]
> + wake-device {
> + [...]
> + interrupt-controller = <&pdc>;
Sorry for not seeing this earlier, but this should be:
interrupt-parent = <&pdc>;
Or as it's not unlikely that clients might use a mix of pdc and non-pdc
interrupts the example could use the form:
interrupts-extended = <&pdc 2 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt = <2 IRQ_TYPE_LEVEL_HIGH>;
> + };
Regards,
Bjorn
On 07/02/18 15:49, Lina Iyer wrote:
> From : Archana Sathyakumar <[email protected]>
>
> The Power Domain Controller (PDC) on QTI SoCs like SDM845 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. Only a subset of the processor's interrupts are
> routed through the PDC to the GIC. The PDC powers on the processors'
> domain, when in low power mode and replays pending interrupts so the GIC
> may wake up the processor.
>
> Signed-off-by: Archana Sathyakumar <[email protected]>
> Signed-off-by: Lina Iyer <[email protected]>
> ---
> drivers/irqchip/Kconfig | 9 ++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/qcom-pdc.c | 311 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 321 insertions(+)
> create mode 100644 drivers/irqchip/qcom-pdc.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index c70476b34a53..506c6aa7f0b4 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -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 (QTI) mobile chips.
> +
> endmenu
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index d2df34a54d38..280723d83916 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -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
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> new file mode 100644
> index 000000000000..376e66c47868
> --- /dev/null
> +++ b/drivers/irqchip/qcom-pdc.c
> @@ -0,0 +1,311 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Coyright (c) 2017-2018, The Linux Foundation. All rights reserved. */
> +
> +#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>
> +
> +#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
> +
> +struct pdc_pin_region {
> + u32 pin_base;
> + u32 parent_base;
> + u32 cnt;
> +};
> +
> +static DEFINE_RAW_SPINLOCK(pdc_lock);
> +static void __iomem *pdc_base;
> +static struct pdc_pin_region *pdc_region;
> +static int pdc_region_cnt;
> +
> +static void pdc_reg_write(int reg, u32 i, u32 val)
> +{
> + writel_relaxed(val, pdc_base + reg + i * sizeof(u32));
> +}
> +
> +static u32 pdc_reg_read(int reg, u32 i)
> +{
> + return readl_relaxed(pdc_base + reg + i * sizeof(u32));
> +}
> +
> +static void pdc_enable_intr(struct irq_data *d, bool on)
> +{
> + int pin_out = d->hwirq;
> + u32 index, mask;
> + u32 enable;
> +
> + index = pin_out / 32;
> + mask = pin_out % 32;
> +
> + raw_spin_lock(&pdc_lock);
> + enable = pdc_reg_read(IRQ_ENABLE_BANK, index);
> + enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask);
> + pdc_reg_write(IRQ_ENABLE_BANK, index, enable);
> + raw_spin_unlock(&pdc_lock);
> +}
> +
> +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);
> +}
> +
> +/*
> + * GIC does not handle falling edge or active low. To allow falling edge and
> + * active low interrupts to be handled at GIC, PDC has an inverter that inverts
> + * falling edge into a rising edge and active low into an active high.
> + * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to
> + * set as per the table below.
> + * (polarity, falling edge, rising edge ) POLARITY
> + * 3'b0 00 Level sensitive active low LOW
> + * 3'b0 01 Rising edge sensitive NOT USED
> + * 3'b0 10 Falling edge sensitive LOW
> + * 3'b0 11 Dual Edge sensitive NOT USED
> + * 3'b1 00 Level sensitive active High HIGH
> + * 3'b1 01 Falling Edge sensitive NOT USED
> + * 3'b1 10 Rising edge sensitive HIGH
> + * 3'b1 11 Dual Edge sensitive HIGH
> + */
> +enum pdc_irq_config_bits {
> + PDC_POLARITY_LOW = 0,
> + PDC_FALLING_EDGE = 2,
> + PDC_POLARITY_HIGH = 4,
> + PDC_RISING_EDGE = 6,
> + PDC_DUAL_EDGE = 7,
> +};
> +
> +/**
> + * qcom_pdc_gic_set_type: Configure PDC for the interrupt
> + *
> + * @d: the interrupt data
> + * @type: the interrupt type
> + *
> + * If @type is edge triggered, forward that as Rising edge as PDC
> + * takes care of converting falling edge to rising edge signal
> + * If @type is level, then forward that as level high as PDC
> + * takes care of converting falling edge to rising edge signal
> + */
> +static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
> +{
> + int pin_out = d->hwirq;
> + enum pdc_irq_config_bits pdc_type;
> +
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + pdc_type = PDC_RISING_EDGE;
> + type = IRQ_TYPE_EDGE_RISING;
> + break;
> + case IRQ_TYPE_EDGE_FALLING:
> + pdc_type = PDC_FALLING_EDGE;
> + type = IRQ_TYPE_EDGE_RISING;
> + break;
> + case IRQ_TYPE_EDGE_BOTH:
> + pdc_type = PDC_DUAL_EDGE;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + pdc_type = PDC_POLARITY_HIGH;
> + type = IRQ_TYPE_LEVEL_HIGH;
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + pdc_type = PDC_POLARITY_LOW;
> + type = IRQ_TYPE_LEVEL_HIGH;
> + break;
> + default:
> + WARN_ON(1);
> + return -EINVAL;
> + }
> +
> + pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type);
> +
> + return irq_chip_set_type_parent(d, type);
> +}
> +
> +static struct irq_chip qcom_pdc_gic_chip = {
> + .name = "PDC",
> + .irq_eoi = irq_chip_eoi_parent,
> + .irq_mask = qcom_pdc_gic_mask,
> + .irq_unmask = qcom_pdc_gic_unmask,
> + .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,
> + .irq_set_vcpu_affinity = irq_chip_set_vcpu_affinity_parent,
> + .irq_set_affinity = irq_chip_set_affinity_parent,
> +};
> +
> +static irq_hw_number_t get_parent_hwirq(int pin)
> +{
> + int i;
> + struct pdc_pin_region *region;
> +
> + for (i = 0; i < pdc_region_cnt; i++) {
> + region = &pdc_region[i];
> + if (pin >= region->pin_base &&
> + pin < region->pin_base + region->cnt)
> + return (region->parent_base + pin - region->pin_base);
> + }
> +
> + WARN_ON(1);
> + return ~0UL;
> +}
> +
> +static int qcom_pdc_translate(struct irq_domain *d,
> + struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type)
> +{
> + if (is_of_node(fwspec->fwnode)) {
> + if (fwspec->param_count != 2)
> + return -EINVAL;
> +
> + *hwirq = fwspec->param[0];
> + *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +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, parent_hwirq;
> + unsigned int type;
> + int ret;
> +
> + ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
> + if (ret)
> + return -EINVAL;
> +
> + parent_hwirq = get_parent_hwirq(hwirq);
Now that you return a well known value on error, how about testing it
and erroring out instead of propagating it to the parent?
> + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> + &qcom_pdc_gic_chip, NULL);
> + if (ret)
> + return ret;
> +
> + if (type & IRQ_TYPE_EDGE_BOTH)
> + type = IRQ_TYPE_EDGE_RISING;
> +
> + if (type & IRQ_TYPE_LEVEL_MASK)
> + type = IRQ_TYPE_LEVEL_HIGH;
> +
> + parent_fwspec.fwnode = domain->parent->fwnode;
> + parent_fwspec.param_count = 3;
> + parent_fwspec.param[0] = 0;
> + parent_fwspec.param[1] = parent_hwirq;
> + parent_fwspec.param[2] = type;
> +
> + 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 int pdc_setup_pin_mapping(struct device_node *np)
> +{
> + u32 *region = NULL;
> + int ret, n;
> +
> + n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
> + if (n <= 0 || n % 3)
> + return -EINVAL;
> +
> + region = kcalloc(n, sizeof(*region), GFP_KERNEL);
> + if (!region)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32_array(np, "qcom,pdc-ranges", region, n);
> + if (ret)
> + goto fail;
> +
> + pdc_region_cnt = n / 3;
> + pdc_region = kcalloc(pdc_region_cnt, sizeof(*pdc_region), GFP_KERNEL);
> + if (!pdc_region) {
> + pdc_region_cnt = 0;
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + for (n = 0; n < pdc_region_cnt; n++, region += 3) {
> + pdc_region[n].pin_base = region[0];
> + pdc_region[n].parent_base = region[1];
> + pdc_region[n].cnt = region[2];
Here's an alternative version that doesn't require any bounce buffer:
for (n = 0; n < pdc_region_cnt; n++) {
ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
n * 3 + 0, &pdc_region[n].pin_base);
if (ret)
goto fail;
ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
n * 3 + 1, &pdc_region[n].parent_base);
if (ret)
goto fail;
ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
n * 3 + 2, &pdc_region[n].cnt);
if (ret)
goto fail;
}
And you can get rid of "region" altogether, because...
> + }
> +
> +fail:
> + kfree(region);
... now that once you've incremented "region" in your for() loop, this
kfree isn't going to do what you think it does.
> + return ret;
> +}
> +
> +static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
> +{
> + struct irq_domain *parent_domain, *pdc_domain;
> + int ret;
> +
> + pdc_base = of_iomap(node, 0);
> + if (!pdc_base) {
> + pr_err("%pOF: unable to map PDC registers\n", node->full_name);
The whole point of the %pOF specifier is that you don't pass the
full_name field, but just the node itself. This is why I pointed you to
commit ce4fecf1fe15 so that you could see how it is used...
> + return -ENXIO;
> + }
> +
> + parent_domain = irq_find_host(parent);
> + if (!parent_domain) {
> + pr_err("%pOF: unable to find PDC's parent domain\n",
> + node->full_name);
> + ret = -ENXIO;
> + goto failure;
> + }
> +
> + ret = pdc_setup_pin_mapping(node);
> + if (ret) {
> + pr_err("%pOF: failed to init PDC pin-hwirq mapping\n",
> + node->full_name);
> + goto failure;
> + }
> +
> + pdc_domain = irq_domain_create_hierarchy(parent_domain, 0, PDC_MAX_IRQS,
> + of_fwnode_handle(node),
> + &qcom_pdc_ops, NULL);
> + if (!pdc_domain) {
> + pr_err("%pOF: GIC domain add failed\n", node->full_name);
> + ret = -ENOMEM;
> + goto failure;
> + }
> +
> + return 0;
> +
> +failure:
> + kfree(pdc_region);
> + iounmap(pdc_base);
> + return ret;
> +}
> +
> +IRQCHIP_DECLARE(pdc_sdm845, "qcom,sdm845-pdc", qcom_pdc_init);
>
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Wed, Feb 07 2018 at 16:43 +0000, Bjorn Andersson wrote:
>On Wed 07 Feb 07:49 PST 2018, Lina Iyer wrote:
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>[..]
>> +Example:
>[..]
>> + wake-device {
>> + [...]
>> + interrupt-controller = <&pdc>;
>
>Sorry for not seeing this earlier, but this should be:
>
> interrupt-parent = <&pdc>;
>
Thats right. Thanks for pointing out.
>Or as it's not unlikely that clients might use a mix of pdc and non-pdc
>interrupts the example could use the form:
>
> interrupts-extended = <&pdc 2 IRQ_TYPE_LEVEL_HIGH>;
>
OK. Will add it as another example.
>> + interrupt = <2 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>
Thanks,
Lina
On Wed, Feb 07 2018 at 16:43 +0000, Marc Zyngier wrote:
>On 07/02/18 15:49, Lina Iyer wrote:
>> From : Archana Sathyakumar <[email protected]>
>>
>> The Power Domain Controller (PDC) on QTI SoCs like SDM845 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. Only a subset of the processor's interrupts are
>> routed through the PDC to the GIC. The PDC powers on the processors'
>> domain, when in low power mode and replays pending interrupts so the GIC
>> may wake up the processor.
>>
>> Signed-off-by: Archana Sathyakumar <[email protected]>
>> Signed-off-by: Lina Iyer <[email protected]>
>> ---
>> + parent_hwirq = get_parent_hwirq(hwirq);
>
>Now that you return a well known value on error, how about testing it
>and erroring out instead of propagating it to the parent?
>
Hmm.. Okay.
>> + region = kcalloc(n, sizeof(*region), GFP_KERNEL);
>> + if (!region)
>> + return -ENOMEM;
>> +
>> + ret = of_property_read_u32_array(np, "qcom,pdc-ranges", region, n);
>> + if (ret)
>> + goto fail;
>> +
>> + pdc_region_cnt = n / 3;
>> + pdc_region = kcalloc(pdc_region_cnt, sizeof(*pdc_region), GFP_KERNEL);
>> + if (!pdc_region) {
>> + pdc_region_cnt = 0;
>> + ret = -ENOMEM;
>> + goto fail;
>> + }
>> +
>> + for (n = 0; n < pdc_region_cnt; n++, region += 3) {
>> + pdc_region[n].pin_base = region[0];
>> + pdc_region[n].parent_base = region[1];
>> + pdc_region[n].cnt = region[2];
>
>Here's an alternative version that doesn't require any bounce buffer:
>
> for (n = 0; n < pdc_region_cnt; n++) {
> ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
> n * 3 + 0, &pdc_region[n].pin_base);
> if (ret)
> goto fail;
> ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
> n * 3 + 1, &pdc_region[n].parent_base);
> if (ret)
> goto fail;
> ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
> n * 3 + 2, &pdc_region[n].cnt);
> if (ret)
> goto fail;
> }
>
>And you can get rid of "region" altogether, because...
>
>> + }
>> +
>> +fail:
>> + kfree(region);
>
>... now that once you've incremented "region" in your for() loop, this
>kfree isn't going to do what you think it does.
>
Sure.
>> + return ret;
>> +}
>> +
>> +static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
>> +{
>> + struct irq_domain *parent_domain, *pdc_domain;
>> + int ret;
>> +
>> + pdc_base = of_iomap(node, 0);
>> + if (!pdc_base) {
>> + pr_err("%pOF: unable to map PDC registers\n", node->full_name);
>
>The whole point of the %pOF specifier is that you don't pass the
>full_name field, but just the node itself. This is why I pointed you to
>commit ce4fecf1fe15 so that you could see how it is used...
>
Oops. I had seen the commit earlier, hence I didn't pay close attention
this time. Sorry, my bad.
Thanks,
Lina