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

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

On Wed May 29, 2024 at 4:30 PM AEST, Cédric Le Goater wrote:
> On 5/29/24 02:24, Nicholas Piggin wrote:
> > 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()?
>
> processor relates to chip and so, processor_id() is not great indeed.
> get_pir_tir() would be enough I think.
>
> What would be good though, since pnv is growing, is to start adding
> documentation to these common helpers.

Okay we'll use that name.

You mean just a comment them in the header? Might as well do that for
new ones at least.

Thanks,
Nick