[PATCH v2 0/1] hw/arm/aspeed: Add fby35 machine type

[PATCH v2 0/1] hw/arm/aspeed: Add fby35 machine type

[Peter Delevoryas]

Resubmitting this patch without an avocado test since it takes too long. A
big part of the boot time is just due to the design of the Facebook OpenBMC
machines, but it might improve in the future, and I'll consider resubmitting
the acceptance test at that time.

I added a link to the releases page and an example of booting the image in
the commit message, since it's no longer included in the test.

Thanks,
Peter

Peter Delevoryas (1):
  hw/arm/aspeed: Add fby35 machine type

 hw/arm/aspeed.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

-- 
2.30.2

[PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Peter Delevoryas]

Add the 'fby35-bmc' machine type based on the kernel DTS[1] and userspace
i2c setup scripts[2]. Undefined values are inherited from the AST2600-EVB.

Reference images can be found in Facebook OpenBMC Github Release assets
as "fby35.mtd". [3]

You can boot the reference images as follows (fby35 uses dual-flash):

qemu-system-arm -machine fby35-bmc \
    -drive file=fby35.mtd,format=raw,if=mtd \
    -drive file=fby35.mtd,format=raw,if=mtd \
    -nographic

[1] https://github.com/facebook/openbmc-linux/blob/412d5053258007117e94b1e36015aefc1301474b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
[2] https://github.com/facebook/openbmc/blob/e2294ff5d31dd65c248fe396a385286d6d5c463d/meta-facebook/meta-fby35/recipes-fby35/plat-utils/files/setup-dev.sh
[3] https://github.com/facebook/openbmc/releases

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---

v2: Removed avocado test, updated commit message.

 hw/arm/aspeed.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index a74c13ab0f..725c169488 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -21,6 +21,7 @@
 #include "hw/misc/led.h"
 #include "hw/qdev-properties.h"
 #include "sysemu/block-backend.h"
+#include "sysemu/reset.h"
 #include "hw/loader.h"
 #include "qemu/error-report.h"
 #include "qemu/units.h"
@@ -951,6 +952,35 @@ static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
     i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67);
 }
 
+static void fby35_i2c_init(AspeedMachineState *bmc)
+{
+    AspeedSoCState *soc = &bmc->soc;
+    I2CBus *i2c[16];
+
+    for (int i = 0; i < 16; i++) {
+        i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
+    }
+
+    i2c_slave_create_simple(i2c[2], TYPE_LM75, 0x4f);
+    i2c_slave_create_simple(i2c[8], TYPE_TMP421, 0x1f);
+    /* Hotswap controller is actually supposed to be mp5920 or ltc4282. */
+    i2c_slave_create_simple(i2c[11], "adm1272", 0x44);
+    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4e);
+    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4f);
+
+    aspeed_eeprom_init(i2c[4], 0x51, 128 * KiB);
+    aspeed_eeprom_init(i2c[6], 0x51, 128 * KiB);
+    aspeed_eeprom_init(i2c[8], 0x50, 32 * KiB);
+    aspeed_eeprom_init(i2c[11], 0x51, 128 * KiB);
+    aspeed_eeprom_init(i2c[11], 0x54, 128 * KiB);
+
+    /*
+     * TODO: There is a multi-master i2c connection to an AST1030 MiniBMC on
+     * buses 0, 1, 2, 3, and 9. Source address 0x10, target address 0x20 on
+     * each.
+     */
+}
+
 static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
 {
     return ASPEED_MACHINE(obj)->mmio_exec;
@@ -1293,6 +1323,35 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
         aspeed_soc_num_cpus(amc->soc_name);
 }
 
+static void fby35_reset(MachineState *state)
+{
+    AspeedMachineState *bmc = ASPEED_MACHINE(state);
+    AspeedGPIOState *gpio = &bmc->soc.gpio;
+
+    qemu_devices_reset();
+
+    /* Board ID */
+    object_property_set_bool(OBJECT(gpio), "gpioV4", true, &error_fatal);
+    object_property_set_bool(OBJECT(gpio), "gpioV5", true, &error_fatal);
+    object_property_set_bool(OBJECT(gpio), "gpioV6", true, &error_fatal);
+    object_property_set_bool(OBJECT(gpio), "gpioV7", false, &error_fatal);
+}
+
+static void aspeed_machine_fby35_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "Facebook fby35 BMC (Cortex-A7)";
+    mc->reset      = fby35_reset;
+    amc->fmc_model = "mx66l1g45g";
+    amc->num_cs    = 2;
+    amc->macs_mask = ASPEED_MAC3_ON;
+    amc->i2c_init  = fby35_i2c_init;
+    /* FIXME: Replace this macro with something more general */
+    mc->default_ram_size = FUJI_BMC_RAM_SIZE;
+}
+
 #define AST1030_INTERNAL_FLASH_SIZE (1024 * 1024)
 /* Main SYSCLK frequency in Hz (200MHz) */
 #define SYSCLK_FRQ 200000000ULL
@@ -1411,6 +1470,10 @@ static const TypeInfo aspeed_machine_types[] = {
         .name          = MACHINE_TYPE_NAME("bletchley-bmc"),
         .parent        = TYPE_ASPEED_MACHINE,
         .class_init    = aspeed_machine_bletchley_class_init,
+    }, {
+        .name          = MACHINE_TYPE_NAME("fby35-bmc"),
+        .parent        = MACHINE_TYPE_NAME("ast2600-evb"),
+        .class_init    = aspeed_machine_fby35_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("ast1030-evb"),
         .parent         = TYPE_ASPEED_MACHINE,
-- 
2.30.2

Re: [PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Cédric Le Goater]

On 5/4/22 00:59, Peter Delevoryas wrote:
> Add the 'fby35-bmc' machine type based on the kernel DTS[1] and userspace
> i2c setup scripts[2]. Undefined values are inherited from the AST2600-EVB.
> 
> Reference images can be found in Facebook OpenBMC Github Release assets
> as "fby35.mtd". [3]
> 
> You can boot the reference images as follows (fby35 uses dual-flash):
> 
> qemu-system-arm -machine fby35-bmc \
>      -drive file=fby35.mtd,format=raw,if=mtd \
>      -drive file=fby35.mtd,format=raw,if=mtd \
>      -nographic
> 
> [1] https://github.com/facebook/openbmc-linux/blob/412d5053258007117e94b1e36015aefc1301474b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
> [2] https://github.com/facebook/openbmc/blob/e2294ff5d31dd65c248fe396a385286d6d5c463d/meta-facebook/meta-fby35/recipes-fby35/plat-utils/files/setup-dev.sh
> [3] https://github.com/facebook/openbmc/releases
> 
> Signed-off-by: Peter Delevoryas <pdel@fb.com>

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

a question below,

> ---
> 
> v2: Removed avocado test, updated commit message.
> 
>   hw/arm/aspeed.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index a74c13ab0f..725c169488 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -21,6 +21,7 @@
>   #include "hw/misc/led.h"
>   #include "hw/qdev-properties.h"
>   #include "sysemu/block-backend.h"
> +#include "sysemu/reset.h"
>   #include "hw/loader.h"
>   #include "qemu/error-report.h"
>   #include "qemu/units.h"
> @@ -951,6 +952,35 @@ static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
>       i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67);
>   }
>   
> +static void fby35_i2c_init(AspeedMachineState *bmc)
> +{
> +    AspeedSoCState *soc = &bmc->soc;
> +    I2CBus *i2c[16];
> +
> +    for (int i = 0; i < 16; i++) {
> +        i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
> +    }
> +
> +    i2c_slave_create_simple(i2c[2], TYPE_LM75, 0x4f);
> +    i2c_slave_create_simple(i2c[8], TYPE_TMP421, 0x1f);
> +    /* Hotswap controller is actually supposed to be mp5920 or ltc4282. */
> +    i2c_slave_create_simple(i2c[11], "adm1272", 0x44);
> +    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4e);
> +    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4f);
> +
> +    aspeed_eeprom_init(i2c[4], 0x51, 128 * KiB);
> +    aspeed_eeprom_init(i2c[6], 0x51, 128 * KiB);
> +    aspeed_eeprom_init(i2c[8], 0x50, 32 * KiB);
> +    aspeed_eeprom_init(i2c[11], 0x51, 128 * KiB);
> +    aspeed_eeprom_init(i2c[11], 0x54, 128 * KiB);
> +
> +    /*
> +     * TODO: There is a multi-master i2c connection to an AST1030 MiniBMC on
> +     * buses 0, 1, 2, 3, and 9. Source address 0x10, target address 0x20 on
> +     * each.
> +     */


Have you considered extending the emulation to include a AST1030 SoC
in a larger machine ?

The AST1030 SoC is merged and I think that QEMU could run a cortex-m4
CPU and a A7 CPU. A + R CPUs is supported (Xilinx boards).

Thanks,

C.


> +}
> +
>   static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>   {
>       return ASPEED_MACHINE(obj)->mmio_exec;
> @@ -1293,6 +1323,35 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
>           aspeed_soc_num_cpus(amc->soc_name);
>   }
>   
> +static void fby35_reset(MachineState *state)
> +{
> +    AspeedMachineState *bmc = ASPEED_MACHINE(state);
> +    AspeedGPIOState *gpio = &bmc->soc.gpio;
> +
> +    qemu_devices_reset();
> +
> +    /* Board ID */
> +    object_property_set_bool(OBJECT(gpio), "gpioV4", true, &error_fatal);
> +    object_property_set_bool(OBJECT(gpio), "gpioV5", true, &error_fatal);
> +    object_property_set_bool(OBJECT(gpio), "gpioV6", true, &error_fatal);
> +    object_property_set_bool(OBJECT(gpio), "gpioV7", false, &error_fatal);
> +}
> +
> +static void aspeed_machine_fby35_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
> +
> +    mc->desc       = "Facebook fby35 BMC (Cortex-A7)";
> +    mc->reset      = fby35_reset;
> +    amc->fmc_model = "mx66l1g45g";
> +    amc->num_cs    = 2;
> +    amc->macs_mask = ASPEED_MAC3_ON;
> +    amc->i2c_init  = fby35_i2c_init;
> +    /* FIXME: Replace this macro with something more general */
> +    mc->default_ram_size = FUJI_BMC_RAM_SIZE;
> +}
> +
>   #define AST1030_INTERNAL_FLASH_SIZE (1024 * 1024)
>   /* Main SYSCLK frequency in Hz (200MHz) */
>   #define SYSCLK_FRQ 200000000ULL
> @@ -1411,6 +1470,10 @@ static const TypeInfo aspeed_machine_types[] = {
>           .name          = MACHINE_TYPE_NAME("bletchley-bmc"),
>           .parent        = TYPE_ASPEED_MACHINE,
>           .class_init    = aspeed_machine_bletchley_class_init,
> +    }, {
> +        .name          = MACHINE_TYPE_NAME("fby35-bmc"),
> +        .parent        = MACHINE_TYPE_NAME("ast2600-evb"),
> +        .class_init    = aspeed_machine_fby35_class_init,
>       }, {
>           .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>           .parent         = TYPE_ASPEED_MACHINE,

Re: [PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Peter Delevoryas]

> On May 4, 2022, at 12:39 AM, Cédric Le Goater <clg@kaod.org> wrote:
> 
> On 5/4/22 00:59, Peter Delevoryas wrote:
>> Add the 'fby35-bmc' machine type based on the kernel DTS[1] and userspace
>> i2c setup scripts[2]. Undefined values are inherited from the AST2600-EVB.
>> Reference images can be found in Facebook OpenBMC Github Release assets
>> as "fby35.mtd". [3]
>> You can boot the reference images as follows (fby35 uses dual-flash):
>> qemu-system-arm -machine fby35-bmc \
>>     -drive file=fby35.mtd,format=raw,if=mtd \
>>     -drive file=fby35.mtd,format=raw,if=mtd \
>>     -nographic
>> [1] https://github.com/facebook/openbmc-linux/blob/412d5053258007117e94b1e36015aefc1301474b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
>> [2] https://github.com/facebook/openbmc/blob/e2294ff5d31dd65c248fe396a385286d6d5c463d/meta-facebook/meta-fby35/recipes-fby35/plat-utils/files/setup-dev.sh
>> [3] https://github.com/facebook/openbmc/releases
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks!

> 
> a question below,
> 
>> ---
>> v2: Removed avocado test, updated commit message.
>>  hw/arm/aspeed.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 63 insertions(+)
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index a74c13ab0f..725c169488 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -21,6 +21,7 @@
>>  #include "hw/misc/led.h"
>>  #include "hw/qdev-properties.h"
>>  #include "sysemu/block-backend.h"
>> +#include "sysemu/reset.h"
>>  #include "hw/loader.h"
>>  #include "qemu/error-report.h"
>>  #include "qemu/units.h"
>> @@ -951,6 +952,35 @@ static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
>>      i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67);
>>  }
>>  +static void fby35_i2c_init(AspeedMachineState *bmc)
>> +{
>> +    AspeedSoCState *soc = &bmc->soc;
>> +    I2CBus *i2c[16];
>> +
>> +    for (int i = 0; i < 16; i++) {
>> +        i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
>> +    }
>> +
>> +    i2c_slave_create_simple(i2c[2], TYPE_LM75, 0x4f);
>> +    i2c_slave_create_simple(i2c[8], TYPE_TMP421, 0x1f);
>> +    /* Hotswap controller is actually supposed to be mp5920 or ltc4282. */
>> +    i2c_slave_create_simple(i2c[11], "adm1272", 0x44);
>> +    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4e);
>> +    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4f);
>> +
>> +    aspeed_eeprom_init(i2c[4], 0x51, 128 * KiB);
>> +    aspeed_eeprom_init(i2c[6], 0x51, 128 * KiB);
>> +    aspeed_eeprom_init(i2c[8], 0x50, 32 * KiB);
>> +    aspeed_eeprom_init(i2c[11], 0x51, 128 * KiB);
>> +    aspeed_eeprom_init(i2c[11], 0x54, 128 * KiB);
>> +
>> +    /*
>> +     * TODO: There is a multi-master i2c connection to an AST1030 MiniBMC on
>> +     * buses 0, 1, 2, 3, and 9. Source address 0x10, target address 0x20 on
>> +     * each.
>> +     */
> 
> 
> Have you considered extending the emulation to include a AST1030 SoC
> in a larger machine ?
> 
> The AST1030 SoC is merged and I think that QEMU could run a cortex-m4
> CPU and a A7 CPU. A + R CPUs is supported (Xilinx boards).
> 

As a matter of fact yes! I tested booting our OpenBIC Zephyr kernel last week with the 1030, that worked. I also used the experimental i2c multi-master patches from Klaus to make a i2c-netdev device that connects two separate QEMU instances through a socket and sends their i2c messages back and forth. I was able to test a basic MCTP transaction.

I’m hoping to help however possible with merging Klaus’s changes, and then propose the i2c-netdev thing too.

> Thanks,
> 
> C.
> 
> 
>> +}
>> +
>>  static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>>  {
>>      return ASPEED_MACHINE(obj)->mmio_exec;
>> @@ -1293,6 +1323,35 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
>>          aspeed_soc_num_cpus(amc->soc_name);
>>  }
>>  +static void fby35_reset(MachineState *state)
>> +{
>> +    AspeedMachineState *bmc = ASPEED_MACHINE(state);
>> +    AspeedGPIOState *gpio = &bmc->soc.gpio;
>> +
>> +    qemu_devices_reset();
>> +
>> +    /* Board ID */
>> +    object_property_set_bool(OBJECT(gpio), "gpioV4", true, &error_fatal);
>> +    object_property_set_bool(OBJECT(gpio), "gpioV5", true, &error_fatal);
>> +    object_property_set_bool(OBJECT(gpio), "gpioV6", true, &error_fatal);
>> +    object_property_set_bool(OBJECT(gpio), "gpioV7", false, &error_fatal);
>> +}
>> +
>> +static void aspeed_machine_fby35_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
>> +
>> +    mc->desc       = "Facebook fby35 BMC (Cortex-A7)";
>> +    mc->reset      = fby35_reset;
>> +    amc->fmc_model = "mx66l1g45g";
>> +    amc->num_cs    = 2;
>> +    amc->macs_mask = ASPEED_MAC3_ON;
>> +    amc->i2c_init  = fby35_i2c_init;
>> +    /* FIXME: Replace this macro with something more general */
>> +    mc->default_ram_size = FUJI_BMC_RAM_SIZE;
>> +}
>> +
>>  #define AST1030_INTERNAL_FLASH_SIZE (1024 * 1024)
>>  /* Main SYSCLK frequency in Hz (200MHz) */
>>  #define SYSCLK_FRQ 200000000ULL
>> @@ -1411,6 +1470,10 @@ static const TypeInfo aspeed_machine_types[] = {
>>          .name          = MACHINE_TYPE_NAME("bletchley-bmc"),
>>          .parent        = TYPE_ASPEED_MACHINE,
>>          .class_init    = aspeed_machine_bletchley_class_init,
>> +    }, {
>> +        .name          = MACHINE_TYPE_NAME("fby35-bmc"),
>> +        .parent        = MACHINE_TYPE_NAME("ast2600-evb"),
>> +        .class_init    = aspeed_machine_fby35_class_init,
>>      }, {
>>          .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>>          .parent         = TYPE_ASPEED_MACHINE,
> 

Re: [PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Peter Delevoryas]

> On May 4, 2022, at 9:34 AM, Peter Delevoryas <pdel@fb.com> wrote:
> 
>> 
>> On May 4, 2022, at 12:39 AM, Cédric Le Goater <clg@kaod.org> wrote:
>> 
>> On 5/4/22 00:59, Peter Delevoryas wrote:
>>> Add the 'fby35-bmc' machine type based on the kernel DTS[1] and userspace
>>> i2c setup scripts[2]. Undefined values are inherited from the AST2600-EVB.
>>> Reference images can be found in Facebook OpenBMC Github Release assets
>>> as "fby35.mtd". [3]
>>> You can boot the reference images as follows (fby35 uses dual-flash):
>>> qemu-system-arm -machine fby35-bmc \
>>> -drive file=fby35.mtd,format=raw,if=mtd \
>>> -drive file=fby35.mtd,format=raw,if=mtd \
>>> -nographic
>>> [1] https://github.com/facebook/openbmc-linux/blob/412d5053258007117e94b1e36015aefc1301474b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
>>> [2] https://github.com/facebook/openbmc/blob/e2294ff5d31dd65c248fe396a385286d6d5c463d/meta-facebook/meta-fby35/recipes-fby35/plat-utils/files/setup-dev.sh
>>> [3] https://github.com/facebook/openbmc/releases
>>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> 
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> Thanks!
> 
>> 
>> a question below,
>> 
>>> ---
>>> v2: Removed avocado test, updated commit message.
>>> hw/arm/aspeed.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 63 insertions(+)
>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>> index a74c13ab0f..725c169488 100644
>>> --- a/hw/arm/aspeed.c
>>> +++ b/hw/arm/aspeed.c
>>> @@ -21,6 +21,7 @@
>>> #include "hw/misc/led.h"
>>> #include "hw/qdev-properties.h"
>>> #include "sysemu/block-backend.h"
>>> +#include "sysemu/reset.h"
>>> #include "hw/loader.h"
>>> #include "qemu/error-report.h"
>>> #include "qemu/units.h"
>>> @@ -951,6 +952,35 @@ static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
>>> i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67);
>>> }
>>> +static void fby35_i2c_init(AspeedMachineState *bmc)
>>> +{
>>> + AspeedSoCState *soc = &bmc->soc;
>>> + I2CBus *i2c[16];
>>> +
>>> + for (int i = 0; i < 16; i++) {
>>> + i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
>>> + }
>>> +
>>> + i2c_slave_create_simple(i2c[2], TYPE_LM75, 0x4f);
>>> + i2c_slave_create_simple(i2c[8], TYPE_TMP421, 0x1f);
>>> + /* Hotswap controller is actually supposed to be mp5920 or ltc4282. */
>>> + i2c_slave_create_simple(i2c[11], "adm1272", 0x44);
>>> + i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4e);
>>> + i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4f);
>>> +
>>> + aspeed_eeprom_init(i2c[4], 0x51, 128 * KiB);
>>> + aspeed_eeprom_init(i2c[6], 0x51, 128 * KiB);
>>> + aspeed_eeprom_init(i2c[8], 0x50, 32 * KiB);
>>> + aspeed_eeprom_init(i2c[11], 0x51, 128 * KiB);
>>> + aspeed_eeprom_init(i2c[11], 0x54, 128 * KiB);
>>> +
>>> + /*
>>> + * TODO: There is a multi-master i2c connection to an AST1030 MiniBMC on
>>> + * buses 0, 1, 2, 3, and 9. Source address 0x10, target address 0x20 on
>>> + * each.
>>> + */
>> 
>> 
>> Have you considered extending the emulation to include a AST1030 SoC
>> in a larger machine ?
>> 
>> The AST1030 SoC is merged and I think that QEMU could run a cortex-m4
>> CPU and a A7 CPU. A + R CPUs is supported (Xilinx boards).
>> 
> 
> As a matter of fact yes! I tested booting our OpenBIC Zephyr kernel last week with the 1030, that worked. I also used the experimental i2c multi-master patches from Klaus to make a i2c-netdev device that connects two separate QEMU instances through a socket and sends their i2c messages back and forth. I was able to test a basic MCTP transaction.
> 
> I’m hoping to help however possible with merging Klaus’s changes, and then propose the i2c-netdev thing too.

Oh wait a minute: You mean I could include both SoC’s in one machine? Oh that’s a good idea actually. Maybe I’ll look into that as an alternative to the socket thing. Still, it might be something useful to submit anyways.

> 
>> Thanks,
>> 
>> C.
>> 
>> 
>>> +}
>>> +
>>> static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>>> {
>>> return ASPEED_MACHINE(obj)->mmio_exec;
>>> @@ -1293,6 +1323,35 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
>>> aspeed_soc_num_cpus(amc->soc_name);
>>> }
>>> +static void fby35_reset(MachineState *state)
>>> +{
>>> + AspeedMachineState *bmc = ASPEED_MACHINE(state);
>>> + AspeedGPIOState *gpio = &bmc->soc.gpio;
>>> +
>>> + qemu_devices_reset();
>>> +
>>> + /* Board ID */
>>> + object_property_set_bool(OBJECT(gpio), "gpioV4", true, &error_fatal);
>>> + object_property_set_bool(OBJECT(gpio), "gpioV5", true, &error_fatal);
>>> + object_property_set_bool(OBJECT(gpio), "gpioV6", true, &error_fatal);
>>> + object_property_set_bool(OBJECT(gpio), "gpioV7", false, &error_fatal);
>>> +}
>>> +
>>> +static void aspeed_machine_fby35_class_init(ObjectClass *oc, void *data)
>>> +{
>>> + MachineClass *mc = MACHINE_CLASS(oc);
>>> + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
>>> +
>>> + mc->desc = "Facebook fby35 BMC (Cortex-A7)";
>>> + mc->reset = fby35_reset;
>>> + amc->fmc_model = "mx66l1g45g";
>>> + amc->num_cs = 2;
>>> + amc->macs_mask = ASPEED_MAC3_ON;
>>> + amc->i2c_init = fby35_i2c_init;
>>> + /* FIXME: Replace this macro with something more general */
>>> + mc->default_ram_size = FUJI_BMC_RAM_SIZE;
>>> +}
>>> +
>>> #define AST1030_INTERNAL_FLASH_SIZE (1024 * 1024)
>>> /* Main SYSCLK frequency in Hz (200MHz) */
>>> #define SYSCLK_FRQ 200000000ULL
>>> @@ -1411,6 +1470,10 @@ static const TypeInfo aspeed_machine_types[] = {
>>> .name = MACHINE_TYPE_NAME("bletchley-bmc"),
>>> .parent = TYPE_ASPEED_MACHINE,
>>> .class_init = aspeed_machine_bletchley_class_init,
>>> + }, {
>>> + .name = MACHINE_TYPE_NAME("fby35-bmc"),
>>> + .parent = MACHINE_TYPE_NAME("ast2600-evb"),
>>> + .class_init = aspeed_machine_fby35_class_init,
>>> }, {
>>> .name = MACHINE_TYPE_NAME("ast1030-evb"),
>>> .parent = TYPE_ASPEED_MACHINE,

Re: [PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Cédric Le Goater]

On 5/4/22 18:34, Peter Delevoryas wrote:
> 
>> On May 4, 2022, at 12:39 AM, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> On 5/4/22 00:59, Peter Delevoryas wrote:
>>> Add the 'fby35-bmc' machine type based on the kernel DTS[1] and userspace
>>> i2c setup scripts[2]. Undefined values are inherited from the AST2600-EVB.
>>> Reference images can be found in Facebook OpenBMC Github Release assets
>>> as "fby35.mtd". [3]
>>> You can boot the reference images as follows (fby35 uses dual-flash):
>>> qemu-system-arm -machine fby35-bmc \
>>>      -drive file=fby35.mtd,format=raw,if=mtd \
>>>      -drive file=fby35.mtd,format=raw,if=mtd \
>>>      -nographic
>>> [1] https://github.com/facebook/openbmc-linux/blob/412d5053258007117e94b1e36015aefc1301474b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
>>> [2] https://github.com/facebook/openbmc/blob/e2294ff5d31dd65c248fe396a385286d6d5c463d/meta-facebook/meta-fby35/recipes-fby35/plat-utils/files/setup-dev.sh
>>> [3] https://github.com/facebook/openbmc/releases
>>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>>
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> Thanks!

Could you please send a v2 with an update of the documentation ?
or a follow up ? no need to resend the first patch unless you
want to change something. A one-liner in :

   https://qemu.readthedocs.io/en/latest/system/arm/aspeed.html

[ ... ]

> As a matter of fact yes! I tested booting our OpenBIC Zephyr kernel last week with the 1030, that worked. I also used the experimental i2c multi-master patches from Klaus to make a i2c-netdev device that connects two separate QEMU instances through a socket and sends their i2c messages back and forth. I was able to test a basic MCTP transaction.

Nice ! And do you need anything special from the I2C Aspeed models ?
Apart from :

  https://patchwork.ozlabs.org/project/qemu-devel/list/?series=292928

> I’m hoping to help however possible with merging Klaus’s changes, 

They don't need much work. Klaus doesn't have the datasheet, so we
should help him with the changes requiring some internal knowledge.

> and then propose the i2c-netdev thing too.

I was going to ask since I didn't see any models for such devices.

I hope to do something similar with the usb-net device but it needs
fixes first.

Thanks,

C.

Re: [PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Cédric Le Goater]

>>> Have you considered extending the emulation to include a AST1030 SoC
>>> in a larger machine ?
>>>
>>> The AST1030 SoC is merged and I think that QEMU could run a cortex-m4
>>> CPU and a A7 CPU. A + R CPUs is supported (Xilinx boards).
>>>
>>
>> As a matter of fact yes! I tested booting our OpenBIC Zephyr kernel last week with the 1030, that worked. I also used the experimental i2c multi-master patches from Klaus to make a i2c-netdev device that connects two separate QEMU instances through a socket and sends their i2c messages back and forth. I was able to test a basic MCTP transaction.
>>
>> I’m hoping to help however possible with merging Klaus’s changes, and then propose the i2c-netdev thing too.
> 
> Oh wait a minute: You mean I could include both SoC’s in one machine? 

Yes. We would need to extend the Aspeed machines to instantiate
2 different SoCs (or more). This is something quite interesting
to design.

> Oh that’s a good idea actually. Maybe I’ll look into that as an alternative to the socket thing. Still, it might be something useful to submit anyways.

Yes. Please do.

Thanks,

C.

Re: [PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Klaus Jensen]

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On May  6 08:30, Cédric Le Goater wrote:
> On 5/4/22 18:34, Peter Delevoryas wrote:
> Nice ! And do you need anything special from the I2C Aspeed models ?
> Apart from :
> 
>  https://patchwork.ozlabs.org/project/qemu-devel/list/?series=292928
> 
> > I’m hoping to help however possible with merging Klaus’s changes,
> 
> They don't need much work. Klaus doesn't have the datasheet, so we
> should help him with the changes requiring some internal knowledge.
> 

I got a little stuck trying to do some QOM interface stuff, but I'm
picking it up for a v2 now.


Cheers,
Klaus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Peter Delevoryas]

> On May 5, 2022, at 11:30 PM, Cédric Le Goater <clg@kaod.org> wrote:
> 
> On 5/4/22 18:34, Peter Delevoryas wrote:
>>> On May 4, 2022, at 12:39 AM, Cédric Le Goater <clg@kaod.org> wrote:
>>> 
>>> On 5/4/22 00:59, Peter Delevoryas wrote:
>>>> Add the 'fby35-bmc' machine type based on the kernel DTS[1] and userspace
>>>> i2c setup scripts[2]. Undefined values are inherited from the AST2600-EVB.
>>>> Reference images can be found in Facebook OpenBMC Github Release assets
>>>> as "fby35.mtd". [3]
>>>> You can boot the reference images as follows (fby35 uses dual-flash):
>>>> qemu-system-arm -machine fby35-bmc \
>>>> -drive file=fby35.mtd,format=raw,if=mtd \
>>>> -drive file=fby35.mtd,format=raw,if=mtd \
>>>> -nographic
>>>> [1] https://github.com/facebook/openbmc-linux/blob/412d5053258007117e94b1e36015aefc1301474b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
>>>> [2] https://github.com/facebook/openbmc/blob/e2294ff5d31dd65c248fe396a385286d6d5c463d/meta-facebook/meta-fby35/recipes-fby35/plat-utils/files/setup-dev.sh
>>>> [3] https://github.com/facebook/openbmc/releases
>>>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>>> 
>>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>> Thanks!
> 
> Could you please send a v2 with an update of the documentation ?
> or a follow up ? no need to resend the first patch unless you
> want to change something. A one-liner in :
> 
> https://qemu.readthedocs.io/en/latest/system/arm/aspeed.html 
> [ ... ]
> 

Oh, yeah I’ll send a follow-up, I don’t have anything else to change on the first patch I suppose.

>> As a matter of fact yes! I tested booting our OpenBIC Zephyr kernel last week with the 1030, that worked. I also used the experimental i2c multi-master patches from Klaus to make a i2c-netdev device that connects two separate QEMU instances through a socket and sends their i2c messages back and forth. I was able to test a basic MCTP transaction.
> 
> Nice ! And do you need anything special from the I2C Aspeed models ?
> Apart from :
> 
> https://patchwork.ozlabs.org/project/qemu-devel/list/?series=292928 

Nope, I don’t need anything else, those exact patches were sufficient. I didn’t need any additional
modifications to the i2c send_async model Klaus made.

My code was not very sophisticated though, it just assumes ACK for every byte in an i2c message,
buffers them, and then sends the i2c message as a UDP packet. The receiver buffers the i2c message
and asynchronously dequeues each byte through send_async (after mastering the bus). I’ll post the
diff as an RFC soon, in case it helps with evaluating Klaus’s changes.

https://github.com/facebook/openbmc-qemu/commit/c03730986d521ca2a06fd4570d29f2452d13f8da

The problem I foresee is that people are interested in NACK emulation, Smbus ARP, and generally,
emulating an i2c _bus_ with a multicast UDP socket and not point-to-point i2c device connections.
That way you can have multiple QEMU’s listen to a single multicast UDP socket or something
and filter out messages that are directed at other i2c devices, something like that. And, probably
send each byte individually instead of buffering and sending. But, buffering stuff works for
my purposes for now, since I’m more interested in enabling higher level MCTP testing than
simulating i2c multi-master contention.

>> I’m hoping to help however possible with merging Klaus’s changes, 
> 
> They don't need much work. Klaus doesn't have the datasheet, so we
> should help him with the changes requiring some internal knowledge.
> 
>> and then propose the i2c-netdev thing too.
> 
> I was going to ask since I didn't see any models for such devices.
> 
> I hope to do something similar with the usb-net device but it needs
> fixes first.

Oh I wasn’t aware anyone was doing something similar, I’ll have to look at how you did
that.

> 
> Thanks,
> 
> C.

Re: [PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Cédric Le Goater]

Hello Peter,

On 5/4/22 00:59, Peter Delevoryas wrote:
> Add the 'fby35-bmc' machine type based on the kernel DTS[1] and userspace
> i2c setup scripts[2]. Undefined values are inherited from the AST2600-EVB.
> 
> Reference images can be found in Facebook OpenBMC Github Release assets
> as "fby35.mtd". [3]
> 
> You can boot the reference images as follows (fby35 uses dual-flash):
> 
> qemu-system-arm -machine fby35-bmc \
>      -drive file=fby35.mtd,format=raw,if=mtd \
>      -drive file=fby35.mtd,format=raw,if=mtd \
>      -nographic
> 
> [1] https://github.com/facebook/openbmc-linux/blob/412d5053258007117e94b1e36015aefc1301474b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
> [2] https://github.com/facebook/openbmc/blob/e2294ff5d31dd65c248fe396a385286d6d5c463d/meta-facebook/meta-fby35/recipes-fby35/plat-utils/files/setup-dev.sh
> [3] https://github.com/facebook/openbmc/releases
> 
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---
> 
> v2: Removed avocado test, updated commit message.
> 
>   hw/arm/aspeed.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index a74c13ab0f..725c169488 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -21,6 +21,7 @@
>   #include "hw/misc/led.h"
>   #include "hw/qdev-properties.h"
>   #include "sysemu/block-backend.h"
> +#include "sysemu/reset.h"
>   #include "hw/loader.h"
>   #include "qemu/error-report.h"
>   #include "qemu/units.h"
> @@ -951,6 +952,35 @@ static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
>       i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67);
>   }
>   
> +static void fby35_i2c_init(AspeedMachineState *bmc)
> +{
> +    AspeedSoCState *soc = &bmc->soc;
> +    I2CBus *i2c[16];
> +
> +    for (int i = 0; i < 16; i++) {
> +        i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
> +    }
> +
> +    i2c_slave_create_simple(i2c[2], TYPE_LM75, 0x4f);
> +    i2c_slave_create_simple(i2c[8], TYPE_TMP421, 0x1f);
> +    /* Hotswap controller is actually supposed to be mp5920 or ltc4282. */
> +    i2c_slave_create_simple(i2c[11], "adm1272", 0x44);
> +    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4e);
> +    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4f);
> +
> +    aspeed_eeprom_init(i2c[4], 0x51, 128 * KiB);
> +    aspeed_eeprom_init(i2c[6], 0x51, 128 * KiB);
> +    aspeed_eeprom_init(i2c[8], 0x50, 32 * KiB);
> +    aspeed_eeprom_init(i2c[11], 0x51, 128 * KiB);
> +    aspeed_eeprom_init(i2c[11], 0x54, 128 * KiB);
> +
> +    /*
> +     * TODO: There is a multi-master i2c connection to an AST1030 MiniBMC on
> +     * buses 0, 1, 2, 3, and 9. Source address 0x10, target address 0x20 on
> +     * each.
> +     */
> +}
> +
>   static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>   {
>       return ASPEED_MACHINE(obj)->mmio_exec;
> @@ -1293,6 +1323,35 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
>           aspeed_soc_num_cpus(amc->soc_name);
>   }
>   
> +static void fby35_reset(MachineState *state)
> +{
> +    AspeedMachineState *bmc = ASPEED_MACHINE(state);
> +    AspeedGPIOState *gpio = &bmc->soc.gpio;
> +
> +    qemu_devices_reset();
> +
> +    /* Board ID */
> +    object_property_set_bool(OBJECT(gpio), "gpioV4", true, &error_fatal);
> +    object_property_set_bool(OBJECT(gpio), "gpioV5", true, &error_fatal);
> +    object_property_set_bool(OBJECT(gpio), "gpioV6", true, &error_fatal);
> +    object_property_set_bool(OBJECT(gpio), "gpioV7", false, &error_fatal);
> +}
> +
> +static void aspeed_machine_fby35_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
> +
> +    mc->desc       = "Facebook fby35 BMC (Cortex-A7)";
> +    mc->reset      = fby35_reset;
> +    amc->fmc_model = "mx66l1g45g";
> +    amc->num_cs    = 2;
> +    amc->macs_mask = ASPEED_MAC3_ON;
> +    amc->i2c_init  = fby35_i2c_init;
> +    /* FIXME: Replace this macro with something more general */
> +    mc->default_ram_size = FUJI_BMC_RAM_SIZE;
> +}
> +
>   #define AST1030_INTERNAL_FLASH_SIZE (1024 * 1024)
>   /* Main SYSCLK frequency in Hz (200MHz) */
>   #define SYSCLK_FRQ 200000000ULL
> @@ -1411,6 +1470,10 @@ static const TypeInfo aspeed_machine_types[] = {
>           .name          = MACHINE_TYPE_NAME("bletchley-bmc"),
>           .parent        = TYPE_ASPEED_MACHINE,
>           .class_init    = aspeed_machine_bletchley_class_init,
> +    }, {
> +        .name          = MACHINE_TYPE_NAME("fby35-bmc"),
> +        .parent        = MACHINE_TYPE_NAME("ast2600-evb"),

Machine "fby35-bmc" inherits from the "ast2600-evb" machine definitions
which means that any changes in "ast2600-evb" will silently modify
"fby35-bmc". Is that what you want ?

Thanks,

C.

> +        .class_init    = aspeed_machine_fby35_class_init,
>       }, {
>           .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>           .parent         = TYPE_ASPEED_MACHINE,

Re: [PATCH v2 1/1] hw/arm/aspeed: Add fby35 machine type

[Peter Delevoryas]

> On May 25, 2022, at 1:41 AM, Cédric Le Goater <clg@kaod.org> wrote:
> 
> Hello Peter,
> 
>> On 5/4/22 00:59, Peter Delevoryas wrote:
>> Add the 'fby35-bmc' machine type based on the kernel DTS[1] and userspace
>> i2c setup scripts[2]. Undefined values are inherited from the AST2600-EVB.
>> Reference images can be found in Facebook OpenBMC Github Release assets
>> as "fby35.mtd". [3]
>> You can boot the reference images as follows (fby35 uses dual-flash):
>> qemu-system-arm -machine fby35-bmc \
>>     -drive file=fby35.mtd,format=raw,if=mtd \
>>     -drive file=fby35.mtd,format=raw,if=mtd \
>>     -nographic
>> [1] https://github.com/facebook/openbmc-linux/blob/412d5053258007117e94b1e36015aefc1301474b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts
>> [2] https://github.com/facebook/openbmc/blob/e2294ff5d31dd65c248fe396a385286d6d5c463d/meta-facebook/meta-fby35/recipes-fby35/plat-utils/files/setup-dev.sh
>> [3] https://github.com/facebook/openbmc/releases
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> ---
>> v2: Removed avocado test, updated commit message.
>>  hw/arm/aspeed.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 63 insertions(+)
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index a74c13ab0f..725c169488 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -21,6 +21,7 @@
>>  #include "hw/misc/led.h"
>>  #include "hw/qdev-properties.h"
>>  #include "sysemu/block-backend.h"
>> +#include "sysemu/reset.h"
>>  #include "hw/loader.h"
>>  #include "qemu/error-report.h"
>>  #include "qemu/units.h"
>> @@ -951,6 +952,35 @@ static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
>>      i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67);
>>  }
>>  +static void fby35_i2c_init(AspeedMachineState *bmc)
>> +{
>> +    AspeedSoCState *soc = &bmc->soc;
>> +    I2CBus *i2c[16];
>> +
>> +    for (int i = 0; i < 16; i++) {
>> +        i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
>> +    }
>> +
>> +    i2c_slave_create_simple(i2c[2], TYPE_LM75, 0x4f);
>> +    i2c_slave_create_simple(i2c[8], TYPE_TMP421, 0x1f);
>> +    /* Hotswap controller is actually supposed to be mp5920 or ltc4282. */
>> +    i2c_slave_create_simple(i2c[11], "adm1272", 0x44);
>> +    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4e);
>> +    i2c_slave_create_simple(i2c[12], TYPE_LM75, 0x4f);
>> +
>> +    aspeed_eeprom_init(i2c[4], 0x51, 128 * KiB);
>> +    aspeed_eeprom_init(i2c[6], 0x51, 128 * KiB);
>> +    aspeed_eeprom_init(i2c[8], 0x50, 32 * KiB);
>> +    aspeed_eeprom_init(i2c[11], 0x51, 128 * KiB);
>> +    aspeed_eeprom_init(i2c[11], 0x54, 128 * KiB);
>> +
>> +    /*
>> +     * TODO: There is a multi-master i2c connection to an AST1030 MiniBMC on
>> +     * buses 0, 1, 2, 3, and 9. Source address 0x10, target address 0x20 on
>> +     * each.
>> +     */
>> +}
>> +
>>  static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>>  {
>>      return ASPEED_MACHINE(obj)->mmio_exec;
>> @@ -1293,6 +1323,35 @@ static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
>>          aspeed_soc_num_cpus(amc->soc_name);
>>  }
>>  +static void fby35_reset(MachineState *state)
>> +{
>> +    AspeedMachineState *bmc = ASPEED_MACHINE(state);
>> +    AspeedGPIOState *gpio = &bmc->soc.gpio;
>> +
>> +    qemu_devices_reset();
>> +
>> +    /* Board ID */
>> +    object_property_set_bool(OBJECT(gpio), "gpioV4", true, &error_fatal);
>> +    object_property_set_bool(OBJECT(gpio), "gpioV5", true, &error_fatal);
>> +    object_property_set_bool(OBJECT(gpio), "gpioV6", true, &error_fatal);
>> +    object_property_set_bool(OBJECT(gpio), "gpioV7", false, &error_fatal);
>> +}
>> +
>> +static void aspeed_machine_fby35_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
>> +
>> +    mc->desc       = "Facebook fby35 BMC (Cortex-A7)";
>> +    mc->reset      = fby35_reset;
>> +    amc->fmc_model = "mx66l1g45g";
>> +    amc->num_cs    = 2;
>> +    amc->macs_mask = ASPEED_MAC3_ON;
>> +    amc->i2c_init  = fby35_i2c_init;
>> +    /* FIXME: Replace this macro with something more general */
>> +    mc->default_ram_size = FUJI_BMC_RAM_SIZE;
>> +}
>> +
>>  #define AST1030_INTERNAL_FLASH_SIZE (1024 * 1024)
>>  /* Main SYSCLK frequency in Hz (200MHz) */
>>  #define SYSCLK_FRQ 200000000ULL
>> @@ -1411,6 +1470,10 @@ static const TypeInfo aspeed_machine_types[] = {
>>          .name          = MACHINE_TYPE_NAME("bletchley-bmc"),
>>          .parent        = TYPE_ASPEED_MACHINE,
>>          .class_init    = aspeed_machine_bletchley_class_init,
>> +    }, {
>> +        .name          = MACHINE_TYPE_NAME("fby35-bmc"),
>> +        .parent        = MACHINE_TYPE_NAME("ast2600-evb"),
> 
> Machine "fby35-bmc" inherits from the "ast2600-evb" machine definitions
> which means that any changes in "ast2600-evb" will silently modify
> "fby35-bmc". Is that what you want ?

When I wrote this, yes, that’s what I intended. i wanted to make the machine definition as simple as possible.

I don’t know what the HW straps should be, so I wanted to inherit them from the EVB.

 But thinking again, I’m not completely sure.

 I think for now, I’m ok with it, but if necessary, changes to the EVB should feel free to break things in this model. Once I know the HW strap values, maybe I could remove the dependency.

> 
> Thanks,
> 
> C.
> 
>> +        .class_init    = aspeed_machine_fby35_class_init,
>>      }, {
>>          .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>>          .parent         = TYPE_ASPEED_MACHINE,
>