Re: [PATCH v2 08/11] aspeed/soc: introduce a new API to get the INTC orgate information

["Cédric Le Goater" <clg@kaod.org>]

Jamin,

On 8/8/24 04: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();
> +}
> +
>   static uint64_t aspeed_ram_capacity_read(void *opaque, hwaddr addr,
>                                                       unsigned int size)
>   {

Here is a proposal, instead please introduce a routine returning
a qemu_irq like sc->get_irq() does :

   static qemu_irq aspeed_soc_ast2700_get_irq_index(AspeedSoCState *s, int dev,
                                                    int index)
   {
       Aspeed27x0SoCState *a = ASPEED27X0_SOC(s);
       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);
               return qdev_get_gpio_in(DEVICE(&a->intc.orgates[i]),
                    aspeed_soc_ast2700_gic_intcmap[i].ptr[dev] + index);
           }
       }

       /*
        * Invalid orgate index, device irq should be 128 to 136.
        */
       g_assert_not_reached();
   }

and in the next patch, replace

         irq = qdev_get_gpio_in(DEVICE(&a->intc.orgates[orgate_info.index]),
                                orgate_info.int_num + i);
with

         irq = aspeed_soc_ast2700_get_irq_index(s, ASPEED_DEV_I2C, i);

I think this should be cleaner.

Thanks,

C.