Sorry for typo > Subject: RE: [PATCH v1 12/15] aspeed/soc: introduce a new API to get the INTC > orgate information > > Hi Cedric, > > > Subject: Re: [PATCH v1 12/15] aspeed/soc: introduce a new API to get > > the INTC orgate information > > > > 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_...@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 for review and suggestion. > > According to the current design of aspeed_soc_get_irq, the function type of > get_irq callback function should be XXX(AspeedSoCState *s, int dev) > > qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev) { > return ASPEED_SOC_GET_CLASS(s)->get_irq(s, dev); } > > struct AspeedSoCClass { > qemu_irq (*get_irq)(AspeedSoCState *s, int dev); } > > If we want to add a new index parameter in aspeed_soc_ast2700_get_irq, I > will change as following. > 1. > static qemu_irq aspeed_soc_ast2700_get_irq(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); > } > } > > return qdev_get_gpio_in(DEVICE(&a->gic), sc->irqmap[dev]); } > > 2. introduce a new get_irq_index function pointer and the function type of > this > callback function as following. > struct AspeedSoCClass { > qemu_irq_index (*get_irq)(AspeedSoCState *s, int dev, int index); } > qemu_irq (*get_irq_index)(AspeedSoCState *s, int dev, int index);
3. Add aspeed_soc_get_irq_index qemu_irq aspeed_soc_get_irq_index(AspeedSoCState *s, int dev, int index) { return ASPEED_SOC_GET_CLASS(s)->get_irq_index(s, dev, index); } 4. I need to modify this function, too bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp) { sysbus_connect_irq(SYS_BUS_DEVICE(smm), 0, aspeed_soc_get_irq(s, uart)); } Thanks-Jamin > Do you have any concern or could you please give me any suggestion? > Thanks-Jamin > > > > Thanks, > > > > C. > > > > > > > > > > > static uint64_t aspeed_ram_capacity_read(void *opaque, hwaddr addr, > > > > unsigned > > int size) > > > { ************* Email Confidentiality Notice ******************** 免責聲明: 本信件(或其附件)可能包含機密資訊,並受法律保護。如 台端非指定之收件者,請以電子郵件通知本電子郵件之發送者, 並請立即刪除本電子郵件及其附件和銷毀所有複印件。謝謝您的合作! DISCLAIMER: This message (and any attachments) may contain legally privileged and/or other confidential information. If you have received it in error, please notify the sender by reply e-mail and immediately delete the e-mail and any attachments without copying or disclosing the contents. Thank you.