Skip to contentSkip to conversation

qcom: support wakeup capable GPIOs

PATCH Superseded Lina Iyer 3 participants 14 messages Jan 24, 2019 – Feb 15, 2019

Subsystems (9)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Lina Iyer

author
#0 lore raw

v2 cover qcom: support wakeup capable GPIOs

Hi all,

This is a bug fix submission of the v1 posted here [1]. The discussion on how to represent the wakeup-parent interrupt controller is on-going [2] here. The reiew comments in [1], from Doug and Stephen are addressed in this patch.

The series attempts to add GPIO chip in hierarchy with PDC interrupt controller that can detect and wakeup the GPIOs routed to it, when the system is suspend/deep sleep mode.

Changes in v2:
- Fix bug related to unmasking PDC interrupt
- Address review comments from Doug and Stepehn
- Rebase on top of 5.0-rc2
- Fix signed-off-by tags
- Enable QCOM_PDC in defconfig

Do note that this patch uses the register address convention updated by Bjorn per [3].

Thanks,
Lina

[1]. https://lkml.org/lkml/2018/12/19/807
[2]. https://lkml.org/lkml/2018/12/19/813
[3]. https://lkml.org/lkml/2019/1/17/924

Lina Iyer (7):
irqdomain: add bus token DOMAIN_BUS_WAKEUP
drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO
drivers: pinctrl: msm: setup GPIO irqchip hierarchy
arm64: dts: qcom: add PDC interrupt controller for SDM845
arm64: dts: qcom: setup PDC as wakeup parent for GPIOs for SDM845
arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845

Thierry Reding (1):
gpio: Add support for hierarchical IRQ domains

 .../bindings/pinctrl/qcom,sdm845-pinctrl.txt  |   7 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |  10 +
 arch/arm64/configs/defconfig                  |   1 +
 drivers/gpio/gpiolib.c                        |  15 +-
 drivers/irqchip/qcom-pdc.c                    | 204 +++++++++++++++++-
 drivers/pinctrl/qcom/pinctrl-msm.c            | 133 ++++++++++--
 include/linux/gpio/driver.h                   |   6 +
 include/linux/irqdomain.h                     |   1 +
 include/linux/soc/qcom/irq.h                  |  23 ++
 9 files changed, 371 insertions(+), 29 deletions(-)
 create mode 100644 include/linux/soc/qcom/irq.h
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#1 lore raw

PATCH v2 1/8 gpio: Add support for hierarchical IRQ domains

From: Thierry Reding <[email protected]>

Hierarchical IRQ domains can be used to stack different IRQ controllers on top of each other. One specific use-case where this can be useful is if a power management controller has top-level controls for wakeup interrupts. In such cases, the power management controller can be a parent to other interrupt controllers and program additional registers when an IRQ has its wake capability enabled or disabled.

---
 drivers/gpio/gpiolib.c      | 15 +++++++++++----
 include/linux/gpio/driver.h |  6 ++++++
 2 files changed, 17 insertions(+), 4 deletions(-)
drivers/gpio/gpiolib.c +11 -4
index 1651d7f0a303..1c3ae1d3c368 100644
@@ -1888,7 +1888,9 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
 		type = IRQ_TYPE_NONE;
 	}
 
-	gpiochip->to_irq = gpiochip_to_irq;
+	if (!gpiochip->to_irq)
+		gpiochip->to_irq = gpiochip_to_irq;
+
 	gpiochip->irq.default_type = type;
 	gpiochip->irq.lock_key = lock_key;
 	gpiochip->irq.request_key = request_key;
@@ -1898,9 +1900,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip,
 	else
 		ops = &gpiochip_domain_ops;
 
-	gpiochip->irq.domain = irq_domain_add_simple(np, gpiochip->ngpio,
-						     gpiochip->irq.first,
-						     ops, gpiochip);
+	if (gpiochip->irq.parent_domain)
+		gpiochip->irq.domain = irq_domain_add_hierarchy(gpiochip->irq.parent_domain,
+								0, gpiochip->ngpio,
+								np, ops, gpiochip);
+	else
+		gpiochip->irq.domain = irq_domain_add_simple(np, gpiochip->ngpio,
+							     gpiochip->irq.first,
+							     ops, gpiochip);
 	if (!gpiochip->irq.domain)
 		return -EINVAL;
 
include/linux/gpio/driver.h +6 -0
index 07cddbf45186..88ef196f96ec 100644
@@ -48,6 +48,12 @@ struct gpio_irq_chip {
 	 */
 	const struct irq_domain_ops *domain_ops;
 
+	/**
+	 * @parent_domain:
+	 *
+	 */
+	struct irq_domain *parent_domain;
+
 	/**
 	 * @handler:
 	 *
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#2 lore raw

PATCH v2 2/8 irqdomain: add bus token DOMAIN_BUS_WAKEUP

In reply to Lina Iyer

Add new bus token to describe domains that are wakeup capable.

---
 include/linux/irqdomain.h | 1 +
 1 file changed, 1 insertion(+)
include/linux/irqdomain.h +1 -0
index 35965f41d7be..05055bc992ab 100644
@@ -82,6 +82,7 @@ enum irq_domain_bus_token {
 	DOMAIN_BUS_NEXUS,
 	DOMAIN_BUS_IPI,
 	DOMAIN_BUS_FSL_MC_MSI,
+	DOMAIN_BUS_WAKEUP,
 };
 
 /**
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#3 lore raw

Re: [PATCH v2 2/8] irqdomain: add bus token DOMAIN_BUS_WAKEUP

Quoting Lina Iyer (2019-01-24 12:21:59)

Quoted 13 lines

Add new bus token to describe domains that are wakeup capable.

---
 include/linux/irqdomain.h | 1 +
 1 file changed, 1 insertion(+)
include/linux/irqdomain.h +0 -0
index 35965f41d7be..05055bc992ab 100644

include/linux/irqdomain.h enum irq_domain_bus_token { · line 87

Quoted 6 lines
include/linux/irqdomain.h +1 -0
@@ -82,6 +82,7 @@ enum irq_domain_bus_token {
        DOMAIN_BUS_NEXUS,
        DOMAIN_BUS_IPI,
        DOMAIN_BUS_FSL_MC_MSI,
+       DOMAIN_BUS_WAKEUP,
 };
 

Marc, can you review this patch? It would be good to know if introducing this token is acceptable, or if we need to come up with some other approach here.

author
#4 lore raw

PATCH v2 3/8 drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs

In reply to Lina Iyer

Introduce a new domain for wakeup capable GPIOs. The domain can be requested using the bus token DOMAIN_BUS_WAKEUP. In the following patches, we will specify PDC as the wakeup-parent for the TLMM GPIO irqchip. Requesting a wakeup GPIO will setup the GPIO and the corresponding PDC interrupt as its parent.

Also, provide the map of the PDC pins for the GPIOs for SDM845.

---
Changes in v2:
- Remove separate file for PDC GPIO map data
- Error checks and return
- Whitespace fixes

---
 drivers/irqchip/qcom-pdc.c   | 204 +++++++++++++++++++++++++++++++++--
 include/linux/soc/qcom/irq.h |  23 ++++
 2 files changed, 216 insertions(+), 11 deletions(-)
 create mode 100644 include/linux/soc/qcom/irq.h
drivers/irqchip/qcom-pdc.c +193 -11
index faa7d61b9d6c..eecf5b920250 100644
@@ -13,12 +13,13 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
+#include <linux/soc/qcom/irq.h>
 #include <linux/spinlock.h>
-#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 
 #define PDC_MAX_IRQS		126
+#define PDC_MAX_GPIO_IRQS	256
 
 #define CLEAR_INTR(reg, intr)	(reg & ~(1 << intr))
 #define ENABLE_INTR(reg, intr)	(reg | (1 << intr))
@@ -32,6 +33,16 @@ struct pdc_pin_region {
 	u32 cnt;
 };
 
+struct pdc_gpio_pin_map {
+	unsigned int gpio;
+	u32 pdc_pin;
+};
+
+struct pdc_gpio_pin_data {
+	size_t size;
+	const struct pdc_gpio_pin_map *map;
+};
+
 static DEFINE_RAW_SPINLOCK(pdc_lock);
 static void __iomem *pdc_base;
 static struct pdc_pin_region *pdc_region;
@@ -47,9 +58,8 @@ 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)
+static void pdc_enable_intr(irq_hw_number_t pin_out, bool on)
 {
-	int pin_out = d->hwirq;
 	u32 index, mask;
 	u32 enable;
 
@@ -65,13 +75,23 @@ static void pdc_enable_intr(struct irq_data *d, bool on)
 
 static void qcom_pdc_gic_mask(struct irq_data *d)
 {
-	pdc_enable_intr(d, false);
+	irq_hw_number_t hwirq = d->hwirq;
+
+	if (hwirq == ULONG_MAX)
+		return;
+
+	pdc_enable_intr(hwirq, false);
 	irq_chip_mask_parent(d);
 }
 
 static void qcom_pdc_gic_unmask(struct irq_data *d)
 {
-	pdc_enable_intr(d, true);
+	irq_hw_number_t hwirq = d->hwirq;
+
+	if (hwirq == ULONG_MAX)
+		return;
+
+	pdc_enable_intr(hwirq, true);
 	irq_chip_unmask_parent(d);
 }
 
@@ -111,9 +131,12 @@ enum pdc_irq_config_bits {
  */
 static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
 {
-	int pin_out = d->hwirq;
+	irq_hw_number_t pin_out = d->hwirq;
 	enum pdc_irq_config_bits pdc_type;
 
+	if (pin_out == ULONG_MAX)
+		return 0;
+
 	switch (type) {
 	case IRQ_TYPE_EDGE_RISING:
 		pdc_type = PDC_EDGE_RISING;
@@ -157,7 +180,7 @@ static struct irq_chip qcom_pdc_gic_chip = {
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 };
 
-static irq_hw_number_t get_parent_hwirq(int pin)
+static irq_hw_number_t get_parent_hwirq(irq_hw_number_t pin)
 {
 	int i;
 	struct pdc_pin_region *region;
@@ -169,7 +192,6 @@ static irq_hw_number_t get_parent_hwirq(int pin)
 			return (region->parent_base + pin - region->pin_base);
 	}
 
-	WARN_ON(1);
 	return ~0UL;
 }
 
@@ -232,6 +254,64 @@ static const struct irq_domain_ops qcom_pdc_ops = {
 	.free		= irq_domain_free_irqs_common,
 };
 
+static irq_hw_number_t qcom_gpio_to_pdc_pin(struct irq_domain *domain,
+					    unsigned int gpio)
+{
+	unsigned int i;
+	const struct pdc_gpio_pin_data *data = domain->host_data;
+
+	if (data) {
+		for (i = 0; i < data->size; i++)
+			if (gpio == data->map[i].gpio)
+				return data->map[i].pdc_pin;
+	}
+
+	return ULONG_MAX;
+}
+
+static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
+			       unsigned int nr_irqs, void *data)
+{
+	struct qcom_irq_fwspec *qcom_fwspec = data;
+	struct irq_fwspec *fwspec = &qcom_fwspec->fwspec;
+	struct irq_fwspec parent_fwspec;
+	irq_hw_number_t hwirq, parent_hwirq;
+	unsigned int type;
+	int ret;
+
+	hwirq = qcom_gpio_to_pdc_pin(domain, fwspec->param[0]);
+	if (hwirq == ULONG_MAX)
+		return 0;
+
+	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;
+
+	qcom_fwspec->mask = true;
+
+	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_gpio_ops = {
+	.alloc		= qcom_pdc_gpio_alloc,
+	.free		= irq_domain_free_irqs_common,
+};
+
 static int pdc_setup_pin_mapping(struct device_node *np)
 {
 	int ret, n;
@@ -268,9 +348,10 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 	return 0;
 }
 
-static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
+static int qcom_pdc_init(struct device_node *node, struct device_node *parent,
+			 struct pdc_gpio_pin_data *data)
 {
-	struct irq_domain *parent_domain, *pdc_domain;
+	struct irq_domain *parent_domain, *pdc_domain, *pdc_gpio_domain;
 	int ret;
 
 	pdc_base = of_iomap(node, 0);
@@ -301,6 +382,18 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 		goto fail;
 	}
 
+	pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain, 0,
+						      PDC_MAX_GPIO_IRQS,
+						      of_fwnode_handle(node),
+						      &qcom_pdc_gpio_ops, data);
+	if (!pdc_gpio_domain) {
+		pr_err("%pOF: GIC domain add failed for GPIO domain\n", node);
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	irq_domain_update_bus_token(pdc_gpio_domain, DOMAIN_BUS_WAKEUP);
+
 	return 0;
 
 fail:
@@ -309,4 +402,93 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 	return ret;
 }
 
-IRQCHIP_DECLARE(pdc_sdm845, "qcom,sdm845-pdc", qcom_pdc_init);
+static const struct pdc_gpio_pin_map sdm845_gpio_pdc_map[] = {
+	{ 1, 30 },
+	{ 3, 31 },
+	{ 5, 32 },
+	{ 10, 33 },
+	{ 11, 34 },
+	{ 20, 35 },
+	{ 22, 36 },
+	{ 24, 37 },
+	{ 26, 38 },
+	{ 30, 39 },
+	{ 31, 117 },
+	{ 32, 41 },
+	{ 34, 42 },
+	{ 36, 43 },
+	{ 37, 44 },
+	{ 38, 45 },
+	{ 39, 46 },
+	{ 40, 47 },
+	{ 41, 115 },
+	{ 43, 49 },
+	{ 44, 50 },
+	{ 46, 51 },
+	{ 48, 52 },
+	{ 49, 118 },
+	{ 52, 54 },
+	{ 53, 55 },
+	{ 54, 56 },
+	{ 56, 57 },
+	{ 57, 58 },
+	{ 58, 59 },
+	{ 59, 60 },
+	{ 60, 61 },
+	{ 61, 62 },
+	{ 62, 63 },
+	{ 63, 64 },
+	{ 64, 65 },
+	{ 66, 66 },
+	{ 68, 67 },
+	{ 71, 68 },
+	{ 73, 69 },
+	{ 77, 70 },
+	{ 78, 71 },
+	{ 79, 72 },
+	{ 80, 73 },
+	{ 84, 74 },
+	{ 85, 75 },
+	{ 86, 76 },
+	{ 88, 77 },
+	{ 89, 116 },
+	{ 91, 79 },
+	{ 92, 80 },
+	{ 95, 81 },
+	{ 96, 82 },
+	{ 97, 83 },
+	{ 101, 84 },
+	{ 103, 85 },
+	{ 104, 86 },
+	{ 115, 90 },
+	{ 116, 91 },
+	{ 117, 92 },
+	{ 118, 93 },
+	{ 119, 94 },
+	{ 120, 95 },
+	{ 121, 96 },
+	{ 122, 97 },
+	{ 123, 98 },
+	{ 124, 99 },
+	{ 125, 100 },
+	{ 127, 102 },
+	{ 128, 103 },
+	{ 129, 104 },
+	{ 130, 105 },
+	{ 132, 106 },
+	{ 133, 107 },
+	{ 145, 108 },
+};
+
+static struct pdc_gpio_pin_data sdm845_gpio_data = {
+	.size = ARRAY_SIZE(sdm845_gpio_pdc_map),
+	.map = sdm845_gpio_pdc_map,
+};
+
+static int qcom_sdm845_pdc_init(struct device_node *node,
+				struct device_node *parent)
+{
+	return qcom_pdc_init(node, parent, &sdm845_gpio_data);
+}
+
+IRQCHIP_DECLARE(pdc_sdm845, "qcom,sdm845-pdc", qcom_sdm845_pdc_init);
include/linux/soc/qcom/irq.h new file +23 -0
new file mode 100644
index 000000000000..bacc9edbce0d
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __QCOM_IRQ_H
+#define __QCOM_IRQ_H
+
+#include <linux/irqdomain.h>
+
+/**
+ * struct qcom_irq_fwspec - qcom specific irq fwspec wrapper
+ * @fwspec: irq fwspec
+ * @mask: if true, keep the irq masked in the gpio controller
+ *
+ * Use this structure to communicate between the parent irq chip, MPM or PDC,
+ * to the gpio chip, TLMM, about the gpio being allocated in the parent
+ * and if the gpio chip should keep the line masked because the parent irq
+ * chip is handling everything about the irq line.
+ */
+struct qcom_irq_fwspec {
+	struct irq_fwspec fwspec;
+	bool mask;
+};
+
+#endif
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#5 lore raw

PATCH v2 4/8 dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO

In reply to Lina Iyer

SDM845 SoC has an always-on interrupt controller (PDC) with select GPIO routed to the PDC as interrupts that can be used to wake the system up from deep low power modes and suspend.

---
 .../devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt    | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt +6 -1
index 665aadb5ea28..f0fedbc5d41a 100644
@@ -29,6 +29,11 @@ SDM845 platform.
 	Definition: must be 2. Specifying the pin number and flags, as defined
 		    in <dt-bindings/interrupt-controller/irq.h>
 
+- wakeup-parent:
+	Usage: optional
+	Value type: <phandle>
+	Definition: A phandle to the wakeup interrupt controller for the SoC.
+
 - gpio-controller:
 	Usage: required
 	Value type: <none>
@@ -53,7 +58,6 @@ pin, a group, or a list of pins or groups. This configuration can include the
 mux function to select on those pin(s)/group(s), and various pin configuration
 parameters, such as pull-up, drive strength, etc.
 
-
 PIN CONFIGURATION NODES:
 
 The name of each subnode is not important; all subnodes should be enumerated
@@ -160,6 +164,7 @@ Example:
 		#gpio-cells = <2>;
 		interrupt-controller;
 		#interrupt-cells = <2>;
+		wake-parent = <&pdc_intc>;
 
 		qup9_active: qup9-active {
 			mux {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#6 lore raw

PATCH v2 5/8 drivers: pinctrl: msm: setup GPIO irqchip hierarchy

In reply to Lina Iyer

To allow GPIOs to wakeup the system from suspend or deep idle, the wakeup capable GPIOs are setup in hierarchy with interrupts from the wakeup-parent irqchip.

In older SoC's, the TLMM will handover detection to the parent irqchip and in newer SoC's, the parent irqchip may also be active as well as the TLMM and therefore the GPIOs need to be masked at TLMM to avoid duplicate interrupts. To enable both these configurations to exist, allow the parent irqchip to dictate the TLMM irqchip's behavior when masking/unmasking the interrupt.

---
Changes in v2:
- Fix bug when unmaksing PDC interrupt

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 133 ++++++++++++++++++++++++++---
 1 file changed, 120 insertions(+), 13 deletions(-)
drivers/pinctrl/qcom/pinctrl-msm.c +120 -13
index ee8119879c4c..e13bead566aa 100644
@@ -17,6 +17,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/pinctrl/machine.h>
 #include <linux/pinctrl/pinctrl.h>
@@ -27,6 +28,7 @@
 #include <linux/gpio/driver.h>
 #include <linux/interrupt.h>
 #include <linux/spinlock.h>
+#include <linux/soc/qcom/irq.h>
 #include <linux/reboot.h>
 #include <linux/pm.h>
 #include <linux/log2.h>
@@ -69,6 +71,7 @@ struct msm_pinctrl {
 
 	DECLARE_BITMAP(dual_edge_irqs, MAX_NR_GPIO);
 	DECLARE_BITMAP(enabled_irqs, MAX_NR_GPIO);
+	DECLARE_BITMAP(wakeup_masked_irqs, MAX_NR_GPIO);
 
 	const struct msm_pinctrl_soc_data *soc;
 	void __iomem *regs[MAX_NR_TILES];
@@ -747,6 +750,13 @@ static void msm_gpio_irq_unmask(struct irq_data *d)
 
 	g = &pctrl->soc->groups[d->hwirq];
 
+	if (d->parent_data)
+		irq_chip_unmask_parent(d);
+
+	/* Monitored by parent wakeup controller? Keep masked */
+	if (test_bit(d->hwirq, pctrl->wakeup_masked_irqs))
+		return;
+
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
 
 	val = msm_readl_intr_cfg(pctrl, g);
@@ -767,6 +777,10 @@ static void msm_gpio_irq_ack(struct irq_data *d)
 	unsigned long flags;
 	u32 val;
 
+	/* Handled by parent wakeup controller? Do nothing */
+	if (test_bit(d->hwirq, pctrl->wakeup_masked_irqs))
+		return;
+
 	g = &pctrl->soc->groups[d->hwirq];
 
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -794,6 +808,13 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 
 	g = &pctrl->soc->groups[d->hwirq];
 
+	if (d->parent_data)
+		irq_chip_set_type_parent(d, type);
+
+	/* Monitored by parent wakeup controller? Keep masked */
+	if (test_bit(d->hwirq, pctrl->wakeup_masked_irqs))
+		return 0;
+
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
 
 	/*
@@ -890,6 +911,9 @@ static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 
 	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
 
+	if (d->parent_data)
+		irq_chip_set_wake_parent(d, on);
+
 	return 0;
 }
 
@@ -967,11 +991,86 @@ static bool msm_gpio_needs_valid_mask(struct msm_pinctrl *pctrl)
 	return device_property_read_u16_array(pctrl->dev, "gpios", NULL, 0) > 0;
 }
 
+static int msm_gpio_domain_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 msm_gpio_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				 unsigned int nr_irqs, void *arg)
+{
+	int ret;
+	irq_hw_number_t hwirq;
+	struct gpio_chip *gc = domain->host_data;
+	struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
+	struct irq_fwspec *fwspec = arg;
+	struct qcom_irq_fwspec parent = { };
+	unsigned int type;
+
+	ret = msm_gpio_domain_translate(domain, fwspec, &hwirq, &type);
+	if (ret)
+		return ret;
+
+	ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+					    &pctrl->irq_chip, gc);
+	if (ret < 0)
+		return ret;
+
+	if (!domain->parent)
+		return 0;
+
+	parent.fwspec.fwnode      = domain->parent->fwnode;
+	parent.fwspec.param_count = 2;
+	parent.fwspec.param[0]    = hwirq;
+	parent.fwspec.param[1]    = type;
+
+	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &parent);
+	if (ret)
+		return ret;
+
+	if (parent.mask)
+		set_bit(hwirq, pctrl->wakeup_masked_irqs);
+
+	return 0;
+}
+
+/*
+ * TODO: Get rid of this and push it into gpiochip_to_irq()
+ */
+static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+	struct irq_fwspec fwspec;
+
+	fwspec.fwnode = of_node_to_fwnode(chip->of_node);
+	fwspec.param[0] = offset;
+	fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
+	fwspec.param_count = 2;
+
+	return irq_create_fwspec_mapping(&fwspec);
+}
+
+static const struct irq_domain_ops msm_gpio_domain_ops = {
+	.translate = msm_gpio_domain_translate,
+	.alloc     = msm_gpio_domain_alloc,
+	.free      = irq_domain_free_irqs_top,
+};
+
 static int msm_gpio_init(struct msm_pinctrl *pctrl)
 {
 	struct gpio_chip *chip;
 	int ret;
 	unsigned ngpio = pctrl->soc->ngpios;
+	struct device_node *dn;
 
 	if (WARN_ON(ngpio > MAX_NR_GPIO))
 		return -EINVAL;
@@ -986,6 +1085,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	chip->need_valid_mask = msm_gpio_needs_valid_mask(pctrl);
 
 	pctrl->irq_chip.name = "msmgpio";
+	pctrl->irq_chip.irq_eoi	= irq_chip_eoi_parent;
 	pctrl->irq_chip.irq_mask = msm_gpio_irq_mask;
 	pctrl->irq_chip.irq_unmask = msm_gpio_irq_unmask;
 	pctrl->irq_chip.irq_ack = msm_gpio_irq_ack;
@@ -994,6 +1094,22 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	pctrl->irq_chip.irq_request_resources = msm_gpio_irq_reqres;
 	pctrl->irq_chip.irq_release_resources = msm_gpio_irq_relres;
 
+	chip->irq.chip = &pctrl->irq_chip;
+	chip->irq.domain_ops = &msm_gpio_domain_ops;
+	chip->irq.handler = handle_edge_irq;
+	chip->irq.default_type = IRQ_TYPE_NONE;
+
+	dn = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
+	if (dn) {
+		chip->irq.parent_domain = irq_find_matching_host(dn,
+						 DOMAIN_BUS_WAKEUP);
+		of_node_put(dn);
+		if (!chip->irq.parent_domain)
+			return -EPROBE_DEFER;
+
+		chip->to_irq = msm_gpio_to_irq;
+	}
+
 	ret = gpiochip_add_data(&pctrl->chip, pctrl);
 	if (ret) {
 		dev_err(pctrl->dev, "Failed register gpiochip\n");
@@ -1015,26 +1131,17 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 			dev_name(pctrl->dev), 0, 0, chip->ngpio);
 		if (ret) {
 			dev_err(pctrl->dev, "Failed to add pin range\n");
-			gpiochip_remove(&pctrl->chip);
-			return ret;
+			goto fail;
 		}
 	}
 
-	ret = gpiochip_irqchip_add(chip,
-				   &pctrl->irq_chip,
-				   0,
-				   handle_edge_irq,
-				   IRQ_TYPE_NONE);
-	if (ret) {
-		dev_err(pctrl->dev, "Failed to add irqchip to gpiochip\n");
-		gpiochip_remove(&pctrl->chip);
-		return -ENOSYS;
-	}
-
 	gpiochip_set_chained_irqchip(chip, &pctrl->irq_chip, pctrl->irq,
 				     msm_gpio_irq_handler);
 
 	return 0;
+fail:
+	gpiochip_remove(&pctrl->chip);
+	return ret;
 }
 
 static int msm_ps_hold_restart(struct notifier_block *nb, unsigned long action,
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#7 lore raw

Re: [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy

Quoting Lina Iyer (2019-01-24 12:22:02)

Quoted 11 lines

To allow GPIOs to wakeup the system from suspend or deep idle, the wakeup capable GPIOs are setup in hierarchy with interrupts from the wakeup-parent irqchip.

In older SoC's, the TLMM will handover detection to the parent irqchip and in newer SoC's, the parent irqchip may also be active as well as the TLMM and therefore the GPIOs need to be masked at TLMM to avoid duplicate interrupts. To enable both these configurations to exist, allow the parent irqchip to dictate the TLMM irqchip's behavior when masking/unmasking the interrupt.

Quoted 6 lines

---
Changes in v2:
- Fix bug when unmaksing PDC interrupt

What was the bug? Is that why the mask callback in this gpio chip no longer calls the parent irq chip? We should keep calling the parent irqchip from what I can tell. Otherwise, we may never mask the irq at the PDC and only mask it at the GPIO level, which may not even care about it if it's being monitored by the PDC.

This causes me to get a bunch of interrupts on my touchscreen when I touch it once vs. only a handful (like 4) when I fix it with the below patch:

Can you fold it in?

drivers/pinctrl/qcom/pinctrl-msm.c +3 -0
index dd72ec8fb8db..9b45219893bd 100644
@@ -682,6 +682,9 @@ static void msm_gpio_irq_mask(struct irq_data *d)
 	clear_bit(d->hwirq, pctrl->enabled_irqs);
 
 	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
+
+	if (d->parent_data)
+		irq_chip_mask_parent(d);
 }
 
 static void msm_gpio_irq_unmask(struct irq_data *d)
author
#8 lore raw

Re: [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy

Quoted 12 lines · Stephen Boyd

Quoting Lina Iyer (2019-01-24 12:22:02)

Quoted 11 lines

To allow GPIOs to wakeup the system from suspend or deep idle, the wakeup capable GPIOs are setup in hierarchy with interrupts from the wakeup-parent irqchip.

In older SoC's, the TLMM will handover detection to the parent irqchip and in newer SoC's, the parent irqchip may also be active as well as the TLMM and therefore the GPIOs need to be masked at TLMM to avoid duplicate interrupts. To enable both these configurations to exist, allow the parent irqchip to dictate the TLMM irqchip's behavior when masking/unmasking the interrupt.

Quoted 8 lines
Quoted 6 lines

---
Changes in v2:
- Fix bug when unmaksing PDC interrupt

What was the bug?

The problem was an incorrect merge (possibly manual), causing the PDC to be not used at all. This is what I had -

static void msm_gpio_irq_mask(struct irq_data *d)                                  {                                                                                  
        struct gpio_chip *gc = irq_data_get_irq_chip_data(d);                      
        struct msm_pinctrl *pctrl = gpiochip_get_data(gc);                         
        const struct msm_pingroup *g;                                              
        unsigned long flags;                                                       
        u32 val;                                                                   
                                                                                   
        g = &pctrl->soc->groups[d->hwirq];                                         
                                                                                   
        if (d->parent_data)                                                        
                irq_chip_unmask_parent(d);                                         
                                                                                   
        /* Monitored by parent wakeup controller? Keep masked */                   
        if (test_bit(d->hwirq, pctrl->wakeup_masked_irqs))                         
                return;                                                            

The above unmask_parent() call in an irq_mask() callback was the bug.

Quoted 10 lines

Is that why the mask callback in this gpio chip no
longer calls the parent irq chip? We should keep calling the parent irqchip from what I can tell. Otherwise, we may never mask the irq at the PDC and only mask it at the GPIO level, which may not even care about it if it's being monitored by the PDC.

This causes me to get a bunch of interrupts on my touchscreen when I touch it once vs. only a handful (like 4) when I fix it with the below patch:

Hmm... I did not see an issue in my local testing :( Thanks for the fix.

Quoted 2 lines

Can you fold it in?

Yes, I will fold it in and send a rev out with the fix.

Thanks,
Lina

Quoted 14 lines
drivers/pinctrl/qcom/pinctrl-msm.c +3 -0
index dd72ec8fb8db..9b45219893bd 100644
@@ -682,6 +682,9 @@ static void msm_gpio_irq_mask(struct irq_data *d)
 	clear_bit(d->hwirq, pctrl->enabled_irqs);
 
 	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
+
+	if (d->parent_data)
+		irq_chip_mask_parent(d);
 }
 
 static void msm_gpio_irq_unmask(struct irq_data *d)
author
#9 lore raw

PATCH v2 6/8 arm64: dts: qcom: add PDC interrupt controller for SDM845

In reply to Lina Iyer

Add PDC interrupt controller device bindings for SDM845.

---
Changes in v2:
- Use updated address specification in reg
- Rename to pdc_intc
- Sort per address in DT

---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm64/boot/dts/qcom/sdm845.dtsi +9 -0
index c27cbd3bcb0a..e55100c2705e 100644
@@ -1534,6 +1534,15 @@
 			#power-domain-cells = <1>;
 		};
 
+		pdc_intc: interrupt-controller@b220000 {
+			compatible = "qcom,sdm845-pdc";
+			reg = <0 0x0b220000 0 0x30000>;
+			qcom,pdc-ranges = <0 480 94>, <94 609 15>, <115 630 7>;
+			#interrupt-cells = <2>;
+			interrupt-parent = <&intc>;
+			interrupt-controller;
+		};
+
 		tsens0: thermal-sensor@c263000 {
 			compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
 			reg = <0xc263000 0x1ff>, /* TM */
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#10 lore raw

PATCH v2 7/8 arm64: dts: qcom: setup PDC as wakeup parent for GPIOs for SDM845

In reply to Lina Iyer

Setup PDC wakeup parent for TLMM for SDM845 SoC.

---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 +
 1 file changed, 1 insertion(+)
arch/arm64/boot/dts/qcom/sdm845.dtsi +1 -0
index e55100c2705e..89982f6ee147 100644
@@ -994,6 +994,7 @@
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			wakeup-parent = <&pdc_intc>;
 
 			qup_i2c0_default: qup-i2c0-default {
 				pinmux {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#11 lore raw

PATCH v2 8/8 arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845

In reply to Lina Iyer

Enable PDC interrupt controller for SDM845 devices. The interrupt controller can detect wakeup capable interrupts when the SoC is in a low power state.

---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)
arch/arm64/configs/defconfig +1 -0
index 2649b7102565..9e7c58803cd5 100644
@@ -658,6 +658,7 @@ CONFIG_QCOM_SMEM=y
 CONFIG_QCOM_SMD_RPM=y
 CONFIG_QCOM_SMP2P=y
 CONFIG_QCOM_SMSM=y
+CONFIG_QCOM_PDC=y
 CONFIG_ROCKCHIP_PM_DOMAINS=y
 CONFIG_ARCH_TEGRA_132_SOC=y
 CONFIG_ARCH_TEGRA_210_SOC=y
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#12 lore raw
In reply to Lina Iyer
Quoted 7 lines · Lina Iyer

This is a bug fix submission of the v1 posted here [1]. The discussion on how to represent the wakeup-parent interrupt controller is on-going [2] here. The reiew comments in [1], from Doug and Stephen are addressed in this patch.

The series attempts to add GPIO chip in hierarchy with PDC interrupt controller that can detect and wakeup the GPIOs routed to it, when the system is suspend/deep sleep mode.

I kind of start to get the hang of this now. This is starting to look finished. Some words on the hierarchical GPIO IRQs:

I have started to look into hierarchical GPIO+irqchip since the usage of such is spreading.

I have been on to Thierry patches trying to make him implement more generic helpers in the gpiolib irqchip library functions.

In short I see the following:

- Hierarchical gpiochips all have .alloc() and .translate() functions that look more or less the same.

- They all fall down to remapping either tuples or entire ranges of IRQs from parent to child with a linear look-up table on allocation.

So my idea would be to add support for this into the gpiolib hierarchical irqchip helper: by supplying a parent irqdomain and a list of translation tuples, we should be able to handle pretty much any hierarchical GPIO irqdomain (famous last words).

Right now I am converting the IXP4xx platform to hierarchical IRQ from the ground up (it is not even using device tree so it is a bit of a challenge) but it seems to be working.

So I will try to post this in some hopefully working form, and on top of those changes or before them introduce some helpers in the core for hierarchical irqs. Or I fail.

Anyways, I do not think my ambitions for refactoring the helpers can stand in the way of support for these use cases and new hardware, so maybe we need to merge a few more hierarchical drivers just bypassing the gpiolib helpers. I don't want to block development, and I suspect Thierry might be getting annoyed at me at this point, so we should maybe just pile up a few more hierarchical chips so I can refactor them later.

What do you think?

Yours,
Linus Walleij

author
#13 lore raw
Quoted 16 lines · Linus Walleij
Quoted 7 lines · Lina Iyer

This is a bug fix submission of the v1 posted here [1]. The discussion on how to represent the wakeup-parent interrupt controller is on-going [2] here. The reiew comments in [1], from Doug and Stephen are addressed in this patch.

The series attempts to add GPIO chip in hierarchy with PDC interrupt controller that can detect and wakeup the GPIOs routed to it, when the system is suspend/deep sleep mode.

I kind of start to get the hang of this now. This is starting to look finished. Some words on the hierarchical GPIO IRQs:

I have started to look into hierarchical GPIO+irqchip since the usage of such is spreading.

Quoted 8 lines

I have been on to Thierry patches trying to make him implement more generic helpers in the gpiolib irqchip library functions.

In short I see the following:

- Hierarchical gpiochips all have .alloc() and .translate() functions that look more or less the same.

Yes.

Quoted 4 lines

- They all fall down to remapping either tuples or entire ranges of IRQs from parent to child with a linear look-up table on allocation.

I would think this would be the generic case, unless its QCOM, where we would want to push the tuples up hierarchy :(

Quoted 6 lines

So my idea would be to add support for this into the gpiolib hierarchical irqchip helper: by supplying a parent irqdomain and a list of translation tuples, we should be able to handle pretty much any hierarchical GPIO irqdomain (famous last words).

We would read the tuples from DT? Also please consider Rob's idea of specifying hierarchy from [2].

Quoted 8 lines

Right now I am converting the IXP4xx platform to hierarchical IRQ from the ground up (it is not even using device tree so it is a bit of a challenge) but it seems to be working.

So I will try to post this in some hopefully working form, and on top of those changes or before them introduce some helpers in the core for hierarchical irqs. Or I fail.

Thanks, please copy me on the thread. I would not want to miss this.

Quoted 11 lines

Anyways, I do not think my ambitions for refactoring the helpers can stand in the way of support for these use cases and new hardware, so maybe we need to merge a few more hierarchical drivers just bypassing the gpiolib helpers. I don't want to block development, and I suspect Thierry might be getting annoyed at me at this point, so we should maybe just pile up a few more hierarchical chips so I can refactor them later.

What do you think?

I attempted refactoring the .alloc and .translate and for a generic case and it seemed like it would fit the bill very well. Will await your patches.

Thanks,
Lina