Re: [PATCH] irqchip/qcom-pdc: add support for v3.2 HW

From: Neil Armstrong
Date: Mon Aug 21 2023 - 08:14:25 EST


On 21/08/2023 10:41, Maulik Shah (mkshah) wrote:
Hi,

On 8/21/2023 1:00 PM, Neil Armstrong wrote:
The PDC Hw version 3.2 and later doesn't have the enable registers
anymore, get the HW version from registers and stop accessing those
registers starting at this version.

Starting PDC v3.2, enable bit is moved from IRQ_ENABLE_BANK to the IRQ_i_CFG register.
we need to set enable bit to tell PDC HW whether IRQ is enabled / disabled in order to wake up from SoC sleep states.


Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
---
  drivers/irqchip/qcom-pdc.c | 64 ++++++++++++++++++++++++++++++++++++++--------
  1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index d96916cf6a41..620efb9fcc96 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -26,6 +26,13 @@
  #define IRQ_ENABLE_BANK        0x10
  #define IRQ_i_CFG        0x110
+#define PDC_VERSION        0x1000
+
+/* Notable PDC versions */
+enum {
+    PDC_VERSION_3_2    = 0x30200,
+};
+
  struct pdc_pin_region {
      u32 pin_base;
      u32 parent_base;
@@ -38,6 +45,7 @@ static DEFINE_RAW_SPINLOCK(pdc_lock);
  static void __iomem *pdc_base;
  static struct pdc_pin_region *pdc_region;
  static int pdc_region_cnt;
+static unsigned int pdc_version;
  static void pdc_reg_write(int reg, u32 i, u32 val)
  {
@@ -183,6 +191,25 @@ static struct irq_chip qcom_pdc_gic_chip = {
      .irq_set_affinity    = irq_chip_set_affinity_parent,
  };
+static struct irq_chip qcom_pdc_gic_chip_3_2 = {
+    .name            = "PDC",
+    .irq_eoi        = irq_chip_eoi_parent,
+    .irq_mask        = irq_chip_mask_parent,
+    .irq_unmask        = irq_chip_unmask_parent,
+    .irq_disable        = irq_chip_disable_parent,
+    .irq_enable        = irq_chip_enable_parent,
+    .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_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,
+};
+
we should not need to have another irqchip defined for same as these remains same for old/new version.

Not sure if you have access to downstream change, or let me know i can post the same.
In downstream we do like...

The 3rd bit in IRQ_i_CFG in PDC v3.2 is for enable/disable,
The 0-2 bits are for the IRQ type in all PDC versions..

    #define IRQ_i_CFG_IRQ_ENABLE   BIT(3)
    #define IRQ_i_CFG_TYPE_MASK    0x7

and modify pdc_enable_intr()

       if (pdc_version < PDC_VERSION_3_2) {
               index = pin_out / 32;
               mask = pin_out % 32;
               enable = pdc_reg_read(IRQ_ENABLE_BANK, index);
               __assign_bit(mask, &enable, on);
               pdc_reg_write(IRQ_ENABLE_BANK, index, enable);
       } else {
               index = d->hwirq;
               enable = pdc_reg_read(IRQ_i_CFG, index);
               __assign_bit(IRQ_i_CFG_IRQ_ENABLE, &enable, on);
               pdc_reg_write(IRQ_i_CFG, index, enable);
       }

and qcom_pdc_gic_set_type(), add line to only take 0-2 bits from old_pdc_type as they are for type.

        old_pdc_type = pdc_reg_read(IRQ_i_CFG, d->hwirq);
+       pdc_type |= (old_pdc_type & ~IRQ_i_CFG_TYPE_MASK);
+        if (pdc_version < PDC_VERSION_3_2)
+            pdc_enable_region(n);
      }

you will still need to clear enable bits in version 3.2.

Ack, thx for the review, I'll fix all that for v2.

Neil


Thanks,
Maulik