* [PATCH v2 1/8] irqchip/qcom-pdc: restructure version support
2026-05-26 10:54 [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
@ 2026-05-26 10:54 ` Maulik Shah
2026-05-26 10:54 ` [PATCH v2 2/8] irqchip/qcom-pdc: Move all statics to struct pdc_desc Maulik Shah
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Maulik Shah @ 2026-05-26 10:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad,
Maulik Shah
PDC irqchip updates IRQ_ENABLE and IRQ_CFG and for three different
versions v2.7, v3.0 and v3.2. These registers are organized in H/W
as below on various SoCs.
+---------------------------------------------------------------+
| SM8350, SM8450 | SM8550, Hamoa | SM8650, SM8750 |
|---------------------------------------------------------------|
| v2.7 | v3.0 | v3.2 |
|---------------------------------------------------------------|
| IRQ_ENABLE_BANK | IRQ_ENABLE_BANK | NA |
|---------------------------------------------------------------|
| IRQ_CFG | IRQ_CFG | IRQ_CFG |
| | | |
| | | [31:6] Unused |
| | [31:5] Unused | [5] GPIO_STATUS |
| | [4] GPIO_STATUS| [4] GPIO_MASK |
| [31:3] Unused | [3] GPIO_MASK | [3] IRQ_ENABLE |
| [0:2] Type | [0:2] Type | [0:2] Type |
+---------------------------------------------------------------|
All SoCs PDC irqchip supports "pass through mode" in which all interrupts
are forwarded to the GIC without any latching at PDC H/W.
So far irqchip did not utilize GPIO_STATUS and GPIO_MASK from IRQ_CFG
register for v3.0 and v3.2 since they are only needed to be configured
when PDC runs in specific mode named "second level interrupt controller"
where it can latch the GPIO interrupts in GPIO_STATUS and forward GPIO
interrupts to GIC as LEVEL_HIGH type SPI interrupt.
All the SoCs defaulted to pass through mode with the exception of some
x1e. x1e PDC may be set to secondary controller mode for builds on CRD
boards whereas it may be set to pass through mode for IoT-EVK boards.
Restructure in preparation to add the second level interrupt controller
mode utilizing GPIO_STATUS and GPIO_MASK bits which changed the bit
positions between v3.0 and v3.2.
No functional impact with the change.
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
drivers/irqchip/qcom-pdc.c | 193 +++++++++++++++++++++++++++++++++++----------
1 file changed, 150 insertions(+), 43 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 638b5d89a141..c5b64649b2a9 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -21,18 +21,10 @@
#include <linux/slab.h>
#include <linux/types.h>
-#define PDC_MAX_GPIO_IRQS 256
-#define PDC_DRV_SIZE 0x10000
-
-/* Valid only on HW version < 3.2 */
-#define IRQ_ENABLE_BANK 0x10
-#define IRQ_ENABLE_BANK_MAX (IRQ_ENABLE_BANK + BITS_TO_BYTES(PDC_MAX_GPIO_IRQS))
-#define IRQ_i_CFG 0x110
+#define PDC_MAX_IRQS 256
+#define IRQ_ENABLE_BANK_MAX BITS_TO_BYTES(PDC_MAX_IRQS)
-/* Valid only on HW version >= 3.2 */
-#define IRQ_i_CFG_IRQ_ENABLE 3
-
-#define IRQ_i_CFG_TYPE_MASK GENMASK(2, 0)
+#define PDC_DRV_SIZE 0x10000
#define PDC_VERSION_REG 0x1000
#define PDC_VERSION_MAJOR GENMASK(23, 16)
@@ -45,6 +37,98 @@
/* Notable PDC versions */
#define PDC_VERSION_3_2 PDC_VERSION(3, 2, 0)
+#define PDC_VERSION_3_0 PDC_VERSION(3, 0, 0)
+#define PDC_VERSION_2_7 PDC_VERSION(2, 7, 0)
+
+/*
+ * PDC H/W registers layout per version:
+ *
+ * IRQ_ENABLE_BANK[b], b = 0....BITS_TO_BYTES(PDC_MAX_IRQS)
+ * IRQ_CFG[n], n = 0....PDC_MAX_IRQS
+ *
+ * +---------------------------------------------------------------+
+ * | v2.7 | v3.0 | v3.2 |
+ * |---------------------------------------------------------------|
+ * | BASE | BASE | BASE |
+ * |---------------------------------------------------------------|
+ * | |
+ * | IRQ_ENABLE_BANK | IRQ_ENABLE_BANK | NA |
+ * |---------------------------------------------------------------|
+ * | IRQ_CFG | IRQ_CFG | IRQ_CFG |
+ * | | | |
+ * | | | [31:6] Unused |
+ * | | [31:5] Unused | [5] GPIO_STATUS |
+ * | | [4] GPIO_STATUS| [4] GPIO_MASK |
+ * | [31:3] Unused | [3] GPIO_MASK | [3] IRQ_ENABLE |
+ * | [0:2] Type | [0:2] Type | [0:2] Type |
+ * +---------------------------------------------------------------+
+ */
+
+/**
+ * struct pdc_regs: PDC registers location
+ *
+ * @irq_en_reg: IRQ_ENABLE_BANK register location
+ * @irq_cfg_reg: IRQ_CFG register location
+ */
+struct pdc_regs {
+ u32 irq_en_reg;
+ u32 irq_cfg_reg;
+};
+
+/**
+ * struct pdc_cfg: bit fields for PDC IRQ_CFG register
+ *
+ * @irq_enable: bit mask for IRQ_ENABLE
+ * @irq_type: bit mask for IRQ_TYPE
+ */
+struct pdc_cfg {
+ u32 irq_enable;
+ u32 irq_type;
+};
+
+/**
+ * struct pdc_desc: PDC driver state
+ *
+ * @base: PDC base register for DRV2 / HLOS
+ * @prev_base: PDC DRV1 base, applicable only for x1e RTL bug.
+ * @version: PDC version
+ * @regs: PDC regs (IRQ_ENABLE_BANK and IRQ_CFG)
+ * @cfg: bit masks within for IRQ_CFG reg
+ */
+struct pdc_desc {
+ void __iomem *base;
+ void __iomem *prev_base;
+ u32 version;
+ const struct pdc_regs *regs;
+ const struct pdc_cfg *cfg;
+};
+
+static const struct pdc_regs pdc_v3_2 = {
+ .irq_cfg_reg = 0x110,
+};
+
+static const struct pdc_cfg pdc_cfg_v3_2 = {
+ .irq_enable = GENMASK(3, 3),
+ .irq_type = GENMASK(2, 0),
+};
+
+static const struct pdc_regs pdc_v3_0 = {
+ .irq_en_reg = 0x10,
+ .irq_cfg_reg = 0x110,
+};
+
+static const struct pdc_cfg pdc_cfg_v3_0 = {
+ .irq_type = GENMASK(2, 0),
+};
+
+static const struct pdc_regs pdc_v2_7 = {
+ .irq_en_reg = 0x10,
+ .irq_cfg_reg = 0x110,
+};
+
+static const struct pdc_cfg pdc_cfg_v2_7 = {
+ .irq_type = GENMASK(2, 0),
+};
struct pdc_pin_region {
u32 pin_base;
@@ -55,12 +139,11 @@ struct pdc_pin_region {
#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 void __iomem *pdc_prev_base;
static struct pdc_pin_region *pdc_region;
static int pdc_region_cnt;
static void (*__pdc_enable_intr)(int pin_out, bool on);
static bool pdc_x1e_quirk;
+static struct pdc_desc *pdc;
static void pdc_base_reg_write(void __iomem *base, int reg, u32 i, u32 val)
{
@@ -69,12 +152,12 @@ static void pdc_base_reg_write(void __iomem *base, int reg, u32 i, u32 val)
static void pdc_reg_write(int reg, u32 i, u32 val)
{
- pdc_base_reg_write(pdc_base, reg, i, val);
+ pdc_base_reg_write(pdc->base, reg, i, val);
}
static u32 pdc_reg_read(int reg, u32 i)
{
- return readl_relaxed(pdc_base + reg + i * sizeof(u32));
+ return readl_relaxed(pdc->base + reg + i * sizeof(u32));
}
static void pdc_x1e_irq_enable_write(u32 bank, u32 enable)
@@ -85,24 +168,24 @@ static void pdc_x1e_irq_enable_write(u32 bank, u32 enable)
switch (bank) {
case 0 ... 1:
/* Use previous DRV (client) region and shift to bank 3-4 */
- base = pdc_prev_base;
+ base = pdc->prev_base;
bank += 3;
break;
case 2 ... 4:
/* Use our own region and shift to bank 0-2 */
- base = pdc_base;
+ base = pdc->base;
bank -= 2;
break;
case 5:
/* No fixup required for bank 5 */
- base = pdc_base;
+ base = pdc->base;
break;
default:
WARN_ON(1);
return;
}
- pdc_base_reg_write(base, IRQ_ENABLE_BANK, bank, enable);
+ pdc_base_reg_write(base, pdc->regs->irq_en_reg, bank, enable);
}
static void pdc_enable_intr_bank(int pin_out, bool on)
@@ -113,22 +196,25 @@ static void pdc_enable_intr_bank(int pin_out, bool on)
index = FIELD_GET(GENMASK(31, 5), pin_out);
mask = FIELD_GET(GENMASK(4, 0), pin_out);
- enable = pdc_reg_read(IRQ_ENABLE_BANK, index);
+ enable = pdc_reg_read(pdc->regs->irq_en_reg, index);
__assign_bit(mask, &enable, on);
if (pdc_x1e_quirk)
pdc_x1e_irq_enable_write(index, enable);
else
- pdc_reg_write(IRQ_ENABLE_BANK, index, enable);
+ pdc_reg_write(pdc->regs->irq_en_reg, index, enable);
}
static void pdc_enable_intr_cfg(int pin_out, bool on)
{
unsigned long enable;
- enable = pdc_reg_read(IRQ_i_CFG, pin_out);
- __assign_bit(IRQ_i_CFG_IRQ_ENABLE, &enable, on);
- pdc_reg_write(IRQ_i_CFG, pin_out, enable);
+ enable = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
+ if (on)
+ enable |= pdc->cfg->irq_enable;
+ else
+ enable &= ~pdc->cfg->irq_enable;
+ pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, enable);
}
static void pdc_enable_intr(struct irq_data *d, bool on)
@@ -216,9 +302,9 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
return -EINVAL;
}
- old_pdc_type = pdc_reg_read(IRQ_i_CFG, d->hwirq);
- pdc_type |= (old_pdc_type & ~IRQ_i_CFG_TYPE_MASK);
- pdc_reg_write(IRQ_i_CFG, d->hwirq, pdc_type);
+ old_pdc_type = pdc_reg_read(pdc->regs->irq_cfg_reg, d->hwirq);
+ pdc_type |= (old_pdc_type & ~pdc->cfg->irq_type);
+ pdc_reg_write(pdc->regs->irq_cfg_reg, d->hwirq, pdc_type);
ret = irq_chip_set_type_parent(d, type);
if (ret)
@@ -375,6 +461,34 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
if (res_size > resource_size(&res))
pr_warn("%pOF: invalid reg size, please fix DT\n", node);
+ pdc = kzalloc_objs(*pdc, GFP_KERNEL);
+ if (!pdc)
+ return -ENOMEM;
+
+ pdc->base = ioremap(res.start, res_size);
+ if (!pdc->base) {
+ pr_err("%pOF: unable to map PDC registers\n", node);
+ ret = -ENXIO;
+ goto fail;
+ }
+
+ pdc->version = pdc_reg_read(PDC_VERSION_REG, 0);
+
+ if (pdc->version >= PDC_VERSION_3_2) {
+ pdc->cfg = &pdc_cfg_v3_2;
+ pdc->regs = &pdc_v3_2;
+ __pdc_enable_intr = pdc_enable_intr_cfg;
+ } else if (pdc->version < PDC_VERSION_3_2 &&
+ pdc->version >= PDC_VERSION_3_0) {
+ pdc->cfg = &pdc_cfg_v3_0;
+ pdc->regs = &pdc_v3_0;
+ __pdc_enable_intr = pdc_enable_intr_bank;
+ } else {
+ pdc->cfg = &pdc_cfg_v2_7;
+ pdc->regs = &pdc_v2_7;
+ __pdc_enable_intr = pdc_enable_intr_bank;
+ }
+
/*
* PDC has multiple DRV regions, each one provides the same set of
* registers for a particular client in the system. Due to a hardware
@@ -384,25 +498,17 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
* region with the expected offset to preserve support for old DTs.
*/
if (of_device_is_compatible(node, "qcom,x1e80100-pdc")) {
- pdc_prev_base = ioremap(res.start - PDC_DRV_SIZE, IRQ_ENABLE_BANK_MAX);
- if (!pdc_prev_base) {
+ pdc->prev_base = ioremap(res.start - PDC_DRV_SIZE,
+ pdc->regs->irq_en_reg + IRQ_ENABLE_BANK_MAX);
+ if (!pdc->prev_base) {
pr_err("%pOF: unable to map previous PDC DRV region\n", node);
- return -ENXIO;
+ ret = -ENXIO;
+ goto fail;
}
pdc_x1e_quirk = true;
}
- pdc_base = ioremap(res.start, res_size);
- if (!pdc_base) {
- pr_err("%pOF: unable to map PDC registers\n", node);
- ret = -ENXIO;
- goto fail;
- }
-
- __pdc_enable_intr = (pdc_reg_read(PDC_VERSION_REG, 0) < PDC_VERSION_3_2) ?
- pdc_enable_intr_bank : pdc_enable_intr_cfg;
-
parent_domain = irq_find_host(parent);
if (!parent_domain) {
pr_err("%pOF: unable to find PDC's parent domain\n", node);
@@ -418,7 +524,7 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
pdc_domain = irq_domain_create_hierarchy(parent_domain,
IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
- PDC_MAX_GPIO_IRQS,
+ PDC_MAX_IRQS,
of_fwnode_handle(node),
&qcom_pdc_ops, NULL);
if (!pdc_domain) {
@@ -433,8 +539,9 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
fail:
kfree(pdc_region);
- iounmap(pdc_base);
- iounmap(pdc_prev_base);
+ iounmap(pdc->base);
+ iounmap(pdc->prev_base);
+ kfree(pdc);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 2/8] irqchip/qcom-pdc: Move all statics to struct pdc_desc
2026-05-26 10:54 [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
2026-05-26 10:54 ` [PATCH v2 1/8] irqchip/qcom-pdc: restructure version support Maulik Shah
@ 2026-05-26 10:54 ` Maulik Shah
2026-05-26 10:54 ` [PATCH v2 3/8] irqchip/qcom-pdc: Remove pdc_enable_intr() wrapper Maulik Shah
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Maulik Shah @ 2026-05-26 10:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad,
Maulik Shah
There are multiple statics used. Move all to struct pdc_desc to better
align with versioning support. Document them.
No functional impact.
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
drivers/irqchip/qcom-pdc.c | 68 +++++++++++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 28 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index c5b64649b2a9..8f7802139e4e 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -92,15 +92,30 @@ struct pdc_cfg {
* @base: PDC base register for DRV2 / HLOS
* @prev_base: PDC DRV1 base, applicable only for x1e RTL bug.
* @version: PDC version
+ * @region: PDC interrupt continuous range
+ * @region_cnt: Total PDC ranges
+ * @x1e_quirk: x1e H/W Bug handling
+ * @lock: lock for IRQ_ENABLE_BANK protection
* @regs: PDC regs (IRQ_ENABLE_BANK and IRQ_CFG)
* @cfg: bit masks within for IRQ_CFG reg
+ * @enable_intr: pointer to enable function based on PDC version
*/
struct pdc_desc {
void __iomem *base;
void __iomem *prev_base;
u32 version;
+
+ struct pdc_pin_region *region;
+ int region_cnt;
+
+ bool x1e_quirk;
+
+ raw_spinlock_t lock;
+
const struct pdc_regs *regs;
const struct pdc_cfg *cfg;
+
+ void (*enable_intr)(int pin_out, bool on);
};
static const struct pdc_regs pdc_v3_2 = {
@@ -138,11 +153,6 @@ struct pdc_pin_region {
#define pin_to_hwirq(r, p) ((r)->parent_base + (p) - (r)->pin_base)
-static DEFINE_RAW_SPINLOCK(pdc_lock);
-static struct pdc_pin_region *pdc_region;
-static int pdc_region_cnt;
-static void (*__pdc_enable_intr)(int pin_out, bool on);
-static bool pdc_x1e_quirk;
static struct pdc_desc *pdc;
static void pdc_base_reg_write(void __iomem *base, int reg, u32 i, u32 val)
@@ -199,7 +209,7 @@ static void pdc_enable_intr_bank(int pin_out, bool on)
enable = pdc_reg_read(pdc->regs->irq_en_reg, index);
__assign_bit(mask, &enable, on);
- if (pdc_x1e_quirk)
+ if (pdc->x1e_quirk)
pdc_x1e_irq_enable_write(index, enable);
else
pdc_reg_write(pdc->regs->irq_en_reg, index, enable);
@@ -221,9 +231,9 @@ static void pdc_enable_intr(struct irq_data *d, bool on)
{
unsigned long flags;
- raw_spin_lock_irqsave(&pdc_lock, flags);
- __pdc_enable_intr(d->hwirq, on);
- raw_spin_unlock_irqrestore(&pdc_lock, flags);
+ raw_spin_lock_irqsave(&pdc->lock, flags);
+ pdc->enable_intr(d->hwirq, on);
+ raw_spin_unlock_irqrestore(&pdc->lock, flags);
}
static void qcom_pdc_gic_disable(struct irq_data *d)
@@ -348,10 +358,10 @@ static struct pdc_pin_region *get_pin_region(int pin)
{
int i;
- for (i = 0; i < pdc_region_cnt; i++) {
- if (pin >= pdc_region[i].pin_base &&
- pin < pdc_region[i].pin_base + pdc_region[i].cnt)
- return &pdc_region[i];
+ for (i = 0; i < pdc->region_cnt; i++) {
+ if (pin >= pdc->region[i].pin_base &&
+ pin < pdc->region[i].pin_base + pdc->region[i].cnt)
+ return &pdc->region[i];
}
return NULL;
@@ -413,32 +423,32 @@ static int pdc_setup_pin_mapping(struct device_node *np)
if (n <= 0 || n % 3)
return -EINVAL;
- pdc_region_cnt = n / 3;
- pdc_region = kzalloc_objs(*pdc_region, pdc_region_cnt);
- if (!pdc_region) {
- pdc_region_cnt = 0;
+ pdc->region_cnt = n / 3;
+ pdc->region = kzalloc_objs(*pdc->region, pdc->region_cnt);
+ if (!pdc->region) {
+ pdc->region_cnt = 0;
return -ENOMEM;
}
- for (n = 0; n < pdc_region_cnt; n++) {
+ for (n = 0; n < pdc->region_cnt; n++) {
ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
n * 3 + 0,
- &pdc_region[n].pin_base);
+ &pdc->region[n].pin_base);
if (ret)
return ret;
ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
n * 3 + 1,
- &pdc_region[n].parent_base);
+ &pdc->region[n].parent_base);
if (ret)
return ret;
ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
n * 3 + 2,
- &pdc_region[n].cnt);
+ &pdc->region[n].cnt);
if (ret)
return ret;
- for (i = 0; i < pdc_region[n].cnt; i++)
- __pdc_enable_intr(i + pdc_region[n].pin_base, 0);
+ for (i = 0; i < pdc->region[n].cnt; i++)
+ pdc->enable_intr(i + pdc->region[n].pin_base, 0);
}
return 0;
@@ -477,16 +487,16 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
if (pdc->version >= PDC_VERSION_3_2) {
pdc->cfg = &pdc_cfg_v3_2;
pdc->regs = &pdc_v3_2;
- __pdc_enable_intr = pdc_enable_intr_cfg;
+ pdc->enable_intr = pdc_enable_intr_cfg;
} else if (pdc->version < PDC_VERSION_3_2 &&
pdc->version >= PDC_VERSION_3_0) {
pdc->cfg = &pdc_cfg_v3_0;
pdc->regs = &pdc_v3_0;
- __pdc_enable_intr = pdc_enable_intr_bank;
+ pdc->enable_intr = pdc_enable_intr_bank;
} else {
pdc->cfg = &pdc_cfg_v2_7;
pdc->regs = &pdc_v2_7;
- __pdc_enable_intr = pdc_enable_intr_bank;
+ pdc->enable_intr = pdc_enable_intr_bank;
}
/*
@@ -506,7 +516,7 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
goto fail;
}
- pdc_x1e_quirk = true;
+ pdc->x1e_quirk = true;
}
parent_domain = irq_find_host(parent);
@@ -522,6 +532,8 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
goto fail;
}
+ raw_spin_lock_init(&pdc->lock);
+
pdc_domain = irq_domain_create_hierarchy(parent_domain,
IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
PDC_MAX_IRQS,
@@ -538,7 +550,7 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
return 0;
fail:
- kfree(pdc_region);
+ kfree(pdc->region);
iounmap(pdc->base);
iounmap(pdc->prev_base);
kfree(pdc);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 3/8] irqchip/qcom-pdc: Remove pdc_enable_intr() wrapper
2026-05-26 10:54 [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
2026-05-26 10:54 ` [PATCH v2 1/8] irqchip/qcom-pdc: restructure version support Maulik Shah
2026-05-26 10:54 ` [PATCH v2 2/8] irqchip/qcom-pdc: Move all statics to struct pdc_desc Maulik Shah
@ 2026-05-26 10:54 ` Maulik Shah
2026-05-26 12:34 ` Stephan Gerhold
2026-05-26 10:54 ` [PATCH v2 4/8] irqchip/qcom-pdc: Differentiate between direct SPI and GPIO as SPI Maulik Shah
` (5 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Maulik Shah @ 2026-05-26 10:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad,
Maulik Shah
pdc->enable_intr() function already points to respective version
specific enable function. pdc_enable_intr() now only kept as wrapper.
Remove the wrapper and invoke pdc->enable_intr() from caller.
Locking in pdc_enable_intr() applies lock to all pdc->enable_intr()
however its only required for pdc_enable_intr_bank() which uses
a shared bank across all interrupts. pdc_enable_intr_cfg() do not
required locking as IRQ_CFG registers are one per interrupt. Move
locking accordingly.
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
drivers/irqchip/qcom-pdc.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 8f7802139e4e..db76737646e1 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -201,11 +201,14 @@ static void pdc_x1e_irq_enable_write(u32 bank, u32 enable)
static void pdc_enable_intr_bank(int pin_out, bool on)
{
unsigned long enable;
+ unsigned long flags;
u32 index, mask;
index = FIELD_GET(GENMASK(31, 5), pin_out);
mask = FIELD_GET(GENMASK(4, 0), pin_out);
+ raw_spin_lock_irqsave(&pdc->lock, flags);
+
enable = pdc_reg_read(pdc->regs->irq_en_reg, index);
__assign_bit(mask, &enable, on);
@@ -213,6 +216,8 @@ static void pdc_enable_intr_bank(int pin_out, bool on)
pdc_x1e_irq_enable_write(index, enable);
else
pdc_reg_write(pdc->regs->irq_en_reg, index, enable);
+
+ raw_spin_unlock_irqrestore(&pdc->lock, flags);
}
static void pdc_enable_intr_cfg(int pin_out, bool on)
@@ -227,24 +232,15 @@ static void pdc_enable_intr_cfg(int pin_out, bool on)
pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, enable);
}
-static void pdc_enable_intr(struct irq_data *d, bool on)
-{
- unsigned long flags;
-
- raw_spin_lock_irqsave(&pdc->lock, flags);
- pdc->enable_intr(d->hwirq, on);
- raw_spin_unlock_irqrestore(&pdc->lock, flags);
-}
-
static void qcom_pdc_gic_disable(struct irq_data *d)
{
- pdc_enable_intr(d, false);
+ pdc->enable_intr(d->hwirq, false);
irq_chip_disable_parent(d);
}
static void qcom_pdc_gic_enable(struct irq_data *d)
{
- pdc_enable_intr(d, true);
+ pdc->enable_intr(d->hwirq, true);
irq_chip_enable_parent(d);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 3/8] irqchip/qcom-pdc: Remove pdc_enable_intr() wrapper
2026-05-26 10:54 ` [PATCH v2 3/8] irqchip/qcom-pdc: Remove pdc_enable_intr() wrapper Maulik Shah
@ 2026-05-26 12:34 ` Stephan Gerhold
0 siblings, 0 replies; 12+ messages in thread
From: Stephan Gerhold @ 2026-05-26 12:34 UTC (permalink / raw)
To: Maulik Shah
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij, linux-arm-msm,
linux-kernel, devicetree, linux-gpio, Sneh Mankad
On Tue, May 26, 2026 at 04:24:39PM +0530, Maulik Shah wrote:
> pdc->enable_intr() function already points to respective version
> specific enable function. pdc_enable_intr() now only kept as wrapper.
> Remove the wrapper and invoke pdc->enable_intr() from caller.
>
> Locking in pdc_enable_intr() applies lock to all pdc->enable_intr()
> however its only required for pdc_enable_intr_bank() which uses
> a shared bank across all interrupts. pdc_enable_intr_cfg() do not
> required locking as IRQ_CFG registers are one per interrupt. Move
> locking accordingly.
Well, pdc_enable_intr_cfg() is still a read-modify-write. If two CPUs
read IRQ_i_CFG at the same time and modify different bits (e.g. enable
and type bits) then write back the modified register one of the
modifications will get lost. Can we be sure that this won't happen?
Perhaps we can since PDC has IRQCHIP_SET_TYPE_MASKED, but personally
I would keep the lock there to be sure, especially with the new GPIO
operations you add that also read-modify-write the same register..
Thanks,
Stephan
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/8] irqchip/qcom-pdc: Differentiate between direct SPI and GPIO as SPI
2026-05-26 10:54 [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
` (2 preceding siblings ...)
2026-05-26 10:54 ` [PATCH v2 3/8] irqchip/qcom-pdc: Remove pdc_enable_intr() wrapper Maulik Shah
@ 2026-05-26 10:54 ` Maulik Shah
2026-05-26 10:54 ` [PATCH v2 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode Maulik Shah
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Maulik Shah @ 2026-05-26 10:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad,
Maulik Shah
Before commit 4dc70713dc24 ("irqchip/qcom-pdc: Kill non-wakeup irqdomain")
there are separate domains for direct SPIs and GPIO used as SPIs. Separate
domains can be useful in case irqchip want to differentiate both of them.
Since commit unified both the domains there is no way to differentiate.
In preparation to add the second level interrupt controller support where
GPIO interrupts get lateched at PDC (but not direct SPIs) there is a need
to differentiate between SPIs and GPIOs as SPIs. Reverting above commit do
not seem a good option either which leads to waste of resources.
PDC HW have the IRQ_PARAM register telling number of direct SPIs and number
of GPIOs as SPIs. Further PDC allocates direct SPIs at the beginning and
all GPIOs as SPIs are allocated at the end. This information can be used in
driver to differentiate them.
Add the support to read this register and keep this information in
struct pdc_desc. Later change utilizes same.
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
drivers/irqchip/qcom-pdc.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index db76737646e1..86379dddc5be 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -61,6 +61,11 @@
* | | [4] GPIO_STATUS| [4] GPIO_MASK |
* | [31:3] Unused | [3] GPIO_MASK | [3] IRQ_ENABLE |
* | [0:2] Type | [0:2] Type | [0:2] Type |
+ * |---------------------------------------------------------------|
+ * | IRQ_PARAM | IRQ_PARAM | IRQ_PARAM |
+ * | | |
+ * | [15:8] NUM_GPIO | [15:8] NUM_GPIO | [15:8] NUM_GPIO |
+ * | [7:0] NUM_SPI | [7:0] NUM_SPI | [7:0] NUM_SPI |
* +---------------------------------------------------------------+
*/
@@ -69,10 +74,12 @@
*
* @irq_en_reg: IRQ_ENABLE_BANK register location
* @irq_cfg_reg: IRQ_CFG register location
+ * @irq_param_reg: IRQ_PARAM register location
*/
struct pdc_regs {
u32 irq_en_reg;
u32 irq_cfg_reg;
+ u32 irq_param_reg;
};
/**
@@ -92,6 +99,8 @@ struct pdc_cfg {
* @base: PDC base register for DRV2 / HLOS
* @prev_base: PDC DRV1 base, applicable only for x1e RTL bug.
* @version: PDC version
+ * @num_spis: Total number of direct SPI interrupts
+ * @num_gpios: Total number of GPIOs forwarded as SPI interrupts
* @region: PDC interrupt continuous range
* @region_cnt: Total PDC ranges
* @x1e_quirk: x1e H/W Bug handling
@@ -104,6 +113,8 @@ struct pdc_desc {
void __iomem *base;
void __iomem *prev_base;
u32 version;
+ u32 num_spis;
+ u32 num_gpios;
struct pdc_pin_region *region;
int region_cnt;
@@ -120,6 +131,7 @@ struct pdc_desc {
static const struct pdc_regs pdc_v3_2 = {
.irq_cfg_reg = 0x110,
+ .irq_param_reg = 0x100c,
};
static const struct pdc_cfg pdc_cfg_v3_2 = {
@@ -130,6 +142,7 @@ static const struct pdc_cfg pdc_cfg_v3_2 = {
static const struct pdc_regs pdc_v3_0 = {
.irq_en_reg = 0x10,
.irq_cfg_reg = 0x110,
+ .irq_param_reg = 0x100c,
};
static const struct pdc_cfg pdc_cfg_v3_0 = {
@@ -139,6 +152,7 @@ static const struct pdc_cfg pdc_cfg_v3_0 = {
static const struct pdc_regs pdc_v2_7 = {
.irq_en_reg = 0x10,
.irq_cfg_reg = 0x110,
+ .irq_param_reg = 0x100c,
};
static const struct pdc_cfg pdc_cfg_v2_7 = {
@@ -457,6 +471,7 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
struct device_node *node = pdev->dev.of_node;
resource_size_t res_size;
struct resource res;
+ u32 irq_param;
int ret;
/* compat with old sm8150 DT which had very small region for PDC */
@@ -515,6 +530,10 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
pdc->x1e_quirk = true;
}
+ irq_param = pdc_reg_read(pdc->regs->irq_param_reg, 0);
+ pdc->num_spis = FIELD_GET(GENMASK(7, 0), irq_param);
+ pdc->num_gpios = FIELD_GET(GENMASK(15, 8), irq_param);
+
parent_domain = irq_find_host(parent);
if (!parent_domain) {
pr_err("%pOF: unable to find PDC's parent domain\n", node);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode
2026-05-26 10:54 [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
` (3 preceding siblings ...)
2026-05-26 10:54 ` [PATCH v2 4/8] irqchip/qcom-pdc: Differentiate between direct SPI and GPIO as SPI Maulik Shah
@ 2026-05-26 10:54 ` Maulik Shah
2026-05-26 12:22 ` Stephan Gerhold
2026-05-26 10:54 ` [PATCH v2 6/8] pinctrl: qcom: Acknowledge IRQs for PDC interrupt controller Maulik Shah
` (3 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Maulik Shah @ 2026-05-26 10:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad,
Maulik Shah
All PDC irqchip supports pass through mode in which both Direct SPIs and
GPIO IRQs (as SPIs) are sent to GIC without latching at PDC.
Newer PDCs (v3.0 onwards) also support additional secondary controller mode
where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
still works same as pass through mode without latching at PDC even in
secondary controller mode.
All the SoCs so far default uses pass through mode with the exception of
x1e. x1e PDC may be set to secondary controller mode for builds on CRD
boards whereas it may be set to pass through mode for IoT-EVK boards.
The mode configuration is done in firmware and initially shipped windows
firmware did not have SCM interface to read or modify the PDC mode.
Later only write access is opened up for non secure world.
Using the write access available add changes to modify the PDC mode to
pass through mode via SCM write. When the write fails (on older firmware)
assume to work in secondary mode.
Co-developed-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
drivers/irqchip/qcom-pdc.c | 109 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 106 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 86379dddc5be..69ddd7d89a83 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -20,12 +20,18 @@
#include <linux/spinlock.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/firmware/qcom/qcom_scm.h>
#define PDC_MAX_IRQS 256
#define IRQ_ENABLE_BANK_MAX BITS_TO_BYTES(PDC_MAX_IRQS)
#define PDC_DRV_SIZE 0x10000
+/* Secure DRV register to configure the PDC mode via qcom_scm_io_writel() */
+#define PDC_GPIO_INT_CTL_ENABLE 0xb2045e8
+#define PDC_PASS_THROUGH_MODE 0x0
+#define PDC_SECONDARY_MODE 0x1
+
#define PDC_VERSION_REG 0x1000
#define PDC_VERSION_MAJOR GENMASK(23, 16)
#define PDC_VERSION_MINOR GENMASK(15, 8)
@@ -85,10 +91,14 @@ struct pdc_regs {
/**
* struct pdc_cfg: bit fields for PDC IRQ_CFG register
*
+ * @gpio_irq_sts: bit mask for GPIO_STATUS
+ * @gpio_irq_mask: bit mask for GPIO_MASK
* @irq_enable: bit mask for IRQ_ENABLE
* @irq_type: bit mask for IRQ_TYPE
*/
struct pdc_cfg {
+ u32 gpio_irq_sts;
+ u32 gpio_irq_mask;
u32 irq_enable;
u32 irq_type;
};
@@ -103,11 +113,14 @@ struct pdc_cfg {
* @num_gpios: Total number of GPIOs forwarded as SPI interrupts
* @region: PDC interrupt continuous range
* @region_cnt: Total PDC ranges
+ * @mode: PDC_PASS_THROUGH_MODE or PDC_SECONDARY_MODE
* @x1e_quirk: x1e H/W Bug handling
* @lock: lock for IRQ_ENABLE_BANK protection
* @regs: PDC regs (IRQ_ENABLE_BANK and IRQ_CFG)
* @cfg: bit masks within for IRQ_CFG reg
* @enable_intr: pointer to enable function based on PDC version
+ * @unmask_gpio: pointer to GPIO irq unmask function
+ * @clear_gpio: pointer to GPIO irq clear function
*/
struct pdc_desc {
void __iomem *base;
@@ -119,6 +132,7 @@ struct pdc_desc {
struct pdc_pin_region *region;
int region_cnt;
+ u8 mode;
bool x1e_quirk;
raw_spinlock_t lock;
@@ -127,6 +141,8 @@ struct pdc_desc {
const struct pdc_cfg *cfg;
void (*enable_intr)(int pin_out, bool on);
+ void (*unmask_gpio)(int pin_out, bool on);
+ void (*clear_gpio)(int pin_out);
};
static const struct pdc_regs pdc_v3_2 = {
@@ -135,6 +151,8 @@ static const struct pdc_regs pdc_v3_2 = {
};
static const struct pdc_cfg pdc_cfg_v3_2 = {
+ .gpio_irq_sts = GENMASK(5, 5),
+ .gpio_irq_mask = GENMASK(4, 4),
.irq_enable = GENMASK(3, 3),
.irq_type = GENMASK(2, 0),
};
@@ -146,6 +164,8 @@ static const struct pdc_regs pdc_v3_0 = {
};
static const struct pdc_cfg pdc_cfg_v3_0 = {
+ .gpio_irq_sts = GENMASK(4, 4),
+ .gpio_irq_mask = GENMASK(3, 3),
.irq_type = GENMASK(2, 0),
};
@@ -184,6 +204,14 @@ static u32 pdc_reg_read(int reg, u32 i)
return readl_relaxed(pdc->base + reg + i * sizeof(u32));
}
+static inline bool pdc_pin_uses_seconary_mode(int pin_out)
+{
+ if (pdc->mode == PDC_SECONDARY_MODE && pin_out >= pdc->num_spis)
+ return true;
+
+ return false;
+}
+
static void pdc_x1e_irq_enable_write(u32 bank, u32 enable)
{
void __iomem *base;
@@ -232,6 +260,36 @@ static void pdc_enable_intr_bank(int pin_out, bool on)
pdc_reg_write(pdc->regs->irq_en_reg, index, enable);
raw_spin_unlock_irqrestore(&pdc->lock, flags);
+
+ if (pdc_pin_uses_seconary_mode(pin_out))
+ pdc->unmask_gpio(pin_out, on);
+}
+
+static void pdc_clear_gpio_cfg(int pin_out)
+{
+ unsigned long gpio_sts;
+
+ if (pdc->version < PDC_VERSION_3_0)
+ return;
+
+ gpio_sts = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
+ gpio_sts &= ~pdc->cfg->gpio_irq_sts;
+ pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, gpio_sts);
+}
+
+static void pdc_unmask_gpio_cfg(int pin_out, bool unmask)
+{
+ unsigned long gpio_mask;
+
+ if (pdc->version < PDC_VERSION_3_0)
+ return;
+
+ gpio_mask = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
+ if (unmask)
+ gpio_mask &= ~pdc->cfg->gpio_irq_mask;
+ else
+ gpio_mask |= pdc->cfg->gpio_irq_mask;
+ pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, gpio_mask);
}
static void pdc_enable_intr_cfg(int pin_out, bool on)
@@ -244,6 +302,9 @@ static void pdc_enable_intr_cfg(int pin_out, bool on)
else
enable &= ~pdc->cfg->irq_enable;
pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, enable);
+
+ if (pdc_pin_uses_seconary_mode(pin_out))
+ pdc->unmask_gpio(pin_out, on);
}
static void qcom_pdc_gic_disable(struct irq_data *d)
@@ -258,6 +319,20 @@ static void qcom_pdc_gic_enable(struct irq_data *d)
irq_chip_enable_parent(d);
}
+static void qcom_pdc_ack(struct irq_data *d)
+{
+ if (pdc_pin_uses_seconary_mode(d->hwirq) && !irqd_is_level_type(d))
+ pdc->clear_gpio(d->hwirq);
+}
+
+static void qcom_pdc_gic_eoi(struct irq_data *d)
+{
+ if (pdc_pin_uses_seconary_mode(d->hwirq) && irqd_is_level_type(d))
+ pdc->clear_gpio(d->hwirq);
+
+ irq_chip_eoi_parent(d);
+}
+
/*
* 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
@@ -291,6 +366,8 @@ enum pdc_irq_config_bits {
* takes care of converting falling edge to rising edge signal
* If @type is level, then forward that as level high as PDC
* takes care of converting falling edge to rising edge signal
+ * If interrupt is GPIO and PDC is in secondary mode forward that
+ * as level high as PDC takes care of coverting all types to level high
*/
static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
{
@@ -326,6 +403,16 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
pdc_type |= (old_pdc_type & ~pdc->cfg->irq_type);
pdc_reg_write(pdc->regs->irq_cfg_reg, d->hwirq, pdc_type);
+ if (pdc_pin_uses_seconary_mode(d->hwirq)) {
+ /*
+ * PDC forwards GPIOs as level high to GIC in secondary
+ * mode. Update the type and clear any previously latched
+ * phantom interrupt at PDC.
+ */
+ type = IRQ_TYPE_LEVEL_HIGH;
+ pdc->clear_gpio(d->hwirq);
+ }
+
ret = irq_chip_set_type_parent(d, type);
if (ret)
return ret;
@@ -347,7 +434,8 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
static struct irq_chip qcom_pdc_gic_chip = {
.name = "PDC",
- .irq_eoi = irq_chip_eoi_parent,
+ .irq_ack = qcom_pdc_ack,
+ .irq_eoi = qcom_pdc_gic_eoi,
.irq_mask = irq_chip_mask_parent,
.irq_unmask = irq_chip_unmask_parent,
.irq_disable = qcom_pdc_gic_disable,
@@ -457,8 +545,10 @@ static int pdc_setup_pin_mapping(struct device_node *np)
if (ret)
return ret;
- for (i = 0; i < pdc->region[n].cnt; i++)
- pdc->enable_intr(i + pdc->region[n].pin_base, 0);
+ for (i = 0; i < pdc->region[n].cnt; i++) {
+ pdc->clear_gpio(i + pdc->region[n].pin_base);
+ pdc->enable_intr(i + pdc->region[n].pin_base, false);
+ }
}
return 0;
@@ -510,6 +600,10 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
pdc->enable_intr = pdc_enable_intr_bank;
}
+ pdc->unmask_gpio = pdc_unmask_gpio_cfg;
+ pdc->clear_gpio = pdc_clear_gpio_cfg;
+ pdc->mode = PDC_PASS_THROUGH_MODE;
+
/*
* PDC has multiple DRV regions, each one provides the same set of
* registers for a particular client in the system. Due to a hardware
@@ -528,6 +622,15 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
}
pdc->x1e_quirk = true;
+
+ if (!qcom_scm_is_available()) {
+ ret = -EPROBE_DEFER;
+ goto fail;
+ }
+
+ ret = qcom_scm_io_writel(PDC_GPIO_INT_CTL_ENABLE, PDC_PASS_THROUGH_MODE);
+ if (ret)
+ pdc->mode = PDC_SECONDARY_MODE;
}
irq_param = pdc_reg_read(pdc->regs->irq_param_reg, 0);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode
2026-05-26 10:54 ` [PATCH v2 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode Maulik Shah
@ 2026-05-26 12:22 ` Stephan Gerhold
0 siblings, 0 replies; 12+ messages in thread
From: Stephan Gerhold @ 2026-05-26 12:22 UTC (permalink / raw)
To: Maulik Shah
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij, linux-arm-msm,
linux-kernel, devicetree, linux-gpio, Sneh Mankad
On Tue, May 26, 2026 at 04:24:41PM +0530, Maulik Shah wrote:
> All PDC irqchip supports pass through mode in which both Direct SPIs and
> GPIO IRQs (as SPIs) are sent to GIC without latching at PDC.
>
> Newer PDCs (v3.0 onwards) also support additional secondary controller mode
> where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
> still works same as pass through mode without latching at PDC even in
> secondary controller mode.
>
> All the SoCs so far default uses pass through mode with the exception of
> x1e. x1e PDC may be set to secondary controller mode for builds on CRD
> boards whereas it may be set to pass through mode for IoT-EVK boards.
> The mode configuration is done in firmware and initially shipped windows
> firmware did not have SCM interface to read or modify the PDC mode.
> Later only write access is opened up for non secure world.
>
> Using the write access available add changes to modify the PDC mode to
> pass through mode via SCM write. When the write fails (on older firmware)
> assume to work in secondary mode.
>
> Co-developed-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
> drivers/irqchip/qcom-pdc.c | 109 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 106 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 86379dddc5be..69ddd7d89a83 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
> [...]
> @@ -135,6 +151,8 @@ static const struct pdc_regs pdc_v3_2 = {
> };
>
> static const struct pdc_cfg pdc_cfg_v3_2 = {
> + .gpio_irq_sts = GENMASK(5, 5),
> + .gpio_irq_mask = GENMASK(4, 4),
BIT(5) / BIT(4) would be clearer here in my opinion.
> .irq_enable = GENMASK(3, 3),
> .irq_type = GENMASK(2, 0),
> };
> [...]
> @@ -184,6 +204,14 @@ static u32 pdc_reg_read(int reg, u32 i)
> return readl_relaxed(pdc->base + reg + i * sizeof(u32));
> }
>
> +static inline bool pdc_pin_uses_seconary_mode(int pin_out)
> +{
> + if (pdc->mode == PDC_SECONDARY_MODE && pin_out >= pdc->num_spis)
> + return true;
> +
> + return false;
Can put this in one line:
return pdc->mode == PDC_SECONDARY_MODE && pin_out >= pdc->num_spis;
> +}
> +
> static void pdc_x1e_irq_enable_write(u32 bank, u32 enable)
> {
> void __iomem *base;
> @@ -232,6 +260,36 @@ static void pdc_enable_intr_bank(int pin_out, bool on)
> pdc_reg_write(pdc->regs->irq_en_reg, index, enable);
>
> raw_spin_unlock_irqrestore(&pdc->lock, flags);
> +
> + if (pdc_pin_uses_seconary_mode(pin_out))
> + pdc->unmask_gpio(pin_out, on);
> +}
> +
> +static void pdc_clear_gpio_cfg(int pin_out)
> +{
> + unsigned long gpio_sts;
> +
> + if (pdc->version < PDC_VERSION_3_0)
> + return;
> +
> + gpio_sts = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
> + gpio_sts &= ~pdc->cfg->gpio_irq_sts;
> + pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, gpio_sts);
Is this guaranteed to be called sequentially, i.e. not in parallel on
another CPU? Otherwise, you need to add the lock here to make sure the
read-modify-write doesn't race with another CPU.
Note that since the irq_cfg_reg is also used in qcom_pdc_gic_set_type()
it would be safest to add the lock there as well (although since PDC has
IRQCHIP_SET_TYPE_MASKED it's probably unlikely to be called in parallel
with another irqchip operation for the same IRQ). In my patch, I handled
this for all users using a new pdc_update_irq_cfg() function [1].
[1]: https://github.com/stephan-gh/linux/commit/59ca2a7335ede83e4a7cf02704dd7c469c725c14
> +}
> +
> +static void pdc_unmask_gpio_cfg(int pin_out, bool unmask)
> +{
> + unsigned long gpio_mask;
> +
> + if (pdc->version < PDC_VERSION_3_0)
> + return;
> +
> + gpio_mask = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
> + if (unmask)
> + gpio_mask &= ~pdc->cfg->gpio_irq_mask;
> + else
> + gpio_mask |= pdc->cfg->gpio_irq_mask;
> + pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, gpio_mask);
> }
>
> static void pdc_enable_intr_cfg(int pin_out, bool on)
> [...]
> @@ -258,6 +319,20 @@ static void qcom_pdc_gic_enable(struct irq_data *d)
> irq_chip_enable_parent(d);
> }
>
> +static void qcom_pdc_ack(struct irq_data *d)
> +{
> + if (pdc_pin_uses_seconary_mode(d->hwirq) && !irqd_is_level_type(d))
> + pdc->clear_gpio(d->hwirq);
> +}
You might need a write memory barrier here and/or read-back here to make
sure the write is complete before the interrupt is unmasked in the GIC.
IIRC I added this in my patch after seeing some test tlmm-test failure..
> +
> +static void qcom_pdc_gic_eoi(struct irq_data *d)
> +{
> + if (pdc_pin_uses_seconary_mode(d->hwirq) && irqd_is_level_type(d))
> + pdc->clear_gpio(d->hwirq);
> +
> + irq_chip_eoi_parent(d);
> +}
> +
> /*
> * 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
> [...]
> @@ -510,6 +600,10 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
> pdc->enable_intr = pdc_enable_intr_bank;
> }
>
> + pdc->unmask_gpio = pdc_unmask_gpio_cfg;
> + pdc->clear_gpio = pdc_clear_gpio_cfg;
What is the purpose of these function pointers if you always assign the
same function?
Thanks,
Stephan
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 6/8] pinctrl: qcom: Acknowledge IRQs for PDC interrupt controller
2026-05-26 10:54 [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
` (4 preceding siblings ...)
2026-05-26 10:54 ` [PATCH v2 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode Maulik Shah
@ 2026-05-26 10:54 ` Maulik Shah
2026-05-26 10:54 ` [PATCH v2 7/8] Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now" Maulik Shah
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Maulik Shah @ 2026-05-26 10:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad,
Maulik Shah, Stephan Gerhold
From: Stephan Gerhold <stephan.gerhold@linaro.org>
PDC needs to acknowledge incoming GPIO interrupts to clear the latched
interrupt status in secondary mode of PDC. For level-triggered IRQs this
happens automatically in irq_eoi() but for edge-triggered IRQs this needs
to happen as early as possible in the IRQ handler.
Implement this by using handle_fasteoi_ack_irq() as IRQ handler in this
situation and forward the irq_ack() callback to the parent IRQ chip.
Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
drivers/pinctrl/qcom/pinctrl-msm.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 45b3a2763eb8..c2938494c6bb 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -995,6 +995,16 @@ static void msm_gpio_irq_ack(struct irq_data *d)
if (test_bit(d->hwirq, pctrl->skip_wake_irqs)) {
if (test_bit(d->hwirq, pctrl->dual_edge_irqs))
msm_gpio_update_dual_edge_parent(d);
+
+ /*
+ * During early initialization of the IRQ hierarchy,
+ * irq_ack() is called by __irq_set_handler() before
+ * the parent IRQ chip has been set up. This is why
+ * we additionally need to check for d->parent_data->chip.
+ */
+
+ if (d->parent_data->chip)
+ irq_chip_ack_parent(d);
return;
}
@@ -1069,7 +1079,10 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type)
if (test_bit(d->hwirq, pctrl->skip_wake_irqs)) {
clear_bit(d->hwirq, pctrl->dual_edge_irqs);
- irq_set_handler_locked(d, handle_fasteoi_irq);
+ if (irqd_is_level_type(d))
+ irq_set_handler_locked(d, handle_fasteoi_irq);
+ else
+ irq_set_handler_locked(d, handle_fasteoi_ack_irq);
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 7/8] Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now"
2026-05-26 10:54 [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
` (5 preceding siblings ...)
2026-05-26 10:54 ` [PATCH v2 6/8] pinctrl: qcom: Acknowledge IRQs for PDC interrupt controller Maulik Shah
@ 2026-05-26 10:54 ` Maulik Shah
2026-05-26 10:54 ` [PATCH v2 8/8] arm64: dts: qcom: x1e80100: Add deepest idle state Maulik Shah
2026-05-26 11:59 ` [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and " Stephan Gerhold
8 siblings, 0 replies; 12+ messages in thread
From: Maulik Shah @ 2026-05-26 10:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad,
Maulik Shah
This reverts commit 602cb14e310a ("pinctrl: qcom: x1e80100: Bypass PDC
wakeup parent for now").
PDC interrupts no more break GPIOs PDC irqchip is updated to work for
pass through or secondary mode. Update nwakeirq_map to reflect the GPIO
to PDC irq map size.
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
drivers/pinctrl/qcom/pinctrl-x1e80100.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-x1e80100.c b/drivers/pinctrl/qcom/pinctrl-x1e80100.c
index 8d2b8246170b..e4c0abcd95b9 100644
--- a/drivers/pinctrl/qcom/pinctrl-x1e80100.c
+++ b/drivers/pinctrl/qcom/pinctrl-x1e80100.c
@@ -1836,9 +1836,7 @@ static const struct msm_pinctrl_soc_data x1e80100_pinctrl = {
.ngroups = ARRAY_SIZE(x1e80100_groups),
.ngpios = 239,
.wakeirq_map = x1e80100_pdc_map,
- /* TODO: Enabling PDC currently breaks GPIO interrupts */
- .nwakeirq_map = 0,
- /* .nwakeirq_map = ARRAY_SIZE(x1e80100_pdc_map), */
+ .nwakeirq_map = ARRAY_SIZE(x1e80100_pdc_map),
.egpio_func = 9,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 8/8] arm64: dts: qcom: x1e80100: Add deepest idle state
2026-05-26 10:54 [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
` (6 preceding siblings ...)
2026-05-26 10:54 ` [PATCH v2 7/8] Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now" Maulik Shah
@ 2026-05-26 10:54 ` Maulik Shah
2026-05-26 11:59 ` [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and " Stephan Gerhold
8 siblings, 0 replies; 12+ messages in thread
From: Maulik Shah @ 2026-05-26 10:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad,
Maulik Shah
Add deepest idle state as GPIO IRQs can work as wakeup capable interrupts
in deepest idle state.
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/hamoa.dtsi | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
index b5516655db8c..5a1b041ea768 100644
--- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
@@ -302,6 +302,14 @@ cluster_cl5: cluster-sleep-1 {
exit-latency-us = <4000>;
min-residency-us = <7000>;
};
+
+ domain_ss3: domain-sleep-0 {
+ compatible = "domain-idle-state";
+ arm,psci-suspend-param = <0x0200c354>;
+ entry-latency-us = <2800>;
+ exit-latency-us = <4400>;
+ min-residency-us = <9000>;
+ };
};
};
@@ -460,7 +468,7 @@ cluster_pd2: power-domain-cpu-cluster2 {
system_pd: power-domain-system {
#power-domain-cells = <0>;
- /* TODO: system-wide idle states */
+ domain-idle-states = <&domain_ss3>;
};
};
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state
2026-05-26 10:54 [PATCH v2 0/8] x1e80100: Enable PDC wake GPIOs and deepest idle state Maulik Shah
` (7 preceding siblings ...)
2026-05-26 10:54 ` [PATCH v2 8/8] arm64: dts: qcom: x1e80100: Add deepest idle state Maulik Shah
@ 2026-05-26 11:59 ` Stephan Gerhold
8 siblings, 0 replies; 12+ messages in thread
From: Stephan Gerhold @ 2026-05-26 11:59 UTC (permalink / raw)
To: Maulik Shah
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Thomas Gleixner, Linus Walleij, linux-arm-msm,
linux-kernel, devicetree, linux-gpio, Sneh Mankad
On Tue, May 26, 2026 at 04:24:36PM +0530, Maulik Shah wrote:
> There are two modes PDC irqchip can work in
> - pass through mode
> - secondary controller mode
>
> Secondary mode is supported depending on SoC using PDC HW Version v3.0
> or higher.
>
> +------------------------------------------------------------------------+
> | SoC | SM8350, SM8450 | SM8550, Hamoa | SM8650, SM8750 |
> |----------------------------------------------------------- ------------|
> | Version | v2.7 | v3.0 | v3.2 |
> |------------------------------------------------------------------------|
> | Pass through | Yes | Yes | Yes |
> |------------------------------------------------------------------------|
> | Secondary | No | Yes | Yes |
> +------------------------------------------------------------------------+
>
> All PDC irqchip supports pass through mode in which both Direct SPIs and
> GPIO IRQs (as SPIs) are sent to GIC without latching at PDC, PDC only does
> inversion when needed for falling edge to rising edge or level low to level
> high, as the GIC do not support falling edge/level low interrupts.
>
> Newer PDCs (v3.0 onwards) also support additional secondary controller mode
> where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
> still works same as pass through mode without latching at PDC even in
> secondary controller mode.
>
> All the SoCs defaulted to pass through mode with the exception of some x1e.
>
> x1e PDC may be set to secondary controller mode for builds on CRD boards
> whereas it may be set to pass through mode for IoT-EVK boards. The mode
> configuration is done in firmware and initially shipped windows firmware
> did not have SCM interface to read or modify the PDC configuration.
> Later only write access is opened up for non secure world.
>
> Using the write access available add changes to modify the PDC mode to
> pass through mode via SCM write. When the write fails (on older firmware)
> assume to work in secondary mode.
>
> As the deepest idle state as the PDC can now wake up SoC from GPIOs and
> revert commit 602cb14e310a ("pinctrl: qcom: x1e80100: Bypass PDC wakeup
> parent for now").
>
> The series has been tested on x1e80100 CRD with both old and new firmware
> and also on kaanapali.
>
Tested how?
I recommend testing with the tlmm-test module Bjorn added, in all
supported configurations, to make sure you don't introduce regressions
for one of them. It would be also good to provide the test results here
in the cover letter.
Thanks,
Stephan
^ permalink raw reply [flat|nested] 12+ messages in thread