Re: [v2 PATCH 1/2] powernv/powerpc:Save/Restore additional SPRs for stop4 cpuidle

[Gautham R Shenoy <ego@linux.vnet.ibm.com>]

Hi Nicholas, Michael,

On Wed, Jul 19, 2017 at 10:07:05PM +1000, Michael Ellerman wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
> >> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> >> index a7b5af3..0262283 100644
> >> --- a/arch/powerpc/kernel/asm-offsets.c
> >> +++ b/arch/powerpc/kernel/asm-offsets.c
> >> @@ -743,6 +743,18 @@ int main(void)
> >>  	OFFSET(PACA_SUBCORE_SIBLING_MASK, paca_struct, subcore_sibling_mask);
> >>  	OFFSET(PACA_SIBLING_PACA_PTRS, paca_struct, thread_sibling_pacas);
> >>  	OFFSET(PACA_REQ_PSSCR, paca_struct, requested_psscr);
> >> +
> >> +	OFFSET(PACA_PID, paca_struct, stop_spr_save_area[0]);
> >> +	OFFSET(PACA_LDBAR, paca_struct, stop_spr_save_area[1]);
> >> +	OFFSET(PACA_FSCR, paca_struct, stop_spr_save_area[2]);
> >> +	OFFSET(PACA_HFSCR, paca_struct, stop_spr_save_area[3]);
> >> +
> >> +	/* On POWER9, we are already saving MMCR0 for ESL=EC=1 */
> >> +	OFFSET(PACA_MMCRA, paca_struct, stop_spr_save_area[4]);
> >> +	OFFSET(PACA_MMCR1, paca_struct, stop_spr_save_area[5]);
> >> +	OFFSET(PACA_MMCR2, paca_struct, stop_spr_save_area[6]);
> >
> > Don't these offset names go against convention?
> >
> > Look at e.g., how PACA_EXGEN is used. I would prefer using that
> > convention. You could make the name slightly shorter too, e.g.,
> > just stop_sprs or so.
> 
> Yes please.
> 
> If I see PACA_MMCRA I'm expecting that's paca->mmcra.

Ah, ok. I will fix this. 

> 
> Also if the same values always go in the same place then please use a
> proper struct, rather than an array. ie.
>

Ok, I will add the struct instead of a array.

> struct stop_sprs
> {
> 	u64 pid;
>         u64 ldbar;
>         ...
> }
> 
> cheers
> 
--
Thanks and Regards
gautham.