diff mbox series

[RFC,v2,04/14] drivers: irqchip: add PDC irqdomain for wakeup capable GPIOs

Message ID 1568411962-1022-5-git-send-email-ilina@codeaurora.org
State New
Headers show
Series Support wakeup capable GPIOs | expand

Commit Message

Lina Iyer Sept. 13, 2019, 9:59 p.m. UTC 
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

Comments

Doug Anderson Nov. 8, 2019, 9:21 p.m. UTC | #1 
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
Lina Iyer Nov. 8, 2019, 9:54 p.m. UTC | #2 
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
Lina Iyer Nov. 8, 2019, 10:16 p.m. UTC | #3 
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
Doug Anderson Nov. 8, 2019, 10:57 p.m. UTC | #4 
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
Lina Iyer Nov. 8, 2019, 11:14 p.m. UTC | #5 
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
diff mbox series

Patch

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