[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH v2 1/6] arm: Uniquely name imx25 I2C
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH v2 1/6] arm: Uniquely name imx25 I2C buses. |
Date: |
Fri, 2 Dec 2016 08:55:21 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
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 <address@hidden>
>>>>>
>>>>> 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 <address@hidden>
>>>>> ---
>>>>> 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.
>>
>
>