|
| From: | Cédric Le Goater |
| Subject: | Re: [PATCH v3 14/14] hw/arm/aspeed: Add oby35-cl machine |
| Date: | Fri, 1 Jul 2022 07:39:01 +0200 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 |
On 7/1/22 01:06, Peter Delevoryas wrote:
On Thu, Jun 30, 2022 at 06:42:52PM +0200, Cédric Le Goater wrote:On 6/30/22 18:15, Peter Delevoryas wrote:On Thu, Jun 30, 2022 at 01:02:54PM +0200, Cédric Le Goater wrote:On 6/30/22 06:51, Peter Delevoryas wrote:From: Peter Delevoryas <pdel@fb.com> The fby35 machine includes 4 server boards, each of which has a "bridge interconnect" (BIC). This chip abstracts the pinout for the server board into a single endpoint that the baseboard management controller (BMC) can talk to using IPMB. This commit adds a machine for testing the BIC on the server board. It runs OpenBIC (https://github.com/facebook/openbic) and the server board is called CraterLake, so the code name is oby35-cl. There's also a variant of the baseboard that replaces the BMC with a BIC, but that machine is not included here. A test image can be built from https://github.com/facebook/openbic using the instructions in the README.md to build the meta-facebook/yv35-cl recipe, or retrieved from my Github: wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.17.01/Y35BCL.elf And you can run this machine with the following command: qemu-system-arm -machine oby35-cl -nographic -kernel Y35BCL.elf It should produce output like the following: [00:00:00.005,000] <inf> usb_dc_aspeed: select ep[0x81] as IN endpoint [00:00:00.006,000] <inf> usb_dc_aspeed: select ep[0x82] as IN endpoint [00:00:00.006,000] <wrn> usb_dc_aspeed: pre-selected ep[0x1] as IN endpoint [00:00:00.006,000] <wrn> usb_dc_aspeed: pre-selected ep[0x2] as IN endpoint [00:00:00.006,000] <inf> usb_dc_aspeed: select ep[0x3] as OUT endpoint *** Booting Zephyr OS build v00.01.05 *** Hello, welcome to yv35 craterlake 2022.25.1 BIC class type(class-1), 1ou present status(0), 2ou present status(0), board revision(0x1) check_vr_type: i2c4 0x62 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x76 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x76 page 1 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x60 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x60 page 1 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x62 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x76 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x76 page 1 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x60 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x60 page 1 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x62 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x76 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x76 page 1 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x60 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x60 page 1 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x62 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x76 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x76 page 1 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x60 page 0 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] check_vr_type: i2c4 0x60 page 1 [04 00 81 d2 49 3c ff ff ff ff ff ff ff ff ff ff] [init_drive_type] sensor 0x14 post sensor read failed! [init_drive_type] sensor 0x30 post sensor read failed! [init_drive_type] sensor 0x39 post sensor read failed! ipmi_init [set_DC_status] gpio number(15) status(0) [set_post_status] gpio number(1) status(1) uart:~$ [00:00:01.010,000] <inf> kcs_aspeed: KCS3: addr=0xca2, idr=0x2c, odr=0x38, str=0x44 [00:00:01.016,000] <err> spi_nor_multi_dev: [1216][spi1_cs0]SFDP magic 00000000 invalid [00:00:01.016,000] <err> spi_nor_multi_dev: [1456]SFDP read failed: -22 [00:00:01.010,000] <inf> kcs_aspeed: KCS3: addr=0xca2, idr=0x2c, odr=0x38, str=0x44 [00:00:01.016,000] <err> spi_nor_multi_dev: [1216][spi1_cs0]SFDP magic 00000000 invalid [00:00:01.016,000] <err> spi_nor_multi_dev: [1456]SFDP read failed: -22 uart:~$ BIC Ready Signed-off-by: Peter Delevoryas <pdel@fb.com>LGTM. That said I would prefer to introduce the machine first and then populate with devices.Ohh ok, I'll submit the machine definition separately all by itself and then submit any extra devices like the CPLD or ME afterwards.I have kept the "full system" in my tree for now : cb4481ae1812 aspeed: Add AST2600 (BMC) to fby35 (full system) c155bf27d3e7 aspeed: Make aspeed_board_init_flashes public (trivial) 3f5485fa88b9 aspeed: Add fby35 skeleton (trivial) because the ROM vs. execute-in-place is being analyzed. Let's see if we can make progress and simplify the initial machine. I have also kept the latest *fby35* emulating the BIC only : 5cfc4b68fdb8 hw/arm/aspeed: Add oby35-cl machine 06f21e024ee7 hw/misc/aspeed: Add intel-me e96a23571599 hw/misc/aspeed: Add fby35-sb-cpld to discuss a bit more on the names, files, IPMI, etc. Until now, we had Aspeed machines modeling EVBs or BMCs. BICs and multi SoC system are new.Oh shoot, I just remembered, the oby35-cl machine won't work without the VR patches.
They look ready for upstream now. C.
I know the pull request you just made didn't include it, just a reminder. I was worried that maybe it had been submitted somewhere.Having a review on the common models in 8-10 would be nice. 2a9be57901a3 hw/sensor: Add Renesas ISL69259 device model 85f8352e213a hw/sensor: Add IC_DEVICE_ID to ISL voltage regulators aea568d56db5 hw/i2c/pmbus: Add idle state to return 0xff's They should not be too problematic to merge. As soon as Titus has time to take a look we will know, and I did a comment. So this can be addressed in parallel with the fby35 machines. Thanks, C.May be it is time to introduce a new machine file. This one seems like it could go in a f35.c file, also because a larger f35-* is in plan. aspeed.c could contain the basic definitions and helpers.Yes, patrick@stwcx.xyz was thinking the same thing. An f35.c (well, maybe yv35.c or fby35.c would be more appropriate) would be a good idea. I'll submit another patch up front to move fby35 stuff to a separate file.--- hw/arm/aspeed.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index a06f7c1b62..75971ef2ca 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -1429,6 +1429,50 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc, amc->macs_mask = 0; } +static void oby35_cl_i2c_init(AspeedMachineState *bmc) +{ + AspeedSoCState *soc = &bmc->soc; + I2CBus *i2c[14]; + I2CBus *ssd[8]; + int i; + + for (i = 0; i < 14; i++) { + i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i); + } + get_pca9548_channels(i2c[1], 0x71, ssd);We should rename to aspeed_get_pca9548_channels+1+ + i2c_slave_create_simple(i2c[0], "fby35-sb-cpld", 0x21); + i2c_slave_create_simple(i2c[1], "tmp105", 0x48); + i2c_slave_create_simple(i2c[1], "tmp105", 0x49); + i2c_slave_create_simple(i2c[1], "tmp105", 0x4a); + i2c_slave_create_simple(i2c[1], "adm1272", 0x40); + i2c_slave_create_simple(i2c[1], "tmp421", 0x4c); + i2c_slave_create_simple(i2c[2], "intel-me", 0x16); + i2c_slave_create_simple(i2c[4], "isl69259", 0x76); + i2c_slave_create_simple(i2c[4], "isl69259", 0x62); + i2c_slave_create_simple(i2c[4], "isl69259", 0x60); + + for (int i = 0; i < 8; i++) { + i2c_slave_create_simple(ssd[i], "tmp105", 0x6a); + } + + /* + * FIXME: This should actually be the BMC, but both the ME and the BMCQEMU has an embedded IPMI BMC simulator.!!! Didn't realize this, definitely going to try using it.+ * are IPMB endpoints, and the current ME implementation is generic + * enough to respond normally to some things. + */ + i2c_slave_create_simple(i2c[6], "intel-me", 0x10); +} + +static void aspeed_machine_oby35_cl_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "Meta Platforms fby35 CraterLake BIC (Cortex-M4)"; + amc->i2c_init = oby35_cl_i2c_init; +} + static const TypeInfo aspeed_machine_types[] = { { .name = MACHINE_TYPE_NAME("palmetto-bmc"), @@ -1494,6 +1538,10 @@ static const TypeInfo aspeed_machine_types[] = { .name = MACHINE_TYPE_NAME("ast1030-evb"), .parent = TYPE_ASPEED_MACHINE, .class_init = aspeed_minibmc_machine_ast1030_evb_class_init, + }, { + .name = MACHINE_TYPE_NAME("oby35-cl"), + .parent = MACHINE_TYPE_NAME("ast1030-evb"),hmm, so we are inheriting from the evb ?Yeah, I remember this was controversial with fby35-bmc too, maybe I'll change this in the follow-up. I just like inheriting from the EVB's because people use the EVB's a lot for testing, most of the time I'm just trying to add some extra i2c devices/etc, so I override the i2c init. But, maybe it's good to decouple them.C.+ .class_init = aspeed_machine_oby35_cl_class_init, }, { .name = TYPE_ASPEED_MACHINE, .parent = TYPE_MACHINE,
| [Prev in Thread] | Current Thread | [Next in Thread] |