* [PATCH v2 1/2] aspeed: introduce a new UART0 device name [not found] <20240207200220.453244-1-jamin_lin@aspeedtech.com> @ 2024-02-07 20:02 ` Jamin Lin via 2024-02-09 8:27 ` Cédric Le Goater 2024-02-07 20:02 ` [PATCH v2 2/2] aspeed: fix hardcode boot address 0 Jamin Lin via 1 sibling, 1 reply; 9+ messages in thread From: Jamin Lin via @ 2024-02-07 20:02 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: troy_lee, jamin_lin The Aspeed datasheet refers to the UART controllers as UART1 - UART13 for the ast10x0, ast2600, ast2500 and ast2400 SoCs and the Aspeed ast2700 introduces an UART0 and the UART controllers as UART0 - UART12. To keep the naming in the QEMU models in sync with the datasheet, let's introduce a new UART0 device name and do the required adjustements, etc ... Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> --- hw/arm/aspeed.c | 13 ++++++++----- hw/arm/aspeed_ast10x0.c | 1 + hw/arm/aspeed_ast2400.c | 2 ++ hw/arm/aspeed_ast2600.c | 1 + hw/arm/aspeed_soc_common.c | 14 +++++++++----- include/hw/arm/aspeed_soc.h | 2 ++ 6 files changed, 23 insertions(+), 10 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 09b1e823ba..06d863958b 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -342,7 +342,7 @@ static void connect_serial_hds_to_uarts(AspeedMachineState *bmc) int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : amc->uart_default; aspeed_soc_uart_set_chr(s, uart_chosen, serial_hd(0)); - for (int i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) { + for (int i = 0, uart = sc->uarts_base; i < sc->uarts_num; i++, uart++) { if (uart == uart_chosen) { continue; } @@ -1094,7 +1094,7 @@ static char *aspeed_get_bmc_console(Object *obj, Error **errp) AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc); int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : amc->uart_default; - return g_strdup_printf("uart%d", uart_chosen - ASPEED_DEV_UART1 + 1); + return g_strdup_printf("uart%d", uart_chosen - ASPEED_DEV_UART0); } static void aspeed_set_bmc_console(Object *obj, const char *value, Error **errp) @@ -1103,6 +1103,8 @@ static void aspeed_set_bmc_console(Object *obj, const char *value, Error **errp) AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc); AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name)); int val; + int start = sc->uarts_base - ASPEED_DEV_UART0; + int end = start + sc->uarts_num; if (sscanf(value, "uart%u", &val) != 1) { error_setg(errp, "Bad value for \"uart\" property"); @@ -1110,11 +1112,12 @@ static void aspeed_set_bmc_console(Object *obj, const char *value, Error **errp) } /* The number of UART depends on the SoC */ - if (val < 1 || val > sc->uarts_num) { - error_setg(errp, "\"uart\" should be in range [1 - %d]", sc->uarts_num); + if (val < start || val >= end) { + error_setg(errp, "\"uart\" should be in range [%d - %d]", + start, end - 1); return; } - bmc->uart_chosen = ASPEED_DEV_UART1 + val - 1; + bmc->uart_chosen = val + ASPEED_DEV_UART0; } static void aspeed_machine_class_props_init(ObjectClass *oc) diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c index c3b5116a6a..2634e0f654 100644 --- a/hw/arm/aspeed_ast10x0.c +++ b/hw/arm/aspeed_ast10x0.c @@ -436,6 +436,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data) sc->wdts_num = 4; sc->macs_num = 1; sc->uarts_num = 13; + sc->uarts_base = ASPEED_DEV_UART1; sc->irqmap = aspeed_soc_ast1030_irqmap; sc->memmap = aspeed_soc_ast1030_memmap; sc->num_cpus = 1; diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c index 8829561bb6..95da85fee0 100644 --- a/hw/arm/aspeed_ast2400.c +++ b/hw/arm/aspeed_ast2400.c @@ -523,6 +523,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, void *data) sc->wdts_num = 2; sc->macs_num = 2; sc->uarts_num = 5; + sc->uarts_base = ASPEED_DEV_UART1; sc->irqmap = aspeed_soc_ast2400_irqmap; sc->memmap = aspeed_soc_ast2400_memmap; sc->num_cpus = 1; @@ -551,6 +552,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, void *data) sc->wdts_num = 3; sc->macs_num = 2; sc->uarts_num = 5; + sc->uarts_base = ASPEED_DEV_UART1; sc->irqmap = aspeed_soc_ast2500_irqmap; sc->memmap = aspeed_soc_ast2500_memmap; sc->num_cpus = 1; diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index 4ee32ea99d..f74561ecdc 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -666,6 +666,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data) sc->wdts_num = 4; sc->macs_num = 4; sc->uarts_num = 13; + sc->uarts_base = ASPEED_DEV_UART1; sc->irqmap = aspeed_soc_ast2600_irqmap; sc->memmap = aspeed_soc_ast2600_memmap; sc->num_cpus = 2; diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c index 123a0c432c..54c875c8d5 100644 --- a/hw/arm/aspeed_soc_common.c +++ b/hw/arm/aspeed_soc_common.c @@ -36,7 +36,7 @@ bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp) AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); SerialMM *smm; - for (int i = 0, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) { + for (int i = 0, uart = sc->uarts_base; i < sc->uarts_num; i++, uart++) { smm = &s->uart[i]; /* Chardev property is set by the machine. */ @@ -58,10 +58,14 @@ bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp) void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev *chr) { AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); - int i = dev - ASPEED_DEV_UART1; - - g_assert(0 <= i && i < ARRAY_SIZE(s->uart) && i < sc->uarts_num); - qdev_prop_set_chr(DEVICE(&s->uart[i]), "chardev", chr); + int uart_num = dev - ASPEED_DEV_UART0; + int start = sc->uarts_base - ASPEED_DEV_UART0; + int end = start + sc->uarts_num; + int index = uart_num - start; + + g_assert(uart_num >= start && uart_num < end); + g_assert(index < ARRAY_SIZE(s->uart)); + qdev_prop_set_chr(DEVICE(&s->uart[index]), "chardev", chr); } /* diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index 9d0af84a8c..5ab0902da0 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -140,6 +140,7 @@ struct AspeedSoCClass { int wdts_num; int macs_num; int uarts_num; + int uarts_base; const int *irqmap; const hwaddr *memmap; uint32_t num_cpus; @@ -151,6 +152,7 @@ const char *aspeed_soc_cpu_type(AspeedSoCClass *sc); enum { ASPEED_DEV_SPI_BOOT, ASPEED_DEV_IOMEM, + ASPEED_DEV_UART0, ASPEED_DEV_UART1, ASPEED_DEV_UART2, ASPEED_DEV_UART3, -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] aspeed: introduce a new UART0 device name 2024-02-07 20:02 ` [PATCH v2 1/2] aspeed: introduce a new UART0 device name Jamin Lin via @ 2024-02-09 8:27 ` Cédric Le Goater 2024-02-15 1:31 ` Jamin Lin 0 siblings, 1 reply; 9+ messages in thread From: Cédric Le Goater @ 2024-02-09 8:27 UTC (permalink / raw) To: Jamin Lin, Cédric Le Goater, Peter Maydell, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: troy_lee Hello Jamin, On 2/7/24 21:02, Jamin Lin via wrote: > The Aspeed datasheet refers to the UART controllers > as UART1 - UART13 for the ast10x0, ast2600, ast2500 > and ast2400 SoCs and the Aspeed ast2700 introduces an UART0 > and the UART controllers as UART0 - UART12. > > To keep the naming in the QEMU models > in sync with the datasheet, let's introduce a new UART0 device name > and do the required adjustements, etc ... Please drop the etc... > > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> > --- > hw/arm/aspeed.c | 13 ++++++++----- > hw/arm/aspeed_ast10x0.c | 1 + > hw/arm/aspeed_ast2400.c | 2 ++ > hw/arm/aspeed_ast2600.c | 1 + > hw/arm/aspeed_soc_common.c | 14 +++++++++----- > include/hw/arm/aspeed_soc.h | 2 ++ > 6 files changed, 23 insertions(+), 10 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 09b1e823ba..06d863958b 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -342,7 +342,7 @@ static void connect_serial_hds_to_uarts(AspeedMachineState *bmc) > int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : amc->uart_default; > > aspeed_soc_uart_set_chr(s, uart_chosen, serial_hd(0)); > - for (int i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) { > + for (int i = 0, uart = sc->uarts_base; i < sc->uarts_num; i++, uart++) { > if (uart == uart_chosen) { > continue; > } > @@ -1094,7 +1094,7 @@ static char *aspeed_get_bmc_console(Object *obj, Error **errp) > AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc); > int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : amc->uart_default; > > - return g_strdup_printf("uart%d", uart_chosen - ASPEED_DEV_UART1 + 1); > + return g_strdup_printf("uart%d", uart_chosen - ASPEED_DEV_UART0); > } > > static void aspeed_set_bmc_console(Object *obj, const char *value, Error **errp) > @@ -1103,6 +1103,8 @@ static void aspeed_set_bmc_console(Object *obj, const char *value, Error **errp) > AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc); > AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name)); > int val; > + int start = sc->uarts_base - ASPEED_DEV_UART0; > + int end = start + sc->uarts_num; To help the reader, I would introduce these helpers at the end of aspeed_soc.h : static inline int aspeed_uart_index(int uart_dev) { return uart_dev - ASPEED_DEV_UART0; } static inline int aspeed_uart_first(AspeedSoCClass *sc) { return aspeed_uart_index(sc->uarts_base); } static inline int aspeed_uart_last(AspeedSoCClass *sc) { return aspeed_uart_first(sc) + sc->uarts_num - 1; } > if (sscanf(value, "uart%u", &val) != 1) { > error_setg(errp, "Bad value for \"uart\" property"); > @@ -1110,11 +1112,12 @@ static void aspeed_set_bmc_console(Object *obj, const char *value, Error **errp) > } > > /* The number of UART depends on the SoC */ > - if (val < 1 || val > sc->uarts_num) { > - error_setg(errp, "\"uart\" should be in range [1 - %d]", sc->uarts_num); > + if (val < start || val >= end) { > + error_setg(errp, "\"uart\" should be in range [%d - %d]", > + start, end - 1); > return; > } > - bmc->uart_chosen = ASPEED_DEV_UART1 + val - 1; > + bmc->uart_chosen = val + ASPEED_DEV_UART0; > } > > static void aspeed_machine_class_props_init(ObjectClass *oc) > diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c > index c3b5116a6a..2634e0f654 100644 > --- a/hw/arm/aspeed_ast10x0.c > +++ b/hw/arm/aspeed_ast10x0.c > @@ -436,6 +436,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data) > sc->wdts_num = 4; > sc->macs_num = 1; > sc->uarts_num = 13; > + sc->uarts_base = ASPEED_DEV_UART1; > sc->irqmap = aspeed_soc_ast1030_irqmap; > sc->memmap = aspeed_soc_ast1030_memmap; > sc->num_cpus = 1; > diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c > index 8829561bb6..95da85fee0 100644 > --- a/hw/arm/aspeed_ast2400.c > +++ b/hw/arm/aspeed_ast2400.c > @@ -523,6 +523,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, void *data) > sc->wdts_num = 2; > sc->macs_num = 2; > sc->uarts_num = 5; > + sc->uarts_base = ASPEED_DEV_UART1; > sc->irqmap = aspeed_soc_ast2400_irqmap; > sc->memmap = aspeed_soc_ast2400_memmap; > sc->num_cpus = 1; > @@ -551,6 +552,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, void *data) > sc->wdts_num = 3; > sc->macs_num = 2; > sc->uarts_num = 5; > + sc->uarts_base = ASPEED_DEV_UART1; > sc->irqmap = aspeed_soc_ast2500_irqmap; > sc->memmap = aspeed_soc_ast2500_memmap; > sc->num_cpus = 1; > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c > index 4ee32ea99d..f74561ecdc 100644 > --- a/hw/arm/aspeed_ast2600.c > +++ b/hw/arm/aspeed_ast2600.c > @@ -666,6 +666,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data) > sc->wdts_num = 4; > sc->macs_num = 4; > sc->uarts_num = 13; > + sc->uarts_base = ASPEED_DEV_UART1; > sc->irqmap = aspeed_soc_ast2600_irqmap; > sc->memmap = aspeed_soc_ast2600_memmap; > sc->num_cpus = 2; > diff --git a/hw/arm/aspeed_soc_common.c b/hw/arm/aspeed_soc_common.c > index 123a0c432c..54c875c8d5 100644 > --- a/hw/arm/aspeed_soc_common.c > +++ b/hw/arm/aspeed_soc_common.c > @@ -36,7 +36,7 @@ bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp) > AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); > SerialMM *smm; > > - for (int i = 0, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) { > + for (int i = 0, uart = sc->uarts_base; i < sc->uarts_num; i++, uart++) { > smm = &s->uart[i]; > > /* Chardev property is set by the machine. */ > @@ -58,10 +58,14 @@ bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp) > void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev *chr) > { > AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); > - int i = dev - ASPEED_DEV_UART1; > - > - g_assert(0 <= i && i < ARRAY_SIZE(s->uart) && i < sc->uarts_num); > - qdev_prop_set_chr(DEVICE(&s->uart[i]), "chardev", chr); > + int uart_num = dev - ASPEED_DEV_UART0; > + int start = sc->uarts_base - ASPEED_DEV_UART0; > + int end = start + sc->uarts_num; > + int index = uart_num - start; > + > + g_assert(uart_num >= start && uart_num < end); I don't think this assert is necessary. Only the second one is. If you want to check the range and return an error, please add an Error **errp argument and have the callers pass &error_fatal. It would have the same effect. Thanks, C. > + g_assert(index < ARRAY_SIZE(s->uart)); > + qdev_prop_set_chr(DEVICE(&s->uart[index]), "chardev", chr); > } > > /* > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h > index 9d0af84a8c..5ab0902da0 100644 > --- a/include/hw/arm/aspeed_soc.h > +++ b/include/hw/arm/aspeed_soc.h > @@ -140,6 +140,7 @@ struct AspeedSoCClass { > int wdts_num; > int macs_num; > int uarts_num; > + int uarts_base; > const int *irqmap; > const hwaddr *memmap; > uint32_t num_cpus; > @@ -151,6 +152,7 @@ const char *aspeed_soc_cpu_type(AspeedSoCClass *sc); > enum { > ASPEED_DEV_SPI_BOOT, > ASPEED_DEV_IOMEM, > + ASPEED_DEV_UART0, > ASPEED_DEV_UART1, > ASPEED_DEV_UART2, > ASPEED_DEV_UART3, ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 1/2] aspeed: introduce a new UART0 device name 2024-02-09 8:27 ` Cédric Le Goater @ 2024-02-15 1:31 ` Jamin Lin 0 siblings, 0 replies; 9+ messages in thread From: Jamin Lin @ 2024-02-15 1:31 UTC (permalink / raw) To: Cédric Le Goater, Cédric Le Goater, Peter Maydell, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Troy Lee > -----Original Message----- > From: Cédric Le Goater <clegoate@redhat.com> > Sent: Friday, February 9, 2024 4:27 PM > To: Jamin Lin <jamin_lin@aspeedtech.com>; Cédric Le Goater <clg@kaod.org>; > Peter Maydell <peter.maydell@linaro.org>; Andrew Jeffery > <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open > list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here > <qemu-devel@nongnu.org> > Cc: Troy Lee <troy_lee@aspeedtech.com> > Subject: Re: [PATCH v2 1/2] aspeed: introduce a new UART0 device name > > Hello Jamin, > Thanks for review and sorry reply you late due to my Chinese new year holiday. > On 2/7/24 21:02, Jamin Lin via wrote: > > The Aspeed datasheet refers to the UART controllers as UART1 - UART13 > > for the ast10x0, ast2600, ast2500 and ast2400 SoCs and the Aspeed > > ast2700 introduces an UART0 and the UART controllers as UART0 - > > UART12. > > > > To keep the naming in the QEMU models > > in sync with the datasheet, let's introduce a new UART0 device name > > and do the required adjustements, etc ... > > Please drop the etc... Will fix > > > > > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> > > --- > > hw/arm/aspeed.c | 13 ++++++++----- > > hw/arm/aspeed_ast10x0.c | 1 + > > hw/arm/aspeed_ast2400.c | 2 ++ > > hw/arm/aspeed_ast2600.c | 1 + > > hw/arm/aspeed_soc_common.c | 14 +++++++++----- > > include/hw/arm/aspeed_soc.h | 2 ++ > > 6 files changed, 23 insertions(+), 10 deletions(-) > > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index > > 09b1e823ba..06d863958b 100644 > > --- a/hw/arm/aspeed.c > > +++ b/hw/arm/aspeed.c > > @@ -342,7 +342,7 @@ static void > connect_serial_hds_to_uarts(AspeedMachineState *bmc) > > int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : > > amc->uart_default; > > > > aspeed_soc_uart_set_chr(s, uart_chosen, serial_hd(0)); > > - for (int i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, > uart++) { > > + for (int i = 0, uart = sc->uarts_base; i < sc->uarts_num; i++, > > + uart++) { > > if (uart == uart_chosen) { > > continue; > > } > > @@ -1094,7 +1094,7 @@ static char *aspeed_get_bmc_console(Object *obj, > Error **errp) > > AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc); > > int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : > > amc->uart_default; > > > > - return g_strdup_printf("uart%d", uart_chosen - ASPEED_DEV_UART1 + > 1); > > + return g_strdup_printf("uart%d", uart_chosen - ASPEED_DEV_UART0); > > } > > > > static void aspeed_set_bmc_console(Object *obj, const char *value, > > Error **errp) @@ -1103,6 +1103,8 @@ static void > aspeed_set_bmc_console(Object *obj, const char *value, Error **errp) > > AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc); > > AspeedSoCClass *sc = > ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name)); > > int val; > > + int start = sc->uarts_base - ASPEED_DEV_UART0; > > + int end = start + sc->uarts_num; > > > To help the reader, I would introduce these helpers at the end of aspeed_soc.h : > > static inline int aspeed_uart_index(int uart_dev) > { > return uart_dev - ASPEED_DEV_UART0; > } > > static inline int aspeed_uart_first(AspeedSoCClass *sc) > { > return aspeed_uart_index(sc->uarts_base); > } > > static inline int aspeed_uart_last(AspeedSoCClass *sc) > { > return aspeed_uart_first(sc) + sc->uarts_num - 1; > } > Will add > > > if (sscanf(value, "uart%u", &val) != 1) { > > error_setg(errp, "Bad value for \"uart\" property"); @@ > > -1110,11 +1112,12 @@ static void aspeed_set_bmc_console(Object *obj, > const char *value, Error **errp) > > } > > > > /* The number of UART depends on the SoC */ > > - if (val < 1 || val > sc->uarts_num) { > > - error_setg(errp, "\"uart\" should be in range [1 - %d]", > sc->uarts_num); > > + if (val < start || val >= end) { > > + error_setg(errp, "\"uart\" should be in range [%d - %d]", > > + start, end - 1); > > return; > > } > > - bmc->uart_chosen = ASPEED_DEV_UART1 + val - 1; > > + bmc->uart_chosen = val + ASPEED_DEV_UART0; > > } > > > > static void aspeed_machine_class_props_init(ObjectClass *oc) diff > > --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c index > > c3b5116a6a..2634e0f654 100644 > > --- a/hw/arm/aspeed_ast10x0.c > > +++ b/hw/arm/aspeed_ast10x0.c > > @@ -436,6 +436,7 @@ static void > aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data) > > sc->wdts_num = 4; > > sc->macs_num = 1; > > sc->uarts_num = 13; > > + sc->uarts_base = ASPEED_DEV_UART1; > > sc->irqmap = aspeed_soc_ast1030_irqmap; > > sc->memmap = aspeed_soc_ast1030_memmap; > > sc->num_cpus = 1; > > diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c index > > 8829561bb6..95da85fee0 100644 > > --- a/hw/arm/aspeed_ast2400.c > > +++ b/hw/arm/aspeed_ast2400.c > > @@ -523,6 +523,7 @@ static void > aspeed_soc_ast2400_class_init(ObjectClass *oc, void *data) > > sc->wdts_num = 2; > > sc->macs_num = 2; > > sc->uarts_num = 5; > > + sc->uarts_base = ASPEED_DEV_UART1; > > sc->irqmap = aspeed_soc_ast2400_irqmap; > > sc->memmap = aspeed_soc_ast2400_memmap; > > sc->num_cpus = 1; > > @@ -551,6 +552,7 @@ static void > aspeed_soc_ast2500_class_init(ObjectClass *oc, void *data) > > sc->wdts_num = 3; > > sc->macs_num = 2; > > sc->uarts_num = 5; > > + sc->uarts_base = ASPEED_DEV_UART1; > > sc->irqmap = aspeed_soc_ast2500_irqmap; > > sc->memmap = aspeed_soc_ast2500_memmap; > > sc->num_cpus = 1; > > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index > > 4ee32ea99d..f74561ecdc 100644 > > --- a/hw/arm/aspeed_ast2600.c > > +++ b/hw/arm/aspeed_ast2600.c > > @@ -666,6 +666,7 @@ static void > aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data) > > sc->wdts_num = 4; > > sc->macs_num = 4; > > sc->uarts_num = 13; > > + sc->uarts_base = ASPEED_DEV_UART1; > > sc->irqmap = aspeed_soc_ast2600_irqmap; > > sc->memmap = aspeed_soc_ast2600_memmap; > > sc->num_cpus = 2; > > diff --git a/hw/arm/aspeed_soc_common.c > b/hw/arm/aspeed_soc_common.c > > index 123a0c432c..54c875c8d5 100644 > > --- a/hw/arm/aspeed_soc_common.c > > +++ b/hw/arm/aspeed_soc_common.c > > @@ -36,7 +36,7 @@ bool aspeed_soc_uart_realize(AspeedSoCState *s, > Error **errp) > > AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); > > SerialMM *smm; > > > > - for (int i = 0, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, > uart++) { > > + for (int i = 0, uart = sc->uarts_base; i < sc->uarts_num; i++, > > + uart++) { > > smm = &s->uart[i]; > > > > /* Chardev property is set by the machine. */ @@ -58,10 > > +58,14 @@ bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp) > > void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev *chr) > > { > > AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); > > - int i = dev - ASPEED_DEV_UART1; > > - > > - g_assert(0 <= i && i < ARRAY_SIZE(s->uart) && i < sc->uarts_num); > > - qdev_prop_set_chr(DEVICE(&s->uart[i]), "chardev", chr); > > + int uart_num = dev - ASPEED_DEV_UART0; > > + int start = sc->uarts_base - ASPEED_DEV_UART0; > > + int end = start + sc->uarts_num; > > + int index = uart_num - start; > > + > > + g_assert(uart_num >= start && uart_num < end); > > I don't think this assert is necessary. Only the second one is. > > If you want to check the range and return an error, please add an Error **errp > argument and have the callers pass &error_fatal. It would have the same > effect. > Will fix. > > Thanks, > > C. > > > > + g_assert(index < ARRAY_SIZE(s->uart)); > > + qdev_prop_set_chr(DEVICE(&s->uart[index]), "chardev", chr); > > } > > > > /* > > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h > > index 9d0af84a8c..5ab0902da0 100644 > > --- a/include/hw/arm/aspeed_soc.h > > +++ b/include/hw/arm/aspeed_soc.h > > @@ -140,6 +140,7 @@ struct AspeedSoCClass { > > int wdts_num; > > int macs_num; > > int uarts_num; > > + int uarts_base; > > const int *irqmap; > > const hwaddr *memmap; > > uint32_t num_cpus; > > @@ -151,6 +152,7 @@ const char *aspeed_soc_cpu_type(AspeedSoCClass > *sc); > > enum { > > ASPEED_DEV_SPI_BOOT, > > ASPEED_DEV_IOMEM, > > + ASPEED_DEV_UART0, > > ASPEED_DEV_UART1, > > ASPEED_DEV_UART2, > > ASPEED_DEV_UART3, ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] aspeed: fix hardcode boot address 0 [not found] <20240207200220.453244-1-jamin_lin@aspeedtech.com> 2024-02-07 20:02 ` [PATCH v2 1/2] aspeed: introduce a new UART0 device name Jamin Lin via @ 2024-02-07 20:02 ` Jamin Lin via 1 sibling, 0 replies; 9+ messages in thread From: Jamin Lin via @ 2024-02-07 20:02 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: troy_lee, jamin_lin In the previous design of ASPEED SOCs QEMU model, it set the boot address at "0" which was the hardcode setting for ast10x0, ast2600, ast2500 and ast2400. According to the design of ast2700, it has bootmcu which is used for executing SPL and initialize DRAM, then, CPUs(cortex-a35) execute u-boot, kernel and rofs. QEMU will only support CPU(cortex-a35) parts and the boot address is "0x4 00000000" for ast2700. Therefore, fixed hardcode boot address 0. Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> --- hw/arm/aspeed.c | 4 +++- hw/arm/aspeed_ast2400.c | 4 ++-- hw/arm/aspeed_ast2600.c | 2 +- include/hw/arm/aspeed_soc.h | 2 -- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 06d863958b..39758557be 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -289,12 +289,14 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk, uint64_t rom_size) { AspeedSoCState *soc = bmc->soc; + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc); memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size, &error_abort); memory_region_add_subregion_overlap(&soc->spi_boot_container, 0, &bmc->boot_rom, 1); - write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, &error_abort); + write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], + rom_size, &error_abort); } void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c index 95da85fee0..d125886207 100644 --- a/hw/arm/aspeed_ast2400.c +++ b/hw/arm/aspeed_ast2400.c @@ -26,7 +26,7 @@ #define ASPEED_SOC_IOMEM_SIZE 0x00200000 static const hwaddr aspeed_soc_ast2400_memmap[] = { - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, + [ASPEED_DEV_SPI_BOOT] = 0x00000000, [ASPEED_DEV_IOMEM] = 0x1E600000, [ASPEED_DEV_FMC] = 0x1E620000, [ASPEED_DEV_SPI1] = 0x1E630000, @@ -61,7 +61,7 @@ static const hwaddr aspeed_soc_ast2400_memmap[] = { }; static const hwaddr aspeed_soc_ast2500_memmap[] = { - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, + [ASPEED_DEV_SPI_BOOT] = 0x00000000, [ASPEED_DEV_IOMEM] = 0x1E600000, [ASPEED_DEV_FMC] = 0x1E620000, [ASPEED_DEV_SPI1] = 0x1E630000, diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index f74561ecdc..174be53770 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -22,7 +22,7 @@ #define ASPEED_SOC_DPMCU_SIZE 0x00040000 static const hwaddr aspeed_soc_ast2600_memmap[] = { - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, + [ASPEED_DEV_SPI_BOOT] = 0x00000000, [ASPEED_DEV_SRAM] = 0x10000000, [ASPEED_DEV_DPMCU] = 0x18000000, /* 0x16000000 0x17FFFFFF : AHB BUS do LPC Bus bridge */ diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index 5ab0902da0..bf43ad8351 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -224,8 +224,6 @@ enum { ASPEED_DEV_FSI2, }; -#define ASPEED_SOC_SPI_BOOT_ADDR 0x0 - qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev); bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp); void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev *chr); -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <20240207195224.452987-1-jamin_lin@aspeedtech.com>]
* [PATCH v2 2/2] aspeed: fix hardcode boot address 0 [not found] <20240207195224.452987-1-jamin_lin@aspeedtech.com> @ 2024-02-07 19:52 ` Jamin Lin via 2024-02-07 20:28 ` Philippe Mathieu-Daudé 2024-02-09 8:31 ` Cédric Le Goater 0 siblings, 2 replies; 9+ messages in thread From: Jamin Lin via @ 2024-02-07 19:52 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: troy_lee, jamin_lin In the previous design of ASPEED SOCs QEMU model, it set the boot address at "0" which was the hardcode setting for ast10x0, ast2600, ast2500 and ast2400. According to the design of ast2700, it has bootmcu which is used for executing SPL and initialize DRAM, then, CPUs(cortex-a35) execute u-boot, kernel and rofs. QEMU will only support CPU(cortex-a35) parts and the boot address is "0x4 00000000" for ast2700. Therefore, fixed hardcode boot address 0. Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> --- hw/arm/aspeed.c | 4 +++- hw/arm/aspeed_ast2400.c | 4 ++-- hw/arm/aspeed_ast2600.c | 2 +- include/hw/arm/aspeed_soc.h | 2 -- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 06d863958b..39758557be 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -289,12 +289,14 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk, uint64_t rom_size) { AspeedSoCState *soc = bmc->soc; + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc); memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size, &error_abort); memory_region_add_subregion_overlap(&soc->spi_boot_container, 0, &bmc->boot_rom, 1); - write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, &error_abort); + write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], + rom_size, &error_abort); } void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c index 95da85fee0..d125886207 100644 --- a/hw/arm/aspeed_ast2400.c +++ b/hw/arm/aspeed_ast2400.c @@ -26,7 +26,7 @@ #define ASPEED_SOC_IOMEM_SIZE 0x00200000 static const hwaddr aspeed_soc_ast2400_memmap[] = { - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, + [ASPEED_DEV_SPI_BOOT] = 0x00000000, [ASPEED_DEV_IOMEM] = 0x1E600000, [ASPEED_DEV_FMC] = 0x1E620000, [ASPEED_DEV_SPI1] = 0x1E630000, @@ -61,7 +61,7 @@ static const hwaddr aspeed_soc_ast2400_memmap[] = { }; static const hwaddr aspeed_soc_ast2500_memmap[] = { - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, + [ASPEED_DEV_SPI_BOOT] = 0x00000000, [ASPEED_DEV_IOMEM] = 0x1E600000, [ASPEED_DEV_FMC] = 0x1E620000, [ASPEED_DEV_SPI1] = 0x1E630000, diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index f74561ecdc..174be53770 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -22,7 +22,7 @@ #define ASPEED_SOC_DPMCU_SIZE 0x00040000 static const hwaddr aspeed_soc_ast2600_memmap[] = { - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, + [ASPEED_DEV_SPI_BOOT] = 0x00000000, [ASPEED_DEV_SRAM] = 0x10000000, [ASPEED_DEV_DPMCU] = 0x18000000, /* 0x16000000 0x17FFFFFF : AHB BUS do LPC Bus bridge */ diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index 5ab0902da0..bf43ad8351 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -224,8 +224,6 @@ enum { ASPEED_DEV_FSI2, }; -#define ASPEED_SOC_SPI_BOOT_ADDR 0x0 - qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev); bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp); void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev *chr); -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] aspeed: fix hardcode boot address 0 2024-02-07 19:52 ` Jamin Lin via @ 2024-02-07 20:28 ` Philippe Mathieu-Daudé 2024-02-15 1:30 ` Jamin Lin 2024-02-09 8:31 ` Cédric Le Goater 1 sibling, 1 reply; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2024-02-07 20:28 UTC (permalink / raw) To: Jamin Lin, Cédric Le Goater, Peter Maydell, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: troy_lee Hi Jamin, On 7/2/24 20:52, Jamin Lin via wrote: > In the previous design of ASPEED SOCs QEMU model, it set the boot > address at "0" which was the hardcode setting for ast10x0, ast2600, > ast2500 and ast2400. > > According to the design of ast2700, it has bootmcu which is used for > executing SPL and initialize DRAM, then, CPUs(cortex-a35) > execute u-boot, kernel and rofs. QEMU will only support CPU(cortex-a35) > parts and the boot address is "0x4 00000000" for ast2700. This justification from here ... > Therefore, fixed hardcode boot address 0. ... to here is still unclear. You provided an explanation in previous patch, maybe worth including it in this description? Otherwise for the code changes: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> > --- > hw/arm/aspeed.c | 4 +++- > hw/arm/aspeed_ast2400.c | 4 ++-- > hw/arm/aspeed_ast2600.c | 2 +- > include/hw/arm/aspeed_soc.h | 2 -- > 4 files changed, 6 insertions(+), 6 deletions(-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 2/2] aspeed: fix hardcode boot address 0 2024-02-07 20:28 ` Philippe Mathieu-Daudé @ 2024-02-15 1:30 ` Jamin Lin 0 siblings, 0 replies; 9+ messages in thread From: Jamin Lin @ 2024-02-15 1:30 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Cédric Le Goater, Peter Maydell, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Troy Lee > -----Original Message----- > From: Philippe Mathieu-Daudé <philmd@linaro.org> > Sent: Thursday, February 8, 2024 4:29 AM > To: Jamin Lin <jamin_lin@aspeedtech.com>; Cédric Le Goater <clg@kaod.org>; > Peter Maydell <peter.maydell@linaro.org>; Andrew Jeffery > <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open > list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here > <qemu-devel@nongnu.org> > Cc: Troy Lee <troy_lee@aspeedtech.com> > Subject: Re: [PATCH v2 2/2] aspeed: fix hardcode boot address 0 > > Hi Jamin, > > On 7/2/24 20:52, Jamin Lin via wrote: > > In the previous design of ASPEED SOCs QEMU model, it set the boot > > address at "0" which was the hardcode setting for ast10x0, ast2600, > > ast2500 and ast2400. > > > > According to the design of ast2700, it has bootmcu which is used for > > executing SPL and initialize DRAM, then, CPUs(cortex-a35) execute > > u-boot, kernel and rofs. QEMU will only support CPU(cortex-a35) parts > > and the boot address is "0x4 00000000" for ast2700. > > This justification from here ... > > > Therefore, fixed hardcode boot address 0. > > ... to here is still unclear. You provided an explanation in previous patch, maybe > worth including it in this description? > > Otherwise for the code changes: Thanks for review and sorry reply you late due to my Chinese new year holiday. Will add. Jamin > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> > > --- > > hw/arm/aspeed.c | 4 +++- > > hw/arm/aspeed_ast2400.c | 4 ++-- > > hw/arm/aspeed_ast2600.c | 2 +- > > include/hw/arm/aspeed_soc.h | 2 -- > > 4 files changed, 6 insertions(+), 6 deletions(-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] aspeed: fix hardcode boot address 0 2024-02-07 19:52 ` Jamin Lin via 2024-02-07 20:28 ` Philippe Mathieu-Daudé @ 2024-02-09 8:31 ` Cédric Le Goater 2024-02-15 1:32 ` Jamin Lin 1 sibling, 1 reply; 9+ messages in thread From: Cédric Le Goater @ 2024-02-09 8:31 UTC (permalink / raw) To: Jamin Lin, Peter Maydell, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: troy_lee On 2/7/24 20:52, Jamin Lin wrote: > In the previous design of ASPEED SOCs QEMU model, it set the boot > address at "0" which was the hardcode setting for ast10x0, ast2600, > ast2500 and ast2400. > > According to the design of ast2700, it has bootmcu which is used for > executing SPL and initialize DRAM, then, CPUs(cortex-a35) > execute u-boot, kernel and rofs. QEMU will only support CPU(cortex-a35) > parts and the boot address is "0x4 00000000" for ast2700. > Therefore, fixed hardcode boot address 0. > > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> I agree with Philippe that the justification could be simpler. This change is just a cleanup preparing ground for future models using a different mapping address. Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > hw/arm/aspeed.c | 4 +++- > hw/arm/aspeed_ast2400.c | 4 ++-- > hw/arm/aspeed_ast2600.c | 2 +- > include/hw/arm/aspeed_soc.h | 2 -- > 4 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 06d863958b..39758557be 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -289,12 +289,14 @@ static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk, > uint64_t rom_size) > { > AspeedSoCState *soc = bmc->soc; > + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc); > > memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size, > &error_abort); > memory_region_add_subregion_overlap(&soc->spi_boot_container, 0, > &bmc->boot_rom, 1); > - write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, &error_abort); > + write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], > + rom_size, &error_abort); > } > > void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, > diff --git a/hw/arm/aspeed_ast2400.c b/hw/arm/aspeed_ast2400.c > index 95da85fee0..d125886207 100644 > --- a/hw/arm/aspeed_ast2400.c > +++ b/hw/arm/aspeed_ast2400.c > @@ -26,7 +26,7 @@ > #define ASPEED_SOC_IOMEM_SIZE 0x00200000 > > static const hwaddr aspeed_soc_ast2400_memmap[] = { > - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, > + [ASPEED_DEV_SPI_BOOT] = 0x00000000, > [ASPEED_DEV_IOMEM] = 0x1E600000, > [ASPEED_DEV_FMC] = 0x1E620000, > [ASPEED_DEV_SPI1] = 0x1E630000, > @@ -61,7 +61,7 @@ static const hwaddr aspeed_soc_ast2400_memmap[] = { > }; > > static const hwaddr aspeed_soc_ast2500_memmap[] = { > - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, > + [ASPEED_DEV_SPI_BOOT] = 0x00000000, > [ASPEED_DEV_IOMEM] = 0x1E600000, > [ASPEED_DEV_FMC] = 0x1E620000, > [ASPEED_DEV_SPI1] = 0x1E630000, > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c > index f74561ecdc..174be53770 100644 > --- a/hw/arm/aspeed_ast2600.c > +++ b/hw/arm/aspeed_ast2600.c > @@ -22,7 +22,7 @@ > #define ASPEED_SOC_DPMCU_SIZE 0x00040000 > > static const hwaddr aspeed_soc_ast2600_memmap[] = { > - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, > + [ASPEED_DEV_SPI_BOOT] = 0x00000000, > [ASPEED_DEV_SRAM] = 0x10000000, > [ASPEED_DEV_DPMCU] = 0x18000000, > /* 0x16000000 0x17FFFFFF : AHB BUS do LPC Bus bridge */ > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h > index 5ab0902da0..bf43ad8351 100644 > --- a/include/hw/arm/aspeed_soc.h > +++ b/include/hw/arm/aspeed_soc.h > @@ -224,8 +224,6 @@ enum { > ASPEED_DEV_FSI2, > }; > > -#define ASPEED_SOC_SPI_BOOT_ADDR 0x0 > - > qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev); > bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp); > void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev *chr); ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v2 2/2] aspeed: fix hardcode boot address 0 2024-02-09 8:31 ` Cédric Le Goater @ 2024-02-15 1:32 ` Jamin Lin 0 siblings, 0 replies; 9+ messages in thread From: Jamin Lin @ 2024-02-15 1:32 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Troy Lee > -----Original Message----- > From: Cédric Le Goater <clg@kaod.org> > Sent: Friday, February 9, 2024 4:32 PM > To: Jamin Lin <jamin_lin@aspeedtech.com>; Peter Maydell > <peter.maydell@linaro.org>; Andrew Jeffery <andrew@codeconstruct.com.au>; > Joel Stanley <joel@jms.id.au>; open list:ASPEED BMCs > <qemu-arm@nongnu.org>; open list:All patches CC here > <qemu-devel@nongnu.org> > Cc: Troy Lee <troy_lee@aspeedtech.com> > Subject: Re: [PATCH v2 2/2] aspeed: fix hardcode boot address 0 > > On 2/7/24 20:52, Jamin Lin wrote: > > In the previous design of ASPEED SOCs QEMU model, it set the boot > > address at "0" which was the hardcode setting for ast10x0, ast2600, > > ast2500 and ast2400. > > > > According to the design of ast2700, it has bootmcu which is used for > > executing SPL and initialize DRAM, then, CPUs(cortex-a35) execute > > u-boot, kernel and rofs. QEMU will only support CPU(cortex-a35) parts > > and the boot address is "0x4 00000000" for ast2700. > > Therefore, fixed hardcode boot address 0. > > > > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> > > I agree with Philippe that the justification could be simpler. This change is just > a cleanup preparing ground for future models using a different mapping > address. > Will fix Jamin > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > Thanks, > > C. > > > > --- > > hw/arm/aspeed.c | 4 +++- > > hw/arm/aspeed_ast2400.c | 4 ++-- > > hw/arm/aspeed_ast2600.c | 2 +- > > include/hw/arm/aspeed_soc.h | 2 -- > > 4 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index > > 06d863958b..39758557be 100644 > > --- a/hw/arm/aspeed.c > > +++ b/hw/arm/aspeed.c > > @@ -289,12 +289,14 @@ static void > aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk, > > uint64_t rom_size) > > { > > AspeedSoCState *soc = bmc->soc; > > + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(soc); > > > > memory_region_init_rom(&bmc->boot_rom, NULL, > "aspeed.boot_rom", rom_size, > > &error_abort); > > memory_region_add_subregion_overlap(&soc->spi_boot_container, > 0, > > &bmc->boot_rom, 1); > > - write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, > &error_abort); > > + write_boot_rom(blk, sc->memmap[ASPEED_DEV_SPI_BOOT], > > + rom_size, &error_abort); > > } > > > > void aspeed_board_init_flashes(AspeedSMCState *s, const char > > *flashtype, diff --git a/hw/arm/aspeed_ast2400.c > > b/hw/arm/aspeed_ast2400.c index 95da85fee0..d125886207 100644 > > --- a/hw/arm/aspeed_ast2400.c > > +++ b/hw/arm/aspeed_ast2400.c > > @@ -26,7 +26,7 @@ > > #define ASPEED_SOC_IOMEM_SIZE 0x00200000 > > > > static const hwaddr aspeed_soc_ast2400_memmap[] = { > > - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, > > + [ASPEED_DEV_SPI_BOOT] = 0x00000000, > > [ASPEED_DEV_IOMEM] = 0x1E600000, > > [ASPEED_DEV_FMC] = 0x1E620000, > > [ASPEED_DEV_SPI1] = 0x1E630000, > > @@ -61,7 +61,7 @@ static const hwaddr aspeed_soc_ast2400_memmap[] = > { > > }; > > > > static const hwaddr aspeed_soc_ast2500_memmap[] = { > > - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, > > + [ASPEED_DEV_SPI_BOOT] = 0x00000000, > > [ASPEED_DEV_IOMEM] = 0x1E600000, > > [ASPEED_DEV_FMC] = 0x1E620000, > > [ASPEED_DEV_SPI1] = 0x1E630000, > > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index > > f74561ecdc..174be53770 100644 > > --- a/hw/arm/aspeed_ast2600.c > > +++ b/hw/arm/aspeed_ast2600.c > > @@ -22,7 +22,7 @@ > > #define ASPEED_SOC_DPMCU_SIZE 0x00040000 > > > > static const hwaddr aspeed_soc_ast2600_memmap[] = { > > - [ASPEED_DEV_SPI_BOOT] = ASPEED_SOC_SPI_BOOT_ADDR, > > + [ASPEED_DEV_SPI_BOOT] = 0x00000000, > > [ASPEED_DEV_SRAM] = 0x10000000, > > [ASPEED_DEV_DPMCU] = 0x18000000, > > /* 0x16000000 0x17FFFFFF : AHB BUS do LPC Bus bridge */ > > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h > > index 5ab0902da0..bf43ad8351 100644 > > --- a/include/hw/arm/aspeed_soc.h > > +++ b/include/hw/arm/aspeed_soc.h > > @@ -224,8 +224,6 @@ enum { > > ASPEED_DEV_FSI2, > > }; > > > > -#define ASPEED_SOC_SPI_BOOT_ADDR 0x0 > > - > > qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev); > > bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp); > > void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev > > *chr); ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-02-15 1:32 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240207200220.453244-1-jamin_lin@aspeedtech.com>
2024-02-07 20:02 ` [PATCH v2 1/2] aspeed: introduce a new UART0 device name Jamin Lin via
2024-02-09 8:27 ` Cédric Le Goater
2024-02-15 1:31 ` Jamin Lin
2024-02-07 20:02 ` [PATCH v2 2/2] aspeed: fix hardcode boot address 0 Jamin Lin via
[not found] <20240207195224.452987-1-jamin_lin@aspeedtech.com>
2024-02-07 19:52 ` Jamin Lin via
2024-02-07 20:28 ` Philippe Mathieu-Daudé
2024-02-15 1:30 ` Jamin Lin
2024-02-09 8:31 ` Cédric Le Goater
2024-02-15 1:32 ` Jamin Lin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).