All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] hw/arm/aspeed: Add Fuji machine type
@ 2021-09-05 18:55 pdel
  2021-09-05 20:33 ` Cédric Le Goater
  2021-09-06  9:15 ` Philippe Mathieu-Daudé
  0 siblings, 2 replies; 4+ messages in thread
From: pdel @ 2021-09-05 18:55 UTC (permalink / raw)
  Cc: clg, joel, qemu-devel, qemu-arm, patrick, f4bug, peter.maydell,
	andrew, Peter Delevoryas

From: Peter Delevoryas <pdel@fb.com>

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

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

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 <pdel@fb.com>
---
 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) {
+    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]);
+    // 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);
+};
+
 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,
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] hw/arm/aspeed: Add Fuji machine type
  2021-09-05 18:55 [PATCH v3] hw/arm/aspeed: Add Fuji machine type pdel
@ 2021-09-05 20:33 ` Cédric Le Goater
  2021-09-06  9:15 ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 4+ messages in thread
From: Cédric Le Goater @ 2021-09-05 20:33 UTC (permalink / raw)
  To: pdel; +Cc: peter.maydell, andrew, f4bug, qemu-devel, patrick, qemu-arm, joel

On 9/5/21 8:55 PM, pdel@fb.com wrote:
> From: Peter Delevoryas <pdel@fb.com>
> 
> 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
> 
> 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
> 
> 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 <pdel@fb.com>

There are a few checkpatch warnings that I fixed directly.
Anyhow,

Reviewed-by: Cédric Le Goater <clg@kaod.org>

I will wait a little before resending the PR :)

I have pushed on my tree [1] the fuji machine and a few fixes for
the ADC model. This is still WIP and if you have some ADC tests
that come to mind, they would be welcomed.

Thanks,

C.

[1]  https://github.com/legoater/qemu/commits/aspeed-6.2



> ---
>  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) {
> +    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]);
> +    // 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);
> +};
> +
>  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,
> 



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] hw/arm/aspeed: Add Fuji machine type
  2021-09-05 18:55 [PATCH v3] hw/arm/aspeed: Add Fuji machine type pdel
  2021-09-05 20:33 ` Cédric Le Goater
@ 2021-09-06  9:15 ` Philippe Mathieu-Daudé
  2021-09-06 13:16   ` Peter Delevoryas
  1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-06  9:15 UTC (permalink / raw)
  To: pdel; +Cc: peter.maydell, andrew, qemu-devel, patrick, qemu-arm, clg, joel

On 9/5/21 8:55 PM, pdel@fb.com wrote:
> From: Peter Delevoryas <pdel@fb.com>
> 
> 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

> 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)

> 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 <pdel@fb.com>
> ---
>  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

   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);

> +};
> +
>  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,
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] hw/arm/aspeed: Add Fuji machine type
  2021-09-06  9:15 ` Philippe Mathieu-Daudé
@ 2021-09-06 13:16   ` Peter Delevoryas
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Delevoryas @ 2021-09-06 13:16 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Cédric Le Goater, Joel Stanley, Cameron Esfahani via,
	qemu-arm, patrick, Peter Maydell, Andrew Jeffery


> On Sep 6, 2021, at 2:15 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
> On 9/5/21 8:55 PM, pdel@fb.com wrote:
>> From: Peter Delevoryas <pdel@fb.com>
>> 
>> 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 <pdel@fb.com>
>> ---
>> 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,


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-06 13:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-05 18:55 [PATCH v3] hw/arm/aspeed: Add Fuji machine type pdel
2021-09-05 20:33 ` Cédric Le Goater
2021-09-06  9:15 ` Philippe Mathieu-Daudé
2021-09-06 13:16   ` Peter Delevoryas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.