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

[Daniel Henrique Barboza]

On 1/3/22 06:00, Cédric Le Goater wrote:
> 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.

Changed in the upcoming v2.


Daniel

> 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;
> >   /*