Re: [PATCH v6 3/3] hw/ppc: N1 chiplet wiring

[Nicholas Piggin]

On Tue Nov 28, 2023 at 3:13 AM AEST, Chalapathi V wrote:
> This part of the patchset connects the nest1 chiplet model to p10 chip.
>
> Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
> ---
>  include/hw/ppc/pnv_chip.h |  2 ++
>  hw/ppc/pnv.c              | 15 +++++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> index 0ab5c42308..9b06c8d87c 100644
> --- a/include/hw/ppc/pnv_chip.h
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -4,6 +4,7 @@
>  #include "hw/pci-host/pnv_phb4.h"
>  #include "hw/ppc/pnv_core.h"
>  #include "hw/ppc/pnv_homer.h"
> +#include "hw/ppc/pnv_n1_chiplet.h"
>  #include "hw/ppc/pnv_lpc.h"
>  #include "hw/ppc/pnv_occ.h"
>  #include "hw/ppc/pnv_psi.h"
> @@ -113,6 +114,7 @@ struct Pnv10Chip {
>      PnvOCC       occ;
>      PnvSBE       sbe;
>      PnvHomer     homer;
> +    PnvN1Chiplet     n1_chiplet;
>  
>      uint32_t     nr_quads;
>      PnvQuad      *quads;
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 0297871bdd..6cf1f3319f 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1680,6 +1680,8 @@ static void pnv_chip_power10_instance_init(Object *obj)
>      object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
>      object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
>      object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
> +    object_initialize_child(obj, "n1_chiplet", &chip10->n1_chiplet,
> +                            TYPE_PNV_N1_CHIPLET);

Another very small nit, we seem to have convention of minus rather than
underscore for these names, so n1-chiplet fits better.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

>  
>      chip->num_pecs = pcc->num_pecs;
>  
> @@ -1849,6 +1851,19 @@ static void pnv_chip_power10_realize(DeviceState *dev, 
> Error **errp)
>      memory_region_add_subregion(get_system_memory(), PNV10_HOMER_BASE(chip),
>                                  &chip10->homer.regs);
>  
> +    /* N1 chiplet */
> +    if (!qdev_realize(DEVICE(&chip10->n1_chiplet), NULL, errp)) {
> +        return;
> +    }
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_N1_CHIPLET_CTRL_REGS_BASE,
> +             &chip10->n1_chiplet.nest_pervasive.xscom_ctrl_regs);
> +
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_N1_PB_SCOM_EQ_BASE,
> +                           &chip10->n1_chiplet.xscom_pb_eq_regs);
> +
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_N1_PB_SCOM_ES_BASE,
> +                           &chip10->n1_chiplet.xscom_pb_es_regs);
> +
>      /* PHBs */
>      pnv_chip_power10_phb_realize(chip, &local_err);
>      if (local_err) {