Re: [PATCH 3/4] pnv_phb4_pec: Move pnv_phb4_get_pec() to rightful file

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 3/2/23 13:37, Frederic Barrat wrote:
> The function pnv_phb4_get_pec() exposes some internals of the PEC and
> PHB logic, yet it was in the higher level hw/ppc/pnv.c file for
> historical reasons: P8 implements the PHBs from pnv.c directly, but on
> P9/P10, it's done through the CEC model, which has its own file. So
> move pnv_phb4_get_pec() to hw/pci-host/pnv_phb4_pec.c, where it fits
> naturally.
> 
> While at it, replace the PnvPHB4 parameter by the PnvPHB front-end,
> since it has all the information needed and simplify it a bit.
> 
> No functional changes.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/pci-host/pnv_phb4_pec.c     | 40 +++++++++++++++++++++++++++++++
>   hw/ppc/pnv.c                   | 44 +---------------------------------
>   include/hw/pci-host/pnv_phb4.h |  1 +
>   3 files changed, 42 insertions(+), 43 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 6c9b386069..6e2e5ae186 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -112,6 +112,46 @@ static const MemoryRegionOps pnv_pec_pci_xscom_ops = {
>       .endianness = DEVICE_BIG_ENDIAN,
>   };
>   
> +PnvPhb4PecState *pnv_phb4_get_pec(PnvChip *chip, PnvPHB *phb, Error **errp)
> +{
> +    PnvPhb4PecState *pecs = NULL;
> +    int chip_id = phb->chip_id;
> +    int index = phb->phb_id;
> +    int i, j;
> +
> +    if (phb->version == 4) {
> +        Pnv9Chip *chip9 = PNV9_CHIP(chip);
> +
> +        pecs = chip9->pecs;
> +    } else if (phb->version == 5) {
> +        Pnv10Chip *chip10 = PNV10_CHIP(chip);
> +
> +        pecs = chip10->pecs;
> +    } else {
> +        g_assert_not_reached();
> +    }
> +
> +    for (i = 0; i < chip->num_pecs; i++) {
> +        /*
> +         * For each PEC, check the amount of phbs it supports
> +         * and see if the given phb4 index matches an index.
> +         */
> +        PnvPhb4PecState *pec = &pecs[i];
> +
> +        for (j = 0; j < pec->num_phbs; j++) {
> +            if (index == pnv_phb4_pec_get_phb_id(pec, j)) {
> +                pec->phbs[j] = phb;
> +                return pec;
> +            }
> +        }
> +    }
> +    error_setg(errp,
> +               "pnv-phb4 chip-id %d index %d didn't match any existing PEC",
> +               chip_id, index);
> +
> +    return NULL;
> +}
> +
>   static PnvPHB *pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
>                                              int stack_no,
>                                              Error **errp)
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 24bf8461d6..46010b30ad 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -284,48 +284,6 @@ static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir,
>       g_free(reg);
>   }
>   
> -static PnvPhb4PecState *pnv_phb4_get_pec(PnvChip *chip, PnvPHB4 *phb,
> -                                         Error **errp)
> -{
> -    PnvPHB *phb_base = phb->phb_base;
> -    PnvPhb4PecState *pecs = NULL;
> -    int chip_id = phb->chip_id;
> -    int index = phb->phb_id;
> -    int i, j;
> -
> -    if (phb_base->version == 4) {
> -        Pnv9Chip *chip9 = PNV9_CHIP(chip);
> -
> -        pecs = chip9->pecs;
> -    } else if (phb_base->version == 5) {
> -        Pnv10Chip *chip10 = PNV10_CHIP(chip);
> -
> -        pecs = chip10->pecs;
> -    } else {
> -        g_assert_not_reached();
> -    }
> -
> -    for (i = 0; i < chip->num_pecs; i++) {
> -        /*
> -         * For each PEC, check the amount of phbs it supports
> -         * and see if the given phb4 index matches an index.
> -         */
> -        PnvPhb4PecState *pec = &pecs[i];
> -
> -        for (j = 0; j < pec->num_phbs; j++) {
> -            if (index == pnv_phb4_pec_get_phb_id(pec, j)) {
> -                pec->phbs[j] = phb->phb_base;
> -                return pec;
> -            }
> -        }
> -    }
> -    error_setg(errp,
> -               "pnv-phb4 chip-id %d index %d didn't match any existing PEC",
> -               chip_id, index);
> -
> -    return NULL;
> -}
> -
>   /*
>    * Adds a PnvPHB to the chip. Returns the parent obj of the
>    * PHB which varies with each version (phb version 3 is parented
> @@ -349,7 +307,7 @@ Object *pnv_chip_add_phb(PnvChip *chip, PnvPHB *phb, Error **errp)
>           return OBJECT(chip);
>       }
>   
> -    phb->pec = pnv_phb4_get_pec(chip, PNV_PHB4(phb->backend), errp);
> +    phb->pec = pnv_phb4_get_pec(chip, phb, errp);
>   
>       return OBJECT(phb->pec);
>   }
> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> index 0b72ef1471..5c5edb2941 100644
> --- a/include/hw/pci-host/pnv_phb4.h
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -157,6 +157,7 @@ struct PnvPHB4 {
>   
>   void pnv_phb4_pic_print_info(PnvPHB4 *phb, Monitor *mon);
>   int pnv_phb4_pec_get_phb_id(PnvPhb4PecState *pec, int stack_index);
> +PnvPhb4PecState *pnv_phb4_get_pec(PnvChip *chip, PnvPHB *phb, Error **errp);
>   void pnv_phb4_bus_init(DeviceState *dev, PnvPHB4 *phb);
>   extern const MemoryRegionOps pnv_phb4_xscom_ops;
>