Re: [PATCH 09/17] pnv_phb4_pec.c: move pnv_pec_phb_offset() to pnv_phb4.c

["Cédric Le Goater" <clg@kaod.org>]

On 12/28/21 20:37, Daniel Henrique Barboza wrote:
> The logic inside pnv_pec_phb_offset() wiil be useful in the next patch

will

> to determine the stack that should contain a PHB4 device.
> 
> Move the function to pnv_phb4.c and make it public since there's no
> pnv_phb4_pec.h header. While we're at it, add 'stack_index' as a
> parameter and make the function return 'phb-id' directly. And rename it
> to pnv_pec_get_phb_id() to be even clearer about the function intent.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Looks good,

> ---
>   hw/pci-host/pnv_phb4.c         | 17 +++++++++++++++++
>   hw/pci-host/pnv_phb4_pec.c     | 15 +--------------
>   include/hw/pci-host/pnv_phb4.h |  2 ++
>   3 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 0ea505cc94..36c56007ba 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1164,6 +1164,23 @@ static AddressSpace *pnv_phb4_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>       return &ds->dma_as;
>   }
>   
> +/*
> + * Return the index/phb-id of a PHB4 that belongs to a
> + * pec->stacks[stack_index] stack.
> + */
> +int pnv_pec_get_phb_id(PnvPhb4PecState *pec, int stack_index)

pnv_phb4_pec_get_phb_id() would be cleaner.

Thanks,

C.



> +{
> +    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
> +    int index = pec->index;
> +    int offset = 0;
> +
> +    while (index--) {
> +        offset += pecc->num_stacks[index];
> +    }
> +
> +    return offset + stack_index;
> +}
> +
>   /*
>    * Set the object properties of a phb in relation with its stack.
>    *
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index 700ee4b185..bc2f8bb8b1 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -374,19 +374,6 @@ static void pnv_pec_instance_init(Object *obj)
>       }
>   }
>   
> -static int pnv_pec_phb_offset(PnvPhb4PecState *pec)
> -{
> -    PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
> -    int index = pec->index;
> -    int offset = 0;
> -
> -    while (index--) {
> -        offset += pecc->num_stacks[index];
> -    }
> -
> -    return offset;
> -}
> -
>   static void pnv_pec_realize(DeviceState *dev, Error **errp)
>   {
>       PnvPhb4PecState *pec = PNV_PHB4_PEC(dev);
> @@ -422,7 +409,7 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp)
>       for (i = 0; i < pec->num_stacks; i++) {
>           PnvPhb4PecStack *stack = &pec->stacks[i];
>           Object *stk_obj = OBJECT(stack);
> -        int phb_id = pnv_pec_phb_offset(pec) + i;
> +        int phb_id =  pnv_pec_get_phb_id(pec, i);
>   
>           object_property_set_int(stk_obj, "stack-no", i, &error_abort);
>           object_property_set_int(stk_obj, "phb-id", phb_id, &error_abort);
> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> index d7838513f1..0fa88ca3fa 100644
> --- a/include/hw/pci-host/pnv_phb4.h
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -15,6 +15,7 @@
>   #include "hw/ppc/xive.h"
>   #include "qom/object.h"
>   
> +typedef struct PnvPhb4PecState PnvPhb4PecState;
>   typedef struct PnvPhb4PecStack PnvPhb4PecStack;
>   typedef struct PnvPHB4 PnvPHB4;
>   typedef struct PnvChip PnvChip;
> @@ -134,6 +135,7 @@ struct PnvPHB4 {
>   void pnv_phb4_pic_print_info(PnvPHB4 *phb, Monitor *mon);
>   void pnv_phb4_update_regions(PnvPhb4PecStack *stack);
>   void pnv_phb4_set_stack_phb_props(PnvPhb4PecStack *stack, PnvPHB4 *phb);
> +int pnv_pec_get_phb_id(PnvPhb4PecState *pec, int stack_index);
>   extern const MemoryRegionOps pnv_phb4_xscom_ops;
>   
>   /*
>