Re: [PATCH] aspeed/i2c: QOMify AspeedI2CBus

[Philippe Mathieu-Daudé]

On 9/7/21 9:30 AM, Cédric Le Goater wrote:
> The AST2600 SoC realize routine needs to be adapted when connecting
> the I2C bus IRQs because the bus IRQs have moved to the AspeedI2CBus
> SysBus device, one level below the Aspeed I2C controller SysBus
> device.

When did they move?

> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/i2c/aspeed_i2c.h |   8 ++-
>  hw/arm/aspeed_ast2600.c     |   7 +--
>  hw/i2c/aspeed_i2c.c         | 103 +++++++++++++++++++++++++++++-------
>  3 files changed, 93 insertions(+), 25 deletions(-)

> +static void aspeed_i2c_bus_realize(DeviceState *dev, Error **errp)
> +{
> +    AspeedI2CBus *s = ASPEED_I2C_BUS(dev);
> +    char name[32];
> +    AspeedI2CClass *aic;
> +
> +    if (!s->controller) {
> +        error_setg(errp, TYPE_ASPEED_I2C_BUS ": 'controller' link not set");
> +        return;
> +    }
> +
> +    aic = ASPEED_I2C_GET_CLASS(s->controller);
> +
> +    snprintf(name, sizeof(name), TYPE_ASPEED_I2C_BUS ".%d", s->id);

Even if this particular case is safe, it is better to use:

  g_autofree char *name = g_strdup_printf(TYPE_ASPEED_I2C_BUS ".%d",
                                          s->id);

So future developer copying your device code will be safe if they
modify the format :)

> +
> +    sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
> +
> +    s->bus = i2c_init_bus(dev, name);
> +
> +    memory_region_init_io(&s->mr, OBJECT(s), &aspeed_i2c_bus_ops,
> +                          s, name, aic->reg_size);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mr);
> +}