All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] irqchip/qcom-pdc: add support for v3.2 HW
@ 2023-08-21  7:30 Neil Armstrong
  2023-08-21  8:41 ` Maulik Shah (mkshah)
  0 siblings, 1 reply; 3+ messages in thread
From: Neil Armstrong @ 2023-08-21  7:30 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Thomas Gleixner,
	Marc Zyngier
  Cc: linux-arm-msm, linux-kernel, Neil Armstrong

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>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] irqchip/qcom-pdc: add support for v3.2 HW
  2023-08-21  7:30 [PATCH] irqchip/qcom-pdc: add support for v3.2 HW Neil Armstrong
@ 2023-08-21  8:41 ` Maulik Shah (mkshah)
  2023-08-21 12:14   ` Neil Armstrong
  0 siblings, 1 reply; 3+ messages in thread
From: Maulik Shah (mkshah) @ 2023-08-21  8:41 UTC (permalink / raw)
  To: Neil Armstrong, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Thomas Gleixner, Marc Zyngier
  Cc: linux-arm-msm, linux-kernel

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] irqchip/qcom-pdc: add support for v3.2 HW
  2023-08-21  8:41 ` Maulik Shah (mkshah)
@ 2023-08-21 12:14   ` Neil Armstrong
  0 siblings, 0 replies; 3+ messages in thread
From: Neil Armstrong @ 2023-08-21 12:14 UTC (permalink / raw)
  To: Maulik Shah (mkshah),
	Andy Gross, Bjorn Andersson, Konrad Dybcio, Thomas Gleixner,
	Marc Zyngier
  Cc: linux-arm-msm, linux-kernel

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
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-08-21 12:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-21  7:30 [PATCH] irqchip/qcom-pdc: add support for v3.2 HW Neil Armstrong
2023-08-21  8:41 ` Maulik Shah (mkshah)
2023-08-21 12:14   ` Neil Armstrong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.