| Message ID | 20240808024916.1262715-9-jamin_lin@aspeedtech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | support I2C for AST2700 | expand |
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.
Hi Cedric, > Subject: Re: [PATCH v2 08/11] aspeed/soc: introduce a new API to get the INTC > orgate information > > 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 > Thanks for suggestion and review. I appreciate your very very good advice and proposal. Will send v3 patch, today. Jamin > 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. >
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) {
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(+)