[PATCH 2/2] hw/arm/aspeed_ast27x0: Avoid hardcoded '256' in IRQ calculation

Peter Maydell posted 2 patches 3 weeks, 1 day ago
[PATCH 2/2] hw/arm/aspeed_ast27x0: Avoid hardcoded '256' in IRQ calculation
Posted by Peter Maydell 3 weeks, 1 day ago
When calculating the index into the GIC's GPIO array for per-CPU
interrupts, we have to start with the number of SPIs.  The code
currently hard-codes this to 'NUM_IRQS = 256'.  However the number of
SPIs is set separately and implicitly by the value of
AST2700_MAX_IRQ, which is the number of SPIs plus 32 (since it is
what we set the GIC num-irq property to).

Define AST2700_MAX_IRQ as the total number of SPIs; this brings
AST2700 into line with AST2600, which defines AST2600_MAX_IRQ as the
number of SPIs not including the 32 internal interrupts.  We can then
use AST2700_MAX_IRQ instead of the hardcoded 256.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/aspeed_ast27x0.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 5638a7a5781..7b246440952 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -66,7 +66,7 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
     [ASPEED_DEV_GPIO]      =  0x14C0B000,
 };
 
-#define AST2700_MAX_IRQ 288
+#define AST2700_MAX_IRQ 256
 
 /* Shared Peripheral Interrupt values below are offset by -32 from datasheet */
 static const int aspeed_soc_ast2700_irqmap[] = {
@@ -403,7 +403,7 @@ static bool aspeed_soc_ast2700_gic_realize(DeviceState *dev, Error **errp)
     gicdev = DEVICE(&a->gic);
     qdev_prop_set_uint32(gicdev, "revision", 3);
     qdev_prop_set_uint32(gicdev, "num-cpu", sc->num_cpus);
-    qdev_prop_set_uint32(gicdev, "num-irq", AST2700_MAX_IRQ);
+    qdev_prop_set_uint32(gicdev, "num-irq", AST2700_MAX_IRQ + GIC_INTERNAL);
 
     redist_region_count = qlist_new();
     qlist_append_int(redist_region_count, sc->num_cpus);
@@ -417,8 +417,7 @@ static bool aspeed_soc_ast2700_gic_realize(DeviceState *dev, Error **errp)
 
     for (i = 0; i < sc->num_cpus; i++) {
         DeviceState *cpudev = DEVICE(&a->cpu[i]);
-        int NUM_IRQS = 256;
-        int intidbase = NUM_IRQS + i * GIC_INTERNAL;
+        int intidbase = AST2700_MAX_IRQ + i * GIC_INTERNAL;
 
         const int timer_irq[] = {
             [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
-- 
2.34.1
Re: [PATCH 2/2] hw/arm/aspeed_ast27x0: Avoid hardcoded '256' in IRQ calculation
Posted by Philippe Mathieu-Daudé 2 weeks, 5 days ago
On 1/11/24 13:11, Peter Maydell wrote:
> When calculating the index into the GIC's GPIO array for per-CPU
> interrupts, we have to start with the number of SPIs.  The code
> currently hard-codes this to 'NUM_IRQS = 256'.  However the number of
> SPIs is set separately and implicitly by the value of
> AST2700_MAX_IRQ, which is the number of SPIs plus 32 (since it is
> what we set the GIC num-irq property to).
> 
> Define AST2700_MAX_IRQ as the total number of SPIs; this brings
> AST2700 into line with AST2600, which defines AST2600_MAX_IRQ as the
> number of SPIs not including the 32 internal interrupts.  We can then
> use AST2700_MAX_IRQ instead of the hardcoded 256.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/aspeed_ast27x0.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Re: [PATCH 2/2] hw/arm/aspeed_ast27x0: Avoid hardcoded '256' in IRQ calculation
Posted by Pierrick Bouvier 3 weeks, 1 day ago

On 11/1/24 09:11, Peter Maydell wrote:
> When calculating the index into the GIC's GPIO array for per-CPU
> interrupts, we have to start with the number of SPIs.  The code
> currently hard-codes this to 'NUM_IRQS = 256'.  However the number of
> SPIs is set separately and implicitly by the value of
> AST2700_MAX_IRQ, which is the number of SPIs plus 32 (since it is
> what we set the GIC num-irq property to).
> 
> Define AST2700_MAX_IRQ as the total number of SPIs; this brings
> AST2700 into line with AST2600, which defines AST2600_MAX_IRQ as the
> number of SPIs not including the 32 internal interrupts.  We can then
> use AST2700_MAX_IRQ instead of the hardcoded 256.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/aspeed_ast27x0.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> index 5638a7a5781..7b246440952 100644
> --- a/hw/arm/aspeed_ast27x0.c
> +++ b/hw/arm/aspeed_ast27x0.c
> @@ -66,7 +66,7 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
>       [ASPEED_DEV_GPIO]      =  0x14C0B000,
>   };
>   
> -#define AST2700_MAX_IRQ 288
> +#define AST2700_MAX_IRQ 256
>   
>   /* Shared Peripheral Interrupt values below are offset by -32 from datasheet */
>   static const int aspeed_soc_ast2700_irqmap[] = {
> @@ -403,7 +403,7 @@ static bool aspeed_soc_ast2700_gic_realize(DeviceState *dev, Error **errp)
>       gicdev = DEVICE(&a->gic);
>       qdev_prop_set_uint32(gicdev, "revision", 3);
>       qdev_prop_set_uint32(gicdev, "num-cpu", sc->num_cpus);
> -    qdev_prop_set_uint32(gicdev, "num-irq", AST2700_MAX_IRQ);
> +    qdev_prop_set_uint32(gicdev, "num-irq", AST2700_MAX_IRQ + GIC_INTERNAL);
>   
>       redist_region_count = qlist_new();
>       qlist_append_int(redist_region_count, sc->num_cpus);
> @@ -417,8 +417,7 @@ static bool aspeed_soc_ast2700_gic_realize(DeviceState *dev, Error **errp)
>   
>       for (i = 0; i < sc->num_cpus; i++) {
>           DeviceState *cpudev = DEVICE(&a->cpu[i]);
> -        int NUM_IRQS = 256;
> -        int intidbase = NUM_IRQS + i * GIC_INTERNAL;
> +        int intidbase = AST2700_MAX_IRQ + i * GIC_INTERNAL;
>   
>           const int timer_irq[] = {
>               [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>