* Re: [Qemu-devel] aspeed qemu question [not found] ` <eb815dd0-f056-d733-f017-bba56b907231@kaod.org> @ 2019-05-17 7:46 ` Wim Vervoorn 2019-05-17 10:08 ` Cédric Le Goater 0 siblings, 1 reply; 7+ messages in thread From: Wim Vervoorn @ 2019-05-17 7:46 UTC (permalink / raw) To: Cédric Le Goater, qemu-devel@nongnu.org, qemu-arm@nongnu.org [-- Attachment #1: Type: text/plain, Size: 5940 bytes --] Hello Cedríc, Thanks for your response. I created and attached the patch. You are right the code snipped turns out unreadable. In the patch I enable the MAC's depending on the value in hwstrap1 just as in the real hardware. In the Palmetto both nics will be enabled (just as in the real palmetto). I added a 2nd AST2400 BMC the Eltan Piestewa Peak. Here I enabled the 2nd NIC only. What I see is that when I use Palmetto I get an assert in net/net.c line 256, this is in the "qemu_net_client_setup()". I assume I have to prepare something regarding the host side of the network implementation but I this point I have no clue what and I have a hard time figuring out how the networking architecture really works. If you could point me in the right direction I would really appreciate it. Best Regards, Wim Vervoorn Eltan B.V. Ambachtstraat 23 5481 SM Schijndel The Netherlands T : +31-(0)73-594 46 64 E : wvervoorn@eltan.com W : http://www.eltan.com -----Original Message----- From: Cédric Le Goater [mailto:clg@kaod.org] Sent: Thursday, May 16, 2019 5:06 PM To: Wim Vervoorn <wvervoorn@eltan.com> Subject: Re: aspeed qemu question Hello Wim, On 5/16/19 10:46 AM, Wim Vervoorn wrote: > Hello Cédric, > > > > I noticed you are responsible for most of the Aspeed additions to Qemu perhaps you can help me. Sure, > > > I am trying to add support for the 2^nd NIC on the Aspeed SoC. ok. This should be possible. > This seemed not that hard. Basically I added a 2^nd structure for the device and initialized that. I included the code below. hmm. This is difficult for me to read. Do you know how to send patches ? It would be good to send a patch to the QEMU devel and the QEMU ARM mailing list so that we can discuss openly. I can help you doing that. Cheers, C. > The code is building fine no problem about that. > > > > The issue is that I get an assert net.c line 256, this only happens if I realize both NICs if I realize only one all works as expected. My question is, am I overlooking something here? Do I need to take more actions to be able to support the 2^nd NIC? If you could point me in the right direction or know where I can find more information on the qemu architecture I would really appreciate it. > > > > > > > > staticvoidqemu_net_client_setup(NetClientState *nc, > > NetClientInfo *info, > > NetClientState *peer, > > constchar*model, > > constchar*name, > > NetClientDestructor *destructor) > > { > > nc->info= info; > > nc->model= g_strdup(model); > > if(name) { > > nc->name= g_strdup(name); > > } else{ > > nc->name= assign_name(nc, model); > > } > > > > if(peer) { > > à> assert(!peer->peer); > > nc->peer= peer; > > peer->peer= nc; > > } > > > > > > object_initialize(&s->ftgmac100[0], sizeof(s->ftgmac100), > TYPE_FTGMAC100); > > object_property_add_child(obj, "ftgmac100[0]", > OBJECT(&s->ftgmac100[0]), NULL); > > qdev_set_parent_bus(DEVICE(&s->ftgmac100[0]), > sysbus_get_default()); > > > > object_initialize(&s->ftgmac100[1], sizeof(s->ftgmac100), > TYPE_FTGMAC100); > > object_property_add_child(obj, "ftgmac100[1]", > OBJECT(&s->ftgmac100[1]), NULL); > > qdev_set_parent_bus(DEVICE(&s->ftgmac100[1]), > sysbus_get_default()); > > > > > > > > /* Net LAN 0*/ > > #if(0) > > qemu_log_mask(LOG_GUEST_ERROR, "%s: LAN0\n", __func__); > > qdev_set_nic_properties(DEVICE(&s->ftgmac100[0]), &nd_table[0]); > > object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "aspeed", > &err); > > object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, > "realized", > > &local_err); > > error_propagate(&err, local_err); > > if (err) { > > error_propagate(errp, err); > > return; > > } > > sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, > ASPEED_SOC_ETH1_BASE); > > sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, > > qdev_get_gpio_in(DEVICE(&s->vic), 2)); > > #endif > > > > /* Net LAN 1*/ > > #if(1) > > qemu_log_mask(LOG_GUEST_ERROR, "%s: LAN1\n", __func__); > > qdev_set_nic_properties(DEVICE(&s->ftgmac100[1]), &nd_table[1]); > > object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "aspeed", > &err); > > object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, > "realized", > > &local_err); > > error_propagate(&err, local_err); > > if(err) { > > error_propagate(errp, err); > > return; > > } > > sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, > ASPEED_SOC_ETH2_BASE); > > sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, > > qdev_get_gpio_in(DEVICE(&s->vic), 3)); > > #endif > > > > > > Best Regards, > > Wim Vervoorn > > > > Eltan B.V. > > Ambachtstraat 23 > > 5481 SM Schijndel > > The Netherlands > > > > T : +31-(0)73-594 46 64 > > E : wvervoorn@eltan.com > > W : http://www.eltan.com <http://www.eltan.com/> > > > > > > "THIS MESSAGE CONTAINS CONFIDENTIAL INFORMATION. UNLESS YOU ARE THE > INTENDED RECIPIENT OF THIS MESSAGE, ANY USE OF THIS MESSAGE IS STRICTLY PROHIBITED. IF YOU HAVE RECEIVED THIS MESSAGE IN ERROR, PLEASE IMMEDIATELY NOTIFY THE SENDER BY TELEPHONE +31-(0)73-5944664 OR REPLY EMAIL, AND IMMEDIATELY DELETE THIS MESSAGE AND ALL COPIES." > > > [-- Attachment #2: 0002-ASPEED-BMC-Add-support-for-2nd-NIC.patch --] [-- Type: application/octet-stream, Size: 7278 bytes --] From bbf9392b84f38531b89e4ea39e06210b99ec7a0c Mon Sep 17 00:00:00 2001 Message-Id: <bbf9392b84f38531b89e4ea39e06210b99ec7a0c.1558078463.git.wvervoorn@eltan.com> From: Wim Vervoorn <wvervoorn@eltan.com> Date: Fri, 17 May 2019 09:26:16 +0200 Subject: [PATCH] ASPEED BMC: Add support for 2nd NIC Add support for 2nd NIC. This solution is using the hwstrap1 value to enable disable the controllers. The Palmetto leaves both NICs enabled while the Piestewa Peak only enables one. The Palmetto asserts in net/net.c line 256 when qemu starts so something must be missing to support the 2nd nic. --- hw/arm/aspeed.c | 26 ++++++++++++++++++++++ hw/arm/aspeed_soc.c | 54 ++++++++++++++++++++++++++++++++------------- include/hw/arm/aspeed_soc.h | 2 +- 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c old mode 100644 new mode 100755 index 0203642..f957a5b --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -43,6 +43,23 @@ struct AspeedBoardState { SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) | \ SCU_HW_STRAP_VGA_CLASS_CODE | \ SCU_HW_STRAP_LPC_RESET_PIN | \ + SCU_HW_STRAP_MAC0_RGMII | \ + SCU_HW_STRAP_MAC1_RGMII | \ + SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) | \ + SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \ + SCU_HW_STRAP_SPI_WIDTH | \ + SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) | \ + SCU_AST2400_HW_STRAP_BOOT_MODE(AST2400_SPI_BOOT)) + +/* Piestewa Peak hardware value: */ +#define ELTANPWP_BMC_HW_STRAP1 ( \ + SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_256MB) | \ + SCU_AST2400_HW_STRAP_DRAM_CONFIG(2 /* DDR3 with CL=6, CWL=5 */) | \ + SCU_AST2400_HW_STRAP_ACPI_DIS | \ + SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) | \ + SCU_HW_STRAP_VGA_CLASS_CODE | \ + SCU_HW_STRAP_LPC_RESET_PIN | \ + SCU_HW_STRAP_MAC1_RGMII | \ SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) | \ SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \ SCU_HW_STRAP_SPI_WIDTH | \ @@ -423,6 +440,15 @@ static const AspeedBoardConfig aspeed_boards[] = { .num_cs = 1, .i2c_init = palmetto_bmc_i2c_init, }, { + .name = MACHINE_TYPE_NAME("eltanpwp-bmc"), + .desc = "Eltan Piestewa Peak BMC (ARM926EJ-S)", + .soc_name = "ast2400-a1", + .hw_strap1 = ELTANPWP_BMC_HW_STRAP1, + .fmc_model = "n25q256a", + .spi_model = "mx25l25635e", + .num_cs = 1, + .i2c_init = palmetto_bmc_i2c_init, + }, { .name = MACHINE_TYPE_NAME("ast2500-evb"), .desc = "Aspeed AST2500 EVB (ARM1176)", .soc_name = "ast2500-a1", diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c old mode 100644 new mode 100755 index 0f6e5be..8ed7902 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -184,9 +184,13 @@ static void aspeed_soc_init(Object *obj) OBJECT(&s->scu), &error_abort); } - object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100); - object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL); - qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default()); + object_initialize(&s->ftgmac100[0], sizeof(s->ftgmac100), TYPE_FTGMAC100); + object_property_add_child(obj, "ftgmac100[0]", OBJECT(&s->ftgmac100[0]), NULL); + qdev_set_parent_bus(DEVICE(&s->ftgmac100[0]), sysbus_get_default()); + + object_initialize(&s->ftgmac100[1], sizeof(s->ftgmac100), TYPE_FTGMAC100); + object_property_add_child(obj, "ftgmac100[1]", OBJECT(&s->ftgmac100[1]), NULL); + qdev_set_parent_bus(DEVICE(&s->ftgmac100[1]), sysbus_get_default()); object_initialize(&s->ibt, sizeof(s->ibt), TYPE_ASPEED_IBT); object_property_add_child(obj, "bt", OBJECT(&s->ibt), NULL); @@ -384,19 +388,39 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) ASPEED_SOC_WDT_BASE + i * 0x20); } - /* Net */ - qdev_set_nic_properties(DEVICE(&s->ftgmac100), &nd_table[0]); - object_property_set_bool(OBJECT(&s->ftgmac100), true, "aspeed", &err); - object_property_set_bool(OBJECT(&s->ftgmac100), true, "realized", - &local_err); - error_propagate(&err, local_err); - if (err) { - error_propagate(errp, err); - return; + /* Net LAN 0*/ + qdev_set_nic_properties(DEVICE(&s->ftgmac100[0]), &nd_table[0]); + object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "aspeed", &err); + if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC0_RGMII) { + qemu_log("%s: LAN0 enabled\n", __func__); + object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "realized", + &local_err); + error_propagate(&err, local_err); + if (err) { + error_propagate(errp, err); + return; + } + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, ASPEED_SOC_ETH1_BASE); + sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, + qdev_get_gpio_in(DEVICE(&s->vic), 2)); + } + + /* Net LAN 1*/ + qdev_set_nic_properties(DEVICE(&s->ftgmac100[1]), &nd_table[1]); + object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "aspeed", &err); + if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC1_RGMII) { + qemu_log("%s: LAN1 enabled\n", __func__); + object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "realized", + &local_err); + error_propagate(&err, local_err); + if (err) { + error_propagate(errp, err); + return; + } + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, ASPEED_SOC_ETH2_BASE); + sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, + qdev_get_gpio_in(DEVICE(&s->vic), 3)); } - sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE); - sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0, - qdev_get_gpio_in(DEVICE(&s->vic), 2)); /* iBT */ object_property_set_bool(OBJECT(&s->ibt), true, "realized", &err); diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h old mode 100644 new mode 100755 index 623223d..0799d61 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -47,7 +47,7 @@ typedef struct AspeedSoCState { AspeedSMCState spi[ASPEED_SPIS_NUM]; AspeedSDMCState sdmc; AspeedWDTState wdt[ASPEED_WDTS_NUM]; - FTGMAC100State ftgmac100; + FTGMAC100State ftgmac100[2]; AspeedIBTState ibt; AspeedGPIOState gpio; AspeedPWMState pwm; -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] aspeed qemu question 2019-05-17 7:46 ` [Qemu-devel] aspeed qemu question Wim Vervoorn @ 2019-05-17 10:08 ` Cédric Le Goater 2019-05-20 7:42 ` Wim Vervoorn 2019-05-23 10:05 ` Wim Vervoorn 0 siblings, 2 replies; 7+ messages in thread From: Cédric Le Goater @ 2019-05-17 10:08 UTC (permalink / raw) To: Wim Vervoorn, qemu-devel@nongnu.org, qemu-arm@nongnu.org, Joel Stanley, Andrew Jeffery, Peter Maydell Hello Win, On 5/17/19 9:46 AM, Wim Vervoorn wrote: > Hello Cedríc, > > Thanks for your response. I created and attached the patch. You are right the code snipped turns out unreadable. You should try to send the patch directly and not attached. Checkout the git send-email commands for it. See also : https://wiki.qemu.org/Contribute/SubmitAPatch > In the patch I enable the MAC's depending on the value in hwstrap1 just as in the real hardware. In the Palmetto both nics will be enabled (just as in the real palmetto). > > I added a 2nd AST2400 BMC the Eltan Piestewa Peak. Here I enabled the 2nd NIC only. > > What I see is that when I use Palmetto I get an assert in net/net.c line 256, this is in the "qemu_net_client_setup()". I assume I have to prepare something regarding the host side of the network implementation but I this point I have no clue what and I have a hard time figuring out how the networking architecture really works. Did you define two nics on the command line ? more comments below. [ ... ] > From bbf9392b84f38531b89e4ea39e06210b99ec7a0c Mon Sep 17 00:00:00 2001 > Message-Id: <bbf9392b84f38531b89e4ea39e06210b99ec7a0c.1558078463.git.wvervoorn@eltan.com> > From: Wim Vervoorn <wvervoorn@eltan.com> > Date: Fri, 17 May 2019 09:26:16 +0200 > Subject: [PATCH] ASPEED BMC: Add support for 2nd NIC > > Add support for 2nd NIC. > > This solution is using the hwstrap1 value to enable disable the > controllers. OK. Let see how this shows in the code. > The Palmetto leaves both NICs enabled while the Piestewa Peak only > enables one. You should send two different patches, one for the NIC, one for Piestewa Peak machine. > The Palmetto asserts in net/net.c line 256 when qemu starts so something > must be missing to support the 2nd nic. You need a "signed-off-by:" tag here > --- > hw/arm/aspeed.c | 26 ++++++++++++++++++++++ > hw/arm/aspeed_soc.c | 54 ++++++++++++++++++++++++++++++++------------- > include/hw/arm/aspeed_soc.h | 2 +- > 3 files changed, 66 insertions(+), 16 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > old mode 100644 > new mode 100755 > index 0203642..f957a5b > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -43,6 +43,23 @@ struct AspeedBoardState { > SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) | \ > SCU_HW_STRAP_VGA_CLASS_CODE | \ > SCU_HW_STRAP_LPC_RESET_PIN | \ > + SCU_HW_STRAP_MAC0_RGMII | \ > + SCU_HW_STRAP_MAC1_RGMII | \ > + SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) | \ > + SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \ > + SCU_HW_STRAP_SPI_WIDTH | \ > + SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) | \ > + SCU_AST2400_HW_STRAP_BOOT_MODE(AST2400_SPI_BOOT)) > + > +/* Piestewa Peak hardware value: */ > +#define ELTANPWP_BMC_HW_STRAP1 ( \ > + SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_256MB) | \ > + SCU_AST2400_HW_STRAP_DRAM_CONFIG(2 /* DDR3 with CL=6, CWL=5 */) | \ > + SCU_AST2400_HW_STRAP_ACPI_DIS | \ > + SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) | \ > + SCU_HW_STRAP_VGA_CLASS_CODE | \ > + SCU_HW_STRAP_LPC_RESET_PIN | \ > + SCU_HW_STRAP_MAC1_RGMII | \ > SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) | \ > SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \ > SCU_HW_STRAP_SPI_WIDTH | \ > @@ -423,6 +440,15 @@ static const AspeedBoardConfig aspeed_boards[] = { > .num_cs = 1, > .i2c_init = palmetto_bmc_i2c_init, > }, { > + .name = MACHINE_TYPE_NAME("eltanpwp-bmc"), > + .desc = "Eltan Piestewa Peak BMC (ARM926EJ-S)", > + .soc_name = "ast2400-a1", > + .hw_strap1 = ELTANPWP_BMC_HW_STRAP1, > + .fmc_model = "n25q256a", > + .spi_model = "mx25l25635e", Are these the correct flash models for your board ? > + .num_cs = 1, > + .i2c_init = palmetto_bmc_i2c_init, > + }, { > .name = MACHINE_TYPE_NAME("ast2500-evb"), > .desc = "Aspeed AST2500 EVB (ARM1176)", > .soc_name = "ast2500-a1", > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c > old mode 100644 > new mode 100755 > index 0f6e5be..8ed7902 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -184,9 +184,13 @@ static void aspeed_soc_init(Object *obj) > OBJECT(&s->scu), &error_abort); > } > > - object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100); > - object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL); > - qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default()); > + object_initialize(&s->ftgmac100[0], sizeof(s->ftgmac100), TYPE_FTGMAC100); > + object_property_add_child(obj, "ftgmac100[0]", OBJECT(&s->ftgmac100[0]), NULL); > + qdev_set_parent_bus(DEVICE(&s->ftgmac100[0]), sysbus_get_default()); > + > + object_initialize(&s->ftgmac100[1], sizeof(s->ftgmac100), TYPE_FTGMAC100); > + object_property_add_child(obj, "ftgmac100[1]", OBJECT(&s->ftgmac100[1]), NULL); > + qdev_set_parent_bus(DEVICE(&s->ftgmac100[1]), sysbus_get_default()); using a loop would be better. The future Aspeed SoCs might have more than two MACs. > > object_initialize(&s->ibt, sizeof(s->ibt), TYPE_ASPEED_IBT); > object_property_add_child(obj, "bt", OBJECT(&s->ibt), NULL); > @@ -384,19 +388,39 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) > ASPEED_SOC_WDT_BASE + i * 0x20); > } > > - /* Net */ > - qdev_set_nic_properties(DEVICE(&s->ftgmac100), &nd_table[0]); > - object_property_set_bool(OBJECT(&s->ftgmac100), true, "aspeed", &err); > - object_property_set_bool(OBJECT(&s->ftgmac100), true, "realized", > - &local_err); > - error_propagate(&err, local_err); > - if (err) { > - error_propagate(errp, err); > - return; > + /* Net LAN 0*/ > + qdev_set_nic_properties(DEVICE(&s->ftgmac100[0]), &nd_table[0]); > + object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "aspeed", &err); This is not the correct indentation. Please run checkpatch.pl on the patch. > + if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC0_RGMII) { > + qemu_log("%s: LAN0 enabled\n", __func__); we don't need this kind of information. > + object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "realized", > + &local_err); > + error_propagate(&err, local_err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, ASPEED_SOC_ETH1_BASE); > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, > + qdev_get_gpio_in(DEVICE(&s->vic), 2)); > + } > + > + /* Net LAN 1*/ > + qdev_set_nic_properties(DEVICE(&s->ftgmac100[1]), &nd_table[1]); > + object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "aspeed", &err); > + if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC1_RGMII) { > + qemu_log("%s: LAN1 enabled\n", __func__); > + object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "realized", > + &local_err); > + error_propagate(&err, local_err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, ASPEED_SOC_ETH2_BASE); > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, > + qdev_get_gpio_in(DEVICE(&s->vic), 3)); I would use a loop such as : for (i = 0; i < nb_nics; i++) { NICInfo *nd = &nd_table[i]; /* test SCU ... */ ... } to realize the NICs. > } > - sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE); > - sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0, > - qdev_get_gpio_in(DEVICE(&s->vic), 2)); > > /* iBT */ > object_property_set_bool(OBJECT(&s->ibt), true, "realized", &err); > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h > old mode 100644 > new mode 100755 > index 623223d..0799d61 > --- a/include/hw/arm/aspeed_soc.h > +++ b/include/hw/arm/aspeed_soc.h > @@ -47,7 +47,7 @@ typedef struct AspeedSoCState { > AspeedSMCState spi[ASPEED_SPIS_NUM]; > AspeedSDMCState sdmc; > AspeedWDTState wdt[ASPEED_WDTS_NUM]; > - FTGMAC100State ftgmac100; > + FTGMAC100State ftgmac100[2]; 2 needs a macro. I have a patchset currently being reviewed which should help you to define the different addresses, interrupt numbers of the MACS. Let me ping you when this is merge and then you can rebase. However, you can send a patch for your board definition. There should not be any conflicts with this addition. Thanks, C. > AspeedIBTState ibt; > AspeedGPIOState gpio; > AspeedPWMState pwm; > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] aspeed qemu question 2019-05-17 10:08 ` Cédric Le Goater @ 2019-05-20 7:42 ` Wim Vervoorn 2019-05-20 15:29 ` Cédric Le Goater 2019-05-23 10:05 ` Wim Vervoorn 1 sibling, 1 reply; 7+ messages in thread From: Wim Vervoorn @ 2019-05-20 7:42 UTC (permalink / raw) To: Cédric Le Goater, qemu-devel@nongnu.org, qemu-arm@nongnu.org, Joel Stanley, Andrew Jeffery, Peter Maydell Hello Cédric, It was not yet my intention to have this patch included in the qemu repo so I didn't pay attention to checking the indentation etc. So I misunderstood your suggestion. I am sorry about that. I will address your remarks in a new patch. Besides this I have another question. First of all I don't think qemu should assert due to a lacking NIC definition on the commandline so that is why I think I am missing something. Secondly I have started qemu with various attempts to define a 2nd NIC all with the same result (an assert). The issue with this is that I am not 100% sure I am defining the NICs in the correct way. If you could provide me with a commandline that is known to be correct I can use that for testing. Thanks for you patience. Best Regards, Wim Vervoorn -----Original Message----- From: Cédric Le Goater [mailto:clg@kaod.org] Sent: Friday, May 17, 2019 12:08 PM To: Wim Vervoorn <wvervoorn@eltan.com>; qemu-devel@nongnu.org; qemu-arm@nongnu.org; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Peter Maydell <peter.maydell@linaro.org> Subject: Re: aspeed qemu question Hello Win, On 5/17/19 9:46 AM, Wim Vervoorn wrote: > Hello Cedríc, > > Thanks for your response. I created and attached the patch. You are right the code snipped turns out unreadable. You should try to send the patch directly and not attached. Checkout the git send-email commands for it. See also : https://wiki.qemu.org/Contribute/SubmitAPatch > In the patch I enable the MAC's depending on the value in hwstrap1 just as in the real hardware. In the Palmetto both nics will be enabled (just as in the real palmetto). > > I added a 2nd AST2400 BMC the Eltan Piestewa Peak. Here I enabled the 2nd NIC only. > > What I see is that when I use Palmetto I get an assert in net/net.c line 256, this is in the "qemu_net_client_setup()". I assume I have to prepare something regarding the host side of the network implementation but I this point I have no clue what and I have a hard time figuring out how the networking architecture really works. Did you define two nics on the command line ? more comments below. [ ... ] > From bbf9392b84f38531b89e4ea39e06210b99ec7a0c Mon Sep 17 00:00:00 2001 > Message-Id: > <bbf9392b84f38531b89e4ea39e06210b99ec7a0c.1558078463.git.wvervoorn@elt > an.com> > From: Wim Vervoorn <wvervoorn@eltan.com> > Date: Fri, 17 May 2019 09:26:16 +0200 > Subject: [PATCH] ASPEED BMC: Add support for 2nd NIC > > Add support for 2nd NIC. > > This solution is using the hwstrap1 value to enable disable the > controllers. OK. Let see how this shows in the code. > The Palmetto leaves both NICs enabled while the Piestewa Peak only > enables one. You should send two different patches, one for the NIC, one for Piestewa Peak machine. > The Palmetto asserts in net/net.c line 256 when qemu starts so > something must be missing to support the 2nd nic. You need a "signed-off-by:" tag here > --- > hw/arm/aspeed.c | 26 ++++++++++++++++++++++ > hw/arm/aspeed_soc.c | 54 ++++++++++++++++++++++++++++++++------------- > include/hw/arm/aspeed_soc.h | 2 +- > 3 files changed, 66 insertions(+), 16 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c old mode 100644 new > mode 100755 index 0203642..f957a5b > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -43,6 +43,23 @@ struct AspeedBoardState { > SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) | \ > SCU_HW_STRAP_VGA_CLASS_CODE | \ > SCU_HW_STRAP_LPC_RESET_PIN | \ > + SCU_HW_STRAP_MAC0_RGMII | \ > + SCU_HW_STRAP_MAC1_RGMII | \ > + SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) | \ > + SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \ > + SCU_HW_STRAP_SPI_WIDTH | \ > + SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) | \ > + SCU_AST2400_HW_STRAP_BOOT_MODE(AST2400_SPI_BOOT)) > + > +/* Piestewa Peak hardware value: */ > +#define ELTANPWP_BMC_HW_STRAP1 ( \ > + SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_256MB) | \ > + SCU_AST2400_HW_STRAP_DRAM_CONFIG(2 /* DDR3 with CL=6, CWL=5 */) | \ > + SCU_AST2400_HW_STRAP_ACPI_DIS | \ > + SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) | \ > + SCU_HW_STRAP_VGA_CLASS_CODE | \ > + SCU_HW_STRAP_LPC_RESET_PIN | \ > + SCU_HW_STRAP_MAC1_RGMII | \ > SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) | \ > SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \ > SCU_HW_STRAP_SPI_WIDTH | \ > @@ -423,6 +440,15 @@ static const AspeedBoardConfig aspeed_boards[] = { > .num_cs = 1, > .i2c_init = palmetto_bmc_i2c_init, > }, { > + .name = MACHINE_TYPE_NAME("eltanpwp-bmc"), > + .desc = "Eltan Piestewa Peak BMC (ARM926EJ-S)", > + .soc_name = "ast2400-a1", > + .hw_strap1 = ELTANPWP_BMC_HW_STRAP1, > + .fmc_model = "n25q256a", > + .spi_model = "mx25l25635e", Are these the correct flash models for your board ? > + .num_cs = 1, > + .i2c_init = palmetto_bmc_i2c_init, > + }, { > .name = MACHINE_TYPE_NAME("ast2500-evb"), > .desc = "Aspeed AST2500 EVB (ARM1176)", > .soc_name = "ast2500-a1", > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c old mode 100644 > new mode 100755 index 0f6e5be..8ed7902 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -184,9 +184,13 @@ static void aspeed_soc_init(Object *obj) > OBJECT(&s->scu), &error_abort); > } > > - object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100); > - object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL); > - qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default()); > + object_initialize(&s->ftgmac100[0], sizeof(s->ftgmac100), TYPE_FTGMAC100); > + object_property_add_child(obj, "ftgmac100[0]", OBJECT(&s->ftgmac100[0]), NULL); > + qdev_set_parent_bus(DEVICE(&s->ftgmac100[0]), > + sysbus_get_default()); > + > + object_initialize(&s->ftgmac100[1], sizeof(s->ftgmac100), TYPE_FTGMAC100); > + object_property_add_child(obj, "ftgmac100[1]", OBJECT(&s->ftgmac100[1]), NULL); > + qdev_set_parent_bus(DEVICE(&s->ftgmac100[1]), > + sysbus_get_default()); using a loop would be better. The future Aspeed SoCs might have more than two MACs. > > object_initialize(&s->ibt, sizeof(s->ibt), TYPE_ASPEED_IBT); > object_property_add_child(obj, "bt", OBJECT(&s->ibt), NULL); @@ > -384,19 +388,39 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) > ASPEED_SOC_WDT_BASE + i * 0x20); > } > > - /* Net */ > - qdev_set_nic_properties(DEVICE(&s->ftgmac100), &nd_table[0]); > - object_property_set_bool(OBJECT(&s->ftgmac100), true, "aspeed", &err); > - object_property_set_bool(OBJECT(&s->ftgmac100), true, "realized", > - &local_err); > - error_propagate(&err, local_err); > - if (err) { > - error_propagate(errp, err); > - return; > + /* Net LAN 0*/ > + qdev_set_nic_properties(DEVICE(&s->ftgmac100[0]), &nd_table[0]); > + object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, > + "aspeed", &err); This is not the correct indentation. Please run checkpatch.pl on the patch. > + if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC0_RGMII) { > + qemu_log("%s: LAN0 enabled\n", __func__); we don't need this kind of information. > + object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "realized", > + &local_err); > + error_propagate(&err, local_err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, ASPEED_SOC_ETH1_BASE); > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, > + qdev_get_gpio_in(DEVICE(&s->vic), 2)); > + } > + > + /* Net LAN 1*/ > + qdev_set_nic_properties(DEVICE(&s->ftgmac100[1]), &nd_table[1]); > + object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "aspeed", &err); > + if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC1_RGMII) { > + qemu_log("%s: LAN1 enabled\n", __func__); > + object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "realized", > + &local_err); > + error_propagate(&err, local_err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, ASPEED_SOC_ETH2_BASE); > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, > + qdev_get_gpio_in(DEVICE(&s->vic), 3)); I would use a loop such as : for (i = 0; i < nb_nics; i++) { NICInfo *nd = &nd_table[i]; /* test SCU ... */ ... } to realize the NICs. > } > - sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE); > - sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0, > - qdev_get_gpio_in(DEVICE(&s->vic), 2)); > > /* iBT */ > object_property_set_bool(OBJECT(&s->ibt), true, "realized", > &err); diff --git a/include/hw/arm/aspeed_soc.h > b/include/hw/arm/aspeed_soc.h old mode 100644 new mode 100755 index > 623223d..0799d61 > --- a/include/hw/arm/aspeed_soc.h > +++ b/include/hw/arm/aspeed_soc.h > @@ -47,7 +47,7 @@ typedef struct AspeedSoCState { > AspeedSMCState spi[ASPEED_SPIS_NUM]; > AspeedSDMCState sdmc; > AspeedWDTState wdt[ASPEED_WDTS_NUM]; > - FTGMAC100State ftgmac100; > + FTGMAC100State ftgmac100[2]; 2 needs a macro. I have a patchset currently being reviewed which should help you to define the different addresses, interrupt numbers of the MACS. Let me ping you when this is merge and then you can rebase. However, you can send a patch for your board definition. There should not be any conflicts with this addition. Thanks, C. > AspeedIBTState ibt; > AspeedGPIOState gpio; > AspeedPWMState pwm; > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] aspeed qemu question 2019-05-20 7:42 ` Wim Vervoorn @ 2019-05-20 15:29 ` Cédric Le Goater 0 siblings, 0 replies; 7+ messages in thread From: Cédric Le Goater @ 2019-05-20 15:29 UTC (permalink / raw) To: Wim Vervoorn, qemu-devel@nongnu.org, qemu-arm@nongnu.org, Joel Stanley, Andrew Jeffery, Peter Maydell Hello Wim, On 5/20/19 9:42 AM, Wim Vervoorn wrote: > Hello Cédric, > > It was not yet my intention to have this patch included in the qemu repo so I didn't pay attention to checking the indentation etc. So I misunderstood your suggestion. I am sorry about that. > > I will address your remarks in a new patch. > > Besides this I have another question. > > First of all I don't think qemu should assert due to a lacking NIC definition on the commandline so that is why I think I am missing something. I think there is another problem. In hw/net/ftgmac100.c, this line in ftgmac100_realize() should be removed : s->conf.peers.ncs[0] = nd_table[0].netdev; I will send a fix. > Secondly I have started qemu with various attempts to define a 2nd NIC all with the same result (an assert). The issue with this is that I am not 100% sure I am defining the NICs in the correct way. If you could provide me with a commandline that is known to be correct I can use that for testing. You could use two devices such as : -net nic,macaddr=C0:FF:EE:00:00:02,model=ftgmac100 -net user,id=netdev0 -net nic,macaddr=C0:FF:EE:00:00:04,model=ftgmac100 -net user,id=netdev1 Providing you have the correct DT, you should see something like : [ 3.628410] libphy: Fixed MDIO Bus: probed [ 3.631462] ftgmac100 1e660000.ethernet: Read MAC address c0:ff:ee:00:00:02 from chip [ 3.631835] ftgmac100 1e660000.ethernet: Using NCSI interface [ 3.646027] ftgmac100 1e660000.ethernet eth0: irq 19, mapped at (ptrval) [ 3.647308] ftgmac100 1e680000.ethernet: Read MAC address c0:ff:ee:00:00:04 from chip [ 4.120223] libphy: ftgmac100_mdio: probed [ 4.121590] RTL8211E Gigabit Ethernet 1e680000.ethernet--1:00: attached PHY driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=1e680000.ethernet--1:00, irq=POLL) [ 4.134884] ftgmac100 1e680000.ethernet eth1: irq 20, mapped at (ptrval) Regards, C. > Thanks for you patience. > > Best Regards, > Wim Vervoorn > > > -----Original Message----- > From: Cédric Le Goater [mailto:clg@kaod.org] > Sent: Friday, May 17, 2019 12:08 PM > To: Wim Vervoorn <wvervoorn@eltan.com>; qemu-devel@nongnu.org; qemu-arm@nongnu.org; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Peter Maydell <peter.maydell@linaro.org> > Subject: Re: aspeed qemu question > > Hello Win, > > On 5/17/19 9:46 AM, Wim Vervoorn wrote: >> Hello Cedríc, >> >> Thanks for your response. I created and attached the patch. You are right the code snipped turns out unreadable. > > You should try to send the patch directly and not attached. Checkout the git send-email commands for it. See also : > > https://wiki.qemu.org/Contribute/SubmitAPatch > >> In the patch I enable the MAC's depending on the value in hwstrap1 just as in the real hardware. In the Palmetto both nics will be enabled (just as in the real palmetto). >> >> I added a 2nd AST2400 BMC the Eltan Piestewa Peak. Here I enabled the 2nd NIC only. >> >> What I see is that when I use Palmetto I get an assert in net/net.c line 256, this is in the "qemu_net_client_setup()". I assume I have to prepare something regarding the host side of the network implementation but I this point I have no clue what and I have a hard time figuring out how the networking architecture really works. > > Did you define two nics on the command line ? > > more comments below. > > [ ... ] > >> From bbf9392b84f38531b89e4ea39e06210b99ec7a0c Mon Sep 17 00:00:00 2001 >> Message-Id: >> <bbf9392b84f38531b89e4ea39e06210b99ec7a0c.1558078463.git.wvervoorn@elt >> an.com> >> From: Wim Vervoorn <wvervoorn@eltan.com> >> Date: Fri, 17 May 2019 09:26:16 +0200 >> Subject: [PATCH] ASPEED BMC: Add support for 2nd NIC >> >> Add support for 2nd NIC. >> >> This solution is using the hwstrap1 value to enable disable the >> controllers. > > OK. Let see how this shows in the code. > >> The Palmetto leaves both NICs enabled while the Piestewa Peak only >> enables one. > > You should send two different patches, one for the NIC, one for Piestewa Peak machine. > >> The Palmetto asserts in net/net.c line 256 when qemu starts so >> something must be missing to support the 2nd nic. > > You need a "signed-off-by:" tag here > >> --- >> hw/arm/aspeed.c | 26 ++++++++++++++++++++++ >> hw/arm/aspeed_soc.c | 54 ++++++++++++++++++++++++++++++++------------- >> include/hw/arm/aspeed_soc.h | 2 +- >> 3 files changed, 66 insertions(+), 16 deletions(-) >> >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c old mode 100644 new >> mode 100755 index 0203642..f957a5b >> --- a/hw/arm/aspeed.c >> +++ b/hw/arm/aspeed.c >> @@ -43,6 +43,23 @@ struct AspeedBoardState { >> SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) | \ >> SCU_HW_STRAP_VGA_CLASS_CODE | \ >> SCU_HW_STRAP_LPC_RESET_PIN | \ >> + SCU_HW_STRAP_MAC0_RGMII | \ >> + SCU_HW_STRAP_MAC1_RGMII | \ >> + SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) | \ >> + SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \ >> + SCU_HW_STRAP_SPI_WIDTH | \ >> + SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) | \ >> + SCU_AST2400_HW_STRAP_BOOT_MODE(AST2400_SPI_BOOT)) >> + >> +/* Piestewa Peak hardware value: */ >> +#define ELTANPWP_BMC_HW_STRAP1 ( \ >> + SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_256MB) | \ >> + SCU_AST2400_HW_STRAP_DRAM_CONFIG(2 /* DDR3 with CL=6, CWL=5 */) | \ >> + SCU_AST2400_HW_STRAP_ACPI_DIS | \ >> + SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) | \ >> + SCU_HW_STRAP_VGA_CLASS_CODE | \ >> + SCU_HW_STRAP_LPC_RESET_PIN | \ >> + SCU_HW_STRAP_MAC1_RGMII | \ >> SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) | \ >> SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \ >> SCU_HW_STRAP_SPI_WIDTH | \ >> @@ -423,6 +440,15 @@ static const AspeedBoardConfig aspeed_boards[] = { >> .num_cs = 1, >> .i2c_init = palmetto_bmc_i2c_init, >> }, { >> + .name = MACHINE_TYPE_NAME("eltanpwp-bmc"), >> + .desc = "Eltan Piestewa Peak BMC (ARM926EJ-S)", >> + .soc_name = "ast2400-a1", >> + .hw_strap1 = ELTANPWP_BMC_HW_STRAP1, >> + .fmc_model = "n25q256a", >> + .spi_model = "mx25l25635e", > > Are these the correct flash models for your board ? > >> + .num_cs = 1, >> + .i2c_init = palmetto_bmc_i2c_init, >> + }, { >> .name = MACHINE_TYPE_NAME("ast2500-evb"), >> .desc = "Aspeed AST2500 EVB (ARM1176)", >> .soc_name = "ast2500-a1", >> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c old mode 100644 >> new mode 100755 index 0f6e5be..8ed7902 >> --- a/hw/arm/aspeed_soc.c >> +++ b/hw/arm/aspeed_soc.c >> @@ -184,9 +184,13 @@ static void aspeed_soc_init(Object *obj) >> OBJECT(&s->scu), &error_abort); >> } >> >> - object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100); >> - object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL); >> - qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default()); >> + object_initialize(&s->ftgmac100[0], sizeof(s->ftgmac100), TYPE_FTGMAC100); >> + object_property_add_child(obj, "ftgmac100[0]", OBJECT(&s->ftgmac100[0]), NULL); >> + qdev_set_parent_bus(DEVICE(&s->ftgmac100[0]), >> + sysbus_get_default()); >> + >> + object_initialize(&s->ftgmac100[1], sizeof(s->ftgmac100), TYPE_FTGMAC100); >> + object_property_add_child(obj, "ftgmac100[1]", OBJECT(&s->ftgmac100[1]), NULL); >> + qdev_set_parent_bus(DEVICE(&s->ftgmac100[1]), >> + sysbus_get_default()); > > using a loop would be better. The future Aspeed SoCs might have more than two MACs. >> >> object_initialize(&s->ibt, sizeof(s->ibt), TYPE_ASPEED_IBT); >> object_property_add_child(obj, "bt", OBJECT(&s->ibt), NULL); @@ >> -384,19 +388,39 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) >> ASPEED_SOC_WDT_BASE + i * 0x20); >> } >> >> - /* Net */ >> - qdev_set_nic_properties(DEVICE(&s->ftgmac100), &nd_table[0]); >> - object_property_set_bool(OBJECT(&s->ftgmac100), true, "aspeed", &err); >> - object_property_set_bool(OBJECT(&s->ftgmac100), true, "realized", >> - &local_err); >> - error_propagate(&err, local_err); >> - if (err) { >> - error_propagate(errp, err); >> - return; >> + /* Net LAN 0*/ >> + qdev_set_nic_properties(DEVICE(&s->ftgmac100[0]), &nd_table[0]); >> + object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, >> + "aspeed", &err); > > This is not the correct indentation. Please run checkpatch.pl on the patch. > >> + if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC0_RGMII) { >> + qemu_log("%s: LAN0 enabled\n", __func__); > > we don't need this kind of information. > >> + object_property_set_bool(OBJECT(&s->ftgmac100[0]), true, "realized", >> + &local_err); >> + error_propagate(&err, local_err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } >> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, ASPEED_SOC_ETH1_BASE); >> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[0]), 0, >> + qdev_get_gpio_in(DEVICE(&s->vic), 2)); >> + } >> + >> + /* Net LAN 1*/ >> + qdev_set_nic_properties(DEVICE(&s->ftgmac100[1]), &nd_table[1]); >> + object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "aspeed", &err); >> + if (s->scu.hw_strap1 & SCU_HW_STRAP_MAC1_RGMII) { >> + qemu_log("%s: LAN1 enabled\n", __func__); >> + object_property_set_bool(OBJECT(&s->ftgmac100[1]), true, "realized", >> + &local_err); >> + error_propagate(&err, local_err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } >> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, ASPEED_SOC_ETH2_BASE); >> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[1]), 0, >> + qdev_get_gpio_in(DEVICE(&s->vic), 3)); > > I would use a loop such as : > > for (i = 0; i < nb_nics; i++) { > NICInfo *nd = &nd_table[i]; > > /* test SCU ... */ > > ... > } > > to realize the NICs. > >> } >> - sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE); >> - sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0, >> - qdev_get_gpio_in(DEVICE(&s->vic), 2)); >> >> /* iBT */ >> object_property_set_bool(OBJECT(&s->ibt), true, "realized", >> &err); diff --git a/include/hw/arm/aspeed_soc.h >> b/include/hw/arm/aspeed_soc.h old mode 100644 new mode 100755 index >> 623223d..0799d61 >> --- a/include/hw/arm/aspeed_soc.h >> +++ b/include/hw/arm/aspeed_soc.h >> @@ -47,7 +47,7 @@ typedef struct AspeedSoCState { >> AspeedSMCState spi[ASPEED_SPIS_NUM]; >> AspeedSDMCState sdmc; >> AspeedWDTState wdt[ASPEED_WDTS_NUM]; >> - FTGMAC100State ftgmac100; >> + FTGMAC100State ftgmac100[2]; > > 2 needs a macro. > > > I have a patchset currently being reviewed which should help you to define the different addresses, interrupt numbers of the MACS. > Let me ping you when this is merge and then you can rebase. > > However, you can send a patch for your board definition. There should not be any conflicts with this addition. > > Thanks, > > C. > > > > >> AspeedIBTState ibt; >> AspeedGPIOState gpio; >> AspeedPWMState pwm; >> -- >> 2.7.4 >> > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] aspeed qemu question 2019-05-17 10:08 ` Cédric Le Goater 2019-05-20 7:42 ` Wim Vervoorn @ 2019-05-23 10:05 ` Wim Vervoorn 2019-05-23 11:13 ` Cédric Le Goater 1 sibling, 1 reply; 7+ messages in thread From: Wim Vervoorn @ 2019-05-23 10:05 UTC (permalink / raw) To: Cédric Le Goater, qemu-devel@nongnu.org, qemu-arm@nongnu.org, Joel Stanley, Andrew Jeffery, Peter Maydell Hello Cédric, I have another question regarding the ASPEED Qemu support. This is regarding the SPI support. I noticed that in general the fmc_model and the spi_model for the flash device are different even though there is only one flash device connected. I would expect that in this case the models used would be identical. Can you explain how this is works and how I select the correct model for the fmc and for the spi controller. Best regards, Wim Vervoorn ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] aspeed qemu question 2019-05-23 10:05 ` Wim Vervoorn @ 2019-05-23 11:13 ` Cédric Le Goater 2019-05-23 11:40 ` Wim Vervoorn 0 siblings, 1 reply; 7+ messages in thread From: Cédric Le Goater @ 2019-05-23 11:13 UTC (permalink / raw) To: Wim Vervoorn, qemu-devel@nongnu.org, qemu-arm@nongnu.org, Joel Stanley, Andrew Jeffery, Peter Maydell On 5/23/19 12:05 PM, Wim Vervoorn wrote: > Hello Cédric, > > I have another question regarding the ASPEED Qemu support. This is regarding the SPI support. > > I noticed that in general the fmc_model and the spi_model for the flash device > are different even though there is only one flash device connected. The flash devices are created but not attached to a file backend unless you define it on the command line with -drive options : -drive file=$flashfile,format=raw,if=mtd The first mtd drive corresponds to the flash chip attached to the FMC controller (BMC Firmware), the second is the flash attached to the SPI1 controller (Host Firmware) > I would expect that in this case the models used would be identical. The flash device models are defined at the machine level in hw/arm/aspeed.c: }, { .name = MACHINE_TYPE_NAME("witherspoon-bmc"), .desc = "OpenPOWER Witherspoon BMC (ARM1176)", .soc_name = "ast2500-a1", .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, .fmc_model = "mx25l25635e", .spi_model = "mx66l1g45g", .num_cs = 2, .i2c_init = witherspoon_bmc_i2c_init, .ram = 512 * MiB, }, > Can you explain how this is works and how I select the correct model > for the fmc and for the spi controller. You need to define a new machine (board) if you want different flash models. C. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] aspeed qemu question 2019-05-23 11:13 ` Cédric Le Goater @ 2019-05-23 11:40 ` Wim Vervoorn 0 siblings, 0 replies; 7+ messages in thread From: Wim Vervoorn @ 2019-05-23 11:40 UTC (permalink / raw) To: Cédric Le Goater, qemu-devel@nongnu.org, qemu-arm@nongnu.org, Joel Stanley, Andrew Jeffery, Peter Maydell Hello Cédric, How stupid of me. I overlooked that the SPI is for the host flash device. The remainder of the solution was clear and understood. Best regards and thanks for the quick response, Wim -----Original Message----- From: Cédric Le Goater [mailto:clg@kaod.org] Sent: Thursday, May 23, 2019 1:14 PM To: Wim Vervoorn <wvervoorn@eltan.com>; qemu-devel@nongnu.org; qemu-arm@nongnu.org; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Peter Maydell <peter.maydell@linaro.org> Subject: Re: aspeed qemu question On 5/23/19 12:05 PM, Wim Vervoorn wrote: > Hello Cédric, > > I have another question regarding the ASPEED Qemu support. This is regarding the SPI support. > > I noticed that in general the fmc_model and the spi_model for the > flash device are different even though there is only one flash device connected. The flash devices are created but not attached to a file backend unless you define it on the command line with -drive options : -drive file=$flashfile,format=raw,if=mtd The first mtd drive corresponds to the flash chip attached to the FMC controller (BMC Firmware), the second is the flash attached to the SPI1 controller (Host Firmware) > I would expect that in this case the models used would be identical. The flash device models are defined at the machine level in hw/arm/aspeed.c: }, { .name = MACHINE_TYPE_NAME("witherspoon-bmc"), .desc = "OpenPOWER Witherspoon BMC (ARM1176)", .soc_name = "ast2500-a1", .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, .fmc_model = "mx25l25635e", .spi_model = "mx66l1g45g", .num_cs = 2, .i2c_init = witherspoon_bmc_i2c_init, .ram = 512 * MiB, }, > Can you explain how this is works and how I select the correct model > for the fmc and for the spi controller. You need to define a new machine (board) if you want different flash models. C. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-23 11:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <d0fea426e8b245769f3022dad121c17e@Eltsrv03.Eltan.local> [not found] ` <eb815dd0-f056-d733-f017-bba56b907231@kaod.org> 2019-05-17 7:46 ` [Qemu-devel] aspeed qemu question Wim Vervoorn 2019-05-17 10:08 ` Cédric Le Goater 2019-05-20 7:42 ` Wim Vervoorn 2019-05-20 15:29 ` Cédric Le Goater 2019-05-23 10:05 ` Wim Vervoorn 2019-05-23 11:13 ` Cédric Le Goater 2019-05-23 11:40 ` Wim Vervoorn
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).