I recently noticed the following message at boot time on my Lenovo
c630 laptop (SDM845, if I'm not mistaken):
<quote>
[ 1.449499] debugfs: File ':soc@0:interrupt-controller@b220000' in directory 'domains' already present!
</quote>
which is usually the sign of something being amiss (multiple irqdomain
using the same fwnode and not being tagged properly).
Looking closer at the qcom-pdc driver (which is the one triggering the
above warning), I realised that this driver could do with some
cleanups:
- Pseudo hwirq indicating the lack of parent. Not completely wrong,
but could be done in a more elegant way.
- Two irq domains, which provide the exact same service to the same
IRQ space. Only the context is different, and the difference is not
significant.
- Broken locking. You just need the right timing and a driver that
disables its interrupt.
- A couple of open coded constructs that duplicate stuff the kernel
already implements.
I've tested this series on the above HW, and nothing broke (suspend
works, interrupts get delivered). If nobody shouts, I'll plan to take
this into 5.18.
Marc Zyngier (5):
irqchip/qcom-pdc: Kill PDC_NO_PARENT_IRQ
irqchip/qcom-pdc: Kill non-wakeup irqdomain
irqchip/qcom-pdc: Kill qcom_pdc_translate helper
irqchip/qcom-pdc: Fix broken locking
irqchip/qcom-pdc: Drop open coded version of __assign_bit()
drivers/irqchip/qcom-pdc.c | 137 ++++++++-----------------------------
1 file changed, 28 insertions(+), 109 deletions(-)
--
2.30.2
A careful look at the way the PDC driver works shows that:
- all interrupts are in the same space
- all interrupts are treated the same
And yet the driver creates two domains based on whether
the interrupt gets mapped directly or from the pinctrl code,
which is obviously a waste of resources.
Kill the non-wakeup domain and unify all the interrupt handling.
Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/qcom-pdc.c | 84 +++++---------------------------------
1 file changed, 10 insertions(+), 74 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 3b214c4e6755..5be531403f50 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -21,7 +21,6 @@
#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))
@@ -224,51 +223,6 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
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;
-
- region = get_pin_region(hwirq);
- if (!region)
- return irq_domain_disconnect_hierarchy(domain->parent, virq);
-
- 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] = pin_to_hwirq(region, hwirq);
- parent_fwspec.param[2] = type;
-
- return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
- &parent_fwspec);
-}
-
-static const struct irq_domain_ops qcom_pdc_ops = {
- .translate = qcom_pdc_translate,
- .alloc = qcom_pdc_alloc,
- .free = irq_domain_free_irqs_common,
-};
-
-static int 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;
- struct pdc_pin_region *region;
- irq_hw_number_t hwirq;
- unsigned int type;
- int ret;
-
ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
if (ret)
return ret;
@@ -301,16 +255,9 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
&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,
+static const struct irq_domain_ops qcom_pdc_ops = {
+ .translate = qcom_pdc_translate,
+ .alloc = qcom_pdc_alloc,
.free = irq_domain_free_irqs_common,
};
@@ -361,7 +308,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 irq_domain *parent_domain, *pdc_domain;
int ret;
pdc_base = of_iomap(node, 0);
@@ -383,32 +330,21 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
goto fail;
}
- pdc_domain = irq_domain_create_hierarchy(parent_domain, 0, PDC_MAX_IRQS,
- of_fwnode_handle(node),
- &qcom_pdc_ops, NULL);
- if (!pdc_domain) {
- pr_err("%pOF: GIC domain add failed\n", node);
- ret = -ENOMEM;
- goto fail;
- }
-
- pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain,
+ pdc_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);
+ &qcom_pdc_ops, NULL);
+ if (!pdc_domain) {
+ pr_err("%pOF: PDC domain add failed\n", node);
ret = -ENOMEM;
- goto remove;
+ goto fail;
}
- irq_domain_update_bus_token(pdc_gpio_domain, DOMAIN_BUS_WAKEUP);
+ irq_domain_update_bus_token(pdc_domain, DOMAIN_BUS_WAKEUP);
return 0;
-remove:
- irq_domain_remove(pdc_domain);
fail:
kfree(pdc_region);
iounmap(pdc_base);
--
2.30.2
The driver uses what looks like an open-coded version of __assign_bit().
Replace it with the real thing.
Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/qcom-pdc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 0cd20ddfae2a..d96916cf6a41 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -23,9 +23,6 @@
#define PDC_MAX_GPIO_IRQS 256
-#define CLEAR_INTR(reg, intr) (reg & ~(1 << intr))
-#define ENABLE_INTR(reg, intr) (reg | (1 << intr))
-
#define IRQ_ENABLE_BANK 0x10
#define IRQ_i_CFG 0x110
@@ -55,16 +52,16 @@ static u32 pdc_reg_read(int reg, u32 i)
static void pdc_enable_intr(struct irq_data *d, bool on)
{
int pin_out = d->hwirq;
+ unsigned long enable;
unsigned long flags;
u32 index, mask;
- u32 enable;
index = pin_out / 32;
mask = pin_out % 32;
raw_spin_lock_irqsave(&pdc_lock, flags);
enable = pdc_reg_read(IRQ_ENABLE_BANK, index);
- enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask);
+ __assign_bit(mask, &enable, on);
pdc_reg_write(IRQ_ENABLE_BANK, index, enable);
raw_spin_unlock_irqrestore(&pdc_lock, flags);
}
--
2.30.2
PDC_NO_PARENT_IRQ is pretty pointless, as all it indicates is
that the PDC terminates the interrupt hierarchy. Which is
exactly the same as not having a mapping in the GIC space.
This is also bad practice to treat the absence of a hwirq
as a hwirq itself.
Just explicitly use the region mapping pointer, and drop
the definition.
Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/qcom-pdc.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 173e6520e06e..3b214c4e6755 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -30,14 +30,14 @@
#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;
u32 cnt;
};
+#define pin_to_hwirq(r, p) ((r)->parent_base + (p) - (r)->pin_base)
+
static DEFINE_RAW_SPINLOCK(pdc_lock);
static void __iomem *pdc_base;
static struct pdc_pin_region *pdc_region;
@@ -186,19 +186,17 @@ static struct irq_chip qcom_pdc_gic_chip = {
.irq_set_affinity = irq_chip_set_affinity_parent,
};
-static irq_hw_number_t get_parent_hwirq(int pin)
+static struct pdc_pin_region *get_pin_region(int pin)
{
int i;
- struct pdc_pin_region *region;
for (i = 0; i < pdc_region_cnt; i++) {
- region = &pdc_region[i];
- if (pin >= region->pin_base &&
- pin < region->pin_base + region->cnt)
- return (region->parent_base + pin - region->pin_base);
+ if (pin >= pdc_region[i].pin_base &&
+ pin < pdc_region[i].pin_base + pdc_region[i].cnt)
+ return &pdc_region[i];
}
- return PDC_NO_PARENT_IRQ;
+ return NULL;
}
static int qcom_pdc_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
@@ -221,7 +219,8 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
{
struct irq_fwspec *fwspec = data;
struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq, parent_hwirq;
+ struct pdc_pin_region *region;
+ irq_hw_number_t hwirq;
unsigned int type;
int ret;
@@ -234,8 +233,8 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
if (ret)
return ret;
- parent_hwirq = get_parent_hwirq(hwirq);
- if (parent_hwirq == PDC_NO_PARENT_IRQ)
+ region = get_pin_region(hwirq);
+ if (!region)
return irq_domain_disconnect_hierarchy(domain->parent, virq);
if (type & IRQ_TYPE_EDGE_BOTH)
@@ -247,7 +246,7 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
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[1] = pin_to_hwirq(region, hwirq);
parent_fwspec.param[2] = type;
return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
@@ -265,7 +264,8 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
{
struct irq_fwspec *fwspec = data;
struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq, parent_hwirq;
+ struct pdc_pin_region *region;
+ irq_hw_number_t hwirq;
unsigned int type;
int ret;
@@ -281,8 +281,8 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
if (ret)
return ret;
- parent_hwirq = get_parent_hwirq(hwirq);
- if (parent_hwirq == PDC_NO_PARENT_IRQ)
+ region = get_pin_region(hwirq);
+ if (!region)
return irq_domain_disconnect_hierarchy(domain->parent, virq);
if (type & IRQ_TYPE_EDGE_BOTH)
@@ -294,7 +294,7 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
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[1] = pin_to_hwirq(region, hwirq);
parent_fwspec.param[2] = type;
return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
--
2.30.2
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: 024e9a021eb7baff3935d8c76fc2d668384398f7
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/024e9a021eb7baff3935d8c76fc2d668384398f7
Author: Marc Zyngier <[email protected]>
AuthorDate: Thu, 24 Feb 2022 10:12:26
Committer: Marc Zyngier <[email protected]>
CommitterDate: Mon, 28 Feb 2022 17:32:26
irqchip/qcom-pdc: Drop open coded version of __assign_bit()
The driver uses what looks like an open-coded version of __assign_bit().
Replace it with the real thing.
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/qcom-pdc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 0cd20dd..d96916c 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -23,9 +23,6 @@
#define PDC_MAX_GPIO_IRQS 256
-#define CLEAR_INTR(reg, intr) (reg & ~(1 << intr))
-#define ENABLE_INTR(reg, intr) (reg | (1 << intr))
-
#define IRQ_ENABLE_BANK 0x10
#define IRQ_i_CFG 0x110
@@ -55,16 +52,16 @@ static u32 pdc_reg_read(int reg, u32 i)
static void pdc_enable_intr(struct irq_data *d, bool on)
{
int pin_out = d->hwirq;
+ unsigned long enable;
unsigned long flags;
u32 index, mask;
- u32 enable;
index = pin_out / 32;
mask = pin_out % 32;
raw_spin_lock_irqsave(&pdc_lock, flags);
enable = pdc_reg_read(IRQ_ENABLE_BANK, index);
- enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask);
+ __assign_bit(mask, &enable, on);
pdc_reg_write(IRQ_ENABLE_BANK, index, enable);
raw_spin_unlock_irqrestore(&pdc_lock, flags);
}
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: 3b26296c1c56db020100f971a39b59e3fa14491f
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/3b26296c1c56db020100f971a39b59e3fa14491f
Author: Marc Zyngier <[email protected]>
AuthorDate: Thu, 24 Feb 2022 10:12:23
Committer: Marc Zyngier <[email protected]>
CommitterDate: Mon, 28 Feb 2022 17:32:25
irqchip/qcom-pdc: Kill non-wakeup irqdomain
A careful look at the way the PDC driver works shows that:
- all interrupts are in the same space
- all interrupts are treated the same
And yet the driver creates two domains based on whether
the interrupt gets mapped directly or from the pinctrl code,
which is obviously a waste of resources.
Kill the non-wakeup domain and unify all the interrupt handling.
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/qcom-pdc.c | 84 ++++---------------------------------
1 file changed, 10 insertions(+), 74 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 3b214c4..5be5314 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -21,7 +21,6 @@
#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))
@@ -228,51 +227,6 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
if (ret)
return ret;
- ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
- &qcom_pdc_gic_chip, NULL);
- if (ret)
- return ret;
-
- region = get_pin_region(hwirq);
- if (!region)
- return irq_domain_disconnect_hierarchy(domain->parent, virq);
-
- 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] = pin_to_hwirq(region, hwirq);
- parent_fwspec.param[2] = type;
-
- return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
- &parent_fwspec);
-}
-
-static const struct irq_domain_ops qcom_pdc_ops = {
- .translate = qcom_pdc_translate,
- .alloc = qcom_pdc_alloc,
- .free = irq_domain_free_irqs_common,
-};
-
-static int 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;
- struct pdc_pin_region *region;
- irq_hw_number_t hwirq;
- unsigned int type;
- int ret;
-
- ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
- if (ret)
- return ret;
-
if (hwirq == GPIO_NO_WAKE_IRQ)
return irq_domain_disconnect_hierarchy(domain, virq);
@@ -301,16 +255,9 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
&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,
+static const struct irq_domain_ops qcom_pdc_ops = {
+ .translate = qcom_pdc_translate,
+ .alloc = qcom_pdc_alloc,
.free = irq_domain_free_irqs_common,
};
@@ -361,7 +308,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 irq_domain *parent_domain, *pdc_domain;
int ret;
pdc_base = of_iomap(node, 0);
@@ -383,32 +330,21 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
goto fail;
}
- pdc_domain = irq_domain_create_hierarchy(parent_domain, 0, PDC_MAX_IRQS,
- of_fwnode_handle(node),
- &qcom_pdc_ops, NULL);
- if (!pdc_domain) {
- pr_err("%pOF: GIC domain add failed\n", node);
- ret = -ENOMEM;
- goto fail;
- }
-
- pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain,
+ pdc_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);
+ &qcom_pdc_ops, NULL);
+ if (!pdc_domain) {
+ pr_err("%pOF: PDC domain add failed\n", node);
ret = -ENOMEM;
- goto remove;
+ goto fail;
}
- irq_domain_update_bus_token(pdc_gpio_domain, DOMAIN_BUS_WAKEUP);
+ irq_domain_update_bus_token(pdc_domain, DOMAIN_BUS_WAKEUP);
return 0;
-remove:
- irq_domain_remove(pdc_domain);
fail:
kfree(pdc_region);
iounmap(pdc_base);
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: 9fbc4f3979658ad30f3239d6a3660892976a8206
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/9fbc4f3979658ad30f3239d6a3660892976a8206
Author: Marc Zyngier <[email protected]>
AuthorDate: Thu, 24 Feb 2022 10:12:22
Committer: Marc Zyngier <[email protected]>
CommitterDate: Mon, 28 Feb 2022 17:32:25
irqchip/qcom-pdc: Kill PDC_NO_PARENT_IRQ
PDC_NO_PARENT_IRQ is pretty pointless, as all it indicates is
that the PDC terminates the interrupt hierarchy. Which is
exactly the same as not having a mapping in the GIC space.
This is also bad practice to treat the absence of a hwirq
as a hwirq itself.
Just explicitly use the region mapping pointer, and drop
the definition.
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/qcom-pdc.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 173e652..3b214c4 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -30,14 +30,14 @@
#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;
u32 cnt;
};
+#define pin_to_hwirq(r, p) ((r)->parent_base + (p) - (r)->pin_base)
+
static DEFINE_RAW_SPINLOCK(pdc_lock);
static void __iomem *pdc_base;
static struct pdc_pin_region *pdc_region;
@@ -186,19 +186,17 @@ static struct irq_chip qcom_pdc_gic_chip = {
.irq_set_affinity = irq_chip_set_affinity_parent,
};
-static irq_hw_number_t get_parent_hwirq(int pin)
+static struct pdc_pin_region *get_pin_region(int pin)
{
int i;
- struct pdc_pin_region *region;
for (i = 0; i < pdc_region_cnt; i++) {
- region = &pdc_region[i];
- if (pin >= region->pin_base &&
- pin < region->pin_base + region->cnt)
- return (region->parent_base + pin - region->pin_base);
+ if (pin >= pdc_region[i].pin_base &&
+ pin < pdc_region[i].pin_base + pdc_region[i].cnt)
+ return &pdc_region[i];
}
- return PDC_NO_PARENT_IRQ;
+ return NULL;
}
static int qcom_pdc_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
@@ -221,7 +219,8 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
{
struct irq_fwspec *fwspec = data;
struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq, parent_hwirq;
+ struct pdc_pin_region *region;
+ irq_hw_number_t hwirq;
unsigned int type;
int ret;
@@ -234,8 +233,8 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
if (ret)
return ret;
- parent_hwirq = get_parent_hwirq(hwirq);
- if (parent_hwirq == PDC_NO_PARENT_IRQ)
+ region = get_pin_region(hwirq);
+ if (!region)
return irq_domain_disconnect_hierarchy(domain->parent, virq);
if (type & IRQ_TYPE_EDGE_BOTH)
@@ -247,7 +246,7 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
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[1] = pin_to_hwirq(region, hwirq);
parent_fwspec.param[2] = type;
return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
@@ -265,7 +264,8 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
{
struct irq_fwspec *fwspec = data;
struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq, parent_hwirq;
+ struct pdc_pin_region *region;
+ irq_hw_number_t hwirq;
unsigned int type;
int ret;
@@ -281,8 +281,8 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
if (ret)
return ret;
- parent_hwirq = get_parent_hwirq(hwirq);
- if (parent_hwirq == PDC_NO_PARENT_IRQ)
+ region = get_pin_region(hwirq);
+ if (!region)
return irq_domain_disconnect_hierarchy(domain->parent, virq);
if (type & IRQ_TYPE_EDGE_BOTH)
@@ -294,7 +294,7 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
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[1] = pin_to_hwirq(region, hwirq);
parent_fwspec.param[2] = type;
return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
Hi,
On 2/24/2022 3:42 PM, Marc Zyngier wrote:
> PDC_NO_PARENT_IRQ is pretty pointless, as all it indicates is
> that the PDC terminates the interrupt hierarchy. Which is
> exactly the same as not having a mapping in the GIC space.
> This is also bad practice to treat the absence of a hwirq
> as a hwirq itself.
>
> Just explicitly use the region mapping pointer, and drop
> the definition.
>
> Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Maulik Shah <[email protected]>
Thanks,
Maulik
> ---
> drivers/irqchip/qcom-pdc.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
>
Hi,
On 2/24/2022 3:42 PM, Marc Zyngier wrote:
> A careful look at the way the PDC driver works shows that:
>
> - all interrupts are in the same space
> - all interrupts are treated the same
>
> And yet the driver creates two domains based on whether
> the interrupt gets mapped directly or from the pinctrl code,
> which is obviously a waste of resources.
The GPIO is kept under separate domain to handle extra configuration for
wake GPIO handling.
On targets like SM8150/SM8250 each wake up capable GPIO (if totan n)
line has dedicated parent PDC irq (if total m, n = m) associated with it.
However on targets like sdx55 PDC has muxes where each wake GPIO (if
total n) line goes through each PDC muxes (if total m, n > m) and
any of these muxes can be used to route any one GPIO to PDC (and parent
GIC) but unlike other targets it doesn't have one to one mapping for
GPIO to GIC interrupt.
So this will need to be kept as is to support sdx55 target.
Thanks,
Maulik
>
> Kill the non-wakeup domain and unify all the interrupt handling.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
> drivers/irqchip/qcom-pdc.c | 84 +++++---------------------------------
> 1 file changed, 10 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 3b214c4e6755..5be531403f50 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
> @@ -21,7 +21,6 @@
> #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))
> @@ -224,51 +223,6 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
> 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;
> -
> - region = get_pin_region(hwirq);
> - if (!region)
> - return irq_domain_disconnect_hierarchy(domain->parent, virq);
> -
> - 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] = pin_to_hwirq(region, hwirq);
> - parent_fwspec.param[2] = type;
> -
> - return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> - &parent_fwspec);
> -}
> -
> -static const struct irq_domain_ops qcom_pdc_ops = {
> - .translate = qcom_pdc_translate,
> - .alloc = qcom_pdc_alloc,
> - .free = irq_domain_free_irqs_common,
> -};
> -
> -static int 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;
> - struct pdc_pin_region *region;
> - irq_hw_number_t hwirq;
> - unsigned int type;
> - int ret;
> -
> ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
> if (ret)
> return ret;
> @@ -301,16 +255,9 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
> &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,
> +static const struct irq_domain_ops qcom_pdc_ops = {
> + .translate = qcom_pdc_translate,
> + .alloc = qcom_pdc_alloc,
> .free = irq_domain_free_irqs_common,
> };
>
> @@ -361,7 +308,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 irq_domain *parent_domain, *pdc_domain;
> int ret;
>
> pdc_base = of_iomap(node, 0);
> @@ -383,32 +330,21 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
> goto fail;
> }
>
> - pdc_domain = irq_domain_create_hierarchy(parent_domain, 0, PDC_MAX_IRQS,
> - of_fwnode_handle(node),
> - &qcom_pdc_ops, NULL);
> - if (!pdc_domain) {
> - pr_err("%pOF: GIC domain add failed\n", node);
> - ret = -ENOMEM;
> - goto fail;
> - }
> -
> - pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain,
> + pdc_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);
> + &qcom_pdc_ops, NULL);
> + if (!pdc_domain) {
> + pr_err("%pOF: PDC domain add failed\n", node);
> ret = -ENOMEM;
> - goto remove;
> + goto fail;
> }
>
> - irq_domain_update_bus_token(pdc_gpio_domain, DOMAIN_BUS_WAKEUP);
> + irq_domain_update_bus_token(pdc_domain, DOMAIN_BUS_WAKEUP);
>
> return 0;
>
> -remove:
> - irq_domain_remove(pdc_domain);
> fail:
> kfree(pdc_region);
> iounmap(pdc_base);
Hi,
On 2/24/2022 3:42 PM, Marc Zyngier wrote:
> The driver uses what looks like an open-coded version of __assign_bit().
> Replace it with the real thing.
>
> Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Maulik Shah <[email protected]>
Thanks,
Maulik
> ---
> drivers/irqchip/qcom-pdc.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
>
On Mon, 28 Feb 2022 19:29:41 +0000,
"Maulik Shah (mkshah)" <[email protected]> wrote:
>
> Hi,
>
> On 2/24/2022 3:42 PM, Marc Zyngier wrote:
> > A careful look at the way the PDC driver works shows that:
> >
> > - all interrupts are in the same space
> > - all interrupts are treated the same
> >
> > And yet the driver creates two domains based on whether
> > the interrupt gets mapped directly or from the pinctrl code,
> > which is obviously a waste of resources.
> The GPIO is kept under separate domain to handle extra configuration
> for wake GPIO handling.
Which extra configuration? The irq_chip structure is the same, the
translation is the same, the GIC mapping is the same, and the select
method only serves to select between two irq domains that do the same
thing.
So please point me to what the difference is.
>
> On targets like SM8150/SM8250 each wake up capable GPIO (if totan n)
> line has dedicated parent PDC irq (if total m, n = m) associated with
> it.
> However on targets like sdx55 PDC has muxes where each wake GPIO (if
> total n) line goes through each PDC muxes (if total m, n > m) and
> any of these muxes can be used to route any one GPIO to PDC (and
> parent GIC) but unlike other targets it doesn't have one to one
> mapping for GPIO to GIC interrupt.
> So this will need to be kept as is to support sdx55 target.
As far as I can tell, the current code doesn't have any support for
this. And if there is a mux involved in the interrupt routing, this
should be something entierly separate, as the current code is strictly
hierarchical.
What am I missing?
M.
--
Without deviation from the norm, progress is not possible.
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: d2febf6bbec5466824432e3d8850fc49e4343572
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/d2febf6bbec5466824432e3d8850fc49e4343572
Author: Marc Zyngier <[email protected]>
AuthorDate: Thu, 24 Feb 2022 10:12:26
Committer: Marc Zyngier <[email protected]>
CommitterDate: Tue, 01 Mar 2022 10:06:25
irqchip/qcom-pdc: Drop open coded version of __assign_bit()
The driver uses what looks like an open-coded version of __assign_bit().
Replace it with the real thing.
Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Maulik Shah <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/qcom-pdc.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 0cd20dd..d96916c 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -23,9 +23,6 @@
#define PDC_MAX_GPIO_IRQS 256
-#define CLEAR_INTR(reg, intr) (reg & ~(1 << intr))
-#define ENABLE_INTR(reg, intr) (reg | (1 << intr))
-
#define IRQ_ENABLE_BANK 0x10
#define IRQ_i_CFG 0x110
@@ -55,16 +52,16 @@ static u32 pdc_reg_read(int reg, u32 i)
static void pdc_enable_intr(struct irq_data *d, bool on)
{
int pin_out = d->hwirq;
+ unsigned long enable;
unsigned long flags;
u32 index, mask;
- u32 enable;
index = pin_out / 32;
mask = pin_out % 32;
raw_spin_lock_irqsave(&pdc_lock, flags);
enable = pdc_reg_read(IRQ_ENABLE_BANK, index);
- enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask);
+ __assign_bit(mask, &enable, on);
pdc_reg_write(IRQ_ENABLE_BANK, index, enable);
raw_spin_unlock_irqrestore(&pdc_lock, flags);
}
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: 8d4c998919320206f8832dc413e23fdd27ef2274
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/8d4c998919320206f8832dc413e23fdd27ef2274
Author: Marc Zyngier <[email protected]>
AuthorDate: Thu, 24 Feb 2022 10:12:22
Committer: Marc Zyngier <[email protected]>
CommitterDate: Tue, 01 Mar 2022 10:06:24
irqchip/qcom-pdc: Kill PDC_NO_PARENT_IRQ
PDC_NO_PARENT_IRQ is pretty pointless, as all it indicates is
that the PDC terminates the interrupt hierarchy. Which is
exactly the same as not having a mapping in the GIC space.
This is also bad practice to treat the absence of a hwirq
as a hwirq itself.
Just explicitly use the region mapping pointer, and drop
the definition.
Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Maulik Shah <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/qcom-pdc.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 173e652..3b214c4 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -30,14 +30,14 @@
#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;
u32 cnt;
};
+#define pin_to_hwirq(r, p) ((r)->parent_base + (p) - (r)->pin_base)
+
static DEFINE_RAW_SPINLOCK(pdc_lock);
static void __iomem *pdc_base;
static struct pdc_pin_region *pdc_region;
@@ -186,19 +186,17 @@ static struct irq_chip qcom_pdc_gic_chip = {
.irq_set_affinity = irq_chip_set_affinity_parent,
};
-static irq_hw_number_t get_parent_hwirq(int pin)
+static struct pdc_pin_region *get_pin_region(int pin)
{
int i;
- struct pdc_pin_region *region;
for (i = 0; i < pdc_region_cnt; i++) {
- region = &pdc_region[i];
- if (pin >= region->pin_base &&
- pin < region->pin_base + region->cnt)
- return (region->parent_base + pin - region->pin_base);
+ if (pin >= pdc_region[i].pin_base &&
+ pin < pdc_region[i].pin_base + pdc_region[i].cnt)
+ return &pdc_region[i];
}
- return PDC_NO_PARENT_IRQ;
+ return NULL;
}
static int qcom_pdc_translate(struct irq_domain *d, struct irq_fwspec *fwspec,
@@ -221,7 +219,8 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
{
struct irq_fwspec *fwspec = data;
struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq, parent_hwirq;
+ struct pdc_pin_region *region;
+ irq_hw_number_t hwirq;
unsigned int type;
int ret;
@@ -234,8 +233,8 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
if (ret)
return ret;
- parent_hwirq = get_parent_hwirq(hwirq);
- if (parent_hwirq == PDC_NO_PARENT_IRQ)
+ region = get_pin_region(hwirq);
+ if (!region)
return irq_domain_disconnect_hierarchy(domain->parent, virq);
if (type & IRQ_TYPE_EDGE_BOTH)
@@ -247,7 +246,7 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
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[1] = pin_to_hwirq(region, hwirq);
parent_fwspec.param[2] = type;
return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
@@ -265,7 +264,8 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
{
struct irq_fwspec *fwspec = data;
struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq, parent_hwirq;
+ struct pdc_pin_region *region;
+ irq_hw_number_t hwirq;
unsigned int type;
int ret;
@@ -281,8 +281,8 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
if (ret)
return ret;
- parent_hwirq = get_parent_hwirq(hwirq);
- if (parent_hwirq == PDC_NO_PARENT_IRQ)
+ region = get_pin_region(hwirq);
+ if (!region)
return irq_domain_disconnect_hierarchy(domain->parent, virq);
if (type & IRQ_TYPE_EDGE_BOTH)
@@ -294,7 +294,7 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
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[1] = pin_to_hwirq(region, hwirq);
parent_fwspec.param[2] = type;
return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: 4dc70713dc24dceeea7f106828674744a6294860
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/4dc70713dc24dceeea7f106828674744a6294860
Author: Marc Zyngier <[email protected]>
AuthorDate: Thu, 24 Feb 2022 10:12:23
Committer: Marc Zyngier <[email protected]>
CommitterDate: Tue, 01 Mar 2022 10:06:24
irqchip/qcom-pdc: Kill non-wakeup irqdomain
A careful look at the way the PDC driver works shows that:
- all interrupts are in the same space
- all interrupts are treated the same
And yet the driver creates two domains based on whether
the interrupt gets mapped directly or from the pinctrl code,
which is obviously a waste of resources.
Kill the non-wakeup domain and unify all the interrupt handling.
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/qcom-pdc.c | 84 ++++---------------------------------
1 file changed, 10 insertions(+), 74 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 3b214c4..5be5314 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -21,7 +21,6 @@
#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))
@@ -228,51 +227,6 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
if (ret)
return ret;
- ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
- &qcom_pdc_gic_chip, NULL);
- if (ret)
- return ret;
-
- region = get_pin_region(hwirq);
- if (!region)
- return irq_domain_disconnect_hierarchy(domain->parent, virq);
-
- 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] = pin_to_hwirq(region, hwirq);
- parent_fwspec.param[2] = type;
-
- return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
- &parent_fwspec);
-}
-
-static const struct irq_domain_ops qcom_pdc_ops = {
- .translate = qcom_pdc_translate,
- .alloc = qcom_pdc_alloc,
- .free = irq_domain_free_irqs_common,
-};
-
-static int 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;
- struct pdc_pin_region *region;
- irq_hw_number_t hwirq;
- unsigned int type;
- int ret;
-
- ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type);
- if (ret)
- return ret;
-
if (hwirq == GPIO_NO_WAKE_IRQ)
return irq_domain_disconnect_hierarchy(domain, virq);
@@ -301,16 +255,9 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
&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,
+static const struct irq_domain_ops qcom_pdc_ops = {
+ .translate = qcom_pdc_translate,
+ .alloc = qcom_pdc_alloc,
.free = irq_domain_free_irqs_common,
};
@@ -361,7 +308,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 irq_domain *parent_domain, *pdc_domain;
int ret;
pdc_base = of_iomap(node, 0);
@@ -383,32 +330,21 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
goto fail;
}
- pdc_domain = irq_domain_create_hierarchy(parent_domain, 0, PDC_MAX_IRQS,
- of_fwnode_handle(node),
- &qcom_pdc_ops, NULL);
- if (!pdc_domain) {
- pr_err("%pOF: GIC domain add failed\n", node);
- ret = -ENOMEM;
- goto fail;
- }
-
- pdc_gpio_domain = irq_domain_create_hierarchy(parent_domain,
+ pdc_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);
+ &qcom_pdc_ops, NULL);
+ if (!pdc_domain) {
+ pr_err("%pOF: PDC domain add failed\n", node);
ret = -ENOMEM;
- goto remove;
+ goto fail;
}
- irq_domain_update_bus_token(pdc_gpio_domain, DOMAIN_BUS_WAKEUP);
+ irq_domain_update_bus_token(pdc_domain, DOMAIN_BUS_WAKEUP);
return 0;
-remove:
- irq_domain_remove(pdc_domain);
fail:
kfree(pdc_region);
iounmap(pdc_base);