Re: [PATCH v1 12/15] aspeed/soc: introduce a new API to get the INTC orgate information

[Cédric Le Goater]

On 7/18/24 08:49, Jamin Lin wrote:
> Currently, users can set the intc mapping table with
> enumerated device id and device irq to get the INTC orgate
> input pins. However, some devices use the continuous bits number in the
> same orgate. To reduce the enumerated device id definition,
> create a new API to get the INTC orgate index and source bit number
> if users only provide the start bus number of device.
>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/arm/aspeed_ast27x0.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>
> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> index 4257b5e8af..0bbd66110b 100644
> --- a/hw/arm/aspeed_ast27x0.c
> +++ b/hw/arm/aspeed_ast27x0.c
> @@ -164,6 +164,11 @@ struct gic_intc_irq_info {
>       const int *ptr;
>   };
>   
> +struct gic_intc_orgate_info {
> +    int index;
> +    int int_num;
> +};
> +
>   static const struct gic_intc_irq_info aspeed_soc_ast2700_gic_intcmap[] = {
>       {128,  aspeed_soc_ast2700_gic128_intcmap},
>       {129,  NULL},
> @@ -193,6 +198,27 @@ static qemu_irq aspeed_soc_ast2700_get_irq(AspeedSoCState 
> *s, int dev)
>       return qdev_get_gpio_in(DEVICE(&a->gic), sc->irqmap[dev]);
>   }
>   
> +static void aspeed_soc_ast2700_get_intc_orgate(AspeedSoCState *s, int dev,
> +    struct gic_intc_orgate_info *orgate_info)
> +{
> +    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast2700_gic_intcmap); i++) {
> +        if (sc->irqmap[dev] == aspeed_soc_ast2700_gic_intcmap[i].irq) {
> +            assert(aspeed_soc_ast2700_gic_intcmap[i].ptr);
> +            orgate_info->index = i;
> +            orgate_info->int_num = aspeed_soc_ast2700_gic_intcmap[i].ptr[dev];
> +            return;
> +        }
> +    }
> +
> +    /*
> +     * Invalid orgate index, device irq should be 128 to 136.
> +     */
> +    g_assert_not_reached();
> +}


This looks redundant. Couldn't we extend aspeed_soc_ast2700_get_irq() with an
index parameter instead ?

Thanks,

C.




>   static uint64_t aspeed_ram_capacity_read(void *opaque, hwaddr addr,
>                                                       unsigned int size)
>   {