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);
}
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.
