qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 1/6] arm: Uniquely name imx25 I2C buses.


From: Alexey Kardashevskiy
Subject: Re: [Qemu-devel] [PATCH v2 1/6] arm: Uniquely name imx25 I2C buses.
Date: Fri, 2 Dec 2016 10:34:56 +1100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

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 :) )?

> 
>       (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. 
> 


-- 
Alexey



reply via email to

[Prev in Thread] Current Thread [Next in Thread]