On Mon, Jan 13, 2020 at 09:15:07AM +0530, Pratik Rajesh Sampat wrote: > Define a bitmask interface to determine support for the Self Restore, > Self Save or both. > > Also define an interface to determine the preference of that SPR to > be strictly saved or restored or encapsulated with an order of preference. > > The preference bitmask is shown as below: > ---------------------------- > |... | 2nd pref | 1st pref | > ---------------------------- > MSB LSB > > The preference from higher to lower is from LSB to MSB with a shift of 8 > bits. > Example: > Prefer self save first, if not available then prefer self > restore > The preference mask for this scenario will be seen as below. > ((SELF_RESTORE_STRICT << PREFERENCE_SHIFT) | SELF_SAVE_STRICT) > --------------------------------- > |... | Self restore | Self save | > --------------------------------- > MSB LSB > > Finally, declare a list of preferred SPRs which encapsulate the bitmaks > for preferred and supported with defaults of both being set to support > legacy firmware. > > This commit also implements using the above interface and retains the > legacy functionality of self restore. > > Signed-off-by: Pratik Rajesh Sampat <psam...@linux.ibm.com> > --- > arch/powerpc/platforms/powernv/idle.c | 327 +++++++++++++++++++++----- > 1 file changed, 271 insertions(+), 56 deletions(-) > > diff --git a/arch/powerpc/platforms/powernv/idle.c > b/arch/powerpc/platforms/powernv/idle.c > index 78599bca66c2..2f328403b0dc 100644 > --- a/arch/powerpc/platforms/powernv/idle.c > +++ b/arch/powerpc/platforms/powernv/idle.c > @@ -32,9 +32,106 @@ > #define P9_STOP_SPR_MSR 2000 > #define P9_STOP_SPR_PSSCR 855 > > +/* Interface for the stop state supported and preference */ > +#define SELF_RESTORE_TYPE 0 > +#define SELF_SAVE_TYPE 1 > + > +#define NR_PREFERENCES 2 > +#define PREFERENCE_SHIFT 4 > +#define PREFERENCE_MASK 0xf > + > +#define UNSUPPORTED 0x0 > +#define SELF_RESTORE_STRICT 0x1 > +#define SELF_SAVE_STRICT 0x2 > + > +/* > + * Bitmask defining the kind of preferences available. > + * Note : The higher to lower preference is from LSB to MSB, with a shift of > + * 4 bits. > + * ---------------------------- > + * | | 2nd pref | 1st pref | > + * ---------------------------- > + * MSB LSB > + */ > +/* Prefer Restore if available, otherwise unsupported */ > +#define PREFER_SELF_RESTORE_ONLY SELF_RESTORE_STRICT > +/* Prefer Save if available, otherwise unsupported */ > +#define PREFER_SELF_SAVE_ONLY SELF_SAVE_STRICT > +/* Prefer Restore when available, otherwise prefer Save */ > +#define PREFER_RESTORE_SAVE ((SELF_SAVE_STRICT << \ > + PREFERENCE_SHIFT)\ > + | SELF_RESTORE_STRICT) > +/* Prefer Save when available, otherwise prefer Restore*/ > +#define PREFER_SAVE_RESTORE ((SELF_RESTORE_STRICT <<\ > + PREFERENCE_SHIFT)\ > + | SELF_SAVE_STRICT) > static u32 supported_cpuidle_states; > struct pnv_idle_states_t *pnv_idle_states; > int nr_pnv_idle_states; > +/* Caching the lpcr & ptcr support to use later */ > +static bool is_lpcr_self_save; > +static bool is_ptcr_self_save; > + > +struct preferred_sprs { > + u64 spr; > + u32 preferred_mode; > + u32 supported_mode; > +}; > + > +struct preferred_sprs preferred_sprs[] = { > + { > + .spr = SPRN_HSPRG0, > + .preferred_mode = PREFER_RESTORE_SAVE, > + .supported_mode = SELF_RESTORE_STRICT, > + }, > + { > + .spr = SPRN_LPCR, > + .preferred_mode = PREFER_RESTORE_SAVE, > + .supported_mode = SELF_RESTORE_STRICT, > + }, > + { > + .spr = SPRN_PTCR, > + .preferred_mode = PREFER_SAVE_RESTORE, > + .supported_mode = SELF_RESTORE_STRICT, > + },
This confuses me. It says SAVE takes precedence over RESTORE. and than it says it is strictly 'RESTORE' only. Maybe you should not initialize the 'supported_mode' ? or put a comment somewhere here, saying this value will be overwritten during system initialization? Otherwise the code looks correct. Reviewed-by: Ram Pai <linux...@us.ibm.com> RP