All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] qcom: support wakeup capable GPIOs
@ 2019-01-24 20:21 Lina Iyer
  2019-01-24 20:21 ` [PATCH v2 1/8] gpio: Add support for hierarchical IRQ domains Lina Iyer
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Lina Iyer @ 2019-01-24 20:21 UTC (permalink / raw)
  To: swboyd, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Lina Iyer

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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/8] gpio: Add support for hierarchical IRQ domains
  2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
@ 2019-01-24 20:21 ` Lina Iyer
  2019-01-24 20:21 ` [PATCH v2 2/8] irqdomain: add bus token DOMAIN_BUS_WAKEUP Lina Iyer
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2019-01-24 20:21 UTC (permalink / raw)
  To: swboyd, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Thierry Reding,
	Lina Iyer

From: Thierry Reding <treding@nvidia.com>

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.

Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 drivers/gpio/gpiolib.c      | 15 +++++++++++----
 include/linux/gpio/driver.h |  6 ++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1651d7f0a303..1c3ae1d3c368 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -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;
 
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 07cddbf45186..88ef196f96ec 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -48,6 +48,12 @@ struct gpio_irq_chip {
 	 */
 	const struct irq_domain_ops *domain_ops;
 
+	/**
+	 * @parent_domain:
+	 *
+	 */
+	struct irq_domain *parent_domain;
+
 	/**
 	 * @handler:
 	 *
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/8] irqdomain: add bus token DOMAIN_BUS_WAKEUP
  2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
  2019-01-24 20:21 ` [PATCH v2 1/8] gpio: Add support for hierarchical IRQ domains Lina Iyer
@ 2019-01-24 20:21 ` Lina Iyer
  2019-02-15 21:18     ` Stephen Boyd
  2019-01-24 20:22 ` [PATCH v2 3/8] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs Lina Iyer
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Lina Iyer @ 2019-01-24 20:21 UTC (permalink / raw)
  To: swboyd, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Lina Iyer

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

Suggested-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 include/linux/irqdomain.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 35965f41d7be..05055bc992ab 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -82,6 +82,7 @@ enum irq_domain_bus_token {
 	DOMAIN_BUS_NEXUS,
 	DOMAIN_BUS_IPI,
 	DOMAIN_BUS_FSL_MC_MSI,
+	DOMAIN_BUS_WAKEUP,
 };
 
 /**
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/8] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
  2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
  2019-01-24 20:21 ` [PATCH v2 1/8] gpio: Add support for hierarchical IRQ domains Lina Iyer
  2019-01-24 20:21 ` [PATCH v2 2/8] irqdomain: add bus token DOMAIN_BUS_WAKEUP Lina Iyer
@ 2019-01-24 20:22 ` Lina Iyer
  2019-01-24 20:22 ` [PATCH v2 4/8] dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO Lina Iyer
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2019-01-24 20:22 UTC (permalink / raw)
  To: swboyd, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, 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.

Co-developed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>

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

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index faa7d61b9d6c..eecf5b920250 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -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);
diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
new file mode 100644
index 000000000000..bacc9edbce0d
--- /dev/null
+++ b/include/linux/soc/qcom/irq.h
@@ -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
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/8] dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO
  2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
                   ` (2 preceding siblings ...)
  2019-01-24 20:22 ` [PATCH v2 3/8] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs Lina Iyer
@ 2019-01-24 20:22 ` Lina Iyer
  2019-01-24 20:22 ` [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy Lina Iyer
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2019-01-24 20:22 UTC (permalink / raw)
  To: swboyd, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Lina Iyer, devicetree

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.

Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 .../devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt    | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
index 665aadb5ea28..f0fedbc5d41a 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,sdm845-pinctrl.txt
@@ -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 {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
  2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
                   ` (3 preceding siblings ...)
  2019-01-24 20:22 ` [PATCH v2 4/8] dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO Lina Iyer
@ 2019-01-24 20:22 ` Lina Iyer
  2019-01-30 22:45     ` Stephen Boyd
  2019-01-24 20:22 ` [PATCH v2 6/8] arm64: dts: qcom: add PDC interrupt controller for SDM845 Lina Iyer
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Lina Iyer @ 2019-01-24 20:22 UTC (permalink / raw)
  To: swboyd, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, 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.

Co-developed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>

---
Changes in v2:
	- Fix bug when unmaksing PDC interrupt
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 133 ++++++++++++++++++++++++++---
 1 file changed, 120 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index ee8119879c4c..e13bead566aa 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -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,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/8] arm64: dts: qcom: add PDC interrupt controller for SDM845
  2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
                   ` (4 preceding siblings ...)
  2019-01-24 20:22 ` [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy Lina Iyer
@ 2019-01-24 20:22 ` Lina Iyer
  2019-01-24 20:22 ` [PATCH v2 7/8] arm64: dts: qcom: setup PDC as wakeup parent for GPIOs " Lina Iyer
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2019-01-24 20:22 UTC (permalink / raw)
  To: swboyd, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Lina Iyer

Add PDC interrupt controller device bindings for SDM845.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>

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

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index c27cbd3bcb0a..e55100c2705e 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -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 */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 7/8] arm64: dts: qcom: setup PDC as wakeup parent for GPIOs for SDM845
  2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
                   ` (5 preceding siblings ...)
  2019-01-24 20:22 ` [PATCH v2 6/8] arm64: dts: qcom: add PDC interrupt controller for SDM845 Lina Iyer
@ 2019-01-24 20:22 ` Lina Iyer
  2019-01-24 20:22 ` [PATCH v2 8/8] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845 Lina Iyer
  2019-01-28 14:17 ` [PATCH v2 0/8] qcom: support wakeup capable GPIOs Linus Walleij
  8 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2019-01-24 20:22 UTC (permalink / raw)
  To: swboyd, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Lina Iyer

Setup PDC wakeup parent for TLMM for SDM845 SoC.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index e55100c2705e..89982f6ee147 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -994,6 +994,7 @@
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			wakeup-parent = <&pdc_intc>;
 
 			qup_i2c0_default: qup-i2c0-default {
 				pinmux {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 8/8] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845
  2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
                   ` (6 preceding siblings ...)
  2019-01-24 20:22 ` [PATCH v2 7/8] arm64: dts: qcom: setup PDC as wakeup parent for GPIOs " Lina Iyer
@ 2019-01-24 20:22 ` Lina Iyer
  2019-01-28 14:17 ` [PATCH v2 0/8] qcom: support wakeup capable GPIOs Linus Walleij
  8 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2019-01-24 20:22 UTC (permalink / raw)
  To: swboyd, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, 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.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 2649b7102565..9e7c58803cd5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -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
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/8] qcom: support wakeup capable GPIOs
  2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
                   ` (7 preceding siblings ...)
  2019-01-24 20:22 ` [PATCH v2 8/8] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845 Lina Iyer
@ 2019-01-28 14:17 ` Linus Walleij
  2019-01-30 17:03   ` Lina Iyer
  8 siblings, 1 reply; 16+ messages in thread
From: Linus Walleij @ 2019-01-28 14:17 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Stephen Boyd, Evan Green, Marc Zyngier, linux-kernel, rplsssn,
	linux-arm-msm, thierry.reding, Bjorn Andersson, Doug Anderson

On Thu, Jan 24, 2019 at 9:22 PM Lina Iyer <ilina@codeaurora.org> wrote:

> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/8] qcom: support wakeup capable GPIOs
  2019-01-28 14:17 ` [PATCH v2 0/8] qcom: support wakeup capable GPIOs Linus Walleij
@ 2019-01-30 17:03   ` Lina Iyer
  0 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2019-01-30 17:03 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Stephen Boyd, Evan Green, Marc Zyngier, linux-kernel, rplsssn,
	linux-arm-msm, thierry.reding, Bjorn Andersson, Doug Anderson

On Mon, Jan 28 2019 at 07:19 -0700, Linus Walleij wrote:
>On Thu, Jan 24, 2019 at 9:22 PM Lina Iyer <ilina@codeaurora.org> wrote:
>
>> 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.
>
Yes.

>- 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 :(

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

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

>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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
  2019-01-24 20:22 ` [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy Lina Iyer
@ 2019-01-30 22:45     ` Stephen Boyd
  0 siblings, 0 replies; 16+ messages in thread
From: Stephen Boyd @ 2019-01-30 22:45 UTC (permalink / raw)
  To: evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Lina Iyer

Quoting Lina Iyer (2019-01-24 12:22:02)
> 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.
> 
> Co-developed-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> 
> ---
> 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?

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index dd72ec8fb8db..9b45219893bd 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -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)

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
@ 2019-01-30 22:45     ` Stephen Boyd
  0 siblings, 0 replies; 16+ messages in thread
From: Stephen Boyd @ 2019-01-30 22:45 UTC (permalink / raw)
  To: Lina Iyer, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Lina Iyer

Quoting Lina Iyer (2019-01-24 12:22:02)
> 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.
> 
> Co-developed-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> 
> ---
> 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?

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index dd72ec8fb8db..9b45219893bd 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -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)

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
  2019-01-30 22:45     ` Stephen Boyd
  (?)
@ 2019-01-31 16:34     ` Lina Iyer
  -1 siblings, 0 replies; 16+ messages in thread
From: Lina Iyer @ 2019-01-31 16:34 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: evgreen, marc.zyngier, linux-kernel, rplsssn, linux-arm-msm,
	thierry.reding, bjorn.andersson, dianders, linus.walleij

On Wed, Jan 30 2019 at 15:45 -0700, Stephen Boyd wrote:
>Quoting Lina Iyer (2019-01-24 12:22:02)
>> 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.
>>
>> Co-developed-by: Stephen Boyd <swboyd@chromium.org>
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>>
>> ---
>> 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.

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

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

Thanks,
Lina

>diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
>index dd72ec8fb8db..9b45219893bd 100644
>--- a/drivers/pinctrl/qcom/pinctrl-msm.c
>+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
>@@ -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)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/8] irqdomain: add bus token DOMAIN_BUS_WAKEUP
  2019-01-24 20:21 ` [PATCH v2 2/8] irqdomain: add bus token DOMAIN_BUS_WAKEUP Lina Iyer
@ 2019-02-15 21:18     ` Stephen Boyd
  0 siblings, 0 replies; 16+ messages in thread
From: Stephen Boyd @ 2019-02-15 21:18 UTC (permalink / raw)
  To: evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Lina Iyer

Quoting Lina Iyer (2019-01-24 12:21:59)
> Add new bus token to describe domains that are wakeup capable.
> 
> Suggested-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
>  include/linux/irqdomain.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 35965f41d7be..05055bc992ab 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/8] irqdomain: add bus token DOMAIN_BUS_WAKEUP
@ 2019-02-15 21:18     ` Stephen Boyd
  0 siblings, 0 replies; 16+ messages in thread
From: Stephen Boyd @ 2019-02-15 21:18 UTC (permalink / raw)
  To: Lina Iyer, evgreen, marc.zyngier
  Cc: linux-kernel, rplsssn, linux-arm-msm, thierry.reding,
	bjorn.andersson, dianders, linus.walleij, Lina Iyer

Quoting Lina Iyer (2019-01-24 12:21:59)
> Add new bus token to describe domains that are wakeup capable.
> 
> Suggested-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
>  include/linux/irqdomain.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 35965f41d7be..05055bc992ab 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-02-15 21:18 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-24 20:21 [PATCH v2 0/8] qcom: support wakeup capable GPIOs Lina Iyer
2019-01-24 20:21 ` [PATCH v2 1/8] gpio: Add support for hierarchical IRQ domains Lina Iyer
2019-01-24 20:21 ` [PATCH v2 2/8] irqdomain: add bus token DOMAIN_BUS_WAKEUP Lina Iyer
2019-02-15 21:18   ` Stephen Boyd
2019-02-15 21:18     ` Stephen Boyd
2019-01-24 20:22 ` [PATCH v2 3/8] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs Lina Iyer
2019-01-24 20:22 ` [PATCH v2 4/8] dt-bindings: sdm845-pinctrl: add wakeup interrupt parent for GPIO Lina Iyer
2019-01-24 20:22 ` [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy Lina Iyer
2019-01-30 22:45   ` Stephen Boyd
2019-01-30 22:45     ` Stephen Boyd
2019-01-31 16:34     ` Lina Iyer
2019-01-24 20:22 ` [PATCH v2 6/8] arm64: dts: qcom: add PDC interrupt controller for SDM845 Lina Iyer
2019-01-24 20:22 ` [PATCH v2 7/8] arm64: dts: qcom: setup PDC as wakeup parent for GPIOs " Lina Iyer
2019-01-24 20:22 ` [PATCH v2 8/8] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845 Lina Iyer
2019-01-28 14:17 ` [PATCH v2 0/8] qcom: support wakeup capable GPIOs Linus Walleij
2019-01-30 17:03   ` Lina Iyer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.