Re: [PATCH v4 21/24] ppc/ppc405: Use an explicit I2C object

[BALATON Zoltan]

On Tue, 9 Aug 2022, Cédric Le Goater wrote:
> Having an explicit I2C model object will help if one day we want to
> add I2C devices on the bus from the machine init routine.
>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/ppc/ppc405.h    |  2 ++
> hw/ppc/ppc405_uc.c | 10 ++++++++--
> 2 files changed, 10 insertions(+), 2 deletions(-)

This just seems to turn 2 lines into 10 for no apparent reason so I don't 
see what is this patch is good for but I don't really mind. I this is 
really needed to add an i2c device then maybe we have a problem in QEMU 
which makes it more difficult to access things than it should be. There 
should be a way to get a bus without havnig to have a pointer to the 
controller or again we need a way to navigate the qom three here.

But this is only a comment not a complaint as I've said above I don't 
mind.

Regards,
BALATON Zoltan

> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
> index 8e67ad0b72c3..56881b181ba1 100644
> --- a/hw/ppc/ppc405.h
> +++ b/hw/ppc/ppc405.h
> @@ -28,6 +28,7 @@
> #include "qom/object.h"
> #include "hw/ppc/ppc4xx.h"
> #include "hw/intc/ppc-uic.h"
> +#include "hw/i2c/ppc4xx_i2c.h"
>
> #define PPC405EP_SDRAM_BASE 0x00000000
> #define PPC405EP_NVRAM_BASE 0xF0000000
> @@ -241,6 +242,7 @@ struct Ppc405SoCState {
>     Ppc405OcmState ocm;
>     Ppc405GpioState gpio;
>     Ppc405DmaState dma;
> +    PPC4xxI2CState i2c;
>     Ppc405EbcState ebc;
>     Ppc405OpbaState opba;
>     Ppc405PobState pob;
> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
> index 328224bb0620..1045f5f13e6c 100644
> --- a/hw/ppc/ppc405_uc.c
> +++ b/hw/ppc/ppc405_uc.c
> @@ -1363,6 +1363,8 @@ static void ppc405_soc_instance_init(Object *obj)
>
>     object_initialize_child(obj, "dma", &s->dma, TYPE_PPC405_DMA);
>
> +    object_initialize_child(obj, "i2c", &s->i2c, TYPE_PPC4xx_I2C);
> +
>     object_initialize_child(obj, "ebc", &s->ebc, TYPE_PPC405_EBC);
>
>     object_initialize_child(obj, "opba", &s->opba, TYPE_PPC405_OPBA);
> @@ -1456,8 +1458,12 @@ static void ppc405_soc_realize(DeviceState *dev, Error 
> **errp)
>     }
>
>     /* I2C controller */
> -    sysbus_create_simple(TYPE_PPC4xx_I2C, 0xef600500,
> -                         qdev_get_gpio_in(DEVICE(&s->uic), 2));
> +    if (!sysbus_realize(SYS_BUS_DEVICE(&s->i2c), errp)) {
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, 0xef600500);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->uic), 2));
>
>     /* GPIO */
>     if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {