On 9/7/21 9:40 AM, Philippe Mathieu-Daudé wrote: > 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?
With this patch. I will rephrase. > >> Signed-off-by: Cédric Le Goater <[email protected]> >> --- >> 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 :) Yeah. I kind of like snprintf but I agree g_strdup_printf() is as good with the g_autofree variables. I will change the patches. Thanks, C. >> + >> + 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); >> +}
