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

Neil Armstrong posted 1 patch 2 years, 3 months ago
There is a newer version of this series
drivers/irqchip/qcom-pdc.c | 64 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 53 insertions(+), 11 deletions(-)
[PATCH] irqchip/qcom-pdc: add support for v3.2 HW
Posted by Neil Armstrong 2 years, 3 months ago
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.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 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,
+};
+
 static struct pdc_pin_region *get_pin_region(int pin)
 {
 	int i;
@@ -202,6 +229,7 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
 	struct irq_fwspec *fwspec = data;
 	struct irq_fwspec parent_fwspec;
 	struct pdc_pin_region *region;
+	struct irq_chip *chip;
 	irq_hw_number_t hwirq;
 	unsigned int type;
 	int ret;
@@ -213,8 +241,12 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
 	if (hwirq == GPIO_NO_WAKE_IRQ)
 		return irq_domain_disconnect_hierarchy(domain, virq);
 
-	ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
-					    &qcom_pdc_gic_chip, NULL);
+	if (pdc_version >= PDC_VERSION_3_2)
+		chip = &qcom_pdc_gic_chip_3_2;
+	else
+		chip = &qcom_pdc_gic_chip;
+
+	ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, chip, NULL);
 	if (ret)
 		return ret;
 
@@ -244,10 +276,23 @@ 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 void pdc_enable_region(unsigned int n)
 {
-	int ret, n, i;
 	u32 irq_index, reg_index, val;
+	int i;
+
+	for (i = 0; i < pdc_region[n].cnt; i++) {
+		reg_index = (i + pdc_region[n].pin_base) >> 5;
+		irq_index = (i + pdc_region[n].pin_base) & 0x1f;
+		val = pdc_reg_read(IRQ_ENABLE_BANK, reg_index);
+		val &= ~BIT(irq_index);
+		pdc_reg_write(IRQ_ENABLE_BANK, reg_index, val);
+	}
+}
+
+static int pdc_setup_pin_mapping(struct device_node *np)
+{
+	int ret, n;
 
 	n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32));
 	if (n <= 0 || n % 3)
@@ -277,13 +322,8 @@ static int pdc_setup_pin_mapping(struct device_node *np)
 		if (ret)
 			return ret;
 
-		for (i = 0; i < pdc_region[n].cnt; i++) {
-			reg_index = (i + pdc_region[n].pin_base) >> 5;
-			irq_index = (i + pdc_region[n].pin_base) & 0x1f;
-			val = pdc_reg_read(IRQ_ENABLE_BANK, reg_index);
-			val &= ~BIT(irq_index);
-			pdc_reg_write(IRQ_ENABLE_BANK, reg_index, val);
-		}
+		if (pdc_version < PDC_VERSION_3_2)
+			pdc_enable_region(n);
 	}
 
 	return 0;
@@ -300,6 +340,8 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent)
 		return -ENXIO;
 	}
 
+	pdc_version = pdc_reg_read(PDC_VERSION, 0);
+
 	parent_domain = irq_find_host(parent);
 	if (!parent_domain) {
 		pr_err("%pOF: unable to find PDC's parent domain\n", node);

---
base-commit: 47d9bb711707d15b19fad18c8e2b4b027a264a3a
change-id: 20230821-topic-sm8x50-upstream-pdc-ver-114ceb45e1ee

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>
Re: [PATCH] irqchip/qcom-pdc: add support for v3.2 HW
Posted by Maulik Shah (mkshah) 2 years, 3 months ago
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@linaro.org>
> ---
>   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.

Thanks,
Maulik

Re: [PATCH] irqchip/qcom-pdc: add support for v3.2 HW
Posted by Neil Armstrong 2 years, 3 months ago
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@linaro.org>
>> ---
>>   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
>