> On Sep 6, 2021, at 2:15 AM, Philippe Mathieu-Daudé <[email protected]> wrote: > > On 9/5/21 8:55 PM, [email protected] wrote: >> From: Peter Delevoryas <[email protected]> >> >> This adds a new machine type "fuji-bmc" based on the following device tree: >> >> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts > > Sorry for being picky, but 'master' is a branch, not a (fixed) tag. > Since there is no tag released with this file, please point to the > commit introducing the file: > > https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts
Oh, that’s ok, I can resend this patch with these links. > >> Most of the i2c devices are not there, they're added here: >> >> https://github.com/facebook/openbmc/blob/helium/meta-facebook/meta-fuji/recipes-utils/openbmc-utils/files/setup_i2c.sh > > Similarly: > > https://github.com/facebook/openbmc/blob/fb2ed12002fb/meta-facebook/meta-fuji/recipes-utils/openbmc-utils/files/setup_i2c.sh > > (note the nice fb*fb SHA-1 :P) ! What are the odds?? > >> I tested this by building a Fuji image from Facebook's OpenBMC repo, >> booting, and ssh'ing from host-to-guest. >> >> Signed-off-by: Peter Delevoryas <[email protected]> >> --- >> hw/arm/aspeed.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 112 insertions(+) >> >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c >> index 7a9459340c..cc2d721ac7 100644 >> --- a/hw/arm/aspeed.c >> +++ b/hw/arm/aspeed.c >> @@ -159,6 +159,10 @@ struct AspeedMachineState { >> #define RAINIER_BMC_HW_STRAP1 0x00000000 >> #define RAINIER_BMC_HW_STRAP2 0x00000000 >> >> +/* Fuji hardware value */ >> +#define FUJI_BMC_HW_STRAP1 0x00000000 >> +#define FUJI_BMC_HW_STRAP2 0x00000000 >> + >> /* >> * The max ram region is for firmwares that scan the address space >> * with load/store to guess how much RAM the SoC has. >> @@ -772,6 +776,90 @@ static void rainier_bmc_i2c_init(AspeedMachineState >> *bmc) >> aspeed_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x50, 64 * KiB); >> } >> >> +static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr, I2CBus >> **channels) { > > Per https://wiki.qemu.org/Contribute/SubmitAPatch#Use_the_QEMU_coding_style Oh yeah, I noticed Cedric mentioned he had to do this, sorry!! I remember reading through this wiki page, and that there was a checkpatch script, but then I forgot to actually run it. I’ll make sure to do that in the future. > > static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr, > I2CBus **channels) > { > >> + I2CSlave *mux = i2c_slave_create_simple(bus, "pca9548", mux_addr); >> + for (int i = 0; i < 8; i++) { >> + channels[i] = pca954x_i2c_get_bus(mux, i); >> + } >> +} >> + >> +#define TYPE_LM75 TYPE_TMP105 >> +#define TYPE_TMP75 TYPE_TMP105 >> +#define TYPE_TMP422 "tmp422" >> + >> +static void fuji_bmc_i2c_init(AspeedMachineState *bmc) >> +{ >> + AspeedSoCState *soc = &bmc->soc; >> + I2CBus *i2c[144] = {}; >> + >> + for (int i = 0; i < 16; i++) { >> + i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i); >> + } >> + I2CBus *i2c180 = i2c[2]; >> + I2CBus *i2c480 = i2c[8]; >> + I2CBus *i2c600 = i2c[11]; >> + >> + get_pca9548_channels(i2c180, 0x70, &i2c[16]); >> + get_pca9548_channels(i2c480, 0x70, &i2c[24]); > > QEMU style: > > /* > >> + // NOTE: The device tree skips [32, 40) in the alias numbering, so we do >> + // the same here. > > */ > >> + get_pca9548_channels(i2c600, 0x77, &i2c[40]); >> + get_pca9548_channels(i2c[24], 0x71, &i2c[48]); >> + get_pca9548_channels(i2c[25], 0x72, &i2c[56]); >> + get_pca9548_channels(i2c[26], 0x76, &i2c[64]); >> + get_pca9548_channels(i2c[27], 0x76, &i2c[72]); >> + for (int i = 0; i < 8; i++) { >> + get_pca9548_channels(i2c[40 + i], 0x76, &i2c[80 + i * 8]); >> + } >> + >> + i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4c); >> + i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4d); >> + >> + aspeed_eeprom_init(i2c[19], 0x52, 64 * KiB); >> + aspeed_eeprom_init(i2c[20], 0x50, 2 * KiB); >> + aspeed_eeprom_init(i2c[22], 0x52, 2 * KiB); >> + >> + i2c_slave_create_simple(i2c[3], TYPE_LM75, 0x48); >> + i2c_slave_create_simple(i2c[3], TYPE_LM75, 0x49); >> + i2c_slave_create_simple(i2c[3], TYPE_LM75, 0x4a); >> + i2c_slave_create_simple(i2c[3], TYPE_TMP422, 0x4c); >> + >> + aspeed_eeprom_init(i2c[8], 0x51, 64 * KiB); >> + i2c_slave_create_simple(i2c[8], TYPE_LM75, 0x4a); >> + >> + i2c_slave_create_simple(i2c[50], TYPE_LM75, 0x4c); >> + aspeed_eeprom_init(i2c[50], 0x52, 64 * KiB); >> + i2c_slave_create_simple(i2c[51], TYPE_TMP75, 0x48); >> + i2c_slave_create_simple(i2c[52], TYPE_TMP75, 0x49); >> + >> + i2c_slave_create_simple(i2c[59], TYPE_TMP75, 0x48); >> + i2c_slave_create_simple(i2c[60], TYPE_TMP75, 0x49); >> + >> + aspeed_eeprom_init(i2c[65], 0x53, 64 * KiB); >> + i2c_slave_create_simple(i2c[66], TYPE_TMP75, 0x49); >> + i2c_slave_create_simple(i2c[66], TYPE_TMP75, 0x48); >> + aspeed_eeprom_init(i2c[68], 0x52, 64 * KiB); >> + aspeed_eeprom_init(i2c[69], 0x52, 64 * KiB); >> + aspeed_eeprom_init(i2c[70], 0x52, 64 * KiB); >> + aspeed_eeprom_init(i2c[71], 0x52, 64 * KiB); >> + >> + aspeed_eeprom_init(i2c[73], 0x53, 64 * KiB); >> + i2c_slave_create_simple(i2c[74], TYPE_TMP75, 0x49); >> + i2c_slave_create_simple(i2c[74], TYPE_TMP75, 0x48); >> + aspeed_eeprom_init(i2c[76], 0x52, 64 * KiB); >> + aspeed_eeprom_init(i2c[77], 0x52, 64 * KiB); >> + aspeed_eeprom_init(i2c[78], 0x52, 64 * KiB); >> + aspeed_eeprom_init(i2c[79], 0x52, 64 * KiB); >> + aspeed_eeprom_init(i2c[28], 0x50, 2 * KiB); >> + >> + for (int i = 0; i < 8; i++) { >> + aspeed_eeprom_init(i2c[81 + i * 8], 0x56, 64 * KiB); >> + i2c_slave_create_simple(i2c[82 + i * 8], TYPE_TMP75, 0x48); >> + i2c_slave_create_simple(i2c[83 + i * 8], TYPE_TMP75, 0x4b); >> + i2c_slave_create_simple(i2c[84 + i * 8], TYPE_TMP75, 0x4a); >> + } >> +} >> + >> static bool aspeed_get_mmio_exec(Object *obj, Error **errp) >> { >> return ASPEED_MACHINE(obj)->mmio_exec; >> @@ -1070,6 +1158,26 @@ static void >> aspeed_machine_rainier_class_init(ObjectClass *oc, void *data) >> aspeed_soc_num_cpus(amc->soc_name); >> }; >> >> +static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data) >> +{ >> + MachineClass *mc = MACHINE_CLASS(oc); >> + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); >> + >> + mc->desc = "Facebook Fuji BMC (Cortex-A7)"; >> + amc->soc_name = "ast2600-a3"; >> + amc->hw_strap1 = FUJI_BMC_HW_STRAP1; >> + amc->hw_strap2 = FUJI_BMC_HW_STRAP2; >> + amc->fmc_model = "mx66l1g45g"; >> + amc->spi_model = "mx66l1g45g"; >> + amc->num_cs = 2; >> + amc->macs_mask = ASPEED_MAC3_ON; >> + amc->i2c_init = fuji_bmc_i2c_init; >> + amc->uart_default = ASPEED_DEV_UART1; >> + mc->default_ram_size = 2 * GiB; >> + mc->default_cpus = mc->min_cpus = mc->max_cpus = >> + aspeed_soc_num_cpus(amc->soc_name); > > Matter of taste: > > mc->default_cpus = mc->min_cpus = mc->max_cpus > = aspeed_soc_num_cpus(amc->soc_name); I actually like that better too, but I think the rest of the file does it this way, so I’ll just leave it as-is to be consistent I suppose. > >> +}; >> + >> static const TypeInfo aspeed_machine_types[] = { >> { >> .name = MACHINE_TYPE_NAME("palmetto-bmc"), >> @@ -1119,6 +1227,10 @@ static const TypeInfo aspeed_machine_types[] = { >> .name = MACHINE_TYPE_NAME("rainier-bmc"), >> .parent = TYPE_ASPEED_MACHINE, >> .class_init = aspeed_machine_rainier_class_init, >> + }, { >> + .name = MACHINE_TYPE_NAME("fuji-bmc"), >> + .parent = TYPE_ASPEED_MACHINE, >> + .class_init = aspeed_machine_fuji_class_init, >> }, { >> .name = TYPE_ASPEED_MACHINE, >> .parent = TYPE_MACHINE,
