On 12/02/2016 12:34 AM, Alexey Kardashevskiy wrote: > On 01/12/16 23:31, Cédric Le Goater wrote: >> On 12/01/2016 01:42 AM, Alastair D'Silva wrote: >>> On Wed, 2016-11-30 at 09:18 +0100, Cédric Le Goater wrote: >>>> On 11/30/2016 06:36 AM, Alastair D'Silva wrote: >>>>> From: Alastair D'Silva <alast...@d-silva.org> >>>>> >>>>> The imx25 chip provides 3 i2c buses, but they have all been named >>>>> "i2c", which makes it difficult to predict which bus a device will >>>>> be connected to when specified on the command line. >>>>> >>>>> This patch addresses the issue by naming the buses uniquely: >>>>> i2c.0 i2c.1 i2c.2 >>>>> >>>>> Signed-off-by: Alastair D'Silva <alast...@d-silva.org> >>>>> --- >>>>> hw/arm/imx25_pdk.c | 4 +--- >>>>> hw/i2c/imx_i2c.c | 6 +++++- >>>>> 2 files changed, 6 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c >>>>> index 025b608..c6f04d3 100644 >>>>> --- a/hw/arm/imx25_pdk.c >>>>> +++ b/hw/arm/imx25_pdk.c >>>>> @@ -138,9 +138,7 @@ static void imx25_pdk_init(MachineState >>>>> *machine) >>>>> * We add it here (only on qtest usage) to be able to do a >>>>> bit >>>>> * of simple qtest. See "make check" for details. >>>>> */ >>>>> - i2c_create_slave((I2CBus *)qdev_get_child_bus(DEVICE(&s- >>>>>> soc.i2c[0]), >>>>> - "i2c"), >>>>> - "ds1338", 0x68); >>>>> + i2c_create_slave(s->soc.i2c[0].bus, "ds1338", 0x68); >>>>> } >>>>> } >>>>> >>>>> diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c >>>>> index 37e5a62..7be10fb 100644 >>>>> --- a/hw/i2c/imx_i2c.c >>>>> +++ b/hw/i2c/imx_i2c.c >>>>> @@ -305,12 +305,16 @@ static const VMStateDescription >>>>> imx_i2c_vmstate = { >>>>> static void imx_i2c_realize(DeviceState *dev, Error **errp) >>>>> { >>>>> IMXI2CState *s = IMX_I2C(dev); >>>>> + static int bus_count; >>>> >>>> hmm, the static is ugly :/ >>>> >>>> Isn't there other ways to achieve this naming ? >>>> >>>> Thanks, >>>> >>>> C. >>>> >>> >>> I'm not seeing an obvious way around it. The busses are realized >>> independently (so I can't implement what we do with the aspeed i2c >>> busses), and it is named before fsl-imx25:fsl_imx25_realize() can apply >>> specific properties to the bus. >>> >>> If you have any suggestions, I'm all ears. >> >> What about that ? >> >> @@ -310,7 +310,7 @@ static void imx_i2c_realize(DeviceState >> IMX_I2C_MEM_SIZE); >> sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem); >> sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); >> - s->bus = i2c_init_bus(DEVICE(dev), "i2c"); >> + s->bus = i2c_init_bus(DEVICE(dev), NULL); >> } >> >> static void imx_i2c_class_init(ObjectClass *klass, void *data) >> >> Which should name automatically the I2C objects : > > > If you ever do migration, you'll have to specify "id" in the command line > anyway. Even in the example below the buses are numbered in messed order, > is that desired effect (may be it is, just asking :) )?
That's how it comes out with qom-tree. I haven't dug deeper to see how this was implemented. C. >> >> (qemu) info qom-tree >> /machine (imx25-pdk-machine) >> /peripheral (container) >> /soc (fsl,imx25) >> /peripheral-anon (container) >> /unattached (container) >> /device[0] (arm926-arm-cpu) >> /unnamed-gpio-in[1] (irq) >> /unnamed-gpio-in[3] (irq) >> /unnamed-gpio-in[2] (irq) >> /unnamed-gpio-in[0] (irq) >> >> /device[15] (imx.i2c) >> /imx.i2c[0] (qemu:memory-region) >> /i2c-bus.0 (i2c-bus) >> /device[17] (imx.i2c) >> /imx.i2c[0] (qemu:memory-region) >> /i2c-bus.2 (i2c-bus) >> /device[16] (imx.i2c) >> /imx.i2c[0] (qemu:memory-region) >> /i2c-bus.1 (i2c-bus) >> .... >> >> >> Cheers, >> >> C. >> > >