Re: [PATCH v4] hw/arm/aspeed: Add Fuji machine type

[Joel Stanley]

On Mon, 6 Sept 2021 at 13:31, <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/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/fb2ed12002fb/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>

Reviewed-by: Joel Stanley <joel@jms.id.au>

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

Wow, this is interesting. How did you go about testing it? Are you
sure you didn't overwrite any of the pointers?

It might be worth coming up with a better way of describing all of the
i2c buses for future machines.

Cheers,

Joel

> +    get_pca9548_channels(i2c480, 0x70, &i2c[24]);
> +    /* NOTE: The device tree skips [32, 40) in the alias numbering */
> +    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]);
> +    }