diff mbox series

[Xen-devel,18/57] ARM: GICv2: introduce gicv2_poke_irq()

Message ID 20180305160415.16760-19-andre.przywara@linaro.org
State Superseded
Headers show
Series New VGIC(-v2) implementation | expand

Commit Message

Andre Przywara March 5, 2018, 4:03 p.m. UTC
The GICv2 uses bitmaps spanning several MMIO registers for holding some
interrupt state. Similar to GICv3, add a poke helper functions to set a bit
for a given irq_desc in one of those bitmaps.
At the moment there is only one use in gic-v2.c, but there will be more
coming soon.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
Changelog RFC ... v1:
- new patch

 xen/arch/arm/gic-v2.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Julien Grall March 6, 2018, 3:23 p.m. UTC | #1
Hi,

On 05/03/18 16:03, Andre Przywara wrote:
> The GICv2 uses bitmaps spanning several MMIO registers for holding some
> interrupt state. Similar to GICv3, add a poke helper functions to set a bit
> for a given irq_desc in one of those bitmaps.
> At the moment there is only one use in gic-v2.c, but there will be more
> coming soon.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
> Changelog RFC ... v1:
> - new patch
> 
>   xen/arch/arm/gic-v2.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 7938a42591..618dd94120 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -235,6 +235,11 @@ static unsigned int gicv2_read_irq(void)
>       return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
>   }
>   
> +static void gicv2_poke_irq(struct irq_desc *irqd, uint32_t offset)

NIT: s/irqd/desc/ to match the naming in Xen.

With that:

Reviewed-by: Julien Grall <julien.grall@arm.com>

> +{
> +    writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4);
> +}
> +
>   static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type)
>   {
>       uint32_t cfg, actual, edgebit;
> @@ -509,7 +514,6 @@ static unsigned int gicv2_read_apr(int apr_reg)
>   static void gicv2_irq_enable(struct irq_desc *desc)
>   {
>       unsigned long flags;
> -    int irq = desc->irq;
>   
>       ASSERT(spin_is_locked(&desc->lock));
>   
> @@ -517,20 +521,19 @@ static void gicv2_irq_enable(struct irq_desc *desc)
>       clear_bit(_IRQ_DISABLED, &desc->status);
>       dsb(sy);
>       /* Enable routing */
> -    writel_gicd((1u << (irq % 32)), GICD_ISENABLER + (irq / 32) * 4);
> +    gicv2_poke_irq(desc, GICD_ISENABLER);
>       spin_unlock_irqrestore(&gicv2.lock, flags);
>   }
>   
>   static void gicv2_irq_disable(struct irq_desc *desc)
>   {
>       unsigned long flags;
> -    int irq = desc->irq;
>   
>       ASSERT(spin_is_locked(&desc->lock));
>   
>       spin_lock_irqsave(&gicv2.lock, flags);
>       /* Disable routing */
> -    writel_gicd(1u << (irq % 32), GICD_ICENABLER + (irq / 32) * 4);
> +    gicv2_poke_irq(desc, GICD_ICENABLER);
>       set_bit(_IRQ_DISABLED, &desc->status);
>       spin_unlock_irqrestore(&gicv2.lock, flags);
>   }
> 

Cheers,
Julien Grall March 6, 2018, 3:25 p.m. UTC | #2
Hi,

On 06/03/18 15:23, Julien Grall wrote:
> Hi,
> 
> On 05/03/18 16:03, Andre Przywara wrote:
>> The GICv2 uses bitmaps spanning several MMIO registers for holding some
>> interrupt state. Similar to GICv3, add a poke helper functions to set 
>> a bit
>> for a given irq_desc in one of those bitmaps.
>> At the moment there is only one use in gic-v2.c, but there will be more
>> coming soon.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>> Changelog RFC ... v1:
>> - new patch
>>
>>   xen/arch/arm/gic-v2.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
>> index 7938a42591..618dd94120 100644
>> --- a/xen/arch/arm/gic-v2.c
>> +++ b/xen/arch/arm/gic-v2.c
>> @@ -235,6 +235,11 @@ static unsigned int gicv2_read_irq(void)
>>       return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
>>   }
>> +static void gicv2_poke_irq(struct irq_desc *irqd, uint32_t offset)
> 
> NIT: s/irqd/desc/ to match the naming in Xen.

Scratch that. It looks like we already have quite a few irqd in the code.

Cheers,

>  > With that:
> 
> Reviewed-by: Julien Grall <julien.grall@arm.com>
> 
>> +{
>> +    writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4);
>> +}
>> +
>>   static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int 
>> type)
>>   {
>>       uint32_t cfg, actual, edgebit;
>> @@ -509,7 +514,6 @@ static unsigned int gicv2_read_apr(int apr_reg)
>>   static void gicv2_irq_enable(struct irq_desc *desc)
>>   {
>>       unsigned long flags;
>> -    int irq = desc->irq;
>>       ASSERT(spin_is_locked(&desc->lock));
>> @@ -517,20 +521,19 @@ static void gicv2_irq_enable(struct irq_desc *desc)
>>       clear_bit(_IRQ_DISABLED, &desc->status);
>>       dsb(sy);
>>       /* Enable routing */
>> -    writel_gicd((1u << (irq % 32)), GICD_ISENABLER + (irq / 32) * 4);
>> +    gicv2_poke_irq(desc, GICD_ISENABLER);
>>       spin_unlock_irqrestore(&gicv2.lock, flags);
>>   }
>>   static void gicv2_irq_disable(struct irq_desc *desc)
>>   {
>>       unsigned long flags;
>> -    int irq = desc->irq;
>>       ASSERT(spin_is_locked(&desc->lock));
>>       spin_lock_irqsave(&gicv2.lock, flags);
>>       /* Disable routing */
>> -    writel_gicd(1u << (irq % 32), GICD_ICENABLER + (irq / 32) * 4);
>> +    gicv2_poke_irq(desc, GICD_ICENABLER);
>>       set_bit(_IRQ_DISABLED, &desc->status);
>>       spin_unlock_irqrestore(&gicv2.lock, flags);
>>   }
>>
> 
> Cheers,
>
diff mbox series

Patch

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 7938a42591..618dd94120 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -235,6 +235,11 @@  static unsigned int gicv2_read_irq(void)
     return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
 }
 
+static void gicv2_poke_irq(struct irq_desc *irqd, uint32_t offset)
+{
+    writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4);
+}
+
 static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type)
 {
     uint32_t cfg, actual, edgebit;
@@ -509,7 +514,6 @@  static unsigned int gicv2_read_apr(int apr_reg)
 static void gicv2_irq_enable(struct irq_desc *desc)
 {
     unsigned long flags;
-    int irq = desc->irq;
 
     ASSERT(spin_is_locked(&desc->lock));
 
@@ -517,20 +521,19 @@  static void gicv2_irq_enable(struct irq_desc *desc)
     clear_bit(_IRQ_DISABLED, &desc->status);
     dsb(sy);
     /* Enable routing */
-    writel_gicd((1u << (irq % 32)), GICD_ISENABLER + (irq / 32) * 4);
+    gicv2_poke_irq(desc, GICD_ISENABLER);
     spin_unlock_irqrestore(&gicv2.lock, flags);
 }
 
 static void gicv2_irq_disable(struct irq_desc *desc)
 {
     unsigned long flags;
-    int irq = desc->irq;
 
     ASSERT(spin_is_locked(&desc->lock));
 
     spin_lock_irqsave(&gicv2.lock, flags);
     /* Disable routing */
-    writel_gicd(1u << (irq % 32), GICD_ICENABLER + (irq / 32) * 4);
+    gicv2_poke_irq(desc, GICD_ICENABLER);
     set_bit(_IRQ_DISABLED, &desc->status);
     spin_unlock_irqrestore(&gicv2.lock, flags);
 }