[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] aspeed/i2c: QOMify AspeedI2CBus
From: |
Cédric Le Goater |
Subject: |
Re: [PATCH] aspeed/i2c: QOMify AspeedI2CBus |
Date: |
Tue, 7 Sep 2021 10:08:33 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
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 <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 :)
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);
>> +}