Re: [RFC PATCH 05/10] ppc/pnv: Extend chip_pir class method to TIR as well

["Nicholas Piggin" <npiggin@gmail.com>]

On Tue May 28, 2024 at 6:32 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 5/26/24 17:56, Nicholas Piggin wrote:
> > The chip_pir chip class method allows the platform to set the PIR
> > processor identification register. Extend this to a more general
> > ID function which also allows the TIR to be set. This is in
> > preparation for "big core", which is a more complicated topology
> > of cores and threads.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   include/hw/ppc/pnv_chip.h |  3 +-
> >   hw/ppc/pnv.c              | 61 ++++++++++++++++++++++++---------------
> >   hw/ppc/pnv_core.c         | 10 ++++---
> >   3 files changed, 45 insertions(+), 29 deletions(-)
> > 
> > diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> > index 8589f3291e..679723926a 100644
> > --- a/include/hw/ppc/pnv_chip.h
> > +++ b/include/hw/ppc/pnv_chip.h
> > @@ -147,7 +147,8 @@ struct PnvChipClass {
> >   
> >       DeviceRealize parent_realize;
> >   
> > -    uint32_t (*chip_pir)(PnvChip *chip, uint32_t core_id, uint32_t thread_id);
> > +    void (*processor_id)(PnvChip *chip, uint32_t core_id, uint32_t thread_id,
> > +                         uint32_t *pir, uint32_t *tir);
>
> Should it be named get_chip_core_thread_regs() ?

Yeah, the name isn't great. It is getting the regs, but the regs are the
"pervasive id" used as well... but maybe that's not too relevant here.
What about we drop chip_ since we have the chip and no other methods use
such prefix, then call it get_thread_pir_tir()?

> > @@ -155,7 +155,7 @@ static int pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> >       char *nodename;
> >       int cpus_offset = get_cpus_node(fdt);
> >   
> > -    pir = pnv_cc->chip_pir(chip, pc->hwid, 0);
> > +    pnv_cc->processor_id(chip, pc->hwid, 0, &pir, &tir);
>
> As a generic helper API and potentially expandable, it should allow 
> passing NULL for registers whose values are not really sought to avoid 
> having to create un-necessary local variables by the caller.

I'll do that.

Thanks,
Nick