linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC v2 00/14] Support wakeup capable GPIOs
@ 2019-09-13 21:59 Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 01/14] irqdomain: add bus token DOMAIN_BUS_WAKEUP Lina Iyer
                   ` (13 more replies)
  0 siblings, 14 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

Thanks for all the helpful reviews. Here is the next revision addressing the
comments.

Changes in RFC v2:
	- Address review comments #3, #4, #6, #7, #8, #9, #10
	- Rebased on top of linux-next GPIO latest patches [1],[3],[4]
	- Increase PDC max irqs in #2 (avoid merge conflicts with downstream)
	- Add Reviewed-by #5

Note: This revision does not update writing the config registers that are
written from the PDC. There needs more discussion in that area. #6, #7

---

This series is another attempt on adding wakeup capable GPIOs for QCOM
SoC. This patchset is based on Linus's support for hierarchical GPIOs
merged into linux-next [1]. The essense of the idea remains the same as
the previous submission [2]. GPIO irqchip TLMM is setup in hierarchy wit
the PDC as the wakeup-parent. PDC's interrupt parent is the GIC. GPIOs
in QCOM SoC that are wakeup capable (when TLMM is powered off) are
routed to the PDC as well and can be detected at the always-on interrupt
controller (PDC). The idea is setup the irqchips in hierarchy and if the
interrupt is handled at the PDC, then TLMM relinquishes control and
configuration of the interrupt to the PDC.

There are few new additions in this submission. The first is the
additional SPI configuration that needs to be done to setup the GPIO
type in a register interface between the PDC and the GIC. This is needed
only for GPIOs. This registers in some QCOM SoCs is access restricted
and has to be written from the TZ. The DT bindings are also updated for
this new requirement. The second change is that with the new
hierarchical support in gpiolib, we could remove the .alloc and
.translate functions from the pinctrl driver. But to distinguish the
case where a wakeup interrupt controller needs the TLMM to configure the
GPIO interrupts (in the case of MPM interrupt controller), irqdomain
flags have been added. The third change is ensure the interrupt
controllers' interrupt pending bits are cleared when the GPIO is enabled
as an interrupt.

Please consider reviewing these patches.

Thanks,
Lina

[1]. https://lore.kernel.org/linux-gpio/20190808123242.5359-1-linus.walleij@linaro.org/
[2]. https://lkml.org/lkml/2019/5/7/1173
[3]. https://lore.kernel.org/r/20190819084904.30027-1-linus.walleij@linaro.org
[4]. https://lore.kernel.org/r/20190724083828.7496-1-linus.walleij@linaro.org


Lina Iyer (12):
  irqdomain: add bus token DOMAIN_BUS_WAKEUP
  drivers: irqchip: qcom-pdc: update max PDC interrupts
  drivers: irqchip: pdc: Do not toggle IRQ_ENABLE during mask/unmask
  drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
  of: irq: document properties for wakeup interrupt parent
  dt-bindings/interrupt-controller: pdc: add SPI config register
  drivers: irqchip: pdc: additionally set type in SPI config registers
  drivers: pinctrl: msm: setup GPIO chip in hierarchy
  drivers: pinctrl: sdm845: add PDC wakeup interrupt map for GPIOs
  arm64: dts: qcom: add PDC interrupt controller for SDM845
  arm64: dts: qcom: setup PDC as the wakeup parent for TLMM on SDM845
  arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845

Maulik Shah (2):
  genirq: Introduce irq_chip_get/set_parent_state calls
  drivers: irqchip: pdc: Add irqchip set/get state calls

 .../bindings/interrupt-controller/interrupts.txt   |  13 ++
 .../bindings/interrupt-controller/qcom,pdc.txt     |  13 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi               |  11 +
 arch/arm64/configs/defconfig                       |   1 +
 drivers/irqchip/qcom-pdc.c                         | 238 +++++++++++++++++++--
 drivers/pinctrl/qcom/pinctrl-msm.c                 | 119 +++++++++++
 drivers/pinctrl/qcom/pinctrl-msm.h                 |  16 ++
 drivers/pinctrl/qcom/pinctrl-sdm845.c              |  23 +-
 include/linux/irq.h                                |   6 +
 include/linux/irqdomain.h                          |   1 +
 include/linux/soc/qcom/irq.h                       |  32 +++
 kernel/irq/chip.c                                  |  44 ++++
 12 files changed, 502 insertions(+), 15 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] 32+ messages in thread

* [PATCH RFC v2 01/14] irqdomain: add bus token DOMAIN_BUS_WAKEUP
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 02/14] drivers: irqchip: qcom-pdc: update max PDC interrupts Lina Iyer
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

A single controller can handle normal interrupts and wake-up interrupts
independently, with a different numbering space. It is thus crucial to
allow the driver for such a controller discriminate between the two.

A simple way to do so is to tag the wake-up irqdomain with a "bus token"
that indicates the wake-up domain. This slightly abuses the notion of
bus, but also radically simplifies the design of such a driver. Between
two evils, we choose the least damaging.

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 07ec8b3..cc846ab 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -83,6 +83,7 @@ enum irq_domain_bus_token {
 	DOMAIN_BUS_IPI,
 	DOMAIN_BUS_FSL_MC_MSI,
 	DOMAIN_BUS_TI_SCI_INTA_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] 32+ messages in thread

* [PATCH RFC v2 02/14] drivers: irqchip: qcom-pdc: update max PDC interrupts
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 01/14] irqdomain: add bus token DOMAIN_BUS_WAKEUP Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 03/14] drivers: irqchip: pdc: Do not toggle IRQ_ENABLE during mask/unmask Lina Iyer
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

Newer SoCs have increased the number of interrupts routed to the PDC
interrupt controller. Update the definition of max PDC interrupts.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 drivers/irqchip/qcom-pdc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index faa7d61..b230794 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
  */
 
 #include <linux/err.h>
@@ -18,7 +18,7 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
-#define PDC_MAX_IRQS		126
+#define PDC_MAX_IRQS		168
 
 #define CLEAR_INTR(reg, intr)	(reg & ~(1 << intr))
 #define ENABLE_INTR(reg, intr)	(reg | (1 << intr))
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH RFC v2 03/14] drivers: irqchip: pdc: Do not toggle IRQ_ENABLE during mask/unmask
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 01/14] irqdomain: add bus token DOMAIN_BUS_WAKEUP Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 02/14] drivers: irqchip: qcom-pdc: update max PDC interrupts Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs Lina Iyer
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

When an interrupt is to be serviced, the convention is to mask the
interrupt at the chip and unmask after servicing the interrupt. Enabling
and disabling the interrupt at the PDC irqchip causes an interrupt storm
due to the way dual edge interrupts are handled in hardware.

Skip configuring the PDC when the IRQ is masked and unmasked, instead
use the irq_enable/irq_disable callbacks to toggle the IRQ_ENABLE
register at the PDC. The PDC's IRQ_ENABLE register is only used during
the monitoring mode when the system is asleep and is not needed for
active mode detection.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 drivers/irqchip/qcom-pdc.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index b230794..5eef5ea 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -63,15 +63,25 @@ static void pdc_enable_intr(struct irq_data *d, bool on)
 	raw_spin_unlock(&pdc_lock);
 }
 
-static void qcom_pdc_gic_mask(struct irq_data *d)
+static void qcom_pdc_gic_disable(struct irq_data *d)
 {
 	pdc_enable_intr(d, false);
+	irq_chip_disable_parent(d);
+}
+
+static void qcom_pdc_gic_enable(struct irq_data *d)
+{
+	pdc_enable_intr(d, true);
+	irq_chip_enable_parent(d);
+}
+
+static void qcom_pdc_gic_mask(struct irq_data *d)
+{
 	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);
 }
 
@@ -148,6 +158,8 @@ static struct irq_chip qcom_pdc_gic_chip = {
 	.irq_eoi		= irq_chip_eoi_parent,
 	.irq_mask		= qcom_pdc_gic_mask,
 	.irq_unmask		= qcom_pdc_gic_unmask,
+	.irq_disable		= qcom_pdc_gic_disable,
+	.irq_enable		= qcom_pdc_gic_enable,
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_set_type		= qcom_pdc_gic_set_type,
 	.flags			= IRQCHIP_MASK_ON_SUSPEND |
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (2 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 03/14] drivers: irqchip: pdc: Do not toggle IRQ_ENABLE during mask/unmask Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-11-08 21:21   ` Doug Anderson
  2019-09-13 21:59 ` [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent Lina Iyer
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	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.

Co-developed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
Changes in RFC v2:
	- Move irq_domain_qcom_handle_wakeup to the patch where it is
	used
	- Replace #define definitons
	- Add Signed-off-by and other minor changes
---
 drivers/irqchip/qcom-pdc.c   | 104 +++++++++++++++++++++++++++++++++++++++----
 include/linux/soc/qcom/irq.h |  19 ++++++++
 2 files changed, 114 insertions(+), 9 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 5eef5ea..4abd775 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		168
+#define PDC_MAX_GPIO_IRQS	256
 
 #define CLEAR_INTR(reg, intr)	(reg & ~(1 << intr))
 #define ENABLE_INTR(reg, intr)	(reg | (1 << intr))
@@ -26,6 +27,8 @@
 #define IRQ_ENABLE_BANK		0x10
 #define IRQ_i_CFG		0x110
 
+#define PDC_NO_PARENT_IRQ	~0UL
+
 struct pdc_pin_region {
 	u32 pin_base;
 	u32 parent_base;
@@ -65,23 +68,35 @@ static void pdc_enable_intr(struct irq_data *d, bool on)
 
 static void qcom_pdc_gic_disable(struct irq_data *d)
 {
+	if (d->hwirq == GPIO_NO_WAKE_IRQ)
+		return;
+
 	pdc_enable_intr(d, false);
 	irq_chip_disable_parent(d);
 }
 
 static void qcom_pdc_gic_enable(struct irq_data *d)
 {
+	if (d->hwirq == GPIO_NO_WAKE_IRQ)
+		return;
+
 	pdc_enable_intr(d, true);
 	irq_chip_enable_parent(d);
 }
 
 static void qcom_pdc_gic_mask(struct irq_data *d)
 {
+	if (d->hwirq == GPIO_NO_WAKE_IRQ)
+		return;
+
 	irq_chip_mask_parent(d);
 }
 
 static void qcom_pdc_gic_unmask(struct irq_data *d)
 {
+	if (d->hwirq == GPIO_NO_WAKE_IRQ)
+		return;
+
 	irq_chip_unmask_parent(d);
 }
 
@@ -124,6 +139,9 @@ 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;
 
+	if (pin_out == GPIO_NO_WAKE_IRQ)
+		return 0;
+
 	switch (type) {
 	case IRQ_TYPE_EDGE_RISING:
 		pdc_type = PDC_EDGE_RISING;
@@ -181,8 +199,7 @@ static irq_hw_number_t get_parent_hwirq(int pin)
 			return (region->parent_base + pin - region->pin_base);
 	}
 
-	WARN_ON(1);
-	return ~0UL;
+	return PDC_NO_PARENT_IRQ;
 }
 
 static int qcom_pdc_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
@@ -211,17 +228,17 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
 
 	ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
 	if (ret)
-		return -EINVAL;
-
-	parent_hwirq = get_parent_hwirq(hwirq);
-	if (parent_hwirq == ~0UL)
-		return -EINVAL;
+		return ret;
 
 	ret  = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
 					     &qcom_pdc_gic_chip, NULL);
 	if (ret)
 		return ret;
 
+	parent_hwirq = get_parent_hwirq(hwirq);
+	if (parent_hwirq == PDC_NO_PARENT_IRQ)
+		return 0;
+
 	if (type & IRQ_TYPE_EDGE_BOTH)
 		type = IRQ_TYPE_EDGE_RISING;
 
@@ -244,6 +261,60 @@ static const struct irq_domain_ops qcom_pdc_ops = {
 	.free		= irq_domain_free_irqs_common,
 };
 
+static int qcom_pdc_gpio_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 ret;
+
+	ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+					    &qcom_pdc_gic_chip, NULL);
+	if (ret)
+		return ret;
+
+	if (hwirq == GPIO_NO_WAKE_IRQ)
+		return 0;
+
+	parent_hwirq = get_parent_hwirq(hwirq);
+	if (parent_hwirq == PDC_NO_PARENT_IRQ)
+		return 0;
+
+	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 int qcom_pdc_gpio_domain_select(struct irq_domain *d,
+				       struct irq_fwspec *fwspec,
+				       enum irq_domain_bus_token bus_token)
+{
+	return bus_token == DOMAIN_BUS_WAKEUP;
+}
+
+static const struct irq_domain_ops qcom_pdc_gpio_ops = {
+	.select		= qcom_pdc_gpio_domain_select,
+	.alloc		= qcom_pdc_gpio_alloc,
+	.free		= irq_domain_free_irqs_common,
+};
+
 static int pdc_setup_pin_mapping(struct device_node *np)
 {
 	int ret, n;
@@ -282,7 +353,7 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 
 static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 {
-	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);
@@ -313,8 +384,23 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 		goto fail;
 	}
 
+	pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain,
+						      IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
+						      PDC_MAX_GPIO_IRQS,
+						      of_fwnode_handle(node),
+						      &qcom_pdc_gpio_ops, NULL);
+	if (!pdc_gpio_domain) {
+		pr_err("%pOF: PDC domain add failed for GPIO domain\n", node);
+		ret = -ENOMEM;
+		goto remove;
+	}
+
+	irq_domain_update_bus_token(pdc_gpio_domain, DOMAIN_BUS_WAKEUP);
+
 	return 0;
 
+remove:
+	irq_domain_remove(pdc_domain);
 fail:
 	kfree(pdc_region);
 	iounmap(pdc_base);
diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
new file mode 100644
index 0000000..85ac4b6
--- /dev/null
+++ b/include/linux/soc/qcom/irq.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __QCOM_IRQ_H
+#define __QCOM_IRQ_H
+
+#define GPIO_NO_WAKE_IRQ	~0U
+
+/**
+ * QCOM specific IRQ domain flags that distinguishes the handling of wakeup
+ * capable interrupts by different interrupt controllers.
+ *
+ * IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP: Line must be masked at TLMM and the
+ *                                  interrupt configuration is done at PDC
+ * IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP: Interrupt configuration is handled at TLMM
+ */
+#define IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP		(IRQ_DOMAIN_FLAG_NONCORE << 0)
+#define IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP		(IRQ_DOMAIN_FLAG_NONCORE << 1)
+
+#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] 32+ messages in thread

* [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (3 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-10-03 12:02   ` Linus Walleij
  2019-11-08 21:29   ` Doug Anderson
  2019-09-13 21:59 ` [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register Lina Iyer
                   ` (8 subsequent siblings)
  13 siblings, 2 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer, devicetree

Some interrupt controllers in a SoC, are always powered on and have a
select interrupts routed to them, so that they can wakeup the SoC from
suspend. Add wakeup-parent DT property to refer to these interrupt
controllers.

Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/interrupt-controller/interrupts.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
index 8a3c408..c10e310 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
@@ -108,3 +108,16 @@ commonly used:
 			sensitivity = <7>;
 		};
 	};
+
+3) Interrupt wakeup parent
+--------------------------
+
+Some interrupt controllers in a SoC, are always powered on and have a select
+interrupts routed to them, so that they can wakeup the SoC from suspend. These
+interrupt controllers do not fall into the category of a parent interrupt
+controller and can be specified by the "wakeup-parent" property and contain a
+single phandle referring to the wakeup capable interrupt controller.
+
+   Example:
+	wakeup-parent = <&pdc_intc>;
+
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (4 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-09-30 22:14   ` Rob Herring
  2019-09-30 22:33   ` Stephen Boyd
  2019-09-13 21:59 ` [PATCH RFC v2 07/14] drivers: irqchip: pdc: additionally set type in SPI config registers Lina Iyer
                   ` (7 subsequent siblings)
  13 siblings, 2 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer, devicetree

In addition to configuring the PDC, additional registers that interface
the GIC have to be configured to match the GPIO type. The registers on
some QCOM SoCs are access restricted, while on other SoCs are not. They
SoCs with access restriction to these SPI registers need to be written
from the firmware using the SCM interface. Add a flag to indicate if the
register is to be written using SCM interface.

Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 .../devicetree/bindings/interrupt-controller/qcom,pdc.txt   | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
index 8e0797c..e329f8d 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
@@ -24,6 +24,9 @@ Properties:
 	Usage: required
 	Value type: <prop-encoded-array>
 	Definition: Specifies the base physical address for PDC hardware.
+		    Optionally, specify the PDC's GIC interface registers that
+		    need to be configured for wakeup capable GPIOs routed to
+		    the PDC.
 
 - interrupt-cells:
 	Usage: required
@@ -50,15 +53,23 @@ Properties:
 		    The second element is the GIC hwirq number for the PDC port.
 		    The third element is the number of interrupts in sequence.
 
+- qcom,scm-spi-cfg:
+	Usage: optional
+	Value type: <bool>
+	Definition: Specifies if the SPI configuration registers have to be
+		    written from the firmware. Sometimes the PDC interface
+		    register to the GIC can only be written from the firmware.
+
 Example:
 
 	pdc: interrupt-controller@b220000 {
 		compatible = "qcom,sdm845-pdc";
-		reg = <0xb220000 0x30000>;
+		reg = <0 0x0b220000 0 0x30000>, <0 0x179900f0 0 0x60>;
 		qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
 		#interrupt-cells = <2>;
 		interrupt-parent = <&intc>;
 		interrupt-controller;
+		qcom,scm-spi-cfg;
 	};
 
 DT binding of a device that wants to use the GIC SPI 514 as a 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] 32+ messages in thread

* [PATCH RFC v2 07/14] drivers: irqchip: pdc: additionally set type in SPI config registers
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (5 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 08/14] genirq: Introduce irq_chip_get/set_parent_state calls Lina Iyer
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

GPIOs that can be configured as wakeup are routed to the PDC wakeup
interrupt controller and from there to the GIC interrupt controller. On
some QCOM SoCs, the interface to the GIC for wakeup capable GPIOs have
additional hardware registers that need to be configured as well to
match the trigger type of the GPIO. This register interfaces the PDC to
the GIC and therefore updated from the PDC driver.

Typically, the firmware intializes the interface registers for the
wakeup capable GPIOs with commonly used GPIO trigger type, but it is
possible that a platform may want to use the GPIO differently. So, in
addition to configuring the PDC, configure the interface registers as
well.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 drivers/irqchip/qcom-pdc.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 4abd775..affb0bfa 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -18,6 +18,8 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+#include <linux/qcom_scm.h>
+
 #define PDC_MAX_IRQS		168
 #define PDC_MAX_GPIO_IRQS	256
 
@@ -35,10 +37,20 @@ struct pdc_pin_region {
 	u32 cnt;
 };
 
+struct spi_cfg_regs {
+	union {
+		u64 start;
+		void __iomem *base;
+	};
+	resource_size_t size;
+	bool scm_io;
+};
+
 static DEFINE_RAW_SPINLOCK(pdc_lock);
 static void __iomem *pdc_base;
 static struct pdc_pin_region *pdc_region;
 static int pdc_region_cnt;
+static struct spi_cfg_regs *spi_cfg;
 
 static void pdc_reg_write(int reg, u32 i, u32 val)
 {
@@ -100,6 +112,57 @@ static void qcom_pdc_gic_unmask(struct irq_data *d)
 	irq_chip_unmask_parent(d);
 }
 
+static u32 __spi_pin_read(unsigned int pin)
+{
+	void __iomem *cfg_reg = spi_cfg->base + pin * 4;
+	u64 scm_cfg_reg = spi_cfg->start + pin * 4;
+
+	if (spi_cfg->scm_io) {
+		unsigned int val;
+
+		qcom_scm_io_readl(scm_cfg_reg, &val);
+		return val;
+	} else {
+		return readl(cfg_reg);
+	}
+}
+
+static void __spi_pin_write(unsigned int pin, unsigned int val)
+{
+	void __iomem *cfg_reg = spi_cfg->base + pin * 4;
+	u64 scm_cfg_reg = spi_cfg->start + pin * 4;
+
+	if (spi_cfg->scm_io)
+		qcom_scm_io_writel(scm_cfg_reg, val);
+	else
+		writel(val, cfg_reg);
+}
+
+static int spi_configure_type(irq_hw_number_t hwirq, unsigned int type)
+{
+	int spi = hwirq - 32;
+	u32 pin = spi / 32;
+	u32 mask = BIT(spi % 32);
+	u32 val;
+	unsigned long flags;
+
+	if (!spi_cfg)
+		return 0;
+
+	if (pin * 4 > spi_cfg->size)
+		return -EFAULT;
+
+	raw_spin_lock_irqsave(&pdc_lock, flags);
+	val = __spi_pin_read(pin);
+	val &= ~mask;
+	if (type & IRQ_TYPE_LEVEL_MASK)
+		val |= mask;
+	__spi_pin_write(pin, val);
+	raw_spin_unlock_irqrestore(&pdc_lock, flags);
+
+	return 0;
+}
+
 /*
  * 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
@@ -137,7 +200,9 @@ enum pdc_irq_config_bits {
 static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
 {
 	int pin_out = d->hwirq;
+	int parent_hwirq = d->parent_data->hwirq;
 	enum pdc_irq_config_bits pdc_type;
+	int ret;
 
 	if (pin_out == GPIO_NO_WAKE_IRQ)
 		return 0;
@@ -168,6 +233,11 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
 
 	pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type);
 
+	/* Additionally, configure (only) the GPIO in the f/w */
+	ret = spi_configure_type(parent_hwirq, type);
+	if (ret)
+		return ret;
+
 	return irq_chip_set_type_parent(d, type);
 }
 
@@ -354,6 +424,7 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 {
 	struct irq_domain *parent_domain, *pdc_domain, *pdc_gpio_domain;
+	struct resource res;
 	int ret;
 
 	pdc_base = of_iomap(node, 0);
@@ -384,6 +455,27 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 		goto fail;
 	}
 
+	ret = of_address_to_resource(node, 1, &res);
+	if (!ret) {
+		spi_cfg = kcalloc(1, sizeof(*spi_cfg), GFP_KERNEL);
+		if (!spi_cfg) {
+			ret = -ENOMEM;
+			goto remove;
+		}
+		spi_cfg->scm_io = of_find_property(node,
+						   "qcom,scm-spi-cfg", NULL);
+		spi_cfg->size = resource_size(&res);
+		if (spi_cfg->scm_io) {
+			spi_cfg->start = res.start;
+		} else {
+			spi_cfg->base = ioremap(res.start, spi_cfg->size);
+			if (!spi_cfg->base) {
+				ret = -ENOMEM;
+				goto remove;
+			}
+		}
+	}
+
 	pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain,
 						      IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
 						      PDC_MAX_GPIO_IRQS,
@@ -401,6 +493,7 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 
 remove:
 	irq_domain_remove(pdc_domain);
+	kfree(spi_cfg);
 fail:
 	kfree(pdc_region);
 	iounmap(pdc_base);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH RFC v2 08/14] genirq: Introduce irq_chip_get/set_parent_state calls
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (6 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 07/14] drivers: irqchip: pdc: additionally set type in SPI config registers Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 09/14] drivers: irqchip: pdc: Add irqchip set/get state calls Lina Iyer
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

From: Maulik Shah <mkshah@codeaurora.org>

On certain QTI chipsets some GPIOs are direct-connect interrupts to the
GIC to be used as regular interrupt lines. When the GPIOs are not used
for interrupt generation the interrupt line is disabled. But disabling
the interrupt at GIC does not prevent the interrupt to be reported as
pending at GIC_ISPEND. Later, when drivers call enable_irq() on the
interrupt, an unwanted interrupt occurs.

Introduce get and set methods for irqchip's parent to clear it's pending
irq state. This then can be invoked by the GPIO interrupt controller on
the parents in it hierarchy to clear the interrupt before enabling the
interrupt.

Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
[updated commit text and minor code fixes]
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
Changes in RFC v2 -
	- Rephrase commit text
	- Address code review comments
---
 include/linux/irq.h |  6 ++++++
 kernel/irq/chip.c   | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index fb301cf..7853eb9 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -610,6 +610,12 @@ extern int irq_chip_pm_put(struct irq_data *data);
 #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
 extern void handle_fasteoi_ack_irq(struct irq_desc *desc);
 extern void handle_fasteoi_mask_irq(struct irq_desc *desc);
+extern int irq_chip_set_parent_state(struct irq_data *data,
+				     enum irqchip_irq_state which,
+				     bool val);
+extern int irq_chip_get_parent_state(struct irq_data *data,
+				     enum irqchip_irq_state which,
+				     bool *state);
 extern void irq_chip_enable_parent(struct irq_data *data);
 extern void irq_chip_disable_parent(struct irq_data *data);
 extern void irq_chip_ack_parent(struct irq_data *data);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index b76703b..b3fa2d8 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1298,6 +1298,50 @@ EXPORT_SYMBOL_GPL(handle_fasteoi_mask_irq);
 #endif /* CONFIG_IRQ_FASTEOI_HIERARCHY_HANDLERS */
 
 /**
+ * irq_chip_set_parent_state - set the state of a parent interrupt.
+ *
+ * @data: Pointer to interrupt specific data
+ * @which: State to be restored (one of IRQCHIP_STATE_*)
+ * @val: Value corresponding to @which
+ *
+ * Conditional success, if the underlying irqchip does not implement it.
+ */
+int irq_chip_set_parent_state(struct irq_data *data,
+			      enum irqchip_irq_state which,
+			      bool val)
+{
+	data = data->parent_data;
+
+	if (!data || !data->chip->irq_set_irqchip_state)
+		return 0;
+
+	return data->chip->irq_set_irqchip_state(data, which, val);
+}
+EXPORT_SYMBOL_GPL(irq_chip_set_parent_state);
+
+/**
+ * irq_chip_get_parent_state - get the state of a parent interrupt.
+ *
+ * @data: Pointer to interrupt specific data
+ * @which: one of IRQCHIP_STATE_* the caller wants to know
+ * @state: a pointer to a boolean where the state is to be stored
+ *
+ * Conditional success, if the underlying irqchip does not implement it.
+ */
+int irq_chip_get_parent_state(struct irq_data *data,
+			      enum irqchip_irq_state which,
+			      bool *state)
+{
+	data = data->parent_data;
+
+	if (!data || !data->chip->irq_get_irqchip_state)
+		return 0;
+
+	return data->chip->irq_get_irqchip_state(data, which, state);
+}
+EXPORT_SYMBOL_GPL(irq_chip_get_parent_state);
+
+/**
  * irq_chip_enable_parent - Enable the parent interrupt (defaults to unmask if
  * NULL)
  * @data:	Pointer to interrupt specific data
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH RFC v2 09/14] drivers: irqchip: pdc: Add irqchip set/get state calls
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (7 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 08/14] genirq: Introduce irq_chip_get/set_parent_state calls Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 10/14] drivers: pinctrl: msm: setup GPIO chip in hierarchy Lina Iyer
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

From: Maulik Shah <mkshah@codeaurora.org>

Add irqchip calls to set/get interrupt state from the parent interrupt
controller. When GPIOs are renabled as interrupt lines, it is desirable
to clear the interrupt state at the GIC. This avoids any unwanted
interrupt as a result of stale pending state recorded when the line was
used as a GPIO.

Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
[updated commit text]
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 drivers/irqchip/qcom-pdc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index affb0bfa..2b49e70 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -5,6 +5,7 @@
 
 #include <linux/err.h>
 #include <linux/init.h>
+#include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/irqchip.h>
 #include <linux/irqdomain.h>
@@ -87,6 +88,24 @@ static void qcom_pdc_gic_disable(struct irq_data *d)
 	irq_chip_disable_parent(d);
 }
 
+static int qcom_pdc_gic_get_irqchip_state(struct irq_data *d,
+		enum irqchip_irq_state which, bool *state)
+{
+	if (d->hwirq == GPIO_NO_WAKE_IRQ)
+		return 0;
+
+	return irq_chip_get_parent_state(d, which, state);
+}
+
+static int qcom_pdc_gic_set_irqchip_state(struct irq_data *d,
+		enum irqchip_irq_state which, bool value)
+{
+	if (d->hwirq == GPIO_NO_WAKE_IRQ)
+		return 0;
+
+	return irq_chip_set_parent_state(d, which, value);
+}
+
 static void qcom_pdc_gic_enable(struct irq_data *d)
 {
 	if (d->hwirq == GPIO_NO_WAKE_IRQ)
@@ -248,6 +267,8 @@ static struct irq_chip qcom_pdc_gic_chip = {
 	.irq_unmask		= qcom_pdc_gic_unmask,
 	.irq_disable		= qcom_pdc_gic_disable,
 	.irq_enable		= qcom_pdc_gic_enable,
+	.irq_get_irqchip_state	= qcom_pdc_gic_get_irqchip_state,
+	.irq_set_irqchip_state	= qcom_pdc_gic_set_irqchip_state,
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_set_type		= qcom_pdc_gic_set_type,
 	.flags			= IRQCHIP_MASK_ON_SUSPEND |
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH RFC v2 10/14] drivers: pinctrl: msm: setup GPIO chip in hierarchy
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (8 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 09/14] drivers: irqchip: pdc: Add irqchip set/get state calls Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-10-03 12:17   ` Linus Walleij
  2019-09-13 21:59 ` [PATCH RFC v2 11/14] drivers: pinctrl: sdm845: add PDC wakeup interrupt map for GPIOs Lina Iyer
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

Some GPIOs are marked as wakeup capable and are routed to another
interrupt controller that is an always-domain and can detect interrupts
even most of the SoC is powered off. The wakeup interrupt controller
wakes up the GIC and replays the interrupt at the GIC.

Setup the TLMM irqchip in hierarchy with the wakeup interrupt controller
and ensure the wakeup GPIOs are handled correctly.

Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
----
Changes in RFC v2:
	- Define irq_domain_qcom_handle_wakeup()
	- Rebase on top of GPIO hierarchy support in linux-next
	- Set the chained irq handler for summary line
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 119 +++++++++++++++++++++++++++++++++++++
 drivers/pinctrl/qcom/pinctrl-msm.h |  16 +++++
 include/linux/soc/qcom/irq.h       |  13 ++++
 3 files changed, 148 insertions(+)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index f355ddd..c5ba389 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -23,6 +23,8 @@
 #include <linux/pm.h>
 #include <linux/log2.h>
 
+#include <linux/soc/qcom/irq.h>
+
 #include "../core.h"
 #include "../pinconf.h"
 #include "pinctrl-msm.h"
@@ -44,6 +46,7 @@
  * @enabled_irqs:   Bitmap of currently enabled irqs.
  * @dual_edge_irqs: Bitmap of irqs that need sw emulated dual edge
  *                  detection.
+ * @skip_wake_irqs: Skip IRQs that are handled by wakeup interrupt contrroller
  * @soc;            Reference to soc_data of platform specific data.
  * @regs:           Base addresses for the TLMM tiles.
  */
@@ -61,6 +64,7 @@ struct msm_pinctrl {
 
 	DECLARE_BITMAP(dual_edge_irqs, MAX_NR_GPIO);
 	DECLARE_BITMAP(enabled_irqs, MAX_NR_GPIO);
+	DECLARE_BITMAP(skip_wake_irqs, MAX_NR_GPIO);
 
 	const struct msm_pinctrl_soc_data *soc;
 	void __iomem *regs[MAX_NR_TILES];
@@ -709,6 +713,12 @@ static void msm_gpio_irq_mask(struct irq_data *d)
 	unsigned long flags;
 	u32 val;
 
+	if (d->parent_data)
+		irq_chip_mask_parent(d);
+
+	if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+		return;
+
 	g = &pctrl->soc->groups[d->hwirq];
 
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -753,6 +763,12 @@ static void msm_gpio_irq_clear_unmask(struct irq_data *d, bool status_clear)
 	unsigned long flags;
 	u32 val;
 
+	if (d->parent_data)
+		irq_chip_unmask_parent(d);
+
+	if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+		return;
+
 	g = &pctrl->soc->groups[d->hwirq];
 
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -780,10 +796,37 @@ static void msm_gpio_irq_clear_unmask(struct irq_data *d, bool status_clear)
 
 static void msm_gpio_irq_enable(struct irq_data *d)
 {
+	/*
+	 * Clear the interrupt that may be pending before we enable
+	 * the line.
+	 * This is especially a problem with the GPIOs routed to the
+	 * PDC. These GPIOs are direct-connect interrupts to the GIC.
+	 * Disabling the interrupt line at the PDC does not prevent
+	 * the interrupt from being latched at the GIC. The state at
+	 * GIC needs to be cleared before enabling.
+	 */
+	if (d->parent_data) {
+		irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, 0);
+		irq_chip_enable_parent(d);
+	}
 
 	msm_gpio_irq_clear_unmask(d, true);
 }
 
+static void msm_gpio_irq_disable(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
+
+	if (d->parent_data)
+		irq_chip_disable_parent(d);
+
+	if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+		return;
+
+	msm_gpio_irq_mask(d);
+}
+
 static void msm_gpio_irq_unmask(struct irq_data *d)
 {
 	msm_gpio_irq_clear_unmask(d, false);
@@ -797,6 +840,9 @@ static void msm_gpio_irq_ack(struct irq_data *d)
 	unsigned long flags;
 	u32 val;
 
+	if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+		return;
+
 	g = &pctrl->soc->groups[d->hwirq];
 
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -822,6 +868,12 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 	unsigned long flags;
 	u32 val;
 
+	if (d->parent_data)
+		irq_chip_set_type_parent(d, type);
+
+	if (test_bit(d->hwirq, pctrl->skip_wake_irqs))
+		return 0;
+
 	g = &pctrl->soc->groups[d->hwirq];
 
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -914,6 +966,15 @@ static int msm_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 	struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
 	unsigned long flags;
 
+	if (d->parent_data)
+		irq_chip_set_wake_parent(d, on);
+
+	/*
+	 * While they may not wake up when the TLMM is powered off,
+	 * some GPIOs would like to wakeup the system from suspend
+	 * when TLMM is powered on. To allow that, enable the GPIO
+	 * summary line to be wakeup capable at GIC.
+	 */
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
 
 	irq_set_irq_wake(pctrl->irq, on);
@@ -992,6 +1053,30 @@ static void msm_gpio_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
+static int msm_gpio_wakeirq(struct gpio_chip *gc,
+			    unsigned int child,
+			    unsigned int child_type,
+			    unsigned int *parent,
+			    unsigned int *parent_type)
+{
+	struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
+	const struct msm_gpio_wakeirq_map *map;
+	int i;
+
+	*parent = GPIO_NO_WAKE_IRQ;
+	*parent_type = IRQ_TYPE_EDGE_RISING;
+
+	for (i = 0; i < pctrl->soc->nwakeirq_map; i++) {
+		map = &pctrl->soc->wakeirq_map[i];
+		if (map->gpio == child) {
+			*parent = map->wakeirq;
+			break;
+		}
+	}
+
+	return 0;
+}
+
 static bool msm_gpio_needs_valid_mask(struct msm_pinctrl *pctrl)
 {
 	if (pctrl->soc->reserved_gpios)
@@ -1006,6 +1091,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 	struct gpio_irq_chip *girq;
 	int ret;
 	unsigned ngpio = pctrl->soc->ngpios;
+	struct device_node *dn;
 
 	if (WARN_ON(ngpio > MAX_NR_GPIO))
 		return -EINVAL;
@@ -1021,17 +1107,40 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 
 	pctrl->irq_chip.name = "msmgpio";
 	pctrl->irq_chip.irq_enable = msm_gpio_irq_enable;
+	pctrl->irq_chip.irq_disable = msm_gpio_irq_disable;
 	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;
+	pctrl->irq_chip.irq_eoi = irq_chip_eoi_parent;
 	pctrl->irq_chip.irq_set_type = msm_gpio_irq_set_type;
 	pctrl->irq_chip.irq_set_wake = msm_gpio_irq_set_wake;
 	pctrl->irq_chip.irq_request_resources = msm_gpio_irq_reqres;
 	pctrl->irq_chip.irq_release_resources = msm_gpio_irq_relres;
 
+	dn = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
+	if (dn) {
+		int i;
+		bool skip;
+		unsigned int gpio;
+
+		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->irq.child_to_parent_hwirq = msm_gpio_wakeirq;
+
+		skip = irq_domain_qcom_handle_wakeup(chip->irq.parent_domain);
+		for (i = 0; skip && i < pctrl->soc->nwakeirq_map; i++) {
+			gpio = pctrl->soc->wakeirq_map[i].gpio;
+			set_bit(gpio, pctrl->skip_wake_irqs);
+		}
+	}
+
 	girq = &chip->irq;
 	girq->chip = &pctrl->irq_chip;
 	girq->parent_handler = msm_gpio_irq_handler;
+	girq->fwnode = pctrl->dev->fwnode;
 	girq->num_parents = 1;
 	girq->parents = devm_kcalloc(pctrl->dev, 1, sizeof(*girq->parents),
 				     GFP_KERNEL);
@@ -1067,6 +1176,16 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 		}
 	}
 
+	/*
+	 * Since we are chained to the GIC using the TLMM summary line
+	 * and in hierarchy with the wakeup parent interrupt controller,
+	 * explicitly set the chained summary line. We need to do this because
+	 * the summary line is not routed to the wakeup parent but directly
+	 * to the GIC.
+	 */
+	gpiochip_set_chained_irqchip(chip, &pctrl->irq_chip, pctrl->irq,
+				     msm_gpio_irq_handler);
+
 	return 0;
 }
 
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.h b/drivers/pinctrl/qcom/pinctrl-msm.h
index 48569cda8..1547020 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.h
+++ b/drivers/pinctrl/qcom/pinctrl-msm.h
@@ -5,6 +5,8 @@
 #ifndef __PINCTRL_MSM_H__
 #define __PINCTRL_MSM_H__
 
+#include <linux/gpio/driver.h>
+
 struct pinctrl_pin_desc;
 
 /**
@@ -92,6 +94,16 @@ struct msm_pingroup {
 };
 
 /**
+ * struct msm_gpio_wakeirq_map - Map of GPIOs and their wakeup pins
+ * @gpio:          The GPIOs that are wakeup capable
+ * @wakeirq:       The interrupt at the always-on interrupt controller
+ */
+struct msm_gpio_wakeirq_map {
+	unsigned int gpio;
+	unsigned int wakeirq;
+};
+
+/**
  * struct msm_pinctrl_soc_data - Qualcomm pin controller driver configuration
  * @pins:	    An array describing all pins the pin controller affects.
  * @npins:	    The number of entries in @pins.
@@ -101,6 +113,8 @@ struct msm_pingroup {
  * @ngroups:	    The numbmer of entries in @groups.
  * @ngpio:	    The number of pingroups the driver should expose as GPIOs.
  * @pull_no_keeper: The SoC does not support keeper bias.
+ * @wakeirq_map:    The map of wakeup capable GPIOs and the pin at PDC/MPM
+ * @nwakeirq_map:   The number of entries in @hierarchy_map
  */
 struct msm_pinctrl_soc_data {
 	const struct pinctrl_pin_desc *pins;
@@ -114,6 +128,8 @@ struct msm_pinctrl_soc_data {
 	const char *const *tiles;
 	unsigned int ntiles;
 	const int *reserved_gpios;
+	const struct msm_gpio_wakeirq_map *wakeirq_map;
+	unsigned int nwakeirq_map;
 };
 
 extern const struct dev_pm_ops msm_pinctrl_dev_pm_ops;
diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
index 85ac4b6..098d57c 100644
--- a/include/linux/soc/qcom/irq.h
+++ b/include/linux/soc/qcom/irq.h
@@ -16,4 +16,17 @@
 #define IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP		(IRQ_DOMAIN_FLAG_NONCORE << 0)
 #define IRQ_DOMAIN_FLAG_QCOM_MPM_WAKEUP		(IRQ_DOMAIN_FLAG_NONCORE << 1)
 
+/**
+ * irq_domain_qcom_handle_wakeup: Return if the domain handles interrupt
+ *                                configuration
+ * @parent: irq domain
+ *
+ * This QCOM specific irq domain call returns if the interrupt controller
+ * requires the interrupt be masked at the child interrupt controller.
+ */
+static inline bool irq_domain_qcom_handle_wakeup(struct irq_domain *parent)
+{
+	return (parent->flags & IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP);
+}
+
 #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] 32+ messages in thread

* [PATCH RFC v2 11/14] drivers: pinctrl: sdm845: add PDC wakeup interrupt map for GPIOs
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (9 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 10/14] drivers: pinctrl: msm: setup GPIO chip in hierarchy Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-10-03 12:18   ` Linus Walleij
  2019-09-13 21:59 ` [PATCH RFC v2 12/14] arm64: dts: qcom: add PDC interrupt controller for SDM845 Lina Iyer
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

Add interrupt parents for wakeup capable GPIOs for Qualcomm SDM845 SoC.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
Changes in RFC v2:
	- Rearranged GPIO wakeup parent map
---
 drivers/pinctrl/qcom/pinctrl-sdm845.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-sdm845.c b/drivers/pinctrl/qcom/pinctrl-sdm845.c
index 39f498c..9cff3a4 100644
--- a/drivers/pinctrl/qcom/pinctrl-sdm845.c
+++ b/drivers/pinctrl/qcom/pinctrl-sdm845.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved.
  */
 
 #include <linux/acpi.h>
@@ -1282,6 +1282,24 @@ static const int sdm845_acpi_reserved_gpios[] = {
 	0, 1, 2, 3, 81, 82, 83, 84, -1
 };
 
+static const struct msm_gpio_wakeirq_map sdm845_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 const struct msm_pinctrl_soc_data sdm845_pinctrl = {
 	.pins = sdm845_pins,
 	.npins = ARRAY_SIZE(sdm845_pins),
@@ -1290,6 +1308,9 @@ static const struct msm_pinctrl_soc_data sdm845_pinctrl = {
 	.groups = sdm845_groups,
 	.ngroups = ARRAY_SIZE(sdm845_groups),
 	.ngpios = 151,
+	.wakeirq_map = sdm845_pdc_map,
+	.nwakeirq_map = ARRAY_SIZE(sdm845_pdc_map),
+
 };
 
 static const struct msm_pinctrl_soc_data sdm845_acpi_pinctrl = {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH RFC v2 12/14] arm64: dts: qcom: add PDC interrupt controller for SDM845
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (10 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 11/14] drivers: pinctrl: sdm845: add PDC wakeup interrupt map for GPIOs Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 13/14] arm64: dts: qcom: setup PDC as the wakeup parent for TLMM on SDM845 Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 14/14] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845 Lina Iyer
  13 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

Add PDC interrupt controller device bindings for SDM845.

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

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index be0022e..41455b8 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2375,6 +2375,16 @@
 			#power-domain-cells = <1>;
 		};
 
+		pdc_intc: interrupt-controller@b220000 {
+			compatible = "qcom,sdm845-pdc";
+			reg = <0 0x0b220000 0 0x30000>, <0 0x179900f0 0 0x60>;
+			qcom,pdc-ranges = <0 480 94>, <94 609 15>, <115 630 7>;
+			#interrupt-cells = <2>;
+			interrupt-parent = <&intc>;
+			interrupt-controller;
+			qcom,scm-spi-cfg;
+		};
+
 		pdc_reset: reset-controller@b2e0000 {
 			compatible = "qcom,sdm845-pdc-global";
 			reg = <0 0x0b2e0000 0 0x20000>;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH RFC v2 13/14] arm64: dts: qcom: setup PDC as the wakeup parent for TLMM on SDM845
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (11 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 12/14] arm64: dts: qcom: add PDC interrupt controller for SDM845 Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  2019-09-13 21:59 ` [PATCH RFC v2 14/14] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845 Lina Iyer
  13 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer

PDC always-on interrupt controller can detect certain GPIOs even when
the TLMM interrupt controller is powered off. Link the PDC as TLMM's
wakeup parent.

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 41455b8..1b70254 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1358,6 +1358,7 @@
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			gpio-ranges = <&tlmm 0 0 150>;
+			wakeup-parent = <&pdc_intc>;
 
 			qspi_clk: qspi-clk {
 				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] 32+ messages in thread

* [PATCH RFC v2 14/14] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845
  2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
                   ` (12 preceding siblings ...)
  2019-09-13 21:59 ` [PATCH RFC v2 13/14] arm64: dts: qcom: setup PDC as the wakeup parent for TLMM on SDM845 Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
  13 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
  To: swboyd, evgreen, maz, linus.walleij
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	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 0e58ef0..310b604 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -729,6 +729,7 @@ CONFIG_ARCH_R8A77970=y
 CONFIG_ARCH_R8A77980=y
 CONFIG_ARCH_R8A77990=y
 CONFIG_ARCH_R8A77995=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] 32+ messages in thread

* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
  2019-09-13 21:59 ` [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register Lina Iyer
@ 2019-09-30 22:14   ` Rob Herring
  2019-09-30 22:33   ` Stephen Boyd
  1 sibling, 0 replies; 32+ messages in thread
From: Rob Herring @ 2019-09-30 22:14 UTC (permalink / raw)
  To: Lina Iyer
  Cc: swboyd, evgreen, maz, linus.walleij, linux-kernel, linux-arm-msm,
	bjorn.andersson, mkshah, linux-gpio, Lina Iyer, devicetree

On Fri, 13 Sep 2019 15:59:14 -0600, Lina Iyer wrote:
> In addition to configuring the PDC, additional registers that interface
> the GIC have to be configured to match the GPIO type. The registers on
> some QCOM SoCs are access restricted, while on other SoCs are not. They
> SoCs with access restriction to these SPI registers need to be written
> from the firmware using the SCM interface. Add a flag to indicate if the
> register is to be written using SCM interface.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
>  .../devicetree/bindings/interrupt-controller/qcom,pdc.txt   | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
  2019-09-13 21:59 ` [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register Lina Iyer
  2019-09-30 22:14   ` Rob Herring
@ 2019-09-30 22:33   ` Stephen Boyd
  2019-10-16  6:27     ` Stephen Boyd
  1 sibling, 1 reply; 32+ messages in thread
From: Stephen Boyd @ 2019-09-30 22:33 UTC (permalink / raw)
  To: Lina Iyer, evgreen, linus.walleij, maz
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer, devicetree

Quoting Lina Iyer (2019-09-13 14:59:14)
> In addition to configuring the PDC, additional registers that interface
> the GIC have to be configured to match the GPIO type. The registers on
> some QCOM SoCs are access restricted, while on other SoCs are not. They
> SoCs with access restriction to these SPI registers need to be written
> from the firmware using the SCM interface. Add a flag to indicate if the
> register is to be written using SCM interface.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
>  .../devicetree/bindings/interrupt-controller/qcom,pdc.txt   | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> index 8e0797c..e329f8d 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> @@ -24,6 +24,9 @@ Properties:
>         Usage: required
>         Value type: <prop-encoded-array>
>         Definition: Specifies the base physical address for PDC hardware.
> +                   Optionally, specify the PDC's GIC interface registers that
> +                   need to be configured for wakeup capable GPIOs routed to
> +                   the PDC.
>  
>  - interrupt-cells:
>         Usage: required
> @@ -50,15 +53,23 @@ Properties:
>                     The second element is the GIC hwirq number for the PDC port.
>                     The third element is the number of interrupts in sequence.
>  
> +- qcom,scm-spi-cfg:
> +       Usage: optional
> +       Value type: <bool>
> +       Definition: Specifies if the SPI configuration registers have to be
> +                   written from the firmware. Sometimes the PDC interface
> +                   register to the GIC can only be written from the firmware.
> +
>  Example:
>  
>         pdc: interrupt-controller@b220000 {
>                 compatible = "qcom,sdm845-pdc";
> -               reg = <0xb220000 0x30000>;
> +               reg = <0 0x0b220000 0 0x30000>, <0 0x179900f0 0 0x60>;
>                 qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
>                 #interrupt-cells = <2>;
>                 interrupt-parent = <&intc>;
>                 interrupt-controller;
> +               qcom,scm-spi-cfg;
>         };

This overlaps register region with the mailbox node. That node is
actually a pile of random "CPU" registers used to ping remote processors
and apparently control how the PDC interacts with the GIC. Maybe this
can be changed to a phandle and then the driver can interogate the
phandle to determine if it's the SCM firmware or if it's the shared
mailbox register? If it's a shared mailbox then it can write to it at
the offset it knows about (because it's sdm845 compatible specific) and
if it's SCM then it can use the hardcoded address as well?

Basically I'm saying that it just needs a phandle.

	qcom,spi-cfg = <&scm>;

or

	qcom,spi-cfg = <&mailbox>;

and then driver knows how to use that to write into random registers.
Maybe we can have an API in regmap that finds the regmap for a given
device node? That way we don't have to funnel everything through syscon
for this.

	of_get_regmap(struct device_node *np, const char *name);

Where NULL name means "first available" and then do the devres search
otherwise for a device that has the matching node pointer.


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

* Re: [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent
  2019-09-13 21:59 ` [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent Lina Iyer
@ 2019-10-03 12:02   ` Linus Walleij
  2019-11-08 21:29   ` Doug Anderson
  1 sibling, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2019-10-03 12:02 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Stephen Boyd, Evan Green, Marc Zyngier, linux-kernel, MSM,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Sep 13, 2019 at 11:59 PM Lina Iyer <ilina@codeaurora.org> wrote:

> Some interrupt controllers in a SoC, are always powered on and have a
> select interrupts routed to them, so that they can wakeup the SoC from
> suspend. Add wakeup-parent DT property to refer to these interrupt
> controllers.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 10/14] drivers: pinctrl: msm: setup GPIO chip in hierarchy
  2019-09-13 21:59 ` [PATCH RFC v2 10/14] drivers: pinctrl: msm: setup GPIO chip in hierarchy Lina Iyer
@ 2019-10-03 12:17   ` Linus Walleij
  2019-11-13 18:35     ` Lina Iyer
  0 siblings, 1 reply; 32+ messages in thread
From: Linus Walleij @ 2019-10-03 12:17 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Stephen Boyd, Evan Green, Marc Zyngier, linux-kernel, MSM,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM

On Fri, Sep 13, 2019 at 11:59 PM Lina Iyer <ilina@codeaurora.org> wrote:

> Some GPIOs are marked as wakeup capable and are routed to another
> interrupt controller that is an always-domain and can detect interrupts
> even most of the SoC is powered off. The wakeup interrupt controller
> wakes up the GIC and replays the interrupt at the GIC.
>
> Setup the TLMM irqchip in hierarchy with the wakeup interrupt controller
> and ensure the wakeup GPIOs are handled correctly.
>
> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ----
> Changes in RFC v2:
>         - Define irq_domain_qcom_handle_wakeup()
>         - Rebase on top of GPIO hierarchy support in linux-next
>         - Set the chained irq handler for summary line

This is looking better every time I look at it, it's really complex
but alas the problem is hard to solve so it requires complex solutions.

> @@ -1006,6 +1091,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
>         struct gpio_irq_chip *girq;
>         int ret;
>         unsigned ngpio = pctrl->soc->ngpios;
> +       struct device_node *dn;

I usually call the variable "np"

> @@ -1021,17 +1107,40 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
>
>         pctrl->irq_chip.name = "msmgpio";
>         pctrl->irq_chip.irq_enable = msm_gpio_irq_enable;
> +       pctrl->irq_chip.irq_disable = msm_gpio_irq_disable;
>         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;
> +       pctrl->irq_chip.irq_eoi = irq_chip_eoi_parent;

This part and the functions called seem fine!

> +       dn = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
> +       if (dn) {
> +               int i;
> +               bool skip;
> +               unsigned int gpio;
> +
> +               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->irq.child_to_parent_hwirq = msm_gpio_wakeirq;
> +
> +               skip = irq_domain_qcom_handle_wakeup(chip->irq.parent_domain);
> +               for (i = 0; skip && i < pctrl->soc->nwakeirq_map; i++) {
> +                       gpio = pctrl->soc->wakeirq_map[i].gpio;
> +                       set_bit(gpio, pctrl->skip_wake_irqs);
> +               }
> +       }

OK I guess this is how we should do it, maybe add a comment to clarify
that we are checking the parent irqdomain of the chained IRQ to see
if we need to avoid disabling the irq as it is used for wakeup. (IIUC
what the code does!)

> +       /*
> +        * Since we are chained to the GIC using the TLMM summary line
> +        * and in hierarchy with the wakeup parent interrupt controller,
> +        * explicitly set the chained summary line. We need to do this because
> +        * the summary line is not routed to the wakeup parent but directly
> +        * to the GIC.
> +        */
> +       gpiochip_set_chained_irqchip(chip, &pctrl->irq_chip, pctrl->irq,
> +                                    msm_gpio_irq_handler);

I don't think this part is needed, we already have:

girq->parent_handler = msm_gpio_irq_handler;
girq->num_parents = 1;
girq->parents = devm_kcalloc(pctrl->dev, 1, sizeof(*girq->parents),
     GFP_KERNEL);
if (!girq->parents)
     return -ENOMEM;
girq->default_type = IRQ_TYPE_NONE;
girq->handler = handle_bad_irq;
girq->parents[0] = pctrl->irq;

This will make the irq chain when calling gpiochip_add_data(), so
just delete this and see if everything works as before.

Other than that it looks fine!

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 11/14] drivers: pinctrl: sdm845: add PDC wakeup interrupt map for GPIOs
  2019-09-13 21:59 ` [PATCH RFC v2 11/14] drivers: pinctrl: sdm845: add PDC wakeup interrupt map for GPIOs Lina Iyer
@ 2019-10-03 12:18   ` Linus Walleij
  0 siblings, 0 replies; 32+ messages in thread
From: Linus Walleij @ 2019-10-03 12:18 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Stephen Boyd, Evan Green, Marc Zyngier, linux-kernel, MSM,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM

On Fri, Sep 13, 2019 at 11:59 PM Lina Iyer <ilina@codeaurora.org> wrote:

> Add interrupt parents for wakeup capable GPIOs for Qualcomm SDM845 SoC.
>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
> Changes in RFC v2:
>         - Rearranged GPIO wakeup parent map

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
  2019-09-30 22:33   ` Stephen Boyd
@ 2019-10-16  6:27     ` Stephen Boyd
  2019-11-05 20:58       ` Lina Iyer
  0 siblings, 1 reply; 32+ messages in thread
From: Stephen Boyd @ 2019-10-16  6:27 UTC (permalink / raw)
  To: Lina Iyer, evgreen, linus.walleij, maz
  Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
	Lina Iyer, devicetree

Quoting Stephen Boyd (2019-09-30 15:33:01)
> Quoting Lina Iyer (2019-09-13 14:59:14)
> > In addition to configuring the PDC, additional registers that interface
> > the GIC have to be configured to match the GPIO type. The registers on
> > some QCOM SoCs are access restricted, while on other SoCs are not. They
> > SoCs with access restriction to these SPI registers need to be written
> > from the firmware using the SCM interface. Add a flag to indicate if the
> > register is to be written using SCM interface.
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> > ---
> >  .../devicetree/bindings/interrupt-controller/qcom,pdc.txt   | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> > index 8e0797c..e329f8d 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> > @@ -24,6 +24,9 @@ Properties:
> >         Usage: required
> >         Value type: <prop-encoded-array>
> >         Definition: Specifies the base physical address for PDC hardware.
> > +                   Optionally, specify the PDC's GIC interface registers that
> > +                   need to be configured for wakeup capable GPIOs routed to
> > +                   the PDC.
> >  
> >  - interrupt-cells:
> >         Usage: required
> > @@ -50,15 +53,23 @@ Properties:
> >                     The second element is the GIC hwirq number for the PDC port.
> >                     The third element is the number of interrupts in sequence.
> >  
> > +- qcom,scm-spi-cfg:
> > +       Usage: optional
> > +       Value type: <bool>
> > +       Definition: Specifies if the SPI configuration registers have to be
> > +                   written from the firmware. Sometimes the PDC interface
> > +                   register to the GIC can only be written from the firmware.
> > +
> >  Example:
> >  
> >         pdc: interrupt-controller@b220000 {
> >                 compatible = "qcom,sdm845-pdc";
> > -               reg = <0xb220000 0x30000>;
> > +               reg = <0 0x0b220000 0 0x30000>, <0 0x179900f0 0 0x60>;
> >                 qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
> >                 #interrupt-cells = <2>;
> >                 interrupt-parent = <&intc>;
> >                 interrupt-controller;
> > +               qcom,scm-spi-cfg;
> >         };
> 
> This overlaps register region with the mailbox node. That node is
> actually a pile of random "CPU" registers used to ping remote processors
> and apparently control how the PDC interacts with the GIC. Maybe this
> can be changed to a phandle and then the driver can interogate the
> phandle to determine if it's the SCM firmware or if it's the shared
> mailbox register? If it's a shared mailbox then it can write to it at
> the offset it knows about (because it's sdm845 compatible specific) and
> if it's SCM then it can use the hardcoded address as well?
> 
> Basically I'm saying that it just needs a phandle.
> 
>         qcom,spi-cfg = <&scm>;
> 
> or
> 
>         qcom,spi-cfg = <&mailbox>;
> 
> and then driver knows how to use that to write into random registers.
> Maybe we can have an API in regmap that finds the regmap for a given
> device node? That way we don't have to funnel everything through syscon
> for this.
> 
>         of_get_regmap(struct device_node *np, const char *name);
> 
> Where NULL name means "first available" and then do the devres search
> otherwise for a device that has the matching node pointer.
> 

I had another idea the other day. Maybe a better approach would be to
make the mailbox or SCM code an interrupt controller with the
appropriate functions to poke the bits necessary to make the interrupts
work. Then we can make it a chip in the hierarchy between the GIC and
PDC and make the interrupts call through from PDC to GIC. The locking
could be handled in each respective driver if necessary, and otherwise
we don't have to use a regmap or remap the same registers (except we may
need to describe if the parent is the mailbox node or the scm fimware
node).


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

* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
  2019-10-16  6:27     ` Stephen Boyd
@ 2019-11-05 20:58       ` Lina Iyer
  2019-11-06  0:53         ` Stephen Boyd
  0 siblings, 1 reply; 32+ messages in thread
From: Lina Iyer @ 2019-11-05 20:58 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: evgreen, linus.walleij, maz, linux-kernel, linux-arm-msm,
	bjorn.andersson, mkshah, linux-gpio, devicetree

Sorry for the late reply.

On Tue, Oct 15 2019 at 00:27 -0600, Stephen Boyd wrote:
>Quoting Stephen Boyd (2019-09-30 15:33:01)
>> Quoting Lina Iyer (2019-09-13 14:59:14)
>> > In addition to configuring the PDC, additional registers that interface
>> > the GIC have to be configured to match the GPIO type. The registers on
>> > some QCOM SoCs are access restricted, while on other SoCs are not. They
>> > SoCs with access restriction to these SPI registers need to be written
>> > from the firmware using the SCM interface. Add a flag to indicate if the
>> > register is to be written using SCM interface.
>> >
>> > Cc: devicetree@vger.kernel.org
>> > Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> > ---
>> >  .../devicetree/bindings/interrupt-controller/qcom,pdc.txt   | 13 ++++++++++++-
>> >  1 file changed, 12 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>> > index 8e0797c..e329f8d 100644
>> > --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>> > +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>> > @@ -24,6 +24,9 @@ Properties:
>> >         Usage: required
>> >         Value type: <prop-encoded-array>
>> >         Definition: Specifies the base physical address for PDC hardware.
>> > +                   Optionally, specify the PDC's GIC interface registers that
>> > +                   need to be configured for wakeup capable GPIOs routed to
>> > +                   the PDC.
>> >
>> >  - interrupt-cells:
>> >         Usage: required
>> > @@ -50,15 +53,23 @@ Properties:
>> >                     The second element is the GIC hwirq number for the PDC port.
>> >                     The third element is the number of interrupts in sequence.
>> >
>> > +- qcom,scm-spi-cfg:
>> > +       Usage: optional
>> > +       Value type: <bool>
>> > +       Definition: Specifies if the SPI configuration registers have to be
>> > +                   written from the firmware. Sometimes the PDC interface
>> > +                   register to the GIC can only be written from the firmware.
>> > +
>> >  Example:
>> >
>> >         pdc: interrupt-controller@b220000 {
>> >                 compatible = "qcom,sdm845-pdc";
>> > -               reg = <0xb220000 0x30000>;
>> > +               reg = <0 0x0b220000 0 0x30000>, <0 0x179900f0 0 0x60>;
>> >                 qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
>> >                 #interrupt-cells = <2>;
>> >                 interrupt-parent = <&intc>;
>> >                 interrupt-controller;
>> > +               qcom,scm-spi-cfg;
>> >         };
>>
>> This overlaps register region with the mailbox node. That node is
>> actually a pile of random "CPU" registers used to ping remote processors
>> and apparently control how the PDC interacts with the GIC. Maybe this
>> can be changed to a phandle and then the driver can interogate the
>> phandle to determine if it's the SCM firmware or if it's the shared
>> mailbox register? If it's a shared mailbox then it can write to it at
>> the offset it knows about (because it's sdm845 compatible specific) and
>> if it's SCM then it can use the hardcoded address as well?
>>
>> Basically I'm saying that it just needs a phandle.
>>
>>         qcom,spi-cfg = <&scm>;
>>
>> or
>>
>>         qcom,spi-cfg = <&mailbox>;
>>
>> and then driver knows how to use that to write into random registers.
>> Maybe we can have an API in regmap that finds the regmap for a given
>> device node? That way we don't have to funnel everything through syscon
>> for this.
>>
>>         of_get_regmap(struct device_node *np, const char *name);
>>
>> Where NULL name means "first available" and then do the devres search
>> otherwise for a device that has the matching node pointer.
>>
>
>I had another idea the other day. Maybe a better approach would be to
>make the mailbox or SCM code an interrupt controller with the
>appropriate functions to poke the bits necessary to make the interrupts
>work. Then we can make it a chip in the hierarchy between the GIC and
>PDC and make the interrupts call through from PDC to GIC. The locking
>could be handled in each respective driver if necessary, and otherwise
>we don't have to use a regmap or remap the same registers (except we may
>need to describe if the parent is the mailbox node or the scm fimware
>node).
>
Wouldn't that be a stretch to image the SCM register write  or a random
register write as an interrupt controller? But I agree that it solves
the issue of determining whether we want to use SCM or regmap.

But, we would still need to add syscon to the mailbox and then regmap
the registers for the interrupt contoller.

Thanks,
Lina



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

* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
  2019-11-05 20:58       ` Lina Iyer
@ 2019-11-06  0:53         ` Stephen Boyd
  2019-11-11 18:37           ` Lina Iyer
  0 siblings, 1 reply; 32+ messages in thread
From: Stephen Boyd @ 2019-11-06  0:53 UTC (permalink / raw)
  To: Lina Iyer
  Cc: evgreen, linus.walleij, maz, linux-kernel, linux-arm-msm,
	bjorn.andersson, mkshah, linux-gpio, devicetree

Quoting Lina Iyer (2019-11-05 12:58:32)
> On Tue, Oct 15 2019 at 00:27 -0600, Stephen Boyd wrote:
> >
> >I had another idea the other day. Maybe a better approach would be to
> >make the mailbox or SCM code an interrupt controller with the
> >appropriate functions to poke the bits necessary to make the interrupts
> >work. Then we can make it a chip in the hierarchy between the GIC and
> >PDC and make the interrupts call through from PDC to GIC. The locking
> >could be handled in each respective driver if necessary, and otherwise
> >we don't have to use a regmap or remap the same registers (except we may
> >need to describe if the parent is the mailbox node or the scm fimware
> >node).
> >
> Wouldn't that be a stretch to image the SCM register write  or a random
> register write as an interrupt controller? But I agree that it solves
> the issue of determining whether we want to use SCM or regmap.

As far as I can tell it's similar to PDC which is basically a gate on
the line from a dedicated chip pad or a GPIO pad that lets the interrupt
flow through to the GIC or not. Isn't this yet another hardware block on
those paths that control the edge type or something?

> 
> But, we would still need to add syscon to the mailbox and then regmap
> the registers for the interrupt contoller.

I'm saying that we can make the mailbox driver an interrupt controller
driver too. Or if that doesn't work, we can map the region twice in each
driver with ioremap and cross fingers that they don't touch the same
register at the same time. It sounds like that is the case. We won't be
able to fancily reserve the register region and map it in one function
call, but maybe that can be fixed by limiting the size or offset that is
reserved for each driver manually based on the same register property
that's described in DT. Basically, one node in DT

 mailbox@f00 {
   reg = <0xf00 0x1000>;
 };

And then each driver will ioremap() the whole register region that's
parsed from DT but each driver will mark sub-regions as reserved for the
respective driver. That way we don't have to worry about using a regmap
here and we'll still know what drivers are using what regions of IO in
/proc/iomem.

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

* Re: [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
  2019-09-13 21:59 ` [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs Lina Iyer
@ 2019-11-08 21:21   ` Doug Anderson
  2019-11-08 21:54     ` Lina Iyer
  0 siblings, 1 reply; 32+ messages in thread
From: Doug Anderson @ 2019-11-08 21:21 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Stephen Boyd, Evan Green, maz, LinusW, LKML, linux-arm-msm,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM

Hi,

On Fri, Sep 13, 2019 at 3:00 PM Lina Iyer <ilina@codeaurora.org> wrote:
>
> diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
> new file mode 100644
> index 0000000..85ac4b6
> --- /dev/null
> +++ b/include/linux/soc/qcom/irq.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __QCOM_IRQ_H
> +#define __QCOM_IRQ_H
> +

I happened to be looking at a pile of patches and one of them added:

+#include <linux/irqdomain.h>

...right here.  If/when you spin your patch, maybe you should too?  At
the moment the patch I was looking at is at:

https://android.googlesource.com/kernel/common/+log/refs/heads/android-mainline-tracking

Specifically:

https://android.googlesource.com/kernel/common/+/448e2302f82a70f52475b6fc32bbe30301052e6b


-Doug

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

* Re: [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent
  2019-09-13 21:59 ` [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent Lina Iyer
  2019-10-03 12:02   ` Linus Walleij
@ 2019-11-08 21:29   ` Doug Anderson
  1 sibling, 0 replies; 32+ messages in thread
From: Doug Anderson @ 2019-11-08 21:29 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Stephen Boyd, Evan Green, maz, LinusW, LKML, linux-arm-msm,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi,

On Fri, Sep 13, 2019 at 3:00 PM Lina Iyer <ilina@codeaurora.org> wrote:
>
> Some interrupt controllers in a SoC, are always powered on and have a
> select interrupts routed to them, so that they can wakeup the SoC from
> suspend. Add wakeup-parent DT property to refer to these interrupt
> controllers.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../devicetree/bindings/interrupt-controller/interrupts.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> index 8a3c408..c10e310 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> @@ -108,3 +108,16 @@ commonly used:
>                         sensitivity = <7>;
>                 };
>         };
> +
> +3) Interrupt wakeup parent
> +--------------------------
> +
> +Some interrupt controllers in a SoC, are always powered on and have a select
> +interrupts routed to them, so that they can wakeup the SoC from suspend. These
> +interrupt controllers do not fall into the category of a parent interrupt
> +controller and can be specified by the "wakeup-parent" property and contain a
> +single phandle referring to the wakeup capable interrupt controller.
> +
> +   Example:
> +       wakeup-parent = <&pdc_intc>;
> +

nit: git flags the above line as whitespace damage.  Please remove
if/when you spin.

Thanks!

-Doug

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

* Re: [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
  2019-11-08 21:21   ` Doug Anderson
@ 2019-11-08 21:54     ` Lina Iyer
  2019-11-08 22:16       ` Lina Iyer
  0 siblings, 1 reply; 32+ messages in thread
From: Lina Iyer @ 2019-11-08 21:54 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Stephen Boyd, Evan Green, maz, LinusW, LKML, linux-arm-msm,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM

On Fri, Nov 08 2019 at 14:22 -0700, Doug Anderson wrote:
>Hi,
>
>On Fri, Sep 13, 2019 at 3:00 PM Lina Iyer <ilina@codeaurora.org> wrote:
>>
>> diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
>> new file mode 100644
>> index 0000000..85ac4b6
>> --- /dev/null
>> +++ b/include/linux/soc/qcom/irq.h
>> @@ -0,0 +1,19 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#ifndef __QCOM_IRQ_H
>> +#define __QCOM_IRQ_H
>> +
>
>I happened to be looking at a pile of patches and one of them added:
>
>+#include <linux/irqdomain.h>
>
>...right here.  If/when you spin your patch, maybe you should too?  At
>the moment the patch I was looking at is at:
>
>https://android.googlesource.com/kernel/common/+log/refs/heads/android-mainline-tracking
>
>Specifically:
>
>https://android.googlesource.com/kernel/common/+/448e2302f82a70f52475b6fc32bbe30301052e6b
>
>
Sure, will take care of it in the next spin.

--Lina


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

* Re: [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
  2019-11-08 21:54     ` Lina Iyer
@ 2019-11-08 22:16       ` Lina Iyer
  2019-11-08 22:57         ` Doug Anderson
  0 siblings, 1 reply; 32+ messages in thread
From: Lina Iyer @ 2019-11-08 22:16 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Stephen Boyd, Evan Green, maz, LinusW, LKML, linux-arm-msm,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM

On Fri, Nov 08 2019 at 14:54 -0700, Lina Iyer wrote:
>On Fri, Nov 08 2019 at 14:22 -0700, Doug Anderson wrote:
>>Hi,
>>
>>On Fri, Sep 13, 2019 at 3:00 PM Lina Iyer <ilina@codeaurora.org> wrote:
>>>
>>>diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
>>>new file mode 100644
>>>index 0000000..85ac4b6
>>>--- /dev/null
>>>+++ b/include/linux/soc/qcom/irq.h
>>>@@ -0,0 +1,19 @@
>>>+/* SPDX-License-Identifier: GPL-2.0-only */
>>>+
>>>+#ifndef __QCOM_IRQ_H
>>>+#define __QCOM_IRQ_H
>>>+
>>
>>I happened to be looking at a pile of patches and one of them added:
>>
>>+#include <linux/irqdomain.h>
>>
>>...right here.  If/when you spin your patch, maybe you should too?  At
>>the moment the patch I was looking at is at:
>>
>>https://android.googlesource.com/kernel/common/+log/refs/heads/android-mainline-tracking
>>
>>Specifically:
>>
>>https://android.googlesource.com/kernel/common/+/448e2302f82a70f52475b6fc32bbe30301052e6b
>>
>>
>Sure, will take care of it in the next spin.
>
Checking for this, it seems like it would not be needed by this header.
There is nothing in this file that would need that header. It was
probably a older version that pulled into that tree.

Is there a reason now that you see this need?

--Lina

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

* Re: [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
  2019-11-08 22:16       ` Lina Iyer
@ 2019-11-08 22:57         ` Doug Anderson
  2019-11-08 23:14           ` Lina Iyer
  0 siblings, 1 reply; 32+ messages in thread
From: Doug Anderson @ 2019-11-08 22:57 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Stephen Boyd, Evan Green, maz, LinusW, LKML, linux-arm-msm,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM

Hi,

On Fri, Nov 8, 2019 at 2:16 PM Lina Iyer <ilina@codeaurora.org> wrote:
>
> On Fri, Nov 08 2019 at 14:54 -0700, Lina Iyer wrote:
> >On Fri, Nov 08 2019 at 14:22 -0700, Doug Anderson wrote:
> >>Hi,
> >>
> >>On Fri, Sep 13, 2019 at 3:00 PM Lina Iyer <ilina@codeaurora.org> wrote:
> >>>
> >>>diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
> >>>new file mode 100644
> >>>index 0000000..85ac4b6
> >>>--- /dev/null
> >>>+++ b/include/linux/soc/qcom/irq.h
> >>>@@ -0,0 +1,19 @@
> >>>+/* SPDX-License-Identifier: GPL-2.0-only */
> >>>+
> >>>+#ifndef __QCOM_IRQ_H
> >>>+#define __QCOM_IRQ_H
> >>>+
> >>
> >>I happened to be looking at a pile of patches and one of them added:
> >>
> >>+#include <linux/irqdomain.h>
> >>
> >>...right here.  If/when you spin your patch, maybe you should too?  At
> >>the moment the patch I was looking at is at:
> >>
> >>https://android.googlesource.com/kernel/common/+log/refs/heads/android-mainline-tracking
> >>
> >>Specifically:
> >>
> >>https://android.googlesource.com/kernel/common/+/448e2302f82a70f52475b6fc32bbe30301052e6b
> >>
> >>
> >Sure, will take care of it in the next spin.
> >
> Checking for this, it seems like it would not be needed by this header.
> There is nothing in this file that would need that header. It was
> probably a older version that pulled into that tree.
>
> Is there a reason now that you see this need?

From the note in the commit I found I'd assume that Maulik Shah (who
is CCed here) has history?

...but looking at it, I see that your header file refers to
"IRQ_DOMAIN_FLAG_NONCORE" which is defined in "linux/irqdomain.h".
That means it's good hygiene for you to include the header, right?
Otherwise all your users need to know that they should include the
header themselves, which is a bit ugly.

-Doug

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

* Re: [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs
  2019-11-08 22:57         ` Doug Anderson
@ 2019-11-08 23:14           ` Lina Iyer
  0 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-11-08 23:14 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Stephen Boyd, Evan Green, maz, LinusW, LKML, linux-arm-msm,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM

On Fri, Nov 08 2019 at 15:57 -0700, Doug Anderson wrote:
>Hi,
>
>On Fri, Nov 8, 2019 at 2:16 PM Lina Iyer <ilina@codeaurora.org> wrote:
>>
>> On Fri, Nov 08 2019 at 14:54 -0700, Lina Iyer wrote:
>> >On Fri, Nov 08 2019 at 14:22 -0700, Doug Anderson wrote:
>> >>Hi,
>> >>
>> >>On Fri, Sep 13, 2019 at 3:00 PM Lina Iyer <ilina@codeaurora.org> wrote:
>> >>>
>> >>>diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h
>> >>>new file mode 100644
>> >>>index 0000000..85ac4b6
>> >>>--- /dev/null
>> >>>+++ b/include/linux/soc/qcom/irq.h
>> >>>@@ -0,0 +1,19 @@
>> >>>+/* SPDX-License-Identifier: GPL-2.0-only */
>> >>>+
>> >>>+#ifndef __QCOM_IRQ_H
>> >>>+#define __QCOM_IRQ_H
>> >>>+
>> >>
>> >>I happened to be looking at a pile of patches and one of them added:
>> >>
>> >>+#include <linux/irqdomain.h>
>> >>
>> >>...right here.  If/when you spin your patch, maybe you should too?  At
>> >>the moment the patch I was looking at is at:
>> >>
>> >>https://android.googlesource.com/kernel/common/+log/refs/heads/android-mainline-tracking
>> >>
>> >>Specifically:
>> >>
>> >>https://android.googlesource.com/kernel/common/+/448e2302f82a70f52475b6fc32bbe30301052e6b
>> >>
>> >>
>> >Sure, will take care of it in the next spin.
>> >
>> Checking for this, it seems like it would not be needed by this header.
>> There is nothing in this file that would need that header. It was
>> probably a older version that pulled into that tree.
>>
>> Is there a reason now that you see this need?
>
>From the note in the commit I found I'd assume that Maulik Shah (who
>is CCed here) has history?
>
>...but looking at it, I see that your header file refers to
>"IRQ_DOMAIN_FLAG_NONCORE" which is defined in "linux/irqdomain.h".
Ah, ok. That would need the file. Will add.

>That means it's good hygiene for you to include the header, right?
>Otherwise all your users need to know that they should include the
>header themselves, which is a bit ugly.
>
>-Doug

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

* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
  2019-11-06  0:53         ` Stephen Boyd
@ 2019-11-11 18:37           ` Lina Iyer
  2019-11-12 11:52             ` Marc Zyngier
  0 siblings, 1 reply; 32+ messages in thread
From: Lina Iyer @ 2019-11-11 18:37 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: evgreen, linus.walleij, maz, linux-kernel, linux-arm-msm,
	bjorn.andersson, mkshah, linux-gpio, devicetree

On Tue, Nov 05 2019 at 17:53 -0700, Stephen Boyd wrote:
>Quoting Lina Iyer (2019-11-05 12:58:32)
>> On Tue, Oct 15 2019 at 00:27 -0600, Stephen Boyd wrote:
>> >
>> >I had another idea the other day. Maybe a better approach would be to
>> >make the mailbox or SCM code an interrupt controller with the
>> >appropriate functions to poke the bits necessary to make the interrupts
>> >work. Then we can make it a chip in the hierarchy between the GIC and
>> >PDC and make the interrupts call through from PDC to GIC. The locking
>> >could be handled in each respective driver if necessary, and otherwise
>> >we don't have to use a regmap or remap the same registers (except we may
>> >need to describe if the parent is the mailbox node or the scm fimware
>> >node).
>> >
>> Wouldn't that be a stretch to image the SCM register write  or a random
>> register write as an interrupt controller? But I agree that it solves
>> the issue of determining whether we want to use SCM or regmap.
>
>As far as I can tell it's similar to PDC which is basically a gate on
>the line from a dedicated chip pad or a GPIO pad that lets the interrupt
>flow through to the GIC or not. Isn't this yet another hardware block on
>those paths that control the edge type or something?
>
>>
>> But, we would still need to add syscon to the mailbox and then regmap
>> the registers for the interrupt contoller.
>
>I'm saying that we can make the mailbox driver an interrupt controller
>driver too. Or if that doesn't work, we can map the region twice in each
>driver with ioremap and cross fingers that they don't touch the same
>register at the same time. It sounds like that is the case. We won't be
>able to fancily reserve the register region and map it in one function
>call, but maybe that can be fixed by limiting the size or offset that is
>reserved for each driver manually based on the same register property
>that's described in DT. Basically, one node in DT
>
> mailbox@f00 {
>   reg = <0xf00 0x1000>;
> };
>
>And then each driver will ioremap() the whole register region that's
>parsed from DT but each driver will mark sub-regions as reserved for the
>respective driver. That way we don't have to worry about using a regmap
>here and we'll still know what drivers are using what regions of IO in
>/proc/iomem.

Marc: What do you think of Stephen's idea? Summarizing my understanding
below -

We need to set an addition register for GPIOs that are routed to PDC and
the register may need to be written using a SCM call (SDM845) or written
from Linux (SDM855). The idea proposed here is -
Create multiple irqchips, one for each type of register access and then
put them in hierarchy based on the target.

SDM845:
TLMM  --> PDC  --> PDC-SCM-IF  --> GIC

SDM855:
TLMM  --> PDC  --> PDC-LNX-IF  --> GIC

The hierarchy would be explicit from the DT. So we would not have to
worry about figuring out using a property in DT or resource name. (May
be we can use a compatible instead?). The use of reserved_resource(),
suggested by Stephen, would help avoid other drivers writing to this
register which is part of a generic dump area for one-off registers.

--Lina

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

* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add  SPI config register
  2019-11-11 18:37           ` Lina Iyer
@ 2019-11-12 11:52             ` Marc Zyngier
  0 siblings, 0 replies; 32+ messages in thread
From: Marc Zyngier @ 2019-11-12 11:52 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Stephen Boyd, evgreen, linus.walleij, linux-kernel,
	linux-arm-msm, bjorn.andersson, mkshah, linux-gpio, devicetree

On 2019-11-11 19:46, Lina Iyer wrote:
> On Tue, Nov 05 2019 at 17:53 -0700, Stephen Boyd wrote:
>>Quoting Lina Iyer (2019-11-05 12:58:32)
>>> On Tue, Oct 15 2019 at 00:27 -0600, Stephen Boyd wrote:
>>> >
>>> >I had another idea the other day. Maybe a better approach would be 
>>> to
>>> >make the mailbox or SCM code an interrupt controller with the
>>> >appropriate functions to poke the bits necessary to make the 
>>> interrupts
>>> >work. Then we can make it a chip in the hierarchy between the GIC 
>>> and
>>> >PDC and make the interrupts call through from PDC to GIC. The 
>>> locking
>>> >could be handled in each respective driver if necessary, and 
>>> otherwise
>>> >we don't have to use a regmap or remap the same registers (except 
>>> we may
>>> >need to describe if the parent is the mailbox node or the scm 
>>> fimware
>>> >node).
>>> >
>>> Wouldn't that be a stretch to image the SCM register write  or a 
>>> random
>>> register write as an interrupt controller? But I agree that it 
>>> solves
>>> the issue of determining whether we want to use SCM or regmap.
>>
>>As far as I can tell it's similar to PDC which is basically a gate on
>>the line from a dedicated chip pad or a GPIO pad that lets the 
>> interrupt
>>flow through to the GIC or not. Isn't this yet another hardware block 
>> on
>>those paths that control the edge type or something?
>>
>>>
>>> But, we would still need to add syscon to the mailbox and then 
>>> regmap
>>> the registers for the interrupt contoller.
>>
>>I'm saying that we can make the mailbox driver an interrupt 
>> controller
>>driver too. Or if that doesn't work, we can map the region twice in 
>> each
>>driver with ioremap and cross fingers that they don't touch the same
>>register at the same time. It sounds like that is the case. We won't 
>> be
>>able to fancily reserve the register region and map it in one 
>> function
>>call, but maybe that can be fixed by limiting the size or offset that 
>> is
>>reserved for each driver manually based on the same register property
>>that's described in DT. Basically, one node in DT
>>
>> mailbox@f00 {
>>   reg = <0xf00 0x1000>;
>> };
>>
>>And then each driver will ioremap() the whole register region that's
>>parsed from DT but each driver will mark sub-regions as reserved for 
>> the
>>respective driver. That way we don't have to worry about using a 
>> regmap
>>here and we'll still know what drivers are using what regions of IO 
>> in
>>/proc/iomem.
>
> Marc: What do you think of Stephen's idea? Summarizing my 
> understanding
> below -
>
> We need to set an addition register for GPIOs that are routed to PDC 
> and
> the register may need to be written using a SCM call (SDM845) or 
> written
> from Linux (SDM855). The idea proposed here is -
> Create multiple irqchips, one for each type of register access and 
> then
> put them in hierarchy based on the target.
>
> SDM845:
> TLMM  --> PDC  --> PDC-SCM-IF  --> GIC
>
> SDM855:
> TLMM  --> PDC  --> PDC-LNX-IF  --> GIC
>
> The hierarchy would be explicit from the DT. So we would not have to
> worry about figuring out using a property in DT or resource name. 
> (May
> be we can use a compatible instead?). The use of reserved_resource(),
> suggested by Stephen, would help avoid other drivers writing to this
> register which is part of a generic dump area for one-off registers.

That seems sensible: the two SoCs use different implementations of
their GPIO configurations (at least apparently, I'm pretty sure it
is the same HW underneath), and it makes sense to abstract that
as separate entities.

As for the DT binding, use whatever makes sense for you (compatible
seems a reasonable choice).

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

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

* Re: [PATCH RFC v2 10/14] drivers: pinctrl: msm: setup GPIO chip in hierarchy
  2019-10-03 12:17   ` Linus Walleij
@ 2019-11-13 18:35     ` Lina Iyer
  0 siblings, 0 replies; 32+ messages in thread
From: Lina Iyer @ 2019-11-13 18:35 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Stephen Boyd, Evan Green, Marc Zyngier, linux-kernel, MSM,
	Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM

On Thu, Oct 03 2019 at 06:17 -0600, Linus Walleij wrote:
>On Fri, Sep 13, 2019 at 11:59 PM Lina Iyer <ilina@codeaurora.org> wrote:
>
>> Some GPIOs are marked as wakeup capable and are routed to another
>> interrupt controller that is an always-domain and can detect interrupts
>> even most of the SoC is powered off. The wakeup interrupt controller
>> wakes up the GIC and replays the interrupt at the GIC.
>>
>> Setup the TLMM irqchip in hierarchy with the wakeup interrupt controller
>> and ensure the wakeup GPIOs are handled correctly.
>>
>> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> ----
>> Changes in RFC v2:
>>         - Define irq_domain_qcom_handle_wakeup()
>>         - Rebase on top of GPIO hierarchy support in linux-next
>>         - Set the chained irq handler for summary line
>
>This is looking better every time I look at it, it's really complex
>but alas the problem is hard to solve so it requires complex solutions.
>
>> @@ -1006,6 +1091,7 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
>>         struct gpio_irq_chip *girq;
>>         int ret;
>>         unsigned ngpio = pctrl->soc->ngpios;
>> +       struct device_node *dn;
>
>I usually call the variable "np"
>
Will change.

>> @@ -1021,17 +1107,40 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
>>
>>         pctrl->irq_chip.name = "msmgpio";
>>         pctrl->irq_chip.irq_enable = msm_gpio_irq_enable;
>> +       pctrl->irq_chip.irq_disable = msm_gpio_irq_disable;
>>         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;
>> +       pctrl->irq_chip.irq_eoi = irq_chip_eoi_parent;
>
>This part and the functions called seem fine!
>
>> +       dn = of_parse_phandle(pctrl->dev->of_node, "wakeup-parent", 0);
>> +       if (dn) {
>> +               int i;
>> +               bool skip;
>> +               unsigned int gpio;
>> +
>> +               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->irq.child_to_parent_hwirq = msm_gpio_wakeirq;
>> +
>> +               skip = irq_domain_qcom_handle_wakeup(chip->irq.parent_domain);
>> +               for (i = 0; skip && i < pctrl->soc->nwakeirq_map; i++) {
>> +                       gpio = pctrl->soc->wakeirq_map[i].gpio;
>> +                       set_bit(gpio, pctrl->skip_wake_irqs);
>> +               }
>> +       }
>
>OK I guess this is how we should do it, maybe add a comment to clarify
>that we are checking the parent irqdomain of the chained IRQ to see
>if we need to avoid disabling the irq as it is used for wakeup. (IIUC
>what the code does!)
>
Okay.

>> +       /*
>> +        * Since we are chained to the GIC using the TLMM summary line
>> +        * and in hierarchy with the wakeup parent interrupt controller,
>> +        * explicitly set the chained summary line. We need to do this because
>> +        * the summary line is not routed to the wakeup parent but directly
>> +        * to the GIC.
>> +        */
>> +       gpiochip_set_chained_irqchip(chip, &pctrl->irq_chip, pctrl->irq,
>> +                                    msm_gpio_irq_handler);
>
>I don't think this part is needed, we already have:
>
>girq->parent_handler = msm_gpio_irq_handler;
>girq->num_parents = 1;
>girq->parents = devm_kcalloc(pctrl->dev, 1, sizeof(*girq->parents),
>     GFP_KERNEL);
>if (!girq->parents)
>     return -ENOMEM;
>girq->default_type = IRQ_TYPE_NONE;
>girq->handler = handle_bad_irq;
>girq->parents[0] = pctrl->irq;
>
>This will make the irq chain when calling gpiochip_add_data(), so
>just delete this and see if everything works as before.
>
I thought it didn't work without this change and I am not sure why it
started working after I did. May be it was a bad set of patches that I
pulled in.

>Other than that it looks fine!
Thanks for your review.

--Lina

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

end of thread, other threads:[~2019-11-13 18:35 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-13 21:59 [PATCH RFC v2 00/14] Support wakeup capable GPIOs Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 01/14] irqdomain: add bus token DOMAIN_BUS_WAKEUP Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 02/14] drivers: irqchip: qcom-pdc: update max PDC interrupts Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 03/14] drivers: irqchip: pdc: Do not toggle IRQ_ENABLE during mask/unmask Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs Lina Iyer
2019-11-08 21:21   ` Doug Anderson
2019-11-08 21:54     ` Lina Iyer
2019-11-08 22:16       ` Lina Iyer
2019-11-08 22:57         ` Doug Anderson
2019-11-08 23:14           ` Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent Lina Iyer
2019-10-03 12:02   ` Linus Walleij
2019-11-08 21:29   ` Doug Anderson
2019-09-13 21:59 ` [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register Lina Iyer
2019-09-30 22:14   ` Rob Herring
2019-09-30 22:33   ` Stephen Boyd
2019-10-16  6:27     ` Stephen Boyd
2019-11-05 20:58       ` Lina Iyer
2019-11-06  0:53         ` Stephen Boyd
2019-11-11 18:37           ` Lina Iyer
2019-11-12 11:52             ` Marc Zyngier
2019-09-13 21:59 ` [PATCH RFC v2 07/14] drivers: irqchip: pdc: additionally set type in SPI config registers Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 08/14] genirq: Introduce irq_chip_get/set_parent_state calls Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 09/14] drivers: irqchip: pdc: Add irqchip set/get state calls Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 10/14] drivers: pinctrl: msm: setup GPIO chip in hierarchy Lina Iyer
2019-10-03 12:17   ` Linus Walleij
2019-11-13 18:35     ` Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 11/14] drivers: pinctrl: sdm845: add PDC wakeup interrupt map for GPIOs Lina Iyer
2019-10-03 12:18   ` Linus Walleij
2019-09-13 21:59 ` [PATCH RFC v2 12/14] arm64: dts: qcom: add PDC interrupt controller for SDM845 Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 13/14] arm64: dts: qcom: setup PDC as the wakeup parent for TLMM on SDM845 Lina Iyer
2019-09-13 21:59 ` [PATCH RFC v2 14/14] arm64: defconfig: enable PDC interrupt controller for Qualcomm SDM845 Lina Iyer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).