Re: [PATCH v2 2/2] ppc/pnv: Connect I2C controller model to powernv9 chip

[Cédric Le Goater]

On 10/12/23 22:08, Glenn Miles wrote:
> From: Cédric Le Goater <clg@kaod.org>
>
> Wires up three I2C controller instances to the powernv9 chip
> XSCOM address space.
>
> Each controller instance is wired up to a single I2C bus of
> its own.  No other I2C devices are connected to the buses
> at this time.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> [milesg: Split wiring from addition of model itself]
> [milesg: Added new commit message]
> [milesg: Moved hardcoded attributes into PnvChipClass]
> Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> ---
>
> Changes in v2:
>      - Moved some hardcoded attributes into PnvChipClass
>
>   hw/ppc/pnv.c              | 29 +++++++++++++++++++++++++++++
>   include/hw/ppc/pnv_chip.h |  8 ++++++++
>   2 files changed, 37 insertions(+)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index eb54f93986..7db6f3abe5 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1438,6 +1438,10 @@ static void pnv_chip_power9_instance_init(Object *obj)
>           object_initialize_child(obj, "pec[*]", &chip9->pecs[i],
>                                   TYPE_PNV_PHB4_PEC);
>       }
> +
> +    for (i = 0; i < pcc->i2c_num_engines; i++) {
> +        object_initialize_child(obj, "i2c[*]", &chip9->i2c[i], TYPE_PNV_I2C);
> +    }
>   }
>   
>   static void pnv_chip_quad_realize_one(PnvChip *chip, PnvQuad *eq,
> @@ -1510,6 +1514,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
> Error **errp)
>       PnvChip *chip = PNV_CHIP(dev);
>       Pnv9Psi *psi9 = &chip9->psi;
>       Error *local_err = NULL;
> +    int i;
>   
>       /* XSCOM bridge is first */
>       pnv_xscom_realize(chip, PNV9_XSCOM_SIZE, &local_err);
> @@ -1613,6 +1618,28 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
> Error **errp)
>           error_propagate(errp, local_err);
>           return;
>       }
> +
> +    /*
> +     * I2C
> +     * TODO: The number of busses is specific to each platform


I would remove the TODO now,

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> +     */
> +    for (i = 0; i < pcc->i2c_num_engines; i++) {
> +        Object *obj =  OBJECT(&chip9->i2c[i]);
> +
> +        object_property_set_int(obj, "engine", i + 1, &error_fatal);
> +        object_property_set_int(obj, "num-busses", pcc->i2c_num_ports,
> +                                &error_fatal);
> +        object_property_set_link(obj, "chip", OBJECT(chip), &error_abort);
> +        if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> +            return;
> +        }
> +        pnv_xscom_add_subregion(chip, PNV9_XSCOM_I2CM_BASE +
> +                               chip9->i2c[i].engine * PNV9_XSCOM_I2CM_SIZE,
> +                                &chip9->i2c[i].xscom_regs);
> +        qdev_connect_gpio_out(DEVICE(&chip9->i2c[i]), 0,
> +                              qdev_get_gpio_in(DEVICE(&chip9->psi),
> +                                               PSIHB9_IRQ_SBE_I2C));
> +    }
>   }
>   
>   static uint32_t pnv_chip_power9_xscom_pcba(PnvChip *chip, uint64_t addr)
> @@ -1640,6 +1667,8 @@ static void pnv_chip_power9_class_init(ObjectClass 
> *klass, void *data)
>       k->xscom_pcba = pnv_chip_power9_xscom_pcba;
>       dc->desc = "PowerNV Chip POWER9";
>       k->num_pecs = PNV9_CHIP_MAX_PEC;
> +    k->i2c_num_engines = PNV9_CHIP_MAX_I2C;
> +    k->i2c_num_ports = PNV9_CHIP_MAX_I2C_PORTS;
>   
>       device_class_set_parent_realize(dc, pnv_chip_power9_realize,
>                                       &k->parent_realize);
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> index 53e1d921d7..90cfbad1a5 100644
> --- a/include/hw/ppc/pnv_chip.h
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -9,6 +9,7 @@
>   #include "hw/ppc/pnv_psi.h"
>   #include "hw/ppc/pnv_sbe.h"
>   #include "hw/ppc/pnv_xive.h"
> +#include "hw/ppc/pnv_i2c.h"
>   #include "hw/sysbus.h"
>   
>   OBJECT_DECLARE_TYPE(PnvChip, PnvChipClass,
> @@ -86,6 +87,10 @@ struct Pnv9Chip {
>   
>   #define PNV9_CHIP_MAX_PEC 3
>       PnvPhb4PecState pecs[PNV9_CHIP_MAX_PEC];
> +
> +#define PNV9_CHIP_MAX_I2C 3
> +#define PNV9_CHIP_MAX_I2C_PORTS 1
> +    PnvI2C      i2c[PNV9_CHIP_MAX_I2C];
>   };
>   
>   /*
> @@ -130,6 +135,9 @@ struct PnvChipClass {
>       uint32_t     num_pecs;
>       uint32_t     num_phbs;
>   
> +    uint32_t     i2c_num_engines;
> +    uint32_t     i2c_num_ports;
> +
>       DeviceRealize parent_realize;
>   
>       uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);