[PATCH v4 0/7] x1e80100: Enable PDC wake GPIOs and deepest idle state

[PATCH v4 0/7] x1e80100: Enable PDC wake GPIOs and deepest idle state

[Maulik Shah]

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. Test conducted with tlmm-test module after
applying [3] as test module needed to be fixed first.

All 17/17 passes in pass through mode and 16/17 passes in secondary mode.
Failing test tlmm_test_rising_while_disabled seems to be because when in
irq disabled state PDC is not latching the edge interrupt.

Test #1: Pass through mode on x1e80100 CRD (New firmware)

root@qcom-armv8a:~# insmod tlmm-test.ko gpio=91

KTAP version 1
1..1
    KTAP version 1
    # Subtest: tlmm-test
    # module: tlmm_test
    1..17
    ok 1 tlmm_test_silent_rising
    ok 2 tlmm_test_silent_falling
    ok 3 tlmm_test_silent_low
    ok 4 tlmm_test_silent_high
    ok 5 tlmm_test_rising
    ok 6 tlmm_test_falling
    ok 7 tlmm_test_high
    ok 8 tlmm_test_low
    ok 9 tlmm_test_rising_in_handler
    ok 10 tlmm_test_falling_in_handler
    ok 11 tlmm_test_thread_rising
    ok 12 tlmm_test_thread_falling
    ok 13 tlmm_test_thread_high
    ok 14 tlmm_test_thread_low
    ok 15 tlmm_test_thread_rising_in_handler
    ok 16 tlmm_test_thread_falling_in_handler
    ok 17 tlmm_test_rising_while_disabled

ok 1 tlmm-test

Test #2: Secondary mode on x1e80100 CRD (Old firmware)

root@qcom-armv8a:~# insmod tlmm-test.ko gpio=91

KTAP version 1
1..1
    KTAP version 1
    # Subtest: tlmm-test
    # module: tlmm_test
    1..17
    ok 1 tlmm_test_silent_rising
    ok 2 tlmm_test_silent_falling
    ok 3 tlmm_test_silent_low
    ok 4 tlmm_test_silent_high
    ok 5 tlmm_test_rising
    ok 6 tlmm_test_falling
    ok 7 tlmm_test_high
    ok 8 tlmm_test_low
    ok 9 tlmm_test_rising_in_handler
    ok 10 tlmm_test_falling_in_handler
    ok 11 tlmm_test_thread_rising
    ok 12 tlmm_test_thread_falling
    ok 13 tlmm_test_thread_high
    ok 14 tlmm_test_thread_low
    ok 15 tlmm_test_thread_rising_in_handler
    ok 16 tlmm_test_thread_falling_in_handler
    # tlmm_test_rising_while_disabled: ASSERTION FAILED at drivers/pinctrl/qcom/tlmm-test.c:545
    Expected atomic_read(&priv->intr_count) == 1, but
        atomic_read(&priv->intr_count) == 0 (0x0)
    not ok 17 tlmm_test_rising_while_disabled

not ok 1 tlmm-test
root@qcom-armv8a:~#

v2 series is dependent on [1] as mostly all changes are already reviewed.
v3 series is dependent on [2] which is already merged in linux-next

[1] https://lore.kernel.org/linux-arm-msm/20260410184124.1068210-1-mukesh.ojha@oss.qualcomm.com/
[2] https://lore.kernel.org/linux-arm-msm/20260527095426.2324504-1-mukesh.ojha@oss.qualcomm.com/
[3] https://lore.kernel.org/linux-arm-msm/20260529-tlmm_test_changes-v1-0-88bfdccb4369@oss.qualcomm.com/

---
Changes in v4:
- pdc_setup_pin_mapping() use dev argument and use dev->of_node within
- Merge v3 patch 2 and 3 into single change
- guard pdc->lock for all pdc->enable_intr() function call for HW v2.7,3.0
- Move lock init before pdc_setup_pin_mapping() for bad spinlock on v3
- Removed unused num_gpios
- Add comment in qcom_pdc_gic_secondary_set_type() from v2
- Modify primary case in qcom_pdc_alloc()
- Remove pdc->version checks from pdc_unmask/clear_gpio_cfg()
- Formatting and commit text updates
- Link to v3: https://patch.msgid.link/20260616-hamoa_pdc_v3-v3-0-4d8e1504ea75@oss.qualcomm.com

Changes in v3:
- Add test results in cover letter with tlmm-test module
- Fix coding style and struct defination
- Convert raw_spin_lock to guard(raw_spin_lock) and remove _irqsave
- Use bit number instead of GENMASK() for single bit fields
- Use __assign_bit() and __clear_bit() APIs for single bit modifications
- Use devm_ioremap() instead of ioremap()
- Use devm_kcalloc() instead of kzalloc_objs()
- Add separate irq chips for pass through and secondary mode IRQs
- Add IRQCHIP_EOI_THREADED flag for threaded IRQ on pinctrl irqchip
- Link to v2: https://patch.msgid.link/20260526-hamoa_pdc-v2-0-f6857af1ce91@oss.qualcomm.com

Changes in v2:
- Update to mention SoC names along with PDC versions in cover letter
- Drop devicetree change to remove scm interconnects
- Use qcom_scm_is_available() to wait for dependency on SCM
- Drop binding change mentioning qcom,qmp and PDC config reg
- Restructure version support and move all statics to struct pdc_desc
- Remove pdc_enable_intr() wrapper
- Differentiate direct SPI and GPIOs as SPI using PDC IRQ PARAM reg
- Add changes to make PDC work in secondary controller mode
- Rework and include Stephan's change to invoke irq_ack() for edge interrupt
- Mention dependency via b4 prerequisites and cover letter
- Link to v1: https://lore.kernel.org/r/20260312-hamoa_pdc-v1-0-760c8593ce50@oss.qualcomm.com
---

To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Thomas Gleixner <tglx@kernel.org>
To: Linus Walleij <linusw@kernel.org>
To: Bartosz Golaszewski <brgl@kernel.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-gpio@vger.kernel.org
Cc: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>

---
Maulik Shah (6):
      irqchip/qcom-pdc: restructure version support
      irqchip/qcom-pdc: Move all statics to struct pdc_desc
      irqchip/qcom-pdc: Differentiate between direct SPI and GPIO as SPI
      irqchip/qcom-pdc: Configure PDC to pass through mode
      Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now"
      arm64: dts: qcom: x1e80100: Add deepest idle state

Stephan Gerhold (1):
      pinctrl: qcom: Acknowledge IRQs for PDC interrupt controller

 arch/arm64/boot/dts/qcom/hamoa.dtsi     |  14 +-
 drivers/irqchip/qcom-pdc.c              | 535 +++++++++++++++++++++++++-------
 drivers/pinctrl/qcom/pinctrl-msm.c      |  15 +-
 drivers/pinctrl/qcom/pinctrl-x1e80100.c |   4 +-
 4 files changed, 450 insertions(+), 118 deletions(-)
---
base-commit: 8e9685d3c41c35dd1b37df70d854137abcb2fbac
change-id: 20260605-hamoa_pdc_v3-6dabc845021c

Best regards,
--  
Maulik Shah <maulik.shah@oss.qualcomm.com>

[PATCH v4 1/7] irqchip/qcom-pdc: restructure version support

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

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 drivers/irqchip/qcom-pdc.c | 219 +++++++++++++++++++++++++++++++--------------
 1 file changed, 152 insertions(+), 67 deletions(-)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 2014dbb0bc43..a77d1d334062 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -21,21 +21,12 @@
 #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 PDC_MAX_IRQS			256
+#define IRQ_ENABLE_BANK_MAX		BITS_TO_BYTES(PDC_MAX_IRQS)
 #define IRQ_ENABLE_BANK_INDEX_MASK	GENMASK(31, 5)
 #define IRQ_ENABLE_BANK_BIT_MASK	GENMASK(4, 0)
-#define IRQ_i_CFG		0x110
-
-/* 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)
 #define PDC_VERSION_MINOR		GENMASK(15, 8)
@@ -46,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_irq_cfg: bit fields for PDC IRQ_CFG register
+ *
+ * @irq_enable:     bit number for IRQ_ENABLE field
+ * @irq_type:       GENMASK for IRQ_TYPE field
+ */
+struct pdc_irq_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_fields:     Fields of IRQ_CFG reg
+ */
+struct pdc_desc {
+	void __iomem			*base;
+	void __iomem			*prev_base;
+	u32				version;
+	const struct pdc_regs		*regs;
+	const struct pdc_irq_cfg	*cfg_fields;
+};
+
+static const struct pdc_regs pdc_v3_2 = {
+	.irq_cfg_reg	= 0x110,
+};
+
+static const struct pdc_irq_cfg pdc_cfg_v3_2 = {
+	.irq_enable	= 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_irq_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_irq_cfg pdc_cfg_v2_7 = {
+	.irq_type	= GENMASK(2, 0),
+};
 
 struct pdc_pin_region {
 	u32 pin_base;
@@ -56,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 unsigned int pdc_version;
 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)
 {
@@ -70,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)
@@ -86,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)
@@ -114,21 +196,21 @@ static void pdc_enable_intr_bank(int pin_out, bool on)
 	index = FIELD_GET(IRQ_ENABLE_BANK_INDEX_MASK, pin_out);
 	mask = FIELD_GET(IRQ_ENABLE_BANK_BIT_MASK, 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 = pdc_reg_read(IRQ_i_CFG, pin_out);
+	unsigned long enable = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
 
-	__assign_bit(IRQ_i_CFG_IRQ_ENABLE, &enable, on);
-	pdc_reg_write(IRQ_i_CFG, pin_out, enable);
+	__assign_bit(pdc->cfg_fields->irq_enable, &enable, on);
+	pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, enable);
 }
 
 static void __pdc_enable_intr(int pin_out, bool on)
@@ -224,9 +306,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_fields->irq_type);
+	pdc_reg_write(pdc->regs->irq_cfg_reg, d->hwirq, pdc_type);
 
 	ret = irq_chip_set_type_parent(d, type);
 	if (ret)
@@ -317,8 +399,7 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
 	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);
+	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &parent_fwspec);
 }
 
 static const struct irq_domain_ops qcom_pdc_ops = {
@@ -327,8 +408,9 @@ static const struct irq_domain_ops qcom_pdc_ops = {
 	.free		= irq_domain_free_irqs_common,
 };
 
-static int pdc_setup_pin_mapping(struct device_node *np)
+static int pdc_setup_pin_mapping(struct device *dev)
 {
+	struct device_node *np = dev->of_node;
 	int ret, n, i;
 
 	n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
@@ -336,25 +418,22 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 		return -EINVAL;
 
 	pdc_region_cnt = n / 3;
-	pdc_region = kzalloc_objs(*pdc_region, pdc_region_cnt);
+	pdc_region = devm_kcalloc(dev, pdc_region_cnt, sizeof(*pdc_region), GFP_KERNEL);
 	if (!pdc_region) {
 		pdc_region_cnt = 0;
 		return -ENOMEM;
 	}
 
 	for (n = 0; n < pdc_region_cnt; n++) {
-		ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
-						 n * 3 + 0,
+		ret = of_property_read_u32_index(np, "qcom,pdc-ranges", n * 3 + 0,
 						 &pdc_region[n].pin_base);
 		if (ret)
 			return ret;
-		ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
-						 n * 3 + 1,
+		ret = of_property_read_u32_index(np, "qcom,pdc-ranges", n * 3 + 1,
 						 &pdc_region[n].parent_base);
 		if (ret)
 			return ret;
-		ret = of_property_read_u32_index(np, "qcom,pdc-ranges",
-						 n * 3 + 2,
+		ret = of_property_read_u32_index(np, "qcom,pdc-ranges", n * 3 + 2,
 						 &pdc_region[n].cnt);
 		if (ret)
 			return ret;
@@ -366,11 +445,11 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 	return 0;
 }
 
-
 static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *parent)
 {
 	struct irq_domain *parent_domain, *pdc_domain;
 	struct device_node *node = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
 	resource_size_t res_size;
 	struct resource res;
 	int ret;
@@ -383,6 +462,30 @@ 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 = devm_kzalloc(dev, sizeof(*pdc), GFP_KERNEL);
+	if (!pdc)
+		return -ENOMEM;
+
+	pdc->base = devm_ioremap(dev, res.start, res_size);
+	if (!pdc->base) {
+		pr_err("%pOF: unable to map PDC registers\n", node);
+		return -ENXIO;
+	}
+
+	pdc->version = pdc_reg_read(PDC_VERSION_REG, 0);
+
+	if (pdc->version >= PDC_VERSION_3_2) {
+		pdc->cfg_fields = &pdc_cfg_v3_2;
+		pdc->regs = &pdc_v3_2;
+	} else if (pdc->version < PDC_VERSION_3_2 &&
+		   pdc->version >= PDC_VERSION_3_0) {
+		pdc->cfg_fields = &pdc_cfg_v3_0;
+		pdc->regs = &pdc_v3_0;
+	} else {
+		pdc->cfg_fields = &pdc_cfg_v2_7;
+		pdc->regs = &pdc_v2_7;
+	}
+
 	/*
 	 * PDC has multiple DRV regions, each one provides the same set of
 	 * registers for a particular client in the system. Due to a hardware
@@ -392,8 +495,9 @@ 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 = devm_ioremap(dev, 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;
 		}
@@ -401,48 +505,29 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 		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_version = pdc_reg_read(PDC_VERSION_REG, 0);
-
 	parent_domain = irq_find_host(parent);
 	if (!parent_domain) {
 		pr_err("%pOF: unable to find PDC's parent domain\n", node);
-		ret = -ENXIO;
-		goto fail;
+		return -ENXIO;
 	}
 
-	ret = pdc_setup_pin_mapping(node);
+	ret = pdc_setup_pin_mapping(dev);
 	if (ret) {
 		pr_err("%pOF: failed to init PDC pin-hwirq mapping\n", node);
-		goto fail;
+		return ret;
 	}
 
-	pdc_domain = irq_domain_create_hierarchy(parent_domain,
-					IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
-					PDC_MAX_GPIO_IRQS,
-					of_fwnode_handle(node),
-					&qcom_pdc_ops, NULL);
+	pdc_domain = irq_domain_create_hierarchy(parent_domain, IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP,
+						 PDC_MAX_IRQS, of_fwnode_handle(node),
+						 &qcom_pdc_ops, NULL);
 	if (!pdc_domain) {
 		pr_err("%pOF: PDC domain add failed\n", node);
-		ret = -ENOMEM;
-		goto fail;
+		return -ENOMEM;
 	}
 
 	irq_domain_update_bus_token(pdc_domain, DOMAIN_BUS_WAKEUP);
 
 	return 0;
-
-fail:
-	kfree(pdc_region);
-	iounmap(pdc_base);
-	iounmap(pdc_prev_base);
-	return ret;
 }
 
 IRQCHIP_PLATFORM_DRIVER_BEGIN(qcom_pdc)

-- 
2.43.0

[PATCH v4 2/7] irqchip/qcom-pdc: Move all statics to struct pdc_desc

[Maulik Shah]

There are multiple statics used. Move all to struct pdc_desc to better
align with versioning support. Document them.

Add pdc->enable_intr() function to point to respective version specific
enable function. Remove pdc_enable_intr() and __pdc_enable_intr() and
invoke pdc->enable_intr() from caller.

Locking in pdc_enable_intr() applies lock to all version specific
pdc->enable_intr() however lock is needed only for pdc_enable_intr_bank()
which uses a shared bank across on PDC v2.7 and PDC v3.0.

pdc_enable_intr_cfg() do not require locking as IRQ_CFG registers are one
per interrupt. Move the locking to only pdc_enable_intr_bank().

No functional impact.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 drivers/irqchip/qcom-pdc.c | 84 ++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 43 deletions(-)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index a77d1d334062..764f7965cfb8 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -92,15 +92,30 @@ struct pdc_irq_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_fields:     Fields of 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_irq_cfg	*cfg_fields;
+
+	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 unsigned int pdc_version;
-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)
@@ -196,10 +206,12 @@ static void pdc_enable_intr_bank(int pin_out, bool on)
 	index = FIELD_GET(IRQ_ENABLE_BANK_INDEX_MASK, pin_out);
 	mask = FIELD_GET(IRQ_ENABLE_BANK_BIT_MASK, pin_out);
 
+	guard(raw_spinlock_irqsave)(&pdc->lock);
+
 	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);
@@ -213,32 +225,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(int pin_out, bool on)
-{
-	if (pdc_version < PDC_VERSION_3_2)
-		pdc_enable_intr_bank(pin_out, on);
-	else
-		pdc_enable_intr_cfg(pin_out, on);
-}
-
-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);
 }
 
@@ -350,12 +345,10 @@ static struct irq_chip qcom_pdc_gic_chip = {
 
 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 (int 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;
@@ -411,35 +404,35 @@ static const struct irq_domain_ops qcom_pdc_ops = {
 static int pdc_setup_pin_mapping(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
-	int ret, n, i;
+	int ret, n;
 
 	n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
 	if (n <= 0 || n % 3)
 		return -EINVAL;
 
-	pdc_region_cnt = n / 3;
-	pdc_region = devm_kcalloc(dev, pdc_region_cnt, sizeof(*pdc_region), GFP_KERNEL);
-	if (!pdc_region) {
-		pdc_region_cnt = 0;
+	pdc->region_cnt = n / 3;
+	pdc->region = devm_kcalloc(dev, pdc->region_cnt, sizeof(*pdc->region), GFP_KERNEL);
+	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 (int i = 0; i < pdc->region[n].cnt; i++)
+			pdc->enable_intr(i + pdc->region[n].pin_base, 0);
 	}
 
 	return 0;
@@ -477,13 +470,16 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 	if (pdc->version >= PDC_VERSION_3_2) {
 		pdc->cfg_fields = &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_fields = &pdc_cfg_v3_0;
 		pdc->regs = &pdc_v3_0;
+		pdc->enable_intr = pdc_enable_intr_bank;
 	} else {
 		pdc->cfg_fields = &pdc_cfg_v2_7;
 		pdc->regs = &pdc_v2_7;
+		pdc->enable_intr = pdc_enable_intr_bank;
 	}
 
 	/*
@@ -502,7 +498,7 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 			return -ENXIO;
 		}
 
-		pdc_x1e_quirk = true;
+		pdc->x1e_quirk = true;
 	}
 
 	parent_domain = irq_find_host(parent);
@@ -511,6 +507,8 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 		return -ENXIO;
 	}
 
+	raw_spin_lock_init(&pdc->lock);
+
 	ret = pdc_setup_pin_mapping(dev);
 	if (ret) {
 		pr_err("%pOF: failed to init PDC pin-hwirq mapping\n", node);

-- 
2.43.0

[PATCH v4 3/7] irqchip/qcom-pdc: Differentiate between direct SPI and GPIO as SPI

[Maulik Shah]

Before commit 4dc70713dc24 ("irqchip/qcom-pdc: Kill non-wakeup irqdomain")
there were separate domains for direct SPIs and GPIOs used as SPIs.
Separate domains can be useful in case the irqchip wants to differentiate
both of them. Since the 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 latched at PDC (but not direct SPIs) there is a need to
differentiate between SPIs and GPIOs as SPIs. Reverting above commit does
not seem a good option 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 | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 764f7965cfb8..53a477aa9765 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,7 @@ struct pdc_irq_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
  * @region:         PDC interrupt continuous range
  * @region_cnt:     Total PDC ranges
  * @x1e_quirk:      x1e H/W Bug handling
@@ -104,6 +112,7 @@ struct pdc_desc {
 	void __iomem			*base;
 	void __iomem			*prev_base;
 	u32				version;
+	u32				num_spis;
 
 	struct pdc_pin_region		*region;
 	int				region_cnt;
@@ -120,6 +129,7 @@ struct pdc_desc {
 
 static const struct pdc_regs pdc_v3_2 = {
 	.irq_cfg_reg	= 0x110,
+	.irq_param_reg	= 0x100c,
 };
 
 static const struct pdc_irq_cfg pdc_cfg_v3_2 = {
@@ -130,6 +140,7 @@ static const struct pdc_irq_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_irq_cfg pdc_cfg_v3_0 = {
@@ -139,6 +150,7 @@ static const struct pdc_irq_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_irq_cfg pdc_cfg_v2_7 = {
@@ -445,6 +457,7 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 	struct device *dev = &pdev->dev;
 	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 */
@@ -501,6 +514,9 @@ 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);
+
 	parent_domain = irq_find_host(parent);
 	if (!parent_domain) {
 		pr_err("%pOF: unable to find PDC's parent domain\n", node);

-- 
2.43.0

[PATCH v4 4/7] irqchip/qcom-pdc: Configure PDC to pass through mode

[Maulik Shah]

All PDC HW versions 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 the GPIO interrupts and sends them to GIC as level
type interrupts. Direct SPIs works the same as pass-through mode without
latching at PDC even in secondary controller mode.

All the SoCs use 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 the 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.

In secondary mode set the separate irqchip for the GPIOs to perform
additional operations only for the GPIO irqs.

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 | 240 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 227 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 53a477aa9765..d122e8e7dc9d 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 IRQ_ENABLE_BANK_INDEX_MASK	GENMASK(31, 5)
 #define IRQ_ENABLE_BANK_BIT_MASK	GENMASK(4, 0)
 
+/* 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_DRV_SIZE			0x10000
 #define PDC_VERSION_REG			0x1000
 #define PDC_VERSION_MAJOR		GENMASK(23, 16)
@@ -85,10 +91,14 @@ struct pdc_regs {
 /**
  * struct pdc_irq_cfg: bit fields for PDC IRQ_CFG register
  *
+ * @gpio_irq_sts:   bit number for GPIO_STATUS field
+ * @gpio_irq_mask:  bit number for GPIO_MASK field
  * @irq_enable:     bit number for IRQ_ENABLE field
  * @irq_type:       GENMASK for IRQ_TYPE field
  */
 struct pdc_irq_cfg {
+	u32 gpio_irq_sts;
+	u32 gpio_irq_mask;
 	u32 irq_enable;
 	u32 irq_type;
 };
@@ -102,11 +112,14 @@ struct pdc_irq_cfg {
  * @num_spis:       Total number of direct 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_fields:     Fields of 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,12 +132,15 @@ struct pdc_desc {
 
 	bool				x1e_quirk;
 
+	u8				mode;
 	raw_spinlock_t			lock;
 
 	const struct pdc_regs		*regs;
 	const struct pdc_irq_cfg	*cfg_fields;
 
 	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 = {
@@ -133,6 +149,8 @@ static const struct pdc_regs pdc_v3_2 = {
 };
 
 static const struct pdc_irq_cfg pdc_cfg_v3_2 = {
+	.gpio_irq_sts	= 5,
+	.gpio_irq_mask	= 4,
 	.irq_enable	= 3,
 	.irq_type	= GENMASK(2, 0),
 };
@@ -144,6 +162,8 @@ static const struct pdc_regs pdc_v3_0 = {
 };
 
 static const struct pdc_irq_cfg pdc_cfg_v3_0 = {
+	.gpio_irq_sts	= 4,
+	.gpio_irq_mask	= 3,
 	.irq_type	= GENMASK(2, 0),
 };
 
@@ -182,6 +202,15 @@ static u32 pdc_reg_read(int reg, u32 i)
 	return readl_relaxed(pdc->base + reg + i * sizeof(u32));
 }
 
+static inline bool pdc_pin_is_gpio(int pin_out)
+{
+	/*
+	 * PDC allocates direct SPIs at the beginning and
+	 * all GPIOs as SPIs are allocated after direct SPIs.
+	 */
+	return pin_out >= pdc->num_spis;
+}
+
 static void pdc_x1e_irq_enable_write(u32 bank, u32 enable)
 {
 	void __iomem *base;
@@ -229,6 +258,24 @@ static void pdc_enable_intr_bank(int pin_out, bool on)
 		pdc_reg_write(pdc->regs->irq_en_reg, index, enable);
 }
 
+static void pdc_clear_gpio_cfg(int pin_out)
+{
+	unsigned long gpio_sts;
+
+	gpio_sts = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
+	__clear_bit(pdc->cfg_fields->gpio_irq_sts, &gpio_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;
+
+	gpio_mask = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
+	__assign_bit(pdc->cfg_fields->gpio_irq_mask, &gpio_mask, !unmask);
+	pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, gpio_mask);
+}
+
 static void pdc_enable_intr_cfg(int pin_out, bool on)
 {
 	unsigned long enable = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
@@ -237,6 +284,13 @@ static void pdc_enable_intr_cfg(int pin_out, bool on)
 	pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, enable);
 }
 
+static void qcom_pdc_gic_secondary_disable(struct irq_data *d)
+{
+	pdc->enable_intr(d->hwirq, false);
+	pdc->unmask_gpio(d->hwirq, false);
+	irq_chip_disable_parent(d);
+}
+
 static void qcom_pdc_gic_disable(struct irq_data *d)
 {
 	pdc->enable_intr(d->hwirq, false);
@@ -249,6 +303,41 @@ static void qcom_pdc_gic_enable(struct irq_data *d)
 	irq_chip_enable_parent(d);
 }
 
+static void qcom_pdc_gic_secondary_enable(struct irq_data *d)
+{
+	pdc->enable_intr(d->hwirq, true);
+	pdc->unmask_gpio(d->hwirq, true);
+	irq_chip_enable_parent(d);
+}
+
+static void qcom_pdc_secondary_ack(struct irq_data *d)
+{
+	if (!irqd_is_level_type(d))
+		pdc->clear_gpio(d->hwirq);
+}
+
+static void qcom_pdc_gic_secondary_eoi(struct irq_data *d)
+{
+	if (irqd_is_level_type(d))
+		pdc->clear_gpio(d->hwirq);
+
+	irq_chip_eoi_parent(d);
+}
+
+static void qcom_pdc_secondary_mask(struct irq_data *d)
+{
+	pdc->enable_intr(d->hwirq, false);
+	pdc->unmask_gpio(d->hwirq, false);
+	irq_chip_mask_parent(d);
+}
+
+static void qcom_pdc_secondary_unmask(struct irq_data *d)
+{
+	pdc->enable_intr(d->hwirq, true);
+	pdc->unmask_gpio(d->hwirq, true);
+	irq_chip_unmask_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
@@ -275,18 +364,18 @@ enum pdc_irq_config_bits {
 /**
  * qcom_pdc_gic_set_type: Configure PDC for the interrupt
  *
- * @d: the interrupt data
+ * @d:    the interrupt data
  * @type: the interrupt type
  *
- * If @type is edge triggered, forward that as Rising edge as PDC
- * takes care of converting falling edge to rising edge signal
+ * If @type is edge triggered, forward that as rising edge as PDC
+ * takes care of converting all edge types 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
+ * takes care of converting all level types to level high signal
  */
 static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
 {
-	enum pdc_irq_config_bits pdc_type;
 	enum pdc_irq_config_bits old_pdc_type;
+	enum pdc_irq_config_bits pdc_type;
 	int ret;
 
 	switch (type) {
@@ -336,6 +425,72 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type)
 	return 0;
 }
 
+/**
+ * qcom_pdc_gic_set_type: Configure PDC for the interrupt
+ *
+ * @d:    the interrupt data
+ * @type: the interrupt type
+ *
+ * All @type are forwarded as level high type to parent GIC
+ */
+static int qcom_pdc_gic_secondary_set_type(struct irq_data *d, unsigned int type)
+{
+	enum pdc_irq_config_bits old_pdc_type;
+	enum pdc_irq_config_bits pdc_type;
+	int ret;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		pdc_type = PDC_EDGE_RISING;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		pdc_type = PDC_EDGE_FALLING;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		pdc_type = PDC_EDGE_DUAL;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		pdc_type = PDC_LEVEL_HIGH;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		pdc_type = PDC_LEVEL_LOW;
+		break;
+	default:
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	old_pdc_type = pdc_reg_read(pdc->regs->irq_cfg_reg, d->hwirq);
+	pdc_type |= (old_pdc_type & ~pdc->cfg_fields->irq_type);
+	pdc_reg_write(pdc->regs->irq_cfg_reg, d->hwirq, pdc_type);
+
+	/*
+	 * 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;
+
+	/*
+	 * When we change types the PDC can give a phantom interrupt.
+	 * Clear it.  Specifically the phantom shows up when reconfiguring
+	 * polarity of interrupt without changing the state of the signal
+	 * but let's be consistent and clear it always.
+	 *
+	 * Doing this works because we have IRQCHIP_SET_TYPE_MASKED so the
+	 * interrupt will be cleared before the rest of the system sees it.
+	 */
+	if (old_pdc_type != pdc_type)
+		irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, false);
+
+	return 0;
+}
+
 static struct irq_chip qcom_pdc_gic_chip = {
 	.name			= "PDC",
 	.irq_eoi		= irq_chip_eoi_parent,
@@ -355,6 +510,26 @@ static struct irq_chip qcom_pdc_gic_chip = {
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 };
 
+static struct irq_chip qcom_pdc_gic_secondary_chip = {
+	.name			= "PDC",
+	.irq_ack		= qcom_pdc_secondary_ack,
+	.irq_eoi		= qcom_pdc_gic_secondary_eoi,
+	.irq_mask		= qcom_pdc_secondary_mask,
+	.irq_unmask		= qcom_pdc_secondary_unmask,
+	.irq_disable		= qcom_pdc_gic_secondary_disable,
+	.irq_enable		= qcom_pdc_gic_secondary_enable,
+	.irq_get_irqchip_state	= irq_chip_get_parent_state,
+	.irq_set_irqchip_state	= irq_chip_set_parent_state,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_type		= qcom_pdc_gic_secondary_set_type,
+	.flags			= IRQCHIP_MASK_ON_SUSPEND |
+				  IRQCHIP_SET_TYPE_MASKED |
+				  IRQCHIP_SKIP_SET_WAKE |
+				  IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND,
+	.irq_set_vcpu_affinity	= irq_chip_set_vcpu_affinity_parent,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+};
+
 static struct pdc_pin_region *get_pin_region(int pin)
 {
 	for (int i = 0; i < pdc->region_cnt; i++) {
@@ -388,16 +563,38 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
 	if (ret)
 		return ret;
 
+	/*
+	 * PDC secondary chip is only set for the GPIO interrupts as SPIs.
+	 * Direct SPI interrupts are still in pass through mode (no latching
+	 * at PDC).
+	 */
+	if (pdc->mode == PDC_SECONDARY_MODE && pdc_pin_is_gpio(hwirq)) {
+		ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+						    &qcom_pdc_gic_secondary_chip,
+						    NULL);
+		if (ret)
+			return ret;
+
+		/* Secondary mode converts all interrupts to LEVEL HIGH type */
+		type = IRQ_TYPE_LEVEL_HIGH;
+	} else {
+		ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
+						    &qcom_pdc_gic_chip,
+						    NULL);
+		if (ret)
+			return ret;
+
+		if (type & IRQ_TYPE_EDGE_BOTH)
+			type = IRQ_TYPE_EDGE_RISING;
+
+		if (type & IRQ_TYPE_LEVEL_MASK)
+			type = IRQ_TYPE_LEVEL_HIGH;
+	}
+
 	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;
@@ -443,8 +640,13 @@ static int pdc_setup_pin_mapping(struct device *dev)
 		if (ret)
 			return ret;
 
-		for (int i = 0; i < pdc->region[n].cnt; i++)
-			pdc->enable_intr(i + pdc->region[n].pin_base, 0);
+		for (int i = 0; i < pdc->region[n].cnt; i++) {
+			if (pdc_pin_is_gpio(i + pdc->region[n].pin_base) &&
+			    pdc->mode == PDC_SECONDARY_MODE)
+				pdc->clear_gpio(i + pdc->region[n].pin_base);
+
+			pdc->enable_intr(i + pdc->region[n].pin_base, false);
+		}
 	}
 
 	return 0;
@@ -495,6 +697,8 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 		pdc->enable_intr = pdc_enable_intr_bank;
 	}
 
+	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
@@ -512,6 +716,16 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
 		}
 
 		pdc->x1e_quirk = true;
+
+		if (!qcom_scm_is_available())
+			return -EPROBE_DEFER;
+
+		ret = qcom_scm_io_writel(PDC_GPIO_INT_CTL_ENABLE, PDC_PASS_THROUGH_MODE);
+		if (ret) {
+			pdc->mode = PDC_SECONDARY_MODE;
+			pdc->unmask_gpio = pdc_unmask_gpio_cfg;
+			pdc->clear_gpio = pdc_clear_gpio_cfg;
+		}
 	}
 
 	irq_param = pdc_reg_read(pdc->regs->irq_param_reg, 0);

-- 
2.43.0

[PATCH v4 5/7] pinctrl: qcom: Acknowledge IRQs for PDC interrupt controller

[Maulik Shah]

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 fe94ce5f9b81..9c720f49465b 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -994,6 +994,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 && d->parent_data->chip->irq_ack)
+			irq_chip_ack_parent(d);
 		return;
 	}
 
@@ -1064,7 +1074,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 (type & IRQ_TYPE_LEVEL_MASK)
+			irq_set_handler_locked(d, handle_fasteoi_irq);
+		else
+			irq_set_handler_locked(d, handle_fasteoi_ack_irq);
 		return 0;
 	}
 

-- 
2.43.0

[PATCH v4 6/7] Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now"

[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

[PATCH v4 7/7] arm64: dts: qcom: x1e80100: Add deepest idle state

[Maulik Shah]

Add deepest idle state as GPIO IRQs can work as wakeup capable interrupts
in deepest idle state.

Update entry/exit-latency-us to follow DSDT for cluster_cl5 idle state.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/hamoa.dtsi | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
index 054f9c4ad192..933d81fe7841 100644
--- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
@@ -299,10 +299,18 @@ cluster_cl4: cluster-sleep-0 {
 			cluster_cl5: cluster-sleep-1 {
 				compatible = "domain-idle-state";
 				arm,psci-suspend-param = <0x01000054>;
-				entry-latency-us = <2200>;
-				exit-latency-us = <4000>;
+				entry-latency-us = <2000>;
+				exit-latency-us = <2000>;
 				min-residency-us = <7000>;
 			};
+
+			domain_ss3: domain-sleep-0 {
+				compatible = "domain-idle-state";
+				arm,psci-suspend-param = <0x0200c354>;
+				entry-latency-us = <2500>;
+				exit-latency-us = <2500>;
+				min-residency-us = <9000>;
+			};
 		};
 	};
 
@@ -461,7 +469,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

Re: [PATCH v4 1/7] irqchip/qcom-pdc: restructure version support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The global `pdc_version` is left uninitialized, completely breaking interrupt enablement on PDC v3.2 devices due to executing the wrong code path.
--

--- Patch [1]: [PATCH v4 1/7] irqchip/qcom-pdc: restructure version support ---
commit 9e72443d7a81acfbf2e3c820dae28eecbd9b0db1
Author: Maulik Shah <maulik.shah@oss.qualcomm.com>

irqchip/qcom-pdc: restructure version support

This commit restructures the PDC driver by introducing a pdc_desc struct to
manage the base addresses, registers, and version information per device.
This prepares the driver to handle different hardware register layouts
and secondary controller modes more cleanly.

> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 2014dbb0bc43a..a77d1d3340629 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
[ ... ]
> @@ -383,6 +462,30 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
[ ... ]
> +	pdc = devm_kzalloc(dev, sizeof(*pdc), GFP_KERNEL);
> +	if (!pdc)
> +		return -ENOMEM;
[ ... ]
> +	pdc->version = pdc_reg_read(PDC_VERSION_REG, 0);
[ ... ]
> @@ -401,48 +505,29 @@ static int qcom_pdc_probe(struct platform_device *pdev, struct device_node *pare
[ ... ]
> -	pdc_version = pdc_reg_read(PDC_VERSION_REG, 0);
> -

[Severity: High]
Since the global pdc_version is no longer initialized here, won't it default
to 0?

If __pdc_enable_intr() still relies on the global pdc_version variable to
branch:

	if (pdc_version < PDC_VERSION_3_2)
		pdc_enable_intr_bank(pin_out, on);
	else
		pdc_enable_intr_cfg(pin_out, on);

will PDC v3.2 hardware incorrectly take the legacy bank configuration path?

Because pdc->regs->irq_en_reg is intentionally 0 for v3.2 devices, this
looks like it might read and write to offset 0 (the PDC base) instead of
the IRQ_CFG register, potentially corrupting unrelated registers and failing
to enable interrupts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-hamoa_pdc_v3-v4-0-dfd1f4a3ae89@oss.qualcomm.com?part=1

Re: [PATCH v4 4/7] irqchip/qcom-pdc: Configure PDC to pass through mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Edge-triggered GPIO interrupts on PDC secondary controllers are never cleared from the hardware latch, leading to an infinite interrupt storm.
- [High] Unprotected Read-Modify-Write (RMW) operations on `irq_cfg_reg` can overwrite and clear the hardware interrupt latch (`gpio_irq_sts`), resulting in lost interrupts.
- [Low] Redundant call to `irq_domain_set_hwirq_and_chip` in `qcom_pdc_alloc`.
--

--- Patch [4]: [PATCH v4 4/7] irqchip/qcom-pdc: Configure PDC to pass through mode ---
commit 283bb75867e93f6367d30274c8a45615e4e22c2c
Author: Maulik Shah <maulik.shah@oss.qualcomm.com>

irqchip/qcom-pdc: Configure PDC to pass through mode

This commit configures the PDC to pass-through mode via SCM write, and
falls back to a secondary controller mode if it fails. It introduces separate
IRQ chip operations to handle GPIO interrupts in secondary mode.

> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 53a477aa97655..d122e8e7dc9d7 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
[ ... ]
> @@ -229,6 +258,24 @@ static void pdc_enable_intr_bank(int pin_out, bool on)
>  		pdc_reg_write(pdc->regs->irq_en_reg, index, enable);
>  }
>  
> +static void pdc_clear_gpio_cfg(int pin_out)
> +{
> +	unsigned long gpio_sts;
> +
> +	gpio_sts = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
> +	__clear_bit(pdc->cfg_fields->gpio_irq_sts, &gpio_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;
> +
> +	gpio_mask = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
> +	__assign_bit(pdc->cfg_fields->gpio_irq_mask, &gpio_mask, !unmask);
> +	pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, gpio_mask);
> +}

[Severity: High]
Can these software Read-Modify-Write operations on irq_cfg_reg race with
hardware setting the interrupt status bit?

If a hardware interrupt arrives between the software read and write, the
software will read a 0 for gpio_irq_sts. When the hardware sets it to 1,
and the software writes back the stale 0, does this inadvertently clear
the newly arrived interrupt and drop it entirely?

[ ... ]
> +static void qcom_pdc_secondary_ack(struct irq_data *d)
> +{
> +	if (!irqd_is_level_type(d))
> +		pdc->clear_gpio(d->hwirq);
> +}
> +
> +static void qcom_pdc_gic_secondary_eoi(struct irq_data *d)
> +{
> +	if (irqd_is_level_type(d))
> +		pdc->clear_gpio(d->hwirq);
> +
> +	irq_chip_eoi_parent(d);
> +}

[Severity: High]
For edge-triggered interrupts, does this logic result in the hardware latch
never being cleared, leading to an interrupt storm?

Since qcom_pdc_alloc() forces these interrupts to IRQ_TYPE_LEVEL_HIGH before
passing them to the parent GIC domain, the GIC will unconditionally configure
them to use handle_fasteoi_irq().

Because handle_fasteoi_irq() only calls .irq_eoi and never calls .irq_ack,
qcom_pdc_secondary_ack() appears to be unreachable dead code. Meanwhile,
qcom_pdc_gic_secondary_eoi() explicitly skips clearing the latch for edge
types.

Would we need to call irq_set_handler_locked(d, handle_fasteoi_ack_irq) in
qcom_pdc_gic_secondary_set_type() for edge interrupts to ensure the latch gets
cleared?

[ ... ]
> @@ -388,16 +563,38 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * PDC secondary chip is only set for the GPIO interrupts as SPIs.
> +	 * Direct SPI interrupts are still in pass through mode (no latching
> +	 * at PDC).
> +	 */
> +	if (pdc->mode == PDC_SECONDARY_MODE && pdc_pin_is_gpio(hwirq)) {
> +		ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> +						    &qcom_pdc_gic_secondary_chip,
> +						    NULL);
> +		if (ret)
> +			return ret;
> +
> +		/* Secondary mode converts all interrupts to LEVEL HIGH type */
> +		type = IRQ_TYPE_LEVEL_HIGH;
> +	} else {
> +		ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> +						    &qcom_pdc_gic_chip,
> +						    NULL);
> +		if (ret)
> +			return ret;

[Severity: Low]
Is this call to irq_domain_set_hwirq_and_chip() for &qcom_pdc_gic_chip
redundant?

There is already an unconditional call to irq_domain_set_hwirq_and_chip()
for &qcom_pdc_gic_chip just before the block added by this hunk. Does this
duplicate the earlier setup?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-hamoa_pdc_v3-v4-0-dfd1f4a3ae89@oss.qualcomm.com?part=4

Re: [PATCH v4 6/7] Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now"

[Krzysztof Kozlowski]

On 07/07/2026 11:21, Maulik Shah wrote:
> 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

"No more" feels like you fixed it in previous commit, so this should be
squashed there, no?

Or was this changed (fixed) some time ago?


Best regards,
Krzysztof

Re: [PATCH v4 0/7] x1e80100: Enable PDC wake GPIOs and deepest idle state

[Thomas Gleixner]

On Tue, Jul 07 2026 at 14:51, Maulik Shah wrote:
> The series has been tested on x1e80100 CRD with both old and new firmware
> and also on kaanapali. Test conducted with tlmm-test module after
> applying [3] as test module needed to be fixed first.
>
> All 17/17 passes in pass through mode and 16/17 passes in secondary mode.
> Failing test tlmm_test_rising_while_disabled seems to be because when in
> irq disabled state PDC is not latching the edge interrupt.

I've merged the first four patches and tagged them as promised for
consumption by the GPIO tree:

   git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-chip-qcom-pdc-for-gpio-07-07-26

Thanks,

        tglx
     

Re: [PATCH v4 0/7] x1e80100: Enable PDC wake GPIOs and deepest idle state

[Bartosz Golaszewski]

On Tue, 7 Jul 2026 22:33:44 +0200, Thomas Gleixner <tglx@kernel.org> said:
> On Tue, Jul 07 2026 at 14:51, Maulik Shah wrote:
>> The series has been tested on x1e80100 CRD with both old and new firmware
>> and also on kaanapali. Test conducted with tlmm-test module after
>> applying [3] as test module needed to be fixed first.
>>
>> All 17/17 passes in pass through mode and 16/17 passes in secondary mode.
>> Failing test tlmm_test_rising_while_disabled seems to be because when in
>> irq disabled state PDC is not latching the edge interrupt.
>
> I've merged the first four patches and tagged them as promised for
> consumption by the GPIO tree:
>
>    git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-chip-qcom-pdc-for-gpio-07-07-26
>
> Thanks,
>
>         tglx
>
>

Pulled, thanks!

Bartosz

Re: (subset) [PATCH v4 0/7] x1e80100: Enable PDC wake GPIOs and deepest idle state

[Bartosz Golaszewski]

On Tue, 07 Jul 2026 14:51:32 +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.
> 
> [...]

Applied, thanks!

[5/7] pinctrl: qcom: Acknowledge IRQs for PDC interrupt controller
      https://git.kernel.org/brgl/c/f790ea0b699d95d18572979b1bc1673d8f31eb3c
[6/7] Revert "pinctrl: qcom: x1e80100: Bypass PDC wakeup parent for now"
      https://git.kernel.org/brgl/c/77fbc756d9cbe53a9496cb2c53ae209d37d5af2d

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>